Iridescent-CM / technovation-app

The team submission platform for the Technovation Challenge
https://technovationchallenge.org
GNU General Public License v3.0
7 stars 4 forks source link

Hello, we a problem in Spain with geolocation. #1824

Closed adrichey closed 5 years ago

adrichey commented 6 years ago

This is a end-user generated bug report from the Technovation Bug Report Google Form.

Describe the bug Hello, we a problem in Spain with geolocation. Since we have a high number of participants, and we are 5 ambassadors, we need to filter by State (each ambassador have a different state). Last year, we were able to do so very easily, the State was filled out by everybody (either automatically by the geolocation or manually by the participant). But this year, the State field is empty in most cases. And it is almost impossible to filter by city, there are many cities in a state.

So, in summary, something is broken. Can you try to see if the geolocation is not filling out the State? Something is not mapped all right?

To give you an example, we have right now 382 participants in Spain, of those, 172 have no State. We can manually fix the location of those, unfortunately I don´t see any other option, but can you fix it ASAP so it doesn´t keep happening? And the ones that are ok, are from previous years. Thank you!!

Steps to reproduce the behavior You can just check the platform and see for yourself.

Expected behavior State must be filled out.

Additional context

joemsak commented 6 years ago

On first attempt to resolve this, I am already seeing that sometimes google geocoding returns a state and sometimes it doesn't - I'm so far not sure what to do to guarantee consistent results. I can't require 'state' as an input in the form because not all places have a "state" (or province) - I'm open to and hoping for better suggestions, if anyone has something

@adrichey @Allicolyer

joemsak commented 6 years ago

@stenington copying you as well, if you happen to have any ideas.

In the meantime, I can make sure 'reverse_geocoding' always runs which should help but is not a guarantee

adrichey commented 6 years ago

Hmmm. I think doing the reverse_geocoding sounds like a good plan, but I honestly can't think of a great solution for this considering that we can't control the responses we are getting from a third party service. :/

joemsak commented 6 years ago

So, on a local copy of production, I was able to confirm:

accounts = Account.where(country: "ES", state_province: nil)
accounts.count
#=> 178

accounts.each do |account|
  account.reverse_geocode
  account.save
end

Account.where(country: "ES", state_province: nil).count
#=> 31

I am going to add a rake task to do this on all current accounts (not just Spain ones)

joemsak commented 6 years ago

On my local copy of production database that takes it down from 1,244 to 402

adrichey commented 6 years ago

Forgive my naiveté as I don't know how possible this is, but when we run that rake task, is there anyway we can record the accounts affected by this so that we can have a record in case something goes awry? I don't expect problems, but it might be good to cover our bases.

joemsak commented 6 years ago

Sure, absolutely we could do that. Good thinking. I’ll update the branch with a solution. Thank you Alan!

adrichey commented 6 years ago

Thanks Joe! Also, I edited your comment because it had personal information in there and I assumed that wasn't intended.

joemsak commented 6 years ago

Oh yeah, good ol' email reply - thank you

adrichey commented 6 years ago

@Allicolyer Joe just wrapped up a solution for this issue in the form of the following PR: https://github.com/Iridescent-CM/technovation-app/pull/1830. We were wondering if this should be hotfixed into production now, or if we should just wait until the next release. Since you are our priorities master, I thought I would ask before we did anything.

joemsak commented 6 years ago

I think we're going to have a problem here... the test on QA changed one of the Palestine users to Colorado, and a Hong Kong user to San Francisco :(

https://drive.google.com/file/d/14N6iQz-qzFu4IrC6depBHyiQCJUfP-DW/view?usp=sharing

This... might be due to the user records having those Lat/Lngs stored on them originally and the address values being overwritten by a tester, but then if that's true it means someone manually changing their location doesn't get their lat/lng updated.

joemsak commented 6 years ago

I'll do another test on a local copy of production data

joemsak commented 6 years ago

At first glance, I'm thinking to ONLY change records which add a state and dont change the city / country

adrichey commented 6 years ago

Thanks for being thorough with this Joe. I think only changing the records to add a state and not changing the city/country as you stated above sounds like a good idea. You may want to reach out to @Allicolyer and/or @ophelieh on this to talk about potential impacts.

