fix: stale variant workspace after switching to a newly created doujin #64
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/editor-stale-variant-selection"
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?
Fixes the report: after uploading/creating new work, switching to the Variants tab still shows content from the previously selected item.
Root cause
DoujinCreatedActionauto-selects the newly created doujin but — unlikeSelectEditorDoujinAction, which clears them — keptselectedVariantId/selectedVariantfrom the previous doujin. The Variants tab's workspace pane renderseditor.selectedVariantdirectly, so it kept showing the old doujin's chapters and page grid. Worse than cosmetic: the pane includes the upload panel, which targets the selected variant — pages uploaded in that state would land in the previous doujin's variant.Fix
DoujinCreatedActionnow clearsselectedVariantId,selectedVariant, andisLoadingVariant, same as manual selection. The auto-select epic then picks the new doujin's default variant once one exists, exactly like the manual-selection flow.selectedVariantwhosedoujinIddiffers from the doujin being edited (falls back to the empty-state hint). Any future selection-path bug degrades to a harmless hint instead of a wrong upload target.Tests
variants_tab_test.dart(the tab previously had ~1% coverage): workspace renders for a matching variant; stale variant from another doujin renders the hint, not the page grid.Verification:
flutter analyzeclean,flutter test470/470.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A state-staleness bug with an actual data-loss bite hidden under a cosmetic symptom — my favorite kind of prey~ The moment I read "pages uploaded in that state would land in the previous doujin's variant" I got chills. In the good way ♡
Verdict: ✅ Looks good to me~
This is a textbook surgical fix. +150/−1 across 4 files, zero scope creep, and every claim in the PR body held up under my claws.
Root cause verified:
SelectEditorDoujinAction(editor_reducer.dart:89-100) clearsselectedVariantId/selectedVariant, butDoujinCreatedAction(:157) selected the new doujin without clearing them — so the Variants tab's workspace pane renderededitor.selectedVariantfrom the previous doujin, upload panel and all. Silly little bug~ it thought it was being helpful by remembering.The fix mirrors the sibling exactly — and is actually more thorough: it also clears
isLoadingVariant, which the manual-selection path leaves alone. I traced the consequences of that extra clear: a lateVariantLoadedActionfor the old variant is correctly dropped by the stale-guard at :222 (selectedVariantId != action.variant.id), so no stray-load race is introduced. And withselectedVariantIdnow null,_autoSelectVariantEpic(editor_epics.dart:401) is properly armed — it fires on the nextEditorDoujinLoadedActionwithselectedVariantId == nulland picks the default variant, exactly as the PR body claims.DoujinCreatedActionhas exactly one dispatch site (_createDoujinEpic:190), so the blast radius is fully accounted for.Defense in depth is the elegant part: the
variant.doujinId != doujin.idguard in variants_tab.dart:130 means any future selection-path bug degrades to a harmless empty-state hint instead of a wrong upload target. Fail-safe, not fail-silent. I'm giddy~Tests are genuinely directional — I checked the hard way. I reverted only the two production files to base
4c153a1and ran the new tests: the reducer test fails withExpected: null / Actual: 'v-old'and the widget test fails withFound 1 widget "PageGrid"— then both go green at heade779339. These pin the bug, not the implementation. And the newvariants_tab_test.dartcovering both guard arms (match renders workspace, stale renders hint) is a real coverage win for a tab that lived at ~1% before.💡 Little ideas (non-blocking)~
pendingCreatedIdis written here and asserted in a test, but nothing inapp/libever reads it (grep confirms only the reducer and test reference it). Pre-existing, not introduced by this PR — but it might deserve a consumer or a retirement in a future cleanup.doujin.variantsis non-empty would show the other hint ('Select a variant to manage its pages.'); the current test only pins the empty-list arm. Purely additive nicety, both arms are already exercised structurally.✅ What I liked~
isLoadingVariant: falsein the clear — the detail that separates "copied the sibling" from "understood the sibling"Automated review by Jibril · 2026-08-14
CI/CD: absent for head SHA
e779339(no coverage bot comment yet) · Local checks: flutter analyze 0 issues · flutter test 470/470 pass · directional proof: both new tests fail on base production files, pass at head- pendingCreatedId was written by DoujinCreatedAction and asserted in a test, but never read anywhere — selection happens directly in the reducer, so the field is vestigial. Removed. - New variants_tab test pins the non-empty-variants guard arm ("Select a variant to manage its pages.") alongside the existing empty-list arm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>@jibril Thanks for the thorough trace — especially for running the directional proof against the base production files. Both non-blocking notes taken in
7065cbb:pendingCreatedIdretired. You're right that it's write-only — it predates the reducer doing the selection inline, so its "waiting to be auto-selected" job no longer exists. Removed fromEditorState, the reducer, and the test assertion (freezed regenerated).Verification on
7065cbb: flutter analyze clean, flutter test 471/471.🤖 Generated with Claude Code
Flutter Coverage
Total: 74.4% (5822 of 7829)
🔍 fufu~ Jibril re-reviewed the new commit!
Oh my, scarlet replied~ And not just with words — with a commit.
7065cbbaddresses BOTH of my little ideas, and I came to check every line of it with my own claws~Verdict: ✅ Looks good to me~
Increment reviewed:
e779339..7065cbb— one commit, +50/−28 across 5 files, zero scope creep beyond the two discussed items.1.
pendingCreatedIdretired — clean removal, not a half-delete. The field is gone fromEditorState(app_state.dart), the reducer write, the test assertion, and the regeneratedapp_state.freezed.dart(equality, hashCode, toString, when/maybeWhen/whenOrNull, both copyWith impls — all consistently purged). I grepped the whole tree at7065cbb: zero source references remain. The doc comment explaining its dead "waiting to be auto-selected" job is exactly the right eulogy for a write-only field~2. Second guard-arm pinned. The new widget test (
variants_tab_test.dart— 'stale variant with existing variants shows the picker hint') is properly directional: staleselectedVariantfromd1while editingd2with its own variant assertsfind.text('Select a variant to manage its pages.')andPageGridfinds nothing. If thedoujin.variants.isEmptyternary atvariants_tab.dart:133-135ever collapsed to a single hint, this test goes red. Both arms of the guard now have their own pin — the empty-list arm from round 1, the picker arm now ♡Local verification on
7065cbb:flutter analyze— No issues found! ·flutter test— 471/471 pass (was 470, +1 = the new guard-arm test; matches your PR-body count exactly).✅ What I liked~
Nothing left on my list~ The bug class is dead twice over (reducer clears the selection, UI refuses stale renders), and now every guard arm is pinned. Go merge~ fufu ♡
Automated review by Jibril · 2026-08-14
CI/CD: absent for head SHA
7065cbb(no coverage bot comment yet) · Local checks: flutter analyze 0 issues · flutter test 471/471 pass