Skip to content

Conversation

@thekaveman
Copy link
Member

@thekaveman thekaveman commented Dec 16, 2025

Closes #3396

  • Moves __init__ kwargs to class attribute for easier inheritance
  • Moves default attribute values into base class where appropriate to reduce duplication
  • Adds server-side validation (not strictly necessary, since we also do HTML client-side validation)
  • Refactors tests to be class-based, check valid and invalid data states

Reviewing

To get setup, start with the fixtures file in LastPass for Littlepay, and add the following entries:

{
  "model": "core.transitagency",
  "pk": 2,
  "fields": {
    "active": true,
    "slug": "mst",
    "short_name": "MST (local)",
    "long_name": "MST (local)",
    "info_url": "https://www.agency-website.com",
    "phone": "1-800-555-5555",
    "eligibility_api_config": 1,
    "logo": "agencies/cst.png"
  }
},
{
  "model": "core.transitagency",
  "pk": 3,
  "fields": {
    "active": true,
    "slug": "sbmtd",
    "short_name": "SBMTD (local)",
    "long_name": "SBMTD (local)",
    "info_url": "https://www.agency-website.com",
    "phone": "1-800-555-5555",
    "eligibility_api_config": 1,
    "logo": "agencies/cst.png"
  }
},
{
  "model": "core.eligibilityapiverificationrequest",
  "pk": 2,
  "fields": {
    "label": "courtesy_card",
    "api_url": "http://server:8000/verify",
    "api_auth_header": "X-Server-API-Key",
    "api_auth_key_secret_name": "agency-card-flow-api-auth-key",
    "api_public_key": 1,
    "api_jwe_cek_enc": "A256CBC-HS512",
    "api_jwe_encryption_alg": "RSA-OAEP",
    "api_jws_signing_alg": "RS256"
  }
},
{
  "model": "core.eligibilityapiverificationrequest",
  "pk": 3,
  "fields": {
    "label": "mobility_pass",
    "api_url": "http://server:8000/verify",
    "api_auth_header": "X-Server-API-Key",
    "api_auth_key_secret_name": "agency-card-flow-api-auth-key",
    "api_public_key": 1,
    "api_jwe_cek_enc": "A256CBC-HS512",
    "api_jwe_encryption_alg": "RSA-OAEP",
    "api_jws_signing_alg": "RS256"
  }
},
{
  "model": "core.enrollmentflow",
  "pk": 6,
  "fields": {
    "system_name": "courtesy_card",
    "label": "Courtesy Cardholder",
    "display_order": 6,
    "api_request": 2,
    "supported_enrollment_methods": ["digital"],
    "transit_agency": 2
  }
},
{
  "model": "core.enrollmentflow",
  "pk": 7,
  "fields": {
    "system_name": "mobility_pass",
    "label": "Mobility Pass Cardholder",
    "display_order": 7,
    "api_request": 3,
    "supported_enrollment_methods": ["digital"],
    "transit_agency": 3
  }
},

Then for each of CST, MST, and SBMTD, confirm:

  1. The agency card option displays on the /eligibility page
  2. Selecting the agency card option displays the agency-specific language on the /eligibility/start page
  3. Continuing to the /eligibility/confirm page shows the agency-specific form/language
  4. Submit the CST form with valid sample data, see the eligibility confirmation page / enrollment index
  5. Submit MST and SBMTD with any sample data, see the ineligible page

@github-actions github-actions bot added back-end Django views, sessions, middleware, models, migrations etc. tests Related to automated testing (unit, UI, integration, etc.) labels Dec 16, 2025
@github-actions
Copy link

github-actions bot commented Dec 16, 2025

Coverage report

Click to see where and how coverage changed

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

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

@thekaveman thekaveman force-pushed the refactor/eligibility-verification-forms branch 4 times, most recently from 6a879b7 to c459fb3 Compare December 18, 2025 17:30
@thekaveman thekaveman force-pushed the refactor/eligibility-verification-forms branch from c459fb3 to 3a488cc Compare January 7, 2026 20:40
we can move the __init__ args to class attributes, following the more
typical Django style for e.g. class-based views and other objects,
removing the need for __init__ definitions in child form classes and
reducing duplication and the amount of overrides needed

additionally, we have CSTAgencyCard inherit from MSTCourtesyCard, as
this is really a special case of MSTCourtesyCard used only for testing
purposes
test valid and invalid data

refactor EnrollmentFlowSelectionForm tests to follow the same pattern
@thekaveman thekaveman force-pushed the refactor/eligibility-verification-forms branch from 3a488cc to 360421e Compare January 12, 2026 19:33
@thekaveman thekaveman marked this pull request as ready for review January 12, 2026 20:36
@thekaveman thekaveman requested a review from a team as a code owner January 12, 2026 20:37
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.

This is a nice cleanup, it looks good to me! I also tested locally and saw no issues.

While going over the new class attributes I did notice that originally name_placeholder for CST was Hernandez-Demarcos but in this PR it becomes Garcia due to the default value of the attribute. This doesn't affect anything though so we're good 👍

@thekaveman
Copy link
Member Author

@angela-tran I saw your 👀 on the PR description -- just checking if you wanted to review before I merge?

@angela-tran
Copy link
Member

@angela-tran I saw your 👀 on the PR description -- just checking if you wanted to review before I merge?

That's ok, you can go ahead with merging. I've skimmed through most of the changes and didn't have any pending thoughts or anything. Thanks for checking!

@thekaveman thekaveman merged commit 35dae6f into main Jan 14, 2026
14 checks passed
@thekaveman thekaveman deleted the refactor/eligibility-verification-forms branch January 14, 2026 17:16
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: refactor existing Eligibility forms

3 participants