From 7b1e98125476a8d304f6c61f47bba9d59b54580c Mon Sep 17 00:00:00 2001 From: Markus Koller Date: Mon, 29 Mar 2021 16:58:56 +0200 Subject: [PATCH] Move project hooks routes under /-/ scope --- .../29572-move-project-hooks-routes.yml | 5 ++++ config/routes/project.rb | 26 ++++++++--------- .../presenters/project_hook_presenter_spec.rb | 4 +-- spec/routing/project_routing_spec.rb | 28 +++++++++++-------- 4 files changed, 36 insertions(+), 27 deletions(-) create mode 100644 changelogs/unreleased/29572-move-project-hooks-routes.yml diff --git a/changelogs/unreleased/29572-move-project-hooks-routes.yml b/changelogs/unreleased/29572-move-project-hooks-routes.yml new file mode 100644 index 00000000000000..5500149b1c112b --- /dev/null +++ b/changelogs/unreleased/29572-move-project-hooks-routes.yml @@ -0,0 +1,5 @@ +--- +title: Move project hooks routes under /-/ scope +merge_request: 57734 +author: +type: performance diff --git a/config/routes/project.rb b/config/routes/project.rb index 21dfe17371530a..bfe2a0a78f1f27 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -397,6 +397,18 @@ to: 'web_ide_schemas#show', format: false, as: :schema + + resources :hooks, only: [:index, :create, :edit, :update, :destroy], constraints: { id: /\d+/ } do + member do + post :test + end + + resources :hook_logs, only: [:show] do + member do + post :retry + end + end + end end # End of the /-/ scope. @@ -460,18 +472,6 @@ draw :legacy_builds - resources :hooks, only: [:index, :create, :edit, :update, :destroy], constraints: { id: /\d+/ } do # rubocop: disable Cop/PutProjectRoutesUnderScope - member do - post :test # rubocop:todo Cop/PutProjectRoutesUnderScope - end - - resources :hook_logs, only: [:show] do # rubocop: disable Cop/PutProjectRoutesUnderScope - member do - post :retry # rubocop:todo Cop/PutProjectRoutesUnderScope - end - end - end - resources :container_registry, only: [:index, :destroy, :show], # rubocop: disable Cop/PutProjectRoutesUnderScope controller: 'registry/repositories' @@ -571,7 +571,7 @@ # Legacy routes. # Introduced in 12.0. # Should be removed with https://gitlab.com/gitlab-org/gitlab/issues/28848. - Gitlab::Routing.redirect_legacy_paths(self, :mirror, :tags, + Gitlab::Routing.redirect_legacy_paths(self, :mirror, :tags, :hooks, :cycle_analytics, :mattermost, :variables, :triggers, :environments, :protected_environments, :error_tracking, :alert_management, :tracing, diff --git a/spec/presenters/project_hook_presenter_spec.rb b/spec/presenters/project_hook_presenter_spec.rb index 061ec38ae34d27..2e4bd17bbe1761 100644 --- a/spec/presenters/project_hook_presenter_spec.rb +++ b/spec/presenters/project_hook_presenter_spec.rb @@ -11,7 +11,7 @@ subject { web_hook.present.logs_details_path(web_hook_log) } let(:expected_path) do - "/#{project.namespace.path}/#{project.name}/hooks/#{web_hook.id}/hook_logs/#{web_hook_log.id}" + "/#{project.namespace.path}/#{project.name}/-/hooks/#{web_hook.id}/hook_logs/#{web_hook_log.id}" end it { is_expected.to eq(expected_path) } @@ -21,7 +21,7 @@ subject { web_hook.present.logs_details_path(web_hook_log) } let(:expected_path) do - "/#{project.namespace.path}/#{project.name}/hooks/#{web_hook.id}/hook_logs/#{web_hook_log.id}" + "/#{project.namespace.path}/#{project.name}/-/hooks/#{web_hook.id}/hook_logs/#{web_hook_log.id}" end it { is_expected.to eq(expected_path) } diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index f7ed8d7d5dc634..c1afc35ee89247 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -335,33 +335,37 @@ end end - # test_project_hook POST /:project_id/hooks/:id/test(.:format) hooks#test - # project_hooks GET /:project_id/hooks(.:format) hooks#index - # POST /:project_id/hooks(.:format) hooks#create - # edit_project_hook GET /:project_id/hooks/:id/edit(.:format) hooks#edit - # project_hook PUT /:project_id/hooks/:id(.:format) hooks#update - # DELETE /:project_id/hooks/:id(.:format) hooks#destroy + # test_project_hook POST /:project_id/-/hooks/:id/test(.:format) hooks#test + # project_hooks GET /:project_id/-/hooks(.:format) hooks#index + # POST /:project_id/-/hooks(.:format) hooks#create + # edit_project_hook GET /:project_id/-/hooks/:id/edit(.:format) hooks#edit + # project_hook PUT /:project_id/-/hooks/:id(.:format) hooks#update + # DELETE /:project_id/-/hooks/:id(.:format) hooks#destroy describe Projects::HooksController, 'routing' do it 'to #test' do - expect(post('/gitlab/gitlabhq/hooks/1/test')).to route_to('projects/hooks#test', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') + expect(post('/gitlab/gitlabhq/-/hooks/1/test')).to route_to('projects/hooks#test', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') end it_behaves_like 'resource routing' do let(:actions) { %i[index create destroy edit update] } - let(:base_path) { '/gitlab/gitlabhq/hooks' } + let(:base_path) { '/gitlab/gitlabhq/-/hooks' } end + + it_behaves_like 'redirecting a legacy path', '/gitlab/gitlabhq/hooks', '/gitlab/gitlabhq/-/hooks' end - # retry_namespace_project_hook_hook_log POST /:project_id/hooks/:hook_id/hook_logs/:id/retry(.:format) projects/hook_logs#retry - # namespace_project_hook_hook_log GET /:project_id/hooks/:hook_id/hook_logs/:id(.:format) projects/hook_logs#show + # retry_namespace_project_hook_hook_log POST /:project_id/-/hooks/:hook_id/hook_logs/:id/retry(.:format) projects/hook_logs#retry + # namespace_project_hook_hook_log GET /:project_id/-/hooks/:hook_id/hook_logs/:id(.:format) projects/hook_logs#show describe Projects::HookLogsController, 'routing' do it 'to #retry' do - expect(post('/gitlab/gitlabhq/hooks/1/hook_logs/1/retry')).to route_to('projects/hook_logs#retry', namespace_id: 'gitlab', project_id: 'gitlabhq', hook_id: '1', id: '1') + expect(post('/gitlab/gitlabhq/-/hooks/1/hook_logs/1/retry')).to route_to('projects/hook_logs#retry', namespace_id: 'gitlab', project_id: 'gitlabhq', hook_id: '1', id: '1') end it 'to #show' do - expect(get('/gitlab/gitlabhq/hooks/1/hook_logs/1')).to route_to('projects/hook_logs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', hook_id: '1', id: '1') + expect(get('/gitlab/gitlabhq/-/hooks/1/hook_logs/1')).to route_to('projects/hook_logs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', hook_id: '1', id: '1') end + + it_behaves_like 'redirecting a legacy path', '/gitlab/gitlabhq/hooks/hook_logs/1', '/gitlab/gitlabhq/-/hooks/hook_logs/1' end # project_commit GET /:project_id/commit/:id(.:format) commit#show {id: /\h{7,40}/, project_id: /[^\/]+/} -- GitLab