search

rubyforgood / pantry_scheduler

MIT License
7 stars 13 forks source link

Counties should be restricted #80

Closed pollygee closed 7 years ago

pollygee commented 7 years ago

County options should be limited to: PG HO AA

Currently in the UI they are listed as Howard, Anne Arundle, but the db is seeded with above, so no county shows up.

jgaskins commented 7 years ago

Might make sense to update the county names in the DB seeds instead. Using the full county names will allow it to scale beyond just Elizabeth House.

pollygee commented 7 years ago

These names are how they're commonly used. Nobody ever says Prince George's county. It is also how the pantry currently uses the names, so what the volunteers are used to . I think it makes sense to stick with the abbreviations.

jgaskins commented 7 years ago

I agree that nobody says it out loud, but if/when this scales beyond Elizabeth House and another food pantry has a "PG" county but also has a county entry that's fully spelled out, I worry that they'd have ambiguous data at that point.

ajohnson052 commented 7 years ago

I tried adding client validations as a part of Issue #17 and began getting errors because of this discrepancy.

There are some advantages to restricting the field now, and then expanding later on. If we only allow three values for that field it's a simple change to start allowing others later. However, if we allow anything, we could end up with inconsistent data.

I'm definitely a proponent of limiting the county field to ["PG", "AA", "HO"]

pollygee commented 7 years ago

Yes. Let's Def go with limiting the county field to ["PG", "AA", "HO"] Thanks @ajohnson052

In speaking with Sean our biggest priority should be getting this working for EH, and beyond that we will have a bunch of work to do anyway. We may consider making this into a SASS for future for other pantries. There will be lots of mods that have to be made because I'm sure some of the things we have not realized are unique to EH.

pollygee commented 7 years ago

@ajohnson052 this issue can be closed correct? This was addressed in your last PR I think!

ajohnson052 commented 7 years ago

@pollygee Yes it can!