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

Add minimum budget limit #2583

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
f6c9f55
Updates validateCampaign with daily limit check
dsawardekar Sep 5, 2024
dbfb972
Adds validateCampaign tests
dsawardekar Sep 5, 2024
fb36ca6
Adds getHighestBudget utility
dsawardekar Sep 5, 2024
41956ca
Adds daily limit data lookup to paid ads setup sections
dsawardekar Sep 5, 2024
9ef16c2
Fixes linter warnings
dsawardekar Sep 5, 2024
5741e6b
Style error message accordingly.
asvinb Sep 6, 2024
ac7753c
Merge branch 'feature/2459-campaign-creation-flow' into update/2459-a…
dsawardekar Sep 11, 2024
beb3700
Adds e2e tests
dsawardekar Sep 11, 2024
c7f5a36
Merge branch 'feature/2459-campaign-creation-flow' into update/2459-a…
dsawardekar Sep 12, 2024
1d41c1b
Uses form opts instead of values
dsawardekar Sep 13, 2024
585bbbd
Update countryCodes and validation callback
dsawardekar Sep 13, 2024
f35ac9d
Updates unit tests
dsawardekar Sep 13, 2024
6804415
Fixes linter warnings
dsawardekar Sep 13, 2024
d62bc02
Do not pass uneeded props.
asvinb Sep 16, 2024
603d9c4
Format currency for recommended budget.
asvinb Sep 17, 2024
f73150b
Update E2E test.
asvinb Sep 17, 2024
f1fd765
Fix E2E test.
asvinb Sep 17, 2024
7adc092
Fix e2e test.
asvinb Sep 17, 2024
90e8d95
Add useValidateCampaignWithCountryCodes hook.
asvinb Sep 18, 2024
811461f
Fix unit test.
asvinb Sep 18, 2024
eb70d33
Do not trigger fetch if there are no countryCodes.
asvinb Sep 19, 2024
a502b7a
Add loading property to know when the hook is ready.
asvinb Sep 19, 2024
34c58a9
Remove condition to display PaidAdsSetupSections.
asvinb Sep 19, 2024
5d794ed
Merge branch 'feature/2459-campaign-creation-flow' into update/2459-a…
asvinb Sep 23, 2024
4d80b45
Add getAdsBudgetRecommendations selector.
asvinb Sep 23, 2024
44d9eed
Update hooks.
asvinb Sep 23, 2024
29b6920
Add JSDocs.
asvinb Sep 23, 2024
a1a5cf4
More descriptive JSDocs.
asvinb Sep 23, 2024
a7a2eba
Fix linting errors.
asvinb Sep 23, 2024
684fd02
Apply change.
asvinb Sep 24, 2024
447bff0
Add JSdocs for returned value.
asvinb Sep 24, 2024
c85bdf1
Fix JS errors.
asvinb Sep 24, 2024
74e0dd8
Fix e2e test.
asvinb Sep 24, 2024
6fc4a6b
Address CR feedback.
asvinb Sep 24, 2024
716581b
Add JSDocs.
asvinb Sep 24, 2024
2c4f863
Simplify hooks.
asvinb Sep 24, 2024
2d51b4a
Fix JS tests.
asvinb Sep 24, 2024
ab6a397
Rename function.
asvinb Sep 24, 2024
edccd57
Delete unused file.
asvinb Sep 24, 2024
7f7261a
Review comments.
asvinb Sep 24, 2024
330697c
Update minimum amount displayed to the user.
asvinb Sep 24, 2024
0d8c6bd
Make sure country codes are loaded.
asvinb Sep 26, 2024
5f2f291
Fix e2e test.
asvinb Sep 26, 2024
03b1167
Take precision settings into consideration.
asvinb Sep 27, 2024
7432760
Fix e2e tests.
asvinb Sep 27, 2024
f23a52c
Convert to float.
asvinb Sep 27, 2024
df07674
Merge branch 'feature/2459-campaign-creation-flow' into update/2459-a…
asvinb Oct 1, 2024
d1ed9cf
Merge branch 'update/2535-consolidate-ad-creation-ccf-merged' into up…
asvinb Oct 1, 2024
dfb1bc9
Fix bad merge.
asvinb Oct 1, 2024
c89ac46
Simplify code where countryCodes is needed.
asvinb Oct 1, 2024
6d7a287
Remove unused props.
asvinb Oct 1, 2024
3ff8276
Fix loaded value
asvinb Oct 1, 2024
78fc8bb
Fix e2e test
asvinb Oct 1, 2024
3fb0832
Use Math.ceil.
asvinb Oct 4, 2024
ce9ad99
Fix failing test.
asvinb Oct 4, 2024
9fe042d
Merge branch 'update/2502-budget-setup-card' into update/2459-add-min…
asvinb Oct 15, 2024
7bf1198
Add minimumAmount prop to CampaignAssetsForm.
asvinb Oct 15, 2024
631252d
Add validation function.
asvinb Oct 15, 2024
2ff822d
Revert changes.
asvinb Oct 15, 2024
e471d55
Fix E2E tests.
asvinb Oct 15, 2024
e56b6f2
Merge branch 'update/2535-consolidate-ad-creation-ccf-merged' into up…
asvinb Oct 16, 2024
02889ea
Merge branch 'update/2502-budget-setup-card' into update/2459-add-min…
asvinb Oct 22, 2024
958dc38
Updated JSDocs,
asvinb Oct 22, 2024
19ec4a0
Merge branch 'feature/2459-campaign-creation-flow' into update/2459-a…
asvinb Oct 22, 2024
99a5e2c
Add minimum amount.
asvinb Oct 22, 2024
3ec5948
Address CR feedback.
asvinb Oct 23, 2024
95ae707
Remove debugging code.
asvinb Oct 23, 2024
bb9ad4b
Restore line.
asvinb Oct 23, 2024
e756955
Remove space.
asvinb Oct 23, 2024
9dd2744
Rename prop to recommendedDailyBudget.
asvinb Oct 23, 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
2 changes: 2 additions & 0 deletions js/src/components/app-input-control/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@
color: $gray-700;
}

