I understand the motivation behind calling geocoder_widget_array_recursive_diff() in geocoder_widget_get_field_value() to prevent unnecessary web-service geocoding calls. However, in practice I've found it doesn't perform well.

2 separate issues:

1. When submitting, e.g. a node form, geocoder_widget_array_recursive_diff() always returns a non-empty, because the source_field_values always have form-specific fields.

2. Take for example an existing node -- one that existed before geocoder was installed.
When calling node_save() programatically, geocoder_widget_array_recursive_diff() interprets the node as not having changed and refuses to geocode the location data.

#1 is not such a big deal. Interactive form submissions _should_, arguably, fire a re-geocode.
#2, however, is very problematic for bulk-updating existing data.

(Note this is not the same issue as #1992762: Programmatic Update of an entity empties a geofield due to regression introduced/exposed in issue #1777934.)

CommentFileSizeAuthor
#8 geocoder-change-detection-improvements-2086073-8.patch1.5 KBeelkeblok
#6 geocoder-2086073-6.patch1.9 KBunivate
#1 geocoder-allow_programattic_geocoding-2086073.patch980 bytesaaronbauman

Comments

aaronbauman’s picture

aaronbauman
he/him
commented
StatusFileSize
new geocoder-allow_programattic_geocoding-2086073.patch980 bytes

This patch addresses #2 above.

aaronbauman’s picture

aaronbauman
he/him
commented
Status: Active » Needs review
ptmkenny’s picture

ptmkenny commented

Before applying the patch, Geocoder was not returning any results for nodes re-saved using VBO. After applying the patch, Geocoder returns appropriate results even when saving (or re-saviing) with VBO.

ptmkenny’s picture

ptmkenny commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
simon georges’s picture

simon georges commented

Does it affect the functionality outside of VBO?
Can anyone else confirm it works?

univate’s picture

univate commented
StatusFileSize
new geocoder-2086073-6.patch1.9 KB

I have tested this and it works to fix the issue. When there is an entity create before geocoder enabled and after geocoder enabled re-saving this entity does not get the change. With this patch you can re-save the entity and the geocoding runs.

The problem with this patch is when a form is submitted the values a field like addressfield may not include all keys as what is saved in the database (e.g. some country do not use all addressfield properties). This means even though the array returned is of empty values it will still attempt to geocode the address when really there is no difference. The values used in the last geocoding run are exactly the same.

The attached patch doesn't return a diff if the values are empty.

socialnicheguru’s picture

socialnicheguru commented
Status: Reviewed & tested by the community » Needs review

new patch. new review.

eelkeblok’s picture

eelkeblok
he/him
Primary language Dutch
Location Netherlands 🇳🇱
commented
StatusFileSize
new geocoder-change-detection-improvements-2086073-8.patch1.5 KB

The patch in #6 wouldn't apply with drush make, but after applying it manually and rerolling, it seems to work fine. Since I'm uploading a new patch I won't set this to RTBC yet, but functionally it seems to do the job.

ptmkenny’s picture

ptmkenny commented
Status: Needs review » Reviewed & tested by the community

Sorry, I forgot to update this, but I've been using the patch in #6 (it applied for me cleanly, I didn't use the re-rolled version in #8) for the past several months.

As I mentioned in my previous comment, this solves the issue in which Geocoder does not return results for nodes re-saved using VBO.

simon georges’s picture

simon georges commented
Status: Reviewed & tested by the community » Fixed

Thanks for the feedback. Committed !

Status: Fixed » Closed (fixed)

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