Formatted address space instead of comma and address_line2 issue

Created on 20 February 2025, about 1 month ago

Problem/Motivation

There are two issues that result in "no results found" for some more addresses (using Nominatim provider...not sure if others are the same). These issues were also tested on https://www.openstreetmap.org with the same results.

ISSUE 1
The formatter for the address returns \n before the city and is then converted to a space in AddressService > addressArrayToGeoString. This produces no results for some addresses. There was a similar task in the 8.x-3 version: https://www.drupal.org/project/geocoder/issues/3335292 🐛 Incorrect geocoding when suite number included in address Postponed: needs info

ISSUE 2
The other issue is that the provide is returning no results when the second address line is used (at least for suite numbers).

Steps to reproduce

Addresses used for testing:
3615 S 31st St
Temple, TX 76502 US

700 Lake Road
Belton, TX 76513 US

Geocoder AddressService addressArrayToGeoString cleanup:
default (newline to space):
3615 S 31st St Temple, TX 76502 US No results found
700 Lake Road Belton, TX 76513 US Result returned

default with suite number added
3615 S 31st St suite 100 Temple, TX 76502 US No results found
700 Lake Road Suite 100 Belton, TX 76513 US' No results found

change newline to comma and space (works with and without space).
3615 S 31st St, Temple, TX 76502 US Result returned
700 Lake Road, Belton, TX 76513 US Result returned

change newline to comma/space and include suite number (tried it with and without comma between address1 and 2).
3615 S 31st St, suite 100, Temple, TX 76502 US No results found
3615 S 31st St suite 100, Temple, TX 76502 US No results found
700 Lake Road, suite 100, Belton, TX 76513 US No results found
700 Lake Road suite 100, Belton, TX 76513 US No results found

Proposed resolution

AddressService > addressArrayToGeoString: exclude address_line2 and replace newlines with comma/space for address string cleanup.

🐛 Bug report
Status

Active

Version

4.0

Component

Code

Created by

🇨🇦Canada tklawsuc

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

Comments & Activities

  • Issue created by @tklawsuc
  • 🇮🇹Italy itamair

    thanks for reporting all this @tklawsuc
    BUT outcomes of my QA and tests reproducing your use cases say that all this happen on singular cases (Issue 1)
    and strangely depending on the Geocoder Provider that you use.
    Issue 2 happens only with Nominatim ... and the address line 2 doesn't compromises correct Geocoding with others, such as Photon or Google Maps.

    The best approach that I would propose is contained into the attached patch,
    that I feel could also benefit the related issue: https://www.drupal.org/project/geocoder/issues/3335292 🐛 Incorrect geocoding when suite number included in address Postponed: needs info

    A new "geocoder_address_values_alter" hook (in the "geocoder_address" submodule) that might allow any custom module to change/alter the Address Values array (at your specific use case) before it is stringified by the AddressService: addressArrayToGeoString.

    That can also do something like the following, to remove the "address_line2" value:

    /**
     * Implements hook_geocoder_address_values_alter().
     */
    function [custom_module]_geocoder_address_values_alter(array &$values) {
      $values["address_line2"] = '';
    }

    Please Qa and test this attached patch.
    I am pretty convinced to commit and add that into a new incoming Drupal geocoder 8.x-4.28 release ...

  • 🇨🇦Canada tklawsuc

    @itamair this is a great solution to address the second issue. However it does not solve the first issue which we encounter more than the second. What if we pass additional contexts to the hook like newline_separator and linebreak_separator?

  • 🇮🇹Italy itamair

    ah ok ... better got it.
    It is CommerceGuys\Addressing DefaultFormatter adding that "\n"
    but it is the Geocoder AddressService here:
    https://git.drupalcode.org/project/geocoder/-/blob/8.x-4.x/modules/geoco...
    that is converting it into a space ... rather than something more clear separator for the Geocoder provider, such comma ...
    right?

    Thus what about replacing both the "\n" and the "
    " eventually into a comma instead (",")??
    You propose ...

    Ok. I attache here an updated patch that adds these new replacements (in place of a space).
    Could you carefully and extensively test that and eventually confirm this would improve the Geocoder AddressService on a general basis, in your feelings?
    I think/assume that a comma (,) separator between different Address parts should be more clear with every Geocoder provider, isn't it?

    Let me know your outcomes with this new patch, please.

  • 🇨🇦Canada tklawsuc

    Unless there is a reason to use space instead of comma, I would make comma (and space) as the default for both separators as that will provide a clear separation between addessline and city. Some providers can't handle certain address without the comma before the city (as per my example 3615 S 31st St Temple, TX 76502 US will not produce results on https://www.openstreetmap.org but if you use the comma it works).

  • 🇨🇦Canada tklawsuc

    Confirming that patch in #4 works perfectly!

  • 🇮🇹Italy itamair

    thus ... could you QA and test the last attached patch and eventually mark this as RTBC eventually ... to approve (green flag) its commit into dev?

    • itamair committed eb262fa8 on 8.x-4.x
      Implementation of hook_geocoder_address_values_alter(), for altering...
  • 🇮🇹Italy itamair

    Patch #4 committed into dev branch, will be part of next Geocoder release.

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

Production build 0.71.5 2024