&.has-error .components-input-control__backdrop,
&--error-character-count .components-input-control .components-input-control__container .components-input-control__backdrop {
border-color: $alert-red;
box-shadow: none;
}

&.has-error .components-base-control__help,
&--error-character-count &__character-count {
color: $alert-red;
}
Expand Down
13 changes: 12 additions & 1 deletion js/src/components/paid-ads/campaign-assets-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import AdaptiveForm from '.~/components/adaptive-form';
import validateCampaign from '.~/components/paid-ads/validateCampaign';
import validateAssetGroup from '.~/components/paid-ads/validateAssetGroup';
import useAdsCurrency from '.~/hooks/useAdsCurrency';

/**
* @typedef {import('.~/components/types.js').CampaignFormValues} CampaignFormValues
Expand Down Expand Up @@ -66,10 +67,12 @@
* @param {Object} props React props.
* @param {CampaignFormValues} props.initialCampaign Initial campaign values.
* @param {AssetEntityGroup} [props.assetEntityGroup] The asset entity group to be used in initializing the form values for editing.
* @param {number} props.recommendedDailyBudget The recommended daily budget for the campaign. The minimum campaign amount will be set to 30% of this value.
*/
export default function CampaignAssetsForm( {
initialCampaign,
assetEntityGroup,
recommendedDailyBudget,
...adaptiveFormProps
} ) {
const initialAssetGroup = useMemo( () => {
Expand All @@ -78,6 +81,7 @@

const [ baseAssetGroup, setBaseAssetGroup ] = useState( initialAssetGroup );
const [ hasImportedAssets, setHasImportedAssets ] = useState( false );
const { formatAmount } = useAdsCurrency();

const extendAdapter = ( formContext ) => {
const assetGroupErrors = validateAssetGroup( formContext.values );
Expand Down Expand Up @@ -117,13 +121,20 @@
};
};

const validateCampaignWithMinimumAmount = ( values ) => {
return validateCampaign( values, {

Check warning on line 125 in js/src/components/paid-ads/campaign-assets-form.js

View check run for this annotation

Codecov / codecov/patch

js/src/components/paid-ads/campaign-assets-form.js#L125

Added line #L125 was not covered by tests
dailyBudget: recommendedDailyBudget,
formatAmount,
} );
};

return (
<AdaptiveForm
initialValues={ {
...initialCampaign,
...initialAssetGroup,
} }
validate={ validateCampaign }
validate={ validateCampaignWithMinimumAmount }
extendAdapter={ extendAdapter }
{ ...adaptiveFormProps }
/>
Expand Down
6 changes: 6 additions & 0 deletions js/src/components/paid-ads/campaign-assets-form.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import userEvent from '@testing-library/user-event';
*/
import CampaignAssetsForm from './campaign-assets-form';

jest.mock( '@wordpress/api-fetch', () => {
const impl = jest.fn().mockName( '@wordpress/api-fetch' );
impl.use = jest.fn().mockName( 'apiFetch.use' );
return impl;
} );

const alwaysValid = () => ( {} );

describe( 'CampaignAssetsForm', () => {
Expand Down
48 changes: 42 additions & 6 deletions js/src/components/paid-ads/validateCampaign.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,62 @@
/**
* External dependencies
*/
import { __ } from '@wordpress/i18n';
import { __, sprintf } from '@wordpress/i18n';

/**
* @typedef {import('.~/components/types.js').CampaignFormValues} CampaignFormValues
*/

/**
* @typedef {Object} ValidateCampaignOptions
* @property {number|undefined} dailyBudget Daily budget for the campaign.
* @property {Function} formatAmount A function to format the budget amount according to the currency settings.
*/

// Minimum percentage of the recommended daily budget.
const BUDGET_MIN_PERCENT = 0.3;

/**
* Validate campaign form. Accepts the form values object and returns errors object.
*
* @param {CampaignFormValues} values Campaign form values.
* @param {ValidateCampaignOptions} opts Extra form options.
* @return {Object} errors.
*/
const validateCampaign = ( values ) => {
const validateCampaign = ( values, opts ) => {
const errors = {};

if (
asvinb marked this conversation as resolved.
Show resolved Hide resolved
Number.isFinite( values.amount ) &&
Number.isFinite( opts.dailyBudget ) &&
opts.dailyBudget > 0
) {
const { amount } = values;
const { dailyBudget, formatAmount } = opts;

const minAmount = Math.ceil( dailyBudget * BUDGET_MIN_PERCENT );

if ( amount < minAmount ) {
return {
amount: sprintf(
/* translators: %1$s: minimum daily budget */
__(
'Please make sure daily average cost is at least %s',
'google-listings-and-ads'
),
formatAmount( minAmount )
),
};
}
}

if ( ! Number.isFinite( values.amount ) || values.amount <= 0 ) {
errors.amount = __(
'Please make sure daily average cost is greater than 0.',
'google-listings-and-ads'
);
return {
amount: __(
'Please make sure daily average cost is greater than 0.',
'google-listings-and-ads'
),
};
}

return errors;
Expand Down
89 changes: 79 additions & 10 deletions js/src/components/paid-ads/validateCampaign.test.js
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,39 @@
*/
import validateCampaign from './validateCampaign';

const mockFormatAmount = jest
.fn()
.mockImplementation( ( amount ) => `Rs ${ amount }` );

/**
* `validateCampaign` function returns an object, and if any checks are not passed,
* set properties respectively with an error message to indicate it.
*/
describe( 'validateCampaign', () => {
let values;
const validateCampaignOptions = {
dailyBudget: undefined,
formatAmount: mockFormatAmount,
};

beforeEach( () => {
// Initial values
values = { amount: 0 };
} );

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

expect( errors ).toStrictEqual( {} );
} );

it( 'should indicate multiple unpassed checks by setting properties in the returned object', () => {
const errors = validateCampaign( values );
const errors = validateCampaign( values, validateCampaignOptions );

expect( errors ).toHaveProperty( 'amount' );
} );
Expand All @@ -33,25 +44,25 @@ describe( 'validateCampaign', () => {
let errors;

values.amount = '';
errors = validateCampaign( values );
errors = validateCampaign( values, validateCampaignOptions );

expect( errors ).toHaveProperty( 'amount' );
expect( errors.amount ).toMatchSnapshot();

values.amount = undefined;
errors = validateCampaign( values );
errors = validateCampaign( values, validateCampaignOptions );

expect( errors ).toHaveProperty( 'amount' );
expect( errors.amount ).toMatchSnapshot();

values.amount = new Date();
errors = validateCampaign( values );
errors = validateCampaign( values, validateCampaignOptions );

expect( errors ).toHaveProperty( 'amount' );
expect( errors.amount ).toMatchSnapshot();

values.amount = NaN;
errors = validateCampaign( values );
errors = validateCampaign( values, validateCampaignOptions );

expect( errors ).toHaveProperty( 'amount' );
expect( errors.amount ).toMatchSnapshot();
Expand All @@ -61,15 +72,73 @@ describe( 'validateCampaign', () => {
let errors;

values.amount = 0;
errors = validateCampaign( values );
errors = validateCampaign( values, validateCampaignOptions );

expect( errors ).toHaveProperty( 'amount' );
expect( errors.amount ).toMatchSnapshot();

values.amount = -0.01;
errors = validateCampaign( values );
errors = validateCampaign( values, validateCampaignOptions );

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

it( 'When a budget is provided and the amount is less than the minimum, should not pass', () => {
values.amount = 10;

const opts = {
dailyBudget: 100,
formatAmount: mockFormatAmount,
};

const errors = validateCampaign( values, opts );

expect( errors ).toHaveProperty( 'amount' );
expect( errors.amount ).toContain( 'is at least Rs 30' );
} );

it( 'When a budget is provided and the amount is same as the minimum, should pass', () => {
values.amount = 30;

const opts = {
dailyBudget: 100,
formatAmount: mockFormatAmount,
};

const errors = validateCampaign( values, opts );
expect( errors ).not.toHaveProperty( 'amount' );
} );

it( 'When a budget is provided and the amount is greater than the minimum, should pass', () => {
values.amount = 35;

const opts = {
dailyBudget: 100,
formatAmount: mockFormatAmount,
};

const errors = validateCampaign( values, opts );
expect( errors ).not.toHaveProperty( 'amount' );
} );

it( 'The minimum amount should be rounded up to the nearest integer', () => {
values.amount = 30.99;

let opts = {
dailyBudget: 101,
formatAmount: mockFormatAmount,
};
const errors = validateCampaign( values, opts );

opts = {
dailyBudget: 102,
formatAmount: mockFormatAmount,
};
const errorsNext = validateCampaign( values, opts );

expect( errors ).toEqual( errorsNext );
expect( errors ).toHaveProperty( 'amount' );
expect( errors.amount ).toContain( 'is at least Rs 31' );
} );
} );
1 change: 1 addition & 0 deletions js/src/pages/create-paid-ads-campaign/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ const CreatePaidAdsCampaign = () => {
initialCampaign={ {
amount: highestDailyBudget,
} }
recommendedDailyBudget={ highestDailyBudget }
onSubmit={ handleSubmit }
>
<Stepper
Expand Down
11 changes: 9 additions & 2 deletions js/src/pages/edit-paid-ads-campaign/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
recordStepperChangeEvent,
recordStepContinueEvent,
} from '.~/utils/tracks';
import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation';

const eventName = 'gla_paid_campaign_step';
const eventContext = 'edit-ads';
Expand Down Expand Up @@ -71,7 +72,8 @@
} = useAppSelectDispatch( 'getCampaignAssetGroups', id );
const campaign = campaigns?.find( ( el ) => el.id === id );
const assetEntityGroup = assetEntityGroups?.at( 0 );

const { highestDailyBudget, hasFinishedResolution } =
useFetchBudgetRecommendation( campaign?.displayCountries );

Check warning on line 76 in js/src/pages/edit-paid-ads-campaign/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/pages/edit-paid-ads-campaign/index.js#L76

Added line #L76 was not covered by tests
useEffect( () => {
if ( campaign && campaign.type !== CAMPAIGN_TYPE_PMAX ) {
getHistory().replace( dashboardURL );
Expand All @@ -84,7 +86,11 @@
getHistory().push( url );
};

if ( ! loaded || ! hasResolvedAssetEntityGroups ) {
if (
! loaded ||
! hasResolvedAssetEntityGroups ||
! hasFinishedResolution

Check warning on line 92 in js/src/pages/edit-paid-ads-campaign/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/pages/edit-paid-ads-campaign/index.js#L90-L92

Added lines #L90 - L92 were not covered by tests
) {
return (
<>
<TopBar
Expand Down Expand Up @@ -181,6 +187,7 @@
initialCampaign={ {
amount: campaign.amount,
} }
recommendedDailyBudget={ highestDailyBudget }
assetEntityGroup={ assetEntityGroup }
onSubmit={ handleSubmit }
>
Expand Down
1 change: 1 addition & 0 deletions js/src/setup-ads/ads-stepper/setup-paid-ads.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ const SetupPaidAds = () => {
initialCampaign={ initialValues }
onChange={ handleChange }
onSubmit={ handleSubmit }
recommendedDailyBudget={ highestDailyBudget }
>
<AdsCampaign
headerTitle={ __(
Expand Down
1 change: 1 addition & 0 deletions js/src/setup-mc/setup-stepper/setup-paid-ads.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ export default function SetupPaidAds() {
return (
<CampaignAssetsForm
initialCampaign={ paidAds }
recommendedDailyBudget={ highestDailyBudget }
onChange={ ( _, values ) => {
if ( values.amount >= highestDailyBudget ) {
clientSession.setCampaign( values );
Expand Down
Loading