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

Feature/address subdivisions event #13361

Merged
merged 6 commits into from
Jun 28, 2023
Merged

Conversation

i-just
Copy link
Contributor

@i-just i-just commented Jun 27, 2023

Description

This PR adds:

  • a mechanism to supplement the commerceguys/addressing subdivision data,
  • event to allow developers to manipulate the subdivision data,
  • GB counties data.

Calls to Craft::$app->getAddresses()->getSubdivisionRepository()->getList() are now going through commerceguys/addressing, and our method. Afterwards a EVENT_DEFINE_ADDRESS_SUBDIVISIONS event is triggered to allow developers to manipulate the data even further.

Taking GB as an example, the counties data is now shipped with Craft, however it's still up to the developer if they want to show the AdministativeArea field in the Control Panel. If yes, you still need to use the EVENT_DEFINE_USED_FIELDS event, and you can change the label of the field via EVENT_DEFINE_FIELD_LABEL.

To manipulate the subdivisions data, you can now use the EVENT_DEFINE_ADDRESS_SUBDIVISIONS, like so:

Event::on(
    Addresses::class,
    Addresses::EVENT_DEFINE_ADDRESS_SUBDIVISIONS,
    function(DefineAddressSubdivisionsEvent $event) {
        if (count($event->parents) == 1) {
	    if ($event->parents[0] === 'GB') {
	        // adds a blank option to the beginning of the counties list
	        $event->subdivisions = ['' => 'Please select'] + $event->subdivisions;
	    } else if ($event->parents[0] === 'PL') {
	        // adds a list of voivodeship when selected country is Poland
	        $event->subdivisions = [
	                'Małopolskie' => 'Małopolskie',
			'Podkarpackie' => 'Podkarpackie',
	                'Ślaskie' => 'Ślaskie',
	                'Wielkopolskie' => 'Wielkopolskie',
	                ...
	            ];
	        }
	    }
        }
    }
);

The data added via the new addresses/SubdivisionRepository and via the EVENT_DEFINE_ADDRESS_SUBDIVISIONS is then used consistently throughout the Control Panel.

e.g. when adding Address for a User
Screenshot 2023-06-27 at 14 00 47

when using Administrative Area Address Condition (e.g. in a Shipping Zone in Commerce):
Screenshot 2023-06-27 at 13 36 27
Screenshot 2023-06-27 at 13 37 22

Related issues

#13101
#11788
#13345

@i-just i-just requested a review from a team as a code owner June 27, 2023 13:13
@brandonkelly brandonkelly merged commit 434342c into 4.5 Jun 28, 2023
@brandonkelly brandonkelly deleted the feature/address-subdivisions-event branch June 28, 2023 22:16
@samhibberd
Copy link

Just testing this feature out in advance of the 4.5 release, got the 'County' included by adding administrativeArea via the EVENT_DEFINE_USED_FIELDS event, but the label identifies the field as a 'Province', which looks to be the default / fallback administrativeAreaType.

It looks like the address format repository for the GB countryCode, returns null for $format->administrativeAreaType().

New to the Craft 4 addressing logic, and understand that I can tweak via the EVENT_DEFINE_FIELD_LABEL event, but not sure if I have missed anything or any required steps?

Also not sure if I have misunderstood, but grabbing the format and then requesting the used fields, skips any craft events, is that by design or should they be called:

// When called via the service the craft events are called:
Craft::$app->getAddresses()->getUsedFields('GB');

// If accessing them the format then they are not?
$format = Craft::$app->getAddresses()->getAddressFormatRepository()->get('GB');
$format->getUsedFields();
$format->getRequiredFields();
$format->getPostalCodePattern();

@i-just
Copy link
Contributor Author

i-just commented Jul 25, 2023

Hi @samhibberd ,
Yes, you'll need EVENT_DEFINE_FIELD_LABEL to change the label of the administrativeArea field.

New to the Craft 4 addressing logic, and understand that I can tweak via the EVENT_DEFINE_FIELD_LABEL event, but not sure if I have missed anything or any required steps?

