Frontend to fetch paginated discussions

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/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/views/projects/issues/_discussion.html.haml

-- add_page_startup_api_call Feature.enabled?(:paginated_discussions, @project) ? discussions_path(@issue, per_page: 20) : 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

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,
       );

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: {