Refactor: Globalize enrollment flows by Scotchester · Pull Request #3509 · cal-itp/benefits

Scotchester · 2026-02-20T18:17:45Z

I think this is most of the work that needs to be done, though we still need to figure out how to handle that bad record in the prod DB. (I left in some print statements I was using to debug that.)

As discussed, it turned out to be a bit more wide-reaching change than we thought, but in the end, I think the changes are pretty straightforward. I broke them up into 3 fairly isolated commits.

~~I haven't touched the tests at all yet, and they'll certainly need some major updates.~~

Successful local tests all the way through enrollment:

CST Older Adult / Littlepay
CST Courtesy Card / Littlepay
CST Older Adult / Switchio
CST Courtesy Card / Switchio

github-actions · 2026-02-20T18:18:08Z

This PR adds a Django migration without updating any fixtures. Is a corresponding fixture update needed?

github-actions · 2026-02-23T19:00:16Z

This PR updates local_fixtures.json. After merging, please be sure to make a corresponding update to the fixtures with secrets in our shared LastPass folder and let other developers know that they should update their local fixtures accordingly.

Scotchester · 2026-02-23T22:38:12Z

I've pushed a commit that resolves almost all of the failing tests, but there are a few that remain that I'd like to discuss with the team. It's a series of five tests shown here:

https://github.com/cal-itp/benefits/actions/runs/22327070430/job/64600237234?pr=3509

test_enroll_success_flow_does_not_support_expiration_no_expiry
test_enroll_success_flow_supports_expiration
test_enroll_success_flow_supports_expiration_no_expiry
test_enroll_success_flow_supports_expiration_is_expired
test_enroll_success_flow_supports_expiration_is_within_reenrollment_window

The proximate cause is clear – they are trying to get the group_id from the test's EnrollmentFlow, which is no longer possible. I could simply get the group_id from model_LittlepayGroup, but then these tests' model_EnrollmentFlow_supports_expiration/model_EnrollmentFlow_does_not_support_expiration would be unused, which seems odd when the purpose of them is to test various expiring/not-expiring conditions.

Maybe that's OK, because they're also bringing in things like mocked_group_funding_source_with_expiry and mocked_session_enrollment_expiry, which seem to be doing the real work in these tests, but it still feels a little odd if we're not actually testing against an EnrollmentFlow that is configured with(out) expiration.

Or maybe we need some new/different assertions in these tests that factor in the EnrollmentFlow's expiration fields?

Other than those tests, I think this is ready for initial review!

thekaveman · 2026-02-24T16:06:57Z

I've pushed a commit that resolves almost all of the failing tests, but there are a few that remain that I'd like to discuss with the team. It's a series of five tests shown here:

https://github.com/cal-itp/benefits/actions/runs/22327070430/job/64600237234?pr=3509
test_enroll_success_flow_does_not_support_expiration_no_expiry
test_enroll_success_flow_supports_expiration
test_enroll_success_flow_supports_expiration_no_expiry
test_enroll_success_flow_supports_expiration_is_expired
test_enroll_success_flow_supports_expiration_is_within_reenrollment_window
The proximate cause is clear – they are trying to get the group_id from the test's EnrollmentFlow, which is no longer possible. I could simply get the group_id from model_LittlepayGroup, but then these tests' model_EnrollmentFlow_supports_expiration/model_EnrollmentFlow_does_not_support_expiration would be unused, which seems odd when the purpose of them is to test various expiring/not-expiring conditions.

Maybe that's OK, because they're also bringing in things like mocked_group_funding_source_with_expiry and mocked_session_enrollment_expiry, which seem to be doing the real work in these tests, but it still feels a little odd if we're not actually testing against an EnrollmentFlow that is configured with(out) expiration.

Or maybe we need some new/different assertions in these tests that factor in the EnrollmentFlow's expiration fields?

Other than those tests, I think this is ready for initial review!

We could consider modifying the asserts to check that session.update is called with the appropriate updates to expiration related data based on the flow. We may also want to use the fixture mocked_session_group (currently referenced but I think unused) instead of the model_LittlepayGroup directly, to make it more realistic.

