Problem/Motivation

The commerce contrib module, in the Price submodule, is declaring a "Currency" constraint and providing an "availableCurrencies" option which collides with the change in #3382104 and provoking the following error :

Symfony\Component\Validator\Exception\InvalidOptionsException: The options "availableCurrencies" do not exist in constraint "Drupal\field_validation\Plugin\Validation\Constraint\CurrencyConstraint". in Symfony\Component\Validator\Constraint->normalizeOptions() (line 180 of /app/vendor/symfony/validator/Constraint.php).

Steps to reproduce

  1. Clean D10 install
  2. Install and enable both Commerce Price & Field Validation modules
  3. Add a "price" field to a content type
  4. Create a new content type and try to validate the form

Proposed resolution

Change the "CurrencyConstraint" plugin ID to avoid collisions with Commerce module.

Remaining tasks

Propose patch.
Review patch.

User interface changes

n/a

API changes

n/a

Data model changes

n/a

Issue fork field_validation-3459251

Command icon 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:

About issue forks

Comments

zaporylie created an issue. See original summary.

qaiser iqbal’s picture

qaiser iqbal commented

My suggestion would be to change the plugin id in https://git.drupalcode.org/project/field_validation/-/blob/3.0.x/src/Plugin/Validation/Constraint/CurrencyConstraint.php?ref_type=heads#L12 to something like SymfonyCurrency (this is the root cause of the conflict) and in https://git.drupalcode.org/project/field_validation/-/blob/3.0.x/src/Plugin/FieldValid[…]le/CurrencyConstraintFieldValidationRule.php?ref_type=heads make sure the updated constraint ID is utilized

zaporylie opened merge request !9

zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
Status: Active » Needs review

MR is in place.

leducdubleuet’s picture

leducdubleuet commented
Status: Needs review » Reviewed & tested by the community

I can confirm the fix is working as expected with version 3.0.0-beta4 and current Commerce 2.40.

Without this patch, you cannot put anything in your cart after enabling the module, this is quite major when using Commerce.

Thank you very much!

g089h515r806’s picture

g089h515r806 commented

If some site already using "Currency" of Field Validation without commerce, update is a big problem for them if we rename the plugin id.
They will get a fatal error when update Field Validation.
Similar issue：
https://www.drupal.org/project/field_validation/issues/3357511

IF there is no site using "Currency" constraint of Field Validation , then we could rename it directly.

leducdubleuet’s picture

leducdubleuet commented

The related issue #3407755: Compatibility issue with Address module had the same fix as this one but did not pose any problem then.

You said in comment #8 of the related issue :

> g089h515r806 commented 3 January 2024 at 01:27
> This change does not need a update path. But people need clear cache if they using this plugin.

Why do this one is problematic now?

g089h515r806’s picture

g089h515r806 commented

it is 11 months ago,
this issue is 5 month ago. If some site using the plugin, they will get fatal error when update.

leducdubleuet’s picture

leducdubleuet commented

What do you mean? I do not see your point?

Why only a clear cache was needed then and now it won't work?

This version is in beta, shouldn't people expect some potential breaking changes for exception like this one?

g089h515r806’s picture

g089h515r806 commented

See issue :

https://www.drupal.org/project/field_validation/issues/3357511
https://www.drupal.org/project/field_validation/issues/3281484

If some site already using "Currency" constraint of Field validation, not use commerce. They will get fatal error, if the plugin name renamed.

g089h515r806’s picture

g089h515r806 commented

They need copy the old Currency constraint to the new version of field validation, then re-config it using SymfonyCurrency plugin. (It is needed in Field validation 8.x-1.1)

Can you confirm that just clear the cache, it will works again for them? (Maybe it is right in 3.x version of field validation )

g089h515r806’s picture

g089h515r806 commented

An error occurred while merging.

The MR not works with latest code.

g089h515r806’s picture

g089h515r806 commented
Status: Reviewed & tested by the community » Needs work

I read the source code of 3.x ,It seems that this is right:
"
This change does not need a update path. But people need clear cache if they using this plugin.
"
I try to merge it. But the MR can not be merged.

rclemings’s picture

rclemings commented
Priority: Normal » Major
Status: Needs work » Needs review

Just bumping this because it's been almost three months since the last activity and v3.0.0 is needed for Drupal 11 compatibility. Happy to try to assist with testing or other needs, but the simple (one line in each of two files) patch from the merge request works fine for me with 3.0.0-beta4 .

Also changing priority to major because, as noted in comment #5:

Without this patch, you cannot put anything in your cart after enabling the module, this is quite major when using Commerce.

Also changing status to "needs review" because I can't see what work is left except the issue in comment #11, which if valid, might be handled with a change record.

anaconda777’s picture

anaconda777 commented

This patch fixed the problem for me:

https://git.drupalcode.org/project/field_validation/-/merge_requests/9.diff

So now commerce works after small testing, able to add variations to cart and the error message and WSOD is gone.

Drupal: 10.4.8
Field_validation: 3.0.0-beta4
Commerce: Version: 3.1.0

Thanks!

muaz7731’s picture

muaz7731
Location Malaysia
commented
Status: Needs review » Reviewed & tested by the community

+1 for RTBC
using #9 patch:
https://git.drupalcode.org/project/field_validation/-/merge_requests/9.p...

after patch, clear cache.

leducdubleuet’s picture

leducdubleuet commented

There was a new release yesterday which did not include this fix.

Would it be possible to have a new release including this fix please?

Thank you!

g089h515r806’s picture

g089h515r806 commented

I could not merge this change, it report an error.
"An error occurred while merging. Try again."

g089h515r806’s picture

g089h515r806 commented
Status: Reviewed & tested by the community » Needs work
zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
Status: Needs work » Reviewed & tested by the community

I've rebased the branch for you (using Gitlab UI). Back to RTBC

abhisekmazumdar’s picture

abhisekmazumdar
he/him
Primary language English
Location India
commented
Status: Reviewed & tested by the community » Fixed

I see this was merged. Marking this as Fixed.

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

abhisekmazumdar’s picture

abhisekmazumdar
he/him
Primary language English
Location India
commented

I see that no release was made which covers these changes(I know one can use the dev branch but I feel this is a significant bug fix that should have a release). I created a new release tag 3.0.0-beta6

Status: Fixed » Closed (fixed)

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