Adapt GTM integration with GA to support GA4 about site-kit-wp HOT 11 CLOSED

@bethanylang @ivonac4 As an update, we are waiting to hear back from Mariya and possibly the GTM team on this one.

from site-kit-wp.

@bethanylang I’ve checked the comment you were referring to in Asana and we just want to wait a little bit more before we can finalise the AC for 7990. I believe we are want to wait for the GTM team to liaise with the GA team on whether they've planned to allow adding GA4 tags to a GTM Amp container.

c.c. @aaemnnosttv @marrrmarrr

from site-kit-wp.

@jimmymadon Should this be assigned back to you to update the AC, since we've determined we're not going to support AMP?

from site-kit-wp.

• Since there is no support for adding GA4 tags to an AMP container, we should remove existing infrastructure (datastore selectors and React components) that cater to AMP mode and tags within an AMP container.

Remove the AMPContainerSelect component and all of its usage.

@jimmymadon while GTM doesn't support configuring GA4, removing GTM support for AMP entirely is out of scope for this issue but also isn't something on our foreseeable roadmap. Folks should still be able to set up GTM using AMP as today, they just can't implement GA4 with it, which is fine since Analytics supports it and makes things a bit easier for us actually. This means we'd always check the web container for a Google tag and suggest setting up Analytics regardless of whether or not the site was using AMP or not.

I'm not sure if this significantly impacts the rest of the IB (which LGTM), but anything which was based on this point about removing AMP support for GTM would need to be reverted/revised.

from site-kit-wp.

Hey @nfmohit, the AC is most of the way there, however, as noted in a related comment, the tagId is actually a Google tag ID, so we need to check if any of the corresponding Google tag's containers are a match for the current measurement ID rather than assuming the measurement ID will be tracked directly in the tag ID field.

If not, we'll determine the most relevant measurement ID for the Google tag following similar logic to that used in GoogleTagIDMismatchNotification, although falling back to the first valid measurement ID if none of them exist in the available Analytics accounts (TBC as per the linked comment).

from site-kit-wp.

Hi @techanvil!

I'm handing this over to @jimmymadon as Jimmy actually added the ACs for this one. @jimmymadon Let's sync if needed to discuss the ACR feedback.

Thank you folks!

from site-kit-wp.

Oh, sorry @nfmohit, I should have realised that. Thanks for pointing this in the right direction.

from site-kit-wp.

@aaemnnosttv I've modified the AC here based on our discussions for your approval.

On a side note, I just wanted to confirm if your reply to my comment regarding supporting AMP is still valid? If yes, then these selectors will assume that an AMP container could contain a googtag which, as mentioned above, is impossible for now. The usage within our existing Tag Manager settings messages can still be preserved, but these will never be reached. I'm not sure this is the best way to proceed as we will be keeping a lot of code that will never be reached. If we need to reintroduce support for AMP, we can potentially refer to this code and re-add it then?

c.c. @techanvil @nfmohit

from site-kit-wp.

we are waiting to hear back from Mariya and possibly the GTM team on this one.

This is still in progress, although if it stalls out, I agree with @jimmymadon that we can essentially prune the unused/unreachable AMP code and re-add it later if/when it becomes necessary, but let's hold on this a bit longer.

from site-kit-wp.

I've set this as a 19 as the IB doesn't cover all removal of code that is unreachable and allowing for additional testing of anything missed in the IB. This will have to be done carefully.

from site-kit-wp.

QA Update: ✅

Verified:

• Within the GTM setup, I performed a regression test to verify:
  • creating a new account
  • creating a new container
  • selecting/saving existing accounts and container
• Performed the same regression tests for editing GTM settings once set up.
• Verified existing Google Tag within a GTM Container:
• Verified the new message as per the AC.
• I edited the tag created above to an invalid Google Tag ID and verified the message does not appear.
• Verified existing Google Tag via a Constant variable within a GTM Container:
• Tested all steps in the QAB and AC.
• I also did some additional testing around AMP on the scenarios highlighted above.
• I also checked the snippets in the source code for GTM with AMP activated and not.

from site-kit-wp.