Stepper: calypso_signup_complete as in Start #94706

renancarvalho · 2024-09-19T10:04:37Z

Fixes #94688

Proposed Changes

Use the same logic as in start for isNewUser and isNew7DUserSite values, these values are used to properly log the calypso_signup_complete tracking event.

Why are these changes being made?

With the new domains step, this was not yet fully implemented. This PR adds it by mimicking the /start logic.

Testing Instructions

Mock wordpress
Signup with social account and with an email
Go after the the plans step and make sure that calypso_signup_complete gets triggered correctly, with the blog_id
Make sure that isNewUser is set to true
Make sure that isNewUser is not true if you signup with an existing user

github-actions · 2024-09-19T10:04:54Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

Automattic for Agencies live (direct link)

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

matticbot · 2024-09-19T10:05:37Z

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

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

name                 parsed_size           gzip_size
entry-subscriptions       +149 B  (+0.0%)      +57 B  (+0.0%)
entry-stepper             +149 B  (+0.0%)      +42 B  (+0.0%)

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

Sections (~1241 bytes added 📈 [gzipped])

name                           parsed_size           gzip_size
write-flow                          +487 B  (+0.1%)     +177 B  (+0.1%)
videopress-tv-flow                  +487 B  (+0.8%)     +199 B  (+0.9%)
videopress-flow                     +487 B  (+0.0%)     +190 B  (+0.1%)
transferring-hosted-site-flow       +487 B  (+0.5%)     +199 B  (+0.7%)
sensei-flow                         +487 B  (+0.1%)     +199 B  (+0.1%)
plugin-bundle-flow                  +487 B  (+0.3%)     +199 B  (+0.3%)
link-in-bio-tld-flow                +487 B  (+0.0%)     +165 B  (+0.0%)
import-hosted-site-flow             +487 B  (+0.1%)     +170 B  (+0.1%)
copy-site-flow                      +487 B  (+0.1%)     +199 B  (+0.1%)
build-flow                          +487 B  (+0.1%)     +234 B  (+0.1%)
subscribers                         +149 B  (+0.0%)      +42 B  (+0.0%)
staging-site                        +149 B  (+0.0%)      +42 B  (+0.0%)
sites-dashboard                     +149 B  (+0.0%)      +42 B  (+0.0%)
site-performance                    +149 B  (+0.0%)      +42 B  (+0.0%)
site-monitoring                     +149 B  (+0.0%)      +42 B  (+0.0%)
site-logs                           +149 B  (+0.0%)      +42 B  (+0.0%)
settings                            +149 B  (+0.0%)      +42 B  (+0.0%)
reader                              +149 B  (+0.0%)      +57 B  (+0.0%)
hosting-features                    +149 B  (+0.0%)      +42 B  (+0.0%)
hosting                             +149 B  (+0.0%)      +42 B  (+0.0%)
home                                +149 B  (+0.0%)      +42 B  (+0.0%)
github-deployments                  +149 B  (+0.0%)      +42 B  (+0.0%)
checkout                            +149 B  (+0.0%)      +42 B  (+0.0%)
jetpack-connect                      +50 B  (+0.0%)       +7 B  (+0.0%)
accept-invite                        +50 B  (+0.0%)       +7 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 (~7 bytes added 📈 [gzipped])

name                                     parsed_size           gzip_size
async-load-signup-steps-user                   +50 B  (+0.0%)       +7 B  (+0.0%)
async-load-signup-steps-subscribe-email        +50 B  (+0.0%)       +7 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.

renancarvalho · 2024-09-19T11:41:07Z

client/landing/stepper/hooks/use-record-signup-complete.ts

@@ -18,16 +23,16 @@ export const useRecordSignupComplete = ( flow: string | null ) => {
 			selectedDomain: ( select( ONBOARD_STORE ) as OnboardSelect ).getSelectedDomain(),
 		};
 	}, [] );
+	const isNewishUser = isUserRegistrationDaysWithinRange( state, null, 0, 7 );
+	const dependencies = getSignupDependencyStore( state );


Note for myself:
I need to figure why getSignupDependencyStore always returns an {}

matticbot · 2024-09-19T14:05:49Z

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

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

notifications
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/domain_tracking on your sandbox.

chriskmnds · 2024-09-19T16:36:18Z

client/landing/stepper/hooks/use-record-signup-complete.ts

@@ -3,68 +3,81 @@ import { useSelect } from '@wordpress/data';
 import { useCallback } from 'react';
 import { USER_STORE, ONBOARD_STORE } from 'calypso/landing/stepper/stores';
 import { SIGNUP_DOMAIN_ORIGIN, recordSignupComplete } from 'calypso/lib/analytics/signup';
