Use NotesFinder to fetch notes on API and Controllers

Fix missing iid query on NotesFinder

Changed parameters of find_noteable, 
so changes across a few files were needed.
MergeRequest also requires iid instead of id query

Make NotesFinder fail with RecordNotFound again

Add specs for target_iid

Using RSpec tablesyntax for target_iid specs

Revert "Using RSpec tablesyntax for target_iid specs"

This reverts commit ba45c7f569a.

Allow find_by! here

Fix variable name

Add readable check

Revert "Add readable check"

This reverts commit 9e3a1a7aa39.

Remove unnecessary assignment

Add required changes for EE

Fix parameter count

Reduce code duplication by extracting a noteable module method

The call to find_noteable was redundant so
multiple files and lines have changed in that
commit to use the newly introduced module
method `noteable`.

Replace casecmp with include check

Add parent_type parameter


Revert "Reduce code duplication by extracting
a noteable module method"

This reverts commit 8c0923babff16.

Method is no longer needed

Check whether noteable can be read by user

5 files changed, 32 insertions, 28 deletions

diff --git a/lib/api/discussions.rb b/lib/api/discussions.rb
index 693172b7d08..cc62ce22a1b 100644
--- a/lib/api/discussions.rb
+++ b/lib/api/discussions.rb
@@ -25,7 +25,7 @@ module API
         end
         # rubocop: disable CodeReuse/ActiveRecord
         get ":id/#{noteables_path}/:noteable_id/discussions" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
 
           notes = noteable.notes
             .inc_relations_for_view
@@ -47,7 +47,7 @@ module API
           requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
         end
         get ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
           notes = readable_discussion_notes(noteable, params[:discussion_id])
 
           if notes.empty?
@@ -82,7 +82,7 @@ module API
           end
         end
         post ":id/#{noteables_path}/:noteable_id/discussions" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
           type = params[:position] ? 'DiffNote' : 'DiscussionNote'
           id_key = noteable.is_a?(Commit) ? :commit_id : :noteable_id
 
@@ -112,7 +112,7 @@ module API
           requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
         end
         get ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
           notes = readable_discussion_notes(noteable, params[:discussion_id])
 
           if notes.empty?
@@ -132,7 +132,7 @@ module API
           optional :created_at, type: String, desc: 'The creation date of the note'
         end
         post ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
           notes = readable_discussion_notes(noteable, params[:discussion_id])
           first_note = notes.first
 
@@ -166,7 +166,7 @@ module API
           requires :note_id, type: Integer, desc: 'The ID of a note'
         end
         get ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
 
           get_note(noteable, params[:note_id])
         end
@@ -183,7 +183,7 @@ module API
           exactly_one_of :body, :resolved
         end
         put ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
 
           if params[:resolved].nil?
             update_note(noteable, params[:note_id])
@@ -201,7 +201,7 @@ module API
           requires :note_id, type: Integer, desc: 'The ID of a note'
         end
         delete ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
 
           delete_note(noteable, params[:note_id])
         end
@@ -216,7 +216,7 @@ module API
             requires :resolved, type: Boolean, desc: 'Mark discussion resolved/unresolved'
           end
           put ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id" do
-            noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+            noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
 
             resolve_discussion(noteable, params[:discussion_id], params[:resolved])
           end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 00bcf6b055b..70b5064c72a 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -183,11 +183,6 @@ module API
       user_project.commit_by(oid: id)
     end
 
-    def find_project_snippet(id)
-      finder_params = { project: user_project }
-      SnippetsFinder.new(current_user, finder_params).find(id)
-    end
-
     # rubocop: disable CodeReuse/ActiveRecord
     def find_merge_request_with_access(iid, access_level = :read_merge_request)
       merge_request = user_project.merge_requests.find_by!(iid: iid)
diff --git a/lib/api/helpers/notes_helpers.rb b/lib/api/helpers/notes_helpers.rb
index a068de4361c..c444766249d 100644
--- a/lib/api/helpers/notes_helpers.rb
+++ b/lib/api/helpers/notes_helpers.rb
@@ -73,16 +73,27 @@ module API
         "read_#{noteable.class.to_s.underscore}".to_sym
       end
 
