diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index 02f4e26ba3bf78a750e5b6bc2a12488e440aa8fc..de73da67fcc4dfc409cbd8a1613d9879e97110d5 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -228,13 +228,11 @@ export default { 'promVariables', 'isUpdatingStarredValue', ]), - ...mapGetters('monitoringDashboard', ['getMetricStates', 'filteredEnvironments']), - firstDashboard() { - return this.allDashboards.length > 0 ? this.allDashboards[0] : {}; - }, - selectedDashboard() { - return this.allDashboards.find(d => d.path === this.currentDashboard) || this.firstDashboard; - }, + ...mapGetters('monitoringDashboard', [ + 'selectedDashboard', + 'getMetricStates', + 'filteredEnvironments', + ]), showRearrangePanelsBtn() { return !this.showEmptyState && this.rearrangePanelsAvailable; }, @@ -242,7 +240,10 @@ export default { return ( this.customMetricsAvailable && !this.showEmptyState && - this.firstDashboard === this.selectedDashboard + // Custom metrics only avaialble on system dashboards because + // they are stored in the database. This can be improved. See: + // https://gitlab.com/gitlab-org/gitlab/-/issues/28241 + this.selectedDashboard?.system_dashboard ); }, shouldShowEnvironmentsDropdownNoMatchedMsg() { @@ -269,7 +270,7 @@ export default { }, expandedPanel: { handler({ group, panel }) { - const dashboardPath = this.currentDashboard || this.firstDashboard.path; + const dashboardPath = this.currentDashboard || this.selectedDashboard?.path; updateHistory({ url: panelToUrl(dashboardPath, group, panel), title: document.title, @@ -341,7 +342,7 @@ export default { this.selectedTimeRange = defaultTimeRange; }, generatePanelUrl(groupKey, panel) { - const dashboardPath = this.currentDashboard || this.firstDashboard.path; + const dashboardPath = this.currentDashboard || this.selectedDashboard?.path; return panelToUrl(dashboardPath, groupKey, panel); }, hideAddMetricModal() { @@ -597,7 +598,10 @@ export default { </gl-modal> </div> - <div v-if="selectedDashboard.can_edit" class="mb-2 mr-2 d-flex d-sm-block"> + <div + v-if="selectedDashboard && selectedDashboard.can_edit" + class="mb-2 mr-2 d-flex d-sm-block" + > <gl-deprecated-button class="flex-grow-1 js-edit-link" :href="selectedDashboard.project_blob_path" diff --git a/app/assets/javascripts/monitoring/stores/getters.js b/app/assets/javascripts/monitoring/stores/getters.js index 12757bed588220364c7a981584130a7ddce33378..63a7d0c643de2cceee1f3bfba924d771d2194bce 100644 --- a/app/assets/javascripts/monitoring/stores/getters.js +++ b/app/assets/javascripts/monitoring/stores/getters.js @@ -14,7 +14,9 @@ const metricsIdsInPanel = panel => export const selectedDashboard = state => { const { allDashboards } = state; return ( - allDashboards.find(({ path }) => path === state.currentDashboard) || allDashboards[0] || null + allDashboards.find(d => d.path === state.currentDashboard) || + allDashboards.find(d => d.default) || + null ); }; diff --git a/spec/frontend/monitoring/components/dashboard_spec.js b/spec/frontend/monitoring/components/dashboard_spec.js index 22bc3eef5f86b15144af3095f68618e61ada6044..1fc02f0abe15cbbcfc22fbceff0da5b6eaab2bcb 100644 --- a/spec/frontend/monitoring/components/dashboard_spec.js +++ b/spec/frontend/monitoring/components/dashboard_spec.js @@ -19,6 +19,7 @@ import DashboardPanel from '~/monitoring/components/dashboard_panel.vue'; import { createStore } from '~/monitoring/stores'; import * as types from '~/monitoring/stores/mutation_types'; import { + setupAllDashboards, setupStoreWithDashboard, setMetricResult, setupStoreWithData, @@ -279,7 +280,7 @@ describe('Dashboard', () => { expect(window.history.pushState).toHaveBeenCalledWith( expect.anything(), // state expect.any(String), // document title - expect.stringContaining(`?${expectedSearch}`), + expect.stringContaining(`${expectedSearch}`), ); }); }); @@ -302,7 +303,7 @@ describe('Dashboard', () => { expect(window.history.pushState).toHaveBeenCalledWith( expect.anything(), // state expect.any(String), // document title - expect.stringContaining(`?${expectedSearch}`), + expect.stringContaining(`${expectedSearch}`), ); }); }); @@ -317,7 +318,7 @@ describe('Dashboard', () => { expect(window.history.pushState).toHaveBeenCalledWith( expect.anything(), // state expect.any(String), // document title - expect.not.stringContaining('?'), // no params + expect.not.stringMatching(/group|title|y_label/), // no panel params ); }); }); @@ -359,6 +360,7 @@ describe('Dashboard', () => { beforeEach(() => { createShallowWrapper(); + setupAllDashboards(store); }); it('toggle star button is shown', () => { @@ -380,10 +382,7 @@ describe('Dashboard', () => { const getToggleTooltip = () => findToggleStar().element.parentElement.getAttribute('title'); beforeEach(() => { - wrapper.vm.$store.commit( - `monitoringDashboard/${types.SET_ALL_DASHBOARDS}`, - dashboardGitResponse, - ); + setupAllDashboards(store); jest.spyOn(store, 'dispatch'); }); @@ -400,7 +399,9 @@ describe('Dashboard', () => { describe('when dashboard is not starred', () => { beforeEach(() => { - wrapper.setProps({ currentDashboard: dashboardGitResponse[0].path }); + store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, { + currentDashboard: dashboardGitResponse[0].path, + }); return wrapper.vm.$nextTick(); }); @@ -415,7 +416,9 @@ describe('Dashboard', () => { describe('when dashboard is starred', () => { beforeEach(() => { - wrapper.setProps({ currentDashboard: dashboardGitResponse[1].path }); + store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, { + currentDashboard: dashboardGitResponse[1].path, + }); return wrapper.vm.$nextTick(); }); @@ -551,7 +554,7 @@ describe('Dashboard', () => { it('sets a link to the expanded panel', () => { const searchQuery = - '?group=System%20metrics%20(Kubernetes)&title=Memory%20Usage%20(Total)&y_label=Total%20Memory%20Used%20(GB)'; + '?dashboard=config%2Fprometheus%2Fcommon_metrics.yml&group=System%20metrics%20(Kubernetes)&title=Memory%20Usage%20(Total)&y_label=Total%20Memory%20Used%20(GB)'; expect(findExpandedPanel().attributes('clipboard-text')).toEqual( expect.stringContaining(searchQuery), @@ -808,10 +811,7 @@ describe('Dashboard', () => { beforeEach(() => { createShallowWrapper({ hasMetrics: true }); - wrapper.vm.$store.commit( - `monitoringDashboard/${types.SET_ALL_DASHBOARDS}`, - dashboardGitResponse, - ); + setupAllDashboards(store); return wrapper.vm.$nextTick(); }); @@ -820,10 +820,11 @@ describe('Dashboard', () => { }); it('is present for a custom dashboard, and links to its edit_path', () => { - const dashboard = dashboardGitResponse[1]; // non-default dashboard - const currentDashboard = dashboard.path; + const dashboard = dashboardGitResponse[1]; + store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, { + currentDashboard: dashboard.path, + }); - wrapper.setProps({ currentDashboard }); return wrapper.vm.$nextTick().then(() => { expect(findEditLink().exists()).toBe(true); expect(findEditLink().attributes('href')).toBe(dashboard.project_blob_path); @@ -834,12 +835,7 @@ describe('Dashboard', () => { describe('Dashboard dropdown', () => { beforeEach(() => { createMountedWrapper({ hasMetrics: true }); - - wrapper.vm.$store.commit( - `monitoringDashboard/${types.SET_ALL_DASHBOARDS}`, - dashboardGitResponse, - ); - + setupAllDashboards(store); return wrapper.vm.$nextTick(); }); @@ -872,7 +868,7 @@ describe('Dashboard', () => { }); describe('Clipboard text in panels', () => { - const currentDashboard = 'TEST_DASHBOARD'; + const currentDashboard = dashboardGitResponse[1].path; const panelIndex = 1; // skip expanded panel const getClipboardTextFirstPanel = () => @@ -882,37 +878,20 @@ describe('Dashboard', () => { .props('clipboardText'); beforeEach(() => { + setupStoreWithData(store); createShallowWrapper({ hasMetrics: true, currentDashboard }); - setupStoreWithData(wrapper.vm.$store); - return wrapper.vm.$nextTick(); }); it('contains a link to the dashboard', () => { - expect(getClipboardTextFirstPanel()).toContain(`dashboard=${currentDashboard}`); + const dashboardParam = `dashboard=${encodeURIComponent(currentDashboard)}`; + + expect(getClipboardTextFirstPanel()).toContain(dashboardParam); expect(getClipboardTextFirstPanel()).toContain(`group=`); expect(getClipboardTextFirstPanel()).toContain(`title=`); expect(getClipboardTextFirstPanel()).toContain(`y_label=`); }); - - it('strips the undefined parameter', () => { - wrapper.setProps({ currentDashboard: undefined }); - - return wrapper.vm.$nextTick(() => { - expect(getClipboardTextFirstPanel()).not.toContain(`dashboard=`); - expect(getClipboardTextFirstPanel()).toContain(`y_label=`); - }); - }); - - it('null parameter is stripped', () => { - wrapper.setProps({ currentDashboard: null }); - - return wrapper.vm.$nextTick(() => { - expect(getClipboardTextFirstPanel()).not.toContain(`dashboard=`); - expect(getClipboardTextFirstPanel()).toContain(`y_label=`); - }); - }); }); describe('add custom metrics', () => { diff --git a/spec/frontend/monitoring/components/dashboard_template_spec.js b/spec/frontend/monitoring/components/dashboard_template_spec.js index 0257515f18e82a02f89a51ec9b8fb0852b577e68..cc0ac348b11fcbb78742514604627a7c132b971b 100644 --- a/spec/frontend/monitoring/components/dashboard_template_spec.js +++ b/spec/frontend/monitoring/components/dashboard_template_spec.js @@ -3,6 +3,7 @@ import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import Dashboard from '~/monitoring/components/dashboard.vue'; import { createStore } from '~/monitoring/stores'; +import { setupAllDashboards } from '../store_utils'; import { propsData } from '../mock_data'; jest.mock('~/lib/utils/url_utility'); @@ -15,6 +16,8 @@ describe('Dashboard template', () => { beforeEach(() => { store = createStore(); mock = new MockAdapter(axios); + + setupAllDashboards(store); }); afterEach(() => { diff --git a/spec/frontend/monitoring/store_utils.js b/spec/frontend/monitoring/store_utils.js index b3f87fb7abf2c010ee2f0787cef0554b54b21c80..3cfd3baaee7bcecc0fe380f469de15300be53da1 100644 --- a/spec/frontend/monitoring/store_utils.js +++ b/spec/frontend/monitoring/store_utils.js @@ -1,5 +1,5 @@ import * as types from '~/monitoring/stores/mutation_types'; -import { metricsResult, environmentData } from './mock_data'; +import { metricsResult, environmentData, dashboardGitResponse } from './mock_data'; import { metricsDashboardPayload } from './fixture_data'; export const setMetricResult = ({ $store, result, group = 0, panel = 0, metric = 0 }) => { @@ -16,11 +16,19 @@ const setEnvironmentData = $store => { $store.commit(`monitoringDashboard/${types.RECEIVE_ENVIRONMENTS_DATA_SUCCESS}`, environmentData); }; +export const setupAllDashboards = $store => { + $store.commit(`monitoringDashboard/${types.SET_ALL_DASHBOARDS}`, dashboardGitResponse); +}; + export const setupStoreWithDashboard = $store => { $store.commit( `monitoringDashboard/${types.RECEIVE_METRICS_DASHBOARD_SUCCESS}`, metricsDashboardPayload, ); + $store.commit( + `monitoringDashboard/${types.RECEIVE_METRICS_DASHBOARD_SUCCESS}`, + metricsDashboardPayload, + ); }; export const setupStoreWithVariable = $store => { @@ -30,6 +38,7 @@ export const setupStoreWithVariable = $store => { }; export const setupStoreWithData = $store => { + setupAllDashboards($store); setupStoreWithDashboard($store); setMetricResult({ $store, result: [], panel: 0 });