Skip to content

Better error messages when coupons are stuck in a pending order. #28592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
roykho merged 7 commits into from
Dec 21, 2020
Merged

Better error messages when coupons are stuck in a pending order. #28592

roykho merged 7 commits into from
Dec 21, 2020

Conversation

@vedanshujain
Copy link
Contributor

@vedanshujain vedanshujain commented Dec 15, 2020 •
edited
Loading

We hold coupons when payment is failed if the setting "hold stock for checkout" is enabled for some minutes. This is to allow the customers to try again if they want, and to give time to complete payment for gateways where it could take some time.

Unfortunately this cause for some bad UX user retries by starting the order from scratch again, and then if they apply the coupon, usage limit gets hit because the earlier coupon is still held and is counted towards the usage. This PR improves the error message in these cases when usage is applied per customer and is hit, by stating to go to my account to complete/cancel payment (in case of logged in user) or to wait for some time in case of guest users.

All Submissions:

Changes proposed in this Pull Request:

Closes #27673.

How to test the changes in this Pull Request:

(While logged in to an account)

  1. Create a coupon with a usage limit of 1 (either per user, or a global usage limit).
  2. Checkout using this coupon, but use a payment method which will payment step so that order is set to pending or failed. (For example, stripe)
  3. Checkout using this coupon again, you will see a better error message which will ask you to go to my account and cancel or complete the order. If my account page is not enabled on the site, it will just say usage limit reached.

(As a guest)

  1. Create a coupon with a usage limit of 1 (either per user, or a global usage limit).
  2. Checkout using this coupon, but use a payment method which will payment step so that order is set to pending or failed. (For example, stripe)
  3. Checkout using this coupon again, you will see a better error message which will ask you to wait for some time or contact support.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Fix - Better error messages when usage limit are reached.

@vedanshujain
Better error messages when coupons are stuck in a pending order.
507e7a2 
We hold coupons when payment is failed if the setting "hold stock for checkout" is enabled for some minutes. This is to allow the customers to try again if they want, and to give time to complete payment for gateways where it could take some time.

Unfortunately this cause for some bad UX user retries by starting the order from scratch again, and then if they apply the coupon, usage limit gets hit because the earlier coupon is still held and is counted towards the usage. This commit improves the error message in these cases when usage is applied per customer and is hit, by stating to go to my account to complete/cancel payment (in case of logged in user) or to wait for some time in case of guest users.
@vedanshujain vedanshujain mentioned this pull request Dec 15, 2020
Closed
5 tasks
@vedanshujain
Better error messages for when coupon are held in other transactions.
6b550ff 
Earlier, we were just showing an "Usage limit reached message", however in some cases, specially when user is logged in, we can also ask them to go to MyAccount page and cancel order if they'd like to (to free up the coupon). This will hopefully make for a better user experience.
@vedanshujain
Fix unit test to use new coupon error messages.
9ca52ee 
This also removes some cart operations which are not needed anymore to since cart already has items that we were adding in those tests.
@vedanshujain
Add unit tests for #28592
981d494
@vedanshujain
Fixup for myaccount page logic, it should be > 0 ot be considered as …
6fa77b3 
…set.
@vedanshujain vedanshujain marked this pull request as ready for review December 16, 2020 14:18
@vedanshujain vedanshujain requested review from a team and roykho and removed request for a team December 16, 2020 14:18
roykho
roykho suggested changes Dec 16, 2020
View reviewed changes
Copy link
Contributor

@roykho roykho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment on coding standards. Besides that I think in general it tested well however if you try to checkout again with valid payment after 1st fail, you will get this message

This message could be misleading as the coupon is currently just locked to their pending order. Perhaps we should add the same message to go to their my account to cancel the order to reinstate this coupon? This way it is more consistent?

The other thing I noticed was that the Travis CI is failing on the e2e tests but I don't think it is related to this PR.

@vedanshujain
Copy link
Contributor Author

vedanshujain commented Dec 21, 2020

if you try to checkout again with valid payment after 1st fail, you will get this message

@roykho Looks like this is coming via https://github.com/woocommerce/woocommerce/blob/master/includes/class-wc-cart.php#L726, which is basically eating up all the coupon error messages.

I'd rather target this in a different PR targeted to next release instead of this one since it will affect all error messages instead of just this one case. And so will need more robust testing and tests as well. Do you think if its fine to target refactoring that method in a different PR?

@vedanshujain vedanshujain added this to the 4.9.0 milestone Dec 21, 2020
@roykho
Copy link
Contributor

roykho commented Dec 21, 2020

I'd rather target this in a different PR targeted to next release instead of this one since it will affect all error messages instead of just this one case.

Ok that's fine.

roykho
roykho approved these changes Dec 21, 2020
View reviewed changes
Copy link
Contributor

@roykho roykho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this and a separate issue will be opened for the error message by author.

@roykho roykho merged commit d2cd617 into master Dec 21, 2020
@roykho roykho deleted the fix/27673 branch December 21, 2020 13:22
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Dec 21, 2020
@vedanshujain vedanshujain mentioned this pull request Dec 21, 2020
Open
@vedanshujain
Copy link
Contributor Author

vedanshujain commented Dec 21, 2020

Closing the loop: #28641

@vedanshujain vedanshujain added release: highlight Issues that have a high user impact and need to be discussed/paid attention to. and removed release: add changelog Mark all PRs that have not had their changelog entries added. [auto] labels Dec 21, 2020
@juliaamosova juliaamosova added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@roykho roykho roykho approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release: highlight Issues that have a high user impact and need to be discussed/paid attention to.

Projects

None yet

Milestone

4.9.0

Development

Successfully merging this pull request may close these issues.

Coupon "usage limit has been reached" after a failed payment

5 participants

@vedanshujain @roykho @juliaamosova @woocommercebot