Remove Individual Module pages

[tofumatt]

Feature Description

As per the Design Doc, we should remove all Module pages and the WP sidebar links to each module page.

This before/after from the design doc shows what we should go from and to:

---

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

Acceptance criteria

• When the unifiedDashboard feature flag is enabled: the Module Pages and sidebar links for Analytics, AdSense, and Search Console should be removed. Tag Manager, PageSpeed Insights, Optimize, and Idea Hub don't have dedicated modular pages/links so they aren't included here.
  • When the feature flag above is not enabled, the links/sidebar links should still function.
• The setup flows for the three modules above that currently have their own Module Pages are currently embedded into these respective pages. When the unifiedDashboard feature flag is enabled, this should be changed accordingly so that they use the regular googlesitekit-dashboard screen as "embedding screen" (similar to how the other modules do it).
  • It should be verified that these setup flows still work as expected as part of QA. There is no expected breakage since only the URL location changes, but it needs to get some basic testing.

Implementation Brief

JavaScript:

• Extract the GoogleSitekitDashboard component from assets/js/googlesitekit-dashboard.js into a new file, assets/js/components/DashboardEntryPoint.js, also renaming the component to DashboardEntryPoint. This will facilitate testing the component.
• Within the DashboardEntryPoint component, move the conditional render of ModuleSetup above the conditional render of DashboardMainApp, i.e. move lines 48-50 above 44-46 as seen here. This is to enable the module setup flow on the dashboard screen when unifiedDashboard is enabled:
  

  site-kit-wp/assets/js/googlesitekit-dashboard.js

  Lines 44 to 50 in 8616abb

  	if ( unifiedDashboardEnabled ) {
  	return <DashboardMainApp />;
  	}
  	if ( !! setupModuleSlug ) {
  	return <ModuleSetup moduleSlug={ setupModuleSlug } />;
  	}

• In assets/js/modules/adsense/datastore/base.js, ensure the adminPagesetting is not defined when the unifiedDashboard feature flag is enabled. Use import { isFeatureEnabled } from '../../../features' and call isFeatureEnabled( 'unifiedDashboard' ) to query the feature.
  

  site-kit-wp/assets/js/modules/adsense/datastore/base.js

  Line 40 in 1a487ae

  adminPage: 'googlesitekit-module-adsense',

• Similarly ensure the adminPage setting is not defined when the unifiedDashboard feature flag is enabled in both assets/js/modules/analytics/datastore/base.js and assets/js/modules/search-console/datastore/index.js.

PHP:

• In includes/Modules/AdSense.php, ensure the register_screen_hook method is not invoked when the unifiedDashboard feature flag is enabled. Use use Google\Site_Kit\Core\Util\Feature_Flags; and call Feature_Flags::enabled( 'unifiedDashboard' ) to query the feature.
  

  site-kit-wp/includes/Modules/AdSense.php

  Line 76 in 1a487ae

  $this->register_screen_hook();

• Similarly ensure the register_screen_hook is not invoked when the unifiedDashboard feature flag is enabled in both includes/Modules/Analytics.php and includes/Modules/Search_Console.php.

Test Coverage

JavaScript:

• Add a test for the DashboardEntryPoint component to verify the module setup flow is active when the unifiedDashboard feature is both enabled and disabled. This will require adding a new test file.
• Add tests for assets/js/modules/adsense/datastore/base.js, assets/js/modules/analytics/datastore/base.js and assets/js/modules/search-console/datastore/index.js to check the adminPage setting is defined correctly depending on the unifiedDashboard feature status. These will each require adding a new corresponding test file.

PHP:

• Add tests for includes/Modules/AdSense.php, includes/Modules/Analytics.php and includes/Modules/Search_Console.php to check the screen is not registered when the unifiedDashboard feature is enabled. These files have corresponding PHPUnit test files which can be updated.

QA Brief

• Install the Tester Plugin and ensure the Unified Dashboard feature is enabled:
  
• Run through the SiteKit setup flow.
• Connect the Analytics, AdSense, and Search Console modules.
• The setup flow for these modules should now be embedded in the Dashboard. This is evident by the Dashboard menu item being highlighted and page=googlesitekit-dashboard being present in the URL during setup:
  
• Verify the the sidebar links no longer appear for these modules, as per the visual in the Feature Description.
• As discussed in the comments, the implementation involved enabling the module setup for all modules when Unified Dashboard is enabled, so it's worth running through the other modules' setup flows too.

Changelog entry

• Hide individual module pages when the unifiedDashboard flag is enabled.

[felixarntz]

@tofumatt This needs to consider a few more implications that removing the module screens has (at least it has one): The module setups for modules that have a module screen currently rely on that screen to render the module setup. I think replacing that is trivial since we have the same infrastructure on the dashboard, but it needs to be included. The module screens after referenced in JS and are used e.g. to redirect to the module setup, which would break if we just removed the server-side setup for it. We can probably just use googlesitekit-dashboard throughout all modules, like is already the default for other modules that don't have their own screen.

[felixarntz]

@tofumatt I completed the ACs here with a notion about the setup flows for those 3 modules.

[techanvil]

@tofumatt @felixarntz, while working on the IB I have noticed, there seems to be an assumption in the AC that the setup for Tag Manager, PageSpeed Insights etc will work when the unifiedDashboard feature is enabled.

As it stands that is not the case - the modules which currently are setup on the googlesitekit-dashboard screen can only be setup when unifiedDashboard is not enabled. This is due to the following:

site-kit-wp/assets/js/googlesitekit-dashboard.js

Lines 41 to 50 in 8616abb

	const GoogleSitekitDashboard = ( { setupModuleSlug } ) => {
	const unifiedDashboardEnabled = useFeature( 'unifiedDashboard' );
	if ( unifiedDashboardEnabled ) {
	return <DashboardMainApp />;
	}
	if ( !! setupModuleSlug ) {
	return <ModuleSetup moduleSlug={ setupModuleSlug } />;
	}

As you can see, when the FF is enabled the DashboardMainApp is always returned, meaning ModuleSetup, which is what's responsible for the setup flow, is never rendered.

My question is, what should we do in this instance - do you think it would be best to enable the setup for Tag Manager, PageSpeed Insights etc on the unified dashboard as a separate item before tackling this issue?

--

Addendum:

Here's a bit more info I have found on the possible implementation for enabling the setup:

Assuming we want to reuse ModuleSetup when unifiedDashboard is enabled then simply moving lines 48-50 above lines 44-46 would enable the setup flow, by the look of it. However it's not a complete solution - the DashboardMainApp component is also missing the DashboardNotifications component which is responsible for showing the success notification when the setup is complete, i.e.:

We could of course add DashboardNotifications to DashboardMainApp if we want to reuse the component.

I'm not sure if there are any other aspects that need to be considered.

[tofumatt]

DashboardMainApp missing notifications is fine (for now). We a ticket open for that issue: #4168.

Aside from that, your proposed solution of restoring the setup flow to GoogleSitekitDashboard makes sense to me.

Making that part of the IB here makes sense to me, if that's all it takes to restore the setup functionality. 👍🏻

[techanvil]

Thanks @tofumatt - sounds good to me, I will include the proposed change to assets/js/googlesitekit-dashboard.js in the IB.

[felixarntz]

@techanvil Good catch, indeed moving the condition for displaying ModuleSetup up is the proper solution here. This should work regardless of whether or not the unifiedDashboard feature is enabled. The lack of a success Notifications component will be resolved separately in #4168.

The IB mostly looks solid. I'm only unsure about the name of the GoogleSitekitDashboard component now that it is being moved into assets/js/components. That name was somewhat irrelevant as long as it was in the separate entry point file, but now that it becomes a "proper" component with its own file and tests, maybe we should reconsider the name, to bring some sense to it (e.g. what does this component do vs just DashboardApp and DashboardMainApp?). Maybe we can name it something like DashboardEntrypoint?

(@eugene-manuilov I see you had marked this but since I was already pinged here I just reviewed the IB)

[techanvil]

@felixarntz thanks for the review, and I agree with your point about renaming GoogleSitekitDashboard. I think DashboardEntrypoint is a good name, and have updated the IB accordingly. Cheers!

[felixarntz]

IB ✅

[wpdarren]

QA Update: ✅

Verified:

With unifiedDashboard enabled

• Successfully connected all modules including Optimize and Tag Manager.
• The sidebar links no longer appear for these modules.

With unifiedDashboard disabled

• As a sanity check. Successfully connected all modules including Optimize and Tag Manager.
• Sidebar links appeared for the modules. The module pages work as expected.