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 13 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
26 changes: 24 additions & 2 deletions js/src/components/paid-ads/validateCampaign.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { __ } from '@wordpress/i18n';
import { __, sprintf } from '@wordpress/i18n';

/**
* @typedef {import('.~/components/types.js').CampaignFormValues} CampaignFormValues
Expand All @@ -11,9 +11,10 @@ import { __ } from '@wordpress/i18n';
* Validate campaign form. Accepts the form values object and returns errors object.
*
* @param {CampaignFormValues} values Campaign form values.
* @param {Object} opts Extra form options.
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
* @return {Object} errors.
*/
const validateCampaign = ( values ) => {
const validateCampaign = ( values, opts ) => {
const errors = {};

if ( ! Number.isFinite( values.amount ) || values.amount <= 0 ) {
Expand All @@ -23,6 +24,27 @@ const validateCampaign = ( values ) => {
);
}

if (
asvinb marked this conversation as resolved.
Show resolved Hide resolved
Number.isFinite( values?.amount ) &&
Number.isFinite( opts?.budget?.daily_budget )
) {
const { amount } = values;
const { budget, budgetMin } = opts;
asvinb marked this conversation as resolved.
Show resolved Hide resolved
const { daily_budget: dailyBudget } = budget;
const minAmount = Math.round( dailyBudget * budgetMin, 2 );

if ( amount < minAmount ) {
errors.amount = sprintf(
/* translators: %s: minimum daily budget */
__(
'Please make sure daily average cost is at least %s.',
asvinb marked this conversation as resolved.
Show resolved Hide resolved
'google-listings-and-ads'
),
minAmount
);
}
}

return errors;
};

Expand Down
30 changes: 30 additions & 0 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 @@ -72,4 +72,34 @@ describe( 'validateCampaign', () => {
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 = {
budget: {
daily_budget: 100,
},
budgetMin: 0.3,
};

const errors = validateCampaign( values, opts );

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

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

const opts = {
budget: {
daily_budget: 100,
},
budgetMin: 0.3,
};

const errors = validateCampaign( values, opts );
expect( errors ).not.toHaveProperty( 'amount' );
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import { Form } from '@woocommerce/components';
* Internal dependencies
*/
import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus';
import useFetchBudgetRecommendationEffect from '.~/components/paid-ads/budget-section/budget-recommendation/useFetchBudgetRecommendationEffect';
import BudgetSection from '.~/components/paid-ads/budget-section';
import BillingCard from '.~/components/paid-ads/billing-card';
import SpinnerCard from '.~/components/spinner-card';
import Section from '.~/wcdl/section';
import validateCampaign from '.~/components/paid-ads/validateCampaign';
import clientSession from './clientSession';
import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants';
import getHighestBudget from '.~/utils/getHighestBudget';

/**
* @typedef {import('.~/data/actions').CountryCode} CountryCode
Expand Down Expand Up @@ -73,6 +75,10 @@ export default function PaidAdsSetupSections( {
return resolveInitialPaidAds( startingPaidAds );
} );

const { data: budgetData } =
useFetchBudgetRecommendationEffect( countryCodes );
const budget = getHighestBudget( budgetData?.recommendations );

const isBillingCompleted =
billingStatus?.status === GOOGLE_ADS_BILLING_STATUS.APPROVED;

Expand Down Expand Up @@ -111,13 +117,20 @@ export default function PaidAdsSetupSections( {
amount: paidAds.amount,
};

const formOpts = {
budget,
asvinb marked this conversation as resolved.
Show resolved Hide resolved
budgetMin: 0.3,
asvinb marked this conversation as resolved.
Show resolved Hide resolved
};

return (
<Form
initialValues={ initialValues }
onChange={ ( _, values, isValid ) => {
setPaidAds( { ...paidAds, ...values, isValid } );
} }
validate={ validateCampaign }
validate={ ( formValues ) =>
validateCampaign( formValues, formOpts )
}
>
{ ( formProps ) => {
return (
Expand Down
19 changes: 19 additions & 0 deletions js/src/utils/getHighestBudget.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* 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).
*/
export default function getHighestBudget( recommendations ) {
asvinb marked this conversation as resolved.
Show resolved Hide resolved
if ( ! recommendations ) {
return null;
}

return recommendations.reduce( ( defender, challenger ) => {
if ( challenger.daily_budget > defender.daily_budget ) {
return challenger;
}
return defender;
} );
}
46 changes: 46 additions & 0 deletions tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,52 @@ test.describe( 'Complete your campaign', () => {
} );
} );

test.describe( 'Validate budget percent', () => {
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
test.beforeAll( async () => {
await setupBudgetPage.mockBudgetRecommendation( {
currency: 'USD',
recommendations: [
{
country: 'US',
daily_budget: 100,
},
],
} );
} );

test( 'should see validation error if lower than the 30%', async () => {
await setupBudgetPage.fillBudget( '10' );
await setupBudgetPage.getBudgetInput().blur();

const error = await page
.locator( '.components-base-control__help' )
.textContent();
await expect( error ).toBe(
'Please make sure daily average cost is at least 30.'
);
} );

test( 'should not see validation error if exactly 30%', async () => {
await setupBudgetPage.fillBudget( '30' );
await setupBudgetPage.getBudgetInput().blur();

const error = await page.locator(
'.components-base-control__help'
);
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
await expect( error ).not.toBeVisible();
} );

test( 'should not see validation error if greater than 30%', async () => {
await setupBudgetPage.fillBudget( '40' );
await setupBudgetPage.getBudgetInput().blur();

const error = await page.locator(
'.components-base-control__help'
);
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
await expect( error ).not.toBeVisible();
} );
} );

test.describe( 'Set up billing', () => {
let newPage;

Expand Down
14 changes: 14 additions & 0 deletions tests/e2e/utils/pages/setup-ads/setup-budget.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,20 @@ export default class SetupBudget extends MockRequests {
);
}

/**
* Mock the budget recommendation.
*
* @param {Object} payload The payload.
* @return {Promise<void>}
*/
async mockBudgetRecommendation( payload ) {
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
await this.fulfillRequest(
/\/wc\/gla\/ads\/campaigns\/budget-recommendation\b/,
payload,
200
);
}

/**
* Mock the campaign creation process and the Ads setup completion.
*
Expand Down