- First commit to issue fork.
- Merge request !57Issue #3074598: Ensure multi-value field items are updated on edit. β (Open) created by lwalley
- πΊπΈUnited States lwalley
The pull request is a rewrite of the logic in
geocoder_field_attach_presave()
on 7.x-1 that decides which field delta items to update.I have only tested it when geocoding from a multi-value addressfield without any overrides. It needs testing with other types of fields, as indicated in the previous issue that modified this logic: #3002517: Geofield remains empty when I choose "Geocode from another field" β .
I based the rewrite on the existing logic and some assumptions:
$geocoded_value = geocoder_widget_get_field_value($entity_type, $instance, $entity);
is always the source of truth for field items regardless of the source field.- If
$entity->geocoder_overridden[$field_name][$langcode][$delta]
is not empty, then we should ignore the newly geocoded value and instead use the existing field item as is if it exists, if it doesn't then use the geocoded value (made should throw an error as that sounds like a logic issue).
- πΊπΈUnited States lwalley
I updated the pull request to account for the field not being set yet on the entity on node/add. Fixes
Warning: Undefined property: stdClass::$field_coordinates in geocoder_field_attach_presave() (line 308 of geocoder.widget.inc).
.I also spent a bit of time looking at the manual override to make sure the logic made sense, it seems to, but it also assumes overrides are actually working for multi-value fields which they don't seem to be. My best guess is that since geocoder module is declaring custom behavior for multiple values that it needs to actually handle cardinality > 1 when rendering the hidden geofield for overrides:
The geocoder_field_widget_info() function declares
'multiple values' => FIELD_BEHAVIOR_CUSTOM
. However,
geocoder_field_widget_form() directly callsgeofield_field_widget_form()
but doesnβt handle deltas. Compare this to e.g., media_field_widget_form() where it checks cardinality and handles adding field elements for each delta.Supporting multi-value overrides is probably out of scope for this issue of keeping geocoded values in sync with address fields. So I haven't made any attempts to fix it. Separate issue should probably be opened if someone confirms it's actually broken, and not just something I couldn't get to work with my configuration.
I have only tested this patch with geocoding from multi-value address fields (both geofield and address field set to cardinality 10). More testing is needed for other scenarios, as indicated in the previous issue that modified this logic: #3002517: Geofield remains empty when I choose "Geocode from another field" β .
There are other issues open that attempt to adjust this function and might also be accidentally solved or further broken with my pull-request, e.g.: π Empty geocoding results do not respect the override checkbox state. Needs review .