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

Update existing tag handling for Google Tag support #6374

Closed
felixarntz opened this issue Jan 5, 2023 · 10 comments
Closed

Update existing tag handling for Google Tag support #6374

felixarntz opened this issue Jan 5, 2023 · 10 comments
Labels
Exp: SP Module: Analytics Google Analytics module related issues P0 High priority Type: Enhancement Improvement of an existing feature

Comments

@felixarntz
Copy link
Member

felixarntz commented Jan 5, 2023

Minor adjustments will be needed to support existing tags in combination with the new Google Tag (also called GTE). Site Kit will not support every single edge-case there as that would be a maintenance nightmare for little benefit, particularly it will not cater for the combinations that also take into account Google Tag Manager (if that module is active).


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • When an existing GA4 tag ("G-") is found during the Analytics setup or settings, the logic to handle it should be slightly adjusted:
    • The getGoogleTagContainer selector should be used, passing the existing tag to it (which effectively is the measurement ID).
    • If the Google Tag container returned by that selector includes that same "G-" ID as one of the tagIds, then Site Kit should continue to behave as today (if the user chooses the matching property, set the toggle to place the snippet off by default).
    • If the Google Tag container returned by that selector does not include the same "G-" ID as one of the tagIds, we are limited by the lack of an API endpoint, which means we can't reasonably determine information on the actual Google Tag with that "G-" tag ID. In other words, in this scenario we should entirely ignore the existing tag and act as if there was none.
  • While the additional lookup and logic is running, the relevant parts of the UI should remain in loading state.

For some background context on why the ACs do not include anything around "GT-" tags: We don't need to consider "GT-" tags (the new Google tag ID format) here at all, since any such tags will automatically come with the new Analytics feature to avoid duplicate tracking. Therefore there is no risk in double tagging in such a case, and as such we don't need to look for those tags in the page.

Implementation Brief

The solution here as discussed with @felixarntz and @aaemnnosttv is to override the GA4 getExistingTag() resolver, and encapsulate the described checks for a valid Google Tag within it.

