From 85c355ce4daabe876cc1e3613bb7bd8b998adf75 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sun, 19 Jul 2026 00:12:54 -0400 Subject: [PATCH] Address Gadfly review on #17: share-response bug + error surfacing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical: - useAddShare/useUpdateShareRole no longer parse the response with the list's Share schema — the backend returns a bare GardenShare (no email/displayName), so every successful invite/role-change was throwing and showing a false error. onSuccess refetches the list, which has the full data. - ShareGardenModal surfaces role-change / remove failures (mutate onError → the dialog's Alert) instead of swallowing them; Modal busy now covers all three pending states. Robustness / defense in depth: - Inspector/PlopInspector patch() early-returns when readOnly, so a blur can't fire a mutation if the role was downgraded mid-edit. - onPickPlant guards canEdit. - Ownership is now the authoritative ownerId==me check (GardenCard via a currentUserId prop, GardenEditorPage via useMe), so a missing my_role can never lock an owner out or show them a Leave button. Cleanup: - Shared cardActionClass/cardDangerClass (GardenCard + PlantCard). - Normalized the read-only
indentation; dropped the dead useShares `enabled` param. tsc --noEmit clean; 24/24 vitest; production build green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi --- web/src/components/gardens/GardenCard.tsx | 23 +- .../components/gardens/ShareGardenModal.tsx | 17 +- web/src/components/plants/PlantCard.tsx | 14 +- web/src/components/ui/cardActions.ts | 10 + web/src/editor/Inspector.tsx | 204 +++++++++--------- web/src/editor/PlopInspector.tsx | 70 +++--- web/src/lib/shares.ts | 15 +- web/src/pages/GardenEditorPage.tsx | 12 +- web/src/pages/GardensPage.tsx | 3 + 9 files changed, 196 insertions(+), 172 deletions(-) create mode 100644 web/src/components/ui/cardActions.ts diff --git a/web/src/components/gardens/GardenCard.tsx b/web/src/components/gardens/GardenCard.tsx index 56f6f06..ebab3c2 100644 --- a/web/src/components/gardens/GardenCard.tsx +++ b/web/src/components/gardens/GardenCard.tsx @@ -1,33 +1,30 @@ import { Link } from '@tanstack/react-router' -import { isOwnerRole, type Garden } from '@/lib/gardens' +import type { Garden } from '@/lib/gardens' import { formatDimensions } from '@/lib/units' - -const actionClass = - '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' +import { cardActionClass, cardDangerClass } from '@/components/ui/cardActions' /** * One garden as a card: the body links into the editor. The footer differs by * 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). + * Ownership is the authoritative ownerId==me check, not the my_role hint. */ export function GardenCard({ garden, + currentUserId, onShare, onEdit, onDelete, onLeave, }: { garden: Garden + currentUserId?: number onShare: () => void onEdit: () => void onDelete: () => void onLeave: () => void }) { - const owner = isOwnerRole(garden.myRole) + const owner = currentUserId != null && garden.ownerId === currentUserId return (
{owner ? ( <> - - - ) : ( - )} diff --git a/web/src/components/gardens/ShareGardenModal.tsx b/web/src/components/gardens/ShareGardenModal.tsx index c9c6bf1..a4a706d 100644 --- a/web/src/components/gardens/ShareGardenModal.tsx +++ b/web/src/components/gardens/ShareGardenModal.tsx @@ -46,8 +46,10 @@ export function ShareGardenModal({ garden, onClose }: { garden: Garden; onClose: } } + const onMutationError = (fallback: string) => (err: unknown) => setError(errorMessage(err, fallback)) + return ( - +
- {!builtin && ( - )} {!builtin && ( - )} diff --git a/web/src/components/ui/cardActions.ts b/web/src/components/ui/cardActions.ts new file mode 100644 index 0000000..241498e --- /dev/null +++ b/web/src/components/ui/cardActions.ts @@ -0,0 +1,10 @@ +// Shared footer-action button styling for list cards (garden/plant), so the +// verbatim class strings don't drift between them. + +export const cardActionClass = + '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' + +export const cardDangerClass = + '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' diff --git a/web/src/editor/Inspector.tsx b/web/src/editor/Inspector.tsx index 610ee9f..13bd9b2 100644 --- a/web/src/editor/Inspector.tsx +++ b/web/src/editor/Inspector.tsx @@ -66,8 +66,10 @@ export function Inspector({ setColor(object.color ?? DEFAULT_COLOR) }, [object.version, object.widthCm, object.heightCm, object.xCm, object.yCm, object.rotationDeg, object.name, object.notes, object.color, unit]) - const patch = (fields: Partial>) => + const patch = (fields: Partial>) => { + if (readOnly) return // a blur mustn't fire a mutation if the role changed mid-edit update.mutate({ id: object.id, version: object.version, ...fields }) + } // Commit a dimension/position field. `positive` gates width/height (must be // ≥ the server minimum) but not x/y, which may be zero or negative. Compare at @@ -119,111 +121,111 @@ export function Inspector({ onBlur={() => name !== object.name && patch({ name })} /> -
- setWidth(e.target.value)} - onBlur={() => commitDim(width, object.widthCm, (cm) => patch({ widthCm: cm }), true)} - /> - setHeight(e.target.value)} - onBlur={() => commitDim(height, object.heightCm, (cm) => patch({ heightCm: cm }), true)} - /> - setX(e.target.value)} - onBlur={() => commitDim(x, object.xCm, (cm) => patch({ xCm: cm }))} - /> - setY(e.target.value)} - onBlur={() => commitDim(y, object.yCm, (cm) => patch({ yCm: cm }))} - /> -
- - setRotation(e.target.value)} - onBlur={() => { - const v = parseFloat(rotation) - if (Number.isFinite(v) && v !== object.rotationDeg) patch({ rotationDeg: v }) - }} - /> - -
-
- - ) => setColor(e.target.value)} - onBlur={() => color !== (object.color ?? DEFAULT_COLOR) && patch({ color })} - className="h-9 w-14 cursor-pointer rounded-md border border-border bg-surface" +
+ setWidth(e.target.value)} + onBlur={() => commitDim(width, object.widthCm, (cm) => patch({ widthCm: cm }), true)} + /> + setHeight(e.target.value)} + onBlur={() => commitDim(height, object.heightCm, (cm) => patch({ heightCm: cm }), true)} + /> + setX(e.target.value)} + onBlur={() => commitDim(x, object.xCm, (cm) => patch({ xCm: cm }))} + /> + setY(e.target.value)} + onBlur={() => commitDim(y, object.yCm, (cm) => patch({ yCm: cm }))} />
- {object.color && ( - - )} -
- -