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

Change agency_type to a Rails enum #4241

Open
dorner opened this issue Mar 31, 2024 · 9 comments · May be fixed by #4254
Open

Change agency_type to a Rails enum #4241

dorner opened this issue Mar 31, 2024 · 9 comments · May be fixed by #4254
Labels

Comments

@dorner
Copy link
Collaborator

dorner commented Mar 31, 2024

Summary

There are a number of tests that check PartnerProfile#agency_type against a string "Other". Some other tests check the value in an array instead:

Partner::AGENCY_TYPES['OTHER']

This seems to be hiding a hidden issue, which is that this field really should be an enum. We don't need to go all the way to defining a Postgres enum; we can just turn the field into a Rails enum as documented here.

Things to consider

A bonus to this would be moving the AGENCY_TYPES map from the Partner class to the PartnerProfile class, as that's where it's actually used.

Criteria for Completion

  • [] Agency type is a Rails enum.
  • Specs are updated to query e.g. profile.other? instead of checking against any specific string value.
  • All other functionality still works as expected.
@veezus
Copy link
Contributor

veezus commented Apr 1, 2024

I'd like to pick this issue up.

@dorner
Copy link
Collaborator Author

dorner commented Apr 1, 2024

Go for it!

@veezus
Copy link
Contributor

veezus commented Apr 1, 2024

It looks like we're currently storing the agency type description string in the database (not the code). Do we want, as part of this PR, a migration to convert agency types from, for example, Government Agency/Affiliate to GOVT?

@dorner
Copy link
Collaborator Author

dorner commented Apr 2, 2024

Hmm... that's a good question. I think it's definitely a better approach than storing the full string. We'd have to be very careful about the rollout, though.

I think we'd have to do something like allowing both long and short values in the enum hash, then doing the migration, then removing the long values from the hash.

Copy link
Contributor

github-actions bot commented May 3, 2024

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

@github-actions github-actions bot added the stale label May 3, 2024
@veezus
Copy link
Contributor

veezus commented May 3, 2024

We're just waiting on a data cleanup (#4261) before we move forward on this one.

Copy link
Contributor

Automatically unassigned after 7 days of inactivity.

@cielf
Copy link
Collaborator

cielf commented May 11, 2024

NDBN sent me back some stuff, but I need to review it -- probably a Monday job at this point.

@cielf cielf added Help Wanted Groomed + open to all! and removed stale labels Oct 11, 2024
@cielf
Copy link
Collaborator

cielf commented Oct 11, 2024

I had provided the requested mapping on #4261 but it hasn't been finished. That would be the next step on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants