From 3f7ab31b2b811b27fbaf7bfbeaf4f495d393b4d7 Mon Sep 17 00:00:00 2001 From: Abhishek S Lal Date: Mon, 17 Nov 2025 16:05:33 +0530 Subject: [PATCH] refactor: enhance deleteItem action to handle item reordering after deletion --- .../DeleteCollectionItem/index.js | 10 ++-- .../ReduxStore/slices/collections/actions.js | 34 +++++------- .../delete-request-sequence-updation.spec.ts | 55 +++++++++++++++++++ tests/utils/page/actions.ts | 51 ++++++++++++++++- 4 files changed, 125 insertions(+), 25 deletions(-) create mode 100644 tests/request/delete-request/delete-request-sequence-updation.spec.ts diff --git a/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/DeleteCollectionItem/index.js b/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/DeleteCollectionItem/index.js index 7e32fdadb..46ddc563e 100644 --- a/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/DeleteCollectionItem/index.js +++ b/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/DeleteCollectionItem/index.js @@ -3,17 +3,16 @@ import Modal from 'components/Modal'; import { isItemAFolder } from 'utils/tabs'; import { useDispatch } from 'react-redux'; import { closeTabs } from 'providers/ReduxStore/slices/tabs'; -import { deleteItem, reorderDirectoryItems } from 'providers/ReduxStore/slices/collections/actions'; +import { deleteItem } from 'providers/ReduxStore/slices/collections/actions'; import { recursivelyGetAllItemUids } from 'utils/collections'; import StyledWrapper from './StyledWrapper'; +import toast from 'react-hot-toast'; const DeleteCollectionItem = ({ onClose, item, collectionUid }) => { const dispatch = useDispatch(); const isFolder = isItemAFolder(item); const onConfirm = () => { - dispatch(deleteItem(item.uid, collectionUid)).then(({ parentDirectory }) => { - dispatch(reorderDirectoryItems(parentDirectory, item.uid)); - + dispatch(deleteItem(item.uid, collectionUid)).then(() => { if (isFolder) { // close all tabs that belong to the folder // including the folder itself and its children @@ -31,6 +30,9 @@ const DeleteCollectionItem = ({ onClose, item, collectionUid }) => { }) ); } + }).catch((error) => { + console.error('Error deleting item', error); + toast.error(error?.message || 'Error deleting item'); }); onClose(); }; diff --git a/packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js b/packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js index 1d0ef22f6..0ffa4a9e6 100644 --- a/packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js +++ b/packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js @@ -926,32 +926,26 @@ export const deleteItem = (itemUid, collectionUid) => (dispatch, getState) => { ipcRenderer .invoke('renderer:delete-item', item.pathname, item.type) - .then(() => { - resolve({ parentDirectory: parentDirectoryItem }); + .then(async () => { + // Reorder items in parent directory after deletion + if (parentDirectoryItem.items) { + const directoryItemsWithoutDeletedItem = parentDirectoryItem.items.filter((i) => i.uid !== itemUid); + const reorderedSourceItems = getReorderedItemsInSourceDirectory({ + items: directoryItemsWithoutDeletedItem + }); + if (reorderedSourceItems?.length) { + await dispatch(updateItemsSequences({ itemsToResequence: reorderedSourceItems })); + } + } + resolve(); }) .catch((error) => reject(error)); + } else { + return reject(new Error('Unable to locate item')); } - return; }); }; -export const reorderDirectoryItems = (directory, itemUid) => (dispatch, getState) => { - if (!directory.items) return; - - const directoryItemsWithoutDeletedItem = directory.items.filter( - (i) => i.uid !== itemUid - ); - - const reorderedSourceItems = getReorderedItemsInSourceDirectory({ - items: directoryItemsWithoutDeletedItem - }); - if (reorderedSourceItems?.length) { - dispatch(updateItemsSequences({ itemsToResequence: reorderedSourceItems })); - } - - return; -} - export const sortCollections = (payload) => (dispatch) => { dispatch(_sortCollections(payload)); }; diff --git a/tests/request/delete-request/delete-request-sequence-updation.spec.ts b/tests/request/delete-request/delete-request-sequence-updation.spec.ts new file mode 100644 index 000000000..185da85be --- /dev/null +++ b/tests/request/delete-request/delete-request-sequence-updation.spec.ts @@ -0,0 +1,55 @@ +import { test, expect } from '../../../playwright'; +import { closeAllCollections, createCollection, createRequest, deleteRequest } from '../../utils/page'; + +test.describe('Delete Request Sequence Updation', () => { + test.afterAll(async ({ page }) => { + await closeAllCollections(page); + }); + + test('Maintain correct sequence after deleting requests', async ({ page, createTmpDir }) => { + const collectionName = 'test-collection'; + + // Create a collection + await createCollection(page, collectionName, await createTmpDir(collectionName), { openWithSandboxMode: 'safe' }); + + // Create request-a + await createRequest(page, 'request-a', collectionName); + + // Create request-b + await createRequest(page, 'request-b', collectionName); + + // Create request-c + await createRequest(page, 'request-c', collectionName); + + // Create request-d + await createRequest(page, 'request-d', collectionName); + + // Verify all requests are created in order + const allRequests = page.locator('.collection-item-name'); + await expect(allRequests.nth(0)).toContainText('request-a'); + await expect(allRequests.nth(1)).toContainText('request-b'); + await expect(allRequests.nth(2)).toContainText('request-c'); + await expect(allRequests.nth(3)).toContainText('request-d'); + + // Delete request-b + await deleteRequest(page, 'request-b', collectionName); + + // Delete request-c + await deleteRequest(page, 'request-c', collectionName); + + // Verify remaining requests are in correct order (a and d) + const remainingRequests = page.locator('.collection-item-name'); + await expect(remainingRequests.nth(0)).toContainText('request-a'); + await expect(remainingRequests.nth(1)).toContainText('request-d'); + + // Create request-e + await createRequest(page, 'request-e', collectionName); + + // Verify request-e is created at the last position (3rd position: a, d, e) + const finalRequests = page.locator('.collection-item-name'); + await expect(finalRequests.nth(0)).toContainText('request-a'); + await expect(finalRequests.nth(1)).toContainText('request-d'); + await expect(finalRequests.nth(2)).toContainText('request-e'); + await expect(finalRequests).toHaveCount(3); + }); +}); diff --git a/tests/utils/page/actions.ts b/tests/utils/page/actions.ts index 0dcd11505..a0589e149 100644 --- a/tests/utils/page/actions.ts +++ b/tests/utils/page/actions.ts @@ -1,4 +1,5 @@ import { test, expect } from '../../../playwright'; +import { buildCommonLocators } from './locators'; /** * Close all collections @@ -78,4 +79,52 @@ const createCollection = async (page, collectionName: string, collectionLocation }); }; -export { closeAllCollections, openCollectionAndAcceptSandbox, createCollection }; +/** + * Create a request in a collection + * @param page - The page object + * @param requestName - The name of the request to create + * @param collectionName - The name of the collection + * @returns void + */ +const createRequest = async (page, requestName: string, collectionName: string) => { + await test.step(`Create request "${requestName}" in collection "${collectionName}"`, async () => { + const locators = buildCommonLocators(page); + const collection = locators.sidebar.collection(collectionName); + + await collection.hover(); + await locators.actions.collectionActions(collectionName).click(); + await locators.dropdown.item('New Request').click(); + await page.getByPlaceholder('Request Name').fill(requestName); + await locators.modal.button('Create').click(); + await expect(locators.sidebar.request(requestName)).toBeVisible(); + }); +}; + +/** + * Delete a request from a collection + * @param page - The page object + * @param requestName - The name of the request to delete + * @param collectionName - The name of the collection + * @returns void + */ +const deleteRequest = async (page, requestName: string, collectionName: string) => { + await test.step(`Delete request "${requestName}" from collection "${collectionName}"`, async () => { + const locators = buildCommonLocators(page); + + // Click on the collection first to open it if it's closed + await locators.sidebar.collection(collectionName).click(); + + // Find the request within the collection's context + // Use the collection container (.collection-name) to scope the search + const collectionContainer = page.locator('.collection-name').filter({ hasText: collectionName }); + const collectionWrapper = collectionContainer.locator('..'); + const request = collectionWrapper.locator('.collection-item-name').filter({ hasText: requestName }); + + await request.locator('.menu-icon').click(); + await locators.dropdown.item('Delete').click(); + await locators.modal.button('Delete').click(); + await expect(request).not.toBeVisible(); + }); +}; + +export { closeAllCollections, openCollectionAndAcceptSandbox, createCollection, createRequest, deleteRequest };