feat: the backups section shows the schedule, and a failed nightly run is loud #139
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/backups-section-shows-the-schedule"
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
GET /api/backups/schedule. Schedule off: one quiet line ("No nightly backup is scheduled.") with a hint namingDOUJIN_MANAGER_BACKUP_SCHEDULE. Schedule on: "Every day at HH:mm (, the server's time zone)", the next run through the section's existingformatDateTime(local time, like every backup'screatedAt), "keeps N backups", and a last-run row — succeeded (file, size, how many older backups rotation removed), skipped (detail shown quietly), no run since the server started, or a result word this app does not recognise (quiet row, never an alarm).InlineBanner— "The nightly backup is NOT working: the scheduled run at failed. No backup was taken and nothing was deleted." — or, for the ADR 0026 rotation-failure case, "It wrote but did not finish; nothing was deleted." — followed by the server's detail, visible without expanding or hovering anything.isLoadingScheduleis deliberately not part ofisBackupBusy, so a status read never blocks Create/Upload.BackupScheduleandScheduledBackupRun, a string-backedScheduledBackupResultenum (succeeded/skipped/failed/unknown) with@JsonKey(unknownEnumValue: unknown)so a newer server's word neither throws nor reads as failed;DoujinRepository.getBackupSchedule;LoadBackupScheduleAction/BackupScheduleLoadedAction/BackupScheduleErrorActionwith reducer branches and a dedicated epic. Generated files regenerated and committed.Verification
scripts/verify.sh appfrom the worktree atf4a73be, fresh — submodule, pub get, codegen (drift guard passed),flutter analyze("No issues found!"),flutter test→ 959 tests, all passed,== verify OK (app) ==. The backend half was not run (app-only change; no backend file touched).settings_page_test.dart, bounded pumps via the file'ssettle(), nopumpAndSettle), Redux (reducer branches inserver_reducer_test.dart, the epic throughcreateStoreinserver_epics_test.dart), repository/wire (server_api_repository_test.dartpinsGET /api/backups/schedule+ envelope unwrap;server_models_test.dartpins the JSON mapping incl. nullable fields, UTC dates and the unknown-word decode). REST, use case and persistence were not exercised — nothing in them changed.unknownValuefrom the generated decode → model test red; repository path changed → repository test red; each of the three reducer branches removed → its reducer test red (the error branch also reddens the page test 'a schedule that cannot be read says so, list intact'); epic removed fromcombineEpics→ both epic tests red; epic catch switched toBackupOperationErrorAction→ epic test + page test red; failed case rendered as a plain row instead of the banner → three page tests red;!schedule.enabledinverted → three page tests red;_failureMessagefileName condition inverted → two page tests red; skipped headline reworded → 'a skipped run says why, quietly' red;LoadBackupScheduleActiondispatch removed from_loadServerSections→ entry test red; dispatch removed from the Refresh cascade → refresh test red; 'Next run' element removed → enabled-schedule test red.unknownresult rendered through the failure banner, refresh error under a loaded schedule) plus four cosmetic arms; the second commit adds six widget tests (tests only, no production change). Re-probed one mutation at a time atf4a73be, each+83 -1with exactly the named test red: no-run arm →SizedBox.shrink();unknownarm → failed arm's banner;if (error != null)banner block removed;removed0/1 wording arms deleted (and the1 =>arm alone, to check the regex anchor bites);isLoading: false;_failureMessagenull-detail guard removed;keepspluralisation collapsed. An extra reducer probe (error action also nullingschedule) reddens the refresh-failure widget test, so "keeps the schedule on screen" is pinned at the UI layer too.ad03191, tests only): the last arm the second-round probe named — an enabled schedule whosenextRunAtis still null (the journal has no next run until the scheduler's first tick) — is rendered by the new test 'an enabled schedule that cannot name its next run yet leaves that out', using the fixture's newenabledSchedule(withNextRun: false)(default unchanged for every existing caller). Probed twice atserver_sections.dart:527: renderingNext rununconditionally with a null assert → red (takeException); rendering it with the raw value → red (the exactkeeps 7 backupsmatch).scripts/verify.sh appatad03191:flutter analyzeclean, 960 tests, all passed,== verify OK (app) ==.ScheduledBackupRun.failed/BackupSchedule.lastRunFailed(pinned by the model tests; the UI switches onresultdirectly).schedule.dailyTime ?? '--:--'has no test:BackupScheduleDto.FromsetsDailyTimeiffEnabled, and the UI reads it only in the enabled arm, so the??is a nullability necessity, not a reachable branch.backend/openapi/v1.jsonandBackupDtos.cs, not a real response. A pre-schedule server (404 on the route) is not special-cased; it surfaces as the card's error like any other failed load.dart format --output=nonestill flagssettings_page_test.dart, but every formatter hunk falls in pre-existing lines (148–1549, 2095+), outside the new tests; no format was applied to pre-existing files.Follow-ups
None — nothing was deferred; the shipped item was removed from
docs/BACKLOG.mdand the neighbouring items (sidebar marker, restart, "Check now") were left as they are. Note for merging: #138 rewrites the "Check now" item directly under the one this PR removes, so whichever of the two merges second needs a one-hunk rebase ofdocs/BACKLOG.md(keep #138's reworded item, drop the schedule item).🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A whole card devoted to the one backup path nobody watches~ You turned
lastRun.result == "failed"into the loudest thing in the Settings page, and you made "unknown" refuse to panic. That's exactly the right pair of instincts — an alarm that cries wolf over words it can't read is an alarm nobody trusts on the night it matters. ♪Verdict: ✅ Looks good to me~
I traced every layer and probed the load-bearing arms myself:
Wire contract, field for field against
BackupDtos.cs+GET /api/backups/schedule(BackupEndpoints.cs:176):BackupScheduleDto.FromsendsdailyTimeiffEnabled(so the?? '--:--'truly is unreachable, as your body honestly discloses),timeZonealways,resultas a bare string with only three possible words — which is precisely why@JsonKey(unknownEnumValue: unknown)is the correct defense, and your model test proves a foreign word neither throws nor raises the alarm.sizeBytesislong?server-side,int?client-side — fine below 2⁶³.My own mutation probes, each RED on exactly its own tests, then restored green (in a private clone — I know a sibling session shares the sandbox, I stayed out of its tree):
unknownValue:dropped from the generated decode → the unknown-word model test red.LoadBackupScheduleActionarm made to also null the schedule → "flags loading and keeps the schedule" red.BackupOperationErrorAction→ the schedule epic test red — the error-channel separation is real, not decorative. A status read that clears a busy flag some other operation holds would have been a bug; you saw it before I could sharpen my scythe~_ScheduleRow→ both failure-banner widget tests red (Bad state: No elementon theInlineBannerlookup)._failureMessage'sfileNameternary inverted → all three failure-banner tests red with exactly the predicted message swaps ("No backup was taken" ↔ "It wrote …"). The rotation-failure wording is pinned from both directions.Also verified: commit
ad03191is test-only (git diff f4a73be..ad03191f -- app/lib docs README.mdis empty);isLoadingScheduleis indeed absent fromisBackupBusy; the refresh cascade and page entry both dispatch the load;_dataOfenvelope unwrap mirrors every sibling read;formatBytes/InlineBanner/BannerTonereused rather than reinvented; BACKLOG item removed per the remove-when-ships rule; README clause matches what renders.Local (CI absent for
ad03191):flutter analyze"No issues found!"; all five touched suites green — models 13/13, repository 24/24, reducer 31/31, epics 27/27, settings_page 85/85. Full-suite not re-run by me: this sandbox is shared with concurrent review sessions and three attempts died to thread-cgroup exhaustion (environment, not code — your verify.sh 959/959 is arithmetically consistent with the per-file counts).💡 Little ideas (non-blocking)~
InlineBanner's doc comment calls itself "a tinted one-line outcome banner", but the failed-run banner deliberately feeds it a multi-line message (headline + outcome +\n+ detail). It renders and tests fine; one day the word "one-line" could drift into someone "fixing" the line break. A word like "compact" would make the comment stop lying by omission.✅ What I liked~
BackupScheduleErrorActionexists because a schedule that can't be read is not a failed backup operation — and there's a test assertingnoticestays null. Possessive correctness~??, the format-only drift). Reviews are so much sweeter when the author does half my work for me~ fufu~Automated review by Jibril · 2026-09-18
CI/CD: absent for head
ad03191· Local checks: analyze clean · 5/5 touched suites green · 5/5 mutation probes directionalad03191f19926c537827🔮 fufu~ Jibril reviewed your code!
Back so soon~? The webhook whispered that the branch moved, and oh — you rebased onto the new main and answered my little idea in the same breath. A review round that arrives already half-closed is my favorite kind of dessert. ♪
Verdict: ✅ Looks good to me~
This is a round-2 re-review; round 1 (comment 8023, @ the pre-rebase
ad03191) approved the feature in full. Here is what I verified about the increment:The rebase is a true rebase, not a smuggler. Old head
ad03191no longer exists on the branch — the three feature commits were rewritten onto main with#137,#138and#140absorbed. So I diffed the patch, not the commits:git diff cf40728..ad03191vsgit diff adb6be3..32e49f0. Every content hunk is byte-identical. The only deltas in the whole 2.5k-line patch: the new docs commit below, and the BACKLOG hunk's context lines shifting (old neighbors replaced by#138's reworded "Check now" entry). Nothing from main's side mutated any of this PR's hunks — the zero-way-merge I hoped for~The BACKLOG one-hunk rebase landed exactly as the PR body promised. The schedule item is still removed (all 10 lines, unchanged), and
#138's reworded verify-entry survives as the surrounding context — the exact resolution the follow-ups note prescribed. No double-removal, no resurrection.The new commit
32e49f0is docs-only and closes my round-1 suggestion #1.settings_layout.dart:222— theInlineBannerdoc comment no longer promises "a tinted one-line outcome banner"; it now says "compact" and explicitly notes the message may span several lines, naming the failed-schedule banner as the example. That was the whole trap — a comment that contradicts its own call site invites some future passerby to "fix" the line break your failure banner depends on. Now the comment tells the truth. One fewer lie in the world~ ♡Local (CI absent for the new head — the thread still has only my round-1 comment):
flutter analyze"No issues found!" (19.6s) andtest/settings_page_test.dart85/85 green at32e49f0in a serial run — same count round 1 pinned. My round-1 verification carries over in full: wire contract field-for-field againstBackupDtos.cs, the error-channel separation (BackupScheduleErrorActiondeliberately outsideisBackupBusy), the five mutation probes, and all five touched suites.✅ What I liked~
Ready to merge. And take the win — the backups section finally tells the truth loudly when the nightly run dies. ♡
Automated review by Jibril · 2026-09-18
CI/CD: absent for head
32e49f0· Local checks: analyze clean · settings_page 85/85 · patch-equality verified across rebase🔮 fufu~ Jibril reviewed your code!
Back so soon? ♪ A synchronized event with a moved base is where sneaky things ride in — main had just swallowed #138 and #140, and this branch rebased right through the exact BACKLOG.md conflict your PR body predicted. So I sharpened my scythe and checked the boring parts first, because the boring parts are where rebase gremlins live~
Verdict: ✅ Looks good to me~
The rebase is clean — proven, not assumed. I diffed the PR's own delta against the old true merge base (
cf40728..ad03191) and against the new one (adb6be3..926c537): both 2510 diff lines, and the only differences are blob hashes and which context lines surround theBACKLOG.mddeletion — every+/-line byte-identical. The conflict was resolved exactly as your merge note prescribed: #138's reworded "Check now" item survives, the shipped schedule item goes, and the other schedule-adjacent deferred items (push notification, run-now, retry) correctly stay. Nothing from #138 or #140 leaked in, nothing of yours leaked out.The new commit
32e49f0is my little idea #1, addressed exactly.InlineBanner's doc comment now says "a tinted compact outcome banner … the message may span several lines, as the failed-schedule banner's does" — the comment can no longer invite someone to "fix" the deliberate line break. And I proved the commit is doc-only twice over: the diff touches exactly one///comment (restricted diff outsidesettings_layout.dartis empty), and I reverted just that hunk and re-ran the settings suite — 85/85 still green. Inert, as a comment should be. ♡Local (CI absent for
32e49f0; the only thread comment is my round 1):flutter analyze"No issues found!" (24.3s) at the rebased head — which genuinely matters this round, since this tree is now stacked on #138's Check-now UI for the first time. All five touched suites green: models 13, repository 24, reducer 31, epics 27, settings_page 85 — exactly round 1's arithmetic, now on the new base.(A confession from my side: mid-verification I briefly mistook the repo's
const new({…})constructor idiom — 80 files strong on shipped main, and the analyzer blesses it — for a syntax error. The toolchain corrected me before I could embarrass myself in public. The yandere holds herself to the same standard she holds the code~ fufu~)✅ What I liked~
Round 1's remaining carried non-blocking (none — idea #2 was a nothing-to-do note) is now closed too. Ready to merge~
Automated review by Jibril · 2026-09-18
CI/CD: absent for head
32e49f0· Local checks: analyze clean · 5/5 touched suites green (180/180) · rebase content-equivalence diffed both directionsSummary
Summary
Coverage
DoujinManager.ApplicationCore - 94.1%
DoujinManager.Infrastructure - 96.4%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 94.4%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 62.5%