[FIX] account: fiscal position computation precedence
Zip range and state filtering were added at
377a6ecab0, but the
precedence between country, group, vat_required,
and not required were not correct.
This commit fixes the precedence as follows and adds
the corresponding tests:
0. Explict position on partner
1. Direct match on all criterions
2. Match on country+zip before country+state
3. Match on country before country group
4. Match on `vat_required` criterion on all
the above before trying them all again without
This commit is contained in:
Olivier Dony
@@ -122,31 +122,36 @@ class AccountFiscalPosition(models.Model):
|
||||
def _get_fpos_by_region(self, country_id=False, state_id=False, zipcode=False, vat_required=False):
|
||||
if not country_id:
|
||||
return False
|
||||
domains = [[('auto_apply', '=', True), ('vat_required', '=', vat_required)]]
|
||||
if vat_required:
|
||||
# Possibly allow fallback to non-VAT positions, if no VAT-required position matches
|
||||
domains += [[('auto_apply', '=', True), ('vat_required', '=', False)]]
|
||||
base_domain = [('auto_apply', '=', True), ('vat_required', '=', vat_required)]
|
||||
null_state_dom = state_domain = [('state_ids', '=', False)]
|
||||
null_zip_dom = zip_domain = [('zip_from', '=', 0), ('zip_to', '=', 0)]
|
||||
|
||||
if zipcode and zipcode.isdigit():
|
||||
zipcode = int(zipcode)
|
||||
domain_zip = [('zip_from', '<=', zipcode), ('zip_to', '>=', zipcode)]
|
||||
zip_domain = [('zip_from', '<=', zipcode), ('zip_to', '>=', zipcode)]
|
||||
else:
|
||||
zipcode, domain_zip = 0, [('zip_from', '=', 0), ('zip_to', '=', 0)]
|
||||
state_domain = [('state_ids', '=', False)]
|
||||
zipcode = 0
|
||||
|
||||
if state_id:
|
||||
state_domain = [('state_ids', '=', state_id)]
|
||||
for domain in domains:
|
||||
# Build domain to search records with exact matching criteria
|
||||
fpos_id = self.search(domain + [('country_id', '=', country_id)] + state_domain + domain_zip, limit=1).id
|
||||
# return records that fit the most the criteria, and fallback on less specific fiscal positions if any can be found
|
||||
if not fpos_id and zipcode:
|
||||
fpos_id = self.search(domain + [('country_id', '=', country_id)] + state_domain + [('zip_from', '=', 0), ('zip_to', '=', 0)], limit=1).id
|
||||
if not fpos_id and state_id:
|
||||
fpos_id = self.search(domain + [('country_id', '=', country_id)] + [('state_ids', '=', False)] + domain_zip, limit=1).id
|
||||
if not fpos_id and state_id and zipcode:
|
||||
fpos_id = self.search(domain + [('country_id', '=', country_id)] + [('state_ids', '=', False)] + [('zip_from', '=', 0), ('zip_to', '=', 0)], limit=1).id
|
||||
if fpos_id:
|
||||
return fpos_id
|
||||
return False
|
||||
|
||||
domain_country = base_domain + [('country_id', '=', country_id)]
|
||||
domain_group = base_domain + [('country_group_id.country_ids', '=', country_id)]
|
||||
|
||||
# Build domain to search records with exact matching criteria
|
||||
fpos = self.search(domain_country + state_domain + zip_domain, limit=1)
|
||||
# return records that fit the most the criteria, and fallback on less specific fiscal positions if any can be found
|
||||
if not fpos and state_id:
|
||||
fpos = self.search(domain_country + null_state_dom + zip_domain, limit=1)
|
||||
if not fpos and zipcode:
|
||||
fpos = self.search(domain_country + state_domain + null_zip_dom, limit=1)
|
||||
if not fpos and state_id and zipcode:
|
||||
fpos = self.search(domain_country + null_state_dom + null_zip_dom, limit=1)
|
||||
|
||||
# fallback: country group with no state/zip range
|
||||
if not fpos:
|
||||
fpos = self.search(domain_group + null_state_dom + null_zip_dom, limit=1)
|
||||
return fpos or False
|
||||
|
||||
@api.model
|
||||
def get_fiscal_position(self, partner_id, delivery_id=None):
|
||||
@@ -156,7 +161,7 @@ class AccountFiscalPosition(models.Model):
|
||||
PartnerObj = self.env['res.partner']
|
||||
partner = PartnerObj.browse(partner_id)
|
||||
|
||||
# if no delivery use invocing
|
||||
# if no delivery use invoicing
|
||||
if delivery_id:
|
||||
delivery = PartnerObj.browse(delivery_id)
|
||||
else:
|
||||
@@ -166,25 +171,23 @@ class AccountFiscalPosition(models.Model):
|
||||
if delivery.property_account_position_id or partner.property_account_position_id:
|
||||
return delivery.property_account_position_id.id or partner.property_account_position_id.id
|
||||
|
||||
fiscal_position_id = self._get_fpos_by_region(delivery.country_id.id, delivery.state_id.id, delivery.zip, bool(partner.vat))
|
||||
if fiscal_position_id:
|
||||
return fiscal_position_id
|
||||
def fallback_search(vat_required):
|
||||
fpos = self._get_fpos_by_region(delivery.country_id.id, delivery.state_id.id, delivery.zip, vat_required)
|
||||
if not fpos:
|
||||
# Fallback on catchall (no country, no group)
|
||||
fpos = self.search([('auto_apply', '=', True), ('vat_required', '=', vat_required),
|
||||
('country_id', '=', None), ('country_group_id', '=', None)], limit=1)
|
||||
return fpos
|
||||
|
||||
domains = [[('auto_apply', '=', True), ('vat_required', '=', bool(partner.vat))]]
|
||||
if partner.vat:
|
||||
# Possibly allow fallback to non-VAT positions, if no VAT-required position matches
|
||||
domains += [[('auto_apply', '=', True), ('vat_required', '=', False)]]
|
||||
# First search only matching VAT positions
|
||||
vat_required = bool(partner.vat)
|
||||
fp = fallback_search(vat_required)
|
||||
|
||||
for domain in domains:
|
||||
if delivery.country_id.id:
|
||||
fiscal_position = self.search(domain + [('country_group_id.country_ids', '=', delivery.country_id.id)], limit=1)
|
||||
if fiscal_position:
|
||||
return fiscal_position.id
|
||||
# Then if VAT required found no match, try positions that do not require it
|
||||
if not fp and vat_required:
|
||||
fp = fallback_search(False)
|
||||
|
||||
fiscal_position = self.search(domain + [('country_id', '=', None), ('country_group_id', '=', None)], limit=1)
|
||||
if fiscal_position:
|
||||
return fiscal_position.id
|
||||
return False
|
||||
return fp.id if fp else False
|
||||
|
||||
|
||||
class AccountFiscalPositionTax(models.Model):
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
from openerp.addons.account.tests.account_test_classes import AccountingTestCase
|
||||
from openerp.tests import common
|
||||
|
||||
class TestFiscalPosition(AccountingTestCase):
|
||||
class TestFiscalPosition(common.TransactionCase):
|
||||
"""Tests for fiscal positions in auto apply (account.fiscal.position).
|
||||
If a partner has a vat number, the fiscal positions with "vat_required=True"
|
||||
are preferred.
|
||||
@@ -8,33 +8,124 @@ class TestFiscalPosition(AccountingTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestFiscalPosition, self).setUp()
|
||||
self.fiscal_position_model = self.registry('account.fiscal.position')
|
||||
self.res_partner_model = self.registry('res.partner')
|
||||
self.res_country_model = self.registry('res.country')
|
||||
self.fp = self.env['account.fiscal.position']
|
||||
|
||||
def test_fiscal_position(self):
|
||||
cr, uid = self.cr, self.uid
|
||||
country_id = self.res_country_model.search(cr, uid, [('name', '=', 'France')])[0]
|
||||
partner_id = self.res_partner_model.create(cr, uid, dict(
|
||||
name="George",
|
||||
vat="BE0477472701",
|
||||
notify_email="always",
|
||||
country_id=country_id))
|
||||
fp_b2c_id = self.fiscal_position_model.create(cr, uid, dict(name="EU-VAT-FR-B2C",
|
||||
auto_apply=True,
|
||||
country_id=country_id,
|
||||
vat_required=False,
|
||||
sequence=1))
|
||||
fp_b2b_id = self.fiscal_position_model.create(cr, uid, dict(name="EU-VAT-FR-B2B",
|
||||
auto_apply=True,
|
||||
country_id=country_id,
|
||||
vat_required=True,
|
||||
sequence=2))
|
||||
res = self.fiscal_position_model.get_fiscal_position(cr, uid, partner_id)
|
||||
self.assertEquals(fp_b2b_id, res,
|
||||
"Fiscal position detection should pick B2B position as 1rst match")
|
||||
# reset any existing FP
|
||||
self.fp.search([]).write({'auto_apply': False})
|
||||
|
||||
self.fiscal_position_model.write(cr, uid, [fp_b2b_id], {'auto_apply': False})
|
||||
res = self.fiscal_position_model.get_fiscal_position(cr, uid, partner_id)
|
||||
self.assertEquals(fp_b2c_id, res,
|
||||
"Fiscal position detection should pick B2C position as 1rst match")
|
||||
self.res_partner = self.env['res.partner']
|
||||
self.be = be = self.env.ref('base.be')
|
||||
self.fr = fr = self.env.ref('base.fr')
|
||||
self.mx = mx = self.env.ref('base.mx')
|
||||
self.eu = eu = self.env.ref('base.europe')
|
||||
self.state_fr = self.env['res.country.state'].create(dict(
|
||||
name="State",
|
||||
code="ST",
|
||||
country_id=fr.id))
|
||||
self.jc = self.res_partner.create(dict(
|
||||
name="JCVD",
|
||||
vat="BE0477472701",
|
||||
notify_email="none",
|
||||
country_id=be.id))
|
||||
self.ben = self.res_partner.create(dict(
|
||||
name="BP",
|
||||
notify_email="none",
|
||||
country_id=be.id))
|
||||
self.george = self.res_partner.create(dict(
|
||||
name="George",
|
||||
vat="BE0477472701",
|
||||
notify_email="none",
|
||||
country_id=fr.id))
|
||||
self.alberto = self.res_partner.create(dict(
|
||||
name="Alberto",
|
||||
vat="BE0477472701",
|
||||
notify_email="none",
|
||||
country_id=mx.id))
|
||||
self.be_nat = self.fp.create(dict(
|
||||
name="BE-NAT",
|
||||
auto_apply=True,
|
||||
country_id=be.id,
|
||||
vat_required=False,
|
||||
sequence=10))
|
||||
self.fr_b2c = self.fp.create(dict(
|
||||
name="EU-VAT-FR-B2C",
|
||||
auto_apply=True,
|
||||
country_id=fr.id,
|
||||
vat_required=False,
|
||||
sequence=40))
|
||||
self.fr_b2b = self.fp.create(dict(
|
||||
name="EU-VAT-FR-B2B",
|
||||
auto_apply=True,
|
||||
country_id=fr.id,
|
||||
vat_required=True,
|
||||
sequence=50))
|
||||
|
||||
def test_10_fp_country(self):
|
||||
def assert_fp(partner, expected_pos, message):
|
||||
self.assertEquals(
|
||||
self.fp.get_fiscal_position(partner.id),
|
||||
expected_pos.id,
|
||||
message)
|
||||
|
||||
george, jc, ben, alberto = self.george, self.jc, self.ben, self.alberto
|
||||
|
||||
# B2B has precedence over B2C for same country even when sequence gives lower precedence
|
||||
self.assertGreater(self.fr_b2b.sequence, self.fr_b2c.sequence)
|
||||
assert_fp(george, self.fr_b2b, "FR-B2B should have precedence over FR-B2C")
|
||||
self.fr_b2b.auto_apply = False
|
||||
assert_fp(george, self.fr_b2c, "FR-B2C should match now")
|
||||
self.fr_b2b.auto_apply = True
|
||||
|
||||
# Create positions matching on Country Group and on NO country at all
|
||||
self.eu_intra_b2b = self.fp.create(dict(
|
||||
name="EU-INTRA B2B",
|
||||
auto_apply=True,
|
||||
country_group_id=self.eu.id,
|
||||
vat_required=True,
|
||||
sequence=20))
|
||||
self.world = self.fp.create(dict(
|
||||
name="WORLD-EXTRA",
|
||||
auto_apply=True,
|
||||
vat_required=False,
|
||||
sequence=30))
|
||||
|
||||
# Country match has higher precedence than group match or sequence
|
||||
self.assertGreater(self.fr_b2b.sequence, self.eu_intra_b2b.sequence)
|
||||
assert_fp(george, self.fr_b2b, "FR-B2B should have precedence over EU-INTRA B2B")
|
||||
|
||||
# B2B has precedence regardless of country or group match
|
||||
self.assertGreater(self.eu_intra_b2b.sequence, self.be_nat.sequence)
|
||||
assert_fp(jc, self.eu_intra_b2b, "EU-INTRA B2B should match before BE-NAT")
|
||||
|
||||
# Lower sequence = higher precedence if country/group and VAT matches
|
||||
self.assertFalse(ben.vat) # No VAT set
|
||||
assert_fp(ben, self.be_nat, "BE-NAT should match before EU-INTRA due to lower sequence")
|
||||
|
||||
# Remove BE from EU group, now BE-NAT should be the fallback match before the wildcard WORLD
|
||||
self.be.write({'country_group_ids': [(3, self.eu.id)]})
|
||||
self.assertTrue(jc.vat) # VAT set
|
||||
assert_fp(jc, self.be_nat, "BE-NAT should match as fallback even w/o VAT match")
|
||||
|
||||
# No country = wildcard match only if nothing else matches
|
||||
self.assertTrue(alberto.vat) # with VAT
|
||||
assert_fp(alberto, self.world, "WORLD-EXTRA should match anything else (1)")
|
||||
alberto.vat = False # or without
|
||||
assert_fp(alberto, self.world, "WORLD-EXTRA should match anything else (2)")
|
||||
|
||||
# Zip range
|
||||
self.fr_b2b_zip100 = self.fr_b2b.copy(dict(zip_from=0, zip_to=5000, sequence=60))
|
||||
george.zip = 6000
|
||||
assert_fp(george, self.fr_b2b, "FR-B2B with wrong zip range should not match")
|
||||
george.zip = 3000
|
||||
assert_fp(george, self.fr_b2b_zip100, "FR-B2B with zip range should have precedence")
|
||||
|
||||
# States
|
||||
self.fr_b2b_state = self.fr_b2b.copy(dict(state_ids=[(4, self.state_fr.id)], sequence=70))
|
||||
george.state_id = self.state_fr
|
||||
assert_fp(george, self.fr_b2b_zip100, "FR-B2B with zip should have precedence over states")
|
||||
george.zip = 0
|
||||
assert_fp(george, self.fr_b2b_state, "FR-B2B with states should have precedence")
|
||||
|
||||
# Dedicated position has max precedence
|
||||
george.property_account_position_id = self.be_nat
|
||||
assert_fp(george, self.be_nat, "Forced position has max precedence")
|
||||
|
||||
Reference in New Issue
Block a user