Wp-calypso: Signup: broken transition from business to ecommerce flow

[jsnajdr]

Steps to reproduce:

1. Go to /start/business
2. Enter "Sell" in the site goal box in the about step
3. Submit the step

You are redirected to /start/ecommerce/domains and that renders an empty page.

Some analysis what happens:
The <Signup> component renders nothing when it thinks that a "flow resume" (abandoning the signup and coming back to the start later, we proceed to the first incomplete step) is in progress. The condition is:

if ( this.getPositionInFlow() > 0 && this.props.progress.length === 0 ) {
  return null;
}

In our case, the position in flow is indeed >0, and the progress is empty for some reason.

The progress is empty because the removeUnneededSteps reducer is called when changing flow name, and it's very picky about the steps it keeps: the steps must have the same name, but also the same index in the array!

The progress array from the business flow contains the data for the about step, but it's not at the expected index. There is plans-business step before it, which was auto-fulfilled by the removeFulfilledSteps code.

Therefore, the filtered progress misses the about step (which is probably unwanted) and is empty. Then it triggers the "render nothing" condition.

Cc @niranjan-uma-shankar as he's the most recent committer of almost all the involved code.

[niranjan-uma-shankar]

and it's very picky about the steps it keeps: the steps must have the same name, but also the same index in the array!

Dropping some context on this.
It needs to be picky to avoid back button issues. When we switch flows, some of the steps in the progress could appear later in the new flow, so back button gets confused.

Example:

1. In the Onboarding flow, chose the Store site type.
2. In the domain step, select a domain and proceed to the next step.
3. Now you will be in the Plans step. Click the back button all the way back to the site type step.
4. Now choose Blog as the site type, and proceed all the way to the Plans step.

At this point, this is how the progress store will look:

The above progress store is correct, and clicking the Back button repeatedly will take you back each step in the right order.

Now, say we were to drop the index check in flowSteps.includes( step.stepName ) && index === flowSteps.indexOf( step.stepName ) and instead just made it flowSteps.includes( step.stepName ), and then ran through steps 1-4 above, this is how the progress store will then look like:

So, with the index check dropped, when at the Plans step you click the Back button, you will end up in the Site Type step, since in the progress store, the plans step is at position 3. Here's how it'll behave:

Also, please note that if the index check is dropped, this bug can manifest on any step that shares the same name across flows. For example, you can confuse the Back button at the domain step too, if both the ecommerce-onboarding flow and onboarding flow had a step name domain (at this moment, the onboarding flow has changed the name of the domain step to domain-with-preview so that's why we don't see it. Try renaming that domain and run through the steps above).

I haven't checked in detail about the bug listed in the report, but just wanted to add context on why the index check is needed, if it helps.

[jsnajdr]

Hi @niranjan-uma-shankar :wave: Thanks for the detailed explanation. In the two examples of a progress store you mention, both stores contain the same and correct data (even the lastKnownFlow is the same) and only the order of steps is different, is that right?

What if we didn't use the progress store to implement the back button, and instead just called window.history.back()? Would that work or is there some gotcha?

[niranjan-uma-shankar]

That's correct, it's only the order of steps that is different, the rest are the same.

On using browser history for the back button, one gotcha I can think of is when you navigate to domain mapping/transfer view. Check this recording. Between 00:00 and 00:10, I'm pressing the browser back button, and you can see the Domain Mapping/Transfer screen show up( because I had entered that view earlier). This is not preferable since we only need to traverse back through the flow steps. Between 00:11 and 00:22 I make a few actions, and again pressed the browser back button between 00:23 and 00:30, and it ends up showing some of the screens multiple times.

Another issue that could come up is that the new site creation flow is designed to be picked up from where it was left off. Check this recording - here, I'm dropping off from the site style step and starting the Add New Site flow again. I'm directly taken to the site style step. So, the Back button as it is implemented now works well to go back to previous steps, however if we use the browser back, then we will end up going to the previous screen (which is not the previous step in the flow).

Disclaimer - I am not aware if the reasons above were specifically the reasons why the Back button was implemented using the progress store, these are just what I could think of top of my mind.

[niranjan-uma-shankar]

This issue manifested in /personal, /premium, /business flows.

Issue:

Starting at the above mentioned flows, choosing the blog site type switches the flow to onboarding-blog flow. The next step in the flow results in a blank page.

Cause:

Starting at /personal, the removeFulfilledSteps function in main.jsx, puts the plans-personal step at the top of the progress store. Thus at the site type step, the progress store looks like this:

Selecting the blog site type here switches to the onboarding-blog flow which has the following steps:

'user', 'site-type', 'site-topic', 'site-title', 'domains', 'plans'

Thus, removeUnneededSteps will remove all steps from the progress.

A temporary fix has been pushed in https://github.com/Automattic/wp-calypso/pull/34173.

[adamziel]

Related PR from @spen #34211

[adamziel]

Just linking back a related PR from Andrew (temporary one, we'll revert once a proper solution lands) https://github.com/Automattic/wp-calypso/pull/35047

[michaeldcain]

Based on the frequency the event from #35093 was occurring, I removed the index check from the progress store in #35109. It was tested pretty thoroughly by myself and others, and we couldn't find the back button issues mentioned above.

That being said, the event is still firing at some interval and needs further investigation. Also, removing the index check introduces an issue when we return to the pre-selected plans removed in #35047 by @andrewserong. If a user visit a pre-selected plan flow, forks to ecommerce-onboarding, and then returns to a different segment flow, the plan is not properly added to the cart.

I'm marking this dependency issue as a high priority for Zelda next week. @adamziel @ramonjd

[michaeldcain]

The event in #35093 was firing every time someone completed Signup. In #35112, I added a check against the old flow name to limit it to only fire when the flow has been forked.

[ramonjd]

Fixed in https://github.com/Automattic/wp-calypso/pull/35184