Commit f1ac764f authored by Dhiraj Bodicherla's avatar Dhiraj Bodicherla

Support short urls for custom metrics dashboards

Custom metrics dashboards currently display the entire
file path in the URL. This MR supports a shorter path
for such dashboards
parent 27fe6f83
...@@ -127,6 +127,7 @@ export default { ...@@ -127,6 +127,7 @@ export default {
'projectPath', 'projectPath',
'canAccessOperationsSettings', 'canAccessOperationsSettings',
'operationsSettingsPath', 'operationsSettingsPath',
'currentDashboard',
]), ]),
...mapGetters('monitoringDashboard', ['selectedDashboard', 'filteredEnvironments']), ...mapGetters('monitoringDashboard', ['selectedDashboard', 'filteredEnvironments']),
isOutOfTheBoxDashboard() { isOutOfTheBoxDashboard() {
...@@ -164,11 +165,14 @@ export default { ...@@ -164,11 +165,14 @@ export default {
methods: { methods: {
...mapActions('monitoringDashboard', ['filterEnvironments', 'toggleStarredValue']), ...mapActions('monitoringDashboard', ['filterEnvironments', 'toggleStarredValue']),
selectDashboard(dashboard) { selectDashboard(dashboard) {
const params = { // Once the sidebar See metrics link is updated to the new URL,
dashboard: encodeURIComponent(dashboard.path), // this sort of hardcoding will not be necessary.
}; // https://gitlab.com/gitlab-org/gitlab/-/issues/229277
const baseURL = `${this.projectPath}/-/metrics`;
redirectTo(mergeUrlParams(params, window.location.href)); const dashboardPath = encodeURIComponent(
dashboard.out_of_the_box_dashboard ? dashboard.path : dashboard.display_name,
);
redirectTo(`${baseURL}/${dashboardPath}`);
}, },
debouncedEnvironmentsSearch: debounce(function environmentsSearchOnInput(searchTerm) { debouncedEnvironmentsSearch: debounce(function environmentsSearchOnInput(searchTerm) {
this.filterEnvironments(searchTerm); this.filterEnvironments(searchTerm);
...@@ -193,6 +197,17 @@ export default { ...@@ -193,6 +197,17 @@ export default {
submitCustomMetricsForm() { submitCustomMetricsForm() {
this.$refs.customMetricsForm.submit(); 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: { modalIds: {
addMetric: 'addMetric', addMetric: 'addMetric',
...@@ -255,7 +270,7 @@ export default { ...@@ -255,7 +270,7 @@ export default {
:key="environment.id" :key="environment.id"
:active="environment.name === currentEnvironmentName" :active="environment.name === currentEnvironmentName"
active-class="is-active" active-class="is-active"
:href="environment.metrics_path" :href="getEnvironmentPath(environment.id)"
>{{ environment.name }}</gl-dropdown-item >{{ environment.name }}</gl-dropdown-item
> >
</div> </div>
......
import Vue from 'vue'; import Vue from 'vue';
import { GlToast } from '@gitlab/ui'; import { GlToast } from '@gitlab/ui';
import { getParameterValues } from '~/lib/utils/url_utility';
import { createStore } from './stores'; import { createStore } from './stores';
import createRouter from './router'; import createRouter from './router';
import { stateAndPropsFromDataset } from './utils'; import { stateAndPropsFromDataset } from './utils';
...@@ -11,11 +10,9 @@ export default (props = {}) => { ...@@ -11,11 +10,9 @@ export default (props = {}) => {
const el = document.getElementById('prometheus-graphs'); const el = document.getElementById('prometheus-graphs');
if (el && el.dataset) { if (el && el.dataset) {
const [encodedDashboard] = getParameterValues('dashboard');
const currentDashboard = encodedDashboard ? decodeURIComponent(encodedDashboard) : null;
const { metricsDashboardBasePath, ...dataset } = el.dataset; const { metricsDashboardBasePath, ...dataset } = el.dataset;
const { initState, dataProps } = stateAndPropsFromDataset({ currentDashboard, ...dataset }); const { initState, dataProps } = stateAndPropsFromDataset(dataset);
const store = createStore(initState); const store = createStore(initState);
const router = createRouter(metricsDashboardBasePath); const router = createRouter(metricsDashboardBasePath);
......
<script> <script>
import { mapActions } from 'vuex';
import Dashboard from '../components/dashboard.vue'; import Dashboard from '../components/dashboard.vue';
export default { export default {
...@@ -11,6 +12,16 @@ export default { ...@@ -11,6 +12,16 @@ export default {
required: true, 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> </script>
<template> <template>
......
export const BASE_DASHBOARD_PAGE = 'dashboard'; export const BASE_DASHBOARD_PAGE = 'dashboard';
export const CUSTOM_DASHBOARD_PAGE = 'custom_dashboard';
export default {}; export default {};
import DashboardPage from '../pages/dashboard_page.vue'; 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 * Because the cluster health page uses the dashboard
...@@ -12,7 +12,12 @@ import { BASE_DASHBOARD_PAGE } from './constants'; ...@@ -12,7 +12,12 @@ import { BASE_DASHBOARD_PAGE } from './constants';
export default [ export default [
{ {
name: BASE_DASHBOARD_PAGE, name: BASE_DASHBOARD_PAGE,
path: '*', path: '/',
component: DashboardPage,
},
{
name: CUSTOM_DASHBOARD_PAGE,
path: '/:dashboard(.*)',
component: DashboardPage, component: DashboardPage,
}, },
]; ];
...@@ -97,6 +97,10 @@ export const clearExpandedPanel = ({ commit }) => { ...@@ -97,6 +97,10 @@ export const clearExpandedPanel = ({ commit }) => {
}); });
}; };
export const setCurrentDashboard = ({ commit }, { currentDashboard }) => {
commit(types.SET_CURRENT_DASHBOARD, currentDashboard);
};
// All Data // All Data
/** /**
......
...@@ -9,6 +9,8 @@ export const REQUEST_DASHBOARD_STARRING = 'REQUEST_DASHBOARD_STARRING'; ...@@ -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_SUCCESS = 'RECEIVE_DASHBOARD_STARRING_SUCCESS';
export const RECEIVE_DASHBOARD_STARRING_FAILURE = 'RECEIVE_DASHBOARD_STARRING_FAILURE'; export const RECEIVE_DASHBOARD_STARRING_FAILURE = 'RECEIVE_DASHBOARD_STARRING_FAILURE';
export const SET_CURRENT_DASHBOARD = 'SET_CURRENT_DASHBOARD';
// Annotations // Annotations
export const RECEIVE_ANNOTATIONS_SUCCESS = 'RECEIVE_ANNOTATIONS_SUCCESS'; export const RECEIVE_ANNOTATIONS_SUCCESS = 'RECEIVE_ANNOTATIONS_SUCCESS';
export const RECEIVE_ANNOTATIONS_FAILURE = 'RECEIVE_ANNOTATIONS_FAILURE'; export const RECEIVE_ANNOTATIONS_FAILURE = 'RECEIVE_ANNOTATIONS_FAILURE';
......
...@@ -97,6 +97,10 @@ export default { ...@@ -97,6 +97,10 @@ export default {
state.isUpdatingStarredValue = false; state.isUpdatingStarredValue = false;
}, },
[types.SET_CURRENT_DASHBOARD](state, currentDashboard) {
state.currentDashboard = currentDashboard;
},
/** /**
* Deployments and environments * Deployments and environments
*/ */
......
---
title: Support short urls for custom metrics dashboards
merge_request: 36740
author:
type: added
...@@ -28,8 +28,9 @@ RSpec.describe 'Environment > Metrics' do ...@@ -28,8 +28,9 @@ RSpec.describe 'Environment > Metrics' do
shared_examples 'has environment selector' do shared_examples 'has environment selector' do
it 'has a working environment selector', :js do it 'has a working environment selector', :js do
click_link('See metrics') click_link('See metrics')
# TODO: See metrics on the sidebar still points to the old metrics URL
expect(page).to have_metrics_path(environment) # 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"]') expect(page).to have_css('[data-qa-selector="environments_dropdown"]')
within('[data-qa-selector="environments_dropdown"]') do within('[data-qa-selector="environments_dropdown"]') do
...@@ -40,7 +41,7 @@ RSpec.describe 'Environment > Metrics' do ...@@ -40,7 +41,7 @@ RSpec.describe 'Environment > Metrics' do
click_on(staging.name) click_on(staging.name)
end 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 wait_for_requests
end end
...@@ -67,8 +68,4 @@ RSpec.describe 'Environment > Metrics' do ...@@ -67,8 +68,4 @@ RSpec.describe 'Environment > Metrics' do
def visit_environment(environment) def visit_environment(environment)
visit project_environment_path(environment.project, environment) visit project_environment_path(environment.project, environment)
end end
def have_metrics_path(environment)
have_current_path(metrics_project_environment_path(project, id: environment.id))
end
end end
...@@ -9,7 +9,7 @@ import { ...@@ -9,7 +9,7 @@ import {
selfMonitoringDashboardGitResponse, selfMonitoringDashboardGitResponse,
dashboardHeaderProps, dashboardHeaderProps,
} from '../mock_data'; } from '../mock_data';
import { redirectTo, mergeUrlParams } from '~/lib/utils/url_utility'; import { redirectTo } from '~/lib/utils/url_utility';
jest.mock('~/lib/utils/url_utility', () => ({ jest.mock('~/lib/utils/url_utility', () => ({
redirectTo: jest.fn(), redirectTo: jest.fn(),
...@@ -46,6 +46,9 @@ describe('Dashboard header', () => { ...@@ -46,6 +46,9 @@ describe('Dashboard header', () => {
}); });
describe('when a dashboard has been duplicated in the duplicate dashboard modal', () => { 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 * The duplicate dashboard modal gets called both by a menu item from the
* dashboards dropdown and by an item from the actions menu. * dashboards dropdown and by an item from the actions menu.
...@@ -58,12 +61,10 @@ describe('Dashboard header', () => { ...@@ -58,12 +61,10 @@ describe('Dashboard header', () => {
window.location = new URL('https://localhost'); window.location = new URL('https://localhost');
const newDashboard = dashboardGitResponse[1]; const newDashboard = dashboardGitResponse[1];
const params = {
dashboard: encodeURIComponent(newDashboard.path),
};
const newDashboardUrl = mergeUrlParams(params, window.location.href);
createShallowWrapper(); createShallowWrapper();
const newDashboardUrl = 'root/sandbox/-/metrics/dashboard.yml';
findDuplicateDashboardModal().vm.$emit('dashboardDuplicated', newDashboard); findDuplicateDashboardModal().vm.$emit('dashboardDuplicated', newDashboard);
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
......
...@@ -433,6 +433,10 @@ describe('Dashboard', () => { ...@@ -433,6 +433,10 @@ describe('Dashboard', () => {
const findDashboardDropdown = () => wrapper.find(DashboardHeader).find(DashboardsDropdown); const findDashboardDropdown = () => wrapper.find(DashboardHeader).find(DashboardsDropdown);
beforeEach(() => { beforeEach(() => {
store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
projectPath: TEST_HOST,
});
delete window.location; delete window.location;
window.location = { ...windowLocation, assign: jest.fn() }; window.location = { ...windowLocation, assign: jest.fn() };
createMountedWrapper(); createMountedWrapper();
...@@ -446,10 +450,11 @@ describe('Dashboard', () => { ...@@ -446,10 +450,11 @@ describe('Dashboard', () => {
it('encodes dashboard param', () => { it('encodes dashboard param', () => {
findDashboardDropdown().vm.$emit('selectDashboard', { findDashboardDropdown().vm.$emit('selectDashboard', {
path: 'dashboard&copy.yml', path: '.gitlab/dashboards/dashboard&copy.yml',
display_name: 'dashboard&copy.yml',
}); });
expect(window.location.assign).toHaveBeenCalledWith( expect(window.location.assign).toHaveBeenCalledWith(
`${TEST_HOST}/?dashboard=dashboard%2526copy.yml`, `${TEST_HOST}/-/metrics/dashboard%26copy.yml`,
); );
}); });
}); });
...@@ -486,6 +491,8 @@ describe('Dashboard', () => { ...@@ -486,6 +491,8 @@ describe('Dashboard', () => {
beforeEach(() => { beforeEach(() => {
store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, { store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
currentEnvironmentName: 'production', currentEnvironmentName: 'production',
currentDashboard: dashboardGitResponse[0].path,
projectPath: TEST_HOST,
}); });
createMountedWrapper({ hasMetrics: true }); createMountedWrapper({ hasMetrics: true });
setupStoreWithData(store); setupStoreWithData(store);
...@@ -498,9 +505,12 @@ describe('Dashboard', () => { ...@@ -498,9 +505,12 @@ describe('Dashboard', () => {
findAllEnvironmentsDropdownItems().wrappers.forEach((itemWrapper, index) => { findAllEnvironmentsDropdownItems().wrappers.forEach((itemWrapper, index) => {
const anchorEl = itemWrapper.find('a'); const anchorEl = itemWrapper.find('a');
if (anchorEl.exists() && environmentData[index].metrics_path) { if (anchorEl.exists()) {
const href = anchorEl.attributes('href'); 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);
} }
}); });
}); });
......
...@@ -10,6 +10,7 @@ import { metricsResult } from './mock_data'; ...@@ -10,6 +10,7 @@ import { metricsResult } from './mock_data';
export const metricsDashboardResponse = getJSONFixture( export const metricsDashboardResponse = getJSONFixture(
'metrics_dashboard/environment_metrics_dashboard.json', 'metrics_dashboard/environment_metrics_dashboard.json',
); );
export const metricsDashboardPayload = metricsDashboardResponse.dashboard; export const metricsDashboardPayload = metricsDashboardResponse.dashboard;
const datasetState = stateAndPropsFromDataset( const datasetState = stateAndPropsFromDataset(
...@@ -22,7 +23,15 @@ 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); export const metricsDashboardViewModel = mapToDashboardViewModel(metricsDashboardPayload);
......
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import { createStore } from '~/monitoring/stores';
import DashboardPage from '~/monitoring/pages/dashboard_page.vue'; import DashboardPage from '~/monitoring/pages/dashboard_page.vue';
import Dashboard from '~/monitoring/components/dashboard.vue'; import Dashboard from '~/monitoring/components/dashboard.vue';
import { dashboardProps } from '../fixture_data'; import { dashboardProps } from '../fixture_data';
describe('monitoring/pages/dashboard_page', () => { describe('monitoring/pages/dashboard_page', () => {
let wrapper; let wrapper;
let store;
let $route;
const buildRouter = () => {
const dashboard = {};
$route = {
params: { dashboard },
query: { dashboard },
};
};
const buildWrapper = (props = {}) => { const buildWrapper = (props = {}) => {
wrapper = shallowMount(DashboardPage, { wrapper = shallowMount(DashboardPage, {
store,
propsData: { propsData: {
...props, ...props,
}, },
mocks: {
$route,
},
}); });
}; };
const findDashboardComponent = () => wrapper.find(Dashboard); const findDashboardComponent = () => wrapper.find(Dashboard);
beforeEach(() => {
buildRouter();
store = createStore();
jest.spyOn(store, 'dispatch').mockResolvedValue();
});
afterEach(() => { afterEach(() => {
if (wrapper) { if (wrapper) {
wrapper.destroy(); wrapper.destroy();
......
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();
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment