Skip to content

fix(filters): wire remaining app book rating filters to api#816

Open
zachyale wants to merge 1 commit intodevelopfrom
fix/fix-app-book-rating-filter-wiring
Open

fix(filters): wire remaining app book rating filters to api#816
zachyale wants to merge 1 commit intodevelopfrom
fix/fix-app-book-rating-filter-wiring

Conversation

@zachyale
Copy link
Copy Markdown
Member

@zachyale zachyale commented Apr 23, 2026

Description

Wires the remaining app book rating sidebar filters through the API filtering, removes the deprecated local sidebar filter helper

Linked Issue: Fixes #815

Changes

The app book browser now relies on backend filtering, but three visible sidebar filters were still not being forwarded from BookBrowserComponent into AppBookFilters:

  • lubimyczytacRating
  • ranobedbRating
  • audibleRating

That meant the filters appeared in the UI and the backend supported them, but selecting them did not affect the /api/v1/app/books query- this PR fixes that, and kills the unused sidebar filter

Summary by CodeRabbit

  • New Features

    • Added support for filtering books by three additional rating sources: Lubimyczytac, Ranobe DB, and Audible ratings.
  • Refactor

    • Restructured internal filtering system for improved maintainability.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ae8d5863-1edb-42e5-babd-62f56f5c4b3b

📥 Commits

Reviewing files that changed from the base of the PR and between 03389d7 and e83f7ab.

📒 Files selected for processing (4)
  • frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
  • frontend/src/app/features/book/components/book-browser/book-browser.component.ts
  • frontend/src/app/features/book/components/book-browser/filters/sidebar-filter.spec.ts
  • frontend/src/app/features/book/components/book-browser/filters/sidebar-filter.ts
