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 37 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
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,20 @@
* Internal dependencies
*/
import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap';
import useFetchBudgetRecommendationEffect from './useFetchBudgetRecommendationEffect';
import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation';
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 @@ -51,26 +35,24 @@

const BudgetRecommendation = ( props ) => {
const { countryCodes, dailyAverageCost = Infinity } = props;
const { data } = useFetchBudgetRecommendationEffect( countryCodes );
const { data, highestDailyBudgetCountryCode, highestDailyBudget } =
useFetchBudgetRecommendation( countryCodes );

Check warning on line 39 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#L39

Added line #L39 was not covered by tests
const map = useCountryKeyNameMap();

if ( ! data ) {
return null;
}

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

const countryName = map[ country ];
const countryName = map[ highestDailyBudgetCountryCode ];

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

Added line #L47 was 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 55 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#L55

Added line #L55 was not covered by tests

return (
<div className="gla-budget-recommendation">
Expand Down
15 changes: 2 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,7 +29,7 @@ const nonInteractableProps = {
*
* @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 {Array<CountryCode>} props.countryCodes Country codes to fetch budget recommendations for.
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
* @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.
*/
Expand All @@ -52,17 +52,6 @@ const BudgetSection = ( {
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 ] );
eason9487 marked this conversation as resolved.
Show resolved Hide resolved

return (
<div className="gla-budget-section">
<Section
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
53 changes: 52 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,50 @@
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'
)
);
}
}
20 changes: 19 additions & 1 deletion js/src/data/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
* Internal dependencies
*/
import { STORE_KEY } from './constants';
import { getReportQuery, getReportKey, getPerformanceQuery } from './utils';
import {
getReportQuery,
getReportKey,
getPerformanceQuery,
getCountryCodesKey,
} from './utils';

/**
* @typedef {import('.~/data/actions').CountryCode} CountryCode
Expand Down Expand Up @@ -406,3 +411,16 @@
export const getGoogleAdsAccountStatus = ( state ) => {
return state.ads.accountStatus;
};

/**
* Retrieves ad budget recommendations for provided country codes.
* If no recommendations are found, it returns `null`.
*
* @param {Object} state The state
* @param {Array<CountryCode>} [countryCodes] - An array of country code strings used to generate a unique key.
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
* @return {Object|null} The recommendations. It will be `null` if not yet fetched or fetched but doesn't exist.
*/
export const getAdsBudgetRecommendations = ( state, countryCodes = [] ) => {
const key = getCountryCodesKey( countryCodes );

Check warning on line 424 in js/src/data/selectors.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/selectors.js#L424

Added line #L424 was not covered by tests
return state.ads.budgetRecommendations[ key ] || null;
};
34 changes: 34 additions & 0 deletions js/src/data/test/reducer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ describe( 'reducer', () => {
inviteLink: null,
step: null,
},
budgetRecommendations: {},
},
} );

Expand Down Expand Up @@ -865,6 +866,39 @@ describe( 'reducer', () => {
} );
} );

describe( 'Ads Budget Recommendations', () => {
const path = 'ads.budgetRecommendations';

it( 'should receive a budget recommendation', () => {
const recommendation = {
countryCodesKey: 'mu_sg',
currency: 'MUR',
recommendations: [
{
country: 'MU',
daily_budget: 15,
},
{
country: 'SG',
daily_budget: 10,
},
],
};

const action = {
type: TYPES.RECEIVE_ADS_BUDGET_RECOMMENDATIONS,
...recommendation,
};
const state = reducer( prepareState(), action );

state.assertConsistentRef();
expect( state ).toHaveProperty( `${ path }.mu_sg`, {
currency: recommendation.currency,
recommendations: recommendation.recommendations,
} );
} );
} );

describe( 'Remaining actions simply update the data payload to the specific path of state and return the updated state', () => {
// The readability is better than applying the formatting here.
/* eslint-disable prettier/prettier */
Expand Down
18 changes: 18 additions & 0 deletions js/src/data/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
*/
import round from '.~/utils/round';

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

export const freeFields = [ 'clicks', 'impressions' ];
export const paidFields = [ 'sales', 'conversions', 'spend', ...freeFields ];
/**
Expand Down Expand Up @@ -190,6 +194,20 @@
);
}

/**
* Generates a unique key (slug) from an array of country codes.
*
* This function sorts the array of country codes alphabetically,
* joins them into a single string with underscore (`_`), and converts
* the result to lowercase.
*
* @param {Array<CountryCode>} countryCodes - An array of country code strings.
* @return {string} A underscore-separated, lowercase string representing the sorted country codes.
*/
export function getCountryCodesKey( countryCodes = [] ) {
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
return [ ...countryCodes ].sort().join( '_' ).toLowerCase();

Check warning on line 208 in js/src/data/utils.js

View check run for this annotation

Codecov / codecov/patch

js/src/data/utils.js#L207-L208

Added lines #L207 - L208 were not covered by tests
}

/**
* Report fields fetched from report API.
*
Expand Down
54 changes: 54 additions & 0 deletions js/src/hooks/useFetchBudgetRecommendation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* External dependencies
*/
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import { STORE_KEY } from '.~/data/constants';
import getHighestBudget from '.~/utils/getHighestBudget';

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

/**
* Fetch the highest budget recommendation for countries in a side effect.
*
* @param {Array<CountryCode>} [countryCodes] An array of country codes. If empty, the fetch will not be triggered.
* @return {Object} Budget recommendation.
*/
const useFetchBudgetRecommendation = ( countryCodes ) => {
return useSelect(
( select ) => {

Check warning on line 24 in js/src/hooks/useFetchBudgetRecommendation.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useFetchBudgetRecommendation.js#L23-L24

Added lines #L23 - L24 were not covered by tests
const { getAdsBudgetRecommendations, hasFinishedResolution } =
select( STORE_KEY );

Check warning on line 26 in js/src/hooks/useFetchBudgetRecommendation.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useFetchBudgetRecommendation.js#L26

Added line #L26 was not covered by tests

const data = getAdsBudgetRecommendations( countryCodes );
let highestDailyBudget = 0;

Check warning on line 29 in js/src/hooks/useFetchBudgetRecommendation.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useFetchBudgetRecommendation.js#L28-L29

Added lines #L28 - L29 were not covered by tests
let highestDailyBudgetCountryCode;

if ( data ) {
const { recommendations } = data;
( {

Check warning on line 34 in js/src/hooks/useFetchBudgetRecommendation.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useFetchBudgetRecommendation.js#L33-L34

Added lines #L33 - L34 were not covered by tests
daily_budget: highestDailyBudget,
country: highestDailyBudgetCountryCode,
} = getHighestBudget( recommendations ) );
}

return {

Check warning on line 40 in js/src/hooks/useFetchBudgetRecommendation.js

View check run for this annotation

Codecov / codecov/patch

js/src/hooks/useFetchBudgetRecommendation.js#L40

Added line #L40 was not covered by tests
data,
highestDailyBudget,
highestDailyBudgetCountryCode,
hasFinishedResolution: hasFinishedResolution(
'getAdsBudgetRecommendations',
[ countryCodes ]
),
};
},
[ countryCodes ]
);
};

export default useFetchBudgetRecommendation;
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
File renamed without changes.
Loading