399 duplicates #529
399 duplicates #529
Conversation
…strooms into 399-duplicates
Store reverse geo-coded address & force unique addresses
|
Looks like we need to stub the geocode require that the if block creates. I don't have time to do this atm, but can get to it later. Also, not really sure if there's a better way to fix the ugly |
| @@ -17,14 +17,17 @@ class Restroom < ApplicationRecord | |||
| using: {tsearch: {dictionary: "english"}}, | |||
| ignoring: :accents | |||
|
|
|||
| validates :name, :street, :city, :state, presence: true | |||
| if Rails.env != "test" | |||
There was a problem hiding this comment.
This does the trick for testing, but I don't like it. I think I'd rather fix the tests to truncate the db and simulate production better. We also need a test for the validation itself.
There was a problem hiding this comment.
Would we be able to "simulate production better" by having real database data from production? I would be able to update db/export.csv with actual, recent data as it exists in production, if that would help.
There was a problem hiding this comment.
@DeeDeeG I think that would make sense. I'd like to also work on killing this conditional though.
There was a problem hiding this comment.
Oh, also FYI. Changing the csv won't fix this issue. There's some sort of hook before the save that tries to geolocate these. I'm not sure what forces it to happen, but it is different than the uniqueness check.
| ) | ||
| restroom.save(validate: false) |
There was a problem hiding this comment.
Ignoring validation during seeding seems reasonable, since those don't really change ever.
|
This should also have a test for it. @tcmal do you think you could add one? |
|
Will do, in a fork or just put the patch on a gist? |
|
Fork and a PR into this branch would be great! Sorry if I made that a pain. It’s tough to work on multiple forks. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add test for restroom reverse-geocoding.
|
The merge conflict here is that two branches each added a different test to the repo's RSpec test suite. Best-case, both tests work as intended in this branch and we just keep them both. But if these conflict with each-other somehow, or one of the tests breaks in this branch, then I guess we have to adjust or delete one of the tests, and/or worst-case, one of the features being tested... |
This is just a slight refactor of the PR I merged to a local branch. Moving back to using
(obj)syntax, because it seems like the methods aren't available in theifblock.