🇦🇹Austria @agoradesign

@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!

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

Same problem as #3 - yes, the url is generated in that form, using commerce_payone

I really hate the whole topic of configuring GTM, especially the consent mode stuff. It should be easy, it looks easy, sometimes it just works, sometimes it's a pita. I never have a good feeling on configuring that. I'm never 100% sure, whether it works like expected.

I (and for sure many many many other people here) would love to see an in-depth guide on how to correctly configure the whole stack - from the Drupal side with google_tag, eu_cookie_compliance and eu_cookie_compliance_gtm, to the GTM side, including GA4, Ads and let's say Facebook Pixel - including steps that other tutorials are often missing, like which triggers to use, etc

eg I stumbled yesterday and today to refactor an older, but small GTM account. The debugger showed me updates of consent states, but in the end, it didn't really matter, whether the storages were denied, because all the tags just kept on firing. I have finally solved it by loading the community template from https://www.simoahava.com/custom-templates/consent-mode/ and also had to build me custom variables, which I then based on the first-party cookie of eu_cookie_compliance, which shouldn't be necessary at all...

So, if anyone has a easy to use recipe for dummies and writes a blog post about this, I'm sure it'll get a lot of views :)

agoradesign → created an issue.

fyi, opened discussion in Slack regarding this: https://drupal.slack.com/archives/C1TLCCF9B/p1709025789941939

Thanks for pointing this out! I wasn't aware of this. imho this is really a huge problem! And I'll raise the priority of this issue, due to the fact that handling payment info is a very sensitive task, and the current implementation might lead to a bunch of problems, and I'll explain why:

First, as described in the issue summary, it's really quite common, that the notification callbacks are called before the customer returns to the site. So, we'll miss a lot of succesful payments due to this. And also as described, you can never rely on desperate workaround like waiting for a second. Because this may help you in let's say 70% of the cases, but sometimes you might need 2 or 3 seconds or even more. so that's never the answer (I have tried similiar workarounds for a different problems with payment notifications on another project)

Second, the main purpose of such webhooks is that a store can receive a reliable feedback on payments. It does happen quite regularly that a user never returns from an offsite payment gateway. Sometimes the user may have closed the browser window too early (some do that deliberately, some by accident). Sometimes the internet connection may hang. Sometimes the Drupal website may have a short down, just in the moment, where the user comes back, or any other problem might occur (eg a database serialization error, etc)

So, it's nice to receive webhooks for status updates, but it is even more important to receive those webhooks to guarantee that we never miss a successful payment, just because the user haven't returned properly to the website!

So, for a payment gateway, that reliably sends webhook notifications, it's far more important to process the payment information in the onNotify() hook than in the onReturn() hook - in other words, it'll better in doubt to skip processing there (only do a short verification of the passed data there) and leave the processing completely to onNotify() than relying on a payment entity created in onReturn()... I can't find the text passage right now, but Stripe even recommends not processing the payment informatin on return, instead rely on the notification callbacks only.

Given my experience with other payment gateways, we might introduce another problem, as soon as we process payment both in onReturn() and onNotify(), because the overlapping requests can cause double-placements and related problems. It's hard to find the perfect solution for this problem then. I've tried different approaches, which all work quite good for us. The latest one was to process payment info in onReturn() directly, and in onNotify() create an advancedqueue job item, which will be processed on cron then. This way, you get rid of 99% of conurrent request problems, can send an immediate response to the payment gateway, which is also important. And in most of the cases, there's no need to do any processing anymore, as the order is already placed in Drupal, when cron runs. But for the small amount of use cases, where the user never returned, the order is placed on cron then

Coming from 📌 Add PayPal support to Payment Element Gateway Active , the outdated PHP library is at least a small problem for me.

I started implementing the PayPal payment method plugin - thought it might be a good idea to store the PayPal e-mail address in a field, so that at least we can use it for building the payment method's label. In the currently used old PHP API (7.x), \Stripe\PaymentMethod class does not contain a StripeObject representing PayPal data, but the current version does have one (was added about 9 months ago)

I'm gonna try to pick the e-mail address from the billing details instead for now, but I may encounter the next problems, when I try to implement another plugin... so I'm totatlly in favour of upgrading the PHP library

PS: also, what about PHP compatibility? Is it fully PHP 8.2 compatible? If yes, that's random good luck, but we may get problems in future versions... so let's start upgrading :D

You'll need this ✨ Add the option to not setup payment methods for future usage Fixed

I know. and as I have written, the mentioned commit seems to have the drawback that currently only credit cards work, others need their own plugins now

I haven't tried them yet, but I'm wondering.. which version do you use? 1.1? because in the meantime (dev, 1.2-rc1) this commit may have broken these payment methods: https://git.drupalcode.org/project/commerce_stripe/-/commit/bf118321d3fb...

agoradesign → created an issue.

agoradesign → created an issue.

Side notice on this issue: I'm coming from 📌 In StripePaymentElement->onReturn do not allow PaymentGatewayException to bubble up if we know the payment was successful on Stripe's end Active because I've realized that any other payment methods than credit cards (and the American ACH banking) are currently not supported by Payment Element, like in my case PayPal was refused on the Drupal side (the Stripe transaction was successful)

I found this issue description here a little bit weird at first, as it clearly states that PayPal payments already have worked at the time of writing this issue. This important refactoring here destroyed PayPal and other payments until a dedicated plugin is created: https://git.drupalcode.org/project/commerce_stripe/-/commit/bf118321d3fb...

I'm already on latest dev/rc. You're right that a necessary refactoring work has already been committed, which paves the way to implement other payment method types, but nevertheless they need to be implemented like 📌 Add ACH support to Payment Element Gateway Fixed recently.

Ok, you're right. I'll open up issues per payment method type

thanks for your input!

I think, the best would be to implement the missing plugins. That shouldn't be that tough imho. I'm of course willing to help.

I just want to coordinate to avoid working in the wrong direction. So basically, here are the plugins we'll want to use:

• PayPal
• EPS (Austrian bank redirect)
• Sofort (bank redirect in AT, DE, CH)
• Google Pay
• Link

The last two are optional for us. The most important will be PayPal.

We should coordinate, which extra fields we want/need, any special treatments (like status processing) that are needed, and so on. I think that there could be either a generic plugin for bank redirects or at least an abstract base class to hold common logic together (rather this I think). Any important thoughts, links to important parts of the Stripe API docs, etc would be very helpful.

As next step, I'd open issues for any plugin that I can help to create. But let's discuss the general approach either here or in Slack first

I'm coming from 💬 Support more payment method types Closed: duplicate . Our client reported a problem in his webshop. A customer paid successfully with PayPal, but got an error message on Drupal side, because PayPal is currently not supported by this module and a PaymentGatewayException is raised with this message "The selected stripe payment method type(stripe_paypal) is not currently supported"

We were not aware that not every activated payment method in Stripe is allowed on Drupal side - and now I'm in a hurry to get a quick workaround. We don't care about re-usable payments here, so having a payment method entity is not important for us. So I'm just thinking about patching the onReturn() function to early exit instead of throwing that exception. --> would that probably lead to other problems inside commerce_stripe module? Because if not, I'll do that as a quick fix. Any feedback is very welcome :) (also on the commerce slack channel). I'm of course open to help implementing outstanding payment providers

agoradesign → created an issue.

@nchin - I've committed the patch from #2

So you can try the latest dev release without patching.

Although the change is tiny and looks 100% logical and correct, I haven't actually tested the change. So I think, I'll wait with a tagged release

Why did you use a leftJoin() @Rhezios? Leaving join() makes more sense imho