Fix Tag Manager useExistingTagEffect to properly work in case of primary AMP

[felixarntz]

The latest useExistingTagEffect() implementation always looks at the containerID module setting. However, in case of primary AMP, only the ampContainerID setting is set, and the hook should rely on that in such a case.

---

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

Acceptance criteria

Relying on the feature/existing-tag-simplification branch:

• The modules/tagmanager store should receive a new selector getPrimaryContainerID().
  • If the site is using primary AMP (see core/site isPrimaryAMP() selector), the selector should return the value of getAMPContainerID().
  • Otherwise (i.e. either no AMP or secondary AMP), the selector should return the value of getContainerID().
  • In case any of the relevant selectors are still "loading" (i.e. returning undefined), the new selector should also return undefined.
• The Tag Manager useExistingTagEffect() hook should be fixed to also support AMP. Basically, instead of relying on getContainerID(), it should rely on the above getPrimaryContainerID() so that it works for either case.

Implementation Brief

Any PR for this must be based on and target the feature/existing-tag-simplification branch.

• Using assets/js/modules/tagmanager/datastore/containers.js,
  • Add a new selector getPrimaryContainerID which should do the following:
    • Check if primary AMP is being used by querying the core/site datastore via the isPrimaryAMP selector.
      • If that is the case, then return the value of getAMPContainerID selector of the module.
      • Else return the value of getContainerID selector of the module.
    • Return undefined if the calls to above selectors return undefined, i.e are still in a "loading" state.
• Using assets/js/modules/tagmanager/hooks/useExistingTagEffect.js,
  • Replace getContainerID selector call with getPrimaryContainerID selector to get the container ID.

Test Coverage

• Tests to be added for the new selector.

QA Brief

• Connect AMP Plugin and set it as Primary.
• All the behaviors from Simplify Tag Manager UX for existing tags #4713 should still work like before. See it's QA Brief For Details.
• Also Test WITHOUT AMP Primary.
• Update: To force a site in AMP Primary mode:
  • Install AMP Plugin and set it up as Standard Mode.
  • Add The AMP Existing Tag via a mu-plugin.

<?php

const GTM_CONTAINER_ID = 'GTM-PNW4ZWH'; 

/**
 * Print amp-analytics.
 */
function add_amp_tag_manager() {
	printf(
		'<amp-analytics config="https://www.googletagmanager.com/amp.json?id=%s" data-credentials="include"></amp-analytics>',
		esc_attr( GTM_CONTAINER_ID )
	);
}

add_action(
	'wp_footer',
	function () {
		if ( function_exists( 'is_amp_endpoint' ) && is_amp_endpoint() ) {
			add_amp_tag_manager();
		}
	}
);

add_action( 'amp_post_template_footer', 'add_amp_tag_manager' );

• Go to Tag Manager Setup Page
• Run this code in console.

googlesitekit.data
	.dispatch( 'core/site' )
	.receiveSiteInfo( {
		...( await googlesitekit.data
			.__experimentalResolveSelect( 'core/site' )
			.getSiteInfo() ),
		ampMode: 'primary',
	} );

• Now when the selected AMP Container is the same as the Existing AMP Tag (Selected via dropdown) the snippet toggle will be disabled and vise versa.

Changelog entry

• Update the Tag Manager useExistingTagEffect hook to use AMP container ID when in the primary AMP mode.

[eugene-manuilov]

IB ✔️

[wpdarren]

QA Update: ⚠️

We are unable to QA this ticket until this ticket is merged. On hold.

[wpdarren]

QA Update: ⚠️

Edit: Had a conversation with Arafat re. the behaviour and it's because:

As we're making the site primary in a hacky way, it's pulling the disappearing act on the web container which is not part of the UX. If the primary thing was set from the server side by editing the PHP file, There would only be the AMP container dropdown from the beginning.

@kuasha420 as per Slack, when I run the code only one container dropdown appears which has the AMP selection. The web container has disappeared. The toggle does disable because the AMP selected is the same container ID in my mu-plugin.

Here's what happens when you do not select any options but run the code

amp.mp4

This is what it looks like if you select the account, web and amp container and run the console code.

This doesn't feel like it's expected behaviour, but I know we're hacking this a little so would like to check.

[wpdarren]

QA Update: ⚠️

@kuasha420 just another check before I approve this. This is not specific to primary AMP, but more of an observation. Is there a scenario where a user could have AMP plugin installed and when setting up tag manager, selects only an account and web container, and leaves AMP container blank? I have just tried this and the confirm and continue button is disabled.

I am not overly up with how tag manager works in this scenario.

Any thoughts?

[kuasha420]

@wpdarren It's the same behavior in develop so I think it's correct. You must select (Or create new one/both) to be able to more forward. Cheers!

[wpdarren]

QA Update: ✅

Verified:

With Primary AMP set up as per the QAB:

• Tag Manager Setup and Settings no longer force select and auto select Accounts & container based on existing tag.
• Tag manager Setup shows updated copy when there is Existing Tag.
• When the Primary AMP code was run in the console, the snippet was disabled for existing tag.
• Tag Manager Settings show the new copy when there is existing tag.
• The snippet toggle behaves correctly when using the direct link to access the Edit Settings Form.

Screenshots:

Note: for Secondary AMP I have discovered a few issues, but they are out of scope so will create new ticket.