Merge branch '354004-add-users-notice-deletepermission' into 'master'

Explain delete permission on multi-project runners

See merge request gitlab-org/gitlab!82805

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

@@ -44,6 +44,11 @@ export default {
   <gl-button-group>
     <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" @deleted="onDeleted" />
+    <runner-delete-button
+      :disabled="!canDelete"
+      :runner="runner"
+      :compact="true"
+      @deleted="onDeleted"
+    />
   </gl-button-group>
 </template>

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

@@ -5,7 +5,12 @@ import { createAlert } from '~/flash';
 import { sprintf } from '~/locale';
 import { captureException } from '~/runner/sentry_utils';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
-import { I18N_DELETE_RUNNER, I18N_DELETED_TOAST } from '../constants';
+import {
+  I18N_DELETE_DISABLED_MANY_PROJECTS,
+  I18N_DELETE_DISABLED_UNKNOWN_REASON,
+  I18N_DELETE_RUNNER,
+  I18N_DELETED_TOAST,
+} from '../constants';
 import RunnerDeleteModal from './runner_delete_modal.vue';
 
 export default {
@@ -26,6 +31,11 @@ export default {
         return runner?.id && runner?.shortSha;
       },
     },
+    disabled: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
     compact: {
       type: Boolean,
       required: false,
@@ -75,7 +85,14 @@ export default {
       return null;
     },
     tooltip() {
-      // Only show tooltip when compact.
+      if (this.disabled && this.runner.projectCount > 1) {
+        return I18N_DELETE_DISABLED_MANY_PROJECTS;
+      }
+      if (this.disabled) {
+        return I18N_DELETE_DISABLED_UNKNOWN_REASON;
+      }
+
+      // Only show basic "delete" tooltip when compact.
       // Also prevent a "sticky" tooltip: If this button is
       // disabled, mouseout listeners don't run leaving the tooltip stuck
       if (this.compact && !this.deleting) {
@@ -83,6 +100,14 @@ export default {
       }
       return '';
     },
+    wrapperTabindex() {
+      if (this.disabled) {
+        // Trigger tooltip on keyboard-focusable wrapper
+        // See https://bootstrap-vue.org/docs/directives/tooltip
+        return '0';
+      }
+      return null;
+    },
   },
   methods: {
     async onDelete() {
@@ -125,20 +150,22 @@ export default {
 </script>
 
 <template>
+  <div v-gl-tooltip="tooltip" class="btn-group" :tabindex="wrapperTabindex">
     <gl-button
-    v-gl-tooltip.hover.viewport="tooltip"
       v-gl-modal="runnerDeleteModalId"
       :aria-label="ariaLabel"
       :icon="icon"
       :class="buttonClass"
       :loading="deleting"
+      :disabled="disabled"
       variant="danger"
     >
       {{ buttonContent }}
+    </gl-button>
     <runner-delete-modal
       :modal-id="runnerDeleteModalId"
       :runner-name="runnerName"
       @primary="onDelete"
     />
-  </gl-button>
+  </div>
 </template>

app/assets/javascripts/runner/constants.js

@@ -46,6 +46,12 @@ export const I18N_RESUME = __('Resume');
 export const I18N_RESUME_TOOLTIP = s__('Runners|Resume accepting jobs');
 
 export const I18N_DELETE_RUNNER = s__('Runners|Delete runner');
+export const I18N_DELETE_DISABLED_MANY_PROJECTS = s__(
+  'Runners|Multi-project runners cannot be deleted',
+);
+export const I18N_DELETE_DISABLED_UNKNOWN_REASON = s__(
+  'Runners|Runner cannot be deleted, please contact your administrator',
+);
 export const I18N_DELETED_TOAST = s__('Runners|Runner %{name} was deleted');
 
 export const I18N_LOCKED_RUNNER_DESCRIPTION = s__('Runners|You cannot assign to other projects');

app/assets/javascripts/runner/graphql/list/group_runners.query.graphql

@@ -30,6 +30,7 @@ query getGroupRunners(
         editUrl
         node {
           ...ListItem
+          projectCount # Used to determine why some project runners can't be deleted
         }
       }
       pageInfo {

locale/gitlab.pot

@@ -32054,6 +32054,9 @@ msgstr ""
 msgid "Runners|Members of the %{type} can register runners"
 msgstr ""
 
+msgid "Runners|Multi-project runners cannot be deleted"
+msgstr ""
+
 msgid "Runners|Name"
 msgstr ""
 
@@ -32156,6 +32159,9 @@ msgstr ""
 msgid "Runners|Runner assigned to project."
 msgstr ""
 
+msgid "Runners|Runner cannot be deleted, please contact your administrator"
+msgstr ""
+
 msgid "Runners|Runner is offline, last contact was %{runner_contact} ago"
 msgstr ""
 

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

@@ -92,6 +92,14 @@ describe('RunnerActionsCell', () => {
       expect(findDeleteBtn().props('compact')).toBe(true);
     });
 
+    it('Passes runner data to delete button', () => {
+      createComponent({
+        runner: mockRunner,
+      });
+
+      expect(findDeleteBtn().props('runner')).toEqual(mockRunner);
+    });
+
     it('Emits delete events', () => {
       const value = { name: 'Runner' };
 
@@ -104,7 +112,7 @@ describe('RunnerActionsCell', () => {
       expect(wrapper.emitted('deleted')).toEqual([[value]]);
     });
 
-    it('Does not render the runner delete button when user cannot delete', () => {
+    it('Renders the runner delete disabled button when user cannot delete', () => {
       createComponent({
         runner: {
           userPermissions: {
@@ -114,7 +122,7 @@ describe('RunnerActionsCell', () => {
         },
       });
 
-      expect(findDeleteBtn().exists()).toBe(false);
+      expect(findDeleteBtn().props('disabled')).toBe(true);
     });
   });
 });

spec/frontend/runner/components/runner_delete_button_spec.js

@@ -9,7 +9,11 @@ import waitForPromises from 'helpers/wait_for_promises';
 import { captureException } from '~/runner/sentry_utils';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import { createAlert } from '~/flash';
-import { I18N_DELETE_RUNNER } from '~/runner/constants';
+import {
+  I18N_DELETE_RUNNER,
+  I18N_DELETE_DISABLED_MANY_PROJECTS,
+  I18N_DELETE_DISABLED_UNKNOWN_REASON,
+} from '~/runner/constants';
 
 import RunnerDeleteButton from '~/runner/components/runner_delete_button.vue';
 import RunnerDeleteModal from '~/runner/components/runner_delete_modal.vue';
@@ -27,11 +31,12 @@ describe('RunnerDeleteButton', () => {
   let wrapper;
   let runnerDeleteHandler;
 
-  const getTooltip = () => getBinding(wrapper.element, 'gl-tooltip').value;
-  const getModal = () => getBinding(wrapper.element, 'gl-modal').value;
   const findBtn = () => wrapper.findComponent(GlButton);
   const findModal = () => wrapper.findComponent(RunnerDeleteModal);
 
+  const getTooltip = () => getBinding(wrapper.element, 'gl-tooltip').value;
+  const getModal = () => getBinding(findBtn().element, 'gl-modal').value;
+
   const createComponent = ({ props = {}, mountFn = shallowMountExtended } = {}) => {
     const { runner, ...propsData } = props;
 
@@ -88,6 +93,10 @@ describe('RunnerDeleteButton', () => {
     expect(findModal().props('runnerName')).toBe(`#${mockRunnerId} (${mockRunner.shortSha})`);
   });
 
+  it('Does not have tabindex when button is enabled', () => {
+    expect(wrapper.attributes('tabindex')).toBeUndefined();
+  });
+
   it('Displays a modal when clicked', () => {
     const modalId = `delete-runner-modal-${mockRunnerId}`;
 
@@ -230,4 +239,29 @@ describe('RunnerDeleteButton', () => {
       });
     });
   });
+
+  describe.each`
+    reason                                     | runner                 | tooltip
+    ${'runner belongs to more than 1 project'} | ${{ projectCount: 2 }} | ${I18N_DELETE_DISABLED_MANY_PROJECTS}
+    ${'unknown reason'}                        | ${{}}                  | ${I18N_DELETE_DISABLED_UNKNOWN_REASON}
+  `('When button is disabled because $reason', ({ runner, tooltip }) => {
+    beforeEach(() => {
+      createComponent({
+        props: {
+          disabled: true,
+          runner,
+        },
+      });
+    });
+
+    it('Displays a disabled delete button', () => {
+      expect(findBtn().props('disabled')).toBe(true);
+    });
+
+    it(`Tooltip "${tooltip}" is shown`, () => {
+      // tabindex is required for a11y
+      expect(wrapper.attributes('tabindex')).toBe('0');
+      expect(getTooltip()).toBe(tooltip);
+    });
+  });
 });