Skip to content
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

Location search not working #214

Closed
nstjean opened this issue Dec 6, 2019 · 33 comments · Fixed by #220
Closed

Location search not working #214

nstjean opened this issue Dec 6, 2019 · 33 comments · Fixed by #220
Labels

Comments

@nstjean
Copy link
Contributor

nstjean commented Dec 6, 2019

Even on the example pages when I type in a location to search the spinner keeps spinning with nothing ever resolving.

FireShot Capture 096 - Leaflet BlurredLocation -

I will try to figure out where the error is.

@nstjean nstjean added the bug label Dec 6, 2019
@welcome
Copy link

welcome bot commented Dec 6, 2019

Thanks for opening your first issue here! Please follow the issue template to help us help you 👍🎉😄
If you have screenshots to share demonstrating the issue, that's really helpful! 📸 You can make a gif too!

@nstjean
Copy link
Contributor Author

nstjean commented Dec 6, 2019

This is related to publiclab/plots2#6915

@nstjean
Copy link
Contributor Author

nstjean commented Dec 6, 2019

When var autocomplete = new google.maps.places.Autocomplete(input); is called I am seeing this error:
js?libraries=places&language=en&key=AIzaSyDWgc7p4WWFsO3y0MTe50vF4l4NUPcPuwE:68 You have exceeded your daily request quota for this API. If you did not set a custom daily request quota, verify your project has an active billing account: http://g.co/dev/maps-no-account For more information on usage limits and the Google Maps JavaScript API services please see: https://developers.google.com/maps/documentation/javascript/usage

@nstjean
Copy link
Contributor Author

nstjean commented Dec 6, 2019

So it looks like this bug is due to this: #160

@sagarpreet-chadha
Copy link
Collaborator

Wait, @jywarren why we can see key in the error log? I think this may be misused and most probably someone may be using it.
Let's hide it in some config bucket or environment variable.

Thanks @nstjean for investigating 😄 !!!

@nstjean
Copy link
Contributor Author

nstjean commented Dec 6, 2019

Ohh, that's a very good point. I wonder if that's why it keeps saying exceeded daily quota? If someone else is using the key.

@jywarren
Copy link
Member

jywarren commented Dec 6, 2019 via email

@sagarpreet-chadha
Copy link
Collaborator

sagarpreet-chadha commented Dec 7, 2019 via email

@nstjean
Copy link
Contributor Author

nstjean commented Dec 9, 2019

That's a good point, can't really hide it very easily in the Javascript. I think setting restrictions and using a try/catch block would do the trick... even if someone sees the code and gets the key they won't be able to use it.

Here's the page that specifies how to set restrictions: https://developers.google.com/maps/documentation/javascript/get-api-key

Just let me know what is the proper key to use and I'll make the changes.

@jywarren
Copy link
Member

jywarren commented Dec 9, 2019

Hi, thanks for all this. I'm going to try to go through it carefully tomorrow. Can you start making a list of places where this happens, because we should check on the API key of each place, and make a permission for each originating domain name? Then I can go through each one and check it off and we won't miss any or have ambiguity. Make sense?

Thanks a ton!

@nstjean
Copy link
Contributor Author

nstjean commented Dec 10, 2019

Ok, I'll start making a list!

@jywarren
Copy link
Member

jywarren commented Dec 10, 2019 via email

@nstjean
Copy link
Contributor Author

nstjean commented Dec 10, 2019

Sure!

@nstjean
Copy link
Contributor Author

nstjean commented Dec 10, 2019

So when looking for where this error shows up I discovered it shows up on every single page.

in plots2 we have this in application.html.erb - note that this is the only spot we are using this particular key, it's only for testing I guess.
https://github.com/publiclab/plots2/blob/319bcd0f40ffdfea83bae12866eef5baa6c8bcb6/app/views/layouts/application.html.erb#L16-L18

