Skip to content

Allow structured Nominatim lookup #1690

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

janko
Copy link

@janko janko commented Jun 16, 2025

The following currently fails with an error:

Geocoder.search(nil, params: {
  street: "...",
  city: "...",
  country: "...",
  state: "...",
  postalcode: "...",
})
Structured query parameters(amenity, street, city, county, state, postalcode, country) cannot be used together with 'q' parameter.

We fix this by skipping including the q parameter in case structured query parameters are passed and query text would be blank.

Closes #1689

@alexreisner
Copy link
Owner

@janko thanks for this. I think your approach is good, just a few thoughts:

  1. The purpose of your test is not clear to me. Perhaps it should be named something like test_q_is_empty_with_structured_lookup? It doesn't matter so much that you get a result back (since it's a fake result). Maybe the assertion should involve a Query object?
  2. Are you sure the condition you've added (!query.params_given? || !query.sanitized_text.empty?) isn't inverted? It seems to me that q should be blank if there are no params given (but I might be confused).

@janko
Copy link
Author

janko commented Jun 19, 2025

Thanks for the quick reply:

  1. Since the test suite doesn't seem to be using mocking libraries such as WebMock, I'm not sure how to make assertions on which query parameters were (not) included in the Nominatim request. Do you maybe have some pointers?
  2. I thought about the least awkward way to write this condition. I want to skip the q parameter if additional parameters were passed (e.g. street, city) AND no text query was passed, i.e. query.params_given? && query.sanitized_text.empty?. Since it's a conditional for when we should add the q parameter, we have to invert it, so it's if !(query.params_given? && query.sanitized_text.empty?), or if !query.params_given? || !query.sanitized_text.empty? if don't want parentheses.

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

Successfully merging this pull request may close these issues.

Unable to perform structured Nominatim lookup
2 participants