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.

Sign up for GitHub

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

Jump to bottom

Refactor: rename EligibilityVerifier eligibility fields #2272

Merged
merged 9 commits into from
Aug 6, 2024
Merged

Refactor: rename EligibilityVerifier eligibility fields #2272

merged 9 commits into from
Aug 6, 2024

Conversation

thekaveman
Copy link
Member

@thekaveman thekaveman commented Aug 2, 2024 •
edited
Loading

Closes #2063

Closes #2238

@thekaveman thekaveman self-assigned this Aug 2, 2024
@github-actions github-actions bot added back-end Django views, sessions, middleware, models, migrations etc. tests Related to automated testing (unit, UI, integration, etc.) migrations [auto] Review for potential model changes/needed data migrations updates labels Aug 2, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 2, 2024 •
edited
Loading

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)		Lines missing
  benefits/core
  models.py
  benefits/eligibility
  views.py
  benefits/oauth
  middleware.py
Project Total  

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

@thekaveman thekaveman force-pushed the refactor/verifier-eligibility-fields branch from e77ae13 to 7726adb Compare August 2, 2024 00:37
@thekaveman thekaveman force-pushed the refactor/models-transitprocessor branch from fb35314 to 8f0f2a7 Compare August 2, 2024 00:41
@thekaveman thekaveman force-pushed the refactor/verifier-eligibility-fields branch from 7726adb to 1bb1ad2 Compare August 2, 2024 00:44
@thekaveman thekaveman changed the title Refactor: rename EligibilityVerifier eligibility fields Refactor: rename EligibilityVerifier eligibility fields Aug 2, 2024
@thekaveman thekaveman force-pushed the refactor/models-transitprocessor branch from 8f0f2a7 to 3941634 Compare August 2, 2024 16:08
Base automatically changed from refactor/models-transitprocessor to main August 2, 2024 18:07
@thekaveman thekaveman force-pushed the refactor/verifier-eligibility-fields branch from 1bb1ad2 to d8d4be4 Compare August 2, 2024 18:08
@thekaveman thekaveman marked this pull request as ready for review August 2, 2024 18:11
@thekaveman thekaveman requested a review from a team as a code owner August 2, 2024 18:11
@thekaveman thekaveman added this to the Admin tool: agency users milestone Aug 2, 2024
@thekaveman thekaveman marked this pull request as draft August 2, 2024 18:14
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 2, 2024

Preview url: https://benefits-2272--cal-itp-previews.netlify.app

@thekaveman thekaveman marked this pull request as ready for review August 2, 2024 18:25
lalver1
lalver1 previously approved these changes Aug 5, 2024
View reviewed changes
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.

Looks good to me! I also tested an IdG and an eligibility API flow to double check and there were no problems.

angela-tran
angela-tran requested changes Aug 5, 2024
View reviewed changes
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.

The field renaming looks good. Just one thought about the start_template refactor

benefits/core/models.py Outdated Show resolved Hide resolved
@thekaveman thekaveman force-pushed the refactor/verifier-eligibility-fields branch from da1d8a7 to 686f660 Compare August 5, 2024 21:15
@thekaveman thekaveman marked this pull request as draft August 5, 2024 21:16
@thekaveman thekaveman force-pushed the refactor/verifier-eligibility-fields branch from 686f660 to 872fd0c Compare August 5, 2024 21:24
@thekaveman thekaveman marked this pull request as ready for review August 5, 2024 21:25
angela-tran
angela-tran approved these changes Aug 5, 2024
View reviewed changes
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.

Looks great!! I tested out that one is not able to blank out the eligibility_start_template 👍 🔥

image

@thekaveman thekaveman force-pushed the refactor/verifier-eligibility-fields branch from 872fd0c to 8880a3e Compare August 6, 2024 19:04
@thekaveman thekaveman enabled auto-merge August 6, 2024 19:04
@thekaveman thekaveman merged commit 60e0bf4 into main Aug 6, 2024
17 checks passed
@thekaveman thekaveman deleted the refactor/verifier-eligibility-fields branch August 6, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angela-tran angela-tran angela-tran approved these changes

@lalver1 lalver1 Awaiting requested review from lalver1

Assignees

@thekaveman thekaveman

Labels
back-end Django views, sessions, middleware, models, migrations etc. migrations [auto] Review for potential model changes/needed data migrations updates tests Related to automated testing (unit, UI, integration, etc.)
Projects
None yet
Milestone
Benefits admin tool (agency users)
3 participants
@thekaveman @lalver1 @angela-tran