Merge branch '7407-label-quick-epics' into 'master'

Fix adding labels to epics using quick actions

Closes #7407

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

app/models/note.rb

@@ -456,6 +456,10 @@ class Note < ActiveRecord::Base
     Upload.find_by(model: self, path: paths)
   end
 
+  def parent
+    project
+  end
+
   private
 
   def keep_around_commit

app/services/notes/quick_actions_service.rb

@@ -32,7 +32,7 @@ module Notes
       return if command_params.empty?
       return unless supported?(note)
 
-      self.class.noteable_update_service(note).new(project, current_user, command_params).execute(note.noteable)
+      self.class.noteable_update_service(note).new(note.parent, current_user, command_params).execute(note.noteable)
     end
   end
 end

app/services/quick_actions/interpret_service.rb

@@ -2,6 +2,7 @@
 
 module QuickActions
   class InterpretService < BaseService
+    include Gitlab::Utils::StrongMemoize
     include Gitlab::QuickActions::Dsl
     attr_reader :issuable
 
@@ -209,15 +210,9 @@ module QuickActions
     end
     params '~label1 ~"label 2"'
     condition do
-      if project
-        available_labels = LabelsFinder
-          .new(current_user, project_id: project.id, include_ancestor_groups: true)
-          .execute
-      end
-
-      project &&
-        current_user.can?(:"admin_#{issuable.to_ability_name}", project) &&
-        available_labels.any?
+      parent &&
+        current_user.can?(:"admin_#{issuable.to_ability_name}", parent) &&
+        find_labels.any?
     end
     command :label do |labels_param|
       label_ids = find_label_ids(labels_param)
@@ -244,7 +239,7 @@ module QuickActions
       issuable.is_a?(Issuable) &&
         issuable.persisted? &&
         issuable.labels.any? &&
-        current_user.can?(:"admin_#{issuable.to_ability_name}", project)
+        current_user.can?(:"admin_#{issuable.to_ability_name}", parent)
     end
     command :unlabel do |labels_param = nil|
       if labels_param.present?
@@ -673,9 +668,25 @@ module QuickActions
       MilestonesFinder.new(params.merge(project_ids: [project.id], group_ids: [project.group&.id])).execute
     end
 
-    def find_labels(labels_param)
-      extract_references(labels_param, :label) |
-        LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split, include_ancestor_groups: true).execute
+    def parent
+      project || group
+    end
+
+    def group
+      strong_memoize(:group) do
+        issuable.group if issuable.respond_to?(:group)
+      end
+    end
+
+    def find_labels(labels_params = nil)
+      finder_params = { include_ancestor_groups: true }
+      finder_params[:project_id] = project.id if project
+      finder_params[:group_id] = group.id if group
+      finder_params[:name] = labels_params.split if labels_params
+
+      result = LabelsFinder.new(current_user, finder_params).execute
+
+      extract_references(labels_params, :label) | result
     end
 
     def find_label_references(labels_param)
@@ -706,9 +717,11 @@ module QuickActions
 
     # rubocop: disable CodeReuse/ActiveRecord
     def extract_references(arg, type)
+      return [] unless arg
+
       ext = Gitlab::ReferenceExtractor.new(project, current_user)
 
-      ext.analyze(arg, author: current_user)
+      ext.analyze(arg, author: current_user, group: group)
 
       ext.references(type)
     end

ee/app/helpers/ee/application_helper.rb

@@ -74,7 +74,7 @@ module EE
       if object.is_a?(Group)
         {
           members: members_group_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id]),
-          labels: labels_group_autocomplete_sources_path(object),
+          labels: labels_group_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id]),
           epics: epics_group_autocomplete_sources_path(object),
           commands: commands_group_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id]),
           milestones: milestones_group_autocomplete_sources_path(object)

ee/app/models/ee/note.rb

@@ -46,6 +46,11 @@ module EE
       for_epic? || super
     end
 
+    override :parent
+    def parent
+      for_epic? ? noteable.group : super
+    end
+
     private
 
     def banzai_context_params

ee/changelogs/unreleased/7407-label-quick-epics.yml

+---
+title: Fix adding labels to epics using quick actions
+merge_request: 8772
+author:
+type: fixed

ee/spec/models/ee/note_spec.rb

@@ -29,4 +29,13 @@ describe Note do
       end
     end
   end
+
+  describe '#parent' do
+    it 'returns group for epic notes' do
+      group = create(:group)
+      note = create(:note_on_epic, noteable: create(:epic, group: group))
+
+      expect(note.parent).to eq(group)
+    end
+  end
 end

ee/spec/services/ee/notes/quick_actions_service_spec.rb

@@ -10,10 +10,6 @@ describe Notes::QuickActionsService do
 
   let(:service) { described_class.new(project, user) }
 
-  before do
-    project.add_maintainer(user)
-  end
-
   def execute(note)
     content, command_params = service.extract_commands(note)
     service.execute(command_params, note)
