From 51b04b5f2273284c674a8813a4c5da13825b431e Mon Sep 17 00:00:00 2001 From: Nick Kipling Date: Fri, 12 Jul 2019 10:27:23 +0100 Subject: Implement multi select deletion for container registry Added checkboxes to each image row Added delete selected images button Changed row delete button to appear on row hover Changed confirmation modal message Changed delete logic to support multi Added tests for multi select Updated pot file Updated rspec test for new functionality --- .../registry/components/table_registry_spec.js | 115 +++++++++++++++++++-- spec/javascripts/registry/mock_data.js | 11 ++ 2 files changed, 115 insertions(+), 11 deletions(-) (limited to 'spec/javascripts') diff --git a/spec/javascripts/registry/components/table_registry_spec.js b/spec/javascripts/registry/components/table_registry_spec.js index 31ac970378e..9ee326325e0 100644 --- a/spec/javascripts/registry/components/table_registry_spec.js +++ b/spec/javascripts/registry/components/table_registry_spec.js @@ -3,15 +3,19 @@ import tableRegistry from '~/registry/components/table_registry.vue'; import store from '~/registry/stores'; import { repoPropsData } from '../mock_data'; -const [firstImage] = repoPropsData.list; +const [firstImage, secondImage] = repoPropsData.list; describe('table registry', () => { let vm; let Component; 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')); - beforeEach(() => { + const createComponent = () => { Component = Vue.extend(tableRegistry); vm = new Component({ store, @@ -19,6 +23,10 @@ describe('table registry', () => { repo: repoPropsData, }, }).$mount(); + }; + + beforeEach(() => { + createComponent(); }); afterEach(() => { @@ -41,23 +49,108 @@ describe('table registry', () => { expect(textRendered).toContain(repoPropsData.list[0].size); }); - describe('delete registry', () => { - it('should be possible to delete a registry', () => { - expect(findDeleteBtn()).toBeDefined(); + describe('multi select', () => { + beforeEach(() => { + vm.itemsToBeDeleted = []; + }); + + it('should support multiselect and selecting a row should enable delete button', done => { + findSelectAllCheckbox().click(); + + vm.selectAll(); + + expect(findSelectAllCheckbox().checked).toBe(true); + + Vue.nextTick(() => { + expect(findDeleteBtn().disabled).toBe(false); + done(); + }); }); - it('should call deleteItem and reset itemToBeDeleted when confirming deletion', done => { - findDeleteBtn().click(); - spyOn(vm, 'deleteItem').and.returnValue(Promise.resolve()); + it('selecting all checkbox should select all rows and enable delete button', done => { + findSelectAllCheckbox().click(); + vm.selectAll(); Vue.nextTick(() => { - document.querySelector(`#${vm.modalId} .btn-danger`).click(); + const checkedValues = findAllRowCheckboxes().filter(x => x.checked); - expect(vm.deleteItem).toHaveBeenCalledWith(firstImage); - expect(vm.itemToBeDeleted).toBeNull(); + expect(checkedValues.length).toBe(repoPropsData.list.length); done(); }); }); + + it('deselecting select all checkbox should deselect all rows and disable delete button', done => { + findSelectAllCheckbox().click(); + vm.selectAll(); // Select them all on + vm.selectAll(); // Select them all off + + 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 => { + findSelectAllCheckbox().click(); + vm.selectAll(); + + Vue.nextTick(() => { + expect(vm.itemsToBeDeleted).toEqual([0, 1]); + expect(findDeleteBtn().disabled).toBe(false); + + findDeleteBtn().click(); + spyOn(vm, 'deleteItem').and.returnValue(Promise.resolve()); + + Vue.nextTick(() => { + const modal = document.querySelector(`#${vm.modalId}`); + document.querySelector(`#${vm.modalId} .btn-danger`).click(); + + expect(modal).toExist(); + + Vue.nextTick(() => { + expect(vm.itemsToBeDeleted).toEqual([]); + expect(vm.deleteItem).toHaveBeenCalledWith(firstImage); + expect(vm.deleteItem).toHaveBeenCalledWith(secondImage); + done(); + }); + }); + }); + }); + }); + + describe('delete 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 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()); + + Vue.nextTick(() => { + document.querySelector(`#${vm.modalId} .btn-danger`).click(); + + expect(vm.itemsToBeDeleted).toEqual([]); + expect(vm.deleteItem).toHaveBeenCalledWith(firstImage); + done(); + }); + }); + }); }); describe('pagination', () => { diff --git a/spec/javascripts/registry/mock_data.js b/spec/javascripts/registry/mock_data.js index 22db203e77f..130ab298e89 100644 --- a/spec/javascripts/registry/mock_data.js +++ b/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', -- cgit v1.2.1 From b5d50025fc8bc232de1a82ccb2d13906a96cd829 Mon Sep 17 00:00:00 2001 From: Nick Kipling Date: Thu, 18 Jul 2019 10:22:53 +0100 Subject: Updating table_registry tests --- .../registry/components/table_registry_spec.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'spec/javascripts') diff --git a/spec/javascripts/registry/components/table_registry_spec.js b/spec/javascripts/registry/components/table_registry_spec.js index 9ee326325e0..9dca29d4702 100644 --- a/spec/javascripts/registry/components/table_registry_spec.js +++ b/spec/javascripts/registry/components/table_registry_spec.js @@ -8,6 +8,7 @@ const [firstImage, secondImage] = repoPropsData.list; describe('table registry', () => { let vm; let Component; + const bulkDeletePath = 'path'; const findDeleteBtn = () => vm.$el.querySelector('.js-delete-registry'); const findDeleteBtnRow = () => vm.$el.querySelector('.js-delete-registry-row'); @@ -101,7 +102,7 @@ describe('table registry', () => { expect(findDeleteBtn().disabled).toBe(false); findDeleteBtn().click(); - spyOn(vm, 'deleteItem').and.returnValue(Promise.resolve()); + spyOn(vm, 'deleteItems').and.returnValue(Promise.resolve()); Vue.nextTick(() => { const modal = document.querySelector(`#${vm.modalId}`); @@ -111,8 +112,10 @@ describe('table registry', () => { Vue.nextTick(() => { expect(vm.itemsToBeDeleted).toEqual([]); - expect(vm.deleteItem).toHaveBeenCalledWith(firstImage); - expect(vm.deleteItem).toHaveBeenCalledWith(secondImage); + expect(vm.deleteItems).toHaveBeenCalledWith({ + path: bulkDeletePath, + items: [firstImage.tag, secondImage.tag], + }); done(); }); }); @@ -135,18 +138,21 @@ describe('table registry', () => { }); }); - it('should call deleteItem and reset itemsToBeDeleted 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, 'deleteItems').and.returnValue(Promise.resolve()); Vue.nextTick(() => { document.querySelector(`#${vm.modalId} .btn-danger`).click(); expect(vm.itemsToBeDeleted).toEqual([]); - expect(vm.deleteItem).toHaveBeenCalledWith(firstImage); + expect(vm.deleteItems).toHaveBeenCalledWith({ + path: bulkDeletePath, + items: [firstImage.tag], + }); done(); }); }); -- cgit v1.2.1 From 237f434ce83488f252b5f593c67ecaf76ceb9e86 Mon Sep 17 00:00:00 2001 From: Nick Kipling Date: Thu, 18 Jul 2019 17:56:09 +0100 Subject: Updating with suggestions as per review --- .../registry/components/table_registry_spec.js | 83 ++++++++++++++-------- 1 file changed, 53 insertions(+), 30 deletions(-) (limited to 'spec/javascripts') diff --git a/spec/javascripts/registry/components/table_registry_spec.js b/spec/javascripts/registry/components/table_registry_spec.js index 9dca29d4702..556a66c5666 100644 --- a/spec/javascripts/registry/components/table_registry_spec.js +++ b/spec/javascripts/registry/components/table_registry_spec.js @@ -1,13 +1,14 @@ 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, 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'); @@ -15,17 +16,19 @@ describe('table registry', () => { 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}`); const createComponent = () => { - Component = Vue.extend(tableRegistry); - vm = new Component({ + vm = mountComponentWithStore(Component, { store, - propsData: { + props: { repo: repoPropsData, }, - }).$mount(); + }); }; + const toggleSelectAll = () => vm.selectAll(); + beforeEach(() => { createComponent(); }); @@ -34,31 +37,28 @@ describe('table registry', () => { vm.$destroy(); }); - it('should render a table with the registry list', () => { - expect(vm.$el.querySelectorAll('table tbody tr').length).toEqual(repoPropsData.list.length); - }); + describe('rendering', () => { + it('should render a table with the registry list', () => { + expect(vm.$el.querySelectorAll('table tbody tr').length).toEqual(repoPropsData.list.length); + }); - it('should render registry tag', () => { - const textRendered = vm.$el - .querySelector('.table tbody tr') - .textContent.trim() - .replace(/\s\s+/g, ' '); + it('should render registry tag', () => { + const textRendered = vm.$el + .querySelector('.table tbody tr') + .textContent.trim() + .replace(/\s\s+/g, ' '); - expect(textRendered).toContain(repoPropsData.list[0].tag); - expect(textRendered).toContain(repoPropsData.list[0].shortRevision); - expect(textRendered).toContain(repoPropsData.list[0].layers); - expect(textRendered).toContain(repoPropsData.list[0].size); + expect(textRendered).toContain(repoPropsData.list[0].tag); + expect(textRendered).toContain(repoPropsData.list[0].shortRevision); + expect(textRendered).toContain(repoPropsData.list[0].layers); + expect(textRendered).toContain(repoPropsData.list[0].size); + }); }); describe('multi select', () => { - beforeEach(() => { - vm.itemsToBeDeleted = []; - }); - it('should support multiselect and selecting a row should enable delete button', done => { findSelectAllCheckbox().click(); - - vm.selectAll(); + toggleSelectAll(); expect(findSelectAllCheckbox().checked).toBe(true); @@ -70,7 +70,7 @@ describe('table registry', () => { it('selecting all checkbox should select all rows and enable delete button', done => { findSelectAllCheckbox().click(); - vm.selectAll(); + toggleSelectAll(); Vue.nextTick(() => { const checkedValues = findAllRowCheckboxes().filter(x => x.checked); @@ -82,8 +82,8 @@ describe('table registry', () => { it('deselecting select all checkbox should deselect all rows and disable delete button', done => { findSelectAllCheckbox().click(); - vm.selectAll(); // Select them all on - vm.selectAll(); // Select them all off + toggleSelectAll(); // Select them all on + toggleSelectAll(); // Select them all off Vue.nextTick(() => { const checkedValues = findAllRowCheckboxes().filter(x => x.checked); @@ -95,7 +95,7 @@ describe('table registry', () => { it('should delete multiple items when multiple items are selected', done => { findSelectAllCheckbox().click(); - vm.selectAll(); + toggleSelectAll(); Vue.nextTick(() => { expect(vm.itemsToBeDeleted).toEqual([0, 1]); @@ -105,8 +105,8 @@ describe('table registry', () => { spyOn(vm, 'deleteItems').and.returnValue(Promise.resolve()); Vue.nextTick(() => { - const modal = document.querySelector(`#${vm.modalId}`); - document.querySelector(`#${vm.modalId} .btn-danger`).click(); + const modal = confirmationModal(); + confirmationModal('.btn-danger').click(); expect(modal).toExist(); @@ -146,7 +146,7 @@ describe('table registry', () => { spyOn(vm, 'deleteItems').and.returnValue(Promise.resolve()); Vue.nextTick(() => { - document.querySelector(`#${vm.modalId} .btn-danger`).click(); + confirmationModal('.btn-danger').click(); expect(vm.itemsToBeDeleted).toEqual([]); expect(vm.deleteItems).toHaveBeenCalledWith({ @@ -164,4 +164,27 @@ describe('table registry', () => { 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 => { + findSelectAllCheckbox().click(); + toggleSelectAll(); + + Vue.nextTick(() => { + expect(vm.modalTitle).toBe('Remove images'); + expect(vm.modalDescription).toContain('2 images'); + done(); + }); + }); + }); }); -- cgit v1.2.1 From 786133d31434d1dbb185b2c0ff5eee663f5841d5 Mon Sep 17 00:00:00 2001 From: Nick Kipling Date: Fri, 19 Jul 2019 16:12:17 +0100 Subject: Updated select all to be more explicit --- .../registry/components/table_registry_spec.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) (limited to 'spec/javascripts') diff --git a/spec/javascripts/registry/components/table_registry_spec.js b/spec/javascripts/registry/components/table_registry_spec.js index 556a66c5666..a96a505a3a4 100644 --- a/spec/javascripts/registry/components/table_registry_spec.js +++ b/spec/javascripts/registry/components/table_registry_spec.js @@ -27,7 +27,8 @@ describe('table registry', () => { }); }; - const toggleSelectAll = () => vm.selectAll(); + const selectAllCheckboxes = () => vm.selectAll(); + const deselectAllCheckboxes = () => vm.deselectAll(); beforeEach(() => { createComponent(); @@ -58,7 +59,7 @@ describe('table registry', () => { describe('multi select', () => { it('should support multiselect and selecting a row should enable delete button', done => { findSelectAllCheckbox().click(); - toggleSelectAll(); + selectAllCheckboxes(); expect(findSelectAllCheckbox().checked).toBe(true); @@ -69,8 +70,7 @@ describe('table registry', () => { }); it('selecting all checkbox should select all rows and enable delete button', done => { - findSelectAllCheckbox().click(); - toggleSelectAll(); + selectAllCheckboxes(); Vue.nextTick(() => { const checkedValues = findAllRowCheckboxes().filter(x => x.checked); @@ -81,9 +81,8 @@ describe('table registry', () => { }); it('deselecting select all checkbox should deselect all rows and disable delete button', done => { - findSelectAllCheckbox().click(); - toggleSelectAll(); // Select them all on - toggleSelectAll(); // Select them all off + selectAllCheckboxes(); + deselectAllCheckboxes(); Vue.nextTick(() => { const checkedValues = findAllRowCheckboxes().filter(x => x.checked); @@ -94,8 +93,7 @@ describe('table registry', () => { }); it('should delete multiple items when multiple items are selected', done => { - findSelectAllCheckbox().click(); - toggleSelectAll(); + selectAllCheckboxes(); Vue.nextTick(() => { expect(vm.itemsToBeDeleted).toEqual([0, 1]); @@ -177,8 +175,7 @@ describe('table registry', () => { }); it('should show the plural title and image count when deleting more than one image', done => { - findSelectAllCheckbox().click(); - toggleSelectAll(); + selectAllCheckboxes(); Vue.nextTick(() => { expect(vm.modalTitle).toBe('Remove images'); -- cgit v1.2.1 From a37d672ff5c4c102a5b507ad744919748cbdcb34 Mon Sep 17 00:00:00 2001 From: Nick Kipling Date: Wed, 24 Jul 2019 19:38:39 +0100 Subject: Applying feedback changes Updated table registry to remove singleItemToBeDeleted Renamed usages of idx to index Tidied and simplified css styling Added clarification comment to test regex Updated pot file --- spec/javascripts/registry/components/table_registry_spec.js | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/javascripts') diff --git a/spec/javascripts/registry/components/table_registry_spec.js b/spec/javascripts/registry/components/table_registry_spec.js index a96a505a3a4..2ca7aa30c0e 100644 --- a/spec/javascripts/registry/components/table_registry_spec.js +++ b/spec/javascripts/registry/components/table_registry_spec.js @@ -47,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); -- cgit v1.2.1 From 918e7d43dfe614475ee2dd2b6f4c765726db6ef4 Mon Sep 17 00:00:00 2001 From: Nick Kipling Date: Fri, 26 Jul 2019 19:42:49 +0100 Subject: 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 --- spec/javascripts/registry/components/table_registry_spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'spec/javascripts') 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], }); -- cgit v1.2.1 From 128a04ef0ec10e4524e138a61143d0d1ba1f54ac Mon Sep 17 00:00:00 2001 From: Nick Kipling Date: Tue, 30 Jul 2019 09:00:30 +0100 Subject: Adjustments to event removal and modal description --- spec/javascripts/registry/components/table_registry_spec.js | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/javascripts') diff --git a/spec/javascripts/registry/components/table_registry_spec.js b/spec/javascripts/registry/components/table_registry_spec.js index b6a37abe4f7..9c7439206ef 100644 --- a/spec/javascripts/registry/components/table_registry_spec.js +++ b/spec/javascripts/registry/components/table_registry_spec.js @@ -177,6 +177,7 @@ describe('table registry', () => { 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'); -- cgit v1.2.1