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

Merged
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
49 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
106c325
Merge branch 'update/2535-consolidate-ad-creation-ccf-merged' into up…
asvinb Oct 14, 2024
8bf78f1
Simplify setting up minimum budget recommendation.
asvinb Oct 14, 2024
bfb1021
Update tests.
asvinb Oct 14, 2024
b655b88
Merge branch 'update/2535-consolidate-ad-creation-ccf-merged' into up…
asvinb Oct 15, 2024
76060e6
Merge branch 'update/2535-consolidate-ad-creation-ccf-merged' into up…
asvinb Oct 16, 2024
69381e3
Merge branch 'feature/2459-campaign-creation-flow' into update/2502-b…
asvinb Oct 16, 2024
e712a2c
Merge branch 'feature/2459-campaign-creation-flow' into update/2502-b…
asvinb Oct 17, 2024
48aeec3
Address CR feedback.
asvinb Oct 17, 2024
fb79ef6
Add invalidation condition.
asvinb Oct 18, 2024
ede72e3
Set value in BudgetSection.
asvinb Oct 18, 2024
fc07258
Add comment.
asvinb Oct 18, 2024
121fc29
Pass props.
asvinb Oct 18, 2024
f194da1
Add extra condition.
asvinb Oct 18, 2024
2ce27c1
Revert changes.
asvinb Oct 18, 2024
f4631e9
Remove unused import.
asvinb Oct 18, 2024
8c074a4
Revert changes.
asvinb Oct 22, 2024
0495e18
Re order components.
asvinb Oct 22, 2024
9067960
Remove check for Google Ads account.
asvinb Oct 22, 2024
a560e90
Add missing JSDoc.
asvinb Oct 22, 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
1 change: 1 addition & 0 deletions js/src/components/paid-ads/ads-campaign/ads-campaign.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export default function AdsCampaign( {
? campaign.displayCountries
: countryCodes
}
context={ context }
>
{ showBillingCard && <BillingCard /> }

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.<br /><em>Tip: Most merchants who sell products in 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.<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,27 +33,29 @@
}

const BudgetRecommendation = ( props ) => {
const { countryCodes, dailyAverageCost = Infinity } = props;
const { data } = useFetchBudgetRecommendationEffect( countryCodes );
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
highestDailyBudget,
highestDailyBudgetCountryCode,
budgetRecommendationData,
} = props;

Check warning on line 41 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#L41

Added line #L41 was not covered by tests

const map = useCountryKeyNameMap();

if ( ! data ) {
if ( ! budgetRecommendationData ) {
return null;
}

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

const countryName = map[ country ];
const { currency, recommendations } = budgetRecommendationData;
const countryName = map[ highestDailyBudgetCountryCode ];

Check warning on line 50 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#L49-L50

Added lines #L49 - L50 were not covered by tests
const recommendationRange = toRecommendationRange(
recommendations.length > 1,
dailyBudget,
highestDailyBudget,
currency,
countryName
);

const showLowerBudgetNotice = dailyAverageCost < dailyBudget;
const showLowerBudgetNotice = dailyAverageCost < highestDailyBudget;

Check warning on line 58 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#L58

Added line #L58 was not covered by tests

return (
<div className="gla-budget-recommendation">
Expand Down

This file was deleted.

118 changes: 78 additions & 40 deletions js/src/components/paid-ads/budget-section/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
import BudgetRecommendation from './budget-recommendation';
import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
import AppInputPriceControl from '.~/components/app-input-price-control';
import AppSpinner from '.~/components/app-spinner';
import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation';
import clientSession from './clientSession';

/**
* @typedef {import('.~/data/actions').CountryCode} CountryCode
Expand All @@ -32,36 +35,56 @@
* @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.
* @param {'create-ads'|'edit-ads'|'setup-ads'|'setup-mc'} props.context A context indicating which page this component is used on.
*/
const BudgetSection = ( {
formProps,
countryCodes,
disabled = false,
children,
context,
} ) => {
const { getInputProps, setValue, values } = formProps;
const { getInputProps, values, setValue } = formProps;

Check warning on line 47 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#L47

Added line #L47 was not covered by tests
const { amount } = values;
const { googleAdsAccount } = useGoogleAdsAccount();
const {
data: budgetRecommendationData,
highestDailyBudget,
highestDailyBudgetCountryCode,
hasFinishedResolution,
} = useFetchBudgetRecommendation( countryCodes );
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
const initialAmountRef = useRef( null );

Check warning on line 56 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#L55-L56

Added lines #L55 - L56 were not covered by tests
const monthlyMaxEstimated = getMonthlyMaxEstimated( amount );
// Display the currency code that will be used by Google Ads, but still use the store's currency formatting settings.
const currency = googleAdsAccount?.currency;

// Wrapping `useRef` is because since WC 6.9, the reference of `setValue` may be changed
// after calling itself and further leads to an infinite re-rendering loop if used in a
// `useEffect`.
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 );
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
}, [ disabled ] );
// Load the amount from client session during the onboarding flow only.
if (
context !== 'setup-mc' ||
! hasFinishedResolution ||
! values.amount

Check warning on line 66 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#L64-L66

Added lines #L64 - L66 were not covered by tests
) {
return;

Check warning on line 68 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#L68

Added line #L68 was not covered by tests
}

