ListReferencedPlants(includeRemoved=true) returned every plant the garden has ever held, so drawing one season loaded a decade of catalog on a decade-old garden. It also meant the plant list and the plop list were selected by different rules, which is the kind of near-miss that stays correct only by accident. It now takes the same year the plops do, so the two selections always agree. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
This commit is contained in:
@@ -271,7 +271,7 @@ func (s *Service) assembleFullFor(ctx context.Context, g *domain.Garden, year *i
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
plants, err := s.store.ListReferencedPlants(ctx, g.ID, year != nil)
|
||||
plants, err := s.store.ListReferencedPlants(ctx, g.ID, year)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -432,3 +432,59 @@ func TestSeasonYearIsBounded(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeasonPlantsAreScopedToTheYear — the plant list has to match the plops it
|
||||
// is there to render. A garden with a decade of history shouldn't load a decade
|
||||
// of catalog to draw one season, and a season shouldn't carry plants that had
|
||||
// nothing in the ground that year.
|
||||
func TestSeasonPlantsAreScopedToTheYear(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)
|
||||
ctx := context.Background()
|
||||
|
||||
oldPlant := seedOwnPlant(t, s, owner, 15)
|
||||
newPlant, err := s.CreatePlant(ctx, owner, PlantInput{
|
||||
Name: "Later", Category: domain.CategoryHerb, SpacingCM: 20, Color: "#4a7c3f", Icon: "🌱",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreatePlant: %v", err)
|
||||
}
|
||||
|
||||
plant2020, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{
|
||||
PlantID: oldPlant.ID, XCM: -40, YCM: 0, RadiusCM: 20, PlantedAt: strPtr("2020-04-01"),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreatePlanting: %v", err)
|
||||
}
|
||||
if _, err := s.UpdatePlanting(ctx, owner, plant2020.ID, PlantingPatch{
|
||||
SetRemovedAt: true, RemovedAt: strPtr("2020-09-01"),
|
||||
}, plant2020.Version); err != nil {
|
||||
t.Fatalf("remove: %v", err)
|
||||
}
|
||||
if _, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{
|
||||
PlantID: newPlant.ID, XCM: 40, YCM: 0, RadiusCM: 20, PlantedAt: strPtr("2026-04-01"),
|
||||
}); err != nil {
|
||||
t.Fatalf("CreatePlanting: %v", err)
|
||||
}
|
||||
|
||||
names := func(year int) []string {
|
||||
t.Helper()
|
||||
full, err := s.GardenFull(ctx, owner, g.ID, &year)
|
||||
if err != nil {
|
||||
t.Fatalf("GardenFull(%d): %v", year, err)
|
||||
}
|
||||
out := []string{}
|
||||
for _, p := range full.Plants {
|
||||
out = append(out, p.Name)
|
||||
}
|
||||
return out
|
||||
}
|
||||
if got := names(2020); len(got) != 1 || got[0] != oldPlant.Name {
|
||||
t.Errorf("2020 plants = %v, want just %q", got, oldPlant.Name)
|
||||
}
|
||||
if got := names(2026); len(got) != 1 || got[0] != "Later" {
|
||||
t.Errorf("2026 plants = %v, want just \"Later\"", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,21 +27,27 @@ func scanPlant(s scanner) (*domain.Plant, error) {
|
||||
// ListReferencedPlants returns the distinct plants used by a garden's plantings
|
||||
// — the catalog subset the editor needs to render them. Always a non-nil slice.
|
||||
//
|
||||
// includeRemoved widens it past the active plops to every plop the garden has
|
||||
// ever held, which is what a past season needs: a plant pulled last July is not
|
||||
// active, but its plops still have to render with the right icon and color.
|
||||
func (d *DB) ListReferencedPlants(ctx context.Context, gardenID int64, includeRemoved bool) ([]domain.Plant, error) {
|
||||
activeOnly := ` AND pl.removed_at IS NULL`
|
||||
if includeRemoved {
|
||||
activeOnly = ``
|
||||
// year nil means the active plops only. A year widens it to the plops that were
|
||||
// in the ground THAT YEAR, which is what a past season needs: a plant pulled
|
||||
// last July is not active, but its plops still have to render with the right
|
||||
// icon and colour. It is scoped to the same year as the plops rather than to the
|
||||
// garden's whole history, so the two selections always agree and a decade-old
|
||||
// garden doesn't load a decade of catalog to draw one season.
|
||||
func (d *DB) ListReferencedPlants(ctx context.Context, gardenID int64, year *int) ([]domain.Plant, error) {
|
||||
where := ` AND pl.removed_at IS NULL`
|
||||
args := []any{gardenID}
|
||||
if year != nil {
|
||||
where = ` AND (pl.planted_at IS NULL OR pl.planted_at <= ?)
|
||||
AND (pl.removed_at IS NULL OR pl.removed_at >= ?)`
|
||||
args = append(args, fmt.Sprintf("%04d-12-31", *year), fmt.Sprintf("%04d-01-01", *year))
|
||||
}
|
||||
rows, err := d.sql.QueryContext(ctx,
|
||||
`SELECT DISTINCT `+qualifyColumns("p", plantColumns)+` FROM plants p
|
||||
JOIN plantings pl ON pl.plant_id = p.id
|
||||
JOIN garden_objects o ON o.id = pl.object_id
|
||||
WHERE o.garden_id = ?`+activeOnly+`
|
||||
WHERE o.garden_id = ?`+where+`
|
||||
ORDER BY p.id`,
|
||||
gardenID,
|
||||
args...,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: list referenced plants: %w", err)
|
||||
|
||||
Reference in New Issue
Block a user