🇫🇷France @nicolasgraph

We need to consider if the queue worker should be kept after the batch operation introduction.
We also need tests for this feature.

nicolasgraph → created an issue.

nicolasgraph → created an issue.

nicolasgraph → created an issue.

I just ran into this issue on a D11 install ; requiring 2.1.x-dev fixed it.

nicolasgraph → created an issue.

nicolasgraph → changed the visibility of the branch 5.x to hidden.

I added a commit for comment #6 📌 [1.x] Guest Suite Needs review and another for comment #10 📌 [1.x] Guest Suite Needs review .
I intentionally did not "fix" the inheritance of classes EntityViewBuilder, EntityListBuilder and FieldItemList . I'd be happy to see any documentation about how to do in a better way, if there is one. In the meanwhile, I sticks to usual practices used by many experienced maintainers.

nicolasgraph → created an issue.

Thanks for your prompt reply @avpaderno!
So I guess ReviewListBuilder.php should not extend EntityListBuilder.
I'm a bit confused about the load of duplicate code needed to remove these parents classes.
Popular modules like Webform → , Commerce → or Paragraphs → do extends the mentioned classes.
And most computed fields examples I find extends a core field class.

Thanks for your replies!

src/ReviewViewBuilder.php
EntityViewBuilder is not a base class and it cannot be used as parent class.

Could you give me a bit more explanation about this?
BlockViewBuilder do extends this class for example.
Commerce LogViewBuilder also uses it as parent.

src/ReviewWarningUrl.php
Similarly, FieldItemList is not a base class and it is not part of the public API.

PathFieldItemList do extend FieldItemList, and I can't see any @internal annotation.
This documentation → also gives an example of a computed field extending this class.

Posting rerolled patch.

I get the mentioned exception with the last version in date. I do not see any of patch related change in the module source code ; $context is still in use.
I cannot reopen the issue ; should I create a new one?

Merging 5.x introduced errors ; waiting #3428466 📌 Automated Drupal 11 compatibility fixes for commerce_payplug Needs review to land before to continue.

nicolasgraph → created an issue.

nicolasgraph → changed the visibility of the branch project-update-bot-only to hidden.

nicolasgraph → changed the visibility of the branch 5.x to hidden.

The previous commit added a wrong dependency declaration on commerce_payment:commerce_payment (>=3.0) instead of commerce:commerce_payment (>=3.0). Also, I think we should keep the D10.3+ support to stick to commerce versions support and make updates smooth. I personnaly uses commerce 3 on a D10.4 install.

nicolasgraph → created an issue.

Thanks for your reply @dom.
Unfortunatly I was not able to reproduce the bug in test mode.
In test mode, I'm not redirected to the site on payment failure, while in live mode I am.
The 5.x branch do not fix the issue bescause onReturn() is still empty and that's the method called when Payplug redirect to the site in my case.
Also I didn't try the v3 but I suspect #3295358 💬 Payments which are not completed, still complete the order. Closed: cannot reproduce to be a duplicate.

I confirm the current MR state do fix my issue in live mode.
Still needs attention and ideally, some tests.

Ok, I get it, in my failing manual test case, onNotify() is not called, the payment is managed through onReturn() which do... nothing.
While the payment entity creation can be managed onNotify() only, the remote payment validation must be processed onReturn() too to throw the DeclineException in case of failure.

nicolasgraph → changed the visibility of the branch 4.x to hidden.

Manual tests failed. The order is still placed.

Tests are now running, but failing, on d10.

This would need tests.
Waiting #3515036 📌 Fix tests Active to be fixed.

nicolasgraph → created an issue.

nicolasgraph → created an issue.

There is a client exception on the settings page after the Key module install; some more work needs to be done to make the install/uninstall as smooth as possible.

nicolasgraph → created an issue.

Completing the issue description and fix strategy.

nicolasgraph → created an issue.

nicolasgraph → created an issue.

nicolasgraph → created an issue.

Extends OrderItemSkuKernelTestBase in test once #3503345 📌 Explore different ways/moments to set the SKU Active is merged.

I reopen this issue to add a setting allowing to use a custom storing strategy.

nicolasgraph → created an issue.

Let's add tests later on and continue with #3505136 ✨ Sync order item SKU on purchased entity update Active .

nicolasgraph → created an issue.

On purchasable entity deletion the SKU storage should be managed via a queue.

I made a mistake in attributes ; fixing that…

This issue try to fix the Commerce 3 compatibility only.
Some more work may be needed to support D11.

nicolasgraph → created an issue.

nicolasgraph → created an issue.

I just created a light contrib module for this feature here → . I would just like to ensure that using "sku" as the trait field is ok ? I wouldn't like to conflict with a future implementation of this field.

An approach to this issue could be to introduce an entity trait for order items to choose to store the SKU if needed + a dedicated order processor. It is what I use for now ; I can help with a patch or a light contrib module.

Commited, thanks!

The balance is not related to the placed order status, it is related to the amount actually defined as paid by Alma. If no payment have already been processed for an order, the balance is 0. However, a queue worker should update the payments balance/status during the cron. Please check your logs and remote payments status or provide more informations to help me reproduce the bug.

Committed, thanks!

Patch #5 won't apply, it is an interdiff.
I created a MR from patch #4 including the suggested change of the interdiff.

@grevil, not sure which of your MR is the one you're woking on but I think you should keep the change I made to ReportsListBuilder.php in the MR#18. See #3342975 📌 Deprecate Url::toRenderArray() and Url::renderAccess() Fixed + #3406432 📌 Remove calls of deprecated toRenderArray() method Fixed .

Thank you all!