Skip to content

Refactor: remove AgencySlug#3505

Merged
angela-tran merged 4 commits intochore/agency-short-namefrom
refactor/remove-agencyslug
Feb 19, 2026
Merged

Refactor: remove AgencySlug#3505
angela-tran merged 4 commits intochore/agency-short-namefrom
refactor/remove-agencyslug

Conversation

@angela-tran
Copy link
Member

@angela-tran angela-tran commented Feb 18, 2026 •
edited
Loading

Closes #3498

Waiting for #3481 and #3472

With the simplification achieved by #3493, #3495, #3472, and #3481, we can remove AgencySlug and no longer need to do a code change to add a new agency to the app.

@angela-tran angela-tran self-assigned this Feb 18, 2026
@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 Feb 18, 2026
@github-actions
Copy link

github-actions bot commented Feb 18, 2026

This PR adds a Django migration without updating any fixtures. Is a corresponding fixture update needed?

@jgravois jgravois force-pushed the chore/agency-short-name branch 2 times, most recently from a3aef00 to 5d376bb Compare February 19, 2026 18:00
@angela-tran
refactor: remove choices from TransitAgency.slug and enforce uniqueness
6e8163a 
remove unneeded AgencySlug class
@angela-tran
fix(test): remove unnecessary creation of new TransitAgency
958f3d0 
this started failing due to the unique constraint being introduced on
TransitAgency.slug, and looking over the test, there isn't actually a need
to create a new TransitAgency.
@angela-tran
fix(test): set a unique slug for creation of new TransitAgency
0be73a1
@angela-tran
fix(test): remove unneeded creation of new TransitAgency
9ed5756 
at one point these tests may have needed a new instance, but reading
over them now, it seems fine to use the original fixture instance.
@angela-tran angela-tran force-pushed the refactor/remove-agencyslug branch from 2013844 to 9ed5756 Compare February 19, 2026 20:16
@angela-tran
Copy link
Member Author

angela-tran commented Feb 19, 2026

This PR adds a Django migration without updating any fixtures. Is a corresponding fixture update needed?

No fixture update needed.

@github-actions
Copy link

github-actions bot commented Feb 19, 2026

Coverage report

Click to see where and how coverage changed

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

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

@github-actions
Copy link

github-actions bot commented Feb 19, 2026

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)		Lines missing
  benefits/core/models
  __init__.py
  transit.py
Project Total  

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

@angela-tran
Copy link
Member Author

angela-tran commented Feb 19, 2026

I'll mark this as ready to review whenever #3472 is reviewed/merged into main.

jgravois
jgravois reviewed Feb 19, 2026
View reviewed changes
Copy link
Member

@jgravois jgravois left a comment •
edited
Loading

Choose a reason for hiding this comment

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

looks superb!

given the minimal scope of the changes here and the fact that we don't know how long we'll have to wait on #3472 i'd also be fine with merging this work into my branch now.

up to you.

@angela-tran
Copy link
Member Author

angela-tran commented Feb 19, 2026

looks superb!

given the minimal scope of the changes here and the fact that we don't know how long we'll have to wait on #3472 i'd also be fine with merging this work into my branch now.

up to you.

Thanks! That's fine with me too. I'll go ahead and open this up for review.

@angela-tran angela-tran marked this pull request as ready for review February 19, 2026 21:57
@angela-tran angela-tran requested a review from a team as a code owner February 19, 2026 21:57
thekaveman
thekaveman approved these changes Feb 19, 2026
View reviewed changes
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.

Wow, such minimal changes to drop this, I love it!

The unique=True works as expected, and the error message in the Admin is useful straight OOTB:

Image

@angela-tran angela-tran merged commit 64721f8 into chore/agency-short-name Feb 19, 2026
16 checks passed
@angela-tran angela-tran deleted the refactor/remove-agencyslug branch February 19, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jgravois jgravois jgravois left review comments

@thekaveman thekaveman thekaveman approved these changes

Assignees

@angela-tran angela-tran

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

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@angela-tran @thekaveman @jgravois