Tag Manager stuck in loading state when reloading edit settings page.

[techanvil]

Bug Description

When editing the Tag Manager settings, upon reloading the page, the Tag Manager section remains in the loading state.

This is because, for some reason, the Tag Manager's ownerID is not being set and this causes the hasModuleOwnershipOrAccess selector to return undefined. This in turn is a condition for showing the <ProgressBar />.

Note, I also checked this scenario for Analytics but the problem did not occur.

Steps to reproduce

1. Set up Site Kit and connect the Tag Manager module.
2. Navigate to the Tag Manager settings and click Edit.
3. Refresh the page.
4. See the Tag Manager section open and stuck in the loading state.

Screenshots

Additional Context

• PHP Version: any
• OS: any
• Browser: any
• Plugin Version: seen 1.92.0
• Device: any

---

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

Acceptance criteria

• When reloading the Tag Manager edit settings page, it should load normally and not remain stuck in the loading state.

Implementation Brief

• In useGAPropertyIDEffect()

  site-kit-wp/assets/js/modules/tagmanager/hooks/useGAPropertyIDEffect.js

  Line 31 in 5b0c5a9

  export default function useGAPropertyIDEffect() {

  • Load tagmanager settings select( MODULES_TAGMANAGER ).getSettings()
  • In the useEffect only call the setGAPropertyID() function if the tagmanger settings are not falsy.

    useEffect( () => {
    	if ( !! existingSettings && singleAnalyticsPropertyID !== undefined ) {
    		setGAPropertyID( singleAnalyticsPropertyID || '' );

Test Coverage

• Update failing test which not need tagmanager setting setup.
• Add a test case to useGAPropertyIDEffect.test.js to verify setGAPropertyID is not called when select( MODULES_TAGMANAGER ).getSettings() is falsy.

QA Brief

• Follow the "Steps to reproduce" from bug description above.
• Verify the tagmanager settings are not stuck in loading state anymore but load the input fields as expected.

Changelog entry

• Fix bug that caused Tag Manager settings screen to be stuck in a "loading" state.

[derweili]

The error is related to the SettingsEdit component loading the useExistingTagEffect side effect (here)

site-kit-wp/assets/js/modules/tagmanager/hooks/useGAPropertyIDEffect.js

Lines 31 to 42 in 5b0c5a9

	export default function useGAPropertyIDEffect() {
	const singleAnalyticsPropertyID = useSelect( ( select ) =>
	select( MODULES_TAGMANAGER ).getSingleAnalyticsPropertyID()
	);
	const { setGAPropertyID } = useDispatch( MODULES_TAGMANAGER );
	useEffect( () => {
	if ( singleAnalyticsPropertyID !== undefined ) {
	setGAPropertyID( singleAnalyticsPropertyID || '' );
	}
	}, [ singleAnalyticsPropertyID, setGAPropertyID ] );
	}

This side effect triggers an update to the tagmanager datastore which at that time is still empty.

Because other functions which normally would trigger the fetchGetSettings() action are executed after the side effect, the settings are not empty anymore, so the *getSettings() resolver does not call the fetchGetSettings().

site-kit-wp/assets/js/googlesitekit/data/create-settings-store.js

Lines 251 to 261 in 5b0c5a9

	*getSettings() {
	const registry = yield Data.commonActions.getRegistry();
	const existingSettings = registry
	.select( STORE_NAME )
	.getSettings();
	// If settings are already present, don't fetch them.
	if ( ! existingSettings ) {
	yield fetchGetSettingsStore.actions.fetchGetSettings();
	}
	},
	};

Therefore the useExistingTagEffect() hook should be update to only call setGAPropertyID() if getSettings() is not falsy.

[eugene-manuilov]

IB ✔️

[wpdarren]

QA Update: ✅

@techanvil this is a great find!

@mohitwp this is a good one to make a note of for when release testing. I'll add a test case for refreshing all settings.

Verified:

• Went through the the steps to reproduce on the latest version, and could.
• Changed to develop branch and on refresh the loading state does not get stuck anymore.

tm-settings.mp4