From f7220f4edc1b403b4b75d6b0088c0c73b91819bd Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Mon, 25 Jul 2022 07:59:25 +1200 Subject: [PATCH 1/7] Add recent events to group hooks Group hook logs can now be viewed via the recent events section within a selected group hook Changelog: added EE: true --- doc/user/project/integrations/webhooks.md | 10 ++-- .../groups/hook_logs_controller.rb | 42 +++++++++++++++ ee/app/controllers/groups/hooks_controller.rb | 5 ++ ee/app/helpers/ee/hooks_helper.rb | 9 ++++ ee/app/models/hooks/group_hook.rb | 1 + ee/app/presenters/group_hook_presenter.rb | 13 +++++ ee/app/views/groups/hook_logs/show.html.haml | 15 ++++++ ee/app/views/groups/hooks/edit.html.haml | 4 ++ ee/config/routes/group.rb | 6 +++ .../groups/hooks_controller_spec.rb | 37 ++++++++++++- ee/spec/helpers/ee/hooks_helper_spec.rb | 11 ++++ .../presenters/group_hook_presenter_spec.rb | 29 ++++++++++ .../groups/hook_logs_controller_spec.rb | 53 +++++++++++++++++++ ee/spec/routing/group_routing_spec.rb | 30 +++++++++++ .../groups/hook_logs/show.html.haml_spec.rb | 20 +++++++ .../views/groups/hooks/edit.html.haml_spec.rb | 22 ++++++++ 16 files changed, 299 insertions(+), 8 deletions(-) create mode 100644 ee/app/controllers/groups/hook_logs_controller.rb create mode 100644 ee/app/presenters/group_hook_presenter.rb create mode 100644 ee/app/views/groups/hook_logs/show.html.haml create mode 100644 ee/spec/presenters/group_hook_presenter_spec.rb create mode 100644 ee/spec/requests/groups/hook_logs_controller_spec.rb create mode 100644 ee/spec/views/groups/hook_logs/show.html.haml_spec.rb create mode 100644 ee/spec/views/groups/hooks/edit.html.haml_spec.rb diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index e4ce736684b16e..fd71b71f79b372 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -137,7 +137,7 @@ For example, to test `push events`, your project should have at least one commit To test a webhook: -1. In your project, on the left sidebar, select **Settings > Webhooks**. +1. In your project or group, on the left sidebar, select **Settings > Webhooks**. 1. Scroll down to the list of configured webhooks. 1. From the **Test** dropdown list, select the type of event to test. @@ -236,12 +236,14 @@ For more information about supported events for Webhooks, go to [Webhook events] ## Troubleshoot webhooks +> The **Recent events** for group webhooks was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325642) in GitLab 15.3. + GitLab records the history of each webhook request. You can view requests made in the last 2 days in the **Recent events** table. To view the table: -1. In your project, on the left sidebar, select **Settings > Webhooks**. +1. In your project or group, on the left sidebar, select **Settings > Webhooks**. 1. Scroll down to the webhooks. 1. Each [failing webhook](#failing-webhooks) has a badge listing it as: @@ -261,10 +263,6 @@ The table includes the following details about each request: ![Recent deliveries](img/webhook_logs.png) -NOTE: -The **Recent events** table is unavailable for group-level webhooks. For more information, read -[issue #325642](https://gitlab.com/gitlab-org/gitlab/-/issues/325642). - Each webhook event has a corresponding **Details** page. This page details the data that GitLab sent (request headers and body) and received (response headers and body). To view the **Details** page, select **View details** for the webhook event. diff --git a/ee/app/controllers/groups/hook_logs_controller.rb b/ee/app/controllers/groups/hook_logs_controller.rb new file mode 100644 index 00000000000000..dcfb9d01da2b2d --- /dev/null +++ b/ee/app/controllers/groups/hook_logs_controller.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Groups + class HookLogsController < Groups::ApplicationController + include ::Integrations::HooksExecution + + before_action :authorize_admin_group! + + before_action :hook, only: [:show, :retry] + before_action :hook_log, only: [:show, :retry] + + respond_to :html + + layout 'group_settings' + + feature_category :integrations + urgency :low, [:retry] + + def show + end + + def retry + execute_hook + redirect_to edit_group_hook_path(@group, @hook) + end + + private + + def execute_hook + result = hook.execute(hook_log.request_data, hook_log.trigger) + set_hook_execution_notice(result) + end + + def hook + @hook ||= @group.hooks.find(params[:hook_id]) + end + + def hook_log + @hook_log ||= hook.web_hook_logs.find(params[:id]) + end + end +end diff --git a/ee/app/controllers/groups/hooks_controller.rb b/ee/app/controllers/groups/hooks_controller.rb index febc3b558bf8bd..0c512f866c273b 100644 --- a/ee/app/controllers/groups/hooks_controller.rb +++ b/ee/app/controllers/groups/hooks_controller.rb @@ -8,6 +8,7 @@ class Groups::HooksController < Groups::ApplicationController before_action :authorize_admin_group! before_action :check_group_webhooks_available! before_action :hook, only: [:edit, :update, :test, :destroy] + before_action :hook_logs, only: :edit before_action -> { check_rate_limit!(:group_testing_hook, scope: [@group, current_user]) }, only: :test respond_to :html @@ -41,6 +42,10 @@ def hook @hook ||= @group.hooks.find(params[:id]) end + def hook_logs + @hook_logs ||= hook.web_hook_logs.recent.page(params[:page]) + end + def trigger_values GroupHook.triggers.values end diff --git a/ee/app/helpers/ee/hooks_helper.rb b/ee/app/helpers/ee/hooks_helper.rb index 5a709509818e83..02a1e2908f3297 100644 --- a/ee/app/helpers/ee/hooks_helper.rb +++ b/ee/app/helpers/ee/hooks_helper.rb @@ -29,5 +29,14 @@ def destroy_hook_path(hook) super end end + + override :hook_log_path + def hook_log_path(hook, hook_log) + if hook.is_a?(GroupHook) + hook_log.present.details_path + else + super + end + end end end diff --git a/ee/app/models/hooks/group_hook.rb b/ee/app/models/hooks/group_hook.rb index babfb531b17c0c..cc8343d9273f47 100644 --- a/ee/app/models/hooks/group_hook.rb +++ b/ee/app/models/hooks/group_hook.rb @@ -3,6 +3,7 @@ class GroupHook < WebHook include CustomModelNaming include TriggerableHooks + include Presentable include Limitable extend ::Gitlab::Utils::Override diff --git a/ee/app/presenters/group_hook_presenter.rb b/ee/app/presenters/group_hook_presenter.rb new file mode 100644 index 00000000000000..46acd1d271d907 --- /dev/null +++ b/ee/app/presenters/group_hook_presenter.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class GroupHookPresenter < Gitlab::View::Presenter::Delegated + presents ::GroupHook, as: :group_hook + + def logs_details_path(log) + group_hook_hook_log_path(group, self, log) + end + + def logs_retry_path(log) + retry_group_hook_hook_log_path(group, self, log) + end +end diff --git a/ee/app/views/groups/hook_logs/show.html.haml b/ee/app/views/groups/hook_logs/show.html.haml new file mode 100644 index 00000000000000..bee8ab1a037d13 --- /dev/null +++ b/ee/app/views/groups/hook_logs/show.html.haml @@ -0,0 +1,15 @@ +- @content_class = 'limit-container-width' unless fluid_layout +- add_to_breadcrumbs _('Webhook Settings'), group_hooks_path(group_id: @group) +- page_title _('Webhook Logs') + +%h1.page-title.gl-font-size-h-display + = _("Request details") + +%hr + +- if @hook_log.oversize? + = button_tag _("Resend Request"), class: "btn gl-button btn-default float-right gl-ml-3 has-tooltip", disabled: true, title: _("Request data is too large") +- else + = link_to _("Resend Request"), @hook_log.present.retry_path, method: :post, class: "btn gl-button btn-default float-right gl-ml-3" + += render partial: 'shared/hook_logs/content', locals: { hook_log: @hook_log } diff --git a/ee/app/views/groups/hooks/edit.html.haml b/ee/app/views/groups/hooks/edit.html.haml index 739c21d0b1dde9..497af5cb02af1b 100644 --- a/ee/app/views/groups/hooks/edit.html.haml +++ b/ee/app/views/groups/hooks/edit.html.haml @@ -14,3 +14,7 @@ %span>= f.submit _('Save changes'), class: 'gl-button btn btn-confirm gl-mr-3' = render 'shared/web_hooks/test_button', hook: @hook = link_to _('Delete'), group_hook_path(@group, @hook), method: :delete, class: 'gl-button btn btn-danger float-right', aria: { label: s_('Webhooks|Delete webhook') }, data: { confirm: s_('Webhooks|Are you sure you want to delete this group hook?'), confirm_btn_variant: 'danger' } + +%hr + += render partial: 'shared/hook_logs/index', locals: { hook: @hook, hook_logs: @hook_logs, group: @group } diff --git a/ee/config/routes/group.rb b/ee/config/routes/group.rb index 027c206112c98a..9bb1154f51fe6b 100644 --- a/ee/config/routes/group.rb +++ b/ee/config/routes/group.rb @@ -101,6 +101,12 @@ member do post :test end + + resources :hook_logs, only: [:show] do + member do + post :retry + end + end end resources :autocomplete_sources, only: [] do diff --git a/ee/spec/controllers/groups/hooks_controller_spec.rb b/ee/spec/controllers/groups/hooks_controller_spec.rb index 4e9a0086ff22ed..4751fcd82c8031 100644 --- a/ee/spec/controllers/groups/hooks_controller_spec.rb +++ b/ee/spec/controllers/groups/hooks_controller_spec.rb @@ -74,11 +74,44 @@ describe 'GET #edit' do let(:hook) { create(:group_hook, group: group) } - it 'is successful' do - get :edit, params: { group_id: group.to_param, id: hook } + let(:params) do + { group_id: group.to_param, id: hook } + end + + render_views + + it 'assigns hook_logs' do + get :edit, params: params + + expect(assigns[:hook]).to be_present + expect(assigns[:hook_logs]).to be_empty + it_renders_correctly + end + + it 'handles when logs are present' do + create_list(:web_hook_log, 3, web_hook: hook) + + get :edit, params: params + + expect(assigns[:hook]).to be_present + expect(assigns[:hook_logs].count).to eq 3 + it_renders_correctly + end + + it 'can paginate logs' do + create_list(:web_hook_log, 21, web_hook: hook) + + get :edit, params: params.merge(page: 2) + + expect(assigns[:hook]).to be_present + expect(assigns[:hook_logs].count).to eq 1 + it_renders_correctly + end + def it_renders_correctly expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:edit) + expect(response).to render_template('shared/hook_logs/_index') expect(group.hooks.size).to eq(1) end end diff --git a/ee/spec/helpers/ee/hooks_helper_spec.rb b/ee/spec/helpers/ee/hooks_helper_spec.rb index c3cd674c7aeba0..b0222c30031e25 100644 --- a/ee/spec/helpers/ee/hooks_helper_spec.rb +++ b/ee/spec/helpers/ee/hooks_helper_spec.rb @@ -13,4 +13,15 @@ .to include("href=\"#{test_group_hook_path(group, group_hook, trigger: trigger)}\"") end end + + describe '#hook_log_path' do + context 'with a group hook' do + let(:web_hook_log) { create(:web_hook_log, web_hook: group_hook) } + + it 'returns group-namespaced link' do + expect(helper.hook_log_path(group_hook, web_hook_log)) + .to eq(web_hook_log.present.details_path) + end + end + end end diff --git a/ee/spec/presenters/group_hook_presenter_spec.rb b/ee/spec/presenters/group_hook_presenter_spec.rb new file mode 100644 index 00000000000000..d3db0f9f2a7d13 --- /dev/null +++ b/ee/spec/presenters/group_hook_presenter_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GroupHookPresenter do + let(:web_hook_log) { create(:web_hook_log, web_hook: web_hook) } + let(:web_hook) { create(:group_hook) } + let(:group) { web_hook_log.web_hook.group } + + describe '#logs_details_path' do + subject { web_hook.present.logs_details_path(web_hook_log) } + + let(:expected_path) do + "/groups/#{group.name}/-/hooks/#{web_hook.id}/hook_logs/#{web_hook_log.id}" + end + + it { is_expected.to eq(expected_path) } + end + + describe '#logs_retry_path' do + subject { web_hook.present.logs_retry_path(web_hook_log) } + + let(:expected_path) do + "/groups/#{group.name}/-/hooks/#{web_hook.id}/hook_logs/#{web_hook_log.id}/retry" + end + + it { is_expected.to eq(expected_path) } + end +end diff --git a/ee/spec/requests/groups/hook_logs_controller_spec.rb b/ee/spec/requests/groups/hook_logs_controller_spec.rb new file mode 100644 index 00000000000000..ff6a5edcc501af --- /dev/null +++ b/ee/spec/requests/groups/hook_logs_controller_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::HookLogsController do + let(:user) { create(:user) } + let(:web_hook_log) { create(:web_hook_log, web_hook: web_hook) } + let(:web_hook) { create(:group_hook) } + let(:log_params) do + { + group_id: web_hook.group.path, + hook_id: web_hook.id, + id: web_hook_log.id + } + end + + before do + sign_in(user) + web_hook.group.add_owner(user) + WebMock.allow_net_connect! + end + + describe 'GET #show' do + it 'renders a 200 if the hook exists' do + get group_hook_hook_log_path(log_params) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('groups/hook_logs/show') + end + + it 'renders a 404 if the hook does not exist' do + web_hook.destroy! + get group_hook_hook_log_path(log_params) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'POST #retry' do + it 'executes the hook and redirects to the service form' do + post retry_group_hook_hook_log_path(log_params) + + expect(subject).to redirect_to(edit_group_hook_url(web_hook.group, web_hook)) + end + + it 'renders a 404 if the hook does not exist' do + web_hook.destroy! + post retry_group_hook_hook_log_path(log_params) + + expect(response).to have_gitlab_http_status(:not_found) + end + end +end diff --git a/ee/spec/routing/group_routing_spec.rb b/ee/spec/routing/group_routing_spec.rb index 18abc25ae15b45..55ad14a5a4703b 100644 --- a/ee/spec/routing/group_routing_spec.rb +++ b/ee/spec/routing/group_routing_spec.rb @@ -77,4 +77,34 @@ expect(get("/groups/gitlabhq/-/discover_premium_and_ultimate")).to route_to('groups/feature_discovery_moments#advanced_features_dashboard', group_id: 'gitlabhq') end end + + # test_group_hook POST /groups/:group_id/-/hooks/:id/test(.:format) hooks#test + # group_hooks GET /groups/:group_id/-/hooks(.:format) hooks#index + # POST /groups/:group_id/-/hooks(.:format) hooks#create + # edit_group_hook GET /groups/:group_id/-/hooks/:id/edit(.:format) hooks#edit + # group_hook PUT /groups/:group_id/-/hooks/:id(.:format) hooks#update + # DELETE /groups/:group_id/-/hooks/:id(.:format) hooks#destroy + describe Groups::HooksController, 'routing' do + it 'to #test' do + expect(post('/groups/gitlabhq/-/hooks/1/test')).to route_to('groups/hooks#test', group_id: 'gitlabhq', id: '1') + end + + it_behaves_like 'resource routing' do + let(:actions) { %i[index create destroy edit update] } + let(:base_path) { '/groups/gitlabhq/-/hooks' } + let(:base_params) { { group_id: 'gitlabhq' } } + end + end + + # retry_group_hook_hook_log POST /groups/:group_id/-/hooks/:hook_id/hook_logs/:id/retry(.:format) groups/hook_logs#retry + # group_hook_hook_log GET /groups/:group_id/-/hooks/:hook_id/hook_logs/:id(.:format) groups/hook_logs#show + describe Groups::HookLogsController, 'routing' do + it 'to #retry' do + expect(post('/groups/gitlabhq/-/hooks/1/hook_logs/1/retry')).to route_to('groups/hook_logs#retry', group_id: 'gitlabhq', hook_id: '1', id: '1') + end + + it 'to #show' do + expect(get('/groups/gitlabhq/-/hooks/1/hook_logs/1')).to route_to('groups/hook_logs#show', group_id: 'gitlabhq', hook_id: '1', id: '1') + end + end end diff --git a/ee/spec/views/groups/hook_logs/show.html.haml_spec.rb b/ee/spec/views/groups/hook_logs/show.html.haml_spec.rb new file mode 100644 index 00000000000000..400c6a6922c775 --- /dev/null +++ b/ee/spec/views/groups/hook_logs/show.html.haml_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'groups/hook_logs/show' do + let(:web_hook_log) { create(:web_hook_log, web_hook: web_hook) } + let(:web_hook) { create(:group_hook) } + + before do + assign :group, web_hook.group + assign :hook, web_hook + assign :hook_log, web_hook_log + render + end + + it 'renders the request details page' do + expect(rendered).to have_text('Request details') + expect(rendered).to have_text('Resend Request') + end +end diff --git a/ee/spec/views/groups/hooks/edit.html.haml_spec.rb b/ee/spec/views/groups/hooks/edit.html.haml_spec.rb new file mode 100644 index 00000000000000..9fd48c30c4a868 --- /dev/null +++ b/ee/spec/views/groups/hooks/edit.html.haml_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'groups/hooks/edit' do + let(:web_hook) { create(:group_hook) } + + before do + assign :group, web_hook.group + assign :hook, web_hook + assign :web_hook_logs, [] + render + end + + it 'renders the edit group hook section' do + expect(rendered).to have_text('Edit Group Hook') + end + + it 'renders the recent events section' do + expect(rendered).to have_text('Recent events') + end +end -- GitLab From fcf66c06366fc846eed31924c00d797b00e9b2dd Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Thu, 4 Aug 2022 13:33:20 +1200 Subject: [PATCH 2/7] Move the presenter to the Webhooks::Group module --- ee/app/models/hooks/group_hook.rb | 4 ++++ ee/app/presenters/group_hook_presenter.rb | 13 ------------- .../presenters/webhooks/group/hook_presenter.rb | 17 +++++++++++++++++ .../group/hook_presenter_spec.rb} | 2 +- 4 files changed, 22 insertions(+), 14 deletions(-) delete mode 100644 ee/app/presenters/group_hook_presenter.rb create mode 100644 ee/app/presenters/webhooks/group/hook_presenter.rb rename ee/spec/presenters/{group_hook_presenter_spec.rb => webhooks/group/hook_presenter_spec.rb} (93%) diff --git a/ee/app/models/hooks/group_hook.rb b/ee/app/models/hooks/group_hook.rb index cc8343d9273f47..a5f90969b67f43 100644 --- a/ee/app/models/hooks/group_hook.rb +++ b/ee/app/models/hooks/group_hook.rb @@ -45,6 +45,10 @@ def parent group end + def present + super(presenter_class: ::Webhooks::Group::HookPresenter) + end + private override :web_hooks_disable_failed? diff --git a/ee/app/presenters/group_hook_presenter.rb b/ee/app/presenters/group_hook_presenter.rb deleted file mode 100644 index 46acd1d271d907..00000000000000 --- a/ee/app/presenters/group_hook_presenter.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -class GroupHookPresenter < Gitlab::View::Presenter::Delegated - presents ::GroupHook, as: :group_hook - - def logs_details_path(log) - group_hook_hook_log_path(group, self, log) - end - - def logs_retry_path(log) - retry_group_hook_hook_log_path(group, self, log) - end -end diff --git a/ee/app/presenters/webhooks/group/hook_presenter.rb b/ee/app/presenters/webhooks/group/hook_presenter.rb new file mode 100644 index 00000000000000..dfc76bb37819a9 --- /dev/null +++ b/ee/app/presenters/webhooks/group/hook_presenter.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Webhooks + module Group + class HookPresenter < Gitlab::View::Presenter::Delegated + presents ::GroupHook, as: :group_hook + + def logs_details_path(log) + group_hook_hook_log_path(group, self, log) + end + + def logs_retry_path(log) + retry_group_hook_hook_log_path(group, self, log) + end + end + end +end diff --git a/ee/spec/presenters/group_hook_presenter_spec.rb b/ee/spec/presenters/webhooks/group/hook_presenter_spec.rb similarity index 93% rename from ee/spec/presenters/group_hook_presenter_spec.rb rename to ee/spec/presenters/webhooks/group/hook_presenter_spec.rb index d3db0f9f2a7d13..1c0f36ece0f7d3 100644 --- a/ee/spec/presenters/group_hook_presenter_spec.rb +++ b/ee/spec/presenters/webhooks/group/hook_presenter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe GroupHookPresenter do +RSpec.describe Webhooks::Group::HookPresenter do let(:web_hook_log) { create(:web_hook_log, web_hook: web_hook) } let(:web_hook) { create(:group_hook) } let(:group) { web_hook_log.web_hook.group } -- GitLab From 28f8551bc1d9594016927f968860d25ca8595a59 Mon Sep 17 00:00:00 2001 From: Ashraf Khamis Date: Mon, 8 Aug 2022 19:46:10 +0000 Subject: [PATCH 3/7] Apply 1 suggestion(s) to 1 file(s) --- doc/user/project/integrations/webhooks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index fd71b71f79b372..1de7440a15d4f8 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -236,7 +236,7 @@ For more information about supported events for Webhooks, go to [Webhook events] ## Troubleshoot webhooks -> The **Recent events** for group webhooks was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325642) in GitLab 15.3. +> **Recent events** for group webhooks [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325642) in GitLab 15.3. GitLab records the history of each webhook request. You can view requests made in the last 2 days in the **Recent events** table. -- GitLab From 63f6b9a7ff4ae0f96562ea99d3c557fbec2aec59 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Thu, 11 Aug 2022 05:09:37 +0000 Subject: [PATCH 4/7] Apply 2 suggestion(s) to 2 file(s) --- ee/spec/presenters/webhooks/group/hook_presenter_spec.rb | 5 +++-- ee/spec/requests/groups/hook_logs_controller_spec.rb | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ee/spec/presenters/webhooks/group/hook_presenter_spec.rb b/ee/spec/presenters/webhooks/group/hook_presenter_spec.rb index 1c0f36ece0f7d3..342060da7eef26 100644 --- a/ee/spec/presenters/webhooks/group/hook_presenter_spec.rb +++ b/ee/spec/presenters/webhooks/group/hook_presenter_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' RSpec.describe Webhooks::Group::HookPresenter do - let(:web_hook_log) { create(:web_hook_log, web_hook: web_hook) } - let(:web_hook) { create(:group_hook) } + let_it_be(:web_hook) { create(:group_hook) } + let_it_be(:web_hook_log) { create(:web_hook_log, web_hook: web_hook) } + let(:group) { web_hook_log.web_hook.group } describe '#logs_details_path' do diff --git a/ee/spec/requests/groups/hook_logs_controller_spec.rb b/ee/spec/requests/groups/hook_logs_controller_spec.rb index ff6a5edcc501af..52a9823f757b71 100644 --- a/ee/spec/requests/groups/hook_logs_controller_spec.rb +++ b/ee/spec/requests/groups/hook_logs_controller_spec.rb @@ -38,9 +38,13 @@ describe 'POST #retry' do it 'executes the hook and redirects to the service form' do + expect_next_found_instance_of(GroupHook) do |hook| + expect(hook).to receive(:execute).and_call_original + end + post retry_group_hook_hook_log_path(log_params) - expect(subject).to redirect_to(edit_group_hook_url(web_hook.group, web_hook)) + expect(response).to redirect_to(edit_group_hook_url(web_hook.group, web_hook)) end it 'renders a 404 if the hook does not exist' do -- GitLab From 0d134e50e2f33a289e3671e664357e2b86fa127d Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Wed, 10 Aug 2022 17:33:30 +1200 Subject: [PATCH 5/7] Fix module naming issue Added underscore to path and fixed camel case in the module name --- ee/app/models/hooks/group_hook.rb | 2 +- .../presenters/{webhooks => web_hooks}/group/hook_presenter.rb | 2 +- .../{webhooks => web_hooks}/group/hook_presenter_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename ee/app/presenters/{webhooks => web_hooks}/group/hook_presenter.rb (95%) rename ee/spec/presenters/{webhooks => web_hooks}/group/hook_presenter_spec.rb (93%) diff --git a/ee/app/models/hooks/group_hook.rb b/ee/app/models/hooks/group_hook.rb index a5f90969b67f43..88a0c81ee143f8 100644 --- a/ee/app/models/hooks/group_hook.rb +++ b/ee/app/models/hooks/group_hook.rb @@ -46,7 +46,7 @@ def parent end def present - super(presenter_class: ::Webhooks::Group::HookPresenter) + super(presenter_class: ::WebHooks::Group::HookPresenter) end private diff --git a/ee/app/presenters/webhooks/group/hook_presenter.rb b/ee/app/presenters/web_hooks/group/hook_presenter.rb similarity index 95% rename from ee/app/presenters/webhooks/group/hook_presenter.rb rename to ee/app/presenters/web_hooks/group/hook_presenter.rb index dfc76bb37819a9..e3e2904369e4b6 100644 --- a/ee/app/presenters/webhooks/group/hook_presenter.rb +++ b/ee/app/presenters/web_hooks/group/hook_presenter.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module Webhooks +module WebHooks module Group class HookPresenter < Gitlab::View::Presenter::Delegated presents ::GroupHook, as: :group_hook diff --git a/ee/spec/presenters/webhooks/group/hook_presenter_spec.rb b/ee/spec/presenters/web_hooks/group/hook_presenter_spec.rb similarity index 93% rename from ee/spec/presenters/webhooks/group/hook_presenter_spec.rb rename to ee/spec/presenters/web_hooks/group/hook_presenter_spec.rb index 342060da7eef26..60ef125d86401b 100644 --- a/ee/spec/presenters/webhooks/group/hook_presenter_spec.rb +++ b/ee/spec/presenters/web_hooks/group/hook_presenter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Webhooks::Group::HookPresenter do +RSpec.describe WebHooks::Group::HookPresenter do let_it_be(:web_hook) { create(:group_hook) } let_it_be(:web_hook_log) { create(:web_hook_log, web_hook: web_hook) } -- GitLab From 9aa2e06ce9b89ac84bfe1db667cf280e8b52f246 Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Thu, 11 Aug 2022 16:51:31 +1200 Subject: [PATCH 6/7] Remove the as from the presents As requested, I have removed this. It doesn't seem to be needed but the other hook presenters had it. --- ee/app/presenters/web_hooks/group/hook_presenter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/presenters/web_hooks/group/hook_presenter.rb b/ee/app/presenters/web_hooks/group/hook_presenter.rb index e3e2904369e4b6..03d9169f7a48bf 100644 --- a/ee/app/presenters/web_hooks/group/hook_presenter.rb +++ b/ee/app/presenters/web_hooks/group/hook_presenter.rb @@ -3,7 +3,7 @@ module WebHooks module Group class HookPresenter < Gitlab::View::Presenter::Delegated - presents ::GroupHook, as: :group_hook + presents ::GroupHook def logs_details_path(log) group_hook_hook_log_path(group, self, log) -- GitLab From 16736ee03df8fff90910b14b2f9b2b4cb63bd2dd Mon Sep 17 00:00:00 2001 From: Anton Smith Date: Mon, 15 Aug 2022 14:29:37 +1200 Subject: [PATCH 7/7] Stub the retry test --- ee/spec/requests/groups/hook_logs_controller_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ee/spec/requests/groups/hook_logs_controller_spec.rb b/ee/spec/requests/groups/hook_logs_controller_spec.rb index 52a9823f757b71..bcab7b377d0c3b 100644 --- a/ee/spec/requests/groups/hook_logs_controller_spec.rb +++ b/ee/spec/requests/groups/hook_logs_controller_spec.rb @@ -17,7 +17,6 @@ before do sign_in(user) web_hook.group.add_owner(user) - WebMock.allow_net_connect! end describe 'GET #show' do @@ -38,6 +37,8 @@ describe 'POST #retry' do it 'executes the hook and redirects to the service form' do + stub_request(:post, web_hook.url) + expect_next_found_instance_of(GroupHook) do |hook| expect(hook).to receive(:execute).and_call_original end -- GitLab