From 9137877843ac1a0d40c135fd76a01f2a6c19f37a Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 17:53:41 +0800 Subject: [PATCH 01/12] feat(spp_cr_type_assign_program): scaffold module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new module declaring its manifest, dependencies, and readme fragments. Module installs cleanly but defines no behaviour yet — detail model, apply strategy, view, CR-type record, and conflict rule land in subsequent commits. --- spp_cr_type_assign_program/__init__.py | 0 spp_cr_type_assign_program/__manifest__.py | 20 ++++++++ spp_cr_type_assign_program/pyproject.toml | 3 ++ .../readme/DESCRIPTION.md | 46 +++++++++++++++++++ spp_cr_type_assign_program/readme/HISTORY.md | 12 +++++ spp_cr_type_assign_program/tests/__init__.py | 1 + .../tests/test_assign_program.py | 5 ++ 7 files changed, 87 insertions(+) create mode 100644 spp_cr_type_assign_program/__init__.py create mode 100644 spp_cr_type_assign_program/__manifest__.py create mode 100644 spp_cr_type_assign_program/pyproject.toml create mode 100644 spp_cr_type_assign_program/readme/DESCRIPTION.md create mode 100644 spp_cr_type_assign_program/readme/HISTORY.md create mode 100644 spp_cr_type_assign_program/tests/__init__.py create mode 100644 spp_cr_type_assign_program/tests/test_assign_program.py diff --git a/spp_cr_type_assign_program/__init__.py b/spp_cr_type_assign_program/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/spp_cr_type_assign_program/__manifest__.py b/spp_cr_type_assign_program/__manifest__.py new file mode 100644 index 00000000..3cd38d52 --- /dev/null +++ b/spp_cr_type_assign_program/__manifest__.py @@ -0,0 +1,20 @@ +{ + "name": "OpenSPP CR Type - Assign to Program", + "version": "19.0.1.0.0", + "sequence": 53, + "category": "OpenSPP", + "summary": "Change request type for assigning a registrant to a program", + "author": "OpenSPP.org", + "website": "https://github.com/OpenSPP/OpenSPP2", + "license": "LGPL-3", + "development_status": "Beta", + "depends": [ + "spp_change_request_v2", + "spp_programs", + ], + "data": [], + "installable": True, + "application": False, + "auto_install": False, + "maintainers": ["jeremi", "gonzalesedwin1123"], +} diff --git a/spp_cr_type_assign_program/pyproject.toml b/spp_cr_type_assign_program/pyproject.toml new file mode 100644 index 00000000..4231d0cc --- /dev/null +++ b/spp_cr_type_assign_program/pyproject.toml @@ -0,0 +1,3 @@ +[build-system] +requires = ["whool"] +build-backend = "whool.buildapi" diff --git a/spp_cr_type_assign_program/readme/DESCRIPTION.md b/spp_cr_type_assign_program/readme/DESCRIPTION.md new file mode 100644 index 00000000..ee64a8ec --- /dev/null +++ b/spp_cr_type_assign_program/readme/DESCRIPTION.md @@ -0,0 +1,46 @@ +Adds a single change request type — `assign_program` — that records a registrant +being assigned to a program. The change request runs through the standard +approval, conflict-detection, and document workflow provided by +`spp_change_request_v2`. On apply, an `spp.program.membership` record is +created in the `draft` state for the `(registrant, program)` pair. + +### Beneficiary semantics + +The CR's registrant **is** the program beneficiary. There is no "select a member +of the household" step. + +- Registrant is a group (household) → eligible programs are those with + `target_type='group'` and `state='active'`. The household itself is enrolled. +- Registrant is an individual → eligible programs are those with + `target_type='individual'` and `state='active'`. The individual is enrolled. + +Standalone individuals (registrants not in any household) are supported. + +### Models defined by this module + +| Model | Kind | Purpose | +| ----- | ---- | ------- | +| `spp.cr.detail.assign_program` | Model | Captures the program selection for the CR | +| `spp.cr.apply.assign_program` | AbstractModel | Apply strategy that creates the membership | + +### Validation rules (apply-time) + +The apply strategy refuses the operation when any of the following hold: + +- the registrant is `disabled` +- the program is not in `state='active'` +- the program's `target_type` does not match the registrant +- a membership for the same `(registrant, program)` pair already exists +- the detail record has no `program_id` set + +### Conflict detection + +Two in-flight `assign_program` change requests targeting the same +`(registrant, program)` pair are treated as conflicting and the second +submission is blocked. Two CRs for the same registrant but different programs +are independent and both proceed. + +### Dependencies + +- `spp_change_request_v2` +- `spp_programs` diff --git a/spp_cr_type_assign_program/readme/HISTORY.md b/spp_cr_type_assign_program/readme/HISTORY.md new file mode 100644 index 00000000..74862b00 --- /dev/null +++ b/spp_cr_type_assign_program/readme/HISTORY.md @@ -0,0 +1,12 @@ +## 19.0.1.0.0 (2026-05-04) + +### Added + +- New module `spp_cr_type_assign_program` with the `assign_program` change + request type. +- Detail model `spp.cr.detail.assign_program` with live program-domain + filtering based on the registrant's target type. +- Apply strategy `spp.cr.apply.assign_program` that creates a draft + `spp.program.membership` record on apply. +- Conflict rule that blocks duplicate in-flight assignments to the same + `(registrant, program)` pair. diff --git a/spp_cr_type_assign_program/tests/__init__.py b/spp_cr_type_assign_program/tests/__init__.py new file mode 100644 index 00000000..27098114 --- /dev/null +++ b/spp_cr_type_assign_program/tests/__init__.py @@ -0,0 +1 @@ +from . import test_assign_program diff --git a/spp_cr_type_assign_program/tests/test_assign_program.py b/spp_cr_type_assign_program/tests/test_assign_program.py new file mode 100644 index 00000000..710c0c79 --- /dev/null +++ b/spp_cr_type_assign_program/tests/test_assign_program.py @@ -0,0 +1,5 @@ +"""Tests for spp_cr_type_assign_program. + +Populated incrementally through the implementation steps; this stub keeps +the tests/ package importable while the skeleton lands. +""" From 24db991e3f6dba93ad04ccd495ca95d686d01c57 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 17:54:12 +0800 Subject: [PATCH 02/12] feat(spp_cr_type_assign_program): add detail model with computed program domain Define `spp.cr.detail.assign_program` with `program_id`, a computed `allowed_program_ids` filtered to active programs whose `target_type` matches the registrant, and `created_membership_id` for the apply-time audit back-reference. ACL rows match the cr_user / cr_validator(_hq) / cr_manager pattern used by other detail models. Tests cover both target-type branches and the active/inactive program filter. --- spp_cr_type_assign_program/__init__.py | 1 + spp_cr_type_assign_program/__manifest__.py | 4 +- .../details/__init__.py | 1 + .../details/assign_program.py | 54 +++++++++++ .../security/ir.model.access.csv | 5 + .../tests/test_assign_program.py | 97 ++++++++++++++++++- 6 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 spp_cr_type_assign_program/details/__init__.py create mode 100644 spp_cr_type_assign_program/details/assign_program.py create mode 100644 spp_cr_type_assign_program/security/ir.model.access.csv diff --git a/spp_cr_type_assign_program/__init__.py b/spp_cr_type_assign_program/__init__.py index e69de29b..a7ea0c4c 100644 --- a/spp_cr_type_assign_program/__init__.py +++ b/spp_cr_type_assign_program/__init__.py @@ -0,0 +1 @@ +from . import details diff --git a/spp_cr_type_assign_program/__manifest__.py b/spp_cr_type_assign_program/__manifest__.py index 3cd38d52..f1b8d7f6 100644 --- a/spp_cr_type_assign_program/__manifest__.py +++ b/spp_cr_type_assign_program/__manifest__.py @@ -12,7 +12,9 @@ "spp_change_request_v2", "spp_programs", ], - "data": [], + "data": [ + "security/ir.model.access.csv", + ], "installable": True, "application": False, "auto_install": False, diff --git a/spp_cr_type_assign_program/details/__init__.py b/spp_cr_type_assign_program/details/__init__.py new file mode 100644 index 00000000..d678cbfd --- /dev/null +++ b/spp_cr_type_assign_program/details/__init__.py @@ -0,0 +1 @@ +from . import assign_program diff --git a/spp_cr_type_assign_program/details/assign_program.py b/spp_cr_type_assign_program/details/assign_program.py new file mode 100644 index 00000000..f4d5bd0a --- /dev/null +++ b/spp_cr_type_assign_program/details/assign_program.py @@ -0,0 +1,54 @@ +from odoo import api, fields, models + + +class SPPCRDetailAssignProgram(models.Model): + """Detail model for the assign-to-program CR type.""" + + _name = "spp.cr.detail.assign_program" + _description = "CR Detail: Assign to Program" + _inherit = ["spp.cr.detail.base", "mail.thread"] + + program_id = fields.Many2one( + "spp.program", + string="Program", + tracking=True, + domain="[('id', 'in', allowed_program_ids)]", + help="Program the registrant will be enrolled in.", + ) + allowed_program_ids = fields.Many2many( + "spp.program", + string="Allowed Programs", + compute="_compute_allowed_program_ids", + ) + registrant_target_type = fields.Selection( + [("group", "Group"), ("individual", "Individual")], + compute="_compute_registrant_target_type", + store=True, + ) + created_membership_id = fields.Many2one( + "spp.program.membership", + string="Created Membership", + readonly=True, + ) + + @api.depends("registrant_id", "registrant_id.is_group") + def _compute_registrant_target_type(self): + for rec in self: + if not rec.registrant_id: + rec.registrant_target_type = False + continue + rec.registrant_target_type = "group" if rec.registrant_id.is_group else "individual" + + @api.depends("registrant_target_type") + def _compute_allowed_program_ids(self): + Program = self.env["spp.program"] + for rec in self: + if not rec.registrant_target_type: + rec.allowed_program_ids = False + continue + rec.allowed_program_ids = Program.search( + [ + ("state", "=", "active"), + ("target_type", "=", rec.registrant_target_type), + ] + ) diff --git a/spp_cr_type_assign_program/security/ir.model.access.csv b/spp_cr_type_assign_program/security/ir.model.access.csv new file mode 100644 index 00000000..27d72795 --- /dev/null +++ b/spp_cr_type_assign_program/security/ir.model.access.csv @@ -0,0 +1,5 @@ +id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink +access_spp_cr_detail_assign_program_user,spp.cr.detail.assign_program user,model_spp_cr_detail_assign_program,spp_change_request_v2.group_cr_user,1,1,1,0 +access_spp_cr_detail_assign_program_validator,spp.cr.detail.assign_program validator,model_spp_cr_detail_assign_program,spp_change_request_v2.group_cr_validator,1,1,1,0 +access_spp_cr_detail_assign_program_validator_hq,spp.cr.detail.assign_program validator hq,model_spp_cr_detail_assign_program,spp_change_request_v2.group_cr_validator_hq,1,1,1,0 +access_spp_cr_detail_assign_program_manager,spp.cr.detail.assign_program manager,model_spp_cr_detail_assign_program,spp_change_request_v2.group_cr_manager,1,1,1,1 diff --git a/spp_cr_type_assign_program/tests/test_assign_program.py b/spp_cr_type_assign_program/tests/test_assign_program.py index 710c0c79..71750a8d 100644 --- a/spp_cr_type_assign_program/tests/test_assign_program.py +++ b/spp_cr_type_assign_program/tests/test_assign_program.py @@ -1,5 +1,94 @@ -"""Tests for spp_cr_type_assign_program. +"""Tests for spp_cr_type_assign_program.""" -Populated incrementally through the implementation steps; this stub keeps -the tests/ package importable while the skeleton lands. -""" +from odoo import fields +from odoo.tests import tagged + +from odoo.addons.spp_change_request_v2.tests.common import CRTestCase + +ASSIGN_PROGRAM_CR_TYPE_DEFS = { + "name": "Assign to Program", + "target_type": "both", + "detail_model": "spp.cr.detail.assign_program", + "apply_strategy": "custom", + "apply_model": "spp.cr.apply.assign_program", +} + + +@tagged("post_install", "-at_install") +class TestAssignProgram(CRTestCase): + """Detail model and apply strategy for the assign-program CR type.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + + cls.Program = cls.env["spp.program"] + cls.ProgramMembership = cls.env["spp.program.membership"] + + cls.cr_type = cls.CRType.search([("code", "=", "assign_program")], limit=1) + if not cls.cr_type: + cls.cr_type = cls.CRType.create({"code": "assign_program", **ASSIGN_PROGRAM_CR_TYPE_DEFS}) + + cls.group_program_active = cls.Program.create({"name": "Group Program (Active)", "target_type": "group"}) + cls.indiv_program_active = cls.Program.create( + {"name": "Individual Program (Active)", "target_type": "individual"} + ) + cls.indiv_program_inactive = cls.Program.create( + {"name": "Individual Program (Inactive)", "target_type": "individual"} + ) + cls.indiv_program_inactive.state = "ended" + + cls.disabled_individual = cls.Partner.create( + { + "name": "Disabled Individual", + "given_name": "Disabled", + "family_name": "Individual", + "is_registrant": True, + "is_group": False, + "disabled": fields.Datetime.now(), + } + ) + + # ------------------------------------------------------------------ + # Helpers + # ------------------------------------------------------------------ + + def _make_cr(self, registrant, program=None): + """Create a CR of this type and (optionally) preset its detail's program. + + Returns (change_request, detail). + """ + cr = self.CR.create({"request_type_id": self.cr_type.id, "registrant_id": registrant.id}) + detail = cr.get_detail() + if program is not None: + detail.program_id = program.id + return cr, detail + + def _strategy(self): + return self.env["spp.cr.apply.assign_program"] + + # ------------------------------------------------------------------ + # Detail model — computed fields (D1-D4) + # ------------------------------------------------------------------ + + def test_d1_registrant_target_type_for_group(self): + _cr, detail = self._make_cr(self.test_group) + self.assertEqual(detail.registrant_target_type, "group") + + def test_d2_registrant_target_type_for_individual(self): + _cr, detail = self._make_cr(self.test_individual) + self.assertEqual(detail.registrant_target_type, "individual") + + def test_d3_allowed_programs_for_group_registrant(self): + _cr, detail = self._make_cr(self.test_group) + allowed = detail.allowed_program_ids + self.assertIn(self.group_program_active, allowed) + self.assertNotIn(self.indiv_program_active, allowed) + self.assertNotIn(self.indiv_program_inactive, allowed) + + def test_d4_allowed_programs_for_individual_registrant(self): + _cr, detail = self._make_cr(self.test_individual) + allowed = detail.allowed_program_ids + self.assertIn(self.indiv_program_active, allowed) + self.assertNotIn(self.group_program_active, allowed) + self.assertNotIn(self.indiv_program_inactive, allowed) From 8644d901e8b4aa350952010f5565923c2e653b20 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 17:54:50 +0800 Subject: [PATCH 03/12] feat(spp_cr_type_assign_program): add apply strategy with full validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement `spp.cr.apply.assign_program` with explicit `validate()`, `apply()`, and `preview()`. Validation rejects: missing program, disabled registrant, inactive program, target_type mismatch (defensive guard against direct ID writes that bypass the form domain), and existing memberships for the same `(registrant, program)` pair — intercepting the DB unique constraint with a friendlier message. On apply, creates a draft `spp.program.membership` and stores its ID on the detail for the audit trail. --- spp_cr_type_assign_program/__init__.py | 1 + .../strategies/__init__.py | 1 + .../strategies/assign_program.py | 100 ++++++++++++++++++ .../tests/test_assign_program.py | 85 +++++++++++++++ 4 files changed, 187 insertions(+) create mode 100644 spp_cr_type_assign_program/strategies/__init__.py create mode 100644 spp_cr_type_assign_program/strategies/assign_program.py diff --git a/spp_cr_type_assign_program/__init__.py b/spp_cr_type_assign_program/__init__.py index a7ea0c4c..7c2f78dc 100644 --- a/spp_cr_type_assign_program/__init__.py +++ b/spp_cr_type_assign_program/__init__.py @@ -1 +1,2 @@ from . import details +from . import strategies diff --git a/spp_cr_type_assign_program/strategies/__init__.py b/spp_cr_type_assign_program/strategies/__init__.py new file mode 100644 index 00000000..d678cbfd --- /dev/null +++ b/spp_cr_type_assign_program/strategies/__init__.py @@ -0,0 +1 @@ +from . import assign_program diff --git a/spp_cr_type_assign_program/strategies/assign_program.py b/spp_cr_type_assign_program/strategies/assign_program.py new file mode 100644 index 00000000..e21f05f0 --- /dev/null +++ b/spp_cr_type_assign_program/strategies/assign_program.py @@ -0,0 +1,100 @@ +import logging + +from odoo import _, models +from odoo.exceptions import UserError + +_logger = logging.getLogger(__name__) + + +class SPPCRApplyAssignProgram(models.AbstractModel): + """Custom apply strategy for the assign-to-program CR type. + + Creates an `spp.program.membership` record linking the CR's registrant to + the program selected on the detail. The membership starts in `draft` so + the program's own enrollment workflow can take over from there. + """ + + _name = "spp.cr.apply.assign_program" + _inherit = "spp.cr.strategy.base" + _description = "CR Apply: Assign to Program" + + def validate(self, change_request): + """Validate the CR can be applied. Raises UserError on any failure.""" + detail = change_request.get_detail() + if not detail: + raise UserError(_("No detail record found for this change request.")) + + program = detail.program_id + if not program: + raise UserError(_("Program is required to assign a registrant.")) + + registrant = change_request.registrant_id + if not registrant: + raise UserError(_("Registrant is required.")) + + if registrant.disabled: + raise UserError(_("Disabled registrants cannot be assigned to a program.")) + + if program.state != "active": + raise UserError(_("Only active programs can accept new registrants.")) + + expected_target_type = "group" if registrant.is_group else "individual" + if program.target_type != expected_target_type: + raise UserError( + _( + "Program '%(program)s' targets '%(program_target)s' " + "registrants but '%(registrant)s' is " + "'%(registrant_target)s'." + ) + % { + "program": program.display_name, + "program_target": program.target_type, + "registrant": registrant.display_name, + "registrant_target": expected_target_type, + } + ) + + existing = self.env["spp.program.membership"].search_count( + [("partner_id", "=", registrant.id), ("program_id", "=", program.id)] + ) + if existing: + raise UserError( + _("%(registrant)s is already in program %(program)s.") + % { + "registrant": registrant.display_name, + "program": program.display_name, + } + ) + + def apply(self, change_request): + """Validate, then create the program membership.""" + self.validate(change_request) + + detail = change_request.get_detail() + registrant = change_request.registrant_id + program = detail.program_id + + membership = self.env["spp.program.membership"].create({"partner_id": registrant.id, "program_id": program.id}) + detail.write({"created_membership_id": membership.id}) + + _logger.info( + "Created program membership id=%s for registrant_id=%s, program_id=%s via CR %s", + membership.id, + registrant.id, + program.id, + change_request.name, + ) + return True + + def preview(self, change_request): + """Preview what will happen on apply.""" + detail = change_request.get_detail() + if not detail or not detail.program_id: + return {} + + return { + "_action": "create_program_membership", + "registrant": change_request.registrant_id.display_name, + "program": detail.program_id.display_name, + "initial_state": "draft", + } diff --git a/spp_cr_type_assign_program/tests/test_assign_program.py b/spp_cr_type_assign_program/tests/test_assign_program.py index 71750a8d..b0718b92 100644 --- a/spp_cr_type_assign_program/tests/test_assign_program.py +++ b/spp_cr_type_assign_program/tests/test_assign_program.py @@ -1,6 +1,7 @@ """Tests for spp_cr_type_assign_program.""" from odoo import fields +from odoo.exceptions import UserError from odoo.tests import tagged from odoo.addons.spp_change_request_v2.tests.common import CRTestCase @@ -92,3 +93,87 @@ def test_d4_allowed_programs_for_individual_registrant(self): self.assertIn(self.indiv_program_active, allowed) self.assertNotIn(self.group_program_active, allowed) self.assertNotIn(self.indiv_program_inactive, allowed) + + # ------------------------------------------------------------------ + # Apply strategy + # ------------------------------------------------------------------ + + def test_a1_apply_creates_membership_for_individual(self): + cr, detail = self._make_cr(self.test_individual, self.indiv_program_active) + + result = self._strategy().apply(cr) + + self.assertTrue(result) + self.assertTrue(detail.created_membership_id) + membership = detail.created_membership_id + self.assertEqual(membership.partner_id, self.test_individual) + self.assertEqual(membership.program_id, self.indiv_program_active) + self.assertEqual(membership.state, "draft") + + def test_a4_apply_without_program_raises(self): + cr, _detail = self._make_cr(self.test_individual) + + with self.assertRaises(UserError): + self._strategy().apply(cr) + + def test_a2_apply_creates_membership_for_group(self): + cr, detail = self._make_cr(self.test_group, self.group_program_active) + + self._strategy().apply(cr) + + self.assertTrue(detail.created_membership_id) + self.assertEqual(detail.created_membership_id.partner_id, self.test_group) + self.assertEqual(detail.created_membership_id.program_id, self.group_program_active) + self.assertEqual(detail.created_membership_id.state, "draft") + + def test_a5_apply_with_disabled_registrant_raises(self): + cr, _detail = self._make_cr(self.disabled_individual, self.indiv_program_active) + + with self.assertRaises(UserError) as cm: + self._strategy().apply(cr) + + self.assertIn("disabled", str(cm.exception).lower()) + + def test_a6_apply_with_inactive_program_raises(self): + cr, _detail = self._make_cr(self.test_individual, self.indiv_program_inactive) + + with self.assertRaises(UserError) as cm: + self._strategy().apply(cr) + + self.assertIn("active", str(cm.exception).lower()) + + def test_a7_apply_with_target_type_mismatch_raises(self): + # Group registrant + individual program — domain would normally prevent + # this in the UI, but the strategy must still defend against direct ID + # writes. + cr, _detail = self._make_cr(self.test_group, self.indiv_program_active) + + with self.assertRaises(UserError) as cm: + self._strategy().apply(cr) + + self.assertIn("target", str(cm.exception).lower()) + + def test_a8_apply_with_existing_membership_raises_friendly_error(self): + # First apply succeeds. + cr_first, _detail = self._make_cr(self.test_individual, self.indiv_program_active) + self._strategy().apply(cr_first) + + # Second CR for the same (registrant, program) pair must be rejected + # with our own friendly message (not the raw DB unique-constraint + # error). + cr_second, _detail2 = self._make_cr(self.test_individual, self.indiv_program_active) + + with self.assertRaises(UserError) as cm: + self._strategy().apply(cr_second) + + self.assertIn("already", str(cm.exception).lower()) + + def test_a9_preview_returns_expected_shape(self): + cr, _detail = self._make_cr(self.test_individual, self.indiv_program_active) + + preview = self._strategy().preview(cr) + + self.assertEqual(preview.get("_action"), "create_program_membership") + self.assertEqual(preview.get("registrant"), self.test_individual.display_name) + self.assertEqual(preview.get("program"), self.indiv_program_active.display_name) + self.assertEqual(preview.get("initial_state"), "draft") From 87d76d41dc1a49fd1ece11068b4afdaf80953388 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 17:55:13 +0800 Subject: [PATCH 04/12] feat(spp_cr_type_assign_program): add form view and CR type record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the detail form view (with a prominent Beneficiary section and a callout explaining that the registrant itself is the program beneficiary) and the `spp.change.request.type` data record that wires the type into the standard Create-Change-Request wizard. The type is non-editable via Studio, system-defined, and has conflict detection enabled — the rule itself lands in the next commit. --- spp_cr_type_assign_program/__manifest__.py | 2 + spp_cr_type_assign_program/data/cr_types.xml | 26 ++++++ .../views/detail_assign_program_views.xml | 81 +++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 spp_cr_type_assign_program/data/cr_types.xml create mode 100644 spp_cr_type_assign_program/views/detail_assign_program_views.xml diff --git a/spp_cr_type_assign_program/__manifest__.py b/spp_cr_type_assign_program/__manifest__.py index f1b8d7f6..30bc2493 100644 --- a/spp_cr_type_assign_program/__manifest__.py +++ b/spp_cr_type_assign_program/__manifest__.py @@ -14,6 +14,8 @@ ], "data": [ "security/ir.model.access.csv", + "views/detail_assign_program_views.xml", + "data/cr_types.xml", ], "installable": True, "application": False, diff --git a/spp_cr_type_assign_program/data/cr_types.xml b/spp_cr_type_assign_program/data/cr_types.xml new file mode 100644 index 00000000..5bc4c3e7 --- /dev/null +++ b/spp_cr_type_assign_program/data/cr_types.xml @@ -0,0 +1,26 @@ + + + + Assign to Program + assign_program + Assign a registrant (individual or household) to a program. The registrant becomes the program beneficiary. + both + + spp.cr.detail.assign_program + + custom + spp.cr.apply.assign_program + fa-hand-holding-heart + 120 + + + + spp_cr_type_assign_program + + This type requires custom Python logic to create program memberships. Cannot be edited via Studio. + + diff --git a/spp_cr_type_assign_program/views/detail_assign_program_views.xml b/spp_cr_type_assign_program/views/detail_assign_program_views.xml new file mode 100644 index 00000000..0253ffd5 --- /dev/null +++ b/spp_cr_type_assign_program/views/detail_assign_program_views.xml @@ -0,0 +1,81 @@ + + + + spp.cr.detail.assign_program.form + spp.cr.detail.assign_program + +
+
+ +
+ +
+

