From 8a3f088398963b21703ebfb7e78a26fa2fb6f529 Mon Sep 17 00:00:00 2001
From: Paul Gascou-Vaillancourt <pgascouvaillancourt@gitlab.com>
Date: Fri, 21 Jun 2019 18:29:11 +0000
Subject: Add confirmation for registry image deletion

Adds a modal to let the user confirm their action before
proceeding to registry/image deletion
---
 app/assets/javascripts/registry/components/app.vue |  4 +-
 .../registry/components/collapsible_container.vue  | 31 ++++++++++--
 .../registry/components/table_registry.vue         | 58 +++++++++++++++++-----
 app/assets/javascripts/registry/stores/actions.js  |  6 +--
 .../unreleased/42399-registry-confirm-deletion.yml |  5 ++
 locale/gitlab.pot                                  | 13 ++++-
 spec/features/container_registry_spec.rb           |  4 ++
 .../components/collapsible_container_spec.js       | 22 +++++++-
 .../registry/components/table_registry_spec.js     | 30 ++++++++++-
 spec/javascripts/registry/stores/actions_spec.js   | 24 +++++++++
 10 files changed, 167 insertions(+), 30 deletions(-)
 create mode 100644 changelogs/unreleased/42399-registry-confirm-deletion.yml

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);
+    });
+  });
 });
-- 
cgit v1.2.1