fix/143: Cancelling Google Pay on checkout shows validation errors #188

Sidsector9 · 2024-07-25T12:50:00Z

All Submissions:

Does your code follow the WooCommerce Sniffs variant of WordPress coding standards?
Have you written new tests for your changes, as applicable?
Have you successfully run tests with your changes locally?
Will this change require new documentation or changes to existing documentation?

Changes proposed in this Pull Request:

Closes #143

This PR does 2 things:

Fixes the issue [#133 followup] Cancelling Google Pay on checkout shows validation errors. #143
Refactors the implementation of Express Payment to align with other Woo gateways.

Steps to test the changes in this Pull Request:

Test Digital Wallets features.

Changelog entry

Fix - Cancelling Google Pay on checkout shows validation errors.

peterwilsoncc

I've added some notes inline.

I am getting a JavaScript error for Apple Pay in Safari so the payment form fails to open but have yet to figure out the cause.

For Google Pay in Safari it re-introduces a bug that was fixed earlier in which the entire page is redirected to the Google Pay form. I tried to find the commit in which it was fixed but without success

Screen.Recording.2024-07-26.at.1.38.23.PM.mov

assets/blocks/digital-wallets/content.js

Sidsector9 · 2024-07-26T18:46:16Z

@peterwilsoncc I've fixed the issues you pointed, requesting a re-review.

peterwilsoncc

This is working great, I've made a suggestion inline to prevent the Apple button displaying in non-Safari Browsers.

The one bug I am seeing with Apple Pay is when there are no cards defined. If I cancel the payment attempt, the Apple and Google Pay buttons remain locked.

I suspect a different event is fired for cancelled payment requests as opposed to invalid requests. I think this might be an existing bug so if it is, I'd recommend opening a follow up ticket rather than letting it block you on this one

The validation error fixes are looking great and working as expected. Nice work.

peterwilsoncc · 2024-07-29T03:39:42Z

assets/blocks/digital-wallets/hooks.js

+		applePayRef.current.style.cssText += `-apple-pay-button-style: ${color};`;
+		applePayRef.current.style.display = 'block';
+		applePayRef.current.classList.add(`wc-square-wallet-button-${color}`);
+	}, [applePayRef]);


Suggested change

}, [applePayRef]);

}, [applePayRef, applePay]);

peterwilsoncc · 2024-07-29T03:40:26Z

assets/blocks/digital-wallets/hooks.js