-      def find_noteable(parent, noteables_str, noteable_id)
-        noteable = public_send("find_#{parent}_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend
+      def find_noteable(parent_type, parent_id, noteable_type, noteable_id)
+        params = params_by_noteable_type_and_id(noteable_type, noteable_id)
 
-        readable = can?(current_user, noteable_read_ability_name(noteable), noteable)
-
-        return not_found!(noteables_str) unless readable
+        noteable = NotesFinder.new(user_project, current_user, params).target
+        noteable = nil unless can?(current_user, noteable_read_ability_name(noteable), noteable)
+        return not_found!(noteable_type) unless noteable
 
         noteable
       end
 
+      def params_by_noteable_type_and_id(type, id)
+        target_type = type.name.underscore
+        { target_type: target_type }.tap do |h|
+          if %w(issue merge_request).include?(target_type)
+            h[:target_iid] = id
+          else
+            h[:target_id] = id
+          end
+        end
+      end
+
       def noteable_parent(noteable)
         public_send("user_#{noteable.class.parent_class.to_s.underscore}") # rubocop:disable GitlabSecurity/PublicSend
       end
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 416cf39d3ec..9381f045144 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -15,8 +15,6 @@ module API
         requires :id, type: String, desc: "The ID of a #{parent_type}"
       end
       resource parent_type.pluralize.to_sym, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
-        noteables_str = noteable_type.to_s.underscore.pluralize
-
         desc "Get a list of #{noteable_type.to_s.downcase} notes" do
           success Entities::Note
         end
@@ -30,7 +28,7 @@ module API
         end
         # rubocop: disable CodeReuse/ActiveRecord
         get ":id/#{noteables_str}/:noteable_id/notes" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
 
           # We exclude notes that are cross-references and that cannot be viewed
           # by the current user. By doing this exclusion at this level and not
@@ -56,7 +54,7 @@ module API
           requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
         end
         get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
           get_note(noteable, params[:note_id])
         end
 
@@ -69,7 +67,7 @@ module API
           optional :created_at, type: String, desc: 'The creation date of the note'
         end
         post ":id/#{noteables_str}/:noteable_id/notes" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
 
           opts = {
             note: params[:body],
@@ -96,7 +94,7 @@ module API
           requires :body, type: String, desc: 'The content of a note'
         end
         put ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
 
           update_note(noteable, params[:note_id])
         end
@@ -109,7 +107,7 @@ module API
           requires :note_id, type: Integer, desc: 'The ID of a note'
         end
         delete ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
-          noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+          noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
 
           delete_note(noteable, params[:note_id])
         end
diff --git a/lib/api/resource_label_events.rb b/lib/api/resource_label_events.rb
index 448bef12cec..505a6c68c9c 100644
--- a/lib/api/resource_label_events.rb
+++ b/lib/api/resource_label_events.rb
@@ -26,7 +26,7 @@ module API
 
         # rubocop: disable CodeReuse/ActiveRecord
         get ":id/#{eventables_str}/:eventable_id/resource_label_events" do
-          eventable = find_noteable(parent_type, eventables_str, params[:eventable_id])
+          eventable = find_noteable(parent_type, params[:id], eventable_type, params[:eventable_id])
           events = eventable.resource_label_events.includes(:label, :user)
 
           present paginate(events), with: Entities::ResourceLabelEvent
@@ -42,7 +42,7 @@ module API
           requires :eventable_id, types: [Integer, String], desc: 'The ID of the eventable'
         end
         get ":id/#{eventables_str}/:eventable_id/resource_label_events/:event_id" do
-          eventable = find_noteable(parent_type, eventables_str, params[:eventable_id])
+          eventable = find_noteable(parent_type, params[:id], eventable_type, params[:eventable_id])
           event = eventable.resource_label_events.find(params[:event_id])
 
           present event, with: Entities::ResourceLabelEvent