Remove unused legacy JS code after removal of legacy components and data API

[felixarntz]

Now that all legacy widget components have been removed (see #1997 and #2077) and the legacy data API as well (see #2258), we should go through the JS codebase again (after a similar issue earlier, #2418) and look for unused code that can be removed.

---

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

Acceptance criteria

• Any code including or only intended for use with legacy dashboard widgets or the legacy dataAPI should be removed (example)

Most of the definition here will be searching through the codebase.

• Note: all of this will likely not be used anymore, but if it is please note what it is in the IB with suggested replacement (if any)
• Check through all module components, and review uses of addAction and addFilter to make sure any remaining uses are still valid
• Ensure references to the old /data REST endpoint are removed (e.g in E2E request interception - search for v1/data)
• Review all TODO comments to ensure there are none left in the code that are actionable related to the removed legacy widgets and dataAPI

Implementation Brief

(See #3799 for a PR that removes request interception in E2E tests as a starting point for this issue.)

• Remove all E2E request inceptions for unused v1/data API calls
• Fix these legacy data TODOs:
  • Remove usageContext param from controlCallback in assets/js/modules/tagmanager/datastore/containers.js. This also involves changing both instances of $usage_context = $data['usageContext'] ?: self::USAGE_CONTEXT_WEB; in /Users/matt/Projects/site-kit-wp/includes/Modules/Tag_Manager.php to be $usage_context = $data['usageContext'] ?: array( self::USAGE_CONTEXT_WEB, self::USAGE_CONTEXT_AMP );
  • Remove fallback in includes/Modules/Tag_Manager.php, route GET:tag-permission, always set $container_id = $data['containerID'];
  • Remove redundant comment in tests/e2e/specs/modules/search-console/dashboard-date-range.test.js (TODO The module pages use the Widget API.)
• Remove the following, unused files:
  • assets/js/components/SignIn.js
• Remove addAction and addFilter calls:
  • addFilter( 'googlesitekit.AdSenseDashboardZeroData', 'googlesitekit.AdSenseDashboardZeroDataRefactored'
  • Refactor assets/js/components/legacy-notifications/notification-counter to call getTotalNotifications() immediately on componentDidMount()—remove the addAction() and removeAction() calls.

Test Coverage

• No tests should need removing or changing; existing tests should all pass.

Visual Regression Changes

• N/A

QA Brief

• Ensure assets/js/components/SignIn.js is gone.
• Ensure no calls to 'v1/data' are made.
• Ensure AC are met.

Changelog entry

• Remove unused legacy JS code after removal of legacy components and data API

[aaemnnosttv]

@felixarntz I added ACs here to move this forward as it seems fairly straightforward in its purpose 🙂

Let me know if there's anything else missing when you get back!

[tofumatt]

From what I can see, the entirely of assets/js/components/legacy-notifications/notification-counter is obsolete and can be removed, because it relies on an unused addAction( 'googlesitekit.dataLoaded', 'googlesitekit.dataLoadedGetTotalNotifications' ), neither of which are present elsewhere in the code anymore.

Are all of the legacy notifications set to go? I didn't think they were and couldn't find a ticket saying so, but maybe I missed it. 😅

[aaemnnosttv]

From what I can see, the entirely of assets/js/components/legacy-notifications/notification-counter is obsolete and can be removed, because it relies on an unused addAction( 'googlesitekit.dataLoaded', 'googlesitekit.dataLoadedGetTotalNotifications' ), neither of which are present elsewhere in the code anymore.

Are all of the legacy notifications set to go? I didn't think they were and couldn't find a ticket saying so, but maybe I missed it. 😅

@tofumatt good catch – this is actually something that seems to have been missed when removing the dataAPI. Legacy notifications are still relevant and don't have a replacement yet so we need to keep these around for now.

Regarding the action you referenced, this can be removed and the total can be fetched right away as there is no longer a collective batch request that is needed to wait for

site-kit-wp/assets/js/components/legacy-notifications/notification-counter.js

Lines 60 to 62 in d77d4fc

	getTotalNotifications().then( ( count ) => {
	this.setState( { count } );
	} );

_googlesitekitLegacyData.setup.errorMessage to use select( CORE_SITE ).getInternalServerError()

This error message still needs to come from the server though to map the error code we get in the URL to the error message by the oAuth client

site-kit-wp/includes/Core/Authentication/Authentication.php

Lines 795 to 800 in 03a2586

	if ( $this->credentials->using_proxy() ) {
	$error_code = $this->user_options->get( OAuth_Client::OPTION_ERROR_CODE );
	if ( ! empty( $error_code ) ) {
	$data['errorMessage'] = $auth_client->get_error_message( $error_code );
	}
	}

If it was only used in the client, we could maybe move this to the client side but that's a bit out of scope for this issue, although something to potentially consider in the future.

• Remove usageContext param from controlCallback in assets/js/modules/tagmanager/datastore/containers.js

As noted in the TODO comment there, we can't do that without also changing the default returned by the datapoint handler on the server side as it currently defaults to web context only if not provided. This issue is about removing unused JS so I would say this is probably out of scope, but it looks like a safe change to make. @felixarntz ?

Remove assets/js/modules/analytics-4/index.js

Not sure why this was added but I'm guessing due to the use of the googlesitekit.SetupWinNotification-{slug} filter? This filter is still used in DashboardSetupAlerts.

Even if that were removed, we wouldn't remove the index module for a module because it provides important exports (e.g. registerStore) that we expect to be there both in our test utils and in the bundle entrypoint.

Remove assets/js/modules/optimize/index.legacy.js

As mentioned before the googlesitekit.SetupWinNotification-{slug} filter is still in use so we can't remove this yet.

Remove assets/js/components/legacy-notifications/notification-counter

As mentioned above, we should remove the use of the unused action from this component but the counter module and legacy notifications

[tofumatt]

Ah, I thought that controlCallback was a safe change to make but didn't realise the PHP part wasn't already done. I've added that bit to the IB and removed some of the more aggressive changes or changes to files I thought weren't in use but I missed their imports.

Should be ready for another look, thanks! 👍🏻

[aaemnnosttv]

IB ✅

[felixarntz]

@tofumatt @aaemnnosttv IB also LGTM from my end - I think it's reasonable to make the one Tag Manager API change as well, even though that is technically a breaking change on the server-side since we're changing the default. Should be okay this time though.

[eugene-manuilov]

@ivankruchkoff, please, add QAB.

[johnPhillips]

QA ✅

• I searched the codebase for v1/data and found no references
• Verified that assets/js/components/SignIn.js is gone
• There are no remaining instances of addAction in the codebase that I could find.
• I also searched the codebases for todo (i.e. for mention of legacy code tasks) and addFilter and I discussed my findings with @tofumatt. We concluded the remaining instances are still valid and not related to this issue.