fix(change_request): make group-scope conflict rules resolve household members

spp_change_request_v2/README.rst

@@ -853,6 +853,22 @@ Before declaring a new CR type complete:
 Changelog
 =========
 
+19.0.3.1.14
+~~~~~~~~~~~
+
+- fix(change_request): group-scope conflict rules work again.
+  ``_get_group_member_ids`` traversed ``spp.group.membership`` records
+  through ``individual_id`` and ``group_id``, but that model names its
+  many2ones ``individual`` and ``group`` — so resolving a household's
+  members raised ``KeyError``/``AttributeError`` instead of returning
+  them. A change request whose type carried an active group-scope
+  conflict rule crashed on creation for any group registrant, and for
+  any individual registrant with a live membership — exactly the
+  registrants the rule exists to check. The one existing test called the
+  method with a member-less individual, the single shape that happened
+  to work; group-scope detection is now tested with real memberships in
+  both directions, including that ended memberships are excluded.
+
 19.0.3.1.13
 ~~~~~~~~~~~
 

spp_change_request_v2/__manifest__.py

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

spp_change_request_v2/models/conflict_mixin.py

@@ -279,17 +279,17 @@ def _get_group_member_ids(self):
         if registrant.is_group:
             if hasattr(registrant, "group_membership_ids"):
                 member_ids.extend(
-                    registrant.group_membership_ids.filtered(lambda m: not m.ended_date).mapped("individual_id.id")
+                    registrant.group_membership_ids.filtered(lambda m: not m.ended_date).mapped("individual.id")
                 )
 
         # If registrant is an individual, find their groups and group members
         else:
             if hasattr(registrant, "individual_membership_ids"):
                 for membership in registrant.individual_membership_ids.filtered(lambda m: not m.ended_date):
-                    group = membership.group_id
+                    group = membership.group
                     member_ids.append(group.id)
                     member_ids.extend(
-                        group.group_membership_ids.filtered(lambda m: not m.ended_date).mapped("individual_id.id")
+                        group.group_membership_ids.filtered(lambda m: not m.ended_date).mapped("individual.id")
                     )
 
         return list(set(member_ids))

spp_change_request_v2/readme/HISTORY.md

@@ -1,3 +1,7 @@
+### 19.0.3.1.14
+
+- fix(change_request): group-scope conflict rules work again. `_get_group_member_ids` traversed `spp.group.membership` records through `individual_id` and `group_id`, but that model names its many2ones `individual` and `group` — so resolving a household's members raised `KeyError`/`AttributeError` instead of returning them. A change request whose type carried an active group-scope conflict rule crashed on creation for any group registrant, and for any individual registrant with a live membership — exactly the registrants the rule exists to check. The one existing test called the method with a member-less individual, the single shape that happened to work; group-scope detection is now tested with real memberships in both directions, including that ended memberships are excluded.
+
 ### 19.0.3.1.13
 
 - fix(change_request): a selectable field on a dynamic-approval type may now be applied through more than one mapping. Apply is narrowed to the field a request was routed and approved on, matched against the mapping's `source_field` — which assumed every selectable value is a physical source field. They need not be: a name may be offered as one choice but stored as separate components, so one selectable value legitimately drives several mappings, and matching on `source_field` alone matched none of them, applying nothing at all. A mapping can now declare the selectable value it serves via `routing_field`, defaulting to `source_field`, so existing configurations are unchanged. Narrowing still holds — a mapping belonging to another routing key is still not applied.

spp_change_request_v2/static/description/index.html

@@ -1339,6 +1339,23 @@ <h2>Changelog</h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.3.1.14</h1>
+<ul class="simple">
+<li>fix(change_request): group-scope conflict rules work again.
+<tt class="docutils literal">_get_group_member_ids</tt> traversed <tt class="docutils literal">spp.group.membership</tt> records
+through <tt class="docutils literal">individual_id</tt> and <tt class="docutils literal">group_id</tt>, but that model names its
+many2ones <tt class="docutils literal">individual</tt> and <tt class="docutils literal">group</tt> — so resolving a household’s
+members raised <tt class="docutils literal">KeyError</tt>/<tt class="docutils literal">AttributeError</tt> instead of returning
+them. A change request whose type carried an active group-scope
+conflict rule crashed on creation for any group registrant, and for
+any individual registrant with a live membership — exactly the
+registrants the rule exists to check. The one existing test called the
+method with a member-less individual, the single shape that happened
+to work; group-scope detection is now tested with real memberships in
+both directions, including that ended memberships are excluded.</li>
+</ul>
+</div>
+<div class="section" id="section-2">
 <h1>19.0.3.1.13</h1>
 <ul class="simple">
 <li>fix(change_request): a selectable field on a dynamic-approval type may
@@ -1355,7 +1372,7 @@ <h1>19.0.3.1.13</h1>
 routing key is still not applied.</li>
 </ul>
 </div>
