From 40ea4eb1a484380447862890021639a02cd3a571 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Wed, 4 Sep 2024 13:32:30 +0100 Subject: [PATCH 01/32] remove ads campaign steps --- js/src/components/paid-ads/ads-campaign.js | 9 +++++---- js/src/setup-ads/ads-stepper/index.js | 13 +------------ 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 9de67da91e..0b8a1b989e 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -32,12 +32,12 @@ import FaqsSection from './faqs-section'; * * @param {Object} props React props. * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. - * @param {() => void} props.onContinue Callback called once continue button is clicked. + * @param {Object} props.formProps Form props forwarded from `Form` component. * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. */ export default function AdsCampaign( { campaign, - onContinue, + formProps: { isSubmitting, handleSubmit }, trackingContext, } ) { const isCreation = ! campaign; @@ -102,9 +102,10 @@ export default function AdsCampaign( { - { __( 'Continue', 'google-listings-and-ads' ) } + { __( 'Launch paid campaign', 'google-listings-and-ads' ) } diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 3979e0dcba..374e78b959 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -10,7 +10,6 @@ import { useState } from '@wordpress/element'; */ import SetupAccounts from './setup-accounts'; import AdsCampaign from '.~/components/paid-ads/ads-campaign'; -import SetupBilling from './setup-billing'; import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter'; import { recordStepperChangeEvent, @@ -58,10 +57,6 @@ const AdsStepper = ( { formProps } ) => { continueStep( '2' ); }; - const handleCreateCampaignContinue = () => { - continueStep( '3' ); - }; - return ( // This Stepper with this class name // should be refactored into separate shared component. @@ -92,17 +87,11 @@ const AdsStepper = ( { formProps } ) => { content: ( ), onClick: handleStepClick, }, - { - key: '3', - label: __( 'Set up billing', 'google-listings-and-ads' ), - content: , - onClick: handleStepClick, - }, ] } /> ); From 8a9a10a978dd55483f8e5a4643867d65acce1794 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Wed, 4 Sep 2024 16:51:49 +0100 Subject: [PATCH 02/32] cs fixes --- js/src/components/paid-ads/ads-campaign.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 0b8a1b989e..b298352296 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -32,7 +32,8 @@ import FaqsSection from './faqs-section'; * * @param {Object} props React props. * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. - * @param {Object} props.formProps Form props forwarded from `Form` component. + * @param {Object} props.formProps.isSubmitting Callback to display loading till Ads campaign is created. + * @param {Object} props.formProps.handleSubmit Callback to handle campaign creation. * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. */ export default function AdsCampaign( { From 94c8208fabd8f385370d44105c572a1d186dcd08 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Wed, 4 Sep 2024 16:55:27 +0100 Subject: [PATCH 03/32] e2e support --- .../add-paid-campaigns.test.js | 172 ++++-------------- 1 file changed, 34 insertions(+), 138 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 805f77e798..9cfe8eae39 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 @@ -280,11 +280,15 @@ test.describe( 'Set up Ads account', () => { } ); test.describe( 'Create your paid campaign', () => { + const launchPaidCampaignButton = page.getByRole( 'button', { + name: 'Launch paid campaign' + } ); + test.beforeAll( async () => { setupBudgetPage = new SetupBudgetPage( page ); } ); - test( 'Continue to create paid campaign', async () => { + test( 'Continue to create paid campaign and create ads', async () => { await setupAdsAccounts.clickContinue(); await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); @@ -303,7 +307,7 @@ test.describe( 'Set up Ads account', () => { ).toBeVisible(); await expect( - page.getByRole( 'button', { name: 'Continue' } ) + launchPaidCampaignButton ).toBeDisabled(); await expect( @@ -371,14 +375,14 @@ test.describe( 'Set up Ads account', () => { await setupBudgetPage.fillBudget( budget ); await expect( - page.getByRole( 'button', { name: 'Continue' } ) + launchPaidCampaignButton ).toBeDisabled(); budget = '1'; await setupBudgetPage.fillBudget( budget ); await expect( - page.getByRole( 'button', { name: 'Continue' } ) + launchPaidCampaignButton ).toBeEnabled(); } ); @@ -387,152 +391,44 @@ test.describe( 'Set up Ads account', () => { page.getByText( 'set a daily budget of 15 USD' ) ).toBeVisible(); } ); - } ); - - test.describe( 'Set up billing', () => { - test.describe( 'Billing status is not approved', () => { - test.beforeAll( async () => { - await setupBudgetPage.fulfillBillingStatusRequest( { - status: 'pending', - } ); - } ); - test( 'It should say that the billing is not setup', async () => { - await page.getByRole( 'button', { name: 'Continue' } ).click(); - await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); - - await expect( - page.getByRole( 'button', { - name: 'Set up billing', - exact: true, - } ) - ).toBeEnabled(); - - await expect( - page.getByText( - 'In order to launch your paid campaign, your billing information is required. You will be billed directly by Google and only pay when someone clicks on your ad.' - ) - ).toBeVisible(); - - await expect( - page.getByRole( 'link', { - name: 'click here instead', - } ) - ).toBeVisible(); - } ); - - // eslint-disable-next-line jest/expect-expect - test( 'should open a popup when clicking set up billing button', async () => { - await checkBillingAdsPopup( page ); - } ); - } ); - - test.describe( 'Billing status is approved', async () => { - test.beforeAll( async () => { - await setupBudgetPage.fulfillBillingStatusRequest( { - status: 'approved', - } ); - - await setupAdsAccounts.mockAdsAccountsResponse( { - id: ADS_ACCOUNTS[ 1 ], - billing_url: null, - } ); - - // Simulate a bit of delay when creating the Ads campaign so we have enough time to test the content in the page before the redirect. - await page.route( - /\/wc\/gla\/ads\/campaigns\b/, - async ( route ) => { - await new Promise( ( f ) => setTimeout( f, 500 ) ); - await route.continue(); - } - ); - } ); - test( 'It should say that the billing is setup', async () => { - //Every 30s the page will check if the billing status is approved and it will trigger the campaign creation. - await setupBudgetPage.awaitForBillingStatusRequest(); - await setupBudgetPage.mockCampaignCreationAndAdsSetupCompletion( - budget, - [ 'US' ] - ); - - await expect( - page.getByText( - 'Great! You already have billing information saved for this' - ) - ).toBeVisible(); - - //It should redirect to the dashboard page - await page.waitForURL( - '/wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fdashboard&guide=campaign-creation-success', - { - timeout: 30000, - waitUntil: LOAD_STATE.DOM_CONTENT_LOADED, - } - ); - } ); - - test( 'It should show the campaign creation success message', async () => { - await expect( - page.getByRole( 'heading', { - name: "You've set up a paid Performance Max Campaign!", - } ) - ).toBeVisible(); - await expect( - page.getByRole( 'button', { - name: 'Create another campaign', - } ) - ).toBeEnabled(); + test( 'Paid Campaign can be created', async () => { + launchPaidCampaignButton.click(); - await expect( - page.getByRole( 'button', { - name: 'Got It', - } ) - ).toBeEnabled(); - - await page - .getByRole( 'button', { - name: 'Got It', - } ) - .click(); - } ); + //It should redirect to the dashboard page + await page.waitForURL( + '/wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fdashboard&guide=campaign-creation-success', + { + timeout: 30000, + waitUntil: LOAD_STATE.DOM_CONTENT_LOADED, + } + ); } ); - } ); - - test.describe( 'Create Ads with billing data already setup', () => { - test( 'Launch paid campaign should be enabled', async () => { - //Click Add paid Campaign - await adsConnectionButton.click(); - await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); - - //Step 1 - Accounts are already set up. - await setupAdsAccounts.clickContinue(); - await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); - - //Step 2 - Fill the budget - await setupBudgetPage.fillBudget( '1' ); - await page.getByRole( 'button', { name: 'Continue' } ).click(); - await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); - //Step 3 - Billing is already setup + test( 'It should show the campaign creation success message', async () => { await expect( - page.getByText( - 'Great! You already have billing information saved for this' - ) + page.getByRole( 'heading', { + name: "You've set up a paid Performance Max Campaign!", + } ) ).toBeVisible(); await expect( - page.getByRole( 'button', { name: 'Launch paid campaign' } ) + page.getByRole( 'button', { + name: 'Create another campaign', + } ) + ).toBeEnabled(); + + await expect( + page.getByRole( 'button', { + name: 'Got It', + } ) ).toBeEnabled(); - const campaignCreation = - setupBudgetPage.mockCampaignCreationAndAdsSetupCompletion( - '1', - [ 'US' ] - ); await page - .getByRole( 'button', { name: 'Launch paid campaign' } ) + .getByRole( 'button', { + name: 'Got It', + } ) .click(); - await campaignCreation; } ); } ); } ); From 39a2090cb98ffa4c3363a75f8aaf3334608e276b Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Wed, 4 Sep 2024 17:18:00 +0100 Subject: [PATCH 04/32] cs fixes --- .../add-paid-campaigns/add-paid-campaigns.test.js | 15 ++++----------- 1 file changed, 4 insertions(+), 11 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 9cfe8eae39..db3dbde322 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 @@ -16,7 +16,6 @@ import { getFAQPanelTitle, getFAQPanelRow, checkFAQExpandable, - checkBillingAdsPopup, } from '../../utils/page'; const ADS_ACCOUNTS = [ @@ -281,7 +280,7 @@ test.describe( 'Set up Ads account', () => { test.describe( 'Create your paid campaign', () => { const launchPaidCampaignButton = page.getByRole( 'button', { - name: 'Launch paid campaign' + name: 'Launch paid campaign', } ); test.beforeAll( async () => { @@ -306,9 +305,7 @@ test.describe( 'Set up Ads account', () => { page.getByRole( 'heading', { name: 'Set your budget' } ) ).toBeVisible(); - await expect( - launchPaidCampaignButton - ).toBeDisabled(); + await expect( launchPaidCampaignButton ).toBeDisabled(); await expect( page.getByRole( 'link', { @@ -374,16 +371,12 @@ test.describe( 'Set up Ads account', () => { budget = '0'; await setupBudgetPage.fillBudget( budget ); - await expect( - launchPaidCampaignButton - ).toBeDisabled(); + await expect( launchPaidCampaignButton ).toBeDisabled(); budget = '1'; await setupBudgetPage.fillBudget( budget ); - await expect( - launchPaidCampaignButton - ).toBeEnabled(); + await expect( launchPaidCampaignButton ).toBeEnabled(); } ); test( 'Budget Recommendation', async () => { From 09871a5edad626e52604cdc5343332509bc195d0 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Wed, 4 Sep 2024 17:24:58 +0100 Subject: [PATCH 05/32] cs fixes --- js/src/components/paid-ads/ads-campaign.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index b298352296..0b4f65270d 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -25,15 +25,15 @@ import FaqsSection from './faqs-section'; /** * Renders the container of the form content for campaign management. * - * Please note that this component relies on an CampaignAssetsForm's context and custom adapter, - * so it expects a `CampaignAssetsForm` to existing in its parents. + * Please note that this component relies on a CampaignAssetsForm's context and custom adapter, + * so it expects a `CampaignAssetsForm` to exist in its parents. * * @fires gla_documentation_link_click with `{ context: 'create-ads' | 'edit-ads' | 'setup-ads', link_id: 'see-what-ads-look-like', href: 'https://support.google.com/google-ads/answer/6275294' }` * * @param {Object} props React props. * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. - * @param {Object} props.formProps.isSubmitting Callback to display loading till Ads campaign is created. - * @param {Object} props.formProps.handleSubmit Callback to handle campaign creation. + * @param {Function} props.formProps.isSubmitting Callback to display loading till Ads campaign is created. + * @param {Function} props.formProps.handleSubmit Callback to handle campaign creation. * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. */ export default function AdsCampaign( { From 5b14c071fc51a11aad28a9fd825cba70eb5611bd Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Wed, 4 Sep 2024 17:50:56 +0100 Subject: [PATCH 06/32] fix cs issue --- js/src/components/paid-ads/ads-campaign.js | 1 + 1 file changed, 1 insertion(+) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 0b4f65270d..85c3c818b0 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -32,6 +32,7 @@ import FaqsSection from './faqs-section'; * * @param {Object} props React props. * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. + * @param {Object} props.formProps Form props forwarded from `Form` component. * @param {Function} props.formProps.isSubmitting Callback to display loading till Ads campaign is created. * @param {Function} props.formProps.handleSubmit Callback to handle campaign creation. * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. From 9728b038f942de62e500bcfda4c3262dcb767e9b Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Sat, 7 Sep 2024 02:47:43 +0100 Subject: [PATCH 07/32] address review comments and also add submitbuttontext --- js/src/components/paid-ads/ads-campaign.js | 17 ++++++++++------- js/src/setup-ads/ads-stepper/index.js | 4 +++- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 85c3c818b0..6795597a52 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -32,19 +32,22 @@ import FaqsSection from './faqs-section'; * * @param {Object} props React props. * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. - * @param {Object} props.formProps Form props forwarded from `Form` component. - * @param {Function} props.formProps.isSubmitting Callback to display loading till Ads campaign is created. - * @param {Function} props.formProps.handleSubmit Callback to handle campaign creation. + * @param {() => void} props.onContinue Callback called once continue button is clicked. + * @param {() => void} props.isLoading Callback called once continue button is clicked. + * @param {String} submitButtonText Text to display on . * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. */ export default function AdsCampaign( { campaign, - formProps: { isSubmitting, handleSubmit }, + onContinue, + isLoading, + submitButtonText, trackingContext, } ) { const isCreation = ! campaign; const formContext = useAdaptiveFormContext(); const { isValidForm } = formContext; + const continueButtonText = submitButtonText || __( 'Continue', 'google-listings-and-ads' ); const disabledBudgetSection = ! formContext.values.countryCodes.length; const helperText = isCreation @@ -104,10 +107,10 @@ export default function AdsCampaign( { - { __( 'Launch paid campaign', 'google-listings-and-ads' ) } + { continueButtonText } diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 374e78b959..2ed1869041 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -87,7 +87,9 @@ const AdsStepper = ( { formProps } ) => { content: ( ), onClick: handleStepClick, From f06b5d279ac0b80020f909b2d9d38c5f5236bc38 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Sat, 7 Sep 2024 02:53:08 +0100 Subject: [PATCH 08/32] fix doc block --- js/src/components/paid-ads/ads-campaign.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 6795597a52..047ab10862 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -34,7 +34,7 @@ import FaqsSection from './faqs-section'; * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. * @param {() => void} props.onContinue Callback called once continue button is clicked. * @param {() => void} props.isLoading Callback called once continue button is clicked. - * @param {String} submitButtonText Text to display on . + * @param {string} props.submitButtonText Text to display on submit button. * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. */ export default function AdsCampaign( { From 340991995463d7466f76f92f44da2d5f2cdb6ded Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Sat, 7 Sep 2024 02:55:27 +0100 Subject: [PATCH 09/32] cs fixes --- js/src/setup-ads/ads-stepper/index.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 2ed1869041..51ce2720b5 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -87,9 +87,12 @@ const AdsStepper = ( { formProps } ) => { content: ( ), onClick: handleStepClick, From 0c74f98f95142e6e3e8f4a845dd83cc93424d296 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 9 Sep 2024 12:30:57 +0100 Subject: [PATCH 10/32] cs fixes --- js/src/components/paid-ads/ads-campaign.js | 3 ++- js/src/setup-ads/ads-stepper/index.js | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 047ab10862..73a25650b6 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -47,7 +47,8 @@ export default function AdsCampaign( { const isCreation = ! campaign; const formContext = useAdaptiveFormContext(); const { isValidForm } = formContext; - const continueButtonText = submitButtonText || __( 'Continue', 'google-listings-and-ads' ); + const continueButtonText = + submitButtonText || __( 'Continue', 'google-listings-and-ads' ); const disabledBudgetSection = ! formContext.values.countryCodes.length; const helperText = isCreation diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 51ce2720b5..a38adf0aeb 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -90,8 +90,8 @@ const AdsStepper = ( { formProps } ) => { onContinue={ formProps.handleSubmit } isLoading={ formProps.isSubmitting } submitButtonText={ __( - 'Launch paid campaign', - 'google-listings-and-ads' + 'Launch paid campaign', + 'google-listings-and-ads' ) } /> ), From 779eac399376799f352053715242bf01f45a2f6e Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Wed, 11 Sep 2024 17:13:08 +0100 Subject: [PATCH 11/32] sert default submitButtonText --- js/src/components/paid-ads/ads-campaign.js | 6 ++---- js/src/setup-ads/ads-stepper/index.js | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 73a25650b6..8d84b4b0cb 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -41,14 +41,12 @@ export default function AdsCampaign( { campaign, onContinue, isLoading, - submitButtonText, + submitButtonText = __( 'Continue', 'google-listings-and-ads' ), trackingContext, } ) { const isCreation = ! campaign; const formContext = useAdaptiveFormContext(); const { isValidForm } = formContext; - const continueButtonText = - submitButtonText || __( 'Continue', 'google-listings-and-ads' ); const disabledBudgetSection = ! formContext.values.countryCodes.length; const helperText = isCreation @@ -111,7 +109,7 @@ export default function AdsCampaign( { loading={ isLoading } onClick={ onContinue } > - { continueButtonText } + { submitButtonText } diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index a38adf0aeb..48291b2f68 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -21,8 +21,8 @@ import { /** * @param {Object} props React props * @param {Object} props.formProps Form props forwarded from `Form` component. - * @fires gla_setup_ads with `{ triggered_by: 'step1-continue-button' | 'step2-continue-button' , action: 'go-to-step2' | 'go-to-step3' }`. - * @fires gla_setup_ads with `{ triggered_by: 'stepper-step1-button' | 'stepper-step2-button', action: 'go-to-step1' | 'go-to-step2' }`. + * @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 = ( { formProps } ) => { const [ step, setStep ] = useState( '1' ); From ef31827103be534bd312aad873cc65f0d8ae9492 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Fri, 13 Sep 2024 11:29:00 +0100 Subject: [PATCH 12/32] update doc block and remove unused files --- js/src/components/paid-ads/ads-campaign.js | 2 +- .../setup-billing/billing-saved-card/index.js | 79 ----------------- .../billing-saved-card/index.scss | 17 ---- .../ads-stepper/setup-billing/index.js | 88 ------------------- 4 files changed, 1 insertion(+), 185 deletions(-) delete mode 100644 js/src/setup-ads/ads-stepper/setup-billing/billing-saved-card/index.js delete mode 100644 js/src/setup-ads/ads-stepper/setup-billing/billing-saved-card/index.scss delete mode 100644 js/src/setup-ads/ads-stepper/setup-billing/index.js diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 8d84b4b0cb..0e5d36ed0b 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -33,7 +33,7 @@ import FaqsSection from './faqs-section'; * @param {Object} props React props. * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. * @param {() => void} props.onContinue Callback called once continue button is clicked. - * @param {() => void} props.isLoading Callback called once continue button is clicked. + * @param {() => boolean} props.isLoading Callback called once continue button is clicked. * @param {string} props.submitButtonText Text to display on submit button. * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. */ diff --git a/js/src/setup-ads/ads-stepper/setup-billing/billing-saved-card/index.js b/js/src/setup-ads/ads-stepper/setup-billing/billing-saved-card/index.js deleted file mode 100644 index 89f2bf01f2..0000000000 --- a/js/src/setup-ads/ads-stepper/setup-billing/billing-saved-card/index.js +++ /dev/null @@ -1,79 +0,0 @@ -/** - * External dependencies - */ -import { __ } from '@wordpress/i18n'; -import GridiconCreditCard from 'gridicons/dist/credit-card'; -import { createInterpolateElement } from '@wordpress/element'; - -/** - * Internal dependencies - */ -import toAccountText from '.~/utils/toAccountText'; -import AppSpinner from '.~/components/app-spinner'; -import TitleButtonLayout from '.~/components/title-button-layout'; -import TrackableLink from '.~/components/trackable-link'; -import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; -import Section from '.~/wcdl/section'; -import './index.scss'; - -/** - * Clicking on a Google Ads account text link. - * - * @event gla_google_ads_account_link_click - * @property {string} context Indicates which page / module the link is in - * @property {string} href Where the user is redirected - * @property {string} link_id A unique ID for the link within the page / module - */ - -/** - * @fires gla_google_ads_account_link_click with `{ context: 'setup-ads', link_id: 'google-ads-account' }` - */ -const BillingSavedCard = () => { - const { googleAdsAccount } = useGoogleAdsAccount(); - - if ( ! googleAdsAccount ) { - return ; - } - - return ( -
- - -
- -
-
- -
- { createInterpolateElement( - __( - 'Great! You already have billing information saved for this Google Ads account.', - 'google-listings-and-ads' - ), - { - link: ( - - ), - } - ) } -
-
-
-
-
- ); -}; - -export default BillingSavedCard; diff --git a/js/src/setup-ads/ads-stepper/setup-billing/billing-saved-card/index.scss b/js/src/setup-ads/ads-stepper/setup-billing/billing-saved-card/index.scss deleted file mode 100644 index 642ce39455..0000000000 --- a/js/src/setup-ads/ads-stepper/setup-billing/billing-saved-card/index.scss +++ /dev/null @@ -1,17 +0,0 @@ -.gla-google-ads-billing-saved-card { - &__account-number { - margin-bottom: calc(var(--main-gap) / 2); - } - - &__description { - display: flex; - gap: calc(var(--main-gap) / 3); - align-items: center; - font-style: italic; - - svg { - fill: $alert-green; - flex: 0 0 auto; - } - } -} diff --git a/js/src/setup-ads/ads-stepper/setup-billing/index.js b/js/src/setup-ads/ads-stepper/setup-billing/index.js deleted file mode 100644 index 1dc6b234ed..0000000000 --- a/js/src/setup-ads/ads-stepper/setup-billing/index.js +++ /dev/null @@ -1,88 +0,0 @@ -/** - * External dependencies - */ -import { __ } from '@wordpress/i18n'; - -/** - * Internal dependencies - */ -import StepContent from '.~/components/stepper/step-content'; -import StepContentHeader from '.~/components/stepper/step-content-header'; -import AppSpinner from '.~/components/app-spinner'; -import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; -import Section from '.~/wcdl/section'; -import { - BillingSetupCard, - fallbackBillingUrl, -} from '.~/components/paid-ads/billing-card'; -import BillingSavedCard from './billing-saved-card'; -import StepContentFooter from '.~/components/stepper/step-content-footer'; -import AppButton from '.~/components/app-button'; -import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; - -const SetupBilling = ( props ) => { - const { - formProps: { isSubmitting, handleSubmit }, - } = props; - - const { billingStatus } = useGoogleAdsAccountBillingStatus(); - - if ( ! billingStatus ) { - return ; - } - - const isApproved = - billingStatus.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; - - return ( - - -
- { isApproved ? ( - - ) : ( - - ) } -
- { isApproved && ( - - - { __( - 'Launch paid campaign', - 'google-listings-and-ads' - ) } - - - ) } -
- ); -}; - -export default SetupBilling; From e7afe1ea4efc1919039ba48310b21ee0ea15aba3 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil <30643833+kt-12@users.noreply.github.com> Date: Fri, 13 Sep 2024 11:31:37 +0100 Subject: [PATCH 13/32] Update tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js change text Co-authored-by: Asvin Balloo --- tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 db3dbde322..2df83e0e9b 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 @@ -287,7 +287,7 @@ test.describe( 'Set up Ads account', () => { setupBudgetPage = new SetupBudgetPage( page ); } ); - test( 'Continue to create paid campaign and create ads', async () => { + test( 'Continue to create paid ad campaign', async () => { await setupAdsAccounts.clickContinue(); await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); From daa483a07711a663113888bf31a4f9300512362d Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Fri, 13 Sep 2024 11:58:20 +0100 Subject: [PATCH 14/32] fix e2e --- .../add-paid-campaigns.test.js | 22 +++++++++---------- .../e2e/utils/pages/setup-ads/setup-budget.js | 12 ++++++++++ 2 files changed, 23 insertions(+), 11 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 db3dbde322..e9e2929601 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 @@ -279,10 +279,6 @@ test.describe( 'Set up Ads account', () => { } ); test.describe( 'Create your paid campaign', () => { - const launchPaidCampaignButton = page.getByRole( 'button', { - name: 'Launch paid campaign', - } ); - test.beforeAll( async () => { setupBudgetPage = new SetupBudgetPage( page ); } ); @@ -305,7 +301,9 @@ test.describe( 'Set up Ads account', () => { page.getByRole( 'heading', { name: 'Set your budget' } ) ).toBeVisible(); - await expect( launchPaidCampaignButton ).toBeDisabled(); + await expect( + setupBudgetPage.getLaunchPaidCampaignButton() + ).toBeDisabled(); await expect( page.getByRole( 'link', { @@ -371,12 +369,16 @@ test.describe( 'Set up Ads account', () => { budget = '0'; await setupBudgetPage.fillBudget( budget ); - await expect( launchPaidCampaignButton ).toBeDisabled(); + await expect( + setupBudgetPage.getLaunchPaidCampaignButton() + ).toBeDisabled(); budget = '1'; await setupBudgetPage.fillBudget( budget ); - await expect( launchPaidCampaignButton ).toBeEnabled(); + await expect( + setupBudgetPage.getLaunchPaidCampaignButton() + ).toBeEnabled(); } ); test( 'Budget Recommendation', async () => { @@ -385,8 +387,8 @@ test.describe( 'Set up Ads account', () => { ).toBeVisible(); } ); - test( 'Paid Campaign can be created', async () => { - launchPaidCampaignButton.click(); + test( 'It should show the campaign creation success message', async () => { + setupBudgetPage.getLaunchPaidCampaignButton().click(); //It should redirect to the dashboard page await page.waitForURL( @@ -396,9 +398,7 @@ test.describe( 'Set up Ads account', () => { waitUntil: LOAD_STATE.DOM_CONTENT_LOADED, } ); - } ); - test( 'It should show the campaign creation success message', async () => { await expect( page.getByRole( 'heading', { name: "You've set up a paid Performance Max Campaign!", diff --git a/tests/e2e/utils/pages/setup-ads/setup-budget.js b/tests/e2e/utils/pages/setup-ads/setup-budget.js index dca6843f64..19d101ef3e 100644 --- a/tests/e2e/utils/pages/setup-ads/setup-budget.js +++ b/tests/e2e/utils/pages/setup-ads/setup-budget.js @@ -84,6 +84,18 @@ export default class SetupBudget extends MockRequests { ); } + /** + * Get the Launch paid cmpaign button. + * + * @return {import('@playwright/test').Locator} Launch paid cmpaign button. + */ + getLaunchPaidCampaignButton() { + return this.page.getByRole( 'button', { + name: 'Launch paid campaign', + exact: true, + } ); + } + /** * Extract budget recommendation value. * From b5a24671f21cb3d51589a53a7336ca8d10c7ea0f Mon Sep 17 00:00:00 2001 From: Karthik Thayyil <30643833+kt-12@users.noreply.github.com> Date: Fri, 13 Sep 2024 20:40:19 +0100 Subject: [PATCH 15/32] Update js/src/components/paid-ads/ads-campaign.js Co-authored-by: Asvin Balloo --- js/src/components/paid-ads/ads-campaign.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index 0e5d36ed0b..fb45848c95 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -33,7 +33,7 @@ import FaqsSection from './faqs-section'; * @param {Object} props React props. * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. * @param {() => void} props.onContinue Callback called once continue button is clicked. - * @param {() => boolean} props.isLoading Callback called once continue button is clicked. + * @param {boolean} [props.isLoading] If true, the Continue button will display a loading spinner . * @param {string} props.submitButtonText Text to display on submit button. * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. */ From 0ca45f13ce02b07040f6450f63c96556ac16c11a Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Tue, 17 Sep 2024 08:33:08 +0100 Subject: [PATCH 16/32] clear jest files --- js/src/setup-ads/ads-stepper/index.test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.test.js b/js/src/setup-ads/ads-stepper/index.test.js index ff712009df..acd0293ca0 100644 --- a/js/src/setup-ads/ads-stepper/index.test.js +++ b/js/src/setup-ads/ads-stepper/index.test.js @@ -8,7 +8,6 @@ jest.mock( './setup-accounts', () => jest.fn().mockName( 'SetupAccounts' ) ); jest.mock( '.~/components/paid-ads/ads-campaign', () => jest.fn().mockName( 'AdsCampaign' ) ); -jest.mock( './setup-billing', () => jest.fn().mockName( 'SetupBilling' ) ); /** * External dependencies @@ -23,7 +22,6 @@ import { recordEvent } from '@woocommerce/tracks'; import AdsStepper from './'; import SetupAccounts from './setup-accounts'; import AdsCampaign from '.~/components/paid-ads/ads-campaign'; -import SetupBilling from './setup-billing'; describe( 'AdsStepper', () => { let continueToStep2; @@ -39,8 +37,6 @@ describe( 'AdsStepper', () => { continueToStep3 = onContinue; return null; } ); - - SetupBilling.mockReturnValue( null ); } ); afterEach( () => { From 98cd72dfeee3e2e0afda59b9b6f1bcf440235cae Mon Sep 17 00:00:00 2001 From: Karthik Thayyil <30643833+kt-12@users.noreply.github.com> Date: Tue, 17 Sep 2024 16:54:58 +0100 Subject: [PATCH 17/32] Update js/src/components/paid-ads/ads-campaign.js Co-authored-by: Asvin Balloo --- js/src/components/paid-ads/ads-campaign.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index fb45848c95..baf88cf262 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -34,7 +34,7 @@ import FaqsSection from './faqs-section'; * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. * @param {() => void} props.onContinue Callback called once continue button is clicked. * @param {boolean} [props.isLoading] If true, the Continue button will display a loading spinner . - * @param {string} props.submitButtonText Text to display on submit button. + * @param {string} [props.submitButtonText] Text to display on submit button. * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. */ export default function AdsCampaign( { From 52ca19d8c7a00034f2893a16436d69c38ed7c5ef Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 19 Sep 2024 09:08:40 +0100 Subject: [PATCH 18/32] conditional enabling continue button --- .wp-env.override.json | 3 +++ js/src/components/paid-ads/ads-campaign.js | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 .wp-env.override.json diff --git a/.wp-env.override.json b/.wp-env.override.json new file mode 100644 index 0000000000..9244c59a2c --- /dev/null +++ b/.wp-env.override.json @@ -0,0 +1,3 @@ +{ + "plugins": [ "../woocommerce/plugins/woocommerce", "." ] +} diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index baf88cf262..dfe3d944d5 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -13,10 +13,13 @@ import StepContentFooter from '.~/components/stepper/step-content-footer'; import AppDocumentationLink from '.~/components/app-documentation-link'; import AppButton from '.~/components/app-button'; import { useAdaptiveFormContext } from '.~/components/adaptive-form'; +import AppSpinner from '.~/components/app-spinner'; +import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; import AudienceSection from './audience-section'; import BudgetSection from './budget-section'; import { CampaignPreviewCard } from './campaign-preview'; import FaqsSection from './faqs-section'; +import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; /** * @typedef {import('.~/data/actions').Campaign} Campaign @@ -47,6 +50,17 @@ export default function AdsCampaign( { const isCreation = ! campaign; const formContext = useAdaptiveFormContext(); const { isValidForm } = formContext; + const { billingStatus } = useGoogleAdsAccountBillingStatus(); + let enableContinue = isValidForm; + + if ( trackingContext === 'setup-ads' ) { + if ( ! billingStatus ) { + return ; + } + const isApproved = + billingStatus.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; + enableContinue = enableContinue && isApproved; + } const disabledBudgetSection = ! formContext.values.countryCodes.length; const helperText = isCreation @@ -105,7 +119,7 @@ export default function AdsCampaign( { From b8178b4140f7390191fab2993ebe72d57e79e49a Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 19 Sep 2024 11:58:05 +0100 Subject: [PATCH 19/32] remove file --- .wp-env.override.json | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .wp-env.override.json diff --git a/.wp-env.override.json b/.wp-env.override.json deleted file mode 100644 index 9244c59a2c..0000000000 --- a/.wp-env.override.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "plugins": [ "../woocommerce/plugins/woocommerce", "." ] -} From c5be7e093eb9b8c9a242098a457a60a836f78a8d Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 19 Sep 2024 12:33:35 +0100 Subject: [PATCH 20/32] update disabled button logic --- js/src/components/paid-ads/ads-campaign.js | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index dfe3d944d5..bef2dd9195 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -13,7 +13,6 @@ import StepContentFooter from '.~/components/stepper/step-content-footer'; import AppDocumentationLink from '.~/components/app-documentation-link'; import AppButton from '.~/components/app-button'; import { useAdaptiveFormContext } from '.~/components/adaptive-form'; -import AppSpinner from '.~/components/app-spinner'; import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; import AudienceSection from './audience-section'; import BudgetSection from './budget-section'; @@ -51,16 +50,6 @@ export default function AdsCampaign( { const formContext = useAdaptiveFormContext(); const { isValidForm } = formContext; const { billingStatus } = useGoogleAdsAccountBillingStatus(); - let enableContinue = isValidForm; - - if ( trackingContext === 'setup-ads' ) { - if ( ! billingStatus ) { - return ; - } - const isApproved = - billingStatus.status === GOOGLE_ADS_BILLING_STATUS.APPROVED; - enableContinue = enableContinue && isApproved; - } const disabledBudgetSection = ! formContext.values.countryCodes.length; const helperText = isCreation @@ -119,7 +108,11 @@ export default function AdsCampaign( { From 1833363260aea805ca6380cf3af501a5ef48d673 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 19 Sep 2024 12:36:48 +0100 Subject: [PATCH 21/32] only to check in setup-ads context --- js/src/components/paid-ads/ads-campaign.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign.js index bef2dd9195..f4acbfc3f6 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign.js @@ -110,8 +110,9 @@ export default function AdsCampaign( { isPrimary disabled={ ! isValidForm || - billingStatus?.status !== - GOOGLE_ADS_BILLING_STATUS.APPROVED + ( trackingContext === 'setup-ads' && + billingStatus?.status !== + GOOGLE_ADS_BILLING_STATUS.APPROVED ) } loading={ isLoading } onClick={ onContinue } From d053b77f5c582e047adcfd07b7717964a3bcc9c4 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 26 Sep 2024 05:31:26 +0100 Subject: [PATCH 22/32] fix billing e2e --- .../e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 a1d598c5ff..dced1b69c1 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 @@ -281,6 +281,9 @@ test.describe( 'Set up Ads account', () => { test.describe( 'Create your paid campaign', () => { test.beforeAll( async () => { setupBudgetPage = new SetupBudgetPage( page ); + await setupBudgetPage.fulfillBillingStatusRequest( { + status: 'approved', + } ); } ); test( 'Continue to create paid ad campaign', async () => { @@ -388,7 +391,7 @@ test.describe( 'Set up Ads account', () => { } ); test( 'It should show the campaign creation success message', async () => { - setupBudgetPage.getLaunchPaidCampaignButton().click(); + await setupBudgetPage.getLaunchPaidCampaignButton().click(); //It should redirect to the dashboard page await page.waitForURL( From c8ee77f1da57d7216302249aad1896413a13ae15 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 26 Sep 2024 16:53:59 +0100 Subject: [PATCH 23/32] ads campaign request --- .../add-paid-campaigns/add-paid-campaigns.test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 dced1b69c1..814acbf346 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 @@ -391,6 +391,20 @@ test.describe( 'Set up Ads account', () => { } ); test( 'It should show the campaign creation success message', async () => { + await setupBudgetPage.fulfillAdsCampaignsRequest( + { + id: 111111111, + name: 'Test Campaign', + status: 'enabled', + type: 'performance_max', + amount: budget, + country: 'US', + targeted_locations: {}, + }, + 200, + [ 'POST' ] + ); + await setupBudgetPage.getLaunchPaidCampaignButton().click(); //It should redirect to the dashboard page From 1b31ca9eff9d6982e22373c489eb560f64173e84 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Fri, 27 Sep 2024 13:45:32 +0100 Subject: [PATCH 24/32] change mock --- .../add-paid-campaigns.test.js | 17 ++++------------- 1 file changed, 4 insertions(+), 13 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 814acbf346..bea67ae048 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 @@ -66,6 +66,7 @@ test.describe( 'Set up Ads account', () => { page = await browser.newPage(); dashboardPage = new DashboardPage( page ); setupAdsAccounts = new SetupAdsAccountsPage( page ); + setupBudgetPage = new SetupBudgetPage( page ); await setOnboardedMerchant(); await setupAdsAccounts.mockAdsAccountsResponse( [] ); await dashboardPage.mockRequests(); @@ -280,7 +281,6 @@ test.describe( 'Set up Ads account', () => { test.describe( 'Create your paid campaign', () => { test.beforeAll( async () => { - setupBudgetPage = new SetupBudgetPage( page ); await setupBudgetPage.fulfillBillingStatusRequest( { status: 'approved', } ); @@ -391,18 +391,9 @@ test.describe( 'Set up Ads account', () => { } ); test( 'It should show the campaign creation success message', async () => { - await setupBudgetPage.fulfillAdsCampaignsRequest( - { - id: 111111111, - name: 'Test Campaign', - status: 'enabled', - type: 'performance_max', - amount: budget, - country: 'US', - targeted_locations: {}, - }, - 200, - [ 'POST' ] + await setupBudgetPage.mockCampaignCreationAndAdsSetupCompletion( + budget, + {} ); await setupBudgetPage.getLaunchPaidCampaignButton().click(); From e7da33a347653f671a4b0eb00d33ed0a11e17cf9 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Fri, 27 Sep 2024 16:00:39 +0100 Subject: [PATCH 25/32] move billing page to bottom --- tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 bea67ae048..a45b61132a 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 @@ -66,7 +66,6 @@ test.describe( 'Set up Ads account', () => { page = await browser.newPage(); dashboardPage = new DashboardPage( page ); setupAdsAccounts = new SetupAdsAccountsPage( page ); - setupBudgetPage = new SetupBudgetPage( page ); await setOnboardedMerchant(); await setupAdsAccounts.mockAdsAccountsResponse( [] ); await dashboardPage.mockRequests(); @@ -281,6 +280,9 @@ test.describe( 'Set up Ads account', () => { test.describe( 'Create your paid campaign', () => { test.beforeAll( async () => { + setupBudgetPage = new SetupBudgetPage( page ); + + // Mock billing approved status. await setupBudgetPage.fulfillBillingStatusRequest( { status: 'approved', } ); From a6d79a0cc4b2c0c7f4505c49130d4ad1dfb1d9e5 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Fri, 27 Sep 2024 21:57:17 +0100 Subject: [PATCH 26/32] create a new mock for campaign creation. --- .../add-paid-campaigns.test.js | 6 ++---- tests/e2e/utils/pages/setup-ads/setup-budget.js | 16 +++++++++++++--- 2 files changed, 15 insertions(+), 7 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 bea67ae048..33c394be22 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 @@ -391,10 +391,8 @@ test.describe( 'Set up Ads account', () => { } ); test( 'It should show the campaign creation success message', async () => { - await setupBudgetPage.mockCampaignCreationAndAdsSetupCompletion( - budget, - {} - ); + // Mock the campaign creation request. + await setupBudgetPage.mockCampaignCreation( budget, {} ); await setupBudgetPage.getLaunchPaidCampaignButton().click(); diff --git a/tests/e2e/utils/pages/setup-ads/setup-budget.js b/tests/e2e/utils/pages/setup-ads/setup-budget.js index 19d101ef3e..58555d5e99 100644 --- a/tests/e2e/utils/pages/setup-ads/setup-budget.js +++ b/tests/e2e/utils/pages/setup-ads/setup-budget.js @@ -200,13 +200,13 @@ export default class SetupBudget extends MockRequests { } /** - * Mock the campaign creation process and the Ads setup completion. + * Mock the campaign creation process. * - * @param {string} budget The campaign budget. + * @param {string} budget The campaign budget. * @param {Array} targetLocations The targeted locations. * @return {Promise} */ - async mockCampaignCreationAndAdsSetupCompletion( budget, targetLocations ) { + async mockCampaignCreation( budget, targetLocations ) { //This step is necessary; otherwise, it will set the ADS_SETUP_COMPLETED_AT option in the database, which could potentially impact other tests. await this.fulfillRequest( /\/wc\/gla\/ads\/setup\/complete\b/, @@ -228,7 +228,17 @@ export default class SetupBudget extends MockRequests { 200, [ 'POST' ] ); + } + /** + * Mock the campaign creation process and the Ads setup completion. + * + * @param {string} budget The campaign budget. + * @param {Array} targetLocations The targeted locations. + * @return {Promise} + */ + async mockCampaignCreationAndAdsSetupCompletion( budget, targetLocations ) { + await this.mockCampaignCreation( budget, targetLocations ); await this.awaitForCampaignCreationRequest( budget, targetLocations ); } } From 4ff5165e260cec2ffaf212b3a445b70eabaf1e10 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Fri, 27 Sep 2024 22:04:29 +0100 Subject: [PATCH 27/32] move budget to top. --- .../e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 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 d0b731601d..671d5427b6 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 @@ -66,6 +66,7 @@ test.describe( 'Set up Ads account', () => { page = await browser.newPage(); dashboardPage = new DashboardPage( page ); setupAdsAccounts = new SetupAdsAccountsPage( page ); + setupBudgetPage = new SetupBudgetPage( page ); await setOnboardedMerchant(); await setupAdsAccounts.mockAdsAccountsResponse( [] ); await dashboardPage.mockRequests(); @@ -280,9 +281,7 @@ test.describe( 'Set up Ads account', () => { test.describe( 'Create your paid campaign', () => { test.beforeAll( async () => { - setupBudgetPage = new SetupBudgetPage( page ); - - // Mock billing approved status. + // `Launch Paid Campaign` button is only enabled when billing status is approved. await setupBudgetPage.fulfillBillingStatusRequest( { status: 'approved', } ); From b90f77cb7e41bf114d44224336f68766c1699ef8 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 30 Sep 2024 18:14:20 +0100 Subject: [PATCH 28/32] e2e cr fixes --- tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js | 3 +-- tests/e2e/utils/pages/setup-ads/setup-budget.js | 4 ++-- 2 files changed, 3 insertions(+), 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 671d5427b6..3f2ce38834 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 @@ -281,7 +281,6 @@ test.describe( 'Set up Ads account', () => { test.describe( 'Create your paid campaign', () => { test.beforeAll( async () => { - // `Launch Paid Campaign` button is only enabled when billing status is approved. await setupBudgetPage.fulfillBillingStatusRequest( { status: 'approved', } ); @@ -393,7 +392,7 @@ test.describe( 'Set up Ads account', () => { test( 'It should show the campaign creation success message', async () => { // Mock the campaign creation request. - await setupBudgetPage.mockCampaignCreation( budget, {} ); + await setupBudgetPage.mockCampaignCreation( budget, [] ); await setupBudgetPage.getLaunchPaidCampaignButton().click(); diff --git a/tests/e2e/utils/pages/setup-ads/setup-budget.js b/tests/e2e/utils/pages/setup-ads/setup-budget.js index 58555d5e99..7ed852f05e 100644 --- a/tests/e2e/utils/pages/setup-ads/setup-budget.js +++ b/tests/e2e/utils/pages/setup-ads/setup-budget.js @@ -85,9 +85,9 @@ export default class SetupBudget extends MockRequests { } /** - * Get the Launch paid cmpaign button. + * Get the Launch paid campaign button. * - * @return {import('@playwright/test').Locator} Launch paid cmpaign button. + * @return {import('@playwright/test').Locator} Launch paid campaign button. */ getLaunchPaidCampaignButton() { return this.page.getByRole( 'button', { From 0c6d250939d9da3dbf94ce2df568932ff5637b15 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 30 Sep 2024 18:23:58 +0100 Subject: [PATCH 29/32] remove title button component --- .../components/title-button-layout/index.js | 19 ------------------- .../components/title-button-layout/index.scss | 5 ----- 2 files changed, 24 deletions(-) delete mode 100644 js/src/components/title-button-layout/index.js delete mode 100644 js/src/components/title-button-layout/index.scss diff --git a/js/src/components/title-button-layout/index.js b/js/src/components/title-button-layout/index.js deleted file mode 100644 index 8c71608853..0000000000 --- a/js/src/components/title-button-layout/index.js +++ /dev/null @@ -1,19 +0,0 @@ -/** - * Internal dependencies - */ -import Subsection from '.~/wcdl/subsection'; -import ContentButtonLayout from '../content-button-layout'; -import './index.scss'; - -const TitleButtonLayout = ( props ) => { - const { title, button } = props; - - return ( - - { title } - { button } - - ); -}; - -export default TitleButtonLayout; diff --git a/js/src/components/title-button-layout/index.scss b/js/src/components/title-button-layout/index.scss deleted file mode 100644 index 88dfb1284d..0000000000 --- a/js/src/components/title-button-layout/index.scss +++ /dev/null @@ -1,5 +0,0 @@ -.gla-title-button-layout { - .title { - margin-bottom: 0; - } -} From 104b3681d2b92e8776b7598cf43ef861bbf98ec2 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Tue, 1 Oct 2024 23:27:14 +0100 Subject: [PATCH 30/32] remove accidently functions --- .../e2e/utils/pages/setup-ads/setup-budget.js | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/tests/e2e/utils/pages/setup-ads/setup-budget.js b/tests/e2e/utils/pages/setup-ads/setup-budget.js index 513bd64cd7..80d1517c53 100644 --- a/tests/e2e/utils/pages/setup-ads/setup-budget.js +++ b/tests/e2e/utils/pages/setup-ads/setup-budget.js @@ -87,36 +87,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. * From 2c8485132e99bc9daef2e8b22e8c8c8296acfe05 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Wed, 2 Oct 2024 08:42:13 +0100 Subject: [PATCH 31/32] remove newly introduced function --- .../add-paid-campaigns.test.js | 8 +++++++- tests/e2e/utils/pages/setup-ads/setup-budget.js | 16 +++------------- 2 files changed, 10 insertions(+), 14 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 3f2ce38834..aa8218b58e 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 @@ -392,10 +392,16 @@ test.describe( 'Set up Ads account', () => { test( 'It should show the campaign creation success message', async () => { // Mock the campaign creation request. - await setupBudgetPage.mockCampaignCreation( budget, [] ); + const campaignCreation = + setupBudgetPage.mockCampaignCreationAndAdsSetupCompletion( + '1', + [ 'US' ] + ); await setupBudgetPage.getLaunchPaidCampaignButton().click(); + await campaignCreation; + //It should redirect to the dashboard page await page.waitForURL( '/wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fdashboard&guide=campaign-creation-success', diff --git a/tests/e2e/utils/pages/setup-ads/setup-budget.js b/tests/e2e/utils/pages/setup-ads/setup-budget.js index 80d1517c53..bf9bab0b71 100644 --- a/tests/e2e/utils/pages/setup-ads/setup-budget.js +++ b/tests/e2e/utils/pages/setup-ads/setup-budget.js @@ -161,13 +161,13 @@ export default class SetupBudget extends MockRequests { } /** - * Mock the campaign creation process. + * Mock the campaign creation process and the Ads setup completion. * - * @param {string} budget The campaign budget. + * @param {string} budget The campaign budget. * @param {Array} targetLocations The targeted locations. * @return {Promise} */ - async mockCampaignCreation( budget, targetLocations ) { + async mockCampaignCreationAndAdsSetupCompletion( budget, targetLocations ) { //This step is necessary; otherwise, it will set the ADS_SETUP_COMPLETED_AT option in the database, which could potentially impact other tests. await this.fulfillRequest( /\/wc\/gla\/ads\/setup\/complete\b/, @@ -189,17 +189,7 @@ export default class SetupBudget extends MockRequests { 200, [ 'POST' ] ); - } - /** - * Mock the campaign creation process and the Ads setup completion. - * - * @param {string} budget The campaign budget. - * @param {Array} targetLocations The targeted locations. - * @return {Promise} - */ - async mockCampaignCreationAndAdsSetupCompletion( budget, targetLocations ) { - await this.mockCampaignCreation( budget, targetLocations ); await this.awaitForCampaignCreationRequest( budget, targetLocations ); } } From fe7bab660d13bab427fa28a3b37e7f51569d5865 Mon Sep 17 00:00:00 2001 From: asvinb Date: Wed, 2 Oct 2024 15:39:45 +0400 Subject: [PATCH 32/32] Add ContinueButton component. --- .../{ => ads-campaign}/ads-campaign.js | 38 +++++--------- .../paid-ads/ads-campaign/continue-button.js | 51 +++++++++++++++++++ .../components/paid-ads/ads-campaign/index.js | 1 + js/src/setup-ads/ads-stepper/index.js | 4 -- 4 files changed, 64 insertions(+), 30 deletions(-) rename js/src/components/paid-ads/{ => ads-campaign}/ads-campaign.js (78%) create mode 100644 js/src/components/paid-ads/ads-campaign/continue-button.js create mode 100644 js/src/components/paid-ads/ads-campaign/index.js diff --git a/js/src/components/paid-ads/ads-campaign.js b/js/src/components/paid-ads/ads-campaign/ads-campaign.js similarity index 78% rename from js/src/components/paid-ads/ads-campaign.js rename to js/src/components/paid-ads/ads-campaign/ads-campaign.js index 3b3a2ad82f..abd9920aed 100644 --- a/js/src/components/paid-ads/ads-campaign.js +++ b/js/src/components/paid-ads/ads-campaign/ads-campaign.js @@ -12,14 +12,12 @@ import StepContentHeader from '.~/components/stepper/step-content-header'; import StepContentFooter from '.~/components/stepper/step-content-footer'; import StepContentActions from '.~/components/stepper/step-content-actions'; import AppDocumentationLink from '.~/components/app-documentation-link'; -import AppButton from '.~/components/app-button'; import { useAdaptiveFormContext } from '.~/components/adaptive-form'; -import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; -import AudienceSection from './audience-section'; -import BudgetSection from './budget-section'; -import { CampaignPreviewCard } from './campaign-preview'; -import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; -import PaidAdsFaqsPanel from './faqs-panel'; +import AudienceSection from '../audience-section'; +import BudgetSection from '../budget-section'; +import { CampaignPreviewCard } from '../campaign-preview'; +import PaidAdsFaqsPanel from '../faqs-panel'; +import ContinueButton from './continue-button'; /** * @typedef {import('.~/data/actions').Campaign} Campaign @@ -35,22 +33,18 @@ import PaidAdsFaqsPanel from './faqs-panel'; * * @param {Object} props React props. * @param {Campaign} [props.campaign] Campaign data to be edited. If not provided, this component will show campaign creation UI. - * @param {() => void} props.onContinue Callback called once continue button is clicked. * @param {boolean} [props.isLoading] If true, the Continue button will display a loading spinner . - * @param {string} [props.submitButtonText] Text to display on submit button. + * @param {() => void} props.onContinue Callback called once continue button is clicked. * @param {'create-ads'|'edit-ads'|'setup-ads'} props.trackingContext A context indicating which page this component is used on. This will be the value of `context` in the track event properties. */ export default function AdsCampaign( { campaign, - onContinue, isLoading, - submitButtonText = __( 'Continue', 'google-listings-and-ads' ), + onContinue, trackingContext, } ) { const isCreation = ! campaign; const formContext = useAdaptiveFormContext(); - const { isValidForm } = formContext; - const { billingStatus } = useGoogleAdsAccountBillingStatus(); const disabledBudgetSection = ! formContext.values.countryCodes.length; const helperText = isCreation @@ -109,19 +103,11 @@ export default function AdsCampaign( { - - { submitButtonText } - + diff --git a/js/src/components/paid-ads/ads-campaign/continue-button.js b/js/src/components/paid-ads/ads-campaign/continue-button.js new file mode 100644 index 0000000000..8cd25fd71a --- /dev/null +++ b/js/src/components/paid-ads/ads-campaign/continue-button.js @@ -0,0 +1,51 @@ +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import AppButton from '.~/components/app-button'; +import useGoogleAdsAccountBillingStatus from '.~/hooks/useGoogleAdsAccountBillingStatus'; +import { GOOGLE_ADS_BILLING_STATUS } from '.~/constants'; +import { useAdaptiveFormContext } from '.~/components/adaptive-form'; + +/** + * Renders the button to either continue through the Stepper or launch a paid campaign. + * + * @param {Object} props React props. + * @param {boolean} props.setupAdsFlow Whether we are in the setup ads flow. + * @param {() => void} props.onContinue Callback called once continue button is clicked. + * @param {boolean} [props.isLoading] If true, the Continue button will display a loading spinner . + */ +const ContinueButton = ( { setupAdsFlow, onContinue, isLoading } ) => { + const { billingStatus } = useGoogleAdsAccountBillingStatus(); + const { isValidForm } = useAdaptiveFormContext(); + + let submitButtonText = __( 'Continue', 'google-listings-and-ads' ); + if ( setupAdsFlow ) { + submitButtonText = __( + 'Launch paid campaign', + 'google-listings-and-ads' + ); + } + + const isDisabled = + ! isValidForm || + ( setupAdsFlow && + billingStatus?.status !== GOOGLE_ADS_BILLING_STATUS.APPROVED ); + + return ( + + { submitButtonText } + + ); +}; + +export default ContinueButton; diff --git a/js/src/components/paid-ads/ads-campaign/index.js b/js/src/components/paid-ads/ads-campaign/index.js new file mode 100644 index 0000000000..19cf67b1b5 --- /dev/null +++ b/js/src/components/paid-ads/ads-campaign/index.js @@ -0,0 +1 @@ +export { default } from './ads-campaign'; diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 48291b2f68..c42befa480 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -89,10 +89,6 @@ const AdsStepper = ( { formProps } ) => { trackingContext="setup-ads" onContinue={ formProps.handleSubmit } isLoading={ formProps.isSubmitting } - submitButtonText={ __( - 'Launch paid campaign', - 'google-listings-and-ads' - ) } /> ), onClick: handleStepClick,