Merge branch '11097-insights-dashboard-anchor' into 'master'

Set active insights dashboard tab from hash fragment

Closes #11097

See merge request gitlab-org/gitlab!16904

ee/app/assets/javascripts/insights/components/insights.vue

@@ -38,7 +38,14 @@ export default {
       }
 
       if (!activeTab) {
-        this.setActiveTab(Object.keys(configData)[0]);
+        if (this.validSpecifiedTab()) {
+          this.setActiveTab(this.specifiedTab);
+        } else {
+          const defaultTab = Object.keys(configData)[0];
+
+          this.setActiveTab(defaultTab);
+          this.$router.replace(defaultTab);
+        }
       }
 
       return Object.keys(configData).map(key => ({
@@ -50,6 +57,9 @@ export default {
     configPresent() {
       return !this.configLoading && this.configData != null;
     },
+    specifiedTab() {
+      return this.$route.params.tabId;
+    },
   },
   mounted() {
     this.fetchConfigData(this.endpoint);
@@ -57,7 +67,15 @@ export default {
   methods: {
     ...mapActions('insights', ['fetchConfigData', 'setActiveTab']),
     onChangePage(page) {
-      this.setActiveTab(page);
+      if (this.validTab(page) && this.activeTab !== page) {
+        this.$router.push(page);
+      }
+    },
+    validSpecifiedTab() {
+      return this.specifiedTab && this.validTab(this.specifiedTab);
+    },
+    validTab(tab) {
+      return Object.prototype.hasOwnProperty.call(this.configData, tab);
     },
   },
 };

ee/app/assets/javascripts/insights/index.js

 import Vue from 'vue';
 import Insights from './components/insights.vue';
+import createRouter from './insights_router';
 import store from './stores';
 
 export default () => {
   const el = document.querySelector('#js-insights-pane');
+  const { endpoint, queryEndpoint } = el.dataset;
+  const router = createRouter(endpoint);
 
   if (!el) return null;
 
   return new Vue({
     el,
     store,
+    router,
     components: {
       Insights,
     },
     render(createElement) {
       return createElement('insights', {
         props: {
-          endpoint: el.dataset.endpoint,
-          queryEndpoint: el.dataset.queryEndpoint,
+          endpoint,
+          queryEndpoint,
         },
       });
     },

ee/app/assets/javascripts/insights/insights_router.js

+import Vue from 'vue';
+import VueRouter from 'vue-router';
+import { joinPaths } from '~/lib/utils/url_utility';
+import store from 'ee/insights/stores';
+
+Vue.use(VueRouter);
+
+export default function createRouter(base) {
+  const router = new VueRouter({
+    mode: 'hash',
+    base: joinPaths(gon.relative_url_root || '', base),
+    routes: [{ path: '/:tabId' }],
+  });
+
+  router.beforeEach((to, from, next) => {
+    const page = to.path.substr(1);
+
+    store.dispatch('insights/setActiveTab', page);
+
+    next();
+  });
+
+  return router;
+}

ee/app/assets/javascripts/insights/stores/modules/insights/actions.js

@@ -52,10 +52,16 @@ export const fetchChartData = ({ dispatch }, { endpoint, chart }) =>
 export const setActiveTab = ({ commit, state }, key) => {
   const { configData } = state;
 
-  const page = configData[key];
+  if (configData) {
+    const page = configData[key];
 
-  commit(types.SET_ACTIVE_TAB, key);
-  commit(types.SET_ACTIVE_PAGE, page);
+    if (page) {
+      commit(types.SET_ACTIVE_TAB, key);
+      commit(types.SET_ACTIVE_PAGE, page);
+    } else {
+      createFlash(__('The specified tab is invalid, please select another'));
+    }
+  }
 };
 
 export const initChartData = ({ commit }, store) => commit(types.INIT_CHART_DATA, store);

ee/changelogs/unreleased/11097-insights-dashboard-anchor.yml

+---
+title: Set active insights dashboard tab from hash fragment
+merge_request: 16904
+author:
+type: added

ee/config/routes/group.rb

@@ -45,7 +45,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
 
     resource :issues_analytics, only: [:show]
 
-    resource :insights, only: [:show] do
+    resource :insights, only: [:show], trailing_slash: true do
       collection do
         post :query
       end

ee/config/routes/project.rb

@@ -77,7 +77,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
         end
       end
 
-      resource :insights, only: [:show] do
+      resource :insights, only: [:show], trailing_slash: true do
         collection do
           post :query
         end

ee/spec/controllers/groups/insights_controller_spec.rb

@@ -10,6 +10,7 @@ describe Groups::InsightsController do
   set(:user) { create(:user) }
   let(:query_params) { { type: 'bar', query: { issuable_type: 'issue', collection_labels: ['bug'] }, projects: projects_params } }
   let(:projects_params) { { only: [project.id, project.full_path] } }
+  let(:params) { { trailing_slash: true } }
 
   before do
     stub_licensed_features(insights: true)
@@ -37,19 +38,19 @@ describe Groups::InsightsController do
   context 'when insights configuration project cannot be read by current user' do
     context 'when visiting the parent group' do
       describe 'GET #show.html' do
-        subject { get :show, params: { group_id: parent_group.to_param } }
+        subject { get :show, params: params.merge(group_id: parent_group.to_param) }
 
         it_behaves_like '404 status'
       end
 
       describe 'GET #show.json' do
-        subject { get :show, params: { group_id: parent_group.to_param }, format: :json }
+        subject { get :show, params: params.merge(group_id: parent_group.to_param), format: :json }
 
         it_behaves_like '404 status'
       end
 
       describe 'POST #query' do
-        subject { post :query, params: query_params.merge(group_id: parent_group.to_param) }
+        subject { post :query, params: params.merge(query_params.merge(group_id: parent_group.to_param)) }
 
         it_behaves_like '404 status'
       end
@@ -57,7 +58,7 @@ describe Groups::InsightsController do
 
     context 'when visiting a nested group' do
       describe 'GET #show.html' do
-        subject { get :show, params: { group_id: nested_group.to_param } }
+        subject { get :show, params: params.merge(group_id: nested_group.to_param) }
 
         # The following expectation should be changed to
         # it_behaves_like '404 status'
@@ -66,7 +67,7 @@ describe Groups::InsightsController do
       end
 
       describe 'GET #show.json' do
-        subject { get :show, params: { group_id: nested_group.to_param }, format: :json }
+        subject { get :show, params: params.merge(group_id: nested_group.to_param), format: :json }
 
         # The following expectation should be changed to
         # it_behaves_like '404 status'
@@ -83,7 +84,7 @@ describe Groups::InsightsController do
       end
 
       describe 'POST #query.json' do
-        subject { post :query, params: query_params.merge(group_id: nested_group.to_param), format: :json }
+        subject { post :query, params: params.merge(query_params.merge(group_id: nested_group.to_param)), format: :json }
 
         # The following expectation should be changed to
         # it_behaves_like '404 status'
@@ -100,19 +101,19 @@ describe Groups::InsightsController do
 
     context 'when the configuration is attached to the current group' do
       describe 'GET #show.html' do
-        subject { get :show, params: { group_id: parent_group.to_param } }
+        subject { get :show, params: params.merge(group_id: parent_group.to_param) }
 
         it_behaves_like '200 status'
       end
 
       describe 'GET #show.sjon' do
-        subject { get :show, params: { group_id: parent_group.to_param }, format: :json }
+        subject { get :show, params: params.merge(group_id: parent_group.to_param), format: :json }
 
         it_behaves_like '200 status'
       end
 
       describe 'POST #query.json' do
-        subject { post :query, params: query_params.merge(group_id: parent_group.to_param), format: :json }
+        subject { post :query, params: params.merge(query_params.merge(group_id: parent_group.to_param)), format: :json }
 
         it_behaves_like '200 status'
       end
@@ -120,19 +121,19 @@ describe Groups::InsightsController do
 
     context 'when the configuration is attached to a nested group' do
       describe 'GET #show.html' do
-        subject { get :show, params: { group_id: nested_group.to_param } }
+        subject { get :show, params: params.merge(group_id: nested_group.to_param) }
 
         it_behaves_like '200 status'
       end
 
       describe 'GET #show.json' do
-        subject { get :show, params: { group_id: nested_group.to_param }, format: :json }
+        subject { get :show, params: params.merge(group_id: nested_group.to_param), format: :json }
 
         it_behaves_like '200 status'
       end
 
       describe 'POST #query.json' do
-        subject { post :query, params: query_params.merge(group_id: nested_group.to_param), format: :json }
+        subject { post :query, params: params.merge(query_params.merge(group_id: nested_group.to_param)), format: :json }
 
         it_behaves_like '200 status'
       end

ee/spec/features/groups/insights_spec.rb

@@ -6,5 +6,6 @@ describe 'Group Insights' do
   it_behaves_like 'Insights page' do
     set(:entity) { create(:group) }
     let(:route) { url_for([entity, :insights]) }
+    let(:path) { group_insights_path(entity) }
   end
 end

ee/spec/features/projects/insights_spec.rb

@@ -6,5 +6,6 @@ describe 'Project Insights' do
   it_behaves_like 'Insights page' do
     set(:entity) { create(:project) }
     let(:route) { url_for([entity.namespace, entity, :insights]) }
+    let(:path) { project_insights_path(entity) }
   end
 end

ee/spec/javascripts/insights/components/insights_spec.js

 import Vue from 'vue';
 import Insights from 'ee/insights/components/insights.vue';
 import { createStore } from 'ee/insights/stores';
-import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
+import createRouter from 'ee/insights/insights_router';
+import { pageInfo } from '../mock_data';
 
 describe('Insights component', () => {
   let vm;
   let store;
   let mountComponent;
   const Component = Vue.extend(Insights);
+  const router = createRouter('');
 
   beforeEach(() => {
     store = createStore();
     spyOn(store, 'dispatch').and.stub();
 
     mountComponent = data => {
+      const el = null;
+
       const props = data || {
         endpoint: gl.TEST_HOST,
         queryEndpoint: `${gl.TEST_HOST}/query`,
       };
-      return mountComponentWithStore(Component, { store, props });
+
+      return new Component({
+        store,
+        router,
+        propsData: props || {},
+      }).$mount(el);
     };
 
     vm = mountComponent();
@@ -100,4 +109,41 @@ describe('Insights component', () => {
       });
     });
   });
+
+  describe('hash fragment present', () => {
+    const defaultKey = 'issues';
+    const selectedKey = 'mrs';
+
+    const configData = {};
+    configData[defaultKey] = {};
+    configData[selectedKey] = {};
+
+    beforeEach(() => {
+      vm.$store.state.insights.configLoading = false;
+      vm.$store.state.insights.configData = configData;
+      vm.$store.state.insights.activePage = pageInfo;
+    });
+
+    afterEach(() => {
+      window.location.hash = '';
+    });
+
+    it('selects the first tab if invalid', done => {
+      window.location.hash = '#/invalid';
+
+      vm.$nextTick(() => {
+        expect(store.dispatch).toHaveBeenCalledWith('insights/setActiveTab', defaultKey);
+      });
+      done();
+    });
+
+    it('selects the specified tab if valid', done => {
+      window.location.hash = `#/${selectedKey}`;
+
+      vm.$nextTick(() => {
+        expect(store.dispatch).toHaveBeenCalledWith('insights/setActiveTab', selectedKey);
+      });
+      done();
+    });
+  });
 });

ee/spec/javascripts/insights/insights_router_spec.js

+import createRouter from 'ee/insights/insights_router';
+import store from 'ee/insights/stores';
+
+describe('insights router', () => {
+  let router;
+
+  beforeEach(() => {
+    router = createRouter('base');
+  });
+
+  it(`sets the activeTab when route changed`, () => {
+    const route = 'route';
+
+    spyOn(store, 'dispatch').and.stub();
+
+    router.push(`/${route}`);
+
+    expect(store.dispatch).toHaveBeenCalledWith('insights/setActiveTab', route);
+  });
+});

ee/spec/javascripts/insights/stores/insights/actions_spec.js

@@ -268,9 +268,13 @@ describe('Insights store actions', () => {
   });
 
   describe('setActiveTab', () => {
-    it('commits SET_ACTIVE_TAB and SET_ACTIVE_PAGE', done => {
-      const state = { configData };
+    let state;
 
+    beforeEach(() => {
+      state = { configData };
+    });
+
+    it('commits SET_ACTIVE_TAB and SET_ACTIVE_PAGE', done => {
       testAction(
         actions.setActiveTab,
         key,
@@ -280,6 +284,16 @@ describe('Insights store actions', () => {
         done,
       );
     });
+
+    it('does not mutate with no configData', done => {
+      state = { configData: null };
+
+      testAction(actions.setActiveTab, key, state, [], [], done);
+    });
+
+    it('does not mutate with no matching tab', done => {
+      testAction(actions.setActiveTab, 'invalidTab', state, [], [], done);
+    });
   });
 
   describe('initChartData', () => {

ee/spec/support/shared_examples/features/insights_shared_examples.rb

@@ -24,6 +24,26 @@ RSpec.shared_examples 'Insights page' do
         expect(page).to have_content('Insights')
       end
 
+      context 'hash fragment navigation', :js do
+        let(:config) { entity.insights_config }
+        let(:non_default_tab_id) { config.keys.last }
+        let(:non_default_tab_title) { config[non_default_tab_id][:title] }
+        let(:hash_fragment) { "#/#{non_default_tab_id}" }
+        let(:route) { path + hash_fragment }
+
+        before do
+          visit route
+
+          wait_for_requests
+        end
+
+        it 'loads the correct page' do
+          page.within(".insights-container") do
+            expect(page).to have_content(non_default_tab_title)
+          end
+        end
+      end
+
       context 'when the feature flag is disabled globally' do
         before do
           stub_feature_flags(insights: false)

locale/gitlab.pot

@@ -16017,6 +16017,9 @@ msgstr ""
 msgid "The snippet is visible to any logged in user."
 msgstr ""
 
+msgid "The specified tab is invalid, please select another"
+msgstr ""
+
 msgid "The staging stage shows the time between merging the MR and deploying code to the production environment. The data will be automatically added once you deploy to production for the first time."
 msgstr ""