Merge branch 'pedropombeiro/334956-fix-nil-project-count' into 'master'

Return 0 for projectCount when no associated projects are found

See merge request gitlab-org/gitlab!65252

app/graphql/types/ci/runner_type.rb

@@ -54,10 +54,12 @@ module Types
       # rubocop: disable CodeReuse/ActiveRecord
       def project_count
         BatchLoader::GraphQL.for(runner.id).batch(key: :runner_project_count) do |ids, loader, args|
-          counts = ::Ci::RunnerProject.select(:runner_id, 'COUNT(*) as count')
-            .where(runner_id: ids)
-            .group(:runner_id)
-            .index_by(&:runner_id)
+          counts = ::Ci::Runner.project_type
+            .select(:id, 'COUNT(ci_runner_projects.id) as count')
+            .left_outer_joins(:runner_projects)
+            .where(id: ids)
+            .group(:id)
+            .index_by(&:id)
 
           ids.each do |id|
             loader.call(id, counts[id]&.count)

spec/requests/api/graphql/ci/runner_spec.rb

@@ -5,25 +5,25 @@ require 'spec_helper'
 RSpec.describe 'Query.runner(id)' do
   include GraphqlHelpers
 
-  let_it_be(:user) { create_default(:user, :admin) }
+  let_it_be(:user) { create(:user, :admin) }
 
-  let_it_be(:active_runner) do
+  let_it_be(:active_instance_runner) do
     create(:ci_runner, :instance, description: 'Runner 1', contacted_at: 2.hours.ago,
            active: true, version: 'adfe156', revision: 'a', locked: true, ip_address: '127.0.0.1', maximum_timeout: 600,
            access_level: 0, tag_list: %w[tag1 tag2], run_untagged: true)
   end
 
-  let_it_be(:inactive_runner) do
+  let_it_be(:inactive_instance_runner) do
     create(:ci_runner, :instance, description: 'Runner 2', contacted_at: 1.day.ago, active: false,
            version: 'adfe157', revision: 'b', ip_address: '10.10.10.10', access_level: 1, run_untagged: true)
   end
 
   def get_runner(id)
     case id
-    when :active_runner
-      active_runner
-    when :inactive_runner
-      inactive_runner
+    when :active_instance_runner
+      active_instance_runner
+    when :inactive_instance_runner
+      inactive_instance_runner
     end
   end
 
@@ -86,7 +86,7 @@ RSpec.describe 'Query.runner(id)' do
   end
 
   describe 'for active runner' do
-    it_behaves_like 'runner details fetch', :active_runner
+    it_behaves_like 'runner details fetch', :active_instance_runner
 
     context 'when tagList is not requested' do
       let(:query) do
@@ -95,7 +95,7 @@ RSpec.describe 'Query.runner(id)' do
 
       let(:query_path) do
         [
-          [:runner, { id: active_runner.to_global_id.to_s }]
+          [:runner, { id: active_instance_runner.to_global_id.to_s }]
         ]
       end
 
@@ -110,29 +110,30 @@ RSpec.describe 'Query.runner(id)' do
   end
 
   describe 'for inactive runner' do
-    it_behaves_like 'runner details fetch', :inactive_runner
+    it_behaves_like 'runner details fetch', :inactive_instance_runner
   end
 
   describe 'for multiple runners' do
     let_it_be(:project1) { create(:project, :test_repo) }
     let_it_be(:project2) { create(:project, :test_repo) }
-    let_it_be(:project_runner) do
-      create(:ci_runner, :project, projects: [project1, project2], description: 'Runner 1', contacted_at: 2.hours.ago,
-             active: true, version: 'adfe156', revision: 'a', locked: true, ip_address: '127.0.0.1', maximum_timeout: 600,
-             access_level: 0, run_untagged: true)
-    end
+    let_it_be(:project_runner1) { create(:ci_runner, :project, projects: [project1, project2], description: 'Runner 1') }
+    let_it_be(:project_runner2) { create(:ci_runner, :project, projects: [], description: 'Runner 2') }
 
-    let!(:job) { create(:ci_build, runner: project_runner) }
+    let!(:job) { create(:ci_build, runner: project_runner1) }
 
     context 'requesting project and job counts' do
       let(:query) do
         %(
           query {
-            projectRunner: runner(id: "#{project_runner.to_global_id}") {
+            projectRunner1: runner(id: "#{project_runner1.to_global_id}") {
               projectCount
               jobCount
             }
-            activeRunner: runner(id: "#{active_runner.to_global_id}") {
+            projectRunner2: runner(id: "#{project_runner2.to_global_id}") {
+              projectCount
+              jobCount
+            }
+            activeInstanceRunner: runner(id: "#{active_instance_runner.to_global_id}") {
               projectCount
               jobCount
             }
@@ -141,17 +142,23 @@ RSpec.describe 'Query.runner(id)' do
       end
 
       before do
+        project_runner2.projects.clear
+
         post_graphql(query, current_user: user)
       end
 
       it 'retrieves expected fields' do
-        runner1_data = graphql_data_at(:project_runner)
-        runner2_data = graphql_data_at(:active_runner)
+        runner1_data = graphql_data_at(:project_runner1)
+        runner2_data = graphql_data_at(:project_runner2)
+        runner3_data = graphql_data_at(:active_instance_runner)
 
         expect(runner1_data).to match a_hash_including(
           'jobCount' => 1,
           'projectCount' => 2)
         expect(runner2_data).to match a_hash_including(
+          'jobCount' => 0,
+          'projectCount' => 0)
+        expect(runner3_data).to match a_hash_including(
           'jobCount' => 0,
           'projectCount' => nil)
       end
@@ -159,15 +166,15 @@ RSpec.describe 'Query.runner(id)' do
   end
 
   describe 'by regular user' do
-    let(:user) { create_default(:user) }
+    let(:user) { create(:user) }
 
-    it_behaves_like 'retrieval by unauthorized user', :active_runner
+    it_behaves_like 'retrieval by unauthorized user', :active_instance_runner
   end
 
   describe 'by unauthenticated user' do
     let(:user) { nil }
 
-    it_behaves_like 'retrieval by unauthorized user', :active_runner
+    it_behaves_like 'retrieval by unauthorized user', :active_instance_runner
   end
 
   describe 'Query limits' do
@@ -185,7 +192,7 @@ RSpec.describe 'Query.runner(id)' do
     let(:single_query) do
       <<~QUERY
         {
-          active: #{runner_query(active_runner)}
+          active: #{runner_query(active_instance_runner)}
         }
       QUERY
     end
@@ -193,8 +200,8 @@ RSpec.describe 'Query.runner(id)' do
     let(:double_query) do
       <<~QUERY
         {
-          active: #{runner_query(active_runner)}
-          inactive: #{runner_query(inactive_runner)}
+          active: #{runner_query(active_instance_runner)}
+          inactive: #{runner_query(inactive_instance_runner)}
         }
       QUERY
     end