🇷🇸Serbia @bojanz

8.x-1.x is closed, so this would need to be reworked on top of the latest 2.x branch (2.1.x at the moment)

Thanks!

Your commerceguys/addressing version is wrong or broken, indicating you didn't install or update Address via Composer.

Let's do this.

Committed. We can open a new issue for any extra deprecations.

I've just dropped Drupal 10.2 support in 2.1.x, so this issue can now proceed, along with any other 10.3 deprecation fixes.

Merged, thanks.

Let's get the CI green in the other issue.

Merged the MR. Thanks!

I'll look at the commerceguys/addressing MR again in about 6 months.

Committed, thanks.

Thank you for your efforts!

Since 10.2 went out of security support this week, maybe we should just fix these now and have min 10.3 support

That sounds fine, let's do that.

CSpell - Most words looked project specific, the only ones that I wasn't sure on are there are a few UI labels using British spelling (e.g. Neighbourhood instead of Neighborhood) - it looks from a quick scan that this was withing the context of screens where the US spelling is presented (e.g Organization instead of Organisation) so I assume these are genuine misspellings.

Yes, those are genuine misspellings we should fix.

I'll go ahead and commit 📌 Drupal 11 compatibility fix for removed views default_argument_skip_url setting Active now.

If a $price->round() is desired, perhaps it take a $digits argument, and then you use Rounder if you want $digits to be auto-resolved for you?

There is a Rounder service. $rounder->round($price) takes the configured number of decimals for a currency and rounds to that precision.
This loading of the number of decimals is why it's a separate service.

If you know the number of decimals yourself, you can always call Calculator::round() directly.

Setting $scale will truncate any extra decimals. In an eCommerce context this is rarely what you want, if you just multiplied/divided something, it would be more precise to round instead. That's why $scale wasn't exposed when we wrote the API, we wanted to guide people towards rounding.

FWIW I am fine with merging and releasing the changes from the MR, the commerceguys/addressing changes are the ones that require bumping the major versions.

We started the 2.x branch a year ago and it is still approaching 50% adoption: https://www.drupal.org/project/usage/address →

So I wouldn't want to do a change like this (requiring a 3.x) for another year.

Is there any downside to waiting that we need to document? Where would the deprecation errors be the most annoying / have to be ignored?

Might have more luck asking in the Ludwig queue, I don't use it anymore so I can't help.

catch → credited bojanz → .

We support custom entities just fine (profiles, commerce stores, etc). Of course, it depends on how you've implemented your own form, as there could be bugs there. Check the dev console in your browser, check watchdog, see if there's an error you can investigate.

I greatly prefer the current summary, and I feel that "international" is descriptive enough. So, let's keep it.

A possible workaround is for example to place the variation field on the product form and use https://www.drupal.org/project/inline_entity_form → to have it right in place, but that also has some downsides and I think it would be worth a general UX improvement.

But we do that automatically when you uncheck "Allow each product to have multiple variations" on your product type, implemented and released 6 years ago. Or am I misunderstanding what you're proposing?

There's also the older discussion at 📌 Clarify order language behavior Needs work which might have important information.

Do you have the Address module installed by any chance? If yes, it has taken over the country list, CountryManager is not used.

