🇫🇷France @solene_ggd

Here is a patch fixing the problem :)

I think I have the same issue than @dinh-bao-tran and it happens when a checkbox triggers an ajax callback. When unchecking a checkbox, the input value is NULL. The current MR only checks if it an ajax request and if the input is NULL to use the default value. But if the triggered checkbox triggers an ajax callback, then when we uncheck the checkbox, the default value will be used instead of the NULL value that we want. In cases where the default value is TRUE, then we cannot uncheck the checkbox anymore.

Got the issue within Drupal Commerce during adding promotions with conditions (checkboxes that trigger ajax callback).

I suggest adding a condition on triggering element name, to check if it not the same as the current element.

Everything works fine, thank you @NicolasGraph :)

solene_ggd → made their first commit to this issue’s fork.

solene_ggd → created an issue.

Well it seems that render the link here was overkill anyway because dblog module can get a link as a string. I suggest to use toString() method instead of injecting the renderer here.

@NicolasGraph, it looks like you forgot to check access for all queries :)

solene_ggd → made their first commit to this issue’s fork.

The #3 patch works fine, thanks :)

Hello @vaidas_a,

If you want to see your display label instead of just "Paypal" when you go back, you need to install this patch in addition (on drupal/commerce_paypal module) : https://www.drupal.org/project/commerce_paypal/issues/3420132#comment-15... 🐛 Use payment gateway label instead of Paypal by default Active

Here is a patch !

solene_ggd → made their first commit to this issue’s fork.

My bad, I did not think my patch could affect on site payment methods. Here is a new patch that should pass tests.

I have to apologize @jsacksick because this isn't a real issue for now. I thought it was the display label that was used in payment option but it is actually the create_label (because it is a payment gateway with payment method) which is "Paypal".

I had the problem because I had installed this patch : https://www.drupal.org/project/commerce/issues/3250069#comment-14649769 📌 List offsite payment gateways that support stored payment methods directly, not grouped by payment type Needs review

However, the fix might still be useful if this parent issue is ever resolved.

I wanted to use this patch to get multiple Paypal options (offsite payments) with the right display label. But when I set one of my Paypal options as default payment method with the #13 patch installed, I get this error :

Drupal\Core\Entity\EntityStorageException : Missing bundle for entity type commerce_payment_method dans Drupal\Core\Entity\ContentEntityStorageBase->doCreate() (ligne 125 de /app/web/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).

This is because the payment method type id is not set in the option with the patch, but it is required by buildPaymentMethodForm() method called to construct the default payment option in PaymentInformation class.
I think this is what @DamienMo wanted to patch in his comment but I don't think he did it the right way.
I think the condition on offsite gateway should not be on the first condition to build the option but on the one inside that checks if there are multiple methods. This solution allows to keep the payment method type id in the option.

And to get the payment gateway display label as option label, I change the default call of createLabel.

Well this is exactly what I needed ! Thank you for your quick response and your patch (I was about to upload mine but yours is better !)

Well I just understood that the label is made with the buildLabel method in the payment method, which allows to display some info like the card type and the ending of the card number. It all makes sense now. So my issue is in the commerce paypal module itself.

I created another issue in commerce paypal here : 🐛 Use payment gateway label instead of Paypal by default Active

I close this issue because it works as designed.

solene_ggd → created an issue.

This issue is related because it talks about same label problem on Paypal : https://www.drupal.org/project/commerce_paypal/issues/3153866 →

The last comment leaved by @starleague here : https://www.drupal.org/project/commerce_paypal/issues/3153866#comment-14... → seems to mention the same solution.

solene_ggd → created an issue.

solene_ggd → created an issue.

Here is a new patch that should fix tests.

Here is a patch :)

solene_ggd → created an issue.

I did not realise that the new patch could failed some tests, I set the issue in Needs work while I fix this.

I think dispatch another event from the promotion module is a better option to avoid requiring commerce_cart.
Here is a new patch that implements this solution.

Thank's for your answer @jsacksick, I understand the problem in my patch.
But something is bothering me, as you said :

The "CART_EMPTY" event to me indicates a deliberate action from the customer emptying its cart

This is exactly the issue here. When a customer removes the X item from his cart (the deliberate action), the Y item is removed too so the cart is empty but the CART_EMPTY event was not triggered at all. In this case, however, the empty cart comes from a deliberate action from the user. In my mind, it is a breaking issue in commerce_cart.

If the problem won't be fixed, maybe we should write it somewhere in a documentation ?

I will keep my patch for now but if anyone got a better idea, I'll be happy to hear it !

Hello, I had the same issue on an unformatted view, I don't think it is specific to the table format. Some entities showed up twice on page 1 and page 2 sort criteria are the same. I fixed it by adding sort on entity ID.

Thanks for your patch.
I still see an entity query created in commerce_dhl_express_cron() that needs an accessCheck() to be D10 compatible.
Moreover, there are some calls of the deprecated hook hook_field_widget_WIDGET_TYPE_form_alter that was removed in Drupal 10. See https://www.drupal.org/node/3180429 → .
Please fix your patch.

