fix(library): always sort series books by index ascending, closes #3709 (#3717)

* fix(library): always sort series books by index ascending, closes #3709 When grouping by series, the sort direction (asc/desc) was applied to within-series book sorting, causing books to appear in reverse series index order when the library was sorted descending. Now series index sort is always ascending (1, 2, 3…) regardless of the global sort direction. Also sort series groups by name when "Sort by Series" is selected instead of falling back to updatedAt. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(library): skip merge sort when inside a series group The allItems.sort at the end of sortedBookshelfItems was re-sorting books using the regular bookSorter (e.g. Date Read), which overrode the within-group sort that correctly prioritized series index. Now when inside a group, the already-sorted books are returned directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-01 20:01:48 +08:00
parent 29df8522fa
commit 74401fc1bb
3 changed files with 103 additions and 10 deletions
@@ -333,6 +333,77 @@ describe('createWithinGroupSorter', () => {
    });
  });

+  describe('series grouping with sort direction', () => {
+    it('should always sort by seriesIndex ascending even when sortAscending is false', () => {
+      const books = [
+        createMockBook({
+          hash: '1',
+          title: 'Book 3',
+          metadata: { seriesIndex: 3 },
+        }),
+        createMockBook({
+          hash: '2',
+          title: 'Book 1',
+          metadata: { seriesIndex: 1 },
+        }),
+        createMockBook({
+          hash: '3',
+          title: 'Book 2',
+          metadata: { seriesIndex: 2 },
+        }),
+      ];
+
+      const sorter = createWithinGroupSorter(
+        LibraryGroupByType.Series,
+        LibrarySortByType.Title,
+        'en',
+        false, // descending
+      );
+      const sorted = [...books].sort(sorter);
+
+      // Series index should always be ascending (1, 2, 3) regardless of sort direction
+      expect(sorted[0]!.metadata?.seriesIndex).toBe(1);
+      expect(sorted[1]!.metadata?.seriesIndex).toBe(2);
+      expect(sorted[2]!.metadata?.seriesIndex).toBe(3);
+    });
+
+    it('should apply sort direction to fallback sort for books without seriesIndex', () => {
+      const books = [
+        createMockBook({ hash: '1', title: 'Apple', metadata: {} }),
+        createMockBook({ hash: '2', title: 'Zebra', metadata: {} }),
+      ];
+
+      const sorterDesc = createWithinGroupSorter(
+        LibraryGroupByType.Series,
+        LibrarySortByType.Title,
+        'en',
+        false, // descending
+      );
+      const sortedDesc = [...books].sort(sorterDesc);
+
+      expect(sortedDesc[0]!.title).toBe('Zebra');
+      expect(sortedDesc[1]!.title).toBe('Apple');
+    });
+
+    it('should place books with seriesIndex before those without regardless of sort direction', () => {
+      const books = [
+        createMockBook({ hash: '1', title: 'Apple', metadata: {} }),
+        createMockBook({ hash: '2', title: 'Book 1', metadata: { seriesIndex: 1 } }),
+      ];
+
+      const sorter = createWithinGroupSorter(
+        LibraryGroupByType.Series,
+        LibrarySortByType.Title,
+        'en',
+        false, // descending
+      );
+      const sorted = [...books].sort(sorter);
+
+      expect(sorted[0]!.hash).toBe('2'); // Has index, comes first
+      expect(sorted[1]!.hash).toBe('1'); // No index
+    });
+  });
+
  describe('author grouping', () => {
    it('should sort by global sort criteria (title)', () => {
      const books = [
@@ -463,6 +534,11 @@ describe('getGroupSortValue', () => {
    expect(getGroupSortValue(group, LibrarySortByType.Published)).toBe(0);
  });

+  it('should return group name for series sort', () => {
+    const group = createMockGroup({ name: 'My Series' });
+    expect(getGroupSortValue(group, LibrarySortByType.Series)).toBe('My Series');
+  });
+
  it('should handle empty groups gracefully', () => {
    const group = createMockGroup({ books: [] });

@@ -186,16 +186,21 @@ const Bookshelf: React.FC<BookshelfProps> = ({
    const groups = currentBookshelfItems.filter((item): item is BooksGroup => 'books' in item);

    // Sort books within each group
-    const withinGroupSorter = createWithinGroupSorter(groupBy, sortBy, uiLanguage);
+    // For series groups, series index is always ascending; sort direction applies to fallback only
+    const sortAscending = sortOrder === 'asc';
+    const withinGroupSorter = createWithinGroupSorter(groupBy, sortBy, uiLanguage, sortAscending);
    groups.forEach((group) => {
-      group.books.sort((a, b) => withinGroupSorter(a, b) * sortOrderMultiplier);
+      group.books.sort(withinGroupSorter);
    });

    // Sort ungrouped books - use within-group sorter if we're inside a group
    // (for series, this ensures books are sorted by series index)
    const bookSorter = createBookSorter(sortBy, uiLanguage);
    if (groupId && groupBy !== LibraryGroupByType.Group && groupBy !== LibraryGroupByType.None) {
-      ungroupedBooks.sort((a, b) => withinGroupSorter(a, b) * sortOrderMultiplier);
+      ungroupedBooks.sort(withinGroupSorter);
+      // When inside a group, books are already sorted correctly — return directly
+      // to avoid the merge sort below overriding the within-group sort order
+      return ungroupedBooks;
    } else {
      ungroupedBooks.sort((a, b) => bookSorter(a, b) * sortOrderMultiplier);
    }
@@ -270,17 +270,27 @@ const createAuthorGroups = (books: Book[]): (Book | BooksGroup)[] => {

 /**
 * Create a sorter for books within a group.
- * For series groups: sort by seriesIndex first, then by global sort for items without index.
+ * For series groups: sort by seriesIndex first (always ascending), then by global sort for items without index.
 * For author groups: follow global sort setting.
+ * @param sortAscending - When true (default), sort direction is ascending. Series index is always
+ *   ascending regardless of this flag; the flag only affects the fallback sort for books without
+ *   series index and non-series groupings.
 */
 export const createWithinGroupSorter =
-  (groupBy: LibraryGroupByType, sortBy: LibrarySortByType, uiLanguage: string) =>
+  (
+    groupBy: LibraryGroupByType,
+    sortBy: LibrarySortByType,
+    uiLanguage: string,
+    sortAscending: boolean = true,
+  ) =>
  (a: Book, b: Book): number => {
+    const sortDirection = sortAscending ? 1 : -1;
+
    if (groupBy === LibraryGroupByType.Series) {
      const aIndex = a.metadata?.seriesIndex;
      const bIndex = b.metadata?.seriesIndex;

-      // Both have series index - sort by index
+      // Both have series index - always sort ascending by index
      if (aIndex != null && bIndex != null) {
        return aIndex - bIndex;
      }
@@ -289,12 +299,12 @@ export const createWithinGroupSorter =
      if (aIndex != null) return -1;
      if (bIndex != null) return 1;

-      // Neither has series index - fall back to global sort
-      return createBookSorter(sortBy, uiLanguage)(a, b);
+      // Neither has series index - fall back to global sort with direction
+      return createBookSorter(sortBy, uiLanguage)(a, b) * sortDirection;
    }

-    // For author and other groupings, use global sort
-    return createBookSorter(sortBy, uiLanguage)(a, b);
+    // For author and other groupings, use global sort with direction
+    return createBookSorter(sortBy, uiLanguage)(a, b) * sortDirection;
  };

 /**
@@ -341,6 +351,7 @@ export const getGroupSortValue = (

  if (books.length === 0) {
    return sortBy === LibrarySortByType.Title ||
+      sortBy === LibrarySortByType.Series ||
      sortBy === LibrarySortByType.Author ||
      sortBy === LibrarySortByType.Format
      ? group.name
@@ -349,6 +360,7 @@ export const getGroupSortValue = (

  switch (sortBy) {
    case LibrarySortByType.Title:
+    case LibrarySortByType.Series:
    case LibrarySortByType.Format:
      return group.name;