Merge branch '24705-multi-selection-for-delete-on-registry-page' into 'master'

Resolve "Multi selection for delete on registry page"

Closes #24705

See merge request gitlab-org/gitlab-ce!30837

app/assets/javascripts/registry/components/collapsible_container.vue

@@ -84,7 +84,7 @@ export default {
           v-gl-modal="modalId"
           :title="s__('ContainerRegistry|Remove repository')"
           :aria-label="s__('ContainerRegistry|Remove repository')"
-          class="js-remove-repo"
+          class="js-remove-repo btn-inverted"
           variant="danger"
         >
           <icon name="remove" />

app/assets/javascripts/registry/components/table_registry.vue

 <script>
 import { mapActions } from 'vuex';
-import { GlButton, GlTooltipDirective, GlModal, GlModalDirective } from '@gitlab/ui';
-import { n__ } from '../../locale';
+import {
+  GlButton,
+  GlFormCheckbox,
+  GlTooltipDirective,
+  GlModal,
+  GlModalDirective,
+} from '@gitlab/ui';
+import { n__, s__, sprintf } from '../../locale';
 import createFlash from '../../flash';
 import ClipboardButton from '../../vue_shared/components/clipboard_button.vue';
 import TablePagination from '../../vue_shared/components/pagination/table_pagination.vue';
@@ -14,6 +20,7 @@ export default {
   components: {
     ClipboardButton,
     TablePagination,
+    GlFormCheckbox,
     GlButton,
     Icon,
     GlModal,
@@ -31,33 +38,98 @@ export default {
   },
   data() {
     return {
-      itemToBeDeleted: null,
+      itemsToBeDeleted: [],
       modalId: `confirm-image-deletion-modal-${this.repo.id}`,
+      selectAllChecked: false,
+      modalDescription: '',
     };
   },
   computed: {
+    bulkDeletePath() {
+      return this.repo.tagsPath ? this.repo.tagsPath.replace('?format=json', '/bulk_destroy') : '';
+    },
     shouldRenderPagination() {
       return this.repo.pagination.total > this.repo.pagination.perPage;
     },
+    modalTitle() {
+      return n__(
+        'ContainerRegistry|Remove image',
+        'ContainerRegistry|Remove images',
+        this.itemsToBeDeleted.length === 0 ? 1 : this.itemsToBeDeleted.length,
+      );
+    },
+  },
+  mounted() {
+    this.$refs.deleteModal.$refs.modal.$on('hide', this.removeModalEvents);
   },
   methods: {
-    ...mapActions(['fetchList', 'deleteItem']),
+    ...mapActions(['fetchList', 'deleteItem', 'multiDeleteItems']),
+    setModalDescription(itemIndex = -1) {
+      if (itemIndex === -1) {
+        this.modalDescription = sprintf(
+          s__(`ContainerRegistry|You are about to delete <b>%{count}</b> images. This will
+              delete the images and all tags pointing to them.`),
+          { count: this.itemsToBeDeleted.length },
+        );
+      } else {
+        const { tag } = this.repo.list[itemIndex];
+
+        this.modalDescription = sprintf(
+          s__(`ContainerRegistry|You are about to delete the image <b>%{title}</b>. This will
+              delete the image and all tags pointing to this image.`),
+          { title: `${this.repo.name}:${tag}` },
+        );
+      }
+    },
     layers(item) {
       return item.layers ? n__('%d layer', '%d layers', item.layers) : '';
     },
     formatSize(size) {
       return numberToHumanSize(size);
     },
-    setItemToBeDeleted(item) {
-      this.itemToBeDeleted = item;
+    removeModalEvents() {
+      this.$refs.deleteModal.$refs.modal.$off('ok');
+    },
+    deleteSingleItem(index) {
+      this.setModalDescription(index);
+
+      this.$refs.deleteModal.$refs.modal.$once('ok', () => {
+        this.removeModalEvents();
+        this.handleSingleDelete(this.repo.list[index]);
+      });
+    },
+    deleteMultipleItems() {
+      if (this.itemsToBeDeleted.length === 1) {
+        this.setModalDescription(this.itemsToBeDeleted[0]);
+      } else if (this.itemsToBeDeleted.length > 1) {
+        this.setModalDescription();
+      }
+
+      this.$refs.deleteModal.$refs.modal.$once('ok', () => {
+        this.removeModalEvents();
+        this.handleMultipleDelete();
+      });
     },
-    handleDeleteRegistry() {
-      const { itemToBeDeleted } = this;
-      this.itemToBeDeleted = null;
-      this.deleteItem(itemToBeDeleted)
+    handleSingleDelete(itemToDelete) {
+      this.deleteItem(itemToDelete)
         .then(() => this.fetchList({ repo: this.repo }))
         .catch(() => this.showError(errorMessagesTypes.DELETE_REGISTRY));
     },
+    handleMultipleDelete() {
+      const { itemsToBeDeleted } = this;
+      this.itemsToBeDeleted = [];
+
+      if (this.bulkDeletePath) {
+        this.multiDeleteItems({
+          path: this.bulkDeletePath,
+          items: itemsToBeDeleted.map(x => this.repo.list[x].tag),
+        })
+          .then(() => this.fetchList({ repo: this.repo }))
+          .catch(() => this.showError(errorMessagesTypes.DELETE_REGISTRY));
+      } else {
+        this.showError(errorMessagesTypes.DELETE_REGISTRY);
+      }
+    },
     onPageChange(pageNumber) {
       this.fetchList({ repo: this.repo, page: pageNumber }).catch(() =>
         this.showError(errorMessagesTypes.FETCH_REGISTRY),
@@ -66,6 +138,35 @@ export default {
     showError(message) {
       createFlash(errorMessages[message]);
     },
+    onSelectAllChange() {
+      if (this.selectAllChecked) {
+        this.deselectAll();
+      } else {
+        this.selectAll();
+      }
+    },
+    selectAll() {
+      this.itemsToBeDeleted = this.repo.list.map((x, index) => index);
+      this.selectAllChecked = true;
+    },
+    deselectAll() {
+      this.itemsToBeDeleted = [];
+      this.selectAllChecked = false;
+    },
+    updateItemsToBeDeleted(index) {
+      const delIndex = this.itemsToBeDeleted.findIndex(x => x === index);
+
+      if (delIndex > -1) {
+        this.itemsToBeDeleted.splice(delIndex, 1);
+        this.selectAllChecked = false;
+      } else {
+        this.itemsToBeDeleted.push(index);
+
+        if (this.itemsToBeDeleted.length === this.repo.list.length) {
+          this.selectAllChecked = true;
+        }
+      }
+    },
   },
 };
 </script>
@@ -74,15 +175,44 @@ export default {
     <table class="table tags">
       <thead>
         <tr>
+          <th>
+            <gl-form-checkbox
+              v-if="repo.canDelete"
+              class="js-select-all-checkbox"
+              :checked="selectAllChecked"
+              @change="onSelectAllChange"
+            />
+          </th>
           <th>{{ s__('ContainerRegistry|Tag') }}</th>
           <th>{{ s__('ContainerRegistry|Tag ID') }}</th>
           <th>{{ s__('ContainerRegistry|Size') }}</th>
           <th>{{ s__('ContainerRegistry|Last Updated') }}</th>
-          <th></th>
+          <th>
+            <gl-button
+              v-if="repo.canDelete"
+              v-gl-tooltip
+              v-gl-modal="modalId"
+              :disabled="!itemsToBeDeleted || itemsToBeDeleted.length === 0"
+              class="js-delete-registry float-right"
+              variant="danger"
+              :title="s__('ContainerRegistry|Remove selected images')"
+              :aria-label="s__('ContainerRegistry|Remove selected images')"
+              @click="deleteMultipleItems()"
+              ><icon name="remove"
+            /></gl-button>
+          </th>
         </tr>
       </thead>
       <tbody>
-        <tr v-for="item in repo.list" :key="item.tag">
+        <tr v-for="(item, index) in repo.list" :key="item.tag" class="registry-image-row">
+          <td class="check">
+            <gl-form-checkbox
+              v-if="item.canDelete"
+              class="js-select-checkbox"
+              :checked="itemsToBeDeleted && itemsToBeDeleted.includes(index)"
+              @change="updateItemsToBeDeleted(index)"
+            />
+          </td>
           <td class="monospace">
             {{ item.tag }}
             <clipboard-button
@@ -111,16 +241,15 @@ export default {
             </span>
           </td>
 
-          <td class="content">
+          <td class="content action-buttons">
             <gl-button
               v-if="item.canDelete"
-              v-gl-tooltip
               v-gl-modal="modalId"
               :title="s__('ContainerRegistry|Remove image')"
               :aria-label="s__('ContainerRegistry|Remove image')"
               variant="danger"
-              class="js-delete-registry d-none d-sm-block float-right"
-              @click="setItemToBeDeleted(item)"
+              class="js-delete-registry-row float-right btn-inverted btn-border-color btn-icon"
+              @click="deleteSingleItem(index)"
             >
               <icon name="remove" />
             </gl-button>
@@ -135,19 +264,10 @@ export default {
       :page-info="repo.pagination"
     />
 
-    <gl-modal :modal-id="modalId" ok-variant="danger" @ok="handleDeleteRegistry">
-      <template v-slot:modal-title>{{ s__('ContainerRegistry|Remove image') }}</template>
-      <template v-slot:modal-ok>{{ s__('ContainerRegistry|Remove image and tags') }}</template>
-      <p
-        v-html="
-          sprintf(
-            s__(
-              'ContainerRegistry|You are about to delete the image <b>%{title}</b>. This will delete the image and all tags pointing to this image.',
-            ),
-            { title: repo.name },
-          )
-        "
-      ></p>
+    <gl-modal ref="deleteModal" :modal-id="modalId" ok-variant="danger">
+      <template v-slot:modal-title>{{ modalTitle }}</template>
+      <template v-slot:modal-ok>{{ s__('ContainerRegistry|Remove image(s) and tags') }}</template>
+      <p v-html="modalDescription"></p>
     </gl-modal>
   </div>
 </template>

app/assets/javascripts/registry/stores/actions.js

@@ -36,6 +36,8 @@ export const fetchList = ({ commit }, { repo, page }) => {
 };
 
 export const deleteItem = (_, item) => axios.delete(item.destroyPath);
+export const multiDeleteItems = (_, { path, items }) =>
+  axios.delete(path, { params: { ids: items } });
 
 export const setMainEndpoint = ({ commit }, data) => commit(types.SET_MAIN_ENDPOINT, data);
 export const toggleLoading = ({ commit }) => commit(types.TOGGLE_MAIN_LOADING);

app/assets/stylesheets/pages/container_registry.scss

@@ -31,4 +31,21 @@
 
 .table.tags {
   margin-bottom: 0;
+
+  .registry-image-row {
+    .check {
+      padding-right: $gl-padding;
+      width: 5%;
+    }
+
+    .action-buttons {
+      opacity: 0;
+    }
+
+    &:hover {
+      .action-buttons {
+        opacity: 1;
+      }
+    }
+  }
 }

app/controllers/projects/registry/tags_controller.rb

@@ -5,6 +5,8 @@ module Projects
     class TagsController < ::Projects::Registry::ApplicationController
       before_action :authorize_destroy_container_image!, only: [:destroy]
 
+      LIMIT = 15
+
       def index
         respond_to do |format|
           format.json do
@@ -28,10 +30,40 @@ module Projects
         end
       end
 
+      def bulk_destroy
+        unless params[:ids].present?
+          head :bad_request
+          return
+        end
+
+        tag_names = params[:ids] || []
+        if tag_names.size > LIMIT
+          head :bad_request
+          return
+        end
+
+        @tags = tag_names.map { |tag_name| image.tag(tag_name) }
+        unless @tags.all? { |tag| tag.valid_name? }
+          head :bad_request
+          return
+        end
+
+        success_count = 0
+        @tags.each do |tag|
+          if tag.delete
+            success_count += 1
+          end
+        end
+
+        respond_to do |format|
+          format.json { head(success_count == @tags.size ? :no_content : :bad_request) }
+        end
+      end
+
       private
 
       def tags
-        Kaminari::PaginatableArray.new(image.tags, limit: 15)
+        Kaminari::PaginatableArray.new(image.tags, limit: LIMIT)
       end
 
       def image

changelogs/unreleased/24705-multi-selection-for-delete-on-registry-page.yml

+---
+title: Added multi-select deletion of container registry images
+merge_request: 30837
+author:
+type: other

config/routes/project.rb

@@ -477,7 +477,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           # in JSON format, or a request for tag named `latest.json`.
           scope format: false do
             resources :tags, only: [:index, :destroy],
-                             constraints: { id: Gitlab::Regex.container_registry_tag_regex }
+                             constraints: { id: Gitlab::Regex.container_registry_tag_regex } do
+              collection do
+                delete :bulk_destroy
+              end
+            end
           end
         end
       end

lib/container_registry/tag.rb

@@ -6,6 +6,9 @@ module ContainerRegistry
 
     attr_reader :repository, :name
 
+    # https://github.com/docker/distribution/commit/3150937b9f2b1b5b096b2634d0e7c44d4a0f89fb
+    TAG_NAME_REGEX = /^[\w][\w.-]{0,127}$/.freeze
+
     delegate :registry, :client, to: :repository
     delegate :revision, :short_revision, to: :config_blob, allow_nil: true
 
@@ -13,6 +16,10 @@ module ContainerRegistry
       @repository, @name = repository, name
     end
 
+    def valid_name?
+      !name.match(TAG_NAME_REGEX).nil?
+    end
+
     def valid?
       manifest.present?
     end

locale/gitlab.pot

@@ -3178,14 +3178,19 @@ msgid "ContainerRegistry|Quick Start"
 msgstr ""
 
 msgid "ContainerRegistry|Remove image"
-msgstr ""
+msgid_plural "ContainerRegistry|Remove images"
+msgstr[0] ""
+msgstr[1] ""
 
-msgid "ContainerRegistry|Remove image and tags"
+msgid "ContainerRegistry|Remove image(s) and tags"
 msgstr ""
 
 msgid "ContainerRegistry|Remove repository"
 msgstr ""
 
+msgid "ContainerRegistry|Remove selected images"
+msgstr ""
+
 msgid "ContainerRegistry|Size"
 msgstr ""
 
@@ -3207,6 +3212,9 @@ msgstr ""
 msgid "ContainerRegistry|With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images. %{docLinkStart}More Information%{docLinkEnd}"
 msgstr ""
 
+msgid "ContainerRegistry|You are about to delete <b>%{count}</b> images. This will delete the images and all tags pointing to them."
+msgstr ""
+
 msgid "ContainerRegistry|You are about to delete the image <b>%{title}</b>. This will delete the image and all tags pointing to this image."
 msgstr ""
 

spec/controllers/projects/registry/tags_controller_spec.rb

@@ -113,4 +113,37 @@ describe Projects::Registry::TagsController do
                      format: :json
     end
   end
+
+  describe 'POST bulk_destroy' do
+    context 'when user has access to registry' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'when there is matching tag present' do
+        before do
+          stub_container_registry_tags(repository: repository.path, tags: %w[rc1 test.])
+        end
+
+        it 'makes it possible to delete tags in bulk' do
+          allow_any_instance_of(ContainerRegistry::Tag).to receive(:delete) { |*args| ContainerRegistry::Tag.delete(*args) }
+          expect(ContainerRegistry::Tag).to receive(:delete).exactly(2).times
+
+          bulk_destroy_tags(['rc1', 'test.'])
+        end
+      end
+    end
+
+    private
+
+    def bulk_destroy_tags(names)
+      post :bulk_destroy, params: {
+                       namespace_id: project.namespace,
+                       project_id: project,
+                       repository_id: repository,
+                       ids: names
+                     },
+                     format: :json
+    end
+  end
 end

spec/features/container_registry_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe "Container Registry", :js do
+describe 'Container Registry', :js do
   let(:user) { create(:user) }
   let(:project) { create(:project) }
 
@@ -40,8 +40,7 @@ describe "Container Registry", :js do
     it 'user removes entire container repository' do
       visit_container_registry
 
-      expect_any_instance_of(ContainerRepository)
-        .to receive(:delete_tags!).and_return(true)
+      expect_any_instance_of(ContainerRepository).to receive(:delete_tags!).and_return(true)
 
       click_on(class: 'js-remove-repo')
       expect(find('.modal .modal-title')).to have_content 'Remove repository'
@@ -54,10 +53,9 @@ describe "Container Registry", :js do
       find('.js-toggle-repo').click
       wait_for_requests
 
-      expect_any_instance_of(ContainerRegistry::Tag)
-        .to receive(:delete).and_return(true)
+      expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete).and_return(true)
 
-      click_on(class: 'js-delete-registry')
+      click_on(class: 'js-delete-registry-row', visible: false)
       expect(find('.modal .modal-title')).to have_content 'Remove image'
       find('.modal .modal-footer .btn-danger').click
     end

spec/javascripts/registry/components/table_registry_spec.js

 import Vue from 'vue';
 import tableRegistry from '~/registry/components/table_registry.vue';
 import store from '~/registry/stores';
+import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
 import { repoPropsData } from '../mock_data';
 
-const [firstImage] = repoPropsData.list;
+const [firstImage, secondImage] = repoPropsData.list;
 
 describe('table registry', () => {
   let vm;
-  let Component;
+  const Component = Vue.extend(tableRegistry);
+  const bulkDeletePath = 'path';
 
   const findDeleteBtn = () => vm.$el.querySelector('.js-delete-registry');
+  const findDeleteBtnRow = () => vm.$el.querySelector('.js-delete-registry-row');
+  const findSelectAllCheckbox = () => vm.$el.querySelector('.js-select-all-checkbox > input');
+  const findAllRowCheckboxes = () =>
+    Array.from(vm.$el.querySelectorAll('.js-select-checkbox input'));
+  const confirmationModal = (child = '') => document.querySelector(`#${vm.modalId} ${child}`);
 
-  beforeEach(() => {
-    Component = Vue.extend(tableRegistry);
-    vm = new Component({
+  const createComponent = () => {
+    vm = mountComponentWithStore(Component, {
       store,
-      propsData: {
+      props: {
         repo: repoPropsData,
       },
-    }).$mount();
+    });
+  };
+
+  const selectAllCheckboxes = () => vm.selectAll();
+  const deselectAllCheckboxes = () => vm.deselectAll();
+
+  beforeEach(() => {
+    createComponent();
   });
 
   afterEach(() => {
     vm.$destroy();
   });
 
+  describe('rendering', () => {
     it('should render a table with the registry list', () => {
       expect(vm.$el.querySelectorAll('table tbody tr').length).toEqual(repoPropsData.list.length);
     });
@@ -33,6 +47,7 @@ describe('table registry', () => {
       const textRendered = vm.$el
         .querySelector('.table tbody tr')
         .textContent.trim()
+        // replace additional whitespace characters (e.g. new lines) with a single empty space
         .replace(/\s\s+/g, ' ');
 
       expect(textRendered).toContain(repoPropsData.list[0].tag);
@@ -40,29 +55,135 @@ describe('table registry', () => {
       expect(textRendered).toContain(repoPropsData.list[0].layers);
       expect(textRendered).toContain(repoPropsData.list[0].size);
     });
+  });
+
+  describe('multi select', () => {
+    it('should support multiselect and selecting a row should enable delete button', done => {
+      findSelectAllCheckbox().click();
+      selectAllCheckboxes();
+
+      expect(findSelectAllCheckbox().checked).toBe(true);
+
+      Vue.nextTick(() => {
+        expect(findDeleteBtn().disabled).toBe(false);
+        done();
+      });
+    });
+
+    it('selecting all checkbox should select all rows and enable delete button', done => {
+      selectAllCheckboxes();
+
+      Vue.nextTick(() => {
+        const checkedValues = findAllRowCheckboxes().filter(x => x.checked);
+
+        expect(checkedValues.length).toBe(repoPropsData.list.length);
+        done();
+      });
+    });
+
+    it('deselecting select all checkbox should deselect all rows and disable delete button', done => {
+      selectAllCheckboxes();
+      deselectAllCheckboxes();
+
+      Vue.nextTick(() => {
+        const checkedValues = findAllRowCheckboxes().filter(x => x.checked);
+
+        expect(checkedValues.length).toBe(0);
+        done();
+      });
+    });
+
+    it('should delete multiple items when multiple items are selected', done => {
+      selectAllCheckboxes();
+
+      Vue.nextTick(() => {
+        expect(vm.itemsToBeDeleted).toEqual([0, 1]);
+        expect(findDeleteBtn().disabled).toBe(false);
+
+        findDeleteBtn().click();
+        spyOn(vm, 'multiDeleteItems').and.returnValue(Promise.resolve());
+
+        Vue.nextTick(() => {
+          const modal = confirmationModal();
+          confirmationModal('.btn-danger').click();
+
+          expect(modal).toExist();
+
+          Vue.nextTick(() => {
+            expect(vm.itemsToBeDeleted).toEqual([]);
+            expect(vm.multiDeleteItems).toHaveBeenCalledWith({
+              path: bulkDeletePath,
+              items: [firstImage.tag, secondImage.tag],
+            });
+            done();
+          });
+        });
+      });
+    });
+  });
 
   describe('delete registry', () => {
-    it('should be possible to delete a registry', () => {
+    beforeEach(() => {
+      vm.itemsToBeDeleted = [0];
+    });
+
+    it('should be possible to delete a registry', done => {
+      Vue.nextTick(() => {
+        expect(vm.itemsToBeDeleted).toEqual([0]);
         expect(findDeleteBtn()).toBeDefined();
+        expect(findDeleteBtn().disabled).toBe(false);
+        expect(findDeleteBtnRow()).toBeDefined();
+        done();
+      });
     });
 
-    it('should call deleteItem and reset itemToBeDeleted when confirming deletion', done => {
+    it('should call deleteItems and reset itemsToBeDeleted when confirming deletion', done => {
+      Vue.nextTick(() => {
+        expect(vm.itemsToBeDeleted).toEqual([0]);
+        expect(findDeleteBtn().disabled).toBe(false);
         findDeleteBtn().click();
-      spyOn(vm, 'deleteItem').and.returnValue(Promise.resolve());
+        spyOn(vm, 'multiDeleteItems').and.returnValue(Promise.resolve());
 
         Vue.nextTick(() => {
-        document.querySelector(`#${vm.modalId} .btn-danger`).click();
+          confirmationModal('.btn-danger').click();
 
-        expect(vm.deleteItem).toHaveBeenCalledWith(firstImage);
-        expect(vm.itemToBeDeleted).toBeNull();
+          expect(vm.itemsToBeDeleted).toEqual([]);
+          expect(vm.multiDeleteItems).toHaveBeenCalledWith({
+            path: bulkDeletePath,
+            items: [firstImage.tag],
+          });
           done();
         });
       });
     });
+  });
 
   describe('pagination', () => {
     it('should be possible to change the page', () => {
       expect(vm.$el.querySelector('.gl-pagination')).toBeDefined();
     });
   });
+
+  describe('modal content', () => {
+    it('should show the singular title and image name when deleting a single image', done => {
+      findDeleteBtnRow().click();
+
+      Vue.nextTick(() => {
+        expect(vm.modalTitle).toBe('Remove image');
+        expect(vm.modalDescription).toContain(firstImage.tag);
+        done();
+      });
+    });
+
+    it('should show the plural title and image count when deleting more than one image', done => {
+      selectAllCheckboxes();
+      vm.setModalDescription();
+
+      Vue.nextTick(() => {
+        expect(vm.modalTitle).toBe('Remove images');
+        expect(vm.modalDescription).toContain('<b>2</b> images');
+        done();
+      });
+    });
+  });
 });

spec/javascripts/registry/mock_data.js

@@ -108,6 +108,17 @@ export const repoPropsData = {
       destroyPath: 'path',
       canDelete: true,
     },
+    {
+      tag: 'test-image',
+      revision: 'b969de599faea2b3d9b6605a8b0897261c571acaa36db1bdc7349b5775b4e0b4',
+      shortRevision: 'b969de599',
+      size: 19,
+      layers: 10,
+      location: 'location-2',
+      createdAt: 1505828744434,
+      destroyPath: 'path-2',
+      canDelete: true,
+    },
   ],
   location: 'location',
   name: 'foo',