summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIgor Drozdov <idrozdov@gitlab.com>2019-08-26 02:55:00 +0300
committerIgor Drozdov <idrozdov@gitlab.com>2019-08-28 08:30:13 +0300
commit72544449cfb8e35b8c044f5bfd08a2fae2253408 (patch)
tree9fb30b7c20955968f090a9f61582ad737f9afc4b
parent7671c592f826f44be5a8a7dc947fba467f5df851 (diff)
downloadgitlab-ce-72544449cfb8e35b8c044f5bfd08a2fae2253408.tar.gz
Change the way totalNotes is calculatedid-change-total-notes-calculation
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
-rw-r--r--app/assets/javascripts/notes/components/notes_app.vue4
-rw-r--r--app/helpers/notes_helper.rb4
-rw-r--r--app/models/concerns/noteable.rb4
-rw-r--r--spec/javascripts/notes/mock_data.js2
-rw-r--r--spec/models/concerns/noteable_spec.rb18
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