+import { useSelector } from 'calypso/state';
+import isUserRegistrationDaysWithinRange from 'calypso/state/selectors/is-user-registration-days-within-range';
 import { useSite } from './use-site';
 import type { UserSelect, OnboardSelect } from '@automattic/data-stores';

 export const useRecordSignupComplete = ( flow: string | null ) => {
 	const site = useSite();


@escapemanuele is this where the issue stems? It looks like it grabs site ID/slug from URL params, which would obviously be undefined unless carried along?

Isn't there a store/state slice that gets populated with site ID when one is created? We should probably look at create-site step and do that if not?

What I ended up doing is passing the data from the processing step directly when calling the useRecordSignupComplete function here. At that point we have all the data needed. Do you think that should be changed?

Cool. Sounds good (you obviously know more about these concepts)

Do you think that should be changed?

I'm not sure. It might not be a bad idea to populate the onboard store with it once a site is created (or pending). It would make it available in anything that follows and needs it.

I will comment up above on something related if you can also take a look pls

renancarvalho · 2024-09-19T19:31:39Z

...nding/stepper/declarative-flow/internals/steps-repository/__user/use-create-user-mutation.ts

@@ -21,6 +24,9 @@ export function useCreateAccountMutation() {
 			} );
 		},
 		onSuccess: ( data ) => {
+			if ( 'isNewAccountCreated' in data && data.isNewAccountCreated ) {
+				setIsNewUser( true );


One thing that, I think, would be nice to make sure it is clear is that the user will be "newUser" just once. For instance, if you create the user, then land on the domain's step and refresh the page, the user is not new anymore.

Well, we could persist that, but I'm always worried about persistence bugs 🤔

Well, we could persist that, but I'm always worried about persistence bugs 🤔

I think we could have a better definition of what is a "newUser"

I replicated what is done during user creation here https://github.com/Automattic/wp-calypso/blob/trunk/client/lib/signup/api/account.tsx#L120 😄

agrullon95

Some differences I noticed /setup vs /start

/setup includes a few extra props: starting_point, theme, intent
I went through /setup with an existing user and the elapsed_time_since_start is logged as null.

escapemanuele · 2024-09-20T08:23:30Z

Thank you @agrullon95!

I don't think that more props are a bad thing
Tested it a little and elapsed_time_since_start is null because recordSignupStart is called only the first time (and it sets the session storage with the starting time). You probably tested while already logged in and so the event is not triggered. Stepper: Record calypso_signup_start unconditionally => multiple logs #94743 should solve it (trigger the events on every refresh)

chriskmnds · 2024-09-20T08:43:20Z

Tested it a little and elapsed_time_since_start is null because recordSignupStart is called only the first time (and it sets the session storage with the starting time). You probably tested while already logged in and so the event is not triggered. Stepper: Record calypso_signup_start unconditionally => multiple logs #94743 should solve it (trigger the events on every refresh)

Just to note here that with #94743 if a flow uses something like:

useEventProps() {
  const { data } = useSiteIntent( 123 );
  return { 
    calypso_signup_start: {
      site_intent: data?.site_intent,
    } ,
  };
}

(or through the current useSignupStartEventProps - which is getting refactored in another PR) then elapsed_time_since_start will get reset when recordSignupStart gets triggered 😬

I think there are two solutions (outside of ignoring useEventProps from dependencies in useSignUpStartTracking):

Ignore/don't care - so let the flow be silly about it
Rework the recordSignupStart function to check if a previous value exists, so to not reset it if it exists.

I think it's not a blocker for the experiment as we don't use extraProps or flowVariant for onboarding flow. I can create an issue to follow up on this, if we proceed with #94743

escapemanuele · 2024-09-20T08:59:39Z

Good catch @chriskmnds, I believe we can open an issue to keep that into consideration!

chriskmnds · 2024-09-20T12:37:20Z

client/landing/stepper/declarative-flow/internals/steps-repository/processing-step/index.tsx

@@ -104,7 +104,7 @@ const ProcessingStep: React.FC< ProcessingStepProps > = function ( props ) {
 			if ( availableFlows[ flow ] ) {
 				availableFlows[ flow ]().then( ( flowExport ) => {
 					if ( flowExport.default.isSignupFlow ) {
-						recordSignupComplete();
+						recordSignupComplete( { ...destinationState } );


Just roughly thinking. I get the impression that a specific step should not control and record the signup-complete event. I would expect this to be triggered via a flow unmount effect (where we equally observe for some "success" flag) or to actually mark a step as the "signup completion" step (so any step can be used for closing). WDYT?

I thought the same as well! But having found the recordSignupComplete I doubted that maybe there was some big decision behind this while I was not involved in Stepper work.

Moreover, useSignUpStartTracking is in FlowRenderer. Logically, it makes sense to have the signup complete at a framework, and not step, level too. But I wonder if that's overcomplicated. It would be better to open an issue.

Yeah, definitely something for a follow-up issue. It does make sense to have this near/in useSignUpStartTracking. It will probably lead to more holistic refactor, but we want these things to be perfect! :D

chriskmnds · 2024-09-20T12:56:58Z

Good catch @chriskmnds, I believe we can open an issue to keep that into consideration!

Cool. Opened #94767

* Use same tracking values as in start * Pass destinationState to signup completion tracking * Add newUser call * Register in the store when it is a new account * Better check --------- Co-authored-by: escapemanuele <escapemanuele@gmail.com>

renancarvalho self-assigned this Sep 19, 2024

renancarvalho commented Sep 19, 2024

View reviewed changes

Use same tracking values as in start

fabcdf2

escapemanuele force-pushed the fix/domain_tracking branch from a0974f7 to fabcdf2 Compare September 19, 2024 13:41

escapemanuele added 2 commits September 19, 2024 16:26

Pass destinationState to signup completion tracking

54f2da0

Add newUser call

cfddcb9

escapemanuele marked this pull request as ready for review September 19, 2024 14:41

escapemanuele requested a review from a team September 19, 2024 14: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 Sep 19, 2024

escapemanuele requested a review from chriskmnds September 19, 2024 14:42

escapemanuele added the Stepper label Sep 19, 2024

chriskmnds reviewed Sep 19, 2024

View reviewed changes

escapemanuele added 2 commits September 19, 2024 21:01

Register in the store when it is a new account

0a37653

Better check

a43fab1

renancarvalho commented Sep 19, 2024

View reviewed changes

agrullon95 requested review from agrullon95 September 19, 2024 21:28

agrullon95 requested changes Sep 19, 2024

View reviewed changes

escapemanuele changed the title ~~Stepper Tracking: Use same tracking values as in start~~ Stepper: calypso_signup_complete as in Start Sep 20, 2024

escapemanuele mentioned this pull request Sep 20, 2024

Stepper: Record calypso_signup_start unconditionally => multiple logs #94743

Merged

7 tasks

escapemanuele self-assigned this Sep 20, 2024

escapemanuele requested a review from chriskmnds September 20, 2024 08:29

escapemanuele merged commit 525322f into trunk Sep 20, 2024
18 of 19 checks passed

escapemanuele deleted the fix/domain_tracking branch September 20, 2024 09:13

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 Sep 20, 2024

chriskmnds reviewed Sep 20, 2024

View reviewed changes

chriskmnds mentioned this pull request Sep 20, 2024

Stepper: Localise recordSignupStart and ensure timers don't get restarted on rerenders #94767

Closed

chriskmnds mentioned this pull request Oct 1, 2024

Stepper: calypso_signup_complete event not firing in es onboarding flow #95047

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stepper: calypso_signup_complete as in Start #94706

Stepper: calypso_signup_complete as in Start #94706

renancarvalho commented Sep 19, 2024 •

edited by escapemanuele

Loading

github-actions bot commented Sep 19, 2024 •

edited by matticbot

Loading

matticbot commented Sep 19, 2024 •

edited

Loading

renancarvalho Sep 19, 2024 •

edited

Loading

matticbot commented Sep 19, 2024 •

edited

Loading

chriskmnds Sep 19, 2024

escapemanuele Sep 20, 2024

chriskmnds Sep 20, 2024

renancarvalho Sep 19, 2024

escapemanuele Sep 20, 2024

renancarvalho Sep 20, 2024

escapemanuele Sep 20, 2024

agrullon95 left a comment

escapemanuele commented Sep 20, 2024

chriskmnds commented Sep 20, 2024 •

edited

Loading

escapemanuele commented Sep 20, 2024

chriskmnds Sep 20, 2024

escapemanuele Sep 20, 2024

chriskmnds Sep 20, 2024

chriskmnds commented Sep 20, 2024

Stepper: calypso_signup_complete as in Start #94706

Stepper: calypso_signup_complete as in Start #94706

Conversation

renancarvalho commented Sep 19, 2024 • edited by escapemanuele Loading

Proposed Changes

Why are these changes being made?

Testing Instructions

github-actions bot commented Sep 19, 2024 • edited by matticbot Loading

matticbot commented Sep 19, 2024 • edited Loading

renancarvalho Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

matticbot commented Sep 19, 2024 • edited Loading

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

Choose a reason for hiding this comment

agrullon95 left a comment

Choose a reason for hiding this comment

escapemanuele commented Sep 20, 2024

chriskmnds commented Sep 20, 2024 • edited Loading

escapemanuele commented Sep 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriskmnds commented Sep 20, 2024

renancarvalho commented Sep 19, 2024 •

edited by escapemanuele

Loading

github-actions bot commented Sep 19, 2024 •

edited by matticbot

Loading

matticbot commented Sep 19, 2024 •

edited

Loading

renancarvalho Sep 19, 2024 •

edited

Loading

matticbot commented Sep 19, 2024 •

edited

Loading

chriskmnds commented Sep 20, 2024 •

edited

Loading