site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Don't render the Key Metrics Selection Panel into the DOM until the feature has been set up

Open techanvil opened this issue 1 year ago • 1 comments

Feature Description

As mentioned on Asana the Key Metrics Selection Panel is unnecessarily rendered into the DOM before the feature has been set up.

It should only be rendered once the feature has been set up.


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

Acceptance criteria

  • The Key Metrics Selection Panel should only be rendered into the DOM once Key Metrics has been set up.

Implementation Brief

  • [ ] Wrap the <MetricsSelectionPanel /> with the condition isKeyMetricsWidgetHidden === false. Changing this will prevent rendering of the setup panel if Key Metrics isn't enabled.
    • https://github.com/google/site-kit-wp/blob/638dbd972e2663626037a4e8f13e21fc8b54521a/assets/js/components/DashboardMainApp.js#L295-L295

Test Coverage

  • No additional test coverage required, confirm existing tests pass.

QA Brief

Changelog entry

techanvil avatar Oct 07 '24 15:10 techanvil

IB ✔

eugene-manuilov avatar Oct 15 '24 22:10 eugene-manuilov

@techanvil There was a mistake in previous PR for the condition to display the component. I've created a follow up PR https://github.com/google/site-kit-wp/pull/9630 and assigning it to you for review.

Thanks

ankitrox avatar Nov 07 '24 07:11 ankitrox

Thanks for the spot and the fix @ankitrox!

techanvil avatar Nov 07 '24 10:11 techanvil

QA Update ❌

  • Tested on dev environment.
  • Verified that googlesitekit-km-selection-panel is not present in markup when the Key Metrics is disabled under Site Kit settings.
  • Verified that googlesitekit-km-selection-panel is present in markup when the Key Metrics is enabled under Site Kit settings.

@ankitrox I found that in the following cases, the googlesitekit-km-selection-panel is present in the markup, but it is not expected:

1) The site is in the gathering data state. The Key Metrics feature is not yet set up, and the setup banner is not displaying. However, googlesitekit-km-selection-panel is still present in the markup. (In this case, the user cannot set up KMW, and the KM option does not appear under settings because site is in gathering data state.)

Image

2) Key Metrics feature is not yet setup and the setup banner is being shown, googlesitekit-km-selection-panel is still present in the markup. (This scenario occurs when a user visits the dashboard for the first time after setup. On the first load of the main dashboard, the KM setup banner should not display, and the markup should not be present in this case. You can use oi.ie to reproduce this.).

Image

3) Analytics is disconnected, KM feature is not yet setup and setup banner is not showing. However, googlesitekit-km-selection-panel is still present in the markup. (In this case, the user cannot set up KMW, and the KM option does not appear under settings because analytics is not connected. You can use oi.ie to reproduce this.)

Image

Image

mohitwp avatar Nov 11 '24 02:11 mohitwp

Thanks for testing this @mohitwp and highlighting the issue.

I have raised a follow-up PR for this problem.

ankitrox avatar Nov 11 '24 11:11 ankitrox

Thanks @ankitrox, that's merged. Back to you for another pass, @mohitwp.

techanvil avatar Nov 11 '24 14:11 techanvil

Hi @ankitrox 👋

We just found out that this issue has caused all E2E tests to fail, most likely because the useDisplayCTAWidget hook makes calls to Analytics selectors without checking if the module is connected. Sending this back to Execution. See conversation here.

CC: @techanvil @mohitwp @aaemnnosttv

nfmohit avatar Nov 11 '24 19:11 nfmohit

I've created a quick PR to address the above issue. Thanks!

nfmohit avatar Nov 11 '24 19:11 nfmohit

QA Update ✅

  • Tested on dev environment.
  • Tested when 'conversionReporting' feature flag is not enabled.
  • Tested when 'conversionReporting' feature flag is enabled.
  • Tested on main and view only dashboard.
  • Verified that when the Key Metrics feature is not yet setup and the setup banner is being shown, googlesitekit-km-selection-panel element present in the markup.
  • Verified When the Key Metrics feature is not yet setup and the setup banner is not showing up, googlesitekit-km-selection-panel element is not present in the markup.
  • Verified the Key Metrics Selection Panel only rendered into the DOM once Key Metrics has been set up.
  • Verified that googlesitekit-km-selection-panel element not present in the markup if Key metrics visibility is disabled under admin settings.
  • Verified that all issues reported above are now resolved.

Image

Issue 2 : FIXED

Image

Issue 3 : FIXED

Image

**PASS CASES -**

Image

Image

https://github.com/user-attachments/assets/1ae0fbc3-4af9-4996-ae84-1060ce4b29fa

View only Dashboard

Image

mohitwp avatar Nov 13 '24 07:11 mohitwp