Skip to content

fix(spp_cr_type_assign_program): scope duplicate-assignment conflict to (registrant, program) #208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gonzalesedwin1123 merged 1 commit into from
May 22, 2026
Merged

fix(spp_cr_type_assign_program): scope duplicate-assignment conflict to (registrant, program) #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion spp_cr_type_assign_program/__manifest__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "OpenSPP CR Type - Assign to Program",
"version": "19.0.1.0.0",
"version": "19.0.1.0.1",
"sequence": 53,
"category": "OpenSPP",
"summary": "Change request type for assigning a registrant to a program",
Expand Down
13 changes: 8 additions & 5 deletions spp_cr_type_assign_program/models/change_request.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
class SPPChangeRequest(models.Model):
"""Custom conflict-detection hook for the assign-program CR type.

The base conflict rule scoped to the same registrant flags every in-flight
`assign_program` CR for that registrant. We narrow the match to those
targeting the same `(registrant, program)` pair so two CRs assigning the
same registrant to *different* programs are allowed to proceed.
The rule uses `scope='custom'`, so the base domain does not pre-filter by
registrant. We narrow the candidate set to those targeting the same
`(registrant, program)` pair: two CRs assigning the same registrant to
*different* programs may proceed, and two CRs assigning *different*
registrants to the same program may also proceed.
"""

_inherit = "spp.change.request"
Expand Down Expand Up @@ -44,4 +45,6 @@ def _check_custom_conflicts(self, candidates, rule):
)
.ids
)
return candidates.filtered(lambda c: c.detail_res_id in matching_detail_ids)
return candidates.filtered(
lambda c: c.detail_res_id in matching_detail_ids and c.registrant_id == self.registrant_id
)
Comment on lines +48 to +50
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Consider swapping the conditions in the filtered lambda to leverage short-circuiting. Since candidates may contain many records for different registrants, checking the registrant_id first is more efficient as it avoids the set lookup for non-matching registrants. Additionally, per repository guidelines, defensively filter out any records where the detail record (detail_res_id) is missing to handle edge cases like newly created records.

Suggested change
return candidates.filtered(
lambda c: c.detail_res_id in matching_detail_ids and c.registrant_id == self.registrant_id
)
return candidates.filtered(
lambda c: c.detail_res_id and c.registrant_id == self.registrant_id and c.detail_res_id in matching_detail_ids
)
References
  1. When processing records that should have related detail records, defensively filter out any records where the detail record is missing to handle edge cases like newly created records.

23 changes: 23 additions & 0 deletions spp_cr_type_assign_program/tests/test_assign_program.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,29 @@ def test_f3_two_crs_for_same_registrant_different_programs_allowed(self):
self.assertEqual(cr2.conflict_status, "none")
self.assertFalse(cr2.conflicting_cr_ids)

def test_f5_two_crs_for_different_registrants_same_program_allowed(self):
# Two different registrants assigning to the *same* program must both
# be able to proceed. The rule's intent is "same (registrant, program)
# pair" — different registrants are not in conflict even when the
# program matches.
other_individual = self.Partner.create(
{
"name": "Other Individual",
"given_name": "Other",
"family_name": "Individual",
"is_registrant": True,
"is_group": False,
}
)

_cr1, _d1 = self._make_cr(self.test_individual, self.indiv_program_active)
cr2, _d2 = self._make_cr(other_individual, self.indiv_program_active)

cr2._run_conflict_checks()

self.assertEqual(cr2.conflict_status, "none")
self.assertFalse(cr2.conflicting_cr_ids)

def test_a10_apply_translates_unique_violation_to_user_error(self):
"""Race-path: a concurrent transaction inserts the same
(registrant, program) pair between our validate() and create().
Expand Down
Loading