[CYS] Avoid duplicating requests during the CYS flow (#44179)

* Avoid duplicating the `/onboarding/themes/recommended` request

* Use `fetchActiveThemeHasMods` in the `fetchIntroData` function

* Add changefile(s) from automation for the following project(s): woocommerce

* WIP

* WIP

* WIP

* WIP

* Move the `activeThemeHasMods` to outside the `intro` key, since it's used also in other places

Refactor, add comments and cleanup

* Remove unnecessary log

* Set the `activeThemeHasMods` on the parent window to true after the design step to avoid having to fetch it again

* Add changefile(s) from automation for the following project(s): woocommerce

* Remove unused import

* Add comment and ref to the new issue

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Luigi Teschio <gigitux@gmail.com>
This commit is contained in:
Alba Rincón 2024-02-05 12:36:33 +01:00 committed by GitHub
parent d858d22930
commit a943de24fa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 156 additions and 129 deletions

View File

@ -336,6 +336,16 @@ const redirectToAssemblerHub = async (
document.body.appendChild( iframe );
// This is a workaround to update the "activeThemeHasMods" in the parent's machine
// state context. We should find a better way to do this using xstate actions,
// since state machines should rely only on their context.
// Will be fixed on: https://github.com/woocommerce/woocommerce/issues/44349
// This is needed because the iframe loads the entire Customize Store app.
// This means that the iframe instance will have different state machines
// than the parent window.
// Check https://github.com/woocommerce/woocommerce/pull/44206 for more details.
window.parent.__wcCustomizeStore.activeThemeHasMods = true;
// Listen for back button click
window.addEventListener(
'popstate',
@ -356,7 +366,7 @@ const redirectToAssemblerHub = async (
'*'
);
// When the user clicks the back button, push state changes to the previous step
// Set it back to the assember hub
// Set it back to the assembler hub
window.history?.pushState( {}, '', assemblerUrl );
}
},

View File

@ -44,6 +44,16 @@ const redirectToAssemblerHub = async () => {
};
document.body.appendChild( iframe );
// This is a workaround to update the "activeThemeHasMods" in the parent's machine
// state context. We should find a better way to do this using xstate actions,
// since state machines should rely only on their context.
// Will be fixed on: https://github.com/woocommerce/woocommerce/issues/44349
// This is needed because the iframe loads the entire Customize Store app.
// This means that the iframe instance will have different state machines
// than the parent window.
// Check https://github.com/woocommerce/woocommerce/pull/44206 for more details.
window.parent.__wcCustomizeStore.activeThemeHasMods = true;
};
const redirectToIntroWithError = sendParent<

View File

