Address Gadfly review on the grow journal

PATCH and DELETE /journal/:id were never registered. Both handlers existed, were
covered by service-level tests, and were completely unreachable — my edit to the
router anchored on a string that only exists on another branch, so it silently
did nothing. Registered, and covered by an API-level test that walks the whole
lifecycle through the router, because that is the only kind of test that could
have caught it. Anything addressed by its own id now has one.

An entry about a plop was invisible under its bed's filter. Naming only a
planting left object_id null, so "notes about this bed" silently excluded every
note written about something growing IN the bed — the two filters disagreed
about what an entry is about, which is precisely the question the reader is
asking. The parent object is now derived from the planting.

checkJournalTarget flattened every store error to ErrInvalidInput, so a real
database failure surfaced as a 400 telling the caller their request was bad, and
never reached the logs. Only "no such row" is a bad reference now; anything else
passes through.

ListJournalEntries repeated the column scan order inline, one column different
from scanJournalEntry — the classic way for a shared column list to drift out of
step with its readers. Both now build from journalScanTargets, with the list
appending the joined author name.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ

internal/api/api.go

@@ -121,6 +121,12 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
 	changeSets := v1.Group("/change-sets", h.requireAuth())
 	changeSets.POST("/:id/revert", h.revertChangeSet)
 
+	// Journal entries are addressed by their own id; the service resolves the
+	// owning garden for the permission check, same as objects and plantings.
+	journal := v1.Group("/journal", h.requireAuth())
+	journal.PATCH("/:id", h.updateJournalEntry)
+	journal.DELETE("/:id", h.deleteJournalEntry)
+
 	// Plant catalog: built-ins (seeded, read-only) plus the actor's own rows.
 	plants := v1.Group("/plants", h.requireAuth())
 	plants.GET("", h.listPlants)

internal/api/journal_test.go

@@ -0,0 +1,157 @@
+package api
+
+import (
+	"net/http"
+	"strconv"
+	"testing"
+)
+
+func journalPath(gardenID int64) string {
+	return "/api/v1/gardens/" + strconv.FormatInt(gardenID, 10) + "/journal"
+}
+
+func journalEntryPath(id int64) string {
+	return "/api/v1/journal/" + strconv.FormatInt(id, 10)
+}
+
+// TestJournalCrudAPI walks the whole entry lifecycle over HTTP.
+//
+// It exists because the service-level tests could not see that PATCH and DELETE
+// were never registered on the router: the handlers were written, tested through
+// the service, and completely unreachable. Anything addressed by its own id
+// needs at least one test that goes through the router.
+func TestJournalCrudAPI(t *testing.T) {
+	r := authEngine(t, localCfg())
+	cookie := registerAndCookie(t, r, "[email protected]")
+	gid := createGardenAPI(t, r, cookie, "G")
+
+	w := doJSON(t, r, http.MethodPost, journalPath(gid), map[string]any{
+		"body": "First frost tonight", "observedAt": "2026-10-12",
+	}, cookie)
+	if w.Code != http.StatusCreated {
+		t.Fatalf("create: status %d, body %s", w.Code, w.Body.String())
+	}
+	entry := decodeMap(t, w.Body.Bytes())
+	id := int64(entry["id"].(float64))
+	if entry["observedAt"] != "2026-10-12" || entry["body"] != "First frost tonight" {
+		t.Errorf("unexpected entry: %+v", entry)
+	}
+
+	// List it back.
+	w = doJSON(t, r, http.MethodGet, journalPath(gid), nil, cookie)
+	if w.Code != http.StatusOK {
+		t.Fatalf("list: status %d, body %s", w.Code, w.Body.String())
+	}
+	body := decodeMap(t, w.Body.Bytes())
+	entries, _ := body["entries"].([]any)
+	if len(entries) != 1 {
+		t.Fatalf("got %d entries, want 1: %s", len(entries), w.Body.String())
+	}
+	if first := entries[0].(map[string]any); first["authorName"] == "" {
+		t.Error("author name missing from the list read")
+	}
+
+	// PATCH — the route this test exists for.
+	w = doJSON(t, r, http.MethodPatch, journalEntryPath(id), map[string]any{
+		"body": "First frost, tomatoes done", "version": entry["version"],
+	}, cookie)
+	if w.Code != http.StatusOK {
+		t.Fatalf("patch: status %d, body %s", w.Code, w.Body.String())
+	}
+	updated := decodeMap(t, w.Body.Bytes())
+	if updated["body"] != "First frost, tomatoes done" {
+		t.Errorf("body = %v", updated["body"])
+	}
+
+	// A stale version conflicts, carrying the current row.
+	w = doJSON(t, r, http.MethodPatch, journalEntryPath(id), map[string]any{
+		"body": "stale", "version": entry["version"],
+	}, cookie)
+	if w.Code != http.StatusConflict {
+		t.Errorf("stale patch: status %d, want 409", w.Code)
+	}
+	if current, _ := decodeMap(t, w.Body.Bytes())["current"].(map[string]any); current == nil {
+		t.Error("409 should carry the current row")
+	}
+
+	// DELETE — likewise.
+	w = doJSON(t, r, http.MethodDelete, journalEntryPath(id), nil, cookie)
+	if w.Code != http.StatusNoContent {
+		t.Fatalf("delete: status %d, body %s", w.Code, w.Body.String())
+	}
+	w = doJSON(t, r, http.MethodGet, journalPath(gid), nil, cookie)
+	if entries, _ := decodeMap(t, w.Body.Bytes())["entries"].([]any); len(entries) != 0 {
+		t.Errorf("%d entries survived the delete", len(entries))
+	}
+}
+
+// TestJournalFiltersAPI covers the query parameters, including a malformed one.
+func TestJournalFiltersAPI(t *testing.T) {
+	r := authEngine(t, localCfg())
+	cookie := registerAndCookie(t, r, "[email protected]")
+	gid := createGardenAPI(t, r, cookie, "G")
+
+	w := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{
+		"kind": "bed", "name": "North Bed", "xCm": 100, "yCm": 100, "widthCm": 100, "heightCm": 100,
+	}, cookie)
+	objectID := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
+
+	doJSON(t, r, http.MethodPost, journalPath(gid), map[string]any{
+		"body": "garden level", "observedAt": "2026-05-01",
+	}, cookie)
+	doJSON(t, r, http.MethodPost, journalPath(gid), map[string]any{
+		"objectId": objectID, "body": "bed level", "observedAt": "2026-06-01",
+	}, cookie)
+
+	countAt := func(query string) int {
+		t.Helper()
+		w := doJSON(t, r, http.MethodGet, journalPath(gid)+query, nil, cookie)
+		if w.Code != http.StatusOK {
+			t.Fatalf("list%s: status %d, body %s", query, w.Code, w.Body.String())
+		}
+		entries, _ := decodeMap(t, w.Body.Bytes())["entries"].([]any)
+		return len(entries)
+	}
+	if n := countAt(""); n != 2 {
+		t.Errorf("unfiltered = %d, want 2", n)
+	}
+	if n := countAt("?objectId=" + strconv.FormatInt(objectID, 10)); n != 1 {
+		t.Errorf("by object = %d, want 1", n)
+	}
+	if n := countAt("?from=2026-05-15&to=2026-12-31"); n != 1 {
+		t.Errorf("by date range = %d, want 1", n)
+	}
+
+	// A malformed filter is an error, not a silently ignored one that widens the
+	// query back to the whole garden.
+	w = doJSON(t, r, http.MethodGet, journalPath(gid)+"?objectId=abc", nil, cookie)
+	if w.Code != http.StatusBadRequest {
+		t.Errorf("malformed objectId: status %d, want 400", w.Code)
+	}
+	w = doJSON(t, r, http.MethodGet, journalPath(gid)+"?from=nonsense", nil, cookie)
+	if w.Code != http.StatusBadRequest {
+		t.Errorf("malformed from: status %d, want 400", w.Code)
+	}
+}
+
+// TestJournalRequiresAccessAPI — anonymous is 401, a stranger is 404.
+func TestJournalRequiresAccessAPI(t *testing.T) {
+	r := authEngine(t, localCfg())
+	owner := registerAndCookie(t, r, "[email protected]")
+	gid := createGardenAPI(t, r, owner, "G")
+	stranger := registerAndCookie(t, r, "[email protected]")
+
+	if w := doJSON(t, r, http.MethodGet, journalPath(gid), nil, nil); w.Code != http.StatusUnauthorized {
+		t.Errorf("anonymous list: status %d, want 401", w.Code)
+	}
+	if w := doJSON(t, r, http.MethodGet, journalPath(gid), nil, stranger); w.Code != http.StatusNotFound {
+		t.Errorf("stranger list: status %d, want 404", w.Code)
+	}
+	w := doJSON(t, r, http.MethodPost, journalPath(gid), map[string]any{"body": "nope"}, stranger)
+	if w.Code != http.StatusNotFound {
+		t.Errorf("stranger write: status %d, want 404", w.Code)
+	}
+	if w := doJSON(t, r, http.MethodDelete, journalEntryPath(1), nil, stranger); w.Code != http.StatusNotFound {
+		t.Errorf("stranger delete: status %d, want 404", w.Code)
+	}
+}