But we also are also declaring a key in these two places:
https://github.com/publiclab/plots2/blob/319bcd0f40ffdfea83bae12866eef5baa6c8bcb6/app/views/users/map.html.erb#L1
https://github.com/publiclab/plots2/blob/319bcd0f40ffdfea83bae12866eef5baa6c8bcb6/app/assets/javascripts/locationForm.js#L152-L156

But then on top of that we also have the same key being declared in 5 files in leaflet-blurred-location:

<script src="https://maps.googleapis.com/maps/api/js?libraries=places&language=en&key=AIzaSyDWgc7p4WWFsO3y0MTe50vF4l4NUPcPuwE"></script>

<script src="https://maps.googleapis.com/maps/api/js?libraries=places&language=en&key=AIzaSyDWgc7p4WWFsO3y0MTe50vF4l4NUPcPuwE"></script>

<script src="https://maps.googleapis.com/maps/api/js?libraries=places&language=en&key=AIzaSyDWgc7p4WWFsO3y0MTe50vF4l4NUPcPuwE"></script>

url:"https://maps.googleapis.com/maps/api/geocode/json?latlng="+lat+","+lng + "&key=AIzaSyDWgc7p4WWFsO3y0MTe50vF4l4NUPcPuwE",

url:"https://maps.googleapis.com/maps/api/geocode/json?latlng="+lat+","+lng + "&key=AIzaSyDWgc7p4WWFsO3y0MTe50vF4l4NUPcPuwE",

var url = "https://maps.googleapis.com/maps/api/geocode/json?address=" + string.split(" ").join("+") + "&key=AIzaSyDWgc7p4WWFsO3y0MTe50vF4l4NUPcPuwE" ;

Do we need all of those?? The instructions say

You must include an API key with every Maps JavaScript API request.

So I would think it would have to be sent with each map, not on every single page. And if it's being sent in leaflet-blurred-location it should it be sent from both the template in plots2 and the blurredlocation?

@jywarren
Copy link
Member

Didn't get a chance to dig into this today, i'm sorry to say. But some quick initial feedback:

  1. some of those usages are just in the example files, so wouldn't be reflected in bugs on plots2 (but thanks for finding them as we may have to adjust them to fix the examples!)
  2. I think the Google API key policies have changed over the years and some of these lines of code may reflect earlier ways of using the key; i think possibly in earlier versions it was possible to include the key in the initial <script> include, but I'm not sure that's the case anymore.

So I think the key is this request i'm seeing in the console

https://maps.googleapis.com/maps/api/js?libraries=places&language=en&key=XXXXXXXXXXXXX

This seems to match what you're saying about needing to supply a correct API key for each. I'll check the keys against what the original <script> tag is showing and try replacing it with a new key.

error_message: "You must use an API key to authenticate each request to Google Maps Platform APIs. For additional information, please refer to http://g.co/dev/maps-no-account"

...confirming - the request actually sent in the "Network" pane of the webdev console shows this:

https://maps.googleapis.com/maps/api/place/js/AutocompletionService.GetPredictions?1ssomerville&4sen&15e3&20s612B0729-484D-4207-B6AE-15FB2518EA6B3y8qzg9czonp&21m1&2e1&callback=_xdc_._16q8my&key=XXXXXXXXXXXX&token=125951

So I think it's actually OK to be putting the key in the initial <script> include, and assuming it gets re-used on each request. Now i'll try to find a new key or trace the old one.

@jywarren
Copy link
Member

Let's use this key, which should be limited to publiclab.org, mapknitter.org and publiclab.github.io: AIzaSyAOLUQngEmJv0_zcG1xkGq-CXIPpLQY8iQ

How do we figure out what domain Travis tests are run from? https://travis-ci.org/publiclab/plots2/jobs/623355464

jywarren added a commit to jywarren/leaflet-blurred-location that referenced this issue Dec 10, 2019
@jywarren
Copy link
Member

Opening an issue with these fixes; let's push to gh-pages branch and see if it works!

@jywarren
Copy link
Member

We will have to do a similar search/replace PR on the plots2 instances!

