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/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")