-
Notifications
You must be signed in to change notification settings - Fork 21
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: Automatically kick off MC & Ads accounts creation #2567
Comments
@mikkamp and @eason9487 while several details are still being worked out about all of the different states that will need to be shown in the Consolidated Google Accounts card, I think we can do this to start to get the initial structure in place for the simplest use case. I'd appreciate your feedback on this approach and specifically the questions listed under "Definition Questions" above. If you need more context, #2509 outlines the overarching goal of these changes. |
I would suggest to create those in parallel to the existing endpoints, and then clean things up once the UI has fully switched over. I find it a little hard to suggest what the technical implementation should look like as this issue seems to describe the happy path going from "no accounts found" to "accounts successfully created/claimed". There are a lot of in between steps / states that are currently being handled for each account, some of which require interactions. Are the account creations going to be fired off in parallel, with possible next steps on each account showing? Or is the account creation going to be handled sequentially, much like is done now. |
Regard of Implementation Brief
Since
If there is no particular reason, it's recommended that the Google Merchant Center account (re)claiming and Google Ads account acceptance be included in the scope of this ticket. Because the definition of "connected completion" for these accounts also include completion of these processes, breaking them down into other tickets might require writing additional temporary logic and tests that would be removed by another PR. Including them will also allow for early clarification of every detail that needs to be addressed. Regard of Definition Questions
My intuition is that this may not be necessary. This could be considered after all the main implementation of #2509 has been completed. There is a lot of branching logic, detail states, interrupt resuming, and error handling in the whole process. The possibility of streamlining it is unclear at this point, and it probably won't make the frontend UI state management any easier. Other suggestionsIt may be difficult to have a more precise discussion before defining how the following features are going to be presented in terms of UI and flow.
|
Thanks for the detailed feedback, @eason9487. We'll plan to implement this new flow via a separate
Totally agree with you. The designs that @michaeleleder are working on in this Figma are getting closer to being finalized, at which point the remainder of the implementation plan can be defined. One thing that I can see wasn't clear in my initial write up here is that we're planning on breaking this issue into several sub-tasks so we can make all of the updates needed in smaller, focused PRs. So, what I meant by "initially, we're going to ignore the need to reclaim and MC account or accept an Ads account" is that we may ignore these states in the first PRs, while we're getting the basic architecture of the components and UI set up, but then we'll add handling for those states in follow-up PRs as this feature is being implemented. I mainly wanted to set the expectation early that initial sub-tasks and their PRs related to this work will not fulfill all of the requirements of the current flow, but at the end of this feature, all should be accounted for. |
I've updated #2566 so that it accounts for creating the new |
@eason9487 I've updated this issue now that the design flows are starting to get close to completion and have created #2582 to handle the Google Ads claim flow. The reason I'd like to split this into separate tasks is that there are many additional scenarios that need to be handled by this new Since the Claim Ads Account flow has several details that need to be worked through, separating that work to a separate task will allow for work on additional scenarios to be started in parallel. |
As part of #2509, when a merchant connects a Google account that doesn't already have associated Google Merchant Center or Google Ads accounts, we'll streamline the process by creating the accounts for them and then prompt them to claim them.
Account creation copy:
Once the accounts are created and accepted, the Google card will be shown as connected, and all three accounts will be displayed on the card as shown in the following screenshot.
Note
Handling the Ads account acceptance from this flow will be handled in a follow-up issue
Acceptance Criteria
/wc/gla/ads/accounts
to create an Ads account/wc/gla/mc/accounts
to create an MC accountImplementation Brief
Note
This work should be branched from feature/2566-google-combo-card (if not already merged) and a PR should be created against the feature/2590-consolidate-google-account-cards branch.
The
GoogleAccountCard
is the wrapper for all the connection states currently. Once the Google Account is connected, it renders theConnectedGoogleAccountCard
component. TheGoogleAccountCard
component will need to be enhanced so it has a prop to optionally handle the account creation for Ads and MC accounts, since it is also used in theReconnectGoogleAccount
component here. When that prop istrue
it will render a newConnectedGoogleComboCard
component, since theConnectedGoogleAccountCard
component is also used as a standalone component on the settings screen and also on the first page of the current ads setup flow (though, this may get removed in #2534).The new
ConnectedGoogleComboCard
component will need to use the useExistingGoogleMCAccounts() anduseExistingGoogleAdsAccounts()
hooks to see if there are no existing accounts and kick off account creation. If so, theuseCreateMCAccount()
anduseUpsertAdsAccount()
hooks to create both accounts. Initially, we're going to ignore the need to reclaim and MC account or accept an Ads account, so the UI should be able to be updated as soon as we have a googleMCAccount.id and googleAdsAccount.it, respectively.Test Coverage
Definition Questions
An alternative to adding a prop to theGoogleAccountCard
to optionally display this combined flow would be to fork it to a newGoogleComboAccountsCard
component that duplicates much of the logic for the initial connection of the Google account and showing theRequestFullAccessGoogleAccountCard
component. Is there a preference?Testing Instructions
While testing this issue, please note that it assumes following points.
A. That site is not already claimed by any other Google Merchant Center account, so that there won't be any 403 error. As this scenario will be handled separately in #2597
B. The Google account we are trying to connect does not have existing MC and Ads account. A custom plugin code provided in PR description can be used to emulate this behaviour for existing accounts.
The text was updated successfully, but these errors were encountered: