Added feature for payment gateways banner #32697

rjchow · 2022-04-20T07:31:31Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR makes a set of changes in order to implement the feature for a payment gateways banner.

There were two possible approaches to implement this - fully in PHP and the other to embed a React tree on a new element that is introduced on the PHP side.

The decision was made to embed a React tree for ease of future maintenance, and also to make the new mount point an experimental Slotfill so that we can add future items to it.

Explat post: pbxNRc-1yD-p2

Closes #32414 .

Control:

Treatment:

How to test the changes in this Pull Request:

There are 5 scenarios to test:

WCPay eligible and WCPay installed and setup completed and assigned to 'treatment' -> Should not see the banner
WCPay eligible and WCPay not installed -> Should not see the banner
WCPay eligible and WCPay installed but setup incomplete and assigned to 'treatment' -> Should see the banner
WCPay eligible and WCPay installed but setup incomplete and assigned to 'control' -> Should not see the banner
WCPay ineligible and/or WCPay not installed -> Should not see the banner

Set up a fresh install of Wordpress with WooCommerce, and depending on the scenario you are testing, install WooCommerce Payments plugin (either as part of onboarding or otherwise)
Install and activate wc-test-helper plugin
Navigate to Tools -> WCA Test Helper
Click Experiments
Toggle the experiment assignment for 'woocommerce_payments_settings_banner_2022_04' to control or treatment depending on the scenario
Visit WooCommerce > Settings, Payments tab, and check if the banner is visible or not

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?
Have you created a changelog file by running pnpm nx affected --target=changelog?

FOR PR REVIEWER ONLY:

I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

This reverts commit 1a7c403.

chihsuan · 2022-04-21T06:38:22Z

plugins/woocommerce-admin/client/payments/payment-settings-banner.tsx

+
+const WcPayBanner = () => {
+	const WC_PAY_SETUP_URL =
+		'./admin.php?page=wc-settings&tab=checkout&section=woocommerce_payments';


I think we could use getAdminLink here:

woocommerce/plugins/woocommerce-admin/client/tasks/fills/products/products.js

Lines 64 to 66 in feba6a8

href: getAdminLink(

'post-new.php?post_type=product&wc_onboarding_active_task=products&tutorial=true'

),

Hmm... that's new to me... however I can't actually find the source for @woocommerce/settings and the only mention I've seen of it is in the changelog where it says its been removed

How does this work? 👀👀👀

Hmm...I think it's still injected into our code via the Dependency Extraction Webpack Plugin, though I also don't know how it exactly works.

however I can't actually find the source for @woocommerce/settings and the only mention I've seen of it is in the changelog where it says its been removed

The settings package used to be under WCAdmin, but now lives in blocks https://github.com/woocommerce/woocommerce-gutenberg-products-block/tree/trunk/assets/js/settings/shared

P2: p90Yrv-2B5-p2

Changed in 3ee2fca

chihsuan · 2022-04-21T06:39:49Z

plugins/woocommerce-admin/client/payments-welcome/index.tsx

+	Discover,
+	JCB,
+	UnionPay,
+} from './cards';


Nice work for refactoring this!

chihsuan · 2022-04-21T06:44:26Z

plugins/woocommerce-admin/client/payments/payment-settings-banner.tsx

+		};
+	} );
+
+	const supportsWCPayments =


Looks like payment-recommendations.tsx also has the same code. How about creating a util function for this?

woocommerce/plugins/woocommerce-admin/client/payments/payment-recommendations.tsx

Lines 84 to 94 in 6151f40

const supportsWCPayments =

paymentGatewaySuggestions &&

paymentGatewaySuggestions.filter(

( paymentGatewaySuggestion: Plugin ) => {

return (

paymentGatewaySuggestion.id.indexOf(

'woocommerce_payments'

) === 0

);

}

).length === 1;

Yea good call! I was planning to extract the entire hook and everything but ended up realising I only needed a small part of the useSelect and forgot that there was this part below that's duplicated

Updated in 11f321a

chihsuan

Tested well on my local for all scenarios. 👍 Just left some suggestions.

ilyasfoo

Tested well for me. Great work, @rjchow! The only thing left I think is to finalize the experiment name if we intend to do it dynamically.

ilyasfoo · 2022-04-21T06:14:08Z

plugins/woocommerce-admin/client/payments/payment-settings-banner.tsx