@ -4,7 +4,7 @@ import { store as coreStore } from '@wordpress/core-data';
/**
* External dependencies
*/
import { EventData, Sender, createMachine } from 'xstate';
import { Sender, createMachine } from 'xstate';
import { useEffect, useMemo, useState } from '@wordpress/element';
import { useMachine, useSelector } from '@xstate/react';
import {
@ -17,7 +17,6 @@ import { dispatch, resolveSelect } from '@wordpress/data';
import { Spinner } from '@woocommerce/components';
import { getAdminLink } from '@woocommerce/settings';
import { PluginArea } from '@wordpress/plugins';
import apiFetch from '@wordpress/api-fetch';
/**
* Internal dependencies
@ -119,18 +118,6 @@ const CYSSpinner = () => (
</div>
);
const fetchIsFontLibraryAvailable = async () => {
try {
await apiFetch( {
path: '/wp/v2/font-collections',
method: 'GET',
} );
return true;
} catch ( err ) {
return false;
}
};
export const machineActions = {
updateQueryStep,
redirectToWooHome,
@ -151,7 +138,7 @@ export const customizeStoreStateMachineServices = {
};
export const customizeStoreStateMachineDefinition = createMachine( {
id: 'customizeStore',
initial: 'navigate',
initial: 'setFlags',
predictableActionArguments: true,
preserveActionOrder: true,
schema: {
@ -173,7 +160,6 @@ export const customizeStoreStateMachineDefinition = createMachine( {
},
},
activeTheme: '',
activeThemeHasMods: false,
customizeStoreTaskCompleted: false,
currentThemeIsAiGenerated: false,
},
@ -182,6 +168,7 @@ export const customizeStoreStateMachineDefinition = createMachine( {
},
flowType: FlowType.noAI,
isFontLibraryAvailable: null,
activeThemeHasMods: undefined,
} as customizeStoreStateMachineContext,
invoke: {
src: 'browserPopstateHandler',
@ -206,6 +193,15 @@ export const customizeStoreStateMachineDefinition = createMachine( {
},
},
states: {
setFlags: {
invoke: {
src: 'setFlags',
onDone: {
actions: 'assignFlags',
target: 'navigate',
},
},
},
navigate: {
always: [
{
@ -299,7 +295,6 @@ export const customizeStoreStateMachineDefinition = createMachine( {
target: 'success',
actions: [
'assignThemeData',
'assignActiveThemeHasMods',
'assignCustomizeStoreCompleted',
'assignCurrentThemeIsAiGenerated',
],
@ -384,27 +379,20 @@ export const customizeStoreStateMachineDefinition = createMachine( {
},
},
assemblerHub: {
initial: 'fetchActiveThemeHasMods',
initial: 'checkActiveThemeHasMods',
states: {
fetchActiveThemeHasMods: {
invoke: {
src: 'fetchIntroData',
onDone: {
target: 'checkActiveThemeHasMods',
actions: [ 'assignActiveThemeHasMods' ],
},
},
},
checkActiveThemeHasMods: {
always: [
{
cond: 'activeThemeIsNotModified',
// Redirect to the "intro step" if the active theme has no modifications.
cond: 'activeThemeHasNoMods',
actions: [
{ type: 'updateQueryStep', step: 'intro' },
],
target: '#customizeStore.intro',
},
{
// Otherwise, proceed to the next step.
cond: 'activeThemeHasMods',
target: 'preCheckAiStatus',
},
@ -534,51 +522,11 @@ declare global {
interface Window {
__wcCustomizeStore: {
isFontLibraryAvailable: boolean | null;
activeThemeHasMods: boolean | undefined;
};
}
}
// HACK: This is a temporary solution to pass flags computed into the iframe instance state machines.
// This is needed because the iframe loads the entire Customize Store app. This means that the iframe instance will have different state machines than the parent window.
// Check https://github.com/woocommerce/woocommerce/pull/44206 for more details.
const setFlagsForIframeInstance = async (
send: (
event: customizeStoreStateMachineEvents,
payload?: EventData | undefined
) => void
) => {
if ( ! window.frameElement ) {
// To improve the readability of the code, we want to use a dictionary where the key is the feature flag name and the value is the function to retrive flag value.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const _featureFlags = {
FONT_LIBRARY_AVAILABLE: ( async () => {
const isFontLibraryAvailable =
await fetchIsFontLibraryAvailable();
window.__wcCustomizeStore = {
...window.__wcCustomizeStore,
isFontLibraryAvailable,
};
} )(),
};
return;
}
// To improve the readability of the code, we want to use a dictionary where the key is the feature flag name and the value is the function to send the event to set the flag value to the iframe instance state machine.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const _featureFlagsEvents = {
FONT_LIBRARY_AVAILABLE: ( async () => {
window.__wcCustomizeStore = window.__wcCustomizeStore ?? {};
const isFontLibraryAvailable =
window.__wcCustomizeStore.isFontLibraryAvailable || false;
send( {
type: 'IS_FONT_LIBRARY_AVAILABLE',
payload: isFontLibraryAvailable,
} );
} )(),
};
};
export const CustomizeStoreController = ( {
actionOverrides,
servicesOverrides,
@ -616,10 +564,10 @@ export const CustomizeStoreController = ( {
isWooExpress: () => isWooExpress(),
isNotWooExpress: () => ! isWooExpress(),
activeThemeHasMods: ( _ctx ) => {
return _ctx.intro.activeThemeHasMods;
return !! _ctx.activeThemeHasMods;
},
activeThemeIsNotModified: ( _ctx ) => {
return ! _ctx.intro.activeThemeHasMods;
activeThemeHasNoMods: ( _ctx ) => {
return ! _ctx.activeThemeHasMods;
},
},
} );
@ -629,10 +577,6 @@ export const CustomizeStoreController = ( {
devTools: process.env.NODE_ENV === 'development',
} );
useEffect( () => {
setFlagsForIframeInstance( send );
}, [ send ] );
// eslint-disable-next-line react-hooks/exhaustive-deps -- false positive due to function name match, this isn't from react std lib
const currentNodeMeta = useSelector( service, ( currentState ) =>
findComponentMeta< CustomizeStoreComponentMeta >(

View File

@ -15,6 +15,7 @@ import {
RecommendThemesAPIResponse,
} from '../types';
import { events } from './';
import { isIframe } from '~/customize-store/utils';
export const assignThemeData = assign<
customizeStoreStateMachineContext,
@ -56,19 +57,6 @@ export const recordTracksBrowseAllThemesClicked = () => {
recordEvent( 'customize_your_store_intro_browse_all_themes_click' );
};
export const assignActiveThemeHasMods = assign<
customizeStoreStateMachineContext,
customizeStoreStateMachineEvents // this is actually the wrong type for the event but I still don't know how to type this properly
>( {
intro: ( context, event ) => {
const activeThemeHasMods = (
event as DoneInvokeEvent< { activeThemeHasMods: boolean } >
).data.activeThemeHasMods;
// type coercion workaround for now
return { ...context.intro, activeThemeHasMods };
},
} );
export const assignCustomizeStoreCompleted = assign<
customizeStoreStateMachineContext,
customizeStoreStateMachineEvents
@ -168,3 +156,24 @@ export const assignIsFontLibraryAvailable = assign<
).payload;
},
} );
export const assignFlags = assign<
customizeStoreStateMachineContext,
customizeStoreStateMachineEvents
>( {
activeThemeHasMods: () => {
if ( ! isIframe( window ) ) {
return window.__wcCustomizeStore.activeThemeHasMods;
}
return window.parent.__wcCustomizeStore.activeThemeHasMods;
},
isFontLibraryAvailable: ( context ) => {
if ( ! isIframe( window ) ) {
return context.isFontLibraryAvailable;
}
const isFontLibraryAvailable =
window.parent.__wcCustomizeStore.isFontLibraryAvailable || false;
return isFontLibraryAvailable;
},
} );

View File

@ -75,10 +75,10 @@ export const Intro: CustomizeStoreComponent = ( { sendEvent, context } ) => {
const {
intro: {
themeData,
activeThemeHasMods,
customizeStoreTaskCompleted,
currentThemeIsAiGenerated,
},
activeThemeHasMods,
} = context;
const isJetpackOffline = false;

View File

@ -12,6 +12,7 @@ import apiFetch from '@wordpress/api-fetch';
* Internal dependencies
*/
import { aiStatusResponse } from '../types';
import { isIframe } from '~/customize-store/utils';
export const fetchAiStatus = () => async (): Promise< aiStatusResponse > => {
const response = await fetch(
@ -30,15 +31,11 @@ export const fetchThemeCards = async () => {
return themes;
};
export const fetchIntroData = async () => {
export const fetchActiveThemeHasMods = async () => {
const currentTemplatePromise =
// @ts-expect-error No types for this exist yet.
resolveSelect( coreStore ).__experimentalGetTemplateForLink( '/' );
const maybePreviousTemplatePromise = resolveSelect(
OPTIONS_STORE_NAME
).getOption( 'woocommerce_admin_customize_store_completed_theme_id' );
const styleRevsPromise =
// @ts-expect-error No types for this exist yet.
resolveSelect( coreStore ).getCurrentThemeGlobalStylesRevisions();
@ -55,36 +52,12 @@ export const fetchIntroData = async () => {
}
);
const getTaskPromise = resolveSelect( ONBOARDING_STORE_NAME ).getTask(
'customize-store'
);
const themeDataPromise = fetchThemeCards();
const [
currentTemplate,
maybePreviousTemplate,
styleRevs,
rawPages,
task,
themeData,
] = await Promise.all( [
const [ currentTemplate, styleRevs, rawPages ] = await Promise.all( [
currentTemplatePromise,
maybePreviousTemplatePromise,
styleRevsPromise,
hasModifiedPagesPromise,
getTaskPromise,
themeDataPromise,
] );
let currentThemeIsAiGenerated = false;
if (
maybePreviousTemplate &&
currentTemplate?.id === maybePreviousTemplate
) {
currentThemeIsAiGenerated = true;
}
const hasModifiedPages = rawPages?.some(
( page: { _links: { [ key: string ]: string[] } } ) => {
return page._links?.[ 'version-history' ]?.length > 1;
@ -96,12 +69,89 @@ export const fetchIntroData = async () => {
styleRevs?.length > 0 ||
hasModifiedPages;
return activeThemeHasMods;
};
export const fetchIntroData = async () => {
const currentTemplatePromise =
// @ts-expect-error No types for this exist yet.
resolveSelect( coreStore ).__experimentalGetTemplateForLink( '/' );
const maybePreviousTemplatePromise = resolveSelect(
OPTIONS_STORE_NAME
).getOption( 'woocommerce_admin_customize_store_completed_theme_id' );
const getTaskPromise = resolveSelect( ONBOARDING_STORE_NAME ).getTask(
'customize-store'
);
const themeDataPromise = fetchThemeCards();
const [ currentTemplate, maybePreviousTemplate, task, themeData ] =
await Promise.all( [
currentTemplatePromise,
maybePreviousTemplatePromise,
getTaskPromise,
themeDataPromise,
] );
let currentThemeIsAiGenerated = false;
if (
maybePreviousTemplate &&
currentTemplate?.id === maybePreviousTemplate
) {
currentThemeIsAiGenerated = true;
}
const customizeStoreTaskCompleted = task?.isComplete;
return {
activeThemeHasMods,
customizeStoreTaskCompleted,
themeData,
currentThemeIsAiGenerated,
};
};
const fetchIsFontLibraryAvailable = async () => {
try {
await apiFetch( {
path: '/wp/v2/font-collections',
method: 'GET',
} );
return true;
} catch ( err ) {
return false;
}
};
export const setFlags = async () => {
if ( ! isIframe( window ) ) {
// To improve the readability of the code, we want to use a dictionary
// where the key is the feature flag name and the value is the
// function to retrieve flag value.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const _featureFlags = {
FONT_LIBRARY_AVAILABLE: ( async () => {
const isFontLibraryAvailable =
await fetchIsFontLibraryAvailable();
window.__wcCustomizeStore = {
...window.__wcCustomizeStore,
isFontLibraryAvailable,
};
} )(),
ACTIVE_THEME_HAS_MODS: ( async () => {
const activeThemeHasMods = await fetchActiveThemeHasMods();
window.__wcCustomizeStore = {
...window.__wcCustomizeStore,
activeThemeHasMods,
};
} )(),
};
// Since the _featureFlags values are promises, we need to wait for
// all of them to resolve before returning.
await Promise.all( Object.values( _featureFlags ) );
}
};

View File

@ -37,7 +37,6 @@ describe( 'Intro Banners', () => {
},
},
},
activeThemeHasMods: false,
customizeStoreTaskCompleted: false,
currentThemeIsAiGenerated: false,
},
@ -47,6 +46,7 @@ describe( 'Intro Banners', () => {
},
flowType: FlowType.AIOnline,
isFontLibraryAvailable: false,
activeThemeHasMods: false,
} }
currentState={ 'intro' }
parentMachine={ null as unknown as AnyInterpreter }
@ -76,7 +76,6 @@ describe( 'Intro Banners', () => {
},
},
},
activeThemeHasMods: false,
customizeStoreTaskCompleted: false,
currentThemeIsAiGenerated: false,
},
@ -86,6 +85,7 @@ describe( 'Intro Banners', () => {
},
flowType: FlowType.AIOnline,
isFontLibraryAvailable: false,
activeThemeHasMods: false,
} }
currentState={ 'intro' }
parentMachine={ null as unknown as AnyInterpreter }
@ -121,7 +121,6 @@ describe( 'Intro Banners', () => {
},
},
},
activeThemeHasMods: false,
customizeStoreTaskCompleted: true,
currentThemeIsAiGenerated: true,
},
@ -131,6 +130,7 @@ describe( 'Intro Banners', () => {
},
flowType: FlowType.AIOnline,
isFontLibraryAvailable: false,
activeThemeHasMods: false,
} }
currentState={ 'intro' }
parentMachine={ null as unknown as AnyInterpreter }

