Merge branch '345710-update-counters-runner--tabs' into 'master'

Update total count of runners for each type

See merge request gitlab-org/gitlab!77752

app/assets/javascripts/runner/admin_runners/admin_runners_app.vue

@@ -22,6 +22,7 @@ import {
   I18N_FETCH_ERROR,
 } from '../constants';
 import getRunnersQuery from '../graphql/get_runners.query.graphql';
+import getRunnersCountQuery from '../graphql/get_runners_count.query.graphql';
 import {
   fromUrlQueryToSearch,
   fromSearchToUrl,
@@ -29,6 +30,17 @@ import {
 } from '../runner_search_utils';
 import { captureException } from '../sentry_utils';
 
+const runnersCountSmartQuery = {
+  query: getRunnersCountQuery,
+  fetchPolicy: fetchPolicies.CACHE_AND_NETWORK,
+  update(data) {
+    return data?.runners?.count;
+  },
+  error(error) {
+    this.reportToSentry(error);
+  },
+};
+
 export default {
   name: 'AdminRunnersApp',
   components: {
@@ -51,22 +63,6 @@ export default {
       type: String,
       required: true,
     },
-    allRunnersCount: {
-      type: String,
-      required: true,
-    },
-    instanceRunnersCount: {
-      type: String,
-      required: true,
-    },
-    groupRunnersCount: {
-      type: String,
-      required: true,
-    },
-    projectRunnersCount: {
-      type: String,
-      required: true,
-    },
   },
   data() {
     return {
@@ -100,11 +96,49 @@ export default {
         this.reportToSentry(error);
       },
     },
+    allRunnersCount: {
+      ...runnersCountSmartQuery,
+      variables() {
+        return this.countVariables;
+      },
+    },
+    instanceRunnersCount: {
+      ...runnersCountSmartQuery,
+      variables() {
+        return {
+          ...this.countVariables,
+          type: INSTANCE_TYPE,
+        };
+      },
+    },
+    groupRunnersCount: {
+      ...runnersCountSmartQuery,
+      variables() {
+        return {
+          ...this.countVariables,
+          type: GROUP_TYPE,
+        };
+      },
+    },
+    projectRunnersCount: {
+      ...runnersCountSmartQuery,
+      variables() {
+        return {
+          ...this.countVariables,
+          type: PROJECT_TYPE,
+        };
+      },
+    },
   },
   computed: {
     variables() {
       return fromSearchToVariables(this.search);
     },
+    countVariables() {
+      // Exclude pagination variables, leave only filters variables
+      const { sort, before, last, after, first, ...countVariables } = this.variables;
+      return countVariables;
+    },
     runnersLoading() {
       return this.$apollo.queries.runners.loading;
     },
@@ -125,7 +159,7 @@ export default {
     search: {
       deep: true,
       handler() {
-        // TODO Implement back button reponse using onpopstate
+        // TODO Implement back button response using onpopstate
         updateHistory({
           url: fromSearchToUrl(this.search),
           title: document.title,
@@ -174,7 +208,7 @@ export default {
       >
         <template #title="{ tab }">
           {{ tab.title }}
-          <gl-badge v-if="tabCount(tab)" class="gl-ml-1" size="sm">
+          <gl-badge v-if="typeof tabCount(tab) == 'number'" class="gl-ml-1" size="sm">
             {{ tabCount(tab) }}
           </gl-badge>
         </template>

app/assets/javascripts/runner/admin_runners/index.js

@@ -27,16 +27,7 @@ export const initAdminRunners = (selector = '#js-admin-runners') => {
 
   // TODO `activeRunnersCount` should be implemented using a GraphQL API
   // https://gitlab.com/gitlab-org/gitlab/-/issues/333806
-  const {
-    runnerInstallHelpPage,
-    registrationToken,
-
-    activeRunnersCount,
-    allRunnersCount,
-    instanceRunnersCount,
-    groupRunnersCount,
-    projectRunnersCount,
-  } = el.dataset;
+  const { runnerInstallHelpPage, registrationToken, activeRunnersCount } = el.dataset;
 
   const apolloProvider = new VueApollo({
     defaultClient: createDefaultClient(),
@@ -53,13 +44,9 @@ export const initAdminRunners = (selector = '#js-admin-runners') => {
         props: {
           registrationToken,
 
-          // All runner counts are returned as formatted
+          // Runner counts are returned as formatted
           // strings, we do not use `parseInt`.
           activeRunnersCount,
-          allRunnersCount,
-          instanceRunnersCount,
-          groupRunnersCount,
-          projectRunnersCount,
         },
       });
     },

app/assets/javascripts/runner/graphql/get_runners_count.query.graphql

+query getRunnersCount(
+  $status: CiRunnerStatus
+  $type: CiRunnerType
+  $tagList: [String!]
+  $search: String
+) {
+  runners(status: $status, type: $type, tagList: $tagList, search: $search) {
+    count
+  }
+}

app/helpers/ci/runners_helper.rb

@@ -67,12 +67,8 @@ module Ci
         runner_install_help_page: 'https://docs.gitlab.com/runner/install/',
         registration_token: Gitlab::CurrentSettings.runners_registration_token,
 
-        # All runner counts are returned as formatted strings
-        active_runners_count: Ci::Runner.online.count.to_s,
-        all_runners_count: limited_counter_with_delimiter(Ci::Runner),
-        instance_runners_count: limited_counter_with_delimiter(Ci::Runner.instance_type),
-        group_runners_count: limited_counter_with_delimiter(Ci::Runner.group_type),
-        project_runners_count: limited_counter_with_delimiter(Ci::Runner.project_type)
+        # Runner counts are returned as formatted strings
+        active_runners_count: Ci::Runner.online.count.to_s
       }
     end
 

spec/features/admin/admin_runners_spec.rb

@@ -131,6 +131,9 @@ RSpec.describe "Admin Runners" do
         it 'shows correct runner when description matches' do
           input_filtered_search_keys('runner-foo')
 
+          expect(page).to have_link('All 1')
+          expect(page).to have_link('Instance 1')
+
           expect(page).to have_content("runner-foo")
           expect(page).not_to have_content("runner-bar")
         end
@@ -138,73 +141,76 @@ RSpec.describe "Admin Runners" do
         it 'shows no runner when description does not match' do
           input_filtered_search_keys('runner-baz')
 
+          expect(page).to have_link('All 0')
+          expect(page).to have_link('Instance 0')
+
           expect(page).to have_text 'No runners found'
         end
       end
 
       describe 'filter by status' do
-        it 'shows correct runner when status matches' do
-          create(:ci_runner, :instance, description: 'runner-active', active: true)
-          create(:ci_runner, :instance, description: 'runner-paused', active: false)
+        let!(:never_contacted) { create(:ci_runner, :instance, description: 'runner-never-contacted', contacted_at: nil) }
+
+        before do
+          create(:ci_runner, :instance, description: 'runner-1', contacted_at: Time.now)
+          create(:ci_runner, :instance, description: 'runner-2', contacted_at: Time.now)
+          create(:ci_runner, :instance, description: 'runner-paused', active: false, contacted_at: Time.now)
 
           visit admin_runners_path
+        end
 
-          expect(page).to have_content 'runner-active'
+        it 'shows all runners' do
+          expect(page).to have_content 'runner-1'
+          expect(page).to have_content 'runner-2'
           expect(page).to have_content 'runner-paused'
+          expect(page).to have_content 'runner-never-contacted'
 
+          expect(page).to have_link('All 4')
+        end
+
+        it 'shows correct runner when status matches' do
           input_filtered_search_filter_is_only('Status', 'Active')
 
-          expect(page).to have_content 'runner-active'
+          expect(page).to have_link('All 3')
+
+          expect(page).to have_content 'runner-1'
+          expect(page).to have_content 'runner-2'
+          expect(page).to have_content 'runner-never-contacted'
           expect(page).not_to have_content 'runner-paused'
         end
 
         it 'shows no runner when status does not match' do
-          create(:ci_runner, :instance, description: 'runner-active', active: true)
-          create(:ci_runner, :instance, description: 'runner-paused', active: false)
-
-          visit admin_runners_path
+          input_filtered_search_filter_is_only('Status', 'Stale')
 
-          input_filtered_search_filter_is_only('Status', 'Online')
-
-          expect(page).not_to have_content 'runner-active'
-          expect(page).not_to have_content 'runner-paused'
+          expect(page).to have_link('All 0')
 
           expect(page).to have_text 'No runners found'
         end
 
         it 'shows correct runner when status is selected and search term is entered' do
-          create(:ci_runner, :instance, description: 'runner-a-1', active: true)
-          create(:ci_runner, :instance, description: 'runner-a-2', active: false)
-          create(:ci_runner, :instance, description: 'runner-b-1', active: true)
-
-          visit admin_runners_path
-
           input_filtered_search_filter_is_only('Status', 'Active')
+          input_filtered_search_keys('runner-1')
 
-          expect(page).to have_content 'runner-a-1'
-          expect(page).to have_content 'runner-b-1'
-          expect(page).not_to have_content 'runner-a-2'
-
-          input_filtered_search_keys('runner-a')
+          expect(page).to have_link('All 1')
 
-          expect(page).to have_content 'runner-a-1'
-          expect(page).not_to have_content 'runner-b-1'
-          expect(page).not_to have_content 'runner-a-2'
+          expect(page).to have_content 'runner-1'
+          expect(page).not_to have_content 'runner-2'
+          expect(page).not_to have_content 'runner-never-contacted'
+          expect(page).not_to have_content 'runner-paused'
         end
 
         it 'shows correct runner when status filter is entered' do
-          never_connected = create(:ci_runner, :instance, description: 'runner-never-contacted', contacted_at: nil)
-          create(:ci_runner, :instance, description: 'runner-contacted', contacted_at: Time.now)
-
-          visit admin_runners_path
-
           # use the string "Never" to avoid using space and trigger an early selection
           input_filtered_search_filter_is_only('Status', 'Never')
 
+          expect(page).to have_link('All 1')
+
+          expect(page).not_to have_content 'runner-1'
+          expect(page).not_to have_content 'runner-2'
+          expect(page).not_to have_content 'runner-paused'
           expect(page).to have_content 'runner-never-contacted'
-          expect(page).not_to have_content 'runner-contacted'
 
-          within "[data-testid='runner-row-#{never_connected.id}']" do
+          within "[data-testid='runner-row-#{never_contacted.id}']" do
             expect(page).to have_selector '.badge', text: 'never contacted'
           end
         end
@@ -219,6 +225,10 @@ RSpec.describe "Admin Runners" do
         it '"All" tab is selected by default' do
           visit admin_runners_path
 
+          expect(page).to have_link('All 2')
+          expect(page).to have_link('Group 1')
+          expect(page).to have_link('Project 1')
+
           page.within('[data-testid="runner-type-tabs"]') do
             expect(page).to have_link('All', class: 'active')
           end
@@ -380,6 +390,13 @@ RSpec.describe "Admin Runners" do
         expect(page).to have_text "Online Runners 0"
         expect(page).to have_text 'No runners found'
       end
+
+      it 'shows tabs with total counts equal to 0' do
+        expect(page).to have_link('All 0')
+        expect(page).to have_link('Instance 0')
+        expect(page).to have_link('Group 0')
+        expect(page).to have_link('Project 0')
+      end
     end
 
     context "when visiting outdated URLs" do
@@ -581,6 +598,8 @@ RSpec.describe "Admin Runners" do
       page.find('input').send_keys(search_term)
       click_on 'Search'
     end
+
+    wait_for_requests
   end
 
   def input_filtered_search_filter_is_only(filter, value)
@@ -597,5 +616,7 @@ RSpec.describe "Admin Runners" do
 
       click_on 'Search'
     end
+
+    wait_for_requests
   end
 end

spec/frontend/fixtures/runner.rb

@@ -49,6 +49,25 @@ RSpec.describe 'Runner (JavaScript fixtures)' do
     end
   end
 
+  describe GraphQL::Query, type: :request do
+    get_runners_count_query_name = 'get_runners_count.query.graphql'
+
+    before do
+      sign_in(admin)
+      enable_admin_mode!(admin)
+    end
+
+    let_it_be(:query) do
+      get_graphql_query_as_string("#{query_path}#{get_runners_count_query_name}")
+    end
+
+    it "#{fixtures_path}#{get_runners_count_query_name}.json" do
+      post_graphql(query, current_user: admin, variables: {})
+
+      expect_graphql_errors_to_be_empty
+    end
+  end
+
   describe GraphQL::Query, type: :request do
     get_runner_query_name = 'get_runner.query.graphql'
 

spec/frontend/runner/admin_runners/admin_runners_app_spec.js

@@ -22,23 +22,22 @@ import {
   CREATED_DESC,
   DEFAULT_SORT,
   INSTANCE_TYPE,
+  GROUP_TYPE,
+  PROJECT_TYPE,
   PARAM_KEY_STATUS,
   PARAM_KEY_TAG,
   STATUS_ACTIVE,
   RUNNER_PAGE_SIZE,
 } from '~/runner/constants';
 import getRunnersQuery from '~/runner/graphql/get_runners.query.graphql';
+import getRunnersCountQuery from '~/runner/graphql/get_runners_count.query.graphql';
 import { captureException } from '~/runner/sentry_utils';
 import FilteredSearch from '~/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue';
 
-import { runnersData, runnersDataPaginated } from '../mock_data';
+import { runnersData, runnersCountData, runnersDataPaginated } from '../mock_data';
 
 const mockRegistrationToken = 'MOCK_REGISTRATION_TOKEN';
 const mockActiveRunnersCount = '2';
-const mockAllRunnersCount = '6';
-const mockInstanceRunnersCount = '3';
-const mockGroupRunnersCount = '2';
-const mockProjectRunnersCount = '1';
 
 jest.mock('~/flash');
 jest.mock('~/runner/sentry_utils');
@@ -53,6 +52,7 @@ localVue.use(VueApollo);
 describe('AdminRunnersApp', () => {
   let wrapper;
   let mockRunnersQuery;
+  let mockRunnersCountQuery;
 
   const findRegistrationDropdown = () => wrapper.findComponent(RegistrationDropdown);
   const findRunnerTypeTabs = () => wrapper.findComponent(RunnerTypeTabs);
@@ -65,7 +65,10 @@ describe('AdminRunnersApp', () => {
   const findFilteredSearch = () => wrapper.findComponent(FilteredSearch);
 
   const createComponent = ({ props = {}, mountFn = shallowMount } = {}) => {
-    const handlers = [[getRunnersQuery, mockRunnersQuery]];
+    const handlers = [
+      [getRunnersQuery, mockRunnersQuery],
+      [getRunnersCountQuery, mockRunnersCountQuery],
+    ];
 
     wrapper = mountFn(AdminRunnersApp, {
       localVue,
@@ -73,10 +76,6 @@ describe('AdminRunnersApp', () => {
       propsData: {
         registrationToken: mockRegistrationToken,
         activeRunnersCount: mockActiveRunnersCount,
-        allRunnersCount: mockAllRunnersCount,
-        instanceRunnersCount: mockInstanceRunnersCount,
-        groupRunnersCount: mockGroupRunnersCount,
-        projectRunnersCount: mockProjectRunnersCount,
         ...props,
       },
     });
@@ -86,6 +85,19 @@ describe('AdminRunnersApp', () => {
     setWindowLocation('/admin/runners');
 
     mockRunnersQuery = jest.fn().mockResolvedValue(runnersData);
+    mockRunnersCountQuery = jest.fn().mockImplementation(({ type }) => {
+      const mockResponse = {
+        [INSTANCE_TYPE]: 3,
+        [GROUP_TYPE]: 2,
+        [PROJECT_TYPE]: 1,
+      };
+      if (mockResponse[type]) {
+        return Promise.resolve({
+          data: { runners: { count: mockResponse[type] } },
+        });
+      }
+      return Promise.resolve(runnersCountData);
+    });
     createComponent();
     await waitForPromises();
   });
@@ -101,7 +113,7 @@ describe('AdminRunnersApp', () => {
     await waitForPromises();
 
     expect(findRunnerTypeTabs().text()).toMatchInterpolatedText(
-      `All ${mockAllRunnersCount} Instance ${mockInstanceRunnersCount} Group ${mockGroupRunnersCount} Project ${mockProjectRunnersCount}`,
+      `All ${runnersCountData.data.runners.count} Instance 3 Group 2 Project 1`,
     );
   });
 

spec/frontend/runner/mock_data.js

@@ -2,6 +2,7 @@
 
 // Admin queries
 import runnersData from 'test_fixtures/graphql/runner/get_runners.query.graphql.json';
+import runnersCountData from 'test_fixtures/graphql/runner/get_runners_count.query.graphql.json';
 import runnersDataPaginated from 'test_fixtures/graphql/runner/get_runners.query.graphql.paginated.json';
 import runnerData from 'test_fixtures/graphql/runner/get_runner.query.graphql.json';
 
@@ -11,6 +12,7 @@ import groupRunnersDataPaginated from 'test_fixtures/graphql/runner/get_group_ru
 
 export {
   runnerData,
+  runnersCountData,
   runnersDataPaginated,
   runnersData,
   groupRunnersData,

spec/helpers/ci/runners_helper_spec.rb

@@ -80,11 +80,7 @@ RSpec.describe Ci::RunnersHelper do
       expect(helper.admin_runners_data_attributes).to eq({
         runner_install_help_page: 'https://docs.gitlab.com/runner/install/',
         registration_token: Gitlab::CurrentSettings.runners_registration_token,
-        active_runners_count: '0',
-        all_runners_count: '2',
-        instance_runners_count: '1',
-        group_runners_count: '0',
-        project_runners_count: '1'
+        active_runners_count: '0'
       })
     end
   end