Incorporate review comments

5 files changed, 12 insertions, 10 deletions

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 16ed7c2b6b4..91ff9407216 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -71,7 +71,7 @@ class Projects::IssuesController < Projects::ApplicationController
     @note     = @project.notes.new(noteable: @issue)
     @noteable = @issue
 
-    preload_max_access_for_authors(@notes, @project) if @notes
+    preload_max_access_for_authors(@notes, @project)
 
     respond_to do |format|
       format.html
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index da1b9c3e48a..23252fa59cc 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -387,7 +387,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       @ref
     )
 
-    preload_max_access_for_authors(@notes, @project) if @notes
+    preload_max_access_for_authors(@notes, @project)
   end
 
   def define_widget_vars
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 6884d99c5a6..e47c5539f60 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -389,10 +389,8 @@ class Ability
     end
 
     def can_edit_note?(user, note)
-      return false unless note.editable?
-      return false unless user.present?
-      return true if note.author == user
-      return true if user.admin?
+      return false if !note.editable? || !user.present?
+      return true if note.author == user || user.admin?
 
       if note.project
         max_access_level = note.project.team.max_member_access(user.id)
diff --git a/app/models/member.rb b/app/models/member.rb
index 44db3d977fa..24ab1276ee9 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -53,6 +53,10 @@ class Member < ActiveRecord::Base
   default_value_for :notification_level, NotificationSetting.levels[:global]
 
   class << self
+    def access_for_user_ids(user_ids)
+      where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h
+    end
+
     def find_by_invite_token(invite_token)
       invite_token = Devise.token_generator.digest(self, :invite_token, invite_token)
       find_by(invite_token: invite_token)
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index 67faea1f9f3..21b3a013673 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -138,20 +138,20 @@ class ProjectTeam
   def max_member_access_for_user_ids(user_ids)
     user_ids = user_ids.uniq
     key = "max_member_access:#{project.id}"
-    RequestStore.store[key] ||= Hash.new
+    RequestStore.store[key] ||= {}
     access = RequestStore.store[key]
 
     # Lookup only the IDs we need
     user_ids = user_ids - access.keys
 
     if user_ids.present?
-      user_ids.map { |id| access[id] = Gitlab::Access::NO_ACCESS }
+      user_ids.each { |id| access[id] = Gitlab::Access::NO_ACCESS }
 
-      member_access = project.members.where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h
+      member_access = project.members.access_for_user_ids(user_ids)
       merge_max!(access, member_access)
 
       if group
-        group_access = group.members.where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h
+        group_access = group.members.access_for_user_ids(user_ids)
         merge_max!(access, group_access)
       end