From 918e7d43dfe614475ee2dd2b6f4c765726db6ef4 Mon Sep 17 00:00:00 2001 From: Nick Kipling Date: Fri, 26 Jul 2019 19:42:49 +0100 Subject: [PATCH] Reworked how deletion works with multi vs single Single deletion no longer requires a prop Modal description is now generated on demand Added dedicated functions for deleting Updated tests to match new function naming Updated css class name to be more specific --- .../registry/components/table_registry.vue | 109 +++++++++--------- .../javascripts/registry/stores/actions.js | 3 +- .../stylesheets/pages/container_registry.scss | 2 +- .../components/table_registry_spec.js | 8 +- 4 files changed, 64 insertions(+), 58 deletions(-) diff --git a/app/assets/javascripts/registry/components/table_registry.vue b/app/assets/javascripts/registry/components/table_registry.vue index 90d6a4ce27f..df665b36c4b 100644 --- a/app/assets/javascripts/registry/components/table_registry.vue +++ b/app/assets/javascripts/registry/components/table_registry.vue @@ -41,6 +41,7 @@ export default { itemsToBeDeleted: [], modalId: `confirm-image-deletion-modal-${this.repo.id}`, selectAllChecked: false, + modalDescription: '', }; }, computed: { @@ -54,67 +55,68 @@ export default { return n__( 'ContainerRegistry|Remove image', 'ContainerRegistry|Remove images', - this.singleItemSelected ? 1 : this.itemsToBeDeleted.length, + this.itemsToBeDeleted.length === 0 ? 1 : this.itemsToBeDeleted.length, ); }, - modalDescription() { - const selectedCount = this.itemsToBeDeleted.length; - - if (this.singleItemSelected) { - // Attempt to pull 'single' property if it's an object in this.itemsToBeDeleted - // Otherwise, simply use the int value of the selected row - const { single: itemIndex = this.itemsToBeDeleted[0] } = this.itemsToBeDeleted[0]; - const { tag } = this.repo.list[itemIndex]; - - return sprintf( - s__(`ContainerRegistry|You are about to delete the image %{title}. This will - delete the image and all tags pointing to this image.`), - { title: `${this.repo.name}:${tag}` }, - ); - } - - return sprintf( - s__(`ContainerRegistry|You are about to delete %{count} images. This will - delete the images and all tags pointing to them.`), - { count: selectedCount }, - ); - }, - singleItemSelected() { - return this.findSingleRowToDelete || this.itemsToBeDeleted.length === 1; - }, - findSingleRowToDelete() { - return this.itemsToBeDeleted.find(x => x.single !== undefined); - }, }, methods: { - ...mapActions(['fetchList', 'deleteItems']), + ...mapActions(['fetchList', 'deleteItem', 'multiDeleteItems']), + setModalDescription(itemsToDeleteLength, itemIndex) { + if (itemsToDeleteLength) { + this.modalDescription = sprintf( + s__(`ContainerRegistry|You are about to delete %{count} images. This will + delete the images and all tags pointing to them.`), + { count: itemsToDeleteLength }, + ); + } else { + const { tag } = this.repo.list[itemIndex]; + + this.modalDescription = sprintf( + s__(`ContainerRegistry|You are about to delete the image %{title}. 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); }, - addSingleItemToBeDeleted(index) { - this.itemsToBeDeleted.push({ single: index }); + removeModalEvents() { + this.$refs.deleteModal.$refs.modal.$off('ok'); + this.$refs.deleteModal.$refs.modal.$off('hide'); }, - removeSingleItemToBeDeleted() { - const singleIndex = this.itemsToBeDeleted.findIndex(x => x.single !== undefined); + deleteSingleItem(index) { + this.setModalDescription(0, index); - if (singleIndex > -1) { - this.itemsToBeDeleted.splice(singleIndex, 1); - } + this.$refs.deleteModal.$refs.modal.$once('ok', () => { + this.removeModalEvents(); + this.handleSingleDelete(this.repo.list[index]); + }); + + this.$refs.deleteModal.$refs.modal.$once('hide', this.removeModalEvents); }, - handleDeleteRegistry() { - let { itemsToBeDeleted } = this; - - if (this.findSingleRowToDelete) { - itemsToBeDeleted = [this.findSingleRowToDelete.single]; - } + deleteMultipleItems() { + this.$refs.deleteModal.$refs.modal.$once('ok', () => { + this.removeModalEvents(); + this.handleMultipleDelete(); + }); + this.$refs.deleteModal.$refs.modal.$once('hide', this.removeModalEvents); + }, + 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.deleteItems({ + this.multiDeleteItems({ path: this.bulkDeletePath, items: itemsToBeDeleted.map(x => this.repo.list[x].tag), }) @@ -142,6 +144,7 @@ export default { selectAll() { this.itemsToBeDeleted = this.repo.list.map((x, index) => index); this.selectAllChecked = true; + this.setModalDescription(this.itemsToBeDeleted.length); }, deselectAll() { this.itemsToBeDeleted = []; @@ -160,6 +163,12 @@ export default { this.selectAllChecked = true; } } + + if (this.itemsToBeDeleted.length === 1) { + this.setModalDescription(0, this.itemsToBeDeleted[0]); + } else if (this.itemsToBeDeleted.length > 1) { + this.setModalDescription(this.itemsToBeDeleted.length); + } }, }, }; @@ -191,13 +200,14 @@ export default { variant="danger" :title="s__('ContainerRegistry|Remove selected images')" :aria-label="s__('ContainerRegistry|Remove selected images')" + @click="deleteMultipleItems()" > - + @@ -257,12 +267,7 @@ export default { :page-info="repo.pagination" /> - +

diff --git a/app/assets/javascripts/registry/stores/actions.js b/app/assets/javascripts/registry/stores/actions.js index 4c20c003c5a..a2e0130e79e 100644 --- a/app/assets/javascripts/registry/stores/actions.js +++ b/app/assets/javascripts/registry/stores/actions.js @@ -36,7 +36,8 @@ export const fetchList = ({ commit }, { repo, page }) => { }; export const deleteItem = (_, item) => axios.delete(item.destroyPath); -export const deleteItems = (_, { path, items }) => axios.delete(path, { params: { ids: items } }); +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); diff --git a/app/assets/stylesheets/pages/container_registry.scss b/app/assets/stylesheets/pages/container_registry.scss index 6e6a3a1a394..0f4bdb219a3 100644 --- a/app/assets/stylesheets/pages/container_registry.scss +++ b/app/assets/stylesheets/pages/container_registry.scss @@ -32,7 +32,7 @@ .table.tags { margin-bottom: 0; - .image-row { + .registry-image-row { .check { padding-right: $gl-padding; width: 5%; diff --git a/spec/javascripts/registry/components/table_registry_spec.js b/spec/javascripts/registry/components/table_registry_spec.js index 2ca7aa30c0e..b6a37abe4f7 100644 --- a/spec/javascripts/registry/components/table_registry_spec.js +++ b/spec/javascripts/registry/components/table_registry_spec.js @@ -101,7 +101,7 @@ describe('table registry', () => { expect(findDeleteBtn().disabled).toBe(false); findDeleteBtn().click(); - spyOn(vm, 'deleteItems').and.returnValue(Promise.resolve()); + spyOn(vm, 'multiDeleteItems').and.returnValue(Promise.resolve()); Vue.nextTick(() => { const modal = confirmationModal(); @@ -111,7 +111,7 @@ describe('table registry', () => { Vue.nextTick(() => { expect(vm.itemsToBeDeleted).toEqual([]); - expect(vm.deleteItems).toHaveBeenCalledWith({ + expect(vm.multiDeleteItems).toHaveBeenCalledWith({ path: bulkDeletePath, items: [firstImage.tag, secondImage.tag], }); @@ -142,13 +142,13 @@ describe('table registry', () => { expect(vm.itemsToBeDeleted).toEqual([0]); expect(findDeleteBtn().disabled).toBe(false); findDeleteBtn().click(); - spyOn(vm, 'deleteItems').and.returnValue(Promise.resolve()); + spyOn(vm, 'multiDeleteItems').and.returnValue(Promise.resolve()); Vue.nextTick(() => { confirmationModal('.btn-danger').click(); expect(vm.itemsToBeDeleted).toEqual([]); - expect(vm.deleteItems).toHaveBeenCalledWith({ + expect(vm.multiDeleteItems).toHaveBeenCalledWith({ path: bulkDeletePath, items: [firstImage.tag], });