Resume the last garden on this device (#97) #108
@@ -30,6 +30,11 @@ export class ApiError extends Error {
|
||||
get isUnauthorized(): boolean {
|
||||
return this.status === 401
|
||||
}
|
||||
|
||||
/** Resource is gone or masked (pansy returns 404 for no-access, not 403). */
|
||||
get isNotFound(): boolean {
|
||||
return this.status === 404
|
||||
}
|
||||
}
|
||||
|
||||
type ParamValue = string | number | boolean | undefined | null
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { getLastGardenId, rememberLastGarden, forgetLastGarden } from './lastGarden'
|
||||
|
||||
// The tests run in the node environment (no DOM), so stand up a minimal
|
||||
// in-memory localStorage rather than pull in jsdom for one thin module.
|
||||
function installStorage(impl?: Partial<Storage>) {
|
||||
const store = new Map<string, string>()
|
||||
const base: Storage = {
|
||||
getItem: (k) => store.get(k) ?? null,
|
||||
setItem: (k, v) => void store.set(k, String(v)),
|
||||
removeItem: (k) => void store.delete(k),
|
||||
clear: () => store.clear(),
|
||||
key: (i) => [...store.keys()][i] ?? null,
|
||||
get length() {
|
||||
return store.size
|
||||
},
|
||||
}
|
||||
;(globalThis as { localStorage?: Storage }).localStorage = { ...base, ...impl }
|
||||
}
|
||||
|
||||
beforeEach(() => installStorage())
|
||||
afterEach(() => {
|
||||
delete (globalThis as { localStorage?: Storage }).localStorage
|
||||
})
|
||||
|
||||
describe('lastGarden', () => {
|
||||
it('round-trips a remembered garden id', () => {
|
||||
expect(getLastGardenId()).toBeNull()
|
||||
rememberLastGarden(42)
|
||||
expect(getLastGardenId()).toBe(42)
|
||||
})
|
||||
|
||||
it('rejects a non-positive or unparseable stored value', () => {
|
||||
localStorage.setItem('pansy:last-garden', 'not-a-number')
|
||||
expect(getLastGardenId()).toBeNull()
|
||||
localStorage.setItem('pansy:last-garden', '0')
|
||||
expect(getLastGardenId()).toBeNull()
|
||||
localStorage.setItem('pansy:last-garden', '-3')
|
||||
expect(getLastGardenId()).toBeNull()
|
||||
})
|
||||
|
||||
it('forgets unconditionally with no argument', () => {
|
||||
rememberLastGarden(7)
|
||||
forgetLastGarden()
|
||||
expect(getLastGardenId()).toBeNull()
|
||||
})
|
||||
|
||||
it('forgets only when the stored id matches onlyIfEquals', () => {
|
||||
rememberLastGarden(5)
|
||||
// A 404 on a different (directly-linked) garden must not wipe the good resume.
|
||||
forgetLastGarden(9)
|
||||
expect(getLastGardenId()).toBe(5)
|
||||
// A 404 on the stored garden itself does clear it.
|
||||
forgetLastGarden(5)
|
||||
expect(getLastGardenId()).toBeNull()
|
||||
})
|
||||
|
||||
it('swallows storage failures instead of throwing', () => {
|
||||
installStorage({
|
||||
setItem: () => {
|
||||
throw new Error('quota')
|
||||
},
|
||||
getItem: () => {
|
||||
throw new Error('blocked')
|
||||
},
|
||||
})
|
||||
expect(() => rememberLastGarden(1)).not.toThrow()
|
||||
expect(getLastGardenId()).toBeNull() // getItem throwing → null, not a crash
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,46 @@
|
||||
// The last garden opened on THIS device, so a returning user resumes where they
|
||||
// were instead of always landing on the gardens list. Per-device, in
|
||||
// localStorage (same rationale as the seed tray and PlantPicker recents): a
|
||||
// convenience, not authoritative state — a quota/availability failure is
|
||||
// swallowed, and a stored id that no longer loads clears itself (see the editor).
|
||||
//
|
||||
// Only the id is stored. The garden is resolved by the editor on load; if it's
|
||||
// gone (deleted, or access revoked), forgetLastGarden() drops it so `/` stops
|
||||
// resuming a garden that can't open.
|
||||
|
||||
const KEY = 'pansy:last-garden'
|
||||
|
||||
/** The last-opened garden id on this device, or null if none/unparseable. */
|
||||
export function getLastGardenId(): number | null {
|
||||
try {
|
||||
const raw = localStorage.getItem(KEY)
|
||||
if (raw == null) return null
|
||||
const n = Number(raw)
|
||||
return Number.isInteger(n) && n > 0 ? n : null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
/** Record the garden the device is now in, so `/` resumes here next time. */
|
||||
export function rememberLastGarden(id: number): void {
|
||||
try {
|
||||
localStorage.setItem(KEY, String(id))
|
||||
} catch {
|
||||
// Resume is a convenience; ignore quota/availability failures.
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Forget the stored last garden. With `onlyIfEquals`, clears only when the stored
|
||||
* id matches — so a 404 on a directly-linked garden can't wipe a different, still
|
||||
* good resume target the user had.
|
||||
*/
|
||||
export function forgetLastGarden(onlyIfEquals?: number): void {
|
||||
try {
|
||||
if (onlyIfEquals != null && getLastGardenId() !== onlyIfEquals) return
|
||||
localStorage.removeItem(KEY)
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
}
|
||||
@@ -36,6 +36,8 @@ import { useJournalCounts } from '@/lib/journal'
|
||||
import { attributableLots, lotsByPlant, useSeedLots, type SeedLot } from '@/lib/seedLots'
|
||||
import { useSeedTray } from '@/lib/seedTray'
|
||||
import { usePageTitle } from '@/lib/usePageTitle'
|
||||
import { rememberLastGarden, forgetLastGarden } from '@/lib/lastGarden'
|
||||
import { ApiError } from '@/lib/api'
|
||||
|
||||
const routeApi = getRouteApi('/gardens/$gardenId')
|
||||
|
||||
@@ -55,6 +57,14 @@ export function GardenEditorPage() {
|
||||
const me = useMe()
|
||||
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 select = useEditorStore((s) => s.select)
|
||||
const selectedPlantingId = useEditorStore((s) => s.selectedPlantingId)
|
||||
@@ -133,6 +143,24 @@ export function GardenEditorPage() {
|
||||
navigate({ search: (prev) => ({ ...prev, focus: focusedObjectId ?? undefined }), replace: true })
|
||||
}, [focusedObjectId, navigate])
|
||||
|
||||
// Remember this garden as the device's last-opened, so `/` resumes here next
|
||||
// visit. Keyed on the live query: which garden EXISTS doesn't depend on the
|
||||
// season being viewed.
|
||||
useEffect(() => {
|
||||
|
|
||||
if (live.isSuccess) rememberLastGarden(gid)
|
||||
}, [live.isSuccess, gid])
|
||||
|
||||
// A last garden that 404s (deleted, or access revoked) must not strand the user
|
||||
// on an error screen the `/` resume keeps returning to: forget it (only if it's
|
||||
// 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.
|
||||
useEffect(() => {
|
||||
if (gardenGone) {
|
||||
forgetLastGarden(gid)
|
||||
navigate({ to: '/gardens' })
|
||||
}
|
||||
}, [gardenGone, gid, navigate])
|
||||
|
||||
// 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.
|
||||
useEffect(() => {
|
||||
@@ -256,12 +284,20 @@ export function GardenEditorPage() {
|
||||
}, [])
|
||||
|
||||
if (full.isPending) return <p className="p-6 text-sm text-muted">Loading garden…</p>
|
||||
if (full.isError)
|
||||
if (full.isError) {
|
||||
// Only promise the bounce when the GARDEN is gone (the effect above keys on
|
||||
// the same gardenGone). A season-view error while the live garden is fine is a
|
||||
// different, non-redirecting failure and gets the generic message.
|
||||
return (
|
||||
<div className="p-6">
|
||||
<Alert>Could not load this garden.</Alert>
|
||||
<Alert>
|
||||
{gardenGone
|
||||
? 'That garden is no longer available — taking you to your gardens…'
|
||||
: 'Could not load this garden.'}
|
||||
</Alert>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
const g = full.data.garden
|
||||
const garden: EditorGarden = {
|
||||
|
||||
@@ -18,6 +18,7 @@ import { SettingsPage } from '@/pages/SettingsPage'
|
||||
import { meQueryOptions } from '@/lib/auth'
|
||||
import { queryClient } from '@/lib/queryClient'
|
||||
import { safeRedirectPath } from '@/lib/redirect'
|
||||
import { getLastGardenId } from '@/lib/lastGarden'
|
||||
|
||||
interface RouterContext {
|
||||
queryClient: QueryClient
|
||||
@@ -66,7 +67,15 @@ async function requireAdmin(context: RouterContext, path: string) {
|
||||
const indexRoute = createRoute({
|
||||
getParentRoute: () => rootRoute,
|
||||
path: '/',
|
||||
// Resume the garden this device was last in, so a returning user lands back in
|
||||
// their plot rather than on the list every time. A stored id that no longer
|
||||
// loads is cleared by the editor and bounces here to /gardens on the next
|
||||
// visit, so this can't trap anyone on a dead garden.
|
||||
beforeLoad: () => {
|
||||
const last = getLastGardenId()
|
||||
if (last != null) {
|
||||
throw redirect({ to: '/gardens/$gardenId', params: { gardenId: String(last) } })
|
||||
}
|
||||
throw redirect({ to: '/gardens' })
|
||||
},
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user
🔴 Missing replace: true in 404 bounce creates back-button trap
correctness, error-handling · flagged by 3 models
web/src/pages/GardenEditorPage.tsx:152— The 404 bounce usesnavigate({ to: '/gardens' })withoutreplace: true, leaving the dead/gardens/$identry in browser history. When a stale last-garden ID triggers this path, pressing Back from/gardensreturns to the dead entry, which 404s again and auto-bounces forward, trapping the user. Addreplace: trueso the dead entry is swapped out.🪰 Gadfly · advisory