From 64f51bfd72935fcfbbf03d8e33b165d2c78402e2 Mon Sep 17 00:00:00 2001 From: Darshan Sawardekar Date: Fri, 9 Aug 2024 08:05:00 +0530 Subject: [PATCH 1/8] Hides jetpack during onboarding if already connected --- .../setup-stepper/setup-accounts/index.js | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/js/src/setup-mc/setup-stepper/setup-accounts/index.js b/js/src/setup-mc/setup-stepper/setup-accounts/index.js index a318189cbb..3aa3f5cf43 100644 --- a/js/src/setup-mc/setup-stepper/setup-accounts/index.js +++ b/js/src/setup-mc/setup-stepper/setup-accounts/index.js @@ -101,6 +101,8 @@ const SetupAccounts = ( props ) => { * and the whole page would display this AppSpinner, which is not desired. */ const isLoadingJetpack = ! jetpack; + const isJetpackInactive = jetpack?.active !== 'yes'; + const isLoadingGoogle = jetpack?.active === 'yes' && ! google; const isLoadingGoogleMCAccount = google?.active === 'yes' && scope.gmcRequired && ! googleMCAccount; @@ -146,20 +148,27 @@ const SetupAccounts = ( props ) => { 'google-listings-and-ads' ) } /> -
- - - - - -
+ { isJetpackInactive && ( +
+ + + + + +
+ ) }
} From 88e0799eaf595a39293acc8d364c12ecf9fde3e3 Mon Sep 17 00:00:00 2001 From: Darshan Sawardekar Date: Mon, 12 Aug 2024 09:09:02 +0530 Subject: [PATCH 2/8] Limits jetpack check to only the WPComAccount card --- .../setup-stepper/setup-accounts/index.js | 35 ++++++++----------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/js/src/setup-mc/setup-stepper/setup-accounts/index.js b/js/src/setup-mc/setup-stepper/setup-accounts/index.js index 3aa3f5cf43..91f8fe4dbd 100644 --- a/js/src/setup-mc/setup-stepper/setup-accounts/index.js +++ b/js/src/setup-mc/setup-stepper/setup-accounts/index.js @@ -148,27 +148,22 @@ const SetupAccounts = ( props ) => { 'google-listings-and-ads' ) } /> - { isJetpackInactive && ( -
- +
+ + { isJetpackInactive && ( - - - -
- ) } + ) } + + + +
} From ad40c2c62f055251b7625a5c5044e435726343da Mon Sep 17 00:00:00 2001 From: Darshan Sawardekar Date: Mon, 12 Aug 2024 09:14:42 +0530 Subject: [PATCH 3/8] Adds e2e tests for jetpack connected & disconnected states --- .../specs/setup-mc/step-1-accounts.test.js | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/e2e/specs/setup-mc/step-1-accounts.test.js b/tests/e2e/specs/setup-mc/step-1-accounts.test.js index c5984bc5d1..00487267c4 100644 --- a/tests/e2e/specs/setup-mc/step-1-accounts.test.js +++ b/tests/e2e/specs/setup-mc/step-1-accounts.test.js @@ -45,7 +45,7 @@ test.describe( 'Set up accounts', () => { await setUpAccountsPage.closePage(); } ); - test( 'should see accounts step header, "Connect your WordPress.com account" & connect button', async () => { + test( 'JetpackDisconnected: should see accounts step header, "Connect your WordPress.com account" & connect button', async () => { await setUpAccountsPage.goto(); await expect( @@ -79,6 +79,28 @@ test.describe( 'Set up accounts', () => { await expect( continueButton ).toBeDisabled(); } ); + test( 'JetpackConnected: should verify that the Jetpack connect button is hidden if already connected', async () => { + await setUpAccountsPage.goto(); + + await expect( + page.getByRole( 'heading', { name: 'Set up your accounts' } ) + ).toBeVisible(); + + await expect( + page.getByText( + 'Connect the accounts required to use Google for WooCommerce.' + ) + ).toBeVisible(); + + await expect( + page.getByRole( 'button', { name: 'Connect' } ).first() + ).toBeEnabled(); + + const firstCard = setUpAccountsPage.getWPAccountCard(); + await expect( firstCard ).toBeEnabled(); + await expect( firstCard ).not.toContainText( 'WordPress.com' ); + } ); + test.describe( 'FAQ panels', () => { test( 'should see two questions in FAQ', async () => { const faqTitles = getFAQPanelTitle( page ); From e2f2420443a7155e95966f0548472a336cb79443 Mon Sep 17 00:00:00 2001 From: Darshan Sawardekar Date: Tue, 13 Aug 2024 09:57:17 +0530 Subject: [PATCH 4/8] Refactors variable names in setup flow --- js/src/setup-mc/setup-stepper/setup-accounts/index.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/js/src/setup-mc/setup-stepper/setup-accounts/index.js b/js/src/setup-mc/setup-stepper/setup-accounts/index.js index 91f8fe4dbd..0a6380eb40 100644 --- a/js/src/setup-mc/setup-stepper/setup-accounts/index.js +++ b/js/src/setup-mc/setup-stepper/setup-accounts/index.js @@ -101,9 +101,9 @@ const SetupAccounts = ( props ) => { * and the whole page would display this AppSpinner, which is not desired. */ const isLoadingJetpack = ! jetpack; - const isJetpackInactive = jetpack?.active !== 'yes'; + const isJetpackActive = jetpack?.active === 'yes'; - const isLoadingGoogle = jetpack?.active === 'yes' && ! google; + const isLoadingGoogle = isJetpackActive && ! google; const isLoadingGoogleMCAccount = google?.active === 'yes' && scope.gmcRequired && ! googleMCAccount; @@ -111,8 +111,6 @@ const SetupAccounts = ( props ) => { return ; } - const isGoogleAccountDisabled = jetpack?.active !== 'yes'; - // 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. @@ -157,10 +155,10 @@ const SetupAccounts = ( props ) => { ) } > - { isJetpackInactive && ( + { ! isJetpackActive && ( ) } - +
From f1d84f6510b588a1c96ad77f16b6a03167f84a76 Mon Sep 17 00:00:00 2001 From: Joe McGill Date: Tue, 13 Aug 2024 14:19:55 -0500 Subject: [PATCH 5/8] Update e2e tests --- .../specs/setup-mc/step-1-accounts.test.js | 69 ++++++------ .../pages/setup-mc/step-1-set-up-accounts.js | 100 +++++++----------- 2 files changed, 76 insertions(+), 93 deletions(-) diff --git a/tests/e2e/specs/setup-mc/step-1-accounts.test.js b/tests/e2e/specs/setup-mc/step-1-accounts.test.js index 00487267c4..9f13bf8971 100644 --- a/tests/e2e/specs/setup-mc/step-1-accounts.test.js +++ b/tests/e2e/specs/setup-mc/step-1-accounts.test.js @@ -79,28 +79,6 @@ test.describe( 'Set up accounts', () => { await expect( continueButton ).toBeDisabled(); } ); - test( 'JetpackConnected: should verify that the Jetpack connect button is hidden if already connected', async () => { - await setUpAccountsPage.goto(); - - await expect( - page.getByRole( 'heading', { name: 'Set up your accounts' } ) - ).toBeVisible(); - - await expect( - page.getByText( - 'Connect the accounts required to use Google for WooCommerce.' - ) - ).toBeVisible(); - - await expect( - page.getByRole( 'button', { name: 'Connect' } ).first() - ).toBeEnabled(); - - const firstCard = setUpAccountsPage.getWPAccountCard(); - await expect( firstCard ).toBeEnabled(); - await expect( firstCard ).not.toContainText( 'WordPress.com' ); - } ); - test.describe( 'FAQ panels', () => { test( 'should see two questions in FAQ', async () => { const faqTitles = getFAQPanelTitle( page ); @@ -138,7 +116,7 @@ test.describe( 'Set up accounts', () => { } ); } ); - test.describe( 'Connect Google account', () => { + test.describe( 'Connected WordPress.com account', async () => { test.beforeAll( async () => { // Mock Jetpack as connected await setUpAccountsPage.mockJetpackConnected( @@ -155,14 +133,45 @@ test.describe( 'Set up accounts', () => { await setUpAccountsPage.goto(); } ); - test( 'should see their WPORG email, "Google" title & connect button', async () => { - const jetpackDescriptionRow = - setUpAccountsPage.getJetpackDescriptionRow(); + test( 'should not show the WP.org connection card when already connected', async () => { + await expect( + page.getByRole( 'heading', { name: 'Set up your accounts' } ) + ).toBeVisible(); + + await expect( + page.getByText( + 'Connect the accounts required to use Google for WooCommerce.' + ) + ).toBeVisible(); - await expect( jetpackDescriptionRow ).toContainText( + await expect( + page.getByRole( 'button', { name: 'Connect' } ).first() + ).toBeEnabled(); + + const wpAccountCard = setUpAccountsPage.getWPAccountCard(); + await expect( wpAccountCard ).not.toBeVisible(); + } ); + + } ); + + test.describe( 'Connect Google account', () => { + test.beforeAll( async () => { + // Mock Jetpack as connected + await setUpAccountsPage.mockJetpackConnected( + 'Test user', 'jetpack@example.com' ); + // Mock google as not connected. + // When pending even WPORG will not render yet. + // If not mocked will fail and render nothing, + // as Jetpack is mocked only on the client-side. + await setUpAccountsPage.mockGoogleNotConnected(); + + await setUpAccountsPage.goto(); + } ); + + test( 'should see their WPORG email, "Google" title & connect button', async () => { const googleAccountCard = setUpAccountsPage.getGoogleAccountCard(); await expect( @@ -415,12 +424,6 @@ test.describe( 'Set up accounts', () => { } ); test( 'should see their WPORG email, Google email, "Google Merchant Center" title & "Create account" button', async () => { - const jetpackDescriptionRow = - setUpAccountsPage.getJetpackDescriptionRow(); - await expect( jetpackDescriptionRow ).toContainText( - 'jetpack@example.com' - ); - const googleDescriptionRow = setUpAccountsPage.getGoogleDescriptionRow(); await expect( googleDescriptionRow ).toContainText( diff --git a/tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js b/tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js index b6ac235f53..e7eeda083c 100644 --- a/tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js +++ b/tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js @@ -69,31 +69,15 @@ export default class SetUpAccountsPage extends MockRequests { return button.locator( ':scope.is-primary' ); } - /** - * Get .gla-account-card__title class. - * - * @return {import('@playwright/test').Locator} Get .gla-account-card__title class. - */ - getCardTitleClass() { - return this.page.locator( '.gla-account-card__title' ); - } - - /** - * Get .gla-account-card__description class. - * - * @return {import('@playwright/test').Locator} Get .gla-account-card__description class. - */ - getCardDescriptionClass() { - return this.page.locator( '.gla-account-card__description' ); - } - /** * Get Jetpack description row. * * @return {import('@playwright/test').Locator} Get Jetpack description row. */ getJetpackDescriptionRow() { - return this.getCardDescriptionClass().first(); + return this.getWPAccountCard().locator( + '.gla-account-card__description' + ); } /** @@ -102,7 +86,9 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get Google description row. */ getGoogleDescriptionRow() { - return this.getCardDescriptionClass().nth( 1 ); + return this.getGoogleAccountCard().locator( + '.gla-account-card__description' + ); } /** @@ -111,7 +97,9 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get Merchant Center description row. */ getMCDescriptionRow() { - return this.getCardDescriptionClass().nth( 3 ); + return this.getMCAccountCard().locator( + '.gla-account-card__description' + ); } /** @@ -120,7 +108,9 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get Google Ads title. */ getAdsTitleRow() { - return this.getCardTitleClass().nth( 2 ); + return this.getGoogleAdsAccountCard().locator( + '.gla-account-card__title' + ); } /** @@ -129,7 +119,7 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get Google Merchant Center title. */ getMCTitleRow() { - return this.getCardTitleClass().nth( 3 ); + return this.getMCAccountCard().locator( '.gla-account-card__title' ); } /** @@ -177,22 +167,13 @@ export default class SetUpAccountsPage extends MockRequests { return this.getModal().locator( 'button.is-secondary' ); } - /** - * Get .gla-connected-icon-label class. - * - * @return {import('@playwright/test').Locator} Get .gla-connected-icon-label class. - */ - getConnectedLabelClass() { - return this.page.locator( '.gla-connected-icon-label' ); - } - /** * Get Jetpack connected label. * * @return {import('@playwright/test').Locator} Get Jetpack connected label. */ getJetpackConnectedLabel() { - return this.getConnectedLabelClass().first(); + return this.getWPAccountCard().locator( '.gla-connected-icon-label' ); } /** @@ -201,7 +182,9 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get Google connected label. */ getGoogleConnectedLabel() { - return this.getConnectedLabelClass().nth( 1 ); + return this.getGoogleAccountCard().locator( + '.gla-connected-icon-label' + ); } /** @@ -210,7 +193,7 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get Merchant Center connected label. */ getMCConnectedLabel() { - return this.getConnectedLabelClass().nth( 2 ); + return this.getMCAccountCard().locator( '.gla-connected-icon-label' ); } /** @@ -246,31 +229,15 @@ export default class SetUpAccountsPage extends MockRequests { return this.page.locator( 'input#inspector-input-control-0' ); } - /** - * Get sub section title row. - * - * @return {import('@playwright/test').Locator} Get sub section title row. - */ - getSubSectionTitleRow() { - return this.page.locator( '.wcdl-subsection-title' ); - } - - /** - * Get section footer row. - * - * @return {import('@playwright/test').Locator} Get section footer row. - */ - getSectionFooterRow() { - return this.page.locator( '.wcdl-section-card-footer' ); - } - /** * Get select existing Merchant Center account title. * * @return {import('@playwright/test').Locator} Get select existing Merchant Center account title. */ getSelectExistingMCAccountTitle() { - return this.getSubSectionTitleRow().nth( 4 ); + return this.getMCAccountCard() + .locator( '.wcdl-subsection-title' ) + .nth( 1 ); } /** @@ -297,10 +264,11 @@ export default class SetUpAccountsPage extends MockRequests { /** * Get account cards. * + * @param {Object} options * @return {import('@playwright/test').Locator} Get account cards. */ - getAccountCards() { - return this.page.locator( '.gla-account-card' ); + getAccountCards( options = {} ) { + return this.page.locator( '.gla-account-card', options ); } /** @@ -309,7 +277,7 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get WordPress account card. */ getWPAccountCard() { - return this.getAccountCards().first(); + return this.getAccountCards( { hasText: 'WordPress.com' } ).first(); } /** @@ -318,7 +286,11 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get Google account card. */ getGoogleAccountCard() { - return this.getAccountCards().nth( 1 ); + return this.getAccountCards( { + has: this.page.locator( '.gla-account-card__title', { + hasText: 'Google', + } ), + } ).first(); } /** @@ -327,7 +299,11 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get Google Ads account card. */ getGoogleAdsAccountCard() { - return this.getAccountCards().nth( 2 ); + return this.getAccountCards( { + has: this.page.locator( '.gla-account-card__title', { + hasText: 'Google Ads', + } ), + } ).first(); } /** @@ -336,7 +312,11 @@ export default class SetUpAccountsPage extends MockRequests { * @return {import('@playwright/test').Locator} Get Merchant Center account card. */ getMCAccountCard() { - return this.getAccountCards().nth( 3 ); + return this.getAccountCards( { + has: this.page.locator( '.gla-account-card__title', { + hasText: 'Google Merchant Center', + } ), + } ).first(); } /** From 7523a05840761afb0ececf626e8fde9fa79354d8 Mon Sep 17 00:00:00 2001 From: Darshan Sawardekar Date: Wed, 14 Aug 2024 10:29:47 +0530 Subject: [PATCH 6/8] Fixes linter warnings --- tests/e2e/specs/setup-mc/step-1-accounts.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/e2e/specs/setup-mc/step-1-accounts.test.js b/tests/e2e/specs/setup-mc/step-1-accounts.test.js index 9f13bf8971..d686a1a3e9 100644 --- a/tests/e2e/specs/setup-mc/step-1-accounts.test.js +++ b/tests/e2e/specs/setup-mc/step-1-accounts.test.js @@ -151,7 +151,6 @@ test.describe( 'Set up accounts', () => { const wpAccountCard = setUpAccountsPage.getWPAccountCard(); await expect( wpAccountCard ).not.toBeVisible(); } ); - } ); test.describe( 'Connect Google account', () => { From 223935e9ca77be1af6133fc400a255699af5e4d5 Mon Sep 17 00:00:00 2001 From: Darshan Sawardekar Date: Mon, 19 Aug 2024 11:25:20 +0530 Subject: [PATCH 7/8] Updates e2e tests per PR feedack & removes unused code --- .../specs/setup-mc/step-1-accounts.test.js | 8 +--- .../pages/setup-mc/step-1-set-up-accounts.js | 42 ------------------- 2 files changed, 1 insertion(+), 49 deletions(-) diff --git a/tests/e2e/specs/setup-mc/step-1-accounts.test.js b/tests/e2e/specs/setup-mc/step-1-accounts.test.js index d686a1a3e9..819e92b076 100644 --- a/tests/e2e/specs/setup-mc/step-1-accounts.test.js +++ b/tests/e2e/specs/setup-mc/step-1-accounts.test.js @@ -58,10 +58,6 @@ test.describe( 'Set up accounts', () => { ) ).toBeVisible(); - await expect( - page.getByRole( 'button', { name: 'Connect' } ).first() - ).toBeEnabled(); - const wpAccountCard = setUpAccountsPage.getWPAccountCard(); await expect( wpAccountCard ).toBeEnabled(); await expect( wpAccountCard ).toContainText( 'WordPress.com' ); @@ -178,9 +174,7 @@ test.describe( 'Set up accounts', () => { ).toBeVisible(); await expect( - googleAccountCard - .getByRole( 'button', { name: 'Connect' } ) - .first() + googleAccountCard.getByRole( 'button', { name: 'Connect' } ) ).toBeEnabled(); const mcAccountCard = setUpAccountsPage.getMCAccountCard(); diff --git a/tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js b/tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js index e7eeda083c..5c90f321cb 100644 --- a/tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js +++ b/tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js @@ -69,17 +69,6 @@ export default class SetUpAccountsPage extends MockRequests { return button.locator( ':scope.is-primary' ); } - /** - * Get Jetpack description row. - * - * @return {import('@playwright/test').Locator} Get Jetpack description row. - */ - getJetpackDescriptionRow() { - return this.getWPAccountCard().locator( - '.gla-account-card__description' - ); - } - /** * Get Google description row. * @@ -102,17 +91,6 @@ export default class SetUpAccountsPage extends MockRequests { ); } - /** - * Get Google Ads title. - * - * @return {import('@playwright/test').Locator} Get Google Ads title. - */ - getAdsTitleRow() { - return this.getGoogleAdsAccountCard().locator( - '.gla-account-card__title' - ); - } - /** * Get Google Merchant Center title. * @@ -167,26 +145,6 @@ export default class SetUpAccountsPage extends MockRequests { return this.getModal().locator( 'button.is-secondary' ); } - /** - * Get Jetpack connected label. - * - * @return {import('@playwright/test').Locator} Get Jetpack connected label. - */ - getJetpackConnectedLabel() { - return this.getWPAccountCard().locator( '.gla-connected-icon-label' ); - } - - /** - * Get Google connected label. - * - * @return {import('@playwright/test').Locator} Get Google connected label. - */ - getGoogleConnectedLabel() { - return this.getGoogleAccountCard().locator( - '.gla-connected-icon-label' - ); - } - /** * Get Merchant Center connected label. * From ecd25fc488aabbee612fae2a3c661a256e2c58b8 Mon Sep 17 00:00:00 2001 From: Darshan Sawardekar Date: Mon, 19 Aug 2024 18:24:14 +0530 Subject: [PATCH 8/8] Updates e2e tests per CR feedback --- tests/e2e/specs/setup-mc/step-1-accounts.test.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/e2e/specs/setup-mc/step-1-accounts.test.js b/tests/e2e/specs/setup-mc/step-1-accounts.test.js index 819e92b076..7b4645c278 100644 --- a/tests/e2e/specs/setup-mc/step-1-accounts.test.js +++ b/tests/e2e/specs/setup-mc/step-1-accounts.test.js @@ -61,6 +61,7 @@ test.describe( 'Set up accounts', () => { const wpAccountCard = setUpAccountsPage.getWPAccountCard(); await expect( wpAccountCard ).toBeEnabled(); await expect( wpAccountCard ).toContainText( 'WordPress.com' ); + await expect( wpAccountCard.getByRole( 'button' ) ).toBeEnabled(); const googleAccountCard = setUpAccountsPage.getGoogleAccountCard(); await expect( googleAccountCard.getByRole( 'button' ) ).toBeDisabled(); @@ -140,10 +141,6 @@ test.describe( 'Set up accounts', () => { ) ).toBeVisible(); - await expect( - page.getByRole( 'button', { name: 'Connect' } ).first() - ).toBeEnabled(); - const wpAccountCard = setUpAccountsPage.getWPAccountCard(); await expect( wpAccountCard ).not.toBeVisible(); } );