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/service/journal.go

@@ -2,6 +2,7 @@ package service
 
 import (
 	"context"
+	"errors"
 	"strings"
 
 	"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
@@ -93,13 +94,14 @@ func (s *Service) CreateJournalEntry(ctx context.Context, actorID, gardenID int6
 	if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleEditor); err != nil {
 		return nil, err
 	}
-	if err := s.checkJournalTarget(ctx, gardenID, in.ObjectID, in.PlantingID); err != nil {
+	objectID, err := s.resolveJournalTarget(ctx, gardenID, in.ObjectID, in.PlantingID)
+	if err != nil {
 		return nil, err
 	}
 
 	e := &domain.JournalEntry{
 		GardenID:   gardenID,
-		ObjectID:   in.ObjectID,
+		ObjectID:   objectID,
 		PlantingID: in.PlantingID,
 		AuthorID:   actorID,
 		Body:       strings.TrimSpace(in.Body),
@@ -115,31 +117,57 @@ func (s *Service) CreateJournalEntry(ctx context.Context, actorID, gardenID int6
 	return s.store.CreateJournalEntry(ctx, e)
 }
 
-// checkJournalTarget verifies that an object/planting an entry points at really
-// lives in this garden.
-func (s *Service) checkJournalTarget(ctx context.Context, gardenID int64, objectID, plantingID *int64) error {
+// resolveJournalTarget validates an entry's target and returns the object id to
+// store against it.
+//
+// A plop-level entry gets its parent object filled in even when the caller
+// didn't name one. Without that, "notes about this bed" would silently exclude
+// every note written about a plant IN the bed — the bed filter and the plop
+// filter would disagree about what an entry is about, which is exactly the
+// question the reader is asking.
+//
+// A store failure that isn't "no such row" is passed through rather than
+// flattened to ErrInvalidInput: a database problem is not the caller's bad
+// request, and mapping it to a 400 would hide it from the logs entirely.
+func (s *Service) resolveJournalTarget(ctx context.Context, gardenID int64, objectID, plantingID *int64) (*int64, error) {
 	if objectID != nil {
 		o, err := s.store.GetObject(ctx, *objectID)
-		if err != nil || o.GardenID != gardenID {
-			return domain.ErrInvalidInput
+		if errors.Is(err, domain.ErrNotFound) {
+			return nil, domain.ErrInvalidInput
 		}
-	}
-	if plantingID != nil {
-		pl, err := s.store.GetPlanting(ctx, *plantingID)
 		if err != nil {
-			return domain.ErrInvalidInput
+			return nil, err
 		}
-		o, err := s.store.GetObject(ctx, pl.ObjectID)
-		if err != nil || o.GardenID != gardenID {
-			return domain.ErrInvalidInput
-		}
-		// A plop-level entry that also names an object must name the RIGHT one,
-		// or the two filters would disagree about which entries are about what.
-		if objectID != nil && *objectID != pl.ObjectID {
-			return domain.ErrInvalidInput
+		if o.GardenID != gardenID {
+			return nil, domain.ErrInvalidInput
 		}
 	}
-	return nil
+	if plantingID == nil {
+		return objectID, nil
+	}
+
+	pl, err := s.store.GetPlanting(ctx, *plantingID)
+	if errors.Is(err, domain.ErrNotFound) {
+		return nil, domain.ErrInvalidInput
+	}
+	if err != nil {
+		return nil, err
+	}
+	o, err := s.store.GetObject(ctx, pl.ObjectID)
+	if errors.Is(err, domain.ErrNotFound) {
+		return nil, domain.ErrInvalidInput
+	}
+	if err != nil {
+		return nil, err
+	}
+	if o.GardenID != gardenID {
+		return nil, domain.ErrInvalidInput
+	}
+	// A plop-level entry that ALSO names an object must name the right one.
+	if objectID != nil && *objectID != pl.ObjectID {
+		return nil, domain.ErrInvalidInput
+	}
+	return &pl.ObjectID, nil
 }
 
 // journalEntryForWrite loads an entry and enforces who may change it: the author

internal/service/journal_test.go

@@ -292,3 +292,34 @@ func TestJournalStaysOutOfRevisionHistory(t *testing.T) {
 		t.Errorf("journal writes produced %d change sets; they should produce none", got-before)
 	}
 }
+
+// TestPlopEntryIsVisibleUnderItsBed — an entry about a plant IN a bed is an entry
+// about that bed. Without deriving the parent object, "notes about this bed"
+// would silently exclude every note written about something growing in it.
+func TestPlopEntryIsVisibleUnderItsBed(t *testing.T) {
+	s := newTestService(t, openConfig())
+	owner := seedUser(t, s, "[email protected]")
+	g := seedGarden(t, s, owner)
+	bed := seedBed(t, s, owner, g.ID)
+	plant := seedOwnPlant(t, s, owner, 15)
+	ctx := context.Background()
+
+	plop, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{
+		PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 20,
+	})
+	if err != nil {
+		t.Fatalf("CreatePlanting: %v", err)
+	}
+
+	// Only the plop is named — no objectId.
+	e := writeEntry(t, s, owner, g.ID, JournalInput{PlantingID: &plop.ID, Body: "Scapes forming"})
+	if e.ObjectID == nil || *e.ObjectID != bed.ID {
+		t.Fatalf("objectId = %v, want the plop's parent bed %d", e.ObjectID, bed.ID)
+	}
+	if byBed := listJournal(t, s, owner, g.ID, JournalQuery{ObjectID: &bed.ID}); len(byBed) != 1 {
+		t.Errorf("the bed filter found %d entries, want the plop's", len(byBed))
+	}
+	if byPlop := listJournal(t, s, owner, g.ID, JournalQuery{PlantingID: &plop.ID}); len(byPlop) != 1 {
+		t.Errorf("the plop filter found %d entries, want 1", len(byPlop))
+	}
+}