Merge branch '336933-improve-the-process-of-deleting-image-tags' into 'master'

Container Details: move delete to action menu

See merge request gitlab-org/gitlab!71541

app/assets/javascripts/registry/explorer/components/details_page/tags_list_row.vue

 <script>
-import { GlFormCheckbox, GlTooltipDirective, GlSprintf, GlIcon } from '@gitlab/ui';
+import {
+  GlFormCheckbox,
+  GlTooltipDirective,
+  GlSprintf,
+  GlIcon,
+  GlDropdown,
+  GlDropdownItem,
+} from '@gitlab/ui';
 import { formatDate } from '~/lib/utils/datetime_utility';
 import { numberToHumanSize } from '~/lib/utils/number_utils';
 import { n__ } from '~/locale';
@@ -11,22 +18,22 @@ import {
   REMOVE_TAG_BUTTON_TITLE,
   DIGEST_LABEL,
   CREATED_AT_LABEL,
-  REMOVE_TAG_BUTTON_DISABLE_TOOLTIP,
   PUBLISHED_DETAILS_ROW_TEXT,
   MANIFEST_DETAILS_ROW_TEST,
   CONFIGURATION_DETAILS_ROW_TEST,
   MISSING_MANIFEST_WARNING_TOOLTIP,
   NOT_AVAILABLE_TEXT,
   NOT_AVAILABLE_SIZE,
+  MORE_ACTIONS_TEXT,
 } from '../../constants/index';