You should also register the administrativeArea via EVENT_DEFINE_USED_SUBDIVISION_FIELDS if you want it to be returned when you call Craft::$app->getAddresses()->getUsedSubdivisionFields('GB');

grabbing the format and then requesting the used fields, skips any craft events

When you call Craft::$app->getAddresses()->getAddressFormatRepository()->get('GB'); you are getting the default formatter for GB (straight from CommerceGuys) which doesn't include administrative area.

It's also worth noting that in 4.5 you'll be able to create your own formatter thanks to this PR: #13242

@samhibberd
Copy link

Hi @i-just, thanks for the detail, all makes sense! So always going to be best running through the Craft service when building out our front end forms.

It's also worth noting that in 4.5 you'll be able to create your own formatter thanks to this PR: #13242

Already got this up and running, really useful!

4.5 getting close?

Thanks!

@i-just
Copy link
Contributor Author

i-just commented Jul 26, 2023

Not a problem 🙂

4.5 is not too far away, but we don't have a firm date.

@samhibberd
Copy link

Hey @i-just, not sure if i'm again missing some obvious stuff here, but struggling to get my head around this PR, the main issue we have is that we are migrating over 130k+ addresses of which most are UK addresses and have a County (assuming maps to an administrativeArea) and an Address Line 3 (which is now a custom field).

This PR allows us to identify the administrativeArea as a used field / and we can give it a custom label of County, but when using a custom formatter (so that these changes can reflect in the CP) none of the values / data (for administrativeArea or addressLine3) are available when building out the address format string, because the formatters use the getUsedFields() methods directly on the formats provided by the third party library.

Is it a case that we need to completely rewrite custom formatter logic to just build output an address string in the same format that craft requires) but by using our own logic and values that are set on the $address for uk addresses only.

public function format(AddressInterface $address, array $options = []): string
{
    if($address->getCountryCode() == 'GB')
    {
        return $address->addressLine1 . $address->administrativeArea;
    }
    return parent::format($address, $options);
}

It feels like we absolutely need this PR, as County is always asked for in the UK, but the complexities to get it setup are a creating a bit of a barrier, over here anyway.

Also not sure if these are bugs or further configurations, but when editing in the cp:

  1. The County is shown above locality, it should be below?
  2. If you change the value set on the now shown County field, it wipes out any value set in the locality (Town/City) input on the first change, although other values are retained.
Screen.Recording.2023-07-26.at.16.06.25.mov

@i-just
Copy link
Contributor Author

i-just commented Jul 26, 2023

This PR allows us to identify the administrativeArea as a used field / and we can give it a custom label of County

To be clear, you were able to define used fields, define field labels and define used subdivision fields since 4.3. This PR adds EVENT_DEFINE_ADDRESS_SUBDIVISIONS which lets you manupulate the subdivision data and also adds that data for GB.

but when using a custom formatter (so that these changes can reflect in the CP) none of the values / data (for administrativeArea or addressLine3) are available when building out the address format string, because the formatters use the getUsedFields() methods directly on the formats provided by the third party library.

I just tested the format you're after for GB - first line of address and county (administrative area) and that shows as expected. Screenshot below. Maybe I'm missing what you're after here?
Screenshot 2023-07-26 at 20 04 49

