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

[kneckinator]

Fixes #476.

Problem

ConflictDetectionMixin._get_group_member_ids traversed spp.group.membership records through individual_id and group_id, but the model names its many2ones individual and group (spp_registry/models/group_membership.py). Resolving a household's members therefore crashed instead of returning them:

• Group registrant: KeyError: 'individual_id' unconditionally — even for an empty group, since mapped() resolves the field name before iterating.
• Individual registrant: AttributeError on the first non-ended membership — exactly when a group-scope rule has work to do.

Any change request type with an active scope = "group" conflict rule crashed on CR creation for these registrants, making group-scope rules unusable.

Why tests didn't catch it

The one existing test called the method directly on an individual with zero memberships — the single shape that happens to work, since the loop body and mapped() calls never execute.

Fix

• Rename the three accesses to the real field names (individual, group) — no behavior change beyond un-breaking the traversal.
• test_group_scope_same_household_members now creates real memberships and asserts both the resolved member set and the end-to-end warning status on the second CR in the household.
• New test_group_scope_group_registrant covers the group-registrant branch.
• New test_group_scope_ended_membership_excluded pins that ended memberships are excluded from conflict candidates.

TDD: the three tests were written first and reproduced both exceptions (KeyError: 'individual_id', AttributeError: ... no attribute 'group_id') before the fix.

Verification

• ./spp t spp_change_request_v2: 0 failed, 0 error(s) of 415 tests (red run before the fix: the 3 new tests errored, 412 passed).
• pre-commit clean && pre-commit run --all-files: clean for this module (semgrep crashes locally on Python 3.14 — CI is the authoritative run; README regeneration of unrelated drifted modules was reverted).

Found while reviewing #418 (pre-existing on 19.0, out of scope there).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.27%. Comparing base (0341801) to head (c2a3817).

Additional details and impacted files

@@           Coverage Diff           @@
##             19.0     #477   +/-   ##
=======================================
  Coverage   76.26%   76.27%           
=======================================
  Files         662      662           
  Lines       44225    44218    -7     
=======================================
- Hits        33729    33727    -2     
+ Misses      10496    10491    -5     

Flag	Coverage Δ
spp_api_v2_change_request	73.37% <ø> (ø)
spp_base_common	91.07% <ø> (ø)
spp_change_request_v2	78.76% <100.00%> (+0.12%)	⬆️
spp_cr_type_assign_program	92.50% <ø> (ø)
spp_dci_demo	94.28% <ø> (ø)
spp_farmer_registry_cr	61.24% <ø> (ø)
spp_farmer_registry_demo	63.39% <ø> (ø)
spp_mis_demo_v2	70.38% <ø> (ø)
spp_programs	67.58% <ø> (ø)
spp_registry	87.79% <ø> (ø)
spp_security	69.56% <ø> (ø)
spp_starter_sp_mis	86.84% <ø> (-2.05%)	⬇️
spp_studio_change_requests	84.85% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
spp_change_request_v2/models/conflict_mixin.py	92.46% <100.00%> (+1.71%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gonzalesedwin1123]

Expert review

Verdict: approve with follow-ups. The fix is correct, minimal, and exactly what #476 describes: spp.group.membership really does name its many2ones individual and group (spp_registry/models/group_membership.py:16,22 — whitelisted o2m inverses in scripts/lint/check_naming.py), and this mixin was the only place in the repo traversing membership records with the _id-suffixed names (repo-wide grep; every other individual_id hit is a detail-line model's own field). Version chain is right: 19.0 is at 3.1.13, so .14 is the correct next.

Independently verified

• Green run: ./spp t spp_change_request_v2 → 0 failed, 0 errors of 415 tests (verified in the unittest log, not the exit code).
• Red check (TDD claim): with only conflict_mixin.py reverted to origin/19.0, exactly the 3 group-scope tests error with exactly the claimed exceptions — KeyError: 'individual_id' (group branch) and AttributeError: 'spp.group.membership' object has no attribute 'group_id' (individual branch) — the other 412 tests unaffected.
• Test integrity: the pre-existing test_group_scope_same_household_members was strengthened, not weakened — its old assertion is implied by the new ones, plus the new end-to-end conflict_status == "warning" check. Nothing removed.
• Lint: clean on the changed files; only the known oca-gen README width quirk (unrelated modules) and two pre-existing pylint W8113s outside the diff.
• Generated files: README.rst/index.html diff is the new changelog section plus pure section-N renumbering; no unrelated modules dragged in.

