Skip to content

chore: Relax agency card form class check#3481

Merged
Scotchester merged 8 commits intomainfrom
chore/relax-agency-card-check
Feb 18, 2026
Merged

chore: Relax agency card form class check#3481
Scotchester merged 8 commits intomainfrom
chore/relax-agency-card-check

Conversation

@Scotchester
Copy link
Member

@Scotchester Scotchester commented Feb 11, 2026

Closes #3398


We no longer want to worry about matching agency slugs, only the flow's system_name. This will allow agencies to honor other agencies' programs.

Testing instructions

  1. Update .devcontainer/server/settings.py to change IMPORT_FILE_PATH to https://raw.githubusercontent.com/cal-itp/eligibility-server/cd5fed032524e4753a97573a7caef6fa17ea398b/data/server.csv
  2. Start the devcontainer (or restart the server container if it was already running)
  3. Shell into the server container
  4. Run flask drop-db followed by flask init-db
    • This is necessary because I believe there is not currently a way to tell the server container to reset its DB when starting up via the devcontainer.
    • You should see output for the second command looking like this:
      calitp@10c2a337d4b7:/calitp/app$ flask init-db
      [2026-02-11 19:59:01,483] WARNING eligibility_server.sentry:61 SENTRY_DSN not set, so won't send events
      [2026-02-11 19:59:01,483] INFO eligibility_server.app:33 Starting Eligibility Server 0.1.dev1+g12a60a598
      Creating table...
      Table created.
      Importing users from: https://raw.githubusercontent.com/cal-itp/eligibility-server/     cd5fed032524e4753a97573a7caef6fa17ea398b/data/server.csv
      [2026-02-11 19:59:01,496] DEBUG urllib3.connectionpool:1049 Starting new HTTPS connection (1): raw.githubusercontent.     com:443
      [2026-02-11 19:59:01,622] DEBUG urllib3.connectionpool:544 https://raw.githubusercontent.com:443 "GET /cal-itp/     eligibility-server/cd5fed032524e4753a97573a7caef6fa17ea398b/data/server.csv HTTP/1.1" 200 2150
      Users added: 26
      Eligibility types added: 1
      Database metadata updated.
      
  5. Start the application and go through the CST Agency Cardholder flow, using standard Courtesy Card test data.

@github-actions github-actions bot added the migrations [auto] Review for potential model changes/needed data migrations updates label Feb 11, 2026
@github-actions github-actions bot added tests Related to automated testing (unit, UI, integration, etc.) back-end Django views, sessions, middleware, models, migrations etc. and removed migrations [auto] Review for potential model changes/needed data migrations updates labels Feb 11, 2026
@github-actions
Copy link

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.

@github-actions
Copy link

github-actions bot commented Feb 11, 2026

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  benefits/core
  views.py 148
  benefits/core/models
  enrollment.py
  benefits/eligibility
  forms.py
  views.py
  benefits/enrollment
  views.py
Project Total  

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

@Scotchester Scotchester changed the title Chore/relax agency card check chore: Relax agency card form class check Feb 11, 2026
@Scotchester Scotchester marked this pull request as ready for review February 11, 2026 21:29
@Scotchester Scotchester requested a review from a team as a code owner February 11, 2026 21:29
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.

Hmm, I guess I didn't realize how far-reaching it would be to remove agency_card as an option from SystemName. Not that I disagree!

Part of the reason we set up CST was to have a "language neutral" agency for demos etc. that wouldn't favor/throw shade on a real agency, and this takes us away from that idea while majorly simplifying the overall codebase, so I think this is the right direction.

I'll just recommend that we also remove the CSTAgencyCard form and its associated copy as well, since it seems that is no longer relevant with these changes.

Copy link
Member

@jgravois jgravois left a comment

Choose a reason for hiding this comment

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

nice work @Scotchester. i'm all for nuking agency_card. 💣

f"This agency/flow combination does not support Eligibility API verification: "
f"{view.agency.slug}, {view.flow.system_name}"
)
expected_error_msg = "The senior flow does not support Eligibility API verification."
Copy link
Member

@jgravois jgravois Feb 13, 2026

Choose a reason for hiding this comment

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

my (slight) preference would be to reuse, but no offense taken if you disagree though.

its very unlikely that someone could change the variable above and miss this reference immediately below.

Suggested change
expected_error_msg = "The senior flow does not support Eligibility API verification."
expected_error_msg = f"The {view.flow.system_name} flow does not support Eligibility API verification."

We no longer worry about matching agency slugs, only the flow's `system_name`.
@Scotchester Scotchester marked this pull request as draft February 17, 2026 20:44
@Scotchester Scotchester force-pushed the chore/relax-agency-card-check branch from 8c3c050 to 837fa05 Compare February 17, 2026 21:08
@Scotchester
Copy link
Member Author

Rebased and feedback addressed! Ready for final review.

@Scotchester Scotchester marked this pull request as ready for review February 17, 2026 21:12
jgravois
jgravois previously approved these changes Feb 18, 2026
Copy link
Member

@jgravois jgravois left a comment

Choose a reason for hiding this comment

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

this code all looks reasonable to me, i didn't have any trouble running the migration locally. afterward the canned Agency card self-serve enrollment flow worked a treat 😄

Your card information may not have been entered correctly.

The number and last name must be entered exactly as they appear on your MST Courtesy Card. Please check your card and try again, or contact your transit agency for help.

https://github.com/cal-itp/eligibility-server/blob/6a26ce54d79c53f5a1f62bd96b13fe3326dccac8/data/server.csv#L2

i'm under the impression that is expected because the flow isn't configured correctly for e2e testing on main locally either.

we can confirm that all is still well on https://dev-benefits.calitp.org after merging. no offense taken if anyone else wants other eyes on this PR before you merge it though.

@Scotchester
Copy link
Member Author

i'm under the impression that is expected because the flow isn't configured correctly for e2e testing on main locally either.

Did you follow the full set of testing instructions I put in the OP? No shame if you overlooked them, I just want to understand why they didn't work if you did.

Co-authored-by: john gravois <jagravois@gmail.com>
@jgravois
Copy link
Member

Did you follow the full set of testing instructions I put in the OP?

i did not and i disagree with you. shame on me! 🤦‍♂️

Copy link
Member

@jgravois jgravois left a comment

Choose a reason for hiding this comment

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

RTFM and confirmed this works e2e. nice work @Scotchester 💪

@Scotchester Scotchester dismissed thekaveman’s stale review February 18, 2026 20:52

Requested change has been made.

@Scotchester
Copy link
Member Author

Thanks @jgravois! Could you also give this a review so I can merge them at the same time? cal-itp/eligibility-server#597

Copy link
Member

@angela-tran angela-tran left a comment

Choose a reason for hiding this comment

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

This looks good to me too. We just need @thekaveman's review and then will be good to go Oh nvm, didn't realize you could dismiss the review like that 😅

@Scotchester
Copy link
Member Author

I'll hold on just the same to make sure he confirms I removed what he asked for :)

angela-tran added a commit that referenced this pull request Feb 18, 2026
this removes the usage of AgencySlug from the ConfirmView
angela-tran added a commit that referenced this pull request Feb 18, 2026
this removes the usage of AgencySlug from the ConfirmView
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.

I didn't go through the QA steps, but my requested change was addressed!

Thanks everyone for the work on this 👍

@Scotchester Scotchester merged commit 576450d into main Feb 18, 2026
19 checks passed
@Scotchester Scotchester deleted the chore/relax-agency-card-check branch February 18, 2026 21:56
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. tests Related to automated testing (unit, UI, integration, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agency cards: relax and/or update form class check

4 participants