RE point 1:
The formatter affects how address shows on the card (e.g. on the list of addresses in user's arccount, like in the screenshot above). It doesn't affect the order of fields in the Control Panel. (And you can control the order of fields on the front-end yourself.)

RE point 2:
That happens for other countries too (e.g. USA). I'll have a look into it - thanks for reporting!

@samhibberd
Copy link

Thanks @i-just, I think perhaps a little bit more reading up required, felt like it was getting rather complex to cover what i thought should be pretty simple.

I just tested the format you're after for GB - first line of address and county (administrative area) and that shows as expected. Screenshot below. Maybe I'm missing what you're after here?

Sorry, my code wasn't clear, I was just providing a basic example of how I might format the address directly not using the logic / methods from the default formatter. I did a bit more reading up and ended up being able to use the getValues() and buildView() methods from the default formatter to get the formatter to behave correctly. Great because I could also include the custom field Address Line 3 in the address output for all addresses (if it has data), and output in the same format as the default formatter:

<?php

namespace modules\findarace\formatters;

use craft\helpers\Html;
use CommerceGuys\Addressing\AddressInterface;
use CommerceGuys\Addressing\AddressFormat\AddressFormat;
use CommerceGuys\Addressing\Locale;

class AddressFormatter extends \CommerceGuys\Addressing\Formatter\DefaultFormatter
{
    protected $defaultOptions = [
        'locale' => 'en',
        'html' => true,
        'html_tag' => 'p',
        'html_attributes' => [],
    ];

    public function format(AddressInterface $address, array $options = []): string
    {
        $this->validateOptions($options);
        $options = array_replace($this->defaultOptions, $options);
        $countryCode = $address->getCountryCode();
        $addressFormat = $this->addressFormatRepository->get($countryCode);

        // Add the country to the bottom or the top of the format string,
        // depending on whether the format is minor-to-major or major-to-minor.
        if (Locale::matchCandidates($addressFormat->getLocale(), $address->getLocale())) {
            $formatString = '%country' . "\n" . $addressFormat->getLocalFormat();
        } else {
            $formatString = $addressFormat->getFormat() . "\n" . '%country';
        }

        // Include addressLine3 for al addresses
        $formatString = str_replace("%locality\n", "%locality\n%administrativeArea\n", $formatString);

        // Include administrativeArea for GB addresses only
        if($countryCode == 'GB') {
            $formatString = str_replace("%addressLine2\n", "%addressLine2\n%addressLine3\n", $formatString);
        }

        $view = $this->buildView($address, $addressFormat, $options);
        $view = $this->renderView($view);

        // Insert the rendered elements into the format string.
        $replacements = [];
        foreach ($view as $key => $element) {
            $replacements['%' . $key] = $element;
        }
        $output = strtr($formatString, $replacements);
        $output = $this->cleanupOutput($output);

        if (!empty($options['html'])) {
            $output = nl2br($output, false);
            // Add the HTML wrapper element.
            $output = Html::tag($options['html_tag'], $output, $options['html_attributes']);
        }

        return $output;
    }

    protected function getValues(AddressInterface $address, AddressFormat $addressFormat): array
    {
        $values = parent::getValues($address, $addressFormat);
        $values['addressLine3'] = $address->addressLine3 ?: null;
        if($address->getCountryCode() == 'GB') {
            $values['administrativeArea'] = $address->administrativeArea ?: null;
        }
        return $values;
    }

    protected function buildView(AddressInterface $address, AddressFormat $addressFormat, array $options): array
    {
        $view = parent::buildView($address, $addressFormat, $options);
        $view['addressLine3'] = [
            'html' => $options['html'],
            'html_tag' => 'span',
            'html_attributes' => ['class' => 'address-line3'],
            'value' => $address->addressLine3 ?: null,
        ];
        if($address->getCountryCode() == 'GB') {
            $view['administrativeArea'] = [
                'html' => $options['html'],
                'html_tag' => 'span',
                'html_attributes' => ['class' => 'administrative-area'],
                'value' => $address->administrativeArea ?: null,
            ];
        }
        return $view;
    }
}
  1. Ok understood, so currently no way to control the order of fields in the cp.
  2. Thanks.

@i-just
Copy link
Contributor Author

i-just commented Jul 27, 2023

Hi @samhibberd, a quick update to clarify that the locality (City/Town) getting cleared if the administrativeArea changes is the intended behaviour. But you are also right, there are some issues there, for which I raised this: #13486.

Here's how the locality/administrativeArea funtionality works on 4.4.16.1:

Screen.Recording.2023-07-27.at.11.33.03.mov

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

Successfully merging this pull request may close these issues.

3 participants