summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLin Jen-Shin <godfat@godfat.org>2019-02-14 01:26:00 +0800
committerLin Jen-Shin <godfat@godfat.org>2019-02-14 15:52:17 +0800
commit50491d324135f14e817d7de9d825b9ce4dacc5ef (patch)
treec0a187360e24d40a95f48b1e1a150840384dd2b3
parent30918929ad079a591b85303d6662ec0a43c5df04 (diff)
downloadgitlab-ce-50491d324135f14e817d7de9d825b9ce4dacc5ef.tar.gz
Instead of returning all or nothing, return whichever passed
And add tests
-rw-r--r--lib/gitlab/graphql/authorize/instrumentation.rb15
-rw-r--r--spec/lib/gitlab/graphql/authorize/instrumentation_spec.rb32
-rw-r--r--spec/requests/api/graphql/project/issues_spec.rb36
3 files changed, 70 insertions, 13 deletions
diff --git a/lib/gitlab/graphql/authorize/instrumentation.rb b/lib/gitlab/graphql/authorize/instrumentation.rb
index d6d3ff300f6..2a3d790d67b 100644
--- a/lib/gitlab/graphql/authorize/instrumentation.rb
+++ b/lib/gitlab/graphql/authorize/instrumentation.rb
@@ -45,15 +45,12 @@ module Gitlab
end
end
- checked =
- case value
- when Array
- value.all?(&check)
- else
- check.call(value)
- end
-
- value if checked
+ case value
+ when Array
+ value.select(&check)
+ else
+ value if check.call(value)
+ end
end
end
end
diff --git a/spec/lib/gitlab/graphql/authorize/instrumentation_spec.rb b/spec/lib/gitlab/graphql/authorize/instrumentation_spec.rb
index 69f53fce715..cf3a8bcc8b4 100644
--- a/spec/lib/gitlab/graphql/authorize/instrumentation_spec.rb
+++ b/spec/lib/gitlab/graphql/authorize/instrumentation_spec.rb
@@ -15,7 +15,7 @@ describe Gitlab::Graphql::Authorize::Instrumentation do
object = double(:object)
abilities.each do |ability|
- spy_ability_check_for(ability, object)
+ spy_ability_check_for(ability, object, passed: true)
end
expect(checker.call(object)).to eq(object)
@@ -26,18 +26,42 @@ describe Gitlab::Graphql::Authorize::Instrumentation do
abilities.each do |ability|
objects.each do |object|
- spy_ability_check_for(ability, object)
+ spy_ability_check_for(ability, object, passed: true)
end
end
expect(checker.call(objects)).to eq(objects)
end
- def spy_ability_check_for(ability, object)
+ context 'when some objects would not pass the check' do
+ it 'returns nil when it is single object' do
+ disallowed = double(:object)
+
+ spy_ability_check_for(abilities.first, disallowed, passed: false)
+
+ expect(checker.call(disallowed)).to be_nil
+ end
+
+ it 'returns only objects which passed when there are more than one' do
+ allowed = double(:allowed)
+ disallowed = double(:disallowed)
+
+ spy_ability_check_for(abilities.first, disallowed, passed: false)
+
+ abilities.each do |ability|
+ spy_ability_check_for(ability, allowed, passed: true)
+ end
+
+ expect(checker.call([disallowed, allowed]))
+ .to contain_exactly(allowed)
+ end
+ end
+
+ def spy_ability_check_for(ability, object, passed: true)
expect(Ability)
.to receive(:allowed?)
.with(current_user, ability, object)
- .and_return(true)
+ .and_return(passed)
end
end
end
diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb
index 355336ad7e2..1e3f11dc4e1 100644
--- a/spec/requests/api/graphql/project/issues_spec.rb
+++ b/spec/requests/api/graphql/project/issues_spec.rb
@@ -56,4 +56,40 @@ describe 'getting an issue list for a project' do
expect(issues_data).to eq []
end
end
+
+ context 'when there is a confidential issue' do
+ let!(:confidential_issue) do
+ create(:issue, :confidential, project: project)
+ end
+
+ context 'when the user cannot see confidential issues' do
+ it 'returns issues without confidential issues' do
+ post_graphql(query, current_user: current_user)
+
+ expect(issues_data.size).to eq(2)
+
+ issues_data.each do |issue|
+ expect(issue.dig('node', 'confidential')).to eq(false)
+ end
+ end
+ end
+
+ context 'when the user can see confidential issues' do
+ before do
+ project.add_developer(current_user)
+ end
+
+ it 'returns issues with confidential issues' do
+ post_graphql(query, current_user: current_user)
+
+ expect(issues_data.size).to eq(3)
+
+ confidentials = issues_data.map do |issue|
+ issue.dig('node', 'confidential')
+ end
+
+ expect(confidentials).to eq([true, false, false])
+ end
+ end
+ end
end