-<div class="section" id="section-2">
+<div class="section" id="section-3">
 <h1>19.0.3.1.12</h1>
 <ul class="simple">
 <li>fix(change_request): auto-apply-on-approve runs through the public
@@ -1370,7 +1387,7 @@ <h1>19.0.3.1.12</h1>
 the applying user is still recorded as the approver.</li>
 </ul>
 </div>
-<div class="section" id="section-3">
+<div class="section" id="section-4">
 <h1>19.0.3.1.11</h1>
 <ul class="simple">
 <li>fix(change_request): field-mapping transform expressions are evaluated
@@ -1409,7 +1426,7 @@ <h1>19.0.3.1.11</h1>
 the full traceback is logged only at DEBUG.</li>
 </ul>
 </div>
-<div class="section" id="section-4">
+<div class="section" id="section-5">
 <h1>19.0.3.1.10</h1>
 <ul class="simple">
 <li>fix(security): conflict and duplicate detection now decide whether a
@@ -1449,7 +1466,7 @@ <h1>19.0.3.1.10</h1>
 configured mapping.</li>
 </ul>
 </div>
-<div class="section" id="section-5">
+<div class="section" id="section-6">
 <h1>19.0.3.1.9</h1>
 <ul class="simple">
 <li>fix(security): duplicate detection now scores the fields both change
@@ -1466,7 +1483,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-6">
+<div class="section" id="section-7">
 <h1>19.0.3.1.8</h1>
 <ul class="simple">
 <li>fix(security): scope the Create-Group member wizards to the parent
@@ -1484,7 +1501,7 @@ <h1>19.0.3.1.8</h1>
 access-control entry grants.</li>
 </ul>
 </div>
-<div class="section" id="section-7">
+<div class="section" id="section-8">
 <h1>19.0.3.1.7</h1>
 <ul class="simple">
 <li>fix(security): require change-request manager rights to apply a change
@@ -1499,7 +1516,7 @@ <h1>19.0.3.1.7</h1>
 endpoint.</strong></li>
 </ul>
 </div>
-<div class="section" id="section-8">
+<div class="section" id="section-9">
 <h1>19.0.3.1.6</h1>
 <ul class="simple">
 <li>fix(security): derive conflict and duplicate detection from the change
@@ -1513,7 +1530,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-9">
+<div class="section" id="section-10">
 <h1>19.0.3.1.5</h1>
 <ul class="simple">
 <li>fix(security): scope the CR Requestor, Local Validator and HQ
@@ -1525,7 +1542,7 @@ <h1>19.0.3.1.5</h1>
 are <tt class="docutils literal">noupdate</tt>.</li>
 </ul>
 </div>
-<div class="section" id="section-10">
+<div class="section" id="section-11">
 <h1>19.0.3.1.4</h1>
 <ul class="simple">
 <li>fix(security): add ownership and area record rules to every concrete
@@ -1542,7 +1559,7 @@ <h1>19.0.3.1.4</h1>
 unrestricted delete their access-control entries grant.</li>
 </ul>
 </div>
-<div class="section" id="section-11">
+<div class="section" id="section-12">
 <h1>19.0.3.1.3</h1>
 <ul class="simple">
 <li>fix(security): route and apply the same single field for
@@ -1555,7 +1572,7 @@ <h1>19.0.3.1.3</h1>
 the routing selector.</li>
 </ul>
 </div>
-<div class="section" id="section-12">
+<div class="section" id="section-13">
 <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
@@ -1564,7 +1581,7 @@ <h1>19.0.3.1.2</h1>
 (#1136)</li>
 </ul>
 </div>
-<div class="section" id="section-13">
+<div class="section" id="section-14">
 <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
@@ -1578,7 +1595,7 @@ <h1>19.0.3.1.1</h1>
 applied) so the constraint applies cleanly on upgrade.</li>
 </ul>
 </div>
-<div class="section" id="section-14">
+<div class="section" id="section-15">
 <h1>19.0.3.1.0</h1>
 <ul class="simple">
 <li>revert(change_request): restore the create-a-new-individual <strong>Add