Assign to Program

+
+ + + + + + + + + + + + + + + + + + + +
+ + From 652c96c48d988a6c1a7929d839b7d06c7a87e00d Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 17:55:38 +0800 Subject: [PATCH 05/12] feat(spp_cr_type_assign_program): block duplicate in-flight program assignments Add a `scope='custom'` conflict rule on the new CR type and the `_check_custom_conflicts` override on `spp.change.request` that narrows the registrant-scoped match to candidates whose detail's `program_id` equals ours. Two CRs assigning the same registrant to the *same* program block; two CRs for the same registrant on *different* programs proceed independently. --- spp_cr_type_assign_program/__init__.py | 1 + spp_cr_type_assign_program/data/cr_types.xml | 13 ++++++++ spp_cr_type_assign_program/models/__init__.py | 1 + .../models/change_request.py | 33 +++++++++++++++++++ .../tests/test_assign_program.py | 26 +++++++++++++++ 5 files changed, 74 insertions(+) create mode 100644 spp_cr_type_assign_program/models/__init__.py create mode 100644 spp_cr_type_assign_program/models/change_request.py diff --git a/spp_cr_type_assign_program/__init__.py b/spp_cr_type_assign_program/__init__.py index 7c2f78dc..1d5fc995 100644 --- a/spp_cr_type_assign_program/__init__.py +++ b/spp_cr_type_assign_program/__init__.py @@ -1,2 +1,3 @@ from . import details +from . import models from . import strategies diff --git a/spp_cr_type_assign_program/data/cr_types.xml b/spp_cr_type_assign_program/data/cr_types.xml index 5bc4c3e7..8a12dbae 100644 --- a/spp_cr_type_assign_program/data/cr_types.xml +++ b/spp_cr_type_assign_program/data/cr_types.xml @@ -23,4 +23,17 @@ name="locked_reason" >This type requires custom Python logic to create program memberships. Cannot be edited via Studio. + + + Duplicate program assignment + + custom + + block + all_active + 10 + Another change request is already assigning this registrant to the same program. Cancel or wait for it to be applied first. + diff --git a/spp_cr_type_assign_program/models/__init__.py b/spp_cr_type_assign_program/models/__init__.py new file mode 100644 index 00000000..126f51df --- /dev/null +++ b/spp_cr_type_assign_program/models/__init__.py @@ -0,0 +1 @@ +from . import change_request diff --git a/spp_cr_type_assign_program/models/change_request.py b/spp_cr_type_assign_program/models/change_request.py new file mode 100644 index 00000000..cdf50959 --- /dev/null +++ b/spp_cr_type_assign_program/models/change_request.py @@ -0,0 +1,33 @@ +from odoo import models + + +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. + """ + + _inherit = "spp.change.request" + + def _check_custom_conflicts(self, candidates, rule): + candidates = super()._check_custom_conflicts(candidates, rule) + + rule_xmlid = "spp_cr_type_assign_program.cr_conflict_rule_assign_program_duplicate" + our_rule = self.env.ref(rule_xmlid, raise_if_not_found=False) + if not our_rule or rule != our_rule: + return candidates + + my_detail = self.get_detail() + if not my_detail or not my_detail.program_id: + return self.env["spp.change.request"] + + my_program = my_detail.program_id + matching = self.env["spp.change.request"] + for candidate in candidates: + cand_detail = candidate.get_detail() + if cand_detail and cand_detail.program_id == my_program: + matching |= candidate + return matching diff --git a/spp_cr_type_assign_program/tests/test_assign_program.py b/spp_cr_type_assign_program/tests/test_assign_program.py index b0718b92..6afa727e 100644 --- a/spp_cr_type_assign_program/tests/test_assign_program.py +++ b/spp_cr_type_assign_program/tests/test_assign_program.py @@ -168,6 +168,32 @@ def test_a8_apply_with_existing_membership_raises_friendly_error(self): self.assertIn("already", str(cm.exception).lower()) + # ------------------------------------------------------------------ + # Conflict detection (F2, F3) + # ------------------------------------------------------------------ + + def test_f2_two_crs_for_same_registrant_program_block_second(self): + cr1, _d1 = self._make_cr(self.test_individual, self.indiv_program_active) + cr2, _d2 = self._make_cr(self.test_individual, self.indiv_program_active) + + cr2._run_conflict_checks() + + self.assertEqual(cr2.conflict_status, "blocked") + self.assertIn(cr1, cr2.conflicting_cr_ids) + + def test_f3_two_crs_for_same_registrant_different_programs_allowed(self): + # Two distinct active individual programs targeting the same registrant + # must both be able to proceed. + other_indiv_program = self.Program.create({"name": "Other Individual Program", "target_type": "individual"}) + + _cr1, _d1 = self._make_cr(self.test_individual, self.indiv_program_active) + cr2, _d2 = self._make_cr(self.test_individual, other_indiv_program) + + cr2._run_conflict_checks() + + self.assertEqual(cr2.conflict_status, "none") + self.assertFalse(cr2.conflicting_cr_ids) + def test_a9_preview_returns_expected_shape(self): cr, _detail = self._make_cr(self.test_individual, self.indiv_program_active) From ac7e7cd8927d5bc996947414d444a13ab5f59f24 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 17:55:51 +0800 Subject: [PATCH 06/12] test(spp_cr_type_assign_program): add full CR lifecycle integration test Approve and apply a draft CR end-to-end, asserting that `cr.action_apply()` creates the program membership and back-links it on the detail. Complements the unit-level apply tests by exercising the strategy through the CR's own apply pipeline (preview snapshot, sudo, audit logging) instead of calling `apply()` directly. --- .../tests/test_assign_program.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spp_cr_type_assign_program/tests/test_assign_program.py b/spp_cr_type_assign_program/tests/test_assign_program.py index 6afa727e..fc78d9f8 100644 --- a/spp_cr_type_assign_program/tests/test_assign_program.py +++ b/spp_cr_type_assign_program/tests/test_assign_program.py @@ -168,6 +168,23 @@ def test_a8_apply_with_existing_membership_raises_friendly_error(self): self.assertIn("already", str(cm.exception).lower()) + # ------------------------------------------------------------------ + # Full CR lifecycle (F1) + # ------------------------------------------------------------------ + + def test_f1_full_cr_lifecycle_creates_membership(self): + cr, detail = self._make_cr(self.test_individual, self.indiv_program_active) + + cr.approval_state = "approved" + cr.action_apply() + + self.assertTrue(cr.is_applied) + self.assertTrue(detail.created_membership_id) + membership = detail.created_membership_id + self.assertEqual(membership.partner_id, self.test_individual) + self.assertEqual(membership.program_id, self.indiv_program_active) + self.assertEqual(membership.state, "draft") + # ------------------------------------------------------------------ # Conflict detection (F2, F3) # ------------------------------------------------------------------ From 2d6738001074a212ece2f1e9fa25539eec4f9e84 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 17:56:00 +0800 Subject: [PATCH 07/12] chore(spp_cr_type_assign_program): generate README from fragments Run the oca-gen-addon-readme pre-commit hook to produce README.rst and static/description/index.html from the readme/ fragments. --- spp_cr_type_assign_program/README.rst | 143 +++++ .../static/description/index.html | 501 ++++++++++++++++++ 2 files changed, 644 insertions(+) create mode 100644 spp_cr_type_assign_program/README.rst create mode 100644 spp_cr_type_assign_program/static/description/index.html diff --git a/spp_cr_type_assign_program/README.rst b/spp_cr_type_assign_program/README.rst new file mode 100644 index 00000000..6a49d539 --- /dev/null +++ b/spp_cr_type_assign_program/README.rst @@ -0,0 +1,143 @@ +=================================== +OpenSPP CR Type - Assign to Program +=================================== + +.. + !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + !! This file is generated by oca-gen-addon-readme !! + !! changes will be overwritten. !! + !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + !! source digest: sha256:2ba79797ea8d497c9a808e1ac44f05ef27897f6d01974e976247fb2ba108d5c6 + !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + +.. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png + :target: https://odoo-community.org/page/development-status + :alt: Beta +.. |badge2| image:: https://img.shields.io/badge/license-LGPL--3-blue.png + :target: http://www.gnu.org/licenses/lgpl-3.0-standalone.html + :alt: License: LGPL-3 +.. |badge3| image:: https://img.shields.io/badge/github-OpenSPP%2FOpenSPP2-lightgray.png?logo=github + :target: https://github.com/OpenSPP/OpenSPP2/tree/19.0/spp_cr_type_assign_program + :alt: OpenSPP/OpenSPP2 + +|badge1| |badge2| |badge3| + +Adds a single change request type — ``assign_program`` — that records a +registrant being assigned to a program. The change request runs through +the standard approval, conflict-detection, and document workflow +provided by ``spp_change_request_v2``. On apply, an +``spp.program.membership`` record is created in the ``draft`` state for +the ``(registrant, program)`` pair. + +Beneficiary semantics +~~~~~~~~~~~~~~~~~~~~~ + +The CR's registrant **is** the program beneficiary. There is no "select +a member of the household" step. + +- Registrant is a group (household) → eligible programs are those with + ``target_type='group'`` and ``state='active'``. The household itself + is enrolled. +- Registrant is an individual → eligible programs are those with + ``target_type='individual'`` and ``state='active'``. The individual is + enrolled. + +Standalone individuals (registrants not in any household) are supported. + +Models defined by this module +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++----------------------------------+---------------+--------------------------+ +| Model | Kind | Purpose | ++==================================+===============+==========================+ +| ``spp.cr.detail.assign_program`` | Model | Captures the program | +| | | selection for the CR | ++----------------------------------+---------------+--------------------------+ +| ``spp.cr.apply.assign_program`` | AbstractModel | Apply strategy that | +| | | creates the membership | ++----------------------------------+---------------+--------------------------+ + +Validation rules (apply-time) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The apply strategy refuses the operation when any of the following hold: + +- the registrant is ``disabled`` +- the program is not in ``state='active'`` +- the program's ``target_type`` does not match the registrant +- a membership for the same ``(registrant, program)`` pair already + exists +- the detail record has no ``program_id`` set + +Conflict detection +~~~~~~~~~~~~~~~~~~ + +Two in-flight ``assign_program`` change requests targeting the same +``(registrant, program)`` pair are treated as conflicting and the second +submission is blocked. Two CRs for the same registrant but different +programs are independent and both proceed. + +Dependencies +~~~~~~~~~~~~ + +- ``spp_change_request_v2`` +- ``spp_programs`` + +**Table of contents** + +.. contents:: + :local: + +Changelog +========= + +19.0.1.0.0 (2026-05-04) +----------------------- + +Added +~~~~~ + +- New module ``spp_cr_type_assign_program`` with the ``assign_program`` + change request type. +- Detail model ``spp.cr.detail.assign_program`` with live program-domain + filtering based on the registrant's target type. +- Apply strategy ``spp.cr.apply.assign_program`` that creates a draft + ``spp.program.membership`` record on apply. +- Conflict rule that blocks duplicate in-flight assignments to the same + ``(registrant, program)`` pair. + +Bug Tracker +=========== + +Bugs are tracked on `GitHub Issues `_. +In case of trouble, please check there if your issue has already been reported. +If you spotted it first, help us to smash it by providing a detailed and welcomed +`feedback `_. + +Do not contact contributors directly about support or help with technical issues. + +Credits +======= + +Authors +------- + +* OpenSPP.org + +Maintainers +----------- + +.. |maintainer-jeremi| image:: https://github.com/jeremi.png?size=40px + :target: https://github.com/jeremi + :alt: jeremi +.. |maintainer-gonzalesedwin1123| image:: https://github.com/gonzalesedwin1123.png?size=40px + :target: https://github.com/gonzalesedwin1123 + :alt: gonzalesedwin1123 + +Current maintainers: + +|maintainer-jeremi| |maintainer-gonzalesedwin1123| + +This module is part of the `OpenSPP/OpenSPP2 `_ project on GitHub. + +You are welcome to contribute. \ No newline at end of file diff --git a/spp_cr_type_assign_program/static/description/index.html b/spp_cr_type_assign_program/static/description/index.html new file mode 100644 index 00000000..aa6fd572 --- /dev/null +++ b/spp_cr_type_assign_program/static/description/index.html @@ -0,0 +1,501 @@ + + + + + +OpenSPP CR Type - Assign to Program + + + +
+

