Multivalue encoding from another field has delta issues

Created on 13 August 2019, over 5 years ago
Updated 18 July 2024, 5 months ago

If you have one multi-valued addressfield field to let the users input there values and use geocoder widget to encode the address from this field, you can end-up with delta issues.

Create an entity with the address field with a delta of 2 values, save the entity. Check the result with your geofield formatter, you will endup with your 2 values. Fine.
Now, edit your entity and add a new address. Save it. Check the map (or whatever formatter). You now have only 2 visible values.
This is due to the logic in geocoder_field_attach_presave() that iterates over the geofield values already stored instead of the source field that may have a delta that varies.

We should have this field updated synchronously with the source field.

πŸ› Bug report
Status

Needs review

Version

1.0

Component

Code

Created by

πŸ‡«πŸ‡·France artusamak Bzh

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • First commit to issue fork.
  • πŸ‡ΊπŸ‡Έ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 calls geofield_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 .

Production build 0.71.5 2024