@@ -169,5 +165,70 @@ describe Notes::QuickActionsService do
         end
       end
     end
+
+    describe '/label' do
+      let(:project) { nil }
+      let!(:bug) { create(:group_label, title: 'bug', group: group)}
+      let!(:project_label) { create(:label, title: 'project_label', project: create(:project, group: group))}
+      let(:note_text) { "/label ~bug ~project_label" }
+      let(:note) { create(:note, noteable: epic, note: note_text) }
+
+      before do
+        group.add_developer(user)
+      end
+
+      context 'when epics are not enabled' do
+        it 'does not add a label to the epic' do
+          expect { execute(note) }.not_to change { epic.reload.labels }
+        end
+      end
+
+      context 'when epics are enabled' do
+        before do
+          stub_licensed_features(epics: true)
+        end
+
+        it 'adds a group label to the epic' do
+          expect { execute(note) }.to change { epic.reload.labels.map(&:title) }.to(['bug'])
+        end
+
+        it 'leaves the note empty' do
+          expect(execute(note)).to eq('')
+        end
+      end
+    end
+
+    describe '/unlabel' do
+      let(:project) { nil }
+      let!(:bug) { create(:group_label, title: 'bug', group: group)}
+      let!(:feature) { create(:group_label, title: 'feature', group: group)}
+      let(:note_text) { "/unlabel ~bug" }
+      let(:note) { create(:note, noteable: epic, note: note_text) }
+
+      before do
+        group.add_developer(user)
+        epic.labels = [bug, feature]
+      end
+
+      context 'when epics are not enabled' do
+        it 'does not remove any label' do
+          expect { execute(note) }.not_to change { epic.reload.labels }
+        end
+      end
+
+      context 'when epics are enabled' do
+        before do
+          stub_licensed_features(epics: true)
+        end
+
+        it 'removes a requested label from the epic' do
+          expect { execute(note) }.to change { epic.reload.labels.map(&:title) }.to(['feature'])
+        end
+
+        it 'leaves the note empty' do
+          expect(execute(note)).to eq('')
+        end
+      end
+    end
   end
 end

ee/spec/services/quick_actions/interpret_service_spec.rb

@@ -199,6 +199,51 @@ describe QuickActions::InterpretService do
       end
     end
 
+    context 'label command for epics' do
+      let(:epic) { create(:epic, group: group)}
+      let(:label) { create(:group_label, title: 'bug', group: group) }
+      let(:project_label) { create(:label, title: 'project_label') }
+      let(:content) { "/label ~#{label.title} ~#{project_label.title}" }
+
+      let(:service) { described_class.new(nil, current_user) }
+
+      context 'when epics are enabled' do
+        before do
+          stub_licensed_features(epics: true)
+        end
+
+        context 'when a user has permissions to label an epic' do
+          before do
+            group.add_developer(current_user)
+          end
+
+          it 'populates valid label ids' do
+            _, updates = service.execute(content, epic)
+
+            expect(updates).to eq(add_label_ids: [label.id])
+          end
+        end
+
+        context 'when a user does not have permissions to label an epic' do
+          it 'does not populate any lables' do
+            _, updates = service.execute(content, epic)
+
+            expect(updates).to be_empty
+          end
+        end
+      end
+
+      context 'when epics are disabled' do
+        it 'does not populate any lables' do
+          group.add_developer(current_user)
+
+          _, updates = service.execute(content, epic)
+
+          expect(updates).to be_empty
+        end
+      end
+    end
+
     context 'remove_epic command' do
       let(:epic) { create(:epic, group: group)}
       let(:content) { "/remove_epic #{epic.to_reference(project)}" }

spec/factories/notes.rb

@@ -14,6 +14,7 @@ FactoryBot.define do
     factory :note_on_merge_request,      traits: [:on_merge_request]
     factory :note_on_project_snippet,    traits: [:on_project_snippet]
     factory :note_on_personal_snippet,   traits: [:on_personal_snippet]
+    factory :note_on_epic,               traits: [:on_epic]
     factory :system_note,                traits: [:system]
 
     factory :discussion_note, class: DiscussionNote
@@ -133,6 +134,11 @@ FactoryBot.define do
       project nil
     end
 
+    trait :on_epic do
+      noteable { create(:epic) }
+      project nil
+    end
+
     trait :system do
       system true
     end

spec/models/note_spec.rb

@@ -890,4 +890,19 @@ describe Note do
       end
     end
   end
+
+  describe '#parent' do
+    it 'returns project for project notes' do
+      project = create(:project)
+      note = create(:note_on_issue, project: project)
+
+      expect(note.parent).to eq(project)
+    end
+
+    it 'returns nil for personal snippet note' do
+      note = create(:note_on_personal_snippet)
+
+      expect(note.parent).to be_nil
+    end
+  end
 end