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.

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

Feat: in-person eligibility policies #2689

Merged
merged 12 commits into from
Feb 26, 2025
Merged

Conversation

angela-tran
Copy link
Member

@angela-tran angela-tran commented Feb 13, 2025

Closes #2632

This PR implements the functionality where, when verifying eligibility in-person, the user must agree they have used the specific eligibility criteria that is shown.

Screen recordings of behavior

(GIFs play only once. You can refresh the page to replay I think.)

User-facing behavior (for transit agency staff user)

Checkbox changes based on selected flow

benefits-in-person-dynamic-checkbox-edited

Checkbox resets if user changes selected flow

benefits-in-person-checkbox-reset-edited

Browser validation messages

benefits-in-person-browser-validation-edited

User-facing behavior (for superusers and Cal-ITP staff users)

  • We currently only have in-person policies for the Older Adult, Medicare Cardholder, and Courtesy Card flows, so any other flows that were marked as supporting In-person enrollment will be automatically updated to have that option unchecked.
  • When attempting to add In-person as a supported enrollment method, that will only work for the three flows mentioned above. For any other flows, an error message will be shown:
Error message when attempting to add `In-person` to a flow without a policy

Screenshot from 2025-02-21 15-12-59

Testing locally

Data migration

  • Start on main branch, load sample fixtures with ./bin/reset_db.sh
    • Launch app, go to /admin, and log in as cst-user. At /in_person/eligibility, you'll see the 5 in-person flows for CST.
    • Log out and log back in as a superuser. Look over your EnrollmentFlows and notice the In-person checkbox is checked.
  • Checkout this branch (git checkout feat/in-person-policies) and run the migration with ./bin/init.sh
    • Launch app, go to /admin, and log in as cst-user. At /in_person/eligibility, you'll see only "Older Adult" and "Medicare Cardholder".
    • Log out and log back in as a superuser. Looking at the EnrollmentFlows, only the "Older Adult" and "Medicare Cardholder" flows have In-person checked.

EnrollmentFlow validation enforces that in-person flows have a policy

  • Try to check In-person for a flow and save it. If it is not one of the flows with an in-person eligibility policy, you will get an error message.

a11y

I tested going through the form with only keyboard interaction, and the tab order / interactions worked as I expected.

I also inspected the accessibility tree and observed that when the checkboxes go from hidden to shown, the accessibility tree makes sense, i.e. it only shows the checkbox that is currently displayed.

@angela-tran angela-tran self-assigned this Feb 13, 2025
@github-actions github-actions bot added deployment-dev [auto] Changes that will trigger a deploy if merged to dev migrations [auto] Review for potential model changes/needed data migrations updates back-end Django views, sessions, middleware, models, migrations etc. front-end HTML/CSS/JavaScript and Django templates and removed migrations [auto] Review for potential model changes/needed data migrations updates labels Feb 13, 2025
Copy link

github-actions bot commented Feb 13, 2025

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  benefits/core/models
  enrollment.py
  benefits/in_person
  forms.py
  benefits/in_person/context
  __init__.py
  eligibility.py
Project Total  

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

@angela-tran angela-tran force-pushed the feat/in-person-policies branch 5 times, most recently from 18443be to 0ec5b92 Compare February 19, 2025 01:56
@angela-tran angela-tran force-pushed the feat/in-person-policies branch 8 times, most recently from 1f8f25e to d27a53e Compare February 21, 2025 21:10
@angela-tran angela-tran marked this pull request as ready for review February 21, 2025 22:07
@angela-tran angela-tran requested a review from a team as a code owner February 21, 2025 22:07
@angela-tran
Copy link
Member Author

Ready for review again

lalver1
lalver1 previously approved these changes Feb 25, 2025
Copy link
Member

@lalver1 lalver1 left a comment

Choose a reason for hiding this comment

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

I went through the code and tested locally and everything looked good, this is a really nice new feature! 👍

This is only a note, that Nevada County Connects is set up in test with 2 in-person flows (disability and youth) with no policy_details yet. Seems like the migration in this PR will reset these to digital so we just need to remember to reconfigure these 2 flows and add policy_details to them after we do an rc release. If this makes sense, I can create a ticket for this chore.

@thekaveman
Copy link
Member

thekaveman commented Feb 25, 2025

@lalver1 good call! Want to update this ticket? #2553

@indexing
Copy link
Member

@lalver1 Quick note: we don't have policy_details for either disability or youth yet; those won't be pathways we offer NevCo with the initial release of in-person enrollments.

@thekaveman
Copy link
Member

@indexing @lalver1

Ah, so it must have been used by us testing the interface. In any case, sounds like there is nothing to do here at all 👍

the mocked agency used by the test did not have any flows, and the test
was passing as a false positive.

use the `mocked_session_flow` fixture so that the agency has a flow.

this is similar to the fix in 3bf128c.
change the form to have a checkbox for each flow. when the user selects
the radio button for a flow, the corresponding checkbox is shown, and
the other checkboxes are hidden.

implement a template specifically for this form.
also fix spelling typo on copy for checkboxes.
previously we could rely on Django's default handling of the single
`BooleanField`, but since we have multiple checkboxes now, and we don't
want to require all of them, we need to implement our own logic to
require that the checkbox that matches the selected flow was checked.

also added Javascript to the template to show the checkbox for the case
where the form didn't pass back-end validation and therefore we show
the form again with what the user had selected.
consumer is responsible for handling any error that might occur from
the key not being found.
data migration handles removing "in_person" from flows that do not have
a policy in the `in_person.context.eligibility_index` dict.

`EnrollmentFlow.clean` handles enforcing this rule going forward when
creating or editing flows.
@angela-tran
Copy link
Member Author

After rebasing onto main which has had #2688 merged into it:

image

@angela-tran
Copy link
Member Author

angela-tran commented Feb 26, 2025

Seems like the migration in this PR will reset these to digital ...

The migration only removes in_person from flows and does not try to set any flow to digital.

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

One minor change: the local_fixtures.json should be updated so that when starting from scratch (e.g. no database, like a fresh local environment), we don't load flows marked for in-person that aren't in the context.

This happens after the database migration here, so they are loaded in an invalid state.

a flow that does not have an in-person policy (as mapped by
`in_person.context.eligibility_index`) is not allowed to support the
`in_person` enrollment method.
@angela-tran
Copy link
Member Author

angela-tran commented Feb 26, 2025

One minor change: the local_fixtures.json should be updated so that when starting from scratch (e.g. no database, like a fresh local environment), we don't load flows marked for in-person that aren't in the context.

This happens after the database migration here, so they are loaded in an invalid state.

Thanks for catching that! Updated them in 98ef80a

@thekaveman
Copy link
Member

thekaveman commented Feb 26, 2025

Thanks for catching that!

NP. But I guess I also didn't realize, we aren't supporting Agency Card here for CST? Would we want that for testing/demoing?

Forget it.

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

🪪 🔍

@angela-tran angela-tran merged commit 5a452c0 into main Feb 26, 2025
12 checks passed
@angela-tran angela-tran deleted the feat/in-person-policies branch February 26, 2025 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev front-end HTML/CSS/JavaScript and Django templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Admin: In-Person - View policy
4 participants