R2 – Enable Enhanced Conversions

[asvinb]

Changes proposed in this Pull Request:

Closes #2216

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

Screenshots:

Detailed test instructions:

1. Complete the plugin onboarding process.
2. User should be redirected to /wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fproduct-feed&guide=submission-success
3. The success modal should be displayed.
4. Refer to the requirements section in https://github.com/woocommerce/google-listings-and-ads/issues/2239 for the different screens to be displayed within the modal and also the modal content.

Additional details:

1. There is a delay for the second screen within the success modal to show up. We must look into how we can grab the data as early as possible.
2. To create paid campaigns without setting up billing data, refer to the following code snippet.

Changelog entry

Add - Enhanced Conversions Tracking panel to the welcome guide and Settings page.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 53.35366% with 153 lines in your changes are missing coverage. Please review.

Project coverage is 62.6%. Comparing base (256c91e) to head (a3fb3e6). Report is 374 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##             develop   #2242     +/-   ##
===========================================
- Coverage       63.8%   62.6%   -1.2%     
- Complexity      4205    4288     +83     
===========================================
  Files            455     770    +315     
  Lines          17878   22116   +4238     
  Branches           0     563    +563     
===========================================
+ Hits           11412   13853   +2441     
- Misses          6466    7796   +1330     
- Partials           0     467    +467     

Flag	Coverage Δ
js-unit-tests	56.0% <70.2%> (?)
php-unit-tests	64.1% <37.1%> (+0.2%)	:arrow_up:

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

Files	Coverage Δ
...anced-conversion-tracking-settings/accept-terms.js	100.0% <100.0%> (ø)
js/src/constants.js	100.0% <100.0%> (ø)
js/src/data/action-types.js	100.0% <ø> (ø)
js/src/data/selectors.js	60.8% <100.0%> (ø)
js/src/hooks/useAllowEnhancedConversions.js	100.0% <100.0%> (ø)
js/src/hooks/useGoogleAdsTermsURL.js	100.0% <100.0%> (ø)
...product-feed/submission-success-guide/constants.js	100.0% <100.0%> (ø)
...mission-success-guide/enhanced-conversion/index.js	100.0% <100.0%> (ø)
...d/submission-success-guide/google-credits/index.js	100.0% <100.0%> (ø)
...ed/submission-success-guide/setup-success/index.js	100.0% <100.0%> (ø)
... and 19 more

... and 311 files with indirect coverage changes

[ankitguptaindia]

@asvinb @ankitrox In the QA, found some places where your inputs/fixes are needed. Please have a look when you have a moment.

QA Status - WIP.

Requirement followed From - https://github.com/woocommerce/google-listings-and-ads/issues/2239#issue-2119640123

Tests Performed on - Localhosted site with ngrok enabled URL.

cc: @joemcgill for progress update.

---

Scenario: Option 2b:

User skips campaign creation during onboarding, and does not have an existing spending campaign (i.e. user has no campaigns, or user has non-GL&A campaigns that never spent)

• (Modal 1/2) “You have successfully setup Google Listings & Ads”
• (Modal 2/2) “Spend $500 get $500 Modal”
• Note: Enhanced Conversions should still be accessible to this user in settings (to be implemented later), but we should not surface this in the modal.

QA_1: Unintended Closure of Modal 1/2 with 'View Product Feed' Button

Modal 1/2 displays a button labeled 'View Product Feed' and if a user clicks on this button, the modal closes, preventing them from proceeding to the next modal to create a paid campaign. There is no way to return to this modal except by using the custom URL. Please confirm if the behaviour of Modal 1/2 is intentional, I couldn't find any details regarding this button in the requirement document.**

Video explanation - https://somup.com/cZnoXJp3pT

Screenshot

---

QA_2: Infinite Spinner Issue When Closing T&S Page Without Acceptance

If a user has created a Paid Campaign and reached Modal 2/2, there is a 'Sign Terms of Service on Google Ads' button. After clicking on this button, it opens the Terms & Service (T&S) page in a new tab. If the user closes the T&S page without accepting the terms, the spinner keeps loading infinitely, and there is no way to reopen the T&S page.

I tried to re-connect Google account, refreshing the page but spinner stuck in loading state.

Video explanation - https://somup.com/cZnoIQp3dw

