Merge branch 'id-change-total-notes-calculation' into 'master'

Change the way totalNotes is calculated

See merge request gitlab-org/gitlab-ce!32191

5 files changed, 28 insertions, 4 deletions

diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue
index a0695f9e191..16a0fb3f33a 100644
--- a/app/assets/javascripts/notes/components/notes_app.vue
+++ b/app/assets/javascripts/notes/components/notes_app.vue
@@ -75,9 +75,9 @@ export default {
     },
     allDiscussions() {
       if (this.isLoading) {
-        const totalNotes = parseInt(this.notesData.totalNotes, 10) || 0;
+        const prerenderedNotesCount = parseInt(this.notesData.prerenderedNotesCount, 10) || 0;
 
-        return new Array(totalNotes).fill({
+        return new Array(prerenderedNotesCount).fill({
           isSkeletonNote: true,
         });
       }
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 2e31a5e2ed4..4e88b379e16 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 module NotesHelper
+  MAX_PRERENDERED_NOTES = 10
+
   def note_target_fields(note)
     if note.noteable
       hidden_field_tag(:target_type, note.noteable.class.name.underscore) +
@@ -169,7 +171,7 @@ module NotesHelper
       closePath: close_issuable_path(issuable),
       reopenPath: reopen_issuable_path(issuable),
       notesPath: notes_url,
-      totalNotes: issuable.discussions.length,
+      prerenderedNotesCount: issuable.capped_notes_count(MAX_PRERENDERED_NOTES),
       lastFetchedAt: Time.now.to_i
     }
   end
diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb
index 4b428b0af83..6a44bc7c401 100644
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@@ -73,6 +73,10 @@ module Noteable
       .discussions(self)
   end
 
+  def capped_notes_count(max)
+    notes.limit(max).count
+  end
+
   def grouped_diff_discussions(*args)
     # Doesn't use `discussion_notes`, because this may include commit diff notes
     # besides MR diff notes, that we do not want to display on the MR Changes tab.
diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js
index 5f81a168498..3812d46f838 100644
--- a/spec/javascripts/notes/mock_data.js
+++ b/spec/javascripts/notes/mock_data.js
@@ -8,7 +8,7 @@ export const notesDataMock = {
   notesPath: '/gitlab-org/gitlab-ce/noteable/issue/98/notes',
   quickActionsDocsPath: '/help/user/project/quick_actions',
   registerPath: '/users/sign_in?redirect_to_referer=yes#register-pane',
-  totalNotes: 1,
+  prerenderedNotesCount: 1,
   closePath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=close',
   reopenPath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=reopen',
   canAwardEmoji: true,
diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb
index e17b98536fa..929b5f52c7c 100644
--- a/spec/models/concerns/noteable_spec.rb
+++ b/spec/models/concerns/noteable_spec.rb
@@ -272,4 +272,22 @@ describe Noteable do
       expect(described_class.resolvable_types).to include('MergeRequest')
     end
   end
+
+  describe '#capped_notes_count' do
+    context 'notes number < 10' do
+      it 'the number of notes is returned' do
+        expect(subject.capped_notes_count(10)).to eq(9)
+      end
+    end
+
+    context 'notes number > 10' do
+      before do
+        create_list(:note, 2, project: project, noteable: subject)
+      end
+
+      it '10 is returned' do
+        expect(subject.capped_notes_count(10)).to eq(10)
+      end
+    end
+  end
 end