Merge branch '228759-add-new-router-page' into 'master'

Add a new router page to "Add a Panel"

See merge request gitlab-org/gitlab!37563

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

+<script>
+import { GlButton, GlTooltipDirective } from '@gitlab/ui';
+import { s__ } from '~/locale';
+import routes from '../router/constants';
+
+export default {
+  components: {
+    GlButton,
+  },
+  directives: {
+    GlTooltip: GlTooltipDirective,
+  },
+  i18n: {
+    backToDashboard: s__('Metrics|Back to dashboard'),
+  },
+  routes,
+};
+</script>
+<template>
+  <div class="gl-display-flex gl-align-items-baseline">
+    <gl-button
+      v-gl-tooltip
+      icon="go-back"
+      :to="{ name: $options.routes.DASHBOARD_PAGE, params: { dashboard: $route.params.dashboard } }"
+      :aria-label="$options.i18n.backToDashboard"
+      :title="$options.i18n.backToDashboard"
+      class="gl-mr-5"
+    />
+    <h1 class="gl-mt-5 gl-font-size-h1">{{ s__('Metrics|Add panel') }}</h1>
+
+    <!-- TODO: Add components. See https://gitlab.com/groups/gitlab-org/-/epics/2882  -->
+  </div>
+</template>

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

-export const BASE_DASHBOARD_PAGE = 'dashboard';
-export const CUSTOM_DASHBOARD_PAGE = 'custom_dashboard';
+export const DASHBOARD_PAGE = 'dashboard';
+export const PANEL_NEW_PAGE = 'panel_new';
 
-export default {};
+export default {
+  DASHBOARD_PAGE,
+  PANEL_NEW_PAGE,
+};

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

 import DashboardPage from '../pages/dashboard_page.vue';
+import PanelNewPage from '../pages/panel_new_page.vue';
 
-import { BASE_DASHBOARD_PAGE, CUSTOM_DASHBOARD_PAGE } from './constants';
+import { DASHBOARD_PAGE, PANEL_NEW_PAGE } from './constants';
 
 /**
  * Because the cluster health page uses the dashboard
@@ -11,13 +12,13 @@ import { BASE_DASHBOARD_PAGE, CUSTOM_DASHBOARD_PAGE } from './constants';
  */
 export default [
   {
-    name: BASE_DASHBOARD_PAGE,
-    path: '/',
-    component: DashboardPage,
+    name: PANEL_NEW_PAGE,
+    path: '/:dashboard(.*)?/panel/new',
+    component: PanelNewPage,
   },
   {
-    name: CUSTOM_DASHBOARD_PAGE,
-    path: '/:dashboard(.*)',
+    name: DASHBOARD_PAGE,
+    path: '/:dashboard(.*)?',
     component: DashboardPage,
   },
 ];

app/controllers/projects/metrics_dashboard_controller.rb

@@ -14,6 +14,10 @@ module Projects
     end
 
     def show
+      if params[:page].present? && !Feature.enabled?(:metrics_dashboard_new_panel_page, project)
+        return render_404
+      end
+
       if environment
         render 'projects/environments/metrics'
       else

config/routes/project.rb

@@ -25,7 +25,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       # Use this scope for all new project routes.
       scope '-' do
         get 'archive/*id', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+?/ }, to: 'repositories#archive', as: 'archive'
-        get 'metrics(/:dashboard_path)', constraints: { dashboard_path: /.+\.yml/ },
+        get 'metrics(/:dashboard_path)(/:page)', constraints: { dashboard_path: /.+\.yml/, page: 'panel/new' },
           to: 'metrics_dashboard#show', as: :metrics_dashboard, format: false
 
         resources :artifacts, only: [:index, :destroy]

locale/gitlab.pot

@@ -14984,9 +14984,15 @@ msgstr ""
 msgid "Metrics|Add metric"
 msgstr ""
 
+msgid "Metrics|Add panel"
+msgstr ""
+
 msgid "Metrics|Avg"
 msgstr ""
 
+msgid "Metrics|Back to dashboard"
+msgstr ""
+
 msgid "Metrics|Cancel"
 msgstr ""
 

spec/frontend/monitoring/pages/panel_new_page_spec.js

