admin/Odoo-Modules
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(plating-perms): address final-reviewer critical + important issues

Browse Source 
Pre-deploy fixes for Phase 1 permissions overhaul branch (catches by
final-reviewer subagent + main session).

CRITICAL FIXES:

C1: groups_id -> group_ids (Odoo 19 field rename). Affected ~30 sites
    across 4 model files, 1 view, 7 test files. Documented project
    gotcha (feedback_odoo19_groups_id_renamed.md) that the implementer
    subagents missed because they don't see user memory.

C2: action_fp_resolve_plating_landing server action now calls
    env['ir.actions.act_window'].sudo()._fp_resolve_landing_for_current_user()
    instead of the old inline priority chain. Phase E's role-based
    dispatch was previously dead code.

C3: New migrations/19.0.21.1.0/post-migrate.py triggers
    _fp_post_init_role_migration(env) on -u. post_init_hook only fires
    on INSTALL in Odoo 19, not UPGRADE -- so Phase H's preview creation
    wouldn't have auto-fired on entech without this script. Module
    version bumped to 19.0.21.1.0 to match the migration directory.

C4: Team kanban template rewritten for Odoo 19 (<t t-name='card'> with
    semantic <aside>/<main>) instead of legacy <t t-name='kanban-box'>.
    Previous template threw 'Missing card template' at render.

IMPORTANT FIXES:

I1: SO state=sent Confirm button (id='action_confirm') now also gated
    to group_fp_sales_manager. Previously only the state=draft button
    was gated; Sales Reps could send-and-confirm via the secondary path.

I2: Designated Officials picker domain uses all_group_ids (transitive)
    instead of group_ids (explicit only). Owner users now correctly
    appear as eligible CGP DO candidates via the implied_ids chain.

I3: test_menu_visibility.py compliance hub xmlid corrected to
    fusion_plating.menu_fp_compliance_hub (was
    fusion_plating_compliance.menu_fp_compliance_hub which doesn't exist
    -- the hub menu is defined in core's fp_menu.xml). Tests were
    silently skipTest-ing.

I4: _inverse_plating_role chatter audit reads old role from DB via SQL
    (bypasses cache) so 'old -> new' displays actual values, and
    short-circuits no-op writes.

I5: _FP_ROLE_MAPPING_RULES reordered: cgp_designated_official fires
    BEFORE admin/uid_1_or_2 so admin+DO users keep the capability_delta
    marker that triggers res.company.x_fc_cgp_designated_official_id
    auto-set during migration.

I6: _cron_purge_expired_migrations skips groups with active users
    instead of unlink-ing unconditionally. Defense against rollback
    safety being bypassed by manual role assignments post-migration.

CLAUDE.md updated with 3 new durable rules (13b kanban card template,
13c group_ids vs all_group_ids, 13d post_init_hook only on install).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsinghpal
2026-05-24 08:37:13 -04:00
parent 5cc1117f75
commit 0047f49d2c
18 changed files with 152 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
fusion_plating/fusion_plating/models/fp_landing.py
View File

@@ -227,7 +227,7 @@ class ResUsers(models.Model):
'x_fc_plating_landing_action_id.',
)
@api.depends('groups_id')
@api.depends('group_ids')
def _compute_accessible_landing_action_ids(self):
Window = self.env['ir.actions.act_window']
pickable = Window.sudo().search([('x_fc_pickable_landing', '=', True)])

37
fusion_plating/fusion_plating/models/fp_migration.py
View File

