[WooCommerce NUX] Fix incorrect redirection for existing user #87042

chihsuan · 2024-01-31T10:47:47Z

Related to #86884

When a user tries to sign up using Google or Apple, and their email matches an existing account that hasn't been connected yet, the user will be redirected to Woo's login screen and it will display a warning notice.

Figma: 4ixWMlzrxllx93tSFsCW6k-fi-5037_90936

Proposed Changes

Fix incorrect link when redirecting to the login page.
Add woo version existing user warning notice to the login screen.

Testing Instructions

Set up local Woo Start environment https://github.com/woocommerce/woocommerce-start-dev-env/tree/trunk to test social login.
Create a WP.com account with Gmail or disconnect Google from your existing WP.com account Please use non-a8c account.
Log out WP.com
Go to https://woo.com/start/
Continue to the account creation step
Sign up via Google
Observe that you're directed to the Woo's login screen
Observe that it shows You already have a WordPress.com account with this email address. Add your password to log in or create a new account. figma: 4ixWMlzrxllx93tSFsCW6k-fi-5037_90936
Confirm "Create a new account" link is valid and notice is dismissible.

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
https://wpcalypso.wordpress.com/devdocs/docs/testing/index.md 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-ajp-p2)?

github-actions · 2024-01-31T10:48:00Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

matticbot · 2024-01-31T10:52:14Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

blaze-dashboard
editing-toolkit
odyssey-stats
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/woo-social-signup-exist-user-error on your sandbox.

matticbot · 2024-01-31T19:05:25Z

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

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

name                 parsed_size           gzip_size
entry-login              +1508 B  (+0.1%)     +527 B  (+0.1%)
entry-subscriptions       +113 B  (+0.0%)      +61 B  (+0.0%)
entry-stepper              +65 B  (+0.0%)      +31 B  (+0.0%)
entry-main                 +65 B  (+0.0%)      +31 B  (+0.0%)

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

Sections (~1826 bytes added 📈 [gzipped])

