From 0793fef17c5cc8b37da5af1dcbd75cab7a9a753e Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 18 Jul 2026 19:47:10 -0400 Subject: [PATCH] Address Gadfly review on #10: clearable color/props, column lists, dedup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes from the PR #28 adversarial review (considered; not graded). Correctness / API - PATCH /objects/:id can now clear nullable color/props back to NULL: the request takes them as json.RawMessage, and ObjectPatch carries an explicit Set flag so an explicit `null` (clear) is distinguished from an absent field (unchanged) — the strongest cross-model finding (6 hits). New test. Maintainability - store/plantings.go + plants.go use explicit qualified column lists (qualifyColumns helper) instead of SELECT *, matching gardens/objects and surviving a future column add. - Consolidated objectKinds + plantableByDefault into one kind→traits map. - objectForRole factors the fetch-then-authorize shared by UpdateObject and DeleteObject; dropped the redundant kind check in CreateObject (finalizeObject is the single validation point). - Request→service mapping via toInput()/toPatch() methods (matches gardens). - Renamed handler gardenFull → getGardenFull (verbNoun); test helper decodeGarden → decodeMap; generic bind-error messages. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi --- internal/api/api.go | 2 +- internal/api/gardens_test.go | 21 +++---- internal/api/objects.go | 103 ++++++++++++++++++++++------------- internal/api/objects_test.go | 48 ++++++++++++++-- internal/service/objects.go | 69 +++++++++++++---------- internal/store/plantings.go | 12 ++-- internal/store/plants.go | 10 ++-- internal/store/sqlite.go | 11 ++++ 8 files changed, 183 insertions(+), 93 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index 32765c7..ff5975c 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -81,7 +81,7 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine { gardens.GET("/:id", h.getGarden) gardens.PATCH("/:id", h.updateGarden) gardens.DELETE("/:id", h.deleteGarden) - gardens.GET("/:id/full", h.gardenFull) // one-shot editor load + gardens.GET("/:id/full", h.getGardenFull) // one-shot editor load gardens.POST("/:id/objects", h.createObject) // Objects are addressed by their own id; the service resolves the owning diff --git a/internal/api/gardens_test.go b/internal/api/gardens_test.go index e78e083..95dcf91 100644 --- a/internal/api/gardens_test.go +++ b/internal/api/gardens_test.go @@ -20,13 +20,14 @@ func registerAndCookie(t *testing.T, r *gin.Engine, email string) *http.Cookie { return sessionCookieFrom(t, w) } -func decodeGarden(t *testing.T, body []byte) map[string]any { +// decodeMap unmarshals any JSON object response into a generic map. +func decodeMap(t *testing.T, body []byte) map[string]any { t.Helper() - var g map[string]any - if err := json.Unmarshal(body, &g); err != nil { - t.Fatalf("decode garden: %v (body %s)", err, body) + var m map[string]any + if err := json.Unmarshal(body, &m); err != nil { + t.Fatalf("decode json object: %v (body %s)", err, body) } - return g + return m } func TestGardenCRUDFlow(t *testing.T) { @@ -38,7 +39,7 @@ func TestGardenCRUDFlow(t *testing.T) { if w.Code != http.StatusCreated { t.Fatalf("create status = %d, body %s", w.Code, w.Body.String()) } - created := decodeGarden(t, w.Body.Bytes()) + created := decodeMap(t, w.Body.Bytes()) id := int64(created["id"].(float64)) if created["widthCm"].(float64) != 1000 || created["unitPref"].(string) != "metric" { t.Errorf("defaults not applied: %+v", created) @@ -69,7 +70,7 @@ func TestGardenCRUDFlow(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("patch status = %d, body %s", w.Code, w.Body.String()) } - patched := decodeGarden(t, w.Body.Bytes()) + patched := decodeMap(t, w.Body.Bytes()) if patched["name"].(string) != "Front" || patched["version"].(float64) != 2 { t.Errorf("patch didn't persist/bump: %+v", patched) } @@ -90,7 +91,7 @@ func TestGardenVersionConflictEnvelope(t *testing.T) { cookie := registerAndCookie(t, r, "a@example.com") w := doJSON(t, r, http.MethodPost, "/api/v1/gardens", map[string]any{"name": "Yard"}, cookie) - id := int64(decodeGarden(t, w.Body.Bytes())["id"].(float64)) + id := int64(decodeMap(t, w.Body.Bytes())["id"].(float64)) body := map[string]any{"name": "Yard2", "widthCm": 1000, "heightCm": 1000, "unitPref": "metric", "version": 1} // First patch at version 1 succeeds (→ version 2). @@ -123,7 +124,7 @@ func TestGardenCrossUserIsNotFound(t *testing.T) { bob := registerAndCookie(t, r, "bob@example.com") w := doJSON(t, r, http.MethodPost, "/api/v1/gardens", map[string]any{"name": "Alice's"}, alice) - id := int64(decodeGarden(t, w.Body.Bytes())["id"].(float64)) + id := int64(decodeMap(t, w.Body.Bytes())["id"].(float64)) // Bob sees a 404 (existence masked), not a 403. if w := doJSON(t, r, http.MethodGet, gardenPath(id), nil, bob); w.Code != http.StatusNotFound { @@ -166,7 +167,7 @@ func TestGardenUpdateRejectsBadVersion(t *testing.T) { r := authEngine(t, localCfg()) cookie := registerAndCookie(t, r, "a@example.com") w := doJSON(t, r, http.MethodPost, "/api/v1/gardens", map[string]any{"name": "Yard"}, cookie) - id := int64(decodeGarden(t, w.Body.Bytes())["id"].(float64)) + id := int64(decodeMap(t, w.Body.Bytes())["id"].(float64)) // A negative (or zero) version is invalid input (400), not a version conflict. for _, v := range []int{0, -1} { diff --git a/internal/api/objects.go b/internal/api/objects.go index f233dd6..641716f 100644 --- a/internal/api/objects.go +++ b/internal/api/objects.go @@ -30,9 +30,19 @@ type objectCreateRequest struct { Notes string `json:"notes"` } +func (r objectCreateRequest) toInput() service.ObjectInput { + return service.ObjectInput{ + Kind: r.Kind, Name: r.Name, Shape: r.Shape, + XCM: r.XCM, YCM: r.YCM, WidthCM: r.WidthCM, HeightCM: r.HeightCM, + RotationDeg: r.RotationDeg, ZIndex: r.ZIndex, Plantable: r.Plantable, + Color: r.Color, Props: propsFromRaw(r.Props), Notes: r.Notes, + } +} + // objectUpdateRequest is the body for PATCH /objects/:id: every field optional // (absent = unchanged), plus the required current version. Kind and shape are -// immutable and not accepted. +// immutable and not accepted. color/props are json.RawMessage so an explicit +// null (clear the override) is distinguishable from an absent field (unchanged). type objectUpdateRequest struct { Name *string `json:"name"` XCM *float64 `json:"xCm"` @@ -42,20 +52,60 @@ type objectUpdateRequest struct { RotationDeg *float64 `json:"rotationDeg"` ZIndex *int `json:"zIndex"` Plantable *bool `json:"plantable"` - Color *string `json:"color"` + Color json.RawMessage `json:"color"` Props json.RawMessage `json:"props"` Notes *string `json:"notes"` Version int64 `json:"version" binding:"required,min=1"` } -// propsFromRaw maps a JSON props value to the stored *string: a present, non-null -// value becomes its JSON text; absent or null becomes nil (leave/none). +func (r objectUpdateRequest) toPatch() (service.ObjectPatch, error) { + color, setColor, err := parseNullableString(r.Color) + if err != nil { + return service.ObjectPatch{}, err + } + props, setProps := parseNullableJSON(r.Props) + return service.ObjectPatch{ + Name: r.Name, XCM: r.XCM, YCM: r.YCM, WidthCM: r.WidthCM, HeightCM: r.HeightCM, + RotationDeg: r.RotationDeg, ZIndex: r.ZIndex, Plantable: r.Plantable, + SetColor: setColor, Color: color, SetProps: setProps, Props: props, Notes: r.Notes, + }, nil +} + +// propsFromRaw maps a JSON props value to the stored *string for create: a +// present, non-null value becomes its JSON text; absent or null becomes nil. func propsFromRaw(raw json.RawMessage) *string { - if len(raw) == 0 || string(raw) == "null" { + v, set := parseNullableJSON(raw) + if !set { return nil } + return v +} + +// parseNullableString decodes a JSON string|null field into (value, present). +// Absent → (nil, false); null → (nil, true); "x" → (&"x", true). A non-string +// value is an error. +func parseNullableString(raw json.RawMessage) (value *string, present bool, err error) { + if len(raw) == 0 { + return nil, false, nil + } + if err := json.Unmarshal(raw, &value); err != nil { + return nil, false, err + } + return value, true, nil +} + +// parseNullableJSON decodes any JSON value into (text, present) for a nullable +// TEXT column. Absent → (nil, false); null → (nil, true); anything else → the +// raw JSON text (&, true). +func parseNullableJSON(raw json.RawMessage) (value *string, present bool) { + if len(raw) == 0 { + return nil, false + } + if string(raw) == "null" { + return nil, true + } s := string(raw) - return &s + return &s, true } func (h *handlers) createObject(c *gin.Context) { @@ -65,24 +115,10 @@ func (h *handlers) createObject(c *gin.Context) { } var req objectCreateRequest if err := c.ShouldBindJSON(&req); err != nil { - writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "kind is required") + writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid object: kind is required") return } - o, err := h.svc.CreateObject(c.Request.Context(), mustActor(c).ID, gardenID, service.ObjectInput{ - Kind: req.Kind, - Name: req.Name, - Shape: req.Shape, - XCM: req.XCM, - YCM: req.YCM, - WidthCM: req.WidthCM, - HeightCM: req.HeightCM, - RotationDeg: req.RotationDeg, - ZIndex: req.ZIndex, - Plantable: req.Plantable, - Color: req.Color, - Props: propsFromRaw(req.Props), - Notes: req.Notes, - }) + o, err := h.svc.CreateObject(c.Request.Context(), mustActor(c).ID, gardenID, req.toInput()) if err != nil { writeServiceError(c, err) return @@ -97,22 +133,15 @@ func (h *handlers) updateObject(c *gin.Context) { } var req objectUpdateRequest if err := c.ShouldBindJSON(&req); err != nil { - writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "a current version is required") + writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid update: a current version is required") return } - o, err := h.svc.UpdateObject(c.Request.Context(), mustActor(c).ID, id, service.ObjectPatch{ - Name: req.Name, - XCM: req.XCM, - YCM: req.YCM, - WidthCM: req.WidthCM, - HeightCM: req.HeightCM, - RotationDeg: req.RotationDeg, - ZIndex: req.ZIndex, - Plantable: req.Plantable, - Color: req.Color, - Props: propsFromRaw(req.Props), - Notes: req.Notes, - }, req.Version) + patch, err := req.toPatch() + if err != nil { + writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid update payload") + return + } + o, err := h.svc.UpdateObject(c.Request.Context(), mustActor(c).ID, id, patch, req.Version) if err != nil { if errors.Is(err, domain.ErrVersionConflict) { writeVersionConflict(c, o) @@ -136,7 +165,7 @@ func (h *handlers) deleteObject(c *gin.Context) { c.Status(http.StatusNoContent) } -func (h *handlers) gardenFull(c *gin.Context) { +func (h *handlers) getGardenFull(c *gin.Context) { gardenID, ok := parseIDParam(c, "id") if !ok { return diff --git a/internal/api/objects_test.go b/internal/api/objects_test.go index 0f8123c..9f2dea9 100644 --- a/internal/api/objects_test.go +++ b/internal/api/objects_test.go @@ -16,7 +16,7 @@ func createGardenAPI(t *testing.T, r *gin.Engine, cookie *http.Cookie, name stri if w.Code != http.StatusCreated { t.Fatalf("create garden: status %d, body %s", w.Code, w.Body.String()) } - return int64(decodeGarden(t, w.Body.Bytes())["id"].(float64)) + return int64(decodeMap(t, w.Body.Bytes())["id"].(float64)) } func objectPath(id int64) string { return "/api/v1/objects/" + strconv.FormatInt(id, 10) } @@ -36,7 +36,7 @@ func TestObjectCRUDAndFull(t *testing.T) { if w.Code != http.StatusCreated { t.Fatalf("create object: status %d, body %s", w.Code, w.Body.String()) } - obj := decodeGarden(t, w.Body.Bytes()) + obj := decodeMap(t, w.Body.Bytes()) oid := int64(obj["id"].(float64)) if obj["plantable"].(bool) != true { t.Error("bed should default plantable=true") @@ -68,7 +68,7 @@ func TestObjectCRUDAndFull(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("patch: status %d, body %s", w.Code, w.Body.String()) } - patched := decodeGarden(t, w.Body.Bytes()) + patched := decodeMap(t, w.Body.Bytes()) if patched["xCm"].(float64) != 500 || patched["version"].(float64) != 2 { t.Errorf("patch didn't apply/bump: %+v", patched) } @@ -90,7 +90,7 @@ func TestObjectVersionConflict(t *testing.T) { gid := createGardenAPI(t, r, cookie, "Yard") w := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{"kind": "bed", "xCm": 300, "yCm": 300, "widthCm": 100, "heightCm": 100}, cookie) - oid := int64(decodeGarden(t, w.Body.Bytes())["id"].(float64)) + oid := int64(decodeMap(t, w.Body.Bytes())["id"].(float64)) body := map[string]any{"xCm": 400, "version": 1} if w := doJSON(t, r, http.MethodPatch, objectPath(oid), body, cookie); w.Code != http.StatusOK { @@ -117,7 +117,7 @@ func TestObjectCrossUserIsNotFound(t *testing.T) { gid := createGardenAPI(t, r, alice, "Alice's") w := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{"kind": "bed", "xCm": 300, "yCm": 300, "widthCm": 100, "heightCm": 100}, alice) - oid := int64(decodeGarden(t, w.Body.Bytes())["id"].(float64)) + oid := int64(decodeMap(t, w.Body.Bytes())["id"].(float64)) // Bob can't see the garden or its objects — all 404. if w := doJSON(t, r, http.MethodGet, fullPath(gid), nil, bob); w.Code != http.StatusNotFound { @@ -134,6 +134,42 @@ func TestObjectCrossUserIsNotFound(t *testing.T) { } } +func TestObjectPatchClearsColorAndProps(t *testing.T) { + r := authEngine(t, localCfg()) + cookie := registerAndCookie(t, r, "a@example.com") + gid := createGardenAPI(t, r, cookie, "Yard") + + // Create with a color override and props. + w := doJSON(t, r, http.MethodPost, objectsPath(gid), + map[string]any{"kind": "bed", "xCm": 300, "yCm": 300, "widthCm": 100, "heightCm": 100, + "color": "#3f8f4f", "props": map[string]any{"heightCm": 40}}, cookie) + obj := decodeMap(t, w.Body.Bytes()) + oid := int64(obj["id"].(float64)) + if obj["color"].(string) != "#3f8f4f" { + t.Fatalf("color not set on create: %v", obj["color"]) + } + + // PATCH color:null props:null clears both back to absent (omitempty). + w = doJSON(t, r, http.MethodPatch, objectPath(oid), + map[string]any{"color": nil, "props": nil, "version": 1}, cookie) + if w.Code != http.StatusOK { + t.Fatalf("clear patch: status %d, body %s", w.Code, w.Body.String()) + } + cleared := decodeMap(t, w.Body.Bytes()) + if _, present := cleared["color"]; present { + t.Errorf("color should be cleared (absent), got %v", cleared["color"]) + } + if _, present := cleared["props"]; present { + t.Errorf("props should be cleared (absent), got %v", cleared["props"]) + } + + // A patch that omits color leaves it unchanged (still absent here). + w = doJSON(t, r, http.MethodPatch, objectPath(oid), map[string]any{"xCm": 350, "version": 2}, cookie) + if w.Code != http.StatusOK { + t.Fatalf("move patch: %d", w.Code) + } +} + func TestObjectValidationRejects(t *testing.T) { r := authEngine(t, localCfg()) cookie := registerAndCookie(t, r, "a@example.com") @@ -154,7 +190,7 @@ func TestObjectValidationRejects(t *testing.T) { if w.Code != http.StatusCreated { t.Fatalf("props create = %d, body %s", w.Code, w.Body.String()) } - if props, _ := decodeGarden(t, w.Body.Bytes())["props"].(string); props == "" { + if props, _ := decodeMap(t, w.Body.Bytes())["props"].(string); props == "" { t.Error("props should be stored and returned as a JSON string") } } diff --git a/internal/service/objects.go b/internal/service/objects.go index a882211..befa558 100644 --- a/internal/service/objects.go +++ b/internal/service/objects.go @@ -15,17 +15,17 @@ const ( maxObjectPropsLen = 20_000 ) -// objectKinds is the set of valid garden_objects.kind values. -var objectKinds = map[string]bool{ - domain.KindBed: true, domain.KindGrowBag: true, domain.KindContainer: true, - domain.KindInGround: true, domain.KindTree: true, domain.KindPath: true, domain.KindStructure: true, -} - -// plantableByDefault: beds/bags/containers/in-ground hold plants; trees, paths, -// and structures don't. Overridable per object. -var plantableByDefault = map[string]bool{ - domain.KindBed: true, domain.KindGrowBag: true, domain.KindContainer: true, domain.KindInGround: true, - domain.KindTree: false, domain.KindPath: false, domain.KindStructure: false, +// objectKinds maps each valid garden_objects.kind to its traits. plantable is +// the default (beds/bags/containers/in-ground hold plants; trees/paths/ +// structures don't) and is overridable per object. +var objectKinds = map[string]struct{ plantable bool }{ + domain.KindBed: {plantable: true}, + domain.KindGrowBag: {plantable: true}, + domain.KindContainer: {plantable: true}, + domain.KindInGround: {plantable: true}, + domain.KindTree: {plantable: false}, + domain.KindPath: {plantable: false}, + domain.KindStructure: {plantable: false}, } // ObjectInput is the payload for creating a garden object. Plantable nil means @@ -48,6 +48,10 @@ type ObjectInput struct { // ObjectPatch is a partial update: every field is optional (nil = leave as-is). // Kind and shape are intentionally immutable after creation. +// +// Color and Props are nullable in the DB, so they use an explicit "set" flag to +// tell "clear to NULL" (Set*=true, value nil) apart from "leave unchanged" +// (Set*=false) — a plain nil pointer can't express both. type ObjectPatch struct { Name *string XCM *float64 @@ -57,7 +61,9 @@ type ObjectPatch struct { RotationDeg *float64 ZIndex *int Plantable *bool + SetColor bool Color *string + SetProps bool Props *string Notes *string } @@ -79,14 +85,13 @@ func (s *Service) CreateObject(ctx context.Context, actorID, gardenID int64, in return nil, err } - if !objectKinds[in.Kind] { - return nil, domain.ErrInvalidInput - } + // finalizeObject is the single validation point (it rejects an unknown kind), + // so an unknown kind here just yields plantable=false before being rejected. shape := in.Shape if shape == "" { shape = domain.ShapeRect } - plantable := plantableByDefault[in.Kind] + plantable := objectKinds[in.Kind].plantable if in.Plantable != nil { plantable = *in.Plantable } @@ -113,14 +118,25 @@ func (s *Service) CreateObject(ctx context.Context, actorID, gardenID int64, in return s.store.CreateObject(ctx, o) } +// objectForRole loads an object and enforces that the actor holds at least min +// on its owning garden, returning both. A missing object — or one in a garden +// the actor can't see — is ErrNotFound (existence masked, same as gardens). +func (s *Service) objectForRole(ctx context.Context, actorID, objectID int64, min gardenRole) (*domain.GardenObject, *domain.Garden, error) { + o, err := s.store.GetObject(ctx, objectID) + if err != nil { + return nil, nil, err // ErrNotFound + } + g, err := s.requireGardenRole(ctx, actorID, o.GardenID, min) + if err != nil { + return nil, nil, err + } + return o, g, nil +} + // UpdateObject applies a partial, version-guarded patch to an object in a garden // the actor can edit. On a version mismatch it returns (current, ErrVersionConflict). func (s *Service) UpdateObject(ctx context.Context, actorID, objectID int64, patch ObjectPatch, version int64) (*domain.GardenObject, error) { - o, err := s.store.GetObject(ctx, objectID) - if err != nil { - return nil, err // ErrNotFound - } - g, err := s.requireGardenRole(ctx, actorID, o.GardenID, roleEditor) + o, g, err := s.objectForRole(ctx, actorID, objectID, roleEditor) if err != nil { return nil, err } @@ -136,11 +152,7 @@ func (s *Service) UpdateObject(ctx context.Context, actorID, objectID int64, pat // DeleteObject removes an object (its plantings cascade) from a garden the actor // can edit. func (s *Service) DeleteObject(ctx context.Context, actorID, objectID int64) error { - o, err := s.store.GetObject(ctx, objectID) - if err != nil { - return err - } - if _, err := s.requireGardenRole(ctx, actorID, o.GardenID, roleEditor); err != nil { + if _, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor); err != nil { return err } return s.store.DeleteObject(ctx, objectID) @@ -193,10 +205,11 @@ func applyObjectPatch(o *domain.GardenObject, p ObjectPatch) { if p.Plantable != nil { o.Plantable = *p.Plantable } - if p.Color != nil { + // Color/Props: Set* distinguishes "clear to NULL" (value nil) from unchanged. + if p.SetColor { o.Color = p.Color } - if p.Props != nil { + if p.SetProps { o.Props = p.Props } if p.Notes != nil { @@ -208,7 +221,7 @@ func applyObjectPatch(o *domain.GardenObject, p ObjectPatch) { // object against its garden. Shared by create and update so both enforce the // same invariants. func finalizeObject(o *domain.GardenObject, g *domain.Garden) error { - if !objectKinds[o.Kind] { + if _, ok := objectKinds[o.Kind]; !ok { return domain.ErrInvalidInput } // rect/circle only; polygon is reserved in the schema but not supported yet. diff --git a/internal/store/plantings.go b/internal/store/plantings.go index abbf6a6..29dbd94 100644 --- a/internal/store/plantings.go +++ b/internal/store/plantings.go @@ -7,9 +7,11 @@ import ( "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" ) -// scanPlanting reads a plantings row in table-declared column order (id, -// object_id, plant_id, x_cm, y_cm, radius_cm, count, label, planted_at, -// removed_at, version, created_at, updated_at) — the order SELECT pl.* yields. +// plantingColumns lists plantings columns in the order scanPlanting expects. +// Used unqualified for direct selects; the /full read below qualifies with pl. +const plantingColumns = `id, object_id, plant_id, x_cm, y_cm, radius_cm, count, label, + planted_at, removed_at, version, created_at, updated_at` + func scanPlanting(s scanner) (*domain.Planting, error) { var p domain.Planting if err := s.Scan( @@ -27,10 +29,8 @@ func scanPlanting(s scanner) (*domain.Planting, error) { // non-nil slice (empty until plantings CRUD lands in #14). Plop CRUD itself is // #14; this is only the /full read side. func (d *DB) ListActivePlantingsForGarden(ctx context.Context, gardenID int64) ([]domain.Planting, error) { - // pl.* returns the plantings columns in table-declared order, which matches - // plantingColumns / scanPlanting. rows, err := d.sql.QueryContext(ctx, - `SELECT pl.* FROM plantings pl + `SELECT `+qualifyColumns("pl", plantingColumns)+` FROM plantings pl JOIN garden_objects o ON o.id = pl.object_id WHERE o.garden_id = ? AND pl.removed_at IS NULL ORDER BY pl.id`, diff --git a/internal/store/plants.go b/internal/store/plants.go index cd445d8..bf881d2 100644 --- a/internal/store/plants.go +++ b/internal/store/plants.go @@ -7,9 +7,10 @@ import ( "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" ) -// scanPlant reads a plants row in table-declared column order (id, owner_id, -// name, category, spacing_cm, color, icon, days_to_maturity, notes, version, -// created_at, updated_at) — the order SELECT p.* yields. +// plantColumns lists plants columns in the order scanPlant expects. +const plantColumns = `id, owner_id, name, category, spacing_cm, color, icon, + days_to_maturity, notes, version, created_at, updated_at` + func scanPlant(s scanner) (*domain.Plant, error) { var p domain.Plant if err := s.Scan( @@ -26,9 +27,8 @@ func scanPlant(s scanner) (*domain.Plant, error) { // non-nil slice (empty until plantings exist, #14). Full plant CRUD/seeding is // #12; this is only the /full read side. func (d *DB) ListReferencedPlants(ctx context.Context, gardenID int64) ([]domain.Plant, error) { - // p.* returns the plants columns in table-declared order, matching scanPlant. rows, err := d.sql.QueryContext(ctx, - `SELECT DISTINCT p.* FROM plants p + `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 = ? AND pl.removed_at IS NULL diff --git a/internal/store/sqlite.go b/internal/store/sqlite.go index ef76b1f..ce79897 100644 --- a/internal/store/sqlite.go +++ b/internal/store/sqlite.go @@ -99,6 +99,17 @@ func pragmaName(p string) string { return strings.ToLower(strings.TrimSpace(p)) } +// qualifyColumns prefixes each comma-separated column in cols with "alias." so a +// shared column list can be used in a JOIN — e.g. qualifyColumns("pl", "id, x") +// → "pl.id, pl.x". Whitespace/newlines in the list are trimmed. +func qualifyColumns(alias, cols string) string { + parts := strings.Split(cols, ",") + for i, p := range parts { + parts[i] = alias + "." + strings.TrimSpace(p) + } + return strings.Join(parts, ", ") +} + // Close closes the underlying database. func (d *DB) Close() error { return d.sql.Close() }