diff options
author | Rémy Coutable <remy@rymai.me> | 2016-12-16 17:00:43 +0000 |
---|---|---|
committer | Alejandro Rodríguez <alejorro70@gmail.com> | 2017-01-23 23:37:38 -0300 |
commit | 5797c80a9de39b38628da0c295d0bbe5ba4535c5 (patch) | |
tree | 4c4057679d641a941408c8c9f3981621d2fd5651 | |
parent | fb93d4c04c0cce470877db55b562eb0d404ba676 (diff) | |
download | gitlab-ce-5797c80a9de39b38628da0c295d0bbe5ba4535c5.tar.gz |
Merge branch '25339-2-webhooks-fired-for-issue-closed-and-reopened' into 'master'
Ensure issuable state changes only fire webhooks once
Webhooks were fired twice when issuables were reopened or closed. Once for the status change and once for the `update` operation
Closes #25339
See merge request !8101
5 files changed, 37 insertions, 1 deletions
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index ce68e433ab8..51cc9a4a64f 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -183,7 +183,8 @@ class IssuableBaseService < BaseService old_labels = issuable.labels.to_a old_mentioned_users = issuable.mentioned_users.to_a - params[:label_ids] = process_label_ids(params, existing_label_ids: issuable.label_ids) + label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) + params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids) if params.present? && update_issuable(issuable, params) # We do not touch as it will affect a update on updated_at field @@ -200,6 +201,10 @@ class IssuableBaseService < BaseService issuable end + def labels_changing?(old_label_ids, new_label_ids) + old_label_ids.sort != new_label_ids.sort + end + def change_state(issuable) case params.delete(:state_event) when 'reopen' diff --git a/changelogs/unreleased/25339-2-webhooks-fired-for-issue-closed-and-reopened.yml b/changelogs/unreleased/25339-2-webhooks-fired-for-issue-closed-and-reopened.yml new file mode 100644 index 00000000000..b12eab26b67 --- /dev/null +++ b/changelogs/unreleased/25339-2-webhooks-fired-for-issue-closed-and-reopened.yml @@ -0,0 +1,4 @@ +--- +title: Ensure issuable state changes only fire webhooks once +merge_request: +author: diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 4c878d748c0..53741ff4010 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -374,5 +374,10 @@ describe Issues::UpdateService, services: true do let(:mentionable) { issue } include_examples 'updating mentions', Issues::UpdateService end + + include_examples 'issuable update service' do + let(:open_issuable) { issue } + let(:closed_issuable) { create(:closed_issue, project: project) } + end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 0bd6db1810a..51618e77d9f 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -318,5 +318,10 @@ describe MergeRequests::UpdateService, services: true do expect(issue_ids).to be_empty end end + + include_examples 'issuable update service' do + let(:open_issuable) { merge_request } + let(:closed_issuable) { create(:closed_merge_request, source_project: project) } + end end end diff --git a/spec/support/services/issuable_update_service_shared_examples.rb b/spec/support/services/issuable_update_service_shared_examples.rb new file mode 100644 index 00000000000..a3336755773 --- /dev/null +++ b/spec/support/services/issuable_update_service_shared_examples.rb @@ -0,0 +1,17 @@ +shared_examples 'issuable update service' do + context 'changing state' do + before { expect(project).to receive(:execute_hooks).once } + + context 'to reopened' do + it 'executes hooks only once' do + described_class.new(project, user, state_event: 'reopen').execute(closed_issuable) + end + end + + context 'to closed' do + it 'executes hooks only once' do + described_class.new(project, user, state_event: 'close').execute(open_issuable) + end + end + end +end |