+	);
+
+	const [ isLoadingExperiment, experimentAssignment ] = useExperiment(
+		'woocommerce_payments_settings_banner_2022_04'


I think this is meant to be a dynamic experiment? If so, we should use dynamic month & year

Done in 2398fdf

ilyasfoo · 2022-04-26T02:30:17Z

I'll be working on my own feedback soon. @chihsuan, would you like to address your feedback as well or would you think it's better to address it as a follow-up?

chihsuan · 2022-04-26T02:52:13Z

I'll be working on my own feedback soon. @chihsuan, would you like to address your feedback as well or would you think it's better to address it as a follow-up?

@ilyasfoo Thanks for the ping. Since those are just small changes, I'll address my feedback today!

adrianduffell · 2022-04-27T09:10:42Z

@ilyasfoo I think we need to dismiss your review to merge? 🤔

ilyasfoo · 2022-04-28T03:54:34Z

The "Payments Methods" header flash of text looks a bit janky to me, and I have concerns that removing the header makes it inconsistent with the other tabs. However we could follow-up on that in another PR after getting design feedback about it.

Sounds good!

@ilyasfoo I think we need to dismiss your review to merge? 🤔

Oops sorry missed this, I've approved it instead!

ilyasfoo · 2022-04-28T03:59:06Z

@adrianduffell do you think we should attempt to merge from trunk to see if it fixes checks? Or should we just merge it anyway?

adrianduffell · 2022-04-28T05:19:54Z

do you think we should attempt to merge from trunk to see if it fixes checks?

@ilyasfoo Ah yep, I think we should run the checks. I'll push a commit with the merge from trunk.

adrianduffell · 2022-04-28T08:10:00Z

Looks like the e2e tests are failing on the payments settings screen. I'll take a closer look tomorrow.

ilyasfoo · 2022-04-29T00:31:19Z

@adrianduffell I think that may be related to the "Payment Methods" header missing. I'll take a look instead, I suspect changing it to detect its table is better as RJ suggested

ilyasfoo · 2022-04-29T01:43:48Z

I think I've fixed e2e with d947699, but it seems some checks are failing due to unrelated errors. I'm not sure yet what it is, but it may have come with merge from trunk.

…ays-banner

ilyasfoo · 2022-04-29T02:57:53Z

Checks are successful now, merging

github-actions · 2022-04-29T02:58:16Z

Hi @ilyasfoo, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

Add the release: add testing instructions label

github-actions bot added focus: react admin package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Apr 20, 2022

rjchow marked this pull request as ready for review April 21, 2022 02:59

rjchow added 9 commits April 21, 2022 11:29

add slotfill mount point

ab3a015

insert react

9905a88

design banner

feb1f42

implement slotfill

9a95dc0

Refactored and added new payment icons

aaf5c9f

added conditions for rendering

07a20ce

types and lint

6088570

cleanup

73634d6

changelogs

bbb1af4

rjchow force-pushed the add/wc-payment-gateways-banner branch from cd644f3 to bbb1af4 Compare April 21, 2022 03:30

Changed e2e payment settings page detection

1a7c403

github-actions bot added the package: @woocommerce/admin-e2e-tests issues related to @woocommerce/admin-e2e-tests label Apr 21, 2022

fixed phpunit tests

c796406

github-actions bot removed the package: @woocommerce/admin-e2e-tests issues related to @woocommerce/admin-e2e-tests label Apr 21, 2022

Revert "Changed e2e payment settings page detection"

6151f40

This reverts commit 1a7c403.

rjchow force-pushed the add/wc-payment-gateways-banner branch from 38059c9 to 6151f40 Compare April 21, 2022 06:09

chihsuan reviewed Apr 21, 2022

View reviewed changes

ilyasfoo suggested changes Apr 21, 2022

View reviewed changes

Change experiment name to dynamic

2398fdf

chihsuan added 2 commits April 26, 2022 10:55

Use getAdminLink to format WC_PAY_SETUP_URL

3ee2fca

Add isWcPaySupported util function for payments

11f321a

ilyasfoo previously approved these changes Apr 28, 2022

View reviewed changes

Merge branch 'trunk' into add/wc-payment-gateways-banner

8a3ae13

Update e2e to use table instead

d947699

ilyasfoo dismissed stale reviews from adrianduffell and themself via d947699 April 29, 2022 01:09

github-actions bot added the package: @woocommerce/admin-e2e-tests issues related to @woocommerce/admin-e2e-tests label Apr 29, 2022

Merge branch 'trunk' into add/wc-payment-gateways-banner

fc74c42

Merge remote-tracking branch 'origin/trunk' into add/wc-payment-gatew…

cb731d4

…ays-banner

ilyasfoo self-requested a review April 29, 2022 02:56

ilyasfoo approved these changes Apr 29, 2022

View reviewed changes

ilyasfoo self-requested a review April 29, 2022 02:57

ilyasfoo merged commit 8b9870b into trunk Apr 29, 2022

ilyasfoo deleted the add/wc-payment-gateways-banner branch April 29, 2022 02:58

github-actions bot added this to the 6.6.0 milestone Apr 29, 2022

chihsuan added the release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] label Apr 29, 2022

github-actions bot mentioned this pull request Jun 15, 2022

Test WooCommerce 6.6 UVLabs/location-picker-at-checkout-for-woocommerce#35

Closed

	href: getAdminLink(
	'post-new.php?post_type=product&wc_onboarding_active_task=products&tutorial=true'
	),

	const supportsWCPayments =
	paymentGatewaySuggestions &&
	paymentGatewaySuggestions.filter(
	( paymentGatewaySuggestion: Plugin ) => {
	return (
	paymentGatewaySuggestion.id.indexOf(
	'woocommerce_payments'
	) === 0
	);
	}
	).length === 1;

Added feature for payment gateways banner #32697

Added feature for payment gateways banner #32697

Uh oh!

Conversation

rjchow commented Apr 20, 2022 • edited by chihsuan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

Control:

Treatment:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chihsuan left a comment

Choose a reason for hiding this comment

Uh oh!

ilyasfoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilyasfoo commented Apr 26, 2022

Uh oh!

chihsuan commented Apr 26, 2022

Uh oh!

adrianduffell commented Apr 27, 2022

Uh oh!

ilyasfoo commented Apr 28, 2022

Uh oh!

ilyasfoo commented Apr 28, 2022

Uh oh!

adrianduffell commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adrianduffell commented Apr 28, 2022

Uh oh!

ilyasfoo commented Apr 29, 2022

Uh oh!

ilyasfoo commented Apr 29, 2022

Uh oh!

ilyasfoo commented Apr 29, 2022

Uh oh!

github-actions bot commented Apr 29, 2022 • edited by chihsuan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rjchow commented Apr 20, 2022 •

edited by chihsuan

Loading

adrianduffell commented Apr 28, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited by chihsuan

Loading