diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index b5f63cc5a1a0592d2e68dd988a94c86bf8e0af81..ab3d2a9a0cd5ed7ed557a99951d3f382a3481327 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -184,7 +184,8 @@ def update(issuable) 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 @@ -201,6 +202,10 @@ def update(issuable) 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 0000000000000000000000000000000000000000..b12eab26b67b18e00a8ebd222b142b72b9f8b917 --- /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 500d224ff98434fa39671e17b419ab13aec52e1c..eafbea46905a9b6cf04bb886da7e6e9eb056ea9d 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -376,5 +376,10 @@ def update_issue(opts) 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 790ef765f3a1b641faacbf46c9a431a19cf1b78a..88c786947d38cba6d4607b845b393d42e7c2c8f5 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -320,5 +320,10 @@ def update_merge_request(opts) 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 0000000000000000000000000000000000000000..a3336755773790acd8315dfffa58aef1de318cb7 --- /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