Within assets/js/modules/analytics-4/datastore/tags.js:

  • If the gteSupport feature is enabled, override the *getExistingTag() resolver that is in existingTagStore.resolvers with a new implementation. This should be copied from the existing implementation in createExistingTagStore(), and modified as follows:
    • Having retrieved existingTag via fetchGetExistingTag(), retrieve the Google Tag container for the existing tag via the getGoogleTagContainer() selector - wait for the selector to resolve via yield Data.commonActions.await( registry.__experimentalResolveSelect( ....
    • If the existing tag is included in the container's tagIds array, dispatch the receiveGetExistingTag() action with the existing tag.
    • Otherwise, as no valid tag is found, dispatch receiveGetExistingTag() with the value null.

Test Coverage

  • Add test coverage for the extended behaviour of the GA4 getExistingTag() selector and resolver.
  • Update any failing tests.

QA Brief

  • Set up Site Kit with the gteSupport feature flag enabled.
  • Add an existing GA4 tag to the site by adding the gtag snippet to the theme header file.
  • Ensure this GA4 tag has a corresponding Google Tag, where the Google Tag has a tag ID and a destination measurement ID that both match the GA4 tag. This is the default configuration for a newly created tag.
  • Verify the tag is recognised as an existing tag, and Site Kit's existing GA4 tag behaviour continues to work as it does now.
  • Repeat the above setup process, but for the following scenarios:
    • Using a GA4 tag which the user doesn't have permission to use.
    • Using a GA4 tag where the measurement ID has been removed as a destination from the container and assigned to another container which doesn't have a matching tag ID.
  • In each of these cases, the tag should not be recognised as an existing tag and Site Kit should behave as though no existing tag is present.

Changelog entry

  • Update logic for handling Google Tag detection.
@felixarntz felixarntz added P0 High priority Type: Enhancement Improvement of an existing feature Module: Analytics Google Analytics module related issues labels Jan 5, 2023
@felixarntz felixarntz assigned felixarntz and unassigned felixarntz Jan 5, 2023
@hussain-t hussain-t self-assigned this Jan 12, 2023
@mxbclang
Copy link

mxbclang commented Feb 6, 2023

@hussain-t Checking in on this one as it's a P0 and it looks like it's been assigned to you for some time. Thanks!

@hussain-t
Copy link
Collaborator

@bethanylang will complete this as soon as possible. Thanks!

@hussain-t hussain-t removed their assignment Feb 7, 2023
@tofumatt tofumatt self-assigned this Feb 14, 2023
@tofumatt
Copy link
Collaborator

IB ✅

@tofumatt tofumatt removed their assignment Feb 14, 2023
@hussain-t hussain-t assigned hussain-t and unassigned hussain-t Feb 15, 2023
@techanvil techanvil self-assigned this Feb 27, 2023
@techanvil
Copy link
Collaborator

Having discussed this with @felixarntz, the IB needs a rework and I've moved the issue back into the IB column to address this.

@techanvil techanvil removed their assignment Mar 2, 2023
@aaemnnosttv aaemnnosttv self-assigned this Mar 3, 2023
@aaemnnosttv
Copy link
Collaborator

@techanvil the IB LGTM, the one thing we might want to add is a resolver for the new getExistingTag selector to preserve the "interface" of a selector that is compatible with resolveSelect?

@aaemnnosttv aaemnnosttv assigned techanvil and unassigned aaemnnosttv Mar 3, 2023
@techanvil
Copy link
Collaborator

Thanks @aaemnnosttv - sounds good to me. I've updated the IB, please take a look.

@techanvil techanvil assigned aaemnnosttv and unassigned techanvil Mar 3, 2023
@aaemnnosttv
Copy link
Collaborator

Thanks for expanding on that @techanvil, one more thought –

I wonder if it wouldn't be simpler to override the getExistingTag resolver rather than renaming it and reimplementing a new one as the old one would still fetch and receive the existingTag state which would be no longer used in this case. The existing resolver follows the standard fetch and receive if not present convention so this seems like it would be perhaps more straightforward?

The overriding of the resolver could be conditional based on the feature flag as well. What do you think?

@aaemnnosttv aaemnnosttv assigned techanvil and unassigned aaemnnosttv Mar 3, 2023
@techanvil
Copy link
Collaborator

Thanks @aaemnnosttv, that's a good idea! Have updated the IB again, cheers.

@techanvil techanvil removed their assignment Mar 3, 2023
@aaemnnosttv
Copy link
Collaborator

Great, thanks @techanvil 👍

IB ✅

@aaemnnosttv aaemnnosttv removed their assignment Mar 3, 2023
@sashadoes sashadoes assigned sashadoes and unassigned sashadoes Mar 6, 2023
@techanvil techanvil self-assigned this Mar 6, 2023
@techanvil techanvil removed their assignment Mar 7, 2023
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Mar 8, 2023
@mohitwp mohitwp self-assigned this Mar 13, 2023
@mohitwp
Copy link
Collaborator

mohitwp commented Mar 15, 2023

QA Update ✅

Verified -

  • Tested on dev environment.

  • If the Google Tag container returned by that selector includes that same "G-" ID as one of the tagIds, then Site Kit behaving as today (if the user chooses the matching property, set the toggle to place the snippet off by default).

  • If the Google Tag container returned by that selector does not include the same "G-" ID as one of the tagIds, in this scenario we entirely ignore the existing tag and no notice of existing tag is showing.

  • Tested by placing code snippet in Theme header.

  • For the below cases – the tag is not recognized as an existing tag and Site Kit behaving as no existing tag is present.

      * Using a GA4 tag which the user doesn't have permission to use.
      * Using a GA4 tag where the measurement ID has been removed as a destination from the container and assigned to another container which doesn't have a matching tag ID.
    

image

image

image

image

  • When Place code toggle option is off -

image

  • When Place code toggle option set to ON-

image

  • When User don't have permission to use Google tag -
Recording.259.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Exp: SP Module: Analytics Google Analytics module related issues P0 High priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

8 participants