Replace isWooCoreProfilerFlow with isPasswordlessJetpackConnectionFlow #95504

moon0326 · 2024-10-17T23:40:04Z

Related to #95041

This is an enhanced version of #95041

Proposed Changes

Replaces isWooCoreProfilerFlow with isPasswordlessJetpackConnectionFlow. This is a more abstract approach
to determining whether to display the new Jetpack connection page. isPasswordlessJetpackConnectionFlow checks for isWooCoreProfilerFlow and isWooCommercePaymentsOnboardingFlow

Testing Instructions

Create a new JN site with this latest woocommerce.
Go through the core profiler and select Jetpack on the extensions page.
Continue and you should be redirected to the Jetpack connection page.
Change from to woocommerce-payments and add plugin_name=woocommerce-payments param to the URL and then copy the URL.
Checkout this branch locally and set it up with woocommerce-start-dev-env
Visit the URL
You should still see the new Jetpack connection page.

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

github-actions · 2024-10-17T23:40:16Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-122607

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-122607&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-122607&env=a8c-for-agencies

matticbot · 2024-10-17T23:40:53Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~114 bytes added 📈 [gzipped])

name                 parsed_size           gzip_size
entry-login               +438 B  (+0.0%)     +114 B  (+0.0%)
entry-stepper             +276 B  (+0.0%)      +67 B  (+0.0%)
entry-main                +276 B  (+0.0%)      +67 B  (+0.0%)
entry-subscriptions        +47 B  (+0.0%)      +20 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~123 bytes added 📈 [gzipped])

name              parsed_size           gzip_size
jetpack-connect        +309 B  (+0.0%)      +64 B  (+0.0%)
security               +183 B  (+0.0%)      +59 B  (+0.0%)
accept-invite          +165 B  (+0.1%)      +47 B  (+0.1%)
purchase-product        +12 B  (+0.0%)       +8 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~46 bytes added 📈 [gzipped])

