From 32cbe36817c91cfaff67cdcb528c2a2370445166 Mon Sep 17 00:00:00 2001
From: Steve Dudenhoeffer <steve@stevedudenhoeffer.com>
Date: Tue, 21 Jul 2026 01:54:11 -0400
Subject: [PATCH] Address Gadfly review on the journal UI
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The prop names were backwards. `canEdit` meant "may delete" and `canWrite` meant
"may rewrite the text" — which is the opposite of how either word reads, on a
component where the distinction is the entire permission model. They are now
canDelete and canRewrite.

An open edit draft never re-synced with its entry, so a refetch after someone
else's edit left a stale draft and a Cancel that restored the wrong text. It now
resyncs when the entry changes underneath, but only while not editing, so a
background refetch can't clobber what's being typed — the version guard is what
catches a genuine collision.

A 409 from an edit had no handling, so the component kept its stale version and
every retry conflicted again: a button that just doesn't work. A conflict now
invalidates, so the fresh version arrives.

Saving an empty edit returned silently, which also reads as a broken button. It
says why nothing happened, and points at Delete if that's what was meant. A
failed delete's error now clears on retry rather than making a second attempt
look like a second failure.

formatObservedAt let out-of-range parts roll over — 2026-13-45 rendering as
February 2027, confidently wrong. It shows the raw string instead.

Also: the scope select guards against a non-numeric value becoming a NaN
objectId and a cryptic 400; the journal total is memoized rather than
spread-and-reduced every render; EditorRail's doc comment no longer calls the
journal "later" now that it's here; and a static class list stopped being split
across two cn() arguments for no reason.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
---
 web/src/editor/EditorRail.tsx      |  6 ++--
 web/src/editor/JournalPanel.tsx    | 51 ++++++++++++++++++++----------
 web/src/lib/journal.ts             | 12 +++++--
 web/src/pages/GardenEditorPage.tsx |  6 +++-
 4 files changed, 53 insertions(+), 22 deletions(-)

diff --git a/web/src/editor/EditorRail.tsx b/web/src/editor/EditorRail.tsx
index 2aa76b4..a836274 100644
--- a/web/src/editor/EditorRail.tsx
+++ b/web/src/editor/EditorRail.tsx
@@ -4,9 +4,9 @@ import { cn } from '@/lib/cn'
 /**
  * The editor's side rail, and the answer to "four things want one rail".
  *
- * The inspector, history, and (later) the journal and chat panel all want the
- * same strip of screen. Rather than each bolting on its own chrome, they are
- * tabs in one rail — which keeps the canvas one width instead of a different
+ * The inspector, history, journal, and (later) the chat panel all want the same
+ * strip of screen. Rather than each bolting on its own chrome, they are tabs in
+ * one rail — which keeps the canvas one width instead of a different
  * width per panel, and means adding the journal or chat is adding a tab.
  *
  * Two constraints shaped it:
diff --git a/web/src/editor/JournalPanel.tsx b/web/src/editor/JournalPanel.tsx
index a9827e9..d37c193 100644
--- a/web/src/editor/JournalPanel.tsx
+++ b/web/src/editor/JournalPanel.tsx
@@ -4,7 +4,6 @@ import { Button } from '@/components/ui/Button'
 import { TextArea } from '@/components/ui/TextArea'
 import { TextField } from '@/components/ui/TextField'
 import { errorMessage } from '@/lib/api'
-import { cn } from '@/lib/cn'
 import {
   formatObservedAt,
   today,
@@ -55,7 +54,10 @@ export function JournalPanel({
         {objects.length > 0 && (
           <select
             value={scopeObjectId ?? ''}
-            onChange={(e) => onScopeChange(e.target.value === '' ? null : Number(e.target.value))}
+            onChange={(e) => {
+              const id = Number(e.target.value)
+              onScopeChange(e.target.value === '' || !Number.isFinite(id) ? null : id)
+            }}
             className="max-w-[9rem] truncate rounded-md border border-border bg-surface px-1.5 py-1 text-xs text-fg outline-none focus-visible:ring-2 focus-visible:ring-accent/40"
           >
             <option value="">Whole garden</option>
@@ -96,8 +98,8 @@ export function JournalPanel({
             entry={e}
             gardenId={gardenId}
             objects={objects}
-            canEdit={canEdit && (e.authorId === currentUserId || isOwner)}
-            canWrite={canEdit && e.authorId === currentUserId}
+            canDelete={canEdit && (e.authorId === currentUserId || isOwner)}
+            canRewrite={canEdit && e.authorId === currentUserId}
           />
         ))}
       </ol>
@@ -185,16 +187,17 @@ function Entry({
   entry,
   gardenId,
   objects,
-  canEdit,
-  canWrite,
+  canDelete,
+  canRewrite,
 }: {
   entry: JournalEntry
   gardenId: number
   objects: EditorObject[]
   /** May remove it: the author, or the garden owner. */
