(R2.1f) Google tag enhanced conversion #2258
Conversation
github-actions
bot
added
the
changelog: update
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/2239-enhanced-conversions-panel #2258 +/- ##
===========================================================================
- Coverage 62.2% 62.0% -0.1%
- Complexity 4220 4239 +19
===========================================================================
Files 767 767
Lines 21848 21913 +65
Branches 554 552 -2
===========================================================================
+ Hits 13587 13595 +8
- Misses 7797 7854 +57
Partials 464 464
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Thanks @ankitrox. Left some feedback inline. I don't see any existing unit tests covering this maybe_display_conversion_and_purchase_event_snippets() function, but there are some E2E tests at /tests/e2e/specs/gtag-events/gtag-events.test.js that could be updated to make sure EC data is added.
joemcgill
changed the title
Update phone number Co-authored-by: Joe McGill <[email protected]>
…s://github.com/woocommerce/google-listings-and-ads into update/52310672-google-tag-enhanced-conversion
ankitrox
changed the base branch from
feature/2239-enhanced-conversions-panel-e2e
to
feature/2239-enhanced-conversions-panel
…310672-google-tag-enhanced-conversion
QA/Test Report- ✅Testing Environment -
Test Results -
Functional Demo / Screencast -
|
joemcgill
force-pushed
the
update/52310672-google-tag-enhanced-conversion
branch
from
c5a40fd
to
1afe1d5
Compare
There was a problem hiding this comment.
@mikkamp This PR is ready for an initial review. This only adds the enhanced conversion data to the gtag data during checkouts, which will be integrated into the rest of #2242.
I've updated the testing instructions in the PR details.
We've been able to confirm this is working the way we think it's supposed to work while testing locally during QA, but I'm having trouble getting the E2E tests working and am finding them difficult to debug since the test environment doesn't seem to be set up locally the same way they are set up for the CI Workflow. If you have any advice, I can work on getting those fixed up.
There was a problem hiding this comment.
Thanks for implementing the code, it works as outlined in the PR description. However I think it's important that it works for Guest orders as well.
Also I'm not able to actually confirm that the data gets sent with the conversion event. What else am I missing to get the data to be sent? It's something we should validate in the E2E test as well.
src/Google/GlobalSiteTag.php
Outdated
| $customer = new WC_Customer( $order->get_customer_id() ); | ||
| $email = $customer->get_billing_email(); | ||
| $fname = $customer->get_billing_first_name(); | ||
| $lname = $customer->get_billing_last_name(); | ||
| $phone = $customer->get_billing_phone(); | ||
| $billing_address = $customer->get_billing_address(); | ||
| $postcode = $customer->get_billing_postcode(); | ||
| $city = $customer->get_billing_city(); | ||
| $region = $customer->get_billing_state(); | ||
| $country = $customer->get_billing_country(); |
There was a problem hiding this comment.
Can you clarify why we need to get this from the customer directly. I just tested this with a guest user and since in those cases the customer_id is 0 it doesn't get any details. The billing details can also be fetched directly from the order. Since the order details are about the actual purchase and not the ones stored on the customer account, I think it would be more accurate to get them from the order anyway.
There was a problem hiding this comment.
Thank you. I have changed this to use $order object instead of customer to add the data which would be more accurate as you mentioned.
|
|
||
| $purchase_user_data_gtag = sprintf( 'gtag("set", "user_data", %s)', wp_json_encode( $ec_data ) ); | ||
|
|
||
| $this->add_inline_event_script( $purchase_user_data_gtag ); |
There was a problem hiding this comment.
This is working, as in it's adding the snippet to the page:
gtag( "set", "user_data", {
"sha256_email_address": "2eb9d564b813d572e4ed03bb5a58459a93caa71b7b227338b682fd038b291a3d",
"sha256_phone_number": "422ce82c6fc1724ac878042f7d055653ab5e983d186e616826a72d4384b68af8",
"address": {
"sha256_first_name": "96d9632f363564cc3032521409cf22a852f2032eec099ed5967c0d000cec607a",
"sha256_last_name": "799ef92a11af918e3fb741df42934f3b568ed2d93ac1df74f1b8d41a27932a6f",
"sha256_postal_code": "ea4ffb75adc0903ab5ff284da63b2abe9f3591415e97226a77751b93acfd5d3e",
"sha256_country": "292c1980ba2805512acfef5d0cf8f43fba5c7b9b73a5a7afad1c37cfacad3c98",
"sha256_street": "6008c26f4452392acb19374bc12a5ec0c360ae17356bce8b786fb128c8720951",
"sha256_city": "11a62c23412b77477a71481aa2dc7323bcc61d076c8449076c4c58a8356c1bb1",
"sha256_region": "18ac3e7343f016890c510e93f935261169d9e3f565436429830faf0934f4f8e4"
}
} )But when I look at the tracking data sent to Google I'm not seeing the valid data being sent. I'm following the instructions: Validate your implementation using Chrome Developer Tools
I can see the regular data about the event:
data: event=conversion
But this part I'm not seeing:
Look for a parameter “em” with a hashed string as the value. The value should start with “tv.1~em” followed by a long string of characters. If you see the "em" parameter, this means that the enhanced conversions tag is picking up and hashing the enhanced_conversion_data object.
How can I confirm it is actually including the data? It seems the E2E tests are also just checking for the user_data being sent in the page, and not for the data being sent in the tracking request. That would make it closer to a unit test if it confirms the data is output in the page, seeing expected data appear in the tracking request like is done for the other tests would be more of an E2E test.
The other way to validate the data is to check the Ads account, but it says I have to wait 48hours for that, so I'll have to get back to you if it ever appears.
There was a problem hiding this comment.
You are right, I can't see the em property as mentioned in the document, but I think if its possible we should wait to check if it is being reflected in ads account after 48 hours as we did all the steps according to the documentation.
src/Google/GlobalSiteTag.php
Outdated
| $ec_data['address']['sha256_postal_code'] = $this->normalize_and_hash( $postcode ); | ||
| $ec_data['address']['sha256_country'] = $this->normalize_and_hash( $country ); |
There was a problem hiding this comment.
The documentation shows there is only a hashed field available for the first_name / last_name. Do the other address fields just not have updated documentation?
This documentation specifically mentions not to hash it: https://developers.google.com/google-ads/api/docs/conversions/enhanced-conversions/web#prepare-data
There was a problem hiding this comment.
@mikkamp Thank you for catching that. As per documentation, it seems that we need to normalize and has
- First name
- Last name
- Phone number
- Email address
Rest of the fields have been changed to the raw format (without hash).
src/Google/GlobalSiteTag.php
Outdated
| * | ||
| * @return string | ||
| */ | ||
| private function normalize_and_hash( string $value, $algo = 'sha256' ): string { |
There was a problem hiding this comment.
Does this function apply all the hashing rules? Looking at this documentation: https://developers.google.com/google-ads/api/docs/conversions/enhanced-conversions/web#php
This one would be missing:
Remove all periods (.) that precede the domain name in gmail.com and googlemail.com email addresses
There was a problem hiding this comment.
Added the normalization to remove any periods in the email address if has domain gmail.com or googlemail.com
User order object to add the data
|
@ankitrox and @mikkamp, I gave this another test and noticed a few things that got messed up during the previous merge conflict resolution, which I've resolved in c2d73c2. From what I can tell, all of the data is being hashed correctly now, per the docs, but I am also not able to verify the presence of an One of the assumptions that this implementation is based on is that Google will automatically enable enhanced conversions if the Data Terms and Conditions has been accepted and a conversion tag is sent that includes |
There was a problem hiding this comment.
Thanks for the additional changes. I agree that everything is now working as intended, the code is working as expected.
The only minor comment I had left is that the function maybe_display_conversion_and_purchase_event_snippets is now over a 150 lines long. To add the enhanced conversion data we really only need the order object, so all that code can easily be moved to a separate function (including the call to add_inline_event_script). That would also allow returning early from the function if ( ! $this->is_enhanced_conversion_enabled() ), which would save a whole lot of indenting and make the code a bit easier to grasp (as well as unit test if we'd resort to that).
That's not a blocker for this PR though so I'll go ahead and approve it.
I did manage to get the enhanced conversion data to show up in the end, but to do so I must enable it in the Ads account (the method I choose doesn't seem to matter, but as soon as I disable it, the data is missing again).
With it enabled I can confirm that the hashes match and the additional address fields are also sent without hashes:
gtag( "set", "user_data", {
"sha256_email_address": "2eb9d564b813d572e4ed03bb5a58459a93caa71b7b227338b682fd038b291a3d",
"sha256_phone_number": "422ce82c6fc1724ac878042f7d055653ab5e983d186e616826a72d4384b68af8",
"address": {
"sha256_first_name": "96d9632f363564cc3032521409cf22a852f2032eec099ed5967c0d000cec607a",
"sha256_last_name": "799ef92a11af918e3fb741df42934f3b568ed2d93ac1df74f1b8d41a27932a6f",
"postal_code": "D02 AF30",
"country": "IE",
"street": "Street",
"city": "City",
"region":"D"
}
} )data: event=conversion
em: tv.1~
em.2eb9d564b813d572e4ed03bb5a58459a93caa71b7b227338b682fd038b291a3d~
pn.422ce82c6fc1724ac878042f7d055653ab5e983d186e616826a72d4384b68af8~
fn0.96d9632f363564cc3032521409cf22a852f2032eec099ed5967c0d000cec607a~
ln0.799ef92a11af918e3fb741df42934f3b568ed2d93ac1df74f1b8d41a27932a6f~
sa0.YAjCb0RSOSrLGTdLwSpewMNgrhc1a86LeG-xKMhyCVE~
ct0.city~
pc0.D02 AF30~
rg0.d~
co0.IE
That seems to show that we are still missing hashing the street, which the documentation isn't consistent about. I'm also curious why the hashes look very different, as far as I can see we are following the documentation to the T, so we might need to ask Google why that's happening.
While that's not a blocker for this PR, it is something that needs to be addressed in #2242 As there it's the expectation that users only accept the terms and conditions. They aren't required to enable the Enhanced Conversions in the Ad account. I also believe Google mentioned there isn't a way to automatically enable it through the API. So that needs to be discussed a bit more otherwise it still won't track anything for users that don't enable it.
|
@mikkamp thank you for double checking the I also agree with you about the potential code cleanup for the I'm going to go ahead and merge this and then we can address any follow-up tasks related to the EC values not being passed in follow-up PRs. |
joemcgill
merged commit ed9b948
into
feature/2239-enhanced-conversions-panel
Changes proposed in this Pull Request:
Related to #2216.
{ 'allow_enhanced_conversions': true }to the global site tag config (see this article).Screenshots:
See testing screenshots in this QA report.
Detailed test instructions:
Steps (when enabled):
Expected Result:
Additional details:
Changelog entry