diff --git a/js/src/components/paid-ads/__snapshots__/validateCampaign.test.js.snap b/js/src/components/paid-ads/__snapshots__/validateCampaign.test.js.snap index 8e5ebb2814..38bbccb4fd 100644 --- a/js/src/components/paid-ads/__snapshots__/validateCampaign.test.js.snap +++ b/js/src/components/paid-ads/__snapshots__/validateCampaign.test.js.snap @@ -11,5 +11,3 @@ exports[`validateCampaign When the amount is not a number, should not pass 4`] = exports[`validateCampaign When the amount is ≤ 0, should not pass 1`] = `"Please make sure daily average cost is greater than 0."`; exports[`validateCampaign When the amount is ≤ 0, should not pass 2`] = `"Please make sure daily average cost is greater than 0."`; - -exports[`validateCampaign When the country codes array is empty, should not pass 1`] = `"Please select at least one country for your ads campaign."`; diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 9de67da91e..2149c78801 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -94,6 +94,7 @@ export default function AdsCampaign( { diff --git a/js/src/components/paid-ads/budget-section/index.js b/js/src/components/paid-ads/budget-section/index.js index 5c461602e6..87f2b940fc 100644 --- a/js/src/components/paid-ads/budget-section/index.js +++ b/js/src/components/paid-ads/budget-section/index.js @@ -14,6 +14,10 @@ import BudgetRecommendation from './budget-recommendation'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import AppInputPriceControl from '.~/components/app-input-price-control'; +/** + * @typedef {import('.~/data/actions').CountryCode} CountryCode + */ + const nonInteractableProps = { noPointerEvents: true, readOnly: true, @@ -25,12 +29,18 @@ 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 {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. */ -const BudgetSection = ( { formProps, disabled = false, children } ) => { +const BudgetSection = ( { + formProps, + countryCodes, + disabled = false, + children, +} ) => { const { getInputProps, setValue, values } = formProps; - const { countryCodes, amount } = values; + 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. @@ -89,7 +99,7 @@ const BudgetSection = ( { formProps, disabled = false, children } ) => { value={ monthlyMaxEstimated } /> - { countryCodes.length > 0 && ( + { countryCodes?.length > 0 && ( { const errors = {}; - if ( values.countryCodes.length === 0 ) { - errors.countryCodes = __( - 'Please select at least one country for your ads campaign.', - 'google-listings-and-ads' - ); - } - if ( ! Number.isFinite( values.amount ) || values.amount <= 0 ) { errors.amount = __( 'Please make sure daily average cost is greater than 0.', diff --git a/js/src/components/paid-ads/validateCampaign.test.js b/js/src/components/paid-ads/validateCampaign.test.js index afad100326..efd4480dac 100644 --- a/js/src/components/paid-ads/validateCampaign.test.js +++ b/js/src/components/paid-ads/validateCampaign.test.js @@ -12,12 +12,11 @@ describe( 'validateCampaign', () => { beforeEach( () => { // Initial values - values = { countryCodes: [], amount: 0 }; + values = { amount: 0 }; } ); it( 'When all checks are passed, should return an empty object', () => { const errors = validateCampaign( { - countryCodes: [ 'US' ], amount: 1, } ); @@ -27,17 +26,9 @@ describe( 'validateCampaign', () => { it( 'should indicate multiple unpassed checks by setting properties in the returned object', () => { const errors = validateCampaign( values ); - expect( errors ).toHaveProperty( 'countryCodes' ); expect( errors ).toHaveProperty( 'amount' ); } ); - it( 'When the country codes array is empty, should not pass', () => { - const errors = validateCampaign( values ); - - expect( errors ).toHaveProperty( 'countryCodes' ); - expect( errors.countryCodes ).toMatchSnapshot(); - } ); - it( 'When the amount is not a number, should not pass', () => { let errors; diff --git a/js/src/setup-mc/setup-stepper/setup-paid-ads/clientSession.js b/js/src/setup-mc/setup-stepper/setup-paid-ads/clientSession.js index 67aeae5d96..7e75702484 100644 --- a/js/src/setup-mc/setup-stepper/setup-paid-ads/clientSession.js +++ b/js/src/setup-mc/setup-stepper/setup-paid-ads/clientSession.js @@ -1,9 +1,6 @@ /** - * @typedef { import(".~/data/actions").CountryCode } CountryCode - * * @typedef {Object} CampaignData * @property {number|undefined} amount Daily average cost of the paid ads campaign. - * @property {Array} countryCodes Audience country codes of the paid ads campaign. Example: 'US'. */ const KEY_PAID_ADS = 'gla-onboarding-paid-ads'; @@ -14,10 +11,9 @@ const clientSession = { /** * @param {CampaignData} data Campaign data to be stored. * @param {number|undefined} data.amount Daily average cost of the campaign. - * @param {Array} data.countryCodes Country codes of the campaign. */ - setCampaign( { amount, countryCodes } ) { - const json = JSON.stringify( { amount, countryCodes } ); + setCampaign( { amount } ) { + const json = JSON.stringify( { amount } ); sessionStorage.setItem( KEY_PAID_ADS, json ); }, 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..7072eab023 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,17 +1,13 @@ /** * External dependencies */ -import { __ } from '@wordpress/i18n'; import { useState, useRef, useEffect } from '@wordpress/element'; import { Form } from '@woocommerce/components'; /** * Internal dependencies */ -import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; -import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes'; import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; -import AudienceSection from '.~/components/paid-ads/audience-section'; import BudgetSection from '.~/components/paid-ads/budget-section'; import BillingCard from '.~/components/paid-ads/billing-card'; import SpinnerCard from '.~/components/spinner-card'; @@ -21,47 +17,31 @@ import clientSession from './clientSession'; import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; /** - * @typedef { import(".~/data/actions").CountryCode } CountryCode - * + * @typedef {import('.~/data/actions').CountryCode} CountryCode + */ + +/** * @typedef {Object} PaidAdsData * @property {number|undefined} amount Daily average cost of the paid ads campaign. - * @property {Array} countryCodes Audience country codes of the paid ads campaign. Example: 'US'. * @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. */ const defaultPaidAds = { amount: 0, - countryCodes: [], isValid: false, isReady: false, }; /** - * Resolve the initial paid ads data from the given paid ads data with the loaded target audience. + * Resolve the initial paid ads data from the given paid ads data. * Parts of the resolved data are used in the `initialValues` prop of `Form` component. * * @param {PaidAdsData} paidAds The paid ads data as the base to be resolved with other states. - * @param {Array} targetAudience Country codes of selected target audience. * @return {PaidAdsData} The resolved paid ads data. */ -function resolveInitialPaidAds( paidAds, targetAudience ) { +function resolveInitialPaidAds( paidAds ) { const nextPaidAds = { ...paidAds }; - - if ( targetAudience ) { - if ( nextPaidAds.countryCodes === defaultPaidAds.countryCodes ) { - // Replace the country codes with the loaded target audience only if the reference is - // the same as the default because the given country codes might be the restored ones. - nextPaidAds.countryCodes = targetAudience; - } else { - // The selected target audience may be changed back and forth during the onboarding flow. - // Remove countries if any don't exist in the latest state. - nextPaidAds.countryCodes = nextPaidAds.countryCodes.filter( - ( code ) => targetAudience.includes( code ) - ); - } - } - nextPaidAds.isValid = ! Object.keys( validateCampaign( nextPaidAds ) ) .length; @@ -69,14 +49,16 @@ function resolveInitialPaidAds( paidAds, targetAudience ) { } /** - * Renders sections of Google Ads account, audience, budget, and billing for setting up the paid ads. + * 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 audience, budget, and billing are updated. + * @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. */ -export default function PaidAdsSetupSections( { onStatesReceived } ) { - const { hasGoogleAdsConnection } = useGoogleAdsAccount(); - const { data: targetAudience } = useTargetAudienceFinalCountryCodes(); +export default function PaidAdsSetupSections( { + onStatesReceived, + countryCodes, +} ) { const { billingStatus } = useGoogleAdsAccountBillingStatus(); const onStatesReceivedRef = useRef(); @@ -88,7 +70,7 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { ...defaultPaidAds, ...clientSession.getCampaign(), }; - return resolveInitialPaidAds( startingPaidAds, targetAudience ); + return resolveInitialPaidAds( startingPaidAds ); } ); const isBillingCompleted = @@ -117,22 +99,7 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { clientSession.setCampaign( nextPaidAds ); }, [ paidAds, isBillingCompleted ] ); - /* - Resolve the initial states after the `targetAudience` is loaded. - - Please note that the loaded `targetAudience` is NOT expected to have further changes - in the runtime. If it happens one day and it will need to update
's internal state - with the changed `targetAudience`, please refer to the following practice. - - https://github.com/woocommerce/google-listings-and-ads/blob/5b6522ca10ad75556e6b2de7c120cc712aab70b1/js/src/components/free-listings/setup-free-listings/index.js#L120-L134 - - 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 ] ); - - if ( ! targetAudience || ! billingStatus ) { + if ( ! billingStatus ) { return (
@@ -141,7 +108,6 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { } const initialValues = { - countryCodes: paidAds.countryCodes, amount: paidAds.amount, }; @@ -154,28 +120,13 @@ export default function PaidAdsSetupSections( { onStatesReceived } ) { validate={ validateCampaign } > { ( formProps ) => { - const { countryCodes } = formProps.values; - const disabledAudience = ! hasGoogleAdsConnection; - const disabledBudget = - disabledAudience || countryCodes.length === 0; - return ( - <> - - - { ! disabledBudget && } - - + + + ); } } 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 ede40b2af9..cd5f77ac13 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 @@ -15,6 +15,7 @@ import useAdminUrl from '.~/hooks/useAdminUrl'; import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useAdsSetupCompleteCallback from '.~/hooks/useAdsSetupCompleteCallback'; +import useTargetAudienceFinalCountryCodes from '.~/hooks/useTargetAudienceFinalCountryCodes'; import StepContent from '.~/components/stepper/step-content'; import StepContentHeader from '.~/components/stepper/step-content-header'; import StepContentFooter from '.~/components/stepper/step-content-footer'; @@ -66,6 +67,7 @@ import { recordGlaEvent } from '.~/utils/tracks'; export default function SetupPaidAds() { const adminUrl = useAdminUrl(); const { createNotice } = useDispatchCoreNotices(); + const { data: countryCodes } = useTargetAudienceFinalCountryCodes(); const { googleAdsAccount, hasGoogleAdsConnection } = useGoogleAdsAccount(); const [ handleSetupComplete ] = useAdsSetupCompleteCallback(); const [ paidAds, setPaidAds ] = useState( {} ); @@ -105,7 +107,7 @@ export default function SetupPaidAds() { const onBeforeFinish = handleSetupComplete.bind( null, paidAds.amount, - paidAds.countryCodes + countryCodes ); await finishOnboardingSetup( event, onBeforeFinish ); }; @@ -169,7 +171,10 @@ export default function SetupPaidAds() { - + { showSkipPaidAdsConfirmationModal && ( @@ -200,7 +205,7 @@ export default function SetupPaidAds() { eventName="gla_onboarding_complete_with_paid_ads_button_click" eventProps={ { budget: paidAds.amount, - audiences: paidAds.countryCodes?.join( ',' ), + audiences: countryCodes?.join( ',' ), } } /> 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 a957d800f4..583a6f4966 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 @@ -11,13 +11,8 @@ import CompleteCampaign from '../../utils/pages/setup-mc/step-4-complete-campaig import SetupAdsAccountPage from '../../utils/pages/setup-ads/setup-ads-accounts'; import { checkFAQExpandable, - fillCountryInSearchBox, - getCountryInputSearchBoxContainer, - getCountryTagsFromInputSearchBoxContainer, getFAQPanelTitle, getFAQPanelRow, - getTreeSelectMenu, - removeCountryFromSearchBox, checkBillingAdsPopup, } from '../../utils/page'; @@ -174,17 +169,6 @@ test.describe( 'Complete your campaign', () => { } ); test.describe( 'Setup up ads to a Google Ads account', () => { - test( 'should see "Ads audience" section is enabled', async () => { - const adsAudienceSection = - completeCampaign.getAdsAudienceSection(); - await expect( adsAudienceSection ).toBeVisible(); - - // Confirm that the section title contains the correct text. - await expect( - adsAudienceSection.locator( 'h1' ) - ).toContainText( 'Ads audience' ); - } ); - test( 'should see "Set your budget" section is enabled', async () => { const budgetSection = completeCampaign.getBudgetSection(); await expect( budgetSection ).toBeVisible(); @@ -210,78 +194,6 @@ test.describe( 'Complete your campaign', () => { await completeCampaign.goto(); } ); - test.describe( 'Select audience', () => { - test( 'should see only three country tags in country input search box', async () => { - const countrySearchBoxContainer = - getCountryInputSearchBoxContainer( page ); - const countryTags = - getCountryTagsFromInputSearchBoxContainer( page ); - await expect( countryTags ).toHaveCount( 3 ); - await expect( countrySearchBoxContainer ).toContainText( - 'United States' - ); - await expect( countrySearchBoxContainer ).toContainText( - 'Taiwan' - ); - await expect( countrySearchBoxContainer ).toContainText( - 'United Kingdom' - ); - } ); - - test( 'should only allow searching for the same set of the countries selected in step 2, which is returned by target audience API', async () => { - const treeSelectMenu = getTreeSelectMenu( page ); - - await fillCountryInSearchBox( page, 'United States' ); - await expect( treeSelectMenu ).toBeVisible(); - - await fillCountryInSearchBox( page, 'United Kingdom' ); - await expect( treeSelectMenu ).toBeVisible(); - - await fillCountryInSearchBox( page, 'Taiwan' ); - await expect( treeSelectMenu ).toBeVisible(); - - await fillCountryInSearchBox( page, 'Japan' ); - await expect( treeSelectMenu ).not.toBeVisible(); - - await fillCountryInSearchBox( page, 'Spain' ); - await expect( treeSelectMenu ).not.toBeVisible(); - } ); - - test( 'should see the budget recommendation value changed, and see the budget recommendation request is triggered when changing the ads audience', async () => { - let textContent = await setupBudgetPage - .getBudgetRecommendationTextRow() - .textContent(); - - const textBeforeRemoveCountry = - setupBudgetPage.extractBudgetRecommendationValue( - textContent - ); - - const responsePromise = - setupBudgetPage.registerBudgetRecommendationResponse(); - - await removeCountryFromSearchBox( - page, - 'United Kingdom (UK)' - ); - - await responsePromise; - - textContent = await setupBudgetPage - .getBudgetRecommendationTextRow() - .textContent(); - - const textAfterRemoveCountry = - setupBudgetPage.extractBudgetRecommendationValue( - textContent - ); - - await expect( textBeforeRemoveCountry ).not.toBe( - textAfterRemoveCountry - ); - } ); - } ); - test.describe( 'Set up budget', () => { test( 'should see the low budget tip when the buget is set lower than the recommended value', async () => { await setupBudgetPage.fillBudget( '1' ); diff --git a/tests/e2e/utils/page.js b/tests/e2e/utils/page.js index e68078d053..54912b00f8 100644 --- a/tests/e2e/utils/page.js +++ b/tests/e2e/utils/page.js @@ -80,17 +80,6 @@ export function getCountryInputSearchBox( page ) { ); } -/** - * Get tree select menu. - * - * @param {import('@playwright/test').Page} page The current page. - * - * @return {import('@playwright/test').Locator} Get tree select menu. - */ -export function getTreeSelectMenu( page ) { - return page.locator( '.woocommerce-tree-select-control__main' ); -} - /** * Get tree item by country name. * diff --git a/tests/e2e/utils/pages/setup-ads/setup-budget.js b/tests/e2e/utils/pages/setup-ads/setup-budget.js index dca6843f64..a94232393c 100644 --- a/tests/e2e/utils/pages/setup-ads/setup-budget.js +++ b/tests/e2e/utils/pages/setup-ads/setup-budget.js @@ -12,15 +12,6 @@ export default class SetupBudget extends MockRequests { this.page = page; } - /** - * Get budget recommendation text row. - * - * @return {import('@playwright/test').Locator} The budget recommendation text row. - */ - getBudgetRecommendationTextRow() { - return this.page.locator( '.components-tip p > em > strong' ); - } - /** * Get budget input. * @@ -84,36 +75,6 @@ export default class SetupBudget extends MockRequests { ); } - /** - * Extract budget recommendation value. - * - * @param {string} text - * - * @return {string} The budget recommendation value. - */ - extractBudgetRecommendationValue( text ) { - const match = text.match( /set a daily budget of (\d+)/ ); - if ( match ) { - return match[ 1 ]; - } - return ''; - } - - /** - * Register the responses when removing an ads audience. - * - * @return {Promise} The response. - */ - registerBudgetRecommendationResponse() { - return this.page.waitForResponse( - ( response ) => - response - .url() - .includes( '/gla/ads/campaigns/budget-recommendation' ) && - response.status() === 200 - ); - } - /** * Fill the budget. * diff --git a/tests/e2e/utils/pages/setup-mc/step-4-complete-campaign.js b/tests/e2e/utils/pages/setup-mc/step-4-complete-campaign.js index 7458a4055b..909f1ea8cd 100644 --- a/tests/e2e/utils/pages/setup-mc/step-4-complete-campaign.js +++ b/tests/e2e/utils/pages/setup-mc/step-4-complete-campaign.js @@ -46,15 +46,6 @@ export default class CompleteCampaign extends MockRequests { return this.page.locator( '.wcdl-section' ); } - /** - * Get ads audience section. - * - * @return {import('@playwright/test').Locator} Get ads audience section. - */ - getAdsAudienceSection() { - return this.getSections().nth( 1 ); - } - /** * Get budget section. *