From eebd74d633c96af8a5ba3fbef03777bfdd240e2d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Date: Mon, 20 May 2019 12:48:41 +0300 Subject: [PATCH 1/5] Move some project routes under /-/ scope Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> --- changelogs/unreleased/dz-scope-project-routes.yml | 5 +++++ config/routes/repository.rb | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/dz-scope-project-routes.yml diff --git a/changelogs/unreleased/dz-scope-project-routes.yml b/changelogs/unreleased/dz-scope-project-routes.yml new file mode 100644 index 000000000000..66eb5d928f07 --- /dev/null +++ b/changelogs/unreleased/dz-scope-project-routes.yml @@ -0,0 +1,5 @@ +--- +title: Move some project routes under /-/ scope +merge_request: 28435 +author: +type: changed diff --git a/config/routes/repository.rb b/config/routes/repository.rb index f5201b9ddbbd..b96315bfe8b3 100644 --- a/config/routes/repository.rb +++ b/config/routes/repository.rb @@ -39,7 +39,7 @@ end end - scope constraints: { id: Gitlab::PathRegex.git_reference_regex } do + scope path: '-', constraints: { id: Gitlab::PathRegex.git_reference_regex } do resources :network, only: [:show] resources :graphs, only: [:show] do -- GitLab From c4c25946cd5694c707ed126541def66be0a53d8f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Date: Mon, 20 May 2019 12:59:01 +0300 Subject: [PATCH 2/5] Move project settings under /-/ scope Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> --- config/routes/project.rb | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/config/routes/project.rb b/config/routes/project.rb index 93d746f3282e..f0f151c60ed0 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -442,16 +442,23 @@ end end end - namespace :settings do - get :members, to: redirect("%{namespace_id}/%{project_id}/project_members") - resource :ci_cd, only: [:show, :update], controller: 'ci_cd' do - post :reset_cache - put :reset_registration_token - end - resource :integrations, only: [:show] - resource :repository, only: [:show], controller: :repository do - post :create_deploy_token, path: 'deploy_token/create' - post :cleanup + + scope '-' do + namespace :settings do + get :members, to: redirect("%{namespace_id}/%{project_id}/project_members") + + resource :ci_cd, only: [:show, :update], controller: 'ci_cd' do + post :reset_cache + put :reset_registration_token + end + + resource :operations, only: [:show, :update] + resource :integrations, only: [:show] + + resource :repository, only: [:show], controller: :repository do + post :create_deploy_token, path: 'deploy_token/create' + post :cleanup + end end end @@ -465,10 +472,6 @@ # its preferable to keep it below all other project routes draw :wiki draw :repository - - namespace :settings do - resource :operations, only: [:show, :update] - end end resources(:projects, -- GitLab From 70decdc2acb4da895bdd63bb15b46014045af661 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Date: Mon, 20 May 2019 15:21:37 +0300 Subject: [PATCH 3/5] Fix specs to match route changes Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> --- .../merge_request_presenter_spec.rb | 4 +- spec/routing/project_routing_spec.rb | 41 ++++++++++--------- spec/services/system_note_service_spec.rb | 2 +- spec/views/help/index.html.haml_spec.rb | 2 +- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb index 451dc88880c5..0e1aed42cc5a 100644 --- a/spec/presenters/merge_request_presenter_spec.rb +++ b/spec/presenters/merge_request_presenter_spec.rb @@ -403,7 +403,7 @@ allow(resource).to receive(:source_branch_exists?) { true } is_expected - .to eq("/#{resource.source_project.full_path}/branches/#{resource.source_branch}") + .to eq("/#{resource.source_project.full_path}/-/branches/#{resource.source_branch}") end end @@ -426,7 +426,7 @@ allow(resource).to receive(:target_branch_exists?) { true } is_expected - .to eq("/#{resource.source_project.full_path}/branches/#{resource.target_branch}") + .to eq("/#{resource.source_project.full_path}/-/branches/#{resource.target_branch}") end end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index a0d01fc8263c..174293fe70c0 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -204,25 +204,25 @@ describe Projects::BranchesController, 'routing' do it 'to #branches' do - expect(get('/gitlab/gitlabhq/branches')).to route_to('projects/branches#index', namespace_id: 'gitlab', project_id: 'gitlabhq') - expect(delete('/gitlab/gitlabhq/branches/feature%2345')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature#45') - expect(delete('/gitlab/gitlabhq/branches/feature%2B45')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45') - expect(delete('/gitlab/gitlabhq/branches/feature@45')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature@45') - expect(delete('/gitlab/gitlabhq/branches/feature%2345/foo/bar/baz')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature#45/foo/bar/baz') - expect(delete('/gitlab/gitlabhq/branches/feature%2B45/foo/bar/baz')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45/foo/bar/baz') - expect(delete('/gitlab/gitlabhq/branches/feature@45/foo/bar/baz')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature@45/foo/bar/baz') + expect(get('/gitlab/gitlabhq/-/branches')).to route_to('projects/branches#index', namespace_id: 'gitlab', project_id: 'gitlabhq') + expect(delete('/gitlab/gitlabhq/-/branches/feature%2345')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature#45') + expect(delete('/gitlab/gitlabhq/-/branches/feature%2B45')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45') + expect(delete('/gitlab/gitlabhq/-/branches/feature@45')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature@45') + expect(delete('/gitlab/gitlabhq/-/branches/feature%2345/foo/bar/baz')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature#45/foo/bar/baz') + expect(delete('/gitlab/gitlabhq/-/branches/feature%2B45/foo/bar/baz')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45/foo/bar/baz') + expect(delete('/gitlab/gitlabhq/-/branches/feature@45/foo/bar/baz')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature@45/foo/bar/baz') end end describe Projects::TagsController, 'routing' do it 'to #tags' do - expect(get('/gitlab/gitlabhq/tags')).to route_to('projects/tags#index', namespace_id: 'gitlab', project_id: 'gitlabhq') - expect(delete('/gitlab/gitlabhq/tags/feature%2345')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature#45') - expect(delete('/gitlab/gitlabhq/tags/feature%2B45')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45') - expect(delete('/gitlab/gitlabhq/tags/feature@45')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature@45') - expect(delete('/gitlab/gitlabhq/tags/feature%2345/foo/bar/baz')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature#45/foo/bar/baz') - expect(delete('/gitlab/gitlabhq/tags/feature%2B45/foo/bar/baz')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45/foo/bar/baz') - expect(delete('/gitlab/gitlabhq/tags/feature@45/foo/bar/baz')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature@45/foo/bar/baz') + expect(get('/gitlab/gitlabhq/-/tags')).to route_to('projects/tags#index', namespace_id: 'gitlab', project_id: 'gitlabhq') + expect(delete('/gitlab/gitlabhq/-/tags/feature%2345')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature#45') + expect(delete('/gitlab/gitlabhq/-/tags/feature%2B45')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45') + expect(delete('/gitlab/gitlabhq/-/tags/feature@45')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature@45') + expect(delete('/gitlab/gitlabhq/-/tags/feature%2345/foo/bar/baz')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature#45/foo/bar/baz') + expect(delete('/gitlab/gitlabhq/-/tags/feature%2B45/foo/bar/baz')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45/foo/bar/baz') + expect(delete('/gitlab/gitlabhq/-/tags/feature@45/foo/bar/baz')).to route_to('projects/tags#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature@45/foo/bar/baz') end end @@ -247,6 +247,7 @@ it_behaves_like 'RESTful project resources' do let(:actions) { [:index, :create, :destroy] } let(:controller) { 'protected_branches' } + let(:controller_path) { '/-/protected_branches' } end end @@ -592,17 +593,17 @@ describe Projects::NetworkController, 'routing' do it 'to #show' do - expect(get('/gitlab/gitlabhq/network/master')).to route_to('projects/network#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master') - expect(get('/gitlab/gitlabhq/network/ends-with.json')).to route_to('projects/network#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'ends-with.json') - expect(get('/gitlab/gitlabhq/network/master?format=json')).to route_to('projects/network#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', format: 'json') + expect(get('/gitlab/gitlabhq/-/network/master')).to route_to('projects/network#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master') + expect(get('/gitlab/gitlabhq/-/network/ends-with.json')).to route_to('projects/network#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'ends-with.json') + expect(get('/gitlab/gitlabhq/-/network/master?format=json')).to route_to('projects/network#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', format: 'json') end end describe Projects::GraphsController, 'routing' do it 'to #show' do - expect(get('/gitlab/gitlabhq/graphs/master')).to route_to('projects/graphs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master') - expect(get('/gitlab/gitlabhq/graphs/ends-with.json')).to route_to('projects/graphs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'ends-with.json') - expect(get('/gitlab/gitlabhq/graphs/master?format=json')).to route_to('projects/graphs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', format: 'json') + expect(get('/gitlab/gitlabhq/-/graphs/master')).to route_to('projects/graphs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master') + expect(get('/gitlab/gitlabhq/-/graphs/ends-with.json')).to route_to('projects/graphs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'ends-with.json') + expect(get('/gitlab/gitlabhq/-/graphs/master?format=json')).to route_to('projects/graphs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', format: 'json') end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 51c5a803dbda..4d33c6f40943 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -132,7 +132,7 @@ end it 'sets the note text' do - link = "http://localhost/#{project.full_path}/tags/#{tag_name}" + link = "http://localhost/#{project.full_path}/-/tags/#{tag_name}" expect(subject.note).to eq "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})" end diff --git a/spec/views/help/index.html.haml_spec.rb b/spec/views/help/index.html.haml_spec.rb index 34e93d929a76..257991549a9c 100644 --- a/spec/views/help/index.html.haml_spec.rb +++ b/spec/views/help/index.html.haml_spec.rb @@ -31,7 +31,7 @@ render expect(rendered).to match '8.0.2' - expect(rendered).to have_link('8.0.2', href: %r{https://gitlab.com/gitlab-org/gitlab-(ce|ee)/tags/v8.0.2}) + expect(rendered).to have_link('8.0.2', href: %r{https://gitlab.com/gitlab-org/gitlab-(ce|ee)/-/tags/v8.0.2}) end it 'shows a link to the commit for pre-releases' do -- GitLab From 6d3f5a33419a6dd52d4f6378f674e28eb22ad323 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Date: Tue, 21 May 2019 11:35:29 +0300 Subject: [PATCH 4/5] Add legacy routes for modified project path For settings, branches and tags Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> --- config/routes/project.rb | 14 ++++++++++++++ spec/routing/project_routing_spec.rb | 12 ++++++++++++ .../legacy_path_redirect_shared_examples.rb | 8 ++++++++ 3 files changed, 34 insertions(+) diff --git a/config/routes/project.rb b/config/routes/project.rb index f0f151c60ed0..65e2d814c7c0 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -496,4 +496,18 @@ end end end + + # Legacy routes. + # Introduced in 12.0. + # Should be removed after 12.1 + scope(path: '*namespace_id', + as: :namespace, + namespace_id: Gitlab::PathRegex.full_namespace_route_regex) do + scope(path: ':project_id', + constraints: { project_id: Gitlab::PathRegex.project_route_regex }, + module: :projects, + as: :project) do + Gitlab::Routing.redirect_legacy_paths(self, :settings, :branches, :tags) + end + end end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 174293fe70c0..9c2a824bab2b 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -212,6 +212,8 @@ expect(delete('/gitlab/gitlabhq/-/branches/feature%2B45/foo/bar/baz')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45/foo/bar/baz') expect(delete('/gitlab/gitlabhq/-/branches/feature@45/foo/bar/baz')).to route_to('projects/branches#destroy', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature@45/foo/bar/baz') end + + it_behaves_like 'redirecting a legacy project path', "/gitlab/gitlabhq/branches", "/gitlab/gitlabhq/-/branches" end describe Projects::TagsController, 'routing' do @@ -597,6 +599,8 @@ expect(get('/gitlab/gitlabhq/-/network/ends-with.json')).to route_to('projects/network#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'ends-with.json') expect(get('/gitlab/gitlabhq/-/network/master?format=json')).to route_to('projects/network#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', format: 'json') end + + it_behaves_like 'redirecting a legacy project path', "/gitlab/gitlabhq/network/master", "/gitlab/gitlabhq/-/network/master" end describe Projects::GraphsController, 'routing' do @@ -662,4 +666,12 @@ end end end + + describe Projects::Settings::RepositoryController, 'routing' do + it 'to #show' do + expect(get('/gitlab/gitlabhq/-/settings/repository')).to route_to('projects/settings/repository#show', namespace_id: 'gitlab', project_id: 'gitlabhq') + end + + it_behaves_like 'redirecting a legacy project path', "/gitlab/gitlabhq/settings/repository", "/gitlab/gitlabhq/-/settings/repository" + end end diff --git a/spec/support/shared_examples/legacy_path_redirect_shared_examples.rb b/spec/support/shared_examples/legacy_path_redirect_shared_examples.rb index f300bdd48b1c..f326e5020920 100644 --- a/spec/support/shared_examples/legacy_path_redirect_shared_examples.rb +++ b/spec/support/shared_examples/legacy_path_redirect_shared_examples.rb @@ -11,3 +11,11 @@ expect(get(source)).not_to redirect_to(target) end end + +shared_examples 'redirecting a legacy project path' do |source, target| + include RSpec::Rails::RequestExampleGroup + + it "redirects #{source} to #{target}" do + expect(get(source)).to redirect_to(target) + end +end -- GitLab From eef6f768219e994fd3847694310e78040eb89815 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Date: Tue, 21 May 2019 12:15:37 +0300 Subject: [PATCH 5/5] Add network and graphs to legacy routes too Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> --- config/routes/project.rb | 2 +- spec/routing/project_routing_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/config/routes/project.rb b/config/routes/project.rb index 65e2d814c7c0..fdc2a3c0086a 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -507,7 +507,7 @@ constraints: { project_id: Gitlab::PathRegex.project_route_regex }, module: :projects, as: :project) do - Gitlab::Routing.redirect_legacy_paths(self, :settings, :branches, :tags) + Gitlab::Routing.redirect_legacy_paths(self, :settings, :branches, :tags, :network, :graphs) end end end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 9c2a824bab2b..dd9540c11d16 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -609,6 +609,8 @@ expect(get('/gitlab/gitlabhq/-/graphs/ends-with.json')).to route_to('projects/graphs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'ends-with.json') expect(get('/gitlab/gitlabhq/-/graphs/master?format=json')).to route_to('projects/graphs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', format: 'json') end + + it_behaves_like 'redirecting a legacy project path', "/gitlab/gitlabhq/graphs/master", "/gitlab/gitlabhq/-/graphs/master" end describe Projects::ForksController, 'routing' do -- GitLab