Merge branch '197923-prevent-package-delete-from-resetting-pagination-position' into 'master'

Update package list delete to return you to the same page

See merge request gitlab-org/gitlab!28638

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

@@ -10,6 +10,7 @@ import {
   DEFAULT_PAGE_SIZE,
   MISSING_DELETE_PATH_ERROR,
 } from '../constants';
+import { getNewPaginationPage } from '../utils';
 
 export const setInitialState = ({ commit }, data) => commit(types.SET_INITIAL_STATE, data);
 export const setLoading = ({ commit }, data) => commit(types.SET_MAIN_LOADING, data);
@@ -48,7 +49,7 @@ export const requestPackagesList = ({ dispatch, state }, params = {}) => {
     });
 };
 
-export const requestDeletePackage = ({ dispatch }, { _links }) => {
+export const requestDeletePackage = ({ dispatch, state }, { _links }) => {
   if (!_links || !_links.delete_api_path) {
     createFlash(DELETE_PACKAGE_ERROR_MESSAGE);
     const error = new Error(MISSING_DELETE_PATH_ERROR);
@@ -59,14 +60,15 @@ export const requestDeletePackage = ({ dispatch }, { _links }) => {
   return axios
     .delete(_links.delete_api_path)
     .then(() => {
-      dispatch('requestPackagesList');
+      const { page: currentPage, perPage, total } = state.pagination;
+      const page = getNewPaginationPage(currentPage, perPage, total - 1);
+
+      dispatch('requestPackagesList', { page });
       createFlash(DELETE_PACKAGE_SUCCESS_MESSAGE, 'success');
     })
     .catch(() => {
-      createFlash(DELETE_PACKAGE_ERROR_MESSAGE);
-    })
-    .finally(() => {
       dispatch('setLoading', false);
+      createFlash(DELETE_PACKAGE_ERROR_MESSAGE);
     });
 };
 

ee/app/assets/javascripts/packages/list/utils.js

@@ -2,3 +2,24 @@ import { LIST_KEY_PROJECT, TABLE_HEADER_FIELDS } from './constants';
 
 export default isGroupPage =>
   TABLE_HEADER_FIELDS.filter(f => f.key !== LIST_KEY_PROJECT || isGroupPage);
+
+/**
+ * A small util function that works out if the delete action has deleted the
+ * last item on the current paginated page and if so, returns the previous
+ * page. This ensures the user won't end up on an empty paginated page.
+ *
+ * @param {number} currentPage The current page the user is on
+ * @param {number} perPage Number of items to display per page
+ * @param {number} totalPackages The total number of items
+ */
+export const getNewPaginationPage = (currentPage, perPage, totalItems) => {
+  if (totalItems <= perPage) {
+    return 1;
+  }
+
+  if (currentPage > 1 && (currentPage - 1) * perPage >= totalItems) {
+    return currentPage - 1;
+  }
+
+  return currentPage;
+};

ee/changelogs/unreleased/197923-prevent-package-delete-from-resetting-pagination-position.yml

+---
+title: Deleting packages from a deeper paginated page will no longer return you to
+  the first page
+merge_request: 28638
+author:
+type: changed

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

@@ -177,12 +177,11 @@ describe('Actions Package list store', () => {
       testAction(
         actions.requestDeletePackage,
         payload,
-        null,
+        { pagination: { page: 1 } },
         [],
         [
           { type: 'setLoading', payload: true },
-          { type: 'requestPackagesList' },
-          { type: 'setLoading', payload: false },
+          { type: 'requestPackagesList', payload: { page: 1 } },
         ],
         done,
       );

ee/spec/frontend/packages/list/utils_spec.js

+import { getNewPaginationPage } from 'ee/packages/list/utils';
+
+describe('Packages list utils', () => {
+  describe('packageTypeDisplay', () => {
+    it('returns the current page when total items exceeds pagniation', () => {
+      expect(getNewPaginationPage(2, 20, 21)).toBe(2);
+    });
+
+    it('returns the previous page when total items is lower than or equal to pagination', () => {
+      expect(getNewPaginationPage(2, 20, 20)).toBe(1);
+    });
+
+    it('returns the first page when totalItems is lower than or equal to perPage', () => {
+      expect(getNewPaginationPage(4, 20, 20)).toBe(1);
+    });
+
+    describe('works when a different perPage is used', () => {
+      it('returns the current page', () => {
+        expect(getNewPaginationPage(2, 10, 11)).toBe(2);
+      });
+
+      it('returns the previous page', () => {
+        expect(getNewPaginationPage(2, 10, 10)).toBe(1);
+      });
+    });
+
+    describe.each`
+      currentPage | totalItems | expectedResult
+      ${1}        | ${20}      | ${1}
+      ${2}        | ${20}      | ${1}
+      ${3}        | ${40}      | ${2}
+      ${4}        | ${60}      | ${3}
+    `(`works across numerious pages`, ({ currentPage, totalItems, expectedResult }) => {
+      it(`when currentPage is ${currentPage} return to the previous page ${expectedResult}`, () => {
+        expect(getNewPaginationPage(currentPage, 20, totalItems)).toBe(expectedResult);
+      });
+    });
+  });
+});