From 0728d467749915d7f7eeae45e3af04d5343e851d Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Tue, 2 Mar 2021 18:05:04 -0500 Subject: [PATCH 1/7] Remove CartData wrapper around CheckoutSystemDecider --- client/my-sites/checkout/controller.jsx | 27 +++++++++++-------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/client/my-sites/checkout/controller.jsx b/client/my-sites/checkout/controller.jsx index d73277e162640..a3028cff2a516 100644 --- a/client/my-sites/checkout/controller.jsx +++ b/client/my-sites/checkout/controller.jsx @@ -21,7 +21,6 @@ import { canUserPurchaseGSuite } from 'calypso/lib/gsuite'; import { getRememberedCoupon } from 'calypso/lib/cart/actions'; import { setSectionMiddleware } from 'calypso/controller'; import { sites } from 'calypso/my-sites/controller'; -import CartData from 'calypso/components/data/cart'; import userFactory from 'calypso/lib/user'; import { getCurrentUser } from 'calypso/state/current-user/selectors'; import { @@ -93,20 +92,18 @@ export function checkout( context, next ) { } context.primary = ( - - - + ); next(); From aafbf7110aa67e427d737781b1b6b8e5c5a56761 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Tue, 2 Mar 2021 18:12:43 -0500 Subject: [PATCH 2/7] Remove cart store race condition guard in CheckoutSystemDecider --- .../checkout/checkout-system-decider.js | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/client/my-sites/checkout/checkout-system-decider.js b/client/my-sites/checkout/checkout-system-decider.js index f71ca4ed401f4..350fbccb7830a 100644 --- a/client/my-sites/checkout/checkout-system-decider.js +++ b/client/my-sites/checkout/checkout-system-decider.js @@ -39,7 +39,6 @@ export default function CheckoutSystemDecider( { redirectTo, isLoggedOutCart, isNoSiteCart, - cart: otherCart, } ) { const reduxDispatch = useDispatch(); const translate = useTranslate(); @@ -80,23 +79,14 @@ export default function CheckoutSystemDecider( { [ reduxDispatch ] ); - // We have to monitor the old cart manager in case it's waiting on a - // requested change. To prevent race conditions, we will return undefined in - // that case, which will cause the ShoppingCartProvider to enter a loading - // state. We have to use null because CalypsoShoppingCartProvider assumes - // undefined means to try for its own cartKey. - const waitForOtherCartUpdates = - otherCart?.hasPendingServerUpdates || ! otherCart?.hasLoadedFromServer; const cartKey = useMemo( () => - waitForOtherCartUpdates - ? null - : getCartKey( { - selectedSite, - isLoggedOutCart, - isNoSiteCart, - } ), - [ waitForOtherCartUpdates, selectedSite, isLoggedOutCart, isNoSiteCart ] + getCartKey( { + selectedSite, + isLoggedOutCart, + isNoSiteCart, + } ), + [ selectedSite, isLoggedOutCart, isNoSiteCart ] ); debug( 'cartKey is', cartKey ); @@ -110,16 +100,13 @@ export default function CheckoutSystemDecider( { } } - const getCart = isLoggedOutCart || isNoSiteCart ? () => Promise.resolve( otherCart ) : undefined; - debug( 'getCart being controlled by', { isLoggedOutCart, isNoSiteCart, otherCart } ); - return ( <> - + Date: Tue, 2 Mar 2021 18:59:02 -0500 Subject: [PATCH 3/7] Allow usePrepareProductsForCart to load products from localStorage --- .../composite-checkout/composite-checkout.tsx | 2 + .../hooks/use-prepare-products-for-cart.ts | 60 +++++++++++++++++-- .../lib/get-cart-from-local-storage.ts | 13 ++++ 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 client/my-sites/checkout/composite-checkout/lib/get-cart-from-local-storage.ts diff --git a/client/my-sites/checkout/composite-checkout/composite-checkout.tsx b/client/my-sites/checkout/composite-checkout/composite-checkout.tsx index a1988f0e6a112..17b72bc5e4b44 100644 --- a/client/my-sites/checkout/composite-checkout/composite-checkout.tsx +++ b/client/my-sites/checkout/composite-checkout/composite-checkout.tsx @@ -232,6 +232,8 @@ export default function CompositeCheckout( { isJetpackNotAtomic, isPrivate, siteSlug, + isLoggedOutCart, + isNoSiteCart, } ); const { diff --git a/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts b/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts index 506c077bfbbce..dbb8ea70b3c0c 100644 --- a/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts +++ b/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts @@ -24,6 +24,7 @@ import { getProductsList, isProductsListFetching } from 'calypso/state/products- import useFetchProductsIfNotLoaded from './use-fetch-products-if-not-loaded'; import doesValueExist from '../lib/does-value-exist'; import useStripProductsFromUrl from './use-strip-products-from-url'; +import getCartFromLocalStorage from '../lib/get-cart-from-local-storage'; const debug = debugFactory( 'calypso:composite-checkout:use-prepare-products-for-cart' ); @@ -46,6 +47,8 @@ export default function usePrepareProductsForCart( { isJetpackNotAtomic, isPrivate, siteSlug, + isLoggedOutCart, + isNoSiteCart, }: { productAliasFromUrl: string | null | undefined; purchaseId: string | number | null | undefined; @@ -53,6 +56,8 @@ export default function usePrepareProductsForCart( { isJetpackNotAtomic: boolean; isPrivate: boolean; siteSlug: string | undefined; + isLoggedOutCart?: boolean; + isNoSiteCart?: boolean; } ): PreparedProductsForCart { const initializePreparedProductsState = ( initialState: PreparedProductsForCart @@ -65,11 +70,14 @@ export default function usePrepareProductsForCart( { initialPreparedProductsState, initializePreparedProductsState ); + debug( 'preparing products for cart from url string', productAliasFromUrl, 'and purchase id', - originalPurchaseId + originalPurchaseId, + 'and signup variables', + { isLoggedOutCart, isNoSiteCart } ); useFetchProductsIfNotLoaded(); @@ -78,10 +86,16 @@ export default function usePrepareProductsForCart( { isLoading: state.isLoading, originalPurchaseId, productAliasFromUrl, + isLoggedOutCart, + isNoSiteCart, } ); // Only one of these should ever operate. The others should bail if they // think another hook will handle the data. + useAddProductsFromLocalStorage( { + dispatch, + addHandler, + } ); useAddProductFromSlug( { productAliasFromUrl, dispatch, @@ -130,32 +144,70 @@ function preparedProductsReducer( } } -type AddHandler = 'addProductFromSlug' | 'addRenewalItems' | 'doNotAdd'; +type AddHandler = 'addProductFromSlug' | 'addRenewalItems' | 'doNotAdd' | 'addFromLocalStorage'; function chooseAddHandler( { isLoading, originalPurchaseId, productAliasFromUrl, + isLoggedOutCart, + isNoSiteCart, }: { isLoading: boolean; originalPurchaseId: string | number | null | undefined; productAliasFromUrl: string | null | undefined; + isLoggedOutCart?: boolean; + isNoSiteCart?: boolean; } ): AddHandler { if ( ! isLoading ) { return 'doNotAdd'; } - if ( isLoading && originalPurchaseId ) { + if ( isLoggedOutCart || isNoSiteCart ) { + return 'addFromLocalStorage'; + } + + if ( originalPurchaseId ) { return 'addRenewalItems'; } - if ( isLoading && ! originalPurchaseId && productAliasFromUrl ) { + if ( ! originalPurchaseId && productAliasFromUrl ) { return 'addProductFromSlug'; } return 'doNotAdd'; } +function useAddProductsFromLocalStorage( { + dispatch, + addHandler, +}: { + dispatch: ( action: PreparedProductsAction ) => void; + addHandler: AddHandler; +} ) { + const translate = useTranslate(); + + useEffect( () => { + if ( addHandler !== 'addFromLocalStorage' ) { + return; + } + + const productsForCart = getCartFromLocalStorage(); + + if ( productsForCart.length < 1 ) { + debug( 'creating products from localStorage failed' ); + dispatch( { + type: 'PRODUCTS_ADD_ERROR', + message: String( translate( 'I tried and failed to create products from signup' ) ), + } ); + return; + } + + debug( 'preparing products requested in localStorage', productsForCart ); + dispatch( { type: 'PRODUCTS_ADD', products: productsForCart } ); + }, [ addHandler, dispatch, translate ] ); +} + function useAddRenewalItems( { originalPurchaseId, productAlias, diff --git a/client/my-sites/checkout/composite-checkout/lib/get-cart-from-local-storage.ts b/client/my-sites/checkout/composite-checkout/lib/get-cart-from-local-storage.ts new file mode 100644 index 0000000000000..d8f3c0a383dfa --- /dev/null +++ b/client/my-sites/checkout/composite-checkout/lib/get-cart-from-local-storage.ts @@ -0,0 +1,13 @@ +/** + * External dependencies + */ +import type { RequestCartProduct } from '@automattic/shopping-cart'; + +// Used by signup; see https://github.com/Automattic/wp-calypso/pull/44206 +export default function getCartFromLocalStorage(): RequestCartProduct[] { + try { + return JSON.parse( window.localStorage.getItem( 'shoppingCart' ) || '[]' ); + } catch ( err ) { + return []; + } +} From 36fa83cda50900f749cf0bc21584d560976ba9ed Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Wed, 3 Mar 2021 13:47:16 -0500 Subject: [PATCH 4/7] Mock initial checkout cart request if no site/no user --- client/my-sites/checkout/checkout-system-decider.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/client/my-sites/checkout/checkout-system-decider.js b/client/my-sites/checkout/checkout-system-decider.js index 350fbccb7830a..cec762dcd20b7 100644 --- a/client/my-sites/checkout/checkout-system-decider.js +++ b/client/my-sites/checkout/checkout-system-decider.js @@ -7,6 +7,7 @@ import debugFactory from 'debug'; import { CheckoutErrorBoundary } from '@automattic/composite-checkout'; import { useTranslate } from 'i18n-calypso'; import { StripeHookProvider } from '@automattic/calypso-stripe'; +import { getEmptyResponseCart } from '@automattic/shopping-cart'; /** * Internal Dependencies @@ -26,6 +27,8 @@ import CalypsoShoppingCartProvider from './calypso-shopping-cart-provider'; // otherwise we get `this is not defined` errors. const wpcom = wp.undocumented(); +const emptyCart = getEmptyResponseCart(); + const debug = debugFactory( 'calypso:checkout-system-decider' ); export default function CheckoutSystemDecider( { @@ -100,13 +103,17 @@ export default function CheckoutSystemDecider( { } } + // If we do not have a site or user, we cannot fetch the initial cart from + // the server, so we'll just mock it as an empty cart here. + const getCart = isLoggedOutCart || isNoSiteCart ? () => Promise.resolve( emptyCart ) : undefined; + return ( <> - + Date: Wed, 3 Mar 2021 16:40:35 -0500 Subject: [PATCH 5/7] Prevent skipping usePrepareProductsForCart if product from url empty --- .../hooks/use-prepare-products-for-cart.ts | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts b/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts index dbb8ea70b3c0c..2aaf0291186fc 100644 --- a/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts +++ b/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts @@ -59,25 +59,17 @@ export default function usePrepareProductsForCart( { isLoggedOutCart?: boolean; isNoSiteCart?: boolean; } ): PreparedProductsForCart { - const initializePreparedProductsState = ( - initialState: PreparedProductsForCart - ): PreparedProductsForCart => ( { - ...initialState, - isLoading: !! productAliasFromUrl, - } ); - const [ state, dispatch ] = useReducer( - preparedProductsReducer, - initialPreparedProductsState, - initializePreparedProductsState - ); + const [ state, dispatch ] = useReducer( preparedProductsReducer, initialPreparedProductsState ); debug( 'preparing products for cart from url string', productAliasFromUrl, 'and purchase id', originalPurchaseId, - 'and signup variables', - { isLoggedOutCart, isNoSiteCart } + 'and isLoggedOutCart', + isLoggedOutCart, + 'and isNoSiteCart', + isNoSiteCart ); useFetchProductsIfNotLoaded(); @@ -89,6 +81,8 @@ export default function usePrepareProductsForCart( { isLoggedOutCart, isNoSiteCart, } ); + debug( 'isLoading', state.isLoading ); + debug( 'handler is', addHandler ); // Only one of these should ever operate. The others should bail if they // think another hook will handle the data. @@ -109,6 +103,7 @@ export default function usePrepareProductsForCart( { dispatch, addHandler, } ); + useNothingToAdd( { addHandler, dispatch } ); // Do not strip products from url until the URL has been parsed const areProductsRetrievedFromUrl = ! state.isLoading && ! isInEditor; @@ -178,6 +173,23 @@ function chooseAddHandler( { return 'doNotAdd'; } +function useNothingToAdd( { + dispatch, + addHandler, +}: { + dispatch: ( action: PreparedProductsAction ) => void; + addHandler: AddHandler; +} ) { + useEffect( () => { + if ( addHandler !== 'doNotAdd' ) { + return; + } + + debug( 'nothing to add' ); + dispatch( { type: 'PRODUCTS_ADD', products: [] } ); + }, [ addHandler, dispatch ] ); +} + function useAddProductsFromLocalStorage( { dispatch, addHandler, From 104a7ed4afa3ce722055ba770fd018f7fe76351f Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Wed, 3 Mar 2021 16:52:08 -0500 Subject: [PATCH 6/7] Fill in product_id for products from localStorage --- .../hooks/use-prepare-products-for-cart.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts b/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts index 2aaf0291186fc..9fcd72221d17c 100644 --- a/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts +++ b/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts @@ -25,6 +25,7 @@ import useFetchProductsIfNotLoaded from './use-fetch-products-if-not-loaded'; import doesValueExist from '../lib/does-value-exist'; import useStripProductsFromUrl from './use-strip-products-from-url'; import getCartFromLocalStorage from '../lib/get-cart-from-local-storage'; +import { fillInSingleCartItemAttributes } from 'calypso/lib/cart-values'; const debug = debugFactory( 'calypso:composite-checkout:use-prepare-products-for-cart' ); @@ -198,13 +199,26 @@ function useAddProductsFromLocalStorage( { addHandler: AddHandler; } ) { const translate = useTranslate(); + const products: Record< + string, + { + product_id: number; + product_slug: string; + } + > = useSelector( getProductsList ); useEffect( () => { if ( addHandler !== 'addFromLocalStorage' ) { return; } + if ( Object.keys( products || {} ).length < 1 ) { + debug( 'waiting on products fetch' ); + return; + } - const productsForCart = getCartFromLocalStorage(); + const productsForCart = getCartFromLocalStorage().map( ( product ) => + fillInSingleCartItemAttributes( product, products ) + ); if ( productsForCart.length < 1 ) { debug( 'creating products from localStorage failed' ); @@ -217,7 +231,7 @@ function useAddProductsFromLocalStorage( { debug( 'preparing products requested in localStorage', productsForCart ); dispatch( { type: 'PRODUCTS_ADD', products: productsForCart } ); - }, [ addHandler, dispatch, translate ] ); + }, [ addHandler, dispatch, translate, products ] ); } function useAddRenewalItems( { From b9122e15d0ec2570f5a3c1337fd9863da6346b70 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Wed, 3 Mar 2021 17:01:37 -0500 Subject: [PATCH 7/7] Correct return type of getCartFromLocalStorage --- .../composite-checkout/hooks/use-prepare-products-for-cart.ts | 2 +- .../composite-checkout/lib/get-cart-from-local-storage.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts b/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts index 9fcd72221d17c..557d92392984c 100644 --- a/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts +++ b/client/my-sites/checkout/composite-checkout/hooks/use-prepare-products-for-cart.ts @@ -216,7 +216,7 @@ function useAddProductsFromLocalStorage( { return; } - const productsForCart = getCartFromLocalStorage().map( ( product ) => + const productsForCart: RequestCartProduct[] = getCartFromLocalStorage().map( ( product ) => fillInSingleCartItemAttributes( product, products ) ); diff --git a/client/my-sites/checkout/composite-checkout/lib/get-cart-from-local-storage.ts b/client/my-sites/checkout/composite-checkout/lib/get-cart-from-local-storage.ts index d8f3c0a383dfa..b5df51a84b0dc 100644 --- a/client/my-sites/checkout/composite-checkout/lib/get-cart-from-local-storage.ts +++ b/client/my-sites/checkout/composite-checkout/lib/get-cart-from-local-storage.ts @@ -4,7 +4,8 @@ import type { RequestCartProduct } from '@automattic/shopping-cart'; // Used by signup; see https://github.com/Automattic/wp-calypso/pull/44206 -export default function getCartFromLocalStorage(): RequestCartProduct[] { +// These products are likely missing product_id. +export default function getCartFromLocalStorage(): Partial< RequestCartProduct >[] { try { return JSON.parse( window.localStorage.getItem( 'shoppingCart' ) || '[]' ); } catch ( err ) {