Merge branch '12026-use-notes-finder-target-for-epics' into 'master'

Use NotesFinder#target to find Epics

Closes #12026

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

ee/app/finders/ee/notes_finder.rb

@@ -7,7 +7,7 @@ module EE
 
     override :noteables_for_type
     def noteables_for_type(noteable_type)
-      if noteable_type == "epic"
+      if noteable_type == 'epic'
         return EpicsFinder.new(@current_user, group_id: @params[:group_id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
       end
 

ee/lib/ee/api/helpers/notes_helpers.rb

@@ -15,9 +15,9 @@ module EE
           end
         end
 
-        def find_noteable(parent_type, parent_id, noteable_type, noteable_id)
-          if noteable_type == ::Epic
-            EpicsFinder.new(current_user, group_id: parent_id).find(noteable_id)
+        def add_parent_to_finder_params(finder_params, noteable_type, parent_id)
+          if noteable_type.name.underscore == 'epic'
+            finder_params[:group_id] = parent_id
           else
             super
           end

ee/spec/finders/notes_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'EE::NotesFinder' do
+  let(:group) { create(:group) }
+  let(:user) { create(:group_member, :owner, group: group, user: create(:user)).user }
+  let(:epic) { create(:epic, :opened, author: user, group: group) }
+  let!(:note) { create(:note_on_epic, noteable: epic) }
+
+  before do
+    stub_licensed_features(epics: true)
+  end
+
+  describe '#target' do
+    subject { NotesFinder.new(user, { target_id: epic.id, target_type: 'epic', group_id: group.id }).target }
+
+    it 'returns an epic' do
+      expect(subject).to eq(epic)
+    end
+
+    it 'fails if group id is missing' do
+      expect {  NotesFinder.new(user, { target_id: epic.id, target_type: 'epic' }).target }.to raise_error(ArgumentError)
+    end
+  end
+
+  describe '#execute' do
+    context 'when using target id and type of epics' do
+      subject { NotesFinder.new(user, { target_id: epic.id, target_type: 'epic', group_id: group.id }).execute }
+
+      it 'returns the expected notes' do
+        expect(subject).to eq([note])
+      end
+
+      it 'fails if group id is missing' do
+        expect { NotesFinder.new(user, { target_id: epic.id, target_type: 'epic' }).execute }.to raise_error(ArgumentError)
+      end
+    end
+
+    context 'when using an explicit epic target' do
+      subject { NotesFinder.new(user, { target: epic }).execute }
+
+      it 'returns the expected notes' do
+        expect(subject).to eq([note])
+      end
+    end
+  end
+end

lib/api/helpers/notes_helpers.rb

@@ -74,14 +74,14 @@ module API
       end
 
       def find_noteable(parent_type, parent_id, noteable_type, noteable_id)
-        params = params_by_noteable_type_and_id(noteable_type, noteable_id)
+        params = finder_params_by_noteable_type_and_id(noteable_type, noteable_id, parent_id)
 
-        noteable = NotesFinder.new(current_user, params.merge(project: user_project)).target
+        noteable = NotesFinder.new(current_user, params).target
         noteable = nil unless can?(current_user, noteable_read_ability_name(noteable), noteable)
         noteable || not_found!(noteable_type)
       end
 
-      def params_by_noteable_type_and_id(type, id)
+      def finder_params_by_noteable_type_and_id(type, id, parent_id)
         target_type = type.name.underscore
         { target_type: target_type }.tap do |h|
           if %w(issue merge_request).include?(target_type)
@@ -89,9 +89,15 @@ module API
           else
             h[:target_id] = id
           end
+
+          add_parent_to_finder_params(h, type, parent_id)
         end
       end
 
+      def add_parent_to_finder_params(finder_params, noteable_type, parent_id)
+        finder_params[:project] = user_project
+      end
+
       def noteable_parent(noteable)
         public_send("user_#{noteable.class.parent_class.to_s.underscore}") # rubocop:disable GitlabSecurity/PublicSend
       end