Skip to content
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.

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

Enable failed receipt sending for Stripe Gateway #14864

Merged
merged 4 commits into from
Jan 15, 2025

Conversation

staskus
Copy link
Contributor

@staskus staskus commented Jan 14, 2025

Closes: #14494

Description

After WooCommerce Stripe Gateway update, failed payment receipts are now supported for Stripe Gateway.

Enabling it for versions >=9.1.0

Steps to reproduce

Eligible + Sending Receipt After

  1. Install and configure WooCommerce Stripe Gateway 9.1.0
  2. Create an order with a failure amount, e.g $1.05
  3. Make a Card Reader / TTP payment
  4. Confirm error alert is shown with the "Email Receipt" option
  5. Send email receipt
  6. Confirm it's received on the email

Eligible + Attach Customer Before Payment

  1. Install and configure WooCommerce Stripe Gateway 9.1.0
  2. Create an order with a failure amount, e.g $1.05
  3. Attach a customer with an email
  4. Make a Card Reader / TTP payment
  5. Confirm error alert is shown with "Email sent" message
  6. Confirm email is received

Non-eligible

  1. Install a non-eligible Stripe Gateway version
  2. Create an order with a failure amount, e.g $1.05
  3. Attach a customer with an email
  4. Make a Card Reader / TTP payment
  5. Confirm error alert is shown with no "Email Receipt" option

Testing information

There are unit tests written for different cases. Also confirmed WooPayments continues to work.


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

@staskus staskus requested review from jaclync and joshheald January 14, 2025 14:58
@staskus staskus added this to the 21.5 milestone Jan 14, 2025
@staskus staskus added type: task An internally driven task. feature: mobile payments Related to mobile payments / card present payments / Woo Payments. labels Jan 14, 2025
@staskus staskus marked this pull request as ready for review January 14, 2025 14:58
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 14, 2025

WooCommerce iOS📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS WooCommerce iOS
Build Numberpr14864-994ad8b
Version21.4
Bundle IDcom.automattic.alpha.woocommerce
Commit994ad8b
App Center BuildWooCommerce - Prototype Builds #12538
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@jaclync jaclync self-assigned this Jan 15, 2025
Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

As before, the email didn't work again for me 😓 I tried two different personal emails (and triple checked I didn't mispell), maybe it's because the admin email is an A8C email. I haven't seen the SPAM email in the A8C spam folder yet. I couldn't find the transactional-emails log in this self-hosted Pressable store either. If you've received the email from testing, I don't think this is blocking.

Comment on lines 108 to 109
async let isWooCommerceSupported = isPluginSupported(Constants.wcPluginName,
minimumVersion: Constants.ReceiptAfterPayment.wcPluginMinimumVersion)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: since both payment gateways check the WC version the same way, how about having a function like isWCPluginSupported to encapsulate the plugin name & version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I can do that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made improvements in 994ad8b

Comment on lines 100 to 102
let wooCommerceResult = await isWooCommerceSupported
let wooPaymentsResult = await isWooPaymentsSupported
let isSupported = wooCommerceResult && wooPaymentsResult
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Could these be shortened to let isSupported = await isWooCommerceSupported && isWooPaymentsSupported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wish or at least I couldn't find a way. It could be shortened to get a tuple:

let supported = await (isWooCommerceSupported, isWooPaymentsSupported)

I'll look for more examples of how to structure such code better.

@staskus
Copy link
Contributor Author

staskus commented Jan 15, 2025

As before, the email didn't work again for me 😓 I tried two different personal emails (and triple checked I didn't mispell), maybe it's because the admin email is an A8C email. I haven't seen the SPAM email in the A8C spam folder yet. I couldn't find the transactional-emails log in this self-hosted Pressable store either. If you've received the email from testing, I don't think this is blocking.

Mmm... this is strange why it doesn't work for you 🤔 The email did arrive for me, I use Pressable as well. When you have time, @jaclync, could you add me to the sites for testing purposes?

@staskus staskus enabled auto-merge January 15, 2025 08:34
@staskus staskus merged commit 907d1d8 into trunk Jan 15, 2025
12 checks passed
@staskus staskus deleted the task/14494-handle-receipt-sending-for-stripe-extension branch January 15, 2025 08:52
@jaclync
Copy link
Contributor

jaclync commented Jan 16, 2025

As before, the email didn't work again for me 😓 I tried two different personal emails (and triple checked I didn't mispell), maybe it's because the admin email is an A8C email. I haven't seen the SPAM email in the A8C spam folder yet. I couldn't find the transactional-emails log in this self-hosted Pressable store either. If you've received the email from testing, I don't think this is blocking.

Mmm... this is strange why it doesn't work for you 🤔 The email did arrive for me, I use Pressable as well. When you have time, @jaclync, could you add me to the sites for testing purposes?

Sure I've sent an invitation link to your A8C email for the self-hosted Pressable test store. I think you already have access to the atomic test store "fun testing" from the wp-admin users.

@staskus
Copy link
Contributor Author

staskus commented Jan 16, 2025

@jaclync, the emails didn't work for me for this particular site as well.

However, replacing the A8C email with an email to contain a subdomain in WooCommerce -> Settings -> Emails it started working.

image

@jaclync
Copy link
Contributor

jaclync commented Jan 17, 2025

However, replacing the A8C email with an email to contain a subdomain in WooCommerce -> Settings -> Emails it started working.

Oh wow, thanks for looking into the email issue. TIL about the email setting and will make this change for other sites with email issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: mobile payments Related to mobile payments / card present payments / Woo Payments. type: task An internally driven task.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TTPReq] Handle receipt sending for Stripe extension
3 participants