github-actions · 2026-02-24T19:14:23Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
benefits/core
mixins.py
session.py
benefits/core/admin
enrollment.py
benefits/core/models
enrollment.py
transit.py
benefits/eligibility
views.py
benefits/enrollment
enrollment.py
views.py
benefits/enrollment_littlepay
enrollment.py
models.py
benefits/enrollment_switchio
enrollment.py
models.py
views.py
Project Total

_{This report was generated by python-coverage-comment-action}

Scotchester · 2026-02-24T19:15:22Z

We could consider modifying the asserts to check that session.update is called with the appropriate updates to expiration related data based on the flow. We may also want to use the fixture mocked_session_group (currently referenced but I think unused) instead of the model_LittlepayGroup directly, to make it more realistic.

Thanks for the pointer in the right direction! The latest commit updates these tests. LMK what you think.

Scotchester · 2026-02-24T19:20:48Z

Oops, need to rebase now.

Scotchester · 2026-02-24T22:33:35Z

Now fully tested!

thekaveman · 2026-02-24T22:36:14Z

Taking a look! 👀

thekaveman

Looking really good. Couple of thoughts/questions.

Scotchester · 2026-02-25T17:14:16Z

@thekaveman Changes pushed!

thekaveman

Very nice work on this, what a huge refactor!

I was able to go through all the following flows successfully:

✅ CST / Medicare / Switchio
✅ CST / Older Adult / Switchio
✅ CST / Agency card / Switchio
✅ CST / Medicare / Littlepay
✅ CST / Older Adult / Littlepay
✅ CST / Agency card / Littlepay

The Admin UI looks pretty good OOTB on an agency page, for selecting the supported flows:

It might be nicer if we can use checkboxes here instead of this default and kinda weird multi-select box that relies on color/highlighting, but that certainly isn't blocking.

I do want to clean up the large commented sections in the migration, and deal with the corrupted EnrollmentEvent record in prod (or at least have a solid plan) before merging though.

Scotchester · 2026-02-25T18:50:14Z

I do want to clean up the large commented sections in the migration, and deal with the corrupted EnrollmentEvent record in prod (or at least have a solid plan) before merging though.

Agree 💯

Here's the plan as I see it:

Today
1. ~~I figure out how to actually delete this record. Whatever bug prevents filtering on this row also seems to prevent deleting it. Still experimenting locally.~~
2. ~~Once I can do that, I test re-adding the record via the Admin (on the main branch), then switch back to this branch and test the migration.~~
3. Figured out how to repair the corruption via SQLite's recovery API and tested running the migration against it.
Tomorrow morning at 9 a.m. ET / 6 a.m. PT
1. I download the latest copy of the prod DB to my local machine
2. I repair the DB using the SQLite recovery API
3. I upload the modified DB back to the prod server, overwriting the previous file
4. I verify that the previously corrupt EnrollmentEvent can now be loaded in the prod Admin.
Tomorrow at 1:00 p.m. ET / 10: a.m. PT
1. We do the deployment and the migration should run without incident

Does that seem right?

thekaveman · 2026-02-25T19:04:32Z

I do want to clean up the large commented sections in the migration, and deal with the corrupted EnrollmentEvent record in prod (or at least have a solid plan) before merging though.

Agree 💯

Here's the plan as I see it:

...

Does that seem right?

Sounds great. Lemme know if you want to chat about fixing the corrupted record at all.

Scotchester · 2026-02-25T20:20:30Z

Updated plan in my comment above based on the DB repair research just completed.

thekaveman

LGTM 👍

jgravois · 2026-02-25T22:07:18Z


 @pytest.fixture
-def model_EnrollmentFlow(model_TransitAgency):
+# def model_EnrollmentFlow(model_TransitAgency):


i think this line is (harmless) detritus

Another good catch, thanks!

jgravois · 2026-02-25T22:08:34Z

+        3,
+        4,
+        5
+      ],


prettier collapses this line for me. did you bypass it?

No, I just hadn't gotten this current with main, as you now realize :) Will clean it up.

sorry, now i understand why @thekaveman suggested that we just merge this and untangle in #3216 🍝

iojk;hasdfiokl;hjasdfiohkl;asdfIoH;ASFIO;HKKASDFGhIkl;

