Onboarding: Create New Google Combo Accounts Card. #2601

ankitrox · 2024-09-11T11:34:45Z

Changes proposed in this Pull Request:

Closes #2566 .

Replace this with a good description of your changes & reasoning.

Screenshots:

Detailed test instructions:

Disconnect all Google accounts if already connected in the plugin.
Start the onboarding process again.
There should be Google card visible in the first step.
The connect button would be disabled if Jetpack is not connected or Terms and conditions are not accepted.
Clicking the "Connect" button initiates the current flow to connect a Google account and grant permissions
After connecting the Google account successfully, the connected version of Google card should be visible.

Additional details:

Changelog entry

codecov · 2024-09-11T11:38:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.7%. Comparing base (71fb4c7) to head (1b8940f).
Report is 19 commits behind head on feature/2590-consolidate-google-account-cards.

Additional details and impacted files

@@                              Coverage Diff                              @@
##           feature/2590-consolidate-google-account-cards   #2601   +/-   ##
=============================================================================
  Coverage                                           62.7%   62.7%           
=============================================================================
  Files                                                319     319           
  Lines                                               5059    5059           
  Branches                                            1229    1229           
=============================================================================
  Hits                                                3171    3171           
  Misses                                              1715    1715           
  Partials                                             173     173

Flag	Coverage Δ
js-unit-tests	`62.7% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Remove ads and merchant center tests.

joemcgill

This is starting to look really good. I've left some feedback inline about the Terms & Conditions links that were previously used.

js/src/components/google-combo-account-card/connect-google-combo-account-card.js

joemcgill

Left a couple more notes, but this is looking right to me besides updating the E2E tests. I think @ankitguptaindia could start some QA testing while the E2E updates are being made.

joemcgill · 2024-09-11T18:32:48Z

js/src/components/google-combo-account-card/connect-google-combo-account-card.js

+ * @param {boolean} props.disabled
+ * @fires gla_google_account_connect_button_click with `{ action: 'authorization', context: 'reconnect' }`
+ * @fires gla_google_account_connect_button_click with `{ action: 'authorization', context: 'setup-mc' }`
+ * @fires gla_documentation_link_click with `{ context: 'setup-mc-accounts', link_id: 'required-google-permissions', href: 'https://woocommerce.com/document/google-for-woocommerce/get-started/setup-and-configuration/#required-google-permissions' }`


This accounts for the readMoreLink component. Will need to add two additional ones for the two new AppDocumentationLink components.

js/src/components/google-combo-account-card/connect-google-combo-account-card.js

asvinb

@ankitrox I left a few comments. Can you kindly check them out and let me know what you think please?
Thanks!

js/src/components/google-combo-account-card/connect-google-combo-account-card.js

js/src/components/google-combo-account-card/index.scss

ankitguptaindia · 2024-09-13T12:52:55Z

QA Test Report

I have completed the QA for this PR and confirmed that the requirements align with the details provided in the connected issue (FR ticket).
Below are the test cases I performed, all of which are functioning as expected:

Google Accounts Card Disabled if Store Not Connected to WP.com - If the store is not connected to WP.com, the Google account card should be disabled. ✅

T&C Acceptance Required - The merchant should not be able to connect their account until the "Accept T&C" checkbox is checked. ✅

External Links Open in New Tabs - Merchant Center and Google Ads links should be open in new tabs. ✅

Button Behavior After T&C Acceptance - After the merchant accepts the T&C checkbox, the 'Continue' button should be disabled, and only the 'Connect' button should be enabled. ✅

Access Screen for Existing Google Account - If the merchant uses an existing Google account that already has the required access, the access request screen shpuld not appear. ✅

Access Request Message - If the merchant doesn't provide full access or only grants partial access, the card shows a message requesting full access to proceed. ✅

Access Screen for New Google Account If the merchant uses a new Google account, the access request screen should be appear. ✅

Quick Demo:

Recording.822.mp4

ankitguptaindia · 2024-09-13T13:43:22Z

@ankitrox Can we align 'Ads' in the same line with 'Google'?

--

QA Note- Will test this PR again quickly after completion of CR.

ankitrox · 2024-09-13T14:50:20Z

@ankitrox Can we align 'Ads' in the same line with 'Google'?

--

QA Note- Will test this PR again quickly after completion of CR.

@ankitguptaindia This has been fixed with the new label prop for CheckboxControl component.

js/src/components/google-combo-account-card/connect-google-combo-account-card.js

asvinb

@ankitrox Changes LGTM. Just a one thing to fix here:
https://github.com/woocommerce/google-listings-and-ads/pull/2601/files#r1759081955
and you can send to QA.
Thanks!

ankitguptaindia · 2024-09-16T11:54:27Z

QA/Test Report- ✅

Testing Environment -

WordPress: 6.6.2
Theme active on store: Twenty Twenty-Four Version: 1.2
WooCommerce - Version 9.3.1
PHP: 8.3
Web Server: Nginx
Browser: Chrome - Version 128
OS: macOS Sonoma 14.6.1

Test Results - Verified all cases after completion of CR. PR changes working as described in AC.

Detailed Test cases status: #2601 (comment)

Functional Demo / Screencast -

Google Card workflow when has all permission:

Google.Connection.mp4

Google Card workflow when has limited permission:

G2.mp4

This PR is ready for the next step, we're discussing some minor concerns related to the button color status over Slack, Will update/create this/new PR if the issue is related to current changes.

eason9487

Thanks for the work. Left some comments that may need to be addressed.

Furthermore:

The e2e testing couldn't pass - https://github.com/woocommerce/google-listings-and-ads/actions/runs/10897655273/job/30239450725

Now these two assertions are false negatives. It would be nicer to be removed together.

google-listings-and-ads/tests/e2e/specs/setup-mc/step-1-accounts.test.js

Lines 69 to 73 in 8a152ba

    
           const mcAccountCard = setUpAccountsPage.getMCAccountCard(); 
        
           await expect( mcAccountCard.getByRole( 'button' ) ).not.toBeVisible(); 
        
           const adsAccountCard = setUpAccountsPage.getGoogleAdsAccountCard(); 
        
           await expect( adsAccountCard.getByRole( 'button' ) ).not.toBeVisible();

js/src/components/google-combo-account-card/connect-google-combo-account-card.js

js/src/components/google-combo-account-card/index.scss

js/src/components/google-combo-account-card/index.js

tests/e2e/specs/setup-mc/step-1-accounts.test.js

tests/e2e/utils/mock-requests.js

tests/e2e/utils/pages/setup-mc/step-1-set-up-accounts.js

js/src/components/google-combo-account-card/connect-google-combo-account-card.js

…oocommerce/google-listings-and-ads into feature/2566-google-combo-card

ankitrox · 2024-09-18T05:43:56Z

Thanks for the work. Left some comments that may need to be addressed.

Furthermore:

The e2e testing couldn't pass - https://github.com/woocommerce/google-listings-and-ads/actions/runs/10897655273/job/30239450725

Now these two assertions are false negatives. It would be nicer to be removed together.

google-listings-and-ads/tests/e2e/specs/setup-mc/step-1-accounts.test.js

Lines 69 to 73 in 8a152ba

const mcAccountCard = setUpAccountsPage.getMCAccountCard();

await expect( mcAccountCard.getByRole( 'button' ) ).not.toBeVisible();

const adsAccountCard = setUpAccountsPage.getGoogleAdsAccountCard();

await expect( adsAccountCard.getByRole( 'button' ) ).not.toBeVisible();

Thank you @eason9487 for reviewing the PR.

I've removed the redundant checks in e2e and also fixed the failing tests.

asvinb · 2024-09-18T08:02:36Z

Changes LGTM @ankitrox .
Assigning to @eason9487 to take another look.

eason9487

Thanks for the updates! LGTM.

There are two unnecessary styles in connect-google-combo-account-card.scss to be removed, and I believe the removal won't need another round of code reviews from Woo. I will be approving this RP in advance.

js/src/components/google-combo-account-card/connect-google-combo-account-card.scss

Onboarding: Create New Google Combo Accounts Card.

b5574cb

ankitrox changed the base branch from feature/2458-streamline-onboarding to feature/2458-consolidate-google-account-cards September 11, 2024 12:28

ankitrox changed the base branch from feature/2458-consolidate-google-account-cards to feature/2458-streamline-onboarding September 11, 2024 12:59

ankitrox added 4 commits September 11, 2024 18:44

Fix linting issues.

303aeb4

Fix linting issues.

fdcadb4

Fix js lint.

4b9ee7a

Disable eslint error.

71fb4c7

joemcgill linked an issue Sep 11, 2024 that may be closed by this pull request

Onboarding: Create New Google Combo Accounts Card #2566

Open

7 tasks

ankitrox changed the base branch from feature/2458-streamline-onboarding to feature/2458-consolidate-google-account-cards September 11, 2024 13:58

ankitrox added 2 commits September 11, 2024 19:38

Resolve conflicts with base.

4f7f967

Fix failing tests.

0dcb4a5

Remove ads and merchant center tests.

joemcgill reviewed Sep 11, 2024

View reviewed changes

Update links.

85dadb0

joemcgill reviewed Sep 11, 2024

View reviewed changes

ankitrox added 2 commits September 12, 2024 10:24

Add E2E tests for combo card.

780daf2

Styling changes

3cfb09e

asvinb requested changes Sep 13, 2024

View reviewed changes

ankitrox added 2 commits September 13, 2024 20:21

Use label prop for checkbox.

204a3da

Styling fixes.

52eaa9b

ankitrox requested a review from asvinb September 13, 2024 15:29

asvinb reviewed Sep 13, 2024

View reviewed changes

js/src/components/google-combo-account-card/connect-google-combo-account-card.js Outdated Show resolved Hide resolved

asvinb approved these changes Sep 13, 2024

View reviewed changes

ankitrox added 2 commits September 13, 2024 21:09

Remove unused variable.

6e5e09f

Update connect-google-combo-account-card.js

8a152ba

ankitguptaindia self-requested a review September 16, 2024 11:55

ankitguptaindia approved these changes Sep 16, 2024

View reviewed changes

mikkamp requested a review from eason9487 September 16, 2024 13:01

eason9487 requested changes Sep 17, 2024

View reviewed changes

eason9487 added the changelog: none Skip changelog entry for this PR label Sep 17, 2024

ankitrox added 3 commits September 18, 2024 10:17

Update css class names and fix tests.

f9564ad

Merge branch 'feature/2566-google-combo-card' of https://github.com/w…

5356633

…oocommerce/google-listings-and-ads into feature/2566-google-combo-card

Fix: e2e checkbox locator.

301c5e0

Fix: JS lint.

f396487

eason9487 approved these changes Sep 18, 2024

View reviewed changes

js/src/components/google-combo-account-card/connect-google-combo-account-card.scss Outdated Show resolved Hide resolved

js/src/components/google-combo-account-card/connect-google-combo-account-card.scss Outdated Show resolved Hide resolved

Remove redundant css rules.

1b8940f

ankitrox merged commit 52a53f4 into feature/2590-consolidate-google-account-cards Sep 18, 2024
5 checks passed

ankitrox deleted the feature/2566-google-combo-card branch September 18, 2024 12:13

ankitrox mentioned this pull request Sep 20, 2024

2593: Update AppSelectControl to single select #2608

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Onboarding: Create New Google Combo Accounts Card. #2601

Onboarding: Create New Google Combo Accounts Card. #2601

ankitrox commented Sep 11, 2024 •

edited

Loading

codecov bot commented Sep 11, 2024 •

edited

Loading

joemcgill left a comment

joemcgill left a comment

joemcgill Sep 11, 2024

asvinb left a comment

ankitguptaindia commented Sep 13, 2024 •

edited

Loading

ankitguptaindia commented Sep 13, 2024

ankitrox commented Sep 13, 2024

asvinb left a comment

ankitguptaindia commented Sep 16, 2024 •

edited

Loading

eason9487 left a comment

ankitrox commented Sep 18, 2024

asvinb commented Sep 18, 2024 •

edited

Loading

eason9487 left a comment

	const mcAccountCard = setUpAccountsPage.getMCAccountCard();
	await expect( mcAccountCard.getByRole( 'button' ) ).not.toBeVisible();

	const adsAccountCard = setUpAccountsPage.getGoogleAdsAccountCard();
	await expect( adsAccountCard.getByRole( 'button' ) ).not.toBeVisible();

Onboarding: Create New Google Combo Accounts Card. #2601

Onboarding: Create New Google Combo Accounts Card. #2601

Conversation

ankitrox commented Sep 11, 2024 • edited Loading

Changes proposed in this Pull Request:

Screenshots:

Detailed test instructions:

Additional details:

Changelog entry

codecov bot commented Sep 11, 2024 • edited Loading

Codecov Report

joemcgill left a comment

Choose a reason for hiding this comment

joemcgill left a comment

Choose a reason for hiding this comment

joemcgill Sep 11, 2024

Choose a reason for hiding this comment

asvinb left a comment

Choose a reason for hiding this comment

ankitguptaindia commented Sep 13, 2024 • edited Loading

QA Test Report

ankitguptaindia commented Sep 13, 2024

ankitrox commented Sep 13, 2024

asvinb left a comment

Choose a reason for hiding this comment

ankitguptaindia commented Sep 16, 2024 • edited Loading

QA/Test Report- ✅

eason9487 left a comment

Choose a reason for hiding this comment

ankitrox commented Sep 18, 2024

asvinb commented Sep 18, 2024 • edited Loading

eason9487 left a comment

Choose a reason for hiding this comment

ankitrox commented Sep 11, 2024 •

edited

Loading

codecov bot commented Sep 11, 2024 •

edited

Loading

ankitguptaindia commented Sep 13, 2024 •

edited

Loading

ankitguptaindia commented Sep 16, 2024 •

edited

Loading

asvinb commented Sep 18, 2024 •

edited

Loading