summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2016-06-29 21:34:46 +0000
committerRobert Speicher <robert@gitlab.com>2016-06-29 21:34:46 +0000
commiteae5f8aa57d61ce17f6b4885b8da463d1d33e971 (patch)
treef86abf947b6279ab60f67b6cc570c2470b787aa9
parent47648a50d7bfab644de96025a3001a666cfe7653 (diff)
parent20688cdf0711f0d7d70abdf01db5a4f3a0671c6c (diff)
downloadgitlab-ce-eae5f8aa57d61ce17f6b4885b8da463d1d33e971.tar.gz
Merge branch 'cache-max-user-access-name' into 'master'
Memoize the maximum access level for the author of notes Cache the maximum access level for each user in a map in the controller 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. See merge request !4982
-rw-r--r--app/helpers/notes_helper.rb10
-rw-r--r--app/views/projects/notes/_note.html.haml2
-rw-r--r--spec/helpers/notes_helper_spec.rb46
3 files changed, 57 insertions, 1 deletions
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index b401c8385be..e85ba76887d 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -69,4 +69,14 @@ 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)
+ @max_access_by_user_id ||= Hash.new do |hash, key|
+ project = key[:project]
+ hash[key] = project.team.human_max_access(key[:user_id])
+ end
+
+ full_key = { project: note.project, user_id: note.author_id }
+ @max_access_by_user_id[full_key]
+ 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..08a93503258
--- /dev/null
+++ b/spec/helpers/notes_helper_spec.rb
@@ -0,0 +1,46 @@
+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
+ original_method = project.team.method(:human_max_access)
+ expect_any_instance_of(ProjectTeam).to receive(:human_max_access).exactly(3).times do |*args|
+ original_method.call(args[1])
+ end
+
+ 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
+
+ it 'handles access in different projects' do
+ second_project = create(:empty_project)
+ second_project.team << [master, :reporter]
+ other_note = create(:note, author: master, project: second_project)
+
+ expect(helper.note_max_access_for_user(master_note)).to eq('Master')
+ expect(helper.note_max_access_for_user(other_note)).to eq('Reporter')
+ end
+ end
+end