Are you using commerce_combine_carts by chance?
If so, it does "add" items from one cart to another, which would trigger this.
I presume this would also occur with the "standard" add to cart message in commerce core, if that is not disabled.
(If not, is this consistently reproducible? Or possibly a transient anomaly immediately after the upgrade?)
@gillmead, Thanks for the report!
In hindsight, maybe we shouldn't be putting the DOB in the label, even if it is the only field returned with the Klarna payment method.
I've modified the display label to return "Klarna ([createTime])", with the creation time of the Klarna payment.
I've merged a small commit that ensures the transition is allowed before invoking it.
While certainly not complete, see [#3515814--16059987] for some of the details behind handling the Stripe Payment Element in this fashion.
tomtech → changed the visibility of the branch 3513042-onreturn-method-of to hidden.
@jonathanshaw, thanks again for the report and MR.
The MR has been merged.
A few things to note:
1. It is necessary for the payment step to be bypassed.
The nature of the PaymentProcess Pane is to be "visible" and create a payment. When that occurs, it will not succeed, an exception would be generated, and the customer redirected back to a previous step...leaving the checkout flow and order in an incongruent state, given the payment was actually successful.
While it might seem that the PaymentProcess would be "invisible" if the order is paid, this only occurs if the payment is "complete". If the payment was only an authorization, (what Stripe calls "manual") the order would still have a balance, until the authorized payment is "captured".
Additionally, if the payment was less than the total, it would also cause the PaymentProcess pane to be "visible". This can happen if the review page with the Stripe payment component was reached with a discount applied, and/or a specific tax rate in effect...but the payment is not completed until later. e.g. Payment started at 11:55 pm December 31st, with a 10% discount applied, and a 7% tax rate, but the customer completes the order at 12:05am an January 1st, when the discount has expired, and the tax rate has changed to 8%.
2. It is possible that the customer completed the payment with Stripe, but the onReturn is NOT invoked. (This can happen when the customer's network connection drops, battery dies, etc...) In this case, the webhook necessarily handles creating the payment and placing the order. In the OOTB handling, the order will move the checkout to the complete step. If you are adding additional steps between payment and complete, be sure to consider this possibility.
Thanks for the report and. the MR, @jonathanshaw!
When the payment is saved, the order is queued for update and this occurs on request destruct, so the total paid and balance do get updated on the same request.
But, as you've observed, they are not yet available when the place transition is invoked after the payment is saved.
While this
Invoking the updateOrder() directly between the payment save and the updateOrder() is appropriate.
@anybody,
This is already being used on production sites.
We are in final testing before committing. (We generally try to test in different use cases/scenarios before committing, especially a larger feature add like this.)
Found some minor issues during recent testing that @vmarchuk has since addressed.
If you have any issues/feedback, please provide! (Note that, depending on the issue, we will likely still commit and tag a release, then follow up in a future commit/release.)
@jonathanshaw,
While ApplePay and GooglePay appear to be distinct payment method types in Stripe, this is only true to some degree. They actually return as the card payment method type, with a wallet.type of apple_pay or google_pay. (I know...confusing, isn't it?)
The initial reason for the implementation of the PaymentElement was to add support for ApplePay and GooglePay. (CardElement does not support them.) We've been adding additional payment method types over time since then.
@aaronbauman and @jonathanshaw,
Thanks for the report and comments on this. (And the other 2 items mentioned.)
Just got back from DrupalCon Atlanta, but will look to get some of this resolved soon.
Note, though, that there are reasons for the implementation, and I'll articulate them for your understanding and consideration. (Makes sense to get it into the documentation and/or a change record.)
Would also appreciate understanding your scenarios, as well, so that the best end result can be achieved.
@aaronbauman, you mention that the order is already marked complete at this point in your site. Given that this is done immediately after the payment is created, would it be reasonable to presume you are doing so when the payment is created? Can you share thought on this?
zaporylie → credited tomtech → .
The reverted database update selects all order ids, then uses it in the delete statement.
That would work with a nominal number of orders, but on sites with hundreds of thousands, or millions of orders, this would not be performant, presuming it completed at all.
The proposed singular not exists delete statement in #3363849-13: commerce_log entries are not removed on source entity delete → is much more performant, and could handle millions of rows.
For some sites that do have a larger number of orders, they might prefer to handle this manually in a separate task than during database update execution. This is where the flag that skips the execution is useful.
The preferred approach would have the not exists statement wrapped in the execution flag.
Hi and thanks for the report!
A few things to note here:
1. This has been the default behavior for ~12+ years, going back to D7. It is really a sample, though. You aren't locked into the view or the template.
2. This can get really complicated to try and handle OOTB, depending on the merchant requirements. Some examples:
If there is a max quantity of 10 for an item, there are currently 8 items in the cart, and the customer attempts to add 4 more, your site might only allow an increase of 2, not the 4 that was attempted.
If you are displaying price, and the price changes when quantity is increased, which price should be displayed? The base price from the product? Or the calculated price on the order item?
A simple way to avoid many of these issues, is to simply display the product that was added, and not display the price or quantity in the message. (Allow the cart to handle displaying the complexities of price/quantity/extended price/adjustments, etc...)
3. Fortunately, the dialog renders a view in a template. Customizations can be achieved by modifying the view. Advanced changes can be achieved by overriding the template in your theme.
Hi!
Thanks for the report!
Both width and maxWidth were set in the js when the dialog was declared. This provided a reasonable desktop width, along with the max width for mobile.
When the jQuery dialog was replaced by the Drupal core dialog, it appears support for the maxWidth property was not carried over.
Setting the width to 90% might work on mobile, but is way too wide for desktop, especially a wide one.
With the changes in theming in a recent MR, #3511910: Cleanup Theming → , the width is still set to 745 for desktop/laptop. max-width is now set in the css to handle this. (You can, of course, style to your desire in your theme!)
Hi @megachriz!
Thanks for the issue and the MR!
Confirmed and merged!
Looks like the tests started failing on a prior commit.
That will need to get resolved before any more merges can occur.
API version has been reworked. There is now a getter (getApiVersion()) for it, that is used for both the SDK and the js initialization.
api_version has been added to the configuration schema, and can be set.
NOTE: While the api_version is now part of the config schema and can be set, it has NOT been added to the gateway UX at this time. Changing API version is an advanced change. Providing an invalid API version will cause exceptions. Even when a valid API version is provided, payloads returned may be different which may cause unexpected behavior, or exceptions. (This MR addresses one such case.)
tomtech → changed the visibility of the branch 3489133-custom-display-label-translatable to hidden.
Hi sorabh.v6!
Thanks for the report!
This is not the appropriate fix, however.
The label in question is not currently showing in the translate tab, as the schema does not currently indicate it is a translatable property.
We need to update the schema to address this.
@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().