Screenshot

---

QA_3: Wording Consistency Issue for Enhanced Conversion Tracking Terms of Services.

The text of the modal mentions, "This feature can also be managed from Google Listings & Ads > Settings."

Modal Screen	Plugin Setting Screen

Both references point to the same but use different wording. In one instance, it's labeled as "Terms of Service," while in another, it's labelled as "Terms & Conditions." It would be good to ensure consistency by using the same label in both places.

---

QA_4: Delay in Loading Second Screen and Content on First Screen

As mentioned in the PR description, there is a delay observed in loading the second screen. Additionally, there seems to be a delay in loading the content on the first screen. The appearance of the 'Next" Button on the first screen also takes some time.

There is a delay for the second screen within the success modal to show up. We must look into how we can grab the data as early as possible.

Video explanation - https://somup.com/cZnoIEp3MT

---

QA_5: Order of the CTA buttons.

According to the requirement, in both states where the user has accepted the Terms of Service (ToS) and where the user has not accepted the ToS, the Close button should appear as the first button in the order.

Screenshot

---

[asvinb]

@ankitguptaindia @joemcgill Some thoughts on the QA feedback:

• QA_1: This is interesting. The "View Product Feed" button only shows up when there is only one screen in the success modal in the current implementation. So in case ads setup has been completed, the Google Ads credits screen does not show up within the modal, thus leaving only the success screen. So we can have either 1 or 2 screens, the second being the Google Ads credits one. However with the changes for enhanced tracking, we will always have 2 screens now. It'll be either the success + enhanced tracking one or the success + Google Ads screen which means that if we keep the current logic, the "View Product Feed" button should never show up. @joemcgill We need to check whether we should completely remove the button or where to place it.
• QA_2: Let's review that when we work on the settings page. We might need to add some working there.
• QA_3: Same as QA_2
• QA_4: Yes, we'll see how we can improve things. The Next button takes some time because we are still fetching the data for the second screen.
• QA_5: Maybe we need to have the "Next" and "Close" buttons to be always in the same position for modal related actions which is easier for the user.

[joemcgill]

Thanks both.

• QA_1, I think we should retain the current expectation for the first panel, that the "View Product Feed" button should be present when there is no second panel, given that the number and order of these screens may change in the future. Instead of rendering that button based on glaData.adsSetupComplete, we should render it based on whether showEnhancedConversionTrackingScreen and showGoogleAdsCreditsScreen are both false (or some similar helper).

• QA_2: @asvinb is this happening because the app gets stuck in a polling state waiting for the ToS to be updated? I think we can probably close the modal as soon as they click the "Sign Terms and Conditions on Google" button and complete the modal. At that point, we'll leave EC in the "pending" state and use a background task to check for when the user has accepted the terms.

• QA_3: Agreed about this being handled in a future PR. The settings panel at this point is just proof of concept, not the final implementation.

• QA_4: Agree that we should see how to speed this up.

• QA_5: For both of the second screens, the buttons should appear in the order and in the style described in #2239. I.e., the "Close" button is on the left and is styled using isSecondary and the "Sign terms of service on Google Ads" or "Confirm" button should be to the right, and styled using isPrimary.

[ankitguptaindia]

@joemcgill

This PR is ready for the next step. Verified all QA feedback and other use cases found in QA. ✅

cc: @asvinb @ankitrox

[mikkamp]

And the Enhanced Conversions is enabled through the API anyways

Seems I was incorrect about that point, it's not enabled through the API. But that means I can accept the terms and conditions without actually enabling Enhanced Conversions in my Ads account, and the extension will proceed as if Enhanced Conversions is enabled. I'd find that confusing as a Merchant and is contradicting what we are telling them in the text:

Clicking confirm will enable Enhanced Conversions on your account and update your tags accordingly. This feature can also be managed from Google Listings & Ads > Settings

[joemcgill]

Thanks for the feedback, @mikkamp. I do agree with your observations about the UX here is a bit confusing.

My current understanding is that there is currently no way to manage the EC status through the Google Ads API. However, what was explained to me is that once we are able to confirm that the data ToS has been accepted, then passing the following in the gtag will be respected { 'allow_enhanced_conversions': true }.

