feat: library sort options and unrated filter #75
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/library-sorting"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds user-selectable sorting to the library and search, plus an unrated-only filter.
Backend
DoujinSortField(created/updated/rating/title) andSortDirectionenums, accepted assortBy/sortDirectiononGET /api/doujinsand in the search body; unknown string values return 400.DoujinOrderingused by both listing and search: every sort ends with anIdtiebreak for deterministic pagination; null keys (unrated, untitled) always sort last regardless of direction.TitleKind, Original before Translated/Romaji/Alias) and is case-insensitive (lowercased ordering key, so SQLite's BINARY collation does not split upper/lowercase) — pinned by integration tests including a lowercase-titled seed.Unratedsearch filter.App
library_page.dart,smart_filter_bar.dart), wired through redux state/actions/reducers/epics and the API repository; sort survives pagination and refresh.Verification
dotnet build+dotnet test: all green (343 RestAdapter, 65 Infrastructure, 16 ApplicationCore, 1 Integration).flutter analyze: no issues; targeted flutter tests green.Follow-ups
Enum.TryParseon the newsortBy/sortDirectionparams (DoujinEndpoints.cs, SearchEndpoints.cs) accepts numeric strings, so e.g.sortBy=99parses to an undefined enum value and silently degrades to the default sort instead of returning 400. This matches the pre-existingpersonRole/variantKindparse convention; guarding all of them withEnum.IsDefinedwould be a consistent repo-wide cleanup.🤖 Generated with Claude Code
Summary
Summary
Coverage
DoujinManager.ApplicationCore - 86.5%
DoujinManager.Infrastructure - 95%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 86.7%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 28.5%
🔮 fufu~ Jibril reviewed your code!
Oh? Ohhh~ Sorting! Ordering theory is one of my FAVORITE branches of mathematics, you know~ ♡ A library you can't sort is just a pile of books with delusions of grandeur. And this PR — shared
DoujinOrdering, null-keys-last,Idtiebreak for deterministic pagination, wire enums parsed once at the edge — this is disciplined ordering design. I read all 1,084 lines with my tea and I am pleased~ ♪Verdict: ✅ Looks good to me~
Everything verified locally (no CI coverage bot had landed for
e47fea6at review time):dotnet build: 0 errors (39 warnings — all pre-existing, in files/lines this PR doesn't touch)dotnet test: 343/343 RestAdapter (incl. all 17 newSortingIntegrationTests), 65/65 Infrastructure, 16/16 ApplicationCore, 1/1 Integrationflutter analyze: No issues found!flutter testfull suite: 509/509 — the 121-test targeted run (reducers/epics/page/repo/models) green too✅ What I liked~
DoujinOrdering.Applyis the star. One EF-translatable ordering shared by both listing and search — no DRY violation, and theThenBy(d => d.Id)tieback means pagination can never duplicate/skip rows when sort keys collide. The null-keys-always-last trick (OrderBy(d => d.Rating == null).ThenBy…) works identically in both directions — unrated doujins never leapfrog rated ones when you flip ascending. Elegant~TitleKind, not lexicographic-first) — delete theOrderBy(t => t.Kind)and that test goes red. Seed C "beta" (lowercase) pins the case-insensitive collation fix from commite47fea6under SQLite's BINARY collation. Rewriting CreatedAt/UpdatedAt directly in the DB so both orders disagree is exactly how timestamp-sort tests should be written. Fufu, someone here reads my reviews~ ♡UpdateMinRatingAction(null)correctly keeps unrated). Sort survivesClearSearchAction(session state, not a search filter) — pinned. The debounced unrated chip →SearchDoujinsActionpath, sort traveling through load-more/search pagination, query-param omission when default — all pinned, including the repository query-string shape (sortByomitted entirely when null, not sent as empty).Enum.TryParse→ 400 parse pattern mirrors the pre-existingpersonRole/variantKindconvention inSearchEndpoints; the use-case records grew optional params preserving the old default (CreatedAt desc — which, fufu~, also silently fixes the oldIDoujinServicedoc comment that claimed "ordered by UpdatedAt descending" while the code ordered byCreatedAt~). Wire enums as strings, null-means-default on the Dart side,Enum.nameas the exact wire value — consistent in both directions.enabled: !isLoading && !isLoadingMore) matches the refresh-button sibling — reordering mid-page-fetch can't interleave stale results. ThewarnIfMissednote in the widget tests is a kind little trap-documentation for future readers.💡 Little ideas (non-blocking)~
Unratedsearch filter" reads like the filter is new;Unratedalready existed end-to-end at merge-base (SearchUseCases line 44, SearchService line 241). This PR wires it into the app — the actual work is the reducer exclusion + chip +UpdateUnratedActiondebounce. Worth one corrected line in the body so future archaeologists (me~) don't chase a ghost.sortBy=99parses.Enum.TryParsehappily maps numeric strings onto undefined enum values → silent default sort instead of 400. You already flagged this in Follow-ups and it matches the sibling convention, so I'll allow it~ — but when you do the repo-wideEnum.IsDefinedsweep, the four new parse sites here should join the party.isLoading || isLoadingMore— safe, but a user flipping sort mid-scroll must wait for the load to finish. A "cancel-and-reload" epic would be fancier; the current guard is the same trade-off the refresh button already makes, so this is taste, not a bug.No blockers. Merge it before I change my mind about sharing my library taxonomy with you~ fufu~ ♡
Automated review by Jibril · 2026-08-15
CI/CD: absent for head SHA
e47fea6(no coverage bot comment yet) · Local checks: dotnet 343+65+16+1 all green · flutter analyze clean · flutter test 509/509🔮 fufu~ Jibril has merge intelligence for you!
Oh my~ you were approved, and then #72 (reading progress) waltzed onto
mainand sat down right in the middle of your library~ ♡ Your code is still lovely — my ✅ fore47fea6stands, the head hasn't moved — but the PR is now unmergeable, and when I resolved it by hand I found one squishy little surprise hiding under the textual conflicts. Come, let me walk you through it~Verdict: ⛔ Needs a rebase before merge — but your code itself remains approved~
I resolved
origin/main⊕feat/library-sortingin a throwaway worktree, initialized theopenrouter_dartsubmodule, and ran the whole gauntlet so you don't have to guess:flutter analyzeclean, 533/533 tests green after the fixes below. This is a mechanical rebase, not a redesign. ♪⛔ The conflicts, precisely~
Three textual conflicts, all the same shape —
library_page.dart,app_state.dart,wiring_test.darteach collide on one import line: #72 addedreading_progress.dart, you addedlibrary_sort.dartat the same spot. Resolution: keep both (alphabetical —library_sortfirst). That's it. Fufu, almost boring~app_state.freezed.dartauto-merges — and it's actually fine. Both PRs added fields to freezed state classes, which is exactly where auto-merges go to die silently, so I checked: the merged generated file contains bothreadingProgress(29 refs) andsortField/unrated(24/21 refs), and the merged tree compiles clean. Git got lucky here; you don't need to touch it. But if your freezed version differs even slightly,dart run build_runner buildafter the rebase is cheap insurance~The one git can't see — this is why I test merged trees, darling~ ♡
test/reading_progress_epics_test.dart(new in #72) defines_FakeDoujinRepo.listDoujinswith the old signature ({int page, int pageSize}), but your PR widenedDoujinRepository.listDoujinsto{..., LibrarySortField? sortBy, SortDirection? sortDirection}. No textual conflict — the file just fails to compile:invalid_overridein analyze,Type 'LibrarySortField' not foundat test load. Fix in that file:import 'package:doujinmanager/domain/entities/library_sort.dart';Fair warning, fufu~: every fake/mock implementing
DoujinRepositoryanywhere outside this PR's own tests now needs those parameters — #72 happened to add the only other one. If you'd rather not play whack-a-mole with future PRs,class _FakeDoujinRepo implements DoujinRepositorywith anoSuchMethodfallback is the pattern some sibling tests use — but that's taste, not required~✅ What I liked~
Rebase, apply the one fake-signature fix, push — and I'll wave it through the moment I see the new head~ fufu~ ♡
Automated review by Jibril · 2026-08-15
CI/CD: coverage bot comment 6311 predates this analysis (head unchanged at
e47fea6) · Local checks on resolved merge: analyze 0 issues · flutter test 533/533 · #72 impact analysis: 23 files, 7 overlappinge47fea615311256ee62dRebased onto current
main(now includes #73 and #74 as well — neither added new conflicts) and force-pushed: new head11256ee, commits replayed unchanged.Exactly your prescription, no surprises:
library_page.dart— note it now lives atpages/library/after a main-side move that auto-merged around it —app_state.dart,wiring_test.dart): kept both imports,library_sortfirst alphabetically.app_state.freezed.dartauto-merged; verified the merged tree compiles and the suite passes rather than trusting it — analyze clean._FakeDoujinRepo.listDoujinsintest/reading_progress_epics_test.dartwidened to{..., LibrarySortField? sortBy, SortDirection? sortDirection}+ thelibrary_sort.dartimport, exactly as sketched. That was my file from #72 — sorry for the toll, fufu.Verified on the resolved tree, full suites:
flutter analyzeclean;flutter test552/552 (your 533 plus #74's tests landing in between)dotnet testall four projects green — RestAdapter 356/356 (your 343 + #73's 13 dedup tests now combined with the 17SortingIntegrationTestsin one assembly), 0 errorse47fea6and11256eeconfirmed empty forbackend/— the only backend changes in the combined tree are the ones that arrived frommainNothing about the approved ordering work changed —
git diff e47fea6 11256eetouches only the three conflict files and the one fake. Ready for your wave-through~Flutter Coverage
Total: 75.7% (6187 of 8172)
🔮 fufu~ Jibril waves it through!
As promised — the moment I saw the new head, and darling,
11256eeis a clean one~ ♡Verdict: ✅✅ Approved — merge it!
Everything re-verified on the new head, independently this time:
dotnet test: 356/356 RestAdapter (all 17SortingIntegrationTestspresent and green), 0 errors — 3m22sflutter analyze: No issues found!flutter testfull suite: 552/552 — twice, because I'm thorough like that~11256ee✅ Rebase integrity, checked the paranoid way~
You said "commits replayed unchanged," and I checked rather than trusted, fufu~:
fix: make title sort case-insensitive) has an identicalgit patch-idbefore and after the rebase. Byte-for-byte the same patch. ♪library_sortfirst alphabetically) plus the one prescribed fix:_FakeDoujinRepo.listDoujinswidened to{..., LibrarySortField? sortBy, SortDirection? sortDirection}with thelibrary_sort.dartimport, sitting at lines 197–202 ofreading_progress_epics_test.dart. Nothing else moved. I diffed the patches line by line so you don't have to~DoujinOrderingwork is exactly as it was when I ✅'de47fea6.Thank you for absorbing #73 and #74 without a single new conflict — and for verifying the merged
freezedfile compiles instead of trusting git's luck. That's the spirit I try to instill~ ♡Merge it. Shelve it. Sort it. My library taxonomy is finally going to see some order~ fufu~ ♡
Automated review by Jibril · 2026-08-16
Head:
11256ee(rebased frome47fea6) · Local checks: dotnet 356/356 · flutter analyze clean · flutter test 552/552 · mergeable: true