@grimreaper,
Thanks for the report!
Took a minute to setup to circle on this one, as it has been a while since I've used a prefix. :)
Direct DB queries do support prefix substitution.
It wasn't possible to write this query using the abstraction layer, because of the subqueries.
As observed in your MR, the table names were properly wrapped with curly braces to handle prefix substitution, but for the subquery joins, the table.column needed to also be wrapped as {table}.column. (When using an alias, this isn't necessary, but for some SQL statements, aliases can't be used. For those, the full table name must be used.)
I've reproduced the issue, and the proposed MR resolves it.
Merging.
Thanks!
Hi @mhawwari,
The PaymentElement actually supports using multiple different Payment Methods.
There are currently 8-10 implemented.
Currently SEPA is not, but only requires implementing a PaymentMethodType to be supported.
Hi @brenm!
It sounds like either:
1. The Strip Review Pane is not present on the Review Checkout Step
2. If it is present, then best place to investigate is the browser console log. It is possible that some other javascript on the site is conflicting/breaking with the stripe js.
Stripe Card Element, as its name implies, only supports cards. (Additionally, it does not support ApplePay or GooglePay.)
As of this writing, this module's Stripe Payment Element currently supports the following Payment Methods:
Card (including ApplePay and GooglePay)
ACH (US Bank Accounts)
Affirm
Alipay
AmazonPay
CashApp
Klarna
WeChatPay
This list is growing!
The initial implementation only supported Card.
The others were added the past year.
You might find MRs/patches for some in the issue queue.
This was originally a D7 issue, which is no longer supported.
ApplePay is already supported in the current version using the Stripe Payment Element. (Stripe does NOT support ApplePay with Stripe Card Element.)
ApplePay will also be supported with the upcoming Express Checkout implementation. ✨ Stripe Express Checkout Element Integration Active
Hi @gillarf,
Thanks for the report!
The StripeReview.php class does not override/implement the create() method, so this is not caused by the class OOTB.
Given the message you posted, the most likely cause is a patch is being applied that adds a create() method, but does not add the use statement.
I would first confirm whether the patch you are applying is still desired. (Has the functionality since been added?)
If you still need the patch, you should check the issue where you obtained the patch, to see if there is an updated one.
If this is a custom patch, you'll need to fix the patch locally.
re-rolled and created as an MR. previous patches hidden.
Hi all,
The MR resolved most of the issues I was also encountering, but the attachBehaviors was firing multiple times. This caused init() to be invoked multiple times stacked. While clicking "Yes" would dismiss the dialog, the subsequent init() calls would still invoke logout shortly thereafter.
The code tries to prevent this by checking if the context is "document", but there can still be multiple attach calls with document as the context.
I've added a commit that uses drupal once, so that the behavior is only attached once, which seems to resolve this issue.
I also add the new classes property, so this works with D10.3.
Lastly, I created a new fork that targets the 2.x branch, given that is the latest version.
tomtech → changed the visibility of the branch 3339695-use-drupal.dialog-call to hidden.
tomtech → changed the visibility of the branch 3339695-use-drupal.dialog-call to active.
tomtech → changed the visibility of the branch 3339695-use-drupal.dialog-call to hidden.
I did want to note a few things:
1. For now, we'll still be marking the order as checkout complete and placed. If we threw an error, then the customer would be left in an indeterminate state, as the payment would be processed, but the order would still be in the checkout flow...which would allow the customer to add/delete items from their cart, causing more issues. More likely, this order should be treated similarly to a failed ACH transaction(if you accepted ACh payments) or a chargeback. Were we to actually throw an error, we would likely need a change in commerce core.
2. return page handling invokes onReturn() on the StripePaymentElement gateway. PaymentProcess should actually not even be encountered, but if it is, it should actually short-circuit because the order has been placed, moving it to the checkout complete pane.
Hi nicolas bouteille, thanks for creating this issue.
We've had this in progress for a minute, but now that we've completed this work(in concert with several other changes), I'm going to go ahead and just create the MR as part of this issue.
Thanks for the report!
This has been resolved as part of the issue ✨ Implement use of getTotalPaid() and getBalance() methods Needs review .
-Tom
Hi @tonytheferg !
Thanks for the report.
I was actually working on something similar for this and a few other properties.
A few items to note:
1. Looks like this fix is specific for the Card Element. We'll need a solution for both card element and payment element.
2. This solution does not take into account Stripe's new "automatic_async" value for capture_method.
3. It would be a good idea to have a fallback, in case there is a new capture_method in the future. I'm leaning towards falling back to manual, as that would allow a CSR/Admin to at least attempt to capture in that scenario. (And if it actually already was captured, then no harm, no foul...) The alternative would be to presume automatic, but if it isn't, then there is no easy way to capture.
4. While handling this particular scenario makes sense, there are other alters that can be done to the PaymentIntent that we won't be able to account for in the module.
@valic, a payment method also has a remote_id, and can have a link to the payment processor.
Since this method is on the gateway, I would recommend we name the method getExternalPaymentUrl(), so that we can also add getExternalPaymentMethodUrl().
There are many different parameters that impact Stripe's automatically provided payment methods.
For example, Off-Session is needed if you are doing recurring payments, e.g. a subscription. But, if you use Off-Session (or On-Session), you won't have access to migro-loans like Klarna and Affirm. You also won't have access to AliPay and WeChatPay.
ACH (us_bank_payment) will work with any of the session options, but won't be available if you use capture method of "manual"...but this means if you take ACH and Card, you can't do a delayed capture, since that requires the "manual" value. So...you either have ACH+Card and no delayed capture, or you can have card+delayed capture, but no ACH.
So many different options and they change regularly. e.g. Klarna might be supporting On-Session/Off-Session in the future, as they are doing a "Private Preview" of this.
@anybody,
Glad these comments were helpful for you!
We are working on additional documentation...it gets challenging, though, because stripe is churning a lot, and what is true today may not be true tomorrow.
Case in point...when I was making the comment #4 above, I pulled up the link I referenced, and noticed the new "Private Preview" for recurring payments with Klarna. Previously, that value was "No".
Be careful with an update statement like the one in comment #4, as that will update ALL payment methods, some of which might not be stripe, and should remain as "credit_card"...and as noted in #5, there are more statements that need to be executed. (All of which occur in commerce_stripe_update_8103.
If you need to re-migrate any of this data, this update hook would likely be what you should model your update off of.
There is already a revision that handles this: commerce_stripe_update_8103.
@gscerb
Thanks for the info.
This seems to be a duplicate of 🐛 "Register after Checkout" Pane causes payment method reuse to fail Fixed , which was addressed for Stripe Payment Element.
Hi @aleddr,
There are a variety of reasons why the Klarna payment method is not showing up.
Reviewing Stripe's Klarna documentation may be helpful. See: https://docs.stripe.com/payments/klarna
Some possibilities:
- The customer address is not in a supported country. Check the current list at the link referenced above.
- The currency is not supported. Check the current list at the link referenced above.
- The amount of the order is below the Klarna minimum. This can change, so hesitant to specify a number in this thread, but if you are doing a $1 test transaction, you won't get it as a payment option. I would suggest $100 for a test amount.
- The Payment Gateway is configured for "Setup Future Usage" with a value of "On-session" or "Off-session". These values allow you to "re-use" the payment method. Since Klarna is essentially a micro-loan, approved on a case by case basis, you normally would need to set the value to "Single use". Stripe and Klarna, though, do have the intention of allowing recurring payments, based on the link above, offering a "Private Preview Sign Up" option. For now, only "Single use" has been tested supported, though the other values may be supported with Klarna in the future.
IRT the webhook signing secret, ensure that you also are in "Test Mode" in stripe when creating the webhook. You don't want to send webhooks from prod to your test environment. Also, I'm presuming you are using the URL provided when you configured your payment gateway, but the pattern you mentioned does not look right. It should be https://[your site domain]/payment/notify/[your payment gateway machine name], e.g. https://www.example.com/payment/notify/my_stripe_gateway
Hi all! Thanks for the report.
1. Looks like you are reporting this against v1.1. Can you confirm? Upgrading to 1.2 is recommended, as it has many features and fixes. Please advise if you still encounter this issue with 1.2.
2. Please advise if you are using the "Stripe Card Element" or "Stripe Payment Element" gateway. They do differ, so it would be helpful in troubleshooting knowing which gateway you are experiencing issues with.
We've had reports that some hosting are running php 8.1.x with patches to resolve the issue, but are not able to upgrade to 8.1.6 or later for some reason.
This change lowers the php version requirement to 8.1 to assist those sites.
It is still highly recommended that you use 8.1.6 or later.
If you are using php 8.1.x < 8.1.6, please ensure you have the patch in place to avoid any issues on your site.
Thanks! (Message was tweaked to output change in capture configuration behavior.)
The (optional) configs are imported when the module is first enabled.
The queue functionality/config was added later, so the db update was created as a convenience to install it for anyone who already had the module installed.
Anyone doing a clean install of the module would get both installed from the config directory when the module is enabled(presuming requirements are met).
If the view is not installed, then most likely:
1. the views module installed when the submodule was first enabled
2. the view was manually deleted
3. config sync is being used, and the views config was not exported and saved. Then, a config import was done, deleting the view
You can easily re-create the view by going to config import in the UX, selecting import single item, selecting view, and then pasting in the contents of this config file.
@ nicolas-bouteille Good catch on the status. Sorry I missed that. :-/
IRT the view/route, the view is the intended UX for the listing page, as it provides all the magic of views, e.g. filtering, adding/reordering fields, etc...
The route callback is a fallback. If the view is deleted or hijacked to another path, it provides a minimalist listing.
Hi Nicolas!
Appreciate the detailed analysis and questions.
You are touching on a lot of intertwined items, and it would require more time to provide a thorough response. I'll give this summarized response for now.
- onReturn() is the "happy path" for the typical scenario and provides the smoothest customer flow in 99% of cases. (for card processing.)
- onNotify() / webhooks handles additional scenarios, like:
- a customer who completed stripe payment on the client/stripe side, but then they lost network connection before the redirect to onReturn() has been invoked.
- refunded/cancelled payments that occur on the processor/bank side.
- delayed payment processing, e.g. ACH(bank payments) that are not guaranteed at the time of order placement. The webhook might not fire for several days to indicate the payment was a success/fail.
As of 1.2, both onReturn and onNotify handle checkout completion, and check if it has already happened, to prevent double processing. onReturn always occurs for the client facing experience, but processing may be skipped when it detects that onNotify has already handled it.
onReturn and onNotify/webhooks is NOT an either/or. They work together to handle many different scenarios.
I would recommend to
- Enable the commerce_stripe_webhook_event submodule (We will likely promote this to the commerce_stripe module in a future version, rather than having it as a submodule.)
- Enable queued webhook processing using advanced queue
- Set up a scheduled job that runs every minute, executing the drush command
drush advancedqueue:queue:process commerce_stripe_webhook_event
There are ways to scale up even more from this, but this is a good starting point.
Presuming this is using a license type other than the built-in role license, as that license implements ExistingRightsFromConfigurationCheckingInterface, which precludes this scenario.
This also does not occur if the product variation type does not enable renewals.
With this change, this error should no longer occur when a customer attempts to add a product that does NOT implement ExistingRightsFromConfigurationCheckingInterface, that is a product variation type that enabled renewals, and the customer has an existing license. They should now properly receive a message that they already have the license, and prevent it from being added to the cart.
We actually shouldn't even get to this code path, as an existing license that is unlimited should not return TRUE for canRenew().
MR incoming that cleans up a few details in the logic.
tomtech → made their first commit to this issue’s fork.
Hi @newaytech!
Thanks for the question.
For php 8.1.x, there is an OPcache bug that is fixed in 8.1.6.
It is recommended to NOT use 8.1.0-8.1.5.
It is surprising to hear that WP only has beta support for PHP8.
In this post, they discuss how to upgrade Ubuntu 22.04 LTS to later PHP 8.1.x:
https://www.drupal.org/forum/support/post-installation/2023-01-04/ubuntu... →
Hopefully, this would resolve the issue for you.
We have now added initial support for SetupIntents in 📌 Add initial support for SetupIntents Active , which is needed for this and other features.
As mentioned in the description, the SupportsZeroBalanceOrderInterface requires Commerce 3, and the current version of this module still supports 2.x.
We anticipate adding this in the 2.x version of the module.
The last patch does not apply to the latest dev or beta.
Also, for a more consistent UX, we probably should disable the button, rather than hide it.
update to the MR incoming.
Some tweaks are in the works!
Hi all!
Thanks for the report and the patch!
Moving to needs work, as we'll address this in a different way shortly.
Q: Any reasons y'all are NOT enabling the sub-module, which resolves this issue? (The summary refers to it as the "logging" module, but it does more than just logging.)
@socialnicheguru, thanks for catching this and calling it out!
Q: If you are hitting this line in the database update, it does imply that you have a checkout pane in your checkout flow that is not defined.
Is this something in your custom code? An orphaned pane from something that was uninstalled? Or is this something else that might need further investigation?
tomtech → made their first commit to this issue’s fork.
Hi @btrd88 !
This is what the renewal window on the Product Variation Type is for.
If your license if valid for one year, and you specify a window of 30 days, then:
1. If it is less than 30 days from expiring, the user will be able to renew and will receive a message indicating the new extended expiration date.
2. If it is more than 30 days from expiring, the user will receive a message indicating the date they can renew.
This is not specific to subscriptions.
Merged!
Hi Andy!
If a setup intent is required in the checkout flow with this change, then it would break support for Klarna, Affirm, WeChat Pay, AliPay, and any other payment method that does not support future usage.
Certainly possible that I'm misunderstanding the proposed change, though. I've not had a chance yet to pull the MR down and give it a go. Will try that soon.
Hi all! Appreciate the dialog!
I'll try and address several items here:
1. Allowing customers to create payment methods in their profile (/user/[uid]/payment-methods) via setup intents. This is a desired feature, so if we get it in this MR, sweet!!
2. Admin created payment methods, on behalf of customers. (Mail Order/Telephone Order, aka MOTO). As noted in comment above #3406817-7: Create add-payment-method for Stripe Payment Element → , there are a lot of requirements about implementing this, including manual activation of MOTO by contacting stripe, MOTO parameters to be passed (which I can only find documented in other APIs and do not see in this MR), and the additional PCI requirements this entails. While other gateways may have supported this in the past, holding a lot of reservations about adding this to new ones. (Or continuing to keep them for previous ones.)
3. Why is the payment element on the review step, rather than a prior step? Payment processing has evolved. :)
The traditional multi-step checkout flow has been:
a. On Payment information step, customer provides their payment information, and a token is returned (or a stored payment token is selected)
b. On Review step, customer confirms their details. (order, payment, shipping, custom items, etc...) They can "complete" the order, moving on to steps (c) -> (d), or they can go back, and make changes to the order, payment method selection, etc...
c. On the (invisible) payment step, the token is used server side to make an API call and create the payment with the payment processor
d. On success, the complete step was displayed. The payment in step (c) could fail, moving us back to step (a).
In the contemporary flow, the payment processor expects to be the last step and creating the token creates the payment. If we revert to creating the token on step (a), and the customer lands on (b), they will have the false impression that they can "go back" and makes changes to the order, or abandon their cart. But...their payment has already been charged. With the implementation of 🐛 Payment entity created after webhook Fixed , we now create a payment on a webhook event, which will complete the order. Any action the customer takes on the review step in a reverted flow, will be irrelevant, as the order will have already moved to completion.
This flow looks more like this:
a. On Payment information step, customer selects an existing payment method, or to create a new one
b. On Review step, customer confirms order details; if creating a new payment method, provides payment information; confirms the order. A payment intent token is returned. (A payment has already been made on Stripe.)
c. On the (invisible) payment step, the token is used to retrieve(rather than create) the payment information from Stripe. Then, we create/update the payment method and payment on the commerce order and complete it. (If the customer happens to lose network connection, or close their browser, the webhook will create the payment method/payment and complete the order without the return browser side.)
d. The complete step is displayed
Note that this flow isn't really new. This is a similar flow to PayPal. The Adyen implementation also takes place on the review step.
4. Differences between card element and payment element. Card element is a stripe implementation that only supports card payments. payment element is their newer and preferred implementation that adds ApplePay, GooglePay, ACH, and many more payment methods in one implementation. It does behave differently, and therefore the implementation is different. Future development is planned around the payment element. (There is more nuance to this, but avoiding going too deep into the weeds.)
HTH clarify things!
This MR does the following:
Creates a payment when a payment intent success webhook event is received, if the payment has not yet been created.
For advanced queue webhook event processing, webhooks are deferred 5s to minimize race conditions with a return url.
Race conditions are improved via $order_storage->loadForUpdate($order_id) (including an enhancement in commerce core 2.40) See:
📌
Order Storage Lock Release
Fixed
Payment will be created by whichever process gets it first...whether the return url from the user browser, the other will await the other to complete, then proceed accordingly.
Closing this out, since we now support logging webhooks with the webhook events submodule.
As @torgosPizza noted, the SDK and API are not version dependent.
The SDK has been updated to support both version 7 and the latest version 15. See: 📌 Support Stripe SDK version 15 Fixed
The SDK major version is being updated on an average of every 3 months, which is pretty quick.
Both work with the current code in the module, but if you are extending or subclassing, you might have a different experience. If so, you can specify the version 7 sdk in your composer.json until you are able to resolve any changes.
Generally speaking the SDK has been extending, rather than modifying things, so this should be unlikely to occur.
The REST API, on the other hand, changes the payload depending on version, so it is more fragile, and require more communication for a change.
@agoradesign, you mentioned not having the paypal object without the latest SDK. This should be resolved with the update, but bear in mind that the properties are added by the SDK dynamically, so it would be present, even without the latest SDK. The same was true for other payment methods, like cashapp, karma, etc... that work with the v7 SDK, even though not explicitly defined in the SDK.
Let's have this issue focus on updating the REST API.