Merge branch 'fix-design-management-router' into 'master'

Fix design management router

Closes gitlab-ce#64667

See merge request gitlab-org/gitlab-ee!14741

config/routes/project.rb

@@ -500,6 +500,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           get :realtime_changes
           post :create_merge_request
           get :discussions, format: :json
+
+          Gitlab.ee do
+            get 'designs(/*vueroute)', to: 'issues#show', format: false
+          end
         end
 
         collection do

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

 import $ from 'jquery';
 import Vue from 'vue';
-import router from './router';
+import createRouter from './router';
 import App from './components/app.vue';
 import apolloProvider from './graphql';
 import allDesigns from './queries/allDesigns.graphql';
@@ -8,7 +8,8 @@ import allDesigns from './queries/allDesigns.graphql';
 export default () => {
   const el = document.getElementById('js-design-management');
   const badge = document.querySelector('.js-designs-count');
-  const { issueIid, projectPath } = el.dataset;
+  const { issueIid, projectPath, issuePath } = el.dataset;
+  const router = createRouter(issuePath);
 
   $('.js-issue-tabs').on('shown.bs.tab', ({ target: { id } }) => {
     if (id === 'designs' && router.currentRoute.name === 'root') {

ee/app/assets/javascripts/design_management/router.js

 import $ from 'jquery';
-import _ from 'underscore';
 import Vue from 'vue';
 import VueRouter from 'vue-router';
 import Home from './pages/index.vue';
@@ -7,52 +6,57 @@ import DesignDetail from './pages/design/index.vue';
 
 Vue.use(VueRouter);
 
-const router = new VueRouter({
-  base: window.location.pathname,
-  routes: [
-    {
-      name: 'root',
-      path: '/',
-      component: Home,
-      meta: {
-        el: 'discussion',
-      },
-    },
-    {
-      name: 'designs',
-      path: '/designs',
-      component: Home,
-      meta: {
-        el: 'designs',
+export default function createRouter(base) {
+  const router = new VueRouter({
+    base,
+    mode: 'history',
+    routes: [
+      {
+        name: 'root',
+        path: '/',
+        component: Home,
+        meta: {
+          el: 'discussion',
+        },
       },
-      children: [
-        {
-          name: 'design',
-          path: ':id',
-          component: DesignDetail,
-          meta: {
-            el: 'designs',
-          },
-          beforeEnter(
-            {
-              params: { id },
+      {
+        name: 'designs',
+        path: '/designs',
+        component: Home,
+        meta: {
+          el: 'designs',
+        },
+        children: [
+          {
+            name: 'design',
+            path: ':id',
+            component: DesignDetail,
+            meta: {
+              el: 'designs',
             },
-            from,
-            next,
-          ) {
-            if (_.isString(id)) next();
+            beforeEnter(
+              {
+                params: { id },
+              },
+              from,
+              next,
+            ) {
+              if (typeof id === 'string') {
+                next();
+              }
+            },
+            props: ({ params: { id } }) => ({ id }),
           },
-          props: ({ params: { id } }) => ({ id }),
-        },
-      ],
-    },
-  ],
-});
+        ],
+      },
+    ],
+  });
 
-router.beforeEach(({ meta: { el } }, from, next) => {
-  $(`#${el}`).tab('show');
+  router.beforeEach(({ meta: { el } }, from, next) => {
+    $(`#${el}`).tab('show');
 
-  next();
-});
+    next();
+  });
 
-export default router;
+  return router;
+}

ee/app/views/projects/issues/_discussion.html.haml

@@ -12,6 +12,6 @@
     #discussion-tab.tab-pane.show.active{ role: 'tabpanel', 'aria-labelledby': 'discussion' }
       = render_ce 'projects/issues/discussion'
     #designs-tab.tab-pane{ role: 'tabpanel', 'aria-labelledby': 'designs' }
-      #js-design-management{ data: { project_path: @project.full_path, issue_iid: @issue.iid } }
+      #js-design-management{ data: { project_path: @project.full_path, issue_iid: @issue.iid, issue_path: project_issue_path(@project, @issue) } }
 - else
   = render_ce 'projects/issues/discussion'

ee/spec/frontend/design_management/router_spec.js

@@ -3,11 +3,12 @@ import VueRouter from 'vue-router';
 import App from 'ee/design_management/components/app.vue';
 import Designs from 'ee/design_management/pages/index.vue';
 import DesignDetail from 'ee/design_management/pages/design/index.vue';
-import router from 'ee/design_management/router';
+import createRouter from 'ee/design_management/router';
 import '~/commons/bootstrap';
 
 describe('Design management router', () => {
   let vm;
+  let router;
 
   function factory() {
     const localVue = createLocalVue();
@@ -16,6 +17,8 @@ describe('Design management router', () => {
 
     window.gon = { sprite_icons: '' };
 
+    router = createRouter('/');
+
     vm = mount(App, {
       localVue,
       router,

spec/controllers/projects/issues_controller_spec.rb

@@ -251,15 +251,13 @@ describe Projects::IssuesController do
     end
   end
 
-  describe 'Redirect after sign in' do
+  # This spec runs as a request-style spec in order to invoke the
+  # Rails router. A controller-style spec matches the wrong route, and
+  # session['user_return_to'] becomes incorrect.
+  describe 'Redirect after sign in', type: :request do
     context 'with an AJAX request' do
       it 'does not store the visited URL' do
-        get :show, params: {
-          format: :json,
-          namespace_id: project.namespace,
-          project_id: project,
-          id: issue.iid
-        }, xhr: true
+        get project_issue_path(project, issue), xhr: true
 
         expect(session['user_return_to']).to be_blank
       end
@@ -267,14 +265,9 @@ describe Projects::IssuesController do
 
     context 'without an AJAX request' do
       it 'stores the visited URL' do
-        get :show,
-          params: {
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            id: issue.iid
-          }
+        get project_issue_path(project, issue)
 
-        expect(session['user_return_to']).to eq("/#{project.namespace.to_param}/#{project.to_param}/issues/#{issue.iid}")
+        expect(session['user_return_to']).to eq(project_issue_path(project, issue))
       end
     end
   end