Garden objects backend: polymorphic CRUD + /full endpoint (#10) #28
@@ -81,6 +81,14 @@ 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.getGardenFull) // one-shot editor load
|
||||
|
|
||||
gardens.POST("/:id/objects", h.createObject)
|
||||
|
||||
// Objects are addressed by their own id; the service resolves the owning
|
||||
// garden for the permission check.
|
||||
objects := v1.Group("/objects", h.requireAuth())
|
||||
objects.PATCH("/:id", h.updateObject)
|
||||
objects.DELETE("/:id", h.deleteObject)
|
||||
|
||||
return r
|
||||
}
|
||||
|
||||
@@ -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, "[email protected]")
|
||||
|
||||
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, "[email protected]")
|
||||
|
||||
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, "[email protected]")
|
||||
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} {
|
||||
|
||||
@@ -0,0 +1,179 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/service"
|
||||
)
|
||||
|
||||
// objectCreateRequest is the body for POST /gardens/:id/objects. Dimensions are
|
||||
// centimeters; the object is positioned by its center. Plantable/color/props are
|
||||
// optional (plantable defaults by kind). props is any JSON value, stored as-is.
|
||||
type objectCreateRequest struct {
|
||||
Kind string `json:"kind" binding:"required"`
|
||||
Name string `json:"name"`
|
||||
Shape string `json:"shape"`
|
||||
XCM float64 `json:"xCm"`
|
||||
YCM float64 `json:"yCm"`
|
||||
WidthCM float64 `json:"widthCm"`
|
||||
HeightCM float64 `json:"heightCm"`
|
||||
RotationDeg float64 `json:"rotationDeg"`
|
||||
ZIndex int `json:"zIndex"`
|
||||
Plantable *bool `json:"plantable"`
|
||||
Color *string `json:"color"`
|
||||
Props json.RawMessage `json:"props"`
|
||||
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,
|
||||
|
gitea-actions
commented
🟠 PATCH cannot clear color or props: null and absent both map to nil (leave-unchanged), so once set these fields are unremovable correctness · flagged by 1 model
🪰 Gadfly · advisory 🟠 **PATCH cannot clear color or props: null and absent both map to nil (leave-unchanged), so once set these fields are unremovable**
_correctness · flagged by 1 model_
- **`internal/api/objects.go:37-39` + `internal/service/objects.go:196-201` — PATCH cannot clear `color` or `props`.** `Color *string` uses pointer-with-nil-means-absent semantics (JSON `null` unmarshals to a nil pointer), and `propsFromRaw` (objects.go:53-59) deliberately maps both *absent* and JSON `null` to `nil`. In `applyObjectPatch`, `nil` means "leave unchanged" (objects.go:196-201). Net effect: once a `color` or `props` value is set on an object, no PATCH can ever remove it — `"color": n…
<sub>🪰 Gadfly · advisory</sub>
|
||||
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. color/props are json.RawMessage so an explicit
|
||||
// null (clear the override) is distinguishable from an absent field (unchanged).
|
||||
|
gitea-actions
commented
🟡 PATCH /objects/:id cannot clear nullable color/props back to NULL (absent vs explicit null indistinguishable) error-handling · flagged by 1 model Finding confirmed at 🪰 Gadfly · advisory 🟡 **PATCH /objects/:id cannot clear nullable color/props back to NULL (absent vs explicit null indistinguishable)**
_error-handling · flagged by 1 model_
Finding confirmed at `internal/api/objects.go:45` (Color) and `internal/api/objects.go:53-59` / `internal/service/objects.go:196-201` (applyObjectPatch). Verification complete — the single finding in the draft holds up against the actual code.
<sub>🪰 Gadfly · advisory</sub>
|
||||
type objectUpdateRequest struct {
|
||||
Name *string `json:"name"`
|
||||
XCM *float64 `json:"xCm"`
|
||||
YCM *float64 `json:"yCm"`
|
||||
WidthCM *float64 `json:"widthCm"`
|
||||
HeightCM *float64 `json:"heightCm"`
|
||||
RotationDeg *float64 `json:"rotationDeg"`
|
||||
ZIndex *int `json:"zIndex"`
|
||||
Plantable *bool `json:"plantable"`
|
||||
Color json.RawMessage `json:"color"`
|
||||
Props json.RawMessage `json:"props"`
|
||||
Notes *string `json:"notes"`
|
||||
Version int64 `json:"version" binding:"required,min=1"`
|
||||
}
|
||||
|
||||
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,
|
||||
|
gitea-actions
commented
🟡 Object create/update request structs are duplicated into service structs inline instead of using the established toInput()-method convention error-handling, maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟡 **Object create/update request structs are duplicated into service structs inline instead of using the established toInput()-method convention**
_error-handling, maintainability · flagged by 2 models_
- `internal/api/objects.go:71-85` and `internal/api/objects.go:103-115` — `objectCreateRequest`/`objectUpdateRequest` duplicate the same ~11 field names three times (request struct → inline `service.ObjectInput{}` literal → inline `service.ObjectPatch{}` literal) instead of following the `toInput()` conversion-method convention this codebase already established in `internal/api/gardens.go:24-26` (`gardenFields.toInput()`). Adding `(req objectCreateRequest) toInput() service.ObjectInput` and `(re…
<sub>🪰 Gadfly · advisory</sub>
|
||||
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 {
|
||||
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) {
|
||||
|
gitea-actions
commented
🟡 Bind-error message hardwired to 'a current version is required' even when the failure is a different field or malformed JSON error-handling, maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **Bind-error message hardwired to 'a current version is required' even when the failure is a different field or malformed JSON**
_error-handling, maintainability · flagged by 1 model_
- **`internal/api/objects.go:100` — misleading bind-error message.** `c.ShouldBindJSON(&req)` can fail for many reasons (malformed JSON, type error on `zIndex`/`widthCm`/etc.), but the message is hardwired to `"a current version is required"`. The handler also re-validates `version` via `binding:"required,min=1"`, so the message is sometimes right, but a malformed `zIndex` produces a 400 claiming the problem is the missing version. Compare `gardens.go:84` (`"name and a current version are requir…
<sub>🪰 Gadfly · advisory</sub>
|
||||
if len(raw) == 0 {
|
||||
return nil, false
|
||||
}
|
||||
if string(raw) == "null" {
|
||||
return nil, true
|
||||
}
|
||||
s := string(raw)
|
||||
return &s, true
|
||||
}
|
||||
|
||||
func (h *handlers) createObject(c *gin.Context) {
|
||||
gardenID, ok := parseIDParam(c, "id")
|
||||
if !ok {
|
||||
|
gitea-actions
commented
🟡 *PATCH cannot clear color/props: string nil means 'unchanged', so explicit null can't reset to NULL correctness · flagged by 1 model
🪰 Gadfly · advisory 🟡 **PATCH cannot clear color/props: *string nil means 'unchanged', so explicit null can't reset to NULL**
_correctness · flagged by 1 model_
- `internal/api/objects.go:113` (with `internal/service/objects.go:196-201`) — PATCH cannot clear `color` (or `props`). For `color`, a `*string` field with `applyObjectPatch`'s `if p.Color != nil` guard means a JSON `null` (→ nil pointer) is treated as "leave unchanged"; sending `""` is rejected by `isHexColor` (length ≠ 4/7), so there's no way to reset color to NULL. The same nil-means-unchanged pattern applies to `props` (via `propsFromRaw`, which is documented as intentional); `color` has the…
<sub>🪰 Gadfly · advisory</sub>
|
||||
return
|
||||
}
|
||||
var req objectCreateRequest
|
||||
if err := c.ShouldBindJSON(&req); err != nil {
|
||||
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid object: kind is required")
|
||||
return
|
||||
}
|
||||
o, err := h.svc.CreateObject(c.Request.Context(), mustActor(c).ID, gardenID, req.toInput())
|
||||
if err != nil {
|
||||
writeServiceError(c, err)
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusCreated, o)
|
||||
}
|
||||
|
||||
func (h *handlers) updateObject(c *gin.Context) {
|
||||
id, ok := parseIDParam(c, "id")
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
var req objectUpdateRequest
|
||||
if err := c.ShouldBindJSON(&req); err != nil {
|
||||
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid update: a current version is required")
|
||||
return
|
||||
}
|
||||
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)
|
||||
return
|
||||
}
|
||||
writeServiceError(c, err)
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, o)
|
||||
}
|
||||
|
||||
func (h *handlers) deleteObject(c *gin.Context) {
|
||||
id, ok := parseIDParam(c, "id")
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
if err := h.svc.DeleteObject(c.Request.Context(), mustActor(c).ID, id); err != nil {
|
||||
writeServiceError(c, err)
|
||||
return
|
||||
}
|
||||
c.Status(http.StatusNoContent)
|
||||
}
|
||||
|
||||
func (h *handlers) getGardenFull(c *gin.Context) {
|
||||
gardenID, ok := parseIDParam(c, "id")
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
full, err := h.svc.GardenFull(c.Request.Context(), mustActor(c).ID, gardenID)
|
||||
if err != nil {
|
||||
writeServiceError(c, err)
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, full)
|
||||
}
|
||||
@@ -0,0 +1,196 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// createGardenAPI creates a garden via the API and returns its id.
|
||||
func createGardenAPI(t *testing.T, r *gin.Engine, cookie *http.Cookie, name string) int64 {
|
||||
t.Helper()
|
||||
w := doJSON(t, r, http.MethodPost, "/api/v1/gardens", map[string]any{"name": name}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("create garden: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
return int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
|
||||
|
gitea-actions
commented
🟡 Test helper decodeGarden misnamed when used for objects maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **Test helper decodeGarden misnamed when used for objects**
_maintainability · flagged by 1 model_
- `internal/api/objects_test.go:19,39,71,93,120,157` reuses `decodeGarden` (defined in `gardens_test.go`) to decode object responses. The helper just unmarshals JSON into a `map[string]any`, but its name says "Garden", which is misleading when reading object tests. **Fix:** rename the helper to a generic name (e.g. `decodeBody`) in the shared test file, or add a local alias `decodeObject = decodeGarden` so the call sites read honestly.
<sub>🪰 Gadfly · advisory</sub>
|
||||
}
|
||||
|
||||
func objectPath(id int64) string { return "/api/v1/objects/" + strconv.FormatInt(id, 10) }
|
||||
func fullPath(id int64) string { return "/api/v1/gardens/" + strconv.FormatInt(id, 10) + "/full" }
|
||||
func objectsPath(id int64) string {
|
||||
return "/api/v1/gardens/" + strconv.FormatInt(id, 10) + "/objects"
|
||||
}
|
||||
|
||||
func TestObjectCRUDAndFull(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
gid := createGardenAPI(t, r, cookie, "Yard")
|
||||
|
||||
// Create a bed.
|
||||
w := doJSON(t, r, http.MethodPost, objectsPath(gid),
|
||||
map[string]any{"kind": "bed", "name": "Bed 1", "xCm": 300, "yCm": 300, "widthCm": 200, "heightCm": 100}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("create object: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
obj := decodeMap(t, w.Body.Bytes())
|
||||
oid := int64(obj["id"].(float64))
|
||||
|
gitea-actions
commented
🟡 decodeGarden helper reused to decode object payloads; misleading name for non-garden responses maintainability · flagged by 1 model 🪰 Gadfly · advisory 🟡 **decodeGarden helper reused to decode object payloads; misleading name for non-garden responses**
_maintainability · flagged by 1 model_
<sub>🪰 Gadfly · advisory</sub>
|
||||
if obj["plantable"].(bool) != true {
|
||||
t.Error("bed should default plantable=true")
|
||||
}
|
||||
|
||||
// /full includes it, with empty plantings/plants.
|
||||
w = doJSON(t, r, http.MethodGet, fullPath(gid), nil, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("full: status %d", w.Code)
|
||||
}
|
||||
var full struct {
|
||||
Garden map[string]any `json:"garden"`
|
||||
Objects []map[string]any `json:"objects"`
|
||||
Plantings []map[string]any `json:"plantings"`
|
||||
Plants []map[string]any `json:"plants"`
|
||||
}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &full); err != nil {
|
||||
t.Fatalf("decode full: %v (body %s)", err, w.Body.String())
|
||||
}
|
||||
if full.Garden == nil || len(full.Objects) != 1 {
|
||||
t.Errorf("full shape wrong: %+v", full)
|
||||
}
|
||||
if full.Plantings == nil || full.Plants == nil || len(full.Plantings) != 0 || len(full.Plants) != 0 {
|
||||
t.Errorf("plantings/plants should be present + empty: %+v %+v", full.Plantings, full.Plants)
|
||||
}
|
||||
|
||||
// Move it (PATCH with version).
|
||||
w = doJSON(t, r, http.MethodPatch, objectPath(oid), map[string]any{"xCm": 500, "yCm": 500, "version": 1}, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("patch: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
patched := decodeMap(t, w.Body.Bytes())
|
||||
if patched["xCm"].(float64) != 500 || patched["version"].(float64) != 2 {
|
||||
t.Errorf("patch didn't apply/bump: %+v", patched)
|
||||
}
|
||||
|
||||
// Delete → 204, then /full has none.
|
||||
if w := doJSON(t, r, http.MethodDelete, objectPath(oid), nil, cookie); w.Code != http.StatusNoContent {
|
||||
t.Fatalf("delete: status %d", w.Code)
|
||||
}
|
||||
w = doJSON(t, r, http.MethodGet, fullPath(gid), nil, cookie)
|
||||
json.Unmarshal(w.Body.Bytes(), &full)
|
||||
if len(full.Objects) != 0 {
|
||||
t.Errorf("object still in /full after delete: %d", len(full.Objects))
|
||||
}
|
||||
}
|
||||
|
||||
func TestObjectVersionConflict(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
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(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 {
|
||||
t.Fatalf("first patch: %d", w.Code)
|
||||
}
|
||||
w = doJSON(t, r, http.MethodPatch, objectPath(oid), body, cookie) // stale version 1
|
||||
if w.Code != http.StatusConflict {
|
||||
t.Fatalf("stale patch: status %d, want 409", w.Code)
|
||||
}
|
||||
var env struct {
|
||||
Error struct{ Code string } `json:"error"`
|
||||
Current map[string]any `json:"current"`
|
||||
}
|
||||
json.Unmarshal(w.Body.Bytes(), &env)
|
||||
if env.Error.Code != "VERSION_CONFLICT" || env.Current["version"].(float64) != 2 {
|
||||
t.Errorf("conflict envelope wrong: %+v", env)
|
||||
}
|
||||
}
|
||||
|
||||
func TestObjectCrossUserIsNotFound(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
alice := registerAndCookie(t, r, "[email protected]")
|
||||
bob := registerAndCookie(t, r, "[email protected]")
|
||||
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(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 {
|
||||
t.Errorf("bob full = %d, want 404", w.Code)
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{"kind": "bed", "xCm": 1, "yCm": 1, "widthCm": 10, "heightCm": 10}, bob); w.Code != http.StatusNotFound {
|
||||
t.Errorf("bob create object = %d, want 404", w.Code)
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodPatch, objectPath(oid), map[string]any{"xCm": 1, "version": 1}, bob); w.Code != http.StatusNotFound {
|
||||
t.Errorf("bob patch = %d, want 404", w.Code)
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodDelete, objectPath(oid), nil, bob); w.Code != http.StatusNotFound {
|
||||
t.Errorf("bob delete = %d, want 404", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestObjectPatchClearsColorAndProps(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
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, "[email protected]")
|
||||
gid := createGardenAPI(t, r, cookie, "Yard")
|
||||
|
||||
// Polygon shape is reserved → 400.
|
||||
if w := doJSON(t, r, http.MethodPost, objectsPath(gid),
|
||||
map[string]any{"kind": "bed", "shape": "polygon", "xCm": 300, "yCm": 300, "widthCm": 100, "heightCm": 100}, cookie); w.Code != http.StatusBadRequest {
|
||||
t.Errorf("polygon create = %d, want 400", w.Code)
|
||||
}
|
||||
// Missing kind → 400.
|
||||
if w := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{"xCm": 1, "yCm": 1, "widthCm": 10, "heightCm": 10}, cookie); w.Code != http.StatusBadRequest {
|
||||
t.Errorf("no-kind create = %d, want 400", w.Code)
|
||||
}
|
||||
// props accepts a JSON object and round-trips.
|
||||
w := doJSON(t, r, http.MethodPost, objectsPath(gid),
|
||||
map[string]any{"kind": "tree", "xCm": 300, "yCm": 300, "widthCm": 100, "heightCm": 100, "props": map[string]any{"species": "oak"}}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("props create = %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if props, _ := decodeMap(t, w.Body.Bytes())["props"].(string); props == "" {
|
||||
t.Error("props should be stored and returned as a JSON string")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,293 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"math"
|
||||
"strings"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
)
|
||||
|
||||
const (
|
||||
maxObjectNameLen = 200
|
||||
maxObjectNotesLen = 10_000
|
||||
maxObjectPropsLen = 20_000
|
||||
)
|
||||
|
||||
// objectKinds maps each valid garden_objects.kind to its traits. plantable is
|
||||
|
gitea-actions
commented
🟠 Parallel maps objectKinds and plantableByDefault are a maintenance hazard maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟠 **Parallel maps objectKinds and plantableByDefault are a maintenance hazard**
_maintainability · flagged by 2 models_
- **`internal/service/objects.go:19`** — `objectKinds` and `plantableByDefault` are parallel maps that must be kept in sync. Adding a new kind to `objectKinds` without also adding it to `plantableByDefault` silently changes behavior (zero-value `false` for plantable). Suggested fix: combine into a single source-of-truth map, e.g. `map[string]struct{ PlantableDefault bool }`, so kind validity and its default are declared once.
<sub>🪰 Gadfly · advisory</sub>
|
||||
// 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
|
||||
// "default for this kind".
|
||||
type ObjectInput struct {
|
||||
Kind string
|
||||
Name string
|
||||
Shape string
|
||||
XCM float64
|
||||
YCM float64
|
||||
WidthCM float64
|
||||
HeightCM float64
|
||||
RotationDeg float64
|
||||
ZIndex int
|
||||
Plantable *bool
|
||||
Color *string
|
||||
Props *string
|
||||
Notes string
|
||||
}
|
||||
|
||||
// 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
|
||||
YCM *float64
|
||||
WidthCM *float64
|
||||
HeightCM *float64
|
||||
RotationDeg *float64
|
||||
ZIndex *int
|
||||
Plantable *bool
|
||||
SetColor bool
|
||||
Color *string
|
||||
SetProps bool
|
||||
Props *string
|
||||
Notes *string
|
||||
}
|
||||
|
||||
// FullGarden is the one-shot editor payload: the garden plus everything drawn in
|
||||
// it. Plantings/Plants stay empty until #14 populates plantings; the shape is
|
||||
// fixed now so the frontend types against it once.
|
||||
type FullGarden struct {
|
||||
Garden *domain.Garden `json:"garden"`
|
||||
Objects []domain.GardenObject `json:"objects"`
|
||||
Plantings []domain.Planting `json:"plantings"`
|
||||
Plants []domain.Plant `json:"plants"`
|
||||
}
|
||||
|
||||
// CreateObject adds an object to a garden the actor can edit.
|
||||
func (s *Service) CreateObject(ctx context.Context, actorID, gardenID int64, in ObjectInput) (*domain.GardenObject, error) {
|
||||
|
gitea-actions
commented
🟡 Redundant objectKinds check in CreateObject; finalizeObject already enforces the same invariant (UpdateObject relies on it alone) maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **Redundant objectKinds check in CreateObject; finalizeObject already enforces the same invariant (UpdateObject relies on it alone)**
_maintainability · flagged by 1 model_
- **`internal/service/objects.go:82` — redundant `objectKinds[in.Kind]` check in `CreateObject`.** `CreateObject` checks `objectKinds[in.Kind]` at lines 82-84, then builds `o` and calls `finalizeObject`, which re-checks `objectKinds[o.Kind]` (line 211). The kind can't change between the two. `UpdateObject` correctly relies on `finalizeObject` alone. The early check is dead defensive code; `finalizeObject` is already the shared invariant gate. Fix: drop the early check and let `finalizeObject` ow…
<sub>🪰 Gadfly · advisory</sub>
|
||||
g, err := s.requireGardenRole(ctx, actorID, gardenID, roleEditor)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// 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 := objectKinds[in.Kind].plantable
|
||||
if in.Plantable != nil {
|
||||
plantable = *in.Plantable
|
||||
}
|
||||
|
||||
o := &domain.GardenObject{
|
||||
GardenID: gardenID,
|
||||
Kind: in.Kind,
|
||||
Name: strings.TrimSpace(in.Name),
|
||||
Shape: shape,
|
||||
XCM: in.XCM,
|
||||
YCM: in.YCM,
|
||||
WidthCM: in.WidthCM,
|
||||
HeightCM: in.HeightCM,
|
||||
RotationDeg: in.RotationDeg,
|
||||
ZIndex: in.ZIndex,
|
||||
Plantable: plantable,
|
||||
Color: in.Color,
|
||||
Props: in.Props,
|
||||
Notes: strings.TrimSpace(in.Notes),
|
||||
}
|
||||
if err := finalizeObject(o, g); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return s.store.CreateObject(ctx, o)
|
||||
|
gitea-actions
commented
⚪ UpdateObject and DeleteObject duplicate the same fetch-then-authorize sequence; could be factored into a shared helper maintainability · flagged by 1 model
🪰 Gadfly · advisory ⚪ **UpdateObject and DeleteObject duplicate the same fetch-then-authorize sequence; could be factored into a shared helper**
_maintainability · flagged by 1 model_
- `internal/service/objects.go:118-126` and `internal/service/objects.go:138-145` — `UpdateObject` and `DeleteObject` repeat the identical fetch-then-authorize sequence: ```go o, err := s.store.GetObject(ctx, objectID) if err != nil { return nil, err } g, err := s.requireGardenRole(ctx, actorID, o.GardenID, roleEditor) if err != nil { return nil, err } ``` A small helper (e.g. `requireObjectRole(ctx, actorID, objectID, min) (*domain.GardenObject, *domain.Garden, error)`) would remove the duplica…
<sub>🪰 Gadfly · advisory</sub>
|
||||
}
|
||||
|
||||
// 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, g, err := s.objectForRole(ctx, actorID, objectID, roleEditor)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
applyObjectPatch(o, patch)
|
||||
if err := finalizeObject(o, g); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
o.Version = version
|
||||
return s.store.UpdateObject(ctx, o)
|
||||
}
|
||||
|
||||
// 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 {
|
||||
if _, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor); err != nil {
|
||||
return err
|
||||
}
|
||||
return s.store.DeleteObject(ctx, objectID)
|
||||
}
|
||||
|
||||
// GardenFull returns the whole editor payload for a garden the actor can view.
|
||||
func (s *Service) GardenFull(ctx context.Context, actorID, gardenID int64) (*FullGarden, error) {
|
||||
g, err := s.requireGardenRole(ctx, actorID, gardenID, roleViewer)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
objects, err := s.store.ListObjectsForGarden(ctx, gardenID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
plantings, err := s.store.ListActivePlantingsForGarden(ctx, gardenID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
plants, err := s.store.ListReferencedPlants(ctx, gardenID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &FullGarden{Garden: g, Objects: objects, Plantings: plantings, Plants: plants}, nil
|
||||
}
|
||||
|
||||
// applyObjectPatch mutates o with each provided (non-nil) patch field.
|
||||
func applyObjectPatch(o *domain.GardenObject, p ObjectPatch) {
|
||||
if p.Name != nil {
|
||||
o.Name = strings.TrimSpace(*p.Name)
|
||||
}
|
||||
if p.XCM != nil {
|
||||
o.XCM = *p.XCM
|
||||
}
|
||||
if p.YCM != nil {
|
||||
o.YCM = *p.YCM
|
||||
}
|
||||
if p.WidthCM != nil {
|
||||
o.WidthCM = *p.WidthCM
|
||||
}
|
||||
if p.HeightCM != nil {
|
||||
|
gitea-actions
commented
🟡 PATCH cannot clear nullable color/props via explicit JSON null (indistinguishable from absent) correctness · flagged by 1 model
🪰 Gadfly · advisory 🟡 **PATCH cannot clear nullable color/props via explicit JSON null (indistinguishable from absent)**
_correctness · flagged by 1 model_
- **`internal/service/objects.go:196-198` (`applyObjectPatch`) / `internal/api/objects.go:44-45,53-59` (`propsFromRaw`)** — PATCH can never clear the nullable `color` or `props` columns back to `NULL`. `objectUpdateRequest.Color` is a plain `*string` (`internal/api/objects.go:45`), so Go's `encoding/json` sets it to `nil` both when the key is *absent* and when it's explicitly `"color": null`. `applyObjectPatch` only assigns when `p.Color != nil` (`internal/service/objects.go:196-198`), so an exp…
<sub>🪰 Gadfly · advisory</sub>
|
||||
o.HeightCM = *p.HeightCM
|
||||
}
|
||||
if p.RotationDeg != nil {
|
||||
o.RotationDeg = *p.RotationDeg
|
||||
}
|
||||
if p.ZIndex != nil {
|
||||
o.ZIndex = *p.ZIndex
|
||||
}
|
||||
if p.Plantable != nil {
|
||||
o.Plantable = *p.Plantable
|
||||
}
|
||||
// Color/Props: Set* distinguishes "clear to NULL" (value nil) from unchanged.
|
||||
if p.SetColor {
|
||||
o.Color = p.Color
|
||||
}
|
||||
if p.SetProps {
|
||||
o.Props = p.Props
|
||||
}
|
||||
if p.Notes != nil {
|
||||
o.Notes = strings.TrimSpace(*p.Notes)
|
||||
}
|
||||
}
|
||||
|
||||
// finalizeObject normalizes (rotation → [0,360)) and validates a built/merged
|
||||
// object against its garden. Shared by create and update so both enforce the
|
||||
|
gitea-actions
commented
🟡 validDimensionCM uses garden-named constants for object validation maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **validDimensionCM uses garden-named constants for object validation**
_maintainability · flagged by 1 model_
- `internal/service/objects.go:221` calls `validDimensionCM`, which lives in `gardens.go` and checks against constants named `minGardenCM`/`maxGardenCM`. Object dimensions happen to share the same 1 cm–100 m range, but the garden-specific naming hides this coupling: a future change to garden bounds would silently change object bounds too. **Fix:** either extract a generic `validDimensionCM` with neutral constants into a shared location, or give objects an explicit `validObjectDimensionCM` so the…
<sub>🪰 Gadfly · advisory</sub>
|
||||
// same invariants.
|
||||
func finalizeObject(o *domain.GardenObject, g *domain.Garden) error {
|
||||
if _, ok := objectKinds[o.Kind]; !ok {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
// rect/circle only; polygon is reserved in the schema but not supported yet.
|
||||
if o.Shape != domain.ShapeRect && o.Shape != domain.ShapeCircle {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
if len(o.Name) > maxObjectNameLen || len(o.Notes) > maxObjectNotesLen {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
if !validDimensionCM(o.WidthCM) || !validDimensionCM(o.HeightCM) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
if !isFinite(o.XCM) || !isFinite(o.YCM) || !isFinite(o.RotationDeg) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
o.RotationDeg = normalizeDegrees(o.RotationDeg)
|
||||
|
||||
// Loose placement check: the (unrotated) bounding box must overlap the garden
|
||||
// at all — partial overhang is fine, fully off-field is not.
|
||||
if !bboxOverlapsGarden(o, g) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
|
||||
if o.Color != nil && !isHexColor(*o.Color) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
if o.Props != nil {
|
||||
if len(*o.Props) > maxObjectPropsLen || !json.Valid([]byte(*o.Props)) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func isFinite(v float64) bool { return !math.IsNaN(v) && !math.IsInf(v, 0) }
|
||||
|
||||
// normalizeDegrees folds an angle into [0, 360).
|
||||
func normalizeDegrees(d float64) float64 {
|
||||
d = math.Mod(d, 360)
|
||||
if d < 0 {
|
||||
d += 360
|
||||
}
|
||||
return d
|
||||
}
|
||||
|
||||
// bboxOverlapsGarden reports whether the object's axis-aligned (unrotated)
|
||||
// bounding box intersects the garden rectangle [0,width]×[0,height].
|
||||
func bboxOverlapsGarden(o *domain.GardenObject, g *domain.Garden) bool {
|
||||
halfW, halfH := o.WidthCM/2, o.HeightCM/2
|
||||
left, right := o.XCM-halfW, o.XCM+halfW
|
||||
top, bottom := o.YCM-halfH, o.YCM+halfH
|
||||
return left < g.WidthCM && right > 0 && top < g.HeightCM && bottom > 0
|
||||
}
|
||||
|
||||
// isHexColor accepts #rgb or #rrggbb (case-insensitive).
|
||||
func isHexColor(s string) bool {
|
||||
if len(s) != 4 && len(s) != 7 {
|
||||
return false
|
||||
}
|
||||
if s[0] != '#' {
|
||||
return false
|
||||
}
|
||||
for _, c := range s[1:] {
|
||||
if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F')) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
@@ -0,0 +1,236 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
)
|
||||
|
||||
// seedGarden creates a garden owned by owner and returns it.
|
||||
func seedGarden(t *testing.T, s *Service, owner int64) *domain.Garden {
|
||||
t.Helper()
|
||||
g, err := s.CreateGarden(context.Background(), owner, GardenInput{Name: "Yard", WidthCM: 1000, HeightCM: 1000})
|
||||
if err != nil {
|
||||
t.Fatalf("seed garden: %v", err)
|
||||
}
|
||||
return g
|
||||
}
|
||||
|
||||
func ptrBool(b bool) *bool { return &b }
|
||||
func strPtr(s string) *string { return &s }
|
||||
|
||||
func TestCreateObjectDefaults(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
|
||||
// A bed at the garden center; no shape/plantable given.
|
||||
o, err := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
|
||||
Kind: domain.KindBed, Name: " Bed 1 ", XCM: 500, YCM: 500, WidthCM: 200, HeightCM: 100,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateObject: %v", err)
|
||||
}
|
||||
if o.Shape != domain.ShapeRect {
|
||||
t.Errorf("shape = %q, want default rect", o.Shape)
|
||||
}
|
||||
if !o.Plantable {
|
||||
t.Error("a bed should default to plantable")
|
||||
}
|
||||
if o.Name != "Bed 1" {
|
||||
t.Errorf("name = %q, want trimmed", o.Name)
|
||||
}
|
||||
if o.Version != 1 || o.GardenID != g.ID {
|
||||
t.Errorf("unexpected object: %+v", o)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateObjectPlantableByKind(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
|
||||
tree, err := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
|
||||
Kind: domain.KindTree, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create tree: %v", err)
|
||||
}
|
||||
if tree.Plantable {
|
||||
t.Error("a tree should default to not plantable")
|
||||
}
|
||||
|
||||
// Override: a plantable path.
|
||||
path, err := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
|
||||
Kind: domain.KindPath, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100, Plantable: ptrBool(true),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create path: %v", err)
|
||||
}
|
||||
if !path.Plantable {
|
||||
t.Error("plantable override should win")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateObjectRotationNormalized(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
|
||||
o, err := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
|
||||
Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100, RotationDeg: 450,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateObject: %v", err)
|
||||
}
|
||||
if o.RotationDeg != 90 {
|
||||
t.Errorf("rotation 450 normalized to %v, want 90", o.RotationDeg)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateObjectValidation(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner) // 1000×1000
|
||||
|
||||
bad := []ObjectInput{
|
||||
{Kind: "spaceship", XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100}, // bad kind
|
||||
{Kind: domain.KindBed, Shape: domain.ShapePolygon, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100}, // polygon reserved
|
||||
{Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 0, HeightCM: 100}, // zero width
|
||||
{Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: -5, HeightCM: 100}, // negative
|
||||
{Kind: domain.KindBed, XCM: 5000, YCM: 5000, WidthCM: 100, HeightCM: 100}, // fully off-field
|
||||
{Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100, Color: strPtr("nope")}, // bad color
|
||||
{Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100, Props: strPtr("{not json")}, // bad props
|
||||
{Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100, Name: strings.Repeat("x", maxObjectNameLen+1)}, // name too long
|
||||
}
|
||||
for i, in := range bad {
|
||||
if _, err := s.CreateObject(context.Background(), owner, g.ID, in); !errors.Is(err, domain.ErrInvalidInput) {
|
||||
t.Errorf("case %d: err = %v, want ErrInvalidInput", i, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Partial overhang IS allowed (center near the edge).
|
||||
if _, err := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
|
||||
Kind: domain.KindBed, XCM: 990, YCM: 990, WidthCM: 200, HeightCM: 200,
|
||||
}); err != nil {
|
||||
t.Errorf("overhang at the edge should be allowed: %v", err)
|
||||
}
|
||||
|
||||
// A valid hex color and valid props JSON pass.
|
||||
if _, err := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
|
||||
Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100,
|
||||
Color: strPtr("#3f8f4f"), Props: strPtr(`{"heightCm":40}`),
|
||||
}); err != nil {
|
||||
t.Errorf("valid color+props should pass: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateObjectPartialAndVersion(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
o, _ := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
|
||||
Kind: domain.KindBed, Name: "Bed", XCM: 100, YCM: 100, WidthCM: 100, HeightCM: 100,
|
||||
})
|
||||
|
||||
// Move + rotate only; name/dimensions untouched.
|
||||
nx, ny, rot := 300.0, 400.0, -90.0
|
||||
updated, err := s.UpdateObject(context.Background(), owner, o.ID, ObjectPatch{XCM: &nx, YCM: &ny, RotationDeg: &rot}, o.Version)
|
||||
if err != nil {
|
||||
t.Fatalf("UpdateObject: %v", err)
|
||||
}
|
||||
if updated.XCM != 300 || updated.YCM != 400 {
|
||||
t.Errorf("move didn't apply: %+v", updated)
|
||||
}
|
||||
if updated.RotationDeg != 270 {
|
||||
t.Errorf("rotation -90 normalized to %v, want 270", updated.RotationDeg)
|
||||
}
|
||||
if updated.Name != "Bed" || updated.WidthCM != 100 {
|
||||
t.Errorf("untouched fields changed: %+v", updated)
|
||||
}
|
||||
if updated.Version != o.Version+1 {
|
||||
t.Errorf("version = %d, want %d", updated.Version, o.Version+1)
|
||||
}
|
||||
|
||||
// Stale version → conflict + current row.
|
||||
current, err := s.UpdateObject(context.Background(), owner, o.ID, ObjectPatch{XCM: &nx}, o.Version)
|
||||
if !errors.Is(err, domain.ErrVersionConflict) {
|
||||
t.Fatalf("stale update err = %v, want ErrVersionConflict", err)
|
||||
}
|
||||
if current == nil || current.Version != 2 {
|
||||
t.Errorf("conflict didn't return current row: %+v", current)
|
||||
}
|
||||
}
|
||||
|
||||
func TestObjectCrossUserIsNotFound(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
alice := seedUser(t, s, "[email protected]")
|
||||
bob := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, alice)
|
||||
o, _ := s.CreateObject(context.Background(), alice, g.ID, ObjectInput{
|
||||
Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100,
|
||||
})
|
||||
|
||||
nx := 1.0
|
||||
if _, err := s.CreateObject(context.Background(), bob, g.ID, ObjectInput{Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100}); !errors.Is(err, domain.ErrNotFound) {
|
||||
t.Errorf("bob create err = %v, want ErrNotFound", err)
|
||||
}
|
||||
if _, err := s.UpdateObject(context.Background(), bob, o.ID, ObjectPatch{XCM: &nx}, o.Version); !errors.Is(err, domain.ErrNotFound) {
|
||||
t.Errorf("bob update err = %v, want ErrNotFound", err)
|
||||
}
|
||||
if err := s.DeleteObject(context.Background(), bob, o.ID); !errors.Is(err, domain.ErrNotFound) {
|
||||
t.Errorf("bob delete err = %v, want ErrNotFound", err)
|
||||
}
|
||||
if _, err := s.GardenFull(context.Background(), bob, g.ID); !errors.Is(err, domain.ErrNotFound) {
|
||||
t.Errorf("bob /full err = %v, want ErrNotFound", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeleteObject(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
o, _ := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
|
||||
Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100,
|
||||
})
|
||||
|
||||
if err := s.DeleteObject(context.Background(), owner, o.ID); err != nil {
|
||||
t.Fatalf("DeleteObject: %v", err)
|
||||
}
|
||||
full, err := s.GardenFull(context.Background(), owner, g.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GardenFull: %v", err)
|
||||
}
|
||||
if len(full.Objects) != 0 {
|
||||
t.Errorf("object still present after delete: %d", len(full.Objects))
|
||||
}
|
||||
}
|
||||
|
||||
func TestGardenFullShape(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
s.CreateObject(context.Background(), owner, g.ID, ObjectInput{Kind: domain.KindBed, XCM: 300, YCM: 300, WidthCM: 100, HeightCM: 100})
|
||||
s.CreateObject(context.Background(), owner, g.ID, ObjectInput{Kind: domain.KindContainer, Shape: domain.ShapeCircle, XCM: 600, YCM: 600, WidthCM: 60, HeightCM: 60})
|
||||
|
||||
full, err := s.GardenFull(context.Background(), owner, g.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GardenFull: %v", err)
|
||||
}
|
||||
if full.Garden == nil || full.Garden.ID != g.ID {
|
||||
t.Error("full.Garden missing")
|
||||
}
|
||||
if len(full.Objects) != 2 {
|
||||
t.Errorf("objects = %d, want 2", len(full.Objects))
|
||||
}
|
||||
// Plantings/plants are empty (non-nil) until #14.
|
||||
if full.Plantings == nil || len(full.Plantings) != 0 {
|
||||
t.Errorf("plantings = %v, want empty non-nil", full.Plantings)
|
||||
}
|
||||
if full.Plants == nil || len(full.Plants) != 0 {
|
||||
t.Errorf("plants = %v, want empty non-nil", full.Plants)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,135 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
)
|
||||
|
||||
// objectColumns lists garden_objects columns in the order scanObject expects.
|
||||
const objectColumns = `id, garden_id, kind, name, shape, points, x_cm, y_cm, width_cm, height_cm,
|
||||
rotation_deg, z_index, plantable, color, props, notes, version, created_at, updated_at`
|
||||
|
||||
func scanObject(s scanner) (*domain.GardenObject, error) {
|
||||
var (
|
||||
o domain.GardenObject
|
||||
plantable int64
|
||||
)
|
||||
if err := s.Scan(
|
||||
&o.ID, &o.GardenID, &o.Kind, &o.Name, &o.Shape, &o.Points,
|
||||
&o.XCM, &o.YCM, &o.WidthCM, &o.HeightCM, &o.RotationDeg, &o.ZIndex,
|
||||
&plantable, &o.Color, &o.Props, &o.Notes, &o.Version, &o.CreatedAt, &o.UpdatedAt,
|
||||
); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
o.Plantable = plantable != 0
|
||||
return &o, nil
|
||||
}
|
||||
|
||||
// CreateObject inserts a garden object (fields already validated by the service)
|
||||
// and returns the stored row.
|
||||
func (d *DB) CreateObject(ctx context.Context, o *domain.GardenObject) (*domain.GardenObject, error) {
|
||||
created, err := scanObject(d.sql.QueryRowContext(ctx,
|
||||
`INSERT INTO garden_objects
|
||||
(garden_id, kind, name, shape, points, x_cm, y_cm, width_cm, height_cm,
|
||||
rotation_deg, z_index, plantable, color, props, notes)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
RETURNING `+objectColumns,
|
||||
o.GardenID, o.Kind, o.Name, o.Shape, o.Points, o.XCM, o.YCM, o.WidthCM, o.HeightCM,
|
||||
o.RotationDeg, o.ZIndex, boolToInt(o.Plantable), o.Color, o.Props, o.Notes,
|
||||
))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: insert object: %w", err)
|
||||
}
|
||||
return created, nil
|
||||
}
|
||||
|
||||
// GetObject returns the object with the given id, or domain.ErrNotFound.
|
||||
func (d *DB) GetObject(ctx context.Context, id int64) (*domain.GardenObject, error) {
|
||||
o, err := scanObject(d.sql.QueryRowContext(ctx,
|
||||
`SELECT `+objectColumns+` FROM garden_objects WHERE id = ?`, id))
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return nil, domain.ErrNotFound
|
||||
}
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: get object: %w", err)
|
||||
}
|
||||
return o, nil
|
||||
}
|
||||
|
||||
// ListObjectsForGarden returns a garden's objects, bottom-to-top (z_index then
|
||||
// id). Always a non-nil slice.
|
||||
func (d *DB) ListObjectsForGarden(ctx context.Context, gardenID int64) ([]domain.GardenObject, error) {
|
||||
rows, err := d.sql.QueryContext(ctx,
|
||||
`SELECT `+objectColumns+` FROM garden_objects WHERE garden_id = ? ORDER BY z_index, id`,
|
||||
gardenID,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: list objects: %w", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
objects := []domain.GardenObject{}
|
||||
for rows.Next() {
|
||||
o, err := scanObject(rows)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: scan object: %w", err)
|
||||
}
|
||||
objects = append(objects, *o)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, fmt.Errorf("store: iterate objects: %w", err)
|
||||
}
|
||||
return objects, nil
|
||||
}
|
||||
|
||||
// UpdateObject applies a version-guarded update of all mutable columns (the
|
||||
// service merges partial patches onto the current row first). Returns the
|
||||
// updated row, or (current row, ErrVersionConflict) / ErrNotFound — the same
|
||||
// contract as UpdateGarden.
|
||||
func (d *DB) UpdateObject(ctx context.Context, o *domain.GardenObject) (*domain.GardenObject, error) {
|
||||
updated, err := scanObject(d.sql.QueryRowContext(ctx,
|
||||
`UPDATE garden_objects
|
||||
SET kind = ?, name = ?, shape = ?, points = ?, x_cm = ?, y_cm = ?,
|
||||
width_cm = ?, height_cm = ?, rotation_deg = ?, z_index = ?,
|
||||
plantable = ?, color = ?, props = ?, notes = ?,
|
||||
version = version + 1,
|
||||
updated_at = strftime('%Y-%m-%dT%H:%M:%SZ', 'now')
|
||||
WHERE id = ? AND version = ?
|
||||
RETURNING `+objectColumns,
|
||||
o.Kind, o.Name, o.Shape, o.Points, o.XCM, o.YCM, o.WidthCM, o.HeightCM,
|
||||
o.RotationDeg, o.ZIndex, boolToInt(o.Plantable), o.Color, o.Props, o.Notes,
|
||||
o.ID, o.Version,
|
||||
))
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
current, gerr := d.GetObject(ctx, o.ID)
|
||||
if gerr != nil {
|
||||
return nil, gerr
|
||||
}
|
||||
return current, domain.ErrVersionConflict
|
||||
}
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: update object: %w", err)
|
||||
}
|
||||
return updated, nil
|
||||
}
|
||||
|
||||
// DeleteObject removes an object; its plantings cascade via the FK. Returns
|
||||
// domain.ErrNotFound if no row was deleted.
|
||||
func (d *DB) DeleteObject(ctx context.Context, id int64) error {
|
||||
res, err := d.sql.ExecContext(ctx, `DELETE FROM garden_objects WHERE id = ?`, id)
|
||||
if err != nil {
|
||||
return fmt.Errorf("store: delete object: %w", err)
|
||||
}
|
||||
n, err := res.RowsAffected()
|
||||
if err != nil {
|
||||
return fmt.Errorf("store: object delete rows: %w", err)
|
||||
}
|
||||
if n == 0 {
|
||||
return domain.ErrNotFound
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,56 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
)
|
||||
|
||||
// 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(
|
||||
&p.ID, &p.ObjectID, &p.PlantID, &p.XCM, &p.YCM, &p.RadiusCM,
|
||||
&p.Count, &p.Label, &p.PlantedAt, &p.RemovedAt,
|
||||
&p.Version, &p.CreatedAt, &p.UpdatedAt,
|
||||
); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &p, nil
|
||||
}
|
||||
|
||||
// ListActivePlantingsForGarden returns every currently-planted plop (removed_at
|
||||
// IS NULL) across all objects in a garden — the editor's one-shot load. Always a
|
||||
|
gitea-actions
commented
🟠 SELECT * deviates from explicit column-list store pattern maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟠 **SELECT * deviates from explicit column-list store pattern**
_maintainability · flagged by 2 models_
- **`internal/store/plantings.go:32`** and **`internal/store/plants.go:31`** — The queries use `SELECT pl.*` / `SELECT DISTINCT p.*`, deviating from the explicit column-list pattern established in `gardens.go` (`gardenColumns`) and `objects.go` (`objectColumns`). Relying on `*` column ordering is fragile against future schema changes (column reordering or additions). Suggested fix: add `plantingColumns` / `plantColumns` constants and use them in the queries, matching the existing store pattern.
<sub>🪰 Gadfly · advisory</sub>
|
||||
// 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) {
|
||||
rows, err := d.sql.QueryContext(ctx,
|
||||
`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`,
|
||||
|
gitea-actions
commented
🟡 SELECT pl. couples scanPlanting to table column order; prefer explicit column list when #14 extends the table* maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **SELECT pl.* couples scanPlanting to table column order; prefer explicit column list when #14 extends the table**
_maintainability · flagged by 1 model_
- **`internal/store/plantings.go:36` / `plants.go:34` — `SELECT pl.*` / `SELECT DISTINCT p.*` rely on table-declared column order.** This is acknowledged in comments, which is good, but it's a latent maintainability hazard: if a column is added/reordered in the `plantings`/`plants` schema later (e.g. in #12/#14), `scanPlanting`/`scanPlant` will silently break at runtime, not compile time. The `objects.go` path avoids this by listing `objectColumns` explicitly. Suggested fix: when #12/#14 extend…
<sub>🪰 Gadfly · advisory</sub>
|
||||
gardenID,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: list plantings: %w", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
plantings := []domain.Planting{}
|
||||
for rows.Next() {
|
||||
p, err := scanPlanting(rows)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: scan planting: %w", err)
|
||||
}
|
||||
plantings = append(plantings, *p)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, fmt.Errorf("store: iterate plantings: %w", err)
|
||||
}
|
||||
return plantings, nil
|
||||
}
|
||||
@@ -0,0 +1,55 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
)
|
||||
|
||||
// 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(
|
||||
&p.ID, &p.OwnerID, &p.Name, &p.Category, &p.SpacingCM, &p.Color, &p.Icon,
|
||||
&p.DaysToMaturity, &p.Notes, &p.Version, &p.CreatedAt, &p.UpdatedAt,
|
||||
); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &p, nil
|
||||
}
|
||||
|
||||
// ListReferencedPlants returns the distinct plants used by a garden's active
|
||||
// plantings — the catalog subset the editor needs to render them. Always a
|
||||
// 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) {
|
||||
rows, err := d.sql.QueryContext(ctx,
|
||||
`SELECT DISTINCT `+qualifyColumns("p", plantColumns)+` FROM plants p
|
||||
|
gitea-actions
commented
🟠 SELECT * deviates from explicit column-list store pattern maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟠 **SELECT * deviates from explicit column-list store pattern**
_maintainability · flagged by 2 models_
- **`internal/store/plantings.go:32`** and **`internal/store/plants.go:31`** — The queries use `SELECT pl.*` / `SELECT DISTINCT p.*`, deviating from the explicit column-list pattern established in `gardens.go` (`gardenColumns`) and `objects.go` (`objectColumns`). Relying on `*` column ordering is fragile against future schema changes (column reordering or additions). Suggested fix: add `plantingColumns` / `plantColumns` constants and use them in the queries, matching the existing store pattern.
<sub>🪰 Gadfly · advisory</sub>
|
||||
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
|
||||
ORDER BY p.id`,
|
||||
gardenID,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: list referenced plants: %w", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
plants := []domain.Plant{}
|
||||
for rows.Next() {
|
||||
p, err := scanPlant(rows)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: scan plant: %w", err)
|
||||
}
|
||||
plants = append(plants, *p)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, fmt.Errorf("store: iterate plants: %w", err)
|
||||
}
|
||||
return plants, nil
|
||||
}
|
||||
@@ -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() }
|
||||
|
||||
|
||||
🟡 Handler name gardenFull breaks verbNoun convention
maintainability · flagged by 1 model
internal/api/api.go:84— Handler registration usesh.gardenFull, breaking theverbNounnaming convention every other handler follows (listGardens,createGarden,getGarden,updateGarden,deleteGarden,createObject,updateObject,deleteObject). Rename togetGardenFullfor consistency.🪰 Gadfly · advisory