Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limits steps in onboarding based on previously completed state #2568

Draft
wants to merge 7 commits into
base: feature/2458-streamline-onboarding
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions js/src/setup-mc/setup-stepper/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@ const SetupStepper = () => {
}

const { status } = mcSetup;
let { step } = mcSetup;
const { step } = mcSetup;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be combined:

const { status, step } = mcSetup;


// If the user has already completed the store requirements, but is currently still on the
// store requirements step, we should skip the store requirements step and go to the paid ads step.
// else they will get stuck on a non-existent step #3
if ( step === 'store_requirements' && hasConfirmedStoreRequirements ) {
step = 'paid_ads';
}
const currentStep =
step === 'store_requirements' && hasConfirmedStoreRequirements
? 'paid_ads'
: step;

if ( status === 'complete' ) {
getHistory().replace( getNewPath( {}, '/google/dashboard' ) );
Expand All @@ -59,7 +60,7 @@ const SetupStepper = () => {

return (
<SavedSetupStepper
savedStep={ stepNameKeyMap[ step ] }
savedStep={ stepNameKeyMap[ currentStep ] }
hasConfirmedStoreRequirements={ hasConfirmedStoreRequirements }
/>
);
Expand Down
82 changes: 82 additions & 0 deletions tests/e2e/specs/setup-mc/step-3-hide-store-requirements.test.js
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than creating a whole new spec here, I think we should test for these scenarios in either the existing tests/e2e/specs/setup-mc/step-3-confirm-store-requirements.test.js spec, or test for this scenario in tests/e2e/specs/setup-mc/step-2-product-listings.test.js whenever the continue button is clicked with the address info already correctly mocked. I think that should look something like this:

productListingsPage.mockContactInformation( {
	phone_number: '+15555555',
	phone_verification_status: 'verified',
	mc_address: {
		street_address: '556 Woo St.',
		locality: 'City',
		region: 'California',
		postal_code: '90210',
		country: 'US',
	},
	wc_address: {
		street_address: '556 Woo St.',
		locality: 'City',
		region: 'California',
		postal_code: '90210',
		country: 'US',
	},
	is_mc_address_different: false,
	wc_address_errors: [],
} );

One tricky part here is that the step is only removed when the stepper is first loaded, so we'll need a beforeAll step that reloads the page after the state is correctly mocked.

I'm going to take a look tomorrow to see if I can get the state for this set up properly, since it's a bit tricky.

Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/**
* Internal dependencies
*/
import SetUpAccountsPage from '../../utils/pages/setup-mc/step-1-set-up-accounts';

/**
* External dependencies
*/
const { test, expect } = require( '@playwright/test' );

test.use( { storageState: process.env.ADMINSTATE } );

test.describe.configure( { mode: 'serial' } );

/**
* @type {import('../../utils/pages/setup-mc/step-1-set-up-accounts.js').default} setUpAccountsPage
*/
let setUpAccountsPage = null;

/**
* @type {import('@playwright/test').Page} page
*/
let page = null;

test.describe( 'Hide Store Requirements Step', () => {
test.beforeAll( async ( { browser } ) => {
page = await browser.newPage();
setUpAccountsPage = new SetUpAccountsPage( page );
await Promise.all( [
// Mock google as connected.
setUpAccountsPage.mockGoogleNotConnected(),

// Mock MC contact information
setUpAccountsPage.mockContactInformation(),
] );
} );

test.afterAll( async () => {
await setUpAccountsPage.closePage();
} );

test( 'should have store requirements step if incomplete', async () => {
await setUpAccountsPage.goto();

// Mock MC step at step 1:
setUpAccountsPage.mockMCSetup( 'incomplete', 'accounts' );

// 1. Assert there are 3 steps
const steps = await page.locator( '.woocommerce-stepper__step' );
await expect( steps ).toHaveCount( 4 );

// 2. Assert the label of the 3rd step is 'Confirm store requirements'
const thirdStepLabel = await steps
.nth( 2 )
.locator( '.woocommerce-stepper__step-label' );
await expect( thirdStepLabel ).toHaveText(
'Confirm store requirements'
);
} );

test( 'should not have store requirements step if complete', async () => {
await setUpAccountsPage.goto();

// TODO: Mock email is verified & address is filled
setUpAccountsPage.mockMCSetup( 'complete', 'accounts' );

// 1. Assert there are 3 steps
const steps = await page.locator( '.woocommerce-stepper__step' );
await expect( steps ).toHaveCount( 3 );

// 2. Assert the label of the 3rd step is not 'Confirm store requirements'
const thirdStepLabel = await steps
.nth( 2 )
.locator( '.woocommerce-stepper__step-label' );
await expect( thirdStepLabel ).not.toHaveText(
'Confirm store requirements'
);

// 3. Assert the label of the 3rd step equals 'Create a campaign'
await expect( thirdStepLabel ).toHaveText( 'Create a campaign' );
} );
} );
Loading