Refactor `getServiceURL` selectors to use `getAccountChooserURL`

[aaemnnosttv]

Feature Description

In #5453 we introduced a new selector for using the proper service proxy for resolving the appropriate URL for a service and a given user email address.

Until now, this has been implemented manually in our getServiceURL selectors by injecting the user's email into different places in the URL based on the service. This issue is about delegating that part to the AccountChooser service.

---

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

Acceptance criteria

• getServiceURL selectors for all modules should no longer pass an authuser parameter or /u/{email} path segment
  • Instead, these selectors should construct the respective user-agnostic URL for the target and return their result using the new getAccountChooserURL selector (see #5453)
  • One exception here is PageSpeed Insights which does not have a user-specific service URL

Implementation Brief

• Locate the getServiceURL selectors for all services (except pagespeed-insights).
• They can be found in:
  • assets/js/modules/adsense/datastore/service.js
  • assets/js/modules/analytics/datastore/service.js
  • assets/js/modules/optimize/datastore/service.js
  • assets/js/modules/search-console/datastore/service.js
  • assets/js/modules/tagmanager/datastore/service.js
• Refactor the selectors:
  • The query for user email should be removed.
  • If the path parameter is defined, sanitize it and append it to the service baseURI with a # (this should be already available).
  • If the query parameter is defined, use addQueryArgs to add them the baseURI (this should be already available).
  • Completely remove the addition of authuser property.
  • Use the createRegistrySelector's select function to select the core/user store and its getAccountChooserURL() selector with the updated baseURI passed to query the account chooser URL.
  • If the account chooser URL is undefied, return undefined from the selector.
  • Otherwise, return the queried account chooser URL.

Test Coverage

• Update the test cases for the updated getServiceURL selectors. They should be expected to match the return value of getAccountChooserURL.

QA Brief

• Navigate to any component linking to external pages for reports, account creation etc.
• The Module -> AdSense -> Setup -> V2 -> SetupMain -> Account: Create Site component is a good one. (?path=/story/modules-adsense-components-setup-v2-setupmain--setup-account-create-site)
• Control that the Add site to AdSense link uses the new account chooser link structure
  • https://accounts.google.com/accountchooser?continue=<URL>&Email=<Users Email>
• Ensure that the link opens normally if the user is logged in with the same email as the Site Kit user
• Ensure that the link opens the account chooser if the currently logged in Google account is not the same as the one in Site Kit

Changelog entry

• Use Google Account chooser URLs for external service/report URLs.

[tofumatt]

Looks good to me!

IB ✅

[wpdarren]

QA Update: ⚠️

@makiost an observation. If the user is logged in with the same email as the Site Kit user, I am redirected to AdSense but the link takes me to the home page rather than the 'Sites' page which is where I would add a site. Is this expected?

I know this change affects any component linking to external pages for reports, account creation etc, but this jumped out at me. When I checked on the current version of the plugin I am redirected to 'Sites'

Verified:

• The Add site to AdSense link uses the new account chooser link structure https://accounts.google.com/accountchooser?continue=<URL>&Email=<Users Email>

• The link opens the account chooser if the currently logged in Google account is not the same as the one in Site Kit

[wpdarren]

@makiost I would like to get this moved forward today, so please could you let me know if this is an issue with the work on this ticket, or, do you want me to create a new ticket? This is only occurring in develop branch right now though, so seems to be a regression from somewhere.

[maciejost]

I think we can go ahead with creating a new issue for it. The decoded link seems to be correct: https://www.google.com/adsense/new?source=site-kit&url=example.com#/pub-2833782679114991/sites/my-sites

[wpdarren]

QA Update: ✅

Verified:

• The Add site to AdSense link uses the new account chooser link structure https://accounts.google.com/accountchooser?continue=<URL>&Email=<Users Email>
• The link opens normally if the user is logged in with the same email as the Site Kit user
• The link opens the account chooser if the currently logged in Google account is not the same as the one in Site Kit

Note: As noted here the link opens normally if the user is logged in with the same email as the Site Kit user, but it does not redirect you to the Sites page, instead you are taken to the AdSense home page. I will create a ticket for this because it appears to be a regression.

[aaemnnosttv]

Approval ❌

The service URL for AdSense is not being constructed in the same way as before as noted above. It's now applying the path as a URL hash instead of as part of the URL's path as it was before. This was likely copied and pasted from another module, but not all service URLs are built the same way. We want to keep the service URLs built the same as before, only removing the authuser param, and passing that through the new account chooser URL selector.

In 1.82.0 https://github.com/google/site-kit-wp/blob/24e553b6ef8076507b298c51d24c09619e062b81/assets/js/modules/adsense/datastore/service.js#L55-L66

On main https://github.com/google/site-kit-wp/blob/0a36c78a8ee04838b9f6d61e5789129da9b1b1e4/assets/js/modules/adsense/datastore/service.js#L57-L60

This also seems to affect Search Console.

Full diff of all `service.js` files

diff --git a/assets/js/modules/optimize/datastore/service.js b/assets/js/modules/optimize/datastore/service.js
index 1046c43c8f..c0786c1ce0 100644
--- a/assets/js/modules/optimize/datastore/service.js
+++ b/assets/js/modules/optimize/datastore/service.js
@@ -41,23 +41,28 @@ export const selectors = {
 	 * @return {(string|undefined)} The URL to the service, or `undefined` if not loaded.
 	 */
 	getServiceURL: createRegistrySelector(
-		( select ) => ( state, { path, query } = {} ) => {
-			const userEmail = select( CORE_USER ).getEmail();
+		( select ) =>
+			( state, { path, query } = {} ) => {
+				let serviceURL = 'https://optimize.google.com/optimize/home/';
 
-			if ( userEmail === undefined ) {
-				return undefined;
-			}
-			const baseURI = 'https://optimize.google.com/optimize/home/';
-			const queryParams = query
-				? { ...query, authuser: userEmail }
-				: { authuser: userEmail };
-			const baseURIWithQuery = addQueryArgs( baseURI, queryParams );
-			if ( path ) {
-				const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
-				return `${ baseURIWithQuery }#${ sanitizedPath }`;
+				if ( query ) {
+					serviceURL = addQueryArgs( serviceURL, query );
+				}
+
+				if ( path ) {
+					const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
+					serviceURL = `${ serviceURL }#${ sanitizedPath }`;
+				}
+
+				const accountChooserBaseURI =
+					select( CORE_USER ).getAccountChooserURL( serviceURL );
+
+				if ( accountChooserBaseURI === undefined ) {
+					return undefined;
+				}
+
+				return accountChooserBaseURI;
 			}
-			return baseURIWithQuery;
-		}
 	),
 };
 
diff --git a/assets/js/modules/search-console/datastore/service.js b/assets/js/modules/search-console/datastore/service.js
index 4d748de695..e6be47240f 100644
--- a/assets/js/modules/search-console/datastore/service.js
+++ b/assets/js/modules/search-console/datastore/service.js
@@ -45,22 +45,28 @@ export const selectors = {
 	 * @return {string} The URL to the service.
 	 */
 	getServiceURL: createRegistrySelector(
-		( select ) => ( state, { path, query } = {} ) => {
-			const userEmail = select( CORE_USER ).getEmail();
-			if ( userEmail === undefined ) {
-				return undefined;
-			}
-			const baseURI = 'https://search.google.com/search-console';
-			const queryArgs = { ...query, authuser: userEmail };
-			if ( path ) {
-				const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
-				return addQueryArgs(
-					`${ baseURI }${ sanitizedPath }`,
-					queryArgs
-				);
+		( select ) =>
+			( state, { path, query } = {} ) => {
+				let serviceURL = 'https://search.google.com/search-console';
+
+				if ( query ) {
+					serviceURL = addQueryArgs( serviceURL, query );
+				}
+
+				if ( path ) {
+					const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
+					serviceURL = `${ serviceURL }#${ sanitizedPath }`;
+				}
+
+				const accountChooserBaseURI =
+					select( CORE_USER ).getAccountChooserURL( serviceURL );
+
+				if ( accountChooserBaseURI === undefined ) {
+					return undefined;
+				}
+
+				return accountChooserBaseURI;
 			}
-			return addQueryArgs( baseURI, queryArgs );
-		}
 	),
 
 	/**
@@ -74,25 +80,48 @@ export const selectors = {
 	 * @return {string} The URL to the service.
 	 */
 	getServiceReportURL: createRegistrySelector(
-		( select ) => ( state, reportArgs = {} ) => {
+		( select ) =>
+			( state, reportArgs = {} ) => {
+				const propertyID = select(
+					MODULES_SEARCH_CONSOLE
+				).getPropertyID();
+				const isDomainProperty = selectors.isDomainProperty( state );
+				const referenceSiteURL =
+					select( CORE_SITE ).getReferenceSiteURL();
+				const {
+					page = isDomainProperty
+						? `*${ untrailingslashit( referenceSiteURL ) }`
+						: undefined,
+					...args
+				} = reportArgs;
+
+				const path = '/performance/search-analytics';
+				const query = {
+					page,
+					...args,
+					resource_id: propertyID,
+				};
+
+				return selectors.getServiceURL( state, { path, query } );
+			}
+	),
+
+	/**
+	 * Gets an entity access URL on the service.
+	 *
+	 * @since 1.83.0
+	 *
+	 * @return {string} The entity access URL to the service.
+	 */
+	getServiceEntityAccessURL: createRegistrySelector(
+		( select ) => ( state ) => {
 			const propertyID = select( MODULES_SEARCH_CONSOLE ).getPropertyID();
-			const isDomainProperty = selectors.isDomainProperty( state );
-			const referenceSiteURL = select( CORE_SITE ).getReferenceSiteURL();
-			const {
-				page = isDomainProperty
-					? `*${ untrailingslashit( referenceSiteURL ) }`
-					: undefined,
-				...args
-			} = reportArgs;
-
-			const path = '/performance/search-analytics';
+
 			const query = {
-				page,
-				...args,
 				resource_id: propertyID,
 			};
 
-			return selectors.getServiceURL( state, { path, query } );
+			return selectors.getServiceURL( state, { query } );
 		}
 	),
 
diff --git a/assets/js/modules/tagmanager/datastore/service.js b/assets/js/modules/tagmanager/datastore/service.js
index 8b60f5e39d..6e393bdf07 100644
--- a/assets/js/modules/tagmanager/datastore/service.js
+++ b/assets/js/modules/tagmanager/datastore/service.js
@@ -41,24 +41,28 @@ export const selectors = {
 	 * @return {(string|undefined)} The URL to the service, or `undefined` if not loaded.
 	 */
 	getServiceURL: createRegistrySelector(
-		( select ) => ( state, { path, query } = {} ) => {
-			const userEmail = select( CORE_USER ).getEmail();
+		( select ) =>
+			( state, { path, query } = {} ) => {
+				let serviceURL = 'https://tagmanager.google.com/';
 
-			if ( userEmail === undefined ) {
-				return undefined;
-			}
+				if ( query ) {
+					serviceURL = addQueryArgs( serviceURL, query );
+				}
+
+				if ( path ) {
+					const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
+					serviceURL = `${ serviceURL }#${ sanitizedPath }`;
+				}
+
+				const accountChooserBaseURI =
+					select( CORE_USER ).getAccountChooserURL( serviceURL );
+
+				if ( accountChooserBaseURI === undefined ) {
+					return undefined;
+				}
 
-			const baseURI = 'https://tagmanager.google.com/';
-			const queryParams = query
-				? { ...query, authuser: userEmail }
-				: { authuser: userEmail };
-			const baseURIWithQuery = addQueryArgs( baseURI, queryParams );
-			if ( path ) {
-				const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
-				return `${ baseURIWithQuery }#${ sanitizedPath }`;
+				return accountChooserBaseURI;
 			}
-			return baseURIWithQuery;
-		}
 	),
 };
 
diff --git a/assets/js/modules/adsense/datastore/service.js b/assets/js/modules/adsense/datastore/service.js
index a22827ef64..1b0f5a61e2 100644
--- a/assets/js/modules/adsense/datastore/service.js
+++ b/assets/js/modules/adsense/datastore/service.js
@@ -46,25 +46,28 @@ export const selectors = {
 	 * @return {(string|undefined)} The URL to the service, or `undefined` if not loaded.
 	 */
 	getServiceURL: createRegistrySelector(
-		( select ) => ( state, { path, query } = {} ) => {
-			const userEmail = select( CORE_USER ).getEmail();
-			if ( userEmail === undefined ) {
-				return undefined;
-			}
+		( select ) =>
+			( state, { path, query } = {} ) => {
+				let serviceURL = 'https://www.google.com/adsense/new';
+
+				if ( query ) {
+					serviceURL = addQueryArgs( serviceURL, query );
+				}
+
+				if ( path ) {
+					const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
+					serviceURL = `${ serviceURL }#${ sanitizedPath }`;
+				}
 
-			const baseURI = 'https://www.google.com/adsense/new/u/0';
-			const queryParams = query
-				? { ...query, authuser: userEmail }
-				: { authuser: userEmail };
-			if ( path ) {
-				const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
-				return addQueryArgs(
-					`${ baseURI }${ sanitizedPath }`,
-					queryParams
-				);
+				const accountChooserBaseURI =
+					select( CORE_USER ).getAccountChooserURL( serviceURL );
+
+				if ( accountChooserBaseURI === undefined ) {
+					return undefined;
+				}
+
+				return accountChooserBaseURI;
 			}
-			return addQueryArgs( baseURI, queryParams );
-		}
 	),
 
 	/**
diff --git a/assets/js/modules/analytics/datastore/service.js b/assets/js/modules/analytics/datastore/service.js
index 149430daa1..a54181d59d 100644
--- a/assets/js/modules/analytics/datastore/service.js
+++ b/assets/js/modules/analytics/datastore/service.js
@@ -49,24 +49,28 @@ export const selectors = {
 	 * @return {(string|undefined)} The URL to the service, or `undefined` if not loaded.
 	 */
 	getServiceURL: createRegistrySelector(
-		( select ) => ( state, { path, query } = {} ) => {
-			const userEmail = select( CORE_USER ).getEmail();
+		( select ) =>
+			( state, { path, query } = {} ) => {
+				let serviceURL = 'https://analytics.google.com/analytics/web/';
 
-			if ( userEmail === undefined ) {
-				return undefined;
-			}
+				if ( query ) {
+					serviceURL = addQueryArgs( serviceURL, query );
+				}
+
+				if ( path ) {
+					const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
+					serviceURL = `${ serviceURL }#${ sanitizedPath }`;
+				}
+
+				const accountChooserBaseURI =
+					select( CORE_USER ).getAccountChooserURL( serviceURL );
 
-			const baseURI = 'https://analytics.google.com/analytics/web/';
-			const queryParams = query
-				? { ...query, authuser: userEmail }
-				: { authuser: userEmail };
-			const baseURIWithQuery = addQueryArgs( baseURI, queryParams );
-			if ( path ) {
-				const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
-				return `${ baseURIWithQuery }#${ sanitizedPath }`;
+				if ( accountChooserBaseURI === undefined ) {
+					return undefined;
+				}
+
+				return accountChooserBaseURI;
 			}
-			return baseURIWithQuery;
-		}
 	),
 
 	/**
@@ -80,28 +84,43 @@ export const selectors = {
 	 * @return {(string|undefined)} The service URL.
 	 */
 	getServiceReportURL: createRegistrySelector(
-		( select ) => ( state, type, reportArgs = {} ) => {
-			const accountID = select( MODULES_ANALYTICS ).getAccountID();
-			const internalWebPropertyID = select(
-				MODULES_ANALYTICS
-			).getInternalWebPropertyID();
-			const profileID = select( MODULES_ANALYTICS ).getProfileID();
+		( select ) =>
+			( state, type, reportArgs = {} ) => {
+				const accountID = select( MODULES_ANALYTICS ).getAccountID();
+				const internalWebPropertyID =
+					select( MODULES_ANALYTICS ).getInternalWebPropertyID();
+				const profileID = select( MODULES_ANALYTICS ).getProfileID();
 
-			invariant( type, 'type is required to get a service report URL.' );
+				invariant(
+					type,
+					'type is required to get a service report URL.'
+				);
 
-			if ( ! accountID || ! internalWebPropertyID || ! profileID ) {
-				return undefined;
-			}
+				if ( ! accountID || ! internalWebPropertyID || ! profileID ) {
+					return undefined;
+				}
+
+				const argsSegment = reportArgsToURLSegment( reportArgs );
+				let path = escapeURI`/report/${ type }/a${ accountID }w${ internalWebPropertyID }p${ profileID }/`;
 
-			const argsSegment = reportArgsToURLSegment( reportArgs );
-			let path = escapeURI`/report/${ type }/a${ accountID }w${ internalWebPropertyID }p${ profileID }/`;
+				if ( argsSegment ) {
+					path += `${ argsSegment }/`;
+				}
 
-			if ( argsSegment ) {
-				path += `${ argsSegment }/`;
+				return selectors.getServiceURL( state, { path } );
 			}
+	),
 
-			return selectors.getServiceURL( state, { path } );
-		}
+	/**
+	 * Gets an entity access URL on the service.
+	 *
+	 * @since 1.83.0
+	 *
+	 * @param {Object} state Data store's state.
+	 * @return {string|undefined} The entity access URL to the service.
+	 */
+	getServiceEntityAccessURL: createRegistrySelector(
+		() => ( state ) => selectors.getServiceReportURL( state, 'report-home' )
 	),
 };

[tofumatt]

Not sure the changes require further QA as they weren't flagged in the first round of QA, so moving this back to Approval with @aaemnnosttv. If it should go through QA again we probably should update the QA Brief.

[wpdarren]

@aaemnnosttv @tofumatt please let me know if there's any additional QA to be completed on this ticket. I did reference the odd behaviour of the AdSense and had a note to create a ticket, but didn't realise the urgency of this and should have flagged it outside of the ticket. Happy to take a further look at this if the QAB is updated.

[aaemnnosttv]

@wpdarren please flag regressions like this before approving as this change is not behind a feature flag. It's good that you raised this otherwise it very likely could have been missed.

I'll update the QAB and send this back for a final pass to make sure service URLs still work as before.

[wpdarren]

@aaemnnosttv noted! I'm going to QA this with a fresh brain on Monday morning my time.

[wpdarren]

QA Update: ✅

Verified:

• The deep links work the same as it did before.
• Checked all appropriate modules including AdSense, GTM, and Google Analytics.
• Went through the module setup including all the different statues for AdSense.
• The issue I reported above about the AdSense link not going to Sites now works as expected.