View File

@ -35,7 +35,6 @@ describe( 'Intro Modals', () => {
},
},
},
activeThemeHasMods: true,
customizeStoreTaskCompleted: false,
currentThemeIsAiGenerated: false,
},
@ -45,6 +44,7 @@ describe( 'Intro Modals', () => {
},
flowType: FlowType.AIOnline,
isFontLibraryAvailable: false,
activeThemeHasMods: true,
} }
currentState={ 'intro' }
parentMachine={ null as unknown as AnyInterpreter }
@ -91,7 +91,6 @@ describe( 'Intro Modals', () => {
},
},
},
activeThemeHasMods: false,
customizeStoreTaskCompleted: true,
currentThemeIsAiGenerated: true,
},
@ -101,6 +100,7 @@ describe( 'Intro Modals', () => {
},
flowType: FlowType.AIOnline,
isFontLibraryAvailable: false,
activeThemeHasMods: false,
} }
currentState={ 'intro' }
parentMachine={ null as unknown as AnyInterpreter }
@ -145,7 +145,6 @@ describe( 'Intro Modals', () => {
},
},
},
activeThemeHasMods: false,
customizeStoreTaskCompleted: true,
currentThemeIsAiGenerated: false,
},
@ -155,6 +154,7 @@ describe( 'Intro Modals', () => {
},
flowType: FlowType.AIOnline,
isFontLibraryAvailable: false,
activeThemeHasMods: false,
} }
currentState={ 'intro' }
parentMachine={ null as unknown as AnyInterpreter }

View File

@ -50,7 +50,6 @@ export type customizeStoreStateMachineContext = {
hasErrors: boolean;
themeData: RecommendThemesAPIResponse;
activeTheme: string;
activeThemeHasMods: boolean;
customizeStoreTaskCompleted: boolean;
currentThemeIsAiGenerated: boolean;
};
@ -59,4 +58,5 @@ export type customizeStoreStateMachineContext = {
};
flowType: FlowType;
isFontLibraryAvailable: boolean | null;
activeThemeHasMods: boolean | undefined;
};

View File

@ -0,0 +1,4 @@
Significance: minor
Type: performance
Avoid duplicating requests to the `/onboarding/themes/recommended` and `/onboarding/tasks` endpoints during the CYS flow.