diff --git a/apps/readest-app/src/__tests__/utils/library-utils.test.ts b/apps/readest-app/src/__tests__/utils/library-utils.test.ts index fe326cca..a50185db 100644 --- a/apps/readest-app/src/__tests__/utils/library-utils.test.ts +++ b/apps/readest-app/src/__tests__/utils/library-utils.test.ts @@ -10,6 +10,7 @@ import { ensureLibraryGroupByType, findGroupById, getGroupDisplayName, + expandBookshelfSelection, } from '../../app/library/utils/libraryUtils'; import { Book, BooksGroup } from '../../types/book'; import { LibraryGroupByType, LibrarySortByType } from '../../types/settings'; @@ -983,3 +984,84 @@ describe('getGroupDisplayName', () => { expect(getGroupDisplayName(items, 'non-existent')).toBeUndefined(); }); }); + +describe('expandBookshelfSelection', () => { + const createMockGroup = (overrides: Partial = {}): BooksGroup => ({ + id: 'test-group', + name: 'Test Group', + displayName: 'Test Display Name', + books: [], + updatedAt: Date.now(), + ...overrides, + }); + + it('passes through standalone book hashes unchanged', () => { + const items: (Book | BooksGroup)[] = [ + createMockBook({ hash: 'book-1' }), + createMockBook({ hash: 'book-2' }), + ]; + + expect(expandBookshelfSelection(['book-1', 'book-2'], items)).toEqual(['book-1', 'book-2']); + }); + + it('expands a group id into every book hash in the rendered rollup', () => { + // generateBookshelfItems rolls nested-folder books into the top-level + // group, so the rendered BooksGroup.books already includes them. The + // helper must rely on that rollup so deleting "MyDir" also catches + // books whose own groupName is "MyDir/sub". + const items: (Book | BooksGroup)[] = [ + createMockGroup({ + id: 'group-mydir', + name: 'MyDir', + books: [ + createMockBook({ hash: 'direct-book', groupName: 'MyDir', groupId: 'group-mydir' }), + createMockBook({ hash: 'nested-book', groupName: 'MyDir/sub', groupId: 'group-sub' }), + ], + }), + ]; + + expect(expandBookshelfSelection(['group-mydir'], items).sort()).toEqual([ + 'direct-book', + 'nested-book', + ]); + }); + + it('deduplicates when a book hash and its parent group are both selected', () => { + const items: (Book | BooksGroup)[] = [ + createMockGroup({ + id: 'group-1', + books: [createMockBook({ hash: 'book-a' }), createMockBook({ hash: 'book-b' })], + }), + ]; + + expect(expandBookshelfSelection(['book-a', 'group-1'], items).sort()).toEqual([ + 'book-a', + 'book-b', + ]); + }); + + it('skips soft-deleted books inside a group', () => { + const items: (Book | BooksGroup)[] = [ + createMockGroup({ + id: 'group-1', + books: [ + createMockBook({ hash: 'alive' }), + createMockBook({ hash: 'gone', deletedAt: Date.now() }), + ], + }), + ]; + + expect(expandBookshelfSelection(['group-1'], items)).toEqual(['alive']); + }); + + it('returns an empty array when no ids are given', () => { + expect(expandBookshelfSelection([], [])).toEqual([]); + }); + + it('preserves unknown ids so callers can surface stale selections', () => { + // Defensive: if the rendered items no longer contain a selected id (e.g. + // it was a hash from another view), the helper leaves it alone rather + // than silently dropping it. + expect(expandBookshelfSelection(['ghost-hash'], [])).toEqual(['ghost-hash']); + }); +}); diff --git a/apps/readest-app/src/app/library/components/Bookshelf.tsx b/apps/readest-app/src/app/library/components/Bookshelf.tsx index 9ff07e2b..9edbcf8e 100644 --- a/apps/readest-app/src/app/library/components/Bookshelf.tsx +++ b/apps/readest-app/src/app/library/components/Bookshelf.tsx @@ -36,6 +36,7 @@ import { createWithinGroupSorter, ensureLibraryGroupByType, ensureLibrarySortByType, + expandBookshelfSelection, getBookSortValue, getGroupSortValue, compareSortValues, @@ -359,16 +360,14 @@ const Bookshelf: React.FC = ({ } }; + // `bookIdsToDelete` always holds book hashes by the time we get here — + // group ids are expanded into their constituent hashes at intake (see + // `deleteSelectedBooks` and `handleDeleteBooksIntent`), so a top-level + // folder is now resolved against the rendered group's `books` rollup, + // which already includes nested sub-folder books. const getBooksToDelete = () => { - const booksToDelete: Book[] = []; - bookIdsToDelete.forEach((id) => { - for (const book of filteredBooks.filter((book) => book.hash === id || book.groupId === id)) { - if (book && !book.deletedAt) { - booksToDelete.push(book); - } - } - }); - return booksToDelete; + const wanted = new Set(bookIdsToDelete); + return filteredBooks.filter((book) => wanted.has(book.hash) && !book.deletedAt); }; const confirmDelete = async () => { @@ -390,7 +389,12 @@ const Bookshelf: React.FC = ({ }; const deleteSelectedBooks = () => { - setBookIdsToDelete(getSelectedBooks()); + // Expand any group ids in the selection into the book hashes they + // visually represent — `generateBookshelfItems` rolls nested-folder + // books into the parent group, and we want every one of them queued + // for deletion, not just the books whose own `groupId` happens to + // match the top-level group's id. + setBookIdsToDelete(expandBookshelfSelection(getSelectedBooks(), sortedBookshelfItems)); setShowSelectModeActions(false); setShowDeleteAlert(true); }; diff --git a/apps/readest-app/src/app/library/components/BookshelfItem.tsx b/apps/readest-app/src/app/library/components/BookshelfItem.tsx index a19cf191..423d7da2 100644 --- a/apps/readest-app/src/app/library/components/BookshelfItem.tsx +++ b/apps/readest-app/src/app/library/components/BookshelfItem.tsx @@ -317,7 +317,12 @@ const BookshelfItem: React.FC = ({ const deleteGroupMenuItem = await MenuItem.new({ text: _('Delete'), action: async () => { - eventDispatcher.dispatch('delete-books', { ids: [group.id] }); + // Dispatch the constituent book hashes — `group.books` is the + // rendered rollup and already includes books from nested sub- + // folders, so the deletion path doesn't need to re-derive what + // belongs to the group from the id alone. + const ids = group.books.filter((book) => !book.deletedAt).map((book) => book.hash); + eventDispatcher.dispatch('delete-books', { ids }); }, }); const menu = await Menu.new(); diff --git a/apps/readest-app/src/app/library/utils/libraryUtils.ts b/apps/readest-app/src/app/library/utils/libraryUtils.ts index 91d34012..ded95ee2 100644 --- a/apps/readest-app/src/app/library/utils/libraryUtils.ts +++ b/apps/readest-app/src/app/library/utils/libraryUtils.ts @@ -59,6 +59,32 @@ export const getGroupDisplayName = ( return group?.displayName || group?.name; }; +/** + * Expand a list of selection ids (book hashes or group ids from the rendered + * bookshelf) into the unique book hashes those ids represent. + * + * Group ids resolve to every (non-soft-deleted) book in the group's visible + * rollup — `generateBookshelfItems` already folds nested-folder books into + * their top-level group, so the rendered `BooksGroup.books` is the source of + * truth. Standalone book hashes (and any unknown ids) pass through unchanged, + * letting callers like the bookshelf delete flow collect the right set up + * front instead of re-deriving it later. + */ +export const expandBookshelfSelection = (ids: string[], items: (Book | BooksGroup)[]): string[] => { + const hashes = new Set(); + for (const id of ids) { + const group = findGroupById(items, id); + if (group) { + for (const book of group.books) { + if (!book.deletedAt) hashes.add(book.hash); + } + } else { + hashes.add(id); + } + } + return [...hashes]; +}; + export const createBookFilter = (queryTerm: string | null) => (item: Book) => { if (!queryTerm) return true; if (item.deletedAt) return false;