diff --git a/app/assets/javascripts/runner/components/cells/runner_actions_cell.vue b/app/assets/javascripts/runner/components/cells/runner_actions_cell.vue index c69321de0018f9904abc670c36654042a9098882..d3535f8942791c88591b57885ba655c5d86f7a37 100644 --- a/app/assets/javascripts/runner/components/cells/runner_actions_cell.vue +++ b/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> diff --git a/app/assets/javascripts/runner/components/runner_delete_button.vue b/app/assets/javascripts/runner/components/runner_delete_button.vue index 854c983f4daac36fdfd4e76f30052a703dc2b123..a546a2788de8793cf6326a5ce6e64768584b702a 100644 --- a/app/assets/javascripts/runner/components/runner_delete_button.vue +++ b/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> - <gl-button - v-gl-tooltip.hover.viewport="tooltip" - v-gl-modal="runnerDeleteModalId" - :aria-label="ariaLabel" - :icon="icon" - :class="buttonClass" - :loading="deleting" - variant="danger" - > - {{ buttonContent }} + <div v-gl-tooltip="tooltip" class="btn-group" :tabindex="wrapperTabindex"> + <gl-button + 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> diff --git a/app/assets/javascripts/runner/constants.js b/app/assets/javascripts/runner/constants.js index bd5be2175adb1f594369edf24dcfcfb3ea34722e..1e6f9d7c6691615ba238fbbb11d81cb9e4901071 100644 --- a/app/assets/javascripts/runner/constants.js +++ b/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'); diff --git a/app/assets/javascripts/runner/graphql/list/group_runners.query.graphql b/app/assets/javascripts/runner/graphql/list/group_runners.query.graphql index b517f5e89a882c0fff874591c3529e7b79832b1e..7aac2c1d4f2d4eb2241257f2d13e1037d23d2c00 100644 --- a/app/assets/javascripts/runner/graphql/list/group_runners.query.graphql +++ b/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 { diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 36f573bb5a3fb00f4e776ff761f5b3bed6aa31a3..fc0be9f54f4ab02074ba92a8b0e7654940363777 100644 --- a/locale/gitlab.pot +++ b/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 "" diff --git a/spec/frontend/runner/components/cells/runner_actions_cell_spec.js b/spec/frontend/runner/components/cells/runner_actions_cell_spec.js index 0d579106860393a2ded2d9e05842c9a957215e7a..9ca99d1109b9d2ad4f8a4e682a40d6e1124a41ba 100644 --- a/spec/frontend/runner/components/cells/runner_actions_cell_spec.js +++ b/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); }); }); }); diff --git a/spec/frontend/runner/components/runner_delete_button_spec.js b/spec/frontend/runner/components/runner_delete_button_spec.js index 81c870f23cfbccfd61590416eace72b62f2a005b..262658800bdafaf8576acbd7c4cbd95ce7e7497c 100644 --- a/spec/frontend/runner/components/runner_delete_button_spec.js +++ b/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); + }); + }); });