-import DeleteButton from '../delete_button.vue';
 
 export default {
   components: {
     GlSprintf,
     GlFormCheckbox,
     GlIcon,
-    DeleteButton,
+    GlDropdown,
+    GlDropdownItem,
     ListItem,
     ClipboardButton,
     TimeAgoTooltip,
@@ -60,11 +67,11 @@ export default {
     REMOVE_TAG_BUTTON_TITLE,
     DIGEST_LABEL,
     CREATED_AT_LABEL,
-    REMOVE_TAG_BUTTON_DISABLE_TOOLTIP,
     PUBLISHED_DETAILS_ROW_TEXT,
     MANIFEST_DETAILS_ROW_TEST,
     CONFIGURATION_DETAILS_ROW_TEST,
     MISSING_MANIFEST_WARNING_TOOLTIP,
+    MORE_ACTIONS_TEXT,
   },
   computed: {
     formattedSize() {
@@ -173,15 +180,26 @@ export default {
       </span>
     </template>
     <template #right-action>
-      <delete-button
-        :disabled="isDeleteDisabled"
-        :title="$options.i18n.REMOVE_TAG_BUTTON_TITLE"
-        :tooltip-title="$options.i18n.REMOVE_TAG_BUTTON_DISABLE_TOOLTIP"
-        :tooltip-disabled="tag.canDelete"
-        data-qa-selector="tag_delete_button"
-        data-testid="single-delete-button"
-        @delete="$emit('delete')"
-      />
+      <gl-dropdown
+        v-if="!isDeleteDisabled"
+        icon="ellipsis_v"
+        :text="$options.i18n.MORE_ACTIONS_TEXT"
+        :text-sr-only="true"
+        category="tertiary"
+        no-caret
+        right
+        data-testid="additional-actions"
+        data-qa-selector="more_actions_menu"
+      >
+        <gl-dropdown-item
+          variant="danger"
+          data-testid="single-delete-button"
+          data-qa-selector="tag_delete_button"
+          @click="$emit('delete')"
+        >
+          {{ $options.i18n.REMOVE_TAG_BUTTON_TITLE }}
+        </gl-dropdown-item>
+      </gl-dropdown>
     </template>
 
     <template v-if="!isInvalidTag" #details-published>

app/assets/javascripts/registry/explorer/constants/common.js

-import { s__ } from '~/locale';
+import { s__, __ } from '~/locale';
 
 export const ROOT_IMAGE_TEXT = s__('ContainerRegistry|Root image');
+export const MORE_ACTIONS_TEXT = __('More actions');

app/assets/javascripts/registry/explorer/constants/details.js

@@ -30,7 +30,7 @@ export const CONFIGURATION_DETAILS_ROW_TEST = s__(
   'ContainerRegistry|Configuration digest: %{digest}',
 );
 
-export const REMOVE_TAG_BUTTON_TITLE = s__('ContainerRegistry|Remove tag');
+export const REMOVE_TAG_BUTTON_TITLE = s__('ContainerRegistry|Delete tag');
 export const REMOVE_TAGS_BUTTON_TITLE = s__('ContainerRegistry|Delete selected tags');
 
 export const REMOVE_TAG_CONFIRMATION_TEXT = s__(
@@ -61,10 +61,6 @@ export const ADMIN_GARBAGE_COLLECTION_TIP = s__(
   'ContainerRegistry|Remember to run %{docLinkStart}garbage collection%{docLinkEnd} to remove the stale data from storage.',
 );
 
-export const REMOVE_TAG_BUTTON_DISABLE_TOOLTIP = s__(
-  'ContainerRegistry|Deletion disabled due to missing or insufficient permissions.',
-);
-
 export const MISSING_MANIFEST_WARNING_TOOLTIP = s__(
   'ContainerRegistry|Invalid tag: missing manifest digest',
 );

locale/gitlab.pot

@@ -8896,10 +8896,10 @@ msgstr ""
 msgid "ContainerRegistry|Delete selected tags"
 msgstr ""
 
-msgid "ContainerRegistry|Deleting the image repository will delete all images and tags inside. This action cannot be undone. Please type the following to confirm: %{code}"
+msgid "ContainerRegistry|Delete tag"
 msgstr ""
 
-msgid "ContainerRegistry|Deletion disabled due to missing or insufficient permissions."
+msgid "ContainerRegistry|Deleting the image repository will delete all images and tags inside. This action cannot be undone. Please type the following to confirm: %{code}"
 msgstr ""
 
 msgid "ContainerRegistry|Digest: %{imageId}"

qa/qa/page/project/registry/show.rb

@@ -9,6 +9,10 @@ module QA
             element :registry_image_content
           end
 
+          view 'app/assets/javascripts/registry/explorer/components/details_page/tags_list_row.vue' do
+            element :more_actions_menu
+          end
+
           view 'app/assets/javascripts/registry/explorer/components/details_page/tags_list_row.vue' do
             element :tag_delete_button
           end
@@ -30,6 +34,7 @@ module QA
           end
 
           def click_delete
+            click_element(:more_actions_menu)
             click_element(:tag_delete_button)
             find_button('Delete').click
           end

spec/features/groups/container_registry_spec.rb

@@ -81,6 +81,7 @@ RSpec.describe 'Container Registry', :js do
         expect(service).to receive(:execute).with(container_repository) { { status: :success } }
         expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(container_repository.project, user, tags: ['latest']) { service }
 
+        first('[data-testid="additional-actions"]').click
         first('[data-testid="single-delete-button"]').click
         expect(find('.modal .modal-title')).to have_content _('Remove tag')
         find('.modal .modal-footer .btn-danger').click

spec/features/projects/container_registry_spec.rb

@@ -96,6 +96,7 @@ RSpec.describe 'Container Registry', :js do
         expect(service).to receive(:execute).with(container_repository) { { status: :success } }
         expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(container_repository.project, user, tags: ['1']) { service }
 
+        first('[data-testid="additional-actions"]').click
         first('[data-testid="single-delete-button"]').click
         expect(find('.modal .modal-title')).to have_content _('Remove tag')
         find('.modal .modal-footer .btn-danger').click

spec/frontend/registry/explorer/components/details_page/tags_list_row_spec.js

-import { GlFormCheckbox, GlSprintf, GlIcon } from '@gitlab/ui';
+import { GlFormCheckbox, GlSprintf, GlIcon, GlDropdown, GlDropdownItem } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import { nextTick } from 'vue';
 
 import { createMockDirective, getBinding } from 'helpers/vue_mock_directive';
-import DeleteButton from '~/registry/explorer/components/delete_button.vue';
+
 import component from '~/registry/explorer/components/details_page/tags_list_row.vue';
 import {
   REMOVE_TAG_BUTTON_TITLE,
-  REMOVE_TAG_BUTTON_DISABLE_TOOLTIP,
   MISSING_MANIFEST_WARNING_TOOLTIP,
   NOT_AVAILABLE_TEXT,
   NOT_AVAILABLE_SIZE,
@@ -25,19 +24,20 @@ describe('tags list row', () => {
 
   const defaultProps = { tag, isMobile: false, index: 0 };
 
-  const findCheckbox = () => wrapper.find(GlFormCheckbox);
+  const findCheckbox = () => wrapper.findComponent(GlFormCheckbox);
   const findName = () => wrapper.find('[data-testid="name"]');
   const findSize = () => wrapper.find('[data-testid="size"]');
   const findTime = () => wrapper.find('[data-testid="time"]');
   const findShortRevision = () => wrapper.find('[data-testid="digest"]');
-  const findClipboardButton = () => wrapper.find(ClipboardButton);
-  const findDeleteButton = () => wrapper.find(DeleteButton);
-  const findTimeAgoTooltip = () => wrapper.find(TimeAgoTooltip);
+  const findClipboardButton = () => wrapper.findComponent(ClipboardButton);
+  const findTimeAgoTooltip = () => wrapper.findComponent(TimeAgoTooltip);
   const findDetailsRows = () => wrapper.findAll(DetailsRow);
   const findPublishedDateDetail = () => wrapper.find('[data-testid="published-date-detail"]');
   const findManifestDetail = () => wrapper.find('[data-testid="manifest-detail"]');
   const findConfigurationDetail = () => wrapper.find('[data-testid="configuration-detail"]');
-  const findWarningIcon = () => wrapper.find(GlIcon);
+  const findWarningIcon = () => wrapper.findComponent(GlIcon);
+  const findAdditionalActionsMenu = () => wrapper.findComponent(GlDropdown);
+  const findDeleteButton = () => wrapper.findComponent(GlDropdownItem);
 
   const mountComponent = (propsData = defaultProps) => {
     wrapper = shallowMount(component, {
@@ -45,6 +45,7 @@ describe('tags list row', () => {
         GlSprintf,
         ListItem,
         DetailsRow,
+        GlDropdown,
       },
       propsData,
       directives: {
@@ -262,44 +263,59 @@ describe('tags list row', () => {
     });
   });
 
-  describe('delete button', () => {
+  describe('additional actions menu', () => {
     it('exists', () => {
       mountComponent();
 
-      expect(findDeleteButton().exists()).toBe(true);
+      expect(findAdditionalActionsMenu().exists()).toBe(true);
     });
 
-    it('has the correct props/attributes', () => {
+    it('has the correct props', () => {
       mountComponent();
 
-      expect(findDeleteButton().attributes()).toMatchObject({
-        title: REMOVE_TAG_BUTTON_TITLE,
-        tooltiptitle: REMOVE_TAG_BUTTON_DISABLE_TOOLTIP,
-        tooltipdisabled: 'true',
+      expect(findAdditionalActionsMenu().props()).toMatchObject({
+        icon: 'ellipsis_v',
+        text: 'More actions',
+        textSrOnly: true,
+        category: 'tertiary',
+        right: true,
       });
     });
 
     it.each`
-      canDelete | digest   | disabled
-      ${true}   | ${null}  | ${true}
-      ${false}  | ${'foo'} | ${true}
-      ${false}  | ${null}  | ${true}
-      ${true}   | ${'foo'} | ${true}
+      canDelete | digest   | disabled | visible
+      ${true}   | ${null}  | ${true}  | ${false}
+      ${false}  | ${'foo'} | ${true}  | ${false}
+      ${false}  | ${null}  | ${true}  | ${false}
+      ${true}   | ${'foo'} | ${true}  | ${false}
+      ${true}   | ${'foo'} | ${false} | ${true}
     `(
-      'is disabled when canDelete is $canDelete and digest is $digest and disabled is $disabled',
-      ({ canDelete, digest, disabled }) => {
+      'is $visible that is visible when canDelete is $canDelete and digest is $digest and disabled is $disabled',
+      ({ canDelete, digest, disabled, visible }) => {
         mountComponent({ ...defaultProps, tag: { ...tag, canDelete, digest }, disabled });
 
-        expect(findDeleteButton().attributes('disabled')).toBe('true');
+        expect(findAdditionalActionsMenu().exists()).toBe(visible);
       },
     );
 
-    it('delete event emits delete', () => {
-      mountComponent();
+    describe('delete button', () => {
+      it('exists and has the correct attrs', () => {
+        mountComponent();
+
+        expect(findDeleteButton().exists()).toBe(true);
+        expect(findDeleteButton().attributes()).toMatchObject({
+          variant: 'danger',
+        });
+        expect(findDeleteButton().text()).toBe(REMOVE_TAG_BUTTON_TITLE);
+      });
 
-      findDeleteButton().vm.$emit('delete');
+      it('delete event emits delete', () => {
+        mountComponent();
 
-      expect(wrapper.emitted('delete')).toEqual([[]]);
+        findDeleteButton().vm.$emit('click');
+
+        expect(wrapper.emitted('delete')).toEqual([[]]);
+      });
     });
   });