From 0047f49d2c9470878458afca161e8105dd681230 Mon Sep 17 00:00:00 2001
From: gsinghpal <gur_preet.singh@outlook.com>
Date: Sun, 24 May 2026 08:37:13 -0400
Subject: [PATCH] fix(plating-perms): address final-reviewer critical +
 important issues

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>
---
 fusion_plating/CLAUDE.md                      | 13 +++++
 fusion_plating/fusion_plating/__manifest__.py |  2 +-
 .../fusion_plating/data/fp_landing_data.xml   | 49 +++----------------
 .../migrations/19.0.21.1.0/post-migrate.py    | 33 +++++++++++++
 .../fusion_plating/models/fp_landing.py       |  2 +-
 .../fusion_plating/models/fp_migration.py     | 37 ++++++++++----
 .../models/fp_role_constants.py               | 11 +++--
 .../fusion_plating/models/res_users.py        | 30 +++++++++---
 .../tests/test_acl_migration.py               |  2 +-
 .../tests/test_landing_resolver.py            |  2 +-
 .../tests/test_menu_visibility.py             |  8 +--
 .../tests/test_migration_workflow.py          |  8 +--
 .../tests/test_quality_split.py               |  4 +-
 .../tests/test_sales_manager_gate.py          |  6 +--
 .../fusion_plating/tests/test_team_page.py    |  8 +--
 .../fusion_plating/views/fp_team_views.xml    | 26 +++++-----
 .../views/res_company_views.xml               |  4 +-
 .../views/sale_order_views.xml                |  5 ++
 18 files changed, 152 insertions(+), 98 deletions(-)
 create mode 100644 fusion_plating/fusion_plating/migrations/19.0.21.1.0/post-migrate.py

diff --git a/fusion_plating/CLAUDE.md b/fusion_plating/CLAUDE.md
index d06df2b7..3a6a027f 100644
--- a/fusion_plating/CLAUDE.md
+++ b/fusion_plating/CLAUDE.md
@@ -208,6 +208,18 @@ Use only: `name`, `model_id`, `state`, `code` (or `function`/`model`), `interval
 11. **XML data ordering**: Window actions must be defined BEFORE `<menuitem>` elements that reference them in the same file.
 12. **Module install on new modules**: Use `--update=base` alongside `-i MODULE` to ensure Odoo rescans the addons path and finds the new module directory.
 13. **Implied group cascade**: `implied_ids` on `res.groups` does NOT reliably propagate to users on module install. Always include `user_ids` to explicitly assign admin, or fix via SQL post-install.
+13b. **Kanban template name — Odoo 19 wants `<t t-name="card">`, NOT `<t t-name="kanban-box">`**. Old name silently fails at render: `Error: Missing 'card' template`. Use the new structure with semantic `<aside>` + `<main>`:
+    ```xml
+    <templates>
+        <t t-name="card" class="flex-row align-items-center">
+            <aside><field name="image_128" widget="image"/></aside>
+            <main class="ms-2"><field name="name"/></main>
+        </t>
+    </templates>
+    ```
+    Reference: `/usr/lib/python3/dist-packages/odoo/addons/web/static/src/views/kanban/kanban_arch_parser.js`. Pre-existing `fp_rack_views.xml` still uses the old name and would also fail at render — fix when next touched. Caught 2026-05-24 by final reviewer of permissions-overhaul branch.
+13c. **`res.users.group_ids` NOT `groups_id`**: Odoo 19 renamed the m2m field. Old name doesn't resolve; `@api.depends('groups_id')` raises `ValueError` at module load. Also: domain on relational pickers should use `all_group_ids` (transitive set incl. implied) instead of `group_ids` (only directly-assigned) — otherwise an Owner user won't match a domain looking for QM members. See `feedback_odoo19_groups_id_renamed.md`.
+13d. **`post_init_hook` ONLY fires on INSTALL, not UPGRADE** in Odoo 19. For logic that must run on `-u` of an existing install (entech case), add a `migrations/<version>/post-migrate.py` with a `migrate(cr, version)` function that calls the same helper. The hook still works on fresh install; the migration script bridges the gap on `-u`. Both should be idempotent so re-runs are safe.
 13a. **Cross-module xmlid refs — base modules CANNOT forward-ref downstream xmlids**: A BASE module's data XML cannot `ref('downstream_module.some_xmlid')` because at fresh install, the base module loads FIRST and `ir.model.data` has no row for the downstream xmlid yet → `ValueError: External ID not found`. This bites on entech (existing DB has the row) but breaks fresh CI/test/demo/new-client installs. **Fix pattern: relocate the cross-module link to the downstream module's own security/data file, using an additive write to the BASE module's record:**
     ```xml
     <!-- In downstream module's security XML -->
@@ -239,6 +251,7 @@ Use only: `name`, `model_id`, `state`, `code` (or `function`/`model`), `interval
 20. **OWL templates expose `Math` but NOT `String` / `Number` / `Array` / `Object` / `Boolean` / `JSON` / `parseInt` / `parseFloat`**: writing `t-on-click="() => this._press(String(d))"` (or similar coercion inside any template expression) throws `Uncaught TypeError: v2 is not a function` at click time — `v2` is OWL's compiled reference to a global that doesn't exist in template scope. The click handler dies before its body runs, so the bug looks like "nothing happens when I press" (no error in the UI, only DevTools shows the trace). **Fixes, in order of preference**: (a) eliminate the coercion entirely — store data in the right type up front, e.g. `t-foreach="['1','2','3']"` instead of `[1,2,3]` so `d` is already a string. (b) Use a JS-side coercion: pass the raw value to the handler and call `String(digit)` inside the component method. (c) Use a pure-expression workaround like string concatenation: `'' + d` does work because `+` is an operator, not a function. **Do NOT try to monkey-patch `String` onto the component (e.g. `this.String = String`) or onto `env` — leaks the global into every component and is fragile across OWL upgrades.** Bit us 2026-05-23 on `pin_pad.xml` — operators couldn't tap PIN digits at all because the click handler died on `String(d)`; the SCSS, reactivity, and `_press` method were all fine, the template scope was the entire bug. Same trap applies to OWL templates anywhere in the codebase: `move_parts_dialog.xml`, `manager_dashboard.xml`, `fp_record_inputs_dialog.xml`, etc. — grep all `t-on-click`, `t-att-*`, and `t-out` expressions for `String(`, `Number(`, `Array(`, `parseInt(`, `parseFloat(`, `JSON.` before merging.
 21. **`ir.actions.act_window_close` is a no-op when the current action was opened with `target: "current"`**: replacing the current action wipes the breadcrumb backstack, so there's nothing to close back to. The user clicks "Back" and nothing happens (no error, no navigation). This bites every OWL client-action surface that calls another client action via `doAction({..., target: "current"})` — the destination has no way to return to the source. **Fix pattern for "Back" buttons in OWL client actions**: navigate EXPLICITLY to the landing/parent action by tag, e.g. `this.action.doAction({ type: "ir.actions.client", tag: "fp_shopfloor_landing", target: "current" })` — works regardless of how the action was reached (kanban tap, QR scan, smart button, direct URL). **Do NOT rely on `act_window_close`, `history.back()`, or `this.env.config.breadcrumbs`** — all three are unreliable across navigation paths. Bit us 2026-05-23 on the Job Workspace Back button after the kanban opened the workspace with `target: "current"`. The same pattern applies to every other "Back" button in shopfloor / manager / portal OWL surfaces — explicit destination via `tag:` is the only robust answer.
 22. **Odoo 19 HTML fields auto-wrap plain-string writes**: writing `co.report_header = 'Plating & Finishing'` to an HTML field (like `res.company.report_header`, `res.partner.comment`, `mail.template.body_html`, `product.template.description_sale`) stores `<p>Plating &amp; Finishing</p>` after Odoo's HTML sanitizer runs. Equality tests against the raw input string FAIL (`payload['tagline'] != 'Plating & Finishing'`). **Three implications**: (a) **In tests**, don't `assertEqual` against the literal string you wrote — strip tags first, OR write the wrapped form (`<p>Plating & Finishing</p>`), OR write an explicit `Markup('<p>...</p>')` so the round-trip stays stable. (b) **In display code**, render HTML fields with `t-out` (QWeb) or `markup(...)` (OWL) — `t-esc` would render the literal `<p>` tags as text. (c) **In comparison logic**, normalize first: `from markupsafe import escape; escape(input_str)` produces the same shape the field stores. Bit us 2026-05-24 testing the lock-screen tagline source (`_lock_company_payload` reads `res.company.report_header`); the test that wrote a plain string and asserted equality failed because the value came back wrapped. The fix was to delete the brittle equality test — the helper's responsibility is just "use the field's value when present, else fall back," which is covered by the empty-field test. Generalizes to ANY HTML-typed Odoo field. Distinct from the `mail.template.body_html is Markup + jsonb` gotcha noted earlier in this file — that's about Markup objects vs strings; this is about the sanitizer wrapping plain strings on write.
+23. **`res.users.group_ids` vs `all_group_ids` for domain filters**: in Odoo 19, `res.users` carries TWO M2M-to-`res.groups` fields and they have different membership semantics. `group_ids` is the user's DIRECTLY-assigned groups (what the user record literally wrote). `all_group_ids` is the TRANSITIVE set — direct groups PLUS every group implied via `implied_ids` chains. **For domain filters on user pickers** (e.g. "show users who can act as a Quality Manager"), ALWAYS use `all_group_ids`, never `group_ids`. An Owner user only carries `group_fp_owner` directly; the QM capability comes via `implied_ids → group_fp_quality_manager`, so a `domain="[('group_ids', 'in', [ref('...quality_manager')])]"` excludes Owners and the picker looks empty. Use `domain="[('all_group_ids', 'in', [ref('...quality_manager'), ref('...owner')])]"` instead. Compute helpers (`@api.depends('group_ids')`) and write vals (`{'group_ids': [(4, gid)]}`) still use `group_ids` because those operate on direct assignments — only domain filters need the transitive set. Bit us 2026-05-24 on the CGP DO + Nadcap Authority pickers on `res.company`. Same gotcha applies to ANY domain that needs "does this user effectively have role X" semantics across user-facing pickers, ACL rules, server actions, and search filters.
 
 ## Naming
 - **New custom models** (post-2026-04): `fp.*` prefix (e.g. `fp.part.catalog`, `fp.certificate`)
diff --git a/fusion_plating/fusion_plating/__manifest__.py b/fusion_plating/fusion_plating/__manifest__.py
index 4941fd3f..cadc5dd8 100644
--- a/fusion_plating/fusion_plating/__manifest__.py
+++ b/fusion_plating/fusion_plating/__manifest__.py
@@ -5,7 +5,7 @@
 
 {
     'name': 'Fusion Plating',
-    'version': '19.0.21.0.6',
+    'version': '19.0.21.1.0',
     'category': 'Manufacturing/Plating',
     'summary': 'Core plating / metal finishing ERP: facilities, processes, tanks, baths, jobs, operators.',
     'description': """
diff --git a/fusion_plating/fusion_plating/data/fp_landing_data.xml b/fusion_plating/fusion_plating/data/fp_landing_data.xml
index f823df61..d52afae4 100644
--- a/fusion_plating/fusion_plating/data/fp_landing_data.xml
+++ b/fusion_plating/fusion_plating/data/fp_landing_data.xml
@@ -24,47 +24,14 @@
         <field name="model_id" ref="base.model_res_users"/>
         <field name="state">code</field>
         <field name="code"><![CDATA[
-# Resolve in priority order:
-#   1. user.x_fc_plating_landing_action_id     (per-user override)
-#   2. company.x_fc_default_landing_action_id  (company default)
-#   3. Shop Floor plant-view kanban             (when x_fc_shopfloor_layout='v2')
-#   4. Sale Orders                              (when v2 flag unset / legacy)
-#   5. Process recipes                          (configurator absent)
-user = env.user
-target = False
-if 'x_fc_plating_landing_action_id' in user._fields and user.x_fc_plating_landing_action_id:
-    target = user.x_fc_plating_landing_action_id.sudo()
-elif 'x_fc_default_landing_action_id' in env.company._fields and env.company.x_fc_default_landing_action_id:
-    target = env.company.x_fc_default_landing_action_id.sudo()
-
-if not target:
-    # 2026-05-23 — plant-view dispatch. Read the layout flag and pick the
-    # appropriate Shop Floor action. Falls through to Sale Orders if no
-    # client action is registered (e.g. shopfloor module not installed).
-    layout = env['ir.config_parameter'].sudo().get_param(
-        'fusion_plating_shopfloor.layout', default='legacy',
-    )
-    if layout == 'v2':
-        target = env.ref(
-            'fusion_plating_shopfloor.action_fp_plant_kanban',
-            raise_if_not_found=False,
-        )
-    # Legacy or v2-missing → fall through to Sale Orders
-    if not target:
-        target = env.ref(
-            'fusion_plating_configurator.action_fp_sale_orders',
-            raise_if_not_found=False,
-        )
-
-if target:
-    action = target.sudo().read()[0]
-    # Strip ids that confuse the act_window dispatcher.
-    action.pop('id', None)
-else:
-    # Last-ditch — open the Plating app's process recipes if even
-    # the Sale Orders action is missing (e.g. configurator not installed).
-    action = env.ref('fusion_plating.action_fp_process_recipe').sudo().read()[0]
-    action.pop('id', None)
+# Delegates to the role-based dispatch helper on ir.actions.act_window
+# (and ir.actions.client for Manager Desk / Plant Kanban / Quality Dashboard).
+# Resolution chain in the helper:
+#   1. user.x_fc_plating_landing_action_id      (per-user override)
+#   2. role-based default per spec Section 3    (Owner→ManagerDesk, etc.)
+#   3. company.x_fc_default_landing_action_id   (company default)
+#   4. action_fp_sale_orders                    (hardcoded last-ditch)
+action = env['ir.actions.act_window'].sudo()._fp_resolve_landing_for_current_user() or False
         ]]></field>
     </record>
 
diff --git a/fusion_plating/fusion_plating/migrations/19.0.21.1.0/post-migrate.py b/fusion_plating/fusion_plating/migrations/19.0.21.1.0/post-migrate.py
new file mode 100644
index 00000000..fbe83312
--- /dev/null
+++ b/fusion_plating/fusion_plating/migrations/19.0.21.1.0/post-migrate.py
@@ -0,0 +1,33 @@
+# -*- coding: utf-8 -*-
+"""Phase H: fire role-migration preview creation on `-u fusion_plating`.
+
+Odoo 19's `post_init_hook` ONLY fires on fresh install — never on
+upgrade. So on entech (and any other already-installed deployment),
+`-u fusion_plating` after this branch lands would otherwise leave the
+post_init_hook's `_fp_post_init_role_migration` un-fired and the
+migration preview never created.
+
+This migration script bridges that gap: on every `-u` that crosses
+this version boundary, it invokes the same idempotent helper. The
+helper short-circuits if a preview is already pending or already
+applied + all users migrated, so re-running is safe.
+"""
+import logging
+
+_logger = logging.getLogger(__name__)
+
+
+def migrate(cr, version):
+    from odoo import api, SUPERUSER_ID
+    env = api.Environment(cr, SUPERUSER_ID, {})
+    try:
+        from odoo.addons.fusion_plating import _fp_post_init_role_migration
+        _fp_post_init_role_migration(env)
+        _logger.info(
+            'Fusion Plating: role-migration preview check ran via post-migrate.py'
+        )
+    except Exception as e:
+        # Migration scripts must not block module upgrade — log and swallow
+        _logger.exception(
+            'Failed to run role-migration preview check (non-fatal): %s', e
+        )
diff --git a/fusion_plating/fusion_plating/models/fp_landing.py b/fusion_plating/fusion_plating/models/fp_landing.py
index 7aabff00..ee7477ed 100644
--- a/fusion_plating/fusion_plating/models/fp_landing.py
+++ b/fusion_plating/fusion_plating/models/fp_landing.py
@@ -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)])
diff --git a/fusion_plating/fusion_plating/models/fp_migration.py b/fusion_plating/fusion_plating/models/fp_migration.py
index a6b2f680..18be39a7 100644
--- a/fusion_plating/fusion_plating/models/fp_migration.py
+++ b/fusion_plating/fusion_plating/models/fp_migration.py
@@ -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 = ''
diff --git a/fusion_plating/fusion_plating/models/fp_role_constants.py b/fusion_plating/fusion_plating/models/fp_role_constants.py
index 71c3ad9f..05afcea9 100644
--- a/fusion_plating/fusion_plating/models/fp_role_constants.py
+++ b/fusion_plating/fusion_plating/models/fp_role_constants.py
@@ -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),
diff --git a/fusion_plating/fusion_plating/models/res_users.py b/fusion_plating/fusion_plating/models/res_users.py
index fdfe4d0c..6afea2d6 100644
--- a/fusion_plating/fusion_plating/models/res_users.py
+++ b/fusion_plating/fusion_plating/models/res_users.py
@@ -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,
                 },
diff --git a/fusion_plating/fusion_plating/tests/test_acl_migration.py b/fusion_plating/fusion_plating/tests/test_acl_migration.py
index 4e86beee..db7f27fc 100644
--- a/fusion_plating/fusion_plating/tests/test_acl_migration.py
+++ b/fusion_plating/fusion_plating/tests/test_acl_migration.py
@@ -14,7 +14,7 @@ class TestAclMigration(TransactionCase):
                 'login': f'fp_test_{login}',
                 'name': f'FP Test {login.title()}',
                 'email': f'fp_test_{login}@example.com',
-                'groups_id': [(6, 0, [self.env.ref(group_xmlid).id])],
+                'group_ids': [(6, 0, [self.env.ref(group_xmlid).id])],
             })
 
         self.u_tech = make('tech', 'fusion_plating.group_fp_technician')
diff --git a/fusion_plating/fusion_plating/tests/test_landing_resolver.py b/fusion_plating/fusion_plating/tests/test_landing_resolver.py
index d9427ab4..0c8e2139 100644
--- a/fusion_plating/fusion_plating/tests/test_landing_resolver.py
+++ b/fusion_plating/fusion_plating/tests/test_landing_resolver.py
@@ -38,7 +38,7 @@ class TestLandingResolver(TransactionCase):
                 'login': f'land_{name}',
                 'name': f'Land {name}',
                 'email': f'land_{name}@example.com',
-                'groups_id': [(6, 0, [self.env.ref(xmlid).id])],
+                'group_ids': [(6, 0, [self.env.ref(xmlid).id])],
             })
 
         self.u_tech = mk('tech', 'fusion_plating.group_fp_technician')
diff --git a/fusion_plating/fusion_plating/tests/test_menu_visibility.py b/fusion_plating/fusion_plating/tests/test_menu_visibility.py
index 8d8fe1a5..bee352a3 100644
--- a/fusion_plating/fusion_plating/tests/test_menu_visibility.py
+++ b/fusion_plating/fusion_plating/tests/test_menu_visibility.py
@@ -12,14 +12,14 @@ class TestMenuVisibility(TransactionCase):
             return Users.create({
                 'login': f'menu_{name}', 'name': f'Menu Test {name}',
                 'email': f'menu_{name}@example.com',
-                'groups_id': [(6, 0, [self.env.ref(xmlid).id])] if xmlid else [(6, 0, [])],
+                'group_ids': [(6, 0, [self.env.ref(xmlid).id])] if xmlid else [(6, 0, [])],
             })
         # "No" user has only base.group_user — no plating group
         no_user = Users.create({
             'login': 'menu_no', 'name': 'Menu Test no',
             'email': 'menu_no@example.com',
         })
-        no_user.write({'groups_id': [(6, 0, [self.env.ref('base.group_user').id])]})
+        no_user.write({'group_ids': [(6, 0, [self.env.ref('base.group_user').id])]})
         self.u_no = no_user
         self.u_tech = mk('tech', 'fusion_plating.group_fp_technician')
         self.u_sr = mk('sr', 'fusion_plating.group_fp_sales_rep')
@@ -73,13 +73,13 @@ class TestMenuVisibility(TransactionCase):
         self.assertTrue(result)
 
     def test_manager_does_not_see_compliance(self):
-        result = self._visible(self.u_mgr, 'fusion_plating_compliance.menu_fp_compliance_hub')
+        result = self._visible(self.u_mgr, 'fusion_plating.menu_fp_compliance_hub')
         if result is None:
             self.skipTest('Compliance hub not found')
         self.assertFalse(result, 'Manager must not see Compliance hub')
 
     def test_qm_sees_compliance(self):
-        result = self._visible(self.u_qm, 'fusion_plating_compliance.menu_fp_compliance_hub')
+        result = self._visible(self.u_qm, 'fusion_plating.menu_fp_compliance_hub')
         if result is None:
             self.skipTest('Compliance hub not found')
         self.assertTrue(result)
diff --git a/fusion_plating/fusion_plating/tests/test_migration_workflow.py b/fusion_plating/fusion_plating/tests/test_migration_workflow.py
index 0e6b31be..23eceb0d 100644
--- a/fusion_plating/fusion_plating/tests/test_migration_workflow.py
+++ b/fusion_plating/fusion_plating/tests/test_migration_workflow.py
@@ -12,14 +12,14 @@ class TestMigrationWorkflow(TransactionCase):
         self.owner = Users.create({
             'login': 'mig_owner', 'name': 'Mig Owner',
             'email': 'mig_owner@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_owner').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_owner').id])],
         })
 
     def test_only_owner_can_approve(self):
         non_owner = self.env['res.users'].with_context(no_reset_password=True).create({
             'login': 'mig_nonowner', 'name': 'Non Owner',
             'email': 'mig_nonowner@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_manager').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_manager').id])],
         })
         preview = self.env['fp.migration.preview'].create({})
         preview._fp_build_lines()
@@ -52,7 +52,7 @@ class TestMigrationWorkflow(TransactionCase):
         u = self.env['res.users'].with_context(no_reset_password=True).create({
             'login': 'mig_rb', 'name': 'RB',
             'email': 'mig_rb@example.com',
-            'groups_id': [(6, 0, [old_mgr.id])],
+            'group_ids': [(6, 0, [old_mgr.id])],
         })
         before_ids = sorted(u.groups_id.ids)
         preview = self.env['fp.migration.preview'].create({})
@@ -74,7 +74,7 @@ class TestMigrationWorkflow(TransactionCase):
         u = self.env['res.users'].with_context(no_reset_password=True).create({
             'login': 'mig_est', 'name': 'Est',
             'email': 'mig_est@example.com',
-            'groups_id': [(6, 0, [est.id])],
+            'group_ids': [(6, 0, [est.id])],
         })
         preview = self.env['fp.migration.preview'].create({})
         preview._fp_build_lines()
diff --git a/fusion_plating/fusion_plating/tests/test_quality_split.py b/fusion_plating/fusion_plating/tests/test_quality_split.py
index 9c459590..1d8597ff 100644
--- a/fusion_plating/fusion_plating/tests/test_quality_split.py
+++ b/fusion_plating/fusion_plating/tests/test_quality_split.py
@@ -13,12 +13,12 @@ class TestQualitySplit(TransactionCase):
         self.u_mgr = Users.create({
             'login': 'qsplit_mgr', 'name': 'QSplit Mgr',
             'email': 'qsplit_mgr@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_manager').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_manager').id])],
         })
         self.u_qm = Users.create({
             'login': 'qsplit_qm', 'name': 'QSplit QM',
             'email': 'qsplit_qm@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_quality_manager').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_quality_manager').id])],
         })
 
     # CAPA: Manager read-only, QM full
diff --git a/fusion_plating/fusion_plating/tests/test_sales_manager_gate.py b/fusion_plating/fusion_plating/tests/test_sales_manager_gate.py
index 51e8d1fe..ae7c67a4 100644
--- a/fusion_plating/fusion_plating/tests/test_sales_manager_gate.py
+++ b/fusion_plating/fusion_plating/tests/test_sales_manager_gate.py
@@ -11,12 +11,12 @@ class TestSalesManagerGate(TransactionCase):
         self.u_sr = Users.create({
             'login': 'gate_sr', 'name': 'Gate SR',
             'email': 'gate_sr@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_sales_rep').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_sales_rep').id])],
         })
         self.u_smg = Users.create({
             'login': 'gate_smg', 'name': 'Gate SMg',
             'email': 'gate_smg@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_sales_manager').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_sales_manager').id])],
         })
         partner = self.env['res.partner'].create({'name': 'Gate Test Customer'})
         product = self.env['product.product'].create({'name': 'Gate Test Product'})
@@ -40,7 +40,7 @@ class TestSalesManagerGate(TransactionCase):
         u_mgr = self.env['res.users'].with_context(no_reset_password=True).create({
             'login': 'gate_mgr', 'name': 'Gate Mgr',
             'email': 'gate_mgr@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_manager').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_manager').id])],
         })
         self.so.with_user(u_mgr).action_confirm()
         self.assertEqual(self.so.state, 'sale')
diff --git a/fusion_plating/fusion_plating/tests/test_team_page.py b/fusion_plating/fusion_plating/tests/test_team_page.py
index 0c95c34c..bb26a7e3 100644
--- a/fusion_plating/fusion_plating/tests/test_team_page.py
+++ b/fusion_plating/fusion_plating/tests/test_team_page.py
@@ -12,12 +12,12 @@ class TestTeamPage(TransactionCase):
         self.owner = Users.create({
             'login': 'team_owner', 'name': 'Team Owner',
             'email': 'team_owner@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_owner').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_owner').id])],
         })
         self.target = Users.create({
             'login': 'team_target', 'name': 'Team Target',
             'email': 'team_target@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_technician').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_technician').id])],
         })
 
     def test_compute_returns_technician(self):
@@ -25,7 +25,7 @@ class TestTeamPage(TransactionCase):
 
     def test_compute_picks_highest_role(self):
         # Add Manager group on top of Technician
-        self.target.write({'groups_id': [(4, self.env.ref('fusion_plating.group_fp_manager').id)]})
+        self.target.write({'group_ids': [(4, self.env.ref('fusion_plating.group_fp_manager').id)]})
         self.target.invalidate_recordset(['x_fc_plating_role'])
         self.assertEqual(self.target.x_fc_plating_role, 'manager')
 
@@ -88,7 +88,7 @@ class TestTeamPage(TransactionCase):
         mgr = self.env['res.users'].with_context(no_reset_password=True).create({
             'login': 'team_mgr', 'name': 'Team Mgr',
             'email': 'team_mgr@example.com',
-            'groups_id': [(6, 0, [self.env.ref('fusion_plating.group_fp_manager').id])],
+            'group_ids': [(6, 0, [self.env.ref('fusion_plating.group_fp_manager').id])],
         })
         visible = self.env['ir.ui.menu'].with_user(mgr).search_count([('id', '=', menu.id)])
         self.assertFalse(visible, 'Manager must not see Team menu (Owner-only)')
diff --git a/fusion_plating/fusion_plating/views/fp_team_views.xml b/fusion_plating/fusion_plating/views/fp_team_views.xml
index a5dd7b21..ffaebd39 100644
--- a/fusion_plating/fusion_plating/views/fp_team_views.xml
+++ b/fusion_plating/fusion_plating/views/fp_team_views.xml
@@ -23,22 +23,20 @@
                     <field name="login_date"/>
                     <field name="name"/>
                     <templates>
-                        <t t-name="kanban-box">
-                            <div t-attf-class="oe_kanban_card oe_kanban_global_click">
-                                <div class="o_kanban_image">
-                                    <img t-att-src="kanban_image('res.users', 'image_128', record.id.raw_value)"
-                                         alt="Photo"/>
+                        <t t-name="card" class="flex-row align-items-center">
+                            <aside class="o_kanban_aside_full">
+                                <field name="image_128" widget="image"
+                                       options="{'preview_image': 'image_128', 'img_class': 'rounded'}"/>
+                            </aside>
+                            <main class="ms-2">
+                                <field name="name" class="fw-bolder fs-5"/>
+                                <div t-if="record.email.raw_value" class="text-muted small">
+                                    <field name="email"/>
                                 </div>
-                                <div class="oe_kanban_details">
-                                    <strong><field name="name"/></strong>
-                                    <div t-if="record.email.raw_value">
-                                        <field name="email"/>
-                                    </div>
-                                    <div t-if="record.login_date.raw_value" class="text-muted">
-                                        Last seen: <field name="login_date" widget="date"/>
-                                    </div>
+                                <div t-if="record.login_date.raw_value" class="text-muted small">
+                                    Last seen: <field name="login_date" widget="date"/>
                                 </div>
-                            </div>
+                            </main>
                         </t>
                     </templates>
                 </kanban>
diff --git a/fusion_plating/fusion_plating/views/res_company_views.xml b/fusion_plating/fusion_plating/views/res_company_views.xml
index f1454c59..8bfaf513 100644
--- a/fusion_plating/fusion_plating/views/res_company_views.xml
+++ b/fusion_plating/fusion_plating/views/res_company_views.xml
@@ -12,9 +12,9 @@
                           groups="fusion_plating.group_fp_owner">
                         <group>
                             <field name="x_fc_cgp_designated_official_id"
-                                   domain="[('groups_id', 'in', [ref('fusion_plating.group_fp_quality_manager'), ref('fusion_plating.group_fp_owner')])]"/>
+                                   domain="[('all_group_ids', 'in', [ref('fusion_plating.group_fp_quality_manager'), ref('fusion_plating.group_fp_owner')])]"/>
                             <field name="x_fc_nadcap_authority_user_id"
-                                   domain="[('groups_id', 'in', [ref('fusion_plating.group_fp_quality_manager'), ref('fusion_plating.group_fp_owner')])]"/>
+                                   domain="[('all_group_ids', 'in', [ref('fusion_plating.group_fp_quality_manager'), ref('fusion_plating.group_fp_owner')])]"/>
                         </group>
                     </page>
                 </xpath>
diff --git a/fusion_plating/fusion_plating_configurator/views/sale_order_views.xml b/fusion_plating/fusion_plating_configurator/views/sale_order_views.xml
index 6de3c70c..966ea0ef 100644
--- a/fusion_plating/fusion_plating_configurator/views/sale_order_views.xml
+++ b/fusion_plating/fusion_plating_configurator/views/sale_order_views.xml
@@ -21,6 +21,11 @@
                 <attribute name="class">btn-primary</attribute>
                 <attribute name="groups">fusion_plating.group_fp_sales_manager</attribute>
             </xpath>
+            <!-- Also gate the state=sent Confirm button (id="action_confirm") so
+                 Sales Reps don't see EITHER variant. Matches the model-level gate. -->
+            <xpath expr="//header/button[@id='action_confirm']" position="attributes">
+                <attribute name="groups">fusion_plating.group_fp_sales_manager</attribute>
+            </xpath>
             <xpath expr="//header/button[@id='quotation_send_primary']" position="attributes">
                 <attribute name="string">Send Email</attribute>
                 <attribute name="class">btn-danger</attribute>