feat: reorder a variant's chapters from the editor #155
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/reorder-chapters"
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?
What's in
ReadingOrder; page sort orders are untouched.PUT /api/variants/{variantId}/chapters/reorderwith{ "chapterIds": [...] }: the listed chapters getSortOrder0..n-1 in list order, unlisted ones follow behind them in their previous relative order, in oneSaveChangesthat ages the variant and doujin throughParentTouchHelper(ADR 0004). A foreign or duplicated id is a 400 naming it, an empty list a 400, an unknown variant a 404; every status is declared (ADR 0021), the chapter collection advertises areorderaction (ADR 0007), the OpenAPI snapshot is regenerated, the README API table and the editor sub-bullet are updated.DoujinRepository.reorderChaptersport + dio implementation,ReorderChaptersActionwhose epic persists the order and then reloads the variant (a failure goes toEditorErrorActionwith no reload), andChapterReorderDialog, a StatefulWidget owning its working order on top of material_ui'sReorderableListView(onReorderItem, own drag handles viaReorderableDragStartListenerso the up/down buttons work without a mouse).ReorderChaptersUseCase, so a bad list is a 400 naming the id;ChapterService.ReorderAsynconly applies the mapping plus the parent touch. This deliberately differs fromPageService.ReorderAsync, which throws and becomes a 500 (see Follow-ups). The static/reorderroute is mapped before/{chapterId:guid}.Verification
Executed:
scripts/verify.sh allfrom the worktree at this HEAD (0b1eb90), green end to end: backend restore / build (Release) / test with 0 failures — ApplicationCore.Tests 56, IntegrationTests 1, Infrastructure.Tests 224, RestAdapter.Tests 712 (1 m 5 s); app submodule / pub get / codegen /dart analyze"No issues found!" /flutter test1044 tests "All tests passed!";== verify OK (all) ==, exit 0. Tree clean before and after.Layers exercised:
ChapterReorderIntegrationTests(8 cases: persists dense 0..n-1 and GET chapters returns it;GET /api/variants/{id}embeds the new order; partial list keeps the unlisted chapters after it in their old order — the fixture first does a full reorder so insertion order cannot satisfy it; foreign id 400 naming it and nothing moves; duplicated id 400 and nothing moves; empty list 400; unknown variant 404; collection advertises the reorder action),ChildWriteTouchesParentIntegrationTestsextended withChildWrite.ReorderChapters(both theories),HypermediaIntegrationTests.ListChapters_Returns_CollectionLinksAndActionsextended.OpenApiSnapshotTestscompare-mode passes against the regeneratedbackend/openapi/v1.json.UseCaseTests.ReorderChaptersUseCaseTests(6 cases: list order passed through, partial list passed through, 404, empty / duplicate / foreign 400s naming the id, service not called on rejection).UpdatedAtadvanced).chapter_reorder_dialog_test(5 cases; the drag case drives a real pointer gesture —startGesture/moveBy/upon the row's handle — and all cases use bounded pumps, nopumpAndSettle),variants_tab_test(new case pinning theeditor.isSavingwiring intoChapterPanel),editor_epics_test(success reload targetsv1; failure surfaceseditor.errorwith no refresh),doujin_api_repository_test(PUT to/api/variants/{id}/chapters/reorderwith thechapterIdsbody).Mutation probes (each applied, the named tests run, red observed, source restored):
ParentTouchHelper.TouchVariantAsyncremoved fromChapterService.ReorderAsync-> bothChildWriteTouchesParentReorderChapters theories red. Duplicate check disabled ->A_Duplicated_Id_Is_Refused_And_Nothing_Moves+Problem_DuplicateId_ReturnsBadRequestNamingItred. Foreign check disabled ->A_Chapter_Of_Another_Variant_Is_Refused_By_Name_And_Nothing_Moves+Problem_ForeignId_ReturnsBadRequestNamingItred. Hypermedia reorder action removed ->The_Chapter_Collection_Advertises_The_Reorder_Action+ListChapters_Returns_CollectionLinksAndActionsred. Unlisted chapters keep their oldSortOrderinstead ofnextUnlisted++->A_Partial_List_Keeps_The_Unlisted_Chapters_After_It_In_Their_Old_Orderred. Empty-list check disabled ->Problem_EmptyList_ReturnsBadRequestred (the integrationAn_Empty_List_Is_Refusedstays green because[MinLength(1)]body validation viaservices.AddValidation()answers first, as intended; the use-case branch is pinned by the unit test only).RefreshVariantActionyield removed from the epic branch -> epic success test red.RefreshVariantAction(action.variantId)->RefreshVariantAction('some-other-variant')-> epic success test red (Expected: 'v1'). ChiponPressedno longer null whileisSaving-> chip test red.ChapterPanel(variant: variant, isSaving: editor.isSaving)->ChapterPanel(variant: variant)invariants_tab.dart-> newvariants_tab_testcase red (Expected: null, Actual: Closure); the same case also goes red when the chip is forced always-disabled._moveinserts atfrominstead ofto-> up/down and drag tests red. dio path changed to/chapters/order-> repository test red.initStatesort removed -> 'rows start in sort order' red. Save always enabled -> up/down test red at its 'Save disabled before any move' assertion.onReorderItemstubbed to a no-op -> drag test red.chapters.length > 1relaxed toisNotEmpty-> chip test red..OrderBy(c => c.SortOrder).ThenBy(c => c.CreatedAt)inChapterService.ReorderAsyncleaves the partial-list test green, because SQLite serves theWhere(VariantId)query from the(VariantId, SortOrder)index in sort order anyway. TheOrderByis a guard against a plan change, not a branch a fixture can reach.Unverified / confirmed by reading only:
SortOrderis read only byPageService.ListByVariantAsync,VariantEndpoints.MapToDetailandPageMapper; integrity checks, thumbnails, backups and stats never read it; offline packaging is app-side and already usesReadingOrder) was established by reading the code, not by a test.chapterIdsrelies on[Required]/[MinLength(1)]andservices.AddValidation()(same as the siblingReorderPagesDto); only the empty case has an integration test.Follow-ups
PageService.ReorderAsyncthrowsArgumentExceptionand the global middleware maps it to Internal Server Error; the chapter reorder validates in its use case and answers 400 naming the id, and the page path should do the same.Closes #146
🤖 Generated with Claude Code
PUT /api/variants/{id}/chapters/reorder puts the listed chapters first, in list order, and leaves the unlisted ones behind them in their previous relative order, in one write that ages the variant and doujin (ADR 0004); a foreign or duplicated id is a 400 naming it, an unknown variant a 404, and the chapter collection advertises the action. The app gets DoujinRepository.reorderChapters, a ReorderChaptersAction whose epic persists and reloads the variant, and a Reorder chip on the chapter panel that opens a dialog with drag handles and up/down buttons. Closes #146. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A chapter reorder with the whole ceremony — use case, service, endpoint, hypermedia action, snapshot, Redux epic, and a drag-handle dialog with a real pointer-gesture test~ You even pre-reordered the fixture so the partial-list test can't be satisfied by row order. Someone has been studying what I like. ♡
Verdict: ✅ Looks good to me~
I read the full diff (both commits, +1063/−8 across 27 files) and the full contents of every changed file, traced it against its siblings, and probed it myself. Nothing gets past me today~
✅ What I liked~
ReorderChaptersUseCase, not the service. A foreign/duplicated/empty list is a 400 naming the id instead of the sibling page path's throw→500. You deliberately diverged fromPageService.ReorderAsync, and — fufu~ this is the important part — you wrote the divergence down: the BACKLOG entry routes the page path to the same treatment instead of pretending parity. That's how a divergence should be shipped.ChapterService.ReorderAsyncis a faithful sibling: same load→map→touch→singleSaveChangesshape as the page reorder,ParentTouchHelper.TouchVariantAsyncin the same save (ADR 0004), unlisted chapters re-densified behind the listed ones. Route order correct (/reordermapped before/{chapterId:guid}, both verified in the file).pumpAndSettle, and_move(from, to)doing remove-then-insert exactly matchingonReorderItem's adjusted-index contract — pinned by both the up/down buttons and a realstartGesture/moveBy/updrag.isSavingsemantics are honest: I checked the reducer — onlyCreateDoujin/SaveMetadata/DeleteDoujinset it, and the ChapterPanel doc comment says precisely that, including why chapter writes don't need it. The chip's disabled state is pinned at two layers (panel + the VariantsTab wiring line).DidNotReceiveproving nothing writes on rejection), both ChildWrite touch theories, the hypermedia action, the OpenAPI snapshot, the epic's success (right variant id) and failure (no reload) arms, and the dio path/body. Nice.🔬 My own probes (CI is absent for
0b1eb90, so I verified locally)Each mutation reddened exactly its own tests, then I restored the tree byte-identical (git status clean):
nextUnlisted++→ old-SortOrder: exactlyA_Partial_List_Keeps_The_Unlisted_Chapters_After_It_In_Their_Old_Orderred (1/8).TouchVariantAsyncremoved fromReorderAsync: exactly bothChildWrite.ReorderChapterstheories red (2/36).Your known-green-mutation disclosure (the
OrderByguard riding the(VariantId, SortOrder)index) is honest and correctly reasoned — a plan-change guard, not a reachable branch.Local results at
0b1eb90: backend build 0 errors (25 warnings, all pre-existing — NU1903 etc., none in PR files); ChapterReorder 8/8, ReorderChapters use-case 6/6, ChildWriteTouchesParent 36/36, OpenApiSnapshot+Hypermedia 40/40.flutter analyzeNo issues. App suites serially: chapter_reorder_dialog 5/5, variants_tab 4/4, editor_epics 10/10, doujin_api_repository 15/15. (Two flutter runs died to the sandbox's known DartWorker thread-exhaustion — environmental, rerun clean; every number above is from a green run.)💡 Little ideas (non-blocking)~
SizedBox(width: 420)vs the siblingvariant_dialog's 400; a shared width constant would keep the editor dialog family perfectly uniform. Pure polish~Merge it. It's clean, it's tested, and it treats its parents (and its siblings) with respect~ ♪
Automated review by Jibril · 2026-09-19
CI/CD: absent for head SHA
0b1eb90(no coverage bot comment yet) · Local checks: build green, 90 backend tests + 34 app tests in touched suites, 3 mutation probes all directionalSummary
Summary
Coverage
DoujinManager.ApplicationCore - 94.3%
DoujinManager.Infrastructure - 96.4%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 94.5%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 64.7%