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

Update Budget Setup Card. #2552

Open
wants to merge 30 commits into
base: update/2535-consolidate-ad-creation-ccf-merged
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a57bf14
Update Budget Setup Card.
ankitrox Aug 22, 2024
273d02b
Fix: e2e tests.
ankitrox Aug 22, 2024
9c30663
Add e2e coverage.
ankitrox Aug 22, 2024
103ae13
CR feedback - round 1.
ankitrox Aug 27, 2024
096c086
Fix: Step change shows no notice.
ankitrox Aug 30, 2024
76328ea
Fix: network error for budget recommendation.
ankitrox Sep 10, 2024
8d9358d
Remove redundant code.
ankitrox Sep 10, 2024
ea68ee8
Update js doc.
ankitrox Sep 10, 2024
dcc6f73
Resolve conflicts and fix tests.
ankitrox Sep 10, 2024
ac68177
Fix: lint issues.
ankitrox Sep 10, 2024
052783e
Remove unused code.
ankitrox Sep 10, 2024
c2fbf1a
Resolve conflict with base branch.
ankitrox Sep 12, 2024
61d73eb
use fetch budget hook in paid-ads-setup-sections.
ankitrox Sep 12, 2024
f15bfb9
Remove redundant code.
ankitrox Sep 12, 2024
85f5b35
Display spinner till data gets loaded.
ankitrox Sep 12, 2024
566476d
Add hook for code resuse.
ankitrox Sep 16, 2024
dc7a4ac
Use hook.
ankitrox Sep 16, 2024
9ddfef4
Stick to old UI/UX.
ankitrox Sep 16, 2024
4af37a3
Remove redundant code.
ankitrox Sep 16, 2024
1b92ecb
Pass recommendations prop.
ankitrox Sep 16, 2024
ab2db99
Remove redundant code.
ankitrox Sep 16, 2024
983438b
Merge branch 'update/2535-consolidate-ad-creation-ccf-merged' into up…
asvinb Oct 2, 2024
eb18178
Try not to use the useBudgetRecommendationData hook.
asvinb Oct 2, 2024
35716f0
Add useFetchBudgetRecommendation hook.
asvinb Oct 2, 2024
0cafdb2
Make use of new hook and add loading indicator.
asvinb Oct 2, 2024
79ccefa
Remove unused hook.
asvinb Oct 2, 2024
fbde5f4
Move clientSession.
asvinb Oct 2, 2024
3ea1c12
Restore complete campaign data from client session.
asvinb Oct 2, 2024
f6f7ac3
Merge pull request #2637 from woocommerce/update/2502-budget-setup-ca…
joemcgill Oct 2, 2024
7aaa968
Set minimum amount to always be the recommended budget.
asvinb Oct 7, 2024
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
24 changes: 23 additions & 1 deletion js/src/components/paid-ads/ads-campaign.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/* eslint-disable react-hooks/exhaustive-deps */
joemcgill marked this conversation as resolved.
Show resolved Hide resolved
/**
* External dependencies
*/
import { __ } from '@wordpress/i18n';
import { createInterpolateElement } from '@wordpress/element';
import { createInterpolateElement, useEffect } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -17,6 +18,8 @@
import BudgetSection from './budget-section';
import { CampaignPreviewCard } from './campaign-preview';
import FaqsSection from './faqs-section';
import useBudgetRecommendationData from '.~/hooks/useBudgetRecommendationData';
import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession';

/**
* @typedef {import('.~/data/actions').Campaign} Campaign
Expand Down Expand Up @@ -55,6 +58,21 @@
'google-listings-and-ads'
);

const {
country,
dailyBudget,
multipleRecommendations,
recommendations,
loading,
} = useBudgetRecommendationData( formContext.values.countryCodes );

Check warning on line 67 in js/src/components/paid-ads/ads-campaign.js

View check run for this annotation

Codecov / codecov/patch

js/src/components/paid-ads/ads-campaign.js#L67

Added line #L67 was not covered by tests

useEffect( () => {

Check warning on line 69 in js/src/components/paid-ads/ads-campaign.js

View check run for this annotation

Codecov / codecov/patch

js/src/components/paid-ads/ads-campaign.js#L69

Added line #L69 was not covered by tests
asvinb marked this conversation as resolved.
Show resolved Hide resolved
if ( ! loading ) {
const { amount } = clientSession.getCampaign() || {};
formContext.setValue( 'amount', amount || dailyBudget );
}
}, [ loading ] );
joemcgill marked this conversation as resolved.
Show resolved Hide resolved

return (
<StepContent>
<StepContentHeader
Expand Down Expand Up @@ -95,6 +113,10 @@
formProps={ formContext }
disabled={ disabledBudgetSection }
countryCodes={ formContext.values.countryCodes }
country={ country }
dailyBudget={ dailyBudget }
isMultiple={ multipleRecommendations }
recommendations={ recommendations }
>
<CampaignPreviewCard />
</BudgetSection>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,19 @@
* Internal dependencies
*/
import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap';
import useFetchBudgetRecommendationEffect from './useFetchBudgetRecommendationEffect';
import './index.scss';

/*
* If a merchant selects more than one country, the budget recommendation
* takes the highest country out from the selected countries.
*
* For example, a merchant selected Brunei (20 USD) and Croatia (15 USD),
* then the budget recommendation should be (20 USD).
*/
function getHighestBudget( recommendations ) {
return recommendations.reduce( ( defender, challenger ) => {
if ( challenger.daily_budget > defender.daily_budget ) {
return challenger;
}
return defender;
} );
}

function toRecommendationRange( isMultiple, ...values ) {
const conversionMap = { strong: <strong />, em: <em />, br: <br /> };
const template = isMultiple
? // translators: it's a range of recommended budget amount. 1: the value of the budget, 2: the currency of amount.
__(
'Google will optimize your ads to maximize performance across the country/s you select.<br /><em>Tip: Most merchants targeting similar countries <strong>set a daily budget of %1$f %2$s</strong></em>',
'We recommend running campaigns at least 1 month so it can learn to optimize for your business.<br /><em>Tip: Most merchants targeting similar countries <strong>set a daily budget of %1$f %2$s</strong></em>',
'google-listings-and-ads'
)
: // translators: it's a range of recommended budget amount. 1: the value of the budget, 2: the currency of amount 3: a country name selected by the merchant.
__(
'Google will optimize your ads to maximize performance across the country/s you select.<br /><em>Tip: Most merchants targeting <strong>%3$s set a daily budget of %1$f %2$s</strong></em>',
'We recommend running campaigns at least 1 month so it can learn to optimize for your business.<br /><em>Tip: Most merchants targeting <strong>%3$s set a daily budget of %1$f %2$s</strong></em>',
'google-listings-and-ads'
);

Expand All @@ -50,21 +33,19 @@
}