OpenSPP CR Type - Assign to Program

+ + +

Beta License: LGPL-3 OpenSPP/OpenSPP2

+

Adds a single change request type — assign_program — that records a +registrant being assigned to a program. The change request runs through +the standard approval, conflict-detection, and document workflow +provided by spp_change_request_v2. On apply, an +spp.program.membership record is created in the draft state for +the (registrant, program) pair.

+
+

Beneficiary semantics

+

The CR’s registrant is the program beneficiary. There is no “select +a member of the household” step.

+
    +
  • Registrant is a group (household) → eligible programs are those with +target_type='group' and state='active'. The household itself +is enrolled.
    • +
  • Registrant is an individual → eligible programs are those with +target_type='individual' and state='active'. The individual is +enrolled.
    • +
+

Standalone individuals (registrants not in any household) are supported.

+
+
+

Models defined by this module

+ +++++ + + + + + + + + + + + + + + + + +
ModelKindPurpose
spp.cr.detail.assign_programModelCaptures the program +selection for the CR
spp.cr.apply.assign_programAbstractModelApply strategy that +creates the membership
+
+
+

Validation rules (apply-time)

+

The apply strategy refuses the operation when any of the following hold:

+
    +
  • the registrant is disabled
    • +
  • the program is not in state='active'
    • +
  • the program’s target_type does not match the registrant
    • +
  • a membership for the same (registrant, program) pair already +exists
    • +
  • the detail record has no program_id set
    • +
