Merge branch 'fix-labels-in-hooks' into 'master'

Fix label serialisation in issue and note hooks

Closes #63473

See merge request gitlab-org/gitlab-ce!29850

7 files changed, 35 insertions, 10 deletions

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 6da6fbe55cb..30e29911758 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -254,6 +254,10 @@ class Issue < ApplicationRecord
     merge_requests_closing_issues.count
   end
 
+  def labels_hook_attrs
+    labels.map(&:hook_attrs)
+  end
+
   private
 
   def ensure_metrics
diff --git a/changelogs/unreleased/fix-labels-in-hooks.yml b/changelogs/unreleased/fix-labels-in-hooks.yml
new file mode 100644
index 00000000000..c0904a860c5
--- /dev/null
+++ b/changelogs/unreleased/fix-labels-in-hooks.yml
@@ -0,0 +1,5 @@
+---
+title: Fix label serialization in issue and note hooks
+merge_request: 29850
+author:
+type: fixed
diff --git a/lib/gitlab/data_builder/note.rb b/lib/gitlab/data_builder/note.rb
index 16e62622ed4..2c4ef73a688 100644
--- a/lib/gitlab/data_builder/note.rb
+++ b/lib/gitlab/data_builder/note.rb
@@ -44,7 +44,7 @@ module Gitlab
           data[:commit] = build_data_for_commit(project, user, note)
         elsif note.for_issue?
           data[:issue] = note.noteable.hook_attrs
-          data[:issue][:labels] = note.noteable.labels(&:hook_attrs)
+          data[:issue][:labels] = note.noteable.labels_hook_attrs
         elsif note.for_merge_request?
           data[:merge_request] = note.noteable.hook_attrs
         elsif note.for_snippet?
diff --git a/lib/gitlab/hook_data/issue_builder.rb b/lib/gitlab/hook_data/issue_builder.rb
index cfc9ebe4f92..e5f86ca02b5 100644
--- a/lib/gitlab/hook_data/issue_builder.rb
+++ b/lib/gitlab/hook_data/issue_builder.rb
@@ -45,7 +45,7 @@ module Gitlab
             human_time_estimate: issue.human_time_estimate,
             assignee_ids: issue.assignee_ids,
             assignee_id: issue.assignee_ids.first, # This key is deprecated
-            labels: issue.labels
+            labels: issue.labels_hook_attrs
         }
 
         issue.attributes.with_indifferent_access.slice(*self.class.safe_hook_attributes)
diff --git a/spec/lib/gitlab/data_builder/note_spec.rb b/spec/lib/gitlab/data_builder/note_spec.rb
index ed9a1e23529..1b5dd2538e0 100644
--- a/spec/lib/gitlab/data_builder/note_spec.rb
+++ b/spec/lib/gitlab/data_builder/note_spec.rb
@@ -38,9 +38,11 @@ describe Gitlab::DataBuilder::Note do
   end
 
   describe 'When asking for a note on issue' do
+    let(:label) { create(:label, project: project) }
+
     let(:issue) do
-      create(:issue, created_at: fixed_time, updated_at: fixed_time,
-                     project: project)
+      create(:labeled_issue, created_at: fixed_time, updated_at: fixed_time,
+             project: project, labels: [label])
     end
 
     let(:note) do
@@ -48,13 +50,16 @@ describe Gitlab::DataBuilder::Note do
     end
 
     it 'returns the note and issue-specific data' do
+      without_timestamps = lambda { |label| label.except('created_at', 'updated_at') }
+      hook_attrs = issue.reload.hook_attrs
+
       expect(data).to have_key(:issue)
-      expect(data[:issue].except('updated_at'))
-        .to eq(issue.reload.hook_attrs.except('updated_at'))
+      expect(data[:issue].except('updated_at', 'labels'))
+        .to eq(hook_attrs.except('updated_at', 'labels'))
       expect(data[:issue]['updated_at'])
-        .to be >= issue.hook_attrs['updated_at']
-      expect(data[:issue]['labels'])
-        .to eq(issue.hook_attrs['labels'])
+        .to be >= hook_attrs['updated_at']
+      expect(data[:issue]['labels'].map(&without_timestamps))
+        .to eq(hook_attrs['labels'].map(&without_timestamps))
     end
 
     context 'with confidential issue' do
diff --git a/spec/lib/gitlab/hook_data/issue_builder_spec.rb b/spec/lib/gitlab/hook_data/issue_builder_spec.rb
index f066c0e3813..b06d05c1c7f 100644
--- a/spec/lib/gitlab/hook_data/issue_builder_spec.rb
+++ b/spec/lib/gitlab/hook_data/issue_builder_spec.rb
@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe Gitlab::HookData::IssueBuilder do
-  set(:issue) { create(:issue) }
+  set(:label) { create(:label) }
+  set(:issue) { create(:labeled_issue, labels: [label], project: label.project) }
   let(:builder) { described_class.new(issue) }
 
   describe '#build' do
@@ -39,6 +40,7 @@ describe Gitlab::HookData::IssueBuilder do
       expect(data).to include(:human_time_estimate)
       expect(data).to include(:human_total_time_spent)
       expect(data).to include(:assignee_ids)
+      expect(data).to include('labels' => [label.hook_attrs])
     end
 
     context 'when the issue has an image in the description' do
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index a5c7e9db2a1..d5b016dc8f6 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -862,4 +862,13 @@ describe Issue do
       end
     end
   end
+
+  describe "#labels_hook_attrs" do
+    let(:label) { create(:label) }
+    let(:issue) { create(:labeled_issue, labels: [label]) }
+
+    it "returns a list of label hook attributes" do
+      expect(issue.labels_hook_attrs).to eq([label.hook_attrs])
+    end
+  end
 end