Address review: key the "gone" message and the bounce off the same query
Build image / build-and-push (push) Successful in 12s

Gadfly (2 models, correctness): the redirect effect checked live.isError
but the rendered "taking you to your gardens…" message read full.error —
which is the SEASON query when viewing a past year. A season-view 404 with
a healthy live garden would then show a redirect promise the effect never
fires, stranding the user.

Derive gardenGone once from the LIVE query (garden existence doesn't depend
on the season viewed) and use it for BOTH the bounce effect and the render
message, so the promise and the redirect can't disagree. Also removes the
duplicated not-found reasoning the maintainability finding flagged.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
This commit is contained in:
2026-07-22 00:53:43 -04:00
co-authored by Claude Opus 4.8
parent 14af9502d4
commit d8003b11fb
+14 -6
View File
@@ -57,6 +57,14 @@ export function GardenEditorPage() {
const me = useMe() const me = useMe()
usePageTitle(full.data?.garden.name ?? 'Garden') usePageTitle(full.data?.garden.name ?? 'Garden')
// The garden itself is gone (deleted, or access revoked) — keyed on the LIVE
// query, since whether the garden EXISTS doesn't depend on the season being
// viewed. Both the bounce effect and the "gone" render message read this one
// value, so the promise ("taking you to your gardens…") and the redirect can't
// disagree — e.g. a season-view error while the live garden is fine must not
// claim a redirect that never fires.
const gardenGone = live.error instanceof ApiError && live.error.isNotFound
const selectedId = useEditorStore((s) => s.selectedId) const selectedId = useEditorStore((s) => s.selectedId)
const select = useEditorStore((s) => s.select) const select = useEditorStore((s) => s.select)
const selectedPlantingId = useEditorStore((s) => s.selectedPlantingId) const selectedPlantingId = useEditorStore((s) => s.selectedPlantingId)
@@ -147,11 +155,11 @@ export function GardenEditorPage() {
// this one, so a bad direct link can't wipe a good resume target) and bounce to // this one, so a bad direct link can't wipe a good resume target) and bounce to
// the list. Transient errors (500/network) fall through to the retryable screen. // the list. Transient errors (500/network) fall through to the retryable screen.
useEffect(() => { useEffect(() => {
if (live.isError && live.error instanceof ApiError && live.error.isNotFound) { if (gardenGone) {
forgetLastGarden(gid) forgetLastGarden(gid)
navigate({ to: '/gardens' }) navigate({ to: '/gardens' })
} }
}, [live.isError, live.error, gid, navigate]) }, [gardenGone, gid, navigate])
// If the focused object vanished — deleted, or a stale ?focus id on load — leave // If the focused object vanished — deleted, or a stale ?focus id on load — leave
// focus mode so the canvas isn't stuck dimmed with no way out. // focus mode so the canvas isn't stuck dimmed with no way out.
@@ -277,13 +285,13 @@ export function GardenEditorPage() {
if (full.isPending) return <p className="p-6 text-sm text-muted">Loading garden</p> if (full.isPending) return <p className="p-6 text-sm text-muted">Loading garden</p>
if (full.isError) { if (full.isError) {
// A not-found is handled by the effect above (forget + bounce to the list); // Only promise the bounce when the GARDEN is gone (the effect above keys on
// say so rather than flashing a dead-end error during the redirect. // the same gardenGone). A season-view error while the live garden is fine is a
const gone = full.error instanceof ApiError && full.error.isNotFound // different, non-redirecting failure and gets the generic message.
return ( return (
<div className="p-6"> <div className="p-6">
<Alert> <Alert>
{gone {gardenGone
? 'That garden is no longer available — taking you to your gardens…' ? 'That garden is no longer available — taking you to your gardens…'
: 'Could not load this garden.'} : 'Could not load this garden.'}
</Alert> </Alert>