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 6 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
42 changes: 37 additions & 5 deletions js/src/setup-mc/setup-stepper/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,61 @@ import { getHistory, getNewPath } from '@woocommerce/navigation';
import AppSpinner from '.~/components/app-spinner';
import SavedSetupStepper from './saved-setup-stepper';
import useMCSetup from '.~/hooks/useMCSetup';
import useStoreAddress from '.~/hooks/useStoreAddress';
import useGoogleMCPhoneNumber from '.~/hooks/useGoogleMCPhoneNumber';
import stepNameKeyMap from './stepNameKeyMap';

const SetupStepper = () => {
const { hasFinishedResolution, data: mcSetup } = useMCSetup();
const { data: address, loaded: addressLoaded } = useStoreAddress();
const { data: phone, loaded: phoneLoaded } = useGoogleMCPhoneNumber();

if ( ! hasFinishedResolution && ! mcSetup ) {
return <AppSpinner />;
}
const hasValidPhoneNumber =
phoneLoaded && phone?.isValid && phone?.isVerified;

const hasValidAddress =
addressLoaded &&
address?.isAddressFilled &&
! address?.isMCAddressDifferent;

const hasConfirmedStoreRequirements =
hasValidPhoneNumber && hasValidAddress;

const hasLoaded =
hasFinishedResolution && mcSetup && addressLoaded && phoneLoaded;

if ( hasFinishedResolution && ! mcSetup ) {
// this means error occurred, we just need to return null here,
// wp-data actions will display an error snackbar at the bottom of the page.
return null;
}

const { status, step } = mcSetup;
if ( ! hasLoaded ) {
return <AppSpinner />;
}

const { status } = mcSetup;
const { 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
const currentStep =
step === 'store_requirements' && hasConfirmedStoreRequirements
? 'paid_ads'
: step;

if ( status === 'complete' ) {
getHistory().replace( getNewPath( {}, '/google/dashboard' ) );
return null;
}

return <SavedSetupStepper savedStep={ stepNameKeyMap[ step ] } />;
return (
<SavedSetupStepper
savedStep={ stepNameKeyMap[ currentStep ] }
hasConfirmedStoreRequirements={ hasConfirmedStoreRequirements }
/>
);
};

export default SetupStepper;
44 changes: 29 additions & 15 deletions js/src/setup-mc/setup-stepper/saved-setup-stepper.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,14 @@
/**
* @param {Object} props React props
* @param {string} [props.savedStep] A saved step overriding the current step
* @param {boolean} [props.hasConfirmedStoreRequirements] Whether the store requirements have been confirmed
* @fires gla_setup_mc with `{ triggered_by: 'step1-continue-button' | 'step2-continue-button', 'step3-continue-button', action: 'go-to-step2' | 'go-to-step3' | 'go-to-step4' }`.
* @fires gla_setup_mc with `{ triggered_by: 'stepper-step1-button' | 'stepper-step2-button' | 'stepper-step3-button', action: 'go-to-step1' | 'go-to-step2' | 'go-to-step3' }`.
*/
const SavedSetupStepper = ( { savedStep } ) => {
const SavedSetupStepper = ( {
savedStep,
hasConfirmedStoreRequirements = false,
} ) => {
const [ step, setStep ] = useState( savedStep );

const { settings } = useSettings();
Expand Down Expand Up @@ -102,7 +106,11 @@
};

const handleSetupListingsContinue = () => {
continueStep( stepNameKeyMap.store_requirements );
if ( hasConfirmedStoreRequirements ) {
continueStep( stepNameKeyMap.paid_ads );

Check warning on line 110 in js/src/setup-mc/setup-stepper/saved-setup-stepper.js

View check run for this annotation

Codecov / codecov/patch

js/src/setup-mc/setup-stepper/saved-setup-stepper.js#L110

Added line #L110 was not covered by tests
} else {
continueStep( stepNameKeyMap.store_requirements );
}
};

const handleStoreRequirementsContinue = () => {
Expand Down Expand Up @@ -209,19 +217,25 @@
),
onClick: handleStepClick,
},
{
key: stepNameKeyMap.store_requirements,
label: __(
'Confirm store requirements',
'google-listings-and-ads'
),
content: (
<StoreRequirements
onContinue={ handleStoreRequirementsContinue }
/>
),
onClick: handleStepClick,
},
...( ! hasConfirmedStoreRequirements
? [
{
key: stepNameKeyMap.store_requirements,
label: __(
'Confirm store requirements',
'google-listings-and-ads'
),
content: (
<StoreRequirements
onContinue={
handleStoreRequirementsContinue
}
/>
),
onClick: handleStepClick,
},
]
: [] ),

Check warning on line 238 in js/src/setup-mc/setup-stepper/saved-setup-stepper.js

View check run for this annotation

Codecov / codecov/patch

js/src/setup-mc/setup-stepper/saved-setup-stepper.js#L238

Added line #L238 was not covered by tests
{
key: stepNameKeyMap.paid_ads,
label: __( 'Create a campaign', 'google-listings-and-ads' ),
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' );
} );
} );