See #3041549: Proximity filter must handle geocode failures which has been closed but in fact it's still not working.

  1. Create a view with an exposed proximity filter. So it's the completely standard and obvious map search: users type in a town or zip/postal code and they get results nearby.
  2. Type in garbage that doesn't geocode, for example "lodnon".
  3. Expected outcome: "No results found"
  4. Actual outcome: Random list of results from anywhere in the country. Surely this is not what any user wants or how any other site works??
CommentFileSizeAuthor
#24 geofield.proximity-failures.3053372-24.patch1.18 KBnikunj.shah
#12 geofield.proximity-failures.3053372-interdiff-10-12.txt1.2 KBadamps
#12 geofield.proximity-failures.3053372-12.patch3.59 KBadamps
#10 geofield.proximity-failures.3053372-10.patch3.92 KBitamair
#2 geofield.proximity-failures.3053372-2.patch635 bytesadamps

Comments

AdamPS created an issue. See original summary.

adamps’s picture

adamps commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new geofield.proximity-failures.3053372-2.patch635 bytes
itamair’s picture

itamair commented

@AdamPS ... again on this,
with this patch from you, if I have an exposed proximity filter and the default Origin (both Coordinates and Geo Address) is empty, the view would not return any result ...
and this looks weird and not correct to me
(the empty exposed filter would just be ignored by the Views query, isn't it?)

itamair’s picture

itamair commented

Please ... let's consider the big picture, and not only your specific use case.

May be that "lodnon" Geocode results is not failing but simply giving unwanted/unpredicted/wrong results?

We should face the most general use cases (first) and then the most specific ...

itamair’s picture

itamair commented

I better checked ...
Regarding your comment here: https://www.drupal.org/project/geofield/issues/3041549#comment-13098599
well ... I verified that "lodnon" Geocode results is not failing but simply giving (may be) unexpected result to you.
It is a Geocoder matter ...
The Proximity Filter View results would be outcome accordingly (and not randomly).

IMO, things are working the way I expect ... so that I don't feel to commit the #2 patch (for now).

You are free to apply (and use) at your project needs, until I am convinced and persuaded otherwise (eventually).

itamair’s picture

itamair commented
Priority: Major » Normal
Status: Needs review » Closed (works as designed)
adamps’s picture

adamps commented
Status: Closed (works as designed) » Needs work

@itamair Thanks for your response

if I have an exposed proximity filter and the default Origin (both Coordinates and Geo Address) is empty, the view would not return any result

OK I see what you mean, thanks for explaining, that's a good point, I agree that if the proximity search box is empty then it should return results. That means the patch isn't correct.

However I think the issue is correct. The scenario in the issue is that if the proximity search box contains text that cannot be geocoded then the view should return no results. I did a quick random test on the internet and every single site I found worked that way:

So valid issue + wrong patch => status "Needs work". Is that OK with you?

At the moment it looks like "geocode failure" gives an empty location so it is impossible for geofield to distinguish. What about if we changed the project to Geocoder? Is there any way to return an "invalid" location if geocoding fails?

well ... I verified that "lodnon" Geocode results is not failing but simply giving (may be) unexpected result to you.

You have tested it with your own choice of geocoder settings which is not the same as what happens with my geocoder settings.

It is definitely failing. If I type Paris, geocodes succeeds, it finds no matches in range and correctly displays no results. If I type lodnon geocode fails and it generates no results.

itamair’s picture

itamair commented

Ok. So the (general) working scenario might/should be the following, both for the Geocoding Origin Exposed Proximity filter, and also the other Proximity Sources.
If the default value of the Origin is empty the Geofield View first load will return All its results (the exposed filter is not acting, as requested).
If the Geocode doesn't succeed (and any valid Location Origin is returned) or even if the Exposed form is submitted (Apply is clicked) with an empty Origin Address (or empty Lat / Lon couple) ... the Geofield View shouldn't return ANY return.

Is this right for you?
... thus Needs Work is ok.

I will try to better figure out this too.

adamps’s picture

adamps commented

Thanks @itamair. I think it needs to be

  1. Blank value of proximity filter (exposed filter is not acting) => return any
  2. Filled in value of proximity filter that fails to geocode (exposed filter is acting but failing) => return none

I'm worried that geocoder will return an empty location for both of these cases, in which case geofield cannot tell the difference. That was why I thought we might need to move this issue to geocoder - in case of geocode failure it could return a non-empty invalid location or if there is no such thing return the south pole??

itamair’s picture

itamair commented
StatusFileSize
new geofield.proximity-failures.3053372-10.patch3.92 KB

@AdamPS .. may you test & review this new attached patch, that is complying with my #8 scenario/use cases,
and verify if it is matching your needs & vision too?
Please test it with a real geocode failure of you, that doesn't produce any origin (lat/lon) result ...
If it is ok for you I will commit into dev.

itamair’s picture

itamair commented
Status: Needs work » Needs review
adamps’s picture

adamps commented
StatusFileSize
new geofield.proximity-failures.3053372-12.patch3.59 KB
new geofield.proximity-failures.3053372-interdiff-10-12.txt1.2 KB

Aha good idea, clever. It wasn't quite right - here is a modified version that works for me.

itamair’s picture

itamair commented

Ah! come on ... you see?
You are mostly focused on your specific use case.
What about the more general scenario, if not a Geocode Origin source plugin is being used (such as Manual Origin)?
It should work the same: if the user send the Exposed form with empty Lat & Lon couple it shouldn't return any result?
Does your patch embrace this case too?

itamair’s picture

itamair commented
adamps’s picture

adamps commented

I don't find your feedback at all constructive. It's not necessary to keep making personal criticisms. Yes of course we each have a different use case. Of course it's easiest for me to test my own use case. It's a very common one - thousands of websites around the world have proximity search based on an exposed field.

This very common use case doesn't work right now, so I raised this issue. Your patch didn't solve the issue, so I made a patch that did.

I am trying to help so I don't see why you need to be so hostile and just hide my patches like that. If my patch fails to cover some other case, then it certainly wasn't deliberate. I would imagine that the ideas in my patch would be a useful step forward even if it's not the right patch. Probably someone needs to pick the ideas from my patch but do it better which is hard to do when you've hidden it.

Anyway I'm going to stop following this issue now and come back in a few weeks when I feel calmer.

itamair’s picture

itamair commented

Ehi, I am not hostile (and I am not hiding your patches ... just made it wrong, also hiding mine),
instead I am trying to manage this issue, of you (taking over my free time).

I made a further test on my last #10 patch, to better check your specific matter, in the following way:
- with the Geocode Origin source plugin selected and a wrongly set Geocoder plugin (such as Bing without any api key) so that I am sure the Geocoder would fail.

Well, everything acted properly, really: all results in the first Geofield View request (exposed proximity filter not acting) and no results after Applying the exposed proximity filter (Geocode failing, and not just finding an expected result).

You #12 patch instead (with the $has_origin) just seems to just look if the 'origin_address' form element is being filled with something (that is very limited and wrong check).

So ... #10 patch looks good to me, and is going to be committed, and this issue closed.
And you will be able to take advantage of all this.

itamair’s picture

itamair commented

  • itamair committed 01ee04b on 8.x-1.x 
    Issue #3053372 by AdamPS, itamair: Proximity filter must handle geocode...
itamair’s picture

itamair commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

adamps’s picture

adamps commented

Recap from #9

  1. Blank value of proximity filter (exposed filter is not acting) => return any
  2. Filled in value of proximity filter that fails to geocode (exposed filter is acting but failing) => return none

I believe the committed patch #10 fixes case 2 but breaks case 1. On my site, the location field is required so I don't actually care, and I will let some other poor soul worry about it.

itamair’s picture

itamair commented

Ah, this still seems not to be totally over.
But after checking further (with new Geofield 1.3 release) still I don't experience the same issues ...

Let's look at the working demo:
http://www.geodemocracy.com/geofield_d8/web/geoplaces-gmap-places-rates

My results are (still and again) the following:
1. Blank value of proximity filter (exposed filter is not acting) => return all results => Works as designed
2. Filled in value of proximity filter that fails to geocode (exposed filter is acting but failing) => return none => => Works as designed

Thus, in my opinion all this Works as Designed, as long other users step in reporting the same issues,
or (much better) find a properly and better working patch for all this.
If you don't care too, due to your specific configuration, I am even happier.

For all the others, well ... other D8 Geomapping modules might a valid term of paragon.

adamps’s picture

adamps commented
Issue summary: View changes

I'm disappointed with myself - I didn't test carefully enough before putting the latest release on our live server. So the site has been bugged for several months. Here is a link to the site: https://www.yogascotland.org.uk/nearby-classes?location%5Bsource_configu... so that you can see what I mean. However note that I will fix the site soon because I don't want to leave it broken.

As far as I can see, this issue is still open for the case in the issue summary, which I have edited slightly to make it as clear as possible.

The "working demo" linked from #22 seems to be for a different case. I don't see an exposed proximity filter on that page.

You are right that the patch from #12 is specific to my use case so can't be committed. It's not clear to me how to write a patch that works for everyone. So I'm going to apply patch #12 to my site because it's good enough for me. So there's not a patch, but there is still a bug - it seems like the status should be "Active".

nikunj.shah’s picture

nikunj.shah commented
StatusFileSize
new geofield.proximity-failures.3053372-24.patch1.18 KB

I was looking for the same and patch #12 is working fine for me. I have rerolled the patch for version 8.x-1.36.