Add user locale query parameter in help links to support.google.com.

[adamsilverstein]

Feature Description

When linking to help articles on support.google.com, Site Kit should set the language/locale query parameter (hl) based on the user's WordPress profile language setting (get_user_locale).

The plugin links to help articles on https://support.google.com/ in numerous places:

Some of these are hard coded to use the english version (&hl=en). Instead, all should use hl=[USER_LOCALE].

---

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

Acceptance criteria

• All external help links to support.google.com should include the current user's locale as the hl (language) query parameter.
• Links to support.google.com in JS should be handled via a getGoogleSupportURL selector of the core/site store. It should receive an object with optional path, query and hash arguments. The selector should automatically add the current locale as hl query parameter to the URL returned.
• All usages of links pointing to support.google.com should use the getGoogleSupportURL selector.

Implementation Brief

• Add getGoogleSupportURL to assets/js/googlesitekit/datastore/site/info.js.
  • It should receive an object with optional path, query and hash arguments.
  • Add query and build the url object with the hl parameter using getLocale() from assets/js/util/i18n.js.

    const url = new URL( addQueryArgs( 'https://support.google.com', { ...query, hl: getLocale() } ) )

  • Add the hash url.hash = hash || ''
  • Return url.toString()
• Use the added selector in the following files using the useSelect hook:
  • SetupAccountCreate.js
  • SetupAccountNoClient.js
  • AdSenseLinkCTA.js
  • AnonymizeIPSwitch.js
  • DashboardGoalsWidget.js
  • LegacyAnalyticsDashboardWidgetTopLevel.js
  • OptimizeIDFieldInstructions.js
  • UseSnippetInstructions.js
  • Anywhere else if missed here
• Remove the translation function call on the URL like the one present in UseSnippetInstructions.js . We don't need to translate the URL anymore since we are adding the hl(locale) parameter.

Test Coverage

• Add Test for the added selector in assets/js/googlesitekit/datastore/site/info.test.js

Visual Regression Changes

• No changes

QA Brief

• The links pointing to support.google.com should have a hl query parameter added to them while everything else stays same.
• To verify reset site kit if already linked. Setup again and go to the AdSense setup page and notice the URL of Learn more link in the setup dialog, it should be as explained in point one.
• Other pages with these links should be checked as well.

Changelog entry

• Introduce getGoogleSupportURL selector to core/site store in JS.

[aaemnnosttv]

@adamsilverstein can we change the anchor argument to hash to match the property of a URL? Anchor is easy to confuse with <a href="">I'm an anchor</a>. I realize the hash in the URL matches to an anchor on the page, but I think it would be preferable to use familiar terms.

---

@kostyalmm

Iterate the query object and add them to the user object using url.searchParams.append( <key>, <value> ) if present

Let's not use searchParams.append for this since it will not encode the value. Instead, let's use addQueryArgs as we do elsewhere to unconditionally add whatever args are given, which will also encode the values.

Add the anchor url.hash = anchor if present

This can be done unconditionally, as a URL with an empty hash will not add the #. The only precaution to add here would be to fallback to an empty string for falsy values to ensure the URL doesn't end up with something like #undefined.
i.e. url.hash = anchor || ''

Get the local using getLocale() from assets/js/util/i18n.js and add it as a search param with key as hl

This should be applied with addQueryArgs as mentioned above. i.e. addQueryArgs( href, { ...query, hl: getLocale() } )

One detail which has not been covered here is how some of the support URLs are localized using a translation string, such as in UseSnippetInstructions shown above. In this case, we should replace the use of the translation string with the new selector since the locale will be added automatically, without requiring translators to add it. We also cannot translate dynamic values so it isn't possible to both use the selector and translate the result.

Also, please assign an estimate once the IB is ready for review 🙂

[adamsilverstein]

can we change the anchor argument to hash

That sounds fine to me, I think @felixarntz wrote the AC though :)

[felixarntz]

@aaemnnosttv @adamsilverstein Updated ACs to use hash.

[aaemnnosttv]

@kostyalmm

addQueryArgs( 'https://support.google.com', { ...query, hl: getLocale() } )

We don't need to have the exact code needed here, but just wanted to point out that the path is not included here.

Aside from that, this LGTM. Regarding the estimate, I'm going to bump it up as 3 is a bit on the low side for everything described here, including CR+QA.

IB ✅

[eugene-manuilov]

@kostyalmm

addQueryArgs( 'https://support.google.com', { ...query, hl: getLocale() } )

We don't need to have the exact code needed here, but just wanted to point out that the path is not included here.

Aside from that, this LGTM. Regarding the estimate, I'm going to bump it up as 3 is a bit on the low side for everything described here, including CR+QA.

IB

@aaemnnosttv does it really need to be a selector? It seems to be a pure utility function that has nothing to do with datastores. By making it a selector, we introduce hooks to components that don't need hooks at all. Even if it doesn't look like a big deal, it increases the complexity of those components and the codebase itself.

What do you think if we convert it to be just a utility function instead of a selector? cc @felixarntz @tofumatt

[aaemnnosttv]

@eugene-manuilov it looks like @felixarntz wrote the ACs for this. I agree that as-is that it doesn't seem to need to be part of the datastore, although perhaps the locale will be moved there? I'm not sure, but as it is, it seems like a utility function would be simpler.

[felixarntz]

@aaemnnosttv @eugene-manuilov While this doesn't use anything from the datastore right now, the getLocale() could eventually indeed come from there. I think alongside all other URL selectors it makes sense to have this one in the datastore as well.

This function is based on the locale, a piece of information it receives implicitly (not a parameter), so I think it's another argument to move it to the datastore.

[aaemnnosttv]

@felixarntz would that mean that all of our i18n utility functions would need to move to the datastore then as well or these would require the locale to be passed when called where they don't now? I think it might make sense to keep the locale out of the datastore since it isn't likely to change during the page load like other data the store manages.

[felixarntz]

@aaemnnosttv We can think about that, but even if we moved the locale to the datastore, we could still select from it in i18n functions I guess. Overall I think it doesn't affect the decision here too much, I think it makes sense to have this function in the datastore alongside other URL functions for Google services.

[wpdarren]

QA Update: Pass ✅

Verified:

The links pointing to support.google.com have a hl query parameter added

• AdSense setup page
  
• Other pages with these links
  
• Changed the user language in Settings and made sure the URLs changed
  
• Click the links and made sure they redirected to the correct page in the appropriate language