diff options
author | Tim Zallmann <tzallmann@gitlab.com> | 2019-06-21 18:29:12 +0000 |
---|---|---|
committer | Tim Zallmann <tzallmann@gitlab.com> | 2019-06-21 18:29:12 +0000 |
commit | 0e9004a0c3118b5054b9d04c6a8786022c960741 (patch) | |
tree | 9fd945e6811b25be772c96f967e1394be9c399c1 | |
parent | 76f49de4e772c4101bcb8df801ad9b7a78adcea7 (diff) | |
parent | 8a3f088398963b21703ebfb7e78a26fa2fb6f529 (diff) | |
download | gitlab-ce-0e9004a0c3118b5054b9d04c6a8786022c960741.tar.gz |
Merge branch '42399-registry-confirm-deletion' into 'master'
Add confirmation for registry image deletion
Closes #42399
See merge request gitlab-org/gitlab-ce!29505
10 files changed, 167 insertions, 30 deletions
diff --git a/app/assets/javascripts/registry/components/app.vue b/app/assets/javascripts/registry/components/app.vue index 9af5660f764..ee973017387 100644 --- a/app/assets/javascripts/registry/components/app.vue +++ b/app/assets/javascripts/registry/components/app.vue @@ -33,7 +33,7 @@ export default { </script> <template> <div> - <gl-loading-icon v-if="isLoading" :size="3" /> + <gl-loading-icon v-if="isLoading" size="md" /> <collapsible-container v-for="item in repos" @@ -45,7 +45,7 @@ export default { <p v-else-if="!isLoading && !repos.length"> {{ __(`No container images stored for this project. -Add one by following the instructions above.`) + Add one by following the instructions above.`) }} </p> </div> diff --git a/app/assets/javascripts/registry/components/collapsible_container.vue b/app/assets/javascripts/registry/components/collapsible_container.vue index 5451c61026c..1e266dd4ced 100644 --- a/app/assets/javascripts/registry/components/collapsible_container.vue +++ b/app/assets/javascripts/registry/components/collapsible_container.vue @@ -1,6 +1,6 @@ <script> import { mapActions } from 'vuex'; -import { GlLoadingIcon, GlButton, GlTooltipDirective } from '@gitlab/ui'; +import { GlLoadingIcon, GlButton, GlTooltipDirective, GlModal, GlModalDirective } from '@gitlab/ui'; import createFlash from '../../flash'; import ClipboardButton from '../../vue_shared/components/clipboard_button.vue'; import Icon from '../../vue_shared/components/icon.vue'; @@ -16,9 +16,11 @@ export default { GlLoadingIcon, GlButton, Icon, + GlModal, }, directives: { GlTooltip: GlTooltipDirective, + GlModal: GlModalDirective, }, props: { repo: { @@ -37,7 +39,7 @@ export default { }, }, methods: { - ...mapActions(['fetchRepos', 'fetchList', 'deleteRepo']), + ...mapActions(['fetchRepos', 'fetchList', 'deleteItem']), toggleRepo() { this.isOpen = !this.isOpen; @@ -46,7 +48,7 @@ export default { } }, handleDeleteRepository() { - this.deleteRepo(this.repo) + this.deleteItem(this.repo) .then(() => { createFlash(__('This container registry has been scheduled for deletion.'), 'notice'); this.fetchRepos(); @@ -78,18 +80,18 @@ export default { <gl-button v-if="repo.canDelete" v-gl-tooltip + v-gl-modal="'confirm-repo-deletion-modal'" :title="s__('ContainerRegistry|Remove repository')" :aria-label="s__('ContainerRegistry|Remove repository')" class="js-remove-repo" variant="danger" - @click="handleDeleteRepository" > <icon name="remove" /> </gl-button> </div> </div> - <gl-loading-icon v-if="repo.isLoading" :size="2" class="append-bottom-20" /> + <gl-loading-icon v-if="repo.isLoading" size="md" class="append-bottom-20" /> <div v-else-if="!repo.isLoading && isOpen" class="container-image-tags"> <table-registry v-if="repo.list.length" :repo="repo" /> @@ -98,5 +100,24 @@ export default { {{ s__('ContainerRegistry|No tags in Container Registry for this container image.') }} </div> </div> + + <gl-modal + modal-id="confirm-repo-deletion-modal" + ok-variant="danger" + @ok="handleDeleteRepository" + > + <template v-slot:modal-title>{{ s__('ContainerRegistry|Remove repository') }}</template> + <p + v-html=" + sprintf( + s__( + 'ContainerRegistry|You are about to remove repository <b>%{title}</b>. Once you confirm, this repository will be permanently deleted.', + ), + { title: repo.name }, + ) + " + ></p> + <template v-slot:modal-ok>{{ __('Remove') }}</template> + </gl-modal> </div> </template> diff --git a/app/assets/javascripts/registry/components/table_registry.vue b/app/assets/javascripts/registry/components/table_registry.vue index f535b2ae9f2..0ec5e2c7a87 100644 --- a/app/assets/javascripts/registry/components/table_registry.vue +++ b/app/assets/javascripts/registry/components/table_registry.vue @@ -1,6 +1,6 @@ <script> import { mapActions } from 'vuex'; -import { GlButton, GlTooltipDirective } from '@gitlab/ui'; +import { GlButton, GlTooltipDirective, GlModal, GlModalDirective } from '@gitlab/ui'; import { n__ } from '../../locale'; import createFlash from '../../flash'; import ClipboardButton from '../../vue_shared/components/clipboard_button.vue'; @@ -16,9 +16,11 @@ export default { TablePagination, GlButton, Icon, + GlModal, }, directives: { GlTooltip: GlTooltipDirective, + GlModal: GlModalDirective, }, mixins: [timeagoMixin], props: { @@ -27,21 +29,31 @@ export default { required: true, }, }, + data() { + return { + itemToBeDeleted: null, + }; + }, computed: { shouldRenderPagination() { return this.repo.pagination.total > this.repo.pagination.perPage; }, }, methods: { - ...mapActions(['fetchList', 'deleteRegistry']), + ...mapActions(['fetchList', 'deleteItem']), layers(item) { return item.layers ? n__('%d layer', '%d layers', item.layers) : ''; }, formatSize(size) { return numberToHumanSize(size); }, - handleDeleteRegistry(registry) { - this.deleteRegistry(registry) + setItemToBeDeleted(item) { + this.itemToBeDeleted = item; + }, + handleDeleteRegistry() { + const { itemToBeDeleted } = this; + this.itemToBeDeleted = null; + this.deleteItem(itemToBeDeleted) .then(() => this.fetchList({ repo: this.repo })) .catch(() => this.showError(errorMessagesTypes.DELETE_REGISTRY)); }, @@ -80,9 +92,9 @@ export default { /> </td> <td> - <span v-gl-tooltip.bottom class="monospace" :title="item.revision">{{ - item.shortRevision - }}</span> + <span v-gl-tooltip.bottom class="monospace" :title="item.revision"> + {{ item.shortRevision }} + </span> </td> <td> {{ formatSize(item.size) }} @@ -93,20 +105,21 @@ export default { </td> <td> - <span v-gl-tooltip.bottom :title="tooltipTitle(item.createdAt)">{{ - timeFormated(item.createdAt) - }}</span> + <span v-gl-tooltip.bottom :title="tooltipTitle(item.createdAt)"> + {{ timeFormated(item.createdAt) }} + </span> </td> <td class="content"> <gl-button v-if="item.canDelete" v-gl-tooltip - :title="s__('ContainerRegistry|Remove tag')" - :aria-label="s__('ContainerRegistry|Remove tag')" + v-gl-modal="'confirm-image-deletion-modal'" + :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="handleDeleteRegistry(item)" + @click="setItemToBeDeleted(item)" > <icon name="remove" /> </gl-button> @@ -120,5 +133,24 @@ export default { :change="onPageChange" :page-info="repo.pagination" /> + + <gl-modal + modal-id="confirm-image-deletion-modal" + 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> </div> </template> diff --git a/app/assets/javascripts/registry/stores/actions.js b/app/assets/javascripts/registry/stores/actions.js index 51d057c62c1..0f5e9cc73a0 100644 --- a/app/assets/javascripts/registry/stores/actions.js +++ b/app/assets/javascripts/registry/stores/actions.js @@ -35,11 +35,7 @@ export const fetchList = ({ commit }, { repo, page }) => { }); }; -// eslint-disable-next-line no-unused-vars -export const deleteRepo = ({ commit }, repo) => axios.delete(repo.destroyPath); - -// eslint-disable-next-line no-unused-vars -export const deleteRegistry = ({ commit }, image) => axios.delete(image.destroyPath); +export const deleteItem = (_, item) => axios.delete(item.destroyPath); export const setMainEndpoint = ({ commit }, data) => commit(types.SET_MAIN_ENDPOINT, data); export const toggleLoading = ({ commit }) => commit(types.TOGGLE_MAIN_LOADING); diff --git a/changelogs/unreleased/42399-registry-confirm-deletion.yml b/changelogs/unreleased/42399-registry-confirm-deletion.yml new file mode 100644 index 00000000000..4d720e16721 --- /dev/null +++ b/changelogs/unreleased/42399-registry-confirm-deletion.yml @@ -0,0 +1,5 @@ +--- +title: Add confirmation for registry image deletion +merge_request: 29505 +author: +type: added diff --git a/locale/gitlab.pot b/locale/gitlab.pot index eb76e62c9f5..ed06389cf05 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2849,10 +2849,13 @@ msgstr "" msgid "ContainerRegistry|Once you log in, you’re free to create and upload a container image using the common %{build} and %{push} commands" msgstr "" -msgid "ContainerRegistry|Remove repository" +msgid "ContainerRegistry|Remove image" +msgstr "" + +msgid "ContainerRegistry|Remove image and tags" msgstr "" -msgid "ContainerRegistry|Remove tag" +msgid "ContainerRegistry|Remove repository" msgstr "" msgid "ContainerRegistry|Size" @@ -2870,6 +2873,12 @@ msgstr "" msgid "ContainerRegistry|With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images." 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 "" + +msgid "ContainerRegistry|You are about to remove repository <b>%{title}</b>. Once you confirm, this repository will be permanently deleted." +msgstr "" + msgid "ContainerRegistry|You can also use a %{deploy_token} for read-only access to the registry images." msgstr "" diff --git a/spec/features/container_registry_spec.rb b/spec/features/container_registry_spec.rb index 6f9901815e1..21d97aba0c5 100644 --- a/spec/features/container_registry_spec.rb +++ b/spec/features/container_registry_spec.rb @@ -42,6 +42,8 @@ describe "Container Registry", :js do .to receive(:delete_tags!).and_return(true) click_on(class: 'js-remove-repo') + expect(find('.modal .modal-title')).to have_content 'Remove repository' + find('.modal .modal-footer .btn-danger').click end it 'user removes a specific tag from container repository' do @@ -54,6 +56,8 @@ describe "Container Registry", :js do .to receive(:delete).and_return(true) click_on(class: 'js-delete-registry') + expect(find('.modal .modal-title')).to have_content 'Remove image' + find('.modal .modal-footer .btn-danger').click end end diff --git a/spec/javascripts/registry/components/collapsible_container_spec.js b/spec/javascripts/registry/components/collapsible_container_spec.js index a3f7ff76dc7..9ed4b04324a 100644 --- a/spec/javascripts/registry/components/collapsible_container_spec.js +++ b/spec/javascripts/registry/components/collapsible_container_spec.js @@ -12,6 +12,8 @@ describe('collapsible registry container', () => { let mock; const Component = Vue.extend(collapsibleComponent); + const findDeleteBtn = () => vm.$el.querySelector('.js-remove-repo'); + beforeEach(() => { mock = new MockAdapter(axios); @@ -67,7 +69,25 @@ describe('collapsible registry container', () => { describe('delete repo', () => { it('should be possible to delete a repo', () => { - expect(vm.$el.querySelector('.js-remove-repo')).not.toBeNull(); + expect(findDeleteBtn()).not.toBeNull(); + }); + + describe('clicked on delete', () => { + beforeEach(done => { + findDeleteBtn().click(); + Vue.nextTick(done); + }); + + it('should open confirmation modal', () => { + expect(vm.$el.querySelector('#confirm-repo-deletion-modal')).not.toBeNull(); + }); + + it('should call deleteItem when confirming deletion', () => { + spyOn(vm, 'deleteItem').and.returnValue(Promise.resolve()); + vm.$el.querySelector('#confirm-repo-deletion-modal .btn-danger').click(); + + expect(vm.deleteItem).toHaveBeenCalledWith(vm.repo); + }); }); }); }); diff --git a/spec/javascripts/registry/components/table_registry_spec.js b/spec/javascripts/registry/components/table_registry_spec.js index 7f5252a7d6c..d366c67a1b9 100644 --- a/spec/javascripts/registry/components/table_registry_spec.js +++ b/spec/javascripts/registry/components/table_registry_spec.js @@ -3,10 +3,14 @@ import tableRegistry from '~/registry/components/table_registry.vue'; import store from '~/registry/stores'; import { repoPropsData } from '../mock_data'; +const [firstImage] = repoPropsData.list; + describe('table registry', () => { let vm; let Component; + const findDeleteBtn = () => vm.$el.querySelector('.js-delete-registry'); + beforeEach(() => { Component = Vue.extend(tableRegistry); vm = new Component({ @@ -37,8 +41,30 @@ describe('table registry', () => { expect(textRendered).toContain(repoPropsData.list[0].size); }); - it('should be possible to delete a registry', () => { - expect(vm.$el.querySelector('.table tbody tr .js-delete-registry')).toBeDefined(); + describe('delete registry', () => { + it('should be possible to delete a registry', () => { + expect(findDeleteBtn()).toBeDefined(); + }); + + describe('clicked on delete', () => { + beforeEach(done => { + findDeleteBtn().click(); + Vue.nextTick(done); + }); + + it('should open confirmation modal and set itemToBeDeleted properly', () => { + expect(vm.itemToBeDeleted).toEqual(firstImage); + expect(vm.$el.querySelector('#confirm-image-deletion-modal')).not.toBeNull(); + }); + + it('should call deleteItem and reset itemToBeDeleted when confirming deletion', () => { + spyOn(vm, 'deleteItem').and.returnValue(Promise.resolve()); + vm.$el.querySelector('#confirm-image-deletion-modal .btn-danger').click(); + + expect(vm.deleteItem).toHaveBeenCalledWith(firstImage); + expect(vm.itemToBeDeleted).toBeNull(); + }); + }); }); describe('pagination', () => { diff --git a/spec/javascripts/registry/stores/actions_spec.js b/spec/javascripts/registry/stores/actions_spec.js index c9aa82dba90..0613ec8e0f1 100644 --- a/spec/javascripts/registry/stores/actions_spec.js +++ b/spec/javascripts/registry/stores/actions_spec.js @@ -105,4 +105,28 @@ describe('Actions Registry Store', () => { ); }); }); + + describe('deleteItem', () => { + it('should perform DELETE request on destroyPath', done => { + const destroyPath = `${TEST_HOST}/mygroup/myproject/container_registry/1.json`; + let deleted = false; + mock.onDelete(destroyPath).replyOnce(() => { + deleted = true; + return [200]; + }); + testAction( + actions.deleteItem, + { + destroyPath, + }, + mockedState, + ) + .then(() => { + expect(mock.history.delete.length).toBe(1); + expect(deleted).toBe(true); + done(); + }) + .catch(done.fail); + }); + }); }); |