name                             parsed_size           gzip_size
jetpack-connect                      +1884 B  (+0.2%)     +618 B  (+0.2%)
accept-invite                         +500 B  (+0.3%)     +142 B  (+0.3%)
stats                                 +292 B  (+0.0%)      +85 B  (+0.0%)
write-flow                             +59 B  (+0.0%)      +32 B  (+0.1%)
videopress-flow                        +59 B  (+0.0%)      +32 B  (+0.0%)
update-design-flow                     +59 B  (+0.0%)      +42 B  (+0.0%)
themes                                 +59 B  (+0.0%)      +40 B  (+0.0%)
theme                                  +59 B  (+0.0%)      +40 B  (+0.0%)
subscribers                            +59 B  (+0.0%)      +33 B  (+0.0%)
sites-dashboard                        +59 B  (+0.0%)      +32 B  (+0.0%)
site-purchases                         +59 B  (+0.0%)      +38 B  (+0.0%)
site-blocks                            +59 B  (+0.0%)      +34 B  (+0.0%)
signup                                 +59 B  (+0.0%)      +35 B  (+0.0%)
settings-writing                       +59 B  (+0.0%)      +47 B  (+0.0%)
settings-security                      +59 B  (+0.0%)      +41 B  (+0.0%)
settings-reading                       +59 B  (+0.0%)      +47 B  (+0.0%)
settings-performance                   +59 B  (+0.0%)      +43 B  (+0.0%)
settings-jetpack                       +59 B  (+0.0%)      +38 B  (+0.0%)
settings-discussion                    +59 B  (+0.0%)      +39 B  (+0.0%)
sensei-flow                            +59 B  (+0.0%)      +39 B  (+0.0%)
security                               +59 B  (+0.0%)      +34 B  (+0.0%)
scan                                   +59 B  (+0.0%)      +28 B  (+0.0%)
purchases                              +59 B  (+0.0%)      +34 B  (+0.0%)
purchase-product                       +59 B  (+0.0%)      +39 B  (+0.1%)
promote-post-i2                        +59 B  (+0.0%)      +44 B  (+0.0%)
privacy                                +59 B  (+0.0%)      +34 B  (+0.0%)
posts-custom                           +59 B  (+0.0%)      +34 B  (+0.0%)
posts                                  +59 B  (+0.0%)      +34 B  (+0.0%)
podcasts-flow                          +59 B  (+0.0%)      +39 B  (+0.0%)
plugins                                +59 B  (+0.0%)      +39 B  (+0.0%)
plans                                  +59 B  (+0.0%)      +39 B  (+0.0%)
people                                 +59 B  (+0.0%)      +42 B  (+0.0%)
pages                                  +59 B  (+0.0%)      +38 B  (+0.0%)
notification-settings                  +59 B  (+0.0%)      +34 B  (+0.0%)
newsletter-post-setup-flow             +59 B  (+0.1%)      +39 B  (+0.1%)
migrate                                +59 B  (+0.0%)      +32 B  (+0.0%)
me                                     +59 B  (+0.0%)      +34 B  (+0.0%)
marketplace                            +59 B  (+0.0%)      +39 B  (+0.0%)
marketing                              +59 B  (+0.0%)      +39 B  (+0.0%)
link-in-bio-tld-flow                   +59 B  (+0.0%)      +32 B  (+0.0%)
link-in-bio-post-setup-flow            +59 B  (+0.1%)      +39 B  (+0.1%)
jetpack-social                         +59 B  (+0.0%)      +39 B  (+0.0%)
jetpack-search                         +59 B  (+0.0%)      +28 B  (+0.0%)
jetpack-cloud-plugin-management        +59 B  (+0.0%)      +39 B  (+0.0%)
jetpack-cloud-partner-portal           +59 B  (+0.0%)      +28 B  (+0.0%)
jetpack-cloud-agency-dashboard         +59 B  (+0.0%)      +28 B  (+0.0%)
jetpack-app                            +59 B  (+0.0%)      +31 B  (+0.0%)
import-hosted-site-flow                +59 B  (+0.0%)      +42 B  (+0.0%)
import-flow                            +59 B  (+0.0%)      +42 B  (+0.0%)
import                                 +59 B  (+0.0%)      +42 B  (+0.0%)
hosting                                +59 B  (+0.0%)      +34 B  (+0.0%)
home                                   +59 B  (+0.0%)      +39 B  (+0.0%)
help                                   +59 B  (+0.0%)      +34 B  (+0.0%)
google-my-business                     +59 B  (+0.0%)      +29 B  (+0.0%)
export                                 +59 B  (+0.0%)      +33 B  (+0.0%)
email                                  +59 B  (+0.0%)      +39 B  (+0.0%)
earn                                   +59 B  (+0.0%)      +32 B  (+0.0%)
domains                                +59 B  (+0.0%)      +24 B  (+0.0%)
domain-connect-authorize               +59 B  (+0.4%)      +42 B  (+0.9%)
developer                              +59 B  (+0.0%)      +34 B  (+0.0%)
copy-site-flow                         +59 B  (+0.0%)      +34 B  (+0.0%)
concierge                              +59 B  (+0.0%)      +34 B  (+0.0%)
comments                               +59 B  (+0.0%)      +34 B  (+0.0%)
checkout                               +59 B  (+0.0%)      +39 B  (+0.0%)
build-flow                             +59 B  (+0.0%)      +32 B  (+0.1%)
backup                                 +59 B  (+0.0%)      +28 B  (+0.0%)
activity                               +59 B  (+0.0%)      +39 B  (+0.0%)
account-close                          +59 B  (+0.0%)      +34 B  (+0.0%)
account                                +59 B  (+0.0%)      +34 B  (+0.0%)
settings-podcast                       +56 B  (+0.0%)      +34 B  (+0.0%)
settings                               +56 B  (+0.0%)      +34 B  (+0.0%)
media                                  +56 B  (+0.0%)      +34 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 (~948 bytes added 📈 [gzipped])