name                                     parsed_size           gzip_size
async-load-design-blocks                      +171 B  (+0.0%)      +53 B  (+0.0%)
async-load-signup-steps-user                  +165 B  (+0.1%)      +47 B  (+0.1%)
async-load-signup-steps-subscribe-email       +165 B  (+0.1%)      +47 B  (+0.1%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

chihsuan

Thanks for working on this! @moon0326 This is a great improvement. 👍 I have a few comments:

I searched the isWooCoreProfiler in the codebase and looks like we still have many references to it. Is this expected?

Personally, I think isPasswordlessJetpackConnectionFlow is bit confusing. Especially, when I see the code like this:

isWooCoreProfilerFlow={ isPasswordlessJetpackConnection }

or

isWoo: isWooOAuth2Client( oauth2Client ) || isPasswordlessJetpackConnectionFlow( state ),

I wonder if we should also have Woo in the function name like isWooPasswordlessJPCFlow or something similar, but maybe require more opinions on this. Just wanted to bring this up.

ilyasfoo

I agree with @chihsuan, isPasswordlessJetpackConnection sounds confusing especially if it's exclusively used by Woo.

ilyasfoo · 2024-10-18T01:55:26Z

client/state/selectors/is-passwordless-jetpack-connection-flow.ts

+		( get( getInitialQueryArguments( state ), 'from' ) === 'woocommerce-payments' ||
+			get( getCurrentQueryArguments( state ), 'from' ) === 'woocommerce-payments' ) &&
+		( get( getInitialQueryArguments( state ), 'plugin_name' ) === 'woocommerce-payments' ||
+			get( getCurrentQueryArguments( state ), 'plugin_name' ) === 'woocommerce-payments' )


Nit: I think we should declare get( getInitialQueryArguments( state ), 'from' ) and get( getInitialQueryArguments( state ), 'plugin_name' ) as variables here, I feel it's too long to read clearly

@ilyasfoo Good call! Updated it in Clean up isWooCommercePaymentsOnboardingFlow function

moon0326 · 2024-10-18T18:48:48Z

@chihsuan @ilyasfoo

I searched the isWooCoreProfiler in the codebase and looks like we still have many references to it. Is this expected?

Not sure how I missed that many 😮 I'll work on it.

isWooPasswordlessJPCFlow
I agree with @chihsuan, isPasswordlessJetpackConnection sounds confusing especially if it's exclusively used by Woo.

Good call. I'll work on it 👍

moon0326 · 2024-10-21T17:35:30Z

@chihsuan @ilyasfoo

I've updated the function name in Rename isPasswordlessJetpackConnectionFlow to isWooPasswordlessJPCFlow and Replace isWooCoreProfiler var with isWooPasswordlessJPC and update th…

ilyasfoo · 2024-10-22T02:45:39Z

Thanks, @moon0326! It seems there are test fails and I suspect is related to the PR:

https://teamcity.a8c.com/buildConfiguration/calypso_calypso_WebApp_Run_All_Unit_Tests/13552599?expandBuildDeploymentsSection=false&hideTestsFromDependencies=false&hideProblemsFromDependencies=false&expandBuildProblemsSection=true&expandBuildChangesSection=true&expandBuildTestsSection=true

…ctionFlow

…ckConnectionFlow

…e import path

…sJPC

moon0326 · 2024-10-22T20:19:53Z

Fix broken test and rename isWooCoreProfiler prop to isWooPasswordles…

Thank you for the catch 👍 Fixed it in Fix broken test and rename isWooCoreProfiler prop to isWooPasswordles…

chihsuan

Nice work! Tested well. 👍

Just left a minor suggestion.

chihsuan · 2024-10-23T02:22:26Z

client/lib/login/index.js

@@ -78,6 +78,14 @@ export function getSignupUrl( currentQuery, currentRoute, oauth2Client, locale,
 			includes( get( currentQuery, 'redirect_to' ), '/jetpack/connect/authorize' ) &&
 			includes( get( currentQuery, 'redirect_to' ), '_wp_nonce' )
 		) {
+			// If the current query has plugin_name param, but redirect_to doesn't, add it to the redirect_to
+			const pluginName = get( currentQuery, 'plugin_name' );
+			const urlObj = new URL( currentQuery.redirect_to );


Perhaps, we can add a try-catch block here to handle the case where the redirect_to is not a valid URL. Of current Query.redirect_to is not a valid URL, it will throw an error.

@chihsuan good suggestion! Updated it in Try catch for invalid redirect_to value

ilyasfoo

Hey, @moon0326. I think there's at least 1 more instance of isWooCoreProfilerFlow

moon0326 · 2024-10-23T18:42:03Z

Hey, @moon0326. I think there's at least 1 more instance of isWooCoreProfilerFlow

@ilyasfoo Thank you for the catch. I think that came from rebase. Updated it in Replace isWooCoreProfiler to isWooPasswordlessJPC

chihsuan

LGTM!

ilyasfoo

Thanks, @moon0326! Tests well, LGTM! 🚢

moon0326 requested review from a team, ilyasfoo and chihsuan October 17, 2024 23:42

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 17, 2024

moon0326 requested a review from rjchow October 17, 2024 23:42

chihsuan reviewed Oct 18, 2024

View reviewed changes

ilyasfoo reviewed Oct 18, 2024

View reviewed changes

moon0326 force-pushed the update/refactor-to-add-wcpay-to-passwordless-core-profiler branch from 4195a01 to 5174b4e Compare October 21, 2024 17:34

moon0326 added 9 commits October 22, 2024 13:04

Add shouldUsePasswordlessJetpackConnection

6ffdae8

Add plugin_name param to the login link

2521a3c

Replace isWooCommerceCoreProfilerFlow with isPasswordlessJetpackConne…

44e24ec

…ctionFlow

More - Replace isWooCommerceCoreProfilerFlow with isPasswordlessJetpa…

829af6a

…ckConnectionFlow

Add plugin_name param to the signup link

0b31ee8

Clean up isWooCommercePaymentsOnboardingFlow function

3d8da7b

Rename isPasswordlessJetpackConnectionFlow to isWooPasswordlessJPCFlow

eb349e9

Replace isWooCoreProfiler var with isWooPasswordlessJPC and update th…

d23f60d

…e import path

Fix broken test and rename isWooCoreProfiler prop to isWooPasswordles…

caf7164

…sJPC

moon0326 force-pushed the update/refactor-to-add-wcpay-to-passwordless-core-profiler branch from 5174b4e to caf7164 Compare October 22, 2024 20:07

chihsuan approved these changes Oct 23, 2024

View reviewed changes

ilyasfoo reviewed Oct 23, 2024

View reviewed changes

Replace isWooCoreProfiler to isWooPasswordlessJPC

bf22f64

Try catch for invalid redirect_to value

c580ced

chihsuan approved these changes Oct 24, 2024

View reviewed changes

ilyasfoo approved these changes Oct 24, 2024

View reviewed changes

moon0326 merged commit 2605247 into trunk Oct 24, 2024
11 checks passed

moon0326 deleted the update/refactor-to-add-wcpay-to-passwordless-core-profiler branch October 24, 2024 18:46

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace isWooCoreProfilerFlow with isPasswordlessJetpackConnectionFlow #95504

Replace isWooCoreProfilerFlow with isPasswordlessJetpackConnectionFlow #95504

moon0326 commented Oct 17, 2024 •

edited

Loading

github-actions bot commented Oct 17, 2024 •

edited by matticbot

Loading

matticbot commented Oct 17, 2024 •

edited

Loading

chihsuan left a comment

ilyasfoo left a comment

ilyasfoo Oct 18, 2024

moon0326 Oct 18, 2024

moon0326 commented Oct 18, 2024

moon0326 commented Oct 21, 2024

ilyasfoo commented Oct 22, 2024

moon0326 commented Oct 22, 2024

chihsuan left a comment

chihsuan Oct 23, 2024

moon0326 Oct 23, 2024

ilyasfoo left a comment

moon0326 commented Oct 23, 2024

chihsuan left a comment

ilyasfoo left a comment

Replace isWooCoreProfilerFlow with isPasswordlessJetpackConnectionFlow #95504

Replace isWooCoreProfilerFlow with isPasswordlessJetpackConnectionFlow #95504

Conversation

moon0326 commented Oct 17, 2024 • edited Loading

Proposed Changes

Testing Instructions

Pre-merge Checklist

github-actions bot commented Oct 17, 2024 • edited by matticbot Loading

matticbot commented Oct 17, 2024 • edited Loading

chihsuan left a comment

Choose a reason for hiding this comment

ilyasfoo left a comment

Choose a reason for hiding this comment

ilyasfoo Oct 18, 2024

Choose a reason for hiding this comment

moon0326 Oct 18, 2024

Choose a reason for hiding this comment

moon0326 commented Oct 18, 2024

moon0326 commented Oct 21, 2024

ilyasfoo commented Oct 22, 2024

moon0326 commented Oct 22, 2024

chihsuan left a comment

Choose a reason for hiding this comment

chihsuan Oct 23, 2024

Choose a reason for hiding this comment

moon0326 Oct 23, 2024

Choose a reason for hiding this comment

ilyasfoo left a comment

Choose a reason for hiding this comment

moon0326 commented Oct 23, 2024

chihsuan left a comment

Choose a reason for hiding this comment

ilyasfoo left a comment

Choose a reason for hiding this comment

moon0326 commented Oct 17, 2024 •

edited

Loading

github-actions bot commented Oct 17, 2024 •

edited by matticbot

Loading

matticbot commented Oct 17, 2024 •

edited

Loading