site-kit-wp
site-kit-wp copied to clipboard
JS errors if Tag Manager is disabled via googlesitekit_available_modules
Bug Description
Similar to #5072, but for covering the case that the Tag Manager module is not available (e.g. via googlesitekit_available_modules
filter): There will be JS errors at least in the Analytics setup as it unconditionally calls modules/tagmanager
selectors.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Any references to
modules/tagmanager
store selectors or actions from a component or store that is part of another module should only occur after verifying that the module is available (available, regardless of whether it is active or not).- In other words, this issue should cover all accesses within
assets/js/modules/*
other thanassets/js/modules/tagmanager
. - A good way to tackle this is to search for any
MODULES_TAGMANAGER
references in those other module directories and adjust/wrap them accordingly. - At a minimum, the check needs to ensure that the respective
MODULES_TAGMANAGER
datastore is available before calling a selector or action on it.
- In other words, this issue should cover all accesses within
Implementation Brief
- Find all instances of
select( MODULES_TAGMANAGER )
/dispatch( MODULES_TAGMANAGER )
and ensure thatisModuleAvailable( MODULES_TAGMANAGER )
istrue
before using those selectors/dispatching those actions. - Find instances of
isModuleActive( MODULES_TAGMANAGER )
and check forisModuleAvailable( MODULES_TAGMANAGER )
.
Note: any code in any module should be checked, not just Tag Manager specific code. Be sure to look for Tag Manager selectors/action dispatches across the whole plugin when making these changes.
Test Coverage
- Add tests to any components with existing tests that disable Tag Manager; ensure they still render without errors/console warnings.
QA Brief
- Add the filter below to your WordPress site, e.g. in your theme's
functions.php
. - Set up the Analytics module and ensure there are no errors due to Tag Manager not being available.
Filter
add_filter(
'googlesitekit_available_modules',
function( $modules ) {
$index = array_search( 'tagmanager', $modules, true );
if ( false !== $index ) {
unset( $modules[ $index ] );
return array_values( $modules );
}
return $modules;
}
);
Changelog entry
(This can probably follow the approach used in #5071, whatever we decide, so it might be worth waiting on the discussion in that issue before tackling this.)
@hussain-t I see this was assigned a month ago before the IB/approach in #5071 was finalised. Since I wrote up that IB I'm going to handle this and #5072, because there's a lot of overlap 🙂
- Find instances of
isModuleActive( MODULES_TAGMANAGER )
and first check forisModuleAvailable( MODULES_ANALYTICS )
(do the same withMODULES_TAGMANAGER
).
@tofumatt why do we need to check whether the Analytics module is available when we check if the Tag Manager module is active? Perhaps you meant "and first check for isModuleAvailable( MODULES_TAGMANAGER )"?
Also could you please add a note in IB to mention that those changes should be done in other modules that use actions and selectors from the Tag Manager module? Just for clarity. Thanks.
The thinking there was that Tag Manager requires Analytics, so if Analytics is disabled Tag Manager would also be disabled… but you're right, I don't think that's needed and just complicates things. I've removed that 👍🏻
I've added a note to the IB as well, thanks 👍🏻
IB ✔️
QA Update ✅
- Verified on both latest and dev environment using QAB.
- On latest environment I'm able to reproduce issue.
- On dev environment errors are not showing if Tag Manager module is not available.
https://user-images.githubusercontent.com/94359491/199218497-59bdd92f-7bef-4b13-9c96-0e87a969d4f2.mp4