Merge branch 'dashboardValidationWarnings' into 'master'

Add YML validation warning to metrics view

See merge request gitlab-org/gitlab!33769

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

@@ -160,6 +160,7 @@ export default {
       'variables',
       'links',
       'currentDashboard',
+      'hasDashboardValidationWarnings',
     ]),
     ...mapGetters('monitoringDashboard', ['selectedDashboard', 'getMetricStates']),
     shouldShowVariablesSection() {
@@ -197,6 +198,19 @@ export default {
     selectedDashboard(dashboard) {
       this.prependToDocumentTitle(dashboard?.display_name);
     },
+    hasDashboardValidationWarnings(hasWarnings) {
+      /**
+       * This watcher is set for future SPA behaviour of the dashboard
+       */
+      if (hasWarnings) {
+        createFlash(
+          s__(
+            'Metrics|Your dashboard schema is invalid. Edit the dashboard to correct the YAML schema.',
+          ),
+          'warning',
+        );
+      }
+    },
   },
   created() {
     window.addEventListener('keyup', this.onKeyup);

app/assets/javascripts/monitoring/queries/getDashboardValidationWarnings.query.graphql

+query getDashboardValidationWarnings(
+  $projectPath: ID!
+  $environmentName: String
+  $dashboardPath: String!
+) {
+  project(fullPath: $projectPath) {
+    id
+    environments(name: $environmentName) {
+      nodes {
+        name
+        metricsDashboard(path: $dashboardPath) {
+          path
+          schemaValidationWarnings
+        }
+      }
+    }
+  }
+}

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

@@ -12,6 +12,7 @@ import {
 import trackDashboardLoad from '../monitoring_tracking_helper';
 import getEnvironments from '../queries/getEnvironments.query.graphql';
 import getAnnotations from '../queries/getAnnotations.query.graphql';
+import getDashboardValidationWarnings from '../queries/getDashboardValidationWarnings.query.graphql';
 import statusCodes from '../../lib/utils/http_status';
 import { backOff, convertObjectPropsToCamelCase } from '../../lib/utils/common_utils';
 import { s__, sprintf } from '../../locale';
@@ -126,7 +127,17 @@ export const fetchDashboard = ({ state, commit, dispatch }) => {
 
   return backOffRequest(() => axios.get(state.dashboardEndpoint, { params }))
     .then(resp => resp.data)
-    .then(response => dispatch('receiveMetricsDashboardSuccess', { response }))
+    .then(response => {
+      dispatch('receiveMetricsDashboardSuccess', { response });
+      /**
+       * After the dashboard is fetched, there can be non-blocking invalid syntax
+       * in the dashboard file. This call will fetch such syntax warnings
+       * and surface a warning on the UI. If the invalid syntax is blocking,
+       * the `fetchDashboard` returns a 404 with error messages that are displayed
+       * on the UI.
+       */
+      dispatch('fetchDashboardValidationWarnings');
+    })
     .catch(error => {
       Sentry.captureException(error);
 
@@ -344,6 +355,46 @@ export const receiveAnnotationsSuccess = ({ commit }, data) =>
   commit(types.RECEIVE_ANNOTATIONS_SUCCESS, data);
 export const receiveAnnotationsFailure = ({ commit }) => commit(types.RECEIVE_ANNOTATIONS_FAILURE);
 
+export const fetchDashboardValidationWarnings = ({ state, dispatch }) => {
+  /**
+   * Normally, the default dashboard won't throw any validation warnings.
+   *
+   * However, if a bug sneaks into the default dashboard making it invalid,
+   * this might come handy for our clients
+   */
+  const dashboardPath = state.currentDashboard || DEFAULT_DASHBOARD_PATH;
+  return gqClient
+    .mutate({
+      mutation: getDashboardValidationWarnings,
+      variables: {
+        projectPath: removeLeadingSlash(state.projectPath),
+        environmentName: state.currentEnvironmentName,
+        dashboardPath,
+      },
+    })
+    .then(resp => resp.data?.project?.environments?.nodes?.[0]?.metricsDashboard)
+    .then(({ schemaValidationWarnings }) => {
+      const hasWarnings = schemaValidationWarnings && schemaValidationWarnings.length !== 0;
+      /**
+       * The payload of the dispatch is a boolean, because at the moment a standard
+       * warning message is shown instead of the warnings the BE returns
+       */
+      dispatch('receiveDashboardValidationWarningsSuccess', hasWarnings || false);
+    })
+    .catch(err => {
+      Sentry.captureException(err);
+      dispatch('receiveDashboardValidationWarningsFailure');
+      createFlash(
+        s__('Metrics|There was an error getting dashboard validation warnings information.'),
+      );
+    });
+};
+
+export const receiveDashboardValidationWarningsSuccess = ({ commit }, hasWarnings) =>
+  commit(types.RECEIVE_DASHBOARD_VALIDATION_WARNINGS_SUCCESS, hasWarnings);
+export const receiveDashboardValidationWarningsFailure = ({ commit }) =>
+  commit(types.RECEIVE_DASHBOARD_VALIDATION_WARNINGS_FAILURE);
+
 // Dashboard manipulation
 
 export const toggleStarredValue = ({ commit, state, getters }) => {

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

@@ -13,6 +13,12 @@ export const RECEIVE_DASHBOARD_STARRING_FAILURE = 'RECEIVE_DASHBOARD_STARRING_FA
 export const RECEIVE_ANNOTATIONS_SUCCESS = 'RECEIVE_ANNOTATIONS_SUCCESS';
 export const RECEIVE_ANNOTATIONS_FAILURE = 'RECEIVE_ANNOTATIONS_FAILURE';
 
+// Dashboard validation warnings
+export const RECEIVE_DASHBOARD_VALIDATION_WARNINGS_SUCCESS =
+  'RECEIVE_DASHBOARD_VALIDATION_WARNINGS_SUCCESS';
+export const RECEIVE_DASHBOARD_VALIDATION_WARNINGS_FAILURE =
+  'RECEIVE_DASHBOARD_VALIDATION_WARNINGS_FAILURE';
+
 // Git project deployments
 export const REQUEST_DEPLOYMENTS_DATA = 'REQUEST_DEPLOYMENTS_DATA';
 export const RECEIVE_DEPLOYMENTS_DATA_SUCCESS = 'RECEIVE_DEPLOYMENTS_DATA_SUCCESS';

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

@@ -125,6 +125,16 @@ export default {
     state.annotations = [];
   },
 
+  /**
+   * Dashboard Validation Warnings
+   */
+  [types.RECEIVE_DASHBOARD_VALIDATION_WARNINGS_SUCCESS](state, hasDashboardValidationWarnings) {
+    state.hasDashboardValidationWarnings = hasDashboardValidationWarnings;
+  },
+  [types.RECEIVE_DASHBOARD_VALIDATION_WARNINGS_FAILURE](state) {
+    state.hasDashboardValidationWarnings = false;
+  },
+
   /**
    * Individual panel/metric results
    */

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

@@ -12,6 +12,7 @@ export default () => ({
   currentDashboard: null,
 
   // Dashboard data
+  hasDashboardValidationWarnings: false,
   emptyState: 'gettingStarted',
   showEmptyState: true,
   showErrorBanner: true,

changelogs/unreleased/dashboardValidationWarnings.yml

+---
+title: Add dashboard validation warning to metrics dashboard
+merge_request: 33769
+author:
+type: added

locale/gitlab.pot

@@ -14221,6 +14221,9 @@ msgstr ""
 msgid "Metrics|There was an error getting annotations information."
 msgstr ""
 
+msgid "Metrics|There was an error getting dashboard validation warnings information."
+msgstr ""
+
 msgid "Metrics|There was an error getting deployment information."
 msgstr ""
 
@@ -14269,6 +14272,9 @@ msgstr ""
 msgid "Metrics|You're about to permanently delete this metric. This cannot be undone."
 msgstr ""
 
+msgid "Metrics|Your dashboard schema is invalid. Edit the dashboard to correct the YAML schema."
+msgstr ""
+
 msgid "Metrics|e.g. HTTP requests"
 msgstr ""
 

spec/frontend/monitoring/components/dashboard_spec.js

@@ -157,6 +157,34 @@ describe('Dashboard', () => {
     });
   });
 
+  describe('dashboard validation warning', () => {
+    it('displays a warning if there are validation warnings', () => {
+      createMountedWrapper({ hasMetrics: true });
+
+      store.commit(
+        `monitoringDashboard/${types.RECEIVE_DASHBOARD_VALIDATION_WARNINGS_SUCCESS}`,
+        true,
+      );
+
+      return wrapper.vm.$nextTick().then(() => {
+        expect(createFlash).toHaveBeenCalled();
+      });
+    });
+
+    it('does not display a warning if there are no validation warnings', () => {
+      createMountedWrapper({ hasMetrics: true });
+
+      store.commit(
+        `monitoringDashboard/${types.RECEIVE_DASHBOARD_VALIDATION_WARNINGS_SUCCESS}`,
+        false,
+      );
+
+      return wrapper.vm.$nextTick().then(() => {
+        expect(createFlash).not.toHaveBeenCalled();
+      });
+    });
+  });
+
   describe('when the URL contains a reference to a panel', () => {
     let location;
 

spec/frontend/monitoring/store/actions_spec.js

@@ -18,6 +18,7 @@ import {
   fetchEnvironmentsData,
   fetchDashboardData,
   fetchAnnotations,
+  fetchDashboardValidationWarnings,
   toggleStarredValue,
   fetchPrometheusMetric,
   setInitialState,
@@ -35,6 +36,7 @@ import {
 } from '~/monitoring/stores/utils';
 import getEnvironments from '~/monitoring/queries/getEnvironments.query.graphql';
 import getAnnotations from '~/monitoring/queries/getAnnotations.query.graphql';
+import getDashboardValidationWarnings from '~/monitoring/queries/getDashboardValidationWarnings.query.graphql';
 import storeState from '~/monitoring/stores/state';
 import {
   deploymentData,
@@ -335,6 +337,106 @@ describe('Monitoring store actions', () => {
     });
   });
 
+  describe('fetchDashboardValidationWarnings', () => {
+    let mockMutate;
+    let mutationVariables;
+
+    beforeEach(() => {
+      state.projectPath = 'gitlab-org/gitlab-test';
+      state.currentEnvironmentName = 'production';
+      state.currentDashboard = '.gitlab/dashboards/dashboard_with_warnings.yml';
+
+      mockMutate = jest.spyOn(gqClient, 'mutate');
+      mutationVariables = {
+        mutation: getDashboardValidationWarnings,
+        variables: {
+          projectPath: state.projectPath,
+          environmentName: state.currentEnvironmentName,
+          dashboardPath: state.currentDashboard,
+        },
+      };
+    });
+
+    it('dispatches receiveDashboardValidationWarningsSuccess with true payload when there are warnings', () => {
+      mockMutate.mockResolvedValue({
+        data: {
+          project: {
+            id: 'gid://gitlab/Project/29',
+            environments: {
+              nodes: [
+                {
+                  name: 'production',
+                  metricsDashboard: {
+                    path: '.gitlab/dashboards/dashboard_errors_test.yml',
+                    schemaValidationWarnings: ["unit: can't be blank"],
+                  },
+                },
+              ],
+            },
+          },
+        },
+      });
+
+      return testAction(
+        fetchDashboardValidationWarnings,
+        null,
+        state,
+        [],
+        [{ type: 'receiveDashboardValidationWarningsSuccess', payload: true }],
+        () => {
+          expect(mockMutate).toHaveBeenCalledWith(mutationVariables);
+        },
+      );
+    });
+
+    it('dispatches receiveDashboardValidationWarningsSuccess with false payload when there are no warnings', () => {
+      mockMutate.mockResolvedValue({
+        data: {
+          project: {
+            id: 'gid://gitlab/Project/29',
+            environments: {
+              nodes: [
+                {
+                  name: 'production',
+                  metricsDashboard: {
+                    path: '.gitlab/dashboards/dashboard_errors_test.yml',
+                    schemaValidationWarnings: [],
+                  },
+                },
+              ],
+            },
+          },
+        },
+      });
+
+      return testAction(
+        fetchDashboardValidationWarnings,
+        null,
+        state,
+        [],
+        [{ type: 'receiveDashboardValidationWarningsSuccess', payload: false }],
+        () => {
+          expect(mockMutate).toHaveBeenCalledWith(mutationVariables);
+        },
+      );
+    });
+
+    it('dispatches receiveDashboardValidationWarningsFailure if the warnings API call fails', () => {
+      mockMutate.mockRejectedValue({});
+
+      return testAction(
+        fetchDashboardValidationWarnings,
+        null,
+        state,
+        [],
+        [{ type: 'receiveDashboardValidationWarningsFailure' }],
+        () => {
+          expect(mockMutate).toHaveBeenCalledWith(mutationVariables);
+        },
+      );
+    });
+  });
+
   describe('Toggles starred value of current dashboard', () => {
     let unstarredDashboard;
     let starredDashboard;
@@ -455,7 +557,7 @@ describe('Monitoring store actions', () => {
       state.dashboardEndpoint = '/dashboard';
     });
 
-    it('on success, dispatches receive and success actions', () => {
+    it('on success, dispatches receive and success actions, then fetches dashboard warnings', () => {
       document.body.dataset.page = 'projects:environments:metrics';
       mock.onGet(state.dashboardEndpoint).reply(200, response);
 
@@ -470,6 +572,7 @@ describe('Monitoring store actions', () => {
             type: 'receiveMetricsDashboardSuccess',
             payload: { response },
           },
+          { type: 'fetchDashboardValidationWarnings' },
         ],
       );
     });