💤 Files with no reviewable changes (2)
  • frontend/src/app/features/book/components/book-browser/filters/sidebar-filter.spec.ts
  • frontend/src/app/features/book/components/book-browser/filters/sidebar-filter.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Test Suite / Frontend Tests
  • GitHub Check: Test Suite / Backend Tests
  • GitHub Check: Analyze (java-kotlin)
  • GitHub Check: Frontend Lint Threshold Check
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (5)
frontend/src/**/*.{ts,tsx,html,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Use 2-space indentation in TypeScript, HTML, and SCSS in frontend code

Files:

  • frontend/src/app/features/book/components/book-browser/book-browser.component.ts
  • frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
frontend/src/**/*.component.ts

📄 CodeRabbit inference engine (AGENTS.md)

Keep Angular code on standalone components. Do not add NgModules in frontend code

Files:

  • frontend/src/app/features/book/components/book-browser/book-browser.component.ts
frontend/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Prefer inject() over constructor injection in frontend code

Files:

  • frontend/src/app/features/book/components/book-browser/book-browser.component.ts
  • frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
frontend/src/**/*.{ts,tsx,html}

📄 CodeRabbit inference engine (AGENTS.md)

frontend/src/**/*.{ts,tsx,html}: Follow frontend/eslint.config.js: component selectors use app-, directive selectors use app, and any is disallowed in frontend code
Put user-facing strings in Transloco files under frontend/src/i18n/

Files:

  • frontend/src/app/features/book/components/book-browser/book-browser.component.ts
  • frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
frontend/src/**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Vitest for tests in frontend code

Files:

  • frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
🧠 Learnings (4)
📚 Learning: 2026-04-04T14:03:28.295Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 372
File: frontend/src/app/features/book/service/app-books-api.service.ts:150-178
Timestamp: 2026-04-04T14:03:28.295Z
Learning: In `frontend/src/app/features/book/service/app-books-api.service.ts`, the `summaryToBook` function intentionally uses `as unknown as Book` to cast a partial object to the `Book` type. The returned object omits some required `Book` fields (e.g. `metadata.bookId`, `pdfProgress.page`) and includes an extra `bookFiles: []` property not in the `Book` interface. This is by design — consumers of `AppBooksApiService.books()` (e.g. `BookCardComponent`) only access the subset of properties that are populated from `AppBookSummary`, so no runtime issues occur. Do not flag this as a type safety error.

Applied to files:

  • frontend/src/app/features/book/components/book-browser/book-browser.component.ts
  • frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
📚 Learning: 2026-04-05T21:16:01.715Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 385
File: frontend/src/app/app.component.ts:55-56
Timestamp: 2026-04-05T21:16:01.715Z
Learning: When reviewing code in the Grimmory frontend (Angular), prefer modern Angular patterns. Specifically: (1) Prefer `DestroyRef` with `takeUntilDestroyed(destroyRef)` for teardown in Angular v16+ instead of manually tracking `Subscription` arrays and calling `unsubscribe()` in `ngOnDestroy()`. (2) Prefer `inject()` for dependency injection over constructor injection where appropriate. (3) Prefer Angular signals (e.g., `signal`, `computed`) over `BehaviorSubject`/`Observable` for state where signals/computed values fit the use case. Flag older patterns when they can be replaced with these modern equivalents without changing behavior.

Applied to files:

  • frontend/src/app/features/book/components/book-browser/book-browser.component.ts
  • frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
📚 Learning: 2026-04-07T09:28:09.587Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 393
File: frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts:255-263
Timestamp: 2026-04-07T09:28:09.587Z
Learning: In this Angular frontend (under frontend/src/app/), flag manual resource management/cleanup patterns when there is an Angular v16+ automatic alternative. Examples to prefer: (1) Instead of manually pairing document/window event listeners with stored cleanup functions (e.g., add/removeEventListener with mouseMoveCleanup/documentClickCleanup/keydownCleanup/touchCleanup fields), register teardown via DestroyRef.onDestroy(cleanupFn) (or equivalent Angular v16+ teardown mechanism). (2) Instead of storing Subscriptions in fields and explicitly unsubscribing in ngOnDestroy (e.g., annotationSaveSubscription/annotationCacheSubscription), use takeUntilDestroyed(destroyRef) (piped into the observable) or other Angular v16+ primitives. (3) If teardown is lifecycle-coupled and can be automated via DestroyRef/takeUntilDestroyed/signals (or other Angular v16+ mechanisms), prefer the automated approach over manual ngOnDestroy cleanup. Raise a review finding for the manual pattern and recommend the aut...

Applied to files:

  • frontend/src/app/features/book/components/book-browser/book-browser.component.ts
  • frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
📚 Learning: 2026-04-11T03:55:57.229Z
Learnt from: zachyale
Repo: grimmory-tools/grimmory PR: 439
File: frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts:178-196
Timestamp: 2026-04-11T03:55:57.229Z
Learning: In this Angular frontend (frontend/src/app/), prefer the team’s reactive i18n “signal/computed” pattern: (1) For individual reactive translated strings, use `translateSignal()` from `jsverse/transloco`. (2) For option/label arrays that must update on language switch, create a single `activeLang` signal with `toSignal(t.langChanges$, { initialValue: t.getActiveLang() })`, then derive the arrays as `computed()` signals that read `activeLang()`. This should avoid manual `langChanges$` subscriptions and any `ngOnDestroy` subscription cleanup; prefer this over subscribing in `ngOnInit` when implementing reactive localization.

Applied to files:

  • frontend/src/app/features/book/components/book-browser/book-browser.component.ts
  • frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
🔀 Multi-repo context grimmory-tools/grimmory-docs

Linked repositories findings

grimmory-tools/grimmory-docs

  • docs mention the three rating filter keys as supported metadata fields (docs/magic-shelf.md). Examples/reference lines include:

    • docs/magic-shelf.md:108 — table entry listing "Audible Rating" -> audibleRating (Decimal 0-5). [::grimmory-tools/grimmory-docs::]
    • docs/magic-shelf.md:110 — table entry listing "RanobeDB Rating" -> ranobedbRating. [::grimmory-tools/grimmory-docs::]
    • docs/magic-shelf.md:111 — table entry listing "Lubimyczytac Rating" -> lubimyczytacRating. [::grimmory-tools/grimmory-docs::]
    • docs/magic-shelf.md:205 — metadata-presence enumeration includes ranobedbRating, lubimyczytacRating, audibleRating. [::grimmory-tools/grimmory-docs::]
    • docs/magic-shelf.md:1414 — example filter JSON uses "field": "audibleRating". [::grimmory-tools/grimmory-docs::]
  • No code-level consumers (components, API clients, or BookBrowserComponent implementations) were found in this documentation repo. Searches for AppBooksApi/AppBookFilters/BookBrowserComponent and for the deleted sidebar-filter utility returned no matches in source code here; only image/docs references to sidebar filters were found (docs/book-browser/grid.md). [::grimmory-tools/grimmory-docs::]

Conclusion: Documentation confirms backend/supporting contract for the three rating keys (strengthening the PR objective that wiring them to the backend is correct). No other cross-repo code consumers were discovered in this repo.

🔇 Additional comments (2)
frontend/src/app/features/book/components/book-browser/book-browser.component.ts (1)

253-255: LGTM — rating filter wiring is consistent.

The three new cases follow the exact same pattern as the adjacent goodreadsRating/hardcoverRating mappings and match the corresponding optional string[] fields on AppBookFilters. This closes the frontend-to-backend mapping gap for the three rating sidebar filters as stated in the PR objectives.

frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts (1)

502-521: Test adequately covers the new wiring.

The test calls onFilterSelected with the three new rating keys and asserts setFilters is invoked with a payload containing libraryId, filterMode, and all three rating arrays — which exercises the new switch branches in syncApiStateEffect. Using expect.objectContaining keeps the assertion robust to unrelated fields.

One small consideration: since the harness's AppBooksApiService.setFilters is installed via useFactory as a plain function (not a method on a prototype), vi.spyOn(appBooksApi, 'setFilters') works because the property is configurable/writable, but it will not capture the very first setFilters call triggered by the initial effect run prior to the spy being attached. That is actually desirable here (you're asserting on the latest call after onFilterSelected), so toHaveBeenLastCalledWith is the right choice. No change required.


📝 Walkthrough

Walkthrough

This PR removes deprecated local filtering logic (sidebar-filter utility and tests) and wires three additional rating filter types (lubimyczytacRating, ranobedbRating, audibleRating) through the server-side filtering path in BookBrowserComponent.

Changes

Cohort / File(s) Summary
Server-side filter synchronization
frontend/src/app/features/book/components/book-browser/book-browser.component.ts, frontend/src/app/features/book/components/book-browser/book-browser.component.spec.ts
Added recognition for three additional sidebar rating filter types and forwarded them to AppBooksApiService.setFilters. New test verifies filters are correctly passed to the API with libraryId and filterMode.
Deprecated local filtering removal
frontend/src/app/features/book/components/book-browser/filters/sidebar-filter.ts, frontend/src/app/features/book/components/book-browser/filters/sidebar-filter.spec.ts
Deleted entire local sidebar filter utility module and its test suite, removing 9 exported helper functions and all client-side filter matching logic that is now handled server-side.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

frontend, enhancement

Suggested reviewers

  • imajes
  • balazs-szucs

Poem

🐰 Dead code departs, three filters find their way,
To backend fields where they belong to stay,
No local logic clutters up our sight,
Just clean wired mappings, crystal bright! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title follows conventional commit format with 'fix' prefix and descriptive message about wiring rating filters to API.
Description check ✅ Passed The description includes all required template sections with clear explanation of changes and properly formatted linked issue reference.
Linked Issues check ✅ Passed All objectives from issue #815 are met: deprecated sidebar-filter utility removed, three rating filters wired to API, and backend coverage verified.
Out of Scope Changes check ✅ Passed All changes directly address issue #815 requirements: wiring three rating filters to API and removing deprecated local filtering utility.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/fix-app-book-rating-filter-wiring
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/fix-app-book-rating-filter-wiring

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix lubimyczytac + ranobedb + audible app book rating filters

1 participant