Merge branch 'sh-remove-nplusone-admin-runners-tags' into 'master'

Remove N+1 query for tags in /admin/runners page

See merge request gitlab-org/gitlab-ce!25572

app/finders/admin/runners_finder.rb

@@ -14,7 +14,7 @@ def execute
     sort!
     paginate!
 
-    @runners
+    @runners.with_tags
   end
 
   def sort_key

app/models/ci/runner.rb

@@ -97,6 +97,7 @@ class Runner < ActiveRecord::Base
 
     scope :order_contacted_at_asc, -> { order(contacted_at: :asc) }
     scope :order_created_at_desc, -> { order(created_at: :desc) }
+    scope :with_tags, -> { preload(:tags) }
 
     validate :tag_constraints
     validates :access_level, presence: true

app/views/admin/runners/_runner.html.haml

@@ -49,7 +49,7 @@
   .table-section.section-10.section-wrap
     .table-mobile-header{ role: 'rowheader' }= _('Tags')
     .table-mobile-content
-      - runner.tag_list.sort.each do |tag|
+      - runner.tags.map(&:name).sort.each do |tag|
         %span.badge.badge-primary
           = tag
 

changelogs/unreleased/sh-remove-nplusone-admin-runners-tags.yml

+---
+title: Remove N+1 query for tags in /admin/runners page
+merge_request: 25572
+author:
+type: performance

spec/controllers/admin/runners_controller_spec.rb

 require 'spec_helper'
 
 describe Admin::RunnersController do
-  let(:runner) { create(:ci_runner) }
+  let!(:runner) { create(:ci_runner) }
 
   before do
     sign_in(create(:admin))
   end
 
   describe '#index' do
+    render_views
+
     it 'lists all runners' do
       get :index
 
       expect(response).to have_gitlab_http_status(200)
     end
+
+    it 'avoids N+1 queries', :request_store do
+      get :index
+
+      control_count = ActiveRecord::QueryRecorder.new { get :index }.count
+
+      create(:ci_runner, :tagged_only)
+
+      # There is still an N+1 query for `runner.builds.count`
+      expect { get :index }.not_to exceed_query_limit(control_count + 1)
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(response.body).to have_content('tag1')
+      expect(response.body).to have_content('tag2')
+    end
   end
 
   describe '#show' do