Commit eae5f8aa authored by Robert Speicher's avatar Robert Speicher

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
parents 47648a50 20688cdf
......@@ -69,4 +69,14 @@ def link_to_reply_discussion(note, line_type = nil)
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
......@@ -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
......
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
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