Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent the Analytics 4 module from being displayed unexpectedly in the UI. #6446

Closed
techanvil opened this issue Jan 23, 2023 · 6 comments
Closed
Labels
Exp: SP Module: Analytics Google Analytics module related issues P0 High priority Type: Enhancement Improvement of an existing feature

Comments

@techanvil
Copy link
Collaborator

techanvil commented Jan 23, 2023

Feature Description

Due to the shareable flag being set for the GA4 GET:report datapoint (introduced in #6172), the Analytics 4 module is now determined to be a shareable module and thus displayed in the Dashboard Sharing modal.

Note: To enable the GA4 GET:report datapoint, turn on the ga4Reporting feature flag.

image (3).png

We need to prevent the GA4 module from appearing in the Dashboard Sharing modal, while ensuring the Analytics sharing settings also apply to the GA4 module so that the GET:report endpoint can be shared.

We should also ensure that the GA4 module does not appear in any other unexpected places in the UI, for example the Recover Modules modal.

It might be advisable, rather than taking GA4 as a special case, to make the solution generically applicable to all modules which are flagged as internal and are also shareable.


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

Acceptance criteria

  • With the ga4Reporting feature enabled:
    • The GA4 module should not be displayed on the Dashboard Sharing modal, the Recover Modules modal, or anywhere else unexpected in the UI.
    • The Analytics sharing settings should apply to GA4 for the purpose of data sharing (i.e. the GA4 GET:report endpoint should be shareable when the Analytics module is shareable).
  • This should be implemented in a generic way that is applicable to any module which is flagged as internal and is also shareable.

Implementation Brief

  • In assets/js/googlesitekit/modules/datastore/modules.js, add a new selector, getShareableModules with the following logic:
    • Extract the shareableModules logic from the DashboardSharingSettings component and omit the getID selector. See:
      const modules = select( CORE_MODULES ).getModules();
      // Return early if modules are not loaded.
      if ( modules === undefined ) {
      return undefined;
      }
      const userID = select( CORE_USER ).getID();
      const shareableModules = Object.values( modules ).filter(
      ( module ) => module.shareable
      );
    • Add a condition that checks if the module is not internal along with the existing shareable condition.
    • Return the result of the filter function.
  • In assets/js/components/dashboard-sharing/DashboardSharingSettings/index.js, replace the shareableModules logic with the value returned by the new getShareableModules selector.
  • In assets/js/googlesitekit/modules/datastore/modules.js, update the *getRecoverableModules resolver to check if the module is not internal along with the existing recoverable condition.

Note: We could work on the logic on the server; however, our current codebase checks whether a module is shareable or recoverable on the client side. So, it's better to keep the logic on the client side.

Test Coverage

  • Add a test case for the getRecoverableModules selector to ensure that the internal modules are not returned.
  • Add tests for the getShareableModules selector.

QA Brief

  • Check the Dashboard sharing with Analytics and GA4 modules enabled
  • GA4 should not be displayed anymore

Screenshot 2023-02-14 at 16 11 19

Changelog entry

  • Prevent "Analytics 4" from appearing separate from "Analytics" in Dashboard Sharing settings when ga4Reporting is enabled.
@techanvil techanvil added Module: Analytics Google Analytics module related issues P0 High priority Type: Enhancement Improvement of an existing feature labels Jan 23, 2023
@techanvil techanvil changed the title Prevent the Analytics 4 module from being displayed in the Dashboard Sharing modal. Prevent the Analytics 4 module from being displayed unexpectedly in the UI. Jan 23, 2023
@tofumatt tofumatt self-assigned this Jan 25, 2023
@tofumatt
Copy link
Collaborator

ACs here are good, moving to IB 👍🏻

@tofumatt tofumatt removed their assignment Jan 25, 2023
@derweili derweili assigned derweili and unassigned derweili Jan 26, 2023
@hussain-t hussain-t self-assigned this Jan 30, 2023
@hussain-t hussain-t removed their assignment Jan 30, 2023
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@sashadoes sashadoes self-assigned this Feb 12, 2023
@sashadoes sashadoes assigned sashadoes and unassigned sashadoes Feb 14, 2023
@tofumatt tofumatt assigned tofumatt and sashadoes and unassigned tofumatt Feb 14, 2023
@sashadoes sashadoes assigned tofumatt and unassigned sashadoes Feb 16, 2023
@tofumatt tofumatt removed their assignment Feb 16, 2023
@mohitwp mohitwp self-assigned this Feb 17, 2023
@mohitwp
Copy link
Collaborator

mohitwp commented Feb 17, 2023

@sashadoes
In AC, it is mentioned that The GA4 module should not be displayed on the Recover Module's modal. Can you please guide how can I access this modal ?

@sashadoes
Copy link
Collaborator

Hi @mohitwp, I don't know what is the Recover Module's modal, sorry.
@techanvil could you please jump in here?

@techanvil
Copy link
Collaborator Author

techanvil commented Feb 17, 2023

As mentioned on Slack, the "Recover Modules modal" refers to the ModuleRecoveryAlert component as seen here. It's actually an alert (implemented as a notification) rather than a modal, this was a mistake in the description.

Searching for ModuleRecoveryAlert will find related issues which have QABs that can help determine how to test it, for example #5287.

However, these may not specifically cover how to get the Analytics module showing in the alert.

This can be achieved as follows:

  • Set up Site Kit, without setting up Analytics.
  • Enable the dashboardSharing feature flag.
  • As a second admin:
    • Set up Site Kit and connect Analytics.
    • Using the Dashboard Sharing modal, enable sharing for Analytics.
    • Disconnect Site Kit.
  • Log back in as the first admin, and you should see the ModuleRecoveryAlert component in the following state:

image

@mohitwp
Copy link
Collaborator

mohitwp commented Feb 20, 2023

QA Update ✅

  • Tested on dev environment.
  • Tested dashboard sharing modal, Recover Module's modal and other places.
  • The GA4 module is not displaying on the Dashboard Sharing modal, the Recover Modules modal, or anywhere else unexpected in the UI.

image

image

image

image

image

@felixarntz felixarntz self-assigned this Feb 23, 2023
@felixarntz felixarntz removed their assignment Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Exp: SP Module: Analytics Google Analytics module related issues P0 High priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

8 participants