site-kit-wp Refactor Key Metric display logic for selection panel, widget area and view only dashboard

Bug Description

In #7741, we introduced logic for view only users that filters out key metrics which require custom dimensions, but now those custom dimensions are unavailable (since view only users can't do much about this).

We are now directly calling displayInList() in the getKeyMetricSettings selector for view-only users which has side-effects when the displayInList implementation contains calls to check if isKeyMetricActive(), like in the case of Most Popular Products and for widgets that use shouldDisplayWidgetWithConversionEvent(). It causes a infinite recursion as seen with the calls below:

getKeyMetrics()
  getUserPickedMetrics()
    getKeyMetricsSettings()
      displayInList() // for "Most popular products" and widgets where shouldDisplayWidgetWithConversionEvent() is used
        isKeyMetricActive()
          getKeyMetrics()

However, this logic was introduced in a central getKeyMetricsSettings() selector even though the main purpose here was to filter out only userPickedMetrics, i.e. metrics the user had once saved but they no longer can benefit from since the custom dimensions required have been deleted or modified. So this should be thoughtfully refactored into the getUserPickedMetrics selector or somewhere more suitable.

https://github.com/google/site-kit-wp/blob/8a891c49d336264431477571d21bca63ebf09f9b/assets/js/googlesitekit/datastore/user/key-metrics.js#L467-L501

We do have a clear separation of concerns when it comes to logic:

that checks if a key metric should be displayed in the metrics selection panel - displayInList()
that checks if a key metric should be displayed within the widget area - isKeyMetricActive()

But we have created a slight overlap in some places to hack things a certain way which makes things redundant and confusing. This can be re-visited and cleaned up.

Steps to reproduce

Activate the Woo Commerce plugin on any wordpress site.
Setup Site Kit, connect Analytics and set up Key Metrics as an Admin.
Share Analytics using Dashboard Sharing.
Now login as a view only user and in the Key Metrics widget, select Pick your own metrics.
Select the Most popular products by pageviews widget is in the list.
See the error

Screen recording

https://github.com/user-attachments/assets/ffbaa75e-5216-40c7-b66d-8c590972ccc5

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

Acceptance criteria

View only users should not see any errors when selecting ANY Key Metric widget tile.
- These specifically include the Most popular products by pageviews widget tile, widget tiles that require Custom Dimensions and the new ACR metric tiles.
The displayInList() callback function is not used for purposes other than displaying a key metric widget tile in the Metrics Selection panel.

Implementation Brief

Fixing the bug

In assets/js/components/KeyMetrics/key-metrics-widgets.js:
- [ ] Create a new displayInWidgetArea callback property for all the key metrics that have a requiredCustomDimension. Set this property to call the shouldDisplayWidgetWithCustomDimensions() function. In the future, this callback can be used by other tiles to add any additional requirements for widgets to be rendered in the widget area which would further override the "user picked" or "answer based" selection.
In assets/js/googlesitekit/datastore/user/key-metrics.js, :
- [ ] Within getKeyMetricSettings() selector: Remove the whole isViewOnly check and snippet to filter out metrics. This will now be handled within the getUserPickedMetrics below.
- [ ] Within getUserPickedMetrics() selector: Filter the widgetSlugs result. Using the KEY_METRICS_WIDGETS data, check if the widget has a displayInWidgetArea property and use the boolean function for filtering. Pass the isViewOnlyDashboard value.

Generic Refactoring

[ ] Rename the displayInList property to displayInSelectionPanel for more clarity as "list" is too generic.
[ ] In getAnswerBasedMetrics selector: Use a new selector for fetching the showConversionTailoredMetrics: https://github.com/google/site-kit-wp/blob/e1ecae073df4dc7f812bc8fccbe0868ee0ec4599/assets/js/googlesitekit/datastore/user/key-metrics.js#L287-L292
[ ] In getKeyMetrics selector: Remove unnecessary filtering of answer based metrics for ACR feature flag as this is already done within the getAnswerBasedMetrics selector. https://github.com/google/site-kit-wp/blob/e1ecae073df4dc7f812bc8fccbe0868ee0ec4599/assets/js/googlesitekit/datastore/user/key-metrics.js#L242-L250
[ ] In getKeyMetrics selector: Move the filtering of userPickedMetrics for the ACR feature flag to a callback function that can be added to the new displayInWidgetArea property of all the new ACR metrics in key-metrics-widgets. Remove the keyMetricsGA4WidgetsNonACR constant if not used. https://github.com/google/site-kit-wp/blob/e1ecae073df4dc7f812bc8fccbe0868ee0ec4599/assets/js/googlesitekit/datastore/user/key-metrics.js#L227-L231

Test Coverage

No new tests needed.

QA Brief

Changelog entry

Oct 22 '24 11:10 jimmymadon

ACs here are good, and thanks for the great explanation/context for the issue 👍🏻

Moving to IB 🙂

Oct 24 '24 12:10 tofumatt

IB ✅

Oct 25 '24 12:10 tofumatt

QA Update ✅

Tested on dev environment.
Verified that issue error is not showing on 'View only dashboard' when user selects 'Most popular products by pageviews' KM tile
Verified KM and user input functionality when 'conversionInfra' feature flag is enabled.
Verified KM and user input functionality when 'conversionInfra' feature flag is not enabled.
Verified Top earning pages KM tile related scenarios.

Top earning pages KM tile related scenarios

Top earning pages KM tile when AdSense is disconnected