Merge branch '32455-merge-request-discussions-api-degrades-with-comments-count' into 'master'

Improve pagination of Discussions API

See merge request gitlab-org/gitlab!27697

app/models/concerns/noteable.rb

@@ -79,6 +79,12 @@ module Noteable
       .discussions(self)
   end
 
+  def discussion_ids_relation
+    notes.select(:discussion_id)
+      .group(:discussion_id)
+      .order('MIN(created_at), MIN(id)')
+  end
+
   def capped_notes_count(max)
     notes.limit(max).count
   end

changelogs/unreleased/32455-merge-request-discussions-api-degrades-with-comments-count.yml

+---
+title: Improve pagination in discussions API
+merge_request: 27697
+author:
+type: performance

lib/api/discussions.rb

@@ -28,10 +28,10 @@ module API
         get ":id/#{noteables_path}/:noteable_id/discussions" do
           noteable = find_noteable(noteable_type, params[:noteable_id])
 
-          notes = readable_discussion_notes(noteable)
-          discussions = Kaminari.paginate_array(Discussion.build_collection(notes, noteable))
+          discussion_ids = paginate(noteable.discussion_ids_relation)
+          notes = readable_discussion_notes(noteable, discussion_ids)
 
-          present paginate(discussions), with: Entities::Discussion
+          present Discussion.build_collection(notes, noteable), with: Entities::Discussion
         end
 
         desc "Get a single #{noteable_type.to_s.downcase} discussion" do
@@ -221,10 +221,9 @@ module API
 
     helpers do
       # rubocop: disable CodeReuse/ActiveRecord
-      def readable_discussion_notes(noteable, discussion_id = nil)
+      def readable_discussion_notes(noteable, discussion_ids)
         notes = noteable.notes
-        notes = notes.where(discussion_id: discussion_id) if discussion_id
-        notes = notes
+          .where(discussion_id: discussion_ids)
           .inc_relations_for_view
           .includes(:noteable)
           .fresh

spec/models/concerns/noteable_spec.rb

@@ -62,6 +62,21 @@ describe Noteable do
     end
   end
 
+  describe '#discussion_ids_relation' do
+    it 'returns ordered discussion_ids' do
+      discussion_ids = subject.discussion_ids_relation.pluck(:discussion_id)
+
+      expect(discussion_ids).to eq([
+        active_diff_note1,
+        active_diff_note3,
+        outdated_diff_note1,
+        discussion_note1,
+        note1,
+        note2
+      ].map(&:discussion_id))
+    end
+  end
+
   describe '#grouped_diff_discussions' do
     let(:grouped_diff_discussions) { subject.grouped_diff_discussions }