const BudgetRecommendation = ( props ) => {
const { countryCodes, dailyAverageCost = Infinity } = props;
const { data } = useFetchBudgetRecommendationEffect( countryCodes );
const map = useCountryKeyNameMap();

if ( ! data ) {
return null;
}
const {
dailyAverageCost = Infinity,

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L37 was not covered by tests
currency,
country,
dailyBudget,
isMultiple,
} = props;

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L42 was not covered by tests

const { currency, recommendations } = data;
const { daily_budget: dailyBudget, country } =
getHighestBudget( recommendations );
const map = useCountryKeyNameMap();

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L44 was not covered by tests

const countryName = map[ country ];
const recommendationRange = toRecommendationRange(
recommendations.length > 1,
isMultiple,
dailyBudget,
currency,
countryName
Expand Down
23 changes: 10 additions & 13 deletions js/src/components/paid-ads/budget-section/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import { __ } from '@wordpress/i18n';
import { useEffect, useRef } from '@wordpress/element';
import { useRef } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -29,15 +29,19 @@
*
* @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 {string} props.country Country code.
* @param {Array<CountryCode>} props.countryCodes Country codes to fetch budget recommendations for.
* @param {number} props.dailyBudget Daily budget.
* @param {boolean} [props.disabled=false] Whether display the Card in disabled style.
* @param {boolean} [props.isMultiple=false] Whether the campaign is targeting multiple countries.
* @param {JSX.Element} [props.children] Extra content to be rendered under the card of budget inputs.
*/
const BudgetSection = ( {
formProps,
countryCodes,
disabled = false,
children,
...recommendationProps
} ) => {
const { getInputProps, setValue, values } = formProps;
const { amount } = values;
Expand All @@ -52,16 +56,7 @@
const setValueRef = useRef();
setValueRef.current = setValue;

/**
* In addition to the initial value setting during initialization, when `disabled` changes
* - from false to true, then clear filled amount to `undefined` for showing a blank <input>.
* - from true to false, then reset amount to the initial value passed from the consumer side.
*/
const initialAmountRef = useRef( amount );
useEffect( () => {
const nextAmount = disabled ? undefined : initialAmountRef.current;
setValueRef.current( 'amount', nextAmount );
}, [ disabled ] );
const { recommendations } = recommendationProps || {};

return (
<div className="gla-budget-section">
Expand Down Expand Up @@ -99,10 +94,12 @@
value={ monthlyMaxEstimated }
/>
</div>
{ countryCodes?.length > 0 && (
{ recommendations?.length > 0 && (

Check warning on line 97 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#L97

Added line #L97 was not covered by tests
<BudgetRecommendation
countryCodes={ countryCodes }
dailyAverageCost={ amount }
currency={ currency }
asvinb marked this conversation as resolved.
Show resolved Hide resolved
{ ...recommendationProps }
/>
) }
</Section.Card.Body>
Expand Down
45 changes: 45 additions & 0 deletions js/src/hooks/useBudgetRecommendationData.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* External dependencies
*/
import { useEffect, useState } from '@wordpress/element';

/**
* Internal dependencies
*/
import useFetchBudgetRecommendationEffect from './useFetchBudgetRecommendationEffect';
import getHighestBudget from '.~/utils/getHighestBudget';

const useBudgetRecommendationData = ( countryCodes ) => {
const { data, loading } =
useFetchBudgetRecommendationEffect( countryCodes );

Check warning on line 14 in js/src/hooks/useBudgetRecommendationData.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useBudgetRecommendationData.js#L14

Added line #L14 was not covered by tests

const [ country, setCountry ] = useState( '' );
const [ dailyBudget, setDailyBudget ] = useState( 0 );

Check warning on line 17 in js/src/hooks/useBudgetRecommendationData.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useBudgetRecommendationData.js#L16-L17

Added lines #L16 - L17 were not covered by tests
asvinb marked this conversation as resolved.
Show resolved Hide resolved
const [ multipleRecommendations, setMultipleRecommendations ] =
useState( false );
const [ recommendations, setRecommendations ] = useState( [] );

Check warning on line 20 in js/src/hooks/useBudgetRecommendationData.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useBudgetRecommendationData.js#L19-L20

Added lines #L19 - L20 were not covered by tests

useEffect( () => {

Check warning on line 22 in js/src/hooks/useBudgetRecommendationData.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useBudgetRecommendationData.js#L22

Added line #L22 was not covered by tests
if ( ! loading ) {
const {
country: budgetCountries = '',

Check warning on line 25 in js/src/hooks/useBudgetRecommendationData.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useBudgetRecommendationData.js#L25

Added line #L25 was not covered by tests
asvinb marked this conversation as resolved.
Show resolved Hide resolved
daily_budget: recommendedDailyBudget,
} = getHighestBudget( data?.recommendations );

Check warning on line 27 in js/src/hooks/useBudgetRecommendationData.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useBudgetRecommendationData.js#L27

Added line #L27 was not covered by tests

setCountry( budgetCountries );
setDailyBudget( recommendedDailyBudget );
setMultipleRecommendations( data?.recommendations.length > 1 );
asvinb marked this conversation as resolved.
Show resolved Hide resolved
setRecommendations( data?.recommendations );

Check warning on line 32 in js/src/hooks/useBudgetRecommendationData.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useBudgetRecommendationData.js#L29-L32

Added lines #L29 - L32 were not covered by tests
}
}, [ data?.recommendations, loading ] );

return {

Check warning on line 36 in js/src/hooks/useBudgetRecommendationData.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useBudgetRecommendationData.js#L36

Added line #L36 was not covered by tests
country,
dailyBudget,
multipleRecommendations,
recommendations,
loading,
};
};

export default useBudgetRecommendationData;
7 changes: 7 additions & 0 deletions js/src/pages/create-paid-ads-campaign/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
recordStepperChangeEvent,
recordStepContinueEvent,
} from '.~/utils/tracks';
import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession';

const eventName = 'gla_paid_campaign_step';
const eventContext = 'create-ads';
Expand Down Expand Up @@ -133,6 +134,12 @@
countryCodes: initialCountryCodes,
} }
onSubmit={ handleSubmit }
onChange={ ( _, values, isValid ) => {

Check warning on line 137 in js/src/pages/create-paid-ads-campaign/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/pages/create-paid-ads-campaign/index.js#L137

Added line #L137 was not covered by tests
// Set the amount in session storage.
if ( isValid && _?.name === 'amount' ) {
clientSession.setCampaign( { amount: _.value } );

Check warning on line 140 in js/src/pages/create-paid-ads-campaign/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/pages/create-paid-ads-campaign/index.js#L140

Added line #L140 was not covered by tests
}
} }
>
<Stepper
currentStep={ step }
Expand Down
8 changes: 7 additions & 1 deletion js/src/setup-ads/setup-ads-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import CampaignAssetsForm from '.~/components/paid-ads/campaign-assets-form';
import AdsStepper from './ads-stepper';
import SetupAdsTopBar from './top-bar';
import { recordGlaEvent } from '.~/utils/tracks';
import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession';

/**
* @fires gla_launch_paid_campaign_button_click on submit
Expand Down Expand Up @@ -67,14 +68,19 @@ const SetupAdsForm = () => {
} );
};

const handleChange = ( _, values ) => {
const handleChange = ( _, values, isValid ) => {
const args = [ initialValues, values ].map(
( { countryCodes, ...v } ) => {
v.countrySet = new Set( countryCodes );
return v;
}
);

// Set the amount in session storage.
if ( isValid && _?.name === 'amount' ) {
asvinb marked this conversation as resolved.
Show resolved Hide resolved
clientSession.setCampaign( { amount: _.value } );
}

setFormChanged( ! isEqual( ...args ) );
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import Section from '.~/wcdl/section';
import validateCampaign from '.~/components/paid-ads/validateCampaign';
import clientSession from './clientSession';
import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants';
import useBudgetRecommendationData from '.~/hooks/useBudgetRecommendationData';

/**
* @typedef {import('.~/data/actions').CountryCode} CountryCode
Expand Down Expand Up @@ -76,6 +77,14 @@ export default function PaidAdsSetupSections( {
const isBillingCompleted =
billingStatus?.status === GOOGLE_ADS_BILLING_STATUS.APPROVED;

const {
country,
dailyBudget,
recommendations,
multipleRecommendations,
loading,
} = useBudgetRecommendationData( countryCodes );

/*
If a merchant has not yet finished the billing setup, the billing status will be
updated by `useAutoCheckBillingStatusEffect` hook in `BillingSetupCard` component
Expand All @@ -91,15 +100,22 @@ export default function PaidAdsSetupSections( {
For example, refresh page during onboarding flow after the billing setup is finished.
*/
useEffect( () => {
const nextPaidAds = {
...paidAds,
isReady: paidAds.isValid && isBillingCompleted,
};
onStatesReceivedRef.current( nextPaidAds );
clientSession.setCampaign( nextPaidAds );
}, [ paidAds, isBillingCompleted ] );

if ( ! billingStatus ) {
if ( ! loading ) {
const sessionCampaign = clientSession.getCampaign();
const sessionAmount = sessionCampaign?.amount;

const nextPaidAds = {
...paidAds,
amount: sessionAmount || dailyBudget,
isReady: paidAds.isValid && isBillingCompleted,
};

onStatesReceivedRef.current( nextPaidAds );
clientSession.setCampaign( nextPaidAds );
}
}, [ dailyBudget, paidAds, isBillingCompleted, loading ] );

if ( ! billingStatus || loading ) {
return (
<Section>
<SpinnerCard />
Expand All @@ -108,14 +124,18 @@ export default function PaidAdsSetupSections( {
}

const initialValues = {
amount: paidAds.amount,
amount: clientSession.getCampaign()?.amount || dailyBudget,
};

return (
<Form
initialValues={ initialValues }
onChange={ ( _, values, isValid ) => {
setPaidAds( { ...paidAds, ...values, isValid } );

if ( isValid ) {
clientSession.setCampaign( values );
asvinb marked this conversation as resolved.
Show resolved Hide resolved
}
} }
validate={ validateCampaign }
>
Expand All @@ -124,6 +144,10 @@ export default function PaidAdsSetupSections( {
<BudgetSection
formProps={ formProps }
countryCodes={ countryCodes }
country={ country }
dailyBudget={ dailyBudget }
isMultiple={ multipleRecommendations }
recommendations={ recommendations }
>
<BillingCard />
</BudgetSection>
Expand Down
10 changes: 6 additions & 4 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 @@ -171,10 +171,12 @@ export default function SetupPaidAds() {
<PaidAdsFeaturesSection
hideBudgetContent={ ! hasGoogleAdsConnection }
/>
<PaidAdsSetupSections
onStatesReceived={ setPaidAds }
countryCodes={ countryCodes }
/>
{ countryCodes?.length > 0 && (
<PaidAdsSetupSections
onStatesReceived={ setPaidAds }
countryCodes={ countryCodes }
/>
) }
<FaqsSection />

{ showSkipPaidAdsConfirmationModal && (
Expand Down
Loading