Sharing UI: invite by email, roles, read-only viewer mode (#17) #36

Merged
steve merged 2 commits from phase-6-sharing-ui into main 2026-07-19 04:14:30 +00:00
10 changed files with 433 additions and 97 deletions
Showing only changes of commit be5ed18db0 - Show all commits
+36 -13
View File
@@ -1,20 +1,33 @@
import { Link } from '@tanstack/react-router' import { Link } from '@tanstack/react-router'
import type { Garden } from '@/lib/gardens' import { isOwnerRole, type Garden } from '@/lib/gardens'
import { formatDimensions } from '@/lib/units' import { formatDimensions } from '@/lib/units'
const actionClass =
Review

🟡 actionClass/dangerClass duplicated verbatim from PlantCard — should be shared

maintainability · flagged by 3 models

  • Duplicated actionClass / dangerClass constantsweb/src/components/gardens/GardenCard.tsx:5-10 defines the exact same two class strings as web/src/components/plants/PlantCard.tsx:5-10 (verified identical character-for-character). This is copy-paste that should be shared — e.g. extract to web/src/components/ui/button.ts or a cardActions helper. As more card components appear, this drift will diverge.

🪰 Gadfly · advisory

🟡 **actionClass/dangerClass duplicated verbatim from PlantCard — should be shared** _maintainability · flagged by 3 models_ - **Duplicated `actionClass` / `dangerClass` constants** — `web/src/components/gardens/GardenCard.tsx:5-10` defines the exact same two class strings as `web/src/components/plants/PlantCard.tsx:5-10` (verified identical character-for-character). This is copy-paste that should be shared — e.g. extract to `web/src/components/ui/button.ts` or a `cardActions` helper. As more card components appear, this drift will diverge. <sub>🪰 Gadfly · advisory</sub>
'rounded-md px-2.5 py-1 text-sm font-medium text-muted outline-none transition-colors ' +
'hover:bg-border/50 hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40'
const dangerClass =
'rounded-md px-2.5 py-1 text-sm font-medium text-muted outline-none transition-colors ' +
'hover:bg-red-500/10 hover:text-red-600 focus-visible:ring-2 focus-visible:ring-red-500/40 dark:hover:text-red-400'
/** /**
* One garden as a card: the body links into the editor (/gardens/:id); the * One garden as a card: the body links into the editor. The footer differs by
* footer has edit/delete actions (kept out of the link so they don't navigate). * role — the owner gets Share / Edit / Delete; a recipient sees a "shared · role"
* badge and a Leave action (garden metadata edit + sharing are owner-only).
*/ */
export function GardenCard({ export function GardenCard({
garden, garden,
onShare,
onEdit, onEdit,
onDelete, onDelete,
onLeave,
}: { }: {
garden: Garden garden: Garden
onShare: () => void
onEdit: () => void onEdit: () => void
onDelete: () => void onDelete: () => void
onLeave: () => void
}) { }) {
const owner = isOwnerRole(garden.myRole)
Outdated
Review

🟡 Garden with undefined myRole is treated as non-owner and shown a Leave (mutating) button with no badge; fail-closed direction is wrong for unknown role

correctness · flagged by 1 model

  • web/src/components/gardens/GardenCard.tsx:30,40,64 — a garden with myRole === undefined is treated as non-owner and gets a Leave button with no badge. isOwnerRole(undefined) is false (confirmed in gardens.ts:37), so the footer falls into the !owner branch and renders Leave; the badge is gated on !owner && garden.myRole (truthy), so a missing role yields a card with no role label and a Leave button that calls remove.mutateAsync(me.data.id) — a self-share delete that like…

🪰 Gadfly · advisory

🟡 **Garden with undefined myRole is treated as non-owner and shown a Leave (mutating) button with no badge; fail-closed direction is wrong for unknown role** _correctness · flagged by 1 model_ - **`web/src/components/gardens/GardenCard.tsx:30,40,64` — a garden with `myRole === undefined` is treated as non-owner and gets a `Leave` button with no badge.** `isOwnerRole(undefined)` is false (confirmed in `gardens.ts:37`), so the footer falls into the `!owner` branch and renders `Leave`; the badge is gated on `!owner && garden.myRole` (truthy), so a missing role yields a card with no role label and a `Leave` button that calls `remove.mutateAsync(me.data.id)` — a self-share delete that like… <sub>🪰 Gadfly · advisory</sub>
return ( return (
<div className="flex flex-col rounded-xl border border-border bg-surface transition-colors hover:border-accent/50"> <div className="flex flex-col rounded-xl border border-border bg-surface transition-colors hover:border-accent/50">
<Link <Link
@@ -22,27 +35,37 @@ export function GardenCard({
params={{ gardenId: String(garden.id) }} params={{ gardenId: String(garden.id) }}
className="flex-1 rounded-t-xl p-4 outline-none focus-visible:ring-2 focus-visible:ring-accent/40" className="flex-1 rounded-t-xl p-4 outline-none focus-visible:ring-2 focus-visible:ring-accent/40"
> >
<div className="flex items-center gap-2">
<h3 className="truncate font-semibold text-fg">{garden.name}</h3> <h3 className="truncate font-semibold text-fg">{garden.name}</h3>
{!owner && garden.myRole && (
<span className="shrink-0 rounded bg-border/60 px-1.5 py-0.5 text-[10px] font-medium uppercase tracking-wide text-muted">
shared · {garden.myRole}
Review

Badge prints raw lowercase enum myRole; casing/wording inconsistent with role labels in ShareGardenModal

maintainability · flagged by 1 model

🪰 Gadfly · advisory

⚪ **Badge prints raw lowercase enum myRole; casing/wording inconsistent with role labels in ShareGardenModal** _maintainability · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
</span>
)}
</div>
<p className="mt-1 text-sm text-muted"> <p className="mt-1 text-sm text-muted">
{formatDimensions(garden.widthCm, garden.heightCm, garden.unitPref)} {formatDimensions(garden.widthCm, garden.heightCm, garden.unitPref)}
</p> </p>
{garden.notes && <p className="mt-2 line-clamp-2 text-sm text-muted">{garden.notes}</p>} {garden.notes && <p className="mt-2 line-clamp-2 text-sm text-muted">{garden.notes}</p>}
</Link> </Link>
<div className="flex justify-end gap-1 border-t border-border px-2 py-1.5"> <div className="flex justify-end gap-1 border-t border-border px-2 py-1.5">
<button {owner ? (
type="button" <>
onClick={onEdit} <button type="button" onClick={onShare} className={actionClass}>
className="rounded-md px-2.5 py-1 text-sm font-medium text-muted outline-none transition-colors hover:bg-border/50 hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40" Share
> </button>
<button type="button" onClick={onEdit} className={actionClass}>
Edit Edit
</button> </button>
<button <button type="button" onClick={onDelete} className={dangerClass}>
type="button"
onClick={onDelete}
className="rounded-md px-2.5 py-1 text-sm font-medium text-muted outline-none transition-colors hover:bg-red-500/10 hover:text-red-600 focus-visible:ring-2 focus-visible:ring-red-500/40 dark:hover:text-red-400"
>
Delete Delete
</button> </button>
</>
) : (
<button type="button" onClick={onLeave} className={dangerClass}>
Leave
</button>
)}
</div> </div>
</div> </div>
) )
@@ -0,0 +1,47 @@
import { useState } from 'react'
import { Modal } from '@/components/ui/Modal'
import { Alert } from '@/components/ui/Alert'
import { Button } from '@/components/ui/Button'
import { errorMessage } from '@/lib/api'
import { useMe } from '@/lib/auth'
import type { Garden } from '@/lib/gardens'
import { useRemoveShare } from '@/lib/shares'
/** Confirmation for a recipient leaving a garden shared with them (removes their
* own share). */
export function LeaveGardenModal({ garden, onClose }: { garden: Garden; onClose: () => void }) {
const me = useMe()
const remove = useRemoveShare(garden.id)
const [error, setError] = useState<string | null>(null)
async function onConfirm() {
if (!me.data) return
setError(null)
try {
await remove.mutateAsync(me.data.id)
onClose()
} catch (err) {
setError(errorMessage(err, 'Could not leave the garden.'))
}
}
return (
<Modal title="Leave garden" onClose={onClose} busy={remove.isPending}>
<div className="flex flex-col gap-4">
<p className="text-sm text-muted">
Leave <span className="font-medium text-fg">{garden.name}</span>? You'll lose access until the owner
shares it with you again.
</p>
{error && <Alert>{error}</Alert>}
<div className="flex justify-end gap-2">
<Button type="button" variant="ghost" onClick={onClose} disabled={remove.isPending}>
Cancel
</Button>
<Button type="button" variant="danger" onClick={onConfirm} disabled={remove.isPending || !me.data}>
Review

🟡 Non-401 useMe failure leaves Leave button disabled with no user-facing message

error-handling · flagged by 1 model

  • web/src/components/gardens/LeaveGardenModal.tsx:40 — a non-401 useMe failure leaves the user stuck with a disabled button and no explanation. Verified in web/src/lib/auth.ts:29-44: meQueryOptions.queryFn returns null only on ApiError.isUnauthorized (401); any other error is rethrown, leaving me.data undefined. LeaveGardenModal.tsx:40 sets disabled={remove.isPending || !me.data} with no error/loading state for me, so a non-401 useMe failure keeps "Leave" permanently di…

🪰 Gadfly · advisory

🟡 **Non-401 useMe failure leaves Leave button disabled with no user-facing message** _error-handling · flagged by 1 model_ - **`web/src/components/gardens/LeaveGardenModal.tsx:40` — a non-401 `useMe` failure leaves the user stuck with a disabled button and no explanation.** Verified in `web/src/lib/auth.ts:29-44`: `meQueryOptions.queryFn` returns `null` only on `ApiError.isUnauthorized` (401); any other error is rethrown, leaving `me.data` undefined. `LeaveGardenModal.tsx:40` sets `disabled={remove.isPending || !me.data}` with no error/loading state for `me`, so a non-401 `useMe` failure keeps "Leave" permanently di… <sub>🪰 Gadfly · advisory</sub>
{remove.isPending ? 'Leaving' : 'Leave'}
</Button>
</div>
</div>
</Modal>
)
}
@@ -0,0 +1,121 @@
import { useState, type FormEvent } from 'react'
import { Modal } from '@/components/ui/Modal'
import { Alert } from '@/components/ui/Alert'
import { Button } from '@/components/ui/Button'
import { Select } from '@/components/ui/Select'
import { TextField } from '@/components/ui/TextField'
import { cn } from '@/lib/cn'
import { fieldControlClass } from '@/components/ui/field'
import { errorMessage } from '@/lib/api'
import type { Garden } from '@/lib/gardens'
import { useAddShare, useRemoveShare, useShares, useUpdateShareRole, type ShareRole } from '@/lib/shares'
const roleOptions = [
{ value: 'viewer', label: 'Viewer (read-only)' },
{ value: 'editor', label: 'Editor (can edit)' },
]
/**
* Owner-only dialog to manage a garden's shares: invite an existing user by
* email as viewer/editor, change a share's role, or remove it. Targets existing
* accounts only (v1 has no invitation emails) — an unknown email surfaces a
* friendly "no account with that email".
*/
export function ShareGardenModal({ garden, onClose }: { garden: Garden; onClose: () => void }) {
const shares = useShares(garden.id)
const add = useAddShare(garden.id)
const updateRole = useUpdateShareRole(garden.id)
const remove = useRemoveShare(garden.id)
const [email, setEmail] = useState('')
const [role, setRole] = useState<ShareRole>('viewer')
const [error, setError] = useState<string | null>(null)
async function onInvite(e: FormEvent) {
e.preventDefault()
setError(null)
if (!email.trim()) {
setError('Enter an email address.')
return
}
try {
await add.mutateAsync({ email: email.trim(), role })
setEmail('')
} catch (err) {
setError(errorMessage(err, 'Could not share the garden.'))
}
}
return (
<Modal title="Share garden" onClose={onClose} busy={add.isPending}>
Outdated
Review

🟠 Modal busy prop does not cover updateRole or remove pending states

error-handling · flagged by 3 models

  • web/src/components/gardens/ShareGardenModal.tsx:50 — The Modal’s busy prop only tracks add.isPending, so Escape / backdrop-click can dismiss the dialog while an in-flight updateRole or remove is still pending. That buries any eventual error and leaves the user unsure whether the action completed. Fix: Pass busy={add.isPending || updateRole.isPending || remove.isPending}.

🪰 Gadfly · advisory

🟠 **Modal busy prop does not cover updateRole or remove pending states** _error-handling · flagged by 3 models_ * **`web/src/components/gardens/ShareGardenModal.tsx:50`** — The Modal’s `busy` prop only tracks `add.isPending`, so Escape / backdrop-click can dismiss the dialog while an in-flight `updateRole` or `remove` is still pending. That buries any eventual error and leaves the user unsure whether the action completed. **Fix:** Pass `busy={add.isPending || updateRole.isPending || remove.isPending}`. <sub>🪰 Gadfly · advisory</sub>
<div className="flex flex-col gap-4">
<form onSubmit={onInvite} className="flex flex-col gap-2">
<TextField
label="Invite by email"
name="email"
type="email"
placeholder="[email protected]"
value={email}
onChange={(e) => setEmail(e.target.value)}
/>
<div className="flex items-end gap-2">
<Select
label="Role"
name="role"
value={role}
onChange={(e) => setRole(e.target.value as ShareRole)}
options={roleOptions}
className="flex-1"
/>
<Button type="submit" disabled={add.isPending}>
{add.isPending ? 'Sharing…' : 'Share'}
</Button>
</div>
{error && <Alert>{error}</Alert>}
</form>
<div>
<h3 className="mb-2 text-sm font-medium text-fg">Shared with</h3>
{shares.isPending && <p className="text-sm text-muted">Loading</p>}
{shares.isError && <Alert>Could not load who this garden is shared with.</Alert>}
{shares.isSuccess && shares.data.length === 0 && (
<p className="text-sm text-muted">Not shared with anyone yet.</p>
)}
<ul className="flex flex-col gap-2">
Review

🟡 Empty ul rendered during loading and error states

maintainability · flagged by 1 model

  • web/src/components/gardens/ShareGardenModal.tsx:84 — The <ul className="flex flex-col gap-2"> is rendered unconditionally, so it sits empty in the DOM during loading and error states alongside the "Loading…" / error messages. Move it inside the shares.isSuccess && shares.data.length > 0 branch.

🪰 Gadfly · advisory

🟡 **Empty ul rendered during loading and error states** _maintainability · flagged by 1 model_ - `web/src/components/gardens/ShareGardenModal.tsx:84` — The `<ul className="flex flex-col gap-2">` is rendered unconditionally, so it sits empty in the DOM during loading and error states alongside the "Loading…" / error messages. Move it inside the `shares.isSuccess && shares.data.length > 0` branch. <sub>🪰 Gadfly · advisory</sub>
{shares.data?.map((sh) => (
<li key={sh.userId} className="flex items-center gap-2">
<div className="min-w-0 flex-1">
<p className="truncate text-sm font-medium text-fg">{sh.displayName}</p>
<p className="truncate text-xs text-muted">{sh.email}</p>
</div>
<select
Review

🔴 updateRole.mutate / remove.mutate silently swallow errors — no onError, no user feedback on failed role change or revoke

correctness, error-handling, maintainability · flagged by 5 models

  • web/src/components/gardens/ShareGardenModal.tsx:91 — Uses a raw <select> element (with manually imported fieldControlClass and cn) for the inline role changer, while the invite form 20 lines above uses the project's <Select> component. This creates two styling paths for the same control and duplicates the viewer/editor options already defined in the module-level roleOptions array. Prefer <Select> (or map roleOptions) to keep the UI consistent and DRY.

🪰 Gadfly · advisory

🔴 **updateRole.mutate / remove.mutate silently swallow errors — no onError, no user feedback on failed role change or revoke** _correctness, error-handling, maintainability · flagged by 5 models_ - `web/src/components/gardens/ShareGardenModal.tsx:91` — Uses a raw `<select>` element (with manually imported `fieldControlClass` and `cn`) for the inline role changer, while the invite form 20 lines above uses the project's `<Select>` component. This creates two styling paths for the same control and duplicates the `viewer`/`editor` options already defined in the module-level `roleOptions` array. Prefer `<Select>` (or map `roleOptions`) to keep the UI consistent and DRY. <sub>🪰 Gadfly · advisory</sub>
value={sh.role}
onChange={(e) => updateRole.mutate({ userId: sh.userId, role: e.target.value as ShareRole })}
aria-label={`Role for ${sh.displayName}`}
className={cn(fieldControlClass, 'w-auto px-2 py-1 text-sm')}
>
<option value="viewer">Viewer</option>
<option value="editor">Editor</option>
</select>
<button
Outdated
Review

🔴 remove mutation has no error handling or pending UI

correctness, error-handling, maintainability · flagged by 5 models

  • web/src/components/gardens/ShareGardenModal.tsx:102remove.mutate(sh.userId) is fire-and-forget with no error surface. useRemoveShare (lib/shares.ts:60-69) defines only onSuccess, no onError, and the call site has no try/catch. If the DELETE fails, the ✕ button appears to do nothing and the user gets no feedback. Same fix applies.

🪰 Gadfly · advisory

🔴 **remove mutation has no error handling or pending UI** _correctness, error-handling, maintainability · flagged by 5 models_ - **`web/src/components/gardens/ShareGardenModal.tsx:102`** — `remove.mutate(sh.userId)` is fire-and-forget with no error surface. `useRemoveShare` (`lib/shares.ts:60-69`) defines only `onSuccess`, no `onError`, and the call site has no try/catch. If the DELETE fails, the ✕ button appears to do nothing and the user gets no feedback. Same fix applies. <sub>🪰 Gadfly · advisory</sub>
type="button"
onClick={() => remove.mutate(sh.userId)}
aria-label={`Remove ${sh.displayName}`}
className="rounded-md px-2 py-1 text-sm text-muted transition-colors hover:bg-red-500/10 hover:text-red-600 dark:hover:text-red-400"
>
</button>
</li>
))}
</ul>
</div>
<div className="flex justify-end">
<Button variant="ghost" onClick={onClose}>
Done
</Button>
</div>
</div>
</Modal>
)
}
+11 -5
View File
@@ -34,11 +34,13 @@ export function GardenCanvas({
objects, objects,
plantings, plantings,
plantsById, plantsById,
canEdit,
}: { }: {
garden: EditorGarden garden: EditorGarden
objects: EditorObject[] objects: EditorObject[]
plantings: EditorPlanting[] plantings: EditorPlanting[]
plantsById: Map<number, Plant> plantsById: Map<number, Plant>
canEdit: boolean
}) { }) {
const svgRef = useRef<SVGSVGElement>(null) const svgRef = useRef<SVGSVGElement>(null)
const containerRef = useRef<HTMLDivElement>(null) const containerRef = useRef<HTMLDivElement>(null)
@@ -108,7 +110,9 @@ export function GardenCanvas({
// or exit focus mode, or just deselect. // or exit focus mode, or just deselect.
function onCanvasPointerDown(e: ReactPointerEvent) { function onCanvasPointerDown(e: ReactPointerEvent) {
const armed = useEditorStore.getState().armedKind const armed = useEditorStore.getState().armedKind
if (armed) { // Defense in depth: a viewer can't place objects even if a stale armed kind
// slipped through (the palette isn't rendered for them).
if (armed && canEdit) {
useEditorStore.getState().setArmedKind(null) useEditorStore.getState().setArmedKind(null)
const def = kindDef(armed) const def = kindDef(armed)
const rect = svgRef.current?.getBoundingClientRect() const rect = svgRef.current?.getBoundingClientRect()
@@ -134,7 +138,7 @@ export function GardenCanvas({
// armed for repeat-placement until Escape / Done). // armed for repeat-placement until Escape / Done).
function onPlace(e: ReactPointerEvent) { function onPlace(e: ReactPointerEvent) {
e.stopPropagation() e.stopPropagation()
if (!focusedObject || !armedPlant || !focusedObject.plantable) return if (!canEdit || !focusedObject || !armedPlant || !focusedObject.plantable) return
const rect = svgRef.current?.getBoundingClientRect() const rect = svgRef.current?.getBoundingClientRect()
if (!rect) return if (!rect) return
const world = screenToWorld({ x: e.clientX - rect.left, y: e.clientY - rect.top }, viewport) const world = screenToWorld({ x: e.clientX - rect.left, y: e.clientY - rect.top }, viewport)
@@ -188,14 +192,16 @@ export function GardenCanvas({
onSelectPlop={selectPlanting} onSelectPlop={selectPlanting}
/> />
{selectedObject && <SelectionOverlay object={selectedObject} gardenId={garden.id} svgRef={svgRef} />} {/* Edit handles are mounted only for editors/owners; viewers can still
{selectedPlop && selectedPlopObject && ( select to inspect (read-only), but never move/resize. */}
{canEdit && selectedObject && <SelectionOverlay object={selectedObject} gardenId={garden.id} svgRef={svgRef} />}
{canEdit && selectedPlop && selectedPlopObject && (
<PlopOverlay plop={selectedPlop} object={selectedPlopObject} gardenId={garden.id} svgRef={svgRef} /> <PlopOverlay plop={selectedPlop} object={selectedPlopObject} gardenId={garden.id} svgRef={svgRef} />
)} )}
{/* Placement capture: a transparent sheet over the focused object while a {/* Placement capture: a transparent sheet over the focused object while a
plant is armed, so taps drop plops instead of selecting the object. */} plant is armed, so taps drop plops instead of selecting the object. */}
{focusedObject && armedPlant && focusedObject.plantable && ( {canEdit && focusedObject && armedPlant && focusedObject.plantable && (
<g transform={objectTransform(focusedObject)}> <g transform={objectTransform(focusedObject)}>
<rect <rect
x={-halfFW} x={-halfFW}
+14 -3
View File
@@ -27,11 +27,13 @@ export function Inspector({
gardenId, gardenId,
unit, unit,
onFocus, onFocus,
readOnly = false,
}: { }: {
object: EditorObject object: EditorObject
gardenId: number gardenId: number
unit: UnitPref unit: UnitPref
onFocus?: () => void onFocus?: () => void
readOnly?: boolean
}) { }) {
const update = useUpdateObject(gardenId) const update = useUpdateObject(gardenId)
const del = useDeleteObject(gardenId) const del = useDeleteObject(gardenId)
@@ -96,12 +98,19 @@ export function Inspector({
</button> </button>
</div> </div>
{object.plantable && onFocus && ( {readOnly && (
<p className="rounded-md bg-border/40 px-2 py-1 text-xs text-muted">View only you can't edit this garden.</p>
Outdated
Review

🟡 Duplicated 'View only' badge string/styling across Inspector, PlopInspector, and GardenEditorPage

maintainability · flagged by 1 model

🪰 Gadfly · advisory

🟡 **Duplicated 'View only' badge string/styling across Inspector, PlopInspector, and GardenEditorPage** _maintainability · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
)}
{!readOnly && object.plantable && onFocus && (
<Button onClick={onFocus} className="w-full"> <Button onClick={onFocus} className="w-full">
🌱 Plant here 🌱 Plant here
</Button> </Button>
)} )}
{/* A disabled fieldset makes every control below read-only for viewers in
one shot (no per-input disabled). */}
<fieldset disabled={readOnly} className="flex min-w-0 flex-col gap-3 border-0 p-0">
Review

🟠 Inconsistent indentation inside fieldset wrapper makes JSX hierarchy unreadable

maintainability · flagged by 5 models

  • web/src/editor/Inspector.tsx:113 — The <fieldset> wrapper added for read-only mode contains children at wildly inconsistent indentation (e.g. the grid div at line 122 sits at the same level as the fieldset tag, while the Name field at line 114 and Notes field at line 219 are indented +2). This makes the JSX hierarchy unreadable and invites future edits that break the intended nesting. Re-indent everything inside the fieldset uniformly.

🪰 Gadfly · advisory

🟠 **Inconsistent indentation inside fieldset wrapper makes JSX hierarchy unreadable** _maintainability · flagged by 5 models_ - `web/src/editor/Inspector.tsx:113` — The `<fieldset>` wrapper added for read-only mode contains children at wildly inconsistent indentation (e.g. the grid div at line 122 sits at the same level as the fieldset tag, while the Name field at line 114 and Notes field at line 219 are indented +2). This makes the JSX hierarchy unreadable and invites future edits that break the intended nesting. Re-indent everything inside the fieldset uniformly. <sub>🪰 Gadfly · advisory</sub>
<TextField <TextField
label="Name" label="Name"
name="name" name="name"
1
@@ -215,8 +224,10 @@ export function Inspector({
onChange={(e) => setNotes(e.target.value)} onChange={(e) => setNotes(e.target.value)}
onBlur={() => notes !== object.notes && patch({ notes })} onBlur={() => notes !== object.notes && patch({ notes })}
/> />
</fieldset>
{confirmingDelete ? ( {!readOnly &&
(confirmingDelete ? (
<div className="flex items-center gap-2"> <div className="flex items-center gap-2">
<Button <Button
variant="danger" variant="danger"
@@ -237,7 +248,7 @@ export function Inspector({
<Button variant="ghost" className="text-red-600 dark:text-red-400" onClick={() => setConfirmingDelete(true)}> <Button variant="ghost" className="text-red-600 dark:text-red-400" onClick={() => setConfirmingDelete(true)}>
Delete object Delete object
</Button> </Button>
)} ))}
</div> </div>
) )
} }
+12
View File
@@ -25,6 +25,7 @@ export function PlopInspector({
unit, unit,
onChangePlant, onChangePlant,
onClose, onClose,
readOnly = false,
}: { }: {
plop: EditorPlanting plop: EditorPlanting
plant?: Plant plant?: Plant
@@ -32,6 +33,7 @@ export function PlopInspector({
unit: UnitPref unit: UnitPref
onChangePlant: () => void onChangePlant: () => void
onClose: () => void onClose: () => void
readOnly?: boolean
}) { }) {
const update = useUpdatePlanting(gardenId) const update = useUpdatePlanting(gardenId)
const remove = useRemovePlanting(gardenId) const remove = useRemovePlanting(gardenId)
@@ -104,6 +106,10 @@ export function PlopInspector({
</button> </button>
</div> </div>
{readOnly && (
<p className="rounded-md bg-border/40 px-2 py-1 text-xs text-muted">View only you can't edit this garden.</p>
)}
<div className="flex items-center gap-2 rounded-lg border border-border p-2"> <div className="flex items-center gap-2 rounded-lg border border-border p-2">
{plant ? ( {plant ? (
<PlantIcon color={plant.color} icon={plant.icon} className="h-9 w-9 rounded-md text-xl" /> <PlantIcon color={plant.color} icon={plant.icon} className="h-9 w-9 rounded-md text-xl" />
@@ -111,11 +117,14 @@ export function PlopInspector({
<span className="grid h-9 w-9 place-items-center rounded-md bg-border/50 text-muted">?</span> <span className="grid h-9 w-9 place-items-center rounded-md bg-border/50 text-muted">?</span>
)} )}
<span className="min-w-0 flex-1 truncate text-sm font-medium text-fg">{plant?.name ?? 'Unknown plant'}</span> <span className="min-w-0 flex-1 truncate text-sm font-medium text-fg">{plant?.name ?? 'Unknown plant'}</span>
{!readOnly && (
<Button variant="ghost" className="px-2 py-1 text-xs" onClick={onChangePlant}> <Button variant="ghost" className="px-2 py-1 text-xs" onClick={onChangePlant}>
Change Change
</Button> </Button>
)}
</div> </div>
<fieldset disabled={readOnly} className="flex min-w-0 flex-col gap-3 border-0 p-0">
Outdated
Review

🟠 Inconsistent indentation inside fieldset wrapper makes JSX hierarchy unreadable

maintainability · flagged by 5 models

  • web/src/editor/PlopInspector.tsx:127 — Same indentation problem: the <fieldset> tag and its first child <div> (line 128) are at the same indentation level, while the inner fields vary between +0 and +2. Uniformly indent all children.

🪰 Gadfly · advisory

🟠 **Inconsistent indentation inside fieldset wrapper makes JSX hierarchy unreadable** _maintainability · flagged by 5 models_ - `web/src/editor/PlopInspector.tsx:127` — Same indentation problem: the `<fieldset>` tag and its first child `<div>` (line 128) are at the same indentation level, while the inner fields vary between +0 and +2. Uniformly indent all children. <sub>🪰 Gadfly · advisory</sub>
<div className="grid grid-cols-2 gap-2"> <div className="grid grid-cols-2 gap-2">
<TextField <TextField
label={`Radius (${u})`} label={`Radius (${u})`}
@@ -159,7 +168,9 @@ export function PlopInspector({
onChange={(e) => setPlanted(e.target.value)} onChange={(e) => setPlanted(e.target.value)}
onBlur={commitPlanted} onBlur={commitPlanted}
/> />
</fieldset>
{!readOnly && (
<Button <Button
variant="ghost" variant="ghost"
className="text-red-600 dark:text-red-400" className="text-red-600 dark:text-red-400"
@@ -171,6 +182,7 @@ export function PlopInspector({
> >
Remove plant Remove plant
</Button> </Button>
)}
</div> </div>
) )
} }
+16
View File
@@ -8,6 +8,9 @@ import type { UnitPref } from './units'
const unitPrefSchema = z.enum(['metric', 'imperial']) const unitPrefSchema = z.enum(['metric', 'imperial'])
export const gardenRoleSchema = z.enum(['owner', 'editor', 'viewer'])
export type GardenRole = z.infer<typeof gardenRoleSchema>
export const gardenSchema = z.object({ export const gardenSchema = z.object({
id: z.number(), id: z.number(),
ownerId: z.number(), ownerId: z.number(),
@@ -19,9 +22,22 @@ export const gardenSchema = z.object({
version: z.number(), version: z.number(),
createdAt: z.string(), createdAt: z.string(),
updatedAt: z.string(), updatedAt: z.string(),
// The actor's effective role on this garden (owner/editor/viewer). Present on
// every accessible response; optional defensively.
myRole: gardenRoleSchema.optional(),
}) })
export type Garden = z.infer<typeof gardenSchema> export type Garden = z.infer<typeof gardenSchema>
/** Whether a role may edit garden contents (objects/plops). Unknown → false. */
export function canEditRole(role?: GardenRole): boolean {
return role === 'owner' || role === 'editor'
}
/** Whether a role owns the garden (may edit metadata, manage shares, delete). */
export function isOwnerRole(role?: GardenRole): boolean {
return role === 'owner'
}
const gardensKey = ['gardens'] as const const gardensKey = ['gardens'] as const
export const gardensQueryOptions = queryOptions({ export const gardensQueryOptions = queryOptions({
+69
View File
@@ -0,0 +1,69 @@
// Garden sharing data layer: zod shapes for /gardens/:id/shares plus the
// react-query hooks the share dialog and "leave garden" action use.
import { queryOptions, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
import { z } from 'zod'
import { api } from './api'
export const shareRoleSchema = z.enum(['viewer', 'editor'])
export type ShareRole = z.infer<typeof shareRoleSchema>
export const shareSchema = z.object({
id: z.number(),
gardenId: z.number(),
userId: z.number(),
role: shareRoleSchema,
createdBy: z.number(),
version: z.number(),
createdAt: z.string(),
updatedAt: z.string(),
email: z.string(),
displayName: z.string(),
})
export type Share = z.infer<typeof shareSchema>
const sharesKey = (gardenId: number) => ['shares', gardenId] as const
export function sharesQueryOptions(gardenId: number) {
return queryOptions({
queryKey: sharesKey(gardenId),
queryFn: async (): Promise<Share[]> => z.array(shareSchema).parse(await api.get(`/gardens/${gardenId}/shares`)),
})
}
/** The shares on a garden (owner-only endpoint; pass enabled=false to skip). */
export function useShares(gardenId: number, enabled = true) {
Outdated
Review

useShares enabled parameter has no caller; dead surface

maintainability · flagged by 1 model

  • web/src/editor/Inspector.tsx:113-227 & PlopInspector.tsx:127-171 — fieldset children not re-indented. The new <fieldset> wrappers enclose a large block of pre-existing JSX, but the children inside were left at their original indentation (e.g. <div className="grid grid-cols-2 gap-2"> at Inspector:122 sits at the same indent as the <fieldset> that owns it; same in PlopInspector:128). The closing </fieldset> at 227 is likewise dedented past the <TextArea> it contains. This flatt…

🪰 Gadfly · advisory

⚪ **useShares `enabled` parameter has no caller; dead surface** _maintainability · flagged by 1 model_ - **`web/src/editor/Inspector.tsx:113-227` & `PlopInspector.tsx:127-171` — fieldset children not re-indented.** The new `<fieldset>` wrappers enclose a large block of pre-existing JSX, but the children inside were left at their original indentation (e.g. `<div className="grid grid-cols-2 gap-2">` at Inspector:122 sits at the same indent as the `<fieldset>` that owns it; same in PlopInspector:128). The closing `</fieldset>` at 227 is likewise dedented past the `<TextArea>` it contains. This flatt… <sub>🪰 Gadfly · advisory</sub>
return useQuery({ ...sharesQueryOptions(gardenId), enabled })
}
export function useAddShare(gardenId: number) {
const qc = useQueryClient()
return useMutation({
mutationFn: async (input: { email: string; role: ShareRole }): Promise<Share> =>
shareSchema.parse(await api.post(`/gardens/${gardenId}/shares`, input)),
Review

🔴 useAddShare parses the POST /shares response with a schema requiring email/displayName, but the backend's AddShare returns a bare GardenShare without them — every successful invite throws and shows a false 'Could not share the garden' error

correctness · flagged by 1 model

🪰 Gadfly · advisory

🔴 **useAddShare parses the POST /shares response with a schema requiring email/displayName, but the backend's AddShare returns a bare GardenShare without them — every successful invite throws and shows a false 'Could not share the garden' error** _correctness · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
onSuccess: () => qc.invalidateQueries({ queryKey: sharesKey(gardenId) }),
})
}
export function useUpdateShareRole(gardenId: number) {
const qc = useQueryClient()
return useMutation({
mutationFn: async ({ userId, role }: { userId: number; role: ShareRole }): Promise<Share> =>
shareSchema.parse(await api.patch(`/gardens/${gardenId}/shares/${userId}`, { role })),
Outdated
Review

🔴 useUpdateShareRole parses the PATCH /shares/:userId response with the same over-strict schema; the backend's UpdateShareRole also omits email/displayName, so every successful role change throws and is swallowed silently (no onError in ShareGardenModal), leaving the UI reverted to the stale role

correctness · flagged by 1 model

🪰 Gadfly · advisory

🔴 **useUpdateShareRole parses the PATCH /shares/:userId response with the same over-strict schema; the backend's UpdateShareRole also omits email/displayName, so every successful role change throws and is swallowed silently (no onError in ShareGardenModal), leaving the UI reverted to the stale role** _correctness · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
onSuccess: () => qc.invalidateQueries({ queryKey: sharesKey(gardenId) }),
})
}
/** Remove a share. Used both by the owner (revoke) and by a recipient leaving a
* garden (their own userId). Refreshes the garden list too, since a leave
* changes what the current user can see. */
export function useRemoveShare(gardenId: number) {
const qc = useQueryClient()
return useMutation({
mutationFn: (userId: number) => api.delete(`/gardens/${gardenId}/shares/${userId}`),
onSuccess: () => {
qc.invalidateQueries({ queryKey: sharesKey(gardenId) })
qc.invalidateQueries({ queryKey: ['gardens'] })
Review

🟡 useRemoveShare invalidates ['gardens'] on every remove, causing avoidable gardens-list refetches in the owner-revoke path

performance · flagged by 1 model

  • web/src/lib/shares.ts:66useRemoveShare.onSuccess unconditionally invalidates ['gardens'] on every remove. This is correct and necessary for the leave path (the garden leaves the recipient's list), but it also fires for the owner revoke path in ShareGardenModal. When the owner manages shares from GardensPage (where the gardens query is active), each ✕ click triggers a full gardens-list refetch in addition to the shares refetch — so removing N shares means N extra gardens-li…

🪰 Gadfly · advisory

🟡 **useRemoveShare invalidates ['gardens'] on every remove, causing avoidable gardens-list refetches in the owner-revoke path** _performance · flagged by 1 model_ - **`web/src/lib/shares.ts:66`** — `useRemoveShare.onSuccess` unconditionally invalidates `['gardens']` on every remove. This is correct and necessary for the *leave* path (the garden leaves the recipient's list), but it also fires for the *owner revoke* path in `ShareGardenModal`. When the owner manages shares from `GardensPage` (where the gardens query is active), each ✕ click triggers a full gardens-list refetch in addition to the shares refetch — so removing N shares means N extra gardens-li… <sub>🪰 Gadfly · advisory</sub>
},
})
}
+23 -4
View File
@@ -10,6 +10,8 @@ import { Palette } from '@/editor/Palette'
import { kindDef } from '@/editor/kinds' import { kindDef } from '@/editor/kinds'
import { useEditorStore } from '@/editor/store' import { useEditorStore } from '@/editor/store'
import type { EditorGarden } from '@/editor/types' import type { EditorGarden } from '@/editor/types'
import { ShareGardenModal } from '@/components/gardens/ShareGardenModal'
import { canEditRole, isOwnerRole } from '@/lib/gardens'
import { toEditorObject, useGardenFull, useUpdatePlanting } from '@/lib/objects' import { toEditorObject, useGardenFull, useUpdatePlanting } from '@/lib/objects'
import { toEditorPlanting } from '@/lib/plantings' import { toEditorPlanting } from '@/lib/plantings'
@@ -39,6 +41,7 @@ export function GardenEditorPage() {
// Which plant-picker flow is open: 'place' arms a plant for repeat placement; // Which plant-picker flow is open: 'place' arms a plant for repeat placement;
// 'change' swaps the selected plop's plant. // 'change' swaps the selected plop's plant.
const [picker, setPicker] = useState<'place' | 'change' | null>(null) const [picker, setPicker] = useState<'place' | 'change' | null>(null)
const [sharing, setSharing] = useState(false)
const serverObjects = full.data?.objects const serverObjects = full.data?.objects
const objects = useMemo(() => serverObjects?.map(toEditorObject) ?? [], [serverObjects]) const objects = useMemo(() => serverObjects?.map(toEditorObject) ?? [], [serverObjects])
@@ -120,6 +123,9 @@ export function GardenEditorPage() {
heightCm: g.heightCm, heightCm: g.heightCm,
unitPref: g.unitPref, unitPref: g.unitPref,
} }
// Role-driven gating from the server's my_role — never guessed client-side.
const canEdit = canEditRole(g.myRole)
Outdated
Review

🟠 canEdit/isOwner false on missing myRole can lock an owner out of their own garden with no error surfaced

error-handling · flagged by 2 models

  • web/src/pages/GardenEditorPage.tsx:127canEdit/isOwner are false when myRole is absent, which can lock an owner out of their own garden with no error surfaced. Verified in web/src/lib/gardens.ts: canEditRole(undefined) (line 32-34) and isOwnerRole(undefined) (line 37-39) both return false, and myRole is declared gardenRoleSchema.optional() (line 27) "defensively." GardenEditorPage.tsx:127-128 gates all UI on these. If the server ever omits my_role (backend bug,…

🪰 Gadfly · advisory

🟠 **canEdit/isOwner false on missing myRole can lock an owner out of their own garden with no error surfaced** _error-handling · flagged by 2 models_ - **`web/src/pages/GardenEditorPage.tsx:127` — `canEdit`/`isOwner` are `false` when `myRole` is absent, which can lock an owner out of their own garden with no error surfaced.** Verified in `web/src/lib/gardens.ts`: `canEditRole(undefined)` (line 32-34) and `isOwnerRole(undefined)` (line 37-39) both return `false`, and `myRole` is declared `gardenRoleSchema.optional()` (line 27) "defensively." `GardenEditorPage.tsx:127-128` gates all UI on these. If the server ever omits `my_role` (backend bug,… <sub>🪰 Gadfly · advisory</sub>
const isOwner = isOwnerRole(g.myRole)
const selectedObject = objects.find((o) => o.id === selectedId) ?? null const selectedObject = objects.find((o) => o.id === selectedId) ?? null
const focusedObject = focusedObjectId != null ? objects.find((o) => o.id === focusedObjectId) ?? null : null const focusedObject = focusedObjectId != null ? objects.find((o) => o.id === focusedObjectId) ?? null : null
@@ -143,7 +149,15 @@ export function GardenEditorPage() {
<h1 className="mb-2 truncate text-lg font-semibold tracking-tight" title={garden.name}> <h1 className="mb-2 truncate text-lg font-semibold tracking-tight" title={garden.name}>
{garden.name} {garden.name}
</h1> </h1>
{focusedObjectId == null && <Palette />} {isOwner && (
<Button variant="ghost" className="mb-2 w-full text-sm" onClick={() => setSharing(true)}>
Share
</Button>
)}
{!canEdit && (
<p className="mb-2 rounded-md bg-border/40 px-2 py-1 text-xs text-muted">👁 View only</p>
)}
{focusedObjectId == null && canEdit && <Palette />}
</div> </div>
<div className="relative min-h-0 flex-1"> <div className="relative min-h-0 flex-1">
@@ -155,7 +169,8 @@ export function GardenEditorPage() {
<span className="max-w-[8rem] truncate text-muted"> <span className="max-w-[8rem] truncate text-muted">
{focusedObject.name || kindDef(focusedObject.kind)?.label || 'Object'} {focusedObject.name || kindDef(focusedObject.kind)?.label || 'Object'}
</span> </span>
{!focusedObject.plantable ? ( {canEdit &&
(!focusedObject.plantable ? (
<span className="text-xs text-muted">Not plantable</span> <span className="text-xs text-muted">Not plantable</span>
) : armedPlant ? ( ) : armedPlant ? (
<Button variant="ghost" className="px-2 py-1 text-xs" onClick={() => setArmedPlant(null)}> <Button variant="ghost" className="px-2 py-1 text-xs" onClick={() => setArmedPlant(null)}>
@@ -165,10 +180,10 @@ export function GardenEditorPage() {
<Button className="px-2 py-1 text-xs" onClick={() => setPicker('place')}> <Button className="px-2 py-1 text-xs" onClick={() => setPicker('place')}>
+ Add plant + Add plant
</Button> </Button>
)} ))}
</div> </div>
)} )}
<GardenCanvas garden={garden} objects={objects} plantings={plantings} plantsById={plantsById} /> <GardenCanvas garden={garden} objects={objects} plantings={plantings} plantsById={plantsById} canEdit={canEdit} />
</div> </div>
{(selectedObject || selectedPlop) && ( {(selectedObject || selectedPlop) && (
@@ -179,6 +194,7 @@ export function GardenEditorPage() {
object={selectedObject} object={selectedObject}
gardenId={gid} gardenId={gid}
unit={garden.unitPref} unit={garden.unitPref}
readOnly={!canEdit}
onFocus={() => { onFocus={() => {
setFocusedObject(selectedObject.id) setFocusedObject(selectedObject.id)
select(null) select(null)
@@ -192,6 +208,7 @@ export function GardenEditorPage() {
plant={plantsById.get(selectedPlop.plantId)} plant={plantsById.get(selectedPlop.plantId)}
gardenId={gid} gardenId={gid}
unit={garden.unitPref} unit={garden.unitPref}
readOnly={!canEdit}
onChangePlant={() => setPicker('change')} onChangePlant={() => setPicker('change')}
onClose={() => selectPlanting(null)} onClose={() => selectPlanting(null)}
/> />
@@ -206,6 +223,8 @@ export function GardenEditorPage() {
onSelect={(p) => onPickPlant(p.id)} onSelect={(p) => onPickPlant(p.id)}
/> />
)} )}
{sharing && <ShareGardenModal garden={g} onClose={() => setSharing(false)} />}
</div> </div>
) )
} }
+14 -2
View File
@@ -4,10 +4,18 @@ import { Button } from '@/components/ui/Button'
import { DeleteGardenModal } from '@/components/gardens/DeleteGardenModal' import { DeleteGardenModal } from '@/components/gardens/DeleteGardenModal'
import { GardenCard } from '@/components/gardens/GardenCard' import { GardenCard } from '@/components/gardens/GardenCard'
import { GardenFormModal } from '@/components/gardens/GardenFormModal' import { GardenFormModal } from '@/components/gardens/GardenFormModal'
import { LeaveGardenModal } from '@/components/gardens/LeaveGardenModal'
import { ShareGardenModal } from '@/components/gardens/ShareGardenModal'
import { useGardens, type Garden } from '@/lib/gardens' import { useGardens, type Garden } from '@/lib/gardens'
// Which modal is open, if any. `edit`/`delete` carry the target garden. // Which modal is open, if any. edit/delete/share/leave carry the target garden.
type Dialog = { kind: 'create' } | { kind: 'edit'; garden: Garden } | { kind: 'delete'; garden: Garden } | null type Dialog =
| { kind: 'create' }
| { kind: 'edit'; garden: Garden }
| { kind: 'delete'; garden: Garden }
| { kind: 'share'; garden: Garden }
| { kind: 'leave'; garden: Garden }
| null
export function GardensPage() { export function GardensPage() {
const gardens = useGardens() const gardens = useGardens()
@@ -42,8 +50,10 @@ export function GardensPage() {
<GardenCard <GardenCard
key={g.id} key={g.id}
garden={g} garden={g}
onShare={() => setDialog({ kind: 'share', garden: g })}
onEdit={() => setDialog({ kind: 'edit', garden: g })} onEdit={() => setDialog({ kind: 'edit', garden: g })}
onDelete={() => setDialog({ kind: 'delete', garden: g })} onDelete={() => setDialog({ kind: 'delete', garden: g })}
onLeave={() => setDialog({ kind: 'leave', garden: g })}
/> />
))} ))}
</div> </div>
@@ -53,6 +63,8 @@ export function GardensPage() {
{dialog?.kind === 'create' && <GardenFormModal onClose={close} />} {dialog?.kind === 'create' && <GardenFormModal onClose={close} />}
{dialog?.kind === 'edit' && <GardenFormModal garden={dialog.garden} onClose={close} />} {dialog?.kind === 'edit' && <GardenFormModal garden={dialog.garden} onClose={close} />}
{dialog?.kind === 'delete' && <DeleteGardenModal garden={dialog.garden} onClose={close} />} {dialog?.kind === 'delete' && <DeleteGardenModal garden={dialog.garden} onClose={close} />}
{dialog?.kind === 'share' && <ShareGardenModal garden={dialog.garden} onClose={close} />}
{dialog?.kind === 'leave' && <LeaveGardenModal garden={dialog.garden} onClose={close} />}
</section> </section>
) )
} }