joemsak commented 6 years ago

Yep, and I think this worked well, here is my log for the rake task being ran on a local copy of production data

https://drive.google.com/file/d/1sl8wHqy1Mo010nwhkysyT-JNu7z1ROTP/view?usp=sharing

For the "CHANGES TOO DRASTIC" notes, I have since changed the task to log "would have gone from" instead of "went from" since nothing is changed or saved in that case

You'll want to peruse the results for "STATE CHANGED" and check random samples to be confident in this update. Definitely a lot of them were in Spain.

adrichey commented 6 years ago

I think this looks good. The changes are consistent based on the city/country combinations, so if something goes haywire, that helps. But I checked on all the Spain city/country combinations and it looks like the data is correct. I am comfortable with this data migration. @Allicolyer, @kmorton2017, or @ophelieh I would feel more comfortable if one of you could take a look at this so we know if we are safe to run this on our production environment.

Allicolyer commented 6 years ago

@adrichey sure I can take a look.

Allicolyer commented 6 years ago

@joemsak and @adrichey thanks for working on this. So looking through the data and it looks correct.

The cities in Spain only get a state some of the time. Do we know why that is?

joemsak commented 6 years ago

@Allicolyer Yeah, as I mentioned in an earlier comment on this issue, sometimes geocoding returns a state and sometimes it doesn't. There's not much I can do to control how a 3rd party service behaves. It's why I'm thinking Alan could build a form that uses the GeoNames API to populate country and state select fields instead.

Also it's possible that the rake task just performs geocoding too rapidly and we end up going over our rate limit

It's just a very tricky feature with no fail-proof solution, unfortunately. We've tried so many different ways to pull this off and it just continues to be a thorn.

kmorton2017 commented 6 years ago

hi all! just seeing if there was any update on this issue. spain is expecting about 500 sign ups within the next week @alan @joemsak @alli

adrichey commented 6 years ago

@kmorton2017 There has been a reverse geocoding fix that has been put into the system that should help with the state population overall, but because we are using a third-party service for validation, it may be difficult to absolutely guarantee that a state will be populated into that field. @joemsak do you think there is anything left to do here?

joemsak commented 5 years ago

Here's a brief recap of all the important bits as I understand them surrounding this issue:

mmmm... this is everything that I can see currently from my POV. Am I missing anything? Thanks!

adrichey commented 5 years ago

Thank you for sharing this run-down and a potential alternative solution, Joe!

I think the GeoNames API might be a good way to approach the problem. We would need to prioritize that work, but in looking at the API and seeing that example, I think that it would be a pretty robust solution. It does of course come with potential rate limiting issues that the other solution currently has. But there might be some caching mechanisms we can use to cut down on that. I ultimately like the solution, and I think the main thing that will be a factor with improving that form will be timing and other priorities.

adrichey commented 5 years ago

Technical notes for @stenington and @joemsak so they don't get lost in email.

Having spent a little more time with this, I think the wrench in any rewrite we do of this is the fact that we use the Carmen::Country gem throughout the application. Because we are grabbing the location from Google/Bing location APIs, but are filtering everything through Carmen before storing it in the system, we may still see the states not being saved as a result. If you look at /app/controllers/registration/locations_controller.rb line 14, you can see how Carmen is setting our state values to nil, causing them to not be stored in the database properly. I think that any solution we write to improve the form should use the Carmen api to populate our country and region/state drop-downs if we go that route, so we know that they are being saved properly on the back-end as they go through Carmen checks.

In any case, if you want to discuss this further, let me know. I think we have a pretty major problem on our hands here and I will need help figuring out any solution we might want to employ. Thanks!

ophelieh commented 5 years ago

Signed up as a new student in Castello, Spain and it saved my location correctly. Looks like it went through a couple of steps before asking me to verify my location.

When I changed my location to Madrid, it suggested a new state and I was able to save my new location.

Then I signed up as a mentor in Madrid, everything worked great!

Finally, when I logged in as an admin and try to add the state for a student in Madrid, it worked right away. :)

Allicolyer commented 5 years ago

Thank you for testing at @ophelieh !! :)