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.

Sign up for GitHub

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

Enable Focused Launchpad for all onboarding users #100526

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
+11 −223
Open

Enable Focused Launchpad for all onboarding users #100526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7645b7a
Enable Focused Launchpad to all onboarding users
paulopmt1 Feb 26, 2025
abb131a
remove test that no longer applies
vykes-mac Feb 27, 2025
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
9 changes: 3 additions & 6 deletions client/landing/stepper/declarative-flow/internals/steps-repository/launchpad/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { urlToSlug } from 'calypso/lib/url';
import { useSelector, useDispatch } from 'calypso/state';
import { isUserLoggedIn } from 'calypso/state/current-user/selectors';
import { successNotice } from 'calypso/state/notices/actions';
import { useShouldShowLaunchpadFirst } from 'calypso/state/selectors/should-show-launchpad-first';
import { shouldShowLaunchpadFirst } from 'calypso/state/selectors/should-show-launchpad-first';
import { useQuery } from '../../../../hooks/use-query';
import StepContent from './step-content';
import { areLaunchpadTasksCompleted } from './task-helper';
Expand Down Expand Up @@ -92,11 +92,8 @@ const Launchpad: Step = ( { navigation, flow }: LaunchpadProps ) => {
}
}, [ verifiedParam, translate, dispatch ] );

const [ loadingShouldShowLaunchpadFirst, shouldShowLaunchpadFirst ] =
useShouldShowLaunchpadFirst( site );

