Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CN address locality issue (invalid for unknown reasons) #183

Open
bakkerpeter opened this issue Jan 31, 2023 · 7 comments
Open

CN address locality issue (invalid for unknown reasons) #183

bakkerpeter opened this issue Jan 31, 2023 · 7 comments

Comments

@bakkerpeter
Copy link

Hi,

I'm having a hard time trying to understand why the following address is not validated as correct. I'm using this address as an example: Marriott Shanghai

 $address = new \CommerceGuys\Addressing\Address(
        "CN",
        "Shanghai Shi",
        "Shanghai",
        "Huangpu District",
        "200003",
        null,
        "555 Middle Xizang Road",
        null,
        "Marriott",
        "John",
        null,
        "Doe",
        "en"
    );

$validator = Validation::createValidator();
$violations = $validator->validate($address, new AddressFormatConstraint());

It turns out that the locality is invalid. When trying to get to the source of the validation error I found that $subdivision could not retrieved from the subdevisionRepository and therefore it fails. I'm not sure what this causes or means, also I'm not sure if it's a bug or me doing something wrong regarding input.

$subdivision = $this->subdivisionRepository->get($values[$field], $parents);

AddressFormatConstraintValidator.php - LN113

If someone could point me out what is going wrong here that would be of great help, thanks!

@bojanz
Copy link
Contributor

bojanz commented Mar 15, 2023

Sorry for the late response here.

These are the valid localities for "Shanghai Shi": https://github.com/commerceguys/addressing/blob/master/resources/subdivision/CN-41c52eb75de1872e3f7d008da80c7aaa.json

As you can see, "Huangpu Qu" is one of them, so "Shanghai" is the missing level.

This is a known issue documented in google/libaddressinput#86 which Google has failed to address for a long time. Now that we no longer auto-generate the subdivisions from Google, we'll be able to proceed with a fix, but I'll need to consult with @skyred about the right shape of the data.

@skyred
Copy link

skyred commented Mar 16, 2023

Below uses the latest version of Address on Drupal,

image

"Shanghai" is not a province or state, and "Huangpu Qu" is not a city.

This is exactly same situation as D.C. in U.S.

image

So, I imagine if you do the same validation on an address in the D.C., then you would see the same invalidation?

@bakkerpeter
Copy link
Author

@skyred I'm not sure if you are asking me a question?

@skyred
Copy link

skyred commented Apr 6, 2023

I didn't look into the code, but from the UI I tested, it looks expected, and it is working for me.

@bakkerpeter
Copy link
Author

I'm sorry, but I'm still not sure what you are saying. Please don't get me wrong, I appreciate your help but basically my question has nothing to do with Drupal whatsoever. I'm only curious to know is the validation error a bug (and therefore is the address valid) or is my input wrong (and thus the address is correctly marked as invalid).

When you say it is working for you I really have no idea if the validation error is caused by the first or the latter.

The thing is, if there is a bug in the underlaying Google data I totally understand that there is not much we can do in this repo but if there is a bug regarding the subDevision retrieval then maybe we can help this great package forward.

@bojanz
Copy link
Contributor

bojanz commented Apr 20, 2023

We no longer just mirror the Google data, we've forked it (in 2.0.0-dev / master branch), so we can make whatever changes we deem necessary. I just don't know what they are in this case :)

@TerraSkye
Copy link

@bojanz if im looking at google/libaddressinput#86 this issue
@skyred suggesteda a fix : in the first comment to duplicate the adminstrative area

is this a solution we can implement?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants