Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Only show supported flows for In-person and Digital flows #2399

Closed
wants to merge 13 commits into from
Closed

Only show supported flows for In-person and Digital flows #2399

wants to merge 13 commits into from

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Sep 24, 2024 •
edited
Loading

closes #2396

What this PR does

  • Adds filter() calls to both Eligibility form methods
  • Updates test mock to return an array of flows
  • Adds tests to the Digital and In-Person flows, testing a scenario in which an agency has 3 flows (digital, in-person and both( and the page should only show 2.

How to test

  • Log into Admin as a superuser and set different enrollment flows to be either in-person-only or digital-only.
  • Log into Admin as a CS user to see that digital-only is filtered out
  • Go to Benefits digital to see that in-person-only is filtered out

Note: Either digital-only or in-person-only HAVE to be checked. You can't set a flow to be neither.

@github-actions github-actions bot added back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev labels Sep 24, 2024
@machikoyasuda machikoyasuda self-assigned this Sep 24, 2024
@github-actions GitHub Actions
Copy link

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

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)		Lines missing
  benefits/eligibility
  forms.py
  benefits/in_person
  forms.py
Project Total  

This report was generated by python-coverage-comment-action

@machikoyasuda machikoyasuda marked this pull request as ready for review September 24, 2024 17:58
@machikoyasuda machikoyasuda requested a review from a team as a code owner September 24, 2024 17:58
angela-tran
angela-tran reviewed Sep 24, 2024
View reviewed changes
tests/pytest/eligibility/test_views.py
Comment on lines 104 to 140
mock_manager = mocker.Mock()
mock_manager.all.return_value = [model_EnrollmentFlow]
type(mock_agency).enrollment_flows = mocker.PropertyMock(return_value=mock_manager)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if these lines are still needed or if they can be removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm probably not, since we aren't calling the .all helper anymore, right? I guess we just need to mock the .filter helper.

Copy link
Member Author

@machikoyasuda machikoyasuda Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! It shouldn't need it but for some reason right now it does for tests to pass.

Marking this PR as In Progress/Draft again b/c I'm trying to write a different spec to test the filtering function.

@machikoyasuda machikoyasuda marked this pull request as draft September 24, 2024 22:15
@machikoyasuda machikoyasuda changed the base branch from main to refactor/transit-agency-enrollment-flow September 26, 2024 18:27
@machikoyasuda machikoyasuda marked this pull request as ready for review September 26, 2024 19:52
@machikoyasuda machikoyasuda marked this pull request as draft September 26, 2024 20:57
@machikoyasuda machikoyasuda marked this pull request as ready for review September 27, 2024 06:08
lalver1 and others added 11 commits September 26, 2024 23:10
@lalver1 @machikoyasuda
refactor(models): change relationship for EnrollmentFlow and TransitA…
598c467 
…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).
@machikoyasuda
chore(fixtures): remove enrollment_methods field from transitagency f…
f9ad809 
…ixture
@machikoyasuda
test(fix): update enrollmentflow, agency changes on fixtures
d18971e
@machikoyasuda
fix(fixtures): add transit_agency_id to enrollmentflow
03024a5
@machikoyasuda
chore(fixtures): remove (CST) from flow label
57440dd
@machikoyasuda
chore(fixtures): give enrollmentflow label the human-readable name
a5295c3
@machikoyasuda
test: update agency/flows relationship in tests
91002f4
@machikoyasuda
fix(fixtures): remove digital, inperson only flows for now - breaking…
6c08166 
… tests
@machikoyasuda
feat(admin): display agency, supported methods on enrollmentflow list
39a5cb0
@machikoyasuda
test(views): fix last test w/ mocked flow+session
f41e34c
@machikoyasuda
feat(migration): add data migration so existing flows have an agency
4e7cdbe
@machikoyasuda machikoyasuda force-pushed the refactor/transit-agency-enrollment-flow branch from 3e56fe8 to 4e7cdbe Compare September 27, 2024 06:10
@machikoyasuda machikoyasuda force-pushed the feat/2396-filter-supported-enrollment-methods branch 2 times, most recently from e53ef3b to 807b0b3 Compare September 27, 2024 06:51
@machikoyasuda machikoyasuda marked this pull request as draft September 27, 2024 06:52
@machikoyasuda machikoyasuda changed the base branch from refactor/transit-agency-enrollment-flow to main September 27, 2024 06:55
@machikoyasuda machikoyasuda force-pushed the feat/2396-filter-supported-enrollment-methods branch from 807b0b3 to d63332c Compare September 27, 2024 07:02
@machikoyasuda machikoyasuda changed the base branch from main to refactor/transit-agency-enrollment-flow September 27, 2024 07:02
@machikoyasuda machikoyasuda force-pushed the feat/2396-filter-supported-enrollment-methods branch from d63332c to 0366d1f Compare September 27, 2024 07:06
@machikoyasuda machikoyasuda force-pushed the feat/2396-filter-supported-enrollment-methods branch from 0366d1f to aa4676e Compare September 27, 2024 07:10
@machikoyasuda machikoyasuda force-pushed the feat/2396-filter-supported-enrollment-methods branch from aa4676e to 81bfcd3 Compare September 27, 2024 07:16
@machikoyasuda machikoyasuda force-pushed the refactor/transit-agency-enrollment-flow branch 3 times, most recently from 12643bb to 890cd55 Compare October 1, 2024 00:09
@machikoyasuda machikoyasuda changed the base branch from refactor/transit-agency-enrollment-flow to main October 1, 2024 04:58
@machikoyasuda machikoyasuda changed the base branch from main to refactor/transit-agency-enrollment-flow October 1, 2024 04:59
@machikoyasuda machikoyasuda changed the base branch from refactor/transit-agency-enrollment-flow to main October 1, 2024 04:59
@machikoyasuda machikoyasuda changed the base branch from main to refactor/transit-agency-enrollment-flow October 1, 2024 06:51
@angela-tran angela-tran mentioned this pull request Oct 1, 2024
Merged
@machikoyasuda machikoyasuda deleted the feat/2396-filter-supported-enrollment-methods branch October 2, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angela-tran angela-tran angela-tran left review comments

@thekaveman thekaveman thekaveman left review comments

Assignees

@machikoyasuda machikoyasuda

Labels
back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev
Projects
None yet
Milestone
Admin tool: in-person eligibility ver...
Development

Successfully merging this pull request may close these issues.

Use supported_enrollment_methods field to filter flows in digital/in-person views
4 participants
@machikoyasuda @thekaveman @angela-tran @lalver1