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

Refactor: remove eligibility item templates #2774

Merged
merged 5 commits into from
Mar 21, 2025

Conversation

thekaveman
Copy link
Member

@thekaveman thekaveman commented Mar 20, 2025

Closes #2759

What this PR does

  • Follows our new context approach for variable template text for a specific flow
  • Removes all eligibility-item include templates
  • Moves rendering of all eligibility items directly into the start.html template where they are needed

Reviewing

  • Checkout this branch in the devcontainer
  • Delete your local database
  • Run bin/reset_db.sh to rebuild and reload the sample fixtures
  • F5 to launch the app
  • Select CST
  • Select a Login.gov flow, continue
  • Confirm no regressions on the start page + translations
  • Go back, select an Agency card flow, continue
  • Confirm no regressions on the start page + translations
  • Go back, select a non-Login.gov, non-Agency card flow, continue
  • Confirm no regressions on the start page + translations

@github-actions github-actions bot added the back-end Django views, sessions, middleware, models, migrations etc. label Mar 20, 2025
@thekaveman thekaveman self-assigned this Mar 20, 2025
@github-actions github-actions bot added deployment-dev [auto] Changes that will trigger a deploy if merged to dev front-end HTML/CSS/JavaScript and Django templates labels Mar 20, 2025
@thekaveman thekaveman added this to the Simplify templates milestone Mar 20, 2025
@thekaveman thekaveman force-pushed the refactor/templates--eligibility-item branch 2 times, most recently from 434d670 to e2efb05 Compare March 20, 2025 23:50
Copy link

github-actions bot commented Mar 20, 2025

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  benefits/eligibility/context
  flow.py
Project Total  

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

@thekaveman thekaveman marked this pull request as ready for review March 20, 2025 23:55
@thekaveman thekaveman requested a review from a team as a code owner March 20, 2025 23:55
this include is only used within the eligibility app
no need for an include here, only used once
remove the include template and make its content the default for
an unconfigured eligibility item
@thekaveman thekaveman force-pushed the refactor/templates--eligibility-item branch from e2efb05 to 63f7a8a Compare March 21, 2025 20:58
@thekaveman thekaveman force-pushed the refactor/templates--eligibility-item branch from 63f7a8a to 7d93478 Compare March 21, 2025 22:28
@thekaveman thekaveman requested a review from angela-tran March 21, 2025 22:29
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.

Nice 💯

@thekaveman thekaveman merged commit 1f39230 into main Mar 21, 2025
14 checks passed
@thekaveman thekaveman deleted the refactor/templates--eligibility-item branch March 21, 2025 22:59
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.

Simplify templates: refactor EligibilityStart.eligibility_item_template
2 participants