From 5eedcaf945b7ba49d76d4b717300977c3f4c6f3d Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Tue, 21 Jul 2026 01:29:24 -0400 Subject: [PATCH] Refuse deleting a plant that has seed lots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit seed_lots.plant_id is ON DELETE CASCADE, so deleting a plant would have silently destroyed the purchase records attached to it — vendor, cost, germination rate, gone with no warning. Plantings were already guarded with ErrPlantInUse because their FK is RESTRICT; lots needed the same guard for the opposite reason, since their FK would have obliged rather than refused. Refusing lets the owner delete the lots deliberately if that is really what they meant, which is the same shape as the existing clones-and-edits path. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ --- internal/service/plants.go | 13 +++++++++++++ internal/service/seed_lots_test.go | 26 ++++++++++++++++++++++++++ internal/store/seed_lots.go | 12 ++++++++++++ 3 files changed, 51 insertions(+) diff --git a/internal/service/plants.go b/internal/service/plants.go index e834217..5b90abf 100644 --- a/internal/service/plants.go +++ b/internal/service/plants.go @@ -129,6 +129,12 @@ func (s *Service) UpdatePlant(ctx context.Context, actorID, plantID int64, patch // planting (even a removed one — the FK is RESTRICT) is refused with // ErrPlantInUse rather than orphaning history; the client clones-and-edits or // clears the plantings first. +// +// Seed lots are refused for a different reason: seed_lots.plant_id is ON DELETE +// CASCADE, so without this check deleting a plant would silently destroy the +// purchase records attached to it — vendor, cost, germination rate, all gone +// with no warning. Refusing lets the owner delete the lots deliberately if that +// is really what they meant. func (s *Service) DeletePlant(ctx context.Context, actorID, plantID int64) error { if _, err := s.writablePlant(ctx, actorID, plantID); err != nil { return err @@ -140,6 +146,13 @@ func (s *Service) DeletePlant(ctx context.Context, actorID, plantID int64) error if n > 0 { return domain.ErrPlantInUse } + lots, err := s.store.CountSeedLotsForPlant(ctx, plantID) + if err != nil { + return err + } + if lots > 0 { + return domain.ErrPlantInUse + } return s.store.DeletePlant(ctx, plantID) } diff --git a/internal/service/seed_lots_test.go b/internal/service/seed_lots_test.go index a148ed7..cb7dadb 100644 --- a/internal/service/seed_lots_test.go +++ b/internal/service/seed_lots_test.go @@ -343,3 +343,29 @@ func TestSeedLotUnitAndQuantityValidation(t *testing.T) { } func intPtr(v int) *int { return &v } + +// TestDeletingAPlantWithLotsIsRefused — seed_lots.plant_id is ON DELETE CASCADE, +// so without a guard, deleting a plant would silently destroy the purchase +// records attached to it: vendor, cost, germination rate, gone with no warning. +func TestDeletingAPlantWithLotsIsRefused(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + plant := seedOwnPlant(t, s, owner, 15) + lot := seedLot(t, s, owner, plant.ID, 100, func(in *SeedLotInput) { in.CostCents = intPtr(499) }) + ctx := context.Background() + + if err := s.DeletePlant(ctx, owner, plant.ID); !errors.Is(err, domain.ErrPlantInUse) { + t.Fatalf("DeletePlant err = %v, want ErrPlantInUse", err) + } + if _, err := s.GetSeedLot(ctx, owner, lot.ID); err != nil { + t.Errorf("the lot was destroyed anyway: %v", err) + } + + // Deleting the lot deliberately then frees the plant. + if err := s.DeleteSeedLot(ctx, owner, lot.ID); err != nil { + t.Fatalf("DeleteSeedLot: %v", err) + } + if err := s.DeletePlant(ctx, owner, plant.ID); err != nil { + t.Errorf("DeletePlant after clearing lots: %v", err) + } +} diff --git a/internal/store/seed_lots.go b/internal/store/seed_lots.go index dfacff1..a26e4b5 100644 --- a/internal/store/seed_lots.go +++ b/internal/store/seed_lots.go @@ -140,6 +140,18 @@ func (d *DB) DeleteSeedLot(ctx context.Context, id int64) error { return nil } +// CountSeedLotsForPlant returns how many lots reference a plant. Used to refuse +// deleting a plant that has purchase records: the FK is ON DELETE CASCADE, so +// without this check the delete would silently destroy them. +func (d *DB) CountSeedLotsForPlant(ctx context.Context, plantID int64) (int, error) { + var n int + if err := d.sql.QueryRowContext(ctx, + `SELECT COUNT(*) FROM seed_lots WHERE plant_id = ?`, plantID).Scan(&n); err != nil { + return 0, fmt.Errorf("store: count seed lots for plant: %w", err) + } + return n, nil +} + // SeedLotUse is one active planting attributed to a lot, carrying just enough to // compute its effective plant count. type SeedLotUse struct {