Thanks for your patch, please use double pipes to add D10 compatibility in core_version_requirement instead of single pipe.
Moreover, I see an entity query created in commerce_chronopost_cron() that needs an accessCheck() to be D10 compatible.
Please fix your patch.

As I didn't get any answer, I made a new patch removing dependency to commerce_cart and calling only emptyCart() method when it's needed. I did not edit any test then.
I put the issue in "Needs review" again but if @jsacksick you think it is still not a right approach, you can close it.
I still post the patch for those who might need it.

When you say "this event", do you talk about the CART_EMPTY event or the CART_ORDER_ITEM_REMOVE event also dispatch in removeOrderItem() ? If you talk about CART_ORDER_ITEM_REMOVE event, then I agree it should not be triggered for an order item removed automatically. And in this case, we could call only emptyCart() method instead of all the removeOrderItem() method. But if you talk about the CART_EMPTY event, I don't understand the problem because the cart is litteraly emptied by the user removing the "X" item.

Moreover, I've just seen that CART_EMPTY is used in commerce_promotion to remove the coupons when the cart is empty as mentionned here : https://www.drupal.org/project/commerce/issues/3065951#comment-13178606 →
I have tested it and indeed it doesn't work when the last item removed comes form a Buy X get Y promotion.

Thank you for your reply :)

I still think we can do something because it really breaks some commerce_cart features.
Isn't it possible to check if the 'commerce_cart' module exists (with the module handler) in the PromotionOrderProcessor to use removeOrderItem() ? And if we don't use dependency injection to inject the cart manager, it won't break any tests.
If you say it's a good idea, I'll make another patch.

Here is a patch that uses the cart manager in the PromotionOrderProcessor to remove the order item.
I had to add the commerce_cart module dependency in some tests to not them to fail.

solene_ggd → created an issue.

Here is a patch including changes in tests. It is a big patch, all tests are affected because the entity clone form path has to be found dynamically now. We can't do anything else, I think, because the 'clone-form' link template is now depending on path defined in other modules.

Ok, I found that using 'canonical' or 'edit-form' link template for the 'clone-form' link template, instead of always /entity_clone/$entity_type_id/{{$entity_type_id}}", fixes the issue. I use the same logic than in the DynamicLocalTasks by using the 'canonical' link template if it exists, and the 'edit-form' link template otherwives.
It is a major change in the module because it changes all entity clone form paths, but I didn't find another solution.
Here is a patch including the fix in the .module file, but tests are failing. Another patch is coming soon for that.

The patch from #17 ✨ Add a "Checkout registration" form mode for creating user accounts at checkout Closed: duplicate does not apply since 2.33 release because another related issue was commited : ✨ Allow to use different form mode than default "Register" in Login checkout pane Fixed
I close the issue and set as duplicated.

Thank you for the work before the release :)

solene_ggd → created an issue.

Here is a patch adding the missing line :)

solene_ggd → created an issue.

Here is a new patch that adds the accessibility validation on the Balance Form formatter.

Here is a patch, needs review.

solene_ggd → created an issue.

Another patch should include a formatter for the pickup_id field of the shipment entity that displays all the details about the pickup point thanks to the new method. I set this issue on "Needs work" status.

Here is the patch.

solene_ggd → created an issue.

I got another warning related to the first issue so I reopen it.

Warning: Undefined variable $selected_option in Drupal\commerce_profile_inline_form_radios\Plugin\Commerce\InlineForm\CustomerProfile->buildInlineForm() (line 119 of modules/contrib/commerce_profile_inline_form_radios/src/Plugin/Commerce/InlineForm/CustomerProfile.php).

I made a patch :)

Thank you ! :)

I did a new patch including a custom ViewBuilder as mentioned in #2 because I do not need canonical links for my storages.

Here is the patch. Note that your storage type needs to allow direct viewing of entities so that the patch works because the default EntityViewBuilder used by Storage needs a direct view of the rendered entity to add contextual links on it.

solene_ggd → created an issue.

Here is the patch !

@Riadh Rahm I cannot apply the #6 patch, the #4 patch still works for me. What did you change ?

I fixed error in tests and added new assertions.

Here is a patch that fixes the interface fatal error and prevents duplicated queue items from being created.

The error came from the shipping method that was changed to DHL (in back-office) after saving another tracking code in the shipment. The module tried to track a tracking code that was not a DHL one, and that was too long.

Here is the patch.

solene_ggd → created an issue.

Thank you !

Thank you !

Changed the patch fixing error when a field doesn't exist in the address field. The error was :

TypeError: _commerce_dhl_express_field_maxlength_alter(): Argument #1 ($field) must be of type array, null given, called in /home/maetvade/web/art/web/modules/contrib/commerce_dhl_express/commerce_dhl_express.module on line 123

Changed the patch fixing warning when the field doesn't have a maxlength :

Warning : Undefined array key "#maxlength" in _commerce_dhl_express_field_maxlength_alter()

I opened a merge request with all the elements.

solene_ggd → created an issue.

Here is the patch.