name                                                      parsed_size           gzip_size
async-load-design-blocks                                      +1443 B  (+0.1%)     +501 B  (+0.1%)
async-load-signup-steps-user                                   +441 B  (+0.3%)     +112 B  (+0.3%)
async-load-store-app-store-stats                                +59 B  (+0.0%)      +36 B  (+0.0%)
async-load-design-playground                                    +59 B  (+0.0%)      +39 B  (+0.0%)
async-load-design                                               +59 B  (+0.0%)      +39 B  (+0.0%)
async-load-calypso-my-sites-current-site-notice                 +59 B  (+0.1%)      +30 B  (+0.2%)
async-load-calypso-my-sites-current-site-domain-warnings        +59 B  (+0.2%)      +40 B  (+0.6%)
async-load-calypso-my-sites-checkout-modal                      +59 B  (+0.0%)      +42 B  (+0.0%)
async-load-calypso-lib-account-settings-helper                  +59 B  (+0.0%)      +34 B  (+0.1%)
async-load-calypso-components-global-notices                    +59 B  (+0.8%)      +40 B  (+1.6%)
async-load-calypso-blocks-editor-checkout-modal                 +59 B  (+0.0%)      +40 B  (+0.0%)
async-load-calypso-post-editor-editor-media-modal               +56 B  (+0.0%)      +34 B  (+0.0%)

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.

ilyasfoo

@chihsuan I think this might be related to the PR, where another error looks broken (my account had 2fa)

Before:

After:

There's also this odd looking prompt when the social login failed (I disconnected sandbox so public-api isn't accessible):

Otherwise this was testing well!

chihsuan · 2024-02-01T07:08:57Z

@chihsuan I think this might be related to the PR, where another error looks broken (my account had 2fa)

Before:

After:

There's also this odd looking prompt when the social login failed (I disconnected sandbox so public-api isn't accessible):
Otherwise this was testing well!

Good catch! @ilyasfoo Fixed in 71fd46f 🙏

ilyasfoo

Tested well! I have minor comments but pre-approving

client/blocks/signup-form/index.jsx

ilyasfoo · 2024-02-01T09:55:16Z

client/blocks/signup-form/index.jsx

@@ -233,7 +234,20 @@ class SignupForm extends Component {

 			this.props.createSocialUserFailed( socialInfo, userExistsError, 'signup' );

-			page( login( { redirectTo: this.props.redirectToAfterLoginUrl } ) );


Is there a potential of unintended behaviour since we're no longer exclusively use this.props.redirectToAfterLoginUrl, especially on non-Woo flows?

Good question! I think it's safe. For signup flow, only Woo users will reach this logic.

wp-calypso/client/signup/steps/user/index.jsx

Line 590 in 23d8e11

shouldDisplayUserExistsError={ ! isWooOAuth2Client( oauth2Client ) }

And we still use this.props.redirectToAfterLoginUrl in the getLoginLink.

wp-calypso/client/blocks/signup-form/index.jsx

Lines 491 to 503 in f4c142f

getLoginLink( { emailAddress } = {} ) {

return login( {

emailAddress,

isJetpack: this.isJetpack(),

from: this.getLoginLinkFrom(),

redirectTo: this.props.redirectToAfterLoginUrl,

locale: this.props.locale,

oauth2ClientId: this.props.oauth2Client && this.props.oauth2Client.id,

wccomFrom: this.props.wccomFrom,

isWhiteLogin: this.props.isReskinned,

signupUrl: window.location.pathname + window.location.search,

} );

}

Moreover, I believe that this login() call without any other additional parameters is a bug for all flows. Without passing any additional parameters, users will be redirected to the WP.com login screen and lose context.

Gotcha, thanks for clearing that up!

Moreover, I believe that this login() call without any other additional parameters is a bug for all flows. Without passing any additional parameters, users will be redirected to the WP.com login screen and lose context.

Great point, I wonder if there's folks to tag about this 🤔

moon0326

LGTM and tested well 👍 🚀

…king state

Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>

a8ci18n · 2024-02-05T10:18:07Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/11264016

Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.

Thank you @chihsuan for including a screenshot in the description! This is really helpful for our translators.

a8ci18n · 2024-02-11T16:34:49Z

Translation for this Pull Request has now been finished.

chihsuan self-assigned this Jan 31, 2024

chihsuan requested review from a team, moon0326 and adrianduffell January 31, 2024 11:02

matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 31, 2024

ilyasfoo requested changes Feb 1, 2024

View reviewed changes

chihsuan force-pushed the fix/woo-social-signup-exist-user-error branch from 71fd46f to ee9efac Compare February 1, 2024 07:03

ilyasfoo approved these changes Feb 1, 2024

View reviewed changes

moon0326 approved these changes Feb 1, 2024

View reviewed changes

chihsuan and others added 8 commits February 5, 2024 15:17

Add cancel-social-account-connect-linking action to reset login.isLin…

17c0094

…king state

Fix alert component name

f2dad7d

Fix redirect login link in signup-form for woocommerce

ae8f582

Enable using custom icon

fc60261

Add woo styled existing user error notice

6455925

Update style

c53534d

Fix borken notices

c60e1d2

Update client/blocks/signup-form/index.jsx

d56b81e

Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>

chihsuan force-pushed the fix/woo-social-signup-exist-user-error branch from b1ade37 to d56b81e Compare February 5, 2024 07:19

chihsuan merged commit 99ed591 into trunk Feb 5, 2024
11 checks passed

chihsuan deleted the fix/woo-social-signup-exist-user-error branch February 5, 2024 07:28

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 Feb 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WooCommerce NUX] Fix incorrect redirection for existing user #87042

