Prevent users from connecting GA4 without access to UA

[aaemnnosttv]

Feature Description

Analytics is a bit of a special case when it comes to modules as it is essentially one but technically implemented as two under the hood. In https://github.com/google/site-kit-wp/issues/4825 we added guardrails to the module settings edit views for modules with service entities to prevent an admin without access to the configured entity to change it.

For GA4, we do the same thing – prevent the user from changing the connection without access. Since older configurations could be setup without GA4, we should additionally require that only admins that have access to the current configured UA entity can connect or change the GA4 connection.

At the same time, we should evaluate if it should even be possible for UA and GA4 to have different module owners, which is currently possible.

---

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

Acceptance criteria

• The Settings Edit screen for Analytics should be modified to prevent an admin from connecting or changing the connected GA4 property without having access to the connected UA entity

  • The loading state should wait for access to be checked for both UA + GA4 (if connected) before showing the main settings form interface

• If GA4 is not connected, the toggle to enable GA4 (not the snippet) should be disabled with an added notice explaining that user needs access to UA in order to connect GA4 (exact copy below). The style here should largely follow what is already in place for the insufficient access restrictions in the settings view. The notice should specifically indicate the owner of the main/UA Analytics module as the point of contact for access.

  {username} configured Analytics and you don’t have access to its configured property. Contact them to share access or setup Google Analytics 4.

• If GA4 is connected, the existing logic in place around insufficient access restrictions for analytics-4 should be amended to also require the user has access to the current UA analytics entity in order to change the connected GA4 entity. In this case, the notice shown should use alternate copy (see below) and have a higher priority over the GA4 notice. E.g. if the user didn't have access to the UA or the GA4 entities, they would only see the notice about UA

  {username} configured Analytics and you don’t have access to its configured property. Contact them to share access or change the configured Google Analytics 4 property.

• The current messages which are shown when a user does not have access to the current configured entity should be amended for consistency

  {username} configured {Analytics} and you don’t have access to its configured property. Contact them to share access or change the configured property.

  Where {Analytics} is either Analytics or Analytics 4 respectively.

Implementation Brief

• In assets/js/modules/analytics/components/settings/SettingsForm.js:
  • Locate the <GA4SettingsControls /> component.
    • Remove the hasModuleAccess prop passed to it.
    • Pass both hasAnalyticsAccess and hasAnalytics4Access props to it.
• In assets/js/modules/analytics/components/common/GA4ActivateSwitch.js:
  • Accept a new prop named disabled.
  • Pass the disabled prop to the <Switch /> component.
• In assets/js/modules/analytics/components/settings/GA4SettingsControls.js:
  • Accept hasAnalyticsAccess and hasAnalytics4Access props to the component instead of hasModuleAccess. Do not replace the usages of hasModuleAccess because we will create a new variable with the same name.
  • Locate the <GA4ActivateSwitch /> component and add the disabled prop to it. The disabled prop should be truthy only if hasAnalyticsAccess is falsey.
  • Check if the analytics-4 module is connected by running the isModuleConnected selector from the core/modules store.
  • Create a new boolean variable named hasModuleAccess. It should be true if analytics-4 is connected, and both hasAnalyticsAccess & hasAnalytics4Access are truthy.
  • Update the <SettingsNotice /> component:
    • Display it if any of the following conditions are met, replacing current condition(s):
      • If analytics-4 is not connected and hasAnalyticsAccess is falsey.
      • If analytics-4 is connected and hasAnalyticsAccess is falsey.
      • If analytics-4 is connected and hasAnalyticsAccess is truthy, but hasAnalytics4Access is falsey.
    • Leaving all the other props in it intact, update the notice prop depending on the above conditions.
      • If the first condition is met, display the first message quoted in the ACs.
      • Otherwise, if the second condition is met, display the second message quoted in the ACs.
      • Otherwise, if the third condition is met, display the third message quoted in the ACs with Analytics 4 replacing {Analytics}.
• In assets/js/components/settings/SettingsActiveModule/Header.js:
  • Display the Connect Google Analytics 4 CTA only if the current user has access to Analytics (UA).
  • In order to check if the current user has access to Analytics (UA), a useSelect hook usage similar to hasAnalyticsAccess from assets/js/modules/analytics/components/settings/SettingsEdit.js can be used.
• In assets/js/modules/analytics/components/settings/SettingsControls.js:
  • Update the notice prop of the <SettingsNotice /> component to match the third message quoted in the ACs with Analytics replacing {Analytics}.
• In assets/js/modules/analytics/components/settings/SettingsForm.stories.js:
  • Replace the Settings w/o module access story with two distinct stories, one with no access to Analytics and the other with no access to Analytics 4.

Test Coverage

• Fix any failing tests. A potential test that could fail is the should render a button to connect GA4 if Analytics is connected but GA4 is not test case in assets/js/components/settings/SettingsActiveModule/Header.test.js.
• Update VRT references if required.

QA Brief

Changelog entry

[techanvil]

IB :white_check_mark:

@nfmohit I made a minor adjustment and fixed a typo which you can see in the edit history, I didn't think these merited the round trip!