@@ -1596,7 +1613,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-15">
+<div class="section" id="section-16">
 <h1>19.0.3.0.0</h1>
 <ul class="simple">
 <li>feat(change_request): redesign the group/membership CR flows (#242) —
@@ -1618,7 +1635,7 @@ <h1>19.0.3.0.0</h1>
 must adapt (see #1133).</li>
 </ul>
 </div>
-<div class="section" id="section-16">
+<div class="section" id="section-17">
 <h1>19.0.2.0.8</h1>
 <ul class="simple">
 <li>fix(views): disable inline creation of CR document types on the Change
@@ -1629,7 +1646,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-17">
+<div class="section" id="section-18">
 <h1>19.0.2.0.7</h1>
 <ul class="simple">
 <li>fix(security): align CR Requestor / CR Local Validator / CR HQ
@@ -1641,7 +1658,7 @@ <h1>19.0.2.0.7</h1>
 dependencies.</li>
 </ul>
 </div>
-<div class="section" id="section-18">
+<div class="section" id="section-19">
 <h1>19.0.2.0.6</h1>
 <ul class="simple">
 <li>fix(views): route post-submit CRs (pending / approved / applied /
@@ -1656,7 +1673,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-19">
+<div class="section" id="section-20">
 <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
@@ -1669,27 +1686,27 @@ <h1>19.0.2.0.5</h1>
 roles).</li>
 </ul>
 </div>
-<div class="section" id="section-20">
+<div class="section" id="section-21">
 <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-21">
+<div class="section" id="section-22">
 <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-22">
+<div class="section" id="section-23">
 <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-23">
+<div class="section" id="section-24">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>

spp_change_request_v2/tests/test_conflict_detection_extended.py

@@ -10,7 +10,7 @@
 - Spec scenarios
 """
 
-from odoo import Command
+from odoo import Command, fields
 from odoo.exceptions import UserError, ValidationError
 from odoo.tests import TransactionCase
 
@@ -493,48 +493,97 @@ def setUpClass(cls):
             }
         )
 
-    def test_group_scope_same_household_members(self):
-        """Test group scope detects conflicts for household members."""
-        # Create household with members
-        self.env["res.partner"].create(
+    def _create_household(self):
+        """Create a household with two member individuals and return all three."""
+        group = self.env["res.partner"].create(
             {
                 "name": "Test Household",
                 "is_registrant": True,
                 "is_group": True,
             }
         )
-
         individual1 = self.env["res.partner"].create(
             {
                 "name": "Member 1",
                 "is_registrant": True,
                 "is_group": False,
             }
         )
-
-        self.env["res.partner"].create(
+        individual2 = self.env["res.partner"].create(
             {
                 "name": "Member 2",
                 "is_registrant": True,
                 "is_group": False,
             }
         )
+        self.env["spp.group.membership"].create(
+            [
+                {"group": group.id, "individual": individual1.id},
+                {"group": group.id, "individual": individual2.id},
+            ]
+        )
+        return group, individual1, individual2
 
-        # Add members to household (if membership model exists)
-        # This is implementation-dependent
-        # For now, test the _get_group_member_ids method directly
+    def test_group_scope_same_household_members(self):
+        """Test group scope detects conflicts for household members."""
+        group, individual1, individual2 = self._create_household()
 
-        # Create CR for individual1
-        cr1 = self.env["spp.change.request"].create(
+        # Create CR for individual2 first, then for individual1: the group-scope
+        # rule must flag the second CR because both registrants share a household.
+        self.env["spp.change.request"].create(
+            {
+                "request_type_id": self.cr_type.id,
+                "registrant_id": individual2.id,
+            }
+        )
+        cr = self.env["spp.change.request"].create(
+            {
+                "request_type_id": self.cr_type.id,
+                "registrant_id": individual1.id,
+            }
+        )
+
+        # _get_group_member_ids resolves the household and every co-member
+        member_ids = cr._get_group_member_ids()
+        self.assertIn(individual1.id, member_ids)
+        self.assertIn(group.id, member_ids)
+        self.assertIn(individual2.id, member_ids)
+
+        # The rule's action is "warn", so the second CR is flagged, not blocked
+        self.assertEqual(cr.conflict_status, "warning")
+
+    def test_group_scope_group_registrant(self):
+        """A CR whose registrant is the group itself resolves its members."""
+        group, individual1, individual2 = self._create_household()
+
+        cr = self.env["spp.change.request"].create(
+            {
+                "request_type_id": self.cr_type.id,
+                "registrant_id": group.id,
+            }
+        )
+
+        member_ids = cr._get_group_member_ids()
+        self.assertIn(group.id, member_ids)
+        self.assertIn(individual1.id, member_ids)
+        self.assertIn(individual2.id, member_ids)
+
+    def test_group_scope_ended_membership_excluded(self):
+        """Members whose membership has ended are not conflict candidates."""
+        group, individual1, individual2 = self._create_household()
+        individual2.individual_membership_ids.ended_date = fields.Datetime.now()
+
+        cr = self.env["spp.change.request"].create(
             {
                 "request_type_id": self.cr_type.id,
                 "registrant_id": individual1.id,
             }
         )
 
-        # Test _get_group_member_ids returns expected members
-        member_ids = cr1._get_group_member_ids()
+        member_ids = cr._get_group_member_ids()
         self.assertIn(individual1.id, member_ids)
+        self.assertIn(group.id, member_ids)
+        self.assertNotIn(individual2.id, member_ids)
 
 
 class TestDuplicateDetectionAdvanced(TransactionCase):