Merge branch '336405-runner-group-ui-edit-link' into 'master'

Add runner edit button to group runners lists

See merge request gitlab-org/gitlab!80924

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

@@ -12,6 +12,7 @@ import RunnerName from '../components/runner_name.vue';
 import RunnerStats from '../components/stat/runner_stats.vue';
 import RunnerPagination from '../components/runner_pagination.vue';
 import RunnerTypeTabs from '../components/runner_type_tabs.vue';
+import RunnerActionsCell from '../components/cells/runner_actions_cell.vue';
 
 import { statusTokenConfig } from '../components/search_tokens/status_token_config';
 import { tagTokenConfig } from '../components/search_tokens/tag_token_config';
@@ -57,6 +58,7 @@ export default {
     RunnerStats,
     RunnerPagination,
     RunnerTypeTabs,
+    RunnerActionsCell,
   },
   props: {
     registrationToken: {
@@ -279,6 +281,9 @@ export default {
             <runner-name :runner="runner" />
           </gl-link>
         </template>
+        <template #runner-actions-cell="{ runner }">
+          <runner-actions-cell :runner="runner" :edit-url="runner.editAdminUrl" />
+        </template>
       </runner-list>
       <runner-pagination
         v-model="search.pagination"

app/assets/javascripts/runner/components/cells/runner_actions_cell.vue

@@ -17,6 +17,11 @@ export default {
       type: Object,
       required: true,
     },
+    editUrl: {
+      type: String,
+      default: null,
+      required: false,
+    },
   },
   computed: {
     canUpdate() {
@@ -31,14 +36,7 @@ export default {
 
 <template>
   <gl-button-group>
-    <!--
-      This button appears for administrators: those with
-      access to the adminUrl. More advanced permissions policies
-      will allow more granular permissions.
-
-      See https://gitlab.com/gitlab-org/gitlab/-/issues/334802
-    -->
-    <runner-edit-button v-if="canUpdate && runner.editAdminUrl" :href="runner.editAdminUrl" />
+    <runner-edit-button v-if="canUpdate && editUrl" :href="editUrl" />
     <runner-pause-button v-if="canUpdate" :runner="runner" :compact="true" />
     <runner-delete-button v-if="canDelete" :runner="runner" :compact="true" />
   </gl-button-group>

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

 <script>
 import { GlButton, GlTooltipDirective } from '@gitlab/ui';
-import { __ } from '~/locale';
-
-const I18N_EDIT = __('Edit');
+import { I18N_EDIT } from '../constants';
 
 export default {
   components: {

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

@@ -5,7 +5,6 @@ import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import { __, s__ } from '~/locale';
 import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
 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';
@@ -16,7 +15,6 @@ export default {
     GlSkeletonLoader,
     TooltipOnTruncate,
     TimeAgo,
-    RunnerActionsCell,
     RunnerSummaryCell,
     RunnerTags,
     RunnerStatusCell,
@@ -121,7 +119,7 @@ export default {
       </template>
 
       <template #cell(actions)="{ item }">
-        <runner-actions-cell :runner="item" />
+        <slot name="runner-actions-cell" :runner="item"></slot>
       </template>
     </gl-table-lite>
 

app/assets/javascripts/runner/constants.js

@@ -35,7 +35,8 @@ export const I18N_STALE_RUNNER_DESCRIPTION = s__(
   'Runners|No contact from this runner in over 3 months',
 );
 
-// Active flag
+// Actions
+export const I18N_EDIT = __('Edit');
 export const I18N_PAUSE = __('Pause');
 export const I18N_RESUME = __('Resume');
 export const I18N_DELETE_RUNNER = s__('Runners|Delete runner');

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

@@ -27,6 +27,7 @@ query getGroupRunners(
     ) {
       edges {
         webUrl
+        editUrl
         node {
           __typename
           ...RunnerNode

app/assets/javascripts/runner/group_runners/group_runners_app.vue

@@ -12,6 +12,7 @@ import RunnerName from '../components/runner_name.vue';
 import RunnerStats from '../components/stat/runner_stats.vue';
 import RunnerPagination from '../components/runner_pagination.vue';
 import RunnerTypeTabs from '../components/runner_type_tabs.vue';
+import RunnerActionsCell from '../components/cells/runner_actions_cell.vue';
 
 import { statusTokenConfig } from '../components/search_tokens/status_token_config';
 import {
@@ -55,6 +56,7 @@ export default {
     RunnerStats,
     RunnerPagination,
     RunnerTypeTabs,
+    RunnerActionsCell,
   },
   props: {
     registrationToken: {
@@ -74,8 +76,8 @@ export default {
     return {
       search: fromUrlQueryToSearch(),
       runners: {
-        webUrls: [],
         items: [],
+        urlsById: {},
         pageInfo: {},
       },
     };
@@ -91,12 +93,23 @@ export default {
         return this.variables;
       },
       update(data) {
-        const { runners } = data?.group || {};
+        const { edges = [], pageInfo = {} } = data?.group?.runners || {};
+
+        const items = [];
+        const urlsById = {};
+
+        edges.forEach(({ node, webUrl, editUrl }) => {
+          items.push(node);
+          urlsById[node.id] = {
+            web: webUrl,
+            edit: editUrl,
+          };
+        });
 
         return {
-          webUrls: runners?.edges.map(({ webUrl }) => webUrl) || [],
-          items: runners?.edges.map(({ node }) => node) || [],
-          pageInfo: runners?.pageInfo || {},
+          items,
+          urlsById,
+          pageInfo,
         };
       },
       error(error) {
@@ -222,6 +235,12 @@ export default {
       }
       return null;
     },
+    webUrl(runner) {
+      return this.runners.urlsById[runner.id]?.web;
+    },
+    editUrl(runner) {
+      return this.runners.urlsById[runner.id]?.edit;
+    },
     reportToSentry(error) {
       captureException({ error, component: this.$options.name });
     },
@@ -273,13 +292,20 @@ export default {
     </div>
     <template v-else>
       <runner-list :runners="runners.items" :loading="runnersLoading">
-        <template #runner-name="{ runner, index }">
-          <gl-link :href="runners.webUrls[index]">
+        <template #runner-name="{ runner }">
+          <gl-link :href="webUrl(runner)">
             <runner-name :runner="runner" />
           </gl-link>
         </template>
+        <template #runner-actions-cell="{ runner }">
+          <runner-actions-cell :runner="runner" :edit-url="editUrl(runner)" />
+        </template>
       </runner-list>
-      <runner-pagination v-model="search.pagination" :page-info="runners.pageInfo" />
+      <runner-pagination
+        v-model="search.pagination"
+        class="gl-mt-3"
+        :page-info="runners.pageInfo"
+      />
     </template>
   </div>
 </template>

spec/frontend/runner/admin_runners/admin_runners_app_spec.js

@@ -19,6 +19,7 @@ import RunnerFilteredSearchBar from '~/runner/components/runner_filtered_search_
 import RunnerList from '~/runner/components/runner_list.vue';
 import RunnerStats from '~/runner/components/stat/runner_stats.vue';
 import RegistrationDropdown from '~/runner/components/registration/registration_dropdown.vue';
+import RunnerActionsCell from '~/runner/components/cells/runner_actions_cell.vue';
 import RunnerPagination from '~/runner/components/runner_pagination.vue';
 
 import {
@@ -188,6 +189,21 @@ describe('AdminRunnersApp', () => {
     expect(runnerLink.attributes('href')).toBe(`http://localhost/admin/runners/${numericId}`);
   });
 
+  it('renders runner actions for each runner', async () => {
+    createComponent({ mountFn: mountExtended });
+
+    await waitForPromises();
+
+    const runnerActions = wrapper.find('tr [data-testid="td-actions"]').find(RunnerActionsCell);
+
+    const runner = runnersData.data.runners.nodes[0];
+
+    expect(runnerActions.props()).toEqual({
+      runner,
+      editUrl: runner.editAdminUrl,
+    });
+  });
+
   it('requests the runners with no filters', () => {
     expect(mockRunnersQuery).toHaveBeenLastCalledWith({
       status: undefined,

spec/frontend/runner/components/cells/runner_actions_cell_spec.js

@@ -15,9 +15,10 @@ describe('RunnerActionsCell', () => {
   const findRunnerPauseBtn = () => wrapper.findComponent(RunnerPauseButton);
   const findDeleteBtn = () => wrapper.findComponent(RunnerDeleteButton);
 
-  const createComponent = (runner = {}, options) => {
+  const createComponent = ({ runner = {}, ...props } = {}) => {
     wrapper = shallowMountExtended(RunnerActionsCell, {
       propsData: {
+        editUrl: mockRunner.editAdminUrl,
         runner: {
           id: mockRunner.id,
           shortSha: mockRunner.shortSha,
@@ -25,8 +26,8 @@ describe('RunnerActionsCell', () => {
           userPermissions: mockRunner.userPermissions,
           ...runner,
         },
+        ...props,
       },
-      ...options,
     });
   };
 
@@ -43,18 +44,20 @@ describe('RunnerActionsCell', () => {
 
     it('Does not render the runner edit link when user cannot update', () => {
       createComponent({
-        userPermissions: {
-          ...mockRunner.userPermissions,
-          updateRunner: false,
+        runner: {
+          userPermissions: {
+            ...mockRunner.userPermissions,
+            updateRunner: false,
+          },
         },
       });
 
       expect(findEditBtn().exists()).toBe(false);
     });
 
-    it('Does not render the runner edit link when editAdminUrl is not provided', () => {
+    it('Does not render the runner edit link when editUrl is not provided', () => {
       createComponent({
-        editAdminUrl: null,
+        editUrl: null,
       });
 
       expect(findEditBtn().exists()).toBe(false);
@@ -70,9 +73,11 @@ describe('RunnerActionsCell', () => {
 
     it('Does not render the runner pause button when user cannot update', () => {
       createComponent({
-        userPermissions: {
-          ...mockRunner.userPermissions,
-          updateRunner: false,
+        runner: {
+          userPermissions: {
+            ...mockRunner.userPermissions,
+            updateRunner: false,
+          },
         },
       });
 
@@ -89,9 +94,11 @@ describe('RunnerActionsCell', () => {
 
     it('Does not render the runner delete button when user cannot delete', () => {
       createComponent({
-        userPermissions: {
-          ...mockRunner.userPermissions,
-          deleteRunner: false,
+        runner: {
+          userPermissions: {
+            ...mockRunner.userPermissions,
+            deleteRunner: false,
+          },
         },
       });
 

spec/frontend/runner/components/runner_list_spec.js

@@ -6,9 +6,6 @@ import {
 } from 'helpers/vue_test_utils_helper';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import RunnerList from '~/runner/components/runner_list.vue';
-import RunnerEditButton from '~/runner/components/runner_edit_button.vue';
-import RunnerPauseButton from '~/runner/components/runner_pause_button.vue';
-import RunnerDeleteButton from '~/runner/components/runner_delete_button.vue';
 import { runnersData } from '../mock_data';
 
 const mockRunners = runnersData.data.runners.nodes;
@@ -24,13 +21,14 @@ describe('RunnerList', () => {
   const findCell = ({ row = 0, fieldKey }) =>
     extendedWrapper(findRows().at(row).find(`[data-testid="td-${fieldKey}"]`));
 
-  const createComponent = ({ props = {} } = {}, mountFn = shallowMountExtended) => {
+  const createComponent = ({ props = {}, ...options } = {}, mountFn = shallowMountExtended) => {
     wrapper = mountFn(RunnerList, {
       propsData: {
         runners: mockRunners,
         activeRunnersCount: mockActiveRunnersCount,
         ...props,
       },
+      ...options,
     });
   };
 
@@ -91,11 +89,31 @@ describe('RunnerList', () => {
     expect(findCell({ fieldKey: 'contactedAt' }).text()).toEqual(expect.any(String));
 
     // Actions
-    const actions = findCell({ fieldKey: 'actions' });
+    expect(findCell({ fieldKey: 'actions' }).exists()).toBe(true);
+  });
+
+  describe('Scoped cell slots', () => {
+    it('Render #runner-name slot in "summary" cell', () => {
+      createComponent(
+        {
+          scopedSlots: { 'runner-name': ({ runner }) => `Summary: ${runner.id}` },
+        },
+        mountExtended,
+      );
+
+      expect(findCell({ fieldKey: 'summary' }).text()).toContain(`Summary: ${mockRunners[0].id}`);
+    });
 
-    expect(actions.findComponent(RunnerEditButton).exists()).toBe(true);
-    expect(actions.findComponent(RunnerPauseButton).exists()).toBe(true);
-    expect(actions.findComponent(RunnerDeleteButton).exists()).toBe(true);
+    it('Render #runner-actions-cell slot in "actions" cell', () => {
+      createComponent(
+        {
+          scopedSlots: { 'runner-actions-cell': ({ runner }) => `Actions: ${runner.id}` },
+        },
+        mountExtended,
+      );
+
+      expect(findCell({ fieldKey: 'actions' }).text()).toBe(`Actions: ${mockRunners[0].id}`);
+    });
   });
 
   describe('Table data formatting', () => {

spec/frontend/runner/group_runners/group_runners_app_spec.js

 import Vue, { nextTick } from 'vue';
-import { GlLink } from '@gitlab/ui';
+import { GlButton, GlLink } from '@gitlab/ui';
 import VueApollo from 'vue-apollo';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import setWindowLocation from 'helpers/set_window_location_helper';
@@ -30,6 +30,7 @@ import {
   PARAM_KEY_STATUS,
   STATUS_ACTIVE,
   RUNNER_PAGE_SIZE,
+  I18N_EDIT,
 } from '~/runner/constants';
 import getGroupRunnersQuery from '~/runner/graphql/get_group_runners.query.graphql';
 import getGroupRunnersCountQuery from '~/runner/graphql/get_group_runners_count.query.graphql';
@@ -42,7 +43,8 @@ Vue.use(VueApollo);
 
 const mockGroupFullPath = 'group1';
 const mockRegistrationToken = 'AABBCC';
-const mockGroupRunnersLimitedCount = groupRunnersData.data.group.runners.edges.length;
+const mockGroupRunnersEdges = groupRunnersData.data.group.runners.edges;
+const mockGroupRunnersLimitedCount = mockGroupRunnersEdges.length;
 
 jest.mock('~/flash');
 jest.mock('~/runner/sentry_utils');
@@ -60,6 +62,7 @@ describe('GroupRunnersApp', () => {
   const findRegistrationDropdown = () => wrapper.findComponent(RegistrationDropdown);
   const findRunnerTypeTabs = () => wrapper.findComponent(RunnerTypeTabs);
   const findRunnerList = () => wrapper.findComponent(RunnerList);
+  const findRunnerRow = (id) => extendedWrapper(wrapper.findByTestId(`runner-row-${id}`));
   const findRunnerPagination = () => extendedWrapper(wrapper.findComponent(RunnerPagination));
   const findRunnerPaginationPrev = () =>
     findRunnerPagination().findByLabelText('Go to previous page');
@@ -156,20 +159,7 @@ describe('GroupRunnersApp', () => {
 
   it('shows the runners list', () => {
     const runners = findRunnerList().props('runners');
-    expect(runners).toEqual(groupRunnersData.data.group.runners.edges.map(({ node }) => node));
-  });
-
-  it('runner item links to the runner group page', async () => {
-    const { webUrl, node } = groupRunnersData.data.group.runners.edges[0];
-    const { id, shortSha } = node;
-
-    createComponent({ mountFn: mountExtended });
-
-    await waitForPromises();
-
-    const runnerLink = wrapper.find('tr [data-testid="td-summary"]').find(GlLink);
-    expect(runnerLink.text()).toBe(`#${getIdFromGraphQLId(id)} (${shortSha})`);
-    expect(runnerLink.attributes('href')).toBe(webUrl);
+    expect(runners).toEqual(mockGroupRunnersEdges.map(({ node }) => node));
   });
 
   it('requests the runners with group path and no other filters', () => {
@@ -196,6 +186,34 @@ describe('GroupRunnersApp', () => {
     );
   });
 
+  describe('Single runner row', () => {
+    const { webUrl, editUrl, node } = mockGroupRunnersEdges[0];
+    const { id: graphqlId, shortSha } = node;
+    const id = getIdFromGraphQLId(graphqlId);
+
+    beforeEach(async () => {
+      createComponent({ mountFn: mountExtended });
+
+      await waitForPromises();
+    });
+
+    it('view link is displayed correctly', () => {
+      const viewLink = findRunnerRow(id).findByTestId('td-summary').findComponent(GlLink);
+
+      expect(viewLink.text()).toBe(`#${id} (${shortSha})`);
+      expect(viewLink.attributes('href')).toBe(webUrl);
+    });
+
+    it('edit link is displayed correctly', () => {
+      const editLink = findRunnerRow(id).findByTestId('td-actions').findComponent(GlButton);
+
+      expect(editLink.attributes()).toMatchObject({
+        'aria-label': I18N_EDIT,
+        href: editUrl,
+      });
+    });
+  });
+
   describe('when a filter is preselected', () => {
     beforeEach(async () => {
       setWindowLocation(`?status[]=${STATUS_ACTIVE}&runner_type[]=${INSTANCE_TYPE}`);