ConversionsAPI-Tag-for-GoogleTagManager icon indicating copy to clipboard operation
ConversionsAPI-Tag-for-GoogleTagManager copied to clipboard

Published 20 hours ago • verified publisher icon facebookincubator

Pull the city, state and country information directly from the request Headers

Open lucianfialho opened this issue 1 year ago • 5 comments

My proposal is to pull the city, state and country information directly from the request Headers made available by Google App Engine.

This will give the option for those who are unable to pull this information directly from the client side and still capture this data.

I saw Simo talking about this in some posts and decided to embed it within the model.

For this to work, I added the read_request permission by analyzing specific headers like:

  • x-appengine-city
  • x-appengine-country
  • x-appengine-region

Additionally, I added an option (OverrideWithAppEngineGeoDataFromHeaders) within the fields so that the user can select if they want to use Google App Engine Headers.

If the user selects the option, I rewrite the addressData object passing the information acquired from the headers

lucianfialho avatar Oct 08 '23 17:10 lucianfialho

I think it may make more sense to add this as a fallback option rather than an override option. The AppEngine header values would be inferred from the user's IP (which could be very inaccurate if they are using a VPN or Apple's relay, for example), vs if the addressData object has values that would be 1st party data and therefore more reliable. — Kevin O'Connor, Digital Marketing Analyst www.iDimension.com http://www.idimension.com/?utm_id=VwkC +1 (866) 524.3733

On Sun, Oct 8, 2023 at 1:19 PM Lucian Fialho @.***> wrote:

My proposal is to pull the city, state and country information directly from the request Headers made available by Google App Engine.

This will give the option for those who are unable to pull this information directly from the client side and still capture this data.

I saw Simo talking about this in some posts and decided to embed it within the model.

For this to work, I added the read_request permission by analyzing specific headers like:

  • x-appengine-city
  • x-appengine-country
  • x-appengine-region

Additionally, I added an option (OverrideWithAppEngineGeoDataFromHeaders) within the fields so that the user can select if they want to use Google App Engine Headers.

If the user selects the option, I rewrite the addressData object passing the information acquired from the headers

You can view, comment on, or merge this pull request online at:

https://github.com/facebookincubator/ConversionsAPI-Tag-for-GoogleTagManager/pull/45 Commit Summary

File Changes

(1 file https://github.com/facebookincubator/ConversionsAPI-Tag-for-GoogleTagManager/pull/45/files )

Patch Links:

https://github.com/facebookincubator/ConversionsAPI-Tag-for-GoogleTagManager/pull/45.patch

https://github.com/facebookincubator/ConversionsAPI-Tag-for-GoogleTagManager/pull/45.diff

— Reply to this email directly, view it on GitHub https://github.com/facebookincubator/ConversionsAPI-Tag-for-GoogleTagManager/pull/45, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE3LD7FFZIT4FBEUKDMAT5DX6LOC3AVCNFSM6AAAAAA5X2EYVSVHI2DSMVQWIX3LMV43ASLTON2WKOZRHEZTCOJTGI2TGOI . You are receiving this because you are subscribed to this thread.Message ID: <facebookincubator/ConversionsAPI-Tag-for-GoogleTagManager/pull/45@ github.com>

koconnor3 avatar Oct 09 '23 13:10 koconnor3

looking at your actual commit, I see that is actually what you have done (other provided address data overwrites the AppEngine values, not the other way around) 👍 maybe just change the wording to reflect that?

koconnor3 avatar Oct 09 '23 13:10 koconnor3

looking at your actual commit, I see that is actually what you have done (other provided address data overwrites the AppEngine values, not the other way around) 👍 maybe just change the wording to reflect that?

Did you say change the name just from the description to replace? To be clearer, is this it? Sorry my english is bad :)

lucianfialho avatar Oct 09 '23 17:10 lucianfialho

no worries, I got that impression from two places:

  1. the option is named Override WithAppEngineGeoDataFromHeaders.
    • "Override" means to replace something, so this made me think you were replacing address values if they already existed
  2. your description noted that you would "rewrite the addressData object passing the information acquired from the headers"
    • similar to "override" above, the use of "rewrite" indicates that the header values would be replacing the values that already existed

The way I would suggest writing those things is to use 'update' instead of 'override':

  • change OverrideWithAppEngineGeoDataFromHeaders to UpdateWithAppEngineGeoDataFromHeaders
  • change "checkboxText": "Override geolocation information directly from AppEngine headers", to "checkboxText": "Update geolocation information directly from AppEngine headers",

koconnor3 avatar Oct 09 '23 18:10 koconnor3

Done :) Thanks for your suggestions @koconnor3

lucianfialho avatar Oct 09 '23 18:10 lucianfialho