// Avoid screen flickering when redirecting to other paths
if ( ! site?.options || loadingShouldShowLaunchpadFirst ) {
if ( ! site?.options ) {
return null;
}

Expand All @@ -105,7 +102,7 @@ const Launchpad: Step = ( { navigation, flow }: LaunchpadProps ) => {
return null;
}

if ( shouldShowLaunchpadFirst ) {
if ( shouldShowLaunchpadFirst( site ) ) {
window.location.replace( `/home/${ siteSlug }` );
return null;
}
Expand Down
9 changes: 3 additions & 6 deletions client/my-sites/customer-home/main.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@ import DocumentHead from 'calypso/components/data/document-head';
import Main from 'calypso/components/main';
import { useGetDomainsQuery } from 'calypso/data/domains/use-get-domains-query';
import PageViewTracker from 'calypso/lib/analytics/page-view-tracker';
import { useShouldShowLaunchpadFirst } from 'calypso/state/selectors/should-show-launchpad-first';
import { shouldShowLaunchpadFirst } from 'calypso/state/selectors/should-show-launchpad-first';
import CelebrateLaunchModal from './components/celebrate-launch-modal';
import { FullScreenLaunchpad } from './components/full-screen-launchpad';
import HomeContent from './components/home-content';
import type { SiteDetails } from '@automattic/data-stores';

export default function CustomerHome( { site }: { site: SiteDetails } ) {
const [ isLoadingShouldShowLaunchpadFirst, shouldShowLaunchpadFirst ] =
useShouldShowLaunchpadFirst( site );

const isSiteLaunched = site?.launch_status === 'launched' || false;

const [ isFullLaunchpadDismissed, setIsFullLaunchpadDismissed ] = useState(
Expand All @@ -35,9 +32,9 @@ export default function CustomerHome( { site }: { site: SiteDetails } ) {
<Main wideLayout>
<PageViewTracker path="/home/:site" title={ translate( 'My Home' ) } />
<DocumentHead title={ translate( 'My Home' ) } />
{ ! isLoadingShouldShowLaunchpadFirst && (
{ site.options && (
<>
{ shouldShowLaunchpadFirst && ! isFullLaunchpadDismissed ? (
{ shouldShowLaunchpadFirst( site ) && ! isFullLaunchpadDismissed ? (
<FullScreenLaunchpad
onClose={ () => setIsFullLaunchpadDismissed( true ) }
onSiteLaunch={ () => {
Expand Down
41 changes: 0 additions & 41 deletions client/my-sites/customer-home/test/customer-home.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,13 @@ import { screen, waitFor } from '@testing-library/react';
import apiFetch from '@wordpress/api-fetch';
import nock from 'nock';
import React, { act } from 'react';
import { loadExperimentAssignment } from 'calypso/lib/explat';
import { reducer as ui } from 'calypso/state/ui/reducer';
import { renderWithProvider } from 'calypso/test-helpers/testing-library';
import CustomerHome from '../main';
import type { SiteDetails } from '@automattic/data-stores';

jest.mock( '@wordpress/api-fetch' );

let mockUseExperimentResult = [ false, true ];

jest.mock( 'calypso/lib/explat', () => ( {
loadExperimentAssignment: jest.fn( ( slug ) =>
slug === 'calypso_signup_onboarding_goals_first_flow_holdout_v2_20250131'
? Promise.resolve( { variationName: 'treatment_cumulative' } )
: Promise.reject( new Error( `Unmocked experiment slug: ${ slug }` ) )
),
useExperiment: jest.fn( () => mockUseExperimentResult ),
} ) );

jest.mock( '../components/home-content', () => () => (
<div data-testid="home-content">Home Content</div>
) );
Expand Down Expand Up @@ -162,33 +150,4 @@ describe( 'CustomerHome', () => {

expect( await screen.findByText( 'Congrats, your site is live!' ) ).toBeInTheDocument();
} );

it( 'shows home content when site would be eligible to show launchpad, but user is in control group', async () => {
const testSite = makeTestSite( {
launch_status: 'unlaunched',
options: { site_creation_flow: 'onboarding', launchpad_screen: false },
} );
( loadExperimentAssignment as jest.Mock ).mockResolvedValue( { variationName: 'control' } );

renderWithProvider( <CustomerHome site={ testSite } /> );

await waitFor( () => expect( screen.getByTestId( 'home-content' ) ).toBeInTheDocument() );
expect( screen.queryByTestId( 'launchpad-first' ) ).not.toBeInTheDocument();
} );

it( 'shows home content when site would be eligible to show launchpad, but user is in treatment_frozen group', async () => {
mockUseExperimentResult = [ false, false ];
const testSite = makeTestSite( {
launch_status: 'unlaunched',
options: { site_creation_flow: 'onboarding', launchpad_screen: false },
} );
( loadExperimentAssignment as jest.Mock ).mockResolvedValue( {
variationName: 'treatment_frozen',
} );

renderWithProvider( <CustomerHome site={ testSite } /> );

await waitFor( () => expect( screen.getByTestId( 'home-content' ) ).toBeInTheDocument() );
expect( screen.queryByTestId( 'launchpad-first' ) ).not.toBeInTheDocument();
} );
} );
53 changes: 3 additions & 50 deletions client/state/selectors/should-show-launchpad-first.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { SiteDetails, Onboard } from '@automattic/data-stores';
import { useEffect, useState } from 'react';
import { loadExperimentAssignment } from 'calypso/lib/explat';

const SiteIntent = Onboard.SiteIntent;

Expand All @@ -9,58 +7,13 @@ const SiteIntent = Onboard.SiteIntent;
* @param site Site object
* @returns Whether launchpad should be shown first
*/
export const shouldShowLaunchpadFirst = async ( site: SiteDetails ): Promise< boolean > => {
export const shouldShowLaunchpadFirst = ( site: SiteDetails ): boolean => {
const wasSiteCreatedOnboardingFlow = site.options?.site_creation_flow === 'onboarding';
const createdAfterExperimentStart =
( site.options?.created_at ?? '' ) > '2025-02-03T10:22:45+00:00'; // If created_at is null then this expression is false
const isBigSkyIntent = site?.options?.site_intent === SiteIntent.AIAssembler;

if ( isBigSkyIntent || ! wasSiteCreatedOnboardingFlow || ! createdAfterExperimentStart ) {
if ( isBigSkyIntent || ! wasSiteCreatedOnboardingFlow ) {
return false;
}

const assignment = await loadExperimentAssignment(
'calypso_signup_onboarding_goals_first_flow_holdout_v2_20250131'
);

return assignment?.variationName === 'treatment_cumulative';
};

export const useShouldShowLaunchpadFirst = ( site?: SiteDetails | null ): [ boolean, boolean ] => {
const [ state, setState ] = useState< boolean | 'loading' >( 'loading' );

useEffect( () => {
let cancel = false;

const getResponse = async () => {
if ( ! site ) {
return;
}

try {
setState( 'loading' );
const result = await shouldShowLaunchpadFirst( site );
if ( ! cancel ) {
setState( result );
}
} catch ( err ) {
if ( ! cancel ) {
setState( false );
}
}
};

getResponse();

return () => {
cancel = true;
};
}, [ site ] );

if ( ! site ) {
// If the site isn't available yet we'll assume we're still loading
return [ true, false ];
}

return [ state === 'loading', state === 'loading' ? false : state ];
return true;
};
122 changes: 2 additions & 120 deletions client/state/selectors/test/should-show-launchpad-first.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
/**
* @jest-environment jsdom
*/
import { act, renderHook, waitFor } from '@testing-library/react';
import { loadExperimentAssignment } from 'calypso/lib/explat';
import {
shouldShowLaunchpadFirst,
useShouldShowLaunchpadFirst,
} from '../should-show-launchpad-first';
import { shouldShowLaunchpadFirst } from '../should-show-launchpad-first';
import type { SiteDetails } from '@automattic/data-stores';

jest.mock( 'calypso/lib/explat', () => ( {
Expand All @@ -25,35 +21,19 @@ describe( 'shouldShowLaunchpadFirst', () => {
const site = {
options: {
site_creation_flow: 'onboarding',
created_at: '2025-02-18T00:00:00+00:00',
},
} as SiteDetails;

expect( await shouldShowLaunchpadFirst( site ) ).toBe( true );
} );

it( 'should return false when site was created via onboarding flow but assigned to control', async () => {
( loadExperimentAssignment as jest.Mock ).mockResolvedValue( {
variationName: 'control',
} );
const site = {
options: {
site_creation_flow: 'onboarding',
created_at: '2025-02-18T00:00:00+00:00',
},
} as SiteDetails;

expect( await shouldShowLaunchpadFirst( site ) ).toBe( false );
} );

it( 'should return false when site was not created via onboarding flow', async () => {
( loadExperimentAssignment as jest.Mock ).mockResolvedValue( {
variationName: 'treatment_cumulative',
} );
const site = {
options: {
site_creation_flow: 'other',
created_at: '2025-02-18T00:00:00+00:00',
},
} as SiteDetails;

Expand All @@ -65,107 +45,9 @@ describe( 'shouldShowLaunchpadFirst', () => {
variationName: 'treatment_cumulative',
} );
const site = {
options: {
created_at: '2025-02-18T00:00:00+00:00',
},
options: {},
} as SiteDetails;

expect( await shouldShowLaunchpadFirst( site ) ).toBe( false );
} );
} );

describe( 'useShouldShowLaunchpadFirst', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel bad removing a good part of this test @p-jackson. Such a good work!!

Copy link
Member

Choose a reason for hiding this comment

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

Yes, using A/B testing to help us make decisions definitely doesn't come for free! Kinda frustrating spending the time to handle all the cases just to have it all removed for some reason 🤖🤓

Copy link
Member

Choose a reason for hiding this comment

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

(I also don't think these test cases took me too long, I think it's just copy pasting the same test cases from above, but then converted to work as a hook)

it( 'returns loading state until promise resolves', async () => {
const { promise, resolve } = promiseWithResolvers();
( loadExperimentAssignment as jest.Mock ).mockReturnValue( promise );
const site = {
options: {
site_creation_flow: 'onboarding',
created_at: '2025-02-18T00:00:00+00:00',
},
} as SiteDetails;

const { result } = renderHook( () => useShouldShowLaunchpadFirst( site ) );

expect( result.current[ 0 ] ).toBe( true );

await act( () => resolve( { variationName: 'treatment_cumulative' } ) );

expect( result.current ).toEqual( [ false, true ] );
} );

it( 'should return true when site was created via onboarding flow and assigned to experiment', async () => {
( loadExperimentAssignment as jest.Mock ).mockResolvedValue( {
variationName: 'treatment_cumulative',
} );
const site = {
options: {
site_creation_flow: 'onboarding',
created_at: '2025-02-18T00:00:00+00:00',
},
} as SiteDetails;

const { result } = renderHook( () => useShouldShowLaunchpadFirst( site ) );

await waitFor( () => expect( result.current ).toEqual( [ false, true ] ) );
} );

it( 'should return false when site was created via onboarding flow but assigned to control', async () => {
( loadExperimentAssignment as jest.Mock ).mockResolvedValue( {
variationName: 'control',
} );
const site = {
options: {
site_creation_flow: 'onboarding',
created_at: '2025-02-18T00:00:00+00:00',
},
} as SiteDetails;

const { result } = renderHook( () => useShouldShowLaunchpadFirst( site ) );

await waitFor( () => expect( result.current ).toEqual( [ false, false ] ) );
} );

it( 'should return false when site was not created via onboarding flow', async () => {
( loadExperimentAssignment as jest.Mock ).mockResolvedValue( {
variationName: 'treatment_cumulative',
} );
const site = {
options: {
site_creation_flow: 'other',
created_at: '2025-02-18T00:00:00+00:00',
},
} as SiteDetails;

const { result } = renderHook( () => useShouldShowLaunchpadFirst( site ) );

await waitFor( () => expect( result.current ).toEqual( [ false, false ] ) );
} );

it( 'should return false when site has no creation flow information', async () => {
( loadExperimentAssignment as jest.Mock ).mockResolvedValue( {
variationName: 'treatment_cumulative',
} );
const site = {
options: {
created_at: '2025-02-18T00:00:00+00:00',
},
} as SiteDetails;

const { result } = renderHook( () => useShouldShowLaunchpadFirst( site ) );

await waitFor( () => expect( result.current ).toEqual( [ false, false ] ) );
} );
} );

// Our TS Config doesn't know the standard Promise.withResolvers is available
// in our version of Node.
function promiseWithResolvers() {
let resolve;
let reject;
const promise = new Promise( ( res, rej ) => {
resolve = res;
reject = rej;
} );
return { promise, resolve, reject };
}