Refactor Enrollment Flow for consistent labeling across apps #2403

lalver1 · 2024-09-25T17:23:50Z

closes #2336

What this PR does

This PR changes the implementation of the relationship between EnrollmentFlow and TransitAgency. EnrollmentFlow now has a TransitAgency foreign key field (many flows to one transit agency). This will allow us to have more flexibility with the names we can use in EnrollmentFlow.label (for example, having the name of the transit agency in this field won't be necessary anymore).
Update EnrollmentFlow labels in fixture data, to match the names used in form fields
Updates fixture data: Removed the digital-only and in-person only enrollment flows for now. It was breaking the Cypress tests, because these flows did not have selection labels and all the necessary HTML templates. Can be re-added when the enrollment flow filtering goes into effect.
Updates test failures, test fixtures. 1 failing test.
Update Admin view for Enrollment Flow to show Agency | Flow label | Supported methods
Add migration for existing data
Updates helptext on a flow's agency
EnrollmentFlow's agency is now not required and can be blank.

How to test

Run ./bin/reset_db.sh
Test both in-person and digital flows
Test re-ordering enrollment flows in Admin
Run all the tests

Screenshots

In Person Eligibility index: Same code, new database data

Admin: Enrollment Flows displayed with label, agency and supported methods

machikoyasuda · 2024-09-25T17:27:07Z

I'll take the rest of the work of this PR - fixing tests and anything else necessary.

benefits/core/migrations/0028_remove_transitagency_enrollment_flows_and_more.py

github-actions · 2024-09-26T16:34:14Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
benefits/core
admin.py
models.py
Project Total

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

angela-tran · 2024-09-26T16:37:24Z

@machikoyasuda Can you update the title of this PR to reflect that it covers more than just the relationship refactor? I think it is covering all of #2336

machikoyasuda · 2024-09-26T18:08:57Z

@angela-tran I couldn't think of a short way to summarize all the changes into one good PR headline 😅

angela-tran · 2024-09-26T18:17:39Z

@angela-tran I couldn't think of a short way to summarize all the changes into one good PR headline 😅

😆 It is a bit long of a title, but it does cover all the technical changes, so I think it's ok.

I guess from a more abstract level, this PR is making it so we don't need to have the transit agency name in the label, so I think another title that could work is Refactor: move transit agency name from EnrollmentFlow.label to admin UI or something like that.

Up to you, either one is fine with me!

thekaveman · 2024-09-26T18:20:12Z

Shorter is better I think... remember this will be the commit message when the PR is merged!

angela-tran

The overall changes look great! There are just a few code clean-ups I think we should do.

benefits/core/migrations/0028_remove_transitagency_enrollment_flows_and_more.py

benefits/core/migrations/local_fixtures.json

tests/pytest/conftest.py

tests/pytest/core/test_models.py

tests/pytest/core/test_views.py

angela-tran · 2024-09-27T17:39:44Z

Comments for lines 351-354:

benefits/benefits/core/models.py

Lines 351 to 354 in 4e7cdbe

    
           label = models.TextField( 
        
               null=True, 
        
               help_text="A human readable label, not shown to end-users. Used as the display text in Admin.", 
        
           )

Do we consider transit agency staff users as "end-users"? I'm wondering if we should remove the part that says not shown to end-users.

angela-tran

The updates look good! Just one more change I see as needed to make sure the migration works.

benefits/core/models.py

machikoyasuda · 2024-09-30T23:00:52Z

Testing the new helptext in Admin:

This text makes way more sense.

…gency This commit changes the implementation of the relationship between EnrollmentFlow and TransitAgency. EnrollmentFlow now has a TransitAgency foreign key field (many flows to one transit agency). This will allow us to have more flexibility with the names we can use in EnrollmentFlow.label (for example, having the name of the transit agency in this field won't be necessary anymore).

…ixture

… tests

…ow blanks for admin

angela-tran

Migration worked, tests pass, app launches locally, admin interface shows new field / help text - looks great ✅

Thanks @machikoyasuda !

thekaveman

This is such a great change 🤩 I love seeing our models get so clean and usable 👍

machikoyasuda self-assigned this Sep 25, 2024

angela-tran mentioned this pull request Sep 26, 2024

Refactor EnrollmentFlow model so in-person eligibility view can use copy from the selection label template #2336

Closed

4 tasks

angela-tran reviewed Sep 26, 2024

View reviewed changes

benefits/core/migrations/0028_remove_transitagency_enrollment_flows_and_more.py Outdated Show resolved Hide resolved

machikoyasuda changed the title ~~Refactor: EnrollmentFlow and TransitAgency relationship~~ Refactor: Refactor TransitAgency/EnrollmentFlow model for proper in person label form view + Admin view updates + Migration Sep 26, 2024

machikoyasuda marked this pull request as ready for review September 26, 2024 18:09

machikoyasuda requested a review from a team as a code owner September 26, 2024 18:09

machikoyasuda force-pushed the refactor/transit-agency-enrollment-flow branch from 04cda58 to 3e56fe8 Compare September 26, 2024 18:09

machikoyasuda changed the title ~~Refactor: Refactor TransitAgency/EnrollmentFlow model for proper in person label form view + Admin view updates + Data migration~~ Refactor: Refactor Enrollment Flow for consistent flow labeling across apps Sep 26, 2024

machikoyasuda changed the title ~~Refactor: Refactor Enrollment Flow for consistent flow labeling across apps~~ Refactor Enrollment Flow for consistent flow labeling across apps Sep 26, 2024

machikoyasuda changed the title ~~Refactor Enrollment Flow for consistent flow labeling across apps~~ Refactor Enrollment Flow for consistent labeling across apps Sep 26, 2024

angela-tran requested changes Sep 27, 2024

View reviewed changes

machikoyasuda force-pushed the refactor/transit-agency-enrollment-flow branch from 3e56fe8 to 4e7cdbe Compare September 27, 2024 06:10

machikoyasuda force-pushed the refactor/transit-agency-enrollment-flow branch from 884cfb3 to f72819f Compare September 30, 2024 06:19

machikoyasuda requested a review from angela-tran September 30, 2024 06:32

angela-tran requested changes Sep 30, 2024

View reviewed changes

benefits/core/models.py Outdated Show resolved Hide resolved

machikoyasuda force-pushed the refactor/transit-agency-enrollment-flow branch from d1e2225 to 12643bb Compare September 30, 2024 23:36

machikoyasuda force-pushed the refactor/transit-agency-enrollment-flow branch from 890cd55 to ee92b87 Compare October 1, 2024 20:07

thekaveman added this to the Admin tool: in-person eligibility verification milestone Oct 1, 2024

lalver1 and others added 19 commits October 1, 2024 20:37

chore(fixtures): remove enrollment_methods field from transitagency f…

c621385

…ixture

test(fix): update enrollmentflow, agency changes on fixtures

f401abe

fix(fixtures): add transit_agency_id to enrollmentflow

68864cc

chore(fixtures): remove (CST) from flow label

208cf21

chore(fixtures): give enrollmentflow label the human-readable name

a73179e

test: update agency/flows relationship in tests

69f388e

fix(fixtures): remove digital, inperson only flows for now - breaking…

2e2e1c0

… tests

feat(admin): display agency, supported methods on enrollmentflow list

a880802

test(views): fix last test w/ mocked flow+session

3bf128c

feat(migration): add data migration so existing flows have an agency

6afab54

refactor(migration): rename migration to a more descriptive title

fc2139d

fix(fixtures): set transitagency directly with object, not id

5f92ab3

fix(tests): assign transit_agency directly, not by id

b73826d

fix: remove old migration

5d01dea

refactor(db): re-word helptext for enrollmentflow label

b2f0840

feat(db): allow enrollmentflow.transit_agency to be null

4017f48

fix(migration): allow enrollmentflow's transit agency to be null, all…

a230cbf

…ow blanks for admin

refactor(db): combine migration into 1 and add migrate_data()

2ffa3d1

machikoyasuda force-pushed the refactor/transit-agency-enrollment-flow branch from ee92b87 to 2ffa3d1 Compare October 2, 2024 03:37

angela-tran mentioned this pull request Oct 2, 2024

Eligibility: Filter enrollment flows by supported method #2420

Merged

chore(tests): remove socket-disabling code

4d7f09f

angela-tran approved these changes Oct 2, 2024

View reviewed changes

machikoyasuda requested a review from thekaveman October 2, 2024 19:52

thekaveman approved these changes Oct 2, 2024

View reviewed changes

machikoyasuda merged commit 00d0009 into main Oct 2, 2024
15 checks passed

machikoyasuda deleted the refactor/transit-agency-enrollment-flow branch October 2, 2024 20:27

angela-tran mentioned this pull request Nov 6, 2024

Clean up EnrollmentFlow labels in dev, test, and prod to remove prefix #2511

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Enrollment Flow for consistent labeling across apps #2403

Refactor Enrollment Flow for consistent labeling across apps #2403

lalver1 commented Sep 25, 2024 •

edited by machikoyasuda

Loading

machikoyasuda commented Sep 25, 2024

github-actions bot commented Sep 26, 2024 •

edited

Loading

angela-tran commented Sep 26, 2024

machikoyasuda commented Sep 26, 2024

angela-tran commented Sep 26, 2024

thekaveman commented Sep 26, 2024

angela-tran left a comment

angela-tran commented Sep 27, 2024

angela-tran left a comment

machikoyasuda commented Sep 30, 2024

angela-tran left a comment

thekaveman left a comment

Refactor Enrollment Flow for consistent labeling across apps #2403

Refactor Enrollment Flow for consistent labeling across apps #2403

Conversation

lalver1 commented Sep 25, 2024 • edited by machikoyasuda Loading

What this PR does

How to test

Screenshots

machikoyasuda commented Sep 25, 2024

github-actions bot commented Sep 26, 2024 • edited Loading

Coverage report

angela-tran commented Sep 26, 2024

machikoyasuda commented Sep 26, 2024

angela-tran commented Sep 26, 2024

thekaveman commented Sep 26, 2024

angela-tran left a comment

Choose a reason for hiding this comment

angela-tran commented Sep 27, 2024

angela-tran left a comment

Choose a reason for hiding this comment

machikoyasuda commented Sep 30, 2024

angela-tran left a comment

Choose a reason for hiding this comment

thekaveman left a comment

Choose a reason for hiding this comment

lalver1 commented Sep 25, 2024 •

edited by machikoyasuda

Loading

github-actions bot commented Sep 26, 2024 •

edited

Loading