Guided onboarding: add intent based plans #90809

alshakero · 2024-05-16T17:10:13Z

Related to 7019-gh-Automattic/dotcom-forge

Proposed Changes

Add mapping to show relevant plans based on user segment.

It should follow this table

P2 Segment to Plan mapping: pdDR7T-1xi-p2

Why are these changes being made?

See pdDR7T-1xi-p2

Testing Instructions

Go to /start/guided
The answers to the survey should follow the following tables and the plans should match

Intent (Q1)	Goals (Q2)	Segment	Plans or action
Migrate or import	n/a	Migration	Redirect to import
Create for clients	n/a	Developer / Agency	Creator, Entrepreneur, Enterprise
Create for self	Unknown (question skipped)	Unknown	All plans
Create for self	includes Get a website built quickly	DIFM	Redircet to DIFM
Create for self	Sell something and not Get a website built quickly	Merchant	Creator, Entrepreneur, Enterprise
Create for self	not Get a website built quickly and not Sell something	Consumer / Business	Starter, Explorer, Creator
Create for self	Publish a blog	Blogger	Free, Starter, Explorer, Creator
Create for self	Educational/nonprofit	Nonprofit	Free, Starter, Explorer, Creator

github-actions · 2024-05-16T17:10:25Z

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

Automattic for Agencies live (direct link)

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

matticbot · 2024-05-16T17:21:10Z

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

Sections (~235 bytes added 📈 [gzipped])

name                  parsed_size           gzip_size
update-design-flow         +391 B  (+0.0%)     +106 B  (+0.0%)
link-in-bio-tld-flow       +391 B  (+0.0%)     +106 B  (+0.0%)
plugins                    +253 B  (+0.0%)      +63 B  (+0.0%)
plans                      +253 B  (+0.0%)      +63 B  (+0.0%)
jetpack-app                +253 B  (+0.1%)      +63 B  (+0.1%)
entrepreneur-flow          +141 B  (+0.1%)      +16 B  (+0.1%)
signup                     +138 B  (+0.0%)      +43 B  (+0.1%)
jetpack-connect            +130 B  (+0.0%)      +53 B  (+0.0%)
checkout                   +130 B  (+0.0%)      +62 B  (+0.0%)
accept-invite              +130 B  (+0.1%)      +49 B  (+0.1%)
videopress-flow             +51 B  (+0.0%)      +17 B  (+0.0%)
sensei-flow                 +51 B  (+0.0%)      +17 B  (+0.0%)
podcasts-flow               +51 B  (+0.0%)      +17 B  (+0.0%)
gutenberg-editor            +51 B  (+0.0%)      +17 B  (+0.0%)
domains                     +51 B  (+0.0%)      +17 B  (+0.0%)
copy-site-flow              +51 B  (+0.0%)      +17 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 (~782 bytes added 📈 [gzipped])

name                                             parsed_size           gzip_size
async-load-signup-steps-plans-theme-preselected      +1022 B  (+0.3%)     +338 B  (+0.3%)
async-load-signup-steps-plans                        +1022 B  (+0.3%)     +337 B  (+0.3%)
async-load-signup-steps-initial-intent                +380 B  (+0.4%)     +108 B  (+0.3%)
async-load-calypso-my-sites-checkout-modal            +130 B  (+0.0%)      +62 B  (+0.0%)
async-load-calypso-blocks-editor-checkout-modal        +79 B  (+0.0%)      +45 B  (+0.0%)
async-load-calypso-blocks-app-banner                   +51 B  (+0.0%)      +17 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.

chriskmnds

There is context being introduced into the grid, which i'm not certain it needs to live in the package. Left a comment in the code to get started, but need to take a closer.

chriskmnds · 2024-05-24T06:29:44Z

packages/plans-grid-next/src/hooks/data-store/types.ts

