🇦🇹Austria @agoradesign

Dear maintainer, could you please commit this tiny change and do a new D11 compatible release? It would be really great, as this is only one of 2 blockers of our current project to upgrade it to D11 :)

Would be great to have this tiny change committed and get a new, D11 compatible release :)

I've just started to write a bugfix for this solution, when I saw that there isn't a bug at all, it's just a lack of documentation.

The module ships with a permission log in as another user. When I installed the module, I've interpreted the permission name in a wrong way. So I gave permission to higher (moderator/admin/...) roles only, but in the end it's very important to have this permission on the auto-login user's role. This way you won't get re-logged in immediately, instead may choose to go to the login form instead and login as any user you like or even stay logged out for a while.

Either way, I'll leave this open and tag it with "needs documentation", as this is a very important feature that obviously not everyone finds out immediately :D

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

already did this + other CS fixes, so this MR has become obsolete

merged and released 2.1.0 :-)

Thanks!

no problem :)

Is it incompatible yet?

We are already restricting in composer.json to "drupal/commerce": "^2.29 || 3"

There are no Commerce specific plugins implemented, where changes in the constructor of the base class could cause problems. I don't think, there should be any problems regarding that.

That said, I haven't tried this module yet in 3.x

If you ask rather for D11 compatibility - there's a merge request created by my new co-maintainer @vladimiraus - however it seems, he had to stop working on that (I however guess that there isn't really more work to do on that issue).
I haven't used Commerce yet on D11, because in our setups there are various other contrib modules that weren't D11 compatible yet (at least they weren't the last time I checked)

confirm #136 - patch #135 works until 10.4.1, stopped working with 10.4.2

couldn't apply the MR diff either, so I'm stuck at 10.4.1 for the given project :(

agoradesign → created an issue.

added you as co-maintainer - let's start with "Write to VCS" and "Maintain issues" permissions

I've created a MR with the changes that worked for us in our customer project

agoradesign → created an issue.

nope,doesn't seem so.. here are the currently implemented payment method types: https://git.drupalcode.org/project/commerce_stripe/-/tree/8.x-1.x/src/Pl...

yes, there's 📌 Add bank redirect support (EPS, Sofort) to Payment Element Gateway Active and 📌 Add PayPal support to Payment Element Gateway Active

@jsacksick I haven't really recognized your comment on friday. just saw it :D

hmm well, yes it is specific, but I believe we should be open to custom adjustment types, allowing them to have a solid way to integrate themselves. why should it harm to use events?

We have actually 2 different places inside prepareOrderRequest(), where we deal with an hardcoded opinionated list of adjustment types (skip, discount). my latest commit considers both (I have added in total 2 events, re-using the same event class)

ok, I see, we also need to fix the $breakdown.. but anyway, this can be done inside CheckoutOrderRequestEvent as well, so having this extra event would still be valuable imho

agoradesign → created an issue.

Exactly... I believe, finding a solution for the different browser scenario would automatically fix the first issue, regardless of SameSite cookie settings

@berdir thanks for your input!

That's basically the same code, we also use always. but idk, I have seen some caching issues anyway, after I thought, that the problems were gone. Anyway, I just tried on two of our projects, and I could not reproduce it though. afaik when it happened, it was always the problem, when an anonymous user without a cart/session added the first item to cart

So I honestly don't know, if our current solutions works 100%. At least it does not happen too often, so that I do not get complaints by our clients :D

I'll test this now in a sandbox, report back then

Added 📌 Controllers handling order data should use loadForUpdate() Active and ✨ Klarna Payments Authorization Callback Active as related issues, as both should improve/solve this problem

agoradesign → created an issue.

Happened again... quite every day now.. one thing, I missed out to add: we never have a second payment entity, but checkout complete event is triggered twice, which triggers the order place transition twice, and any event subscriber will trigger twice. So the customer will receive 2 e-mails with two different order numbers, etc

The back button problem seems to happen quite often, at least at the website, we have added Klarna a few weeks ago

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

regarding the wishlist items cache context:

I maybe the wrong person to answer this, as I keep struggling for years to find a way to find the right cache metadata for cart links in the page header, that show the number of items in the cart. Everytime I thought, I solved that, it happened again, that the counter kept staying at 0.

This is little bit different of course. Because every single link depends on the actual items in the wishlist. We may have to think about caching here in general

works for me in test environment. Tried to catch different scenarios (with new created session, with updated session, buy, cancel, select different methods, etc)

Project page is ok for me.. maybe your browser cache has fooled you?

Well, that depends of course, what is standing in your composer.json - if you have defined a fixed version constraint to a specific release, than of course, you'll have to change it. If you're using something like ~1.0 or ^1.0, you should be fine.

there's one thing I've seen and changed now. the 8.x-1.x branch was not marked as "recommended" I did that now - now the "releases" box on the bottom of the project page is green (not yellow anymore). Maybe this also changes the composer behaviour?!

isn't that the same issue as it was fixed in 🐛 contents of dropdown button go to the left and can be off the visible page RTBC ?

Maybe he was using a patch at that time, but now this parameter is part of the 6.1.5 release, see https://git.drupalcode.org/project/linkit/-/commit/9412642029f3e5e4d840e...

However, this patch here is outdated

While it's not ideal to have a jungle of different patches and merge requests that cover multiple issues at once, I do not want to hide my solution and share my patch with you

I'll copy here my comment in the Slack thread:

I have seen that there is 📌 Add Capture, Void and Refund / Partial Refund capabilities RTBC adding refund capability. The MR5 of that issue is incorporating further changes into one MR. It handles the API version issue as well ( ✨ Make it compatible with latest Saferpay API Active ), and also tries to solve the concurrency issue with the sleep() command ( 🐛 Saferpay webhook faster than customer Active )
Additionally, I needed to allow not collecting billing information from ✨ Allow not collecting billing information Active
As I'm not a big fan of the sleep() solution, I've worked toward the solution I have done in commerce_opp, to:
do process the payment in onReturn()
add a dependency on advancedqueue and implement a JobType for processing the payment
only do some basic checks inside onNotify(), but instead processing it, create a queue item and let it handle by cron
additionally, to avoid clashing with a cron run in the same second, I've delayed the processing of the queue item to 60 seconds (which we could make configurable)
I've done different test payments now in staging. If the customer returns, the payment is processed successfully. On cron, the job can early exit, as the order is already paid. It's also easy to close the browser tab after finishing the payment but before the customer has returned to the page. So the order stays as cart, until the queue job has been processed, where the payment is processed and the order placed
So the big question is, if we wanna add a dependency on advancedqueue?! Alternatively, we could do a moduleExists() check and either use the queue or do the sleep (and optionally locking) stuff

Convinced :)

Well, this is one of those modules, that you needed in 1 or 2 customer projects, but isn't used heavily there. You implement it as open-source module, but are just unsure, if it's really that stable that you think it is. Also, you may have some ideas on how to extend it. Provide a block with a nice template for example.

On one project, I've added a block plugin in a custom module for example. This had some project specific touch, which didn't fit 1:1 into the general contrib module, but you hope to find time to find a generic solution for the contrib module...

After some years without complaints, you're right that is time to release 1.0.0

I won't do D11 compatibility check right now. This could be part of a new 2.x branch that uses the semantic versioning syntax (1.x is using the old 8.x-1.x syntax), giving us some theoretical time to add other improvements to that version

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

ok, we actually should have a look at commerce_paypal eg. -> https://git.drupalcode.org/project/commerce_paypal/-/blob/8.x-1.x/src/Ch...

I have to move on now, hopefully finding time on another day

This patch fixes the included promotion rules, but the other ones still fail.. I don't get it :DDD

but maybe it can help in some way...

ATTENTION:

Being under time pressure, I've uploaded my patch, that is hardcoding to restrict the allowed categories to 'pay_later'. Of course, we'd need to define a configuration option for the payment gateway, allowing to optionally restrict the categories, and then doing the filtering.

That's why I'm immediately hiding this patch

However, this is a starting point for anyone coming with the same feature request here, and for finishing the final implementation of this

the last alpha is pointing to the latest commit, so there's currently no difference between them... so I recommend the tagged release

agoradesign → created an issue.

agoradesign → created an issue.

I believe, it could be solved by skipping inside RequestBuilder line 162 not only positive adjustments, but also checking for if $adjustment->isIncluded() -> if it's already included, then we'll need to skip it

I hopefully can find a few minutes to test this theory

Why do you think so?

It's kinda minimally maintained. We still use it on 2 projects currently, where one of them is a quite important one for us. eg if the first one of these sites is upgraded to D11, we'll take care of the necessary changes.

In general, help is always welcome. So, if you want to participate in any way, let me know

agree. this leads me thoughts into the direction that it would be nice to have the possiblity to add some kind of messages to a cart. Like "item XY is not available anymore. Please remove it from your cart..." or maybe then we even could remove it, but show the message to the customer... or like Amazon notify of price changes. afaik, there's already an issue for this

It looks quite random, that the tests fail here and not in general:

Drupal\Tests\commerce_product\Functional\Jsonapi\ProductVariationResourceTest::testIndividual
The 'errors' member was not as expected.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     0 => Array (
-        'detail' => 'title: Title: this field cannot hold more than 1 values.'
+        'detail' => 'sku: The SKU "ABC123" is already in use and must be unique.'
         'source' => Array (
-            'pointer' => '/data/attributes/title'
+            'pointer' => '/data/attributes/sku'
         )
         'status' => '422'
         'title' => 'Unprocessable Content'
     )
+    1 => [...]
 )

