Merge branch '10609-system-notes-scoped-labels' into 'master'

Create system notes for scoped labels

See merge request gitlab-org/gitlab-ee!14487

app/models/label_note.rb

@@ -62,19 +62,27 @@ class LabelNote < Note
   end
 
   def note_text(html: false)
-    added = labels_str('added', label_refs_by_action('add', html))
-    removed = labels_str('removed', label_refs_by_action('remove', html))
+    added = labels_str(label_refs_by_action('add', html), prefix: 'added', suffix: added_suffix)
+    removed = labels_str(label_refs_by_action('remove', html), prefix: removed_prefix)
 
     [added, removed].compact.join(' and ')
   end
 
+  def removed_prefix
+    'removed'
+  end
+
+  def added_suffix
+    ''
+  end
+
   # returns string containing added/removed labels including
   # count of deleted labels:
   #
   # added ~1 ~2 + 1 deleted label
   # added 3 deleted labels
   # added ~1 ~2 labels
-  def labels_str(prefix, label_refs)
+  def labels_str(label_refs, prefix: '', suffix: '')
     existing_refs = label_refs.select { |ref| ref.present? }.sort
     refs_str = existing_refs.empty? ? nil : existing_refs.join(' ')
 
@@ -84,9 +92,9 @@ class LabelNote < Note
     return unless refs_str || deleted_str
 
     label_list_str = [refs_str, deleted_str].compact.join(' + ')
-    suffix = 'label'.pluralize(deleted > 0 ? deleted : existing_refs.count)
+    suffix += ' label'.pluralize(deleted > 0 ? deleted : existing_refs.count)
 
-    "#{prefix} #{label_list_str} #{suffix}"
+    "#{prefix} #{label_list_str} #{suffix.squish}"
   end
 
   def label_refs_by_action(action, html)
@@ -95,3 +103,5 @@ class LabelNote < Note
     events.select { |e| e.action == action }.map(&field)
   end
 end
+
+LabelNote.prepend(EE::LabelNote)

app/models/resource_label_event.rb

@@ -36,10 +36,9 @@ class ResourceLabelEvent < ApplicationRecord
     issue || merge_request
   end
 
-  # create same discussion id for all actions with the same user and time
   def discussion_id(resource = nil)
     strong_memoize(:discussion_id) do
-      Digest::SHA1.hexdigest([self.class.name, created_at, user_id].join("-"))
+      Digest::SHA1.hexdigest(discussion_id_key.join("-"))
     end
   end
 
@@ -121,6 +120,10 @@ class ResourceLabelEvent < ApplicationRecord
   def resource_parent
     issuable.project || issuable.group
   end
+
+  def discussion_id_key
+    [self.class.name, created_at, user_id]
+  end
 end
 
 ResourceLabelEvent.prepend(EE::ResourceLabelEvent)

ee/app/models/ee/label_note.rb

+# frozen_string_literal: true
+
+module EE
+  module LabelNote
+    extend ActiveSupport::Concern
+    extend ::Gitlab::Utils::Override
+
+    private
+
+    override :removed_prefix
+    def removed_prefix
+      scoped_labels_event? ?  'automatically removed' : super
+    end
+
+    override :added_suffix
+    def added_suffix
+      scoped_labels_event? ?  'scoped' : super
+    end
+
+    def scoped_labels_event?
+      events.first.label.scoped_label?
+    end
+  end
+end

ee/app/models/ee/resource_label_event.rb

@@ -15,6 +15,11 @@ module EE
       end
     end
 
+    override :discussion_id_key
+    def discussion_id_key
+      (super + [label&.scoped_label?]).compact # label can be nil as we don't remove resource events when deleting labels
+    end
+
     override :issuable
     def issuable
       epic || super

ee/changelogs/unreleased/10609-system-notes-scoped-labels.yml

+---
+title: Create system notes for scoped labels
+merge_request: 14487
+author:
+type: added

ee/spec/services/ee/resource_events/merge_into_notes_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ResourceEvents::MergeIntoNotesService do
+  def create_event(params)
+    event_params = { action: :add, label: label, issue: resource,
+                     user: user }
+
+    create(:resource_label_event, event_params.merge(params))
+  end
+
+  def create_note(params)
+    opts = { noteable: resource, project: project }
+
+    create(:note_on_issue, opts.merge(params))
+  end
+
+  set(:project) { create(:project) }
+  set(:user) { create(:user) }
+  set(:resource) { create(:issue, project: project) }
+  set(:label) { create(:label, project: project) }
+  set(:label2) { create(:label, project: project) }
+  set(:scoped_label_group1_1) { create(:label, project: project, title: 'key::value') }
+  set(:scoped_label_group1_2) { create(:label, project: project, title: 'key::value2') }
+  set(:scoped_label_group2_1) { create(:label, project: project, title: 'key2::value') }
+  set(:scoped_label_group2_2) { create(:label, project: project, title: 'key2::value2') }
+  set(:scoped_label_group3_1) { create(:label, project: project, title: 'key3::value') }
+  let(:time) { Time.now }
+
+  describe '#execute' do
+    it 'squashes events with same time and author into single note but scoped labels are separated' do
+      user2 = create(:user)
+
+      create_event(created_at: time)
+      create_event(created_at: time, label: label2, action: :remove)
+      create_event(created_at: time, label: scoped_label_group1_1, action: :remove)
+      create_event(created_at: time, label: scoped_label_group1_2, action: :add)
+      create_event(created_at: time, label: scoped_label_group2_2, action: :remove)
+      create_event(created_at: time, label: scoped_label_group2_1, action: :add)
+      create_event(created_at: time, label: scoped_label_group3_1, action: :add)
+      create_event(created_at: time, user: user2)
+      create_event(created_at: 1.day.ago, label: label2)
+
+      notes = described_class.new(resource, user).execute
+
+      expected = [
+        "added #{scoped_label_group1_2.to_reference} #{scoped_label_group2_1.to_reference} " \
+          "#{scoped_label_group3_1.to_reference} scoped labels " \
+          "and automatically removed " \
+          "#{scoped_label_group1_1.to_reference} #{scoped_label_group2_2.to_reference} labels",
+        "added #{label.to_reference} label and removed #{label2.to_reference} label",
+        "added #{label.to_reference} label",
+        "added #{label2.to_reference} label"
+      ]
+
+      expect(notes.count).to eq(4)
+      expect(notes.map(&:note)).to match_array(expected)
+    end
+  end
+end