I've rebased locally but Prettier is still behaving the same way. Which makes sense, because your PR didn't change anything about how VS Code handles Prettier, only pre-commit.

But still! pre-commit didn't yell at me either. I dunno, man.

I gotta run, but I'll try to figure it out in the morning.

i'll pull down your branch and play with it but personally i don't have a problem merging this PR as-is and solving the mystery when i rebase #3216.

I think the main issue I'm trying to avoid is: Scott's local working branch is now significantly out of date with this one.

In the best case, he can git fetch && git reset --hard origin/refactor/global-enrollment-flows to align with these updates.

In the worst case, he has some local work (either still in progress or committed locally but not pushed) that has to be dealt with to get in sync with this newly rebased branch.

Even if the underlying changes of the commits are the same, rebasing is still destructive to the commit history (SHAs change). We should try to avoid introducing additional merge conflicts or merge work for our colleagues unless they ask for help, is what I'm ultimately trying to say.

A good point in general, Kegan. Can definitely be annoying if a branch gets rebased out from under you. Hopefully this is the last change on this PR and I won't even have to update my local branch, though :)

understood. it was overzealous for me to -f push onto this branch without getting a greenlight from @Scotchester first. won't happen again 💂

epilogue: @Scotchester and i were able to verify this morning that the inconsistency in formatting on his machine had nothing to do with #3520.

he had a VS Code user setting pertaining to .json files specifically that sidestepped prettier entirely in favor of 'JSON Language Features'.

- Add `enrollment_flows` many-to-many field to `TransitAgency` - Add `transit_agency` FK to `EnrollmentGroup`, since groups will now need to keep track of which agency they belong to (since the flow they belong to no longer will) - Convert `EnrollmentGroup.enrollment_flow` from `OneToOneField` to `ForeignKey`, since flows will now be used by more than one group - Remove `group_id` property from `EnrollmentFlow` since they will no longer relate to a single group - Update `EnrollmentFlow.clean()` to reflect that it no longer has a `group_id` and soon will not have a `transit_agency` - Schema and data migration to handle all of the above

Since it can no longer be retrieved from a flow.

Including taking the agency name out of the selection label template name.

Co-authored-by: Kegan Maher <kegan@compiler.la>

thekaveman

🚀

lalver1

Wow, this was a big refactor! Great job @Scotchester!

The code changes look good to me, and I tested locally Older Adult and Courtesy Card and saw no issues 👍

I'm almost finished reviewing, but I did notice something with the migration. As part of looking into the corrupted database row issue, I made a copy of the prod database. Since I had that copy, I ran this PR's migration on it to get a better view of how this PR changes the data model at the SQL table level. After doing that I noticed that SacRT, MST, and SBMTD were missing the Medicare flow; and NevCo was missing the Veteran flow. I may be mistaken, and I'll double check by looking at the migration code again, but wanted to see if you had noticed this too.

lalver1 · 2026-02-26T02:33:32Z

Sorry, it was my mistake 🤦‍♂️. I was using the test database so that's why the flows were missing from those agencies.

Scotchester · 2026-02-26T14:26:12Z

@lalver1 Thank you for your diligent testing, regardless!

github-actions Bot assigned Scotchester Feb 20, 2026

github-actions Bot added migrations [auto] Review for potential model changes/needed data migrations updates back-end Django views, sessions, middleware, models, migrations etc. labels Feb 20, 2026

Scotchester force-pushed the refactor/global-enrollment-flows branch from e53ef82 to f9de81d Compare February 23, 2026 18:59

Scotchester force-pushed the refactor/global-enrollment-flows branch from 4ed611a to c13bc05 Compare February 23, 2026 22:14

Scotchester marked this pull request as ready for review February 23, 2026 22:38

Scotchester requested a review from a team as a code owner February 23, 2026 22:38

Scotchester changed the title ~~[WIP] Refactor: Globalize enrollment flows~~ Refactor: Globalize enrollment flows Feb 23, 2026

Scotchester marked this pull request as draft February 24, 2026 19:20

Scotchester force-pushed the refactor/global-enrollment-flows branch 3 times, most recently from 4c8ad5b to 163e386 Compare February 24, 2026 22:27

Scotchester marked this pull request as ready for review February 24, 2026 22:33

thekaveman reviewed Feb 24, 2026

