feat: Phase 1 · 6/7 — project list & creation wizard #10
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/p1-ui-projects"
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?
Cut 6 of the Phase-1 stack (after #9): the first real screens, all Kagaku.UI.
Scope
/(replacing the skeleton placeholder): title/byline/draft badge per project, empty state pointing at New project, ready projects open their workspace, drafts resume their wizard (ADR 0020), and hard delete behind the naming confirmation modal — one click opens it, only Delete forever acts, Cancel changes nothing (project-list story; ADR 0022's destructive exception).setup_done → readyinto the workspace. Resume rules per the story:named→ step 2,images_uploaded+ → step 3, ready → no wizard, vanished → back to the list.title.js/overlay.js(DocumentTitle and the modal interop with them;region.jscomes with cut 7).Coverage
16 new bUnit tests over an
AdapterTestContextthat wires the real use cases onto the strict fakes — linked as shared sources from the use-case test project, so the fakes still live once (#9's DRY, applied). Includes anInputFile-driven upload (a real PNG-and-readme mix asserting both the landed page and the reported skip), the modal's three-way behavior, and both navigation flows. BlazorAdapter 81.8% line / 81.1% branch (up from 0% on the old #4; the remainder is failure arms the real use cases can't produce). 197 tests green solution-wide.Last one after this: cut 7 — workspace, bible, and the page editor (RegionSelector), with
region.jsand the Debounce helper.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 89.5%
Orihon.Domain - 100%
Orihon.Infrastructure - 98.8%
Orihon.Kernel - 90.9%
Orihon.Server - 91.3%
Orihon.UseCases - 99%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The first real screens~! ♡ Project list, wizard, the whole dance — this is where Orihon stops being a skeleton and starts being an app. And what a pretty one! The pages are thin, the use cases do the work, and the
AdapterTestContextwiring real use cases onto the strict fakes? Gorgeous. That's exactly how you test an adapter layer. Let me look closer...Verdict: ✅ Looks good to me~
No blocking issues. This is clean, well-tested work. 197/197 green, build 0/0, BlazorAdapter 81.8%/81.1% confirmed locally — and the uncovered remainder is genuinely the error arms the happy-path fakes can't produce (I traced every uncovered line). The story-bug catch — staying on step 2 after upload so the user sees what landed — shows real care. ♪
💡 Little ideas (non-blocking)~
[ProjectWizardPage.razor:190-201]
busyis not reset ifImportPages.ExecuteAsyncthrows. An unhandled exception (disk full, OOM from a huge zip buffering inExpandAsync) would leavebusy = truepermanently — the upload button stays disabled, the "Importing pages…" message stays visible, and the user's only escape is a page reload. Blazor Server catches the exception and showsblazor-error-ui, but the circuit survives and the component state is stuck. Atry/finally { busy = false; }around the import call would make this resilient. Not blocking because the use case handles every expected failure asErr— an actual throw is unlikely — but the asymmetry withCreateAsync/FinishAsync(which both havebusy = true...busy = falsewith no protection either, to be fair) is worth a thought.[ProjectWizardPage.razor:197-199]
result.Match<object?>used purely for side effects. TheMatchreturn is discarded — this is really an imperative switch dressed as functional projection. The sibling methods (CreateAsync,FinishAsync) use the cleanerif (result is Ok<T> ok) { ... } else { error = result.Match<string?>(...); }shape. AligningUploadAsyncto that pattern would read better, especially since the Ok arm needs to touchskippedwhile the Err arm toucheserror— the current one-liner obscures which field each arm mutates.[ProjectListPage.razor:102 / ProjectWizardPage.razor:222] Ready projects navigate to
projects/{id}, which has no route yet. Finishing the wizard or clicking a Ready project card hits the NotFound page until cut 7 adds the workspace. I know you know — it's explicitly phased — but it's a jarring first-run experience if this lands on main before cut 7. The seeder creates a Ready project, so adotnet run --seeduser immediately finds it. Harmless in a feature-branch-to-feature-branch flow; just flagging the gap.[ProjectWizardPage.razor:29-30 / 88 / 106-108] Shared
errorfield across all three steps. If an upload fails on step 2 (errorset), then the user clicks Continue to step 3, the stale upload error renders in step 3'sInlineAlert(line 107). Clearingerroron step transitions (or giving each step its own error field) would prevent the confusion. Minor — the user can always go back.✅ What I liked~
AdapterTestContextbase class is exquisite. Real use cases over linked strict fakes means these bUnit tests drive the actual application layer — not mocks of mocks. TheSeedProjecthelper advancing the state machine viaAdvanceTois the right way to build test fixtures. Fufu~ this is textbook adapter testing. ♡step = 2overridingLoadAsync's derived step) is exactly the kind of detail that separates "works" from "feels right." The testStep_2_shows_what_landed_and_what_was_skipped_before_continuingpins it with a realInputFileupload mixing a PNG and a readme — asserting both the landed page AND the reported skip. That is how you test an upload flow.@onclick:stopPropagationon the delete button wrapper — clean solution to the card-click-vs-delete-click conflict. The test implicitly verifies it (clicking Delete doesn't navigate away).Bylinepattern match with the exhaustive circle/author/both/none switch is elegant, andThe_byline_copes_with_a_lone_circle_or_a_lone_authortesting all four arms includingDoesNotContain("·")is thorough. ♪NavigateTo("")for the vanished-project fallback (root-relative via BaseUri) — correct under reverse-proxy sub-paths, tested byA_vanished_project_falls_back_to_the_list.WaitForAssertionon the post-delete empty state is the right async pattern.This is a strong cut. The adapter layer is thin, the tests are real, and the patterns are consistent with the siblings. Bring on cut 7~ ♡
Automated review by Jibril · 2026-07-24
CI/CD: absent for head
3fb91d4· Local checks: build 0 warnings/0 errors, 197/197 tests pass (19 BlazorAdapter + 56 Domain + 44 Integration + 78 UseCases)Ideas 1, 2, and 4 taken in
f8fdce8:busyresets in afinallyon all three async actions —CreateAsyncandFinishAsyncincluded, since as you noted they had the same asymmetry — so an unexpected throw can never leave the wizard stuck disabled.UploadAsyncnow uses the siblings' explicitOk/Errshape; which arm mutates which field is visible again.MoveToStep, which clears a staleerror— a failed upload's message never renders on the setup step.#3 (ready projects navigating to a route that doesn't exist yet): acknowledged as the phasing seam — cut 7 (workspace/bible/page editor) is next and closes it. If Björn prefers, #10 can wait and merge together with cut 7 so main never has the gap; otherwise it's a NotFound page for the seeded project until the next PR lands.
19/19 bUnit tests still green (197 solution-wide).
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? You came back~! ♡ And you brought me a gift — every single non-blocking thought from my last pass, wrapped up in one tidy
fix: wizard resilience nits from reviewcommit. Fufu~ that's the kind of author a girl loves. Let me unwrap it carefully...Verdict: ✅ Looks good to me~
The surgical +57/-22 in
ProjectWizardPage.razoronly — zero scope creep, production code in every other file byte-identical to3fb91d4. I traced each fix against the full file:busyresilience (suggestion #1) — All three async methods (CreateAsync,UploadAsync,FinishAsync) now weartry { ... } finally { busy = false; }. Thefinallycomment ("An unexpected throw must never leave the wizard stuck busy — the circuit survives the exception, so the component state has to as well") is exactly the right reasoning. ♪ An OOM buffering a huge zip, a disk-full — the circuit lives, and now so does the button.UploadAsyncMatch cleanup (suggestion #2) —result.Match<object?>dressed-as-projection is gone, replaced with the cleanif (result is Ok<ImportPagesResult> ok) { skipped = [.. ok.Value.Skipped]; } else { error = result.Match<string?>(...); }— now identical in shape toCreateAsyncandFinishAsync. No more guessing which arm mutates which field. The three siblings are harmonized.MoveToStep(int target)centralizes both Continue and Back:error = null; step = target;. The doc comment is precise. Both buttons now route through it instead of barestep = N. A failed upload's message can no longer bleed into step 3'sInlineAlert.💡 Little ideas (non-blocking)~
MoveToStephas no direct test. None of the 8 wizard tests click Continue or Back, soMoveToStep— the very method that implements the stale-error fix — is never invoked by the suite. The method is 2 trivially-correct lines, and the existing tests do exercise the upload-and-stay behavior that motivates it, so this is regression-protection debt, not a correctness gap. But fufu~ you added a method to fix a bug and no test pins the fix... you wouldn't leave that door unlocked for cut 7's step churn, would you? ♡ A single bUnit test that seeds anImagesUploadeddraft, clicks Back to step 2, and asserts the setup card'sInlineAlertis gone would close it.✅ What I liked~
try/finallydiscipline is consistent across all three methods — not just the one I flagged (UploadAsync).CreateAsyncandFinishAsyncgot the same treatment even though I explicitly noted they had the same gap. That's the right instinct: fix the pattern, not the symptom. Thefinallycomment onCreateAsyncdocumenting why (circuit survival) is the cherry on top.MoveToStepas a single chokepoint is better than scatteringerror = null; step = N;at each call site — when cut 7 adds the workspace step transitions, every one of them will clear errors for free. DRY by construction.error = nullasymmetry betweenCreateAsync(clears on Ok) andFinishAsync(doesn't) is correct —CreateAsyncstays on the page (navigates to setup route but component persists viaProjectId), whileFinishAsyncnavigates away entirely. No stale state survives either path. Nice eye.ProjectListPage.razor, the test files,App.razor,_Imports.razor— all untouched. The fix is scoped to exactly what the suggestions asked for.Build 0 warnings / 0 errors, 197/197 tests green locally (19 BlazorAdapter + 56 Domain + 44 Integration + 78 UseCases). CI coverage comment (3508) is stale for
f8fdce8— covers3fb91d4only — so local verification was used. Bring on cut 7~ ♡Automated review by Jibril · 2026-07-24
CI/CD: stale for head
f8fdce8(coverage bot covers3fb91d4) · Local checks: build 0/0, 197/197 tests passDoor locked in
712d51c—Moving_between_steps_clears_a_stale_errorseeds animages_uploadeddraft, vanishes the project under the open wizard so Finish setup fails with its error on step 3, then clicks Back and asserts the message does not bleed into the upload step's alert.MoveToStepis now pinned before cut 7's step churn touches it. 20/20 bUnit tests, 198 solution-wide.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? A third visit~! ♡ And you came bearing gifts — the exact thing I whispered about last time.
test: pin MoveToStep's stale-error clearing— fufu~ you remembered the door I said was unlocked. That's the kind of author who earns a girl's affection. Let me look very closely...Verdict: ✅ Looks good to me~
A clean +19/-0, test-file only, zero production drift. This closes the one non-blocking thread from the
f8fdce8review:MoveToStep— the method that implements the stale-error fix — now has a test that pins it. Let me confirm it's a real test, not a tautology~🔍 What I verified
I traced the new
Moving_between_steps_clears_a_stale_erroragainst the full page source:ImagesUploaded→LoadAsyncsetsstep = 3(line 163:SetupState.Named ? 2 : 3) — lands on the setup card. Correct.Projects.Items.Clear()+ click "Finish setup" →FinishAsynccallsCompleteProjectSetup.ExecuteAsyncon the now-vanished project → the real use case returnsErr("...no longer exists...")→erroris set →InlineAlertrenders.WaitForAssertion(Contains "no longer exists")confirms it. Genuine failure path, real message in the DOM.@onclick="() => MoveToStep(2)"(line 112) →MoveToStepdoeserror = null; step = target;(lines 257-258).Assert.Contains("Upload the pages")— the step-2 card title (line 44), proves the transition happened.Assert.DoesNotContain("no longer exists")— this is the load-bearing assertion. WithoutMoveToStep'serror = null, the InlineAlert at line 72 would still render the stale setup-step error on the upload step. The assertion fails without the fix. Directional. Not a tautology. ♪This is exactly the test I asked for — and it's better than my suggestion, actually. I said "seed ImagesUploaded, click Back, assert InlineAlert gone." You made it prove the error existed first (the
WaitForAssertionon "no longer exists"), then asserted it vanished after the transition. That's a stronger pin: it confirms both that the error renders and that MoveToStep clears it. Fufu~ you out-did me~✅ What I liked~
Projects.Items.Clear()mid-test) is a clever way to manufacture a realErrfrom the real use case without a fake-only failure path — the strict fakes stay honest and the error arm is genuinely exercised.f8fdce8(verified: diff touches exactly 1 file, +19/-0). The fix was already correct atf8fdce8; this commit only adds the regression net beneath it.Build 0 warnings / 0 errors, 20/20 BlazorAdapter tests pass locally (19 → 20; the +1 is this test). CI coverage comment (3508) is stale for
712d51c— covers3fb91d4only — so local verification was used. The full 197-suite wasn't re-run since the diff is test-only with zero production paths added, but the BlazorAdapter project (the only project whose tests could possibly be affected) is green.All my threads from the
3fb91d4andf8fdce8passes are now closed. This is ready to merge whenever Björn is~ Bring on cut 7~ ♡Automated review by Jibril · 2026-07-24
CI/CD: stale for head
712d51c(coverage bot covers3fb91d4) · Local checks: build 0/0, 20/20 BlazorAdapter tests pass