From da4702493d046e6de24c0a071b0162818e9d990c Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Wed, 19 Jun 2019 14:17:08 +0100
Subject: [PATCH] Fix label serialisation in issue and note hooks

We were not calling hook_attrs on the labels correctly. Specs were
passing because the issues under test did not have any labels!
---
 changelogs/unreleased/fix-labels-in-hooks.yml |  5 +++++
 lib/gitlab/data_builder/note.rb               |  2 +-
 lib/gitlab/hook_data/issue_builder.rb         |  2 +-
 spec/lib/gitlab/data_builder/note_spec.rb     | 19 ++++++++++++-------
 .../gitlab/hook_data/issue_builder_spec.rb    |  4 +++-
 5 files changed, 22 insertions(+), 10 deletions(-)
 create mode 100644 changelogs/unreleased/fix-labels-in-hooks.yml

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..9a2fe4e9abf 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.map(&: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..22ca23cde2a 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.map(&: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
-- 
GitLab