Commit 05533458 authored by Stan Hu's avatar Stan Hu

Fix issue redirects going to /issues/:id/designs

When a project is redirected to a new project, Rails attempts to
construct the proper URL in `url_for` by finding the first route that
matches the given parameters. In this case, `namespace_id`,
`project_id`, `id` matched the `/designs` route first, so it used that.

This happens because the Rails `resources` block defines all the
standard #index, #show, #update, etc. routes after the block runs.

To ensure that `/issues/:id` is used to display an issue, we need to use
a separate action for `/issues/:id/designs`.

Closes https://gitlab.com/gitlab-org/gitlab/issues/31357
parent 69cb23c6
...@@ -30,6 +30,7 @@ module IssuableActions ...@@ -30,6 +30,7 @@ module IssuableActions
respond_to do |format| respond_to do |format|
format.html do format.html do
@issuable_sidebar = serializer.represent(issuable, serializer: 'sidebar') # rubocop:disable Gitlab/ModuleWithInstanceVariables @issuable_sidebar = serializer.represent(issuable, serializer: 'sidebar') # rubocop:disable Gitlab/ModuleWithInstanceVariables
render 'show'
end end
format.json do format.json do
......
...@@ -21,7 +21,8 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -21,7 +21,8 @@ class Projects::IssuesController < Projects::ApplicationController
prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) } prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
prepend_before_action(only: [:calendar]) { authenticate_sessionless_user!(:ics) } prepend_before_action(only: [:calendar]) { authenticate_sessionless_user!(:ics) }
prepend_before_action :authenticate_user!, only: [:new] prepend_before_action :authenticate_user!, only: [:new]
prepend_before_action :store_uri, only: [:new, :show] # designs is only applicable to EE, but defining a prepend_before_action in EE code would overwrite this
prepend_before_action :store_uri, only: [:new, :show, :designs]
before_action :whitelist_query_limiting, only: [:create, :create_merge_request, :move, :bulk_update] before_action :whitelist_query_limiting, only: [:create, :create_merge_request, :move, :bulk_update]
before_action :check_issues_available! before_action :check_issues_available!
...@@ -43,6 +44,8 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -43,6 +44,8 @@ class Projects::IssuesController < Projects::ApplicationController
respond_to :html respond_to :html
alias_method :designs, :show
def index def index
@issues = @issuables @issues = @issuables
......
...@@ -518,7 +518,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -518,7 +518,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
get :discussions, format: :json get :discussions, format: :json
Gitlab.ee do Gitlab.ee do
get 'designs(/*vueroute)', to: 'issues#show', as: :designs, format: false get 'designs(/*vueroute)', to: 'issues#designs', as: :designs, format: false
end end
end end
......
---
title: Fix issue redirects going to /issues/:id/designs
merge_request: 16638
author:
type: fixed
...@@ -239,6 +239,33 @@ describe Projects::IssuesController do ...@@ -239,6 +239,33 @@ describe Projects::IssuesController do
end end
end end
describe 'GET #designs' do
context 'when project has moved' do
let(:new_project) { create(:project) }
let(:issue) { create(:issue, project: new_project) }
before do
sign_in(user)
project.route.destroy
new_project.redirect_routes.create!(path: project.full_path)
new_project.add_developer(user)
end
it 'redirects from an old issue/designs correctly' do
get :designs,
params: {
namespace_id: project.namespace,
project_id: project,
id: issue
}
expect(response).to redirect_to(designs_project_issue_path(new_project, issue))
expect(response).to have_gitlab_http_status(302)
end
end
end
describe 'GET #discussions' do describe 'GET #discussions' do
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
......
...@@ -36,6 +36,31 @@ describe Projects::IssuesController do ...@@ -36,6 +36,31 @@ describe Projects::IssuesController do
expect(response).to render_template(:index) expect(response).to render_template(:index)
end end
end end
context 'when project has moved' do
let(:new_project) { create(:project) }
let(:issue) { create(:issue, project: new_project) }
before do
project.route.destroy
new_project.redirect_routes.create!(path: project.full_path)
new_project.add_developer(user)
end
it 'redirects to the new issue tracker from the old one' do
get :index, params: { namespace_id: project.namespace, project_id: project }
expect(response).to redirect_to(project_issues_path(new_project))
expect(response).to have_gitlab_http_status(302)
end
it 'redirects from an old issue correctly' do
get :show, params: { namespace_id: project.namespace, project_id: project, id: issue }
expect(response).to redirect_to(project_issue_path(new_project, issue))
expect(response).to have_gitlab_http_status(302)
end
end
end end
context 'internal issue tracker' do context 'internal issue tracker' do
......
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