+
+
+

Conflict detection

+

Two in-flight assign_program change requests targeting the same +(registrant, program) pair are treated as conflicting and the second +submission is blocked. Two CRs for the same registrant but different +programs are independent and both proceed.

+
+
+

Dependencies

+
    +
  • spp_change_request_v2
    • +
  • spp_programs
    • +
+

Table of contents

+ + +
+
+

Added

+
    +
  • New module spp_cr_type_assign_program with the assign_program +change request type.
    • +
  • Detail model spp.cr.detail.assign_program with live program-domain +filtering based on the registrant’s target type.
    • +
  • Apply strategy spp.cr.apply.assign_program that creates a draft +spp.program.membership record on apply.
    • +
  • Conflict rule that blocks duplicate in-flight assignments to the same +(registrant, program) pair.
    • +
+
+

Bug Tracker

+

Bugs are tracked on GitHub Issues. +In case of trouble, please check there if your issue has already been reported. +If you spotted it first, help us to smash it by providing a detailed and welcomed +feedback.

+

Do not contact contributors directly about support or help with technical issues.

+
+
+

Credits

+
+

Authors

+
    +
  • OpenSPP.org
    • +
+
+
+

Maintainers

+

Current maintainers:

+

jeremi gonzalesedwin1123

+

This module is part of the OpenSPP/OpenSPP2 project on GitHub.

