fix(smart-scope): refresh sidebar counts after saving

Return matching item counts with create and update responses so the sidebar can display the saved scope count without requiring a page reload. Reuse the listing count logic and verify visibility and shared sidebar state.
2026-11-03 00:11:37 -06:00 · 2026-10-07 20:12:54 +00:00
parent d719c4ee5b
commit 5c093333c5
4 changed files with 165 additions and 52 deletions
@@ -41,27 +41,30 @@ describe('useSmartScopes', () => {
    apiMock.mockReset()
  })

-  it('replaces the smartScope array when creating a smartScope', async () => {
-    const created = makeSmartScope()
+  it('adds the created scope and its book count to the shared sidebar state', async () => {
+    const created = makeSmartScope({ bookCount: 7 })
    apiMock.mockResolvedValueOnce(makeResponse(created))

    const { useSmartScopes } = await import('../useSmartScopes')
    const { smartScopes, createSmartScope } = useSmartScopes()
    const previous = smartScopes.value
+    const sidebar = useSmartScopes()

    await createSmartScope({ name: created.name, icon: 'Aperture', defaultSort: [] })

    expect(smartScopes.value).toEqual([created])
    expect(smartScopes.value).not.toBe(previous)
+    expect(sidebar.bookScopes.value[0]?.bookCount).toBe(7)
  })

-  it('replaces the smartScope array when updating a smartScope', async () => {
-    const created = makeSmartScope()
-    const updated = makeSmartScope({ name: 'Updated SmartScope' })
+  it('replaces the cached book count with the count returned after an update', async () => {
+    const created = makeSmartScope({ bookCount: 7 })
+    const updated = makeSmartScope({ name: 'Updated SmartScope', bookCount: 3 })
    apiMock.mockResolvedValueOnce(makeResponse(created)).mockResolvedValueOnce(makeResponse(updated))

    const { useSmartScopes } = await import('../useSmartScopes')
    const { smartScopes, createSmartScope, updateSmartScope } = useSmartScopes()
+    const sidebar = useSmartScopes()

    await createSmartScope({ name: created.name, icon: 'Aperture', defaultSort: [] })
    const previous = smartScopes.value
@@ -69,6 +72,7 @@ describe('useSmartScopes', () => {

    expect(smartScopes.value).toEqual([updated])
    expect(smartScopes.value).not.toBe(previous)
+    expect(sidebar.bookScopes.value[0]?.bookCount).toBe(3)
  })

  it('patches the sharing flag and reflects the shared scope returned by the server', async () => {
@@ -76,7 +76,7 @@ function makeService() {
    buildOrderBy: vi.fn(),
  };
  const libraryService = {
-    findAccessibleLibraryIds: vi.fn(),
+    findAccessibleLibraryIds: vi.fn().mockResolvedValue([]),
  };
  const bookService = {
    executeBooksQuery: vi.fn(),
@@ -272,6 +272,28 @@ describe('SmartScopeService', () => {
      expect(smartScopeRepo.insert).toHaveBeenCalled();
    });

+    it.each(['create', 'update'] as const)('%s returns the saved playlist episode count', async (operation) => {
+      const { service, smartScopeRepo, libraryService, podcastEpisodeRepo, bookReadService } = makeService();
+      const scope = podcastScope();
+      smartScopeRepo.insert.mockResolvedValue([scope]);
+      smartScopeRepo.findById.mockResolvedValue([scope]);
+      smartScopeRepo.update.mockResolvedValue([scope]);
+      libraryService.findAccessibleLibraryIds.mockResolvedValue([4]);
+      podcastEpisodeRepo.countEpisodes.mockResolvedValue(6);
+
+      const result =
+        operation === 'create'
+          ? await service.create(
+              { name: scope.name, icon: 'Podcast', mediaType: 'podcasts', libraryId: 4, filter: RULES, defaultSort: [] },
+              makeUser(),
+            )
+          : await service.update(scope.id, { filter: RULES }, makeUser());
+
+      expect(result).toEqual(expect.objectContaining({ episodeCount: 6 }));
+      expect(podcastEpisodeRepo.countEpisodes).toHaveBeenCalledExactlyOnceWith(4, 12, expect.objectContaining({ filter: 'unplayed' }));
+      expect(bookReadService.countWhere).not.toHaveBeenCalled();
+    });
+
    it('leaves book scopes uncapped, which has always been their contract', async () => {
      const { service, smartScopeRepo } = makeService();
      smartScopeRepo.countOwnedByMediaType.mockResolvedValue(PODCAST_PLAYLIST_MAX_SAVED * 10);
@@ -580,7 +602,53 @@ describe('SmartScopeService', () => {
      isPublic: false,
      syncToKobo: false,
    });
-    expect(result).toEqual({ ...created, isOwner: true, koboSyncEnabled: false });
+    expect(result).toEqual({ ...created, isOwner: true, koboSyncEnabled: false, bookCount: 0 });
+  });
+
+  it('create returns the matching book count using the requesting user visibility and timezone', async () => {
+    const { service, smartScopeRepo, libraryService, queryBuilder, bookReadService } = makeService();
+    const created = makeSmartScope({
+      filter: { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'title', operator: 'contains', value: 'space' }] },
+    });
+    smartScopeRepo.insert.mockResolvedValue([created]);
+    libraryService.findAccessibleLibraryIds.mockResolvedValue([2, 3]);
+    queryBuilder.buildWhere.mockReturnValue('visible-matches');
+    bookReadService.countWhere.mockResolvedValue(7);
+    const user = makeUser({ settings: { timezone: 'America/Denver' } });
+
+    const result = await service.create({ name: created.name, icon: 'Aperture', filter: created.filter, defaultSort: [] }, user);
+
+    expect(result).toEqual({ ...created, isOwner: true, koboSyncEnabled: false, bookCount: 7 });
+    expect(queryBuilder.buildWhere).toHaveBeenCalledWith(created.filter, {
+      accessibleLibraryIds: [2, 3],
+      userId: user.id,
+      timeZone: 'America/Denver',
+      contentFilters: user.contentFilters,
+    });
+    expect(bookReadService.countWhere).toHaveBeenCalledExactlyOnceWith('visible-matches');
+    expect(smartScopeRepo.findAllForUser).not.toHaveBeenCalled();
+  });
+
+  it('update counts the saved filter instead of the previous filter', async () => {
+    const { service, smartScopeRepo, libraryService, queryBuilder, bookReadService } = makeService();
+    const existing = makeSmartScope({
+      filter: { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'title', operator: 'contains', value: 'old' }] },
+    });
+    const updated = makeSmartScope({
+      filter: { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'title', operator: 'contains', value: 'new' }] },
+    });
+    smartScopeRepo.findById.mockResolvedValue([existing]);
+    smartScopeRepo.update.mockResolvedValue([updated]);
+    libraryService.findAccessibleLibraryIds.mockResolvedValue([2]);
+    queryBuilder.buildWhere.mockReturnValue('updated-matches');
+    bookReadService.countWhere.mockResolvedValue(3);
+
+    const result = await service.update(existing.id, { filter: updated.filter }, makeUser());
+
+    expect(result).toEqual({ ...updated, isOwner: true, koboSyncEnabled: false, bookCount: 3 });
+    expect(queryBuilder.buildWhere).toHaveBeenCalledWith(updated.filter, expect.objectContaining({ accessibleLibraryIds: [2], userId: 12 }));
+    expect(bookReadService.countWhere).toHaveBeenCalledExactlyOnceWith('updated-matches');
+    expect(smartScopeRepo.findAllForUser).not.toHaveBeenCalled();
  });

  it('create rejects missing icons', async () => {
@@ -613,7 +681,7 @@ describe('SmartScopeService', () => {
      defaultSort: undefined,
      isPublic: undefined,
    });
-    expect(result).toEqual({ ...updated, isOwner: false, koboSyncEnabled: false });
+    expect(result).toEqual({ ...updated, isOwner: false, koboSyncEnabled: false, bookCount: 0 });
  });

  describe('sharing an existing smartScope (issue #805)', () => {
@@ -627,7 +695,7 @@ describe('SmartScopeService', () => {
      const result = await service.update(3, { isPublic: true }, makeUser({ id: 12 }));

      expect(smartScopeRepo.update).toHaveBeenCalledWith(3, 12, expect.objectContaining({ isPublic: true }));
-      expect(result).toEqual({ ...shared, isOwner: true, koboSyncEnabled: false });
+      expect(result).toEqual({ ...shared, isOwner: true, koboSyncEnabled: false, bookCount: 0 });
    });

    it('update unshares a public smartScope', async () => {
@@ -722,7 +790,9 @@ describe('SmartScopeService', () => {
    smartScopeRepo.findById.mockResolvedValue([existing]);
    smartScopeRepo.update.mockResolvedValue([{ ...existing, filter: null }]);

-    await service.update(3, { filter: null }, makeUser({ id: 12 }));
+    const result = await service.update(3, { filter: null }, makeUser({ id: 12 }));
+
+    expect(result).toEqual(expect.objectContaining({ filter: null, bookCount: 0 }));

    expect(smartScopeRepo.update).toHaveBeenCalledWith(
      3,
@@ -129,53 +129,55 @@ export class SmartScopeService {
  async findAll(user: RequestUser) {
    const smartScopes = await this.smartScopeRepo.findAllForUser(user.id);
    const accessibleLibraryIds = await this.libraryService.findAccessibleLibraryIds(user);
-    /** Null means every library. A shared podcast scope must not count episodes the viewer cannot see. */
-    const canSeeLibrary = (libraryId: number | null) =>
-      libraryId !== null && (user.isSuperuser || accessibleLibraryIds === null || accessibleLibraryIds.includes(libraryId));
-    const timeZone = resolveTimeZone((user.settings as { timezone?: unknown } | undefined)?.timezone, 'UTC');
    const sharedScopeIds = smartScopes.filter((smartScope) => smartScope.userId !== user.id).map((smartScope) => smartScope.id);
    const subscribedIds = new Set(await this.smartScopeRepo.findKoboSubscribedScopeIds(user.id, sharedScopeIds));
    const koboSyncEnabledFor = (smartScope: SmartScope) => (smartScope.userId === user.id ? smartScope.syncToKobo : subscribedIds.has(smartScope.id));
-    return mapWithConcurrency(smartScopes, SCOPE_COUNT_CONCURRENCY, async (smartScope) => {
-      const countKey = this.isPodcastScope(smartScope) ? 'episodeCount' : 'bookCount';
-      if (!smartScope.filter) {
-        return { ...this.toResponse(smartScope, user, koboSyncEnabledFor(smartScope)), [countKey]: 0 };
-      }
-      const startedAt = Date.now();
-      try {
-        const filter = this.validateFilterFor(smartScope.mediaType, smartScope.filter);
-        if (!filter) return { ...this.toResponse(smartScope, user, koboSyncEnabledFor(smartScope)), [countKey]: 0 };
+    return mapWithConcurrency(smartScopes, SCOPE_COUNT_CONCURRENCY, (smartScope) =>
+      this.toCountedResponse(smartScope, user, koboSyncEnabledFor(smartScope), accessibleLibraryIds),
+    );
+  }

-        if (this.isPodcastScope(smartScope)) {
-          if (!canSeeLibrary(smartScope.libraryId)) {
-            return { ...this.toResponse(smartScope, user, koboSyncEnabledFor(smartScope)), episodeCount: null };
-          }
-          const episodeCount = await this.podcastEpisodeRepo.countEpisodes(
-            smartScope.libraryId as number,
-            user.id,
-            toEpisodeRuleQuery(filter as PodcastScopeRules),
-          );
-          return { ...this.toResponse(smartScope, user, koboSyncEnabledFor(smartScope)), episodeCount };
+  private async toCountedResponse(smartScope: SmartScope, user: RequestUser, koboSyncEnabled: boolean, accessibleLibraryIds?: number[]) {
+    const response = this.toResponse(smartScope, user, koboSyncEnabled);
+    const countKey = this.isPodcastScope(smartScope) ? 'episodeCount' : 'bookCount';
+    if (!smartScope.filter) return { ...response, [countKey]: 0 };
+
+    const startedAt = Date.now();
+    try {
+      const filter = this.validateFilterFor(smartScope.mediaType, smartScope.filter);
+      if (!filter) return { ...response, [countKey]: 0 };
+      const libraryIds = accessibleLibraryIds ?? (await this.libraryService.findAccessibleLibraryIds(user));
+
+      if (this.isPodcastScope(smartScope)) {
+        if (smartScope.libraryId === null || (!user.isSuperuser && !libraryIds.includes(smartScope.libraryId))) {
+          return { ...response, episodeCount: null };
        }
-
-        const where = this.queryBuilder.buildWhere(filter as GroupRule, {
-          accessibleLibraryIds,
-          userId: user.id,
-          timeZone,
-          contentFilters: user.isSuperuser ? undefined : user.contentFilters,
-        });
-        const bookCount = await this.bookReadService.countWhere(where);
-        return { ...this.toResponse(smartScope, user, koboSyncEnabledFor(smartScope)), bookCount };
-      } catch (err) {
-        if (!(err instanceof BadRequestException)) throw err;
-        const errorClass = err.constructor.name;
-        const error = sanitizeLogValue(err.message);
-        this.logger.error(
-          `[smart_scope.count] [fail] scopeId=${smartScope.id} userId=${user.id} mediaType=${smartScope.mediaType} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${error}" - smart scope filter is invalid`,
+        const episodeCount = await this.podcastEpisodeRepo.countEpisodes(
+          smartScope.libraryId,
+          user.id,
+          toEpisodeRuleQuery(filter as PodcastScopeRules),
        );
-        return { ...this.toResponse(smartScope, user, koboSyncEnabledFor(smartScope)), [countKey]: null };
+        return { ...response, episodeCount };
      }
-    });
+
+      const timeZone = resolveTimeZone((user.settings as { timezone?: unknown } | undefined)?.timezone, 'UTC');
+      const where = this.queryBuilder.buildWhere(filter as GroupRule, {
+        accessibleLibraryIds: libraryIds,
+        userId: user.id,
+        timeZone,
+        contentFilters: user.isSuperuser ? undefined : user.contentFilters,
+      });
+      const bookCount = await this.bookReadService.countWhere(where);
+      return { ...response, bookCount };
+    } catch (err) {
+      if (!(err instanceof BadRequestException)) throw err;
+      const errorClass = err.constructor.name;
+      const error = sanitizeLogValue(err.message);
+      this.logger.error(
+        `[smart_scope.count] [fail] scopeId=${smartScope.id} userId=${user.id} mediaType=${smartScope.mediaType} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${error}" - smart scope filter is invalid`,
+      );
+      return { ...response, [countKey]: null };
+    }
  }

  async findOne(id: number, user: RequestUser) {
@@ -240,7 +242,7 @@ export class SmartScopeService {
      isPublic: dto.isPublic ?? false,
      syncToKobo,
    });
-    return this.toResponse(smartScope, user, smartScope.syncToKobo);
+    return this.toCountedResponse(smartScope, user, smartScope.syncToKobo);
  }

  /**
@@ -297,7 +299,7 @@ export class SmartScopeService {
      isPublic: dto.isPublic,
      syncToKobo: dto.syncToKobo,
    });
-    return this.toResponse(updated, user, await this.resolveKoboSyncEnabled(updated, user));
+    return this.toCountedResponse(updated, user, await this.resolveKoboSyncEnabled(updated, user));
  }

  async remove(id: number, user: RequestUser) {
@@ -149,6 +149,43 @@ describe('Catalog visibility (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () => {
    }
  });

+  it('returns current visible book counts when creating and updating a smart scope', async () => {
+    const created = await ctx.app.inject({
+      method: 'POST',
+      url: '/api/v1/smart-scopes',
+      headers: authHeader(scopedReader.accessToken),
+      payload: {
+        name: `${queryToken} Scope`,
+        icon: 'Aperture',
+        defaultSort: [],
+        filter: { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'title', operator: 'contains', value: queryToken }] },
+      },
+    });
+    expect(created.statusCode).toBe(201);
+    const scope = created.json() as { id: number; bookCount: number };
+    expect(scope.bookCount).toBe(1);
+
+    const updated = await ctx.app.inject({
+      method: 'PATCH',
+      url: `/api/v1/smart-scopes/${scope.id}`,
+      headers: authHeader(scopedReader.accessToken),
+      payload: {
+        filter: { type: 'group', join: 'AND', rules: [{ type: 'rule', field: 'title', operator: 'contains', value: `${queryToken} Filtered` }] },
+      },
+    });
+    expect(updated.statusCode).toBe(200);
+    expect(updated.json()).toEqual(expect.objectContaining({ id: scope.id, bookCount: 0 }));
+
+    const cleared = await ctx.app.inject({
+      method: 'PATCH',
+      url: `/api/v1/smart-scopes/${scope.id}`,
+      headers: authHeader(scopedReader.accessToken),
+      payload: { filter: null },
+    });
+    expect(cleared.statusCode).toBe(200);
+    expect(cleared.json()).toEqual(expect.objectContaining({ id: scope.id, filter: null, bookCount: 0 }));
+  });
+
  it('returns no suggestions when the reader cannot access a library', async () => {
    for (const endpoint of Object.keys(visible.names) as Array<keyof CatalogNames>) {
      await expect(search(endpoint, noLibraryReader.accessToken)).resolves.toEqual([]);