@@ -26,6 +26,7 @@ export interface UseGridPlansParams {
 * Provide a map of plan slug keyed strings to display as the highlight label on top of each plan.
 */
 highlightLabelOverrides?: { [ K in PlanSlug ]?: TranslateResult };
+ segmentationAnswers?: { [ key: string ]: string[] };


Let's clarify what this property is and why it needs to be introduced in the grid. Also the signature is too open ended. Have you considered defining a more concrete type for the values?

chriskmnds · 2024-05-24T09:49:45Z

packages/plans-grid-next/src/hooks/data-store/use-segment-based-plans.ts

This hook needs to go/live outside of plans-grid-next. Part of our goal with this package is to keep it as lean as possible, with context like "answers to a survey" not really being part of anything. Also, any new properties, whether on the grid components or its helpers/hooks to be of generic nature, not context specific (e.g. segmentationAnswers is of the latter).

I think there are a couple of options:

Define a useIntentFromSegmentationAnswers and return a different PlansIntent for every combination of plan types - so you'd be returning plans-guided-default, plans-guided-blogger, plans-guided-consumer, etc. Hopefully, this also serves as a signal to define a better Segment type for the survey work - cause it's all strings right now. You'd introduce those intents in the main switch block of use-grid-plans to define the plan type combinations (as done for everything else - no other logic/change there). The hook would live either near the logic or somewhere else in Calypso e.g. in plans-features-main.

Define a usePlanTypesWithIntentOverride( { intent } ) (the name kinda slips to me now) and pass that into the hooks. In there, you can call this hook and return whatever type combination for the "one" intent defined. We'd somehow work that into the grid hooks as an optional override.

I'll give it some more thought/perspective, but these are options, even if intermediate.

p.s. the difference between multiple intents and single intent doesn't seem to be significant right now. So either option doesn't seem to deviate too far. If in a later scenario, you consider "flow divergence" (so diverging to a different flow on every answer) - which would give you a different "site intent" in the end (I think!) - then the first option is probably better due to mapping across to "plan intent" already.

cc @Automattic/martech-decepticons

What @chriskmnds suggested above should already work pretty well. Fwiw I'd like to throw in another option to consider :)

The general idea of the classic signup framework is that it's a pipeline filling up the required "depedency data" through calls to the submitSignupStep function, e.g. how the domains step does it. That also means a former step can produce data that can be consumed by a latter step to act on accordingly. e.g. again see how the domain step gets the design type data set by a former step so it can pick different TLDs accordingly.

So, if the segmentation survey was a new signup step, one more way to do it is to:

Let it submit a new dependency data entry, e.g. segmentationSurveyAnswers.

In the plans step component, derive the intent in some ways using segmentaionSurveyAnswers in the plansFeaturesList function before feeding it into PlansFeaturesMain: https://github.com/Automattic/wp-calypso/blob/trunk/client/signup/steps/plans/index.jsx#L144.

A slight benefit of doing this way is that the step submission will be tracked by default using calypso_signup_step_submit event, so you won't need to setup additional tracking.

I don't have strong preference on either way; just want to raise another option here in case it helps :)

@southp yep. it sounds like a good way to go (keep it near the step logic and pass it into PlansFeaturesMain via the prop)

The hook would live either near the logic or somewhere else in Calypso e.g. in plans-features-main.

I think what you outlined would precisely cover the "near the logic" part 😁

That's a great idea. I would love to submit the survey answers via the submitSignupStep function in a next PR. I don't love the fact that we have to post and fetch between step 1 and 3.

chriskmnds · 2024-05-24T09:52:29Z

packages/plans-grid-next/src/hooks/data-store/use-segment-based-plans.ts

+ TYPE_PERSONAL,
+ TYPE_PREMIUM,
+} from '@automattic/calypso-products';
+import useGuidedFlowGetSegment from './use-guided-flow-get-segment';


I guess still WIP? This doesn't exist in the package, right❓ (I hope not, we'd need to refactor out now 😬 )

Still WIP yes, the hook now lives in another part https://github.com/Automattic/wp-calypso/pull/91017/files.

@agrullon95, @chriskmnds is right, as our hook to retrieve the segment is not correlated to the plans, but to be used in various places, so we need to find another place for it :)

Maybe packages/onboarding?

Maybe that's fine?

We won't be querying/using that package in plans-grid-next either, so these still need to be pulled in differently here. The package is imported here, but I think it should be safe to remove - it's only for some styles that we may have cleaned up already.

…ypso into add/intent-based-plans

chriskmnds · 2024-05-30T08:10:40Z

client/my-sites/plans-features-main/index.tsx

+ const { segment: intentFromSegmentationSurvey, isLoadingSegment } = useSegmentedIntent(
+ flowName === 'guided',
+ siteId
+ );


I may be missing something but is there anything blocking from moving this call to the consumer of plans-features-main and avoid the unnecessary logic/conditioning in the main hook below? Just pass the intent you want into plans-features-main. That's what the intent prop is meant for.

You can still call it here to get the isLoadiingSegment state if that's meaningful to anything, but I'd rather we avoid intentFromSegmentationSurvey being defined explicitly here. You are overriding the intentFromProps indirectly with this. I would not expect any changes to the useEffect hook below 🤷‍♂️

That's when I started, really. But the parent component is a class component, and doing network requests there sucks. I'd have to make Redux actions, selector, reducer, etc... I can do all of these, but IMHO the trade-off is not worth it. 4 lines vs a bunch of files.

You are overriding the intentFromProps indirectly with this.

That's a great point! I wasn't worried about it because the hook only engages for this flow. But it's still code smell to ignore props.

How about this?

Hey Omar. I haven't had a chance to check the last update. Concerning the class component though, you can write a simple HOC in that case, call the hook and propagate the value in the props. It can all be done in the same file, 3-4 lines wrapping the connected component, no?

^ that's been our strategy for cases in plans-grid that we needed the interplay of hooks and classes, before refactoring everything to function form

I think here you can find examples of a wrapper component or hoc to follow:

wp-calypso/client/my-sites/plan-features-2023-grid/index.tsx

Line 1060 in 0d716cc

const WrappedPlanFeatures2023Grid = ( props: PlanFeatures2023GridType ) => {

heavyweight · 2024-05-30T08:13:34Z

Love how much simpler this got

southp · 2024-05-30T10:21:31Z

client/my-sites/plans-features-main/index.tsx

 const [ intent, setIntent ] = useState< PlansIntent | undefined >( undefined );
+
+ const { segment: intentFromSegmentationSurvey, isLoadingSegment } = useSegmentedIntent(


It looks like we are deriving the intent value according to the segmentation survey here. There are two main drawbacks here:

Once a site is created, the intent value will be stored in the stie meta. e.g. that's how a site created from /setup/newsletter will see the same plan mix at the logged-in /plans. Ideally all the survey data handling should only happen in the signup flow. After that, the site should just use the stored meta data.

An extra GET request to the survey endpoint will be fired everytime <PlansFeaturesMain> is rendered no matter it's needed or not, e.g. an existing account without ever seeing the survey visits /plans.

I have a feeling that it might be something that can be addressed together with @chriskmnds 's comment below since it's likely also about pulling this to the consumer end. If not, it'd be wonderful if we can address as a follow-up :)

Finally I see the light. Thank you so much @southp and @chriskmnds! Pushed.

…answers-query.ts

…rvey-answers-query.test.tsx

alshakero · 2024-05-30T14:28:33Z

Note: don't merge because this introduces a regression. The survey answers are now carried over from the guided flow into the following visits of the plans grid in other flows.

Edit: Fixed.

southp · 2024-05-31T03:15:26Z

client/signup/steps/plans/index.jsx

- const { siteUrl, domainItem, siteTitle, username, coupon } = signupDependencies;
+ const { siteUrl, domainItem, siteTitle, username, coupon, segmentationSurveyAnswers } =
+ signupDependencies;
+


Suggested change

// TBD:

// The reason that we need to narrow down the effect here by checking the flow name is because the signup progress is shared among flows. However, that behavior is problematic here

// It used to cause another issue here: https://github.com/Automattic/wp-calypso/pull/74329

// We need to find a better way to define a signup session with cleaner boundary of shared storage.

Great catch. It's a framework-limitation that will need some more careful thoughts to fix, so this looks like a good workaround for now. I've also suggested to add an inlined TBD note for posterity.

Thank you @southp. We will test this PR for regressions and then probably merge on Monday

Awesome, @escapemanuele . To better track this, I've created a tracking issue for it: #91377. Thanks to the work here, we now have one solid instance to check when we propose a solution to it :)

I think the issue here stems from the solution/approach, not the framework's limitation (which does exist, but it's kinda concequential that it surfaces).

If plans-intent was a property on the flow, not something computed dynamically, then we'd propably not need to bother at all. So at a higher level, instead of deriving intent dynamically, send user to a different flow that has that intent configured. I proposed this earlier as "flow divergence", and seeing that it's only relevant for a couple of intents, then it sounds more tempting now, unless any other underlying concerns.

‐--------

p.s. #91377 brings a bit of a "fix the symptom" vibe to me, but will comment there 🤔

I'm worried about the complexity that will emerge in tracks and the A/B/C experiment we'll run if we split this into 5 flows. I'll merge this, but please don't take it as the end of accepting (and appreciating) feedback!

solid. thanks for the patience, refactors, and eventual enlightenment. not touching plans-grid-next or plans-features-main makes it for a happy ending :-)

