Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions spp_programs/models/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 14 additions & 4 deletions spp_programs/models/managers/program_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 7 additions & 2 deletions spp_programs/models/program_membership.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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),
Expand Down
105 changes: 105 additions & 0 deletions spp_programs/tests/test_program_enrollment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading