woocommerce / google-listings-and-ads

Sync your store with Google to list products for free, run paid ads, and track performance straight from your store dashboard.
https://woo.com/products/google-listings-and-ads/
GNU General Public License v3.0
47 stars 21 forks source link

Consolidate accounts onboarding: Automatically create Google Merchant Center or Google Ads accounts when the connected Google account doesn't have a respective existing one #2618

Closed ankitrox closed 4 weeks ago

ankitrox commented 2 months ago

Changes proposed in this Pull Request:

Closes #2567 .

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

Screenshots:

Detailed test instructions

  1. Create a new site and setup the GLA plugin (Make sure you are testing on the feature branch if this is not merged).

  2. Go to Marketing > Google for Woocommerce and start onboarding process.

  3. Connect to the fresh Google account. This account should not have any existing merchant center accounts or Google ads account.

  4. As soon as user sign-in flow is completed and user lands back to onboarding screen, there should be the card status changed to the one which should read You don’t have Merchant Center nor Google Ads accounts, so we’re creating them for you. as per the Figma designs.

  5. Once Merchant Center account and Ads account is created, card state will be changed this. Which will display Merchant Center ID and Google Ads ID (for 1-2 seconds ads ID may be seen as zero because ads request gets resolved later).

  6. Note that there would be some error notices popping up regarding ads account connection and ads connection status, but this can be ignored for this issue and will be handled in #2582

Use custom plugin to test with existing account.

As it is very inconvenient to test with the fresh account everytime; especially in the scenario where Google may not allow more accounts to be created and asks for new mobile number (where we are out of phone numbers). Following method can help us to test with existing Google account.

  1. Following plugin mimics the scenario such that for the first request to MC or Ads, it mocks the request such that there are no MC and Ads account.
<?php
/**
 * Plugin name: 2567 Test
 * Description: A plugin to provide REST API helper utilities.
 */

add_filter('rest_pre_dispatch', function($result, $server, $request) {
    $method = $request->get_method();
    $route  = $request->get_route();

    if ( 'POST' === $method ) {
        switch ($route) {
            case '/wc/gla/ads/accounts':
                $result = [
                    'message' => 'Account must be accepted before completing setup.',
                    'has_access' => false,
                    'step' => 'account_access',
                    'invite_link' => 'https://ads.google.com/nav/startacceptinvite?ivid=202079690&ocid=6741519560&eivid=ARZPQJ79CnAPjRUtJ1N2kGxycD2IjlRsgCNVRdiDRtvIyQRvkslrzx0',
                ];
                break;
            case '/wc/gla/mc/accounts':
                $result = [
                    'id'         => 5432178,
                    'name'       => null,
                    'subaccount' => null,
                    'domain'     => null,
                ];
                break;
        }
    }

    return $result;
}, 10, 3);

/**
 * Filter the response before it is returned to the client.
 * 
 * @param WP_HTTP_Response $response Result to send to the client. Usually a WP_REST_Response.
 * @param WP_REST_Server $server Server instance.
 * @param WP_REST_Request $request Request used to generate the response.
 */
add_filter('rest_post_dispatch', function($response, $server, $request) {

    // Mock accounts endpoints.
    if( '/wc/gla/ads/accounts' === $request->get_route() ) {
        $method = $request->get_method();

        if ( 'POST' === $method ) {
            $response->set_status( 428 );
        }

        if ( 'GET' === $method ) {
            $ads_request_number = get_option('gla_ads_account_request_number', 0);
            update_option('gla_ads_account_request_number', $ads_request_number + 1);

            $data = [];

            if ( $ads_request_number > 0 ) {
               $data[] = [
                'id'   => 78787878,
                'name' => 'gla.instawp.xyz',
               ];
            }

            $response->set_data( $data );
        }
    }

    if( '/wc/gla/mc/accounts' === $request->get_route() ) {
        $method = $request->get_method();

        if ( 'GET' === $method ) {
            $mc_request_number = get_option('gla_mc_account_request_number', 0);
            update_option('gla_mc_account_request_number', $mc_request_number + 1);

            $data = [];
            if ($mc_request_number > 0) {
                $data[] = [
                    'id'         => 5432178,
                    'name'       => null,
                    'subaccount' => null,
                    'domain'     => null,
                ];
            }

            $response->set_data( $data );
        }
    }

    // Mock connections endpoints.
    if( '/wc/gla/ads/connection' === $request->get_route() ) {
        $method = $request->get_method();
        if ( 'GET' === $method ) {
            $ads_request_number = get_option('gla_ads_connection_request_number', 0);
            update_option('gla_ads_connection_request_number', $ads_request_number + 1);

            $data = [
                'id'         => 0,
                'currency'   => 'USD',
                'status'     => 'disconnected',
                'symbol'     => '$',
            ];

            if ($ads_request_number > 0) {
                $data['id']     = 78787878;
                $data['status'] = 'incomplete';
                $data['step']   = 'account_access';
            }

            $response->set_data( $data );
        }
    }

    if( '/wc/gla/mc/connection' === $request->get_route() ) {
        $method = $request->get_method();
        if ( 'GET' === $method ) {
            $ads_request_number = get_option('gla_mc_connection_request_number', 0);
            update_option('gla_mc_connection_request_number', $ads_request_number + 1);

            $data = [
                'id'         => 0,
                'name'       => null,
                'subaccount' => null,
                'domain'     => null,
            ];

            if ($ads_request_number > 0) {
                $data['id']     = 5432178;
                $data['status'] = 'incomplete';
                $data['step']   = 'claim';
            }

            $response->set_data( $data );
        }
    }

    return $response;
}, 10, 3);