That's part of the core's jsonapi base class test testPostIndividual(). Entities with a label key are posted twice, in order to trigger the "field cannot hold more than 1 valoues" error for the title field. As the SKU field has a unique constraint, we also trigger this error. I wonder, why this works for the regular branch, but not for this MR

agoradesign → created an issue.

TomTech → credited agoradesign → .

I also have no idea, why this may happen :( As mentioned, I've already seen this on another site long time ago, when the underlying issue wasn't even committed to Commerce, but we were using that issue's patch.

The fact, that I can only reproduce it on this specific production environment, makes it quite tricky to debug. I haven't spotted any other suspicious code that could mess things up here.

I mainly opened this issue, in order to have this documented and if we're lucky, other people see this too, so that they either find help for themselves or even better someone knows the reason, why the other syntax makes a difference here

For our specific project, there's no budget to invest a lot of time in investigating this problem, as our fix/workaround is patching a single line back to its original syntax

agoradesign → created an issue.

looks good, although I haven't tested it honestly. However, this won't break anything, so I'll merge it

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

I've added this in the past because of problems with advagg, depending on the configuration (I believe, it appeared when using the JS minification sub-module and maybe depending on how that is configured).

But as this is anyway a workaround on behalf of another module (advagg), which became unusable with Drupal 10.1+, I think it's a good idea to merge your MR. Hopefully, advagg will solve that, if there we'll see a working version again sometime :)

There may be more places in code, so I'll leave the issue open. The last commit also fixes some important typos in the exception messages, so I'll add a release immediately

agoradesign → created an issue.

happened to me in 10.3.0 too, #14 didn't help though.

I had a look at the stack trace, it is related to comment_type's route access checking. admin_toolbar is calling the menu link tree's transform():

    #0 /app/web/core/lib/Drupal/Core/Config/ConfigManager.php(489): Drupal\Core\Entity\EntityDisplayBase->onDependencyRemoval(Array)
#1 /app/web/core/lib/Drupal/Core/Config/ConfigManager.php(352): Drupal\Core\Config\ConfigManager->callOnDependencyRemoval(Object(Drupal\Core\Entity\Entity\EntityViewDisplay), Array, 'config', Array)
#2 /app/web/core/modules/user/src/Form/EntityPermissionsForm.php(96): Drupal\Core\Config\ConfigManager->getConfigEntitiesToChangeOnDependencyRemoval('config', Array)
#3 /app/web/core/modules/user/src/Form/EntityPermissionsForm.php(185): Drupal\user\Form\EntityPermissionsForm->permissionsByProvider()
#4 [internal function]: Drupal\user\Form\EntityPermissionsForm->access(Object(Symfony\Component\Routing\Route), Object(Drupal\Core\Routing\RouteMatch), NULL)
#5 /app/web/core/lib/Drupal/Core/Access/CustomAccessCheck.php(84): call_user_func_array(Array, Array)
#6 [internal function]: Drupal\Core\Access\CustomAccessCheck->access(Object(Symfony\Component\Routing\Route), Object(Drupal\Core\Routing\RouteMatch), Object(Drupal\Core\Session\AccountProxy), NULL)
#7 /app/web/core/lib/Drupal/Core/Access/AccessManager.php(160): call_user_func_array(Array, Array)
#8 /app/web/core/lib/Drupal/Core/Access/AccessManager.php(136): Drupal\Core\Access\AccessManager->performCheck('access_check.cu...', Object(Drupal\Component\Utility\ArgumentsResolver))
#9 /app/web/core/lib/Drupal/Core/Access/AccessManager.php(93): Drupal\Core\Access\AccessManager->check(Object(Drupal\Core\Routing\RouteMatch), Object(Drupal\Core\Session\AccountProxy), NULL, true)
#10 /app/web/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php(218): Drupal\Core\Access\AccessManager->checkNamedRoute('entity.comment_...', Array, Object(Drupal\Core\Session\AccountProxy), true)
#11 /app/web/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php(107): Drupal\Core\Menu\DefaultMenuLinkTreeManipulators->menuLinkCheckAccess(Object(Drupal\Core\Menu\MenuLinkDefault))
#12 /app/web/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php(111): Drupal\Core\Menu\DefaultMenuLinkTreeManipulators->checkAccess(Array)
#13 /app/web/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php(111): Drupal\Core\Menu\DefaultMenuLinkTreeManipulators->checkAccess(Array)
#14 /app/web/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php(111): Drupal\Core\Menu\DefaultMenuLinkTreeManipulators->checkAccess(Array)
#15 [internal function]: Drupal\Core\Menu\DefaultMenuLinkTreeManipulators->checkAccess(Array)
#16 /app/web/core/lib/Drupal/Core/Menu/MenuLinkTree.php(153): call_user_func(Array, Array)
#17 /app/web/modules/contrib/admin_toolbar/src/Render/Element/AdminToolbar.php(47): Drupal\Core\Menu\MenuLinkTree->transform(Array, Array)
#18 [internal function]: Drupal\admin_toolbar\Render\Element\AdminToolbar::preRenderTray(Array)

🐛 Admin Toolbar Tools breaks Dynamic Page Cache on Drupal 10.3 when update module is enabled Active is definitely related imho. A comment there is pointing to 🐛 [regression] Do not bypass route access with 'link to any page' permissions for menu links Fixed as the root - looking at the changes made there, compared with the calls in the stack trace, this seems very realistic to me

I have the problem described here 🐛 Fix access checks for bundle permissions to avoid triggering a config validation error Fixed since updating to 10.3.0, I saw these lines in the stacktrace:

#17 /app/web/modules/contrib/admin_toolbar/src/Render/Element/AdminToolbar.php(47): Drupal\Core\Menu\MenuLinkTree->transform(Array, Array)
#18 [internal function]: Drupal\admin_toolbar\Render\Element\AdminToolbar::preRenderTray(Array)

I'm sure that these issues are related. Has anyone checked mrstrelan's comment #6?

oh that would be great :)

Note, that a manual re-saving of the payment gateway configurations is necessary

The commit contains some refactorings too, but is mainly about this issue

First of all, thank you very much for your MR, Jen :)

It looks 99% good to me, especially the custom event part. One thing I'm not happy with, is the hardcoded and very opinionated CSS selector used for the counter update. eg, this would apply to none of our Commerce sites at all. Would you mind making this selector configurable + empty check for this?

solved as part of #3454250-4: [META] 2.x roadmap →

This is actually a nice-to-have feature in 2.x, probably will be added later

2.x will contain a number of refactorings, causting the single issues not to be separated. A single commit containing all 2.x changes will land soon

ok, we'll actually fetch the JSON from https://docs.prtso.com/brands/json/index.php, ship it with the module and add some needed extra informations manually

agoradesign → created an issue.

After some additional years of implementing/using/debugging/patching Commerce payment gateways, I now see that we have used the payment workflow states in a uncommon way. We were using the authorization state for sole payment intents, where the payment and order process hasn't finished yet. It works very good internally, but upon implementing this task, this is causing a lot of problems for us. So we have first to correct the worklow (use "new" instead auf "authorization" for payment intents, then implement the two different workflows)

agoradesign → created an issue.

agoradesign → created an issue.

agoradesign → created an issue.

agoradesign → created an issue.

I've good and bad news regarding this issue - first the bad one:

The patch does not apply anymore in 10.2.6 due to the changes in 🐛 Image derivative generation does not work if effect "Convert" in use and file stored in private filesystem Fixed

The good news: the patch is not necessary anymore, as the changes in 🐛 Image derivative generation does not work if effect "Convert" in use and file stored in private filesystem Fixed solved this problem in a drive-by :)

Maybe at least another person should confirm that this works now, before we close this issue here

agoradesign → created an issue.

Why should this function return a string? It returned an array before, which makes sense. Now it's a string and the call from the .module file now fails, as you'd now need to decode it again before iterating

looks good, the line in 9.3.3 now is:

if ( isset($action_result['error']) ) {

Great news! :)

Thanks!