diff --git a/spp_programs/README.rst b/spp_programs/README.rst index c05dac423..b79966144 100644 --- a/spp_programs/README.rst +++ b/spp_programs/README.rst @@ -254,6 +254,15 @@ Dependencies Changelog ========= +19.0.2.2.1 +~~~~~~~~~~ + +- fix(spp_programs): stop Enroll Eligible undoing a deliberate pause. A + paused membership is now left alone wherever eligibility is re-run — + the enrol pass, the disenrol sweep that would otherwise have moved it + to Not Eligible, and the per-membership methods reachable over RPC. + Pausing is a decision that only Resume reverses (#1117) + 19.0.2.1.3 ~~~~~~~~~~ diff --git a/spp_programs/__manifest__.py b/spp_programs/__manifest__.py index 8dac1cba2..48590c716 100644 --- a/spp_programs/__manifest__.py +++ b/spp_programs/__manifest__.py @@ -4,7 +4,7 @@ "name": "OpenSPP Programs", "summary": "Manage programs, cycles, beneficiary enrollment, entitlements (cash and in-kind), payments, and fund tracking for social protection.", "category": "OpenSPP/Core", - "version": "19.0.2.2.0", + "version": "19.0.2.2.1", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_programs/models/constants.py b/spp_programs/models/constants.py index efb0e14d3..10c442b6f 100644 --- a/spp_programs/models/constants.py +++ b/spp_programs/models/constants.py @@ -7,6 +7,18 @@ STATE_ENDED = "ended" STATE_CANCELLED = "cancelled" +#: Membership states that only their own workflow may move a member out of. +#: Re-running eligibility — "Enroll Eligible" / "Verify Eligibility" — must step +#: over these rather than re-deciding them: +#: +#: - ``duplicated`` is resolved by deduplication +#: - ``exited`` is a closed record, reopened only by re-enrolling deliberately +#: - ``paused`` is a program officer's explicit decision, undone only by Resume +#: +#: ``paused`` was missing here, so Enroll Eligible silently resumed paused +#: members and, on the other branch, demoted them to not_eligible (OP#1117). +PROTECTED_MEMBERSHIP_STATES = ("duplicated", "exited", "paused") + MANAGER_ELIGIBILITY = 1 MANAGER_CYCLE = 2 MANAGER_PROGRAM = 3 diff --git a/spp_programs/models/managers/program_manager.py b/spp_programs/models/managers/program_manager.py index 7622e5f0b..21f71337c 100644 --- a/spp_programs/models/managers/program_manager.py +++ b/spp_programs/models/managers/program_manager.py @@ -7,6 +7,7 @@ from odoo.addons.job_worker.delay import group +from .. import constants from ..programs import SPPProgram from .pagination_utils import compute_id_ranges @@ -262,10 +263,13 @@ def _enroll_eligible_registrants(self, states, offset=0, limit=None, min_id=None for el in eligibility_managers: members = el.enroll_eligible_registrants(members) # enroll the one not already enrolled: - # Exclude members that are duplicated or exited — those states - # should only be changed through their own workflows. + # Exclude members in a state only its own workflow may leave — see + # PROTECTED_MEMBERSHIP_STATES. Notably `paused`: a program officer + # paused that member deliberately, and only Resume may undo it (OP#1117). _logger.debug("members filtered: %s", members) - not_enrolled = members.filtered(lambda m: m.state not in ("enrolled", "duplicated", "exited")) + not_enrolled = members.filtered( + lambda m: m.state != "enrolled" and m.state not in constants.PROTECTED_MEMBERSHIP_STATES + ) _logger.debug("not_enrolled: %s", not_enrolled) # Run pre-enrollment hooks (e.g., scoring eligibility checks). @@ -321,9 +325,15 @@ def _enroll_eligible_registrants(self, states, offset=0, limit=None, min_id=None for member in enrollable: program._post_enrollment_hook(member.partner_id) # dis-enroll the one not eligible anymore: + # Same protected states apply on the way down. A paused member the + # eligibility manager did not return was being swept into not_eligible, + # which destroys the pause just as thoroughly as re-enrolling it would + # (OP#1117) — that is a second, separate path to the same bug. enrolled_members_ids = members.ids members_to_remove = member_before.filtered( - lambda m: m.state not in ("not_eligible", "duplicated", "exited") and m.id not in enrolled_members_ids + lambda m: m.state != "not_eligible" + and m.state not in constants.PROTECTED_MEMBERSHIP_STATES + and m.id not in enrolled_members_ids ) # _logger.debug("members_to_remove: %s", members_to_remove) members_to_remove.write( diff --git a/spp_programs/models/program_membership.py b/spp_programs/models/program_membership.py index d4c5d76f2..0b3c5a532 100644 --- a/spp_programs/models/program_membership.py +++ b/spp_programs/models/program_membership.py @@ -280,7 +280,10 @@ def verify_eligibility(self): member = self for em in eligibility_managers: member = em.enroll_eligible_registrants(member) - if len(member) == 0: + if len(member) == 0 and self.state not in constants.PROTECTED_MEMBERSHIP_STATES: + # Leave duplicated / exited / paused alone: each is owned by its own + # workflow, and demoting a paused member to not_eligible would undo a + # deliberate pause just as surely as re-enrolling it (OP#1117). self.state = "not_eligible" return @@ -293,7 +296,9 @@ def enroll_eligible_registrants(self): member = em.enroll_eligible_registrants(member) if len(member) > 0: - if self.state in ("duplicated", "exited"): + if self.state in constants.PROTECTED_MEMBERSHIP_STATES: + # Includes paused: resuming is the Resume button's job, not + # something re-running eligibility may decide (OP#1117). message = _( "Cannot enroll: beneficiary is currently %s.", dict(self._fields["state"].selection).get(self.state, self.state), diff --git a/spp_programs/readme/HISTORY.md b/spp_programs/readme/HISTORY.md index 826b3233a..37046e6ba 100644 --- a/spp_programs/readme/HISTORY.md +++ b/spp_programs/readme/HISTORY.md @@ -1,3 +1,7 @@ +### 19.0.2.2.1 + +- fix(spp_programs): stop Enroll Eligible undoing a deliberate pause. A paused membership is now left alone wherever eligibility is re-run — the enrol pass, the disenrol sweep that would otherwise have moved it to Not Eligible, and the per-membership methods reachable over RPC. Pausing is a decision that only Resume reverses (#1117) + ### 19.0.2.1.3 - fix(security): align Program Viewer / Validator / Cycle Approver roles with the OP#951 menu audit — Program Viewer additionally gets `group_registry_viewer` + `group_approval_viewer` (read-only Registry + Approvals access); all three program roles get `group_hazard_viewer` + `group_gis_report_user` so they retain Hazard / GIS Reports visibility once those menu roots are gated. Adds `spp_hazard` and `spp_gis_report` to module dependencies. diff --git a/spp_programs/static/description/index.html b/spp_programs/static/description/index.html index 50e35cbb3..71163ce80 100644 --- a/spp_programs/static/description/index.html +++ b/spp_programs/static/description/index.html @@ -658,6 +658,16 @@

Changelog

+

19.0.2.2.1

+ +
+

19.0.2.1.3

-
+

19.0.2.1.2

-
+

19.0.2.1.1

  • fix(views): apply spp_registry.x2many_no_padding widget to the @@ -699,7 +709,7 @@

    19.0.2.1.1

    19 inserts on inline list-in-form views (#943).
-
+

19.0.2.0.11

  • Fix TypeError: 'NoneType' object is not iterable when clicking @@ -710,7 +720,7 @@

    19.0.2.0.11

    omit the state filter instead of crashing on tuple(None)
-
+

19.0.2.0.10

  • Increase parallel-safe channel limits (cycle, eligibility_manager, @@ -723,7 +733,7 @@

    19.0.2.0.10

    submission on double-click
-
+

19.0.2.0.9

  • Add context flags (skip_registrant_statistics, @@ -736,7 +746,7 @@

    19.0.2.0.9

    _compute_has_members
-
+

19.0.2.0.8

  • Replace OFFSET pagination with NTILE-based ID-range batching in all @@ -747,7 +757,7 @@

    19.0.2.0.8

    program and cycle
-
+

19.0.2.0.7

  • Bulk membership creation using raw SQL INSERT ON CONFLICT DO NOTHING @@ -756,7 +766,7 @@

    19.0.2.0.7

    _add_beneficiaries with bulk SQL path
-
+

19.0.2.0.6

  • Remove unused entitlement_base_model.py (dead code, never imported)
    • @@ -765,34 +775,34 @@

    19.0.2.0.6

    payment, and fund tests (172 → 492 tests)
-
+

19.0.2.0.5

  • Batch create entitlements and payments instead of one-by-one ORM creates
-
+

19.0.2.0.4

  • Fetch fund balance once per approval batch instead of per entitlement
-
+

19.0.2.0.3

  • Replace cycle computed fields (total_amount, entitlements_count, approval flags) with SQL aggregation queries
-
+

19.0.2.0.2

  • Add composite indexes for frequent query patterns on entitlements and program memberships
-
+

19.0.2.0.1

  • Replace Python-level uniqueness checks with SQL UNIQUE constraints for @@ -801,7 +811,7 @@

    19.0.2.0.1

    constraint creation
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • diff --git a/spp_programs/tests/test_program_enrollment.py b/spp_programs/tests/test_program_enrollment.py index f4a288441..59e8b5b1c 100644 --- a/spp_programs/tests/test_program_enrollment.py +++ b/spp_programs/tests/test_program_enrollment.py @@ -94,6 +94,111 @@ def test_enrollment_skips_exited(self): membership.invalidate_recordset() self.assertEqual(membership.state, "exited") + def test_enrollment_skips_paused(self): + """OP#1117: enrollment does not change paused state to enrolled. + + A pause is a deliberate decision by a program officer and may only be + undone through Resume, so re-running eligibility must step over it — + the same treatment duplicated and exited already get above. + """ + group = self._create_group("Paused Group") + membership = self._enroll(group, "paused") + + self.pm_default._enroll_eligible_registrants(["paused"]) + + membership.invalidate_recordset() + self.assertEqual(membership.state, "paused") + + def test_enroll_eligible_button_leaves_paused_alone(self): + """OP#1117 as reported: via the program's Enroll Eligible button. + + The button passes no state, so every membership is considered — which + is how a paused one was being swept back into enrolled. + """ + group = self._create_group("Paused Via Button") + membership = self._enroll(group, "enrolled") + membership.action_pause() + self.assertEqual(membership.state, "paused", "precondition: membership is paused") + + self.program.enroll_eligible_registrants() + + membership.invalidate_recordset() + self.assertEqual( + membership.state, + "paused", + "Enroll Eligible re-enrolled a paused membership, undoing the pause", + ) + + def test_paused_is_not_demoted_to_not_eligible(self): + """OP#1117, second path: the disenrollment sweep must skip paused too. + + A paused member the eligibility manager does not return was being + written to not_eligible, which destroys the pause just as thoroughly as + re-enrolling it. + """ + group = self._create_group("Paused Ineligible") + membership = self._enroll(group, "paused") + + # Empty state list -> the manager returns nothing, so every member is a + # demotion candidate. + self.pm_default._enroll_eligible_registrants(["paused"]) + + membership.invalidate_recordset() + self.assertEqual(membership.state, "paused") + + def test_paused_skip_does_not_block_other_members(self): + """Guard the fix: skipping paused must not skip everyone else.""" + draft_group = self._create_group("Draft Alongside Paused") + draft = self._enroll(draft_group, "draft") + paused_group = self._create_group("Paused Alongside Draft") + paused = self._enroll(paused_group, "paused") + + self.pm_default._enroll_eligible_registrants(["draft", "paused"]) + + draft.invalidate_recordset() + paused.invalidate_recordset() + self.assertEqual(draft.state, "enrolled", "a draft member should still be enrolled") + self.assertEqual(paused.state, "paused") + + def test_membership_level_enroll_refuses_a_paused_member(self): + """OP#1117, third path: the per-membership Enroll button. + + Its button is hidden unless the membership is draft, but the method is + public and reachable over RPC or from a server action, so it is guarded + rather than left to the view. + """ + group = self._create_group("Paused Single Enroll") + membership = self._enroll(group, "enrolled") + membership.action_pause() + + membership.enroll_eligible_registrants() + + membership.invalidate_recordset() + self.assertEqual(membership.state, "paused") + + def test_membership_level_verify_does_not_demote_a_paused_member(self): + """OP#1117: per-membership Verify must not push paused to not_eligible.""" + group = self._create_group("Paused Single Verify") + membership = self._enroll(group, "enrolled") + membership.action_pause() + + membership.verify_eligibility() + + membership.invalidate_recordset() + self.assertEqual(membership.state, "paused") + + def test_resume_remains_the_only_way_back(self): + """Pause is undone deliberately, through Resume.""" + group = self._create_group("Resumable Group") + membership = self._enroll(group, "enrolled") + membership.action_pause() + self.program.enroll_eligible_registrants() + + membership.invalidate_recordset() + membership.action_resume() + + self.assertEqual(membership.state, "enrolled") + def test_enrollment_enrolls_draft(self): """Enrollment changes draft state to enrolled.""" group = self._create_group("Draft Group")