+

You are welcome to contribute.

+
+
+
+
+ + From 9fe34ab4d0261b19ea88fc74ef1282953591a1e3 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 18:15:40 +0800 Subject: [PATCH 08/12] perf(spp_cr_type_assign_program): batch program and detail searches Two N+1 query patterns identified by review: - `_compute_allowed_program_ids` ran one search per detail record. Cache results by `registrant_target_type` so a recordset of N details runs at most 2 queries (one per target type). - `_check_custom_conflicts` called `get_detail()` per candidate, each triggering its own load. Replace with one `search` over the candidate detail IDs filtered by `program_id`. `check_same_type_only=True` on our rule guarantees candidates share our detail model; defensive guards still skip candidates whose detail model or detail_res_id is missing. --- .../details/assign_program.py | 20 ++++++++----- .../models/change_request.py | 28 ++++++++++++++----- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/spp_cr_type_assign_program/details/assign_program.py b/spp_cr_type_assign_program/details/assign_program.py index f4d5bd0a..fcd565df 100644 --- a/spp_cr_type_assign_program/details/assign_program.py +++ b/spp_cr_type_assign_program/details/assign_program.py @@ -42,13 +42,19 @@ def _compute_registrant_target_type(self): @api.depends("registrant_target_type") def _compute_allowed_program_ids(self): Program = self.env["spp.program"] + # target_type only has two distinct values; cache the search per + # value so a recordset of N details runs at most 2 queries. + cache = {} for rec in self: - if not rec.registrant_target_type: + tt = rec.registrant_target_type + if not tt: rec.allowed_program_ids = False continue - rec.allowed_program_ids = Program.search( - [ - ("state", "=", "active"), - ("target_type", "=", rec.registrant_target_type), - ] - ) + if tt not in cache: + cache[tt] = Program.search( + [ + ("state", "=", "active"), + ("target_type", "=", tt), + ] + ) + rec.allowed_program_ids = cache[tt] diff --git a/spp_cr_type_assign_program/models/change_request.py b/spp_cr_type_assign_program/models/change_request.py index cdf50959..21d52ddd 100644 --- a/spp_cr_type_assign_program/models/change_request.py +++ b/spp_cr_type_assign_program/models/change_request.py @@ -24,10 +24,24 @@ def _check_custom_conflicts(self, candidates, rule): if not my_detail or not my_detail.program_id: return self.env["spp.change.request"] - my_program = my_detail.program_id - matching = self.env["spp.change.request"] - for candidate in candidates: - cand_detail = candidate.get_detail() - if cand_detail and cand_detail.program_id == my_program: - matching |= candidate - return matching + # `check_same_type_only=True` on our rule guarantees all candidates + # share our detail model, but defend against edge cases where a + # candidate has no detail yet. + detail_model = my_detail._name + candidate_detail_ids = [ + c.detail_res_id for c in candidates if c.detail_res_model == detail_model and c.detail_res_id + ] + if not candidate_detail_ids: + return self.env["spp.change.request"] + + matching_detail_ids = set( + self.env[detail_model] + .search( + [ + ("id", "in", candidate_detail_ids), + ("program_id", "=", my_detail.program_id.id), + ] + ) + .ids + ) + return candidates.filtered(lambda c: c.detail_res_id in matching_detail_ids) From 9a4a2cb4456bb4708925f77dc7783fb6d3a5c4d5 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 18:29:23 +0800 Subject: [PATCH 09/12] fix(spp_cr_type_assign_program): translate concurrent insert race to UserError `validate()` checks the (registrant, program) pair is unique before `apply()` creates the membership, but a concurrent transaction can insert the same pair between the two operations. The DB unique constraint on `spp.program.membership(partner_id, program_id)` fires and surfaces to the user as a raw `psycopg2.errors.UniqueViolation` plus a poisoned transaction (subsequent ORM calls fail with `InFailedSqlTransaction`). Wrap the create in `cr.savepoint()` so the parent transaction stays usable, mute the noisy SQL error log, and translate `UniqueViolation` into the same friendly UserError the validate() path produces. Add a test that mocks `spp.program.membership.create` to raise `UniqueViolation`, asserting the strategy raises UserError with the expected message. Also add a test for the conflict-hook short-circuit: when `_check_custom_conflicts` is called with a rule that isn't ours, candidates must be returned unchanged so other modules' custom conflict logic isn't suppressed. Document the deliberate UI-staleness of `_compute_allowed_program_ids` when a program transitions active <-> ended mid-form: the apply strategy revalidates `state == 'active'`, so staleness is UI-only. --- .../details/assign_program.py | 4 ++ .../strategies/assign_program.py | 24 +++++++++- .../tests/test_assign_program.py | 45 +++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/spp_cr_type_assign_program/details/assign_program.py b/spp_cr_type_assign_program/details/assign_program.py index fcd565df..0ea83e30 100644 --- a/spp_cr_type_assign_program/details/assign_program.py +++ b/spp_cr_type_assign_program/details/assign_program.py @@ -44,6 +44,10 @@ def _compute_allowed_program_ids(self): Program = self.env["spp.program"] # target_type only has two distinct values; cache the search per # value so a recordset of N details runs at most 2 queries. + # Note: the result can become stale if a program transitions + # active <-> ended while a CR form is open. Acceptable: the apply + # strategy revalidates `state == 'active'` at apply time, so + # staleness is a UI-only concern. cache = {} for rec in self: tt = rec.registrant_target_type diff --git a/spp_cr_type_assign_program/strategies/assign_program.py b/spp_cr_type_assign_program/strategies/assign_program.py index e21f05f0..4f2fb11c 100644 --- a/spp_cr_type_assign_program/strategies/assign_program.py +++ b/spp_cr_type_assign_program/strategies/assign_program.py @@ -1,7 +1,10 @@ import logging +import psycopg2 + from odoo import _, models from odoo.exceptions import UserError +from odoo.tools import mute_logger _logger = logging.getLogger(__name__) @@ -74,7 +77,26 @@ def apply(self, change_request): registrant = change_request.registrant_id program = detail.program_id - membership = self.env["spp.program.membership"].create({"partner_id": registrant.id, "program_id": program.id}) + # `validate()` checks the (registrant, program) pair is unique, but a + # concurrent transaction can insert the same pair between that read + # and the create below. The DB unique constraint on + # spp.program.membership(partner_id, program_id) catches the race; + # wrap the create in a savepoint so the parent transaction stays + # usable, and translate the psycopg2 error into the same friendly + # UserError the validate() path produces. + try: + with self.env.cr.savepoint(), mute_logger("odoo.sql_db"): + membership = self.env["spp.program.membership"].create( + {"partner_id": registrant.id, "program_id": program.id} + ) + except psycopg2.errors.UniqueViolation as exc: + raise UserError( + _("%(registrant)s is already in program %(program)s.") + % { + "registrant": registrant.display_name, + "program": program.display_name, + } + ) from exc detail.write({"created_membership_id": membership.id}) _logger.info( diff --git a/spp_cr_type_assign_program/tests/test_assign_program.py b/spp_cr_type_assign_program/tests/test_assign_program.py index fc78d9f8..bacdba80 100644 --- a/spp_cr_type_assign_program/tests/test_assign_program.py +++ b/spp_cr_type_assign_program/tests/test_assign_program.py @@ -1,5 +1,9 @@ """Tests for spp_cr_type_assign_program.""" +from unittest.mock import patch + +import psycopg2 + from odoo import fields from odoo.exceptions import UserError from odoo.tests import tagged @@ -198,6 +202,27 @@ def test_f2_two_crs_for_same_registrant_program_block_second(self): self.assertEqual(cr2.conflict_status, "blocked") self.assertIn(cr1, cr2.conflicting_cr_ids) + def test_f4_conflict_hook_passes_through_non_our_rules(self): + """If _check_custom_conflicts is invoked with a rule that isn't + ours, the hook must return the input candidates unchanged so other + modules' custom conflict logic isn't accidentally suppressed. + """ + cr1, _d1 = self._make_cr(self.test_individual, self.indiv_program_active) + cr2, _d2 = self._make_cr(self.test_individual, self.indiv_program_active) + + other_rule = self.env["spp.cr.conflict.rule"].create( + { + "name": "Unrelated rule", + "cr_type_id": self.cr_type.id, + "scope": "custom", + "action": "warn", + } + ) + + result = cr2._check_custom_conflicts(cr1, other_rule) + + self.assertEqual(result, cr1) + def test_f3_two_crs_for_same_registrant_different_programs_allowed(self): # Two distinct active individual programs targeting the same registrant # must both be able to proceed. @@ -211,6 +236,26 @@ 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_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(). + The DB UNIQUE constraint fires; the strategy must translate it + into the same friendly UserError the validate() path produces, + not let the raw psycopg2 error surface. + """ + cr, _detail = self._make_cr(self.test_individual, self.indiv_program_active) + + membership_cls = type(self.ProgramMembership) + + def boom(self_, *args, **kwargs): + raise psycopg2.errors.UniqueViolation("simulated race") + + with patch.object(membership_cls, "create", boom): + with self.assertRaises(UserError) as cm: + self._strategy().apply(cr) + + self.assertIn("already", str(cm.exception).lower()) + def test_a9_preview_returns_expected_shape(self): cr, _detail = self._make_cr(self.test_individual, self.indiv_program_active) From c882ec0ef944a24bc9d653606a2319ccb4294df0 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 4 May 2026 18:32:48 +0800 Subject: [PATCH 10/12] feat(spp_cr_type_assign_program): make the beneficiary type explicit in the form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address UX-review findings on PR #187 around beneficiary disambiguation and missing affordances: 1. Surface `registrant_target_type` as a visible labelled field ("Beneficiary type") next to `registrant_id` in the Beneficiary group. Single highest-leverage change for users coming from the household form: makes "Group" vs "Individual" enrolment explicit instead of inferred. 2. Rewrite the explanatory alert to: - name the registrant as the beneficiary (not just "the registrant above"), - tie the enrollment shape to the new visible Beneficiary type field, - direct users who actually want to enroll a household *member* to start the CR from the member's own record. 3. Add an `alert-warning` shown only when no active programs match the beneficiary's target type — replaces the silent empty-dropdown dead end with concrete guidance to ask a Program Manager. `aria-live="polite"` so screen readers announce the state when it appears. 4. Rename the post-apply group "Result" -> "Created Membership". The `created_membership_id` field is already openable (no `no_open: True` set), so the user can navigate from here to the membership record without an extra smart button. 5. Enrich the `program_id` help text to surface the filter rule (active + matching target type) and the post-apply state (Draft, Program Manager activates). 6. Mirror the in-form guidance in `readme/DESCRIPTION.md` so the module description and the in-form alert tell the same story. --- spp_cr_type_assign_program/README.rst | 5 +++ .../details/assign_program.py | 6 +++- .../readme/DESCRIPTION.md | 5 +++ .../static/description/index.html | 4 +++ .../views/detail_assign_program_views.xml | 33 ++++++++++++++++--- 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/spp_cr_type_assign_program/README.rst b/spp_cr_type_assign_program/README.rst index 6a49d539..0a1e94bb 100644 --- a/spp_cr_type_assign_program/README.rst +++ b/spp_cr_type_assign_program/README.rst @@ -44,6 +44,11 @@ a member of the household" step. Standalone individuals (registrants not in any household) are supported. +To enroll a specific member of a household (not the household itself), +open that member's individual record and start a change request from +there — the CR's registrant is the member, and the form filters programs +to those targeting individuals. + Models defined by this module ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/spp_cr_type_assign_program/details/assign_program.py b/spp_cr_type_assign_program/details/assign_program.py index 0ea83e30..b2f19772 100644 --- a/spp_cr_type_assign_program/details/assign_program.py +++ b/spp_cr_type_assign_program/details/assign_program.py @@ -13,7 +13,11 @@ class SPPCRDetailAssignProgram(models.Model): string="Program", tracking=True, domain="[('id', 'in', allowed_program_ids)]", - help="Program the registrant will be enrolled in.", + help=( + "Active programs whose target type matches this beneficiary. " + "On apply, a Draft membership is created — a Program Manager " + "activates it from there." + ), ) allowed_program_ids = fields.Many2many( "spp.program", diff --git a/spp_cr_type_assign_program/readme/DESCRIPTION.md b/spp_cr_type_assign_program/readme/DESCRIPTION.md index ee64a8ec..26727000 100644 --- a/spp_cr_type_assign_program/readme/DESCRIPTION.md +++ b/spp_cr_type_assign_program/readme/DESCRIPTION.md @@ -16,6 +16,11 @@ of the household" step. Standalone individuals (registrants not in any household) are supported. +To enroll a specific member of a household (not the household itself), open +that member's individual record and start a change request from there — the +CR's registrant is the member, and the form filters programs to those +targeting individuals. + ### Models defined by this module | Model | Kind | Purpose | diff --git a/spp_cr_type_assign_program/static/description/index.html b/spp_cr_type_assign_program/static/description/index.html index aa6fd572..d4d2f4ff 100644 --- a/spp_cr_type_assign_program/static/description/index.html +++ b/spp_cr_type_assign_program/static/description/index.html @@ -389,6 +389,10 @@

