Merge branch 'short-url-for-custom-metrics-dashboards' into 'master'

Support short urls for custom metrics dashboards

See merge request gitlab-org/gitlab!36740

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

@@ -127,6 +127,7 @@ export default {
       'projectPath',
       'canAccessOperationsSettings',
       'operationsSettingsPath',
+      'currentDashboard',
     ]),
     ...mapGetters('monitoringDashboard', ['selectedDashboard', 'filteredEnvironments']),
     isOutOfTheBoxDashboard() {
@@ -164,11 +165,14 @@ export default {
   methods: {
     ...mapActions('monitoringDashboard', ['filterEnvironments', 'toggleStarredValue']),
     selectDashboard(dashboard) {
-      const params = {
-        dashboard: encodeURIComponent(dashboard.path),
-      };
-
-      redirectTo(mergeUrlParams(params, window.location.href));
+      // Once the sidebar See metrics link is updated to the new URL,
+      // this sort of hardcoding will not be necessary.
+      // https://gitlab.com/gitlab-org/gitlab/-/issues/229277
+      const baseURL = `${this.projectPath}/-/metrics`;
+      const dashboardPath = encodeURIComponent(
+        dashboard.out_of_the_box_dashboard ? dashboard.path : dashboard.display_name,
+      );
+      redirectTo(`${baseURL}/${dashboardPath}`);
     },
     debouncedEnvironmentsSearch: debounce(function environmentsSearchOnInput(searchTerm) {
       this.filterEnvironments(searchTerm);
@@ -193,6 +197,17 @@ export default {
     submitCustomMetricsForm() {
       this.$refs.customMetricsForm.submit();
     },
+    getEnvironmentPath(environment) {
+      // Once the sidebar See metrics link is updated to the new URL,
+      // this sort of hardcoding will not be necessary.
+      // https://gitlab.com/gitlab-org/gitlab/-/issues/229277
+      const baseURL = `${this.projectPath}/-/metrics`;
+      const dashboardPath = encodeURIComponent(this.currentDashboard || '');
+      // The environment_metrics_spec.rb requires the URL to not have
+      // slashes. Hence, this additional check.
+      const url = dashboardPath ? `${baseURL}/${dashboardPath}` : baseURL;
+      return mergeUrlParams({ environment }, url);
+    },
   },
   modalIds: {
     addMetric: 'addMetric',
@@ -255,7 +270,7 @@ export default {
               :key="environment.id"
               :active="environment.name === currentEnvironmentName"
               active-class="is-active"
-              :href="environment.metrics_path"
+              :href="getEnvironmentPath(environment.id)"
               >{{ environment.name }}</gl-dropdown-item
             >
           </div>

app/assets/javascripts/monitoring/monitoring_app.js

 import Vue from 'vue';
 import { GlToast } from '@gitlab/ui';
-import { getParameterValues } from '~/lib/utils/url_utility';
 import { createStore } from './stores';
 import createRouter from './router';
 import { stateAndPropsFromDataset } from './utils';
@@ -11,11 +10,9 @@ export default (props = {}) => {
   const el = document.getElementById('prometheus-graphs');
 
   if (el && el.dataset) {
-    const [encodedDashboard] = getParameterValues('dashboard');
-    const currentDashboard = encodedDashboard ? decodeURIComponent(encodedDashboard) : null;
     const { metricsDashboardBasePath, ...dataset } = el.dataset;
 
-    const { initState, dataProps } = stateAndPropsFromDataset({ currentDashboard, ...dataset });
+    const { initState, dataProps } = stateAndPropsFromDataset(dataset);
     const store = createStore(initState);
     const router = createRouter(metricsDashboardBasePath);
 

app/assets/javascripts/monitoring/pages/dashboard_page.vue

 <script>
+import { mapActions } from 'vuex';
 import Dashboard from '../components/dashboard.vue';
 
 export default {
@@ -11,6 +12,16 @@ export default {
       required: true,
     },
   },
+  created() {
+    // This is to support the older URL <project>/-/environments/:env_id/metrics?dashboard=:path
+    // and the new format <project>/-/metrics/:dashboardPath
+    const encodedDashboard = this.$route.query.dashboard || this.$route.params.dashboard;
+    const currentDashboard = encodedDashboard ? decodeURIComponent(encodedDashboard) : null;
+    this.setCurrentDashboard({ currentDashboard });
+  },
+  methods: {
+    ...mapActions('monitoringDashboard', ['setCurrentDashboard']),
+  },
 };
 </script>
 <template>

app/assets/javascripts/monitoring/router/constants.js

 export const BASE_DASHBOARD_PAGE = 'dashboard';
+export const CUSTOM_DASHBOARD_PAGE = 'custom_dashboard';
 
 export default {};

app/assets/javascripts/monitoring/router/routes.js

 import DashboardPage from '../pages/dashboard_page.vue';
 
-import { BASE_DASHBOARD_PAGE } from './constants';
+import { BASE_DASHBOARD_PAGE, CUSTOM_DASHBOARD_PAGE } from './constants';
 
 /**
  * Because the cluster health page uses the dashboard
@@ -12,7 +12,12 @@ import { BASE_DASHBOARD_PAGE } from './constants';
 export default [
   {
     name: BASE_DASHBOARD_PAGE,
-    path: '*',
+    path: '/',
+    component: DashboardPage,
+  },
+  {
+    name: CUSTOM_DASHBOARD_PAGE,
+    path: '/:dashboard(.*)',
     component: DashboardPage,
   },
 ];

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

@@ -97,6 +97,10 @@ export const clearExpandedPanel = ({ commit }) => {
   });
 };
 
+export const setCurrentDashboard = ({ commit }, { currentDashboard }) => {
+  commit(types.SET_CURRENT_DASHBOARD, currentDashboard);
+};
+
 // All Data
 
 /**

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

@@ -9,6 +9,8 @@ export const REQUEST_DASHBOARD_STARRING = 'REQUEST_DASHBOARD_STARRING';
 export const RECEIVE_DASHBOARD_STARRING_SUCCESS = 'RECEIVE_DASHBOARD_STARRING_SUCCESS';
 export const RECEIVE_DASHBOARD_STARRING_FAILURE = 'RECEIVE_DASHBOARD_STARRING_FAILURE';
 
+export const SET_CURRENT_DASHBOARD = 'SET_CURRENT_DASHBOARD';
+
 // Annotations
 export const RECEIVE_ANNOTATIONS_SUCCESS = 'RECEIVE_ANNOTATIONS_SUCCESS';
 export const RECEIVE_ANNOTATIONS_FAILURE = 'RECEIVE_ANNOTATIONS_FAILURE';

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

@@ -97,6 +97,10 @@ export default {
     state.isUpdatingStarredValue = false;
   },
 
+  [types.SET_CURRENT_DASHBOARD](state, currentDashboard) {
+    state.currentDashboard = currentDashboard;
+  },
+
   /**
    * Deployments and environments
    */

changelogs/unreleased/short-url-for-custom-metrics-dashboards.yml

+---
+title: Support short urls for custom metrics dashboards
+merge_request: 36740
+author:
+type: added

spec/features/projects/environments/environment_metrics_spec.rb

@@ -28,8 +28,9 @@ RSpec.describe 'Environment > Metrics' do
   shared_examples 'has environment selector' do
     it 'has a working environment selector', :js do
       click_link('See metrics')
-
-      expect(page).to have_metrics_path(environment)
+      # TODO: See metrics on the sidebar still points to the old metrics URL
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/229277
+      expect(page).to have_current_path(metrics_project_environment_path(project, id: environment.id))
       expect(page).to have_css('[data-qa-selector="environments_dropdown"]')
 
       within('[data-qa-selector="environments_dropdown"]') do
@@ -40,7 +41,7 @@ RSpec.describe 'Environment > Metrics' do
         click_on(staging.name)
       end
 
-      expect(page).to have_metrics_path(staging)
+      expect(page).to have_current_path(project_metrics_dashboard_path(project, environment: staging.id))
 
       wait_for_requests
     end
@@ -67,8 +68,4 @@ RSpec.describe 'Environment > Metrics' do
   def visit_environment(environment)
     visit project_environment_path(environment.project, environment)
   end
-
-  def have_metrics_path(environment)
-    have_current_path(metrics_project_environment_path(project, id: environment.id))
-  end
 end

spec/frontend/monitoring/components/dashboard_header_spec.js

@@ -9,7 +9,7 @@ import {
   selfMonitoringDashboardGitResponse,
   dashboardHeaderProps,
 } from '../mock_data';
-import { redirectTo, mergeUrlParams } from '~/lib/utils/url_utility';
+import { redirectTo } from '~/lib/utils/url_utility';
 
 jest.mock('~/lib/utils/url_utility', () => ({
   redirectTo: jest.fn(),
@@ -46,6 +46,9 @@ describe('Dashboard header', () => {
   });
 
   describe('when a dashboard has been duplicated in the duplicate dashboard modal', () => {
+    beforeEach(() => {
+      store.state.monitoringDashboard.projectPath = 'root/sandbox';
+    });
     /**
      * The duplicate dashboard modal gets called both by a menu item from the
      * dashboards dropdown and by an item from the actions menu.
@@ -58,12 +61,10 @@ describe('Dashboard header', () => {
       window.location = new URL('https://localhost');
 
       const newDashboard = dashboardGitResponse[1];
-      const params = {
-        dashboard: encodeURIComponent(newDashboard.path),
-      };
-      const newDashboardUrl = mergeUrlParams(params, window.location.href);
 
       createShallowWrapper();
+
+      const newDashboardUrl = 'root/sandbox/-/metrics/dashboard.yml';
       findDuplicateDashboardModal().vm.$emit('dashboardDuplicated', newDashboard);
 
       return wrapper.vm.$nextTick().then(() => {

spec/frontend/monitoring/components/dashboard_spec.js

@@ -433,6 +433,10 @@ describe('Dashboard', () => {
       const findDashboardDropdown = () => wrapper.find(DashboardHeader).find(DashboardsDropdown);
 
       beforeEach(() => {
+        store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
+          projectPath: TEST_HOST,
+        });
+
         delete window.location;
         window.location = { ...windowLocation, assign: jest.fn() };
         createMountedWrapper();
@@ -446,10 +450,11 @@ describe('Dashboard', () => {
 
       it('encodes dashboard param', () => {
         findDashboardDropdown().vm.$emit('selectDashboard', {
-          path: 'dashboard&copy.yml',
+          path: '.gitlab/dashboards/dashboard&copy.yml',
+          display_name: 'dashboard&copy.yml',
         });
         expect(window.location.assign).toHaveBeenCalledWith(
-          `${TEST_HOST}/?dashboard=dashboard%2526copy.yml`,
+          `${TEST_HOST}/-/metrics/dashboard%26copy.yml`,
         );
       });
     });
@@ -486,6 +491,8 @@ describe('Dashboard', () => {
     beforeEach(() => {
       store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
         currentEnvironmentName: 'production',
+        currentDashboard: dashboardGitResponse[0].path,
+        projectPath: TEST_HOST,
       });
       createMountedWrapper({ hasMetrics: true });
       setupStoreWithData(store);
@@ -498,9 +505,12 @@ describe('Dashboard', () => {
 
       findAllEnvironmentsDropdownItems().wrappers.forEach((itemWrapper, index) => {
         const anchorEl = itemWrapper.find('a');
-        if (anchorEl.exists() && environmentData[index].metrics_path) {
+        if (anchorEl.exists()) {
           const href = anchorEl.attributes('href');
-          expect(href).toBe(environmentData[index].metrics_path);
+          const currentDashboard = encodeURIComponent(dashboardGitResponse[0].path);
+          const environmentId = encodeURIComponent(environmentData[index].id);
+          const url = `${TEST_HOST}/-/metrics/${currentDashboard}?environment=${environmentId}`;
+          expect(href).toBe(url);
         }
       });
     });

spec/frontend/monitoring/fixture_data.js

@@ -10,6 +10,7 @@ import { metricsResult } from './mock_data';
 export const metricsDashboardResponse = getJSONFixture(
   'metrics_dashboard/environment_metrics_dashboard.json',
 );
+
 export const metricsDashboardPayload = metricsDashboardResponse.dashboard;
 
 const datasetState = stateAndPropsFromDataset(
@@ -22,7 +23,15 @@ const datasetState = stateAndPropsFromDataset(
   ),
 );
 
-export const dashboardProps = datasetState.dataProps;
+// new properties like addDashboardDocumentationPath prop and alertsEndpoint
+// was recently added to dashboard.vue component this needs to be
+// added to fixtures data
+// https://gitlab.com/gitlab-org/gitlab/-/issues/229256
+export const dashboardProps = {
+  ...datasetState.dataProps,
+  addDashboardDocumentationPath: 'https://path/to/docs',
+  alertsEndpoint: null,
+};
 
 export const metricsDashboardViewModel = mapToDashboardViewModel(metricsDashboardPayload);
 

spec/frontend/monitoring/pages/dashboard_page_spec.js

 import { shallowMount } from '@vue/test-utils';
+import { createStore } from '~/monitoring/stores';
 import DashboardPage from '~/monitoring/pages/dashboard_page.vue';
 import Dashboard from '~/monitoring/components/dashboard.vue';
 import { dashboardProps } from '../fixture_data';
 
 describe('monitoring/pages/dashboard_page', () => {
   let wrapper;
+  let store;
+  let $route;
+
+  const buildRouter = () => {
+    const dashboard = {};
+    $route = {
+      params: { dashboard },
+      query: { dashboard },
+    };
+  };
 
   const buildWrapper = (props = {}) => {
     wrapper = shallowMount(DashboardPage, {
+      store,
       propsData: {
         ...props,
       },
+      mocks: {
+        $route,
+      },
     });
   };
 
   const findDashboardComponent = () => wrapper.find(Dashboard);
 
+  beforeEach(() => {
+    buildRouter();
+    store = createStore();
+    jest.spyOn(store, 'dispatch').mockResolvedValue();
+  });
+
   afterEach(() => {
     if (wrapper) {
       wrapper.destroy();

spec/frontend/monitoring/router_spec.js

+import { mount, createLocalVue } from '@vue/test-utils';
+import VueRouter from 'vue-router';
+import DashboardPage from '~/monitoring/pages/dashboard_page.vue';
+import Dashboard from '~/monitoring/components/dashboard.vue';
+import { createStore } from '~/monitoring/stores';
+import createRouter from '~/monitoring/router';
+import { dashboardProps } from './fixture_data';
+import { dashboardHeaderProps } from './mock_data';
+
+describe('Monitoring router', () => {
+  let router;
+  let store;
+  const propsData = { dashboardProps: { ...dashboardProps, ...dashboardHeaderProps } };
+  const NEW_BASE_PATH = '/project/my-group/test-project/-/metrics';
+  const OLD_BASE_PATH = '/project/my-group/test-project/-/environments/71146/metrics';
+
+  const createWrapper = (basePath, routeArg) => {
+    const localVue = createLocalVue();
+    localVue.use(VueRouter);
+
+    router = createRouter(basePath);
+    if (routeArg !== undefined) {
+      router.push(routeArg);
+    }
+
+    return mount(DashboardPage, {
+      localVue,
+      store,
+      router,
+      propsData,
+    });
+  };
+
+  beforeEach(() => {
+    store = createStore();
+    jest.spyOn(store, 'dispatch').mockResolvedValue();
+  });
+
+  afterEach(() => {
+    window.location.hash = '';
+  });
+
+  describe('support old URL with full dashboard path', () => {
+    it.each`
+      route                          | currentDashboard
+      ${'/dashboard.yml'}            | ${'dashboard.yml'}
+      ${'/folder1/dashboard.yml'}    | ${'folder1/dashboard.yml'}
+      ${'/?dashboard=dashboard.yml'} | ${'dashboard.yml'}
+    `('sets component as $componentName for path "$route"', ({ route, currentDashboard }) => {
+      const wrapper = createWrapper(OLD_BASE_PATH, route);
+
+      expect(store.dispatch).toHaveBeenCalledWith('monitoringDashboard/setCurrentDashboard', {
+        currentDashboard,
+      });
+
+      expect(wrapper.find(Dashboard)).toExist();
+    });
+  });
+
+  describe('supports new URL with short dashboard path', () => {
+    it.each`
+      route                                       | currentDashboard
+      ${'/'}                                      | ${null}
+      ${'/dashboard.yml'}                         | ${'dashboard.yml'}
+      ${'/folder1/dashboard.yml'}                 | ${'folder1/dashboard.yml'}
+      ${'/folder1%2Fdashboard.yml'}               | ${'folder1/dashboard.yml'}
+      ${'/dashboard.yml'}                         | ${'dashboard.yml'}
+      ${'/config/prometheus/common_metrics.yml'}  | ${'config/prometheus/common_metrics.yml'}
+      ${'/config/prometheus/pod_metrics.yml'}     | ${'config/prometheus/pod_metrics.yml'}
+      ${'/config%2Fprometheus%2Fpod_metrics.yml'} | ${'config/prometheus/pod_metrics.yml'}
+    `('sets component as $componentName for path "$route"', ({ route, currentDashboard }) => {
+      const wrapper = createWrapper(NEW_BASE_PATH, route);
+
+      expect(store.dispatch).toHaveBeenCalledWith('monitoringDashboard/setCurrentDashboard', {
+        currentDashboard,
+      });
+
+      expect(wrapper.find(Dashboard)).toExist();
+    });
+  });
+});