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

Enhanced conversion email/phone prepping before hashing #321

Closed
StSaens opened this issue Feb 3, 2024 · 10 comments
Closed

Enhanced conversion email/phone prepping before hashing #321

StSaens opened this issue Feb 3, 2024 · 10 comments

Comments

@StSaens
Copy link

StSaens commented Feb 3, 2024

Hi,
I glanced at some code changes you made for enhanced conversion reporting. As an FYI, you can't just hash the email and assume it's done.
Google has (almost hidden for whatever reason) a few steps to do before it's hashed. See: https://developers.google.com/google-ads/api/docs/conversions/enhanced-conversions/web

  1. Remove leading/trailing whitespaces.
  2. Convert the text to lowercase.
  3. Format phone numbers according to the E164 standard.
  4. Remove all periods (.) that precede the domain name in gmail.com and googlemail.com email addresses.

Maybe you have implemented this. I only glanced at some changes superficially.
I don't know why they haven't signaled this in various documents more clearly, but it is what it is.

@duracelltomi
Copy link
Owner

Hi,

Thanks for pointing this out.
I am quite sure most of those sanitizations are done while WooCommerce processes the email address.

Have you tried to place an order entering an email address that doesn't fulfill any of those requirements?

@StSaens
Copy link
Author

StSaens commented Feb 4, 2024

Hi.
Point 2 and 4 are for sure not done by woo.

@duracelltomi
Copy link
Owner

Thanks!

v1.20 will be released tomorrow, although this code seems to me a low risk change, I will add it into either 1.20.1 or 1.21

@morvy
Copy link
Contributor

morvy commented Feb 4, 2024

@StSaens @duracelltomi how should be an empty email handled? Now even empty string is hashed and empty string does generate a hash, so I believe this is not correct too.

I have this in my "hotfixes":

add_filter( 'gtm4wp_compile_datalayer', function($dataLayer) {
    if (empty(trim($dataLayer['customerBillingEmail']))) {
	$dataLayer['customerBillingEmailHash'] = "";
    }

    return $dataLayer;
}, 99, 1 );

@duracelltomi
Copy link
Owner

Partly done: 4aad785

TODO:

  • Format phone numbers according to the E164 standard.
  • Remove all periods (.) that precede the domain name in gmail.com and googlemail.com email addresses.

@duracelltomi
Copy link
Owner

... and 3f141f0

@duracelltomi
Copy link
Owner

I've added better normalization into the next version:
a08afa9

However I have doubts adding phone number formatting as it seems to be a more complex code that I though. I've found this library but adding this to GTM4WP would be a large addition for one specific feature:
https://github.com/giggsey/libphonenumber-for-php

Currently, I think some sort of normalization should be added into WooCommerce itself so that all plugins can benefit from the results.

@StSaens
Copy link
Author

StSaens commented Mar 12, 2024

Great that the email formatting is implemented. Happy with that. Should be enough for most eCommerce shops.

With regards to the phone nr. That will mean that, unless the customer inputs it exactly right, Google won't be able to match that hash for enhanced conversion tracking. Maybe this should be clarified somewhere in gtm4wp docs.

@duracelltomi
Copy link
Owner

@morvy customerBillingEmailHash is not the right variable to use.

https://gtm4wp.com/google-tag-manager-for-woocommerce/enhanced-conversions-for-google-ads-with-woocommerce-how-to-setup

Variables in orderData are filtered to prevent hashing of empty strings.

I will fix this with customerBillingEmailHash too

@duracelltomi
Copy link
Owner

@morvy 42ef9d2

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

No branches or pull requests

3 participants