[WooCommerce NUX] Fix incorrect redirection for existing user #87042

chihsuan commented Jan 31, 2024 •

edited

Loading

github-actions bot commented Jan 31, 2024 •

edited by matticbot

Loading

matticbot commented Jan 31, 2024

matticbot commented Jan 31, 2024 •

edited

Loading

ilyasfoo left a comment •

edited

Loading

chihsuan commented Feb 1, 2024

ilyasfoo left a comment

ilyasfoo Feb 1, 2024

chihsuan Feb 1, 2024 •

edited

Loading

ilyasfoo Feb 1, 2024

moon0326 left a comment

a8ci18n commented Feb 5, 2024

a8ci18n commented Feb 11, 2024

		@@ -233,7 +234,20 @@ class SignupForm extends Component {

		this.props.createSocialUserFailed( socialInfo, userExistsError, 'signup' );

		page( login( { redirectTo: this.props.redirectToAfterLoginUrl } ) );

	getLoginLink( { emailAddress } = {} ) {
	return login( {
	emailAddress,
	isJetpack: this.isJetpack(),
	from: this.getLoginLinkFrom(),
	redirectTo: this.props.redirectToAfterLoginUrl,
	locale: this.props.locale,
	oauth2ClientId: this.props.oauth2Client && this.props.oauth2Client.id,
	wccomFrom: this.props.wccomFrom,
	isWhiteLogin: this.props.isReskinned,
	signupUrl: window.location.pathname + window.location.search,
	} );
	}

[WooCommerce NUX] Fix incorrect redirection for existing user #87042

[WooCommerce NUX] Fix incorrect redirection for existing user #87042

Conversation

chihsuan commented Jan 31, 2024 • edited Loading

Proposed Changes

Testing Instructions

Pre-merge Checklist

github-actions bot commented Jan 31, 2024 • edited by matticbot Loading

matticbot commented Jan 31, 2024

matticbot commented Jan 31, 2024 • edited Loading

ilyasfoo left a comment • edited Loading

Choose a reason for hiding this comment

chihsuan commented Feb 1, 2024

ilyasfoo left a comment

Choose a reason for hiding this comment

ilyasfoo Feb 1, 2024

Choose a reason for hiding this comment

chihsuan Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

ilyasfoo Feb 1, 2024

Choose a reason for hiding this comment

moon0326 left a comment

Choose a reason for hiding this comment

a8ci18n commented Feb 5, 2024

a8ci18n commented Feb 11, 2024

chihsuan commented Jan 31, 2024 •

edited

Loading

github-actions bot commented Jan 31, 2024 •

edited by matticbot

Loading

matticbot commented Jan 31, 2024 •

edited

Loading

ilyasfoo left a comment •

edited

Loading

chihsuan Feb 1, 2024 •

edited

Loading