Skip to content

[ProgramSlugUrl] Refactor ineligible page to be associated with a GET route - #13367

Merged
rockycodes merged 10 commits into
mainfrom
rockycodes/refactor-ineligble-route
Jun 1, 2026
Merged

[ProgramSlugUrl] Refactor ineligible page to be associated with a GET route#13367
rockycodes merged 10 commits into
mainfrom
rockycodes/refactor-ineligble-route

Conversation

@rockycodes

@rockycodes rockycodes commented May 28, 2026

Copy link
Copy Markdown
Contributor

Description

Refactor the way we serve up the ineligible page so that it is served as a GET request from it's own controller, rather than rendered directly from a POST request when an applicant is found ineligible while updating their answers. This is necessary to allow language selection to work with program slug urls. Also pages should generally be accessible by a GET request, so this is fixing some less-than-ideal code paths we introduced at some point.

I used Claude to help me figure out the error handling in ApplicantProgramIneligibleController and to audit my test coverage (it suggested testing a bunch of error paths, but that seems like overkill to me so I left those off).

Checklist

General

Read the full guidelines for PRs here

  • Added the correct label (see docs for more info): < feature | enhancement | bug | under-development | dependencies | infrastructure | ignore-for-release | database >
  • Assigned to a specific person, civiform/developers, or a more specific round-robin list
  • Added an additional reviewer from civiform/eng-admin as FYI (if this PR includes functionality changes and neither you nor the primary reviewer is an admin)
  • Added an additional reviewer as required if changes potentially affect the security of the application.
  • Removed the release notes section if the title is sufficient for the release notes description, or put more details in that section.
  • Created unit and/or browser tests which fail without the change (if possible)
  • Performed manual testing (Chrome and Firefox if it includes front-end changes)
  • Extended the README / documentation, if necessary. For user-facing features, consider updating the user docs. For "under-the-hood" changes or things more relevant to developers, consider updating the dev wiki.
  • Ensured PII wasn't added to any new logs, unless it was guarded by isDevOrStaging
  • Noted in the PR description which, if any, code in this PR was generated by AI.

Instructions for manual testing

With the PROGRAM_SLUG_URLS_ENABLED flag on:

  1. As an admin, create a program with an eligibility condition (any question type but address question)
  2. As an applicant, fill out the program and answer the question so you are ineligible --> see the ineligible screen
  3. Switch the language on the ineligible screen --> see the translated ineligible screen
  4. Click "edit my responses" to be taken back to the review screen
  5. Click "submit application" again --> see ineligible screen again
  6. Switch the language on the ineligible screen --> see the translated ineligible screen

With the PROGRAM_SLUG_URLS_ENABLED flag off:

  1. All of the above steps should work in addition to...
  2. As and admin, add an address question with address correction enabled and an eligibility condition attached to the address question
  3. As an applicant, fill out the program and answer the address question so you are ineligible --> see the ineligible screen
  4. Switch the language on the ineligible screen --> get taken back to the screen with the address question (existing behavior)
  5. Go to the review screen
  6. Click "submit application" --> see ineligible screen again
  7. Switch the language on the ineligible screen --> get taken back to the review screen (existing behavior)

Issue(s) this completes

Fixes #13365

@codecov

codecov Bot commented May 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.45%. Comparing base (6001d06) to head (8f0e231).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
...rs/applicant/ApplicantProgramBlocksController.java 0.00% 4 Missing ⚠️
...ontrollers/admin/AdminProgramBlocksController.java 62.50% 3 Missing ⚠️
...controllers/admin/AdminProgramImageController.java 0.00% 3 Missing ⚠️
...s/admin/AdminProgramBlockPredicatesController.java 50.00% 1 Missing ⚠️
.../app/controllers/admin/AdminProgramController.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13367      +/-   ##
============================================
- Coverage     81.49%   81.45%   -0.04%     
- Complexity     5354     5361       +7     
============================================
  Files           595      597       +2     
  Lines         23982    24023      +41     
  Branches       1773     1777       +4     
============================================
+ Hits          19544    19568      +24     
- Misses         3779     3797      +18     
+ Partials        659      658       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rockycodes rockycodes changed the title Rockycodes/refactor ineligble route [ProgramSlugUrl] Refactor ineligible page to be associated with a GET route May 29, 2026
@rockycodes rockycodes added the enhancement Enhancement of an existing feature or functionality. label May 29, 2026
@rockycodes
rockycodes marked this pull request as ready for review May 29, 2026 21:14
Comment thread browser-test/src/support/applicant_questions.ts Outdated
Comment thread server/app/views/applicant/ApplicantBaseView.java
Comment thread server/conf/routes
Comment thread server/conf/routes
@rockycodes
rockycodes requested review from a team and gwendolyngoetz and removed request for a team May 29, 2026 21:47
Comment thread browser-test/src/support/applicant_questions.ts Outdated
Comment thread server/app/controllers/applicant/ApplicantProgramIneligibleController.java Outdated
Comment thread server/app/controllers/applicant/ApplicantProgramIneligibleController.java Outdated
@rockycodes

rockycodes commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

@gwendolyngoetz these are the warnings I'm seeing. But maybe it's pointing to other issues in that file and I just assumed those lines were included because they're using page methods
image

@gwendolyngoetz

gwendolyngoetz commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

@gwendolyngoetz these are the warnings I'm seeing. But maybe it's pointing to other issues in that file and I just assumed those lines were included because they're using page methods

I think that's a bug in what they are considering a locator. Playwright clearly calls page.getByText a locator. I recommend we disable this rule; I think it causes more harm than good.

@rockycodes

Copy link
Copy Markdown
Contributor Author

I think that's a bug in what they are considering a locator. Playwright clearly calls page.getByText a locator. I recommend we disable this rule; I think it causes more harm than good.

I'll remove that change from this PR and make a follow-up PR to disable that rule. It is very noisy!

@rockycodes
rockycodes merged commit e4d33b3 into main Jun 1, 2026
25 checks passed
@rockycodes
rockycodes deleted the rockycodes/refactor-ineligble-route branch June 1, 2026 17:21
@shreyachatterjee00

Copy link
Copy Markdown
Contributor
  1. With the flag OFF, it all works but I'm getting this with whatever address I enter, Seattle or not. Seems wrong - and don't we usually show the address before asking them to confirm?
Screenshot 2026-06-12 at 12 36 43 PM
  1. With flag ON, I'm getting this error.
    Steps to repro:
  • on "test eligibility program" (one name + one address question), enter the name to be ineligible
  • click 'edit my responses' get taken to the review page, and enter the address question with the name still in-eligible
  • click continue, back, or review to see the error page
  • even if I change the name to be eligible, I still get this error page

I also got this error page once when trying to enter an in-eligible name question but can't seem to reproduce that now.
Screenshot 2026-06-12 at 12 39 24 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement of an existing feature or functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ProgramSlugUrl] Refactor ineligible page to be associated with a GET route

3 participants