+import { shallowMount } from '@vue/test-utils';
+import { GlButton } from '@gitlab/ui';
+import { DASHBOARD_PAGE } from '~/monitoring/router/constants';
+import PanelNewPage from '~/monitoring/pages/panel_new_page.vue';
+
+const dashboard = 'dashboard.yml';
+
+// Button stub that can accept `to` as router links do
+// https://bootstrap-vue.org/docs/components/button#comp-ref-b-button-props
+const GlButtonStub = {
+  extends: GlButton,
+  props: {
+    to: [String, Object],
+  },
+};
+
+describe('monitoring/pages/panel_new_page', () => {
+  let wrapper;
+  let $route;
+
+  const mountComponent = (propsData = {}, routeParams = { dashboard }) => {
+    $route = {
+      params: routeParams,
+    };
+
+    wrapper = shallowMount(PanelNewPage, {
+      propsData,
+      stubs: {
+        GlButton: GlButtonStub,
+      },
+      mocks: {
+        $route,
+      },
+    });
+  };
+
+  const findBackButton = () => wrapper.find(GlButtonStub);
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('back to dashboard button', () => {
+    it('is rendered', () => {
+      mountComponent();
+      expect(findBackButton().exists()).toBe(true);
+      expect(findBackButton().props('icon')).toBe('go-back');
+    });
+
+    it('links back to the dashboard', () => {
+      const dashboardLocation = {
+        name: DASHBOARD_PAGE,
+        params: { dashboard },
+      };
+
+      expect(findBackButton().props('to')).toEqual(dashboardLocation);
+    });
+  });
+});

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 PanelNewPage from '~/monitoring/pages/panel_new_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';
 
+const LEGACY_BASE_PATH = '/project/my-group/test-project/-/environments/71146/metrics';
+const BASE_PATH = '/project/my-group/test-project/-/metrics';
+
+const MockApp = {
+  data() {
+    return {
+      dashboardProps: { ...dashboardProps, ...dashboardHeaderProps },
+    };
+  },
+  template: `<router-view  :dashboard-props="dashboardProps"/>`,
+};
+
 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();