add_filter( 'woocommerce_gla_ads_billing_setup_status', function( $status ) {
    return 'approved';
} );
  1. Once the test is run, you will need to clear following options from *_options table.

    • gla_ads_account_request_number
    • gla_mc_account_request_number
    • gla_ads_connection_request_number
    • gla_mc_connection_request_number

These options can also be cleared using following npm command in local setup.

npm run wp-env run cli wp option delete gla_ads_account_request_number && npm run wp-env run cli wp option delete gla_mc_account_request_number && npm run wp-env run cli wp option delete gla_ads_connection_request_number && npm run wp-env run cli wp option delete gla_mc_connection_request_number
  1. Once above command ran successfully, the test can again be performed with given steps.

Additional details:

Changelog entry

Add - During onboarding, automatically create Google Merchant Center or Google Ads accounts when the connected Google account doesn't have a respective existing one.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 80.64516% with 12 lines in your changes missing coverage. Please review.

Please upload report for BASE (feature/2509-consolidate-google-account-cards@7d31e5c). Learn more about missing BASE report. Report is 94 commits behind head on feature/2509-consolidate-google-account-cards.

Files with missing lines Patch % Lines
js/src/hooks/useGoogleMCAccount.js 12.5% 6 Missing and 1 partial :warning:
js/src/hooks/useAutoCreateAdsMCAccounts.js 93.8% 3 Missing :warning:
js/src/hooks/useExistingGoogleAdsAccounts.js 0.0% 2 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618/graphs/tree.svg?width=650&height=150&src=pr&token=UROWUPF1LX&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce)](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce) ```diff @@ Coverage Diff @@ ## feature/2509-consolidate-google-account-cards #2618 +/- ## =============================================================================== Coverage ? 62.7% =============================================================================== Files ? 324 Lines ? 5153 Branches ? 1259 =============================================================================== Hits ? 3232 Misses ? 1744 Partials ? 177 ``` | [Flag](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce) | Coverage Δ | | |---|---|---| | [js-unit-tests](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce) | `62.7% <80.6%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files with missing lines](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce) | Coverage Δ | | |---|---|---| | [.../components/google-combo-account-card/constants.js](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618?src=pr&el=tree&filepath=js%2Fsrc%2Fcomponents%2Fgoogle-combo-account-card%2Fconstants.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#diff-anMvc3JjL2NvbXBvbmVudHMvZ29vZ2xlLWNvbWJvLWFjY291bnQtY2FyZC9jb25zdGFudHMuanM=) | `100.0% <100.0%> (ø)` | | | [js/src/constants.js](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618?src=pr&el=tree&filepath=js%2Fsrc%2Fconstants.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#diff-anMvc3JjL2NvbnN0YW50cy5qcw==) | `100.0% <100.0%> (ø)` | | | [js/src/hooks/useCreateMCAccount.js](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618?src=pr&el=tree&filepath=js%2Fsrc%2Fhooks%2FuseCreateMCAccount.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#diff-anMvc3JjL2hvb2tzL3VzZUNyZWF0ZU1DQWNjb3VudC5qcw==) | `6.7% <ø> (ø)` | | | [js/src/hooks/useExistingGoogleAdsAccounts.js](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618?src=pr&el=tree&filepath=js%2Fsrc%2Fhooks%2FuseExistingGoogleAdsAccounts.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#diff-anMvc3JjL2hvb2tzL3VzZUV4aXN0aW5nR29vZ2xlQWRzQWNjb3VudHMuanM=) | `16.7% <0.0%> (ø)` | | | [js/src/hooks/useAutoCreateAdsMCAccounts.js](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618?src=pr&el=tree&filepath=js%2Fsrc%2Fhooks%2FuseAutoCreateAdsMCAccounts.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#diff-anMvc3JjL2hvb2tzL3VzZUF1dG9DcmVhdGVBZHNNQ0FjY291bnRzLmpz) | `93.8% <93.8%> (ø)` | | | [js/src/hooks/useGoogleMCAccount.js](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2618?src=pr&el=tree&filepath=js%2Fsrc%2Fhooks%2FuseGoogleMCAccount.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#diff-anMvc3JjL2hvb2tzL3VzZUdvb2dsZU1DQWNjb3VudC5qcw==) | `13.3% <12.5%> (ø)` | |
joemcgill commented 1 month ago

We had a typo in the feature branch for #2509. I've updated the branch and am changing the base of this PR to merge into the correct branch now.

ankitguptaindia commented 1 month ago

@ankitrox I faced some issues when playing with the below scenario. Please check and let me know if these are already related to another PR.

Point 1: Continuous Spinner on Account Creation

While creating accounts, the spinner keeps running indefinitely without any response. Screencast: : Link


EDIT: As discussed on the call, Points 2 and 3 are being addressed/developed in another PR, so please skip both points. Points 4 and 5 followed the instructions discussed on the call for using Postman to delete account details, so these can also be skipped.

Point 2: Missing "Claim Your Google Ads Account" Card After encountering the above issue and refreshing the page, the account Id details start showing. However, the "Claim Your Google Ads Account" card is missing as shown [here](https://www.figma.com/design/fqR0EHi63lWahRcVTKCcba/Google-Listings-%26-Ads-v2.x?node-id=7363-4521&node-type=frame&t=RDgyblprY6tuzPTD-0) Screencast: [Link](https://github.com/user-attachments/assets/32e2833a-9c83-4632-aa76-4f767af1f19a) Point 3: When using a Google account that already has Ads/MC accounts, there is no information displayed regarding their status. The Ads/MC account cards do not appear, leaving the user unaware of any issues with the missing info. (At the time testing this case, custom plugin code mentioned in the description, was not active.) Screenshot: [Link](https://i.imgur.com/kDmOKVx.jpeg) Screencast: [Link](https://github.com/user-attachments/assets/7ccee7b4-1c38-47bd-80ae-0d40ba22aff5) Point 4: Automatic Ads/MC Account Creation on Fresh Setup On fresh setup (after deleting all site data via wp-reset plugin), when connecting WP.com account, it doesn't ask Google account to be connected and started creating Ads/MC a/c automatically. Screencast: [Link](https://github.com/user-attachments/assets/227c458e-8657-4ab4-b918-3a74ed4c43d8) Point 5: Tried to login with new Google account but no option to remove previously connected account. [Link](https://github.com/user-attachments/assets/c0e03b90-a8dd-4816-8489-f71e0d0eccec)

Point-6:

Apart from using the custom plugin to connect an existing Google account, I attempted to create Ads/MC accounts using a new real Google account and encountered issues with Ads account creation.

image

Please see the console logs in the video:

https://github.com/user-attachments/assets/4f1a81c7-93db-4649-b3fd-a72d5472d2b4

Points:7:

Once both accounts are created, a green "Connected" label (not pictured) should be shown and connected accounts should be displayed.

When using custom plugin to creating new accounts with old Google accounts, can not see a green "Connected" label.

image

ankitrox commented 1 month ago

@ankitguptaindia Thanks for testing the ticket. I've addressed the points reported by you, please check the below mentioned comments.

Point 1: Continuous Spinner on Account Creation

This has been fixed. There was a problem where a brand new account is connected, this was working for the first time, but as soon as we refreshed the page, the connecting status was shown and stayed on the page.

Point-6: Apart from using the custom plugin to connect an existing Google account, I attempted to create Ads/MC accounts using a new real Google account and encountered issues with Ads account creation.

As discussed over our call, I have discussed this with @joemcgill over slack conversation and we will handle this in #2582

Point-7 When using custom plugin to creating new accounts with old Google accounts, can not see a green "Connected" label.

Added.

Screenshot 2024-10-08 at 6 49 36 PM
asvinb commented 1 month ago

Changes LGTM @ankitrox

ankitguptaindia commented 1 month ago

Hi @ankitrox Thanks for the fixes, Point-1 has been fixed and the account is created without any error but Point-7, the success green mark is still not appearing for me and there is a new network error appearing. Please check the attached screencast for error details.

https://github.com/user-attachments/assets/04024d18-5f21-4764-9716-6362ca54ed01

ankitguptaindia commented 1 month ago

@ankitrox

Point 1: Continuous Spinner on Account Creation

When using existing Google accounts with Ads/MC accounts and the custom plugin, everything works as expected. However, when using a new real Google account without any Ads/MC accounts, the "creating..." spinner runs continuously for an extended period. After refreshing the page, the new accounts appear as connected.

Here’s a screencast showing my steps: https://somup.com/cZ66ITH3eJ Please have a look.

ankitguptaindia commented 1 month ago

Update regarding the issue reported above in https://github.com/woocommerce/google-listings-and-ads/pull/2618#issuecomment-2405197648

This issue occurs when the store/site has already been claimed by another MC account, and we are attempting to create new MC/Ads accounts in the first step of onboarding. If both the site and Google accounts are new, the continuous spinner issue ("Creating...") does not occur.

This scenario will be handled separately in issue #2597.

image

ankitrox commented 1 month ago

Thanks for the feedback @eason9487

I have addressed the changes and responded to other comments. Can you please look over them and let me know in case of any questions?

Over to you for another review.

Thanks again.

joemcgill commented 1 month ago

I've left a bit of feedback, but would appreciate if @eason9487 could give the latest iteration another look.

ankitrox commented 1 month ago

Thanks for your valuable suggestion on E2E @eason9487. I have addressed these changes and left response to your comments. Assigning this to you for another pass.

ankitrox commented 4 weeks ago

Thanks for your review and valuable suggestions @eason9487 .

I've made the changes and merging the PR.