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

2501: Remove Audience section from paids ads setup. #2551

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,3 @@ exports[`validateCampaign When the amount is not a number, should not pass 4`] =
exports[`validateCampaign When the amount is ≤ 0, should not pass 1`] = `"Please make sure daily average cost is greater than 0."`;

exports[`validateCampaign When the amount is ≤ 0, should not pass 2`] = `"Please make sure daily average cost is greater than 0."`;

exports[`validateCampaign When the country codes array is empty, should not pass 1`] = `"Please select at least one country for your ads campaign."`;
1 change: 1 addition & 0 deletions js/src/components/paid-ads/ads-campaign.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export default function AdsCampaign( {
<BudgetSection
formProps={ formContext }
disabled={ disabledBudgetSection }
countryCodes={ formContext.values.countryCodes }
>
<CampaignPreviewCard />
</BudgetSection>
Expand Down
16 changes: 13 additions & 3 deletions js/src/components/paid-ads/budget-section/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
import AppInputPriceControl from '.~/components/app-input-price-control';

/**
* @typedef {import('.~/data/actions').CountryCode} CountryCode
*/

const nonInteractableProps = {
noPointerEvents: true,
readOnly: true,
Expand All @@ -25,12 +29,18 @@
*
* @param {Object} props React props.
* @param {Object} props.formProps Form props forwarded from `Form` component.
* @param {Array<CountryCode>|undefined} props.countryCodes Country codes to fetch budget recommendations for.
* @param {boolean} [props.disabled=false] Whether display the Card in disabled style.
* @param {JSX.Element} [props.children] Extra content to be rendered under the card of budget inputs.
*/
const BudgetSection = ( { formProps, disabled = false, children } ) => {
const BudgetSection = ( {
formProps,
countryCodes,
disabled = false,

Check warning on line 39 in js/src/components/paid-ads/budget-section/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/components/paid-ads/budget-section/index.js#L39

Added line #L39 was not covered by tests
children,
} ) => {

Check warning on line 41 in js/src/components/paid-ads/budget-section/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/components/paid-ads/budget-section/index.js#L41

Added line #L41 was not covered by tests
const { getInputProps, setValue, values } = formProps;
const { countryCodes, amount } = values;
const { amount } = values;

Check warning on line 43 in js/src/components/paid-ads/budget-section/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/components/paid-ads/budget-section/index.js#L43

Added line #L43 was not covered by tests
const { googleAdsAccount } = useGoogleAdsAccount();
const monthlyMaxEstimated = getMonthlyMaxEstimated( amount );
// Display the currency code that will be used by Google Ads, but still use the store's currency formatting settings.
Expand Down Expand Up @@ -89,7 +99,7 @@
value={ monthlyMaxEstimated }
/>
</div>
{ countryCodes.length > 0 && (
{ countryCodes?.length > 0 && (

Check warning on line 102 in js/src/components/paid-ads/budget-section/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/components/paid-ads/budget-section/index.js#L102

Added line #L102 was not covered by tests
<BudgetRecommendation
countryCodes={ countryCodes }
dailyAverageCost={ amount }
Expand Down
7 changes: 0 additions & 7 deletions js/src/components/paid-ads/validateCampaign.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ import { __ } from '@wordpress/i18n';
const validateCampaign = ( values ) => {
const errors = {};

if ( values.countryCodes.length === 0 ) {
errors.countryCodes = __(
'Please select at least one country for your ads campaign.',
'google-listings-and-ads'
);
}

if ( ! Number.isFinite( values.amount ) || values.amount <= 0 ) {
errors.amount = __(
'Please make sure daily average cost is greater than 0.',
Expand Down
11 changes: 1 addition & 10 deletions js/src/components/paid-ads/validateCampaign.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ describe( 'validateCampaign', () => {

beforeEach( () => {
// Initial values
values = { countryCodes: [], amount: 0 };
values = { amount: 0 };
} );

it( 'When all checks are passed, should return an empty object', () => {
const errors = validateCampaign( {
countryCodes: [ 'US' ],
amount: 1,
} );

Expand All @@ -27,17 +26,9 @@ describe( 'validateCampaign', () => {
it( 'should indicate multiple unpassed checks by setting properties in the returned object', () => {
const errors = validateCampaign( values );

expect( errors ).toHaveProperty( 'countryCodes' );
expect( errors ).toHaveProperty( 'amount' );
} );

it( 'When the country codes array is empty, should not pass', () => {
const errors = validateCampaign( values );

expect( errors ).toHaveProperty( 'countryCodes' );
expect( errors.countryCodes ).toMatchSnapshot();
} );

it( 'When the amount is not a number, should not pass', () => {
let errors;

Expand Down
8 changes: 2 additions & 6 deletions js/src/setup-mc/setup-stepper/setup-paid-ads/clientSession.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
/**
* @typedef { import(".~/data/actions").CountryCode } CountryCode
*
* @typedef {Object} CampaignData
* @property {number|undefined} amount Daily average cost of the paid ads campaign.
* @property {Array<CountryCode>} countryCodes Audience country codes of the paid ads campaign. Example: 'US'.
*/

const KEY_PAID_ADS = 'gla-onboarding-paid-ads';
Expand All @@ -14,10 +11,9 @@ const clientSession = {
/**
* @param {CampaignData} data Campaign data to be stored.
* @param {number|undefined} data.amount Daily average cost of the campaign.
* @param {Array<CountryCode>} data.countryCodes Country codes of the campaign.
*/
setCampaign( { amount, countryCodes } ) {
const json = JSON.stringify( { amount, countryCodes } );
setCampaign( { amount } ) {
const json = JSON.stringify( { amount } );
sessionStorage.setItem( KEY_PAID_ADS, json );
},

Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
/**
* External dependencies
*/
import { __ } from '@wordpress/i18n';
import { useState, useRef, useEffect } from '@wordpress/element';
import { Form } from '@woocommerce/components';

/**
* Internal dependencies
*/
import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes';
import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus';
import AudienceSection from '.~/components/paid-ads/audience-section';
import BudgetSection from '.~/components/paid-ads/budget-section';
import BillingCard from '.~/components/paid-ads/billing-card';
import SpinnerCard from '.~/components/spinner-card';
Expand All @@ -21,62 +17,48 @@ import clientSession from './clientSession';
import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants';

/**
* @typedef { import(".~/data/actions").CountryCode } CountryCode
*
* @typedef {import('.~/data/actions').CountryCode} CountryCode
*/

/**
* @typedef {Object} PaidAdsData
* @property {number|undefined} amount Daily average cost of the paid ads campaign.
* @property {Array<CountryCode>} countryCodes Audience country codes of the paid ads campaign. Example: 'US'.
* @property {boolean} isValid Whether the campaign data are valid values.
* @property {boolean} isReady Whether the campaign data and the billing setting are ready for completing the paid ads setup.
*/

const defaultPaidAds = {
amount: 0,
countryCodes: [],
isValid: false,
isReady: false,
};

/**
* Resolve the initial paid ads data from the given paid ads data with the loaded target audience.
* Resolve the initial paid ads data from the given paid ads data.
* Parts of the resolved data are used in the `initialValues` prop of `Form` component.
*
* @param {PaidAdsData} paidAds The paid ads data as the base to be resolved with other states.
* @param {Array<CountryCode>} targetAudience Country codes of selected target audience.
* @return {PaidAdsData} The resolved paid ads data.
*/
function resolveInitialPaidAds( paidAds, targetAudience ) {
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
function resolveInitialPaidAds( paidAds ) {
const nextPaidAds = { ...paidAds };

if ( targetAudience ) {
if ( nextPaidAds.countryCodes === defaultPaidAds.countryCodes ) {
// Replace the country codes with the loaded target audience only if the reference is
// the same as the default because the given country codes might be the restored ones.
nextPaidAds.countryCodes = targetAudience;
} else {
// The selected target audience may be changed back and forth during the onboarding flow.
// Remove countries if any don't exist in the latest state.
nextPaidAds.countryCodes = nextPaidAds.countryCodes.filter(
( code ) => targetAudience.includes( code )
);
}
}

nextPaidAds.isValid = ! Object.keys( validateCampaign( nextPaidAds ) )
.length;

return nextPaidAds;
}

/**
* Renders sections of Google Ads account, audience, budget, and billing for setting up the paid ads.
* Renders sections of Google Ads account, budget and billing for setting up the paid ads.
*
* @param {Object} props React props.
* @param {(onStatesReceived: PaidAdsData)=>void} props.onStatesReceived Callback to receive the data for setting up paid ads when initial and also when the audience, budget, and billing are updated.
* @param {(onStatesReceived: PaidAdsData)=>void} props.onStatesReceived Callback to receive the data for setting up paid ads when initial and also when the budget and billing are updated.
* @param {Array<CountryCode>|undefined} props.countryCodes Country codes for the campaign.
*/
export default function PaidAdsSetupSections( { onStatesReceived } ) {
const { hasGoogleAdsConnection } = useGoogleAdsAccount();
const { data: targetAudience } = useTargetAudienceFinalCountryCodes();
export default function PaidAdsSetupSections( {
onStatesReceived,
countryCodes,
} ) {
const { billingStatus } = useGoogleAdsAccountBillingStatus();

const onStatesReceivedRef = useRef();
Expand All @@ -88,7 +70,7 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) {
...defaultPaidAds,
...clientSession.getCampaign(),
};
return resolveInitialPaidAds( startingPaidAds, targetAudience );
return resolveInitialPaidAds( startingPaidAds );
} );

const isBillingCompleted =
Expand Down Expand Up @@ -117,22 +99,7 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) {
clientSession.setCampaign( nextPaidAds );
}, [ paidAds, isBillingCompleted ] );

/*
Resolve the initial states after the `targetAudience` is loaded.

Please note that the loaded `targetAudience` is NOT expected to have further changes
in the runtime. If it happens one day and it will need to update <Form>'s internal state
with the changed `targetAudience`, please refer to the following practice.
- https://github.com/woocommerce/google-listings-and-ads/blob/5b6522ca10ad75556e6b2de7c120cc712aab70b1/js/src/components/free-listings/setup-free-listings/index.js#L120-L134
- https://github.com/woocommerce/google-listings-and-ads/blob/5b6522ca10ad75556e6b2de7c120cc712aab70b1/js/src/components/free-listings/setup-free-listings/index.js#L172-L186
*/
useEffect( () => {
setPaidAds( ( currentPaidAds ) =>
resolveInitialPaidAds( currentPaidAds, targetAudience )
);
}, [ targetAudience ] );

if ( ! targetAudience || ! billingStatus ) {
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
if ( ! billingStatus ) {
return (
<Section>
<SpinnerCard />
Expand All @@ -141,7 +108,6 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) {
}

const initialValues = {
countryCodes: paidAds.countryCodes,
amount: paidAds.amount,
};

Expand All @@ -154,28 +120,13 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) {
validate={ validateCampaign }
>
{ ( formProps ) => {
const { countryCodes } = formProps.values;
const disabledAudience = ! hasGoogleAdsConnection;
const disabledBudget =
disabledAudience || countryCodes.length === 0;

return (
<>
<AudienceSection
formProps={ formProps }
disabled={ disabledAudience }
countrySelectHelperText={ __(
'You can only choose from countries you’ve selected during product listings configuration.',
'google-listings-and-ads'
) }
/>
<BudgetSection
formProps={ formProps }
disabled={ disabledBudget }
>
{ ! disabledBudget && <BillingCard /> }
</BudgetSection>
</>
<BudgetSection
formProps={ formProps }
countryCodes={ countryCodes }
>
<BillingCard />
</BudgetSection>
);
} }
</Form>
Expand Down
11 changes: 8 additions & 3 deletions js/src/setup-mc/setup-stepper/setup-paid-ads/setup-paid-ads.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import useAdminUrl from '.~/hooks/useAdminUrl';
import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices';
import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
import useAdsSetupCompleteCallback from '.~/hooks/useAdsSetupCompleteCallback';
import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes';
import StepContent from '.~/components/stepper/step-content';
import StepContentHeader from '.~/components/stepper/step-content-header';
import StepContentFooter from '.~/components/stepper/step-content-footer';
Expand Down Expand Up @@ -66,6 +67,7 @@ import { recordGlaEvent } from '.~/utils/tracks';
export default function SetupPaidAds() {
const adminUrl = useAdminUrl();
const { createNotice } = useDispatchCoreNotices();
const { data: countryCodes } = useTargetAudienceFinalCountryCodes();
const { googleAdsAccount, hasGoogleAdsConnection } = useGoogleAdsAccount();
const [ handleSetupComplete ] = useAdsSetupCompleteCallback();
const [ paidAds, setPaidAds ] = useState( {} );
Expand Down Expand Up @@ -105,7 +107,7 @@ export default function SetupPaidAds() {
const onBeforeFinish = handleSetupComplete.bind(
null,
paidAds.amount,
paidAds.countryCodes
countryCodes
);
await finishOnboardingSetup( event, onBeforeFinish );
};
Expand Down Expand Up @@ -169,7 +171,10 @@ export default function SetupPaidAds() {
<PaidAdsFeaturesSection
hideBudgetContent={ ! hasGoogleAdsConnection }
/>
<PaidAdsSetupSections onStatesReceived={ setPaidAds } />
<PaidAdsSetupSections
onStatesReceived={ setPaidAds }
countryCodes={ countryCodes }
/>
<FaqsSection />

{ showSkipPaidAdsConfirmationModal && (
Expand Down Expand Up @@ -200,7 +205,7 @@ export default function SetupPaidAds() {
eventName="gla_onboarding_complete_with_paid_ads_button_click"
eventProps={ {
budget: paidAds.amount,
audiences: paidAds.countryCodes?.join( ',' ),
audiences: countryCodes?.join( ',' ),
} }
/>
</Flex>
Expand Down
Loading