@@ -145,13 +145,13 @@ class FpMigrationPreview(models.Model):
for line in self.line_ids:
user = line.user_id
# Snapshot current groups_id for rollback
line.applied_groups_snapshot = json.dumps(user.groups_id.ids)
# Snapshot current group_ids for rollback
line.applied_groups_snapshot = json.dumps(user.group_ids.ids)
# Remove old plating-role groups
if old_group_ids:
user.sudo().write({
'groups_id': [(3, gid) for gid in old_group_ids]
'group_ids': [(3, gid) for gid in old_group_ids]
})
# Add the new role group (no-op for 'no')
@@ -159,7 +159,7 @@ class FpMigrationPreview(models.Model):
if target_xmlid:
target = self.env.ref(target_xmlid, raise_if_not_found=False)
if target:
user.sudo().write({'groups_id': [(4, target.id)]})
user.sudo().write({'group_ids': [(4, target.id)]})
# Audit chatter on the user
user.message_post(
@@ -197,7 +197,7 @@ class FpMigrationPreview(models.Model):
for line in self.line_ids:
if line.applied_groups_snapshot:
old_ids = json.loads(line.applied_groups_snapshot)
line.user_id.sudo().write({'groups_id': [(6, 0, old_ids)]})
line.user_id.sudo().write({'group_ids': [(6, 0, old_ids)]})
self.state = 'rolled_back'
@api.model
@@ -222,8 +222,25 @@ class FpMigrationPreview(models.Model):
if g:
old_group_ids.append(g.id)
if old_group_ids:
self.env['res.groups'].browse(old_group_ids).unlink()
_logger.info('Fusion Plating migration: purged %d expired old plating groups', len(old_group_ids))
# I6 safety check — never unlink a group that still has active
# internal users on it. If anyone still references the group
# we'd cascade-strip them silently from their permissions.
safe_to_unlink = []
skipped = []
for old_group in self.env['res.groups'].browse(old_group_ids).exists():
active_users = old_group.users.filtered(lambda u: u.active and not u.share)
if active_users:
skipped.append((old_group.name, active_users.mapped('login')))
else:
safe_to_unlink.append(old_group.id)
if skipped:
_logger.warning(
'Fusion Plating migration purge: skipped %d old groups with active users: %s',
len(skipped), skipped)
if safe_to_unlink:
self.env['res.groups'].browse(safe_to_unlink).unlink()
_logger.info('Fusion Plating migration: purged %d expired old plating groups',
len(safe_to_unlink))
class FpMigrationPreviewLine(models.Model):
@@ -237,12 +254,12 @@ class FpMigrationPreviewLine(models.Model):
capability_delta = fields.Char()
warning = fields.Boolean()
notes = fields.Text(help='Owner may annotate before approving')
applied_groups_snapshot = fields.Text(help='JSON of pre-migration groups_id for rollback')
applied_groups_snapshot = fields.Text(help='JSON of pre-migration group_ids for rollback')
@api.depends('user_id', 'user_id.groups_id')
@api.depends('user_id', 'user_id.group_ids')
def _compute_current_groups(self):
for line in self:
if line.user_id:
line.current_groups = ', '.join(line.user_id.groups_id.mapped('name'))
line.current_groups = ', '.join(line.user_id.group_ids.mapped('name'))
else:
line.current_groups = ''

11
fusion_plating/fusion_plating/models/fp_role_constants.py
View File

@@ -38,15 +38,20 @@ _NEW_ROLE_XMLID = {
# Highest precedence first; first match wins.
# Predicate is a callable taking a res.users record; returns bool.
_FP_ROLE_MAPPING_RULES = [
# cgp_designated_official MUST be first so admin/uid_1/uid_2 users who ALSO
# hold the DO group still get the capability_delta marker — which is what
# triggers action_approve_and_run to set res.company.x_fc_cgp_designated_official_id.
# If admin matched first, the DO field would never get populated for shops
# where the admin is also the registered PSPC Designated Official.
('cgp_designated_official',
lambda u: u.has_group('fusion_plating_cgp.group_fusion_plating_cgp_designated_official'),
'owner', 'Was CGP DO; field set on res.company'),
('uid_1_or_2',
lambda u: u.id in (1, 2),
'owner', None),
('admin',
lambda u: u.has_group('fusion_plating.group_fusion_plating_admin'),
'owner', None),
('cgp_designated_official',
lambda u: u.has_group('fusion_plating_cgp.group_fusion_plating_cgp_designated_official'),
'owner', 'Was CGP DO; field set on res.company'),
('cgp_officer',
lambda u: u.has_group('fusion_plating_cgp.group_fusion_plating_cgp_officer'),
'quality_manager', None),

30
fusion_plating/fusion_plating/models/res_users.py
View File

@@ -53,7 +53,7 @@ class ResUsers(models.Model):
'old plating groups, adds new). Posts an audit chatter message.',
)
@api.depends('groups_id')
@api.depends('group_ids')
def _compute_plating_role(self):
# Resolve xmlids once
role_to_group = {}
@@ -65,7 +65,7 @@ class ResUsers(models.Model):
user.x_fc_plating_role = 'no'
for candidate in _FP_ROLE_PRECEDENCE:
grp = role_to_group.get(candidate)
if grp and grp in user.groups_id:
if grp and grp in user.group_ids:
user.x_fc_plating_role = candidate
break
@@ -79,14 +79,30 @@ class ResUsers(models.Model):
role_to_group[role] = grp
all_role_ids.append(grp.id)
# I4 fix — capture old roles BEFORE the cache mutates by reading
# the stored x_fc_plating_role column directly from PostgreSQL.
# `user._origin.x_fc_plating_role` returns the IN-CACHE new value
# (the assignment that triggered the inverse), not the prior DB
# value, so the chatter audit displayed "X -> X" instead of the
# actual old -> new transition.
self.env.cr.execute(
"SELECT id, x_fc_plating_role FROM res_users WHERE id IN %s",
(tuple(self.ids),) if self.ids else ((0,),),
)
old_role_by_id = dict(self.env.cr.fetchall())
for user in self:
old_role = user._origin.x_fc_plating_role if user._origin else None
old_role = old_role_by_id.get(user.id) or 'unset'
new_role = user.x_fc_plating_role
if old_role == new_role:
# No actual change — skip both the writes and the audit so
# we don't spam chatter with "X -> X" rows.
continue
# Remove every plating-role group (additive-by-default Odoo
# m2m write of (3, id) removes single rows)
user.sudo().write({
'groups_id': [(3, gid) for gid in all_role_ids]
'group_ids': [(3, gid) for gid in all_role_ids]
})
# Add the chosen role (no-op for 'no')
@@ -94,15 +110,15 @@ class ResUsers(models.Model):
target = role_to_group.get(new_role)
if target:
user.sudo().write({
'groups_id': [(4, target.id)]
'group_ids': [(4, target.id)]
})
# Post audit (Markup() so role names render as bold, not literal HTML)
# Post audit (Markup so role names render bold, not literal HTML)
user.message_post(
body=Markup(_(
'Plating role changed: <b>%(old)s</b> -> <b>%(new)s</b> by %(actor)s'
)) % {
'old': old_role or 'unset',
'old': old_role,
'new': new_role or 'unset',
'actor': self.env.user.name,
},