Right. Unfortunately, I don't see what we can do on our end, since it's not our formatter (nor addressfield's).

If you end up solving it, please share here.

From a quick search it looks like the linear formatter was provided by https://www.drupal.org/project/addressfield_tokens → , and now your system is complaining that it doesn't exist on the D8 side.

We do have migration plugins for addressfield, defined here:
https://git.drupalcode.org/project/address/-/blob/2.1.x/src/Plugin/migra...
https://git.drupalcode.org/project/address/-/blob/2.1.x/src/Plugin/migra...
but I don't know what the right workaround here would be. I guess you need to reset the used formatter during the migration?

How is this fixed if we didn't commit the patch?

A full upgrade path was provided, in the sense that you can actually update an existing site from 8.x-1.x to 2.0.x while the old data is kept functional (and is auto-converted to the new administrative area format).

Line3 was not hidden because we assumed existing installs would appreciate extra room for address details, and it's not breaking because it's optional. 7 months and 35 000 installs later, you are the first one complaining, so it feels like the right choice.

You can edit your address field and set the line3 as hidden in the field settings.

This is for compatibility with Diff 2.0.x, right? Diff 8.x-1.x still works?

This is a bug in the field_validation module, which is accidentally overriding the Commerce-provided currency constraint.

Based on #9, it sounds like we haven't

The original code has the right intent, we can't make the fingerprint unique across job statuses, because that would mean that once a job completes, it can never be requeued again. Same with failure, if we gave up on a job, it needs to be requeueable.

However, that doesn't cover failing-but-retrying jobs, if a job is retrying, we should not allow a duplicate to be queued. I think that's what we missed.

By definition a fingerprint must be unique. Therefore it should be impossible to queue two jobs with the same fingerprint. Producers who wish to run a job after all can influence the fingerprint by including an always-unique component such as a timestamp.

Build is green, let's proceed.

Found one in AddressItem as well.

I also disagree with

Ideally, modules shouldn't need to set "drupal/core" as a dependency, because in the context of Drupal, this is totally implied.

As a module developer, I want `composer require drupal/mymodule` to fail for users who have an unsupported / old core version.

I just ran into the same problem with Address: #3459145: "phpunit (max PHP version)" running with Drupal 10.1.8 → .

The composer.json requirement was "^10 || ^11" and it caused Drupal 10.1 to be used for the max PHP tests instead of 10.3. Removing the requirement fixed the build.

I can confirm that removing the composer.json requirement solved the problem.

Marking the support request as fixed, and providing info in the other issue.

bojanz → changed the visibility of the branch 3425922-error-cannot-assign to hidden.

@VladimirAus
Ever managed to look at this?

I'd still like to do the simplification from #14.

Better status.

At this point I think 8.x-1.x-dev is closed.

Possibly related: #3458238: CI installs wrong Drupal version → .

2.1.x declares a drupal/core requirement of "^10 || ^11", but i'd expect ^10 to still result in ^10.3.

Thanks jsacksick, opened #3459145: "phpunit (max PHP version)" running with Drupal 10.1.8 → to get clarity on why this is happening on this branch only.

bojanz → created an issue.

Our next release is 2.1.0, so let's change the version and start an MR.

Address 2.0.2 is now out: https://www.drupal.org/project/address/releases/2.0.2 →

@deepakkm
You hijacked an issue for automated patches, then opened an MR that had 11 commits, none of which contained any Views fix that I could see. If you have a fix to discuss, open a new issue with more information.

The current patch modifies the behavior of the filter instead of introducing a setting which would control $only_used. Which means that it cannot be committed in this form.

The postal_code_pattern for IT is:

                'postal_code_pattern' => '\d{5}',

So the only requirement is "5 digits", and 24050 fits that rule.

There is no state-specific validation. Address 2.x removed it, after 1.x deprecated it in #2828128: Stop performing subdivision-level postal code validation → , due to user complaints.

FWIW at the time we considered it an absolute requirement that manually placed orders can also get order receipt emails, as not supporting that was considered notable in 1.x.

I think there will always be users in both groups (who want and don't want the email to be sent).

This problem was solved by integrating with Layout Builder. If you're not using Layout Builder, then you write your own Twig template. There is no middle ground, and there won't be any other fix.

Look at comment #3, the commit was made on "16 March 2024 at 14:23", so that seems to match?

If you mean 2.0.x, it's already there?

I think it's nice for Address to have a US-specific service that takes a postal code and returns an administrative area. That service can then hardcode the data it needs (the postal code prefixes that are matched against).

Would it make sense to call it a locator? \Drupal\address\Locator\UnitedStates? I'll let you and Jonathan bikeshed that.

@mishradeepak1991 I have no clue what you're doing. All code posted to this issue was already committed.

There you go then. You'll have to fix that patch (just add the use statement you're missing).

And no, that feature is not in 2.0.x yet.

AddressDefaultFormatter does not have a settingsForm() method in the 2.x branch:
https://git.drupalcode.org/project/address/-/blob/2.x/src/Plugin/Field/F...

It sounds like you are applying a patch which is incomplete (didn't add a use statement for the FormStateInterface).

I honestly don't see why the processor should every quit when the queue is empty. I know we originally implemented it that way, but I have no recollection why. Most other queue processors will sleep for a configurable amount of seconds, then try to claim a job again.

bojanz → created an issue.

8.x-1.x will not be made compatible with Drupal 11.

Quoting myself from the Address issue [#3439726#comment-15544880]:

I am fine with not having or using this setting at all. We only introduced it in the 2.0 release ~4 months ago, to have a fallback when the field has no default value configured, and it was community requested.

Similarly, Commerce uses the site country as a last fallback simply because it's there, the main source of the default country is the Store entity.

+1 to what dww said.

I am fine with not having or using this setting at all. We only introduced it in the 2.0 release ~4 months ago, to have a fallback when the field has no default value configured, and it was community requested.

1. Shipping rate: base flat rate plus extra per item. It's like minimum charge is $10 and add $2 per item in the order;

Commerce Shipping supports per-item rates, but it doesn't support setting a base rate yet. You need either ✨ Base rate for per item flat rate shipping Needs review or ✨ Add Shipping method with combination of Flat rate + Flat rate per item Needs review .

4. customizable workflow: configure more status or different status as needed

https://docs.drupalcommerce.org/commerce2/developer-guide/orders/workflows

This was a decision made by Google, who defined our initial dataset for Chrome/Android use.

The organization comes first for 31 countries:
- AT, AU, AX, BE, BL, BR, BY, CC, CH, CX, DE, FI, FR, GF, GP, HM, IQ, IR, LI, LT, LU, MF, MQ, NC, NF, NL, PM, RE, SE, WF, YT

So this includes Austria/Germany/Switzerland, France/Luxembourg/Belgium, Australia, and others.

I can do some research to see whether this default still makes sense. In the meantime, you can implement an event subscriber for the AddressFormatEvent, and alter any format as you wish.

@VladimirAus
Can you give us the output of this:

    var_dump($language_manager->getConfigOverrideLanguage());
   var_dump($language_manager->getConfigOverrideLanguage()->getId());

From what I can see, core issue #2684873: ConfigurableLanguageManager::getConfigOverrideLanguage() returns NULL → was fixed ages ago (Drupal 8.3), so our logic should be as simple as:

    $this->defaultLocale = $language_manager->getConfigOverrideLanguage()->getId();

According to the interfaces, getConfigOverrideLanguage() always returns a language, and getId() always returns a string, so a crash should be impossible.

Apologies. I haven't used this module in almost a decade, so I do not monitor its issue queue. Additionally, I do not have any email from cafuego in my inbox, nor did I get pinged on Drupal Slack.

I did find the email from gisle, but GMail decided to send it to spam, which is the first time I've seen that happen to drupal.org emails.

I've now given cafuego full access to the project.

https://www.drupal.org/project/simple_oauth → is what others seem to be using.

This is the crashing code:

  public function __construct(CacheBackendInterface $cache, LanguageManagerInterface $language_manager) {
    parent::__construct();

    $this->cache = $cache;
    // The getCurrentLanguage() fallback is a workaround for core bug #2684873.
    $language = $language_manager->getConfigOverrideLanguage() ?: $language_manager->getCurrentLanguage();
    $this->defaultLocale = $language->getId();
  }

It should not be possible for $language->getId() to be null. It indicates something very wrong with your site (or a core bug).

I suggest adding some debug code here to see which language is used (config override or current language) and what the value of $language is.

Is there enough value in actually support the "merge" strategy? Or the concept of strategies at all?

In all other implementations I've used, a job/task is either unique/deduplicated (meaning that a job with the same fingerprint is rejected on enqueue) or it isn't.

My assumption without reading the code would be that I do:

// If I don't provide a fingerprint, it is autogenerated for me.
$job = Job::createUnique("my_type", $payload, ['fingerprint' => 'my-unique-value']);

enqueueJobs() can check if $job->getFingerprint() is non-empty, and if the backend does not implement SupportsUniqueJobsInterface, throw an exception.

3. I REALLY want to rename the DB column we're adding from unique_id to something like job_hash. It's going to be a source of confusion and bugs with the current name. I appreciate that sites are already using this patch.... but that's a risk you take when using a patch.

FWIW I've been using "fingerprint" as the column name for this purpose in a few other places. It is fairly common in various libraries.

I do agree that "job_hash" sounds better than "unique_id" at least. Every ID is unique so the column name sounds confusing.

This issue is now unblocked (we switched the module to addressing v2.2.0)

Thanks, everyone!

Drupal.org is insisting that "Bluespark" is the customer I'm doing this for, no clue why.

Anyway, committed.

bojanz → created an issue.

You need to subscribe to the AddressFormat event.

You can then modify $definition['format'] to always have %additionalName.

See https://git.drupalcode.org/project/address/-/blame/2.x/tests/modules/add... for an example.

@lazzyvn
We need either a test or a small form class where the problem can be observed.

Changing status to "needs review" to signal that there is a patch here.

Changing version to 2.x-dev, this will have to go into the 2.1.0 release, 8.x-1.x is now closed.

Went ahead and tagged 2.0.1: https://www.drupal.org/project/address/releases/2.0.1 →

I am fine with proceeding with 2.0.1 without 🐛 Mismatched field definitions after address upgrade Needs review . It only affects a portion of the users, and I personally have no time to work on it, so I don't want to hold off other fixes because of it.

Layout Builder integration was done to solve the site builder use case.

So those are the two theming options. Twig or Layout Builder. Full control just via the "manage fields" screen will never be possible.

Can we please try to debug the original update function, and fix the problem there, before we introduce update functions with workarounds?

Our main problem here is that none of the maintainers have sites that are affected, so we need a db dump where the problem can be observed, or we need a developer with such a site to do the debugging themselves.

We're already discussing this in 🐛 Mismatched field definitions after address upgrade Needs review so let's continue there. I am glad you are able to reproduce the problem, that means you should be able to debug the existing update function and see why it's failing (is the field not getting found? what is the error behind the WSOD? etc)

Let's go borderless.

There is no patch here, changing status.

No, it's an Address bug for sure.

Unfortunately, I have no time to chase it, someone from the Commerce (Reporting) side will have to take a look.

This module needs a full rewrite and hasn't had a real sponsor since 2016. It is completely unsustainable. But that's the reality of most of Drupal contrib.

There was a Drupal core issue about implementing IEF in core: #2820235: Add inline entity form to core as experimental → but it didn't receive a lot of attention so far.

I misunderstood the request when I first closed this issue.

Unfortunately, Address has no designated property for US counties (Google calls it `administrative_area_level2`), it's not a use case we support. Adding a new property from outside the module would be impossible, you'd have to essentially fork the entire module codebase. A workaround is needed: repurposing an existing property.

We have two possible ways to achieve this:

1) Repurpose address_line3 to hold the county.
Implement a hook_field_widget_form_alter() + #after_build for the address element, which looks at the currently selected country, and if it's US, converts address_line3 to a dropdown, gives it a new label, and populates the options based on the administrative area.

2) Repurpose the locality field to hold the county, and move the City to the dependent_locality field.
This allows you to use SubdivisionEvent to define the counties per administrative area, and it will just work. An alter is only needed to adjust the labels. However, you need to teach all other code (payment gateways, shipping methods) to pass $address->getDependentLocality() instead of $address->getLocality() for the city.
You also need an AddressFormatEvent to add the %dependentLocality to the format.

Updating version. We will commit this for Address 2.1.0.

I've done what I promised in #24, the library has been fixed, now it just needs a new release.

Thanks!

This is now 2.1.0 material, so bumping the version.

Made a 2.x branch and a 2.x-dev release with the same warnings as 11.x:
https://www.drupal.org/project/address/releases/2.x-dev →