+	}, [payments, paymentRequest]);
+
+	useEffect(() => {
+		if (!applePayRef?.current) {


Required with above suggestion to ensure the Apple Pay button is only shown in Safari.

Suggested change

if (!applePayRef?.current) {

if (!applePayRef?.current || !applePay) {

@peterwilsoncc I've added these suggestions.

…nto fix/143

peterwilsoncc

Thanks Sid, this looks good and is working as expected.

Testing notes

In Safari, checkout page (blocks):

Click Apple Pay button
Cancel
Validation errors are not shown (as expected)
Click Google Pay button
Cancel
Validation errors are not shown (as expected)
Click Apple Pay button
Proceed through payment
Order processed as expected
Re-order, return to checkout
Click Google Pay button
Proceed through payment
Order processed as expected

In Safari, cart page (blocks)

Click Apple Pay button
Cancel
Click Google Pay button
Cancel
Click Apple Pay button
Proceed through payment
Order processed as expected
Re-order, return to cart
Click Google Pay button
Proceed through payment
Order processed as expected

In Chrome, checkout page (blocks):

Apple Pay button not shown
Click Google Pay button
Cancel
Validation errors are not shown (as expected)
Click Google Pay button
Proceed through payment
Order processed as expected

In Chrome, cart page (blocks)

Apple Pay button not shown
Click Google Pay button
Cancel
Click Google Pay button
Proceed through payment
Order processed as expected

qasumitbagthariya · 2024-08-05T10:16:53Z

QA Update ✅

I have verified this PR in the fix/143 branch has been fixed and is functioning as intended.

I tested the following on this branch:

Google Pay at checkout page ✅
Validation errors are not shown ✅

Testing Environment

WordPress: 6.6.1
Theme: Storefront 4.6.0
Theme: Twenty Twenty-Four 1.2
WooCommerce - 9.1.4
PHP: 8.0.30
Web Server: Nginx 1.20.2
Browser: Chrome
OS: macOS Ventura 13.3
Branch: fix/143

Screen.Recording.2024-08-05.at.3.43.24.PM.mov

Steps to Test- As mentioned in the PR description.
Test Results - It is working as expected.
Functional Demo / Screencast -
Special Notes - Ready for code review (Woo)
Testing Document status:
Cases related to this Issue/PR are added to the Critical Flow Wiki pages:

Yes
Not Required/Applicable for this PR

vikrampm1 · 2024-08-07T11:52:42Z

@Mayisha this PR is ready for your review!

Mayisha

Changes look good to me and it fixed the original issue.

I can reproduce the issue in trunk. When I cancel Google Pay on the block checkout page, it triggers the form validation.

In this branch canceling Google Pay does not trigger the form validation on the block checkout page.

I just marked a few lines in my comment below which look like typos to me.

Mayisha · 2024-08-07T11:12:28Z

src/blocks/digital-wallets/hooks.js

+			try {
+				await payments.googlePay(paymentRequest);
+				const __googlePay = await payments.googlePay(paymentRequest);
+				await __googlePay.attach(googlePayRef.current, {


await payments.googlePay(paymentRequest); is twice here.

Mayisha · 2024-08-07T11:15:41Z

src/blocks/digital-wallets/hooks.js

+			try {
+				await payments.applePay(paymentRequest);
+				const __applePay = await payments.applePay(paymentRequest);
+


await payments.applePay(paymentRequest) is also called twice similar to googlePay. Is this intentional?

Mayisha · 2024-08-07T11:31:52Z

src/blocks/digital-wallets/hooks.js

+				async function handlePaymentProcessing() {
+					let response = { type: emitResponse.responseTypes.SUCCESS };
+
+					if (!tokenResult) {


Is this check going to work as we are already returning from line 212 above for the same condition?

Thanks for catching these, I've made the changes in 33a8a84

Thanks @Sidsector9 for the update. The PR is already approved.
I have tested this branchagain and confirmed that

Canceling Google Pay does not trigger the form validation on the block checkout page.

I can successfully purchase a product with Google Pay.

…nto fix/143

qasumitbagthariya · 2024-08-14T20:40:45Z

Regression / Smoke Test Report ✅

Tested with Archive File created via "php woorelease.phar build repo_URL" (Composer version 2.5.5, npm version 8.19.4, node version 16.20.0)

Status- Working expected with Plugin Archive/Zip file same as fix specific branch.

Testing Environment

WordPress: 6.6.1
Theme: Storefront 4.6.0
Theme: Twenty Twenty-Four 1.2
WooCommerce - 9.1.4
PHP: 8.0.30
Web Server: Nginx 1.20.2
Browser: Chrome
OS: macOS Ventura 13.3
Branch: smoke-testing

Next Step- Ready to Merge 🚀

Sidsector9 added 8 commits July 25, 2024 02:34

consistent express payment

c78c392

working payment

810ddde

remove redundancy

5ec93dd

fix bug where modal doesn't reopen after closing

497ef1f

remove redundant files

53739a0

fix eslint errors

5aa6b8d

add missing doc

8e90d5f

use emitResponse

bb5fe31

Sidsector9 self-assigned this Jul 25, 2024

Sidsector9 added the needs: e2e testing label Jul 25, 2024

fix eslint errors

4215496

vikrampm1 added this to the Future Release milestone Jul 25, 2024

peterwilsoncc reviewed Jul 26, 2024

View reviewed changes

assets/blocks/digital-wallets/content.js Outdated Show resolved Hide resolved

assets/blocks/digital-wallets/content.js Outdated Show resolved Hide resolved

Sidsector9 added 6 commits July 26, 2024 14:41

add missing classes on apple pay wrapper

0f1d209

add try/catch block

7b98a0c

fix gpay popup in safari

7e01bf9

add early return to click handler

1bf478e

replace onPaymentProcessing with onPaymentSetup

3d3a553

Merge branch 'trunk' into fix/143

4d57b4e

Sidsector9 requested a review from peterwilsoncc July 26, 2024 18:46

github-actions bot added the status: e2e tests passing label Jul 26, 2024

peterwilsoncc reviewed Jul 29, 2024

View reviewed changes

Sidsector9 added 3 commits July 30, 2024 07:38

Merge branch 'trunk' into fix/143

bae9495

Merge branch 'fix/143' of github.com:woocommerce/woocommerce-square i…

343afda

…nto fix/143

disable apple pay buttons for non-safari browsers

ce66f9f

Sidsector9 force-pushed the fix/143 branch from 76cf900 to ce66f9f Compare July 30, 2024 02:41

github-actions bot added the status: e2e tests failing label Jul 30, 2024

Sidsector9 requested a review from peterwilsoncc July 30, 2024 02:51

peterwilsoncc removed the status: e2e tests failing label Jul 30, 2024

github-actions bot added the status: e2e tests failing label Jul 30, 2024

peterwilsoncc approved these changes Jul 30, 2024

View reviewed changes

Sidsector9 mentioned this pull request Jul 30, 2024

Express Payment buttons are frozen when cancelling Apply Pay popup #192

Open

Merge branch 'trunk' into fix/143

b0eed44

Sidsector9 force-pushed the fix/143 branch 2 times, most recently from 6a2c425 to b0eed44 Compare August 5, 2024 04:25

qasumitbagthariya requested review from a team and Mayisha and removed request for a team August 5, 2024 10:17

qasumitbagthariya added needs: e2e testing and removed needs: e2e testing status: e2e tests failing labels Aug 5, 2024

github-actions bot added the status: e2e tests failing label Aug 5, 2024

Mayisha approved these changes Aug 7, 2024

View reviewed changes

Sidsector9 added 4 commits August 8, 2024 15:05

PR review changes

33a8a84

Merge branch 'trunk' into fix/143

c824cae

Merge branch 'fix/143' of github.com:woocommerce/woocommerce-square i…

9777d31

…nto fix/143

fix eslint errors

a2c474f

Merge branch 'trunk' into fix/143

8a0ff9d

github-actions bot added status: e2e tests passing and removed status: e2e tests failing labels Aug 23, 2024

vikrampm1 modified the milestones: Future Release, 4.8.0 Aug 23, 2024

vikrampm1 marked this pull request as ready for review August 23, 2024 15:10

vikrampm1 merged commit dcab786 into trunk Aug 23, 2024
6 checks passed

vikrampm1 deleted the fix/143 branch August 23, 2024 15:11

vikrampm1 mentioned this pull request Aug 23, 2024

Release version 4.8.0 #203

Closed

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix/143: Cancelling Google Pay on checkout shows validation errors #188

fix/143: Cancelling Google Pay on checkout shows validation errors #188

Sidsector9 commented Jul 25, 2024 •

edited by vikrampm1

Loading

peterwilsoncc left a comment

Sidsector9 commented Jul 26, 2024

peterwilsoncc left a comment

peterwilsoncc Jul 29, 2024

peterwilsoncc Jul 29, 2024

Sidsector9 Jul 30, 2024

peterwilsoncc left a comment •

edited

Loading

qasumitbagthariya commented Aug 5, 2024

vikrampm1 commented Aug 7, 2024

Mayisha left a comment

Mayisha Aug 7, 2024

Mayisha Aug 7, 2024

Mayisha Aug 7, 2024

Sidsector9 Aug 8, 2024

Mayisha Aug 9, 2024

qasumitbagthariya commented Aug 14, 2024

	if (!applePayRef?.current) {
	if (!applePayRef?.current \|\| !applePay) {

fix/143: Cancelling Google Pay on checkout shows validation errors #188

fix/143: Cancelling Google Pay on checkout shows validation errors #188

Conversation

Sidsector9 commented Jul 25, 2024 • edited by vikrampm1 Loading

All Submissions:

Changes proposed in this Pull Request:

Steps to test the changes in this Pull Request:

Changelog entry

peterwilsoncc left a comment

Choose a reason for hiding this comment

Sidsector9 commented Jul 26, 2024

peterwilsoncc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterwilsoncc left a comment • edited Loading

Choose a reason for hiding this comment

qasumitbagthariya commented Aug 5, 2024

QA Update ✅

vikrampm1 commented Aug 7, 2024

Mayisha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qasumitbagthariya commented Aug 14, 2024

Regression / Smoke Test Report ✅

Sidsector9 commented Jul 25, 2024 •

edited by vikrampm1

Loading

peterwilsoncc left a comment •

edited

Loading