fix(change_request): make group-scope conflict rules resolve household members - #477
Open
kneckinator wants to merge 2 commits into
Open
fix(change_request): make group-scope conflict rules resolve household members#477kneckinator wants to merge 2 commits into
kneckinator wants to merge 2 commits into
Conversation
…d members _get_group_member_ids traversed spp.group.membership through individual_id and group_id, but the model names its many2ones individual and group. Resolving a household's members therefore 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 resolution is now tested with real memberships in both directions, including that ended memberships are excluded. Fixes #476
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
_create_household already creates both membership rows, so the ended membership test can set ended_date through the individual's own membership instead of querying the pair back by (group, individual). That drops a query and, more usefully, a second place spelling the membership field names — the very drift this branch repairs. Also name the change request under test `cr`, matching the sibling tests; the `cr1` name dated from a `cr2` that no longer exists. Restore the README table padding and column widths emitted by CI's generator: regenerating locally on Python 3.14 re-renders an unrelated docutils table one column wider than CI's Python 3.11, which failed the pre-commit check. Only the changelog entry this branch owns remains.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #476.
Problem
ConflictDetectionMixin._get_group_member_idstraversedspp.group.membershiprecords throughindividual_idandgroup_id, but the model names its many2onesindividualandgroup(spp_registry/models/group_membership.py). Resolving a household's members therefore crashed instead of returning them:KeyError: 'individual_id'unconditionally — even for an empty group, sincemapped()resolves the field name before iterating.AttributeErroron 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
individual,group) — no behavior change beyond un-breaking the traversal.test_group_scope_same_household_membersnow creates real memberships and asserts both the resolved member set and the end-to-endwarningstatus on the second CR in the household.test_group_scope_group_registrantcovers the group-registrant branch.test_group_scope_ended_membership_excludedpins 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).