jywarren added a commit that referenced this issue Dec 10, 2019
* fix re: #214

* package.json bump
@jywarren
Copy link
Member

Tried the fix in #215, testing on gh-pages now!

@jywarren
Copy link
Member

OK, awesome! Fixed the main issue! Now, I'm seeing the second bug as I just reported at
publiclab/plots2#6915 (comment)

i went to https://publiclab.org/post and tried using the input, i was able to use By place name and choose a location, but the map did not move and I got a loading spinner with the JS error:

Uncaught TypeError: Cannot read property 'geometry' of undefined
    at geocodeStringAndPan (Leaflet.BlurredLocation.js:1014)
    at Leaflet.BlurredLocation.js:956

I think maybe we're parsing the response of the locations api incorrectly?

OK! I've captured (lol) a sample response that I think we have to reconfigure around. I wonder when this changed and when it stopped working?

https://gist.github.com/jywarren/eec9fcec10528caaf463df6068846b0e

image

@nstjean would you like to try fixing that one? Thank you so much for your deep research, which made this easy to fix!

@nstjean
Copy link
Contributor Author

nstjean commented Dec 11, 2019

@jywarren Sure I'll do that today! 😃

@nstjean
Copy link
Contributor Author

nstjean commented Dec 11, 2019

This should be correct now, but I can't test it on my localhost.
#217

@sagarpreet-chadha
Copy link
Collaborator

Hey you can push this PR to gh-pages of LBL.
As @jywarren mentioned above the API key is valid for domain: publiclab.github.io
You can test this by checking into gh-pages branch in LBL and adding your changes in that branch and pushing it (if you do not have push access to gh-pages branch, you can make a new PR against gh-pages branch and i will merge it) 😄

@nstjean
Copy link
Contributor Author

nstjean commented Dec 11, 2019

Oh ok!! I will try that out!

@nstjean
Copy link
Contributor Author

nstjean commented Dec 11, 2019

PR for gh-pages: #218

Though when I go to publiclab.github.io I'm getting a 404 error.

@nstjean
Copy link
Contributor Author

nstjean commented Dec 11, 2019

Oh I figured out what URL to use. :)

@nstjean
Copy link
Contributor Author

nstjean commented Dec 11, 2019

Ok. I've pushed some changes (after some trial and error with refreshing the page!) to gh-pages so you can take a look. I've added checking for a status code and displaying the error if there is one instead of just spinning forever.

The error shown is:

{,…}
error_message: "You must enable Billing on the Google Cloud Project at https://console.cloud.google.com/project/_/billing/enable Learn more at https://developers.google.com/maps/gmp-get-started"
results: []
status: "REQUEST_DENIED"

@jywarren I believe the "cannot read geometry" error was due to there being no results, not because the parsing was incorrect!

@jywarren
Copy link
Member

jywarren commented Dec 11, 2019 via email

@nstjean
Copy link
Contributor Author

nstjean commented Dec 12, 2019

I'd like to see if it performs correctly when the billing setting is correct just to make sure it works...

In normal use there wouldn't be no results I think, because it only gets sent if a user clicks on a city name while filling out the autocomplete, right? Though we could certainly put a a "no results" message on the screen near the box in case something like this happens again.

@jywarren
Copy link
Member

jywarren commented Dec 12, 2019 via email

@nstjean
Copy link
Contributor Author

nstjean commented Dec 12, 2019

Another thing I just thought of: If anyone tries to use LBL on a different they will have to update the API keys. I don't think there's any information about that on in the docs.

@nstjean
Copy link
Contributor Author

nstjean commented Dec 12, 2019

I opened a pull request, but it's doing that thing where it shows a huge list of commits. I'll try looking up what I did last time this happened!

@nstjean
Copy link
Contributor Author

nstjean commented Dec 12, 2019

I tried again thinking that it was my branch that was acting up because of using gh-pages. But brand new branch is doing the same thing. And all the tests are failing, so I'll try to figure out why it worked fine on gh-pages but not in a branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants