fix(spp_user_roles): prevent validation error when assigning groups to a new role#177
fix(spp_user_roles): prevent validation error when assigning groups to a new role#177anthonymarkQA wants to merge 1 commit into19.0from
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a workaround for an Odoo cache-clearing bug by overriding the create method in the ResUsersRoleCustomSPP model and updates the implied_ids field display in the XML view. Feedback includes a recommendation to narrow the scope of the create override to specifically target implied_ids to prevent interference with standard mechanisms, and a correction for a likely invalid field name (full_name) in the view definition.
| domain="[('role_id', '=', False)]" | ||
| > | ||
| <list create="0"> | ||
| <field name="full_name" string="Group" /> |
There was a problem hiding this comment.
The field full_name does not exist in the standard Odoo res.groups model. Unless it is added by a custom module not shown in this context, this will cause a traceback when loading the view. You should likely use name or display_name instead.
| <field name="full_name" string="Group" /> | |
| <field name="name" string="Group" /> |
| groups_vals_list = [] | ||
| group_fields = set(self.env["res.groups"]._fields) - {"name"} | ||
| for vals in vals_list: | ||
| group_vals = {} | ||
| for field in group_fields: | ||
| if field in vals: | ||
| group_vals[field] = vals.pop(field) | ||
| groups_vals_list.append(group_vals) | ||
|
|
||
| new_records = super().create(vals_list) | ||
|
|
||
| for record, group_vals in zip(new_records, groups_vals_list): | ||
| if group_vals: | ||
| record.group_id.write(group_vals) |
There was a problem hiding this comment.
The current implementation of the create override is broader than necessary. By popping all fields belonging to res.groups (except name), you risk interfering with the standard Odoo _inherits mechanism for fields that don't suffer from the cache-clearing bug. It also makes the creation fragile if other modules add required fields to res.groups. It is recommended to target only the problematic fields, specifically implied_ids, as mentioned in the PR description and mirroring the write workaround in base_user_role.
| groups_vals_list = [] | |
| group_fields = set(self.env["res.groups"]._fields) - {"name"} | |
| for vals in vals_list: | |
| group_vals = {} | |
| for field in group_fields: | |
| if field in vals: | |
| group_vals[field] = vals.pop(field) | |
| groups_vals_list.append(group_vals) | |
| new_records = super().create(vals_list) | |
| for record, group_vals in zip(new_records, groups_vals_list): | |
| if group_vals: | |
| record.group_id.write(group_vals) | |
| implied_ids_list = [vals.pop("implied_ids", None) for vals in vals_list] | |
| new_records = super().create(vals_list) | |
| for record, implied_ids in zip(new_records, implied_ids_list): | |
| if implied_ids: | |
| record.group_id.write({"implied_ids": implied_ids}) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #177 +/- ##
=======================================
Coverage 71.48% 71.48%
=======================================
Files 932 932
Lines 54840 54840
=======================================
Hits 39201 39201
Misses 15639 15639
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
1 participant
Clicking "Add a line" in the Groups tab triggered inline creation of a
new res.groups record. Since res.groups.name is required and left blank,
Odoo threw a validation error and blocked the save entirely.
Two changes:
so "Add a line" opens a search dialog to select existing groups instead
of creating an inline row
directly to group_id after creation, mirroring the existing write()
workaround in base_user_role for the same Odoo _inherits cache-clearing bug
Why is this change needed?
Users unable to create a role and assign a group in one save. previously users must create an empty role first then save, then add a group into it. which is inneficient
How was the change implemented?
How was the change implemented?
1.
spp_user_roles/views/role.xmlReplaced the
implied_idsfield override with one that includes an explicitinner
<list create="0">. Settingcreate="0"on the inner list tells Odooto replace the "Add a line" inline row with an "Add" button that opens a
search/select dialog. This ensures only existing
res.groupsrecords can belinked to a role — no blank inline records are ever created.
2.
spp_user_roles/models/role.pyAdded a
create()override that mirrors the existingwrite()workaroundalready present in
base_user_role. Theres.users.rolemodel uses Odoo's_inheritsdelegation tores.groups, which means fields likeimplied_idsbelong to the parent
res.groupsrecord. On create, passingimplied_idsthrough the
_inheritsmechanism triggers an Odoo cache-clearing bug(documented in the upstream
write()override) that causes the value to bedropped. The fix extracts
implied_idsfrom vals before callingsuper().create(), lets the role and its associatedgroup_idbe createdfirst, then writes
implied_idsdirectly togroup_id— bypassing the bug.New unit tests
Unit tests executed by the author
How to test manually
Related links
https://projects.acn.fr/projects/acn-eng/work_packages/979/activity