fix(user-settings): fix settings loading errors#527
Merged
balazs-szucs merged 6 commits intoApr 15, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUser repository eager-fetching methods were moved from the JPA interface into a new Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as Controller/Service
participant RepoIface as UserRepository (interface)
participant RepoImpl as UserRepositoryImpl
participant EM as EntityManager
participant DB as Database
Controller->>RepoIface: findByIdWithDetails(id)
RepoIface->>RepoImpl: delegate to custom impl
RepoImpl->>EM: createQuery("SELECT DISTINCT u ... LEFT JOIN FETCH ...")
EM->>DB: execute SQL (joins)
DB-->>EM: result rows
EM-->>RepoImpl: managed entity graph
RepoImpl->>RepoImpl: Hibernate.initialize(libraries, libraryPaths)
RepoImpl-->>Controller: Optional<BookLoreUserEntity> (initialized)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (1 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
booklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java (1)
11-14: Consider constructor injection for consistency with codebase patterns.While
@PersistenceContextfield injection is standard JPA, the coding guidelines prefer constructor injection via Lombok patterns. Using@RequiredArgsConstructorwith afinalfield would be more consistent.♻️ Suggested refactor
+import lombok.RequiredArgsConstructor; + +@RequiredArgsConstructor public class UserRepositoryImpl implements UserRepositoryCustom { - `@PersistenceContext` - private EntityManager em; + private final EntityManager em;As per coding guidelines: "Prefer constructor injection via Lombok patterns already used in the codebase."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java` around lines 11 - 14, Change field injection of the EntityManager to Lombok-style constructor injection: make the em field "private final EntityManager em" in UserRepositoryImpl, annotate the class with Lombok's `@RequiredArgsConstructor`, remove the `@PersistenceContext` annotation from the field, and apply `@PersistenceContext` to the generated constructor parameter (or otherwise ensure the container injects the constructor parameter). This preserves JPA injection for EntityManager while using constructor injection consistent with the codebase patterns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@booklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java`:
- Around line 11-14: Change field injection of the EntityManager to Lombok-style
constructor injection: make the em field "private final EntityManager em" in
UserRepositoryImpl, annotate the class with Lombok's `@RequiredArgsConstructor`,
remove the `@PersistenceContext` annotation from the field, and apply
`@PersistenceContext` to the generated constructor parameter (or otherwise ensure
the container injects the constructor parameter). This preserves JPA injection
for EntityManager while using constructor injection consistent with the codebase
patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3258903d-ebe3-43c3-b106-39b530b07660
📒 Files selected for processing (3)
booklore-api/src/main/java/org/booklore/repository/UserRepository.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryCustom.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java
📜 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 (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Frontend Lint Threshold Check
🧰 Additional context used
📓 Path-based instructions (1)
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/repository/UserRepositoryCustom.javabooklore-api/src/main/java/org/booklore/repository/UserRepository.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to booklore-api/src/**/*Entity.java : Keep JPA entities on the `*Entity` suffix
📚 Learning: 2026-04-10T08:15:37.436Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepositoryCustom.javabooklore-api/src/main/java/org/booklore/repository/UserRepository.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java
📚 Learning: 2026-04-14T12:43:08.698Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepositoryCustom.javabooklore-api/src/main/java/org/booklore/repository/UserRepository.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java
📚 Learning: 2026-04-13T05:02:01.463Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 486
File: booklore-api/src/main/java/org/booklore/service/oidc/OidcGroupMappingService.java:130-159
Timestamp: 2026-04-13T05:02:01.463Z
Learning: In grimmory-tools/grimmory, treat `permissionDemoUser`/`permission_demo_user` as a restricting “limited demo mode” flag: it must never be set to `true` as part of any “grant all permissions” or admin-grant flow. Specifically, in code paths that grant admin permissions (for example the OIDC `isAdmin` branch in `OidcGroupMappingService.applyPermissions`), ensure `permissionDemoUser` is intentionally excluded from the admin-permission block and cannot be turned on; enabling it for admin users is counterproductive.
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepositoryCustom.javabooklore-api/src/main/java/org/booklore/repository/UserRepository.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java
📚 Learning: 2026-04-14T12:46:56.673Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:46:56.673Z
Learning: For this project (grimmory-tools/grimmory / booklore-api), prefer modern Java 25 idioms and language/library features when reviewing or suggesting changes. Favor current constructs such as unnamed variables (e.g., `_`) where applicable, records, pattern matching for `switch`/`instanceof`, sealed classes, sequenced collections, `Stream.toList()`, and text blocks. Avoid proposing regressions to older/legacy equivalents unless there’s a concrete compatibility constraint (e.g., a required older runtime/library behavior).
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepositoryCustom.javabooklore-api/src/main/java/org/booklore/repository/UserRepository.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to booklore-api/src/**/*Entity.java : Keep JPA entities on the `*Entity` suffix
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepository.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to booklore-api/src/**/*.java : Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce `Autowired` field injection
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepository.java
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to booklore-api/src/**/*.java : Use MapStruct for entity/DTO mapping
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepository.java
🔇 Additional comments (6)
booklore-api/src/main/java/org/booklore/repository/UserRepositoryCustom.java (1)
1-13: LGTM!Clean custom repository interface following Spring Data JPA conventions. The method signatures appropriately define the contract for fetching users with eagerly-loaded associations.
booklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java (2)
16-41: Well-structured multi-query approach.The two-query pattern correctly avoids Cartesian products by fetching different association subsets separately, relying on the persistence context to merge results on the same managed entity. The
@Transactional(readOnly = true)ensures both queries operate within the same persistence context.
43-64: LGTM!Same effective pattern as
findByIdWithDetailsapplied to bulk fetching. The second query correctly populates the persistence context for all users' libraries without filtering.booklore-api/src/main/java/org/booklore/repository/UserRepository.java (3)
15-16: LGTM!Correct integration of the custom repository interface following Spring Data JPA conventions.
25-38: LGTM on the explicit JPQL queries.The queries correctly fetch the necessary associations for each use case, and
findByIdWithLibrariesappropriately includespermissionsto support downstream access patterns.
16-16: Review comment inaccurately identifies the problematic caller.While the concern about lazy-loading is valid, the original review incorrectly flags
BookUpdateService.findUserOrThrow(). This method is only called from@Transactionalmethods (assignShelvesToBooks,updateReadStatus,updatePersonalRating,resetPersonalRating), ensuring lazy associations remain within transaction boundaries.The actual problem is
OpdsBookService.validateBookContentAccess(), which is not@Transactionalyet accesses the lazypermissionsfield on line 440 (entity.getPermissions().isPermissionAdmin()). This method is invoked from controller@GetMappingendpoints without transaction context, creating a realLazyInitializationExceptionrisk when the permissions association is accessed.> Likely an incorrect or invalid review comment.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts`:
- Around line 56-72: The effect loadFontsEffect reads this.userSettings (and
uses resetToDefaultFont()) but userSettings is an `@Input`() and not a tracked
signal, so the effect won't re-run when preferences change; convert userSettings
into an input signal using Angular's input() (e.g., declare userSettings =
input<IUserSettings | undefined>() in the component) and update references
inside loadFontsEffect to use the signal (e.g., this.userSettings() or the
chosen accessor) so the effect re-triggers when preferences change; ensure
resetToDefaultFont() still works with the new signal and keep the existing
customFontService.fonts(), isFontsLoading(), and loadAllFonts() logic intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0c5af53-1374-4bc2-83b1-7a58395aacb3
📒 Files selected for processing (6)
booklore-api/src/main/java/org/booklore/repository/UserRepository.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryCustom.javabooklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.javafrontend/src/app/features/settings/custom-fonts/custom-fonts.component.tsfrontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.htmlfrontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- booklore-api/src/main/java/org/booklore/repository/UserRepositoryCustom.java
- booklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java
📜 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). (3)
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (4)
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.htmlfrontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.tsfrontend/src/app/features/settings/custom-fonts/custom-fonts.component.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.tsfrontend/src/app/features/settings/custom-fonts/custom-fonts.component.ts
frontend/src/app/**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
Keep Angular code on standalone components. Do not add NgModules
Files:
frontend/src/app/features/settings/custom-fonts/custom-fonts.component.ts
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/repository/UserRepository.java
🧠 Learnings (8)
📚 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/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.tsfrontend/src/app/features/settings/custom-fonts/custom-fonts.component.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/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.tsfrontend/src/app/features/settings/custom-fonts/custom-fonts.component.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/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.tsfrontend/src/app/features/settings/custom-fonts/custom-fonts.component.ts
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to booklore-api/src/**/*Entity.java : Keep JPA entities on the `*Entity` suffix
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepository.java
📚 Learning: 2026-04-10T08:15:37.436Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepository.java
📚 Learning: 2026-04-14T12:43:08.698Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepository.java
📚 Learning: 2026-04-13T05:02:01.463Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 486
File: booklore-api/src/main/java/org/booklore/service/oidc/OidcGroupMappingService.java:130-159
Timestamp: 2026-04-13T05:02:01.463Z
Learning: In grimmory-tools/grimmory, treat `permissionDemoUser`/`permission_demo_user` as a restricting “limited demo mode” flag: it must never be set to `true` as part of any “grant all permissions” or admin-grant flow. Specifically, in code paths that grant admin permissions (for example the OIDC `isAdmin` branch in `OidcGroupMappingService.applyPermissions`), ensure `permissionDemoUser` is intentionally excluded from the admin-permission block and cannot be turned on; enabling it for admin users is counterproductive.
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepository.java
📚 Learning: 2026-04-14T12:46:56.673Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:46:56.673Z
Learning: For this project (grimmory-tools/grimmory / booklore-api), prefer modern Java 25 idioms and language/library features when reviewing or suggesting changes. Favor current constructs such as unnamed variables (e.g., `_`) where applicable, records, pattern matching for `switch`/`instanceof`, sealed classes, sequenced collections, `Stream.toList()`, and text blocks. Avoid proposing regressions to older/legacy equivalents unless there’s a concrete compatibility constraint (e.g., a required older runtime/library behavior).
Applied to files:
booklore-api/src/main/java/org/booklore/repository/UserRepository.java
🔇 Additional comments (6)
booklore-api/src/main/java/org/booklore/repository/UserRepository.java (2)
11-11: Clean repository composition pattern.Extending
UserRepositoryCustomalongsideJpaRepositoryis the correct Spring Data JPA approach for delegating complex eager-fetching logic to a custom implementation. Spring will automatically wireUserRepositoryImplby naming convention.
10-20: Removal is safe—no usages offindByUsernameWithDetailsfound in the codebase.The method does not appear in
UserRepository,UserRepositoryCustom, orUserRepositoryImpl, and a comprehensive search confirms zero references across all Java files. The removal incurs no breaking changes.frontend/src/app/features/settings/custom-fonts/custom-fonts.component.ts (2)
39-40: Good source-of-truth cleanup.Delegating
customFontsandisLoadingtoCustomFontServiceremoves duplicate component state and keeps the UI aligned with the query cache updates.
85-89: Nice teardown handling.Using
takeUntilDestroyed(this.destroyRef)here avoids dialog-close subscription cleanup drifting back into manual lifecycle code. Based on learnings: preferDestroyRefwithtakeUntilDestroyed(destroyRef)for teardown in Angular v16+ instead of manually trackingSubscriptionarrays and callingunsubscribe()inngOnDestroy().frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts (1)
45-54: Nice move to computed state.This is a cleaner fit for the service-driven font model than keeping local mutable arrays/flags in sync by hand. Based on learnings: prefer Angular signals (e.g.,
signal,computed) overBehaviorSubject/Observablefor state where signals/computed values fit the use case.frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.html (1)
75-92: Template update matches the signal refactor.Calling
fonts()/customFontsReady()here and tracking byfont.keyis the right follow-through for the computed-signal change in the component.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts`:
- Around line 45-62: customFontsReady currently returns true as soon as metadata
exists, but actual registration happens asynchronously in loadFontsEffect using
customFontService.loadAllFonts() (which uses Promise.allSettled and therefore
won’t reject), so derive readiness from the real load result: add a reactive
state (e.g., fontsLoadResult or fontsLoaded boolean) updated inside
loadFontsEffect after awaiting this.customFontService.loadAllFonts(fonts),
inspect the allSettled results to log per-font errors and set the state true
only when there are fulfilled entries (or false if all failed), and change
computed customFontsReady to return based on that state (falling back to
isFontsLoading()/metadata only while loading); update the existing .catch usage
since loadAllFonts won’t reject — handle errors by reading the settled results
instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed684a50-9519-4bc8-a363-199b7784e03a
📒 Files selected for processing (1)
frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts
📜 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). (4)
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (2)
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts
🧠 Learnings (3)
📚 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/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.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/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.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/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts
🔇 Additional comments (1)
frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts (1)
29-54: Nice signal-based cleanup.Using
input.required()pluscomputed()here removes the old imperative syncing cleanly and fits the rest of the frontend’s signal-first direction.Based on learnings: "Prefer Angular signals (e.g.,
signal,computed) overBehaviorSubject/Observablefor state where signals/computed values fit the use case."
…ate management again
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/app/shared/service/custom-font.service.ts`:
- Around line 30-32: The isFontsReadyReadonly flag can stay true after logout
because only the query cache is cleared; update the token-null effect that
handles clearing auth state to also reset the font readiness by calling
isFontsReadyInternal.set(false) (referencing the service's isFontsReadyInternal
and isFontsReady symbols) so consumers don't see a stale ready state after the
token is cleared.
- Line 130: The method declaration async loadAllFonts(fonts: CustomFont[]):
Promise<void> is over-indented; align it to the file's 2-space TypeScript
indentation style by moving the line left to match other top-level/ class method
declarations (ensure the method's opening line, its body and closing brace use
2-space indentation consistent with other methods in the class such as any
nearby methods like loadFont or unloadFont).
- Around line 130-144: loadAllFonts can race: an older call may set
isFontsReadyInternal true while a newer call is still running; fix by
introducing a per-instance incremental loadId (or token) checked by each
invocation of loadAllFonts and by loadFontFace resolution—on entry increment and
capture currentId, and only set isFontsReadyInternal.set(true) (including the
early-return path when fonts.length===0) if the captured currentId equals the
latest instance loadId; update references in loadAllFonts and any
completion/error paths so only the latest call flips readiness.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46c55fdd-0f28-4fb4-9379-aecb0cbe4459
📒 Files selected for processing (2)
frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.tsfrontend/src/app/shared/service/custom-font.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/features/settings/reader-preferences/epub-reader-preferences/epub-reader-preferences-component.ts
📜 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). (3)
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (2)
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/shared/service/custom-font.service.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/shared/service/custom-font.service.ts
🧠 Learnings (3)
📚 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/shared/service/custom-font.service.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/shared/service/custom-font.service.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/shared/service/custom-font.service.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/src/app/shared/service/custom-font.service.ts (1)
35-40:⚠️ Potential issue | 🟠 MajorInvalidate font-load runs on logout to prevent stale
ready=true.Line 39 resets readiness, but an older
loadAllFonts()can still complete and setisFontsReadyInternalback totrueat Line 155 (it still passes the run-id check at Line 146). Also, an unauthenticatedloadAllFonts([])call can currently mark readytrue. Invalidate active runs on token clear and guard unauthenticated loads.🔧 Proposed fix
constructor() { effect(() => { const token = this.token(); if (token === null) { + // Invalidate any in-flight loadAllFonts run. + this.loadAllFontsRunId++; this.queryClient.removeQueries({queryKey: CUSTOM_FONTS_QUERY_KEY}); this.isFontsReadyInternal.set(false); } }); } @@ async loadAllFonts(fonts: CustomFont[]): Promise<void> { + if (!this.token()) { + this.isFontsReadyInternal.set(false); + return; + } + const runId = ++this.loadAllFontsRunId; this.isFontsReadyInternal.set(false);Also applies to: 133-139, 146-147, 155-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/service/custom-font.service.ts` around lines 35 - 40, When token becomes null in the effect, invalidate any active font-load run and prevent unauthenticated completions from flipping readiness: explicitly bump/cancel the font-load run identifier used by loadAllFonts (the same run-id checked in loadAllFonts) and keep calling this.queryClient.removeQueries({queryKey: CUSTOM_FONTS_QUERY_KEY}) and this.isFontsReadyInternal.set(false). In loadAllFonts(), before setting this.isFontsReadyInternal to true, ensure the current token is present (this.token() !== null) and that the completing run-id still matches the latest run-id; also guard against empty unauthenticated loads (e.g., require token or non-empty fonts) so an unauthenticated loadAllFonts([]) cannot mark ready.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/src/app/shared/service/custom-font.service.ts`:
- Around line 35-40: When token becomes null in the effect, invalidate any
active font-load run and prevent unauthenticated completions from flipping
readiness: explicitly bump/cancel the font-load run identifier used by
loadAllFonts (the same run-id checked in loadAllFonts) and keep calling
this.queryClient.removeQueries({queryKey: CUSTOM_FONTS_QUERY_KEY}) and
this.isFontsReadyInternal.set(false). In loadAllFonts(), before setting
this.isFontsReadyInternal to true, ensure the current token is present
(this.token() !== null) and that the completing run-id still matches the latest
run-id; also guard against empty unauthenticated loads (e.g., require token or
non-empty fonts) so an unauthenticated loadAllFonts([]) cannot mark ready.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54a3136e-1626-4e2c-9ee8-532dbb6e32ee
📒 Files selected for processing (2)
booklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.javafrontend/src/app/shared/service/custom-font.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- booklore-api/src/main/java/org/booklore/repository/UserRepositoryImpl.java
📜 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). (1)
- GitHub Check: Packaging Smoke Test
🧰 Additional context used
📓 Path-based instructions (2)
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/shared/service/custom-font.service.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/shared/service/custom-font.service.ts
🧠 Learnings (4)
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to frontend/src/**/*.{ts,tsx,html,scss} : Use 2-space indentation in TypeScript, HTML, and SCSS files
Applied to files:
frontend/src/app/shared/service/custom-font.service.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/shared/service/custom-font.service.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/shared/service/custom-font.service.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/shared/service/custom-font.service.ts
🔇 Additional comments (1)
frontend/src/app/shared/service/custom-font.service.ts (1)
31-33: Good readonly signal exposure for readiness state.
isFontsReadyis exposed as readonly while mutation stays internal, which prevents accidental external writes.
zachyale
approved these changes
Apr 15, 2026
zachyale
pushed a commit
to zachyale/grimmory
that referenced
this pull request
Apr 17, 2026
* refactor(user): optimize detail fetching with custom repository implementation * refactor(fonts): fix font loading and improve custom font handling * refactor(user-data): update userSettings to use input() for better reactivity * refactor(fonts): improve font loading logic and improve readi ness state management again * refactor(user-data): simplify user fetching logic and improve batch loading * refactor(custom-fonts): improve font loading logic with run ID for better state management
zachyale
pushed a commit
to zachyale/grimmory
that referenced
this pull request
Apr 17, 2026
* refactor(user): optimize detail fetching with custom repository implementation * refactor(fonts): fix font loading and improve custom font handling * refactor(user-data): update userSettings to use input() for better reactivity * refactor(fonts): improve font loading logic and improve readi ness state management again * refactor(user-data): simplify user fetching logic and improve batch loading * refactor(custom-fonts): improve font loading logic with run ID for better state management
zachyale
pushed a commit
that referenced
this pull request
Apr 17, 2026
* refactor(user): optimize detail fetching with custom repository implementation * refactor(fonts): fix font loading and improve custom font handling * refactor(user-data): update userSettings to use input() for better reactivity * refactor(fonts): improve font loading logic and improve readi ness state management again * refactor(user-data): simplify user fetching logic and improve batch loading * refactor(custom-fonts): improve font loading logic with run ID for better state management
zachyale
pushed a commit
that referenced
this pull request
Apr 22, 2026
* refactor(user): optimize detail fetching with custom repository implementation * refactor(fonts): fix font loading and improve custom font handling * refactor(user-data): update userSettings to use input() for better reactivity * refactor(fonts): improve font loading logic and improve readi ness state management again * refactor(user-data): simplify user fetching logic and improve batch loading * refactor(custom-fonts): improve font loading logic with run ID for better state management
dsmouse
pushed a commit
to dsmouse/grimmory
that referenced
this pull request
May 11, 2026
* refactor(user): optimize detail fetching with custom repository implementation * refactor(fonts): fix font loading and improve custom font handling * refactor(user-data): update userSettings to use input() for better reactivity * refactor(fonts): improve font loading logic and improve readi ness state management again * refactor(user-data): simplify user fetching logic and improve batch loading * refactor(custom-fonts): improve font loading logic with run ID for better state management
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Linked Issue: Fixes #
Changes
This pull request refactors how user data is fetched in the backend and improves the handling and reactivity of custom fonts in the Angular frontend. The most significant changes include moving complex user queries to a custom repository implementation for better maintainability and efficiency, and updating the frontend to use computed properties and effects for managing custom fonts, resulting in cleaner and more robust code.
Backend: User Repository Refactor
UserRepositoryto a newUserRepositoryCustominterface and its implementationUserRepositoryImpl, using custom JPQL queries and explicit Hibernate initialization for related collections. This improves maintainability and avoids N+1 query issues.Frontend: Custom Fonts Handling Improvements
customFontsandisLoadinginCustomFontsComponentwith reactive getters that delegate toCustomFontService, simplifying the component and ensuring state consistency.EpubReaderPreferencesComponentto usecomputedproperties forfontsandcustomFontsReady, and replaced imperative font dropdown updating logic with a reactive approach. This ensures the UI stays in sync with font state and properly handles font deletion or loading errors.fonts()andcustomFontsReady()as functions, reflecting the switch to computed properties.Summary by CodeRabbit
Refactor
New Features
Bug Fixes