Skip to content

fix(spp_programs): stop Enroll Eligible undoing a deliberate pause - #436

Open
emjay0921 wants to merge 2 commits into
19.0from
fix/1117-enroll-eligible-skips-paused
Open

fix(spp_programs): stop Enroll Eligible undoing a deliberate pause#436
emjay0921 wants to merge 2 commits into
19.0from
fix/1117-enroll-eligible-skips-paused

Conversation

@emjay0921

Copy link
Copy Markdown
Contributor

Why is this change needed?

Pausing a membership is a program officer's explicit decision, meant to be undone only by Resume. Enroll Eligible re-evaluated paused members along with everyone else and wrote them back to enrolled, reversing that decision with no warning (OP#1117). Low frequency, high trust impact: an officer who pauses a beneficiary expects it to stick.

The concept was already half-present in the code — a comment noting that duplicated and exited "should only be changed through their own workflows" — but paused was not in the set.

How was the change implemented?

  • The set is now explicit as constants.PROTECTED_MEMBERSHIP_STATES and applied everywhere re-running eligibility decides a membership's state.
  • Three paths, not one. The reported symptom is the enrol branch of _enroll_eligible_registrants, but its disenrol sweep had the same gap and would write a paused member the eligibility manager did not return to not_eligible — destroying the pause just as thoroughly. The per-membership enroll_eligible_registrants and verify_eligibility on spp.program.membership were the third: their buttons are hidden unless the record is draft, but the methods are public and reachable over RPC, and the point of the ticket is that a pause should be trustworthy.
  • The async branch, taken for programs above MIN_ROW_JOB_QUEUE, dispatches into the same _enroll_eligible_registrants, so both branches are covered. Program-level verify_eligibility passes ["enrolled", "not_eligible"] and never sees paused.
  • Nothing outside spp_programs overrides this logic, so SP-MIS and Farmer Registry pick the fix up from the shared code, as the ticket expects.

Deliberately untouched: deduplication still flags a paused member as duplicated. A duplicate is a duplicate regardless of pause, and that is a different action from the one reported here — though it does mean dedup protects exited and not_eligible while leaving paused open.

New unit tests

spp_programs/tests/test_program_enrollment.py — four assertions that a paused membership is still paused after Enroll Eligible (enrol path, disenrol sweep, and the two per-membership methods), plus one that Resume still has something to resume. Written first and confirmed failing against the unfixed code, then passing.

Unit tests executed by the author

Full spp_programs suite on this branch after merging 19.0 in: 684 tests, 0 failed, 0 errors.

How to test manually

  1. Open a registrant enrolled in a program, and on the Participation tab click Pause on the membership.
  2. Go to the program and click Enroll Eligible.
  3. The membership is still Paused — before, it went back to Enrolled.
  4. Click Resume on the membership: it returns to Enrolled, which is the only route that should do so.
  5. Make a paused member fail the eligibility rules, then run Enroll Eligible again — it stays Paused rather than being swept to not_eligible.

Related links

Pausing a membership is a program officer's explicit decision, undone only
by Resume. Enroll Eligible re-evaluated paused members along with everyone
else and wrote them back to enrolled, silently reversing that decision.

The concept was already in the code — a comment noting that duplicated and
exited "should only be changed through their own workflows" — but paused
was not in the set. Make it explicit as
constants.PROTECTED_MEMBERSHIP_STATES and apply it everywhere re-running
eligibility decides a membership's state.

Three paths, not one. The reported symptom is the enrol branch of
_enroll_eligible_registrants, but its disenrol sweep had the same gap and
wrote a paused member the eligibility manager did not return to
not_eligible — destroying the pause just as thoroughly. The per-membership
enroll_eligible_registrants and verify_eligibility on spp.program.membership
were a third: their buttons are hidden unless the record is draft, but the
methods are public and reachable over RPC, and the ticket's point is that a
pause should be trustworthy.

The async branch, taken for programs above MIN_ROW_JOB_QUEUE, dispatches
into the same _enroll_eligible_registrants, so both branches are covered.
Program-level verify_eligibility passes ["enrolled", "not_eligible"] and
never sees paused. Nothing outside spp_programs overrides this logic, so
SP-MIS and Farmer Registry pick the fix up from the shared code as the
ticket expects.

Tests were written first and confirmed failing against the unfixed code —
four assertions of 'enrolled' != 'paused' plus an error where Resume had
nothing paused left to resume — then passing after.

Deliberately untouched: deduplication still flags a paused member as
duplicated. A duplicate is a duplicate regardless of pause, and that is a
different action from the one reported here, though it does mean dedup
protects exited and not_eligible while leaving paused open.

OP#1117
@emjay0921
emjay0921 marked this pull request as ready for review August 18, 2026 07:56
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.04%. Comparing base (0820667) to head (dec8f8c).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             19.0     #436      +/-   ##
==========================================
+ Coverage   72.24%   73.04%   +0.80%     
==========================================
  Files         419      480      +61     
  Lines       29813    32228    +2415     
==========================================
+ Hits        21539    23542    +2003     
- Misses       8274     8686     +412     
Flag Coverage Δ
spp_api_v2_cycles 71.03% <ø> (ø)
spp_api_v2_entitlements 70.23% <ø> (ø)
spp_api_v2_programs 92.22% <ø> (ø)
spp_api_v2_simulation 71.19% <ø> (?)
spp_base_common 91.07% <ø> (ø)
spp_case_entitlements 100.00% <ø> (?)
spp_case_programs 100.00% <ø> (?)
spp_cr_type_assign_program 92.07% <ø> (ø)
spp_dci_compliance 93.01% <ø> (?)
spp_dci_demo 94.28% <ø> (ø)
spp_dci_server_social 89.57% <ø> (?)
spp_farmer_registry_demo 61.06% <ø> (ø)
spp_gis_report_programs 100.00% <ø> (?)
spp_grm_demo 80.13% <ø> (?)
spp_grm_programs 92.13% <ø> (?)
spp_hazard_programs 98.55% <ø> (?)
spp_programs 65.53% <100.00%> (+0.25%) ⬆️
spp_registry 87.22% <ø> (+0.07%) ⬆️
spp_security 69.56% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
spp_programs/models/constants.py 100.00% <100.00%> (ø)
spp_programs/models/managers/program_manager.py 86.47% <100.00%> (+0.08%) ⬆️
spp_programs/models/program_membership.py 67.33% <100.00%> (+8.54%) ⬆️

... and 63 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant