Commit 72544449 authored by Igor Drozdov's avatar Igor Drozdov

Change the way totalNotes is calculated

totalNotes is only used to prerender a number of
skeleton containers until real notes are loaded

issuable.discussions makes multiple requests, so
too expensive for this

This commit uses mere notes for this and sends
actual totalNotes number if it's less than 10;
otherwise it sends 10 - it allows us to avoid
bunch of skeleton prerenderings, which are not
necessary since they doesn't fit into the whole
screen and disappear quite fast
parent 7671c592
...@@ -75,9 +75,9 @@ export default { ...@@ -75,9 +75,9 @@ export default {
}, },
allDiscussions() { allDiscussions() {
if (this.isLoading) { 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, isSkeletonNote: true,
}); });
} }
......
# frozen_string_literal: true # frozen_string_literal: true
module NotesHelper module NotesHelper
MAX_PRERENDERED_NOTES = 10
def note_target_fields(note) def note_target_fields(note)
if note.noteable if note.noteable
hidden_field_tag(:target_type, note.noteable.class.name.underscore) + hidden_field_tag(:target_type, note.noteable.class.name.underscore) +
...@@ -169,7 +171,7 @@ module NotesHelper ...@@ -169,7 +171,7 @@ module NotesHelper
closePath: close_issuable_path(issuable), closePath: close_issuable_path(issuable),
reopenPath: reopen_issuable_path(issuable), reopenPath: reopen_issuable_path(issuable),
notesPath: notes_url, notesPath: notes_url,
totalNotes: issuable.discussions.length, prerenderedNotesCount: issuable.capped_notes_count(MAX_PRERENDERED_NOTES),
lastFetchedAt: Time.now.to_i lastFetchedAt: Time.now.to_i
} }
end end
......
...@@ -73,6 +73,10 @@ module Noteable ...@@ -73,6 +73,10 @@ module Noteable
.discussions(self) .discussions(self)
end end
def capped_notes_count(max)
notes.limit(max).count
end
def grouped_diff_discussions(*args) def grouped_diff_discussions(*args)
# Doesn't use `discussion_notes`, because this may include commit diff notes # 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. # besides MR diff notes, that we do not want to display on the MR Changes tab.
......
...@@ -8,7 +8,7 @@ export const notesDataMock = { ...@@ -8,7 +8,7 @@ export const notesDataMock = {
notesPath: '/gitlab-org/gitlab-ce/noteable/issue/98/notes', notesPath: '/gitlab-org/gitlab-ce/noteable/issue/98/notes',
quickActionsDocsPath: '/help/user/project/quick_actions', quickActionsDocsPath: '/help/user/project/quick_actions',
registerPath: '/users/sign_in?redirect_to_referer=yes#register-pane', 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', closePath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=close',
reopenPath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=reopen', reopenPath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=reopen',
canAwardEmoji: true, canAwardEmoji: true,
......
...@@ -272,4 +272,22 @@ describe Noteable do ...@@ -272,4 +272,22 @@ describe Noteable do
expect(described_class.resolvable_types).to include('MergeRequest') expect(described_class.resolvable_types).to include('MergeRequest')
end end
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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment