Problem/Motivation
Address formatter can't accept null values yet will try anyway resulting in errors like TypeError: CommerceGuys\Addressing\Address::withDependentLocality(): Argument #1 ($dependentLocality) must be of type string, null given
Steps to reproduce
Create an address field and leave out locality and save.
Proposed resolution
Check if values are null in address service before using the formatter.
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork geocoder-3367781
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
amy.h commentedMerge requested and ready for review. My first merge request was done by accident to the wrong branch. Branch 3367781- is the intended branch.
Comment #5
amy.h commentedComment #6
finex commentedHi, the patch works fine. Thank you!
Comment #7
chucksimply commentedMerge patch works great. New release please!
Comment #8
itamair commentedNo sorry! I don't get this issue in the described use case ...
First of all I don't get why you are mentioning the Address Formatter and then pointing to the AdressService Service: those are very different things in Drupal terminology.
But btw ...
I can set an Address Field properties and having set as optional some fields (such as Country, Address 1 and Locality, and Postal Code),
and there is no kind of error in the AddressService->addressArrayToGeoString method if I omit some of them. It simply generates the consequent Address string with the input values and try to Geocode on the basis of it.
So, for instance, if I just input:
Address 1: Fifth Avenue
Country: United States
it will generate this
And then generate the following CommerceGuys\Addressing\Address $address object:
and finally the following $address_string = "Fifth Avenue US"
and consequently try to Geocode for it ...
Indeed I don't see any of the issue that you reported ... and I am consequently not going to merge this MR, unless you feel to provide more evidence and better explanation of any real general issue on this use case (and not only very specific to kind of your custom implementation).
Comment #9
dadderley commentedGeocoder 8.x-4.15
PHP 8.1.26
This release killed the functionality on 2 different sites.
Editing any field in the address causes the error below and the WSOD.
TypeError: CommerceGuys\Addressing\Address::withDependentLocality(): Argument #1 ($dependentLocality) must be of type string, null given, called in /home/customer/www/mysite.com/public_html/modules/contrib/geocoder/modules/geocoder_address/src/AddressService.php on line 135 in CommerceGuys\Addressing\Address->withDependentLocality() (line 228 of /home/customer/www/mysite.com/public_html/vendor/commerceguys/addressing/src/Address.php).Comment #10
dadderley commentedSince the error message is different, I created an new issue over here.
https://www.drupal.org/project/geocoder/issues/3408026
On another site, I have a slightly different error.
TypeError: CommerceGuys\Addressing\Address::withPostalCode(): Argument #1 ($postalCode) must be of type string, null given, called in /home/customer/www/myothersite.ca/public_html/modules/contrib/geocoder/modules/geocoder_address/src/AddressService.php on line 133 in CommerceGuys\Addressing\Address->withPostalCode() (line 247 of /home/customer/www/myothersite.ca/public_html/vendor/commerceguys/addressing/src/Address.php).PHP 8.2.13
In this use case I am only using the Country, City and Province fields.
Now I cannot create new nodes with an address field or edit any fields in the address fields.
Comment #11
itamair commentedOk ... in doubts it could be related to my PHP version I gave another try to this, and run the last reported use case (so a Geocoding on an Address field with only Country, City and Province fields.) on PHP 8.2 and Drupal v. 10.1.6 and Geocoder 8.x-4.15 ... and all goes very smooth to me.
For instance this Address:
City: Marzabotto
Province: Bologna
Country: Italy
becomes this $value:
in the same AddressService->addressArrayToGeoString method
and is correctly geocoded in the target Geofield without any errors.
Indeed I still don't see any evidence of a general bug on this and don't feel to merge anything from here, in the 8.x-4.x branch.
Anyway ... I really experience some bugs or issues in his specific Drupal instance is still able to apply the related patch generated by this MR: https://git.drupalcode.org/project/geocoder/-/merge_requests/30.diff
I will accept this as Open and general bug only when evidence of a general reproductivity of it will be provided.
Please properly Xdebug your issue use case and come back with clear evidence of where you code workflow is breaking ... and why.
Comment #12
dadderley commentedOK, found the problem and is not geocoder.
On further examination, I realize that the error message
TypeError: CommerceGuys\Addressing\Address::withDependentLocality(): Argument #1 ($dependentLocality) must be of type string, null given, called in /home/customer/www/mysite.com/public_html/modules/contrib/geocoder/modules/geocoder_address/src/AddressService.php on line 135 in CommerceGuys\Addressing\Address->withDependentLocality() (line 228 of /home/customer/www/mysite.com/public_html/vendor/commerceguys/addressing/src/Address.php).Is deceiving, I really thought that it was a geocoder issue.
On these sites I had this version of the Address module installed 2.0.0-beta3 released 1 December 2023.
On the site where I had this error message
TypeError: CommerceGuys\Addressing\Address::withPostalCode(): Argument #1 ($postalCode) must be of type string, null given, called in /home/customer/www/myothersite.ca/public_html/modules/contrib/geocoder/modules/geocoder_address/src/AddressService.php on line 133 in CommerceGuys\Addressing\Address->withPostalCode() (line 247 of /home/customer/www/myothersite.ca/public_html/vendor/commerceguys/addressing/src/Address.php).I rolled address back to Address 8.x-1.12 released 21 May 2023
The error went away, no problem.
So, it seems that my specific problem is with this Address 2.0.0-beta3 released 1 December 2023.
Sorry for the noise and thanks for your time.
Comment #13
bojanz commentedYou can't pass NULL to a method expecting a string.
The reason why not everyone is seeing the same error is because typehints were added in Address 2.x and commerceguys/addressing 2.x, so only people using the latest code get the crash.
So, AddressService needs some
$value ?? ''love.Comment #14
dadderley commentedThanks bojanz.
This has been my experience so far,
Geocoder 8.x-4.15 will work with Address 8.x-1.12
Geocoder 8.x-4.15 will not work with Address2.0.0-beta3
https://www.drupal.org/project/address/issues/3408634
Comment #15
itamair commentedthanks @bojanz ... this finally makes more sense to me
Added as reference the Address module issue ...
Going to run a final review and merge this MR asap, and deploy a new release.
Comment #16
itamair commentedComment #19
itamair commentedThanks everybody for the great contribution (and for pushing me) here.
MR merged in both 3.x and 4.x branch, all will be part of next Geocoder releases.
Comment #20
dadderley commentedI look forward to this release and can help test.
Comment #21
itamair commentedThanks @dadderley ... new Geocoder releases deployed.
But everything already tested before of it, in this MR ...
(Review and test/qa should happen before the new release deploy, but feel free to double check all working fine on this side now).
Comment #22
dadderley commentedThis has resolved the issue on the 2 sites mentioned above.
Geocoder 8.x-4.16 will not work with Address2.0.0-beta3
Thank you itamair.