fix(spp_programs): stop Enroll Eligible undoing a deliberate pause - #436
Open
emjay0921 wants to merge 2 commits into
Open
fix(spp_programs): stop Enroll Eligible undoing a deliberate pause#436emjay0921 wants to merge 2 commits into
emjay0921 wants to merge 2 commits into
Conversation
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
marked this pull request as ready for review
August 18, 2026 07:56
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
duplicatedandexited"should only be changed through their own workflows" — butpausedwas not in the set.How was the change implemented?
constants.PROTECTED_MEMBERSHIP_STATESand applied everywhere re-running eligibility decides a membership's state._enroll_eligible_registrants, but its disenrol sweep had the same gap and would write a paused member the eligibility manager did not return tonot_eligible— destroying the pause just as thoroughly. The per-membershipenroll_eligible_registrantsandverify_eligibilityonspp.program.membershipwere 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.MIN_ROW_JOB_QUEUE, dispatches into the same_enroll_eligible_registrants, so both branches are covered. Program-levelverify_eligibilitypasses["enrolled", "not_eligible"]and never sees paused.spp_programsoverrides 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
exitedandnot_eligiblewhile leavingpausedopen.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_programssuite on this branch after merging19.0in: 684 tests, 0 failed, 0 errors.How to test manually
not_eligible.Related links