Add intent based plans

0f6f41f

Return all if segment is unknown

9f385b1

alshakero added 2 commits May 21, 2024 20:43

Merge branch 'trunk' into add/intent-based-plans

4bbb40d

Merge branch 'trunk' into add/intent-based-plans

35c548b

escapemanuele added this to To do in Signup and Login Improvements via automation May 22, 2024

escapemanuele assigned alshakero May 22, 2024

escapemanuele moved this from To do to In progress in Signup and Login Improvements May 22, 2024

escapemanuele added the Guided Onboarding label May 22, 2024

agrullon95 added 4 commits May 22, 2024 18:10

Merge branch 'trunk' into add/intent-based-plans

3e037f5

Add custom plans page for guided flow

2fb25fa

Add plans-guided step

74b3ddf

Update plan type imports

f646b06

agrullon95 self-assigned this May 23, 2024

agrullon95 added 5 commits May 23, 2024 11:34

Merge branch 'trunk' into add/intent-based-plans

6144488

Add segmentation survey key constant

6692574

Move segmentation functions

3ef1510

Add guided flow segmentation plans logic

51b3f41

Updated survey key usage

cf42c5f

agrullon95 marked this pull request as ready for review May 24, 2024 04:58

agrullon95 requested a review from a team as a code owner May 24, 2024 04:58

agrullon95 requested a review from chriskmnds May 24, 2024 04:58

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

agrullon95 requested a review from a team May 24, 2024 04:58

chriskmnds requested changes May 24, 2024

View reviewed changes

Signup and Login Improvements automation moved this from In progress to Needs review May 24, 2024

chriskmnds reviewed May 24, 2024

View reviewed changes

Merge branch 'trunk' into add/intent-based-plans

6672521

Merge branch 'add/intent-based-plans' of github.com:Automattic/wp-cal…

461e4fe

…ypso into add/intent-based-plans

chriskmnds reviewed May 30, 2024

View reviewed changes

alshakero added 2 commits May 30, 2024 11:41

Give precedence to intentFromProps

d9636d4

Merge branch 'trunk' into add/intent-based-plans

d4675e8

southp reviewed May 30, 2024

View reviewed changes

alshakero added 8 commits May 30, 2024 13:43

Resolve conflict correctly

0518c6e

Move to signup store

e3b995b

Handle nested array in the tracks event

e560256

Cleanup

37c3b52

