From 25801ac591f7e0d52c7e4f843112ba0cb746e9ac Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 9 Sep 2024 09:10:02 +0100 Subject: [PATCH 01/28] remove accounts setup page. --- js/src/setup-ads/ads-stepper/index.js | 22 +- .../add-paid-campaigns.test.js | 195 +----------------- 2 files changed, 6 insertions(+), 211 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 3979e0dcba..a1545871c6 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -8,7 +8,6 @@ import { useState } from '@wordpress/element'; /** * Internal dependencies */ -import SetupAccounts from './setup-accounts'; import AdsCampaign from '.~/components/paid-ads/ads-campaign'; import SetupBilling from './setup-billing'; import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter'; @@ -54,12 +53,8 @@ const AdsStepper = ( { formProps } ) => { setStep( to ); }; - const handleSetupAccountsContinue = () => { - continueStep( '2' ); - }; - const handleCreateCampaignContinue = () => { - continueStep( '3' ); + continueStep( '2' ); }; return ( @@ -72,19 +67,6 @@ const AdsStepper = ( { formProps } ) => { steps={ [ { key: '1', - label: __( - 'Set up your accounts', - 'google-listings-and-ads' - ), - content: ( - - ), - onClick: handleStepClick, - }, - { - key: '2', label: __( 'Create your paid campaign', 'google-listings-and-ads' @@ -98,7 +80,7 @@ const AdsStepper = ( { formProps } ) => { onClick: handleStepClick, }, { - key: '3', + key: '2', label: __( 'Set up billing', 'google-listings-and-ads' ), content: , onClick: handleStepClick, 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..32c7096836 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 @@ -7,7 +7,6 @@ import { expect, test } from '@playwright/test'; */ import { clearOnboardedMerchant, setOnboardedMerchant } from '../../utils/api'; import DashboardPage from '../../utils/pages/dashboard.js'; -import SetupAdsAccountsPage from '../../utils/pages/setup-ads/setup-ads-accounts.js'; import SetupBudgetPage from '../../utils/pages/setup-ads/setup-budget'; import { LOAD_STATE } from '../../utils/constants'; import { @@ -59,16 +58,14 @@ let setupBudgetPage = null; */ let page = null; -test.describe( 'Set up Ads account', () => { +test.describe( 'Set up paid campaign', () => { // The campaign budget let budget = null; test.beforeAll( async ( { browser } ) => { page = await browser.newPage(); dashboardPage = new DashboardPage( page ); - setupAdsAccounts = new SetupAdsAccountsPage( page ); await setOnboardedMerchant(); - await setupAdsAccounts.mockAdsAccountsResponse( [] ); await dashboardPage.mockRequests(); await dashboardPage.goto(); } ); @@ -91,203 +88,19 @@ test.describe( 'Set up Ads account', () => { test.describe( 'Set up your accounts page', async () => { test.beforeAll( async () => { - await setupAdsAccounts.mockAdsAccountsResponse( [] ); - await adsConnectionButton.click(); - await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); - } ); - - test( 'Page header should be "Set up your accounts"', async () => { - await expect( - page.getByRole( 'heading', { name: 'Set up your accounts' } ) - ).toBeVisible(); - await expect( - page.getByText( - 'Connect your Google account and your Google Ads account to set up a paid Performance Max campaign.' - ) - ).toBeVisible(); - } ); - - test( 'Google Account should show as connected', async () => { - await expect( - page.getByText( - 'This Google account is connected to your store’s product feed.' - ) - ).toBeVisible(); - } ); - - test( 'Continue Button should be disabled', async () => { - await expect( setupAdsAccounts.getContinueButton() ).toBeDisabled(); - } ); - } ); - - test.describe( 'Add paid campaigns with no Ads account', async () => { - test( 'Create an account should be visible', async () => { - const createAccountButton = page.getByRole( 'button', { - name: 'Create account', - } ); - - await expect( createAccountButton ).toBeVisible(); - - await expect( setupAdsAccounts.getContinueButton() ).toBeDisabled(); - - await expect( - page.getByText( - 'Required to set up conversion measurement and create campaigns.' - ) - ).toBeVisible(); - - await createAccountButton.click(); - } ); - - test( 'Create account button should be disable if the ToS have not been accepted.', async () => { - await expect( - page.getByRole( 'heading', { - name: 'Create Google Ads Account', - } ) - ).toBeVisible(); - - await expect( - page.getByText( - 'By creating a Google Ads account, you agree to the following terms and conditions:' - ) - ).toBeVisible(); - - await expect( - setupAdsAccounts.getCreateAdsAccountButtonModal() - ).toBeDisabled(); - } ); - - test( 'Accept terms and conditions to enable the create account button', async () => { - await setupAdsAccounts.getAcceptTermCreateAccount().check(); - - await expect( - setupAdsAccounts.getCreateAdsAccountButtonModal() - ).toBeEnabled(); - } ); - test( 'Create an Ads account', async () => { - // Intercept Ads connection request. - const connectAdsAccountRequest = - setupAdsAccounts.registerConnectAdsAccountRequests(); - - await setupAdsAccounts.mockAdsAccountsResponse( ADS_ACCOUNTS ); - - // Mock request to fulfill Ads connection. - await setupAdsAccounts.fulfillAdsConnection( { - id: ADS_ACCOUNTS[ 0 ].id, - currency: 'USD', - symbol: '$', - status: 'incomplete', - step: 'account_access', - } ); - - await setupAdsAccounts.mockAdsStatusNotClaimed(); - - await setupAdsAccounts.getCreateAdsAccountButtonModal().click(); - - await connectAdsAccountRequest; - - const modal = setupAdsAccounts.getAcceptAccountModal(); - await expect( modal ).toBeVisible(); - } ); - - test( 'Show Unclaimed Ads account', async () => { - await setupAdsAccounts.clickCloseAcceptAccountButtonFromModal(); - - const claimButton = setupAdsAccounts.getAdsClaimAccountButton(); - const claimText = setupAdsAccounts.getAdsClaimAccountText(); - - await expect( claimButton ).toBeVisible(); - await expect( claimText ).toBeVisible(); - - await expect( setupAdsAccounts.getContinueButton() ).toBeDisabled(); - } ); - - test( 'Show Claimed Ads account', async () => { - // Intercept Ads connection request. - await setupAdsAccounts.fulfillAdsConnection( { - id: ADS_ACCOUNTS[ 0 ].id, - currency: 'USD', - symbol: '$', - status: 'connected', - step: '', - } ); - - await setupAdsAccounts.mockAdsStatusClaimed(); - - await page.reload(); - - await expect( setupAdsAccounts.getContinueButton() ).toBeEnabled(); - - await expect( - page.getByRole( 'link', { - name: `Account ${ ADS_ACCOUNTS[ 0 ].id }`, - } ) - ).toBeVisible(); - - await expect( setupAdsAccounts.getContinueButton() ).toBeEnabled(); - } ); - } ); - - test.describe( 'Add paid campaigns with existing Ads accounts', () => { - test.beforeAll( async () => { - await setupAdsAccounts.mockAdsAccountsResponse( ADS_ACCOUNTS ); - //Disconnect the account from the previous test - setupAdsAccounts.fulfillAdsConnection( { - id: ADS_ACCOUNTS[ 1 ].id, - currency: 'EUR', - symbol: '\u20ac', - status: 'disconnected', - } ); - - await page.reload(); } ); - test( 'Select one existing account', async () => { - const adsAccountSelected = `${ ADS_ACCOUNTS[ 1 ].id }`; - - await expect( - setupAdsAccounts.getConnectAdsButton() - ).toBeDisabled(); - - await setupAdsAccounts.selectAnExistingAdsAccount( - adsAccountSelected - ); - - await expect( - setupAdsAccounts.getConnectAdsButton() - ).toBeEnabled(); - - //Intercept Ads connection request - const connectAdsAccountRequest = - setupAdsAccounts.registerConnectAdsAccountRequests( - adsAccountSelected - ); - - //Mock request to fulfill Ads connection - setupAdsAccounts.fulfillAdsConnection( { - id: ADS_ACCOUNTS[ 1 ].id, - currency: 'EUR', - symbol: '\u20ac', - status: 'connected', - } ); - - await setupAdsAccounts.clickConnectAds(); - await connectAdsAccountRequest; - - await expect( setupAdsAccounts.getContinueButton() ).toBeEnabled(); - } ); } ); test.describe( 'Create your paid campaign', () => { test.beforeAll( async () => { setupBudgetPage = new SetupBudgetPage( page ); - } ); - - test( 'Continue to create paid campaign', async () => { - await setupAdsAccounts.clickContinue(); + await adsConnectionButton.click(); await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); + } ); + test( 'Create paid campaign page', async () => { await expect( page.getByRole( 'heading', { name: 'Create your paid campaign', From 02cfc1d525af3ee91e61f22177d74bc85f2718b4 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 9 Sep 2024 09:21:52 +0100 Subject: [PATCH 02/28] remove setup accounts component. --- .../country-modal/country-amount.js | 102 ------------------ .../free-ad-credit/country-modal/index.js | 50 --------- .../free-ad-credit/country-modal/index.scss | 16 --- .../setup-accounts/free-ad-credit/index.js | 84 --------------- .../setup-accounts/free-ad-credit/index.scss | 24 ----- .../ads-stepper/setup-accounts/index.js | 90 ---------------- 6 files changed, 366 deletions(-) delete mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/country-amount.js delete mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.js delete mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.scss delete mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.js delete mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.scss delete mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/index.js diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/country-amount.js b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/country-amount.js deleted file mode 100644 index 908506a17f..0000000000 --- a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/country-amount.js +++ /dev/null @@ -1,102 +0,0 @@ -const countryAmount = [ - [ 'DZ', 250, 'USD' ], - [ 'AR', 25000, 'ARS' ], - [ 'AM', 300, 'USD' ], - [ 'AU', 600, 'AUD' ], - [ 'AT', 400, 'EUR' ], - [ 'AZ', 300, 'USD' ], - [ 'BH', 300, 'USD' ], - [ 'BY', 300, 'USD' ], - [ 'BE', 400, 'EUR' ], - [ 'BO', 350, 'USD' ], - [ 'BA', 350, 'EUR' ], - [ 'BR', 1200, 'BRL' ], - [ 'BG', 700, 'BGN' ], - [ 'CA', 600, 'CAD' ], - [ 'CL', 350, 'USD' ], - [ 'CN', 3000, 'CNY' ], - [ 'CO', 350, 'USD' ], - [ 'CR', 350, 'USD' ], - [ 'HR', 2500, 'HRK' ], - [ 'CY', 350, 'EUR' ], - [ 'CZ', 8500, 'CZK' ], - [ 'DK', 3000, 'DKK' ], - [ 'DO', 350, 'USD' ], - [ 'EC', 350, 'USD' ], - [ 'EG', 3000, 'EGP' ], - [ 'SV', 350, 'USD' ], - [ 'EE', 350, 'EUR' ], - [ 'FI', 400, 'EUR' ], - [ 'FR', 400, 'EUR' ], - [ 'GE', 300, 'USD' ], - [ 'DE', 400, 'EUR' ], - [ 'GR', 350, 'EUR' ], - [ 'GT', 350, 'USD' ], - [ 'HN', 350, 'USD' ], - [ 'HK', 3000, 'HKD' ], - [ 'HU', 120000, 'HUF' ], - [ 'IS', 400, 'EUR' ], - [ 'IN', 20000, 'INR' ], - [ 'ID', 3000000, 'IDR' ], - [ 'IE', 400, 'EUR' ], - [ 'IL', 1500, 'ILS' ], - [ 'IT', 400, 'EUR' ], - [ 'JP', 60000, 'JPY' ], - [ 'JO', 250, 'USD' ], - [ 'KZ', 300, 'USD' ], - [ 'KE', 300, 'USD' ], - [ 'KR', 600000, 'KRW' ], - [ 'KW', 300, 'USD' ], - [ 'LV', 350, 'EUR' ], - [ 'LB', 250, 'USD' ], - [ 'LY', 250, 'USD' ], - [ 'LT', 350, 'EUR' ], - [ 'LU', 400, 'EUR' ], - [ 'MK', 300, 'USD' ], - [ 'MY', 1500, 'MYR' ], - [ 'MT', 350, 'EUR' ], - [ 'MX', 7000, 'MXN' ], - [ 'ME', 350, 'EUR' ], - [ 'MA', 250, 'USD' ], - [ 'NL', 400, 'EUR' ], - [ 'NZ', 600, 'NZD' ], - [ 'NI', 350, 'USD' ], - [ 'NG', 300, 'USD' ], - [ 'NO', 4000, 'NOK' ], - [ 'OM', 250, 'USD' ], - [ 'PK', 400, 'USD' ], - [ 'PS', 250, 'USD' ], - [ 'PA', 350, 'USD' ], - [ 'PY', 350, 'USD' ], - [ 'PE', 350, 'USD' ], - [ 'PH', 20000, 'PHP' ], - [ 'PL', 1200, 'PLN' ], - [ 'PT', 400, 'EUR' ], - [ 'PR', 350, 'USD' ], - [ 'QA', 300, 'USD' ], - [ 'RO', 1500, 'RON' ], - [ 'RU', 30000, 'RUB' ], - [ 'SA', 1300, 'SAR' ], - [ 'RS', 350, 'EUR' ], - [ 'SG', 600, 'SGD' ], - [ 'SK', 350, 'EUR' ], - [ 'SI', 350, 'EUR' ], - [ 'ZA', 6000, 'ZAR' ], - [ 'ES', 400, 'EUR' ], - [ 'LK', 400, 'USD' ], - [ 'SE', 4000, 'SEK' ], - [ 'CH', 400, 'CHF' ], - [ 'TW', 12000, 'TWD' ], - [ 'TH', 12000, 'THB' ], - [ 'TN', 250, 'USD' ], - [ 'TR', 2500, 'TRY' ], - [ 'UA', 10000, 'UAH' ], - [ 'AE', 1600, 'AED' ], - [ 'GB', 400, 'GBP' ], - [ 'US', 500, 'USD' ], - [ 'UY', 350, 'USD' ], - [ 'VE', 350, 'USD' ], - [ 'VN', 5600000, 'VND' ], -]; - -export default countryAmount; diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.js b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.js deleted file mode 100644 index 07aa1b7e22..0000000000 --- a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.js +++ /dev/null @@ -1,50 +0,0 @@ -/** - * External dependencies - */ -import { __ } from '@wordpress/i18n'; - -/** - * Internal dependencies - */ -import AppModal from '.~/components/app-modal'; -import countryAmount from './country-amount'; -import './index.scss'; -import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap'; - -const CountryModal = ( props ) => { - const map = useCountryKeyNameMap(); - - return ( - -

- { __( - 'Whatever you spend in the next month will be added back to your Google Ads account as free credit, up to a maximum limit depending on your store’s country.', - 'google-listings-and-ads' - ) } -

- - - { countryAmount.map( ( el, idx ) => { - const [ countryCode, amount, currencyCode ] = el; - - return ( - - - - - ); - } ) } - -
{ map[ countryCode ] }{ `${ amount } ${ currencyCode }` }
- - ); -}; - -export default CountryModal; diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.scss b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.scss deleted file mode 100644 index 8b3ac7ff75..0000000000 --- a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.scss +++ /dev/null @@ -1,16 +0,0 @@ -.gla-free-ad-credit-country-modal { - max-width: $gla-width-large; - overflow: auto; - - table { - margin-left: calc(var(--large-gap) * 2); - border-spacing: calc(var(--main-gap) / 3 * 2) 0; - - tbody { - td:nth-child(1) { - font-weight: 600; - text-align: right; - } - } - } -} diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.js b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.js deleted file mode 100644 index 62606f1a57..0000000000 --- a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.js +++ /dev/null @@ -1,84 +0,0 @@ -/** - * External dependencies - */ -import { __ } from '@wordpress/i18n'; -import { createInterpolateElement, useState } from '@wordpress/element'; -import GridiconGift from 'gridicons/dist/gift'; - -/** - * Internal dependencies - */ -import AppDocumentationLink from '.~/components/app-documentation-link'; -import './index.scss'; -import TrackableLink from '.~/components/trackable-link'; -import CountryModal from './country-modal'; - -/** - * Clicking on the link to view free ad credit value by country. - * - * @event gla_free_ad_credit_country_click - * @property {string} context Indicates which page the link is in. - */ - -/** - * @fires gla_free_ad_credit_country_click with `{ context: 'setup-ads' }`. - * @fires gla_documentation_link_click with `{ context: 'setup-ads', link_id: 'free-ad-credit-terms', href: 'https://www.google.com/ads/coupons/terms/' }` - */ -const FreeAdCredit = () => { - const [ showModal, setShowModal ] = useState( false ); - - const handleClick = () => { - setShowModal( true ); - }; - - const handleRequestClose = () => { - setShowModal( false ); - }; - - return ( -
- -
-
- { __( - 'Spend $500 to get $500 in Google Ads credits!', - 'google-listings-and-ads' - ) } -
-
- { createInterpolateElement( - __( - 'New to Google Ads? Get $500 in ad credit when you spend $500 within your first 60 days. Check how much credit you can receive in your country here. Terms and conditions apply.', - 'google-listings-and-ads' - ), - { - checkLink: ( - - ), - termLink: ( - - ), - } - ) } -
- { showModal && ( - - ) } -
-
- ); -}; - -export default FreeAdCredit; diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.scss b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.scss deleted file mode 100644 index f36176c07d..0000000000 --- a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.scss +++ /dev/null @@ -1,24 +0,0 @@ -.gla-free-ad-credit { - background-color: #dcf7dd; - display: flex; - align-items: center; - gap: calc(var(--main-gap) / 3 * 2); - padding: calc(var(--main-gap) / 3 * 2); - color: $black; - - svg { - flex: 0 0 auto; - fill: #007017; - } - - &__title { - font-size: $default-font-size; - font-weight: 600; - margin-bottom: $grid-unit; - } - - &__description { - font-size: $helptext-font-size; - font-style: italic; - } -} diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/index.js b/js/src/setup-ads/ads-stepper/setup-accounts/index.js deleted file mode 100644 index 27133451b2..0000000000 --- a/js/src/setup-ads/ads-stepper/setup-accounts/index.js +++ /dev/null @@ -1,90 +0,0 @@ -/** - * External dependencies - */ -import { __ } from '@wordpress/i18n'; - -/** - * Internal dependencies - */ -import AppButton from '.~/components/app-button'; -import StepContent from '.~/components/stepper/step-content'; -import StepContentHeader from '.~/components/stepper/step-content-header'; -import StepContentFooter from '.~/components/stepper/step-content-footer'; -import VerticalGapLayout from '.~/components/vertical-gap-layout'; -import { ConnectedGoogleAccountCard } from '.~/components/google-account-card'; -import GoogleAdsAccountCard from '.~/components/google-ads-account-card'; -import FreeAdCredit from './free-ad-credit'; -import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; -import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus'; -import useGoogleAccount from '.~/hooks/useGoogleAccount'; -import useFreeAdCredit from '.~/hooks/useFreeAdCredit'; -import AppSpinner from '.~/components/app-spinner'; -import Section from '.~/wcdl/section'; - -const SetupAccounts = ( props ) => { - const { onContinue = () => {} } = props; - const { google } = useGoogleAccount(); - const { googleAdsAccount, hasGoogleAdsConnection } = useGoogleAdsAccount(); - const { hasAccess, step } = useGoogleAdsAccountStatus(); - const hasFreeAdCredit = useFreeAdCredit(); - - if ( ! google || ( google.active === 'yes' && ! googleAdsAccount ) ) { - return ; - } - - // Ads is ready when we have a connection and verified and verified access. - // Billing is not required, and the 'link_merchant' step will be resolved - // when the MC the account is connected. - const isGoogleAdsReady = - hasGoogleAdsConnection && - hasAccess && - [ '', 'billing', 'link_merchant' ].includes( step ); - - const isContinueButtonDisabled = ! isGoogleAdsReady; - - return ( - - -
- - - - { hasFreeAdCredit && } - -
- - - { __( 'Continue', 'google-listings-and-ads' ) } - - - - ); -}; - -export default SetupAccounts; From 423a2c12abb8193c5f39ca1b5e5af4aa4e158d14 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 9 Sep 2024 12:17:47 +0100 Subject: [PATCH 03/28] restore setup account --- .../add-paid-campaigns/add-paid-campaigns.test.js | 10 +++------- 1 file changed, 3 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 32c7096836..40cb4f71d9 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 @@ -7,6 +7,7 @@ import { expect, test } from '@playwright/test'; */ import { clearOnboardedMerchant, setOnboardedMerchant } from '../../utils/api'; import DashboardPage from '../../utils/pages/dashboard.js'; +import SetupAdsAccountsPage from '../../utils/pages/setup-ads/setup-ads-accounts.js'; import SetupBudgetPage from '../../utils/pages/setup-ads/setup-budget'; import { LOAD_STATE } from '../../utils/constants'; import { @@ -65,7 +66,9 @@ test.describe( 'Set up paid campaign', () => { test.beforeAll( async ( { browser } ) => { page = await browser.newPage(); dashboardPage = new DashboardPage( page ); + setupAdsAccounts = new SetupAdsAccountsPage( page ); await setOnboardedMerchant(); + await setupAdsAccounts.mockAdsAccountsResponse( [] ); await dashboardPage.mockRequests(); await dashboardPage.goto(); } ); @@ -86,13 +89,6 @@ test.describe( 'Set up paid campaign', () => { await expect( adsConnectionButton ).toBeEnabled(); } ); - test.describe( 'Set up your accounts page', async () => { - test.beforeAll( async () => { - - } ); - - } ); - test.describe( 'Create your paid campaign', () => { test.beforeAll( async () => { setupBudgetPage = new SetupBudgetPage( page ); From 0b38575701df7dc1b0ce68c2871d0551ecc16309 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 12 Sep 2024 21:03:43 +0100 Subject: [PATCH 04/28] if google connection is present show only step 2 --- js/src/setup-ads/ads-stepper/index.js | 72 +++++++++++++++++++-------- 1 file changed, 51 insertions(+), 21 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index a1545871c6..f506a69ab5 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -8,6 +8,7 @@ import { useState } from '@wordpress/element'; /** * Internal dependencies */ +import SetupAccounts from './setup-accounts'; import AdsCampaign from '.~/components/paid-ads/ads-campaign'; import SetupBilling from './setup-billing'; import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter'; @@ -17,7 +18,7 @@ import { FILTER_ONBOARDING, CONTEXT_ADS_ONBOARDING, } from '.~/utils/tracks'; - +import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; /** * @param {Object} props React props * @param {Object} props.formProps Form props forwarded from `Form` component. @@ -26,6 +27,7 @@ import { */ const AdsStepper = ( { formProps } ) => { const [ step, setStep ] = useState( '1' ); + const { hasGoogleAdsConnection } = useGoogleAdsAccount(); useEventPropertiesFilter( FILTER_ONBOARDING, { context: CONTEXT_ADS_ONBOARDING, @@ -53,39 +55,67 @@ const AdsStepper = ( { formProps } ) => { setStep( to ); }; + const handleSetupAccountsContinue = () => { + continueStep( hasGoogleAdsConnection ? '1' : '2' ); + }; + const handleCreateCampaignContinue = () => { - continueStep( '2' ); + continueStep( hasGoogleAdsConnection ? '2' : '3' ); }; - return ( - // This Stepper with this class name - // should be refactored into separate shared component. - // It is also used in the Setup MC flow. - { + let steps = [ + { + key: hasGoogleAdsConnection ? '1' : '2', + label: __( + 'Create your paid campaign', + 'google-listings-and-ads' + ), + content: ( + + ), + onClick: handleStepClick, + }, + { + key: hasGoogleAdsConnection ? '2' : '3', + label: __( 'Set up billing', 'google-listings-and-ads' ), + content: , + onClick: handleStepClick, + }, + ]; + if ( ! hasGoogleAdsConnection ) { + steps = [ { key: '1', label: __( - 'Create your paid campaign', + 'Set up your accounts', 'google-listings-and-ads' ), content: ( - ), onClick: handleStepClick, }, - { - key: '2', - label: __( 'Set up billing', 'google-listings-and-ads' ), - content: , - onClick: handleStepClick, - }, - ] } + ...steps, + ]; + } + + return steps; + }; + + return ( + // This Stepper with this class name + // should be refactored into separate shared component. + // It is also used in the Setup MC flow. + ); }; From 46fca311c61cebd5d908a35590df7e44d3365159 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 12 Sep 2024 21:09:46 +0100 Subject: [PATCH 05/28] restore accounts file --- .../country-modal/country-amount.js | 102 ++++++++++++++++++ .../free-ad-credit/country-modal/index.js | 50 +++++++++ .../free-ad-credit/country-modal/index.scss | 16 +++ .../setup-accounts/free-ad-credit/index.js | 84 +++++++++++++++ .../setup-accounts/free-ad-credit/index.scss | 24 +++++ .../ads-stepper/setup-accounts/index.js | 90 ++++++++++++++++ 6 files changed, 366 insertions(+) create mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/country-amount.js create mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.js create mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.scss create mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.js create mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.scss create mode 100644 js/src/setup-ads/ads-stepper/setup-accounts/index.js diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/country-amount.js b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/country-amount.js new file mode 100644 index 0000000000..908506a17f --- /dev/null +++ b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/country-amount.js @@ -0,0 +1,102 @@ +const countryAmount = [ + [ 'DZ', 250, 'USD' ], + [ 'AR', 25000, 'ARS' ], + [ 'AM', 300, 'USD' ], + [ 'AU', 600, 'AUD' ], + [ 'AT', 400, 'EUR' ], + [ 'AZ', 300, 'USD' ], + [ 'BH', 300, 'USD' ], + [ 'BY', 300, 'USD' ], + [ 'BE', 400, 'EUR' ], + [ 'BO', 350, 'USD' ], + [ 'BA', 350, 'EUR' ], + [ 'BR', 1200, 'BRL' ], + [ 'BG', 700, 'BGN' ], + [ 'CA', 600, 'CAD' ], + [ 'CL', 350, 'USD' ], + [ 'CN', 3000, 'CNY' ], + [ 'CO', 350, 'USD' ], + [ 'CR', 350, 'USD' ], + [ 'HR', 2500, 'HRK' ], + [ 'CY', 350, 'EUR' ], + [ 'CZ', 8500, 'CZK' ], + [ 'DK', 3000, 'DKK' ], + [ 'DO', 350, 'USD' ], + [ 'EC', 350, 'USD' ], + [ 'EG', 3000, 'EGP' ], + [ 'SV', 350, 'USD' ], + [ 'EE', 350, 'EUR' ], + [ 'FI', 400, 'EUR' ], + [ 'FR', 400, 'EUR' ], + [ 'GE', 300, 'USD' ], + [ 'DE', 400, 'EUR' ], + [ 'GR', 350, 'EUR' ], + [ 'GT', 350, 'USD' ], + [ 'HN', 350, 'USD' ], + [ 'HK', 3000, 'HKD' ], + [ 'HU', 120000, 'HUF' ], + [ 'IS', 400, 'EUR' ], + [ 'IN', 20000, 'INR' ], + [ 'ID', 3000000, 'IDR' ], + [ 'IE', 400, 'EUR' ], + [ 'IL', 1500, 'ILS' ], + [ 'IT', 400, 'EUR' ], + [ 'JP', 60000, 'JPY' ], + [ 'JO', 250, 'USD' ], + [ 'KZ', 300, 'USD' ], + [ 'KE', 300, 'USD' ], + [ 'KR', 600000, 'KRW' ], + [ 'KW', 300, 'USD' ], + [ 'LV', 350, 'EUR' ], + [ 'LB', 250, 'USD' ], + [ 'LY', 250, 'USD' ], + [ 'LT', 350, 'EUR' ], + [ 'LU', 400, 'EUR' ], + [ 'MK', 300, 'USD' ], + [ 'MY', 1500, 'MYR' ], + [ 'MT', 350, 'EUR' ], + [ 'MX', 7000, 'MXN' ], + [ 'ME', 350, 'EUR' ], + [ 'MA', 250, 'USD' ], + [ 'NL', 400, 'EUR' ], + [ 'NZ', 600, 'NZD' ], + [ 'NI', 350, 'USD' ], + [ 'NG', 300, 'USD' ], + [ 'NO', 4000, 'NOK' ], + [ 'OM', 250, 'USD' ], + [ 'PK', 400, 'USD' ], + [ 'PS', 250, 'USD' ], + [ 'PA', 350, 'USD' ], + [ 'PY', 350, 'USD' ], + [ 'PE', 350, 'USD' ], + [ 'PH', 20000, 'PHP' ], + [ 'PL', 1200, 'PLN' ], + [ 'PT', 400, 'EUR' ], + [ 'PR', 350, 'USD' ], + [ 'QA', 300, 'USD' ], + [ 'RO', 1500, 'RON' ], + [ 'RU', 30000, 'RUB' ], + [ 'SA', 1300, 'SAR' ], + [ 'RS', 350, 'EUR' ], + [ 'SG', 600, 'SGD' ], + [ 'SK', 350, 'EUR' ], + [ 'SI', 350, 'EUR' ], + [ 'ZA', 6000, 'ZAR' ], + [ 'ES', 400, 'EUR' ], + [ 'LK', 400, 'USD' ], + [ 'SE', 4000, 'SEK' ], + [ 'CH', 400, 'CHF' ], + [ 'TW', 12000, 'TWD' ], + [ 'TH', 12000, 'THB' ], + [ 'TN', 250, 'USD' ], + [ 'TR', 2500, 'TRY' ], + [ 'UA', 10000, 'UAH' ], + [ 'AE', 1600, 'AED' ], + [ 'GB', 400, 'GBP' ], + [ 'US', 500, 'USD' ], + [ 'UY', 350, 'USD' ], + [ 'VE', 350, 'USD' ], + [ 'VN', 5600000, 'VND' ], +]; + +export default countryAmount; diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.js b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.js new file mode 100644 index 0000000000..07aa1b7e22 --- /dev/null +++ b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.js @@ -0,0 +1,50 @@ +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import AppModal from '.~/components/app-modal'; +import countryAmount from './country-amount'; +import './index.scss'; +import useCountryKeyNameMap from '.~/hooks/useCountryKeyNameMap'; + +const CountryModal = ( props ) => { + const map = useCountryKeyNameMap(); + + return ( + +

+ { __( + 'Whatever you spend in the next month will be added back to your Google Ads account as free credit, up to a maximum limit depending on your store’s country.', + 'google-listings-and-ads' + ) } +

+ + + { countryAmount.map( ( el, idx ) => { + const [ countryCode, amount, currencyCode ] = el; + + return ( + + + + + ); + } ) } + +
{ map[ countryCode ] }{ `${ amount } ${ currencyCode }` }
+ + ); +}; + +export default CountryModal; diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.scss b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.scss new file mode 100644 index 0000000000..8b3ac7ff75 --- /dev/null +++ b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/country-modal/index.scss @@ -0,0 +1,16 @@ +.gla-free-ad-credit-country-modal { + max-width: $gla-width-large; + overflow: auto; + + table { + margin-left: calc(var(--large-gap) * 2); + border-spacing: calc(var(--main-gap) / 3 * 2) 0; + + tbody { + td:nth-child(1) { + font-weight: 600; + text-align: right; + } + } + } +} diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.js b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.js new file mode 100644 index 0000000000..62606f1a57 --- /dev/null +++ b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.js @@ -0,0 +1,84 @@ +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; +import { createInterpolateElement, useState } from '@wordpress/element'; +import GridiconGift from 'gridicons/dist/gift'; + +/** + * Internal dependencies + */ +import AppDocumentationLink from '.~/components/app-documentation-link'; +import './index.scss'; +import TrackableLink from '.~/components/trackable-link'; +import CountryModal from './country-modal'; + +/** + * Clicking on the link to view free ad credit value by country. + * + * @event gla_free_ad_credit_country_click + * @property {string} context Indicates which page the link is in. + */ + +/** + * @fires gla_free_ad_credit_country_click with `{ context: 'setup-ads' }`. + * @fires gla_documentation_link_click with `{ context: 'setup-ads', link_id: 'free-ad-credit-terms', href: 'https://www.google.com/ads/coupons/terms/' }` + */ +const FreeAdCredit = () => { + const [ showModal, setShowModal ] = useState( false ); + + const handleClick = () => { + setShowModal( true ); + }; + + const handleRequestClose = () => { + setShowModal( false ); + }; + + return ( +
+ +
+
+ { __( + 'Spend $500 to get $500 in Google Ads credits!', + 'google-listings-and-ads' + ) } +
+
+ { createInterpolateElement( + __( + 'New to Google Ads? Get $500 in ad credit when you spend $500 within your first 60 days. Check how much credit you can receive in your country here. Terms and conditions apply.', + 'google-listings-and-ads' + ), + { + checkLink: ( + + ), + termLink: ( + + ), + } + ) } +
+ { showModal && ( + + ) } +
+
+ ); +}; + +export default FreeAdCredit; diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.scss b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.scss new file mode 100644 index 0000000000..f36176c07d --- /dev/null +++ b/js/src/setup-ads/ads-stepper/setup-accounts/free-ad-credit/index.scss @@ -0,0 +1,24 @@ +.gla-free-ad-credit { + background-color: #dcf7dd; + display: flex; + align-items: center; + gap: calc(var(--main-gap) / 3 * 2); + padding: calc(var(--main-gap) / 3 * 2); + color: $black; + + svg { + flex: 0 0 auto; + fill: #007017; + } + + &__title { + font-size: $default-font-size; + font-weight: 600; + margin-bottom: $grid-unit; + } + + &__description { + font-size: $helptext-font-size; + font-style: italic; + } +} diff --git a/js/src/setup-ads/ads-stepper/setup-accounts/index.js b/js/src/setup-ads/ads-stepper/setup-accounts/index.js new file mode 100644 index 0000000000..27133451b2 --- /dev/null +++ b/js/src/setup-ads/ads-stepper/setup-accounts/index.js @@ -0,0 +1,90 @@ +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import AppButton from '.~/components/app-button'; +import StepContent from '.~/components/stepper/step-content'; +import StepContentHeader from '.~/components/stepper/step-content-header'; +import StepContentFooter from '.~/components/stepper/step-content-footer'; +import VerticalGapLayout from '.~/components/vertical-gap-layout'; +import { ConnectedGoogleAccountCard } from '.~/components/google-account-card'; +import GoogleAdsAccountCard from '.~/components/google-ads-account-card'; +import FreeAdCredit from './free-ad-credit'; +import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; +import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus'; +import useGoogleAccount from '.~/hooks/useGoogleAccount'; +import useFreeAdCredit from '.~/hooks/useFreeAdCredit'; +import AppSpinner from '.~/components/app-spinner'; +import Section from '.~/wcdl/section'; + +const SetupAccounts = ( props ) => { + const { onContinue = () => {} } = props; + const { google } = useGoogleAccount(); + const { googleAdsAccount, hasGoogleAdsConnection } = useGoogleAdsAccount(); + const { hasAccess, step } = useGoogleAdsAccountStatus(); + const hasFreeAdCredit = useFreeAdCredit(); + + if ( ! google || ( google.active === 'yes' && ! googleAdsAccount ) ) { + return ; + } + + // Ads is ready when we have a connection and verified and verified access. + // Billing is not required, and the 'link_merchant' step will be resolved + // when the MC the account is connected. + const isGoogleAdsReady = + hasGoogleAdsConnection && + hasAccess && + [ '', 'billing', 'link_merchant' ].includes( step ); + + const isContinueButtonDisabled = ! isGoogleAdsReady; + + return ( + + +
+ + + + { hasFreeAdCredit && } + +
+ + + { __( 'Continue', 'google-listings-and-ads' ) } + + + + ); +}; + +export default SetupAccounts; From 2bf96b31452d3e30101a1762acbaf4665e6717f4 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 12 Sep 2024 21:50:46 +0100 Subject: [PATCH 06/28] add spinner when page loads --- js/src/setup-ads/ads-stepper/index.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index f506a69ab5..b83587f865 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -19,6 +19,7 @@ import { CONTEXT_ADS_ONBOARDING, } from '.~/utils/tracks'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; +import AppSpinner from '.~/components/app-spinner'; /** * @param {Object} props React props * @param {Object} props.formProps Form props forwarded from `Form` component. @@ -27,13 +28,18 @@ import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; */ const AdsStepper = ( { formProps } ) => { const [ step, setStep ] = useState( '1' ); - const { hasGoogleAdsConnection } = useGoogleAdsAccount(); + const { hasFinishedResolution, hasGoogleAdsConnection } = + useGoogleAdsAccount(); useEventPropertiesFilter( FILTER_ONBOARDING, { context: CONTEXT_ADS_ONBOARDING, step, } ); + if ( ! hasFinishedResolution && ! hasGoogleAdsConnection ) { + return ; + } + // Allow the users to go backward only, not forward. // Users can only go forward by clicking on the Continue button. const handleStepClick = ( value ) => { From dc08372197176163e853158bcb0312aeb22b98de Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 16 Sep 2024 10:37:28 +0100 Subject: [PATCH 07/28] E2E test for with ad account. --- .../add-paid-campaigns-has-account.test.js | 356 ++++++++++++++++++ .../add-paid-campaigns.test.js | 201 +++++++++- 2 files changed, 552 insertions(+), 5 deletions(-) create mode 100644 tests/e2e/specs/add-paid-campaigns/add-paid-campaigns-has-account.test.js diff --git a/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns-has-account.test.js b/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns-has-account.test.js new file mode 100644 index 0000000000..7ee9e31d32 --- /dev/null +++ b/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns-has-account.test.js @@ -0,0 +1,356 @@ +/** + * External dependencies + */ +import { expect, test } from '@playwright/test'; +/** + * Internal dependencies + */ +import { clearOnboardedMerchant, setOnboardedMerchant } from '../../utils/api'; +import DashboardPage from '../../utils/pages/dashboard.js'; +import SetupAdsAccountsPage from '../../utils/pages/setup-ads/setup-ads-accounts.js'; +import SetupBudgetPage from '../../utils/pages/setup-ads/setup-budget'; +import { LOAD_STATE } from '../../utils/constants'; +import { + getCountryInputSearchBoxContainer, + getCountryTagsFromInputSearchBoxContainer, + getFAQPanelTitle, + getFAQPanelRow, + checkFAQExpandable, + checkBillingAdsPopup, +} from '../../utils/page'; + +const ADS_ACCOUNTS = [ + { + id: 1111111, + name: 'Test 1', + }, + { + id: 2222222, + name: 'Test 2', + }, +]; + +test.use( { storageState: process.env.ADMINSTATE } ); + +test.describe.configure( { mode: 'serial' } ); + +/** + * @type {import('../../utils/pages/dashboard.js').default} dashboardPage + */ +let dashboardPage = null; + +/** + * @type {import('../../utils/pages/setup-ads/setup-ads-accounts').default} setupAdsAccounts + */ +let setupAdsAccounts = null; + +/** + * @type {import('@playwright/test').Locator} adsConnectionButton + */ +let adsConnectionButton = null; + +/** + * @type {import('../../utils/pages/setup-ads/setup-budget.js').default} setupBudgetPage + */ +let setupBudgetPage = null; + +/** + * @type {import('@playwright/test').Page} page + */ +let page = null; + +test.describe( 'Set up paid campaign', () => { + // The campaign budget + let budget = null; + + test.beforeAll( async ( { browser } ) => { + page = await browser.newPage(); + dashboardPage = new DashboardPage( page ); + setupAdsAccounts = new SetupAdsAccountsPage( page ); + await setOnboardedMerchant(); + await setupAdsAccounts.mockAdsAccountsResponse( [] ); + await dashboardPage.mockRequests(); + await dashboardPage.goto(); + + // Add a connected Ads account. + await setupAdsAccounts.fulfillAdsConnection( { + id: ADS_ACCOUNTS[ 0 ].id, + currency: 'USD', + symbol: '$', + status: 'connected', + step: '', + } ); + } ); + + test.afterAll( async () => { + await clearOnboardedMerchant(); + await page.close(); + } ); + + test( 'Dashboard page contains Add Paid campaign buttons', async () => { + //Add page campaign in the Performance (Paid Campaigns) section + await expect( + dashboardPage.getAdsConnectionAllProgramsButton( 'summary-card' ) + ).toBeEnabled(); + + //Add page campaign in the programs section. + adsConnectionButton = dashboardPage.getAdsConnectionAllProgramsButton(); + await expect( adsConnectionButton ).toBeEnabled(); + } ); + + test.describe( 'Create your paid campaign', () => { + test.beforeAll( async () => { + setupBudgetPage = new SetupBudgetPage( page ); + await adsConnectionButton.click(); + await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); + } ); + + test( 'Create paid campaign page', async () => { + await expect( + page.getByRole( 'heading', { + name: 'Create your paid campaign', + } ) + ).toBeVisible(); + + await expect( + page.getByRole( 'heading', { name: 'Ads audience' } ) + ).toBeVisible(); + + await expect( + page.getByRole( 'heading', { name: 'Set your budget' } ) + ).toBeVisible(); + + await expect( + page.getByRole( 'button', { name: 'Continue' } ) + ).toBeDisabled(); + + await expect( + page.getByRole( 'link', { + name: 'See what your ads will look like.', + } ) + ).toBeVisible(); + } ); + + test.describe( 'Preview product ad', () => { + test( 'Preview product ad should be visible', async () => { + await expect( + page.getByText( 'Preview product ad' ) + ).toBeVisible(); + await expect( + page.getByText( + "Each of your product variants will have its own ad. Previews shown here are examples and don't include all possible formats." + ) + ).toBeVisible(); + } ); + + test( 'Change image buttons should be enabled', async () => { + const buttonsToChangeImage = page.locator( + '.gla-campaign-preview-card__moving-button' + ); + + expect( buttonsToChangeImage ).toHaveCount( 2 ); + + for ( const button of await buttonsToChangeImage.all() ) { + await expect( button ).toBeEnabled(); + } + } ); + } ); + + test.describe( 'FAQ panels', () => { + test( 'should see five questions in FAQ', async () => { + const faqTitles = getFAQPanelTitle( page ); + await expect( faqTitles ).toHaveCount( 5 ); + } ); + + test( 'should not see FAQ rows when FAQ titles are not clicked', async () => { + const faqRows = getFAQPanelRow( page ); + await expect( faqRows ).toHaveCount( 0 ); + } ); + + // eslint-disable-next-line jest/expect-expect + test( 'should see FAQ rows when all FAQ titles are clicked', async () => { + await checkFAQExpandable( page ); + } ); + } ); + + test( 'Audience should be United States', async () => { + const countrySearchBoxContainer = + getCountryInputSearchBoxContainer( page ); + const countryTags = + getCountryTagsFromInputSearchBoxContainer( page ); + await expect( countryTags ).toHaveCount( 1 ); + await expect( countrySearchBoxContainer ).toContainText( + 'United States' + ); + } ); + + test( 'Set the budget', async () => { + budget = '0'; + await setupBudgetPage.fillBudget( budget ); + + await expect( + page.getByRole( 'button', { name: 'Continue' } ) + ).toBeDisabled(); + + budget = '1'; + await setupBudgetPage.fillBudget( budget ); + + await expect( + page.getByRole( 'button', { name: 'Continue' } ) + ).toBeEnabled(); + } ); + + test( 'Budget Recommendation', async () => { + await expect( + 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(); + + await expect( + page.getByRole( 'button', { + name: 'Got It', + } ) + ).toBeEnabled(); + + await page + .getByRole( 'button', { + name: 'Got It', + } ) + .click(); + } ); + } ); + } ); + + 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 + await expect( + page.getByText( + 'Great! You already have billing information saved for this' + ) + ).toBeVisible(); + + await expect( + page.getByRole( 'button', { name: 'Launch paid campaign' } ) + ).toBeEnabled(); + + const campaignCreation = + setupBudgetPage.mockCampaignCreationAndAdsSetupCompletion( + '1', + [ 'US' ] + ); + await page + .getByRole( 'button', { name: 'Launch paid campaign' } ) + .click(); + await campaignCreation; + } ); + } ); +} ); 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 40cb4f71d9..8771b1166b 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 @@ -59,7 +59,7 @@ let setupBudgetPage = null; */ let page = null; -test.describe( 'Set up paid campaign', () => { +test.describe( 'Set up Ads account', () => { // The campaign budget let budget = null; @@ -89,14 +89,205 @@ test.describe( 'Set up paid campaign', () => { await expect( adsConnectionButton ).toBeEnabled(); } ); - test.describe( 'Create your paid campaign', () => { + test.describe( 'Set up your accounts page', async () => { test.beforeAll( async () => { - setupBudgetPage = new SetupBudgetPage( page ); + await setupAdsAccounts.mockAdsAccountsResponse( [] ); await adsConnectionButton.click(); await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); } ); - test( 'Create paid campaign page', async () => { + test( 'Page header should be "Set up your accounts"', async () => { + await expect( + page.getByRole( 'heading', { name: 'Set up your accounts' } ) + ).toBeVisible(); + await expect( + page.getByText( + 'Connect your Google account and your Google Ads account to set up a paid Performance Max campaign.' + ) + ).toBeVisible(); + } ); + + test( 'Google Account should show as connected', async () => { + await expect( + page.getByText( + 'This Google account is connected to your store’s product feed.' + ) + ).toBeVisible(); + } ); + + test( 'Continue Button should be disabled', async () => { + await expect( setupAdsAccounts.getContinueButton() ).toBeDisabled(); + } ); + } ); + + test.describe( 'Add paid campaigns with no Ads account', async () => { + test( 'Create an account should be visible', async () => { + const createAccountButton = page.getByRole( 'button', { + name: 'Create account', + } ); + + await expect( createAccountButton ).toBeVisible(); + + await expect( setupAdsAccounts.getContinueButton() ).toBeDisabled(); + + await expect( + page.getByText( + 'Required to set up conversion measurement and create campaigns.' + ) + ).toBeVisible(); + + await createAccountButton.click(); + } ); + + test( 'Create account button should be disable if the ToS have not been accepted.', async () => { + await expect( + page.getByRole( 'heading', { + name: 'Create Google Ads Account', + } ) + ).toBeVisible(); + + await expect( + page.getByText( + 'By creating a Google Ads account, you agree to the following terms and conditions:' + ) + ).toBeVisible(); + + await expect( + setupAdsAccounts.getCreateAdsAccountButtonModal() + ).toBeDisabled(); + } ); + + test( 'Accept terms and conditions to enable the create account button', async () => { + await setupAdsAccounts.getAcceptTermCreateAccount().check(); + + await expect( + setupAdsAccounts.getCreateAdsAccountButtonModal() + ).toBeEnabled(); + } ); + + test( 'Create an Ads account', async () => { + // Intercept Ads connection request. + const connectAdsAccountRequest = + setupAdsAccounts.registerConnectAdsAccountRequests(); + + await setupAdsAccounts.mockAdsAccountsResponse( ADS_ACCOUNTS ); + + // Mock request to fulfill Ads connection. + await setupAdsAccounts.fulfillAdsConnection( { + id: ADS_ACCOUNTS[ 0 ].id, + currency: 'USD', + symbol: '$', + status: 'incomplete', + step: 'account_access', + } ); + + await setupAdsAccounts.mockAdsStatusNotClaimed(); + + await setupAdsAccounts.getCreateAdsAccountButtonModal().click(); + + await connectAdsAccountRequest; + + const modal = setupAdsAccounts.getAcceptAccountModal(); + await expect( modal ).toBeVisible(); + } ); + + test( 'Show Unclaimed Ads account', async () => { + await setupAdsAccounts.clickCloseAcceptAccountButtonFromModal(); + + const claimButton = setupAdsAccounts.getAdsClaimAccountButton(); + const claimText = setupAdsAccounts.getAdsClaimAccountText(); + + await expect( claimButton ).toBeVisible(); + await expect( claimText ).toBeVisible(); + + await expect( setupAdsAccounts.getContinueButton() ).toBeDisabled(); + } ); + + test( 'Show Claimed Ads account', async () => { + // Intercept Ads connection request. + await setupAdsAccounts.fulfillAdsConnection( { + id: ADS_ACCOUNTS[ 0 ].id, + currency: 'USD', + symbol: '$', + status: 'connected', + step: '', + } ); + + await setupAdsAccounts.mockAdsStatusClaimed(); + + await page.reload(); + + await expect( setupAdsAccounts.getContinueButton() ).toBeEnabled(); + + await expect( + page.getByRole( 'link', { + name: `Account ${ ADS_ACCOUNTS[ 0 ].id }`, + } ) + ).toBeVisible(); + + await expect( setupAdsAccounts.getContinueButton() ).toBeEnabled(); + } ); + } ); + + test.describe( 'Add paid campaigns with existing Ads accounts', () => { + test.beforeAll( async () => { + await setupAdsAccounts.mockAdsAccountsResponse( ADS_ACCOUNTS ); + //Disconnect the account from the previous test + setupAdsAccounts.fulfillAdsConnection( { + id: ADS_ACCOUNTS[ 1 ].id, + currency: 'EUR', + symbol: '\u20ac', + status: 'disconnected', + } ); + + await page.reload(); + } ); + + test( 'Select one existing account', async () => { + const adsAccountSelected = `${ ADS_ACCOUNTS[ 1 ].id }`; + + await expect( + setupAdsAccounts.getConnectAdsButton() + ).toBeDisabled(); + + await setupAdsAccounts.selectAnExistingAdsAccount( + adsAccountSelected + ); + + await expect( + setupAdsAccounts.getConnectAdsButton() + ).toBeEnabled(); + + //Intercept Ads connection request + const connectAdsAccountRequest = + setupAdsAccounts.registerConnectAdsAccountRequests( + adsAccountSelected + ); + + //Mock request to fulfill Ads connection + setupAdsAccounts.fulfillAdsConnection( { + id: ADS_ACCOUNTS[ 1 ].id, + currency: 'EUR', + symbol: '\u20ac', + status: 'connected', + } ); + + await setupAdsAccounts.clickConnectAds(); + await connectAdsAccountRequest; + + await expect( setupAdsAccounts.getContinueButton() ).toBeEnabled(); + } ); + } ); + + test.describe( 'Create your paid campaign', () => { + test.beforeAll( async () => { + setupBudgetPage = new SetupBudgetPage( page ); + } ); + + test( 'Continue to create paid campaign', async () => { + await setupAdsAccounts.clickContinue(); + await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); + await expect( page.getByRole( 'heading', { name: 'Create your paid campaign', @@ -344,4 +535,4 @@ test.describe( 'Set up paid campaign', () => { await campaignCreation; } ); } ); -} ); +} ); \ No newline at end of file From a40f273be1880f9e5f8c932f7da1f204421be6b2 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 23 Sep 2024 15:01:06 +0100 Subject: [PATCH 08/28] check from global --- js/src/setup-ads/ads-stepper/index.js | 55 +++++++++++++-------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index b83587f865..544789cfef 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -18,8 +18,8 @@ import { FILTER_ONBOARDING, CONTEXT_ADS_ONBOARDING, } from '.~/utils/tracks'; -import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; -import AppSpinner from '.~/components/app-spinner'; +import { glaData } from '.~/constants'; + /** * @param {Object} props React props * @param {Object} props.formProps Form props forwarded from `Form` component. @@ -28,18 +28,13 @@ import AppSpinner from '.~/components/app-spinner'; */ const AdsStepper = ( { formProps } ) => { const [ step, setStep ] = useState( '1' ); - const { hasFinishedResolution, hasGoogleAdsConnection } = - useGoogleAdsAccount(); + const hasGoogleAdsId = glaData.initialWpData.adsId; useEventPropertiesFilter( FILTER_ONBOARDING, { context: CONTEXT_ADS_ONBOARDING, step, } ); - if ( ! hasFinishedResolution && ! hasGoogleAdsConnection ) { - return ; - } - // Allow the users to go backward only, not forward. // Users can only go forward by clicking on the Continue button. const handleStepClick = ( value ) => { @@ -62,17 +57,25 @@ const AdsStepper = ( { formProps } ) => { }; const handleSetupAccountsContinue = () => { - continueStep( hasGoogleAdsConnection ? '1' : '2' ); + continueStep( '2' ); }; const handleCreateCampaignContinue = () => { - continueStep( hasGoogleAdsConnection ? '2' : '3' ); + continueStep( hasGoogleAdsId ? '2' : '3' ); }; const getSteps = () => { let steps = [ { - key: hasGoogleAdsConnection ? '1' : '2', + key: '1', + label: __( 'Set up your accounts', 'google-listings-and-ads' ), + content: ( + + ), + onClick: handleStepClick, + }, + { + key: '2', label: __( 'Create your paid campaign', 'google-listings-and-ads' @@ -86,29 +89,23 @@ const AdsStepper = ( { formProps } ) => { onClick: handleStepClick, }, { - key: hasGoogleAdsConnection ? '2' : '3', + key: '3', label: __( 'Set up billing', 'google-listings-and-ads' ), content: , onClick: handleStepClick, }, ]; - if ( ! hasGoogleAdsConnection ) { - steps = [ - { - key: '1', - label: __( - 'Set up your accounts', - 'google-listings-and-ads' - ), - content: ( - - ), - onClick: handleStepClick, - }, - ...steps, - ]; + + if ( hasGoogleAdsId ) { + // Remove first step if there's an Ads account connected. + steps.shift(); + + steps = steps.map( ( singleStep ) => { + return { + ...singleStep, + key: ( parseInt( singleStep.key, 10 ) - 1 ).toString(), + }; + }, [] ); } return steps; From 265181483ee2ac92c049cd1221ce27a82df870e9 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 23 Sep 2024 15:11:28 +0100 Subject: [PATCH 09/28] remove function. --- js/src/setup-ads/ads-stepper/index.js | 83 ++++++++++++--------------- 1 file changed, 38 insertions(+), 45 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 544789cfef..fde9187efe 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -64,52 +64,45 @@ const AdsStepper = ( { formProps } ) => { continueStep( hasGoogleAdsId ? '2' : '3' ); }; - const getSteps = () => { - let steps = [ - { - key: '1', - label: __( 'Set up your accounts', 'google-listings-and-ads' ), - content: ( - - ), - onClick: handleStepClick, - }, - { - key: '2', - label: __( - 'Create your paid campaign', - 'google-listings-and-ads' - ), - content: ( - - ), - onClick: handleStepClick, - }, - { - key: '3', - label: __( 'Set up billing', 'google-listings-and-ads' ), - content: , - onClick: handleStepClick, - }, - ]; + let steps = [ + { + key: '1', + label: __( 'Set up your accounts', 'google-listings-and-ads' ), + content: ( + + ), + onClick: handleStepClick, + }, + { + key: '2', + label: __( 'Create your paid campaign', 'google-listings-and-ads' ), + content: ( + + ), + onClick: handleStepClick, + }, + { + key: '3', + label: __( 'Set up billing', 'google-listings-and-ads' ), + content: , + onClick: handleStepClick, + }, + ]; - if ( hasGoogleAdsId ) { - // Remove first step if there's an Ads account connected. - steps.shift(); + if ( hasGoogleAdsId ) { + // Remove first step if there's an Ads account connected. + steps.shift(); - steps = steps.map( ( singleStep ) => { - return { - ...singleStep, - key: ( parseInt( singleStep.key, 10 ) - 1 ).toString(), - }; - }, [] ); - } - - return steps; - }; + steps = steps.map( ( singleStep ) => { + return { + ...singleStep, + key: ( parseInt( singleStep.key, 10 ) - 1 ).toString(), + }; + }, [] ); + } return ( // This Stepper with this class name @@ -118,7 +111,7 @@ const AdsStepper = ( { formProps } ) => { ); }; From c26a029c1080eec3a89f5334d6d30cb33e0218a5 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 23 Sep 2024 15:13:30 +0100 Subject: [PATCH 10/28] remove new line. --- 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 8771b1166b..805f77e798 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 @@ -535,4 +535,4 @@ test.describe( 'Set up Ads account', () => { await campaignCreation; } ); } ); -} ); \ No newline at end of file +} ); From 84b1ed04cdfdbd86cb0493d640d2204f0059cd8f Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 26 Sep 2024 08:20:42 +0100 Subject: [PATCH 11/28] address review comment. --- js/src/setup-ads/ads-stepper/index.js | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index fde9187efe..5860afaba2 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -3,7 +3,7 @@ */ import { Stepper } from '@woocommerce/components'; import { __ } from '@wordpress/i18n'; -import { useState } from '@wordpress/element'; +import { useState, useRef } from '@wordpress/element'; /** * Internal dependencies @@ -12,13 +12,13 @@ import SetupAccounts from './setup-accounts'; import AdsCampaign from '.~/components/paid-ads/ads-campaign'; import SetupBilling from './setup-billing'; import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter'; +import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import { recordStepperChangeEvent, recordStepContinueEvent, FILTER_ONBOARDING, CONTEXT_ADS_ONBOARDING, } from '.~/utils/tracks'; -import { glaData } from '.~/constants'; /** * @param {Object} props React props @@ -28,13 +28,22 @@ import { glaData } from '.~/constants'; */ const AdsStepper = ( { formProps } ) => { const [ step, setStep ] = useState( '1' ); - const hasGoogleAdsId = glaData.initialWpData.adsId; + const { googleAdsAccount, hasGoogleAdsConnection } = useGoogleAdsAccount(); + const initHasAdsConnectionRef = useRef( null ); useEventPropertiesFilter( FILTER_ONBOARDING, { context: CONTEXT_ADS_ONBOARDING, step, } ); + if ( ! googleAdsAccount ) { + return null; // or a spinner + } + + if ( initHasAdsConnectionRef.current === null ) { + initHasAdsConnectionRef.current = hasGoogleAdsConnection; + } + // Allow the users to go backward only, not forward. // Users can only go forward by clicking on the Continue button. const handleStepClick = ( value ) => { @@ -61,7 +70,7 @@ const AdsStepper = ( { formProps } ) => { }; const handleCreateCampaignContinue = () => { - continueStep( hasGoogleAdsId ? '2' : '3' ); + continueStep( initHasAdsConnectionRef.current ? '2' : '3' ); }; let steps = [ @@ -92,16 +101,16 @@ const AdsStepper = ( { formProps } ) => { }, ]; - if ( hasGoogleAdsId ) { - // Remove first step if there's an Ads account connected. + if ( initHasAdsConnectionRef.current ) { + // Remove first step if the initial connection state of Ads account is connected. steps.shift(); - steps = steps.map( ( singleStep ) => { + steps = steps.map( ( singleStep, index ) => { return { ...singleStep, - key: ( parseInt( singleStep.key, 10 ) - 1 ).toString(), + key: index.toString(), }; - }, [] ); + } ); } return ( From bd4756706cdf4658e26ee19f6ce267c1d3f4306b Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 26 Sep 2024 16:23:17 +0100 Subject: [PATCH 12/28] remvoe comment --- js/src/setup-ads/ads-stepper/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 5860afaba2..354cd124a1 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -37,7 +37,7 @@ const AdsStepper = ( { formProps } ) => { } ); if ( ! googleAdsAccount ) { - return null; // or a spinner + return null; } if ( initHasAdsConnectionRef.current === null ) { From 0f52790525ce1a608c9925c559e32cc6a57692df Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 30 Sep 2024 08:09:01 +0100 Subject: [PATCH 13/28] fix step value --- js/src/setup-ads/ads-stepper/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 354cd124a1..db7aa1e2b4 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -108,7 +108,7 @@ const AdsStepper = ( { formProps } ) => { steps = steps.map( ( singleStep, index ) => { return { ...singleStep, - key: index.toString(), + key: ( index + 1 ).toString(), }; } ); } From c84fa6f00b079e78d580d33876414bd7333ce69a Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 30 Sep 2024 08:59:44 +0100 Subject: [PATCH 14/28] fix e2e --- .../add-paid-campaigns/add-paid-campaigns.test.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 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..73f2051d3a 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 @@ -215,7 +215,7 @@ test.describe( 'Set up Ads account', () => { await setupAdsAccounts.mockAdsStatusClaimed(); - await page.reload(); + await page.waitForTimeout( 30000 ); await expect( setupAdsAccounts.getContinueButton() ).toBeEnabled(); @@ -504,16 +504,19 @@ test.describe( 'Set up Ads account', () => { 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 ); + // Check if add account page is skipped. + await expect( + page.getByRole( 'heading', { + name: 'Create your paid campaign', + } ) + ).toBeVisible(); - //Step 2 - Fill the budget + //Step 1 - 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 + //Step 2 - Billing is already setup await expect( page.getByText( 'Great! You already have billing information saved for this' From bfec77368693e28cd95e7e41ba5d224899b52b4b Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 30 Sep 2024 09:01:00 +0100 Subject: [PATCH 15/28] remove additional test --- .../add-paid-campaigns-has-account.test.js | 356 ------------------ 1 file changed, 356 deletions(-) delete mode 100644 tests/e2e/specs/add-paid-campaigns/add-paid-campaigns-has-account.test.js diff --git a/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns-has-account.test.js b/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns-has-account.test.js deleted file mode 100644 index 7ee9e31d32..0000000000 --- a/tests/e2e/specs/add-paid-campaigns/add-paid-campaigns-has-account.test.js +++ /dev/null @@ -1,356 +0,0 @@ -/** - * External dependencies - */ -import { expect, test } from '@playwright/test'; -/** - * Internal dependencies - */ -import { clearOnboardedMerchant, setOnboardedMerchant } from '../../utils/api'; -import DashboardPage from '../../utils/pages/dashboard.js'; -import SetupAdsAccountsPage from '../../utils/pages/setup-ads/setup-ads-accounts.js'; -import SetupBudgetPage from '../../utils/pages/setup-ads/setup-budget'; -import { LOAD_STATE } from '../../utils/constants'; -import { - getCountryInputSearchBoxContainer, - getCountryTagsFromInputSearchBoxContainer, - getFAQPanelTitle, - getFAQPanelRow, - checkFAQExpandable, - checkBillingAdsPopup, -} from '../../utils/page'; - -const ADS_ACCOUNTS = [ - { - id: 1111111, - name: 'Test 1', - }, - { - id: 2222222, - name: 'Test 2', - }, -]; - -test.use( { storageState: process.env.ADMINSTATE } ); - -test.describe.configure( { mode: 'serial' } ); - -/** - * @type {import('../../utils/pages/dashboard.js').default} dashboardPage - */ -let dashboardPage = null; - -/** - * @type {import('../../utils/pages/setup-ads/setup-ads-accounts').default} setupAdsAccounts - */ -let setupAdsAccounts = null; - -/** - * @type {import('@playwright/test').Locator} adsConnectionButton - */ -let adsConnectionButton = null; - -/** - * @type {import('../../utils/pages/setup-ads/setup-budget.js').default} setupBudgetPage - */ -let setupBudgetPage = null; - -/** - * @type {import('@playwright/test').Page} page - */ -let page = null; - -test.describe( 'Set up paid campaign', () => { - // The campaign budget - let budget = null; - - test.beforeAll( async ( { browser } ) => { - page = await browser.newPage(); - dashboardPage = new DashboardPage( page ); - setupAdsAccounts = new SetupAdsAccountsPage( page ); - await setOnboardedMerchant(); - await setupAdsAccounts.mockAdsAccountsResponse( [] ); - await dashboardPage.mockRequests(); - await dashboardPage.goto(); - - // Add a connected Ads account. - await setupAdsAccounts.fulfillAdsConnection( { - id: ADS_ACCOUNTS[ 0 ].id, - currency: 'USD', - symbol: '$', - status: 'connected', - step: '', - } ); - } ); - - test.afterAll( async () => { - await clearOnboardedMerchant(); - await page.close(); - } ); - - test( 'Dashboard page contains Add Paid campaign buttons', async () => { - //Add page campaign in the Performance (Paid Campaigns) section - await expect( - dashboardPage.getAdsConnectionAllProgramsButton( 'summary-card' ) - ).toBeEnabled(); - - //Add page campaign in the programs section. - adsConnectionButton = dashboardPage.getAdsConnectionAllProgramsButton(); - await expect( adsConnectionButton ).toBeEnabled(); - } ); - - test.describe( 'Create your paid campaign', () => { - test.beforeAll( async () => { - setupBudgetPage = new SetupBudgetPage( page ); - await adsConnectionButton.click(); - await page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED ); - } ); - - test( 'Create paid campaign page', async () => { - await expect( - page.getByRole( 'heading', { - name: 'Create your paid campaign', - } ) - ).toBeVisible(); - - await expect( - page.getByRole( 'heading', { name: 'Ads audience' } ) - ).toBeVisible(); - - await expect( - page.getByRole( 'heading', { name: 'Set your budget' } ) - ).toBeVisible(); - - await expect( - page.getByRole( 'button', { name: 'Continue' } ) - ).toBeDisabled(); - - await expect( - page.getByRole( 'link', { - name: 'See what your ads will look like.', - } ) - ).toBeVisible(); - } ); - - test.describe( 'Preview product ad', () => { - test( 'Preview product ad should be visible', async () => { - await expect( - page.getByText( 'Preview product ad' ) - ).toBeVisible(); - await expect( - page.getByText( - "Each of your product variants will have its own ad. Previews shown here are examples and don't include all possible formats." - ) - ).toBeVisible(); - } ); - - test( 'Change image buttons should be enabled', async () => { - const buttonsToChangeImage = page.locator( - '.gla-campaign-preview-card__moving-button' - ); - - expect( buttonsToChangeImage ).toHaveCount( 2 ); - - for ( const button of await buttonsToChangeImage.all() ) { - await expect( button ).toBeEnabled(); - } - } ); - } ); - - test.describe( 'FAQ panels', () => { - test( 'should see five questions in FAQ', async () => { - const faqTitles = getFAQPanelTitle( page ); - await expect( faqTitles ).toHaveCount( 5 ); - } ); - - test( 'should not see FAQ rows when FAQ titles are not clicked', async () => { - const faqRows = getFAQPanelRow( page ); - await expect( faqRows ).toHaveCount( 0 ); - } ); - - // eslint-disable-next-line jest/expect-expect - test( 'should see FAQ rows when all FAQ titles are clicked', async () => { - await checkFAQExpandable( page ); - } ); - } ); - - test( 'Audience should be United States', async () => { - const countrySearchBoxContainer = - getCountryInputSearchBoxContainer( page ); - const countryTags = - getCountryTagsFromInputSearchBoxContainer( page ); - await expect( countryTags ).toHaveCount( 1 ); - await expect( countrySearchBoxContainer ).toContainText( - 'United States' - ); - } ); - - test( 'Set the budget', async () => { - budget = '0'; - await setupBudgetPage.fillBudget( budget ); - - await expect( - page.getByRole( 'button', { name: 'Continue' } ) - ).toBeDisabled(); - - budget = '1'; - await setupBudgetPage.fillBudget( budget ); - - await expect( - page.getByRole( 'button', { name: 'Continue' } ) - ).toBeEnabled(); - } ); - - test( 'Budget Recommendation', async () => { - await expect( - 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(); - - await expect( - page.getByRole( 'button', { - name: 'Got It', - } ) - ).toBeEnabled(); - - await page - .getByRole( 'button', { - name: 'Got It', - } ) - .click(); - } ); - } ); - } ); - - 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 - await expect( - page.getByText( - 'Great! You already have billing information saved for this' - ) - ).toBeVisible(); - - await expect( - page.getByRole( 'button', { name: 'Launch paid campaign' } ) - ).toBeEnabled(); - - const campaignCreation = - setupBudgetPage.mockCampaignCreationAndAdsSetupCompletion( - '1', - [ 'US' ] - ); - await page - .getByRole( 'button', { name: 'Launch paid campaign' } ) - .click(); - await campaignCreation; - } ); - } ); -} ); From eadc4455946512ebc34fda2ea9ee5bf1cfaaa6f0 Mon Sep 17 00:00:00 2001 From: Joe McGill Date: Fri, 4 Oct 2024 08:39:18 -0500 Subject: [PATCH 16/28] Ensure Ads account is claimed before skipping step 1 --- js/src/setup-ads/ads-stepper/index.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index db7aa1e2b4..39bc952aeb 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -13,6 +13,7 @@ import AdsCampaign from '.~/components/paid-ads/ads-campaign'; import SetupBilling from './setup-billing'; import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; +import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus'; import { recordStepperChangeEvent, recordStepContinueEvent, @@ -29,8 +30,11 @@ import { const AdsStepper = ( { formProps } ) => { const [ step, setStep ] = useState( '1' ); const { googleAdsAccount, hasGoogleAdsConnection } = useGoogleAdsAccount(); + const { hasAccess } = useGoogleAdsAccountStatus(); const initHasAdsConnectionRef = useRef( null ); + const isGoogleAdsReady = hasGoogleAdsConnection && hasAccess; + useEventPropertiesFilter( FILTER_ONBOARDING, { context: CONTEXT_ADS_ONBOARDING, step, @@ -41,7 +45,7 @@ const AdsStepper = ( { formProps } ) => { } if ( initHasAdsConnectionRef.current === null ) { - initHasAdsConnectionRef.current = hasGoogleAdsConnection; + initHasAdsConnectionRef.current = isGoogleAdsReady; } // Allow the users to go backward only, not forward. From 49ae5da8aba4291ac4877ae1ba5aec7ce5b9bb97 Mon Sep 17 00:00:00 2001 From: Joe McGill <801097+joemcgill@users.noreply.github.com> Date: Fri, 4 Oct 2024 08:40:30 -0500 Subject: [PATCH 17/28] Speed up E2E test case Co-authored-by: Eason --- tests/e2e/specs/add-paid-campaigns/add-paid-campaigns.test.js | 3 ++- 1 file changed, 2 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 73f2051d3a..06e0c19504 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 @@ -215,7 +215,8 @@ test.describe( 'Set up Ads account', () => { await setupAdsAccounts.mockAdsStatusClaimed(); - await page.waitForTimeout( 30000 ); + await page.dispatchEvent( 'body', 'blur' ); + await page.dispatchEvent( 'body', 'focus' ); await expect( setupAdsAccounts.getContinueButton() ).toBeEnabled(); From ec5d05a60b24b695d891937567fc10199022af9f Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Tue, 8 Oct 2024 13:31:24 +0100 Subject: [PATCH 18/28] check if google account is not ready --- js/src/setup-ads/ads-stepper/index.js | 29 +++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 39bc952aeb..6c131970f2 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -12,6 +12,7 @@ import SetupAccounts from './setup-accounts'; import AdsCampaign from '.~/components/paid-ads/ads-campaign'; import SetupBilling from './setup-billing'; import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter'; +import useGoogleAccount from '.~/hooks/useGoogleAccount'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus'; import { @@ -20,6 +21,7 @@ import { FILTER_ONBOARDING, CONTEXT_ADS_ONBOARDING, } from '.~/utils/tracks'; +import { GOOGLE_ADS_ACCOUNT_STATUS } from '.~/constants'; /** * @param {Object} props React props @@ -29,21 +31,40 @@ import { */ const AdsStepper = ( { formProps } ) => { const [ step, setStep ] = useState( '1' ); - const { googleAdsAccount, hasGoogleAdsConnection } = useGoogleAdsAccount(); - const { hasAccess } = useGoogleAdsAccountStatus(); const initHasAdsConnectionRef = useRef( null ); + const { + hasFinishedResolution: hasResolvedGoogleAccount, + } = useGoogleAccount(); - const isGoogleAdsReady = hasGoogleAdsConnection && hasAccess; + const { + googleAdsAccount, + hasFinishedResolution: hasResolvedGoogleAdsAccount, + } = useGoogleAdsAccount(); + + const { + hasAccess, + hasFinishedResolution: hasResolvedAdsAccountStatus, + } = useGoogleAdsAccountStatus(); useEventPropertiesFilter( FILTER_ONBOARDING, { context: CONTEXT_ADS_ONBOARDING, step, } ); - if ( ! googleAdsAccount ) { + if ( + ! hasResolvedGoogleAccount || + ! hasResolvedGoogleAdsAccount || + ! hasResolvedAdsAccountStatus || + googleAdsAccount === null // Catch errors retrieving accounts. + ) { return null; } + const isGoogleAdsReady = + googleAdsAccount.status === GOOGLE_ADS_ACCOUNT_STATUS.CONNECTED && + hasAccess === true && + !( hasAccess === true && step === 'conversion_action' ); + if ( initHasAdsConnectionRef.current === null ) { initHasAdsConnectionRef.current = isGoogleAdsReady; } From c7ad7f56e7f4ed4242a86f4155721fc14b75a360 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Tue, 8 Oct 2024 13:38:10 +0100 Subject: [PATCH 19/28] check if google account is not disconnected instead --- js/src/setup-ads/ads-stepper/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 6c131970f2..b2fe6f0bcb 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -55,13 +55,13 @@ const AdsStepper = ( { formProps } ) => { ! hasResolvedGoogleAccount || ! hasResolvedGoogleAdsAccount || ! hasResolvedAdsAccountStatus || - googleAdsAccount === null // Catch errors retrieving accounts. + googleAdsAccount === null ) { return null; } const isGoogleAdsReady = - googleAdsAccount.status === GOOGLE_ADS_ACCOUNT_STATUS.CONNECTED && + googleAdsAccount.status !== GOOGLE_ADS_ACCOUNT_STATUS.DISCONNECTED && hasAccess === true && !( hasAccess === true && step === 'conversion_action' ); From 64051b6d7471970e601472ef6949980995ca5655 Mon Sep 17 00:00:00 2001 From: Joe McGill Date: Tue, 8 Oct 2024 09:40:34 -0500 Subject: [PATCH 20/28] Adjust isGoogleRead logic and fix linting issues --- js/src/setup-ads/ads-stepper/index.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index b2fe6f0bcb..674221e571 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -21,7 +21,6 @@ import { FILTER_ONBOARDING, CONTEXT_ADS_ONBOARDING, } from '.~/utils/tracks'; -import { GOOGLE_ADS_ACCOUNT_STATUS } from '.~/constants'; /** * @param {Object} props React props @@ -32,18 +31,19 @@ import { GOOGLE_ADS_ACCOUNT_STATUS } from '.~/constants'; const AdsStepper = ( { formProps } ) => { const [ step, setStep ] = useState( '1' ); const initHasAdsConnectionRef = useRef( null ); - const { - hasFinishedResolution: hasResolvedGoogleAccount, - } = useGoogleAccount(); + const { hasFinishedResolution: hasResolvedGoogleAccount } = + useGoogleAccount(); const { googleAdsAccount, hasFinishedResolution: hasResolvedGoogleAdsAccount, + hasGoogleAdsConnection, } = useGoogleAdsAccount(); const { hasAccess, hasFinishedResolution: hasResolvedAdsAccountStatus, + step: adsAccountSetupStep, } = useGoogleAdsAccountStatus(); useEventPropertiesFilter( FILTER_ONBOARDING, { @@ -60,10 +60,10 @@ const AdsStepper = ( { formProps } ) => { return null; } - const isGoogleAdsReady = - googleAdsAccount.status !== GOOGLE_ADS_ACCOUNT_STATUS.DISCONNECTED && - hasAccess === true && - !( hasAccess === true && step === 'conversion_action' ); + const isGoogleAdsReady = + hasGoogleAdsConnection && + hasAccess === true && + adsAccountSetupStep !== 'conversion_action'; if ( initHasAdsConnectionRef.current === null ) { initHasAdsConnectionRef.current = isGoogleAdsReady; From c064183c42828f2cb1e26db92d348f5452ec8642 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 14 Oct 2024 08:48:40 +0100 Subject: [PATCH 21/28] remove google account check --- js/src/setup-ads/ads-stepper/index.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 9ad7320e1c..28aeac9cca 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -12,7 +12,6 @@ import SetupAccounts from './setup-accounts'; import AppButton from '.~/components/app-button'; import AdsCampaign from '.~/components/paid-ads/ads-campaign'; import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter'; -import useGoogleAccount from '.~/hooks/useGoogleAccount'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus'; import { @@ -31,8 +30,6 @@ import { const AdsStepper = ( { formProps } ) => { const [ step, setStep ] = useState( '1' ); const initHasAdsConnectionRef = useRef( null ); - const { hasFinishedResolution: hasResolvedGoogleAccount } = - useGoogleAccount(); const { googleAdsAccount, @@ -52,7 +49,6 @@ const AdsStepper = ( { formProps } ) => { } ); if ( - ! hasResolvedGoogleAccount || ! hasResolvedGoogleAdsAccount || ! hasResolvedAdsAccountStatus || googleAdsAccount === null From bb6fd03d53644d20a333a214ed63414aa95b108e Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 17 Oct 2024 08:50:41 +0100 Subject: [PATCH 22/28] update condition again --- js/src/setup-ads/ads-stepper/index.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 28aeac9cca..98dce1bc8c 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -48,20 +48,19 @@ const AdsStepper = ( { formProps } ) => { step, } ); - if ( - ! hasResolvedGoogleAdsAccount || - ! hasResolvedAdsAccountStatus || - googleAdsAccount === null - ) { - return null; - } + if ( initHasAdsConnectionRef.current === null ) { + if ( + ! ( hasResolvedGoogleAdsAccount && hasResolvedAdsAccountStatus ) + ) { + return null; + } - const isGoogleAdsReady = - hasGoogleAdsConnection && - hasAccess === true && - adsAccountSetupStep !== 'conversion_action'; + const isGoogleAdsReady = + googleAdsAccount !== null && + hasGoogleAdsConnection && + hasAccess === true && + adsAccountSetupStep !== 'conversion_action'; - if ( initHasAdsConnectionRef.current === null ) { initHasAdsConnectionRef.current = isGoogleAdsReady; } From 82d6a33a20a1be02493ab6137791d90845a651d0 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 17 Oct 2024 13:35:58 +0100 Subject: [PATCH 23/28] introduce wait for for the button to appear --- js/src/setup-ads/ads-stepper/index.test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/js/src/setup-ads/ads-stepper/index.test.js b/js/src/setup-ads/ads-stepper/index.test.js index feae4d028b..ad6cfc806d 100644 --- a/js/src/setup-ads/ads-stepper/index.test.js +++ b/js/src/setup-ads/ads-stepper/index.test.js @@ -11,7 +11,7 @@ jest.mock( '.~/components/paid-ads/ads-campaign', () => /** * External dependencies */ -import { screen, render } from '@testing-library/react'; +import { screen, render, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { recordEvent } from '@woocommerce/tracks'; @@ -39,6 +39,10 @@ describe( 'AdsStepper', () => { it( 'Should record events after calling back to `onContinue`', async () => { render( ); + await waitFor( () => { + expect( continueToStep2 ).toBeDefined(); + } ); + await continueToStep2(); expect( recordEvent ).toHaveBeenCalledTimes( 1 ); From 4bb48b19992bf8795782eaf4e73755e5c1a8f946 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Thu, 17 Oct 2024 21:53:41 +0100 Subject: [PATCH 24/28] spinner --- js/src/setup-ads/ads-stepper/index.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 299ffa1deb..dbc6a04920 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -9,6 +9,7 @@ import { useState, useRef } from '@wordpress/element'; * Internal dependencies */ import SetupAccounts from './setup-accounts'; +import AppSpinner from '.~/components/app-spinner'; import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter'; import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount'; import useGoogleAdsAccountStatus from '.~/hooks/useGoogleAdsAccountStatus'; @@ -49,13 +50,13 @@ const AdsStepper = ( { isSubmitting } ) => { if ( initHasAdsConnectionRef.current === null ) { if ( - ! ( hasResolvedGoogleAdsAccount && hasResolvedAdsAccountStatus ) + ! ( hasResolvedGoogleAdsAccount && hasResolvedAdsAccountStatus ) || + googleAdsAccount === null // Catch errors retrieving accounts. ) { - return null; + return ; } const isGoogleAdsReady = - googleAdsAccount !== null && hasGoogleAdsConnection && hasAccess === true && adsAccountSetupStep !== 'conversion_action'; From ac52735dc0522168089d43a2de15a66a2458b3b0 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Fri, 18 Oct 2024 08:01:55 +0100 Subject: [PATCH 25/28] remvoe googleAdsAccount null check --- js/src/setup-ads/ads-stepper/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index dbc6a04920..97b524c4b7 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -50,8 +50,7 @@ const AdsStepper = ( { isSubmitting } ) => { if ( initHasAdsConnectionRef.current === null ) { if ( - ! ( hasResolvedGoogleAdsAccount && hasResolvedAdsAccountStatus ) || - googleAdsAccount === null // Catch errors retrieving accounts. + ! ( hasResolvedGoogleAdsAccount && hasResolvedAdsAccountStatus ) ) { return ; } From 110f48c8aa16a1349cee55dab55f6284585111b1 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Fri, 18 Oct 2024 08:08:20 +0100 Subject: [PATCH 26/28] test work independetly --- js/src/setup-ads/ads-stepper/index.test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/js/src/setup-ads/ads-stepper/index.test.js b/js/src/setup-ads/ads-stepper/index.test.js index ad6cfc806d..e1491c875b 100644 --- a/js/src/setup-ads/ads-stepper/index.test.js +++ b/js/src/setup-ads/ads-stepper/index.test.js @@ -57,6 +57,10 @@ describe( 'AdsStepper', () => { render( ); + await waitFor( () => { + expect( screen ).toBeDefined(); + } ); + const step1 = screen.getByRole( 'button', { name: /accounts/ } ); // Step 2 -> Step 1 From 95cabc973a9fcc554a260b627ec1f63b1e103771 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Fri, 18 Oct 2024 08:14:37 +0100 Subject: [PATCH 27/28] remove defnition --- js/src/setup-ads/ads-stepper/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/js/src/setup-ads/ads-stepper/index.js b/js/src/setup-ads/ads-stepper/index.js index 97b524c4b7..40a960169b 100644 --- a/js/src/setup-ads/ads-stepper/index.js +++ b/js/src/setup-ads/ads-stepper/index.js @@ -32,7 +32,6 @@ const AdsStepper = ( { isSubmitting } ) => { const initHasAdsConnectionRef = useRef( null ); const { - googleAdsAccount, hasFinishedResolution: hasResolvedGoogleAdsAccount, hasGoogleAdsConnection, } = useGoogleAdsAccount(); From 9eaff54a7626bc7df55e8710bcc7d6960f65df15 Mon Sep 17 00:00:00 2001 From: Karthik Thayyil Date: Mon, 21 Oct 2024 13:52:45 +0100 Subject: [PATCH 28/28] replace with findbyrole --- js/src/setup-ads/ads-stepper/index.test.js | 6 ++---- 1 file changed, 2 insertions(+), 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 e1491c875b..72d39541d6 100644 --- a/js/src/setup-ads/ads-stepper/index.test.js +++ b/js/src/setup-ads/ads-stepper/index.test.js @@ -57,12 +57,10 @@ describe( 'AdsStepper', () => { render( ); - await waitFor( () => { - expect( screen ).toBeDefined(); + const step1 = await screen.findByRole( 'button', { + name: /accounts/, } ); - const step1 = screen.getByRole( 'button', { name: /accounts/ } ); - // Step 2 -> Step 1 await continueToStep2(); recordEvent.mockClear();