summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2016-06-28 15:14:11 -0700
committerStan Hu <stanhu@gmail.com>2016-06-29 06:26:00 -0700
commit8c29b0b06554eb9549fe9bd2f33e80ce149752fd (patch)
tree9c0f24bc41ada0eba858dc21cb50ed0d300dd202
parenta4b1fe4de951cb18a061b2be7b85988e6db26f5b (diff)
downloadgitlab-ce-8c29b0b06554eb9549fe9bd2f33e80ce149752fd.tar.gz
Memoize the maximum access level for the author of notes
In #19273, we saw that retrieving ProjectTeam#human_max_access for each note takes the bulk of the time when rendering certain issues or merge requests. We observe that most of the comments in an issue are typically done by the same users. This MR memoizes the max access level by user ID.
-rw-r--r--app/helpers/notes_helper.rb8
-rw-r--r--app/views/projects/notes/_note.html.haml2
-rw-r--r--spec/helpers/notes_helper_spec.rb32
3 files changed, 41 insertions, 1 deletions
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index b401c8385be..80d588aaf5e 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -69,4 +69,12 @@ module NotesHelper
button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
data: data, title: 'Add a reply'
end
+
+ def note_max_access_for_user(note)
+ user_id = note.author.id
+ project = note.project
+ @max_access_by_user_id ||= Hash.new { |hash, key| hash[key] = project.team.human_max_access(key) }
+
+ @max_access_by_user_id[user_id]
+ end
end
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index a5e163b91e9..af0046886fb 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/app/views/projects/notes/_note.html.haml
@@ -17,7 +17,7 @@
%a{ href: "##{dom_id(note)}" }
= time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
.note-actions
- - access = note.project.team.human_max_access(note.author.id)
+ - access = note_max_access_for_user(note)
- if access and not note.system
%span.note-role.hidden-xs= access
- if current_user and not note.system
diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb
new file mode 100644
index 00000000000..8325af0dcf8
--- /dev/null
+++ b/spec/helpers/notes_helper_spec.rb
@@ -0,0 +1,32 @@
+require "spec_helper"
+
+describe NotesHelper do
+ describe "#notes_max_access_for_users" do
+ let(:owner) { create(:owner) }
+ let(:group) { create(:group) }
+ let(:project) { create(:empty_project, namespace: group) }
+ let(:master) { create(:user) }
+ let(:reporter) { create(:user) }
+ let(:guest) { create(:user) }
+
+ let(:owner_note) { create(:note, author: owner, project: project) }
+ let(:master_note) { create(:note, author: master, project: project) }
+ let(:reporter_note) { create(:note, author: reporter, project: project) }
+ let!(:notes) { [owner_note, master_note, reporter_note] }
+
+ before do
+ group.add_owner(owner)
+ project.team << [master, :master]
+ project.team << [reporter, :reporter]
+ project.team << [guest, :guest]
+ end
+
+ it 'return human access levels' do
+ expect(helper.note_max_access_for_user(owner_note)).to eq('Owner')
+ expect(helper.note_max_access_for_user(master_note)).to eq('Master')
+ expect(helper.note_max_access_for_user(reporter_note)).to eq('Reporter')
+ # Call it again to ensure value is cached
+ expect(helper.note_max_access_for_user(owner_note)).to eq('Owner')
+ end
+ end
+end