Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Onboarding: Connect to existing MC account in Google Accounts Card #2597

Open
5 tasks
Tracked by #2509
joemcgill opened this issue Sep 9, 2024 · 6 comments · Fixed by #2639
Open
5 tasks
Tracked by #2509

Onboarding: Connect to existing MC account in Google Accounts Card #2597

joemcgill opened this issue Sep 9, 2024 · 6 comments · Fixed by #2639
Assignees

Comments

@joemcgill
Copy link
Collaborator

joemcgill commented Sep 9, 2024

Part of #2509

After connecting a Google account, we will provide the UI to allow the merchant to connect to an existing MC account.

image

Acceptance Criteria

  • When the Google account has 1 or more existing MC accounts, show the MC connection UI
  • If only one account exists, it should automatically be selected.
  • If multiple accouns exists, the user should be able to select which to connect.
  • The user can click the "connect" button to connect the selected MC account.
  • If there is no Ads account and one is being created, the card description should only reference Ads (see designs)

Implementation Brief

  • Using the GoogleComboAccountCard created in Onboarding: Create New Google Combo Accounts Card #2566.
  • Check if the Google connected account has an Merchant Center account connected.
    • This is done by checking googleMCAccount.status === connected via the useGoogleMCAccount hook.
  • Create connect-mc/index.js in the same folder as GoogleComboAccountCard which will house the ConnectMC component.
  • It should follow the logic contained in js/src/components/google-mc-account-card/connect-mc/index.js to display the data (not the UI) and styled as per the designs.
    • Use the existing components, for e.g Section.Card.Body for the layout.
    • Rendering a select if there are multiple options or text if there's only one option is handled in Update AppSelectControl when only one account is available #2593.
    • Clicking the "Connect" button should follow the same logic as handleConnectMC in js/src/components/google-mc-account-card/connect-mc/index.js.
  • If there's no connected Google Merchant Center account and the user has at least one MC account, render ConnectMC within GoogleComboAccountCard.
    • To check if the user has at least a MC account, use the useExistingGoogleMCAccounts hook.
  • Additionally, if there is no Ads account, GoogleComboAccountCard should automatically create one.
    • The logic for this should be handled in Onboarding: Automatically kick off MC & Ads accounts creation #2567.
    • The logic for updating the account creation text "You don’t have Google Ads account..." should be made in the same component that adds the "You don’t have Merchant Center nor Google Ads accounts..." text from Onboarding: Automatically kick off MC & Ads accounts creation #2567.
    • Evaluate whether there are any race conditions when creating/connecting either MC or Ads accounts. In this case, one possible solution would be to disable the MC connection section while the Ads account is being created. This is highly unlikely but something to look for during implementation.

Test Coverage

  • E2E tests should be added to assert the main points mentioned in the AC.

Definition Questions

  • If there is only one MC account, do we really need to prompt the user to connect it? Instead, can we automatically connect the single account? The user could always switch to a different MC account later through Edit mode if needed.
  • There isn't a specific design for when users click the "Or, create a new Merchant Center account" button. However, we can display the WarningModal from js/src/components/google-mc-account-card/warning-modal/index.js, and then proceed with creating the new MC account. If that's the approach, how should the UX look while the account is being created? Should we hide the ConnectMC component.
@asvinb
Copy link
Collaborator

asvinb commented Sep 12, 2024

@joemcgill I've updated the IB. Can you kindly take a look and let me know what you think please?

@joemcgill
Copy link
Collaborator Author

If there is only one MC account, do we really need to prompt the user to connect it? Instead, can we automatically connect the single account? The user could always switch to a different MC account later through Edit mode if needed.

Similar to my answer in #2596 (comment), we always want the user initiate the connection to an existing account at this point.

There isn't a specific design for when users click the "Or, create a new Merchant Center account" button. However, we can display the WarningModal from js/src/components/google-mc-account-card/warning-modal/index.js, and then proceed with creating the new MC account. If that's the approach, how should the UX look while the account is being created? Should we hide the ConnectMC component.

The requirements for connecting a new account are described in #2604. We could consider combining these but wanted to try to keep the tasks fairly focused.

@asvinb
Copy link
Collaborator

asvinb commented Sep 16, 2024

Thanks @joemcgill

@joemcgill
Copy link
Collaborator Author

joemcgill commented Sep 24, 2024

I've made a few updates to the Implementation Brief, mainly simplifying the description for kicking off creation of an Ads account if none exists, since that will all be handled in #2567. This issue should only be responsible for updating the text that is displayed during account creation when there are already existing MC accounts.

One additional consideration for this issue is if we should go ahead and handle the reclaim URL flow as part of this issue, or create a follow-up issue for it. Currently, it exists as part of the requirements for #2604, but I think it would be better to have this flow in place sooner.

@eason9487 this one is ready for you to review as well, please.

@joemcgill joemcgill added the needs feedback The issue/PR needs a response from any of the parties involved in the issue. label Sep 24, 2024
@eason9487
Copy link
Member

One thing may need to be clarified is similar to #2596 (comment).

@asvinb
Copy link
Collaborator

asvinb commented Sep 26, 2024

Thanks @eason9487 . I added a note in the IB for that. It'll be highly unlikely but something to watch out during implementation.

@macka61 macka61 removed status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. needs feedback The issue/PR needs a response from any of the parties involved in the issue. labels Sep 26, 2024
@kt-12 kt-12 assigned kt-12 and unassigned kt-12 Oct 1, 2024
@asvinb asvinb assigned asvinb and unassigned kt-12 Oct 7, 2024
@asvinb asvinb assigned joemcgill and unassigned asvinb Oct 8, 2024
@joemcgill joemcgill linked a pull request Oct 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants