Updated diffs notes app to use paginated notes

This removes the need to use discussions.json
and also removes calls to this endpoint.
Instead we always use the notes endpoint and populate
the discussions from this endpoint.

https://gitlab.com/gitlab-org/gitlab/-/issues/209784

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

@@ -4,6 +4,7 @@ import OrderedLayout from '~/vue_shared/components/ordered_layout.vue';
 import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user';
 import { __ } from '~/locale';
 import initUserPopovers from '~/user_popovers';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility';
 import { deprecatedCreateFlash as Flash } from '../../flash';
 import * as constants from '../constants';
@@ -30,6 +31,7 @@ export default {
     discussionFilterNote,
     OrderedLayout,
   },
+  mixins: [glFeatureFlagsMixin()],
   props: {
     noteableData: {
       type: Object,
@@ -57,7 +59,6 @@ export default {
   },
   data() {
     return {
-      isFetching: false,
       currentFilter: null,
     };
   },
@@ -68,6 +69,7 @@ export default {
       'convertedDisscussionIds',
       'getNotesDataByProp',
       'isLoading',
+      'isFetching',
       'commentsDisabled',
       'getNoteableData',
       'userCanReply',
@@ -153,6 +155,7 @@ export default {
   },
   methods: {
     ...mapActions([
+      'setFetchingState',
       'setLoadingState',
       'fetchDiscussions',
       'poll',
@@ -183,7 +186,11 @@ export default {
     fetchNotes() {
       if (this.isFetching) return null;
 
-      this.isFetching = true;
+      this.setFetchingState(true);
+
+      if (this.glFeatures.paginatedNotes) {
+        return this.initPolling();
+      }
 
       return this.fetchDiscussions(this.getFetchDiscussionsConfig())
         .then(this.initPolling)
@@ -191,7 +198,7 @@ export default {
           this.setLoadingState(false);
           this.setNotesFetchedState(true);
           eventHub.$emit('fetchedNotesData');
-          this.isFetching = false;
+          this.setFetchingState(false);
         })
         .then(this.$nextTick)
         .then(this.startTaskList)

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

@@ -15,6 +15,7 @@ import loadAwardsHandler from '../../awards_handler';
 import sidebarTimeTrackingEventHub from '../../sidebar/event_hub';
 import { isInViewport, scrollToElement, isInMRPage } from '../../lib/utils/common_utils';
 import { mergeUrlParams } from '../../lib/utils/url_utility';
+import eventHub from '../event_hub';
 import mrWidgetEventHub from '../../vue_merge_request_widget/event_hub';
 import * as utils from './utils';
 import * as types from './mutation_types';
@@ -420,11 +421,21 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
     .catch(processErrors);
 };
 
+export const setFetchingState = ({ commit }, fetchingState) =>
+  commit(types.SET_NOTES_FETCHING_STATE, fetchingState);
+
 const pollSuccessCallBack = (resp, commit, state, getters, dispatch) => {
   if (state.isResolvingDiscussion) {
     return null;
   }
 
+  if (window.gon?.features?.paginatedNotes && !resp.more && state.isFetching) {
+    eventHub.$emit('fetchedNotesData');
+    dispatch('setFetchingState', false);
+    dispatch('setNotesFetchedState', true);
+    dispatch('setLoadingState', false);
+  }
+
   if (resp.notes?.length) {
     dispatch('updateOrCreateNotes', resp.notes);
     dispatch('startTaskList');

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

@@ -48,6 +48,8 @@ export const persistSortOrder = (state) => state.persistSortOrder;
 
 export const timelineEnabled = (state) => state.isTimelineEnabled;
 
+export const isFetching = (state) => state.isFetching;
+
 export const isLoading = (state) => state.isLoading;
 
 export const getNotesDataByProp = (state) => (prop) => state.notesData[prop];

app/assets/javascripts/notes/stores/modules/index.js

@@ -47,6 +47,7 @@ export default () => ({
     unresolvedDiscussionsCount: 0,
     descriptionVersions: {},
     isTimelineEnabled: false,
+    isFetching: false,
   },
   actions,
   getters,

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

@@ -14,6 +14,7 @@ export const UPDATE_NOTE = 'UPDATE_NOTE';
 export const UPDATE_DISCUSSION = 'UPDATE_DISCUSSION';
 export const UPDATE_DISCUSSION_POSITION = 'UPDATE_DISCUSSION_POSITION';
 export const SET_DISCUSSION_DIFF_LINES = 'SET_DISCUSSION_DIFF_LINES';
+export const SET_NOTES_FETCHING_STATE = 'SET_NOTES_FETCHING_STATE';
 export const SET_NOTES_FETCHED_STATE = 'SET_NOTES_FETCHED_STATE';
 export const SET_NOTES_LOADING_STATE = 'SET_NOTES_LOADING_STATE';
 export const DISABLE_COMMENTS = 'DISABLE_COMMENTS';

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

@@ -323,6 +323,10 @@ export default {
     state.isLoading = value;
   },
 
+  [types.SET_NOTES_FETCHING_STATE](state, value) {
+    state.isFetching = value;
+  },
+
   [types.SET_DISCUSSION_DIFF_LINES](state, { discussionId, diffLines }) {
     const discussion = utils.findNoteObjectById(state.discussions, discussionId);
 

app/controllers/projects/merge_requests_controller.rb

@@ -42,6 +42,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     push_frontend_feature_flag(:codequality_mr_diff, @project)
     push_frontend_feature_flag(:suggestions_custom_commit, @project)
     push_frontend_feature_flag(:local_file_reviews, default_enabled: :yaml)
+    push_frontend_feature_flag(:paginated_notes, @project, default_enabled: :yaml)
 
     record_experiment_user(:invite_members_version_a)
     record_experiment_user(:invite_members_version_b)

app/helpers/notes_helper.rb

@@ -175,7 +175,9 @@ module NotesHelper
     end
   end
 
-  def notes_data(issuable)
+  def notes_data(issuable, start_at_zero = false)
+    initial_last_fetched_at = start_at_zero ? 0 : Time.current.to_i * ::Gitlab::UpdatedNotesPaginator::MICROSECOND
+
     data = {
       discussionsPath: discussions_path(issuable),
       registerPath: new_session_path(:user, redirect_to_referer: 'yes', anchor: 'register-pane'),
@@ -186,7 +188,7 @@ module NotesHelper
       reopenPath: reopen_issuable_path(issuable),
       notesPath: notes_url,
       prerenderedNotesCount: issuable.capped_notes_count(MAX_PRERENDERED_NOTES),
-      lastFetchedAt: Time.now.to_i * ::Gitlab::UpdatedNotesPaginator::MICROSECOND
+      lastFetchedAt: initial_last_fetched_at
     }
 
     if issuable.is_a?(MergeRequest)

app/views/projects/merge_requests/show.html.haml

@@ -56,10 +56,13 @@
               = render "projects/merge_requests/widget"
               = render "projects/merge_requests/awards_block"
               - if mr_action === "show"
-                - add_page_startup_api_call discussions_path(@merge_request)
+                - if Feature.enabled?(:paginated_notes, @project)
+                  - add_page_startup_api_call notes_url
+                - else
+                  - add_page_startup_api_call discussions_path(@merge_request)
                 - add_page_startup_api_call widget_project_json_merge_request_path(@project, @merge_request, format: :json)
                 - add_page_startup_api_call cached_widget_project_json_merge_request_path(@project, @merge_request, format: :json)
-              #js-vue-mr-discussions{ data: { notes_data: notes_data(@merge_request).to_json,
+              #js-vue-mr-discussions{ data: { notes_data: notes_data(@merge_request, Feature.enabled?(:paginated_notes, @project)).to_json,
                 noteable_data: serialize_issuable(@merge_request, serializer: 'noteable'),
                 noteable_type: 'MergeRequest',
                 target_type: 'merge_request',

spec/frontend/notes/stores/actions_spec.js

@@ -294,6 +294,40 @@ describe('Actions Notes Store', () => {
           ],
         ));
     });
+
+    describe('paginated notes feature flag enabled', () => {
+      const lastFetchedAt = '12358';
+
+      beforeEach(() => {
+        window.gon = { features: { paginatedNotes: true } };
+
+        axiosMock.onGet(notesDataMock.notesPath).replyOnce(200, {
+          notes: discussionMock.notes,
+          more: false,
+          last_fetched_at: lastFetchedAt,
+        });
+      });
+
+      afterEach(() => {
+        window.gon = null;
+      });
+
+      it('should dispatch setFetchingState, setNotesFetchedState, setLoadingState, updateOrCreateNotes, startTaskList and commit SET_LAST_FETCHED_AT', () => {
+        return testAction(
+          actions.fetchData,
+          null,
+          { notesData: notesDataMock, isFetching: true },
+          [{ type: 'SET_LAST_FETCHED_AT', payload: lastFetchedAt }],
+          [
+            { type: 'setFetchingState', payload: false },
+            { type: 'setNotesFetchedState', payload: true },
+            { type: 'setLoadingState', payload: false },
+            { type: 'updateOrCreateNotes', payload: discussionMock.notes },
+            { type: 'startTaskList' },
+          ],
+        );
+      });
+    });
   });
 
   describe('poll', () => {
@@ -1355,4 +1389,17 @@ describe('Actions Notes Store', () => {
       );
     });
   });
+
+  describe('setFetchingState', () => {
+    it('commits SET_NOTES_FETCHING_STATE', (done) => {
+      testAction(
+        actions.setFetchingState,
+        true,
+        null,
+        [{ type: mutationTypes.SET_NOTES_FETCHING_STATE, payload: true }],
+        [],
+        done,
+      );
+    });
+  });
 });

spec/helpers/notes_helper_spec.rb

@@ -316,4 +316,15 @@ RSpec.describe NotesHelper do
       end
     end
   end
+
+  describe '#notes_data' do
+    let(:issue) { create(:issue, project: project) }
+
+    it 'sets last_fetched_at to 0 when start_at_zero is true' do
+      @project = project
+      @noteable = issue
+
+      expect(helper.notes_data(issue, true)[:lastFetchedAt]).to eq(0)
+    end
+  end
 end