-
-
Notifications
You must be signed in to change notification settings - Fork 504
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
[WIP] Partner::AGENCY_TYPES is an enum on Partners::Profile #4254
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a suggestion here. Overall approach looks good though!
@@ -99,6 +99,47 @@ class Profile < Base | |||
validate :client_share_is_0_or_100 | |||
validate :has_at_least_one_request_setting | |||
|
|||
enum :agency_type, { | |||
"career" => "Career technical training", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might not be what you had in mind? The keys for enum
are the strings we're going to use to access it, but the values are what is stored in the database. We don't want these big strings stored in the database. I would have expected an enum that looks something like this
enum :agency_type, %w(career abuse church college ...).to_h { |s| [s, s] }
AGENCY_TYPE_LABELS = {
"career" => "Career technical training",
"abuse" => "Child abuse resource center",
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are going to move the big strings out of the database, we (the core team) will need to check whether there is anything in the db that does not match up to the current list. Just because it's been a long time and there may have been earlier adjustments. If so, the migration might have to be a little more sophisticated, yes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...there are tons of agency types that aren't in this list. Yeah we'll need to decide what to do with them.
501 C 3 Non Profit: 1
501c3: 1
ASO: 1
Basic Needs: 1
Career technical training: 28
Case Management: 1
Charitable Non Profit 501C3: 1
Child Advocacy Center: 1
Child Care Center: 1
Child Placing Agency: 1
Child abuse resource center: 16
Children and Family Services : 1
Church: 2
Church : 1
Church food pantry: 1
Church ministry: 1
Church outreach ministry: 122
Clothing, closet for children birth to age 7: 1
Community Ministry: 1
Community Service Organization: 1
Community development corporation: 77
Community health program: 87
Community health program or clinic: 2
Community outreach services: 181
Correctional Facilities / Jail / Prison / Legal System: 1
Crisis/Disaster services: 5
Developmental disabilities program: 9
Diaper Bank: 1
Diaper agency: 1
Domestic Violence/Non-Profit: 1
Domestic violence shelter: 27
Early childhood services: 88
Education: 1
Education program: 229
Educational: 1
Emergency Crisis/Human Trafficking/Refuee Service: 1
Family resource center: 91
Food Pantry: 5
Food Pantry - Emergency Diaper Distribution: 1
Food bank: 1
Food bank/pantry: 174
Food pantry: 1
Foster Care: 1
Government Agency/Affiliate: 65
Head Start/Early Head Start: 45
Home visits: 56
Homeless resource center: 54
Hospital : 1
Human Services: 1
Infant/Child Pantry/Closet: 20
Law Enforcement Agency: 1
Maternity Home: 1
Mental Health: 1
NON PROFIT: 1
Non Profit: 2
Non-Profit: 5
Non-Profit - Education: 1
Non-Profit Victim Services and Family Resource Center: 1
Non-Profit. Clothing, Food and Diaper Assistance: 1
Nonprofit: 1
Nonprofit preschool: 1
Nonprofit/School: 1
Not for Profit: 1
Other: 347
PAT Program: 1
Perinatal Mental Health Peer Support: 1
Pregnancy Center - Charitable Health : 1
Pregnancy Resource Center: 1
Pregnancy resource center: 37
Primary Care Doctor: 1
Public Health: 1
Public School: 1
Refugee resource center: 12
Reproductive Health Care, Maternity Services: 1
Safety Net Organization: 1
School: 2
School - High School: 3
School - Middle School: 3
School District: 2
School District : 1
Treatment clinic: 25
Vocational training program : 1
Women Substance Abuse: 1
Women, Infants and Children: 59
Youth Services: 1
charitable organization: 1
non profit: 2
non-profit: 2
nonprofit : 1
public elementary school : 1
test: 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I'm silly. All we have to do is move those non-standard names to "other_agency_type" and set "agency_type" to "Other".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so loop through each profile, see if its current agency_type is in the approved list, and if not, do the swap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - most of those are a close match to one of the types. I wonder if it was initially just implemented as a text field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I'll work on this on Monday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that some of those will just be a difference in capitalization or spacing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I'll make a separate data migration PR to clean up the non-standard agency types, and then I'll write a SQL migration in this PR that converts from standard long-form agency type to agency type enum.
It'll be a clearer separation of concerns and we won't have two places to consider if something goes wrong.
93fdde5
to
c6f194a
Compare
@veezus Is this still a [WIP]? |
This is also going to need the 3 new agency types added into the partner profile version of the agency types: |
Whoops. This was the wrong one to close. #4801 should unblock this. |
This is now unblocked - 4801 just got merged |
Resolves #4241
Description
This PR converts
Partner::AGENCY_TYPES
from a constant on partner to an enum onPartners::Profile
. It introduces a newagency_types_description
method which accesses the value stored in the database.Type of change
How Has This Been Tested?
agency_type
checks have been converted toagency_type_description
Screenshots
Here's the data originally, migrated, rolled back, and migrated again: