From 50491d324135f14e817d7de9d825b9ce4dacc5ef Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 14 Feb 2019 01:26:00 +0800
Subject: Instead of returning all or nothing, return whichever passed

And add tests
---
 lib/gitlab/graphql/authorize/instrumentation.rb    | 15 ++++-----
 .../graphql/authorize/instrumentation_spec.rb      | 32 ++++++++++++++++---
 spec/requests/api/graphql/project/issues_spec.rb   | 36 ++++++++++++++++++++++
 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
-- 
cgit v1.2.1