fix(change_request): run auto-apply through the public action_apply

spp_change_request_v2/README.rst

@@ -853,6 +853,20 @@ Before declaring a new CR type complete:
 Changelog
 =========
 
+19.0.3.1.12
+~~~~~~~~~~~
+
+- fix(change_request): auto-apply-on-approve runs through the public
+  ``action_apply`` again. Requiring change-request manager rights to
+  apply meant auto-apply was routed to the internal mechanism instead,
+  so the approver could be a validator — but ``action_apply`` is the
+  extension point modules override to hang post-apply work off an apply,
+  and bypassing it left those overrides silently not running on
+  approval: no error, just missing side effects. Auto-apply now calls
+  ``action_apply`` under ``sudo()``, which the manager gate already
+  exempts. ``sudo()`` sets superuser mode without changing the user, so
+  the applying user is still recorded as the approver.
+
 19.0.3.1.11
 ~~~~~~~~~~~
 

spp_change_request_v2/__manifest__.py

@@ -1,6 +1,6 @@
 {
     "name": "OpenSPP Change Request V2",
-    "version": "19.0.3.1.11",
+    "version": "19.0.3.1.12",
     "sequence": 50,
     "category": "OpenSPP",
     "summary": "Configuration-driven change request system with UX improvements, conflict detection and duplicate prevention",

spp_change_request_v2/models/change_request.py

@@ -1093,10 +1093,16 @@ def _on_approve(self):
         self._create_log("approved")
         if self.request_type_id.auto_apply_on_approve:
             # Auto-apply is authorized by the approval workflow itself, so it
-            # goes through the internal mechanism rather than the manager-gated
-            # public action_apply (the approver may be a validator, not a
-            # manager).
-            self._apply_change_request()
+            # runs with sudo: ``action_apply``'s manager gate exempts
+            # ``env.su``, which lets the approver be a validator rather than a
+            # manager. Going through the public entry point rather than the
+            # internal mechanism keeps ``action_apply`` the single extension
+            # point for apply -- downstream modules override it to hang
+            # post-apply work off the apply, and routing around it left those
+            # overrides silently not running on approval. ``sudo()`` sets
+            # ``su`` without changing ``uid``, so ``applied_by_id`` still
+            # records the real approver.
+            self.sudo().action_apply()  # nosemgrep: odoo-sudo-without-context
 
     def _on_reject(self, reason):
         super()._on_reject(reason)
@@ -1448,9 +1454,16 @@ def action_apply(self):
         roles cannot (e.g. ``spp.group.membership``), so it must be gated
         server-side to managers: the XML button ``groups=`` is NOT an
         authorization boundary because Odoo object methods are callable over
-        RPC. Superuser (sudo) callers and the auto-apply-on-approve path (which
-        invokes ``_apply_change_request`` directly, already authorized by the
-        approval workflow) are unaffected.
+        RPC. Superuser (``env.su``) callers are exempt, and
+        auto-apply-on-approve is one of them: ``_on_approve`` reaches this
+        method through ``sudo()``, already authorized by the approval workflow
+        itself, so the approver may be a validator rather than a manager.
+
+        This is also the extension point for apply, so an override runs on both
+        paths -- under ``su`` when auto-applied on approval, and as the manager
+        who clicked Apply on the manual path. ``sudo()`` sets ``su`` without
+        changing ``uid``, so ``self.env.user`` is the approver either way and
+        ``applied_by_id`` records them, not the superuser.
         """
         if not (self.env.su or self.env.user.has_group("spp_change_request_v2.group_cr_manager")):
             raise AccessError(_("Only Change Request managers can apply change requests."))

spp_change_request_v2/readme/HISTORY.md

@@ -1,3 +1,7 @@
+### 19.0.3.1.12
+
+- fix(change_request): auto-apply-on-approve runs through the public `action_apply` again. Requiring change-request manager rights to apply meant auto-apply was routed to the internal mechanism instead, so the approver could be a validator — but `action_apply` is the extension point modules override to hang post-apply work off an apply, and bypassing it left those overrides silently not running on approval: no error, just missing side effects. Auto-apply now calls `action_apply` under `sudo()`, which the manager gate already exempts. `sudo()` sets superuser mode without changing the user, so the applying user is still recorded as the approver.
+
 ### 19.0.3.1.11
 
 - fix(change_request): field-mapping transform expressions are evaluated again. `_eval_expression` passed `nocopy=True` to `safe_eval`, which takes no such argument in Odoo 19, so every expression raised `TypeError`; the blanket fallback swallowed it and the **untransformed** value was written to the registrant. A configured transform was therefore ignored, reported only as a warning in the log. **Behaviour change:** request types that already have an Expression transform configured will start transforming values on upgrade, having silently passed the raw value through until now.

spp_change_request_v2/static/description/index.html

@@ -1339,6 +1339,21 @@ <h2>Changelog</h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.3.1.12</h1>
+<ul class="simple">
+<li>fix(change_request): auto-apply-on-approve runs through the public
+<tt class="docutils literal">action_apply</tt> again. Requiring change-request manager rights to
+apply meant auto-apply was routed to the internal mechanism instead,
+so the approver could be a validator — but <tt class="docutils literal">action_apply</tt> is the
+extension point modules override to hang post-apply work off an apply,
+and bypassing it left those overrides silently not running on
+approval: no error, just missing side effects. Auto-apply now calls
+<tt class="docutils literal">action_apply</tt> under <tt class="docutils literal">sudo()</tt>, which the manager gate already
+exempts. <tt class="docutils literal">sudo()</tt> sets superuser mode without changing the user, so
+the applying user is still recorded as the approver.</li>
+</ul>
+</div>
+<div class="section" id="section-2">
 <h1>19.0.3.1.11</h1>
 <ul class="simple">
 <li>fix(change_request): field-mapping transform expressions are evaluated
@@ -1377,7 +1392,7 @@ <h1>19.0.3.1.11</h1>
 the full traceback is logged only at DEBUG.</li>
 </ul>
 </div>
-<div class="section" id="section-2">
+<div class="section" id="section-3">
 <h1>19.0.3.1.10</h1>
 <ul class="simple">
 <li>fix(security): conflict and duplicate detection now decide whether a
@@ -1417,7 +1432,7 @@ <h1>19.0.3.1.10</h1>
 configured mapping.</li>
 </ul>
 </div>
-<div class="section" id="section-3">
+<div class="section" id="section-4">
 <h1>19.0.3.1.9</h1>
 <ul class="simple">
 <li>fix(security): duplicate detection now scores the fields both change
@@ -1434,7 +1449,7 @@ <h1>19.0.3.1.9</h1>
 requester-writable <tt class="docutils literal">selected_field_name</tt> / <tt class="docutils literal">field_to_modify</tt>.</li>
 </ul>
 </div>
-<div class="section" id="section-4">
+<div class="section" id="section-5">
 <h1>19.0.3.1.8</h1>
 <ul class="simple">
 <li>fix(security): scope the Create-Group member wizards to the parent
@@ -1452,7 +1467,7 @@ <h1>19.0.3.1.8</h1>
 access-control entry grants.</li>
 </ul>
 </div>
-<div class="section" id="section-5">
+<div class="section" id="section-6">
 <h1>19.0.3.1.7</h1>
 <ul class="simple">
 <li>fix(security): require change-request manager rights to apply a change
@@ -1467,7 +1482,7 @@ <h1>19.0.3.1.7</h1>
 endpoint.</strong></li>
 </ul>
 </div>
-<div class="section" id="section-6">
+<div class="section" id="section-7">
 <h1>19.0.3.1.6</h1>
 <ul class="simple">
 <li>fix(security): derive conflict and duplicate detection from the change
@@ -1481,7 +1496,7 @@ <h1>19.0.3.1.6</h1>
 an empty one, so detection cannot silently disable itself.</li>
 </ul>
 </div>
-<div class="section" id="section-7">
+<div class="section" id="section-8">
 <h1>19.0.3.1.5</h1>
 <ul class="simple">
 <li>fix(security): scope the CR Requestor, Local Validator and HQ
@@ -1493,7 +1508,7 @@ <h1>19.0.3.1.5</h1>
 are <tt class="docutils literal">noupdate</tt>.</li>
 </ul>
 </div>
-<div class="section" id="section-8">
+<div class="section" id="section-9">
 <h1>19.0.3.1.4</h1>
 <ul class="simple">
 <li>fix(security): add ownership and area record rules to every concrete
@@ -1510,7 +1525,7 @@ <h1>19.0.3.1.4</h1>
 unrestricted delete their access-control entries grant.</li>
 </ul>
 </div>
-<div class="section" id="section-9">
+<div class="section" id="section-10">
 <h1>19.0.3.1.3</h1>
 <ul class="simple">
 <li>fix(security): route and apply the same single field for
@@ -1523,7 +1538,7 @@ <h1>19.0.3.1.3</h1>
 the routing selector.</li>
 </ul>
 </div>
-<div class="section" id="section-10">
+<div class="section" id="section-11">
 <h1>19.0.3.1.2</h1>
 <ul class="simple">
 <li>fix(change_request_v2): adding an ID now looks for a live one of that
@@ -1532,7 +1547,7 @@ <h1>19.0.3.1.2</h1>
 (#1136)</li>
 </ul>
 </div>
-<div class="section" id="section-11">
+<div class="section" id="section-12">
 <h1>19.0.3.1.1</h1>
 <ul class="simple">
 <li>fix(change_request): enforce the <tt class="docutils literal">(cr_type_id, reason)</tt> uniqueness
@@ -1546,7 +1561,7 @@ <h1>19.0.3.1.1</h1>
 applied) so the constraint applies cleanly on upgrade.</li>
 </ul>
 </div>
-<div class="section" id="section-12">
+<div class="section" id="section-13">
 <h1>19.0.3.1.0</h1>
 <ul class="simple">
 <li>revert(change_request): restore the create-a-new-individual <strong>Add
@@ -1564,7 +1579,7 @@ <h1>19.0.3.1.0</h1>
 <strong>not</strong> restored here; reinstate separately if needed.</li>
 </ul>
 </div>
-<div class="section" id="section-13">
+<div class="section" id="section-14">
 <h1>19.0.3.0.0</h1>
 <ul class="simple">
 <li>feat(change_request): redesign the group/membership CR flows (#242) —
@@ -1586,7 +1601,7 @@ <h1>19.0.3.0.0</h1>
 must adapt (see #1133).</li>
 </ul>
 </div>
-<div class="section" id="section-14">
+<div class="section" id="section-15">
 <h1>19.0.2.0.8</h1>
 <ul class="simple">
 <li>fix(views): disable inline creation of CR document types on the Change
@@ -1597,7 +1612,7 @@ <h1>19.0.2.0.8</h1>
 Documents” modal (missing Name field) that blocked saving (#1125)</li>
 </ul>
 </div>
-<div class="section" id="section-15">
+<div class="section" id="section-16">
 <h1>19.0.2.0.7</h1>
 <ul class="simple">
 <li>fix(security): align CR Requestor / CR Local Validator / CR HQ
@@ -1609,7 +1624,7 @@ <h1>19.0.2.0.7</h1>
 dependencies.</li>
 </ul>
 </div>
-<div class="section" id="section-16">
+<div class="section" id="section-17">
 <h1>19.0.2.0.6</h1>
 <ul class="simple">
 <li>fix(views): route post-submit CRs (pending / approved / applied /
@@ -1624,7 +1639,7 @@ <h1>19.0.2.0.6</h1>
 list so row-click goes through the stage router.</li>
 </ul>
 </div>
-<div class="section" id="section-17">
+<div class="section" id="section-18">
 <h1>19.0.2.0.5</h1>
 <ul class="simple">
 <li>fix(security): add a global <tt class="docutils literal">ir.rule</tt> on <tt class="docutils literal">spp.change.request</tt> that
@@ -1637,27 +1652,27 @@ <h1>19.0.2.0.5</h1>
 roles).</li>
 </ul>
 </div>
-<div class="section" id="section-18">
+<div class="section" id="section-19">
 <h1>19.0.2.0.3</h1>
 <ul class="simple">
 <li>fix: add HTML escaping to all computed Html fields with
 <tt class="docutils literal">sanitize=False</tt> to prevent stored XSS (#50)</li>
 </ul>
 </div>
-<div class="section" id="section-19">
+<div class="section" id="section-20">
 <h1>19.0.2.0.2</h1>
 <ul class="simple">
 <li>fix: fix batch approval wizard line deletion (#130)</li>
 </ul>
 </div>
-<div class="section" id="section-20">
+<div class="section" id="section-21">
 <h1>19.0.2.0.1</h1>
 <ul class="simple">
 <li>fix: skip field types before getattr and isolate detail prefetch
 (#129)</li>
 </ul>
 </div>
-<div class="section" id="section-21">
+<div class="section" id="section-22">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>

spp_change_request_v2/tests/test_apply_authorization.py

@@ -99,10 +99,46 @@ def test_manager_can_apply(self):
         self.assertTrue(cr.is_applied)
         self.assertTrue(self.membership.ended_date)
 
+    def test_auto_apply_runs_the_public_apply_entry_point(self):
+        """``action_apply`` is the extension point downstream modules override.
+
+        Auto-apply used to bypass it, so any override hung off apply silently
+        stopped running on approval -- no error, just missing side effects.
+        """
+        from unittest.mock import patch
+
+        cr = self._make_approved_cr()
+        cr.request_type_id.auto_apply_on_approve = True
+
+        seen = []
+        original = type(cr).action_apply
+
+        def spy(records):
+            seen.append(tuple(records.ids))
+            return original(records)
+
+        with patch.object(type(cr), "action_apply", spy):
+            cr.with_user(self.cr_validator)._on_approve()
+
+        self.assertTrue(seen, "auto-apply must go through the public action_apply")
+        self.assertTrue(cr.is_applied)
+
+    def test_auto_apply_records_the_real_approver(self):
+        """sudo() sets su without changing uid, so attribution is preserved."""
+        cr = self._make_approved_cr()
+        cr.request_type_id.auto_apply_on_approve = True
+        cr.with_user(self.cr_validator)._on_approve()
+        self.assertEqual(
+            cr.applied_by_id,
+            self.cr_validator,
+            "applying under sudo must still record the approver, not the superuser",
+        )
+
     def test_auto_apply_on_approve_runs_for_non_manager_approver(self):
         """Auto-apply-on-approve must still work when the approver is a
-        validator (not a manager): _on_approve routes through the internal
-        apply mechanism, which is not gated."""
+        validator (not a manager): ``_on_approve`` reaches the public
+        ``action_apply`` through ``sudo()``, and the manager gate exempts
+        superuser callers."""
         cr = self._make_approved_cr()
         cr.request_type_id.auto_apply_on_approve = True
         cr.with_user(self.cr_validator)._on_approve()