View reviewed changes

Comment thread benefits/core/migrations/0079_remove_enrollmentflow_transit_agency.py Outdated

Comment thread benefits/enrollment_switchio/enrollment.py Outdated

Comment thread tests/pytest/core/test_session.py

Comment thread tests/pytest/conftest.py Outdated

Scotchester force-pushed the refactor/global-enrollment-flows branch from 9ad973d to ddbeaff Compare February 25, 2026 17:10

thekaveman requested changes Feb 25, 2026

View reviewed changes

Comment thread benefits/core/migrations/0078_global_enrollmentflows.py Outdated

Comment thread benefits/core/migrations/local_fixtures.json Outdated

thekaveman previously approved these changes Feb 25, 2026

View reviewed changes

thekaveman mentioned this pull request Feb 25, 2026

chore(pre-commit): autoupdate hooks #3216

Merged

jgravois reviewed Feb 25, 2026

View reviewed changes

Scotchester and others added 10 commits February 25, 2026 22:42

refactor(flows): Add group to session

04d7692

Since it can no longer be retrieved from a flow.

refactor(flows): Update enrollment handling to use session.group

e7c3219

refactor(flows): Remove EnrollmentFlow.transit_agency

ec350a4

Including taking the agency name out of the selection label template name.

test: Fix tests and one small issue they revealed

6c20cce

test: Fix failing tests around enrolling with expiring flows

7413be4

test: Restore coverage

fe8bdf1

chore: Address code review comments

b57d9f7

Co-authored-by: Kegan Maher <kegan@compiler.la>

test: Rename new session.group tests

26f2cad

chore: Remove debug output from migration

00eb7c9

jgravois dismissed thekaveman’s stale review via 46bcff5 February 25, 2026 22:45

jgravois force-pushed the refactor/global-enrollment-flows branch from 38e9169 to 46bcff5 Compare February 25, 2026 22:45

chore: prettier 3x formatting

d80877b

jgravois force-pushed the refactor/global-enrollment-flows branch from 46bcff5 to d80877b Compare February 25, 2026 22:48

thekaveman approved these changes Feb 25, 2026

View reviewed changes

lalver1 reviewed Feb 25, 2026

View reviewed changes

Scotchester merged commit a1d4e97 into main Feb 26, 2026
19 checks passed

Scotchester deleted the refactor/global-enrollment-flows branch February 26, 2026 14:26

Scotchester mentioned this pull request Feb 26, 2026

Chore: dumpdata helper script #3523

Merged

angela-tran mentioned this pull request Mar 3, 2026

Update agency onboarding scaffolding workflow to reflect global EnrollmentFlows #3549

Closed

1 task

This was referenced Mar 12, 2026

Fix: update In-Person session with group #3578

Merged

Group is not set correctly in session before enrollment begins #3581

Closed

Scotchester mentioned this pull request Mar 26, 2026

Fix issues with retry (and error) enrollment analytics #3627

Merged

+,
+,
+
+                    ],

Uh oh!

Conversation

Scotchester commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Feb 20, 2026

Uh oh!

github-actions Bot commented Feb 23, 2026

Uh oh!

Scotchester commented Feb 23, 2026

Uh oh!

thekaveman commented Feb 24, 2026

Uh oh!

github-actions Bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report

Uh oh!

Scotchester commented Feb 24, 2026

Uh oh!

Scotchester commented Feb 24, 2026

Uh oh!

Scotchester commented Feb 24, 2026

Uh oh!

thekaveman commented Feb 24, 2026

Uh oh!

thekaveman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Scotchester commented Feb 25, 2026

Uh oh!

thekaveman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Scotchester commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thekaveman commented Feb 25, 2026

Uh oh!

Scotchester commented Feb 25, 2026

Uh oh!

thekaveman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgravois Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thekaveman Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgravois Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thekaveman left a comment

Scotchester commented Feb 20, 2026 •

edited

Loading

github-actions Bot commented Feb 24, 2026 •

edited

Loading

Scotchester commented Feb 25, 2026 •

edited

Loading

jgravois Feb 25, 2026 •

edited

Loading

thekaveman Feb 25, 2026 •

edited

Loading

jgravois Feb 26, 2026 •

edited

Loading