-
-
Notifications
You must be signed in to change notification settings - Fork 530
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
4241 change agency type to enum #4931
Changes from 11 commits
2566e16
4e156a2
b8a4d61
7c9d8a8
9521d48
f840bb0
de2d69f
c5e260d
6c1625c
c62835c
93feb8f
ec45472
21a8cd3
f32237d
74073cb
a516804
1f4bd43
596e79c
2987a2a
b5606af
2e5a0a1
9fc8950
723d43d
761195b
4ac097d
01a9ebf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,6 +95,17 @@ class Profile < Base | |
accepts_nested_attributes_for :served_areas, allow_destroy: true | ||
|
||
has_many_attached :documents | ||
enum :agency_type, other: "OTHER", career: "CAREER", abuse: "ABUSE", bnb: "BNB", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be reformatted so each value is on a single line? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made this change in the recent commits. |
||
church: "CHURCH", college: "COLLEGE", cdc: "CDC", health: "HEALTH", | ||
outreach: "OUTREACH", legal: "LEGAL", crisis: "CRISIS", disab: "DISAB", | ||
district: "DISTRICT", domv: "DOMV", ece: "ECE", child: "CHILD", | ||
edu: "EDU", family: "FAMILY", food: "FOOD", foster: "FOSTER", govt: "GOVT", | ||
headstart: "HEADSTART", homevisit: "HOMEVISIT", homeless: "HOMELESS", | ||
hosp: "HOSP", infpan: "INFPAN", lib: "LIB", mhealth: "MHEALTH", military: "MILITARY", | ||
police: "POLICE", preg: "PREG", presch: "PRESCH", ref: "REF", es: "ES", | ||
hs: "HS", ms: "MS", senior: "SENIOR", tribal: "TRIBAL", treat: "TREAT", | ||
twoycollege: "2YCOLLEGE", wic: "WIC" | ||
|
||
validate :check_social_media, on: :edit | ||
|
||
validate :client_share_is_0_or_100 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,13 @@ | |
</div> | ||
<div class="card-body"> | ||
<%= form.input :name, label: "Agency Name", class: "form-control", wrapper: :input_group %> | ||
<%= profile_form.input :agency_type, collection: Partner::AGENCY_TYPES.values, label: "Agency Type", class: "form-control", wrapper: :input_group %> | ||
<%= profile_form.input :agency_type, | ||
# Symbolize keys, because simple_form will only do a I18n lookup for | ||
# symbols | ||
collection: Partners::Profile.agency_types.symbolize_keys.keys, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably a bit more performant to do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made this change in the recent commits. For my own edification, why was my implementation less performant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Beacuse you go through the list of values twice, once to symbolize every key and then again to collect the keys from the entries. Like I said, it's a nitpick. :) |
||
label: "Agency Type", | ||
class: "form-control", | ||
wrapper: :input_group %> | ||
<%= profile_form.input :other_agency_type, label: "Other Agency Type", class: "form-control", wrapper: :input_group %> | ||
<div class="form-group row"> | ||
<label class="control-label col-md-3">501(c)(3) IRS Determination Letter or other Proof of Agency Status</label> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
agency_type_map: &agency_type_map | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Functional issue: "Other" is appearing at the top of the list of types when selecting. It should be at the bottom. Everything else should be in alphabetical order (by the full text) (though I note it isn't in staging --so the alpha thing could be a separate issue) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cielf alright this should be addressed in the most recent commit. Below is a screenshot demonstrating the new behavior. |
||
other: "Other" | ||
career: "Career technical training" | ||
abuse: "Child abuse resource center" | ||
bnb: "Basic Needs Bank" | ||
church: "Church outreach ministry" | ||
college: "College and Universities" | ||
cdc: "Community development corporation" | ||
health: "Community health program or clinic" | ||
outreach: "Community outreach services" | ||
legal: "Correctional Facilities / Jail / Prison / Legal System" | ||
crisis: "Crisis/Disaster services" | ||
disab: "Developmental disabilities program" | ||
district: "School District" | ||
domv: "Domestic violence shelter" | ||
ece: "Early Childhood Education/Childcare" | ||
child: "Early childhood services" | ||
edu: "Education program" | ||
family: "Family resource center" | ||
food: "Food bank/pantry" | ||
foster: "Foster Program" | ||
govt: "Government Agency/Affiliate" | ||
headstart: "Head Start/Early Head Start" | ||
homevisit: "Home visits" | ||
homeless: "Homeless resource center" | ||
hosp: "Hospital" | ||
infpan: "Infant/Child Pantry/Closet" | ||
lib: "Library" | ||
mhealth: "Mental Health" | ||
military: "Military Bases/Veteran Services" | ||
police: "Police Station" | ||
preg: "Pregnancy resource center" | ||
presch: "Preschool" | ||
ref: "Refugee resource center" | ||
es: "School - Elementary School" | ||
hs: "School - High School" | ||
ms: "School - Middle School" | ||
senior: "Senior Center" | ||
tribal: "Tribal/Native-Based Organization" | ||
treat: "Treatment clinic" | ||
twoycollege: "Two-Year College" | ||
wic: "Women, Infants and Children" | ||
|
||
en: | ||
partners_profile: | ||
*agency_type_map | ||
|
||
simple_form: | ||
options: | ||
profile: | ||
agency_type: | ||
*agency_type_map |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,77 @@ | ||||||||
class ConvertPartnerAgencyTypesToEnum < ActiveRecord::Migration[7.2] | ||||||||
|
||||||||
def up | ||||||||
# A mapping from the lowercase full descriptive strings previously stored in | ||||||||
# agency_type to the newly enumerated values | ||||||||
string_to_value_mapping = { | ||||||||
"other" => "OTHER", | ||||||||
"career technical training" => "CAREER", | ||||||||
"child abuse resource center" => "ABUSE", | ||||||||
"basic needs bank" => "BNB", | ||||||||
"church outreach ministry" => "CHURCH", | ||||||||
"college and universities" => "COLLEGE", | ||||||||
"community development corporation" => "CDC", | ||||||||
"community health program or clinic" => "HEALTH", | ||||||||
"community outreach services" => "OUTREACH", | ||||||||
"correctional facilities / jail / prison / legal system" => "LEGAL", | ||||||||
"crisis/disaster services" => "CRISIS", | ||||||||
"developmental disabilities program" => "DISAB", | ||||||||
"school district" => "DISTRICT", | ||||||||
"domestic violence shelter" => "DOMV", | ||||||||
"early childhood education/childcare" => "ECE", | ||||||||
"early childhood services" => "CHILD", | ||||||||
"education program" => "EDU", | ||||||||
"family resource center" => "FAMILY", | ||||||||
"food bank/pantry" => "FOOD", | ||||||||
"foster program" => "FOSTER", | ||||||||
"government agency/affiliate" => "GOVT", | ||||||||
"head start/early head start" => "HEADSTART", | ||||||||
"home visits" => "HOMEVISIT", | ||||||||
"homeless resource center" => "HOMELESS", | ||||||||
"hospital" => "HOSP", | ||||||||
"infant/child pantry/closet" => "INFPAN", | ||||||||
"library" => "LIB", | ||||||||
"mental health" => "MHEALTH", | ||||||||
"military bases/veteran services" => "MILITARY", | ||||||||
"police station" => "POLICE", | ||||||||
"pregnancy resource center" => "PREG", | ||||||||
"preschool" => "PRESCH", | ||||||||
"refugee resource center" => "REF", | ||||||||
"school - elementary school" => "ES", | ||||||||
"school - high school" => "HS", | ||||||||
"school - middle school" => "MS", | ||||||||
"senior center" => "SENIOR", | ||||||||
"tribal/native-based organization" => "TRIBAL", | ||||||||
"treatment clinic" => "TREAT", | ||||||||
"two-year college" => "2YCOLLEGE", | ||||||||
"women, infants and children" => "WIC" | ||||||||
} | ||||||||
|
||||||||
# Some agency types have trailing spaces -- need to get rid of them first | ||||||||
update 'UPDATE partner_profiles SET agency_type = TRIM(agency_type);' | ||||||||
|
||||||||
profiles = Partners::Profile.all() | ||||||||
profiles.each do |profile| | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made this change in the recent commits. |
||||||||
# Read the agency_type without casting so values not part of the enum aren't | ||||||||
# read as nil | ||||||||
current_agency_type = profile.read_attribute_before_type_cast(:agency_type) | ||||||||
puts profile.id | ||||||||
puts current_agency_type | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove these There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made this change in the recent commits. |
||||||||
if !current_agency_type.nil? | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made this change in the recent commits. |
||||||||
# If a profile has a descriptive string (ignoring case) as an agency type | ||||||||
# update it to the associated value code. | ||||||||
if( string_to_value_mapping.key?( current_agency_type.downcase ) ) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(similarly below - not sure why lint didn't pick this up) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, for some reason the linter missed this file entirely. It only made corrections when I specified this file explicitly. Regardless, ran this through the inter in the recent commits. |
||||||||
profile.agency_type = string_to_value_mapping[ current_agency_type.downcase ] | ||||||||
# If a profile has a value code (ignoring case) as an agency type, uppercase it. | ||||||||
elsif( string_to_value_mapping.value?( current_agency_type.upcase ) ) | ||||||||
profile.agency_type = current_agency_type.upcase | ||||||||
end | ||||||||
profile.save! | ||||||||
end | ||||||||
end | ||||||||
end | ||||||||
|
||||||||
def down | ||||||||
# Irreversible data migration | ||||||||
end | ||||||||
end |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -294,7 +294,7 @@ | |||||
let(:agency_state) { "ND" } | ||||||
let(:agency_zipcode) { "09980-7010" } | ||||||
let(:agency_website) { "bosco.example" } | ||||||
let(:agency_type) { Partner::AGENCY_TYPES["OTHER"] } | ||||||
let(:agency_type) { Partners::Profile.agency_types[:other] } | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made this change, and also fixed the test improperly expecting the enum value ( |
||||||
let(:other_agency_type) { "Another Agency Name" } | ||||||
let(:notes) { "Some notes" } | ||||||
let(:providing_diapers) { {value: "N", index: 13} } | ||||||
|
@@ -335,7 +335,7 @@ | |||||
agency_state, | ||||||
agency_zipcode, | ||||||
agency_website, | ||||||
"#{Partner::AGENCY_TYPES["OTHER"]}: #{other_agency_type}", | ||||||
"#{Partners::Profile.agency_types[:other]}: #{other_agency_type}", | ||||||
contact_name, | ||||||
contact_phone, | ||||||
contact_email, | ||||||
|
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'm assuming the values here are for display? Then I'd suggest
agency_type: (symbolic_agency_type == :other) ? "Other: #{profile.other_agency_type}" : profile.agency_type
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.
Actually - would we want to do
i18n.translate
for these values? As far as I can tell this is only used in the CSV export, so we should have readable strings here.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 had originally left
profile.agency_type
because @cielf had a recollection that the type codes were used in the CSV and some of the full descriptive strings seemed quite long for a CSV column..Below are some screenshots of the CSV under different conditions. Which looks best?
CSV with full descriptive strings
CSV with full descriptive strings (agency type column fully expanded)
CSV with untranslated type codes
CSV with untranslated type codes, including `other_agency_type` for `:other`