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

Can't toggle "Display key metrics in dashboard" when user-defined key metrics are not set. #7441

Closed
techanvil opened this issue Aug 15, 2023 · 2 comments
Labels
P0 High priority Type: Bug Something isn't working

Comments

@techanvil
Copy link
Collaborator

techanvil commented Aug 15, 2023

Bug Description

This is an issue raised in the KM bug bash, see https://app.asana.com/0/1204713925711993/1205270012034069/f.

The "Display key metrics in dashboard" toggle doesn't work unless the user has defined their own set of widgets via "I'll pick metrics myself", or "Change Metrics".

This is a result of a change in the PR for #7349.

Steps to reproduce

  1. Setup Site Kit with the userInput feature flag enabled and GA4 connected.
  2. Answer the questionnaire by selecting "Get tailored metrics", but don't customize the KM selection.
  3. Navigate to Admin > Admin Settings, and open devtools.
  4. Toggle the "Display key metrics in dashboard" switch to the off position.
  5. Notice the request to /wp-json/google-site-kit/v1/core/user/data/key-metrics will have failed with a 400 status code and "Selected metrics cannot be empty" error message.
  6. Refresh the page to see that the "Display key metrics in dashboard" switch reverts to the the on position.

Screenshots

image

cant_toggle_isWidgetHidden_with_empty_km_array.mp4

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

Acceptance criteria

  • The "Display key metrics in dashboard" switch on the Key Metrics admin page should toggle and persist its corresponding value regardless of whether the Key Metrics selection has been customized by the user or not.
  • The /wp-json/google-site-kit/v1/core/user/data/key-metrics endpoint should continue to validate that there is at least one key metric slug defined, but only in the case where the user has customized the selection.

Implementation Brief

  • In includes/Core/Key_Metrics/REST_Key_Metrics_Controller.php:
    • For the POST:core/user/data/key-metrics route:
      • Make the widgetSlugs arg optional by changing the 'required' => true to 'required' => false.
      • Inside the route callback,
        • Only run the logic related to $num_widgets when widgetSlugs is set in $data['settings'].
        • Only set the $this->key_metrics_setup_completed to true when widgetSlugs is set in $data['settings'].
  • In assets/js/googlesitekit/datastore/user/key-metrics.js:
    • In the saveKeyMetricsSettings action:
      • At the very end of this action, only dispatch the setKeyMetricsSetupCompleted when settings.widgetSlugs is not falsey.
  • In assets/js/components/settings/SettingsKeyMetrics.js:
    • In the handleKeyMetricsToggle callback:
      • Pass { widgetSlugs: undefined } parameter to the saveKeyMetricsSettings call.

Test Coverage

  • Add test coverage for above changes.

QA Brief

  • On a fully reset site (KM not set up yet), complete the user input question.
  • Answer based KM tiles should be visible.
  • Visit SK settings and turn off Key Metrics toggle.
  • The KM tiles should be hidden .
  • Enable the widget again from Settings
  • Tiles should get re-enabled.
  • Customize Key Metrics tiles.
  • The customization should be saved correctly.
  • Verify the above display behavior of the Key Metrics Widgets for a secondary admin user who has not yet completed the user input questionnaire.

Changelog entry

  • Fix bug that caused the "Display key metrics in dashboard" toggle not to work when no user-defined metrics were saved.
@techanvil techanvil added P0 High priority Type: Bug Something isn't working labels Aug 15, 2023
@kuasha420 kuasha420 assigned kuasha420 and unassigned kuasha420 Aug 20, 2023
@tofumatt tofumatt self-assigned this Aug 21, 2023
@tofumatt
Copy link
Collaborator

IB ✅

@tofumatt tofumatt removed their assignment Aug 23, 2023
@kuasha420 kuasha420 self-assigned this Aug 23, 2023
@kuasha420 kuasha420 removed their assignment Aug 27, 2023
@hussain-t hussain-t assigned hussain-t and unassigned hussain-t Aug 28, 2023
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Aug 28, 2023
tofumatt added a commit that referenced this issue Aug 29, 2023
Fix Key Metrics toggle in SK Settings when answer based matrices are used
@wpdarren wpdarren self-assigned this Aug 30, 2023
@wpdarren
Copy link
Collaborator

wpdarren commented Sep 1, 2023

QA Update: ✅

Verified:

  • When I turn off Key Metrics toggle, the KM tiles are hidden .
  • When I enable the widget again from settings, the tiles are re-enabled.
  • When I customize Key Metrics tiles, the customization is saved correctly.
  • The above behavior of the Key Metrics Widgets is the same for a secondary admin user who has not yet completed the user input questionnaire.

Additional QA:

  • Checked the network table in browser when the toggle is enabled and disabled to ensure that no Error 400 appears as per the issue description. Also did the same when changing the metrics and testing with a 2nd admin user.
Screenshots

image
image
image
image

@wpdarren wpdarren removed their assignment Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants