Merge branch 'move-packages-to-axios' into 'master'

Move package deletion from rails-js to axios

See merge request gitlab-org/gitlab!41668

app/assets/javascripts/packages/details/components/app.vue

@@ -25,8 +25,9 @@ import { numberToHumanSize } from '~/lib/utils/number_utils';
 import timeagoMixin from '~/vue_shared/mixins/timeago';
 import FileIcon from '~/vue_shared/components/file_icon.vue';
 import { __, s__ } from '~/locale';
-import { PackageType, TrackingActions } from '../../shared/constants';
+import { PackageType, TrackingActions, SHOW_DELETE_SUCCESS_ALERT } from '../../shared/constants';
 import { packageTypeToTrackCategory } from '../../shared/utils';
+import { objectToQueryString } from '~/lib/utils/common_utils';
 
 export default {
   name: 'PackagesApp',
@@ -62,17 +63,15 @@ export default {
       'packageFiles',
       'isLoading',
       'canDelete',
-      'destroyPath',
       'svgPath',
       'npmPath',
       'npmHelpPath',
+      'projectListUrl',
+      'groupListUrl',
     ]),
     isValidPackage() {
       return Boolean(this.packageEntity.name);
     },
-    canDeletePackage() {
-      return this.canDelete && this.destroyPath;
-    },
     filesTableRows() {
       return this.packageFiles.map(x => ({
         name: x.file_name,
@@ -100,7 +99,7 @@ export default {
     },
   },
   methods: {
-    ...mapActions(['fetchPackageVersions']),
+    ...mapActions(['deletePackage', 'fetchPackageVersions']),
     formatSize(size) {
       return numberToHumanSize(size);
     },
@@ -112,6 +111,16 @@ export default {
         this.fetchPackageVersions();
       }
     },
+    async confirmPackageDeletion() {
+      this.track(TrackingActions.DELETE_PACKAGE);
+      await this.deletePackage();
+      const returnTo =
+        !this.groupListUrl || document.referrer.includes(this.projectName)
+          ? this.projectListUrl
+          : this.groupListUrl; // to avoid security issue url are supplied from backend
+      const modalQuery = objectToQueryString({ [SHOW_DELETE_SUCCESS_ALERT]: true });
+      window.location.replace(`${returnTo}?${modalQuery}`);
+    },
   },
   i18n: {
     deleteModalTitle: s__(`PackageRegistry|Delete Package Version`),
@@ -150,7 +159,7 @@ export default {
     <package-title>
       <template #delete-button>
         <gl-button
-          v-if="canDeletePackage"
+          v-if="canDelete"
           v-gl-modal="'delete-modal'"
           class="js-delete-button"
           variant="danger"
@@ -272,12 +281,10 @@ export default {
             <gl-button @click="cancelDelete">{{ __('Cancel') }}</gl-button>
             <gl-button
               ref="modal-delete-button"
-              data-method="delete"
-              :to="destroyPath"
               variant="danger"
               category="primary"
               data-qa-selector="delete_modal_button"
-              @click="track($options.trackingActions.DELETE_PACKAGE)"
+              @click="confirmPackageDeletion"
             >
               {{ __('Delete') }}
             </gl-button>

app/assets/javascripts/packages/details/store/actions.js

 import Api from '~/api';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
 import { FETCH_PACKAGE_VERSIONS_ERROR } from '../constants';
+import { DELETE_PACKAGE_ERROR_MESSAGE } from '~/packages/shared/constants';
 import * as types from './mutation_types';
 
-export default ({ commit, state }) => {
+export const fetchPackageVersions = ({ commit, state }) => {
   commit(types.SET_LOADING, true);
 
   const { project_id, id } = state.packageEntity;
@@ -21,3 +22,13 @@ export default ({ commit, state }) => {
       commit(types.SET_LOADING, false);
     });
 };
+
+export const deletePackage = ({
+  state: {
+    packageEntity: { project_id, id },
+  },
+}) => {
+  return Api.deleteProjectPackage(project_id, id).catch(() => {
+    createFlash(DELETE_PACKAGE_ERROR_MESSAGE);
+  });
+};

app/assets/javascripts/packages/details/store/index.js

 import Vue from 'vue';
 import Vuex from 'vuex';
-import fetchPackageVersions from './actions';
+import * as actions from './actions';
 import * as getters from './getters';
 import mutations from './mutations';
 
@@ -8,9 +8,7 @@ Vue.use(Vuex);
 
 export default (initialState = {}) =>
   new Vuex.Store({
-    actions: {
-      fetchPackageVersions,
-    },
+    actions,
     getters,
     mutations,
     state: {

app/assets/javascripts/packages/list/components/packages_list_app.vue

@@ -2,11 +2,14 @@
 import { mapActions, mapState } from 'vuex';
 import { GlEmptyState, GlTab, GlTabs, GlLink, GlSprintf } from '@gitlab/ui';
 import { s__, sprintf } from '~/locale';
+import createFlash from '~/flash';
 import PackageFilter from './packages_filter.vue';
 import PackageList from './packages_list.vue';
 import PackageSort from './packages_sort.vue';
-import { PACKAGE_REGISTRY_TABS } from '../constants';
+import { PACKAGE_REGISTRY_TABS, DELETE_PACKAGE_SUCCESS_MESSAGE } from '../constants';
 import PackagesComingSoon from '../coming_soon/packages_coming_soon.vue';
+import { SHOW_DELETE_SUCCESS_ALERT } from '~/packages/shared/constants';
+import { historyReplaceState } from '~/lib/utils/common_utils';
 
 export default {
   components: {
@@ -34,6 +37,7 @@ export default {
   },
   mounted() {
     this.requestPackagesList();
+    this.checkDeleteAlert();
   },
   methods: {
     ...mapActions(['requestPackagesList', 'requestDeletePackage', 'setSelectedType']),
@@ -64,6 +68,16 @@ export default {
 
       return s__('PackageRegistry|There are no packages yet');
     },
+    checkDeleteAlert() {
+      const urlParams = new URLSearchParams(window.location.search);
+      const showAlert = urlParams.get(SHOW_DELETE_SUCCESS_ALERT);
+      if (showAlert) {
+        // to be refactored to use gl-alert
+        createFlash({ message: DELETE_PACKAGE_SUCCESS_MESSAGE, type: 'notice' });
+        const cleanUrl = window.location.href.split('?')[0];
+        historyReplaceState(cleanUrl);
+      }
+    },
   },
   i18n: {
     widenFilters: s__('PackageRegistry|To widen your search, change or remove the filters above.'),

app/assets/javascripts/packages/list/constants.js

@@ -5,7 +5,6 @@ export const FETCH_PACKAGES_LIST_ERROR_MESSAGE = __(
   'Something went wrong while fetching the packages list.',
 );
 export const FETCH_PACKAGE_ERROR_MESSAGE = __('Something went wrong while fetching the package.');
-export const DELETE_PACKAGE_ERROR_MESSAGE = __('Something went wrong while deleting the package.');
 export const DELETE_PACKAGE_SUCCESS_MESSAGE = __('Package deleted successfully');
 
 export const DEFAULT_PAGE = 1;

app/assets/javascripts/packages/list/stores/actions.js

 import Api from '~/api';
 import axios from '~/lib/utils/axios_utils';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
+import { DELETE_PACKAGE_ERROR_MESSAGE } from '~/packages/shared/constants';
 import * as types from './mutation_types';
 import {
   FETCH_PACKAGES_LIST_ERROR_MESSAGE,
-  DELETE_PACKAGE_ERROR_MESSAGE,
   DELETE_PACKAGE_SUCCESS_MESSAGE,
   DEFAULT_PAGE,
   DEFAULT_PAGE_SIZE,

app/assets/javascripts/packages/shared/constants.js

+import { __ } from '~/locale';
+
 export const PackageType = {
   CONAN: 'conan',
   MAVEN: 'maven',
@@ -22,3 +24,6 @@ export const TrackingCategories = {
   [PackageType.NPM]: 'NpmPackages',
   [PackageType.CONAN]: 'ConanPackages',
 };
+
+export const SHOW_DELETE_SUCCESS_ALERT = 'showSuccessDeleteAlert';
+export const DELETE_PACKAGE_ERROR_MESSAGE = __('Something went wrong while deleting the package.');

app/views/projects/packages/packages/show.html.haml

@@ -8,7 +8,6 @@
   .col-12
     #js-vue-packages-detail{ data: { package: package_from_presenter(@package),
       can_delete: can?(current_user, :destroy_package, @project).to_s,
-      destroy_path: project_package_path(@project, @package),
       svg_path: image_path('illustrations/no-packages.svg'),
       npm_path: package_registry_instance_url(:npm),
       npm_help_path: help_page_path('user/packages/npm_registry/index'),
@@ -23,4 +22,6 @@
       pypi_help_path: help_page_path('user/packages/pypi_repository/index'),
       composer_path: composer_registry_url(@project&.group&.id),
       composer_help_path: help_page_path('user/packages/composer_repository/index'),
-      project_name: @project.name} }
+      project_name: @project.name,
+      project_list_url: project_packages_path(@project),
+      group_list_url: @project.group ? group_packages_path(@project.group) : ''} }

spec/frontend/packages/details/components/app_spec.js

@@ -34,12 +34,15 @@ describe('PackagesApp', () => {
   let wrapper;
   let store;
   const fetchPackageVersions = jest.fn();
+  const deletePackage = jest.fn();
+  const defaultProjectName = 'bar';
+  const { location } = window;
 
   function createComponent({
     packageEntity = mavenPackage,
     packageFiles = mavenFiles,
     isLoading = false,
-    oneColumnView = false,
+    projectName = defaultProjectName,
   } = {}) {
     store = new Vuex.Store({
       state: {
@@ -47,14 +50,15 @@ describe('PackagesApp', () => {
         packageEntity,
         packageFiles,
         canDelete: true,
-        destroyPath: 'destroy-package-path',
         emptySvgPath: 'empty-illustration',
         npmPath: 'foo',
         npmHelpPath: 'foo',
-        projectName: 'bar',
-        oneColumnView,
+        projectName,
+        projectListUrl: 'project_url',
+        groupListUrl: 'group_url',
       },
       actions: {
+        deletePackage,
         fetchPackageVersions,
       },
       getters,
@@ -95,8 +99,14 @@ describe('PackagesApp', () => {
   const findAdditionalMetadata = () => wrapper.find(AdditionalMetadata);
   const findInstallationCommands = () => wrapper.find(InstallationCommands);
 
+  beforeEach(() => {
+    delete window.location;
+    window.location = { replace: jest.fn() };
+  });
+
   afterEach(() => {
     wrapper.destroy();
+    window.location = location;
   });
 
   it('renders the app and displays the package title', () => {
@@ -240,44 +250,94 @@ describe('PackagesApp', () => {
     });
   });
 
-  describe('tracking', () => {
-    let eventSpy;
-    let utilSpy;
-    const category = 'foo';
+  describe('tracking and delete', () => {
+    const doDelete = async () => {
+      deleteButton().trigger('click');
+      await wrapper.vm.$nextTick();
+      modalDeleteButton().trigger('click');
+    };
+
+    describe('delete', () => {
+      const originalReferrer = document.referrer;
+      const setReferrer = (value = defaultProjectName) => {
+        Object.defineProperty(document, 'referrer', {
+          value,
+          configurable: true,
+        });
+      };
+
+      afterEach(() => {
+        Object.defineProperty(document, 'referrer', {
+          value: originalReferrer,
+          configurable: true,
+        });
+      });
 
-    beforeEach(() => {
-      eventSpy = jest.spyOn(Tracking, 'event');
-      utilSpy = jest.spyOn(SharedUtils, 'packageTypeToTrackCategory').mockReturnValue(category);
-    });
+      it('calls the proper vuex action', async () => {
+        createComponent({ packageEntity: npmPackage });
+        await doDelete();
+        expect(deletePackage).toHaveBeenCalled();
+      });
 
-    it('tracking category calls packageTypeToTrackCategory', () => {
-      createComponent({ packageEntity: conanPackage });
-      expect(wrapper.vm.tracking.category).toBe(category);
-      expect(utilSpy).toHaveBeenCalledWith('conan');
+      it('when referrer contains project name calls window.replace with project url', async () => {
+        setReferrer();
+        deletePackage.mockResolvedValue();
+        createComponent({ packageEntity: npmPackage });
+        await doDelete();
+        await deletePackage();
+        expect(window.location.replace).toHaveBeenCalledWith(
+          'project_url?showSuccessDeleteAlert=true',
+        );
+      });
+
+      it('when referrer does not contain project name calls window.replace with group url', async () => {
+        setReferrer('baz');
+        deletePackage.mockResolvedValue();
+        createComponent({ packageEntity: npmPackage });
+        await doDelete();
+        await deletePackage();
+        expect(window.location.replace).toHaveBeenCalledWith(
+          'group_url?showSuccessDeleteAlert=true',
+        );
+      });
     });
 
-    it(`delete button on delete modal call event with ${TrackingActions.DELETE_PACKAGE}`, () => {
-      createComponent({ packageEntity: conanPackage });
-      deleteButton().trigger('click');
-      return wrapper.vm.$nextTick().then(() => {
-        modalDeleteButton().trigger('click');
+    describe('tracking', () => {
+      let eventSpy;
+      let utilSpy;
+      const category = 'foo';
+
+      beforeEach(() => {
+        eventSpy = jest.spyOn(Tracking, 'event');
+        utilSpy = jest.spyOn(SharedUtils, 'packageTypeToTrackCategory').mockReturnValue(category);
+      });
+
+      it('tracking category calls packageTypeToTrackCategory', () => {
+        createComponent({ packageEntity: conanPackage });
+        expect(wrapper.vm.tracking.category).toBe(category);
+        expect(utilSpy).toHaveBeenCalledWith('conan');
+      });
+
+      it(`delete button on delete modal call event with ${TrackingActions.DELETE_PACKAGE}`, async () => {
+        createComponent({ packageEntity: npmPackage });
+        await doDelete();
         expect(eventSpy).toHaveBeenCalledWith(
           category,
           TrackingActions.DELETE_PACKAGE,
           expect.any(Object),
         );
       });
-    });
 
-    it(`file download link call event with ${TrackingActions.PULL_PACKAGE}`, () => {
-      createComponent({ packageEntity: conanPackage });
+      it(`file download link call event with ${TrackingActions.PULL_PACKAGE}`, () => {
+        createComponent({ packageEntity: conanPackage });
 
-      firstFileDownloadLink().vm.$emit('click');
-      expect(eventSpy).toHaveBeenCalledWith(
-        category,
-        TrackingActions.PULL_PACKAGE,
-        expect.any(Object),
-      );
+        firstFileDownloadLink().vm.$emit('click');
+        expect(eventSpy).toHaveBeenCalledWith(
+          category,
+          TrackingActions.PULL_PACKAGE,
+          expect.any(Object),
+        );
+      });
     });
   });
 });

spec/frontend/packages/details/store/actions_spec.js

 import testAction from 'helpers/vuex_action_helper';
 import Api from '~/api';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
-import fetchPackageVersions from '~/packages/details/store/actions';
+import { fetchPackageVersions, deletePackage } from '~/packages/details/store/actions';
 import * as types from '~/packages/details/store/mutation_types';
 import { FETCH_PACKAGE_VERSIONS_ERROR } from '~/packages/details/constants';
+import { DELETE_PACKAGE_ERROR_MESSAGE } from '~/packages/shared/constants';
 import { npmPackage as packageEntity } from '../../mock_data';
 
 jest.mock('~/flash.js');
@@ -73,4 +74,25 @@ describe('Actions Package details store', () => {
       );
     });
   });
+
+  describe('deletePackage', () => {
+    it('should call Api.deleteProjectPackage', done => {
+      Api.deleteProjectPackage = jest.fn().mockResolvedValue();
+      testAction(deletePackage, undefined, { packageEntity }, [], [], () => {
+        expect(Api.deleteProjectPackage).toHaveBeenCalledWith(
+          packageEntity.project_id,
+          packageEntity.id,
+        );
+        done();
+      });
+    });
+    it('should create flash on API error', done => {
+      Api.deleteProjectPackage = jest.fn().mockRejectedValue();
+
+      testAction(deletePackage, undefined, { packageEntity }, [], [], () => {
+        expect(createFlash).toHaveBeenCalledWith(DELETE_PACKAGE_ERROR_MESSAGE);
+        done();
+      });
+    });
+  });
 });

spec/frontend/packages/list/components/packages_list_app_spec.js

 import Vuex from 'vuex';
 import { shallowMount, createLocalVue } from '@vue/test-utils';
 import { GlEmptyState, GlTab, GlTabs, GlSprintf, GlLink } from '@gitlab/ui';
+import * as commonUtils from '~/lib/utils/common_utils';
+import createFlash from '~/flash';
 import PackageListApp from '~/packages/list/components/packages_list_app.vue';
+import { SHOW_DELETE_SUCCESS_ALERT } from '~/packages/shared/constants';
+import { DELETE_PACKAGE_SUCCESS_MESSAGE } from '~/packages/list/constants';
+
+jest.mock('~/lib/utils/common_utils');
+jest.mock('~/flash');
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -145,4 +152,46 @@ describe('packages_list_app', () => {
       );
     });
   });
+
+  describe('delete alert handling', () => {
+    const { location } = window.location;
+    const search = `?${SHOW_DELETE_SUCCESS_ALERT}=true`;
+
+    beforeEach(() => {
+      createStore('foo');
+      jest.spyOn(commonUtils, 'historyReplaceState').mockImplementation(() => {});
+      delete window.location;
+      window.location = {
+        href: `foo_bar_baz${search}`,
+        search,
+      };
+    });
+
+    afterEach(() => {
+      window.location = location;
+    });
+
+    it(`creates a flash if the query string contains ${SHOW_DELETE_SUCCESS_ALERT}`, () => {
+      mountComponent();
+
+      expect(createFlash).toHaveBeenCalledWith({
+        message: DELETE_PACKAGE_SUCCESS_MESSAGE,
+        type: 'notice',
+      });
+    });
+
+    it('calls historyReplaceState with a clean url', () => {
+      mountComponent();
+
+      expect(commonUtils.historyReplaceState).toHaveBeenCalledWith('foo_bar_baz');
+    });
+
+    it(`does nothing if the query string does not contain ${SHOW_DELETE_SUCCESS_ALERT}`, () => {
+      window.location.search = '';
+      mountComponent();
+
+      expect(createFlash).not.toHaveBeenCalled();
+      expect(commonUtils.historyReplaceState).not.toHaveBeenCalled();
+    });
+  });
 });

spec/frontend/packages/list/stores/actions_spec.js

@@ -5,7 +5,8 @@ import Api from '~/api';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
 import * as actions from '~/packages/list/stores/actions';
 import * as types from '~/packages/list/stores/mutation_types';
-import { MISSING_DELETE_PATH_ERROR, DELETE_PACKAGE_ERROR_MESSAGE } from '~/packages/list/constants';
+import { MISSING_DELETE_PATH_ERROR } from '~/packages/list/constants';
+import { DELETE_PACKAGE_ERROR_MESSAGE } from '~/packages/shared/constants';
 
 jest.mock('~/flash.js');
 jest.mock('~/api.js');