@@ -23,11 +33,10 @@ describe('Monitoring router', () => {
       router.push(routeArg);
     }
 
-    return mount(DashboardPage, {
+    return mount(MockApp, {
       localVue,
       store,
       router,
-      propsData,
     });
   };
 
@@ -40,26 +49,32 @@ describe('Monitoring router', () => {
     window.location.hash = '';
   });
 
-  describe('support old URL with full dashboard path', () => {
+  describe('support legacy URLs with full dashboard path to visit dashboard page', () => {
     it.each`
-      route                          | currentDashboard
+      path                           | 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);
+    `('"$path" renders page with dashboard "$currentDashboard"', ({ path, currentDashboard }) => {
+      const wrapper = createWrapper(LEGACY_BASE_PATH, path);
 
       expect(store.dispatch).toHaveBeenCalledWith('monitoringDashboard/setCurrentDashboard', {
         currentDashboard,
       });
 
-      expect(wrapper.find(Dashboard)).toExist();
+      expect(wrapper.find(DashboardPage).exists()).toBe(true);
+      expect(
+        wrapper
+          .find(DashboardPage)
+          .find(Dashboard)
+          .exists(),
+      ).toBe(true);
     });
   });
 
-  describe('supports new URL with short dashboard path', () => {
+  describe('supports URLs to visit dashboard page', () => {
     it.each`
-      route                                       | currentDashboard
+      path                                        | currentDashboard
       ${'/'}                                      | ${null}
       ${'/dashboard.yml'}                         | ${'dashboard.yml'}
       ${'/folder1/dashboard.yml'}                 | ${'folder1/dashboard.yml'}
@@ -68,14 +83,35 @@ describe('Monitoring router', () => {
       ${'/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);
+    `('"$path" renders page with dashboard "$currentDashboard"', ({ path, currentDashboard }) => {
+      const wrapper = createWrapper(BASE_PATH, path);
 
       expect(store.dispatch).toHaveBeenCalledWith('monitoringDashboard/setCurrentDashboard', {
         currentDashboard,
       });
 
-      expect(wrapper.find(Dashboard)).toExist();
+      expect(wrapper.find(DashboardPage).exists()).toBe(true);
+      expect(
+        wrapper
+          .find(DashboardPage)
+          .find(Dashboard)
+          .exists(),
+      ).toBe(true);
+    });
+  });
+
+  describe('supports URLs to visit new panel page', () => {
+    it.each`
+      path                                                     | currentDashboard
+      ${'/panel/new'}                                          | ${undefined}
+      ${'/dashboard.yml/panel/new'}                            | ${'dashboard.yml'}
+      ${'/config/prometheus/common_metrics.yml/panel/new'}     | ${'config/prometheus/common_metrics.yml'}
+      ${'/config%2Fprometheus%2Fcommon_metrics.yml/panel/new'} | ${'config/prometheus/common_metrics.yml'}
+    `('"$path" renders page with dashboard "$currentDashboard"', ({ path, currentDashboard }) => {
+      const wrapper = createWrapper(BASE_PATH, path);
+
+      expect(wrapper.vm.$route.params.dashboard).toBe(currentDashboard);
+      expect(wrapper.find(PanelNewPage).exists()).toBe(true);
     });
   });
 });

spec/requests/projects/metrics_dashboard_spec.rb

@@ -79,6 +79,26 @@ RSpec.describe 'metrics dashboard page' do
     end
   end
 
+  describe 'GET :/namespace/:project/-/metrics/:page' do
+    it 'returns 200 with path param page and feature flag enabled' do
+      stub_feature_flags(metrics_dashboard_new_panel_page: true)
+
+      # send_request(page: 'panel/new') cannot be used because it encodes '/'
+      get "/#{project.namespace.to_param}/#{project.to_param}/-/metrics/panel/new"
+
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+
+    it 'returns 404 with path param page and feature flag disabled' do
+      stub_feature_flags(metrics_dashboard_new_panel_page: false)
+
+      # send_request(page: 'panel/new') cannot be used because it encodes '/'
+      get "/#{project.namespace.to_param}/#{project.to_param}/-/metrics/panel/new"
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
   def send_request(params = {})
     get namespace_project_metrics_dashboard_path(namespace_id: project.namespace, project_id: project, **params)
   end

spec/routing/project_routing_spec.rb

@@ -823,4 +823,66 @@ RSpec.describe 'project routing' do
                      project_id: 'gitlabhq', snippet_id: '1', ref: 'master', path: 'lib/version.rb')
     end
   end
+
+  describe Projects::MetricsDashboardController, 'routing' do
+    it 'routes to #show with no dashboard_path and no page' do
+      expect(get: "/gitlab/gitlabhq/-/metrics").to route_to(
+        "projects/metrics_dashboard#show",
+        **base_params
+      )
+    end
+
+    it 'routes to #show with only dashboard_path' do
+      expect(get: "/gitlab/gitlabhq/-/metrics/dashboard1.yml").to route_to(
+        "projects/metrics_dashboard#show",
+        dashboard_path: 'dashboard1.yml',
+        **base_params
+      )
+    end
+
+    it 'routes to #show with only page' do
+      expect(get: "/gitlab/gitlabhq/-/metrics/panel/new").to route_to(
+        "projects/metrics_dashboard#show",
+        page: 'panel/new',
+        **base_params
+      )
+    end
+
+    it 'routes to #show with dashboard_path and page' do
+      expect(get: "/gitlab/gitlabhq/-/metrics/config%2Fprometheus%2Fcommon_metrics.yml/panel/new").to route_to(
+        "projects/metrics_dashboard#show",
+        dashboard_path: 'config/prometheus/common_metrics.yml',
+        page: 'panel/new',
+        **base_params
+      )
+    end
+
+    it 'routes to 404 with invalid page' do
+      expect(get: "/gitlab/gitlabhq/-/metrics/invalid_page").to route_to(
+        'application#route_not_found',
+        unmatched_route: 'gitlab/gitlabhq/-/metrics/invalid_page'
+      )
+    end
+
+    it 'routes to 404 with invalid dashboard_path' do
+      expect(get: "/gitlab/gitlabhq/-/metrics/invalid_dashboard").to route_to(
+        'application#route_not_found',
+        unmatched_route: 'gitlab/gitlabhq/-/metrics/invalid_dashboard'
+      )
+    end
+
+    it 'routes to 404 with invalid dashboard_path and valid page' do
+      expect(get: "/gitlab/gitlabhq/-/metrics/dashboard1/panel/new").to route_to(
+        'application#route_not_found',
+        unmatched_route: 'gitlab/gitlabhq/-/metrics/dashboard1/panel/new'
+      )
+    end
+
+    it 'routes to 404 with valid dashboard_path and invalid page' do
+      expect(get: "/gitlab/gitlabhq/-/metrics/dashboard1.yml/invalid_page").to route_to(
+        'application#route_not_found',
+        unmatched_route: 'gitlab/gitlabhq/-/metrics/dashboard1.yml/invalid_page'
+      )
+    end
+  end
 end