From a57bf145402a83c7f2b3c6439890e0c9dfd737aa Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 22 Aug 2024 11:45:13 +0530 Subject: [PATCH 01/40] Update Budget Setup Card. --- .../budget-recommendation/index.js | 47 +++++-------------- .../paid-ads/budget-section/index.js | 7 ++- .../setup-paid-ads/paid-ads-setup-sections.js | 43 +++++++++++++++-- 3 files changed, 56 insertions(+), 41 deletions(-) diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index 2492685c36..44152fa22c 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -9,37 +9,19 @@ import GridiconNoticeOutline from 'gridicons/dist/notice-outline'; /** * 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: , em: , 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.
Tip: Most merchants targeting similar countries set a daily budget of %1$f %2$s', + 'We recommend running campaigns at least 1 month so it can learn to optimize for your business.
Tip: Most merchants targeting similar countries set a daily budget of %1$f %2$s', '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.
Tip: Most merchants targeting %3$s set a daily budget of %1$f %2$s', + 'We recommend running campaigns at least 1 month so it can learn to optimize for your business.
Tip: Most merchants targeting %3$s set a daily budget of %1$f %2$s', 'google-listings-and-ads' ); @@ -50,27 +32,22 @@ function toRecommendationRange( isMultiple, ...values ) { } const BudgetRecommendation = ( props ) => { - const { countryCodes, dailyAverageCost = Infinity } = props; - const { data } = useFetchBudgetRecommendationEffect( countryCodes ); - const map = useCountryKeyNameMap(); - - if ( ! data ) { - return null; - } - - const { currency, recommendations } = data; - const { daily_budget: dailyBudget, country } = - getHighestBudget( recommendations ); + const { + countryCodes, + dailyBudget: recommendedBudget, + country: countryName, + currency, + value: currentBudget, + } = props; - const countryName = map[ country ]; const recommendationRange = toRecommendationRange( - recommendations.length > 1, - dailyBudget, + countryCodes.length > 1, + recommendedBudget, currency, countryName ); - const showLowerBudgetNotice = dailyAverageCost < dailyBudget; + const showLowerBudgetNotice = currentBudget < recommendedBudget; return (
diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index 5c461602e6..b3d647d303 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -30,7 +30,7 @@ const nonInteractableProps = { */ const BudgetSection = ( { formProps, disabled = false, children } ) => { const { getInputProps, setValue, values } = formProps; - const { countryCodes, amount } = values; + const { countryCodes, amount, country, dailyBudget } = values; const { googleAdsAccount } = useGoogleAdsAccount(); const monthlyMaxEstimated = getMonthlyMaxEstimated( amount ); // Display the currency code that will be used by Google Ads, but still use the store's currency formatting settings. @@ -92,7 +92,10 @@ const BudgetSection = ( { formProps, disabled = false, children } ) => { { countryCodes.length > 0 && ( ) } diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index 86c3739bf7..59b65b40f1 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -11,6 +11,7 @@ import { Form } from '@woocommerce/components'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes'; import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; +import useFetchBudgetRecommendationEffect from '.~/components/paid-ads/budget-section/budget-recommendation/useFetchBudgetRecommendationEffect'; import AudienceSection from '.~/components/paid-ads/audience-section'; import BudgetSection from '.~/components/paid-ads/budget-section'; import BillingCard from '.~/components/paid-ads/billing-card'; @@ -32,11 +33,29 @@ import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; const defaultPaidAds = { amount: 0, + country: undefined, countryCodes: [], + dailyBudget: 0, isValid: false, isReady: false, }; +/* + * 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; + } ); +} + /** * Resolve the initial paid ads data from the given paid ads data with the loaded target audience. * Parts of the resolved data are used in the `initialValues` prop of `Form` component. @@ -78,6 +97,14 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { const { hasGoogleAdsConnection } = useGoogleAdsAccount(); const { data: targetAudience } = useTargetAudienceFinalCountryCodes(); const { billingStatus } = useGoogleAdsAccountBillingStatus(); + const { data: recommendationData } = + useFetchBudgetRecommendationEffect( targetAudience ); + const { recommendations } = recommendationData || {}; + + const { daily_budget: dailyBudget, country } = + recommendations !== undefined + ? getHighestBudget( recommendations ) + : {}; const onStatesReceivedRef = useRef(); onStatesReceivedRef.current = onStatesReceived; @@ -127,10 +154,16 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { - https://github.com/woocommerce/google-listings-and-ads/blob/5b6522ca10ad75556e6b2de7c120cc712aab70b1/js/src/components/free-listings/setup-free-listings/index.js#L172-L186 */ useEffect( () => { - setPaidAds( ( currentPaidAds ) => - resolveInitialPaidAds( currentPaidAds, targetAudience ) - ); - }, [ targetAudience ] ); + setPaidAds( ( currentPaidAds ) => { + currentPaidAds = { + ...currentPaidAds, + amount: dailyBudget ? dailyBudget : currentPaidAds.amount, + country: country ? country : currentPaidAds.country, + dailyBudget, + }; + return resolveInitialPaidAds( currentPaidAds, targetAudience ); + } ); + }, [ targetAudience, dailyBudget, country ] ); if ( ! targetAudience || ! billingStatus ) { return ( @@ -143,6 +176,8 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { const initialValues = { countryCodes: paidAds.countryCodes, amount: paidAds.amount, + country: paidAds.country, + dailyBudget: paidAds.dailyBudget, }; return ( From 273d02bb5f10112c2471e464d8a6199f5f809747 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 22 Aug 2024 20:20:49 +0530 Subject: [PATCH 02/40] Fix: e2e tests. --- .../budget-recommendation/index.js | 48 ++++++++++++++----- .../paid-ads/budget-section/index.js | 7 ++- .../setup-paid-ads/paid-ads-setup-sections.js | 14 +++--- .../setup-mc/step-4-complete-campaign.test.js | 27 ++++++++--- tests/e2e/utils/mock-requests.js | 15 ++++++ 5 files changed, 81 insertions(+), 30 deletions(-) diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index 44152fa22c..4bc5a8e995 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -9,19 +9,36 @@ import GridiconNoticeOutline from 'gridicons/dist/notice-outline'; /** * Internal dependencies */ +import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap'; 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: , em: , br:
}; const template = isMultiple ? // translators: it's a range of recommended budget amount. 1: the value of the budget, 2: the currency of amount. __( - 'We recommend running campaigns at least 1 month so it can learn to optimize for your business.
Tip: Most merchants targeting similar countries set a daily budget of %1$f %2$s', + 'Google will optimize your ads to maximize performance across the country/s you select.
Tip: Most merchants targeting similar countries set a daily budget of %1$f %2$s', '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. __( - 'We recommend running campaigns at least 1 month so it can learn to optimize for your business.
Tip: Most merchants targeting %3$s set a daily budget of %1$f %2$s', + 'Google will optimize your ads to maximize performance across the country/s you select.
Tip: Most merchants targeting %3$s set a daily budget of %1$f %2$s', 'google-listings-and-ads' ); @@ -32,22 +49,29 @@ function toRecommendationRange( isMultiple, ...values ) { } const BudgetRecommendation = ( props ) => { - const { - countryCodes, - dailyBudget: recommendedBudget, - country: countryName, - currency, - value: currentBudget, - } = props; + const { countryCodes, dailyAverageCost = Infinity, data } = props; + const map = useCountryKeyNameMap(); + + if ( ! data ) { + return null; + } + + const { currency, recommendations } = data; + const { daily_budget: dailyBudget, country } = getHighestBudget( + recommendations.filter( ( recommendation ) => + countryCodes.includes( recommendation.country ) + ) + ); + const countryName = map[ country ]; const recommendationRange = toRecommendationRange( - countryCodes.length > 1, - recommendedBudget, + recommendations.length > 1, + dailyBudget, currency, countryName ); - const showLowerBudgetNotice = currentBudget < recommendedBudget; + const showLowerBudgetNotice = dailyAverageCost < dailyBudget; return (
diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index b3d647d303..c4d44c0993 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -30,7 +30,7 @@ const nonInteractableProps = { */ const BudgetSection = ( { formProps, disabled = false, children } ) => { const { getInputProps, setValue, values } = formProps; - const { countryCodes, amount, country, dailyBudget } = values; + const { countryCodes, amount, recommendationData } = values; const { googleAdsAccount } = useGoogleAdsAccount(); const monthlyMaxEstimated = getMonthlyMaxEstimated( amount ); // Display the currency code that will be used by Google Ads, but still use the store's currency formatting settings. @@ -92,10 +92,9 @@ const BudgetSection = ( { formProps, disabled = false, children } ) => { { countryCodes.length > 0 && ( ) } diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index 59b65b40f1..98e6154f2c 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -33,11 +33,11 @@ import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; const defaultPaidAds = { amount: 0, - country: undefined, countryCodes: [], - dailyBudget: 0, + recommendations: [], isValid: false, isReady: false, + recommendationData: {}, }; /* @@ -101,7 +101,7 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { useFetchBudgetRecommendationEffect( targetAudience ); const { recommendations } = recommendationData || {}; - const { daily_budget: dailyBudget, country } = + const { daily_budget: dailyBudget } = recommendations !== undefined ? getHighestBudget( recommendations ) : {}; @@ -158,12 +158,11 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { currentPaidAds = { ...currentPaidAds, amount: dailyBudget ? dailyBudget : currentPaidAds.amount, - country: country ? country : currentPaidAds.country, - dailyBudget, + recommendationData, }; return resolveInitialPaidAds( currentPaidAds, targetAudience ); } ); - }, [ targetAudience, dailyBudget, country ] ); + }, [ targetAudience, dailyBudget, recommendationData ] ); if ( ! targetAudience || ! billingStatus ) { return ( @@ -176,8 +175,7 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { const initialValues = { countryCodes: paidAds.countryCodes, amount: paidAds.amount, - country: paidAds.country, - dailyBudget: paidAds.dailyBudget, + recommendationData: paidAds.recommendationData, }; return ( diff --git a/tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js b/tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js index 53b2a4a497..9a9c757fe8 100644 --- a/tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js +++ b/tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js @@ -84,6 +84,24 @@ test.describe( 'Complete your campaign', () => { [ 'GET' ] ), + completeCampaign.fulfillBudgetRecommendations( { + currency: 'USD', + recommendations: [ + { + country: 'US', + daily_budget: 10, + }, + { + country: 'TW', + daily_budget: 8, + }, + { + country: 'GB', + daily_budget: 20, + }, + ], + } ), + // The following mocks are requests will happen after completing the onboarding completeCampaign.mockSuccessfulSettingsSyncRequest(), @@ -266,16 +284,11 @@ test.describe( 'Complete your campaign', () => { textContent ); - const responsePromise = - setupBudgetPage.registerBudgetRecommendationResponse(); - await removeCountryFromSearchBox( page, 'United Kingdom (UK)' ); - await responsePromise; - textContent = await setupBudgetPage .getBudgetRecommendationTextRow() .textContent(); @@ -425,7 +438,9 @@ test.describe( 'Complete your campaign', () => { } ); test( 'should also see the url contains product-feed', async () => { - expect( page.url() ).toMatch( /path=%2Fgoogle%2Fproduct-feed/ ); + await expect( page.url() ).toMatch( + /path=%2Fgoogle%2Fproduct-feed/ + ); } ); test( 'should see buttons on Dashboard for Google Ads onboarding', async () => { diff --git a/tests/e2e/utils/mock-requests.js b/tests/e2e/utils/mock-requests.js index 8cb320cc6f..b52c794546 100644 --- a/tests/e2e/utils/mock-requests.js +++ b/tests/e2e/utils/mock-requests.js @@ -375,6 +375,21 @@ export default class MockRequests { ); } + /** + * Fulfill the budget recommendations request. + * + * @param {Object} payload + * @return {Promise} + */ + async fulfillBudgetRecommendations( payload ) { + await this.fulfillRequest( + /\/wc\/gla\/ads\/campaigns\/budget-recommendation\b/, + payload, + 200, + [ 'GET' ] + ); + } + /** * Mock the request to connect Jetpack * From 9c3066341bc3aa4b7aac0918a48a34d4ec4b3732 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 22 Aug 2024 21:53:08 +0530 Subject: [PATCH 03/40] Add e2e coverage. --- .../budget-recommendation/index.js | 4 ++-- .../setup-mc/step-4-complete-campaign.test.js | 16 ++++++++++++++++ tests/e2e/utils/pages/setup-ads/setup-budget.js | 9 +++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index 4bc5a8e995..21169b4f94 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -33,12 +33,12 @@ function toRecommendationRange( isMultiple, ...values ) { 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.
Tip: Most merchants targeting similar countries set a daily budget of %1$f %2$s', + 'We recommend running campaigns at least 1 month so it can learn to optimize for your business.
Tip: Most merchants targeting similar countries set a daily budget of %1$f %2$s', '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.
Tip: Most merchants targeting %3$s set a daily budget of %1$f %2$s', + 'We recommend running campaigns at least 1 month so it can learn to optimize for your business.
Tip: Most merchants targeting %3$s set a daily budget of %1$f %2$s', 'google-listings-and-ads' ); diff --git a/tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js b/tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js index 9a9c757fe8..b076ea7263 100644 --- a/tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js +++ b/tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js @@ -302,9 +302,25 @@ test.describe( 'Complete your campaign', () => { textAfterRemoveCountry ); } ); + + test( 'should have the tip text "We recommend running campaigns at least 1 month so it can learn to optimize for your business."', async () => { + const tipText = + setupBudgetPage.getBudgetRecommendationTip(); + await expect( tipText ).toContainText( + 'We recommend running campaigns at least 1 month so it can learn to optimize for your business.' + ); + } ); } ); test.describe( 'Set up budget', () => { + test( '"Daily average cost" input should have highest value set', async () => { + const dailyAverageCostInput = + setupBudgetPage.getBudgetInput(); + await expect( dailyAverageCostInput ).toHaveValue( + '20.00' + ); + } ); + test( 'should see the low budget tip when the buget is set lower than the recommended value', async () => { await setupBudgetPage.fillBudget( '1' ); const lowBudgetTip = setupBudgetPage.getLowerBudgetTip(); diff --git a/tests/e2e/utils/pages/setup-ads/setup-budget.js b/tests/e2e/utils/pages/setup-ads/setup-budget.js index dca6843f64..ac83cf73ea 100644 --- a/tests/e2e/utils/pages/setup-ads/setup-budget.js +++ b/tests/e2e/utils/pages/setup-ads/setup-budget.js @@ -12,6 +12,15 @@ export default class SetupBudget extends MockRequests { this.page = page; } + /** + * Get budget recommendation tip section. + * + * @return {import('@playwright/test').Locator} The budget recommendation text row. + */ + getBudgetRecommendationTip() { + return this.page.locator( '.components-tip p' ); + } + /** * Get budget recommendation text row. * From 103ae13b1474ed672ee9c492561923df3d47e9ca Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 27 Aug 2024 22:35:33 +0530 Subject: [PATCH 04/40] CR feedback - round 1. --- .../setup-stepper/setup-paid-ads/paid-ads-setup-sections.js | 2 +- tests/e2e/utils/pages/setup-ads/setup-budget.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index 98e6154f2c..cffcf604cc 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -37,7 +37,7 @@ const defaultPaidAds = { recommendations: [], isValid: false, isReady: false, - recommendationData: {}, + recommendationData: null, }; /* diff --git a/tests/e2e/utils/pages/setup-ads/setup-budget.js b/tests/e2e/utils/pages/setup-ads/setup-budget.js index ac83cf73ea..56e914412a 100644 --- a/tests/e2e/utils/pages/setup-ads/setup-budget.js +++ b/tests/e2e/utils/pages/setup-ads/setup-budget.js @@ -18,7 +18,9 @@ export default class SetupBudget extends MockRequests { * @return {import('@playwright/test').Locator} The budget recommendation text row. */ getBudgetRecommendationTip() { - return this.page.locator( '.components-tip p' ); + return this.page.locator( + '.gla-budget-recommendation > .components-tip' + ); } /** From 096c0866ad7f6b310410077f0aa4c9e57079fc13 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Fri, 30 Aug 2024 14:55:57 +0530 Subject: [PATCH 05/40] Fix: Step change shows no notice. --- .../setup-paid-ads/paid-ads-setup-sections.js | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index cffcf604cc..570a2dbc95 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -34,10 +34,9 @@ import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; const defaultPaidAds = { amount: 0, countryCodes: [], - recommendations: [], isValid: false, isReady: false, - recommendationData: null, + recommendationData: undefined, }; /* @@ -154,17 +153,27 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { - https://github.com/woocommerce/google-listings-and-ads/blob/5b6522ca10ad75556e6b2de7c120cc712aab70b1/js/src/components/free-listings/setup-free-listings/index.js#L172-L186 */ useEffect( () => { - setPaidAds( ( currentPaidAds ) => { - currentPaidAds = { - ...currentPaidAds, - amount: dailyBudget ? dailyBudget : currentPaidAds.amount, - recommendationData, - }; - return resolveInitialPaidAds( currentPaidAds, targetAudience ); - } ); - }, [ targetAudience, dailyBudget, recommendationData ] ); - - if ( ! targetAudience || ! billingStatus ) { + if ( dailyBudget !== undefined ) { + // If the amount is already set in session, use that one. + const sessionData = clientSession.getCampaign(); + const { amount: sessionAmount } = sessionData; + + setPaidAds( ( currentPaidAds ) => { + currentPaidAds = { + ...currentPaidAds, + amount: sessionAmount || dailyBudget, + recommendationData, + }; + return resolveInitialPaidAds( currentPaidAds, targetAudience ); + } ); + } + }, [ dailyBudget, targetAudience, recommendationData ] ); + + if ( + ! targetAudience || + ! billingStatus || + paidAds.recommendationData === undefined + ) { return (
From 76328ea62f9830811c25de6154b2b44bc6440e69 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 10 Sep 2024 16:40:25 +0530 Subject: [PATCH 06/40] Fix: network error for budget recommendation. --- js/src/components/paid-ads/ads-campaign.js | 11 ++++ .../budget-recommendation/index.js | 55 +++++++++++++++---- .../paid-ads/budget-section/index.js | 53 +++++++++++++----- .../useFetchBudgetRecommendationEffect.js | 0 .../setup-paid-ads/paid-ads-setup-sections.js | 54 ++---------------- 5 files changed, 101 insertions(+), 72 deletions(-) rename js/src/{components/paid-ads/budget-section/budget-recommendation => hooks}/useFetchBudgetRecommendationEffect.js (100%) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 9de67da91e..a8832d6a79 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -1,3 +1,4 @@ +/* eslint-disable react-hooks/exhaustive-deps */ /** * External dependencies */ @@ -17,6 +18,8 @@ import AudienceSection from './audience-section'; import BudgetSection from './budget-section'; import { CampaignPreviewCard } from './campaign-preview'; import FaqsSection from './faqs-section'; +import { useEffect } from 'react'; +import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; /** * @typedef {import('.~/data/actions').Campaign} Campaign @@ -55,6 +58,14 @@ export default function AdsCampaign( { 'google-listings-and-ads' ); + // Set the amount from client session if it is available. + useEffect( () => { + const sessionData = clientSession.getCampaign(); + if ( sessionData?.amount !== undefined ) { + formContext.setValue( 'amount', sessionData.amount ); + } + }, [] ); // eslint-disable-line react-hooks/exhaustive-deps + return ( { - if ( challenger.daily_budget > defender.daily_budget ) { - return challenger; + return recommendations.reduce( + ( defender, challenger ) => { + if ( challenger.daily_budget > defender.daily_budget ) { + return challenger; + } + return defender; + }, + { + daily_budget: 0, } - return defender; - } ); + ); } function toRecommendationRange( isMultiple, ...values ) { @@ -49,20 +57,45 @@ function toRecommendationRange( isMultiple, ...values ) { } const BudgetRecommendation = ( props ) => { - const { countryCodes, dailyAverageCost = Infinity, data } = props; + const { + countryCodes, + dailyAverageCost = Infinity, + setRecommendedBudget, + setRecommendationsLoaded, + } = props; + const map = useCountryKeyNameMap(); + const { data, loading } = + useFetchBudgetRecommendationEffect( countryCodes ); - if ( ! data ) { - return null; - } + const recommendationsLoaded = ! loading; - const { currency, recommendations } = data; + const { currency, recommendations = [] } = data || {}; const { daily_budget: dailyBudget, country } = getHighestBudget( recommendations.filter( ( recommendation ) => countryCodes.includes( recommendation.country ) ) ); + useEffect( () => { + if ( recommendationsLoaded ) { + const sessionData = clientSession.getCampaign(); + const sessionAmount = + sessionData?.amount !== undefined + ? dailyAverageCost + : dailyBudget; + + setRecommendationsLoaded( true ); + setRecommendedBudget( sessionAmount ); + } + }, [ + dailyAverageCost, + dailyBudget, + recommendationsLoaded, + setRecommendationsLoaded, + setRecommendedBudget, + ] ); + const countryName = map[ country ]; const recommendationRange = toRecommendationRange( recommendations.length > 1, diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index c4d44c0993..c12d639b4c 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -2,7 +2,7 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import { useEffect, useRef } from '@wordpress/element'; +import { useEffect, useRef, useState } from '@wordpress/element'; /** * Internal dependencies @@ -13,6 +13,7 @@ import './index.scss'; import BudgetRecommendation from './budget-recommendation'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import AppInputPriceControl from '.~/components/app-input-price-control'; +import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; const nonInteractableProps = { noPointerEvents: true, @@ -30,7 +31,7 @@ const nonInteractableProps = { */ const BudgetSection = ( { formProps, disabled = false, children } ) => { const { getInputProps, setValue, values } = formProps; - const { countryCodes, amount, recommendationData } = values; + const { countryCodes, amount } = values; const { googleAdsAccount } = useGoogleAdsAccount(); const monthlyMaxEstimated = getMonthlyMaxEstimated( amount ); // Display the currency code that will be used by Google Ads, but still use the store's currency formatting settings. @@ -42,16 +43,40 @@ const BudgetSection = ( { formProps, disabled = false, children } ) => { 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 . - * - from true to false, then reset amount to the initial value passed from the consumer side. - */ - const initialAmountRef = useRef( amount ); + const [ recommendedBudget, setRecommendedBudget ] = useState( null ); + const [ recommendationsLoaded, setRecommendationsLoaded ] = + useState( false ); + useEffect( () => { - const nextAmount = disabled ? undefined : initialAmountRef.current; - setValueRef.current( 'amount', nextAmount ); - }, [ disabled ] ); + if ( ! recommendationsLoaded || recommendedBudget === null ) { + return; + } + + const sessionData = clientSession.getCampaign(); + + let sessionAmount; + if ( sessionData?.amount === undefined ) { + sessionAmount = recommendedBudget; + } else { + sessionAmount = amount; + } + + if ( disabled ) { + sessionAmount = undefined; + } + + clientSession.setCampaign( { + amount: sessionAmount, + countryCodes, + } ); + setValueRef.current( 'amount', sessionAmount ); + }, [ + amount, + countryCodes, + disabled, + recommendationsLoaded, + recommendedBudget, + ] ); return (
@@ -93,8 +118,10 @@ const BudgetSection = ( { formProps, disabled = false, children } ) => { ) } diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/useFetchBudgetRecommendationEffect.js b/js/src/hooks/useFetchBudgetRecommendationEffect.js similarity index 100% rename from js/src/components/paid-ads/budget-section/budget-recommendation/useFetchBudgetRecommendationEffect.js rename to js/src/hooks/useFetchBudgetRecommendationEffect.js diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index 570a2dbc95..215137fe68 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -11,7 +11,6 @@ import { Form } from '@woocommerce/components'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes'; import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; -import useFetchBudgetRecommendationEffect from '.~/components/paid-ads/budget-section/budget-recommendation/useFetchBudgetRecommendationEffect'; import AudienceSection from '.~/components/paid-ads/audience-section'; import BudgetSection from '.~/components/paid-ads/budget-section'; import BillingCard from '.~/components/paid-ads/billing-card'; @@ -36,25 +35,8 @@ const defaultPaidAds = { countryCodes: [], isValid: false, isReady: false, - recommendationData: undefined, }; -/* - * 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; - } ); -} - /** * Resolve the initial paid ads data from the given paid ads data with the loaded target audience. * Parts of the resolved data are used in the `initialValues` prop of `Form` component. @@ -96,14 +78,6 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { const { hasGoogleAdsConnection } = useGoogleAdsAccount(); const { data: targetAudience } = useTargetAudienceFinalCountryCodes(); const { billingStatus } = useGoogleAdsAccountBillingStatus(); - const { data: recommendationData } = - useFetchBudgetRecommendationEffect( targetAudience ); - const { recommendations } = recommendationData || {}; - - const { daily_budget: dailyBudget } = - recommendations !== undefined - ? getHighestBudget( recommendations ) - : {}; const onStatesReceivedRef = useRef(); onStatesReceivedRef.current = onStatesReceived; @@ -140,7 +114,7 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { isReady: paidAds.isValid && isBillingCompleted, }; onStatesReceivedRef.current( nextPaidAds ); - clientSession.setCampaign( nextPaidAds ); + // clientSession.setCampaign( nextPaidAds ); }, [ paidAds, isBillingCompleted ] ); /* @@ -153,27 +127,12 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { - https://github.com/woocommerce/google-listings-and-ads/blob/5b6522ca10ad75556e6b2de7c120cc712aab70b1/js/src/components/free-listings/setup-free-listings/index.js#L172-L186 */ useEffect( () => { - if ( dailyBudget !== undefined ) { - // If the amount is already set in session, use that one. - const sessionData = clientSession.getCampaign(); - const { amount: sessionAmount } = sessionData; - - setPaidAds( ( currentPaidAds ) => { - currentPaidAds = { - ...currentPaidAds, - amount: sessionAmount || dailyBudget, - recommendationData, - }; - return resolveInitialPaidAds( currentPaidAds, targetAudience ); - } ); - } - }, [ dailyBudget, targetAudience, recommendationData ] ); + setPaidAds( ( currentPaidAds ) => + resolveInitialPaidAds( currentPaidAds, targetAudience ) + ); + }, [ targetAudience ] ); - if ( - ! targetAudience || - ! billingStatus || - paidAds.recommendationData === undefined - ) { + if ( ! targetAudience || ! billingStatus ) { return (
@@ -184,7 +143,6 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { const initialValues = { countryCodes: paidAds.countryCodes, amount: paidAds.amount, - recommendationData: paidAds.recommendationData, }; return ( From 8d9358dd83248e31c929d9fe804bed0fa0bf6318 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 10 Sep 2024 17:30:04 +0530 Subject: [PATCH 07/40] Remove redundant code. --- .../budget-section/budget-recommendation/index.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index 9f26d108b8..8f7a33d657 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -13,7 +13,6 @@ import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap'; import './index.scss'; import useFetchBudgetRecommendationEffect from '.~/hooks/useFetchBudgetRecommendationEffect'; import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; -import { useCallback } from 'react'; /* * If a merchant selects more than one country, the budget recommendation @@ -71,11 +70,8 @@ const BudgetRecommendation = ( props ) => { const recommendationsLoaded = ! loading; const { currency, recommendations = [] } = data || {}; - const { daily_budget: dailyBudget, country } = getHighestBudget( - recommendations.filter( ( recommendation ) => - countryCodes.includes( recommendation.country ) - ) - ); + const { daily_budget: dailyBudget, country } = + getHighestBudget( recommendations ); useEffect( () => { if ( recommendationsLoaded ) { From ea68ee8a3ab43fd564f3116bf4dae6aa4ef0cde8 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 10 Sep 2024 17:35:17 +0530 Subject: [PATCH 08/40] Update js doc. --- tests/e2e/utils/pages/setup-ads/setup-budget.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/utils/pages/setup-ads/setup-budget.js b/tests/e2e/utils/pages/setup-ads/setup-budget.js index 56e914412a..78cb682aec 100644 --- a/tests/e2e/utils/pages/setup-ads/setup-budget.js +++ b/tests/e2e/utils/pages/setup-ads/setup-budget.js @@ -15,7 +15,7 @@ export default class SetupBudget extends MockRequests { /** * Get budget recommendation tip section. * - * @return {import('@playwright/test').Locator} The budget recommendation text row. + * @return {import('@playwright/test').Locator} The budget recommendation tip. */ getBudgetRecommendationTip() { return this.page.locator( From ac68177e193f71d642e1881ec71b159a16e2fe36 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 10 Sep 2024 18:36:08 +0530 Subject: [PATCH 09/40] Fix: lint issues. --- js/src/components/paid-ads/ads-campaign.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index a8832d6a79..13ea475116 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -3,7 +3,7 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import { createInterpolateElement } from '@wordpress/element'; +import { createInterpolateElement, useEffect } from '@wordpress/element'; /** * Internal dependencies @@ -18,7 +18,6 @@ import AudienceSection from './audience-section'; import BudgetSection from './budget-section'; import { CampaignPreviewCard } from './campaign-preview'; import FaqsSection from './faqs-section'; -import { useEffect } from 'react'; import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; /** From 052783e75fa704a8c117ed27be597d925b8e10c7 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 10 Sep 2024 18:45:18 +0530 Subject: [PATCH 10/40] Remove unused code. --- .../setup-stepper/setup-paid-ads/paid-ads-setup-sections.js | 1 - 1 file changed, 1 deletion(-) diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index 215137fe68..ac80d0b273 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -114,7 +114,6 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { isReady: paidAds.isValid && isBillingCompleted, }; onStatesReceivedRef.current( nextPaidAds ); - // clientSession.setCampaign( nextPaidAds ); }, [ paidAds, isBillingCompleted ] ); /* From 61d73ebc1b369801a8b271aab69197582ae7c03e Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 12 Sep 2024 15:19:48 +0530 Subject: [PATCH 11/40] use fetch budget hook in paid-ads-setup-sections. --- .../budget-recommendation/index.js | 36 ++--------- .../paid-ads/budget-section/index.js | 64 +++++-------------- .../setup-paid-ads/paid-ads-setup-sections.js | 44 ++++++++++--- .../setup-paid-ads/setup-paid-ads.js | 10 +-- js/src/utils/getHighestBudget.js | 25 ++++++++ 5 files changed, 86 insertions(+), 93 deletions(-) create mode 100644 js/src/utils/getHighestBudget.js diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index 8f7a33d657..ae2f52b69f 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -57,44 +57,18 @@ function toRecommendationRange( isMultiple, ...values ) { const BudgetRecommendation = ( props ) => { const { - countryCodes, + isMultiple, dailyAverageCost = Infinity, - setRecommendedBudget, - setRecommendationsLoaded, + dailyBudget, + country, + currency, } = props; const map = useCountryKeyNameMap(); - const { data, loading } = - useFetchBudgetRecommendationEffect( countryCodes ); - - const recommendationsLoaded = ! loading; - - const { currency, recommendations = [] } = data || {}; - const { daily_budget: dailyBudget, country } = - getHighestBudget( recommendations ); - - useEffect( () => { - if ( recommendationsLoaded ) { - const sessionData = clientSession.getCampaign(); - const sessionAmount = - sessionData?.amount !== undefined - ? dailyAverageCost - : dailyBudget; - - setRecommendationsLoaded( true ); - setRecommendedBudget( sessionAmount ); - } - }, [ - dailyAverageCost, - dailyBudget, - recommendationsLoaded, - setRecommendationsLoaded, - setRecommendedBudget, - ] ); const countryName = map[ country ]; const recommendationRange = toRecommendationRange( - recommendations.length > 1, + isMultiple, dailyBudget, currency, countryName diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index cff195546b..611a4bba61 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -2,7 +2,7 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import { useEffect, useRef, useState } from '@wordpress/element'; +import { useRef, useEffect, useState } from '@wordpress/element'; /** * Internal dependencies @@ -13,7 +13,6 @@ import './index.scss'; import BudgetRecommendation from './budget-recommendation'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import AppInputPriceControl from '.~/components/app-input-price-control'; -import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; /** * @typedef {import('.~/data/actions').CountryCode} CountryCode @@ -30,15 +29,21 @@ const nonInteractableProps = { * * @param {Object} props React props. * @param {Object} props.formProps Form props forwarded from `Form` component. - * @param {Array|undefined} props.countryCodes Country codes to fetch budget recommendations for. + * @param {string} props.country Country code. + * @param {Array} 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, + country, countryCodes, + dailyBudget, disabled = false, children, + isMultiple, } ) => { const { getInputProps, setValue, values } = formProps; const { amount } = values; @@ -53,41 +58,6 @@ const BudgetSection = ( { const setValueRef = useRef(); setValueRef.current = setValue; - const [ recommendedBudget, setRecommendedBudget ] = useState( null ); - const [ recommendationsLoaded, setRecommendationsLoaded ] = - useState( false ); - - useEffect( () => { - if ( ! recommendationsLoaded || recommendedBudget === null ) { - return; - } - - const sessionData = clientSession.getCampaign(); - - let sessionAmount; - if ( sessionData?.amount === undefined ) { - sessionAmount = recommendedBudget; - } else { - sessionAmount = amount; - } - - if ( disabled ) { - sessionAmount = undefined; - } - - clientSession.setCampaign( { - amount: sessionAmount, - countryCodes, - } ); - setValueRef.current( 'amount', sessionAmount ); - }, [ - amount, - countryCodes, - disabled, - recommendationsLoaded, - recommendedBudget, - ] ); - return (
- { countryCodes?.length > 0 && ( - - ) } + { children } diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index 89e8a86341..051b2f5d16 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { useState, useRef, useEffect } from '@wordpress/element'; +import { useState, useRef, useEffect, useCallback } from '@wordpress/element'; import { Form } from '@woocommerce/components'; /** @@ -15,6 +15,8 @@ import Section from '.~/wcdl/section'; import validateCampaign from '.~/components/paid-ads/validateCampaign'; import clientSession from './clientSession'; import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; +import useFetchBudgetRecommendationEffect from '.~/hooks/useFetchBudgetRecommendationEffect'; +import getHighestBudget from '.~/utils/getHighestBudget'; /** * @typedef {import('.~/data/actions').CountryCode} CountryCode @@ -76,6 +78,13 @@ export default function PaidAdsSetupSections( { const isBillingCompleted = billingStatus?.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; + const { data: budgetData, loading } = + useFetchBudgetRecommendationEffect( countryCodes ); + const { country = '', daily_budget: dailyBudget } = getHighestBudget( + budgetData?.recommendations || [] + ); + const multipleRecommendations = budgetData?.recommendations.length > 1; + /* If a merchant has not yet finished the billing setup, the billing status will be updated by `useAutoCheckBillingStatusEffect` hook in `BillingSetupCard` component @@ -91,14 +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 ); - }, [ 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 (
@@ -107,7 +124,7 @@ export default function PaidAdsSetupSections( { } const initialValues = { - amount: paidAds.amount, + amount: clientSession.getCampaign()?.amount || dailyBudget, }; return ( @@ -115,6 +132,10 @@ export default function PaidAdsSetupSections( { initialValues={ initialValues } onChange={ ( _, values, isValid ) => { setPaidAds( { ...paidAds, ...values, isValid } ); + + if ( isValid ) { + clientSession.setCampaign( values ); + } } } validate={ validateCampaign } > @@ -123,6 +144,9 @@ export default function PaidAdsSetupSections( { diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/setup-paid-ads.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/setup-paid-ads.js index cd5f77ac13..1321f2c4af 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/setup-paid-ads.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/setup-paid-ads.js @@ -171,10 +171,12 @@ export default function SetupPaidAds() { - + { countryCodes?.length > 0 && ( + + ) } { showSkipPaidAdsConfirmationModal && ( diff --git a/js/src/utils/getHighestBudget.js b/js/src/utils/getHighestBudget.js new file mode 100644 index 0000000000..9d9dbb3c48 --- /dev/null +++ b/js/src/utils/getHighestBudget.js @@ -0,0 +1,25 @@ +/* + * 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 ) { + if ( ! recommendations ) { + return null; + } + + return recommendations.reduce( + ( defender, challenger ) => { + if ( challenger.daily_budget > defender.daily_budget ) { + return challenger; + } + return defender; + }, + { + daily_budget: 0, + country: '', + } + ); +} From f15bfb965e655d47189e676d4e95bffd99ae927e Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 12 Sep 2024 15:21:44 +0530 Subject: [PATCH 12/40] Remove redundant code. --- .../budget-recommendation/index.js | 25 +------------------ .../paid-ads/budget-section/index.js | 2 +- .../setup-paid-ads/paid-ads-setup-sections.js | 2 +- 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index ae2f52b69f..b9a580a643 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -2,7 +2,7 @@ * External dependencies */ import { __, sprintf } from '@wordpress/i18n'; -import { createInterpolateElement, useEffect } from '@wordpress/element'; +import { createInterpolateElement } from '@wordpress/element'; import { Tip } from '@wordpress/components'; import GridiconNoticeOutline from 'gridicons/dist/notice-outline'; @@ -11,29 +11,6 @@ import GridiconNoticeOutline from 'gridicons/dist/notice-outline'; */ import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap'; import './index.scss'; -import useFetchBudgetRecommendationEffect from '.~/hooks/useFetchBudgetRecommendationEffect'; -import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; - -/* - * 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; - }, - { - daily_budget: 0, - } - ); -} function toRecommendationRange( isMultiple, ...values ) { const conversionMap = { strong: , em: , br:
}; diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index 611a4bba61..9737e9296a 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -2,7 +2,7 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import { useRef, useEffect, useState } from '@wordpress/element'; +import { useRef } from '@wordpress/element'; /** * Internal dependencies diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index 051b2f5d16..6820c33ed4 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { useState, useRef, useEffect, useCallback } from '@wordpress/element'; +import { useState, useRef, useEffect } from '@wordpress/element'; import { Form } from '@woocommerce/components'; /** From 85f5b3574c7ddf6d39d5d8a996af369eae3f8fd4 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Thu, 12 Sep 2024 17:22:43 +0530 Subject: [PATCH 13/40] Display spinner till data gets loaded. --- js/src/components/paid-ads/ads-campaign.js | 46 ++++++++++++++++------ 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index f2c48600cd..506f3ca2e9 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -19,6 +19,10 @@ import BudgetSection from './budget-section'; import { CampaignPreviewCard } from './campaign-preview'; import FaqsSection from './faqs-section'; import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; +import useFetchBudgetRecommendationEffect from '.~/hooks/useFetchBudgetRecommendationEffect'; +import getHighestBudget from '.~/utils/getHighestBudget'; +import Section from '.~/wcdl/section'; +import SpinnerCard from '../spinner-card'; /** * @typedef {import('.~/data/actions').Campaign} Campaign @@ -57,13 +61,23 @@ export default function AdsCampaign( { 'google-listings-and-ads' ); + const { data: budgetData, loading } = useFetchBudgetRecommendationEffect( + formContext.values.countryCodes + ); + + const { country = '', daily_budget: dailyBudget } = getHighestBudget( + budgetData?.recommendations || [] + ); + + const multipleRecommendations = budgetData?.recommendations.length > 1; + // Set the amount from client session if it is available. useEffect( () => { - const sessionData = clientSession.getCampaign(); - if ( sessionData?.amount !== undefined ) { - formContext.setValue( 'amount', sessionData.amount ); + if ( ! loading ) { + const { amount } = clientSession.getCampaign() || {}; + formContext.setValue( 'amount', amount || dailyBudget ); } - }, [] ); // eslint-disable-line react-hooks/exhaustive-deps + }, [ loading ] ); return ( @@ -101,13 +115,23 @@ export default function AdsCampaign( { countrySelectHelperText={ helperText } formProps={ formContext } /> - - - + { ! loading && ( + + + + ) } + { loading && ( +
+ +
+ ) } Date: Mon, 16 Sep 2024 23:52:28 +0530 Subject: [PATCH 14/40] Add hook for code resuse. --- js/src/hooks/useBudgetRecommendationData.js | 45 +++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 js/src/hooks/useBudgetRecommendationData.js diff --git a/js/src/hooks/useBudgetRecommendationData.js b/js/src/hooks/useBudgetRecommendationData.js new file mode 100644 index 0000000000..37266aaddb --- /dev/null +++ b/js/src/hooks/useBudgetRecommendationData.js @@ -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 ); + + const [ country, setCountry ] = useState( '' ); + const [ dailyBudget, setDailyBudget ] = useState( 0 ); + const [ multipleRecommendations, setMultipleRecommendations ] = + useState( false ); + const [ recommendations, setRecommendations ] = useState( [] ); + + useEffect( () => { + if ( ! loading ) { + const { + country: budgetCountries = '', + daily_budget: recommendedDailyBudget, + } = getHighestBudget( data?.recommendations ); + + setCountry( budgetCountries ); + setDailyBudget( recommendedDailyBudget ); + setMultipleRecommendations( data?.recommendations.length > 1 ); + setRecommendations( data?.recommendations ); + } + }, [ data?.recommendations, loading ] ); + + return { + country, + dailyBudget, + multipleRecommendations, + recommendations, + loading, + }; +}; + +export default useBudgetRecommendationData; From dc7a4ac326b13180ee7e4d8226d16e8bbaf7df50 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Mon, 16 Sep 2024 23:52:58 +0530 Subject: [PATCH 15/40] Use hook. --- js/src/components/paid-ads/ads-campaign.js | 23 ++++++++----------- .../budget-recommendation/index.js | 6 ++--- .../paid-ads/budget-section/index.js | 22 +++++++++--------- .../pages/create-paid-ads-campaign/index.js | 7 ++++++ js/src/setup-ads/setup-ads-form.js | 8 ++++++- .../setup-paid-ads/paid-ads-setup-sections.js | 9 +++----- 6 files changed, 41 insertions(+), 34 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 506f3ca2e9..a8e5d6f6f8 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -18,11 +18,10 @@ import AudienceSection from './audience-section'; import BudgetSection from './budget-section'; import { CampaignPreviewCard } from './campaign-preview'; import FaqsSection from './faqs-section'; -import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; -import useFetchBudgetRecommendationEffect from '.~/hooks/useFetchBudgetRecommendationEffect'; -import getHighestBudget from '.~/utils/getHighestBudget'; import Section from '.~/wcdl/section'; import SpinnerCard from '../spinner-card'; +import useBudgetRecommendationData from '.~/hooks/useBudgetRecommendationData'; +import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; /** * @typedef {import('.~/data/actions').Campaign} Campaign @@ -61,17 +60,14 @@ export default function AdsCampaign( { 'google-listings-and-ads' ); - const { data: budgetData, loading } = useFetchBudgetRecommendationEffect( - formContext.values.countryCodes - ); - - const { country = '', daily_budget: dailyBudget } = getHighestBudget( - budgetData?.recommendations || [] - ); - - const multipleRecommendations = budgetData?.recommendations.length > 1; + const { + country, + dailyBudget, + multipleRecommendations, + recommendations, + loading, + } = useBudgetRecommendationData( formContext.values.countryCodes ); - // Set the amount from client session if it is available. useEffect( () => { if ( ! loading ) { const { amount } = clientSession.getCampaign() || {}; @@ -123,6 +119,7 @@ export default function AdsCampaign( { country={ country } dailyBudget={ dailyBudget } isMultiple={ multipleRecommendations } + recommendations={ recommendations } > diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index b9a580a643..8dbe83a4b4 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -34,11 +34,11 @@ function toRecommendationRange( isMultiple, ...values ) { const BudgetRecommendation = ( props ) => { const { - isMultiple, dailyAverageCost = Infinity, - dailyBudget, - country, currency, + country, + dailyBudget, + isMultiple, } = props; const map = useCountryKeyNameMap(); diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index 9737e9296a..39af15ab06 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -38,12 +38,10 @@ const nonInteractableProps = { */ const BudgetSection = ( { formProps, - country, countryCodes, - dailyBudget, disabled = false, children, - isMultiple, + ...recommendationProps } ) => { const { getInputProps, setValue, values } = formProps; const { amount } = values; @@ -58,6 +56,8 @@ const BudgetSection = ( { const setValueRef = useRef(); setValueRef.current = setValue; + const { recommendations } = recommendationProps || {}; + return (
- + { recommendations?.length > 0 && ( + + ) } { children } diff --git a/js/src/pages/create-paid-ads-campaign/index.js b/js/src/pages/create-paid-ads-campaign/index.js index e076243e6a..2a373e0a00 100644 --- a/js/src/pages/create-paid-ads-campaign/index.js +++ b/js/src/pages/create-paid-ads-campaign/index.js @@ -32,6 +32,7 @@ import { 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'; @@ -133,6 +134,12 @@ const CreatePaidAdsCampaign = () => { countryCodes: initialCountryCodes, } } onSubmit={ handleSubmit } + onChange={ ( _, values, isValid ) => { + // Set the amount in session storage. + if ( isValid && _?.name === 'amount' ) { + clientSession.setCampaign( { amount: _.value } ); + } + } } > { } ); }; - const handleChange = ( _, values ) => { + const handleChange = ( _, values, isValid ) => { const args = [ initialValues, values ].map( ( { countryCodes, ...v } ) => { v.countrySet = new Set( countryCodes ); @@ -75,6 +76,11 @@ const SetupAdsForm = () => { } ); + // Set the amount in session storage. + if ( isValid && _?.name === 'amount' ) { + clientSession.setCampaign( { amount: _.value } ); + } + setFormChanged( ! isEqual( ...args ) ); }; diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index 6820c33ed4..b7738747aa 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -17,6 +17,7 @@ import clientSession from './clientSession'; import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; import useFetchBudgetRecommendationEffect from '.~/hooks/useFetchBudgetRecommendationEffect'; import getHighestBudget from '.~/utils/getHighestBudget'; +import useBudgetRecommendationData from '.~/hooks/useBudgetRecommendationData'; /** * @typedef {import('.~/data/actions').CountryCode} CountryCode @@ -78,12 +79,8 @@ export default function PaidAdsSetupSections( { const isBillingCompleted = billingStatus?.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; - const { data: budgetData, loading } = - useFetchBudgetRecommendationEffect( countryCodes ); - const { country = '', daily_budget: dailyBudget } = getHighestBudget( - budgetData?.recommendations || [] - ); - const multipleRecommendations = budgetData?.recommendations.length > 1; + const { country, dailyBudget, multipleRecommendations, loading } = + useBudgetRecommendationData( countryCodes ); /* If a merchant has not yet finished the billing setup, the billing status will be From 9ddfef46fe6156eb688c17e53d292b3a121ac221 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 17 Sep 2024 00:12:26 +0530 Subject: [PATCH 16/40] Stick to old UI/UX. --- js/src/components/paid-ads/ads-campaign.js | 29 ++++++++-------------- 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index a8e5d6f6f8..e3e2b22945 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -111,24 +111,17 @@ export default function AdsCampaign( { countrySelectHelperText={ helperText } formProps={ formContext } /> - { ! loading && ( - - - - ) } - { loading && ( -
- -
- ) } + + + Date: Tue, 17 Sep 2024 00:13:09 +0530 Subject: [PATCH 17/40] Remove redundant code. --- js/src/components/paid-ads/ads-campaign.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index e3e2b22945..816937b7b0 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -18,8 +18,6 @@ import AudienceSection from './audience-section'; import BudgetSection from './budget-section'; import { CampaignPreviewCard } from './campaign-preview'; import FaqsSection from './faqs-section'; -import Section from '.~/wcdl/section'; -import SpinnerCard from '../spinner-card'; import useBudgetRecommendationData from '.~/hooks/useBudgetRecommendationData'; import clientSession from '.~/setup-mc/setup-stepper/setup-paid-ads/clientSession'; From 1b92ecb5c6f8befdd636841a441b79481ef07602 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 17 Sep 2024 00:49:22 +0530 Subject: [PATCH 18/40] Pass recommendations prop. --- .../setup-paid-ads/paid-ads-setup-sections.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index b7738747aa..9543b1fb9f 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -79,8 +79,13 @@ export default function PaidAdsSetupSections( { const isBillingCompleted = billingStatus?.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; - const { country, dailyBudget, multipleRecommendations, loading } = - useBudgetRecommendationData( countryCodes ); + const { + country, + dailyBudget, + recommendations, + multipleRecommendations, + loading, + } = useBudgetRecommendationData( countryCodes ); /* If a merchant has not yet finished the billing setup, the billing status will be @@ -141,9 +146,10 @@ export default function PaidAdsSetupSections( { From ab2db99ecb2af26eb5fc9ff8f7d63564e24e94f0 Mon Sep 17 00:00:00 2001 From: Ankit Gade Date: Tue, 17 Sep 2024 00:51:52 +0530 Subject: [PATCH 19/40] Remove redundant code. --- .../setup-stepper/setup-paid-ads/paid-ads-setup-sections.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js index 9543b1fb9f..f8776df8be 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js @@ -15,8 +15,6 @@ import Section from '.~/wcdl/section'; import validateCampaign from '.~/components/paid-ads/validateCampaign'; import clientSession from './clientSession'; import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; -import useFetchBudgetRecommendationEffect from '.~/hooks/useFetchBudgetRecommendationEffect'; -import getHighestBudget from '.~/utils/getHighestBudget'; import useBudgetRecommendationData from '.~/hooks/useBudgetRecommendationData'; /** From eb1817855752a74abcbc2010463dbc90b283a8ab Mon Sep 17 00:00:00 2001 From: asvinb Date: Wed, 2 Oct 2024 20:11:00 +0400 Subject: [PATCH 20/40] Try not to use the useBudgetRecommendationData hook. --- .../paid-ads/ads-campaign/ads-campaign.js | 26 +++++----- .../ads-campaign/paid-ads-setup-sections.js | 16 +++++-- .../budget-recommendation/index.js | 47 +++++++++++++++---- js/src/hooks/useBudgetRecommendationData.js | 1 + .../pages/create-paid-ads-campaign/index.js | 7 --- js/src/setup-ads/setup-ads-form.js | 8 +--- js/src/utils/getHighestBudget.js | 5 +- 7 files changed, 69 insertions(+), 41 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign/ads-campaign.js b/js/src/components/paid-ads/ads-campaign/ads-campaign.js index 57fec98243..fc9e3aba20 100644 --- a/js/src/components/paid-ads/ads-campaign/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign/ads-campaign.js @@ -24,7 +24,6 @@ import PaidAdsFeaturesSection from './paid-ads-features-section'; import PaidAdsSetupSections from './paid-ads-setup-sections'; import SkipButton from './skip-button'; import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes'; -import useBudgetRecommendationData from '.~/hooks/useBudgetRecommendationData'; import { ACTION_SKIP, ACTION_COMPLETE } from './constants'; /** @@ -70,7 +69,6 @@ export default function AdsCampaign( { const { isValidForm, setValue } = formContext; const [ completing, setCompleting ] = useState( null ); const [ paidAds, setPaidAds ] = useState( {} ); - const { dailyBudget } = useBudgetRecommendationData( countryCodes ); useEffect( () => { if ( hasError ) { @@ -150,17 +148,19 @@ export default function AdsCampaign( { { isOnboardingFlow && } - + { /* { ! loading && ( + + ) } */ } diff --git a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections.js b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections.js index d48c4abced..68e670af55 100644 --- a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections.js +++ b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections.js @@ -81,22 +81,28 @@ export default function PaidAdsSetupSections( { onStatesReceivedRef.current = onStatesReceived; const [ paidAds, setPaidAds ] = useState( () => { - // Resolve the starting paid ads data with the campaign data stored in the client session. let startingPaidAds = { ...defaultPaidAds, }; - if ( loadCampaignFromClientSession ) { + // If we are creating a new campaign, set the amount with the recommended daily amount. + if ( ! campaign ) { startingPaidAds = { ...startingPaidAds, - ...clientSession.getCampaign(), + amount: recommendedDailyAmount, }; - } else { + } + + // Resolve the starting paid ads data with the campaign data stored in the client session if any. + if ( loadCampaignFromClientSession ) { startingPaidAds = { ...startingPaidAds, - amount: recommendedDailyAmount, + amount: + clientSession.getCampaign()?.amount || + recommendedDailyAmount, }; } + return resolveInitialPaidAds( startingPaidAds ); } ); diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index f688a954d2..1a3dfc99ac 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -10,10 +10,25 @@ import GridiconNoticeOutline from 'gridicons/dist/notice-outline'; * Internal dependencies */ import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap'; -import useAdsCurrency from '.~/hooks/useAdsCurrency'; -import useBudgetRecommendationData from '.~/hooks/useBudgetRecommendationData'; +import useFetchBudgetRecommendationEffect from '.~/hooks/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: , em: , br:
}; const template = isMultiple @@ -36,21 +51,37 @@ function toRecommendationRange( isMultiple, ...values ) { const BudgetRecommendation = ( props ) => { const { countryCodes, dailyAverageCost = Infinity } = props; - const { country, dailyBudget, multipleRecommendations } = - useBudgetRecommendationData( countryCodes ); + const { data } = useFetchBudgetRecommendationEffect( countryCodes ); const map = useCountryKeyNameMap(); - const { - currencyConfig: { code: currency }, - } = useAdsCurrency(); + + if ( ! data ) { + return null; + } + + const { currency, recommendations } = data; + const { daily_budget: dailyBudget, country } = + getHighestBudget( recommendations ); const countryName = map[ country ]; const recommendationRange = toRecommendationRange( - multipleRecommendations, + recommendations.length > 1, dailyBudget, currency, countryName ); + // const { currency, recommendations } = data; + // const { daily_budget: dailyBudget, country } = + // getHighestBudget( recommendations ); + + // const countryName = map[ country ]; + // const recommendationRange = toRecommendationRange( + // multipleRecommendations, + // dailyBudget, + // currency, + // countryName + // ); + const showLowerBudgetNotice = dailyAverageCost < dailyBudget; return ( diff --git a/js/src/hooks/useBudgetRecommendationData.js b/js/src/hooks/useBudgetRecommendationData.js index 37266aaddb..542188b3f1 100644 --- a/js/src/hooks/useBudgetRecommendationData.js +++ b/js/src/hooks/useBudgetRecommendationData.js @@ -39,6 +39,7 @@ const useBudgetRecommendationData = ( countryCodes ) => { multipleRecommendations, recommendations, loading, + data, }; }; diff --git a/js/src/pages/create-paid-ads-campaign/index.js b/js/src/pages/create-paid-ads-campaign/index.js index aaa2d48ea9..534c75bcb7 100644 --- a/js/src/pages/create-paid-ads-campaign/index.js +++ b/js/src/pages/create-paid-ads-campaign/index.js @@ -32,7 +32,6 @@ import { 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'; @@ -134,12 +133,6 @@ const CreatePaidAdsCampaign = () => { countryCodes: initialCountryCodes, } } onSubmit={ handleSubmit } - onChange={ ( _, values, isValid ) => { - // Set the amount in session storage. - if ( isValid && _?.name === 'amount' ) { - clientSession.setCampaign( { amount: _.value } ); - } - } } > { } ); }; - const handleChange = ( _, values, isValid ) => { + const handleChange = ( _, values ) => { const args = [ initialValues, values ].map( ( { countryCodes, ...v } ) => { v.countrySet = new Set( countryCodes ); @@ -76,11 +75,6 @@ const SetupAdsForm = () => { } ); - // Set the amount in session storage. - if ( isValid && _?.name === 'amount' ) { - clientSession.setCampaign( { amount: _.value } ); - } - setFormChanged( ! isEqual( ...args ) ); }; diff --git a/js/src/utils/getHighestBudget.js b/js/src/utils/getHighestBudget.js index 9d9dbb3c48..b31b46742e 100644 --- a/js/src/utils/getHighestBudget.js +++ b/js/src/utils/getHighestBudget.js @@ -7,7 +7,10 @@ */ export default function getHighestBudget( recommendations ) { if ( ! recommendations ) { - return null; + return { + daily_budget: 0, + country: '', + }; } return recommendations.reduce( From 35716f0e216165fa12befbc9d2c265a3acf14239 Mon Sep 17 00:00:00 2001 From: asvinb Date: Wed, 2 Oct 2024 20:22:52 +0400 Subject: [PATCH 21/40] Add useFetchBudgetRecommendation hook. --- .../paid-ads/ads-campaign/ads-campaign.js | 24 ++++----- js/src/data/action-types.js | 1 + js/src/data/reducer.js | 14 +++++ js/src/data/resolvers.js | 53 ++++++++++++++++++- js/src/data/selectors.js | 20 ++++++- js/src/data/test/reducer.test.js | 34 ++++++++++++ js/src/data/utils.js | 18 +++++++ js/src/hooks/useFetchBudgetRecommendation.js | 40 ++++++++++++++ 8 files changed, 189 insertions(+), 15 deletions(-) create mode 100644 js/src/hooks/useFetchBudgetRecommendation.js diff --git a/js/src/components/paid-ads/ads-campaign/ads-campaign.js b/js/src/components/paid-ads/ads-campaign/ads-campaign.js index fc9e3aba20..d3b1367f2f 100644 --- a/js/src/components/paid-ads/ads-campaign/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign/ads-campaign.js @@ -148,19 +148,17 @@ export default function AdsCampaign( { { isOnboardingFlow && } - { /* { ! loading && ( - - ) } */ } + diff --git a/js/src/data/action-types.js b/js/src/data/action-types.js index 4d27fb8155..92afeceeef 100644 --- a/js/src/data/action-types.js +++ b/js/src/data/action-types.js @@ -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; diff --git a/js/src/data/reducer.js b/js/src/data/reducer.js index a4c887025b..5f363d4b7e 100644 --- a/js/src/data/reducer.js +++ b/js/src/data/reducer.js @@ -69,6 +69,7 @@ const DEFAULT_STATE = { inviteLink: null, step: null, }, + budgetRecommendations: {}, }, }; @@ -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: diff --git a/js/src/data/resolvers.js b/js/src/data/resolvers.js index c29cb52bdd..39eb137906 100644 --- a/js/src/data/resolvers.js +++ b/js/src/data/resolvers.js @@ -15,7 +15,7 @@ import { } 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'; @@ -48,6 +48,10 @@ import { receiveTour, } from './actions'; +/** + * @typedef {import('.~/data/actions').CountryCode} CountryCode + */ + export function* getShippingRates() { yield fetchShippingRates(); } @@ -510,3 +514,50 @@ export function* getGoogleAdsAccountStatus() { getGoogleAdsAccountStatus.shouldInvalidate = ( action ) => { return action.type === TYPES.DISCONNECT_ACCOUNTS_GOOGLE_ADS; }; + +/** + * Fetch ad budget recommendations for the specified country codes. + * + * @param {Array} [countryCodes] An array of country codes for which to fetch budget recommendations. + */ +export function* getAdsBudgetRecommendations( countryCodes ) { + if ( ! countryCodes || ! countryCodes.length ) { + return; + } + + const countryCodesKey = getCountryCodesKey( countryCodes ); + const endpoint = `${ API_NAMESPACE }/ads/campaigns/budget-recommendation`; + const query = { country_codes: countryCodes }; + const path = addQueryArgs( endpoint, query ); + + try { + const { data } = yield fetchWithHeaders( { + path, + } ); + + const { currency, recommendations } = data; + + return { + 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; + } + + const bodyPromise = response?.json() || response?.text(); + const error = yield awaitPromise( bodyPromise ); + + handleApiError( + error, + __( + 'There was an error getting the budget recommendation.', + 'google-listings-and-ads' + ) + ); + } +} diff --git a/js/src/data/selectors.js b/js/src/data/selectors.js index 123b09f7d5..ea14cc3eb3 100644 --- a/js/src/data/selectors.js +++ b/js/src/data/selectors.js @@ -8,7 +8,12 @@ import createSelector from 'rememo'; * 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 @@ -406,3 +411,16 @@ export const getTour = ( state, tourId ) => { 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} [countryCodes] - An array of country code strings used to generate a unique key. + * @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 ); + return state.ads.budgetRecommendations[ key ] || null; +}; diff --git a/js/src/data/test/reducer.test.js b/js/src/data/test/reducer.test.js index b48277b36a..35416da5b3 100644 --- a/js/src/data/test/reducer.test.js +++ b/js/src/data/test/reducer.test.js @@ -72,6 +72,7 @@ describe( 'reducer', () => { inviteLink: null, step: null, }, + budgetRecommendations: {}, }, } ); @@ -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 */ diff --git a/js/src/data/utils.js b/js/src/data/utils.js index 2de2394f78..45b66cf8bc 100644 --- a/js/src/data/utils.js +++ b/js/src/data/utils.js @@ -9,6 +9,10 @@ import { getCurrentDates } from '@woocommerce/date'; */ import round from '.~/utils/round'; +/** + * @typedef { import(".~/data/actions").CountryCode } CountryCode + */ + export const freeFields = [ 'clicks', 'impressions' ]; export const paidFields = [ 'sales', 'conversions', 'spend', ...freeFields ]; /** @@ -190,6 +194,20 @@ export function mapReportFieldsToPerformance( ); } +/** + * 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} countryCodes - An array of country code strings. + * @return {string} A underscore-separated, lowercase string representing the sorted country codes. + */ +export function getCountryCodesKey( countryCodes = [] ) { + return [ ...countryCodes ].sort().join( '_' ).toLowerCase(); +} + /** * Report fields fetched from report API. * diff --git a/js/src/hooks/useFetchBudgetRecommendation.js b/js/src/hooks/useFetchBudgetRecommendation.js new file mode 100644 index 0000000000..12184a0e87 --- /dev/null +++ b/js/src/hooks/useFetchBudgetRecommendation.js @@ -0,0 +1,40 @@ +/** + * External dependencies + */ +import { useSelect } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { STORE_KEY } from '.~/data/constants'; + +/** + * @typedef { import(".~/data/actions").CountryCode } CountryCode + */ + +/** + * Fetch the highest budget recommendation for countries in a side effect. + * + * @param {Array} [countryCodes] An array of country codes. If empty, the fetch will not be triggered. + * @return {Object} Budget recommendation. + */ +const useFetchBudgetRecommendation = ( countryCodes ) => { + return useSelect( + ( select ) => { + const { getAdsBudgetRecommendations, hasFinishedResolution } = + select( STORE_KEY ); + + const data = getAdsBudgetRecommendations( countryCodes ); + return { + data, + hasFinishedResolution: hasFinishedResolution( + 'getAdsBudgetRecommendations', + [ countryCodes ] + ), + }; + }, + [ countryCodes ] + ); +}; + +export default useFetchBudgetRecommendation; From 0cafdb2792bad97d3a093f1528fbf857ebce6e0b Mon Sep 17 00:00:00 2001 From: asvinb Date: Wed, 2 Oct 2024 21:07:27 +0400 Subject: [PATCH 22/40] Make use of new hook and add loading indicator. --- .../paid-ads/ads-campaign/ads-campaign.js | 1 - .../paid-ads-setup-sections/index.js | 1 + .../paid-ads-setup-form.js} | 31 +++------ .../paid-ads-setup-sections.js | 66 +++++++++++++++++++ .../budget-recommendation/index.js | 33 +--------- js/src/utils/getHighestBudget.js | 21 ++---- 6 files changed, 84 insertions(+), 69 deletions(-) create mode 100644 js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/index.js rename js/src/components/paid-ads/ads-campaign/{paid-ads-setup-sections.js => paid-ads-setup-sections/paid-ads-setup-form.js} (83%) create mode 100644 js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-sections.js diff --git a/js/src/components/paid-ads/ads-campaign/ads-campaign.js b/js/src/components/paid-ads/ads-campaign/ads-campaign.js index d3b1367f2f..0ff2497eac 100644 --- a/js/src/components/paid-ads/ads-campaign/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign/ads-campaign.js @@ -153,7 +153,6 @@ export default function AdsCampaign( { campaign={ campaign } countryCodes={ countryCodes } loadCampaignFromClientSession={ isOnboardingFlow } - recommendedDailyAmount={ dailyBudget } showCampaignPreviewCard={ trackingContext === 'setup-ads' || trackingContext === 'create-ads' diff --git a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/index.js b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/index.js new file mode 100644 index 0000000000..478d76b757 --- /dev/null +++ b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/index.js @@ -0,0 +1 @@ +export { default } from './paid-ads-setup-sections'; diff --git a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections.js b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js similarity index 83% rename from js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections.js rename to js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js index 68e670af55..35d29a612a 100644 --- a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections.js +++ b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js @@ -10,10 +10,8 @@ import { Form } from '@woocommerce/components'; import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; 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 clientSession from '../clientSession'; import CampaignPreviewCard from '.~/components/paid-ads/campaign-preview/campaign-preview-card'; import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; @@ -27,11 +25,7 @@ import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; */ /** - * - * @typedef {Object} PaidAdsData - * @property {number|undefined} amount Daily average cost of the paid ads campaign. - * @property {boolean} isValid Whether the campaign data are valid values. - * @property {boolean} isReady Whether the campaign data and the billing setting are ready for completing the paid ads setup. + * @typedef {import('./paid-ads-setup-sections').PaidAdsData} PaidAdsData */ const defaultPaidAds = { @@ -64,15 +58,15 @@ function resolveInitialPaidAds( paidAds ) { * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. * @param {boolean} [props.showCampaignPreviewCard=false] Whether to show the campaign preview card. * @param {boolean} [props.loadCampaignFromClientSession=false] Whether to load the campaign data from the client session. - * @param {number} props.recommendedDailyAmount The recommended daily budget. + * @param {number} props.recommendedBudget The recommended budget. */ -export default function PaidAdsSetupSections( { +export default function PaidAdsSetupForm( { onStatesReceived, countryCodes, campaign, loadCampaignFromClientSession, showCampaignPreviewCard = false, - recommendedDailyAmount, + recommendedBudget, } ) { const isCreation = ! campaign; const { billingStatus } = useGoogleAdsAccountBillingStatus(); @@ -89,7 +83,7 @@ export default function PaidAdsSetupSections( { if ( ! campaign ) { startingPaidAds = { ...startingPaidAds, - amount: recommendedDailyAmount, + amount: recommendedBudget, }; } @@ -98,8 +92,7 @@ export default function PaidAdsSetupSections( { startingPaidAds = { ...startingPaidAds, amount: - clientSession.getCampaign()?.amount || - recommendedDailyAmount, + clientSession.getCampaign()?.amount || recommendedBudget, }; } @@ -107,7 +100,7 @@ export default function PaidAdsSetupSections( { } ); const isBillingCompleted = - billingStatus?.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; + billingStatus.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; /* If a merchant has not yet finished the billing setup, the billing status will be @@ -132,14 +125,6 @@ export default function PaidAdsSetupSections( { clientSession.setCampaign( nextPaidAds ); }, [ paidAds, isBillingCompleted ] ); - if ( ! billingStatus ) { - return ( -
- -
- ); - } - const initialValues = { amount: isCreation ? paidAds.amount : campaign.amount, }; diff --git a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-sections.js b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-sections.js new file mode 100644 index 0000000000..294dc5ed55 --- /dev/null +++ b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-sections.js @@ -0,0 +1,66 @@ +/** + * Internal dependencies + */ +import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; +import SpinnerCard from '.~/components/spinner-card'; +import Section from '.~/wcdl/section'; +import PaidAdsSetupForm from './paid-ads-setup-form'; +import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; +import getHighestBudget from '.~/utils/getHighestBudget'; + +/** + * + * @typedef {import('.~/data/actions').Campaign} Campaign + */ + +/** + * @typedef {import('.~/data/actions').CountryCode} CountryCode + */ + +/** + * + * @typedef {Object} PaidAdsData + * @property {number|undefined} amount Daily average cost of the paid ads campaign. + * @property {boolean} isValid Whether the campaign data are valid values. + * @property {boolean} isReady Whether the campaign data and the billing setting are ready for completing the paid ads setup. + */ + +/** + * Renders sections of Google Ads account, budget and billing for setting up the paid ads. + * + * @param {Object} props React props. + * @param {(onStatesReceived: PaidAdsData)=>void} props.onStatesReceived Callback to receive the data for setting up paid ads when initial and also when the budget and billing are updated. + * @param {Array|undefined} props.countryCodes Country codes for the campaign. + * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. + * @param {boolean} [props.showCampaignPreviewCard=false] Whether to show the campaign preview card. + * @param {boolean} [props.loadCampaignFromClientSession=false] Whether to load the campaign data from the client session. + */ +export default function PaidAdsSetupSections( props ) { + const { billingStatus } = useGoogleAdsAccountBillingStatus(); + const { hasFinishedResolution, data } = useFetchBudgetRecommendation( + props.countryCodes + ); + + let recommendedBudget = 0; + if ( data ) { + const { recommendations } = data; + const { daily_budget: dailyBudget } = + getHighestBudget( recommendations ); + recommendedBudget = dailyBudget; + } + + if ( ! billingStatus || ! hasFinishedResolution ) { + return ( +
+ +
+ ); + } + + return ( + + ); +} diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index 1a3dfc99ac..14613bb40b 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -10,25 +10,10 @@ import GridiconNoticeOutline from 'gridicons/dist/notice-outline'; * Internal dependencies */ import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap'; -import useFetchBudgetRecommendationEffect from '.~/hooks/useFetchBudgetRecommendationEffect'; +import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; +import getHighestBudget from '.~/utils/getHighestBudget'; 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: , em: , br:
}; const template = isMultiple @@ -51,7 +36,7 @@ function toRecommendationRange( isMultiple, ...values ) { const BudgetRecommendation = ( props ) => { const { countryCodes, dailyAverageCost = Infinity } = props; - const { data } = useFetchBudgetRecommendationEffect( countryCodes ); + const { data } = useFetchBudgetRecommendation( countryCodes ); const map = useCountryKeyNameMap(); if ( ! data ) { @@ -70,18 +55,6 @@ const BudgetRecommendation = ( props ) => { countryName ); - // const { currency, recommendations } = data; - // const { daily_budget: dailyBudget, country } = - // getHighestBudget( recommendations ); - - // const countryName = map[ country ]; - // const recommendationRange = toRecommendationRange( - // multipleRecommendations, - // dailyBudget, - // currency, - // countryName - // ); - const showLowerBudgetNotice = dailyAverageCost < dailyBudget; return ( diff --git a/js/src/utils/getHighestBudget.js b/js/src/utils/getHighestBudget.js index b31b46742e..e2fef429e9 100644 --- a/js/src/utils/getHighestBudget.js +++ b/js/src/utils/getHighestBudget.js @@ -7,22 +7,13 @@ */ export default function getHighestBudget( recommendations ) { if ( ! recommendations ) { - return { - daily_budget: 0, - country: '', - }; + return null; } - return recommendations.reduce( - ( defender, challenger ) => { - if ( challenger.daily_budget > defender.daily_budget ) { - return challenger; - } - return defender; - }, - { - daily_budget: 0, - country: '', + return recommendations.reduce( ( defender, challenger ) => { + if ( challenger.daily_budget > defender.daily_budget ) { + return challenger; } - ); + return defender; + } ); } From 79ccefa3f35e3012c63f8c5ef3a1de34a6f298a1 Mon Sep 17 00:00:00 2001 From: asvinb Date: Wed, 2 Oct 2024 21:17:15 +0400 Subject: [PATCH 23/40] Remove unused hook. --- .../paid-ads-setup-sections.js | 2 +- js/src/hooks/useBudgetRecommendationData.js | 46 ------------------- 2 files changed, 1 insertion(+), 47 deletions(-) delete mode 100644 js/src/hooks/useBudgetRecommendationData.js diff --git a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-sections.js b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-sections.js index 294dc5ed55..16cfccddee 100644 --- a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-sections.js +++ b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-sections.js @@ -41,7 +41,7 @@ export default function PaidAdsSetupSections( props ) { props.countryCodes ); - let recommendedBudget = 0; + let recommendedBudget; if ( data ) { const { recommendations } = data; const { daily_budget: dailyBudget } = diff --git a/js/src/hooks/useBudgetRecommendationData.js b/js/src/hooks/useBudgetRecommendationData.js deleted file mode 100644 index 542188b3f1..0000000000 --- a/js/src/hooks/useBudgetRecommendationData.js +++ /dev/null @@ -1,46 +0,0 @@ -/** - * 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 ); - - const [ country, setCountry ] = useState( '' ); - const [ dailyBudget, setDailyBudget ] = useState( 0 ); - const [ multipleRecommendations, setMultipleRecommendations ] = - useState( false ); - const [ recommendations, setRecommendations ] = useState( [] ); - - useEffect( () => { - if ( ! loading ) { - const { - country: budgetCountries = '', - daily_budget: recommendedDailyBudget, - } = getHighestBudget( data?.recommendations ); - - setCountry( budgetCountries ); - setDailyBudget( recommendedDailyBudget ); - setMultipleRecommendations( data?.recommendations.length > 1 ); - setRecommendations( data?.recommendations ); - } - }, [ data?.recommendations, loading ] ); - - return { - country, - dailyBudget, - multipleRecommendations, - recommendations, - loading, - data, - }; -}; - -export default useBudgetRecommendationData; From fbde5f4d976eaff3646579235d24e7698a0a63b1 Mon Sep 17 00:00:00 2001 From: asvinb Date: Wed, 2 Oct 2024 21:18:53 +0400 Subject: [PATCH 24/40] Move clientSession. --- .../ads-campaign/{ => paid-ads-setup-sections}/clientSession.js | 0 .../ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename js/src/components/paid-ads/ads-campaign/{ => paid-ads-setup-sections}/clientSession.js (100%) diff --git a/js/src/components/paid-ads/ads-campaign/clientSession.js b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/clientSession.js similarity index 100% rename from js/src/components/paid-ads/ads-campaign/clientSession.js rename to js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/clientSession.js diff --git a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js index 35d29a612a..dd37fc5636 100644 --- a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js +++ b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js @@ -11,7 +11,7 @@ import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillin import BudgetSection from '.~/components/paid-ads/budget-section'; import BillingCard from '.~/components/paid-ads/billing-card'; import validateCampaign from '.~/components/paid-ads/validateCampaign'; -import clientSession from '../clientSession'; +import clientSession from './clientSession'; import CampaignPreviewCard from '.~/components/paid-ads/campaign-preview/campaign-preview-card'; import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; From 3ea1c12c80f37a26e555b42907edc43bcbdb97c8 Mon Sep 17 00:00:00 2001 From: asvinb Date: Wed, 2 Oct 2024 21:20:45 +0400 Subject: [PATCH 25/40] Restore complete campaign data from client session. --- .../paid-ads-setup-sections/paid-ads-setup-form.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js index dd37fc5636..6f465d0fc2 100644 --- a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js +++ b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js @@ -91,6 +91,7 @@ export default function PaidAdsSetupForm( { if ( loadCampaignFromClientSession ) { startingPaidAds = { ...startingPaidAds, + ...clientSession.getCampaign(), amount: clientSession.getCampaign()?.amount || recommendedBudget, }; @@ -100,7 +101,7 @@ export default function PaidAdsSetupForm( { } ); const isBillingCompleted = - billingStatus.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; + billingStatus?.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; /* If a merchant has not yet finished the billing setup, the billing status will be From 7aaa968e918120a26f276798a16d43b6fcfaebf5 Mon Sep 17 00:00:00 2001 From: asvinb Date: Mon, 7 Oct 2024 16:00:02 +0400 Subject: [PATCH 26/40] Set minimum amount to always be the recommended budget. --- .../paid-ads-setup-sections/paid-ads-setup-form.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js index 6f465d0fc2..386da41b30 100644 --- a/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js +++ b/js/src/components/paid-ads/ads-campaign/paid-ads-setup-sections/paid-ads-setup-form.js @@ -89,11 +89,15 @@ export default function PaidAdsSetupForm( { // Resolve the starting paid ads data with the campaign data stored in the client session if any. if ( loadCampaignFromClientSession ) { + const initialAmount = Math.max( + clientSession.getCampaign()?.amount || 0, + recommendedBudget + ); + startingPaidAds = { ...startingPaidAds, ...clientSession.getCampaign(), - amount: - clientSession.getCampaign()?.amount || recommendedBudget, + amount: initialAmount, }; } From 8bf78f17ccdde3f875e1f034692e6c5e31f5efad Mon Sep 17 00:00:00 2001 From: asvinb Date: Mon, 14 Oct 2024 20:21:33 +0400 Subject: [PATCH 27/40] Simplify setting up minimum budget recommendation. --- js/src/hooks/useFetchBudgetRecommendation.js | 2 +- .../pages/create-paid-ads-campaign/index.js | 7 ++- js/src/setup-ads/setup-ads-form.js | 15 +++++-- .../setup-mc/setup-stepper/setup-paid-ads.js | 44 +++++++------------ 4 files changed, 35 insertions(+), 33 deletions(-) diff --git a/js/src/hooks/useFetchBudgetRecommendation.js b/js/src/hooks/useFetchBudgetRecommendation.js index d33a01fc3e..bea96033ce 100644 --- a/js/src/hooks/useFetchBudgetRecommendation.js +++ b/js/src/hooks/useFetchBudgetRecommendation.js @@ -26,7 +26,7 @@ const useFetchBudgetRecommendation = ( countryCodes ) => { select( STORE_KEY ); const data = getAdsBudgetRecommendations( countryCodes ); - let highestDailyBudget; + let highestDailyBudget = 0; let highestDailyBudgetCountryCode; if ( data ) { diff --git a/js/src/pages/create-paid-ads-campaign/index.js b/js/src/pages/create-paid-ads-campaign/index.js index b32e544e9a..d03f42c8ea 100644 --- a/js/src/pages/create-paid-ads-campaign/index.js +++ b/js/src/pages/create-paid-ads-campaign/index.js @@ -33,6 +33,7 @@ import { recordStepContinueEvent, } from '.~/utils/tracks'; import ContinueButton from '.~/components/paid-ads/continue-button'; +import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; const eventName = 'gla_paid_campaign_step'; const eventContext = 'create-ads'; @@ -52,6 +53,8 @@ const CreatePaidAdsCampaign = () => { const { createAdsCampaign, updateCampaignAssetGroup } = useAppDispatch(); const { createNotice } = useDispatchCoreNotices(); const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); + const { highestDailyBudget, hasFinishedResolution } = + useFetchBudgetRecommendation( countryCodes ); const handleStepperClick = ( nextStep ) => { recordStepperChangeEvent( @@ -114,7 +117,7 @@ const CreatePaidAdsCampaign = () => { getHistory().push( getDashboardUrl( { campaign: 'saved' } ) ); }; - if ( ! countryCodes ) { + if ( ! countryCodes || ! hasFinishedResolution ) { return null; } @@ -130,7 +133,7 @@ const CreatePaidAdsCampaign = () => { /> diff --git a/js/src/setup-ads/setup-ads-form.js b/js/src/setup-ads/setup-ads-form.js index 5db4e31d27..36055d3b2d 100644 --- a/js/src/setup-ads/setup-ads-form.js +++ b/js/src/setup-ads/setup-ads-form.js @@ -17,6 +17,8 @@ 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 useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; +import AppSpinner from '.~/components/app-spinner'; /** * @fires gla_launch_paid_campaign_button_click on submit @@ -27,9 +29,11 @@ const SetupAdsForm = () => { const [ handleSetupComplete, isSubmitting ] = useAdsSetupCompleteCallback(); const adminUrl = useAdminUrl(); const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); + const { highestDailyBudget, hasFinishedResolution } = + useFetchBudgetRecommendation( countryCodes ); const initialValues = { - amount: 0, + amount: highestDailyBudget, }; useEffect( () => { @@ -74,8 +78,13 @@ const SetupAdsForm = () => { setFormChanged( ! isEqual( ...args ) ); }; - if ( ! countryCodes ) { - return null; + if ( ! countryCodes || ! hasFinishedResolution ) { + return ( + <> + + + + ); } return ( diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads.js b/js/src/setup-mc/setup-stepper/setup-paid-ads.js index e10875dc46..a4a5c634ec 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads.js @@ -21,9 +21,10 @@ import { getProductFeedUrl } from '.~/utils/urls'; import { API_NAMESPACE } from '.~/data/constants'; import { GUIDE_NAMES, GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; import { ACTION_COMPLETE, ACTION_SKIP } from './constants'; -import validateCampaign from '.~/components/paid-ads/validateCampaign'; import SkipButton from './skip-button'; import clientSession from './clientSession'; +import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; +import AppSpinner from '.~/components/app-spinner'; /** * Clicking on the "Complete setup" button to complete the onboarding flow with paid ads. @@ -40,29 +41,6 @@ import clientSession from './clientSession'; * @property {boolean} isValid Whether the campaign data are valid values. */ -const defaultPaidAds = { - amount: 0, - isValid: false, -}; - -/** - * Resolve the initial paid ads data from the given paid ads data. - * Parts of the resolved data are used in the `initialCampaign` prop of `CampaignAssetsForm` component. - * - * @return {PaidAdsData} The resolved paid ads data. - */ -function resolveInitialPaidAds() { - const startingPaidAds = { - ...defaultPaidAds, - ...clientSession.getCampaign(), - }; - const nextPaidAds = { ...startingPaidAds }; - nextPaidAds.isValid = ! Object.keys( validateCampaign( nextPaidAds ) ) - .length; - - return nextPaidAds; -} - /** * Renders the onboarding step for setting up the paid ads (Google Ads account and paid campaign) * or skipping it, and then completing the onboarding flow. @@ -73,9 +51,10 @@ export default function SetupPaidAds() { const [ completing, setCompleting ] = useState( null ); const { createNotice } = useDispatchCoreNotices(); const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); + const { highestDailyBudget, hasFinishedResolution } = + useFetchBudgetRecommendation( countryCodes ); const [ handleSetupComplete ] = useAdsSetupCompleteCallback(); const { billingStatus } = useGoogleAdsAccountBillingStatus(); - const paidAds = resolveInitialPaidAds(); const isBillingCompleted = billingStatus?.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; @@ -156,11 +135,22 @@ export default function SetupPaidAds() { ); }; + const paidAds = { + amount: highestDailyBudget, + ...clientSession.getCampaign(), + }; + + if ( ! hasFinishedResolution || ! countryCodes ) { + return ; + } + return ( { - clientSession.setCampaign( { ...values, isValid } ); + onChange={ ( _, values ) => { + if ( values.amount >= highestDailyBudget ) { + clientSession.setCampaign( { ...values } ); + } } } > Date: Mon, 14 Oct 2024 20:45:40 +0400 Subject: [PATCH 28/40] Update tests. --- tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js b/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js index 987a7111ae..48ec190682 100644 --- a/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js +++ b/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js @@ -294,10 +294,6 @@ test.describe( 'Set up Ads account', () => { page.getByRole( 'heading', { name: 'Set your budget' } ) ).toBeVisible(); - await expect( - setupBudgetPage.getLaunchPaidCampaignButton() - ).toBeDisabled(); - await expect( page.getByRole( 'link', { name: 'See what your ads will look like.', From 48aeec39d95edc688bece8ad072ba7d37a4aa5fc Mon Sep 17 00:00:00 2001 From: asvinb Date: Thu, 17 Oct 2024 20:19:29 +0400 Subject: [PATCH 29/40] Address CR feedback. --- .../paid-ads/budget-section/index.js | 11 ++----- js/src/data/selectors.js | 2 +- js/src/data/utils.js | 2 +- .../useFetchBudgetRecommendationEffect.js | 29 ------------------- .../setup-mc/setup-stepper/setup-paid-ads.js | 2 +- 5 files changed, 5 insertions(+), 41 deletions(-) delete mode 100644 js/src/hooks/useFetchBudgetRecommendationEffect.js diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index f905b7cf9f..121c74976a 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -2,7 +2,6 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import { useRef } from '@wordpress/element'; /** * Internal dependencies @@ -29,7 +28,7 @@ const nonInteractableProps = { * * @param {Object} props React props. * @param {Object} props.formProps Form props forwarded from `Form` component. - * @param {Array} props.countryCodes Country codes to fetch budget recommendations for. + * @param {Array|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. */ @@ -39,19 +38,13 @@ const BudgetSection = ( { disabled = false, children, } ) => { - const { getInputProps, setValue, values } = formProps; + const { getInputProps, values } = formProps; const { amount } = values; const { googleAdsAccount } = useGoogleAdsAccount(); 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; - return (
{ * If no recommendations are found, it returns `null`. * * @param {Object} state The state - * @param {Array} [countryCodes] - An array of country code strings used to generate a unique key. + * @param {Array} [countryCodes] - An array of country code strings to retrieve the budget recommendations for. * @return {Object|null} The recommendations. It will be `null` if not yet fetched or fetched but doesn't exist. */ export const getAdsBudgetRecommendations = ( state, countryCodes = [] ) => { diff --git a/js/src/data/utils.js b/js/src/data/utils.js index 45b66cf8bc..a42b7bd171 100644 --- a/js/src/data/utils.js +++ b/js/src/data/utils.js @@ -201,7 +201,7 @@ export function mapReportFieldsToPerformance( * joins them into a single string with underscore (`_`), and converts * the result to lowercase. * - * @param {Array} countryCodes - An array of country code strings. + * @param {Array} [countryCodes] - An array of country code strings. * @return {string} A underscore-separated, lowercase string representing the sorted country codes. */ export function getCountryCodesKey( countryCodes = [] ) { diff --git a/js/src/hooks/useFetchBudgetRecommendationEffect.js b/js/src/hooks/useFetchBudgetRecommendationEffect.js deleted file mode 100644 index 6d63fad522..0000000000 --- a/js/src/hooks/useFetchBudgetRecommendationEffect.js +++ /dev/null @@ -1,29 +0,0 @@ -/** - * External dependencies - */ -import { addQueryArgs } from '@wordpress/url'; - -/** - * Internal dependencies - */ -import { API_NAMESPACE } from '.~/data/constants'; -import useApiFetchEffect from '.~/hooks/useApiFetchEffect'; - -/** - * @typedef { import(".~/data/actions").CountryCode } CountryCode - */ - -/** - * Fetch the budget recommendation for a country in a side effect. - * - * @param {Array} countryCodes Country code array. - * @return {Object} Budget recommendation. - */ -const useFetchBudgetRecommendationEffect = ( countryCodes ) => { - const url = `${ API_NAMESPACE }/ads/campaigns/budget-recommendation`; - const query = { country_codes: countryCodes }; - const path = addQueryArgs( url, query ); - return useApiFetchEffect( { path } ); -}; - -export default useFetchBudgetRecommendationEffect; diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads.js b/js/src/setup-mc/setup-stepper/setup-paid-ads.js index 0fb2f853e2..2bb83b2e9f 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads.js @@ -142,7 +142,7 @@ export default function SetupPaidAds() { initialCampaign={ paidAds } onChange={ ( _, values ) => { if ( values.amount >= highestDailyBudget ) { - clientSession.setCampaign( { ...values } ); + clientSession.setCampaign( values ); } } } > From fb79ef6e2cdf1ac693af9454619dffe374fd1da3 Mon Sep 17 00:00:00 2001 From: asvinb Date: Fri, 18 Oct 2024 16:45:22 +0400 Subject: [PATCH 30/40] Add invalidation condition. --- js/src/data/resolvers.js | 4 ++++ js/src/hooks/useFetchBudgetRecommendation.js | 21 +++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/js/src/data/resolvers.js b/js/src/data/resolvers.js index 39eb137906..ee22689b64 100644 --- a/js/src/data/resolvers.js +++ b/js/src/data/resolvers.js @@ -561,3 +561,7 @@ export function* getAdsBudgetRecommendations( countryCodes ) { ); } } + +getAdsBudgetRecommendations.shouldInvalidate = ( action ) => { + return action.type === TYPES.RECEIVE_ACCOUNTS_GOOGLE_ADS; +}; diff --git a/js/src/hooks/useFetchBudgetRecommendation.js b/js/src/hooks/useFetchBudgetRecommendation.js index bea96033ce..45caf54318 100644 --- a/js/src/hooks/useFetchBudgetRecommendation.js +++ b/js/src/hooks/useFetchBudgetRecommendation.js @@ -8,6 +8,7 @@ import { useSelect } from '@wordpress/data'; */ import { STORE_KEY } from '.~/data/constants'; import getHighestBudget from '.~/utils/getHighestBudget'; +import useGoogleAdsAccount from './useGoogleAdsAccount'; /** * @typedef { import(".~/data/actions").CountryCode } CountryCode @@ -20,8 +21,22 @@ import getHighestBudget from '.~/utils/getHighestBudget'; * @return {Object} Budget recommendation. */ const useFetchBudgetRecommendation = ( countryCodes ) => { + const { + hasGoogleAdsConnection, + hasFinishedResolution: hasFinishedResolutionAdsAccount, + } = useGoogleAdsAccount(); + return useSelect( ( select ) => { + if ( ! hasGoogleAdsConnection ) { + return { + data: undefined, + highestDailyBudget: 0, + highestDailyBudgetCountryCode: undefined, + hasFinishedResolution: hasFinishedResolutionAdsAccount, + }; + } + const { getAdsBudgetRecommendations, hasFinishedResolution } = select( STORE_KEY ); @@ -47,7 +62,11 @@ const useFetchBudgetRecommendation = ( countryCodes ) => { ), }; }, - [ countryCodes ] + [ + countryCodes, + hasGoogleAdsConnection, + hasFinishedResolutionAdsAccount, + ] ); }; From ede72e38e4aa971ba2b8cb0b7b8c5055fbc391cc Mon Sep 17 00:00:00 2001 From: asvinb Date: Fri, 18 Oct 2024 18:27:53 +0400 Subject: [PATCH 31/40] Set value in BudgetSection. --- .../paid-ads/ads-campaign/ads-campaign.js | 1 + .../paid-ads/budget-section}/clientSession.js | 0 .../paid-ads/budget-section/index.js | 93 ++++++++++++++----- .../pages/create-paid-ads-campaign/index.js | 9 +- js/src/setup-ads/setup-ads-form.js | 15 +-- .../setup-mc/setup-stepper/setup-paid-ads.js | 20 +--- 6 files changed, 73 insertions(+), 65 deletions(-) rename js/src/{setup-mc/setup-stepper => components/paid-ads/budget-section}/clientSession.js (100%) diff --git a/js/src/components/paid-ads/ads-campaign/ads-campaign.js b/js/src/components/paid-ads/ads-campaign/ads-campaign.js index 34324fd40c..98e1d126be 100644 --- a/js/src/components/paid-ads/ads-campaign/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign/ads-campaign.js @@ -99,6 +99,7 @@ export default function AdsCampaign( { ? campaign.displayCountries : countryCodes } + context={ context } > { showBillingCard && } diff --git a/js/src/setup-mc/setup-stepper/clientSession.js b/js/src/components/paid-ads/budget-section/clientSession.js similarity index 100% rename from js/src/setup-mc/setup-stepper/clientSession.js rename to js/src/components/paid-ads/budget-section/clientSession.js diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index 121c74976a..11b5f0ddfe 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -2,6 +2,7 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; +import { useRef, useEffect } from '@wordpress/element'; /** * Internal dependencies @@ -12,6 +13,9 @@ import './index.scss'; 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 @@ -31,20 +35,52 @@ const nonInteractableProps = { * @param {Array|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, values } = formProps; + const initialAmountRef = useRef( null ); + const { getInputProps, values, setValue } = formProps; const { amount } = values; const { googleAdsAccount } = useGoogleAdsAccount(); + const { highestDailyBudget, hasFinishedResolution } = + useFetchBudgetRecommendation( countryCodes ); 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; + useEffect( () => { + if ( + context !== 'setup-mc' || + ! hasFinishedResolution || + ! values.amount + ) { + return; + } + + if ( values.amount >= highestDailyBudget ) { + clientSession.setCampaign( values ); + } + }, [ values, highestDailyBudget, context, hasFinishedResolution ] ); + + if ( ! initialAmountRef.current && ! amount && hasFinishedResolution ) { + let clientSessionAmount = 0; + if ( context === 'setup-mc' ) { + ( { amount: clientSessionAmount } = clientSession.getCampaign() ); + } + + initialAmountRef.current = true; + setValue( + 'amount', + Math.max( clientSessionAmount, highestDailyBudget ) + ); + } + return (
-
- - +
+ + +
+ { countryCodes?.length > 0 && ( + ) } - suffix={ currency } - value={ monthlyMaxEstimated } - /> -
- { countryCodes?.length > 0 && ( - + + ) : ( + ) }
diff --git a/js/src/pages/create-paid-ads-campaign/index.js b/js/src/pages/create-paid-ads-campaign/index.js index 3a2596af86..0aa604b2da 100644 --- a/js/src/pages/create-paid-ads-campaign/index.js +++ b/js/src/pages/create-paid-ads-campaign/index.js @@ -33,7 +33,6 @@ import { recordStepperChangeEvent, recordStepContinueEvent, } from '.~/utils/tracks'; -import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; const eventName = 'gla_paid_campaign_step'; const eventContext = 'create-ads'; @@ -53,8 +52,6 @@ const CreatePaidAdsCampaign = () => { const { createAdsCampaign, updateCampaignAssetGroup } = useAppDispatch(); const { createNotice } = useDispatchCoreNotices(); const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); - const { highestDailyBudget, hasFinishedResolution } = - useFetchBudgetRecommendation( countryCodes ); const handleStepperClick = ( nextStep ) => { recordStepperChangeEvent( @@ -117,10 +114,6 @@ const CreatePaidAdsCampaign = () => { getHistory().push( getDashboardUrl( { campaign: 'saved' } ) ); }; - if ( ! countryCodes || ! hasFinishedResolution ) { - return null; - } - return ( <> { /> diff --git a/js/src/setup-ads/setup-ads-form.js b/js/src/setup-ads/setup-ads-form.js index 6f82cd01bc..062deec7b5 100644 --- a/js/src/setup-ads/setup-ads-form.js +++ b/js/src/setup-ads/setup-ads-form.js @@ -17,8 +17,6 @@ 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 useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; -import AppSpinner from '.~/components/app-spinner'; /** * @fires gla_launch_paid_campaign_button_click on submit @@ -29,11 +27,9 @@ const SetupAdsForm = () => { const [ handleSetupComplete, isSubmitting ] = useAdsSetupCompleteCallback(); const adminUrl = useAdminUrl(); const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); - const { highestDailyBudget, hasFinishedResolution } = - useFetchBudgetRecommendation( countryCodes ); const initialValues = { - amount: highestDailyBudget, + amount: 0, }; useEffect( () => { @@ -74,15 +70,6 @@ const SetupAdsForm = () => { setFormChanged( ! isEqual( initialValues, values ) ); }; - if ( ! countryCodes || ! hasFinishedResolution ) { - return ( - <> - - - - ); - } - return ( ; - } - return ( { - if ( values.amount >= highestDailyBudget ) { - clientSession.setCampaign( values ); - } + initialCampaign={ { + amount: 0, } } > Date: Fri, 18 Oct 2024 18:29:31 +0400 Subject: [PATCH 32/40] Add comment. --- js/src/components/paid-ads/budget-section/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index 11b5f0ddfe..501c71f6ba 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -55,6 +55,7 @@ const BudgetSection = ( { const currency = googleAdsAccount?.currency; useEffect( () => { + // Load the amount from client session during the onboarding flow only. if ( context !== 'setup-mc' || ! hasFinishedResolution || From 121fc290591f8bca20a7d140ecb91f4e58acae66 Mon Sep 17 00:00:00 2001 From: asvinb Date: Fri, 18 Oct 2024 18:35:34 +0400 Subject: [PATCH 33/40] Pass props. --- .../budget-recommendation/index.js | 15 +++++++++------ .../paid-ads/budget-section/index.js | 18 +++++++++++++++--- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index 909005094a..153697ec71 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -10,7 +10,6 @@ import GridiconNoticeOutline from 'gridicons/dist/notice-outline'; * Internal dependencies */ import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap'; -import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; import './index.scss'; function toRecommendationRange( isMultiple, ...values ) { @@ -34,16 +33,20 @@ function toRecommendationRange( isMultiple, ...values ) { } const BudgetRecommendation = ( props ) => { - const { countryCodes, dailyAverageCost = Infinity } = props; - const { data, highestDailyBudgetCountryCode, highestDailyBudget } = - useFetchBudgetRecommendation( countryCodes ); + const { + dailyAverageCost = Infinity, + highestDailyBudget, + highestDailyBudgetCountryCode, + budgetRecommendationData, + } = props; + const map = useCountryKeyNameMap(); - if ( ! data ) { + if ( ! budgetRecommendationData ) { return null; } - const { currency, recommendations } = data; + const { currency, recommendations } = budgetRecommendationData; const countryName = map[ highestDailyBudgetCountryCode ]; const recommendationRange = toRecommendationRange( recommendations.length > 1, diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index 501c71f6ba..3ce7daae27 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -48,8 +48,12 @@ const BudgetSection = ( { const { getInputProps, values, setValue } = formProps; const { amount } = values; const { googleAdsAccount } = useGoogleAdsAccount(); - const { highestDailyBudget, hasFinishedResolution } = - useFetchBudgetRecommendation( countryCodes ); + const { + data: budgetRecommendationData, + highestDailyBudget, + highestDailyBudgetCountryCode, + hasFinishedResolution, + } = useFetchBudgetRecommendation( countryCodes ); 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; @@ -123,8 +127,16 @@ const BudgetSection = ( {
{ countryCodes?.length > 0 && ( ) } From f194da1897f56a87c555ad9f8d60f01c64a0985e Mon Sep 17 00:00:00 2001 From: asvinb Date: Fri, 18 Oct 2024 18:54:31 +0400 Subject: [PATCH 34/40] Add extra condition. --- js/src/data/resolvers.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/js/src/data/resolvers.js b/js/src/data/resolvers.js index ee22689b64..ca1f9eab06 100644 --- a/js/src/data/resolvers.js +++ b/js/src/data/resolvers.js @@ -563,5 +563,8 @@ export function* getAdsBudgetRecommendations( countryCodes ) { } getAdsBudgetRecommendations.shouldInvalidate = ( action ) => { - return action.type === TYPES.RECEIVE_ACCOUNTS_GOOGLE_ADS; + return ( + action.type === TYPES.RECEIVE_ACCOUNTS_GOOGLE_ADS || + action.type === TYPES.DISCONNECT_ACCOUNTS_GOOGLE_ADS + ); }; From 2ce27c1c875498a101c641399f7c28a1abd6096f Mon Sep 17 00:00:00 2001 From: asvinb Date: Fri, 18 Oct 2024 19:14:28 +0400 Subject: [PATCH 35/40] Revert changes. --- js/src/components/paid-ads/budget-section/index.js | 4 ++-- js/src/pages/create-paid-ads-campaign/index.js | 4 ++++ js/src/setup-ads/setup-ads-form.js | 4 ++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index 3ce7daae27..85c27085ef 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -2,7 +2,7 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import { useRef, useEffect } from '@wordpress/element'; +import { useEffect, useRef } from '@wordpress/element'; /** * Internal dependencies @@ -44,7 +44,6 @@ const BudgetSection = ( { children, context, } ) => { - const initialAmountRef = useRef( null ); const { getInputProps, values, setValue } = formProps; const { amount } = values; const { googleAdsAccount } = useGoogleAdsAccount(); @@ -54,6 +53,7 @@ const BudgetSection = ( { highestDailyBudgetCountryCode, hasFinishedResolution, } = useFetchBudgetRecommendation( countryCodes ); + const initialAmountRef = useRef( null ); 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; diff --git a/js/src/pages/create-paid-ads-campaign/index.js b/js/src/pages/create-paid-ads-campaign/index.js index 0aa604b2da..ae16722d76 100644 --- a/js/src/pages/create-paid-ads-campaign/index.js +++ b/js/src/pages/create-paid-ads-campaign/index.js @@ -114,6 +114,10 @@ const CreatePaidAdsCampaign = () => { getHistory().push( getDashboardUrl( { campaign: 'saved' } ) ); }; + if ( ! countryCodes ) { + return null; + } + return ( <> { setFormChanged( ! isEqual( initialValues, values ) ); }; + if ( ! countryCodes ) { + return null; + } + return ( Date: Fri, 18 Oct 2024 19:23:17 +0400 Subject: [PATCH 36/40] Remove unused import. --- js/src/setup-mc/setup-stepper/setup-paid-ads.js | 1 - 1 file changed, 1 deletion(-) diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads.js b/js/src/setup-mc/setup-stepper/setup-paid-ads.js index 8a8ca212b7..39ed7f3008 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads.js @@ -22,7 +22,6 @@ import { API_NAMESPACE } from '.~/data/constants'; import { GUIDE_NAMES, GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; import { ACTION_COMPLETE, ACTION_SKIP } from './constants'; import SkipButton from './skip-button'; -import AppSpinner from '.~/components/app-spinner'; /** * Clicking on the "Complete setup" button to complete the onboarding flow with paid ads. From 8c074a456e271cf55dc64b63df453a78394f0125 Mon Sep 17 00:00:00 2001 From: asvinb Date: Tue, 22 Oct 2024 12:38:47 +0400 Subject: [PATCH 37/40] Revert changes. --- .../paid-ads/ads-campaign/ads-campaign.js | 1 - .../budget-recommendation/index.js | 14 +-- .../paid-ads/budget-section/index.js | 106 +++++------------- js/src/data/resolvers.js | 5 +- .../pages/create-paid-ads-campaign/index.js | 7 +- js/src/setup-ads/setup-ads-form.js | 15 ++- .../setup-stepper}/clientSession.js | 0 .../setup-mc/setup-stepper/setup-paid-ads.js | 21 +++- 8 files changed, 68 insertions(+), 101 deletions(-) rename js/src/{components/paid-ads/budget-section => setup-mc/setup-stepper}/clientSession.js (100%) diff --git a/js/src/components/paid-ads/ads-campaign/ads-campaign.js b/js/src/components/paid-ads/ads-campaign/ads-campaign.js index 98e1d126be..34324fd40c 100644 --- a/js/src/components/paid-ads/ads-campaign/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign/ads-campaign.js @@ -99,7 +99,6 @@ export default function AdsCampaign( { ? campaign.displayCountries : countryCodes } - context={ context } > { showBillingCard && } diff --git a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js index 153697ec71..573ecad293 100644 --- a/js/src/components/paid-ads/budget-section/budget-recommendation/index.js +++ b/js/src/components/paid-ads/budget-section/budget-recommendation/index.js @@ -10,6 +10,7 @@ import GridiconNoticeOutline from 'gridicons/dist/notice-outline'; * Internal dependencies */ import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap'; +import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; import './index.scss'; function toRecommendationRange( isMultiple, ...values ) { @@ -33,20 +34,17 @@ function toRecommendationRange( isMultiple, ...values ) { } const BudgetRecommendation = ( props ) => { - const { - dailyAverageCost = Infinity, - highestDailyBudget, - highestDailyBudgetCountryCode, - budgetRecommendationData, - } = props; + const { countryCodes, dailyAverageCost = Infinity } = props; + const { data, highestDailyBudgetCountryCode, highestDailyBudget } = + useFetchBudgetRecommendation( countryCodes ); const map = useCountryKeyNameMap(); - if ( ! budgetRecommendationData ) { + if ( ! data ) { return null; } - const { currency, recommendations } = budgetRecommendationData; + const { currency, recommendations } = data; const countryName = map[ highestDailyBudgetCountryCode ]; const recommendationRange = toRecommendationRange( recommendations.length > 1, diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index 85c27085ef..121c74976a 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -2,7 +2,6 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import { useEffect, useRef } from '@wordpress/element'; /** * Internal dependencies @@ -13,9 +12,6 @@ import './index.scss'; 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 @@ -35,57 +31,20 @@ const nonInteractableProps = { * @param {Array|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, values, setValue } = formProps; + const { getInputProps, values } = formProps; const { amount } = values; const { googleAdsAccount } = useGoogleAdsAccount(); - const { - data: budgetRecommendationData, - highestDailyBudget, - highestDailyBudgetCountryCode, - hasFinishedResolution, - } = useFetchBudgetRecommendation( countryCodes ); - const initialAmountRef = useRef( null ); 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; - useEffect( () => { - // Load the amount from client session during the onboarding flow only. - if ( - context !== 'setup-mc' || - ! hasFinishedResolution || - ! values.amount - ) { - return; - } - - if ( values.amount >= highestDailyBudget ) { - clientSession.setCampaign( values ); - } - }, [ values, highestDailyBudget, context, hasFinishedResolution ] ); - - if ( ! initialAmountRef.current && ! amount && hasFinishedResolution ) { - let clientSessionAmount = 0; - if ( context === 'setup-mc' ) { - ( { amount: clientSessionAmount } = clientSession.getCampaign() ); - } - - initialAmountRef.current = true; - setValue( - 'amount', - Math.max( clientSessionAmount, highestDailyBudget ) - ); - } - return (
- { hasFinishedResolution ? ( - <> -
- - -
- { countryCodes?.length > 0 && ( - +
+ + - ) : ( - + suffix={ currency } + value={ monthlyMaxEstimated } + /> +
+ { countryCodes?.length > 0 && ( + ) }
diff --git a/js/src/data/resolvers.js b/js/src/data/resolvers.js index ca1f9eab06..25a1cd99fd 100644 --- a/js/src/data/resolvers.js +++ b/js/src/data/resolvers.js @@ -563,8 +563,5 @@ export function* getAdsBudgetRecommendations( countryCodes ) { } getAdsBudgetRecommendations.shouldInvalidate = ( action ) => { - return ( - action.type === TYPES.RECEIVE_ACCOUNTS_GOOGLE_ADS || - action.type === TYPES.DISCONNECT_ACCOUNTS_GOOGLE_ADS - ); + return action.type === TYPES.DISCONNECT_ACCOUNTS_GOOGLE_ADS; }; diff --git a/js/src/pages/create-paid-ads-campaign/index.js b/js/src/pages/create-paid-ads-campaign/index.js index ae16722d76..3a2596af86 100644 --- a/js/src/pages/create-paid-ads-campaign/index.js +++ b/js/src/pages/create-paid-ads-campaign/index.js @@ -33,6 +33,7 @@ import { recordStepperChangeEvent, recordStepContinueEvent, } from '.~/utils/tracks'; +import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; const eventName = 'gla_paid_campaign_step'; const eventContext = 'create-ads'; @@ -52,6 +53,8 @@ const CreatePaidAdsCampaign = () => { const { createAdsCampaign, updateCampaignAssetGroup } = useAppDispatch(); const { createNotice } = useDispatchCoreNotices(); const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); + const { highestDailyBudget, hasFinishedResolution } = + useFetchBudgetRecommendation( countryCodes ); const handleStepperClick = ( nextStep ) => { recordStepperChangeEvent( @@ -114,7 +117,7 @@ const CreatePaidAdsCampaign = () => { getHistory().push( getDashboardUrl( { campaign: 'saved' } ) ); }; - if ( ! countryCodes ) { + if ( ! countryCodes || ! hasFinishedResolution ) { return null; } @@ -130,7 +133,7 @@ const CreatePaidAdsCampaign = () => { /> diff --git a/js/src/setup-ads/setup-ads-form.js b/js/src/setup-ads/setup-ads-form.js index 74b298db71..6f82cd01bc 100644 --- a/js/src/setup-ads/setup-ads-form.js +++ b/js/src/setup-ads/setup-ads-form.js @@ -17,6 +17,8 @@ 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 useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; +import AppSpinner from '.~/components/app-spinner'; /** * @fires gla_launch_paid_campaign_button_click on submit @@ -27,9 +29,11 @@ const SetupAdsForm = () => { const [ handleSetupComplete, isSubmitting ] = useAdsSetupCompleteCallback(); const adminUrl = useAdminUrl(); const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); + const { highestDailyBudget, hasFinishedResolution } = + useFetchBudgetRecommendation( countryCodes ); const initialValues = { - amount: 0, + amount: highestDailyBudget, }; useEffect( () => { @@ -70,8 +74,13 @@ const SetupAdsForm = () => { setFormChanged( ! isEqual( initialValues, values ) ); }; - if ( ! countryCodes ) { - return null; + if ( ! countryCodes || ! hasFinishedResolution ) { + return ( + <> + + + + ); } return ( diff --git a/js/src/components/paid-ads/budget-section/clientSession.js b/js/src/setup-mc/setup-stepper/clientSession.js similarity index 100% rename from js/src/components/paid-ads/budget-section/clientSession.js rename to js/src/setup-mc/setup-stepper/clientSession.js diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads.js b/js/src/setup-mc/setup-stepper/setup-paid-ads.js index 39ed7f3008..2bb83b2e9f 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads.js @@ -22,6 +22,9 @@ import { API_NAMESPACE } from '.~/data/constants'; import { GUIDE_NAMES, GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; import { ACTION_COMPLETE, ACTION_SKIP } from './constants'; import SkipButton from './skip-button'; +import clientSession from './clientSession'; +import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; +import AppSpinner from '.~/components/app-spinner'; /** * Clicking on the "Complete setup" button to complete the onboarding flow with paid ads. @@ -41,6 +44,8 @@ export default function SetupPaidAds() { const [ completing, setCompleting ] = useState( null ); const { createNotice } = useDispatchCoreNotices(); const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); + const { highestDailyBudget, hasFinishedResolution } = + useFetchBudgetRecommendation( countryCodes ); const [ handleSetupComplete ] = useAdsSetupCompleteCallback(); const { billingStatus } = useGoogleAdsAccountBillingStatus(); @@ -123,10 +128,22 @@ export default function SetupPaidAds() { ); }; + const paidAds = { + amount: highestDailyBudget, + ...clientSession.getCampaign(), + }; + + if ( ! hasFinishedResolution || ! countryCodes ) { + return ; + } + return ( { + if ( values.amount >= highestDailyBudget ) { + clientSession.setCampaign( values ); + } } } > Date: Tue, 22 Oct 2024 13:11:44 +0400 Subject: [PATCH 38/40] Re order components. --- .../setup-ads/ads-stepper/setup-paid-ads.js | 119 ++++++++++++++---- js/src/setup-ads/index.js | 10 +- js/src/setup-ads/setup-ads-form.js | 98 --------------- 3 files changed, 101 insertions(+), 126 deletions(-) delete mode 100644 js/src/setup-ads/setup-ads-form.js diff --git a/js/src/setup-ads/ads-stepper/setup-paid-ads.js b/js/src/setup-ads/ads-stepper/setup-paid-ads.js index 6e38e09c92..0f2d6fd69b 100644 --- a/js/src/setup-ads/ads-stepper/setup-paid-ads.js +++ b/js/src/setup-ads/ads-stepper/setup-paid-ads.js @@ -2,6 +2,9 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; +import { isEqual } from 'lodash'; +import { useState, useEffect } from '@wordpress/element'; +import { getNewPath } from '@woocommerce/navigation'; /** * Internal dependencies @@ -9,42 +12,106 @@ import { __ } from '@wordpress/i18n'; import AppButton from '.~/components/app-button'; import AdsCampaign from '.~/components/paid-ads/ads-campaign'; import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; +import useAdminUrl from '.~/hooks/useAdminUrl'; +import useNavigateAwayPromptEffect from '.~/hooks/useNavigateAwayPromptEffect'; +import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes'; +import useAdsSetupCompleteCallback from '.~/hooks/useAdsSetupCompleteCallback'; +import CampaignAssetsForm from '.~/components/paid-ads/campaign-assets-form'; +import { recordGlaEvent } from '.~/utils/tracks'; +import useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; +import AppSpinner from '.~/components/app-spinner'; import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; const { APPROVED } = GOOGLE_ADS_BILLING_STATUS; /** * Renders the step to setup paid ads - * - * @param {Object} props Component props. - * @param {boolean} props.isSubmitting Indicates if the form is currently being submitted. */ -const SetupPaidAds = ( { isSubmitting } ) => { +const SetupPaidAds = () => { const { billingStatus } = useGoogleAdsAccountBillingStatus(); + const [ didFormChanged, setFormChanged ] = useState( false ); + const [ isSubmitted, setSubmitted ] = useState( false ); + const [ handleSetupComplete, isSubmitting ] = useAdsSetupCompleteCallback(); + const adminUrl = useAdminUrl(); + const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); + const { highestDailyBudget, hasFinishedResolution } = + useFetchBudgetRecommendation( countryCodes ); + + const initialValues = { + amount: highestDailyBudget, + }; + + useEffect( () => { + if ( isSubmitted ) { + // Force reload WC admin page to initiate the relevant dependencies of the Dashboard page. + const nextPath = getNewPath( + { guide: 'campaign-creation-success' }, + '/google/dashboard' + ); + window.location.href = adminUrl + nextPath; + } + }, [ isSubmitted, adminUrl ] ); + + const shouldPreventLeave = didFormChanged && ! isSubmitted; + + useNavigateAwayPromptEffect( + __( + 'You have unsaved campaign data. Are you sure you want to leave?', + 'google-listings-and-ads' + ), + shouldPreventLeave + ); + + const handleSubmit = ( values ) => { + const { amount } = values; + + recordGlaEvent( 'gla_launch_paid_campaign_button_click', { + audiences: countryCodes.join( ',' ), + budget: amount, + } ); + + handleSetupComplete( amount, countryCodes, () => { + setSubmitted( true ); + } ); + }; + + const handleChange = ( _, values ) => { + setFormChanged( ! isEqual( initialValues, values ) ); + }; + + if ( ! countryCodes || ! hasFinishedResolution ) { + return ; + } return ( - ( - - ) } - /> + + ( + + ) } + /> + ); }; diff --git a/js/src/setup-ads/index.js b/js/src/setup-ads/index.js index f191ada062..2f05026b64 100644 --- a/js/src/setup-ads/index.js +++ b/js/src/setup-ads/index.js @@ -2,12 +2,18 @@ * Internal dependencies */ import useLayout from '.~/hooks/useLayout'; -import SetupAdsForm from './setup-ads-form'; +import SetupAdsTopBar from './top-bar'; +import AdsStepper from './ads-stepper'; const SetupAds = () => { useLayout( 'full-page' ); - return ; + return ( + <> + + + + ); }; export default SetupAds; diff --git a/js/src/setup-ads/setup-ads-form.js b/js/src/setup-ads/setup-ads-form.js deleted file mode 100644 index 6f82cd01bc..0000000000 --- a/js/src/setup-ads/setup-ads-form.js +++ /dev/null @@ -1,98 +0,0 @@ -/** - * External dependencies - */ -import { isEqual } from 'lodash'; -import { __ } from '@wordpress/i18n'; -import { useState, useEffect } from '@wordpress/element'; -import { getNewPath } from '@woocommerce/navigation'; - -/** - * Internal dependencies - */ -import useAdminUrl from '.~/hooks/useAdminUrl'; -import useNavigateAwayPromptEffect from '.~/hooks/useNavigateAwayPromptEffect'; -import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes'; -import useAdsSetupCompleteCallback from '.~/hooks/useAdsSetupCompleteCallback'; -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 useFetchBudgetRecommendation from '.~/hooks/useFetchBudgetRecommendation'; -import AppSpinner from '.~/components/app-spinner'; - -/** - * @fires gla_launch_paid_campaign_button_click on submit - */ -const SetupAdsForm = () => { - const [ didFormChanged, setFormChanged ] = useState( false ); - const [ isSubmitted, setSubmitted ] = useState( false ); - const [ handleSetupComplete, isSubmitting ] = useAdsSetupCompleteCallback(); - const adminUrl = useAdminUrl(); - const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); - const { highestDailyBudget, hasFinishedResolution } = - useFetchBudgetRecommendation( countryCodes ); - - const initialValues = { - amount: highestDailyBudget, - }; - - useEffect( () => { - if ( isSubmitted ) { - // Force reload WC admin page to initiate the relevant dependencies of the Dashboard page. - const nextPath = getNewPath( - { guide: 'campaign-creation-success' }, - '/google/dashboard' - ); - window.location.href = adminUrl + nextPath; - } - }, [ isSubmitted, adminUrl ] ); - - const shouldPreventLeave = didFormChanged && ! isSubmitted; - - useNavigateAwayPromptEffect( - __( - 'You have unsaved campaign data. Are you sure you want to leave?', - 'google-listings-and-ads' - ), - shouldPreventLeave - ); - - const handleSubmit = ( values ) => { - const { amount } = values; - - recordGlaEvent( 'gla_launch_paid_campaign_button_click', { - audiences: countryCodes.join( ',' ), - budget: amount, - } ); - - handleSetupComplete( amount, countryCodes, () => { - setSubmitted( true ); - } ); - }; - - const handleChange = ( _, values ) => { - setFormChanged( ! isEqual( initialValues, values ) ); - }; - - if ( ! countryCodes || ! hasFinishedResolution ) { - return ( - <> - - - - ); - } - - return ( - - - - - ); -}; - -export default SetupAdsForm; From 90679602b8505a24d1d62753a0dfedaa956f6fbc Mon Sep 17 00:00:00 2001 From: asvinb Date: Tue, 22 Oct 2024 13:21:16 +0400 Subject: [PATCH 39/40] Remove check for Google Ads account. --- js/src/hooks/useFetchBudgetRecommendation.js | 21 +------------------- js/src/setup-ads/ads-stepper/index.js | 6 ++---- 2 files changed, 3 insertions(+), 24 deletions(-) diff --git a/js/src/hooks/useFetchBudgetRecommendation.js b/js/src/hooks/useFetchBudgetRecommendation.js index 45caf54318..bea96033ce 100644 --- a/js/src/hooks/useFetchBudgetRecommendation.js +++ b/js/src/hooks/useFetchBudgetRecommendation.js @@ -8,7 +8,6 @@ import { useSelect } from '@wordpress/data'; */ import { STORE_KEY } from '.~/data/constants'; import getHighestBudget from '.~/utils/getHighestBudget'; -import useGoogleAdsAccount from './useGoogleAdsAccount'; /** * @typedef { import(".~/data/actions").CountryCode } CountryCode @@ -21,22 +20,8 @@ import useGoogleAdsAccount from './useGoogleAdsAccount'; * @return {Object} Budget recommendation. */ const useFetchBudgetRecommendation = ( countryCodes ) => { - const { - hasGoogleAdsConnection, - hasFinishedResolution: hasFinishedResolutionAdsAccount, - } = useGoogleAdsAccount(); - return useSelect( ( select ) => { - if ( ! hasGoogleAdsConnection ) { - return { - data: undefined, - highestDailyBudget: 0, - highestDailyBudgetCountryCode: undefined, - hasFinishedResolution: hasFinishedResolutionAdsAccount, - }; - } - const { getAdsBudgetRecommendations, hasFinishedResolution } = select( STORE_KEY ); @@ -62,11 +47,7 @@ const useFetchBudgetRecommendation = ( countryCodes ) => { ), }; }, - [ - countryCodes, - hasGoogleAdsConnection, - hasFinishedResolutionAdsAccount, - ] + [ countryCodes ] ); }; diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 51fe990ffb..5a7d70cdc3 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -19,12 +19,10 @@ import { import SetupPaidAds from './setup-paid-ads'; /** - * @param {Object} props React props - * @param {boolean} props.isSubmitting When the form in the parent component, i.e SetupAdsForm, is currently being submitted via the useAdsSetupCompleteCallback hook. * @fires gla_setup_ads with `{ triggered_by: 'step1-continue-button', action: 'go-to-step2' }`. * @fires gla_setup_ads with `{ triggered_by: 'stepper-step1-button', action: 'go-to-step1'}`. */ -const AdsStepper = ( { isSubmitting } ) => { +const AdsStepper = () => { const [ step, setStep ] = useState( '1' ); useEventPropertiesFilter( FILTER_ONBOARDING, { @@ -84,7 +82,7 @@ const AdsStepper = ( { isSubmitting } ) => { 'Create your paid campaign', 'google-listings-and-ads' ), - content: , + content: , onClick: handleStepClick, }, ] } From a560e908a692e62e033e11faf26c27e6ab85b97f Mon Sep 17 00:00:00 2001 From: asvinb Date: Tue, 22 Oct 2024 13:24:18 +0400 Subject: [PATCH 40/40] Add missing JSDoc. --- js/src/setup-ads/ads-stepper/setup-paid-ads.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/src/setup-ads/ads-stepper/setup-paid-ads.js b/js/src/setup-ads/ads-stepper/setup-paid-ads.js index 0f2d6fd69b..905d51bc2d 100644 --- a/js/src/setup-ads/ads-stepper/setup-paid-ads.js +++ b/js/src/setup-ads/ads-stepper/setup-paid-ads.js @@ -26,6 +26,8 @@ const { APPROVED } = GOOGLE_ADS_BILLING_STATUS; /** * Renders the step to setup paid ads + * + * @fires gla_launch_paid_campaign_button_click on submit */ const SetupPaidAds = () => { const { billingStatus } = useGoogleAdsAccountBillingStatus();