-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix registration if GEO_URL is not known #84
base: master
Are you sure you want to change the base?
Conversation
@@ -18,7 +18,7 @@ def with_existing_record | |||
private | |||
|
|||
def location | |||
return if street && street_number && city && city_part && geo_entry_id && geo_unit_id && geo_coord_x && geo_coord_y |
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.
While testing the registration process I found out, that geo coordinates were missing and all the other location related attributes were present. IMO it is better to make the geo_coord_x
and geo_coord_y
optional so the registration can be completed without them.
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.
Well, but that is really undesired behavior. If we want to look up volunteers in some radius by resolved location, these coordinates must be present.
@janzikan Thanks for PR :) In case of optional coords, we need to discuss this. So, review may take some time ;) |
Please confirm that the undesired behaviour is happening. When working on this PR I was not able to get the geo coords (x, y) at all, but when I was testing the registration process yesterday the coordinates were fetched. Does the javascript code for getting coordinates need all parts of the address to be present? We may not force the users to provide their full address not to discourage them from finishing the registration. #85 |
@janzikan I like that you've spotted this and also the solution. Let's wait with it for #134 - that may change how the address is fetched and stored. Also, please note @arthurwozniak's comments about the need to keep lat / lon mandatory. Thanks 👍 |
Close #74