if ( values.amount >= highestDailyBudget ) {
clientSession.setCampaign( values );

Check warning on line 72 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#L72

Added line #L72 was not covered by tests
}
}, [ values, highestDailyBudget, context, hasFinishedResolution ] );

if ( ! initialAmountRef.current && ! amount && hasFinishedResolution ) {
let clientSessionAmount = 0;

Check warning on line 77 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#L77

Added line #L77 was not covered by tests
if ( context === 'setup-mc' ) {
( { amount: clientSessionAmount } = clientSession.getCampaign() );

Check warning on line 79 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#L79

Added line #L79 was not covered by tests
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
}

initialAmountRef.current = true;
setValue(

Check warning on line 83 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#L82-L83

Added lines #L82 - L83 were not covered by tests
'amount',
Math.max( clientSessionAmount, highestDailyBudget )
);
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
}

return (
<div className="gla-budget-section">
Expand All @@ -79,31 +102,46 @@
>
<Section.Card>
<Section.Card.Body className="gla-budget-section__card-body">
<div className="gla-budget-section__card-body__cost">
<AppInputPriceControl
label={ __(
'Daily average cost',
'google-listings-and-ads'
) }
suffix={ currency }
{ ...getInputProps( 'amount' ) }
{ ...( disabled && nonInteractableProps ) }
/>
<AppInputPriceControl
disabled
label={ __(
'Monthly max, estimated',
'google-listings-and-ads'
{ hasFinishedResolution ? (
<>

Check warning on line 106 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#L106

Added line #L106 was not covered by tests
<div className="gla-budget-section__card-body__cost">
<AppInputPriceControl
label={ __(
'Daily average cost',
'google-listings-and-ads'
) }
suffix={ currency }
{ ...getInputProps( 'amount' ) }
{ ...( disabled &&
nonInteractableProps ) }

Check warning on line 116 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#L115-L116

Added lines #L115 - L116 were not covered by tests
/>
<AppInputPriceControl
disabled
label={ __(
'Monthly max, estimated',
'google-listings-and-ads'
) }
suffix={ currency }
value={ monthlyMaxEstimated }
/>
</div>
{ countryCodes?.length > 0 && (
<BudgetRecommendation

Check warning on line 129 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#L128-L129

Added lines #L128 - L129 were not covered by tests
dailyAverageCost={ amount }
highestDailyBudget={
highestDailyBudget
}
highestDailyBudgetCountryCode={
highestDailyBudgetCountryCode
}
budgetRecommendationData={
budgetRecommendationData
}
/>
) }
suffix={ currency }
value={ monthlyMaxEstimated }
/>
</div>
{ countryCodes?.length > 0 && (
<BudgetRecommendation
countryCodes={ countryCodes }
dailyAverageCost={ amount }
/>
</>
) : (
<AppSpinner />

Check warning on line 144 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#L144

Added line #L144 was not covered by tests
) }
</Section.Card.Body>
</Section.Card>
Expand Down
1 change: 1 addition & 0 deletions js/src/data/action-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const TYPES = {
UPSERT_TOUR: 'UPSERT_TOUR',
HYDRATE_PREFETCHED_DATA: 'HYDRATE_PREFETCHED_DATA',
RECEIVE_GOOGLE_ADS_ACCOUNT_STATUS: 'RECEIVE_GOOGLE_ADS_ACCOUNT_STATUS',
RECEIVE_ADS_BUDGET_RECOMMENDATIONS: 'RECEIVE_ADS_BUDGET_RECOMMENDATIONS',
};

export default TYPES;
14 changes: 14 additions & 0 deletions js/src/data/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const DEFAULT_STATE = {
inviteLink: null,
step: null,
},
budgetRecommendations: {},
},
};

Expand Down Expand Up @@ -504,6 +505,19 @@ const reducer = ( state = DEFAULT_STATE, action ) => {
.end();
}

case TYPES.RECEIVE_ADS_BUDGET_RECOMMENDATIONS: {
const { countryCodesKey, currency, recommendations } = action;

return setIn(
state,
[ 'ads', 'budgetRecommendations', countryCodesKey ],
{
currency,
recommendations,
}
);
}

// Page will be reloaded after all accounts have been disconnected, so no need to mutate state.
case TYPES.DISCONNECT_ACCOUNTS_ALL:
default:
Expand Down
60 changes: 59 additions & 1 deletion js/src/data/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
} from '.~/constants';
import TYPES from './action-types';
import { API_NAMESPACE } from './constants';
import { getReportKey } from './utils';
import { getReportKey, getCountryCodesKey } from './utils';
import { handleApiError } from '.~/utils/handleError';
import { adaptAdsCampaign, adaptAssetGroup } from './adapters';
import { fetchWithHeaders, awaitPromise } from './controls';
Expand Down Expand Up @@ -48,6 +48,10 @@
receiveTour,
} from './actions';

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

export function* getShippingRates() {
yield fetchShippingRates();
}
Expand Down Expand Up @@ -510,3 +514,57 @@
getGoogleAdsAccountStatus.shouldInvalidate = ( action ) => {
return action.type === TYPES.DISCONNECT_ACCOUNTS_GOOGLE_ADS;
};

/**
* Fetch ad budget recommendations for the specified country codes.
*
* @param {Array<CountryCode>} [countryCodes] An array of country codes for which to fetch budget recommendations.
*/
export function* getAdsBudgetRecommendations( countryCodes ) {

Check warning on line 523 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L523

Added line #L523 was not covered by tests
if ( ! countryCodes || ! countryCodes.length ) {
return;

Check warning on line 525 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L525

Added line #L525 was not covered by tests
}

const countryCodesKey = getCountryCodesKey( countryCodes );
const endpoint = `${ API_NAMESPACE }/ads/campaigns/budget-recommendation`;
const query = { country_codes: countryCodes };
const path = addQueryArgs( endpoint, query );

Check warning on line 531 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L528-L531

Added lines #L528 - L531 were not covered by tests

try {
const { data } = yield fetchWithHeaders( {

Check warning on line 534 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L533-L534

Added lines #L533 - L534 were not covered by tests
path,
} );

const { currency, recommendations } = data;

Check warning on line 538 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L538

Added line #L538 was not covered by tests

return {

Check warning on line 540 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L540

Added line #L540 was not covered by tests
type: TYPES.RECEIVE_ADS_BUDGET_RECOMMENDATIONS,
countryCodesKey,
currency,
recommendations,
};
} catch ( response ) {
// Intentionally silence the specific in case the no budget recommendations are found from the API.
if ( response.status === 404 ) {
return;

Check warning on line 549 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L549

Added line #L549 was not covered by tests
}

const bodyPromise = response?.json() || response?.text();
const error = yield awaitPromise( bodyPromise );

Check warning on line 553 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L553

Added line #L553 was not covered by tests

handleApiError(

Check warning on line 555 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L555

Added line #L555 was not covered by tests
error,
__(
'There was an error getting the budget recommendation.',
'google-listings-and-ads'
)
);
}
}

getAdsBudgetRecommendations.shouldInvalidate = ( action ) => {
return (
action.type === TYPES.RECEIVE_ACCOUNTS_GOOGLE_ADS ||
action.type === TYPES.DISCONNECT_ACCOUNTS_GOOGLE_ADS

Check warning on line 568 in js/src/data/resolvers.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/resolvers.js#L566-L568

Added lines #L566 - L568 were not covered by tests
);
};
Loading
Loading