summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTim Zallmann <tzallmann@gitlab.com>2019-06-21 18:29:12 +0000
committerTim Zallmann <tzallmann@gitlab.com>2019-06-21 18:29:12 +0000
commit0e9004a0c3118b5054b9d04c6a8786022c960741 (patch)
tree9fd945e6811b25be772c96f967e1394be9c399c1
parent76f49de4e772c4101bcb8df801ad9b7a78adcea7 (diff)
parent8a3f088398963b21703ebfb7e78a26fa2fb6f529 (diff)
downloadgitlab-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
-rw-r--r--app/assets/javascripts/registry/components/app.vue4
-rw-r--r--app/assets/javascripts/registry/components/collapsible_container.vue31
-rw-r--r--app/assets/javascripts/registry/components/table_registry.vue58
-rw-r--r--app/assets/javascripts/registry/stores/actions.js6
-rw-r--r--changelogs/unreleased/42399-registry-confirm-deletion.yml5
-rw-r--r--locale/gitlab.pot13
-rw-r--r--spec/features/container_registry_spec.rb4
-rw-r--r--spec/javascripts/registry/components/collapsible_container_spec.js22
-rw-r--r--spec/javascripts/registry/components/table_registry_spec.js30
-rw-r--r--spec/javascripts/registry/stores/actions_spec.js24
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&rsquo;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);
+ });
+ });
});