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

Storage Add Ons: Implement dropdown checkout functionality #81340

Merged
merged 19 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { connect } from 'react-redux';
import QueryPlans from 'calypso/components/data/query-plans';
import { LoadingEllipsis } from 'calypso/components/loading-ellipsis';
import { useSite } from 'calypso/landing/stepper/hooks/use-site';
import { getPlanCartItem } from 'calypso/lib/cart-values/cart-items';
import PlansFeaturesMain from 'calypso/my-sites/plans-features-main';
import PlanFAQ from 'calypso/my-sites/plans-features-main/components/plan-faq';
import StepWrapper from 'calypso/signup/step-wrapper';
Expand Down Expand Up @@ -92,11 +93,11 @@ const PlansWrapper: React.FC< Props > = ( props ) => {
? reduxHideFreePlan && 'plans-blog-onboarding' === plansIntent
: reduxHideFreePlan;

const onSelectPlan = ( selectedPlan: any ) => {
if ( selectedPlan ) {
const onSelectPlan = ( cartItems?: MinimalRequestCartProduct[] | null ) => {
const planCartItem = getPlanCartItem( cartItems );
if ( planCartItem ) {
recordTracksEvent( 'calypso_signup_plan_select', {
product_slug: selectedPlan?.product_slug,
free_trial: selectedPlan?.free_trial,
Copy link
Contributor Author

@jeyip jeyip Sep 22, 2023

Choose a reason for hiding this comment

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

I remove the free_trial property because, according to tracks registration, it's not a valid event prop for calypso_signup_plan_select. https://github.com/Automattic/tracks-events-registration/blob/master/events/calypso_signup_plan_select.event.json

Copy link
Contributor

Choose a reason for hiding this comment

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

Great finding! Thanks for digging deep into it. There is also a deprecation note from the backend, so I believe it's safe to remove: 31d84-pb.

product_slug: planCartItem?.product_slug,
from_section: 'default',
} );
} else {
Expand All @@ -105,8 +106,8 @@ const PlansWrapper: React.FC< Props > = ( props ) => {
} );
}

setPlanCartItem( selectedPlan );
props.onSubmit?.( selectedPlan );
setPlanCartItem( planCartItem );
props.onSubmit?.( planCartItem );
};

const getPaidDomainName = () => {
Expand Down
15 changes: 15 additions & 0 deletions client/lib/cart-values/cart-items.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
isAkismetProduct,
isWpcomEnterpriseGridPlan,
is100Year,
PLAN_FREE,
} from '@automattic/calypso-products';
import { isWpComProductRenewal as isRenewal } from '@automattic/wpcom-checkout';
import { getTld } from 'calypso/lib/domains';
Expand Down Expand Up @@ -898,3 +899,17 @@ export function hasStaleItem( cart: ObjectWithProducts ): boolean {
);
} );
}

export function getPlanCartItem( cartItems?: MinimalRequestCartProduct[] | null ) {
/**
* A null planCartItem corresponds to a free plan. It seems like this is case throughout the signup/plans
* onboarding codebase. There are, however, tests in client/signup/steps/plans/test/index.jsx that
* represent a free plan as a non null product.
*
* Additionally, free plans are, in fact, represented as products with product slugs elsewhere in the
* codebase. This is why we check for both cases here. When we conduct a more thorough investigation and
* determine that PLAN_FREE is no longer, in fact, used to represent free plans in signup/onboarding, we
* can remove PLAN_FREE check. p4TIVU-aLF-p2
*/
return cartItems?.find( ( item ) => isPlan( item ) || item.product_slug === PLAN_FREE ) ?? null;
}
10 changes: 9 additions & 1 deletion client/lib/signup/flow-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
import { ProgressState } from 'calypso/state/signup/progress/schema';
import { getSignupProgress } from 'calypso/state/signup/progress/selectors';
import { getSiteSlug } from 'calypso/state/sites/selectors';
import { getPlanCartItem } from '../cart-values/cart-items';
import type { Flow, Dependencies } from '../../signup/types';

const debug = debugModule( 'calypso:signup' );
Expand Down Expand Up @@ -442,7 +443,14 @@ export default class SignupFlowController {

_getNeedsToGoThroughCheckout() {
const progress = getSignupDependencyProgress( this._reduxStore.getState() );
return !! progress?.plans?.cartItem;

if ( ! progress?.plans?.cartItems ) {
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we really have to check whether there is a plan in cartItems. Shouldn't we always need to go through checkout if there is anything in the cart? 🤔

Copy link
Contributor Author

@jeyip jeyip Sep 20, 2023

Choose a reason for hiding this comment

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

Ah this discussion was part of the todos I wanted to get to yesterday but didn't have time for #81340 (comment). We briefly touched on this in our chat yesterday

The answer to your question is yes, and there is another check for generic cartItems here 9d67605#diff-a7bd84bdbc16a9cd2414a1ac5c5d990a00617b9f9a6c678a867e1257646b9385R34-R36.

That method is eventually called by filterDestination, which, like _getNeedsToGoThroughCheckout also decides whether or not to redirect to checkout 😵 I haven't dug into why two checks exist

function filterDestination( destination, dependencies, flowName, localeSlug ) {
if ( dependenciesContainCartItem( dependencies ) ) {
return getCheckoutUrl( dependencies, localeSlug, flowName );
}

I was planning to create an issue for this because it feels out of scope, and, in my opinion also seems like it might be a rabbit hole to investigate and fix now ( there's been quite a lot of those in the signup codebase ). Open to hearing your thoughts though 🙂


return (
progress.plans.cartItems.length && Boolean( getPlanCartItem( progress.plans.cartItems ) )
);
}

_destination( dependencies: Dependencies ): string {
Expand Down
70 changes: 35 additions & 35 deletions client/lib/signup/step-actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
supportsPrivacyProtectionPurchase,
planItem as getCartItemForPlan,
marketplaceThemeProduct,
getPlanCartItem,
} from 'calypso/lib/cart-values/cart-items';
import { getLocaleSlug } from 'calypso/lib/i18n-utils';
import { fetchSitesAndUser } from 'calypso/lib/signup/step-actions/fetch-sites-and-user';
Expand Down Expand Up @@ -507,12 +508,12 @@ function findMarketplacePlugin( state, pluginSlug, billingPeriod = '' ) {
}

export function addWithThemePlanToCart( callback, dependencies, stepProvidedItems, reduxStore ) {
const { cartItem } = stepProvidedItems;

const { cartItems } = stepProvidedItems;
const planCartItem = getPlanCartItem( cartItems );
/**
* If the user selected the free option, then we should abort the checkout part.
*/
if ( ! cartItem ) {
if ( ! planCartItem ) {
defer( callback );
return;
}
Expand All @@ -528,7 +529,7 @@ export function addWithThemePlanToCart( callback, dependencies, stepProvidedItem
reduxStore.dispatch,
themeSlug,
dependencies.siteSlug,
cartItem
planCartItem
).then( () => {} );
} else {
addPlanToCart( callback, dependencies, stepProvidedItems, reduxStore );
Expand Down Expand Up @@ -607,8 +608,8 @@ export function addPlanToCart( callback, dependencies, stepProvidedItems, reduxS
// Note that we pull in emailItem to avoid race conditions from multiple step API functions
// trying to fetch and update the cart simultaneously, as both of those actions are asynchronous.
const { emailItem, siteSlug, plugin, billing_period: billingPeriod } = dependencies;
const { cartItem, lastKnownFlow } = stepProvidedItems;
if ( isEmpty( cartItem ) && isEmpty( emailItem ) ) {
const { cartItems, lastKnownFlow } = stepProvidedItems;
if ( isEmpty( cartItems ) && isEmpty( emailItem ) ) {
// the user selected the free plan
defer( callback );

Expand All @@ -620,9 +621,10 @@ export function addPlanToCart( callback, dependencies, stepProvidedItems, reduxS
pluginItem = findMarketplacePlugin( reduxStore.getState(), plugin, billingPeriod );
}

const providedDependencies = { cartItem };
const newCartItems = [ cartItem, emailItem, pluginItem ].filter( ( item ) => item );

const providedDependencies = { cartItems };
const newCartItems = [ ...( cartItems ? cartItems : [] ), emailItem, pluginItem ].filter(
( item ) => item
);
processItemCart( providedDependencies, newCartItems, callback, reduxStore, siteSlug, {
lastKnownFlow,
} );
Expand Down Expand Up @@ -1219,29 +1221,29 @@ export function isPlanFulfilled( stepName, defaultDependencies, nextProps ) {
isDomainTransfer( signupDependencies.domainItem );

if ( isPaidPlan ) {
const cartItem = undefined;
const cartItems = undefined;
submitSignupStep(
{ stepName, cartItem, wasSkipped: true },
{ cartItem, ...dependenciesFromDefaults }
{ stepName, cartItems, wasSkipped: true },
{ cartItems, ...dependenciesFromDefaults }
);
recordExcludeStepEvent( stepName, sitePlanSlug );
fulfilledDependencies.push( 'cartItem' );
fulfilledDependencies.push( 'cartItems' );
} else if ( defaultDependencies && defaultDependencies.cartItem ) {
const cartItem = getCartItemForPlan( defaultDependencies.cartItem );
const cartItems = [ getCartItemForPlan( defaultDependencies.cartItem ) ];
submitSignupStep(
{ stepName, cartItem, wasSkipped: true },
{ cartItem, ...dependenciesFromDefaults }
{ stepName, cartItems, wasSkipped: true },
{ cartItems, ...dependenciesFromDefaults }
);
recordExcludeStepEvent( stepName, defaultDependencies.cartItem );
fulfilledDependencies.push( 'cartItem' );
fulfilledDependencies.push( 'cartItems' );
} else if ( isTransferSelectedInDomainTransferFlow ) {
const cartItem = null;
const cartItems = null;
submitSignupStep(
{ stepName, cartItem, wasSkipped: true },
{ cartItem, ...dependenciesFromDefaults }
{ stepName, cartItems, wasSkipped: true },
{ cartItems, ...dependenciesFromDefaults }
);
recordExcludeStepEvent( stepName, sitePlanSlug );
fulfilledDependencies.push( 'cartItem' );
fulfilledDependencies.push( 'cartItems' );
}

if ( shouldExcludeStep( stepName, fulfilledDependencies ) ) {
Expand Down Expand Up @@ -1270,7 +1272,7 @@ export function isNewOrExistingSiteFulfilled( stepName, defaultDependencies, nex
}
}

export const buildUpgradeFunction = ( planProps, cartItem ) => {
export const buildUpgradeFunction = ( planProps, cartItems ) => {
const {
additionalStepData,
flowName,
Expand All @@ -1282,11 +1284,12 @@ export const buildUpgradeFunction = ( planProps, cartItem ) => {
goToNextStep,
submitSignupStep,
} = planProps;
const planCartItem = getPlanCartItem( cartItems );

if ( cartItem ) {
if ( planCartItem ) {
planProps.recordTracksEvent( 'calypso_signup_plan_select', {
product_slug: cartItem.product_slug,
free_trial: cartItem.free_trial,
product_slug: planCartItem.product_slug,
free_trial: planCartItem.free_trial,
from_section: stepSectionName ? stepSectionName : 'default',
} );

Expand All @@ -1295,9 +1298,9 @@ export const buildUpgradeFunction = ( planProps, cartItem ) => {
// activated at the end of the checkout process.
if (
flowName === 'ecommerce' &&
planHasFeature( cartItem.product_slug, FEATURE_UPLOAD_THEMES_PLUGINS )
planHasFeature( planCartItem.product_slug, FEATURE_UPLOAD_THEMES_PLUGINS )
) {
cartItem.extra = Object.assign( cartItem.extra || {}, {
planCartItem.extra = Object.assign( planCartItem.extra || {}, {
is_store_signup: true,
} );
}
Expand All @@ -1310,28 +1313,25 @@ export const buildUpgradeFunction = ( planProps, cartItem ) => {
const step = {
stepName,
stepSectionName,
cartItem,
cartItems,
...additionalStepData,
};

if ( selectedSite && flowName === 'site-selected' && ! cartItem ) {
submitSignupStep( step, {
cartItem,
} );
if ( selectedSite && flowName === 'site-selected' && ! planCartItem ) {
submitSignupStep( step, { cartItems } );
goToNextStep();
return;
}

const signupVals = {
cartItem,
cartItems,
...( themeSlugWithRepo && { themeSlugWithRepo } ),
...( launchSite && { comingSoon: 0 } ),
};

if ( cartItem && isEcommerce( cartItem ) ) {
if ( planCartItem && isEcommerce( planCartItem ) ) {
signupVals.themeSlugWithRepo = 'pub/twentytwentytwo';
}

submitSignupStep( step, signupVals );
goToNextStep();
};
2 changes: 1 addition & 1 deletion client/lib/signup/test/mocks/signup/config/steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export default {
plans: {
stepName: 'plans',
dependencies: [ 'siteSlug' ],
providesDependencies: [ 'cartItem' ],
providesDependencies: [ 'cartItems' ],
},

'site-type': {
Expand Down
10 changes: 5 additions & 5 deletions client/lib/signup/test/step-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,16 +202,16 @@ describe( 'isPlanFulfilled()', () => {
submitSignupStep,
};
const defaultDependencies = { cartItem: 'testPlan' };
const cartItem = { product_slug: defaultDependencies.cartItem };
const cartItems = [ { product_slug: defaultDependencies.cartItem } ];

expect( flows.excludeStep ).not.toHaveBeenCalled();
expect( submitSignupStep ).not.toHaveBeenCalled();

isPlanFulfilled( stepName, defaultDependencies, nextProps );

expect( submitSignupStep ).toHaveBeenCalledWith(
{ stepName, cartItem, wasSkipped: true },
{ cartItem }
{ stepName, cartItems, wasSkipped: true },
{ cartItems }
);
expect( flows.excludeStep ).toHaveBeenCalledWith( stepName );
} );
Expand All @@ -236,8 +236,8 @@ describe( 'isPlanFulfilled()', () => {
isPlanFulfilled( stepName, undefined, nextProps );

expect( submitSignupStep ).toHaveBeenCalledWith(
{ stepName, cartItem: null, wasSkipped: true },
{ cartItem: null }
{ stepName, cartItems: null, wasSkipped: true },
{ cartItems: null }
);
expect( flows.excludeStep ).toHaveBeenCalledWith( stepName );
} );
Expand Down
30 changes: 15 additions & 15 deletions client/my-sites/add-ons/hooks/use-add-ons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import useAddOnPrices from './use-add-on-prices';
import type { AddOnMeta } from '@automattic/data-stores';

// some memoization. executes far too many times
const useAddOns = ( siteId?: number ): ( AddOnMeta | null )[] => {
const useAddOns = ( siteId?: number, isInSignup = false ): ( AddOnMeta | null )[] => {
const translate = useTranslate();

const addOnsActive = [
Expand Down Expand Up @@ -124,28 +124,28 @@ const useAddOns = ( siteId?: number ): ( AddOnMeta | null )[] => {
// if upgrade is bought - show as manage
// if upgrade is not bought - only show it if available storage and if it's larger than previously bought upgrade
const { data: mediaStorage } = useMediaStorageQuery( siteId );
const { billingTransactions, isLoading } = usePastBillingTransactions();
const { billingTransactions, isLoading } = usePastBillingTransactions( isInSignup );

return useSelector( ( state ): ( AddOnMeta | null )[] => {
// get the list of supported features
const siteFeatures = getFeaturesBySiteId( state, siteId );
const filter = getBillingTransactionFilters( state, 'past' );
const filteredTransactions =
billingTransactions && filterTransactions( billingTransactions, filter, siteId );

const spaceUpgradesPurchased: number[] = [];

if ( filteredTransactions?.length ) {
for ( const transaction of filteredTransactions ) {
transaction.items?.length &&
spaceUpgradesPurchased.push(
...transaction.items
.filter( ( item ) => item.wpcom_product_slug === PRODUCT_1GB_SPACE )
.map( ( item ) => Number( item.licensed_quantity ) )
);
if ( billingTransactions && ! isInSignup ) {
const filter = getBillingTransactionFilters( state, 'past' );
const filteredTransactions = filterTransactions( billingTransactions, filter, siteId );
jeyip marked this conversation as resolved.
Show resolved Hide resolved

if ( filteredTransactions?.length ) {
for ( const transaction of filteredTransactions ) {
transaction.items?.length &&
spaceUpgradesPurchased.push(
...transaction.items
.filter( ( item ) => item.wpcom_product_slug === PRODUCT_1GB_SPACE )
.map( ( item ) => Number( item.licensed_quantity ) )
);
}
}
}

// Determine which Stats Add-On to show based on the site's commercial classification.
const isSiteMarkedCommercial = getSiteOption( state, siteId, 'is_commercial' );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,10 @@ export type UsePricingMetaForGridPlans = ( {
planSlugs,
withoutProRatedCredits,
storageAddOns,
currencyCode,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note:

currencyCode was an unused argument

}: {
planSlugs: PlanSlug[];
withoutProRatedCredits?: boolean;
storageAddOns: ( AddOnMeta | null )[] | null;
currencyCode?: string | null;
} ) => { [ planSlug: string ]: PricingMetaForGridPlan } | null;

// TODO clk: move to types. will consume plan properties
Expand Down
Loading