Merge branch '347856-runner-utils-refactor' into 'master'

Refactor runner utils functions

See merge request gitlab-org/gitlab!80592

app/assets/javascripts/runner/components/runner_list.vue

@@ -2,31 +2,14 @@
 import { GlTable, GlTooltipDirective, GlSkeletonLoader } from '@gitlab/ui';
 import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
-import { formatNumber, __, s__ } from '~/locale';
+import { __, s__ } from '~/locale';
 import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
-import { RUNNER_JOB_COUNT_LIMIT } from '../constants';
+import { formatJobCount, tableField } from '../utils';
 import RunnerActionsCell from './cells/runner_actions_cell.vue';
 import RunnerSummaryCell from './cells/runner_summary_cell.vue';
 import RunnerStatusCell from './cells/runner_status_cell.vue';
 import RunnerTags from './runner_tags.vue';
 
-const tableField = ({ key, label = '', thClasses = [] }) => {
-  return {
-    key,
-    label,
-    thClass: [
-      'gl-bg-transparent!',
-      'gl-border-b-solid!',
-      'gl-border-b-gray-100!',
-      'gl-border-b-1!',
-      ...thClasses,
-    ],
-    tdAttr: {
-      'data-testid': `td-${key}`,
-    },
-  };
-};
-
 export default {
   components: {
     GlTable,
@@ -54,10 +37,7 @@ export default {
   },
   methods: {
     formatJobCount(jobCount) {
-      if (jobCount > RUNNER_JOB_COUNT_LIMIT) {
-        return `${formatNumber(RUNNER_JOB_COUNT_LIMIT)}+`;
-      }
-      return formatNumber(jobCount);
+      return formatJobCount(jobCount);
     },
     runnerTrAttr(runner) {
       if (runner) {

app/assets/javascripts/runner/components/runner_projects.vue

@@ -9,6 +9,7 @@ import {
   I18N_FETCH_ERROR,
   RUNNER_DETAILS_PROJECTS_PAGE_SIZE,
 } from '../constants';
+import { getPaginationVariables } from '../utils';
 import { captureException } from '../sentry_utils';
 import RunnerAssignedItem from './runner_assigned_item.vue';
 import RunnerPagination from './runner_pagination.vue';
@@ -62,19 +63,9 @@ export default {
   computed: {
     variables() {
       const { id } = this.runner;
-      const { before, after } = this.pagination;
-
-      if (before) {
-        return {
-          id,
-          before,
-          last: RUNNER_DETAILS_PROJECTS_PAGE_SIZE,
-        };
-      }
       return {
         id,
-        after,
-        first: RUNNER_DETAILS_PROJECTS_PAGE_SIZE,
+        ...getPaginationVariables(this.pagination, RUNNER_DETAILS_PROJECTS_PAGE_SIZE),
       };
     },
     loading() {

app/assets/javascripts/runner/runner_search_utils.js

@@ -18,6 +18,7 @@ import {
   RUNNER_PAGE_SIZE,
   STATUS_NEVER_CONTACTED,
 } from './constants';
+import { getPaginationVariables } from './utils';
 
 /**
  * The filters and sorting of the runners are built around
@@ -184,30 +185,27 @@ export const fromSearchToVariables = ({
   sort = null,
   pagination = {},
 } = {}) => {
-  const variables = {};
+  const filterVariables = {};
 
   const queryObj = filterToQueryObject(processFilters(filters), {
     filteredSearchTermKey: PARAM_KEY_SEARCH,
   });
 
-  [variables.status] = queryObj[PARAM_KEY_STATUS] || [];
-  variables.search = queryObj[PARAM_KEY_SEARCH];
-  variables.tagList = queryObj[PARAM_KEY_TAG];
+  [filterVariables.status] = queryObj[PARAM_KEY_STATUS] || [];
+  filterVariables.search = queryObj[PARAM_KEY_SEARCH];
+  filterVariables.tagList = queryObj[PARAM_KEY_TAG];
 
   if (runnerType) {
-    variables.type = runnerType;
+    filterVariables.type = runnerType;
   }
   if (sort) {
-    variables.sort = sort;
+    filterVariables.sort = sort;
   }
 
-  if (pagination.before) {
-    variables.before = pagination.before;
-    variables.last = RUNNER_PAGE_SIZE;
-  } else {
-    variables.after = pagination.after;
-    variables.first = RUNNER_PAGE_SIZE;
-  }
+  const paginationVariables = getPaginationVariables(pagination, RUNNER_PAGE_SIZE);
 
-  return variables;
+  return {
+    ...filterVariables,
+    ...paginationVariables,
+  };
 };

app/assets/javascripts/runner/utils.js

+import { formatNumber } from '~/locale';
+import { DEFAULT_TH_CLASSES } from '~/lib/utils/constants';
+import { RUNNER_JOB_COUNT_LIMIT } from './constants';
+
+/**
+ * Formats a job count, limited to a max number
+ *
+ * @param {Number} jobCount
+ * @returns Formatted string
+ */
+export const formatJobCount = (jobCount) => {
+  if (typeof jobCount !== 'number') {
+    return '';
+  }
+  if (jobCount > RUNNER_JOB_COUNT_LIMIT) {
+    return `${formatNumber(RUNNER_JOB_COUNT_LIMIT)}+`;
+  }
+  return formatNumber(jobCount);
+};
+
+/**
+ * Returns a GlTable fields with a given key and label
+ *
+ * @param {Object} options
+ * @returns Field object to add to GlTable fields
+ */
+export const tableField = ({ key, label = '', thClasses = [] }) => {
+  return {
+    key,
+    label,
+    thClass: [DEFAULT_TH_CLASSES, ...thClasses],
+    tdAttr: {
+      'data-testid': `td-${key}`,
+    },
+  };
+};
+
+/**
+ * Returns variables for a GraphQL query that uses keyset
+ * pagination.
+ *
+ * https://docs.gitlab.com/ee/development/graphql_guide/pagination.html#keyset-pagination
+ *
+ * @param {Object} pagination - Contains before, after, page
+ * @param {Number} pageSize
+ * @returns Variables
+ */
+export const getPaginationVariables = (pagination, pageSize = 10) => {
+  const { before, after } = pagination;
+
+  // first + after: Next page
+  // Get the first N items after item X
+  if (after) {
+    return {
+      after,
+      first: pageSize,
+    };
+  }
+
+  // last + before: Prev page
+  // Get the first N items before item X, when you click on Prev
+  if (before) {
+    return {
+      before,
+      last: pageSize,
+    };
+  }
+
+  // first page
+  // Get the first N items
+  return { first: pageSize };
+};

spec/frontend/runner/admin_runner_show/admin_runner_show_app_spec.js

@@ -7,7 +7,6 @@ import { createAlert } from '~/flash';
 
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import RunnerHeader from '~/runner/components/runner_header.vue';
-import RunnerDetails from '~/runner/components/runner_details.vue';
 import RunnerPauseButton from '~/runner/components/runner_pause_button.vue';
 import RunnerEditButton from '~/runner/components/runner_edit_button.vue';
 import getRunnerQuery from '~/runner/graphql/get_runner.query.graphql';
@@ -30,7 +29,6 @@ describe('AdminRunnerShowApp', () => {
   let mockRunnerQuery;
 
   const findRunnerHeader = () => wrapper.findComponent(RunnerHeader);
-  const findRunnerDetails = () => wrapper.findComponent(RunnerDetails);
   const findRunnerEditButton = () => wrapper.findComponent(RunnerEditButton);
   const findRunnerPauseButton = () => wrapper.findComponent(RunnerPauseButton);
 
@@ -80,8 +78,7 @@ describe('AdminRunnerShowApp', () => {
     });
 
     it('shows basic runner details', async () => {
-      const expected = `Details
-                        Description Instance runner
+      const expected = `Description Instance runner
                         Last contact Never contacted
                         Version 1.0.0
                         IP Address 127.0.0.1
@@ -89,7 +86,7 @@ describe('AdminRunnerShowApp', () => {
                         Maximum job timeout None
                         Tags None`.replace(/\s+/g, ' ');
 
-      expect(findRunnerDetails().text()).toMatchInterpolatedText(expected);
+      expect(wrapper.text().replace(/\s+/g, ' ')).toContain(expected);
     });
 
     describe('when runner cannot be updated', () => {

spec/frontend/runner/components/runner_details_spec.js

 import { GlSprintf, GlIntersperse } from '@gitlab/ui';
 import { createWrapper, ErrorWrapper } from '@vue/test-utils';
-import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper';
+import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
 import { useFakeDate } from 'helpers/fake_date';
 import { ACCESS_LEVEL_REF_PROTECTED, ACCESS_LEVEL_NOT_PROTECTED } from '~/runner/constants';
@@ -8,6 +8,8 @@ import { ACCESS_LEVEL_REF_PROTECTED, ACCESS_LEVEL_NOT_PROTECTED } from '~/runner
 import RunnerDetails from '~/runner/components/runner_details.vue';
 import RunnerDetail from '~/runner/components/runner_detail.vue';
 import RunnerGroups from '~/runner/components/runner_groups.vue';
+import RunnerTags from '~/runner/components/runner_tags.vue';
+import RunnerTag from '~/runner/components/runner_tag.vue';
 
 import { runnerData, runnerWithGroupData } from '../mock_data';
 
@@ -37,16 +39,14 @@ describe('RunnerDetails', () => {
 
   const findDetailGroups = () => wrapper.findComponent(RunnerGroups);
 
-  const createComponent = ({ props = {}, mountFn = shallowMountExtended } = {}) => {
+  const createComponent = ({ props = {}, mountFn = shallowMountExtended, stubs } = {}) => {
     wrapper = mountFn(RunnerDetails, {
       propsData: {
         ...props,
       },
       stubs: {
-        GlIntersperse,
-        GlSprintf,
-        TimeAgo,
         RunnerDetail,
+        ...stubs,
       },
     });
   };
@@ -65,76 +65,85 @@ describe('RunnerDetails', () => {
     expect(wrapper.text()).toBe('');
   });
 
-  describe.each`
-    field                    | runner                                                             | expectedValue
-    ${'Description'}         | ${{ description: 'My runner' }}                                    | ${'My runner'}
-    ${'Description'}         | ${{ description: null }}                                           | ${'None'}
-    ${'Last contact'}        | ${{ contactedAt: mockOneHourAgo }}                                 | ${'1 hour ago'}
-    ${'Last contact'}        | ${{ contactedAt: null }}                                           | ${'Never contacted'}
-    ${'Version'}             | ${{ version: '12.3' }}                                             | ${'12.3'}
-    ${'Version'}             | ${{ version: null }}                                               | ${'None'}
-    ${'IP Address'}          | ${{ ipAddress: '127.0.0.1' }}                                      | ${'127.0.0.1'}
-    ${'IP Address'}          | ${{ ipAddress: null }}                                             | ${'None'}
-    ${'Configuration'}       | ${{ accessLevel: ACCESS_LEVEL_REF_PROTECTED, runUntagged: true }}  | ${'Protected, Runs untagged jobs'}
-    ${'Configuration'}       | ${{ accessLevel: ACCESS_LEVEL_REF_PROTECTED, runUntagged: false }} | ${'Protected'}
-    ${'Configuration'}       | ${{ accessLevel: ACCESS_LEVEL_NOT_PROTECTED, runUntagged: true }}  | ${'Runs untagged jobs'}
-    ${'Configuration'}       | ${{ accessLevel: ACCESS_LEVEL_NOT_PROTECTED, runUntagged: false }} | ${'None'}
-    ${'Maximum job timeout'} | ${{ maximumTimeout: null }}                                        | ${'None'}
-    ${'Maximum job timeout'} | ${{ maximumTimeout: 0 }}                                           | ${'0 seconds'}
-    ${'Maximum job timeout'} | ${{ maximumTimeout: 59 }}                                          | ${'59 seconds'}
-    ${'Maximum job timeout'} | ${{ maximumTimeout: 10 * 60 + 5 }}                                 | ${'10 minutes 5 seconds'}
-  `('"$field" field', ({ field, runner, expectedValue }) => {
-    beforeEach(() => {
-      createComponent({
-        props: {
-          runner: {
-            ...mockRunner,
-            ...runner,
+  describe('Details tab', () => {
+    describe.each`
+      field                    | runner                                                             | expectedValue
+      ${'Description'}         | ${{ description: 'My runner' }}                                    | ${'My runner'}
+      ${'Description'}         | ${{ description: null }}                                           | ${'None'}
+      ${'Last contact'}        | ${{ contactedAt: mockOneHourAgo }}                                 | ${'1 hour ago'}
+      ${'Last contact'}        | ${{ contactedAt: null }}                                           | ${'Never contacted'}
+      ${'Version'}             | ${{ version: '12.3' }}                                             | ${'12.3'}
+      ${'Version'}             | ${{ version: null }}                                               | ${'None'}
+      ${'IP Address'}          | ${{ ipAddress: '127.0.0.1' }}                                      | ${'127.0.0.1'}
+      ${'IP Address'}          | ${{ ipAddress: null }}                                             | ${'None'}
+      ${'Configuration'}       | ${{ accessLevel: ACCESS_LEVEL_REF_PROTECTED, runUntagged: true }}  | ${'Protected, Runs untagged jobs'}
+      ${'Configuration'}       | ${{ accessLevel: ACCESS_LEVEL_REF_PROTECTED, runUntagged: false }} | ${'Protected'}
+      ${'Configuration'}       | ${{ accessLevel: ACCESS_LEVEL_NOT_PROTECTED, runUntagged: true }}  | ${'Runs untagged jobs'}
+      ${'Configuration'}       | ${{ accessLevel: ACCESS_LEVEL_NOT_PROTECTED, runUntagged: false }} | ${'None'}
+      ${'Maximum job timeout'} | ${{ maximumTimeout: null }}                                        | ${'None'}
+      ${'Maximum job timeout'} | ${{ maximumTimeout: 0 }}                                           | ${'0 seconds'}
+      ${'Maximum job timeout'} | ${{ maximumTimeout: 59 }}                                          | ${'59 seconds'}
+      ${'Maximum job timeout'} | ${{ maximumTimeout: 10 * 60 + 5 }}                                 | ${'10 minutes 5 seconds'}
+    `('"$field" field', ({ field, runner, expectedValue }) => {
+      beforeEach(() => {
+        createComponent({
+          props: {
+            runner: {
+              ...mockRunner,
+              ...runner,
+            },
           },
-        },
+          stubs: {
+            GlIntersperse,
+            GlSprintf,
+            TimeAgo,
+          },
+        });
       });
-    });
 
-    it(`displays expected value "${expectedValue}"`, () => {
-      expect(findDd(field).text()).toBe(expectedValue);
+      it(`displays expected value "${expectedValue}"`, () => {
+        expect(findDd(field).text()).toBe(expectedValue);
+      });
     });
-  });
 
-  describe('"Tags" field', () => {
-    it('displays expected value "tag-1 tag-2"', () => {
-      createComponent({
-        props: {
-          runner: { ...mockRunner, tagList: ['tag-1', 'tag-2'] },
-        },
-        mountFn: mountExtended,
-      });
+    describe('"Tags" field', () => {
+      const stubs = { RunnerTags, RunnerTag };
 
-      expect(findDd('Tags').text().replace(/\s+/g, ' ')).toBe('tag-1 tag-2');
-    });
+      it('displays expected value "tag-1 tag-2"', () => {
+        createComponent({
+          props: {
+            runner: { ...mockRunner, tagList: ['tag-1', 'tag-2'] },
+          },
+          stubs,
+        });
 
-    it('displays "None" when runner has no tags', () => {
-      createComponent({
-        props: {
-          runner: { ...mockRunner, tagList: [] },
-        },
-        mountFn: mountExtended,
+        expect(findDd('Tags').text().replace(/\s+/g, ' ')).toBe('tag-1 tag-2');
       });
 
-      expect(findDd('Tags').text().replace(/\s+/g, ' ')).toBe('None');
-    });
-  });
+      it('displays "None" when runner has no tags', () => {
+        createComponent({
+          props: {
+            runner: { ...mockRunner, tagList: [] },
+          },
+          stubs,
+        });
 
-  describe('Group runners', () => {
-    beforeEach(() => {
-      createComponent({
-        props: {
-          runner: mockGroupRunner,
-        },
+        expect(findDd('Tags').text().replace(/\s+/g, ' ')).toBe('None');
       });
     });
 
-    it('Shows a group runner details', () => {
-      expect(findDetailGroups().props('runner')).toEqual(mockGroupRunner);
+    describe('Group runners', () => {
+      beforeEach(() => {
+        createComponent({
+          props: {
+            runner: mockGroupRunner,
+          },
+        });
+      });
+
+      it('Shows a group runner details', () => {
+        expect(findDetailGroups().props('runner')).toEqual(mockGroupRunner);
+      });
     });
   });
 });

spec/frontend/runner/utils_spec.js

+import { formatJobCount, tableField, getPaginationVariables } from '~/runner/utils';
+
+describe('~/runner/utils', () => {
+  describe('formatJobCount', () => {
+    it('formats a number', () => {
+      expect(formatJobCount(1)).toBe('1');
+      expect(formatJobCount(99)).toBe('99');
+    });
+
+    it('formats a large count', () => {
+      expect(formatJobCount(1000)).toBe('1,000');
+      expect(formatJobCount(1001)).toBe('1,000+');
+    });
+
+    it('returns an empty string for non-numeric values', () => {
+      expect(formatJobCount(undefined)).toBe('');
+      expect(formatJobCount(null)).toBe('');
+      expect(formatJobCount('number')).toBe('');
+    });
+  });
+
+  describe('tableField', () => {
+    it('a field with options', () => {
+      expect(tableField({ key: 'name' })).toEqual({
+        key: 'name',
+        label: '',
+        tdAttr: { 'data-testid': 'td-name' },
+        thClass: expect.any(Array),
+      });
+    });
+
+    it('a field with a label', () => {
+      const label = 'A field name';
+
+      expect(tableField({ key: 'name', label })).toMatchObject({
+        label,
+      });
+    });
+
+    it('a field with custom classes', () => {
+      const mockClasses = ['foo', 'bar'];
+
+      expect(tableField({ thClasses: mockClasses })).toMatchObject({
+        thClass: expect.arrayContaining(mockClasses),
+      });
+    });
+  });
+
+  describe('getPaginationVariables', () => {
+    const after = 'AFTER_CURSOR';
+    const before = 'BEFORE_CURSOR';
+
+    it.each`
+      case                         | pagination    | pageSize     | variables
+      ${'next page'}               | ${{ after }}  | ${undefined} | ${{ after, first: 10 }}
+      ${'prev page'}               | ${{ before }} | ${undefined} | ${{ before, last: 10 }}
+      ${'first page'}              | ${{}}         | ${undefined} | ${{ first: 10 }}
+      ${'next page with N items'}  | ${{ after }}  | ${20}        | ${{ after, first: 20 }}
+      ${'prev page with N items'}  | ${{ before }} | ${20}        | ${{ before, last: 20 }}
+      ${'first page with N items'} | ${{}}         | ${20}        | ${{ first: 20 }}
+    `('navigates to $case', ({ pagination, pageSize, variables }) => {
+      expect(getPaginationVariables(pagination, pageSize)).toEqual(variables);
+    });
+  });
+});