Beneficiary semantics

enrolled.

Standalone individuals (registrants not in any household) are supported.

+

To enroll a specific member of a household (not the household itself), +open that member’s individual record and start a change request from +there — the CR’s registrant is the member, and the form filters programs +to those targeting individuals.

Models defined by this module

diff --git a/spp_cr_type_assign_program/views/detail_assign_program_views.xml b/spp_cr_type_assign_program/views/detail_assign_program_views.xml index 0253ffd5..52392ffd 100644 --- a/spp_cr_type_assign_program/views/detail_assign_program_views.xml +++ b/spp_cr_type_assign_program/views/detail_assign_program_views.xml @@ -52,14 +52,34 @@ readonly="1" options="{'no_open': True}" /> + - + @@ -70,7 +90,10 @@ /> - + From 2267e0cf374c7d333220c47fb523a272d369cf71 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Tue, 5 May 2026 15:13:14 +0800 Subject: [PATCH 11/12] feat(spp_cr_type_assign_program): hide already-enrolled programs from the dropdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move duplicate prevention from "discovered at apply, after a wasted approval cycle" to "discovered at form-fill." Extend `_compute_allowed_program_ids` with `registrant_id.program_membership_ids` in its depends, then subtract the registrant's existing memberships from the cached active-matching set. The per-target-type cache stays — it saves the expensive program search; the per-registrant exclusion is a cheap Python set subtraction. Refine the empty-state warning copy to cover both reasons the dropdown can be empty (no active programs match the beneficiary's type, or the registrant is enrolled in all of them). The existing validate() check and the savepoint-protected create remain the safety net for races between form-fill and apply. Add `test_d5_allowed_programs_excludes_already_enrolled` asserting the exclusion. --- .../details/assign_program.py | 15 +++++++++++---- .../tests/test_assign_program.py | 17 +++++++++++++++++ .../views/detail_assign_program_views.xml | 9 +++++---- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/spp_cr_type_assign_program/details/assign_program.py b/spp_cr_type_assign_program/details/assign_program.py index b2f19772..da30dade 100644 --- a/spp_cr_type_assign_program/details/assign_program.py +++ b/spp_cr_type_assign_program/details/assign_program.py @@ -43,11 +43,17 @@ def _compute_registrant_target_type(self): continue rec.registrant_target_type = "group" if rec.registrant_id.is_group else "individual" - @api.depends("registrant_target_type") + @api.depends( + "registrant_target_type", + "registrant_id.program_membership_ids.program_id", + ) def _compute_allowed_program_ids(self): Program = self.env["spp.program"] - # target_type only has two distinct values; cache the search per - # value so a recordset of N details runs at most 2 queries. + # target_type only has two distinct values; cache the per-type + # active-program search so a recordset of N details runs at most + # 2 queries against spp.program. The per-registrant exclusion of + # already-enrolled programs is then applied in Python via set + # subtraction. # Note: the result can become stale if a program transitions # active <-> ended while a CR form is open. Acceptable: the apply # strategy revalidates `state == 'active'` at apply time, so @@ -65,4 +71,5 @@ def _compute_allowed_program_ids(self): ("target_type", "=", tt), ] ) - rec.allowed_program_ids = cache[tt] + already_in = rec.registrant_id.program_membership_ids.program_id + rec.allowed_program_ids = cache[tt] - already_in diff --git a/spp_cr_type_assign_program/tests/test_assign_program.py b/spp_cr_type_assign_program/tests/test_assign_program.py index bacdba80..7baf7f89 100644 --- a/spp_cr_type_assign_program/tests/test_assign_program.py +++ b/spp_cr_type_assign_program/tests/test_assign_program.py @@ -98,6 +98,23 @@ def test_d4_allowed_programs_for_individual_registrant(self): self.assertNotIn(self.group_program_active, allowed) self.assertNotIn(self.indiv_program_inactive, allowed) + def test_d5_allowed_programs_excludes_already_enrolled(self): + """Programs the registrant is already in must be filtered out so + duplicates surface at form-fill time, not after a wasted approval + cycle.""" + self.ProgramMembership.create( + { + "partner_id": self.test_individual.id, + "program_id": self.indiv_program_active.id, + } + ) + other_program = self.Program.create({"name": "Other Active Individual Program", "target_type": "individual"}) + + _cr, detail = self._make_cr(self.test_individual) + + self.assertNotIn(self.indiv_program_active, detail.allowed_program_ids) + self.assertIn(other_program, detail.allowed_program_ids) + # ------------------------------------------------------------------ # Apply strategy # ------------------------------------------------------------------ diff --git a/spp_cr_type_assign_program/views/detail_assign_program_views.xml b/spp_cr_type_assign_program/views/detail_assign_program_views.xml index 52392ffd..e41e36e7 100644 --- a/spp_cr_type_assign_program/views/detail_assign_program_views.xml +++ b/spp_cr_type_assign_program/views/detail_assign_program_views.xml @@ -76,10 +76,11 @@ aria-live="polite" invisible="allowed_program_ids or approval_state not in ('draft', 'revision')" > - No active programs match this beneficiary's type. - Ask a Program Manager to activate a program with - matching target type, then return to this change - request. + No eligible programs found. Either no active + programs match this beneficiary's type, or the + registrant is already enrolled in all of them. + Ask a Program Manager to activate a new program, + or pick a different registrant.
From b7d9028ea62e24965e9c3d6638c70f8010c5963d Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Tue, 5 May 2026 15:24:44 +0800 Subject: [PATCH 12/12] fix(spp_cr_type_assign_program): drop misleading household-member redirect line Remove the sentence advising users to "open that member's record and start a change request from there." Verified there is no in-form path to start a CR from a partner record: - `spp_change_request_v2/models/res_partner.py` adds no smart button - `spp_change_request_v2/views/` has no `res.partner` view inheritance - `action_cr_create_wizard` has no `binding_model`, so it does not appear in the partner form's Action menu The wizard's `default_get` would pre-fill `registrant_id` from `active_model='res.partner'` context, but nothing actually opens it that way. Telling users to "start a CR from there" is misleading. Drop the line from the in-form info alert and from `readme/DESCRIPTION.md`. The disambiguating copy on the "registrant IS the beneficiary" semantic stays. Regenerate README.rst and static/description/index.html to match. --- spp_cr_type_assign_program/README.rst | 5 ----- spp_cr_type_assign_program/readme/DESCRIPTION.md | 5 ----- spp_cr_type_assign_program/static/description/index.html | 4 ---- .../views/detail_assign_program_views.xml | 3 --- 4 files changed, 17 deletions(-) diff --git a/spp_cr_type_assign_program/README.rst b/spp_cr_type_assign_program/README.rst index 0a1e94bb..6a49d539 100644 --- a/spp_cr_type_assign_program/README.rst +++ b/spp_cr_type_assign_program/README.rst @@ -44,11 +44,6 @@ a member of the household" step. Standalone individuals (registrants not in any household) are supported. -To enroll a specific member of a household (not the household itself), -open that member's individual record and start a change request from -there — the CR's registrant is the member, and the form filters programs -to those targeting individuals. - Models defined by this module ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/spp_cr_type_assign_program/readme/DESCRIPTION.md b/spp_cr_type_assign_program/readme/DESCRIPTION.md index 26727000..ee64a8ec 100644 --- a/spp_cr_type_assign_program/readme/DESCRIPTION.md +++ b/spp_cr_type_assign_program/readme/DESCRIPTION.md @@ -16,11 +16,6 @@ of the household" step. Standalone individuals (registrants not in any household) are supported. -To enroll a specific member of a household (not the household itself), open -that member's individual record and start a change request from there — the -CR's registrant is the member, and the form filters programs to those -targeting individuals. - ### Models defined by this module | Model | Kind | Purpose | diff --git a/spp_cr_type_assign_program/static/description/index.html b/spp_cr_type_assign_program/static/description/index.html index d4d2f4ff..aa6fd572 100644 --- a/spp_cr_type_assign_program/static/description/index.html +++ b/spp_cr_type_assign_program/static/description/index.html @@ -389,10 +389,6 @@

Beneficiary semantics

enrolled.

Standalone individuals (registrants not in any household) are supported.

-

To enroll a specific member of a household (not the household itself), -open that member’s individual record and start a change request from -there — the CR’s registrant is the member, and the form filters programs -to those targeting individuals.

Models defined by this module

diff --git a/spp_cr_type_assign_program/views/detail_assign_program_views.xml b/spp_cr_type_assign_program/views/detail_assign_program_views.xml index e41e36e7..e3ad7f96 100644 --- a/spp_cr_type_assign_program/views/detail_assign_program_views.xml +++ b/spp_cr_type_assign_program/views/detail_assign_program_views.xml @@ -66,9 +66,6 @@ (Group) or as an individual based on the Beneficiary type. The initial membership state will be Draft. - To enroll a household member instead, open that - member's record and start a change request from - there.