-  canEdit: boolean
-  /** May rewrite the text: the author only. */
-  canWrite: boolean
+  canDelete: boolean
+  /** May rewrite the text: the author only — rewriting someone else's
+   *  observation under their name is a different act from removing it. */
+  canRewrite: boolean
 }) {
   const update = useUpdateJournalEntry(gardenId)
   const del = useDeleteJournalEntry(gardenId)
@@ -203,9 +206,24 @@ function Entry({
   const [error, setError] = useState<string | null>(null)
   const about = objects.find((o) => o.id === entry.objectId) ?? null
 
+  // Re-sync the draft when the entry changes underneath — a refetch after
+  // someone else's edit, or this entry's own successful save. Skipped while
+  // editing so a background refetch can't clobber what's being typed; the
+  // version guard is what catches a genuine collision.
+  const [syncedBody, setSyncedBody] = useState(entry.body)
+  if (!editing && entry.body !== syncedBody) {
+    setSyncedBody(entry.body)
+    setDraft(entry.body)
+  }
+
   const save = () => {
     const text = draft.trim()
-    if (!text) return
+    if (!text) {
+      // Silence here reads as a broken button; say why nothing happened.
+      setError('An entry needs some text. Delete it instead if that\'s what you meant.')
+      return
+    }
+    setError(null)
     update.mutate(
       { id: entry.id, version: entry.version, body: text },
       {
@@ -257,26 +275,27 @@ function Entry({
         {entry.plantingId != null && <span className="rounded bg-border/60 px-1 py-px">planting</span>}
       </p>
 
-      {(canWrite || canEdit) && !editing && (
+      {(canRewrite || canDelete) && !editing && (
         <div className="mt-1 flex justify-end gap-1">
-          {canWrite && (
+          {canRewrite && (
             <button
               type="button"
               onClick={() => setEditing(true)}
-              className={cn('rounded px-1.5 py-0.5 text-xs text-muted outline-none hover:text-fg', 'focus-visible:ring-2 focus-visible:ring-accent/40')}
+              className="rounded px-1.5 py-0.5 text-xs text-muted outline-none hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40"
             >
               Edit
             </button>
           )}
-          {canEdit && (
+          {canDelete && (
             <button
               type="button"
               disabled={del.isPending}
-              onClick={() =>
+              onClick={() => {
+                setError(null) // clear a previous failure so a retry isn't read as another
                 del.mutate(entry.id, {
                   onError: (err) => setError(errorMessage(err, "Couldn't delete that note.")),
                 })
-              }
+              }}
               className="rounded px-1.5 py-0.5 text-xs text-red-700 outline-none hover:underline focus-visible:ring-2 focus-visible:ring-accent/40 dark:text-red-400"
             >
               Delete
diff --git a/web/src/lib/journal.ts b/web/src/lib/journal.ts
index d9e159e..772eaf6 100644
--- a/web/src/lib/journal.ts
+++ b/web/src/lib/journal.ts
@@ -6,7 +6,7 @@
 
 import { useInfiniteQuery, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
 import { z } from 'zod'
-import { api } from './api'
+import { ApiError, api } from './api'
 
 export const journalEntrySchema = z.object({
   id: z.number(),
@@ -108,6 +108,12 @@ export function useUpdateJournalEntry(gardenId: number) {
       return journalEntrySchema.parse(await api.patch(`/journal/${id}`, rest))
     },
     onSuccess: () => invalidateJournal(qc, gardenId),
+    onError: (err) => {
+      // A 409 means the row moved on. Refetch so the component receives the
+      // current version — otherwise a retry resends the stale one and conflicts
+      // forever, which reads as a button that simply doesn't work.
+      if (err instanceof ApiError && err.isConflict) invalidateJournal(qc, gardenId)
+    },
   })
 }
 
@@ -133,6 +139,8 @@ export function today(): string {
  *  through a Date (which would shift it by the timezone offset). */
 export function formatObservedAt(iso: string): string {
   const [y, m, d] = iso.split('-').map(Number)
-  if (!y || !m || !d) return iso
+  // Out-of-range parts would silently roll over — 2026-13-45 becoming February
+  // 2027 — so show the raw string instead of a confidently wrong date.
+  if (!y || !m || !d || m < 1 || m > 12 || d < 1 || d > 31) return iso
   return new Date(y, m - 1, d).toLocaleDateString(undefined, { month: 'short', day: 'numeric', year: 'numeric' })
 }
diff --git a/web/src/pages/GardenEditorPage.tsx b/web/src/pages/GardenEditorPage.tsx
index 3cef24d..c1655e8 100644
--- a/web/src/pages/GardenEditorPage.tsx
+++ b/web/src/pages/GardenEditorPage.tsx
@@ -63,6 +63,10 @@ export function GardenEditorPage() {
   const journalObjectId = useEditorStore((s) => s.journalObjectId)
   const setJournalObjectId = useEditorStore((s) => s.setJournalObjectId)
   const journalCounts = useJournalCounts(gid)
+  const journalTotal = useMemo(
+    () => [...(journalCounts.data?.values() ?? [])].reduce((a, b) => a + b, 0),
+    [journalCounts.data],
+  )
   const armedPlant = useEditorStore((s) => s.armedPlant)
   const setArmedPlant = useEditorStore((s) => s.setArmedPlant)
 
@@ -349,7 +353,7 @@ export function GardenEditorPage() {
       label: 'Journal',
       // The total on the tab is the discoverability half: a garden with a
       // season's notes in it shouldn't look identical to an empty one.
-      badge: [...(journalCounts.data?.values() ?? [])].reduce((a, b) => a + b, 0),
+      badge: journalTotal,
       render: () => (
         <JournalPanel
           gardenId={gid}