Follow-ups worth filing (neither blocks this PR)

1. not m.ended_date disagrees with the model's own "ended" semantics. is_ended/status treat a membership as ended only when ended_date <= now, but the mixin's filter drops any membership with ended_date set — including a future-scheduled exit, which this very module produces (strategies/exit_registrant.py writes user-supplied detail.exit_date). A member with a scheduled future exit is still a live member today but silently vanishes from conflict candidates — a false negative in a control that can be configured action="block". Note that simply switching to is_ended isn't right either: it's a stored compute depending only on ended_date but compared against now() at write time, so a future-dated exit stays is_ended=False forever after the date passes (nothing recomputes it — that staleness affects every repo consumer of is_ended). The correct predicate is not (m.ended_date and m.ended_date <= fields.Datetime.now()); the stale stored compute in spp_registry deserves its own issue.
2. Group-scope detection fails open for the requester role. _detect_conflicts searches as the current user, and rule_cr_user (security/rules.xml) restricts a group_cr_user to their own CRs — so a co-member's CR created by someone else is invisible and a group-scope rule (even block) never fires for requesters; it only works for validators/managers. Pre-existing, but this PR makes the path executable for the first time. Not a simple sudo() fix: conflicting_cr_ids is displayed on the CR form, so populating it with unreadable records trades fail-open for an AccessError/display_name leak.

Suggestions (take or leave)

• tests/test_conflict_detection_extended.py: the ended-membership test exercises only the individual branch (nested filter, L292); the group-registrant branch's filter (L282) never runs in an excluding state. One line in test_group_scope_group_registrant — end a membership, assert absence — closes it.
• test_group_scope_group_registrant pairs the shared target_type="individual" CR type with a group registrant — a combination the UI blocks (onchange-only guard, so create() accepts it). A target_type="group"/"both" type would make the test a reachable configuration.
• The hasattr() guards (L280/287) are dead code — spp_registry is a hard dep, so the fields always exist — and they read as defensive while the actual failure mode was a hard crash one line inside them. Same pattern in models/change_request.py, models/res_partner.py, wizards/create_wizard.py; worth a cleanup sweep.
• list(set(member_ids)) → sorted(set(...)) for reproducible domains (cosmetic, pre-existing).
• The docstring is an explicit override point but doesn't document the branch asymmetry: an individual expands to group + co-members, a group only to itself + its members (not to members' other groups). One sentence would help overriders.

[kneckinator]

Thanks for the review @gonzalesedwin1123 — the follow-ups and suggestions are now filed against origin/19.0 @ 23f43963:

• spp_change_request_v2: group-scope conflict candidates drop members with a future-dated exit (not m.ended_date is not "ended") #493 — not m.ended_date disagrees with the model's own "ended" semantics: a future-dated exit (which exit_registrant produces from user input) silently drops a live member from group-scope candidates, fail-open for action = "block". Includes the correct predicate, the note on why is_ended/status are not the fix, the missing test for the group-registrant filter, sorted(set(...)), and the docstring asymmetry.
• security(spp_change_request_v2): group-scope conflict rules never fire for CR requesters — record rule hides co-members' CRs (fail-open) #494 — group-scope detection fails open for the requester role: rule_cr_user hides co-members' CRs, so the rule only ever fires for validators/managers, and the _on_submit blocking guard is inert for requesters. Filed with the reason a plain sudo() is not the fix.
• spp_change_request_v2: CR type target_type is enforced only by an onchange — create()/API accept a mismatched registrant #495 — target_type is enforced only by an onchange and the registrant domain; create(), import, and spp_api_v2_change_request.create_change_request all accept a mismatched registrant. Covers the fixture in test_group_scope_group_registrant too.
• chore(spp_change_request_v2): drop dead hasattr() guards on hard-dependency fields (one of them is always False) #496 — the dead hasattr() cleanup sweep. Worth a look: hasattr(reg, "spp_id") (change_request.py:585) is not merely dead but always False — no module in this repo defines spp_id on res.partner, so the ID badge never renders and test_registrant_summary_escapes_spp_id wraps all of its assertions in that guard, i.e. it passes trivially. Two related loose ends noted there (spp_api_v2/routers/consent.py:236,238 and the review-panel template).

The stale stored-compute half of follow-up 1 was already filed as #417 (with #421 / #420 for the start_date and active gaps in the same model), so #493 references those rather than duplicating them.