Merge branch '340172-paginate-discussions' into 'master'

Paginate discussions endpoint

See merge request gitlab-org/gitlab!69933

app/assets/javascripts/notes/components/notes_app.vue

@@ -98,15 +98,17 @@ export default {
       return this.noteableData.noteableType;
     },
     allDiscussions() {
+      let skeletonNotes = [];
+
       if (this.renderSkeleton || this.isLoading) {
         const prerenderedNotesCount = parseInt(this.notesData.prerenderedNotesCount, 10) || 0;
 
-        return new Array(prerenderedNotesCount).fill({
+        skeletonNotes = new Array(prerenderedNotesCount).fill({
           isSkeletonNote: true,
         });
       }
 
-      return this.discussions;
+      return this.discussions.concat(skeletonNotes);
     },
     canReply() {
       return this.userCanReply && !this.commentsDisabled && !this.timelineEnabled;

app/assets/javascripts/notes/stores/actions.js

@@ -70,7 +70,7 @@ export const setUserData = ({ commit }, data) => commit(types.SET_USER_DATA, dat
 export const setLastFetchedAt = ({ commit }, data) => commit(types.SET_LAST_FETCHED_AT, data);
 
 export const setInitialNotes = ({ commit }, discussions) =>
-  commit(types.SET_INITIAL_DISCUSSIONS, discussions);
+  commit(types.ADD_OR_UPDATE_DISCUSSIONS, discussions);
 
 export const setTargetNoteHash = ({ commit }, data) => commit(types.SET_TARGET_NOTE_HASH, data);
 
@@ -89,14 +89,51 @@ export const fetchDiscussions = ({ commit, dispatch }, { path, filter, persistFi
       ? { params: { notes_filter: filter, persist_filter: persistFilter } }
       : null;
 
+  if (window.gon?.features?.paginatedIssueDiscussions) {
+    return dispatch('fetchDiscussionsBatch', { path, config, perPage: 20 });
+  }
+
   return axios.get(path, config).then(({ data }) => {
-    commit(types.SET_INITIAL_DISCUSSIONS, data);
+    commit(types.ADD_OR_UPDATE_DISCUSSIONS, data);
     commit(types.SET_FETCHING_DISCUSSIONS, false);
 
     dispatch('updateResolvableDiscussionsCounts');
   });
 };
 
+export const fetchDiscussionsBatch = ({ commit, dispatch }, { path, config, cursor, perPage }) => {
+  const params = { ...config?.params, per_page: perPage };
+
+  if (cursor) {
+    params.cursor = cursor;
+  }
+
+  return axios.get(path, { params }).then(({ data, headers }) => {
+    commit(types.ADD_OR_UPDATE_DISCUSSIONS, data);
+
+    if (headers['x-next-page-cursor']) {
+      const nextConfig = { ...config };
+
+      if (config?.params?.persist_filter) {
+        delete nextConfig.params.notes_filter;
+        delete nextConfig.params.persist_filter;
+      }
+
+      return dispatch('fetchDiscussionsBatch', {
+        path,
+        config: nextConfig,
+        cursor: headers['x-next-page-cursor'],
+        perPage: Math.min(Math.round(perPage * 1.5), 100),
+      });
+    }
+
+    commit(types.SET_FETCHING_DISCUSSIONS, false);
+    dispatch('updateResolvableDiscussionsCounts');
+
+    return undefined;
+  });
+};
+
 export const updateDiscussion = ({ commit, state }, discussion) => {
   commit(types.UPDATE_DISCUSSION, discussion);
 

app/assets/javascripts/notes/stores/mutation_types.js

 export const ADD_NEW_NOTE = 'ADD_NEW_NOTE';
 export const ADD_NEW_REPLY_TO_DISCUSSION = 'ADD_NEW_REPLY_TO_DISCUSSION';
+export const ADD_OR_UPDATE_DISCUSSIONS = 'ADD_OR_UPDATE_DISCUSSIONS';
 export const DELETE_NOTE = 'DELETE_NOTE';
 export const REMOVE_PLACEHOLDER_NOTES = 'REMOVE_PLACEHOLDER_NOTES';
 export const SET_NOTES_DATA = 'SET_NOTES_DATA';
 export const SET_NOTEABLE_DATA = 'SET_NOTEABLE_DATA';
 export const SET_USER_DATA = 'SET_USER_DATA';
-export const SET_INITIAL_DISCUSSIONS = 'SET_INITIAL_DISCUSSIONS';
 export const SET_LAST_FETCHED_AT = 'SET_LAST_FETCHED_AT';
 export const SET_TARGET_NOTE_HASH = 'SET_TARGET_NOTE_HASH';
 export const SHOW_PLACEHOLDER_NOTE = 'SHOW_PLACEHOLDER_NOTE';

app/assets/javascripts/notes/stores/mutations.js

@@ -129,8 +129,8 @@ export default {
     Object.assign(state, { userData: data });
   },
 
-  [types.SET_INITIAL_DISCUSSIONS](state, discussionsData) {
-    const discussions = discussionsData.reduce((acc, d) => {
+  [types.ADD_OR_UPDATE_DISCUSSIONS](state, discussionsData) {
+    discussionsData.forEach((d) => {
       const discussion = { ...d };
       const diffData = {};
 
@@ -145,27 +145,38 @@ export default {
       // To support legacy notes, should be very rare case.
       if (discussion.individual_note && discussion.notes.length > 1) {
         discussion.notes.forEach((n) => {
-          acc.push({
+          const newDiscussion = {
             ...discussion,
             ...diffData,
             notes: [n], // override notes array to only have one item to mimick individual_note
-          });
+          };
+          const oldDiscussion = state.discussions.find(
+            (existingDiscussion) =>
+              existingDiscussion.id === discussion.id && existingDiscussion.notes[0].id === n.id,
+          );
+
+          if (oldDiscussion) {
+            state.discussions.splice(state.discussions.indexOf(oldDiscussion), 1, newDiscussion);
+          } else {
+            state.discussions.push(newDiscussion);
+          }
         });
       } else {
-        const oldNote = utils.findNoteObjectById(state.discussions, discussion.id);
+        const oldDiscussion = utils.findNoteObjectById(state.discussions, discussion.id);
 
-        acc.push({
-          ...discussion,
-          ...diffData,
-          expanded: oldNote ? oldNote.expanded : discussion.expanded,
-        });
+        if (oldDiscussion) {
+          state.discussions.splice(state.discussions.indexOf(oldDiscussion), 1, {
+            ...discussion,
+            ...diffData,
+            expanded: oldDiscussion.expanded,
+          });
+        } else {
+          state.discussions.push({ ...discussion, ...diffData });
+        }
       }
-
-      return acc;
-    }, []);
-
-    Object.assign(state, { discussions });
+    });
   },
+
   [types.SET_LAST_FETCHED_AT](state, fetchedAt) {
     Object.assign(state, { lastFetchedAt: fetchedAt });
   },

app/controllers/concerns/issuable_actions.rb

@@ -149,8 +149,20 @@ module IssuableActions
                 .includes(:noteable)
                 .fresh
 
+    if paginated_discussions
+      paginated_discussions_by_type = paginated_discussions.records.group_by(&:table_name)
+
+      notes = if paginated_discussions_by_type['notes'].present?
+                notes.with_discussion_ids(paginated_discussions_by_type['notes'].map(&:discussion_id))
+              else
+                notes.none
+              end
+
+      response.headers['X-Next-Page-Cursor'] = paginated_discussions.cursor_for_next_page if paginated_discussions.has_next_page?
+    end
+
     if notes_filter != UserPreference::NOTES_FILTERS[:only_comments]
-      notes = ResourceEvents::MergeIntoNotesService.new(issuable, current_user).execute(notes)
+      notes = ResourceEvents::MergeIntoNotesService.new(issuable, current_user, paginated_notes: paginated_discussions_by_type).execute(notes)
     end
 
     notes = prepare_notes_for_rendering(notes)
@@ -170,6 +182,17 @@ module IssuableActions
 
   private
 
+  def paginated_discussions
+    return if params[:per_page].blank?
+    return unless issuable.instance_of?(Issue) && Feature.enabled?(:paginated_issue_discussions, project, default_enabled: :yaml)
+
+    strong_memoize(:paginated_discussions) do
+      issuable
+        .discussion_root_note_ids(notes_filter: notes_filter)
+        .keyset_paginate(cursor: params[:cursor], per_page: params[:per_page].to_i)
+    end
+  end
+
   def notes_filter
     strong_memoize(:notes_filter) do
       notes_filter_param = params[:notes_filter]&.to_i

app/controllers/projects/issues_controller.rb

@@ -52,6 +52,7 @@ class Projects::IssuesController < Projects::ApplicationController
     push_frontend_feature_flag(:confidential_notes, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:issue_assignees_widget, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:labels_widget, @project, default_enabled: :yaml)
+    push_frontend_feature_flag(:paginated_issue_discussions, @project, default_enabled: :yaml)
 
     experiment(:invite_members_in_comment, namespace: @project.root_ancestor) do |experiment_instance|
       experiment_instance.exclude! unless helpers.can_admin_project_member?(@project)

app/helpers/notes_helper.rb

@@ -167,11 +167,11 @@ module NotesHelper
     }
   end
 
-  def discussions_path(issuable)
+  def discussions_path(issuable, **params)
     if issuable.is_a?(Issue)
-      discussions_project_issue_path(@project, issuable, format: :json)
+      discussions_project_issue_path(@project, issuable, params.merge(format: :json))
     else
-      discussions_project_merge_request_path(@project, issuable, format: :json)
+      discussions_project_merge_request_path(@project, issuable, params.merge(format: :json))
     end
   end
 

app/models/concerns/noteable.rb

@@ -98,6 +98,27 @@ module Noteable
       .order('MIN(created_at), MIN(id)')
   end
 
+  # This does not consider OutOfContextDiscussions in MRs
+  # where notes from commits are overriden so that they have
+  # the same discussion_id
+  def discussion_root_note_ids(notes_filter:)
+    relations = []
+
+    relations << discussion_notes.select(
+      "'notes' AS table_name",
+      'discussion_id',
+      'MIN(id) AS id',
+      'MIN(created_at) AS created_at'
+    ).with_notes_filter(notes_filter)
+     .group(:discussion_id)
+
+    if notes_filter != UserPreference::NOTES_FILTERS[:only_comments]
+      relations += synthetic_note_ids_relations
+    end
+
+    Note.from_union(relations, remove_duplicates: false).fresh
+  end
+
   def capped_notes_count(max)
     notes.limit(max).count
   end
@@ -179,6 +200,18 @@ module Noteable
 
     project_email.sub('@', "-#{iid}@")
   end
+
+  private
+
+  # Synthetic system notes don't have discussion IDs because these are generated dynamically
+  # in Ruby. These are always root notes anyway so we don't need to group by discussion ID.
+  def synthetic_note_ids_relations
+    [
+      resource_label_events.select("'resource_label_events'", "'NULL'", :id, :created_at),
+      resource_milestone_events.select("'resource_milestone_events'", "'NULL'", :id, :created_at),
+      resource_state_events.select("'resource_state_events'", "'NULL'", :id, :created_at)
+    ]
+  end
 end
 
 Noteable.extend(Noteable::ClassMethods)

app/models/note.rb

@@ -114,6 +114,7 @@ class Note < ApplicationRecord
   scope :fresh, -> { order_created_asc.with_order_id_asc }
   scope :updated_after, ->(time) { where('updated_at > ?', time) }
   scope :with_updated_at, ->(time) { where(updated_at: time) }
+  scope :with_discussion_ids, ->(discussion_ids) { where(discussion_id: discussion_ids) }
   scope :with_suggestions, -> { joins(:suggestions) }
   scope :inc_author, -> { includes(:author) }
   scope :with_api_entity_associations, -> { preload(:note_diff_file, :author) }

app/services/resource_events/base_synthetic_notes_builder_service.rb

@@ -24,10 +24,18 @@ module ResourceEvents
     private
 
     def apply_common_filters(events)
+      events = apply_pagination(events)
       events = apply_last_fetched_at(events)
       apply_fetch_until(events)
     end
 
+    def apply_pagination(events)
+      return events if params[:paginated_notes].nil?
+      return events.none if params[:paginated_notes][table_name].blank?
+
+      events.id_in(params[:paginated_notes][table_name].map(&:id))
+    end
+
     def apply_last_fetched_at(events)
       return events unless params[:last_fetched_at].present?
 
@@ -47,5 +55,9 @@ module ResourceEvents
         resource.project || resource.group
       end
     end
+
+    def table_name
+      raise NotImplementedError
+    end
   end
 end

app/services/resource_events/synthetic_label_notes_builder_service.rb

@@ -23,5 +23,9 @@ module ResourceEvents
 
       events.group_by { |event| event.discussion_id }
     end
+
+    def table_name
+      'resource_label_events'
+    end
   end
 end

app/services/resource_events/synthetic_milestone_notes_builder_service.rb

@@ -21,5 +21,9 @@ module ResourceEvents
       events = resource.resource_milestone_events.includes(user: :status) # rubocop: disable CodeReuse/ActiveRecord
       apply_common_filters(events)
     end
+
+    def table_name
+      'resource_milestone_events'
+    end
   end
 end

app/services/resource_events/synthetic_state_notes_builder_service.rb

@@ -16,5 +16,9 @@ module ResourceEvents
       events = resource.resource_state_events.includes(user: :status) # rubocop: disable CodeReuse/ActiveRecord
       apply_common_filters(events)
     end
+
+    def table_name
+      'resource_state_events'
+    end
   end
 end

app/views/projects/issues/_discussion.html.haml

-- add_page_startup_api_call discussions_path(@issue)
+- add_page_startup_api_call Feature.enabled?(:paginated_issue_discussions, @project, default_enabled: :yaml) ? discussions_path(@issue, per_page: 20) : discussions_path(@issue)
 
 - @gfm_form = true
 

config/feature_flags/development/paginated_issue_discussions.yml

+---
+name: paginated_issue_discussions
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69933
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345351
+milestone: '14.5'
+type: development
+group: group::project management
+default_enabled: false

ee/app/helpers/ee/notes_helper.rb

@@ -13,9 +13,9 @@ module EE
     end
 
     override :discussions_path
-    def discussions_path(issuable)
-      return discussions_group_epic_path(issuable.group, issuable, format: :json) if issuable.is_a?(Epic)
-      return discussions_project_security_vulnerability_path(issuable.project, issuable, format: :json) if issuable.is_a?(Vulnerability)
+    def discussions_path(issuable, **params)
+      return discussions_group_epic_path(issuable.group, issuable, params.merge(format: :json)) if issuable.is_a?(Epic)
+      return discussions_project_security_vulnerability_path(issuable.project, issuable, params.merge(format: :json)) if issuable.is_a?(Vulnerability)
 
       super
     end

ee/app/models/concerns/ee/noteable.rb

@@ -24,5 +24,22 @@ module EE
         super
       end
     end
+
+    private
+
+    override :synthetic_note_ids_relations
+    def synthetic_note_ids_relations
+      relations = super
+
+      if respond_to?(:resource_weight_events)
+        relations << resource_weight_events.select("'resource_weight_events'", "'NULL'", :id, :created_at)
+      end
+
+      if respond_to?(:resource_iteration_events)
+        relations << resource_iteration_events.select("'resource_iteration_events'", "'NULL'", :id, :created_at)
+      end
+
+      relations
+    end
   end
 end

ee/app/services/ee/resource_events/synthetic_iteration_notes_builder_service.rb

@@ -22,6 +22,10 @@ module EE
         events = resource.resource_iteration_events.includes(user: :status) # rubocop: disable CodeReuse/ActiveRecord
         apply_common_filters(events)
       end
+
+      def table_name
+        'resource_iteration_events'
+      end
     end
   end
 end

ee/app/services/ee/resource_events/synthetic_weight_notes_builder_service.rb

@@ -22,6 +22,10 @@ module EE
         events = resource.resource_weight_events.includes(user: :status).order(:id) # rubocop: disable CodeReuse/ActiveRecord
         apply_common_filters(events)
       end
+
+      def table_name
+        'resource_weight_events'
+      end
     end
   end
 end

ee/spec/models/concerns/ee/noteable_spec.rb

@@ -14,4 +14,44 @@ RSpec.describe EE::Noteable do
       expect(klazz.replyable_types).to include("Vulnerability")
     end
   end
+
+  describe '#discussion_root_note_ids' do
+    let_it_be(:issue) { create(:issue) }
+    let_it_be(:weight_event) { create(:resource_weight_event, issue: issue) }
+    let_it_be(:regular_note) { create(:note, project: issue.project, noteable: issue) }
+    let_it_be(:iteration_event) { create(:resource_iteration_event, issue: issue) }
+
+    it 'includes weight and iteration synthetic notes' do
+      discussions = issue.discussion_root_note_ids(notes_filter: UserPreference::NOTES_FILTERS[:all_notes]).map do |n|
+        { table_name: n.table_name, discussion_id: n.discussion_id, id: n.id }
+      end
+
+      expect(discussions).to match([
+        a_hash_including(table_name: 'resource_weight_events', id: weight_event.id),
+        a_hash_including(table_name: 'notes', discussion_id: regular_note.discussion_id),
+        a_hash_including(table_name: 'resource_iteration_events', id: iteration_event.id)
+      ])
+    end
+
+    it 'filters by comments only' do
+      discussions = issue.discussion_root_note_ids(notes_filter: UserPreference::NOTES_FILTERS[:only_comments]).map do |n|
+        { table_name: n.table_name, discussion_id: n.discussion_id, id: n.id }
+      end
+
+      expect(discussions).to match([
+        a_hash_including(table_name: 'notes', discussion_id: regular_note.discussion_id)
+      ])
+    end
+
+    it 'filters by system notes only' do
+      discussions = issue.discussion_root_note_ids(notes_filter: UserPreference::NOTES_FILTERS[:only_activity]).map do |n|
+        { table_name: n.table_name, discussion_id: n.discussion_id, id: n.id }
+      end
+
+      expect(discussions).to match([
+        a_hash_including(table_name: 'resource_weight_events', id: weight_event.id),
+        a_hash_including(table_name: 'resource_iteration_events', id: iteration_event.id)
+      ])
+    end
+  end
 end

ee/spec/services/ee/resource_events/synthetic_iteration_notes_builder_service_spec.rb

@@ -21,5 +21,7 @@ RSpec.describe EE::ResourceEvents::SyntheticIterationNotesBuilderService do
         expect(notes.size).to eq(3)
       end
     end
+
+    it_behaves_like 'filters by paginated notes', :resource_iteration_event
   end
 end

ee/spec/services/ee/resource_events/synthetic_weight_notes_builder_service_spec.rb

@@ -4,18 +4,20 @@ require 'spec_helper'
 
 RSpec.describe EE::ResourceEvents::SyntheticWeightNotesBuilderService do
   describe '#execute' do
-    let!(:user) { create(:user) }
+    let_it_be(:user) { create(:user) }
 
-    let!(:issue) { create(:issue, author: user) }
+    let_it_be(:issue) { create(:issue, author: user) }
 
-    let!(:event1) { create(:resource_weight_event, issue: issue) }
-    let!(:event2) { create(:resource_weight_event, issue: issue) }
-    let!(:event3) { create(:resource_weight_event, issue: issue) }
+    let_it_be(:event1) { create(:resource_weight_event, issue: issue) }
+    let_it_be(:event2) { create(:resource_weight_event, issue: issue) }
+    let_it_be(:event3) { create(:resource_weight_event, issue: issue) }
 
     it 'returns the expected synthetic notes' do
       notes = EE::ResourceEvents::SyntheticWeightNotesBuilderService.new(issue, user).execute
 
       expect(notes.size).to eq(3)
     end
+
+    it_behaves_like 'filters by paginated notes', :resource_weight_event
   end
 end

lib/api/discussions.rb

@@ -239,7 +239,7 @@ module API
       # rubocop: disable CodeReuse/ActiveRecord
       def readable_discussion_notes(noteable, discussion_ids)
         notes = noteable.notes
-          .where(discussion_id: discussion_ids)
+          .with_discussion_ids(discussion_ids)
           .inc_relations_for_view
           .includes(:noteable)
           .fresh

spec/frontend/notes/components/discussion_counter_spec.js

@@ -53,7 +53,7 @@ describe('DiscussionCounter component', () => {
 
   describe('has no resolvable discussions', () => {
     it('does not render', () => {
-      store.commit(types.SET_INITIAL_DISCUSSIONS, [{ ...discussionMock, resolvable: false }]);
+      store.commit(types.ADD_OR_UPDATE_DISCUSSIONS, [{ ...discussionMock, resolvable: false }]);
       store.dispatch('updateResolvableDiscussionsCounts');
       wrapper = shallowMount(DiscussionCounter, { store, localVue });
 
@@ -64,7 +64,7 @@ describe('DiscussionCounter component', () => {
   describe('has resolvable discussions', () => {
     const updateStore = (note = {}) => {
       discussionMock.notes[0] = { ...discussionMock.notes[0], ...note };
-      store.commit(types.SET_INITIAL_DISCUSSIONS, [discussionMock]);
+      store.commit(types.ADD_OR_UPDATE_DISCUSSIONS, [discussionMock]);
       store.dispatch('updateResolvableDiscussionsCounts');
     };
 
@@ -97,7 +97,7 @@ describe('DiscussionCounter component', () => {
     let toggleAllButton;
     const updateStoreWithExpanded = (expanded) => {
       const discussion = { ...discussionMock, expanded };
-      store.commit(types.SET_INITIAL_DISCUSSIONS, [discussion]);
+      store.commit(types.ADD_OR_UPDATE_DISCUSSIONS, [discussion]);
       store.dispatch('updateResolvableDiscussionsCounts');
       wrapper = shallowMount(DiscussionCounter, { store, localVue });
       toggleAllButton = wrapper.find('.toggle-all-discussions-btn');

spec/frontend/notes/stores/actions_spec.js

@@ -119,7 +119,7 @@ describe('Actions Notes Store', () => {
         actions.setInitialNotes,
         [individualNote],
         { notes: [] },
-        [{ type: 'SET_INITIAL_DISCUSSIONS', payload: [individualNote] }],
+        [{ type: 'ADD_OR_UPDATE_DISCUSSIONS', payload: [individualNote] }],
         [],
         done,
       );
@@ -1395,4 +1395,93 @@ describe('Actions Notes Store', () => {
       );
     });
   });
+
+  describe('fetchDiscussions', () => {
+    const discussion = { notes: [] };
+
+    afterEach(() => {
+      window.gon = {};
+    });
+
+    it('updates the discussions and dispatches `updateResolvableDiscussionsCounts`', (done) => {
+      axiosMock.onAny().reply(200, { discussion });
+      testAction(
+        actions.fetchDiscussions,
+        {},
+        null,
+        [
+          { type: mutationTypes.ADD_OR_UPDATE_DISCUSSIONS, payload: { discussion } },
+          { type: mutationTypes.SET_FETCHING_DISCUSSIONS, payload: false },
+        ],
+        [{ type: 'updateResolvableDiscussionsCounts' }],
+        done,
+      );
+    });
+
+    it('dispatches `fetchDiscussionsBatch` action if `paginatedIssueDiscussions` feature flag is enabled', (done) => {
+      window.gon = { features: { paginatedIssueDiscussions: true } };
+
+      testAction(
+        actions.fetchDiscussions,
+        { path: 'test-path', filter: 'test-filter', persistFilter: 'test-persist-filter' },
+        null,
+        [],
+        [
+          {
+            type: 'fetchDiscussionsBatch',
+            payload: {
+              config: {
+                params: { notes_filter: 'test-filter', persist_filter: 'test-persist-filter' },
+              },
+              path: 'test-path',
+              perPage: 20,
+            },
+          },
+        ],
+        done,
+      );
+    });
+  });
+
+  describe('fetchDiscussionsBatch', () => {
+    const discussion = { notes: [] };
+
+    const config = {
+      params: { notes_filter: 'test-filter', persist_filter: 'test-persist-filter' },
+    };
+
+    const actionPayload = { config, path: 'test-path', perPage: 20 };
+
+    it('updates the discussions and dispatches `updateResolvableDiscussionsCounts if there are no headers', (done) => {
+      axiosMock.onAny().reply(200, { discussion }, {});
+      testAction(
+        actions.fetchDiscussionsBatch,
+        actionPayload,
+        null,
+        [
+          { type: mutationTypes.ADD_OR_UPDATE_DISCUSSIONS, payload: { discussion } },
+          { type: mutationTypes.SET_FETCHING_DISCUSSIONS, payload: false },
+        ],
+        [{ type: 'updateResolvableDiscussionsCounts' }],
+        done,
+      );
+    });
+
+    it('dispatches itself if there is `x-next-page-cursor` header', (done) => {
+      axiosMock.onAny().reply(200, { discussion }, { 'x-next-page-cursor': 1 });
+      testAction(
+        actions.fetchDiscussionsBatch,
+        actionPayload,
+        null,
+        [{ type: mutationTypes.ADD_OR_UPDATE_DISCUSSIONS, payload: { discussion } }],
+        [
+          {
+            type: 'fetchDiscussionsBatch',
+            payload: { ...actionPayload, perPage: 30, cursor: 1 },
+          },
+        ],
+        done,
+      );
+    });
+  });
 });

spec/frontend/notes/stores/mutation_spec.js

@@ -159,7 +159,7 @@ describe('Notes Store mutations', () => {
     });
   });
 
-  describe('SET_INITIAL_DISCUSSIONS', () => {
+  describe('ADD_OR_UPDATE_DISCUSSIONS', () => {
     it('should set the initial notes received', () => {
       const state = {
         discussions: [],
@@ -169,15 +169,17 @@ describe('Notes Store mutations', () => {
         individual_note: true,
         notes: [
           {
+            id: 100,
             note: '1',
           },
           {
+            id: 101,
             note: '2',
           },
         ],
       };
 
-      mutations.SET_INITIAL_DISCUSSIONS(state, [note, legacyNote]);
+      mutations.ADD_OR_UPDATE_DISCUSSIONS(state, [note, legacyNote]);
 
       expect(state.discussions[0].id).toEqual(note.id);
       expect(state.discussions[1].notes[0].note).toBe(legacyNote.notes[0].note);
@@ -190,7 +192,7 @@ describe('Notes Store mutations', () => {
         discussions: [],
       };
 
-      mutations.SET_INITIAL_DISCUSSIONS(state, [
+      mutations.ADD_OR_UPDATE_DISCUSSIONS(state, [
         {
           ...note,
           diff_file: {
@@ -208,7 +210,7 @@ describe('Notes Store mutations', () => {
         discussions: [],
       };
 
-      mutations.SET_INITIAL_DISCUSSIONS(state, [
+      mutations.ADD_OR_UPDATE_DISCUSSIONS(state, [
         {
           ...note,
           diff_file: {

spec/models/concerns/noteable_spec.rb

@@ -77,6 +77,70 @@ RSpec.describe Noteable do
     end
   end
 
+  describe '#discussion_root_note_ids' do
+    let!(:label_event) { create(:resource_label_event, merge_request: subject) }
+    let!(:system_note) { create(:system_note, project: project, noteable: subject) }
+    let!(:milestone_event) { create(:resource_milestone_event, merge_request: subject) }
+    let!(:state_event) { create(:resource_state_event, merge_request: subject) }
+
+    it 'returns ordered discussion_ids and synthetic note ids' do
+      discussions = subject.discussion_root_note_ids(notes_filter: UserPreference::NOTES_FILTERS[:all_notes]).map do |n|
+        { table_name: n.table_name, discussion_id: n.discussion_id, id: n.id }
+      end
+
+      expect(discussions).to match([
+        a_hash_including(table_name: 'notes', discussion_id: active_diff_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: active_diff_note3.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: outdated_diff_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: discussion_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_diff_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_note2.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_discussion_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_discussion_note3.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: note2.discussion_id),
+        a_hash_including(table_name: 'resource_label_events', id: label_event.id),
+        a_hash_including(table_name: 'notes', discussion_id: system_note.discussion_id),
+        a_hash_including(table_name: 'resource_milestone_events', id: milestone_event.id),
+        a_hash_including(table_name: 'resource_state_events', id: state_event.id)
+      ])
+    end
+
+    it 'filters by comments only' do
+      discussions = subject.discussion_root_note_ids(notes_filter: UserPreference::NOTES_FILTERS[:only_comments]).map do |n|
+        { table_name: n.table_name, discussion_id: n.discussion_id, id: n.id }
+      end
+
+      expect(discussions).to match([
+        a_hash_including(table_name: 'notes', discussion_id: active_diff_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: active_diff_note3.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: outdated_diff_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: discussion_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_diff_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_note2.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_discussion_note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: commit_discussion_note3.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: note1.discussion_id),
+        a_hash_including(table_name: 'notes', discussion_id: note2.discussion_id)
+      ])
+    end
+
+    it 'filters by system notes only' do
+      discussions = subject.discussion_root_note_ids(notes_filter: UserPreference::NOTES_FILTERS[:only_activity]).map do |n|
+        { table_name: n.table_name, discussion_id: n.discussion_id, id: n.id }
+      end
+
+      expect(discussions).to match([
+        a_hash_including(table_name: 'resource_label_events', id: label_event.id),
+        a_hash_including(table_name: 'notes', discussion_id: system_note.discussion_id),
+        a_hash_including(table_name: 'resource_milestone_events', id: milestone_event.id),
+        a_hash_including(table_name: 'resource_state_events', id: state_event.id)
+      ])
+    end
+  end
+
   describe '#grouped_diff_discussions' do
     let(:grouped_diff_discussions) { subject.grouped_diff_discussions }
 

spec/requests/projects/issues_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::IssuesController do
+  let_it_be(:issue) { create(:issue) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { issue.project }
+  let_it_be(:user) { issue.author }
+
+  before do
+    login_as(user)
+  end
+
+  describe 'GET #discussions' do
+    let_it_be(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
+    let_it_be(:discussion_reply) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, in_reply_to: discussion) }
+    let_it_be(:state_event) { create(:resource_state_event, issue: issue) }
+    let_it_be(:discussion_2) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
+    let_it_be(:discussion_3) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
+
+    context 'pagination' do
+      def get_discussions(**params)
+        get discussions_project_issue_path(project, issue, params: params.merge(format: :json))
+      end
+
+      it 'returns paginated notes and cursor based on per_page param' do
+        get_discussions(per_page: 2)
+
+        discussions = Gitlab::Json.parse(response.body)
+        notes = discussions.flat_map { |d| d['notes'] }
+
+        expect(discussions.count).to eq(2)
+        expect(notes).to match([
+          a_hash_including('id' => discussion.id.to_s),
+          a_hash_including('id' => discussion_reply.id.to_s),
+          a_hash_including('type' => 'StateNote')
+        ])
+
+        cursor = response.header['X-Next-Page-Cursor']
+        expect(cursor).to be_present
+
+        get_discussions(per_page: 1, cursor: cursor)
+
+        discussions = Gitlab::Json.parse(response.body)
+        notes = discussions.flat_map { |d| d['notes'] }
+
+        expect(discussions.count).to eq(1)
+        expect(notes).to match([
+          a_hash_including('id' => discussion_2.id.to_s)
+        ])
+      end
+
+      context 'when paginated_issue_discussions is disabled' do
+        before do
+          stub_feature_flags(paginated_issue_discussions: false)
+        end
+
+        it 'returns all discussions and ignores per_page param' do
+          get_discussions(per_page: 2)
+
+          discussions = Gitlab::Json.parse(response.body)
+          notes = discussions.flat_map { |d| d['notes'] }
+
+          expect(discussions.count).to eq(4)
+          expect(notes.count).to eq(5)
+        end
+      end
+    end
+  end
+end

spec/services/resource_events/synthetic_label_notes_builder_service_spec.rb

@@ -4,18 +4,20 @@ require 'spec_helper'
 
 RSpec.describe ResourceEvents::SyntheticLabelNotesBuilderService do
   describe '#execute' do
-    let!(:user) { create(:user) }
+    let_it_be(:user) { create(:user) }
 
-    let!(:issue) { create(:issue, author: user) }
+    let_it_be(:issue) { create(:issue, author: user) }
 
-    let!(:event1) { create(:resource_label_event, issue: issue) }
-    let!(:event2) { create(:resource_label_event, issue: issue) }
-    let!(:event3) { create(:resource_label_event, issue: issue) }
+    let_it_be(:event1) { create(:resource_label_event, issue: issue) }
+    let_it_be(:event2) { create(:resource_label_event, issue: issue) }
+    let_it_be(:event3) { create(:resource_label_event, issue: issue) }
 
     it 'returns the expected synthetic notes' do
       notes = ResourceEvents::SyntheticLabelNotesBuilderService.new(issue, user).execute
 
       expect(notes.size).to eq(3)
     end
+
+    it_behaves_like 'filters by paginated notes', :resource_label_event
   end
 end

spec/services/resource_events/synthetic_milestone_notes_builder_service_spec.rb

@@ -24,5 +24,7 @@ RSpec.describe ResourceEvents::SyntheticMilestoneNotesBuilderService do
         'removed milestone'
       ])
     end
+
+    it_behaves_like 'filters by paginated notes', :resource_milestone_event
   end
 end

spec/services/resource_events/synthetic_state_notes_builder_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ResourceEvents::SyntheticStateNotesBuilderService do
+  describe '#execute' do
+    let_it_be(:user) { create(:user) }
+
+    it_behaves_like 'filters by paginated notes', :resource_state_event
+  end
+end

spec/support/shared_examples/services/resource_events/synthetic_notes_builder_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'filters by paginated notes' do |event_type|
+  let(:event) { create(event_type) } # rubocop:disable Rails/SaveBang
+
+  before do
+    create(event_type, issue: event.issue)
+  end
+
+  it 'only returns given notes' do
+    paginated_notes = { event_type.to_s.pluralize => [double(id: event.id)] }
+    notes = described_class.new(event.issue, user, paginated_notes: paginated_notes).execute
+
+    expect(notes.size).to eq(1)
+    expect(notes.first.event).to eq(event)
+  end
+
+  context 'when paginated notes is empty' do
+    it 'does not return any notes' do
+      notes = described_class.new(event.issue, user, paginated_notes: {}).execute
+
+      expect(notes.size).to eq(0)
+    end
+  end
+end