Discard changes to client/data/segmentaton-survey/queries/use-survey-…

0e772fb

…answers-query.ts

Discard changes to client/data/segmentaton-survey/queries/test/use-su…

8bf1e49

…rvey-answers-query.test.tsx

Discard changes to client/my-sites/plans-features-main/index.tsx

fc5aaa6

Restrict anwers to the current flow

df04775

alshakero added the DO NOT MERGE label May 30, 2024

Only consider segments in guided flow

ba14394

alshakero removed the DO NOT MERGE label May 30, 2024

alshakero and others added 3 commits May 30, 2024 16:55

Merge branch 'trunk' into add/intent-based-plans

8d5fc06

Remove incorrect code comment

27bc7fa

Ensure onSubmit only fires onContinue once

3f7520b

southp reviewed May 31, 2024

View reviewed changes

southp mentioned this pull request Jun 3, 2024

Signup: avoid unintended state sharing between flows #91377

Open

Merge branch 'trunk' into add/intent-based-plans

b3e271c

alshakero merged commit 7e6cbb6 into trunk Jun 3, 2024
11 checks passed

alshakero deleted the add/intent-based-plans branch June 3, 2024 10:41

Signup and Login Improvements automation moved this from Needs review to Done Jun 3, 2024

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 Jun 3, 2024

alshakero changed the title ~~Add intent based plans~~ Guided onboarding: add intent based plans Jun 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Guided onboarding: add intent based plans #90809

Guided onboarding: add intent based plans #90809

alshakero commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024 •

edited by matticbot

matticbot commented May 16, 2024 •

edited

chriskmnds left a comment

chriskmnds May 24, 2024

chriskmnds May 24, 2024 •

edited

chriskmnds May 24, 2024 •

edited

chriskmnds May 24, 2024

southp May 28, 2024

chriskmnds May 28, 2024

alshakero May 28, 2024

chriskmnds May 24, 2024

escapemanuele May 24, 2024

escapemanuele May 24, 2024

chriskmnds May 24, 2024

chriskmnds May 30, 2024 •

edited

alshakero May 30, 2024

chriskmnds May 30, 2024

chriskmnds May 30, 2024 •

edited

heavyweight commented May 30, 2024

southp May 30, 2024

alshakero May 30, 2024

alshakero commented May 30, 2024 •

edited

southp May 31, 2024

escapemanuele May 31, 2024

southp Jun 3, 2024

chriskmnds Jun 3, 2024

alshakero Jun 3, 2024

		const [ intent, setIntent ] = useState< PlansIntent \| undefined >( undefined );

		const { segment: intentFromSegmentationSurvey, isLoadingSegment } = useSegmentedIntent(

+// TBD:
+// The reason that we need to narrow down the effect here by checking the flow name is because the signup progress is shared among flows. However, that behavior is problematic here
+// It used to cause another issue here: https://github.com/Automattic/wp-calypso/pull/74329
+// We need to find a better way to define a signup session with cleaner boundary of shared storage.

Guided onboarding: add intent based plans #90809

Guided onboarding: add intent based plans #90809

Conversation

alshakero commented May 16, 2024 • edited

Proposed Changes

Why are these changes being made?

Testing Instructions

github-actions bot commented May 16, 2024 • edited by matticbot

matticbot commented May 16, 2024 • edited

chriskmnds left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriskmnds May 24, 2024 • edited

Choose a reason for hiding this comment

chriskmnds May 24, 2024 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriskmnds May 30, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriskmnds May 30, 2024 • edited

Choose a reason for hiding this comment

heavyweight commented May 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alshakero commented May 30, 2024 • edited

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

alshakero commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024 •

edited by matticbot

matticbot commented May 16, 2024 •

edited

chriskmnds May 24, 2024 •

edited

chriskmnds May 24, 2024 •

edited

chriskmnds May 30, 2024 •

edited

chriskmnds May 30, 2024 •

edited

alshakero commented May 30, 2024 •

edited