Merge branch '207549-add-refresh-dashboard-button-second-iteration' into 'master'

Dashboard refresh button refreshes data dynamically

See merge request gitlab-org/gitlab!28756

app/assets/javascripts/monitoring/components/dashboard.vue

@@ -19,12 +19,7 @@ import PanelType from 'ee_else_ce/monitoring/components/panel_type.vue';
 import { s__ } from '~/locale';
 import createFlash from '~/flash';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
-import {
-  mergeUrlParams,
-  redirectTo,
-  refreshCurrentPage,
-  updateHistory,
-} from '~/lib/utils/url_utility';
+import { mergeUrlParams, redirectTo, updateHistory } from '~/lib/utils/url_utility';
 import invalidUrl from '~/lib/utils/invalid_url';
 import Icon from '~/vue_shared/components/icon.vue';
 import DateTimePicker from '~/vue_shared/components/date_time_picker/date_time_picker.vue';
@@ -273,6 +268,7 @@ export default {
     ...mapActions('monitoringDashboard', [
       'setTimeRange',
       'fetchData',
+      'fetchDashboardData',
       'setGettingStartedEmptyState',
       'setInitialState',
       'setPanelGroupMetrics',
@@ -360,7 +356,7 @@ export default {
     },
 
     refreshDashboard() {
-      refreshCurrentPage();
+      this.fetchDashboardData();
     },
 
     onTimeRangeZoom({ start, end }) {
@@ -475,7 +471,7 @@ export default {
             ref="refreshDashboardBtn"
             v-gl-tooltip
             variant="default"
-            :title="s__('Metrics|Reload this page')"
+            :title="s__('Metrics|Refresh dashboard')"
             @click="refreshDashboard"
           >
             <icon name="retry" />

app/assets/javascripts/monitoring/components/panel_type.vue

@@ -4,6 +4,7 @@ import { pickBy } from 'lodash';
 import invalidUrl from '~/lib/utils/invalid_url';
 import {
   GlResizeObserverDirective,
+  GlLoadingIcon,
   GlDropdown,
   GlDropdownItem,
   GlModal,
@@ -37,6 +38,7 @@ export default {
     MonitorStackedColumnChart,
     MonitorEmptyChart,
     Icon,
+    GlLoadingIcon,
     GlTooltip,
     GlDropdown,
     GlDropdownItem,
@@ -104,13 +106,17 @@ export default {
       // This method is extended by ee functionality
       return false;
     },
-    graphDataHasMetrics() {
+    graphDataHasResult() {
       return (
         this.graphData.metrics &&
         this.graphData.metrics[0].result &&
         this.graphData.metrics[0].result.length > 0
       );
     },
+    graphDataIsLoading() {
+      const { metrics = [] } = this.graphData;
+      return metrics.some(({ loading }) => loading);
+    },
     logsPathWithTimeRange() {
       const timeRange = this.zoomedTimeRange || this.timeRange;
 
@@ -140,7 +146,7 @@ export default {
     },
     isContextualMenuShown() {
       return (
-        this.graphDataHasMetrics &&
+        this.graphDataHasResult &&
         !this.isPanelType('single-stat') &&
         !this.isPanelType('heatmap') &&
         !this.isPanelType('column') &&
@@ -193,7 +199,7 @@ export default {
 </script>
 <template>
   <div v-gl-resize-observer="onResize" class="prometheus-graph">
-    <div class="prometheus-graph-header">
+    <div class="d-flex align-items-center mr-3">
       <h5
         ref="graphTitle"
         class="prometheus-graph-title gl-font-size-large font-weight-bold text-truncate append-right-8"
@@ -203,23 +209,27 @@ export default {
       <gl-tooltip :target="() => $refs.graphTitle" :disabled="!showTitleTooltip">
         {{ title }}
       </gl-tooltip>
-      <div
-        v-if="isContextualMenuShown"
-        class="prometheus-graph-widgets js-graph-widgets flex-fill"
-        data-qa-selector="prometheus_graph_widgets"
-      >
-        <div class="d-flex align-items-center">
       <alert-widget
-            v-if="alertWidgetAvailable"
+        v-if="isContextualMenuShown && alertWidgetAvailable"
+        class="mx-1"
         :modal-id="`alert-modal-${index}`"
         :alerts-endpoint="alertsEndpoint"
         :relevant-queries="graphData.metrics"
         :alerts-to-manage="getGraphAlerts(graphData.metrics)"
         @setAlerts="setAlerts"
       />
+      <div class="flex-grow-1"></div>
+      <div v-if="graphDataIsLoading" class="mx-1 mt-1">
+        <gl-loading-icon />
+      </div>
+      <div
+        v-if="isContextualMenuShown"
+        class="js-graph-widgets"
+        data-qa-selector="prometheus_graph_widgets"
+      >
+        <div class="d-flex align-items-center">
           <gl-dropdown
             v-gl-tooltip
-            class="ml-auto mx-3"
             toggle-class="btn btn-transparent border-0"
             data-qa-selector="prometheus_widgets_dropdown"
             right
@@ -275,28 +285,28 @@ export default {
     </div>
 
     <monitor-single-stat-chart
-      v-if="isPanelType('single-stat') && graphDataHasMetrics"
+      v-if="isPanelType('single-stat') && graphDataHasResult"
       :graph-data="graphData"
     />
     <monitor-heatmap-chart
-      v-else-if="isPanelType('heatmap') && graphDataHasMetrics"
+      v-else-if="isPanelType('heatmap') && graphDataHasResult"
       :graph-data="graphData"
     />
     <monitor-bar-chart
-      v-else-if="isPanelType('bar') && graphDataHasMetrics"
+      v-else-if="isPanelType('bar') && graphDataHasResult"
       :graph-data="graphData"
     />
     <monitor-column-chart
-      v-else-if="isPanelType('column') && graphDataHasMetrics"
+      v-else-if="isPanelType('column') && graphDataHasResult"
       :graph-data="graphData"
     />
     <monitor-stacked-column-chart
-      v-else-if="isPanelType('stacked-column') && graphDataHasMetrics"
+      v-else-if="isPanelType('stacked-column') && graphDataHasResult"
       :graph-data="graphData"
     />
     <component
       :is="timeChartComponent"
-      v-else-if="graphDataHasMetrics"
+      v-else-if="graphDataHasResult"
       ref="timeChart"
       :graph-data="graphData"
       :deployment-data="deploymentData"

app/assets/javascripts/monitoring/constants.js

@@ -10,7 +10,10 @@ export const metricStates = {
   OK: 'OK',
 
   /**
-   * Metric data is being fetched
+   * Metric data is being fetched for the first time.
+   *
+   * Not used during data refresh, if data is available in
+   * the metric, the recommneded state is OK.
    */
   LOADING: 'LOADING',
 

app/assets/javascripts/monitoring/stores/actions.js

@@ -128,7 +128,7 @@ export const receiveMetricsDashboardSuccess = ({ commit, dispatch }, { response
   commit(types.RECEIVE_METRICS_DASHBOARD_SUCCESS, dashboard);
   commit(types.SET_ENDPOINTS, convertObjectPropsToCamelCase(metrics_data));
 
-  return dispatch('fetchPrometheusMetrics');
+  return dispatch('fetchDashboardData');
 };
 export const receiveMetricsDashboardFailure = ({ commit }, error) => {
   commit(types.RECEIVE_METRICS_DASHBOARD_FAILURE, error);
@@ -140,7 +140,7 @@ export const receiveMetricsDashboardFailure = ({ commit }, error) => {
  * Loads timeseries data: Prometheus data points and deployment data from the project
  * @param {Object} Vuex store
  */
-export const fetchPrometheusMetrics = ({ state, dispatch, getters }) => {
+export const fetchDashboardData = ({ state, dispatch, getters }) => {
   dispatch('fetchDeploymentsData');
 
   if (!state.timeRange) {

app/assets/javascripts/monitoring/stores/mutations.js

-import Vue from 'vue';
 import pick from 'lodash/pick';
 import * as types from './mutation_types';
 import { mapToDashboardViewModel, normalizeQueryResult } from './utils';
@@ -26,24 +25,6 @@ const findMetricInDashboard = (metricId, dashboard) => {
   return res;
 };
 
-/**
- * Set a new state for a metric.
- *
- * Initally metric data is not populated, so `Vue.set` is
- * used to add new properties to the metric.
- *
- * @param {Object} metric - Metric object as defined in the dashboard
- * @param {Object} state - New state
- * @param {Array|null} state.result - Array of results
- * @param {String} state.error - Error code from metricStates
- * @param {Boolean} state.loading - True if the metric is loading
- */
-const setMetricState = (metric, { result = null, loading = false, state = null }) => {
-  Vue.set(metric, 'result', result);
-  Vue.set(metric, 'loading', loading);
-  Vue.set(metric, 'state', state);
-};
-
 /**
  * Maps a backened error state to a `metricStates` constant
  * @param {Object} error - Error from backend response
@@ -116,39 +97,32 @@ export default {
    */
   [types.REQUEST_METRIC_RESULT](state, { metricId }) {
     const metric = findMetricInDashboard(metricId, state.dashboard);
-    setMetricState(metric, {
-      loading: true,
-      state: metricStates.LOADING,
-    });
+    metric.loading = true;
+    if (!metric.result) {
+      metric.state = metricStates.LOADING;
+    }
   },
   [types.RECEIVE_METRIC_RESULT_SUCCESS](state, { metricId, result }) {
-    if (!metricId) {
-      return;
-    }
-
+    const metric = findMetricInDashboard(metricId, state.dashboard);
+    metric.loading = false;
     state.showEmptyState = false;
 
-    const metric = findMetricInDashboard(metricId, state.dashboard);
     if (!result || result.length === 0) {
-      setMetricState(metric, {
-        state: metricStates.NO_DATA,
-      });
+      metric.state = metricStates.NO_DATA;
+      metric.result = null;
     } else {
       const normalizedResults = result.map(normalizeQueryResult);
-      setMetricState(metric, {
-        result: Object.freeze(normalizedResults),
-        state: metricStates.OK,
-      });
+
+      metric.state = metricStates.OK;
+      metric.result = Object.freeze(normalizedResults);
     }
   },
   [types.RECEIVE_METRIC_RESULT_FAILURE](state, { metricId, error }) {
-    if (!metricId) {
-      return;
-    }
     const metric = findMetricInDashboard(metricId, state.dashboard);
-    setMetricState(metric, {
-      state: emptyStateFromError(error),
-    });
+
+    metric.state = emptyStateFromError(error);
+    metric.loading = false;
+    metric.result = null;
   },
   [types.SET_INITIAL_STATE](state, initialState = {}) {
     Object.assign(state, pick(initialState, initialStateKeys));

app/assets/javascripts/monitoring/stores/utils.js

@@ -76,6 +76,12 @@ const mapToMetricsViewModel = metrics =>
     queryRange: query_range,
     prometheusEndpointPath: prometheus_endpoint_path,
     metricId: uniqMetricsId({ metric_id, id }),
+
+    // metric data
+    loading: false,
+    result: null,
+    state: null,
+
     ...metric,
   }));
 

app/assets/stylesheets/pages/prometheus.scss

@@ -84,13 +84,6 @@
   border-radius: $border-radius-default;
 }
 
-.prometheus-graph-header {
-  display: flex;
-  align-items: center;
-  justify-content: space-between;
-  margin-bottom: $gl-padding-8;
-}
-
 .alert-current-setting {
   max-width: 240px;
 

changelogs/unreleased/207549-add-refresh-dashboard-button-second-iteration.yml

+---
+title: Refresh metrics dashboard data without reloading the page
+merge_request: 28756
+author:
+type: added

locale/gitlab.pot

@@ -12929,7 +12929,7 @@ msgstr ""
 msgid "Metrics|Prometheus Query Documentation"
 msgstr ""
 
-msgid "Metrics|Reload this page"
+msgid "Metrics|Refresh dashboard"
 msgstr ""
 
 msgid "Metrics|Show last"

spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap

@@ -92,7 +92,7 @@ exports[`Dashboard template matches the default snapshot 1`] = `
       >
         <gl-deprecated-button-stub
           size="md"
-          title="Reload this page"
+          title="Refresh dashboard"
           variant="default"
         >
           <icon-stub

spec/frontend/monitoring/store/actions_spec.js

@@ -15,7 +15,7 @@ import {
   receiveMetricsDashboardSuccess,
   fetchDeploymentsData,
   fetchEnvironmentsData,
-  fetchPrometheusMetrics,
+  fetchDashboardData,
   fetchPrometheusMetric,
   setInitialState,
   filterEnvironments,
@@ -375,7 +375,7 @@ describe('Monitoring store actions', () => {
 
         metricsDashboardResponse.dashboard,
       );
-      expect(dispatch).toHaveBeenCalledWith('fetchPrometheusMetrics');
+      expect(dispatch).toHaveBeenCalledWith('fetchDashboardData');
     });
     it('sets the dashboards loaded from the repository', () => {
       const params = {};
@@ -395,7 +395,7 @@ describe('Monitoring store actions', () => {
       expect(commit).toHaveBeenCalledWith(types.SET_ALL_DASHBOARDS, dashboardGitResponse);
     });
   });
-  describe('fetchPrometheusMetrics', () => {
+  describe('fetchDashboardData', () => {
     let commit;
     let dispatch;
     let state;
@@ -413,7 +413,7 @@ describe('Monitoring store actions', () => {
       const getters = {
         metricsWithData: () => [],
       };
-      fetchPrometheusMetrics({ state, commit, dispatch, getters })
+      fetchDashboardData({ state, commit, dispatch, getters })
         .then(() => {
           expect(Tracking.event).toHaveBeenCalledWith(
             document.body.dataset.page,
@@ -442,7 +442,7 @@ describe('Monitoring store actions', () => {
         metricsWithData: () => [metric.id],
       };
 
-      fetchPrometheusMetrics({ state, commit, dispatch, getters })
+      fetchDashboardData({ state, commit, dispatch, getters })
         .then(() => {
           expect(dispatch).toHaveBeenCalledWith('fetchPrometheusMetric', {
             metric,
@@ -478,7 +478,7 @@ describe('Monitoring store actions', () => {
       dispatch.mockRejectedValueOnce(new Error('Error fetching this metric'));
       dispatch.mockResolvedValue();
 
-      fetchPrometheusMetrics({ state, commit, dispatch })
+      fetchDashboardData({ state, commit, dispatch })
         .then(() => {
           expect(dispatch).toHaveBeenCalledTimes(10); // one per metric plus 1 for deployments
           expect(dispatch).toHaveBeenCalledWith('fetchDeploymentsData');

spec/frontend/monitoring/store/mutations_spec.js

@@ -202,15 +202,12 @@ describe('Monitoring mutations', () => {
 
         mutations[types.REQUEST_METRIC_RESULT](stateCopy, {
           metricId,
-          result,
         });
 
         expect(stateCopy.showEmptyState).toBe(true);
         expect(getMetric()).toEqual(
           expect.objectContaining({
             loading: true,
-            result: null,
-            state: metricStates.LOADING,
           }),
         );
       });
@@ -232,7 +229,7 @@ describe('Monitoring mutations', () => {
       });
 
       it('adds results to the store', () => {
-        expect(getMetric().result).toBe(undefined);
+        expect(getMetric().result).toBe(null);
 
         mutations[types.RECEIVE_METRIC_RESULT_SUCCESS](stateCopy, {
           metricId,

spec/frontend/monitoring/store/utils_spec.js

@@ -5,6 +5,7 @@ import {
   removeLeadingSlash,
   mapToDashboardViewModel,
 } from '~/monitoring/stores/utils';
+import { NOT_IN_DB_PREFIX } from '~/monitoring/constants';
 
 const projectPath = 'gitlab-org/gitlab-test';
 
@@ -256,6 +257,9 @@ describe('mapToDashboardViewModel', () => {
       expect(getMappedMetric(dashboard)).toEqual({
         label: expect.any(String),
         metricId: expect.any(String),
+        loading: false,
+        result: null,
+        state: null,
       });
     });
 
@@ -307,7 +311,7 @@ describe('mapToDashboardViewModel', () => {
 
 describe('uniqMetricsId', () => {
   [
-    { input: { id: 1 }, expected: 'NO_DB_1' },
+    { input: { id: 1 }, expected: `${NOT_IN_DB_PREFIX}_1` },
     { input: { metric_id: 2 }, expected: '2_undefined' },
     { input: { metric_id: 2, id: 21 }, expected: '2_21' },
     { input: { metric_id: 22, id: 1 }, expected: '22_1' },