For any new accounts, we are asking them to accept ToS during the onboarding flow, so I don't know how common it would be for customers to need to be directed to the Ads settings screen. Regardless, I do think this is the right question:

Shouldn't we be reflecting the account status, not a local one?

Looking again at the Ads API, it looks like we can see whether EC for Leads is enabled, but not general EC. Perhaps we should be basing EC status on the basic conversion_tracking_status property instead? I can try to get clarity from folks on the Google side about this.

[joemcgill]

Update: I just got confirmation from @peteroliv that we should be able to improve the deep linking experience by adding eppn=customerDataTerms along with the OCID in that URL, like https://ads.google.com/aw/conversions/customersettings?ocid=%5BINSERTCIDHERE%5D&eppn=customerDataTerms. We'll make that change.

[mikkamp]

Just wanted to clarify that we've previously been told that the OCID is not the same as the CID we work with. But rather an internal CID, which isn't available in the API. So far we know that we get this OCID when we create a new account through the inviteLink. But that doesn't allow us to get it for anyone that connects an existing account. Ideally we'd be able to get it some other way through an API call.

[asvinb]

Hello @eason9487 Thanks for your review! It's been quite helpful and informative. I went ahead and made changes to the branch. Can you kindly review please? For the issues you mentioned here:

1. The second panel won't show up if the adsConnected property is not true which means that we have a valid Ads account connected (with billing set up). We might need to revisit that once R1 is merged since we will be able to check whether we have access to the Google Ads account (even though billing has not been set up)
2. This should be fixed now.

As for your comment here, that's a valid concern. I think we should probably not show the modal if enhanced conversion status is already enabled. What do you think @joemcgill ?

[joemcgill]

@asvinb

As for your https://github.com/woocommerce/google-listings-and-ads/pull/2242#pullrequestreview-1905425020, that's a valid concern. I think we should probably not show the modal if enhanced conversion status is already enabled. What do you think @joemcgill ?

That makes sense to me 👍🏻

[asvinb]

@asvinb

As for your #2242 (review), that's a valid concern. I think we should probably not show the modal if enhanced conversion status is already enabled. What do you think @joemcgill ?

That makes sense to me 👍🏻

Hi @asvinb, thanks for the update. I have finished the second round for the frontend changes.

For the issues you mentioned here:

1. The second panel won't show up if the adsConnected property is not true which means that we have a valid Ads account connected (with billing set up). [...]
2. This should be fixed now.

Sorry, I'm not pretty sure about this.

Kapture.2024-03-07.at.19.59.27.mp4 In the video, I skipped campaign creation during onboarding and then the modal shows only one page. Could you check again if it fit with the following requirements quoted from #2239?

• Option 2a: User skips campaign creation during onboarding, but has an existing spending campaign

  • (Modal 1/2) “You have successfully setup Google Listings & Ads”
  • (Modal 2/2) Enhanced Conversions Setup Modal

• Option 2b: User skips campaign creation during onboarding, and does not have an existing spending campaign (i.e. user has no campaigns, or user has non-GL&A campaigns that never spent)

  • (Modal 1/2) “You have successfully setup Google Listings & Ads”
  • (Modal 2/2) “Spend $500 get $500 Modal”

In addition, the video also shows the second issue still occurred.

@ankitrox To display the second screen, we are checking whether the global glaData.adsConnected property is set to true. Can you kindly advise when exactly this property is set to true and whether there's a better property to use to know when a valid Google Ads account has been connected (please note this should be available upon page load [server side]) so that we can immediately know whether we should show one or 2 pages without any delay.

[joemcgill]

@eason9487 and @mikkamp, we've finished processing all of the feedback from earlier rounds and have merged in the approved tagging PR from #2258 so that this should now represent the whole feature set for #2216. I've updated the description for this PR to reflect the latest state.

The only known issues still to address:

• [ ] EC data is not being sent unless the user manually enables EC in the GAFE settings screen (context) – pending feedback from Google.
• [ ] The OCID for deep linking to the settings panel is hooked up, but the option will not be available until after #2205 is merged.

I'd like to get the whole feature reviewed so we can address feedback while waiting for #2205 to be merged, at which point, I would update this branch so that we can test both features as an integrated set of changes before this PR is merged as well. Let me know if you have any questions or concerns.

[ankitrox]

@mikkamp Both of the points mentioned have been addressed. Sorry for inconvenience as earlier the fix was added as part of another PR which got closed.

1. For fatal error issue, a check has been added that $ads_id exists and if it doesn't, we are returning false
2. For strtolowe, this has been removed and the unit test has been added in AccountsController to ensure that it returns error when unexpected case is passed in parameter value.

CC: @joemcgill

[joemcgill]

@eason9487 while waiting for another review from you, I wanted to clarify a couple of the changes we made since your last review, which affects two main issues you had previously raised:

1. The success modal could sometimes be shown with only one page (comment)

We have removed the logic that checks for a connected Ads account prior to showing the second screen entirely here because connecting an Ads account is a requirement to complete the setup and trigger this modal in the first place, so a second screen will always be shown.

2. It is possible for the EC modal to be displayed in an enabled sate (comment)

This is still technically true, but we are now making sure the previous options are removed when an Ads account is disconnected (here). This fixes the situation you mentioned where disconnecting accounts and going back through the setup process could result in a user seeing this state.

Now, the only way that I think a user could end in this state is if the options failed to delete for some reason, or if they got set manually outside of the expected onboarding process. This all seems pretty edge case to me, so we've not added an additional check to disable the second page of the modal if paid ads are already set up AND EC has already been enabled.

[joemcgill]

Thanks for the review thus far, @eason9487. While you're still reviewing, I took a deeper look into the bug you reported here, where an account that already has PMax campaigns set up is connected and confirm the behavior.

This seems to be related to an existing bug with the useAdsCampaigns() hook related this TODO in the codebase where the hook relies on first determining whether the Ads setup is complete. This data is based on the ADS_SETUP_COMPLETED_AT option in this class, which does not seem to fire whenever a user ends the setup flow by clicking the "Skip this step for now" button. The result is that, even currently, existing paid campaigns are not listed on the dashboard until after the user sets up a paid campaign through the plugin, which will complete the Ads setup.

I don't know if there is any potential side effect to making sure that choosing "Skip this step for now" triggers the API call to the ads/setup/complete endpoint as well if billing has already been set up, but I think that would resolve the issue. Alternately, the check to see if the Ads setup is complete needs to be removed from the useAdsCampaigns() hook and campaigns should be queried regardless if we have an a valid account with billing set up.

Curious to get your thoughts on this.

[eason9487]

Hi @joemcgill

This seems to be related to an existing bug with the useAdsCampaigns() hook related this TODO in the codebase where the hook relies on first determining whether the Ads setup is complete. This data is based on the ADS_SETUP_COMPLETED_AT option in this class, which does not seem to fire whenever a user ends the setup flow by clicking the "Skip this step for now" button.

The TODO is one of the improvement ideas about letting the frontend side get the value in a different way, and I won't consider it as an existing bug because I don't recall an existing issue related to it.

Ads setup was not initially part of the onboarding flow. When it was added to the onboarding flow in 2022, we didn't change its definition after overall consideration of the way it was jointed together, and "Skip this step for now" wasn't considered as completed ads setup then.

The result is that, even currently, existing paid campaigns are not listed on the dashboard until after the user sets up a paid campaign through the plugin, which will complete the Ads setup.

Yes, this is how this plugin was designed before this PR. If I remember it right, this is the first PR that needs to know if there is an existing paid campaign before the "Ads setup" is completed.

I don't know if there is any potential side effect to making sure that choosing "Skip this step for now" triggers the API call to the ads/setup/complete endpoint as well if billing has already been set up, but I think that would resolve the issue.

The definition of ADS_SETUP_COMPLETED_AT is only within this plugin. The side effects of changing its update timing can be discovered by searching for the relevant keywords in this codebase.

Given the impact scope, trying to change the logic of calling to ads/setup/complete endpoint would involve a much wider scope of changes than this PR is supposed to do. If this is to be done, it will be necessary to return to the requirements review step and further validate the additional impact scope for feasibility.

Alternately, the check to see if the Ads setup is complete needs to be removed from the useAdsCampaigns() hook and campaigns should be queried regardless if we have an a valid account with billing set up.

useAdsCampaigns is used in a number of places, and it is equally necessary to check the impact scope and the feasibility for any out of scope changes.