Added IssueCollection

This class can be used to reduce a list of issues down to a subset based
on user permissions. This class operates in such a way that it can
reduce issues using as few queries as possible, if any at all.
This commit is contained in:
Yorick Peterse 2016-10-12 14:01:34 +02:00
parent 89bb29b247
commit f694f94c49
No known key found for this signature in database
GPG key ID: EDD30D2BEB691AC9
6 changed files with 127 additions and 8 deletions

View file

@ -286,6 +286,11 @@ module Issuable
false false
end end
def assignee_or_author?(user)
# We're comparing IDs here so we don't need to load any associations.
author_id == user.id || assignee_id == user.id
end
def record_metrics def record_metrics
metrics = self.metrics || create_metrics metrics = self.metrics || create_metrics
metrics.record! metrics.record!

View file

@ -0,0 +1,40 @@
# IssueCollection can be used to reduce a list of issues down to a subset.
#
# IssueCollection is not meant to be some sort of Enumerable, instead it's meant
# to take a list of issues and return a new list of issues based on some
# criteria. For example, given a list of issues you may want to return a list of
# issues that can be read or updated by a given user.
class IssueCollection
attr_reader :collection
def initialize(collection)
@collection = collection
end
# Returns all the issues that can be updated by the user.
def updatable_by_user(user)
return collection if user.admin?
# Given all the issue projects we get a list of projects that the current
# user has at least reporter access to.
projects_with_reporter_access = user.
projects_with_reporter_access_limited_to(project_ids).
pluck(:id)
collection.select do |issue|
if projects_with_reporter_access.include?(issue.project_id)
true
elsif issue.is_a?(Issue)
issue.assignee_or_author?(user)
else
false
end
end
end
private
def project_ids
@project_ids ||= collection.map(&:project_id).uniq
end
end

View file

@ -4,7 +4,7 @@ class IssuablePolicy < BasePolicy
end end
def rules def rules
if @user && (@subject.author == @user || @subject.assignee == @user) if @user && @subject.assignee_or_author?(@user)
can! :"read_#{action_name}" can! :"read_#{action_name}"
can! :"update_#{action_name}" can! :"update_#{action_name}"
end end

View file

@ -8,9 +8,8 @@ class IssuePolicy < IssuablePolicy
if @subject.confidential? && !can_read_confidential? if @subject.confidential? && !can_read_confidential?
cannot! :read_issue cannot! :read_issue
cannot! :admin_issue
cannot! :update_issue cannot! :update_issue
cannot! :read_issue cannot! :admin_issue
end end
end end
@ -18,11 +17,7 @@ class IssuePolicy < IssuablePolicy
def can_read_confidential? def can_read_confidential?
return false unless @user return false unless @user
return true if @user.admin?
return true if @subject.author == @user
return true if @subject.assignee == @user
return true if @subject.project.team.member?(@user, Gitlab::Access::REPORTER)
false IssueCollection.new([@subject]).updatable_by_user(@user).any?
end end
end end

View file

@ -341,4 +341,25 @@ describe Issue, "Issuable" do
expect(Issue.with_label([bug.title, enhancement.title])).to match_array([issue2]) expect(Issue.with_label([bug.title, enhancement.title])).to match_array([issue2])
end end
end end
describe '#assignee_or_author?' do
let(:user) { build(:user, id: 1) }
let(:issue) { build(:issue) }
it 'returns true for a user that is assigned to an issue' do
issue.assignee = user
expect(issue.assignee_or_author?(user)).to eq(true)
end
it 'returns true for a user that is the author of an issue' do
issue.author = user
expect(issue.assignee_or_author?(user)).to eq(true)
end
it 'returns false for a user that is not the assignee or author' do
expect(issue.assignee_or_author?(user)).to eq(false)
end
end
end end

View file

@ -0,0 +1,58 @@
require 'spec_helper'
describe IssueCollection do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:issue1) { create(:issue, project: project) }
let(:issue2) { create(:issue, project: project) }
let(:collection) { described_class.new([issue1, issue2]) }
describe '#collection' do
it 'returns the issues in the same order as the input Array' do
expect(collection.collection).to eq([issue1, issue2])
end
end
describe '#updatable_by_user' do
context 'using an admin user' do
it 'returns all issues' do
user = create(:admin)
expect(collection.updatable_by_user(user)).to eq([issue1, issue2])
end
end
context 'using a user that has no access to the project' do
it 'returns no issues when the user is not an assignee or author' do
expect(collection.updatable_by_user(user)).to be_empty
end
it 'returns the issues the user is assigned to' do
issue1.assignee = user
expect(collection.updatable_by_user(user)).to eq([issue1])
end
it 'returns the issues for which the user is the author' do
issue1.author = user
expect(collection.updatable_by_user(user)).to eq([issue1])
end
end
context 'using a user that has reporter access to the project' do
it 'returns the issues of the project' do
project.team << [user, :reporter]
expect(collection.updatable_by_user(user)).to eq([issue1, issue2])
end
end
context 'using a user that is the owner of a project' do
it 'returns the issues of the project' do
expect(collection.updatable_by_user(project.namespace.owner)).
to eq([issue1, issue2])
end
end
end
end