Skip to content
Permalink
Browse files

[FIX] base, account: remove group from all users when removing settings

Adding the implied users in a group is done automatically when adding
the implied_ids. Moreover it is done in SQL, bypassing record rules.

Asymmetrically, the users are not removed when removing a group; it is
done manually. However, since this is done via the ORM,
it takes record rules into account so some users might not be visible.

The standard case where this happens is multi-company.

Take the case of some mutually exclusive groups, TaxB2B and TaxB2C,
and a user U in company C1 and C2, in TaxB2B.
In company C2, change the settings to TaxB2C.
U is in another company, so U is not removed from TaxB2B.
But then U is added to taxB2C, which causes a validation error.

<rant>
Without the validation error, the inconsistency would have been more
pernicious to debug, so it's still a good thing to have it.

BTW, regarding the feature, it would seem that each company should be
able to decide if the Taxes should be displayed as B2B or B2C;
however it's not possible to do it because it's handled by groups.
If instead of removing all users we tried add/remov'ing users based on
the company of the settings, it wouldn't work because users
(internal and portal) can be shared between companies.
So either this kind of "display groups" should be company-dependant,
or the user groups should change depending on the company.
But then what if the user is in 2 companies at the same time through the
contextified company of version 13?
</rant>

opw 2118337

X-original-commit: 2c1b92f
  • Loading branch information
len-odoo committed Nov 27, 2019
1 parent 51e289d commit bafacacbfe2c914462c0c74de75f208d0194206d
Showing with 23 additions and 1 deletion.
  1. +22 −0 addons/account/tests/test_settings.py
  2. +1 −1 odoo/addons/base/models/res_config.py
@@ -14,7 +14,9 @@ def test_switch_taxB2B_taxB2C(self):
# at each setting change, all users should be removed from one group and added to the other
# so picking an arbitrary witness should be equivalent to checking that everything worked.
config = self.env['res.config.settings'].create({})
self.switch_tax_settings(config)

def switch_tax_settings(self, config):
config.show_line_subtotals_tax_selection = "tax_excluded"
config._onchange_sale_tax()
config.flush()
@@ -35,3 +37,23 @@ def test_switch_taxB2B_taxB2C(self):
config.execute()
self.assertEqual(self.env.user.has_group('account.group_show_line_subtotals_tax_excluded'), True)
self.assertEqual(self.env.user.has_group('account.group_show_line_subtotals_tax_included'), False)

def test_switch_taxB2B_taxB2C_multicompany(self):
"""
Contrarily to the (apparently reasonable) assumption that adding users
to group and removing them was symmetrical, it may not be the case
if one is done in SQL and the other via the ORM.
Because the latter automatically takes into account record rules that
might make some users invisible.
This one is identical to the previous, except that we do the actions
with a non-superuser user, and in a new company with one user in common
with another company which has a different taxB2X setting.
"""
user = self.env.ref('base.user_admin')
company = self.env['res.company'].create({'name': 'oobO'})
user.write({'company_ids': [(4, company.id)], 'company_id': company.id})
Settings = self.env['res.config.settings'].with_user(user.id)
config = Settings.create({})

self.switch_tax_settings(config)
@@ -576,7 +576,7 @@ def set_values(self):
groups.write({'implied_ids': [(4, implied_group.id)]})
else:
groups.write({'implied_ids': [(3, implied_group.id)]})
implied_group.write({'users': [(3, user.id) for user in groups.users]})
implied_group.sudo().write({'users': [(5,)]})

# config fields: store ir.config_parameters
IrConfigParameter = self.env['ir.config_parameter'].sudo()

0 comments on commit bafacac

Please sign in to comment.
You can’t perform that action at this time.