Plantings backend: plop CRUD, derived counts, removed_at (#14) #33
@@ -89,6 +89,13 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
|
||||
objects := v1.Group("/objects", h.requireAuth())
|
||||
objects.PATCH("/:id", h.updateObject)
|
||||
objects.DELETE("/:id", h.deleteObject)
|
||||
objects.POST("/:id/plantings", h.createPlanting) // place a plop in this object
|
||||
|
||||
// Plantings ("plops") are addressed by their own id; the service resolves the
|
||||
// owning object/garden for the permission check.
|
||||
plantings := v1.Group("/plantings", h.requireAuth())
|
||||
plantings.PATCH("/:id", h.updatePlanting)
|
||||
plantings.DELETE("/:id", h.deletePlanting)
|
||||
|
||||
// Plant catalog: built-ins (seeded, read-only) plus the actor's own rows.
|
||||
plants := v1.Group("/plants", h.requireAuth())
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
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"
|
||||
)
|
||||
|
||||
// plantingCreateRequest is the body for POST /objects/:id/plantings. x/y are in
|
||||
// the object's local frame (origin at object center); count omitted = derived;
|
||||
// plantedAt omitted = today.
|
||||
type plantingCreateRequest struct {
|
||||
PlantID int64 `json:"plantId" binding:"required"`
|
||||
XCM float64 `json:"xCm"`
|
||||
YCM float64 `json:"yCm"`
|
||||
RadiusCM float64 `json:"radiusCm"`
|
||||
Count *int `json:"count"`
|
||||
Label *string `json:"label"`
|
||||
PlantedAt *string `json:"plantedAt"`
|
||||
}
|
||||
|
||||
func (r plantingCreateRequest) toInput() service.PlantingInput {
|
||||
return service.PlantingInput{
|
||||
PlantID: r.PlantID, XCM: r.XCM, YCM: r.YCM, RadiusCM: r.RadiusCM,
|
||||
Count: r.Count, Label: r.Label, PlantedAt: r.PlantedAt,
|
||||
}
|
||||
}
|
||||
|
||||
// plantingUpdateRequest is the body for PATCH /plantings/:id: every field
|
||||
// optional (absent = unchanged) plus the required current version. The nullable
|
||||
// fields are json.RawMessage so an explicit null (clear) is distinct from an
|
||||
// absent field (unchanged) — e.g. `removedAt: "2026-07-01"` soft-removes and
|
||||
// `removedAt: null` un-removes.
|
||||
type plantingUpdateRequest struct {
|
||||
PlantID *int64 `json:"plantId"`
|
||||
XCM *float64 `json:"xCm"`
|
||||
YCM *float64 `json:"yCm"`
|
||||
RadiusCM *float64 `json:"radiusCm"`
|
||||
Count json.RawMessage `json:"count"`
|
||||
Label json.RawMessage `json:"label"`
|
||||
PlantedAt json.RawMessage `json:"plantedAt"`
|
||||
RemovedAt json.RawMessage `json:"removedAt"`
|
||||
Version int64 `json:"version" binding:"required,min=1"`
|
||||
}
|
||||
|
||||
func (r plantingUpdateRequest) toPatch() (service.PlantingPatch, error) {
|
||||
count, setCount, err := parseNullableInt(r.Count)
|
||||
if err != nil {
|
||||
return service.PlantingPatch{}, err
|
||||
}
|
||||
label, setLabel, err := parseNullableString(r.Label)
|
||||
if err != nil {
|
||||
return service.PlantingPatch{}, err
|
||||
}
|
||||
plantedAt, setPlantedAt, err := parseNullableString(r.PlantedAt)
|
||||
if err != nil {
|
||||
return service.PlantingPatch{}, err
|
||||
}
|
||||
removedAt, setRemovedAt, err := parseNullableString(r.RemovedAt)
|
||||
if err != nil {
|
||||
return service.PlantingPatch{}, err
|
||||
}
|
||||
return service.PlantingPatch{
|
||||
PlantID: r.PlantID, XCM: r.XCM, YCM: r.YCM, RadiusCM: r.RadiusCM,
|
||||
SetCount: setCount, Count: count,
|
||||
SetLabel: setLabel, Label: label,
|
||||
SetPlantedAt: setPlantedAt, PlantedAt: plantedAt,
|
||||
SetRemovedAt: setRemovedAt, RemovedAt: removedAt,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (h *handlers) createPlanting(c *gin.Context) {
|
||||
objectID, ok := parseIDParam(c, "id")
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
var req plantingCreateRequest
|
||||
if err := c.ShouldBindJSON(&req); err != nil {
|
||||
|
|
||||
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid planting: a plantId is required")
|
||||
return
|
||||
}
|
||||
p, err := h.svc.CreatePlanting(c.Request.Context(), mustActor(c).ID, objectID, req.toInput())
|
||||
if err != nil {
|
||||
writeServiceError(c, err)
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusCreated, p)
|
||||
}
|
||||
|
||||
func (h *handlers) updatePlanting(c *gin.Context) {
|
||||
id, ok := parseIDParam(c, "id")
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
var req plantingUpdateRequest
|
||||
if err := c.ShouldBindJSON(&req); err != nil {
|
||||
|
gitea-actions
commented
🟡 ShouldBindJSON error swallowed by fixed misleading message in updatePlanting error-handling · flagged by 1 model
🪰 Gadfly · advisory 🟡 **ShouldBindJSON error swallowed by fixed misleading message in updatePlanting**
_error-handling · flagged by 1 model_
- **`internal/api/plantings.go:101`** — `updatePlanting` swallows the actual `ShouldBindJSON` error and always emits a fixed message. If the request body contains malformed JSON, wrong types, or syntax errors, the client still receives *"invalid update: a current version is required"*, which is misleading and makes debugging harder. **Fix:** inspect `err` and return the underlying detail, or at least a generic "invalid request body" when the required field is actually present.
<sub>🪰 Gadfly · advisory</sub>
|
||||
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
|
||||
}
|
||||
p, err := h.svc.UpdatePlanting(c.Request.Context(), mustActor(c).ID, id, patch, req.Version)
|
||||
if err != nil {
|
||||
if errors.Is(err, domain.ErrVersionConflict) {
|
||||
writeVersionConflict(c, p)
|
||||
return
|
||||
}
|
||||
writeServiceError(c, err)
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, p)
|
||||
}
|
||||
|
||||
func (h *handlers) deletePlanting(c *gin.Context) {
|
||||
id, ok := parseIDParam(c, "id")
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
if err := h.svc.DeletePlanting(c.Request.Context(), mustActor(c).ID, id); err != nil {
|
||||
writeServiceError(c, err)
|
||||
return
|
||||
}
|
||||
c.Status(http.StatusNoContent)
|
||||
}
|
||||
@@ -0,0 +1,99 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func plantingPath(id int64) string { return "/api/v1/plantings/" + strconv.FormatInt(id, 10) }
|
||||
func objectPlantingsPath(id int64) string { return objectPath(id) + "/plantings" }
|
||||
|
||||
func TestPlantingCreatePatchFullDelete(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
gid := createGardenAPI(t, r, cookie, "Yard")
|
||||
|
||||
// A plantable bed centered in the garden.
|
||||
w := doJSON(t, r, http.MethodPost, objectsPath(gid),
|
||||
map[string]any{"kind": "bed", "xCm": 500, "yCm": 500, "widthCm": 200, "heightCm": 200}, cookie)
|
||||
oid := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
|
||||
|
||||
// A seeded built-in plant to place.
|
||||
w = doJSON(t, r, http.MethodGet, "/api/v1/plants", nil, cookie)
|
||||
plantID, _ := firstBuiltinID(t, w.Body.Bytes())
|
||||
|
||||
// Place a plop → 201 with a derivedCount.
|
||||
w = doJSON(t, r, http.MethodPost, objectPlantingsPath(oid),
|
||||
map[string]any{"plantId": plantID, "xCm": 0, "yCm": 0, "radiusCm": 20}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("create planting: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
pl := decodeMap(t, w.Body.Bytes())
|
||||
pid := int64(pl["id"].(float64))
|
||||
dc, ok := pl["derivedCount"].(float64)
|
||||
if !ok || dc < 1 {
|
||||
t.Errorf("response missing a positive derivedCount: %+v", pl)
|
||||
}
|
||||
|
||||
// /full includes the plop and its referenced plant.
|
||||
w = doJSON(t, r, http.MethodGet, fullPath(gid), nil, cookie)
|
||||
var full struct {
|
||||
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", err)
|
||||
}
|
||||
if len(full.Plantings) != 1 || len(full.Plants) != 1 {
|
||||
t.Fatalf("full = %d plantings / %d plants, want 1 / 1", len(full.Plantings), len(full.Plants))
|
||||
}
|
||||
|
||||
// Soft-remove (clear-bed style) → it leaves /full.
|
||||
w = doJSON(t, r, http.MethodPatch, plantingPath(pid), map[string]any{"removedAt": "2030-06-01", "version": 1}, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("soft-remove: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
w = doJSON(t, r, http.MethodGet, fullPath(gid), nil, cookie)
|
||||
full.Plantings = nil
|
||||
json.Unmarshal(w.Body.Bytes(), &full)
|
||||
if len(full.Plantings) != 0 {
|
||||
t.Errorf("removed plop still in /full: %d", len(full.Plantings))
|
||||
}
|
||||
|
||||
// Hard delete → 204.
|
||||
if w := doJSON(t, r, http.MethodDelete, plantingPath(pid), nil, cookie); w.Code != http.StatusNoContent {
|
||||
t.Errorf("delete planting = %d, want 204", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingIntoNonPlantableIsRejected(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
gid := createGardenAPI(t, r, cookie, "Yard")
|
||||
|
||||
// A tree is not plantable.
|
||||
w := doJSON(t, r, http.MethodPost, objectsPath(gid),
|
||||
map[string]any{"kind": "tree", "xCm": 500, "yCm": 500, "widthCm": 100, "heightCm": 100}, cookie)
|
||||
oid := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
|
||||
|
||||
w = doJSON(t, r, http.MethodGet, "/api/v1/plants", nil, cookie)
|
||||
plantID, _ := firstBuiltinID(t, w.Body.Bytes())
|
||||
|
||||
w = doJSON(t, r, http.MethodPost, objectPlantingsPath(oid),
|
||||
map[string]any{"plantId": plantID, "xCm": 0, "yCm": 0, "radiusCm": 10}, cookie)
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("plant into tree = %d, want 400", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingsRequireAuth(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
if w := doJSON(t, r, http.MethodPost, "/api/v1/objects/1/plantings", map[string]any{"plantId": 1}, nil); w.Code != http.StatusUnauthorized {
|
||||
t.Errorf("unauthenticated create = %d, want 401", w.Code)
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodDelete, "/api/v1/plantings/1", nil, nil); w.Code != http.StatusUnauthorized {
|
||||
t.Errorf("unauthenticated delete = %d, want 401", w.Code)
|
||||
}
|
||||
}
|
||||
@@ -186,4 +186,10 @@ type Planting struct {
|
||||
Version int64 `json:"version"`
|
||||
CreatedAt string `json:"createdAt"`
|
||||
UpdatedAt string `json:"updatedAt"`
|
||||
|
||||
// DerivedCount is the plant count implied by the plop's area and the plant's
|
||||
// mature spacing: max(1, round(π·r² / spacing²)). It is computed by the
|
||||
// service, never persisted (the store scan leaves it 0). The effective count
|
||||
// a client shows is Count when non-nil, else DerivedCount.
|
||||
DerivedCount int `json:"derivedCount"`
|
||||
}
|
||||
|
||||
@@ -69,8 +69,8 @@ type ObjectPatch struct {
|
||||
}
|
||||
|
||||
// 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.
|
||||
// it — objects, active plantings (each with its DerivedCount filled in), and the
|
||||
// plants those plantings reference.
|
||||
type FullGarden struct {
|
||||
Garden *domain.Garden `json:"garden"`
|
||||
Objects []domain.GardenObject `json:"objects"`
|
||||
@@ -176,6 +176,15 @@ func (s *Service) GardenFull(ctx context.Context, actorID, gardenID int64) (*Ful
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// Fill each plop's DerivedCount from its plant's spacing (referenced plants
|
||||
// are already loaded, so this is a map lookup rather than N queries).
|
||||
spacing := make(map[int64]float64, len(plants))
|
||||
for _, pl := range plants {
|
||||
spacing[pl.ID] = pl.SpacingCM
|
||||
}
|
||||
for i := range plantings {
|
||||
plantings[i].DerivedCount = derivedCount(plantings[i].RadiusCM, spacing[plantings[i].PlantID])
|
||||
}
|
||||
return &FullGarden{Garden: g, Objects: objects, Plantings: plantings, Plants: plants}, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,272 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"math"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
)
|
||||
|
||||
// dateLayout is the 'YYYY-MM-DD' format plantings store planted_at/removed_at in.
|
||||
const dateLayout = "2006-01-02"
|
||||
|
||||
const (
|
||||
maxPlantingLabelLen = 200
|
||||
maxRadiusCM = 10_000 // 100 m — a generous plop ceiling
|
||||
maxExplicitCount = 1_000_000 // sanity cap on a manual count override
|
||||
)
|
||||
|
||||
// derivedCount is the plant count implied by a plop's area and the plant's
|
||||
// mature spacing: max(1, round(π·r² / spacing²)). It is the single source of the
|
||||
// formula (also feeds #15's display and #19's FillRegion). A non-positive or
|
||||
// non-finite radius/spacing collapses to the floor of 1; the result is capped at
|
||||
// maxExplicitCount so a huge radius / tiny spacing can't yield an absurd (or, on
|
||||
|
gitea-actions
commented
🟠 derivedCount missing math.IsInf guard — corrupted radius can yield implementation-defined int conversion error-handling · flagged by 1 model
🪰 Gadfly · advisory 🟠 **derivedCount missing math.IsInf guard — corrupted radius can yield implementation-defined int conversion**
_error-handling · flagged by 1 model_
* **`internal/service/plantings.go:26-36`** — `derivedCount` guards against `NaN` and non-positive inputs but **not `+Inf`**. If corrupted data (or a radius whose square overflows float64 to `+Inf`) reaches `GardenFull`, `int(math.Round(+Inf))` produces implementation-defined results (possible wrap or panic per the Go spec). `finalizePlanting` already validates with `isFinite` (which includes `!math.IsInf`), so the helper should be symmetrically defensive.
<sub>🪰 Gadfly · advisory</sub>
|
||||
// a 32-bit int, overflowing) value — the same ceiling a manual override honors.
|
||||
func derivedCount(radiusCM, spacingCM float64) int {
|
||||
if radiusCM <= 0 || spacingCM <= 0 || !isFinite(radiusCM) || !isFinite(spacingCM) {
|
||||
return 1
|
||||
}
|
||||
|
gitea-actions
commented
🟠 derivedCount has no upper bound; huge radius / tiny spacing yields unbounded int (manual count is capped at 1e6 but derived is not) error-handling · flagged by 2 models
🪰 Gadfly · advisory 🟠 **derivedCount has no upper bound; huge radius / tiny spacing yields unbounded int (manual count is capped at 1e6 but derived is not)**
_error-handling · flagged by 2 models_
- **`internal/service/plantings.go:31` — `derivedCount` has no upper cap (derived path unbounded).** `area / spacing²` (line 31) can be astronomically large for `maxRadiusCM = 10_000` against a tiny spacing, and `int(math.Round(...))` converts to an `int` with no ceiling. The manual `count` override is capped at `maxExplicitCount = 1_000_000` (line 223), but the derived path has no equivalent clamp, so a huge-radius / tiny-spacing plop yields an unbounded `derivedCount` flowing into the JSON res…
<sub>🪰 Gadfly · advisory</sub>
|
||||
area := math.Pi * radiusCM * radiusCM
|
||||
n := int(math.Round(area / (spacingCM * spacingCM)))
|
||||
if n < 1 {
|
||||
return 1
|
||||
}
|
||||
if n > maxExplicitCount {
|
||||
return maxExplicitCount
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
// PlantingInput is the payload for placing a plop. Count nil = derived; PlantedAt
|
||||
// nil defaults to today.
|
||||
type PlantingInput struct {
|
||||
PlantID int64
|
||||
XCM float64
|
||||
YCM float64
|
||||
RadiusCM float64
|
||||
Count *int
|
||||
Label *string
|
||||
PlantedAt *string
|
||||
}
|
||||
|
||||
// PlantingPatch is a partial update. The nullable fields (Count/Label/PlantedAt/
|
||||
// RemovedAt) use a Set* flag to tell "clear to NULL" from "leave unchanged".
|
||||
// Setting RemovedAt is how "clear bed"/soft-remove works; clearing it un-removes.
|
||||
type PlantingPatch struct {
|
||||
PlantID *int64
|
||||
XCM *float64
|
||||
YCM *float64
|
||||
RadiusCM *float64
|
||||
SetCount bool
|
||||
Count *int
|
||||
SetLabel bool
|
||||
Label *string
|
||||
SetPlantedAt bool
|
||||
PlantedAt *string
|
||||
SetRemovedAt bool
|
||||
RemovedAt *string
|
||||
}
|
||||
|
||||
// CreatePlanting places a plop in a plantable object the actor can edit.
|
||||
func (s *Service) CreatePlanting(ctx context.Context, actorID, objectID int64, in PlantingInput) (*domain.Planting, error) {
|
||||
o, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if !o.Plantable {
|
||||
return nil, domain.ErrInvalidInput // trees/paths/structures can't hold plants
|
||||
}
|
||||
plant, err := s.visiblePlant(ctx, actorID, in.PlantID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
p := &domain.Planting{
|
||||
ObjectID: objectID,
|
||||
PlantID: in.PlantID,
|
||||
XCM: in.XCM,
|
||||
YCM: in.YCM,
|
||||
RadiusCM: in.RadiusCM,
|
||||
Count: in.Count,
|
||||
Label: trimStringPtr(in.Label),
|
||||
PlantedAt: in.PlantedAt,
|
||||
}
|
||||
if p.PlantedAt == nil {
|
||||
today := s.now().UTC().Format(dateLayout)
|
||||
p.PlantedAt = &today
|
||||
}
|
||||
if err := finalizePlanting(p, o, true); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
created, err := s.store.CreatePlanting(ctx, p)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
gitea-actions
commented
🟠 UpdatePlanting re-validates unchanged position against current object bounds, permanently blocking patches (including soft-remove) after the parent object is resized smaller error-handling · flagged by 1 model
🪰 Gadfly · advisory 🟠 **UpdatePlanting re-validates unchanged position against current object bounds, permanently blocking patches (including soft-remove) after the parent object is resized smaller**
_error-handling · flagged by 1 model_
- `internal/service/plantings.go:107-140` (`UpdatePlanting`) unconditionally re-validates the plop's position against the object's *current* bounds via `finalizePlanting` (line 125), even when the patch touches neither `XCM` nor `YCM`. `finalizeObject` (`internal/service/objects.go:232-266`) lets an editor shrink `WidthCM`/`HeightCM` at any time with no check against existing plantings inside the object. Once an object is resized smaller than an existing plop's stored position, `finalizePlanting…
<sub>🪰 Gadfly · advisory</sub>
|
||||
created.DerivedCount = derivedCount(created.RadiusCM, plant.SpacingCM)
|
||||
return created, nil
|
||||
}
|
||||
|
||||
// UpdatePlanting applies a partial, version-guarded patch to a plop in an object
|
||||
// the actor can edit. On a version mismatch it returns (current, ErrVersionConflict).
|
||||
|
gitea-actions
commented
🟠 UpdatePlanting missing plantable check correctness · flagged by 1 model
🪰 Gadfly · advisory 🟠 **UpdatePlanting missing plantable check**
_correctness · flagged by 1 model_
- **`internal/service/plantings.go:113` — `UpdatePlanting` omits the plantable check.** `CreatePlanting` correctly rejects non-plantable objects (`!o.Plantable → domain.ErrInvalidInput`), but `UpdatePlanting` fetches the object via `objectForRole` and proceeds straight to `finalizePlanting` without ever verifying `o.Plantable`. The PR description explicitly states that UpdatePlanting requires a plantable object ("CreatePlanting / UpdatePlanting / DeletePlanting … Object must be `plantable`"), so…
<sub>🪰 Gadfly · advisory</sub>
|
||||
func (s *Service) UpdatePlanting(ctx context.Context, actorID, plantingID int64, patch PlantingPatch, version int64) (*domain.Planting, error) {
|
||||
pl, err := s.store.GetPlanting(ctx, plantingID)
|
||||
if err != nil {
|
||||
return nil, err // ErrNotFound
|
||||
}
|
||||
o, _, err := s.objectForRole(ctx, actorID, pl.ObjectID, roleEditor)
|
||||
|
gitea-actions
commented
🟠 UpdatePlanting always re-validates position bounds against the current object, blocking soft-remove/other edits on plops orphaned outside bounds by a later object resize correctness · flagged by 1 model
🪰 Gadfly · advisory 🟠 **UpdatePlanting always re-validates position bounds against the current object, blocking soft-remove/other edits on plops orphaned outside bounds by a later object resize**
_correctness · flagged by 1 model_
- `internal/service/plantings.go:119-127` (`UpdatePlanting`) unconditionally calls `finalizePlanting(pl, o)` after `applyPlantingPatch`, re-validating the plop's `x_cm`/`y_cm` against the object's *current* `WidthCM`/`HeightCM` even when the patch doesn't touch position (e.g. a pure `removedAt` soft-remove). `internal/service/objects.go:202-207,138-150` lets an editor shrink an object's dimensions via `PATCH /objects/:id` (`applyObjectPatch`) with no check against existing plantings in `finalize…
<sub>🪰 Gadfly · advisory</sub>
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
applyPlantingPatch(pl, patch)
|
||||
// The plant may have changed; the (possibly new) plant must be visible.
|
||||
plant, err := s.visiblePlant(ctx, actorID, pl.PlantID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// Only re-check the object bounds when the position is actually being moved.
|
||||
// A plop left outside its object's bounds by a later object resize must still
|
||||
// be editable (radius, dates, soft-remove) — otherwise it becomes a row you
|
||||
// can neither fix nor remove.
|
||||
checkBounds := patch.XCM != nil || patch.YCM != nil
|
||||
if err := finalizePlanting(pl, o, checkBounds); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
pl.Version = version
|
||||
|
||||
updated, err := s.store.UpdatePlanting(ctx, pl)
|
||||
if err != nil {
|
||||
if errors.Is(err, domain.ErrVersionConflict) && updated != nil {
|
||||
// Enrich the current row with its own plant's derived count.
|
||||
s.enrichDerived(ctx, updated)
|
||||
}
|
||||
return updated, err
|
||||
}
|
||||
updated.DerivedCount = derivedCount(updated.RadiusCM, plant.SpacingCM)
|
||||
|
gitea-actions
commented
🟡 DeletePlanting missing plantable check correctness · flagged by 1 model
🪰 Gadfly · advisory 🟡 **DeletePlanting missing plantable check**
_correctness · flagged by 1 model_
- **`internal/service/plantings.go:148` — `DeletePlanting` similarly omits the plantable check.** Hard-deleting a planting in a non-plantable object is arguably less problematic (it's cleanup), but the PR description groups Delete with the same plantable requirement. If the intent is to allow cleanup, the PR description should be updated; otherwise add the same guard.
<sub>🪰 Gadfly · advisory</sub>
|
||||
return updated, nil
|
||||
}
|
||||
|
||||
// DeletePlanting hard-deletes a plop in an object the actor can edit. (Soft
|
||||
// removal — "clear bed" / harvested — sets removed_at via UpdatePlanting.)
|
||||
func (s *Service) DeletePlanting(ctx context.Context, actorID, plantingID int64) error {
|
||||
pl, err := s.store.GetPlanting(ctx, plantingID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if _, _, err := s.objectForRole(ctx, actorID, pl.ObjectID, roleEditor); err != nil {
|
||||
return err
|
||||
}
|
||||
return s.store.DeletePlanting(ctx, plantingID)
|
||||
}
|
||||
|
||||
// visiblePlant loads a plant the actor may reference in a planting: a built-in or
|
||||
// one the actor owns. An unknown plant or another user's plant is an invalid
|
||||
// reference (ErrInvalidInput) rather than leaking existence.
|
||||
func (s *Service) visiblePlant(ctx context.Context, actorID, plantID int64) (*domain.Plant, error) {
|
||||
p, err := s.store.GetPlant(ctx, plantID)
|
||||
if errors.Is(err, domain.ErrNotFound) {
|
||||
return nil, domain.ErrInvalidInput
|
||||
}
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
gitea-actions
commented
🟡 enrichDerived swallows GetPlant error; 409 current row may carry derivedCount 0 inconsistently with normal GET error-handling · flagged by 3 models
🪰 Gadfly · advisory 🟡 **enrichDerived swallows GetPlant error; 409 current row may carry derivedCount 0 inconsistently with normal GET**
_error-handling · flagged by 3 models_
- **`internal/service/plantings.go:174` — `enrichDerived` swallows `GetPlant` error; 409 current row may carry `derivedCount: 0`.** On the version-conflict path (lines 132-134), `enrichDerived` ignores the `GetPlant` error (best-effort by design), so the returned `current` row's `DerivedCount` stays 0 if the plant lookup fails — inconsistent with a normal GET that would carry a positive value. Low severity; the degraded response carries no signal of the swallowed failure.
<sub>🪰 Gadfly · advisory</sub>
|
||||
}
|
||||
if p.OwnerID != nil && *p.OwnerID != actorID {
|
||||
return nil, domain.ErrInvalidInput
|
||||
}
|
||||
return p, nil
|
||||
}
|
||||
|
||||
// enrichDerived best-effort sets p.DerivedCount from its plant's spacing (used
|
||||
// when we don't already hold the plant, e.g. a conflict's current row).
|
||||
func (s *Service) enrichDerived(ctx context.Context, p *domain.Planting) {
|
||||
plant, err := s.store.GetPlant(ctx, p.PlantID)
|
||||
if err == nil {
|
||||
p.DerivedCount = derivedCount(p.RadiusCM, plant.SpacingCM)
|
||||
}
|
||||
}
|
||||
|
||||
// applyPlantingPatch mutates pl with each provided patch field.
|
||||
func applyPlantingPatch(pl *domain.Planting, p PlantingPatch) {
|
||||
if p.PlantID != nil {
|
||||
pl.PlantID = *p.PlantID
|
||||
}
|
||||
if p.XCM != nil {
|
||||
pl.XCM = *p.XCM
|
||||
}
|
||||
if p.YCM != nil {
|
||||
pl.YCM = *p.YCM
|
||||
}
|
||||
if p.RadiusCM != nil {
|
||||
pl.RadiusCM = *p.RadiusCM
|
||||
}
|
||||
if p.SetCount {
|
||||
pl.Count = p.Count
|
||||
}
|
||||
if p.SetLabel {
|
||||
pl.Label = trimStringPtr(p.Label)
|
||||
}
|
||||
if p.SetPlantedAt {
|
||||
|
gitea-actions
commented
⚪ finalizePlanting validation order tucks date checks after unrelated count/label checks, hurting readability maintainability · flagged by 1 model
🪰 Gadfly · advisory ⚪ **finalizePlanting validation order tucks date checks after unrelated count/label checks, hurting readability**
_maintainability · flagged by 1 model_
- **`internal/service/plantings.go:211` — `finalizePlanting` validation order tucks the date checks (`:229`) last, after unrelated count (`:223`) and label (`:226`) checks.** Confirmed: order is radius → coords → bounds → count → label → dates. Readability nit only; behavior is correct. Not blocking.
<sub>🪰 Gadfly · advisory</sub>
|
||||
pl.PlantedAt = p.PlantedAt
|
||||
}
|
||||
if p.SetRemovedAt {
|
||||
pl.RemovedAt = p.RemovedAt
|
||||
}
|
||||
}
|
||||
|
||||
// finalizePlanting validates a built/merged plop against its parent object.
|
||||
// Shared by create and update so both enforce the same invariants. checkBounds
|
||||
// gates the center-in-object-bounds test: create always checks it, update only
|
||||
// when the position is being moved (so a resize that orphans a plop outside the
|
||||
// shrunken object doesn't block editing/removing it).
|
||||
func finalizePlanting(p *domain.Planting, o *domain.GardenObject, checkBounds bool) error {
|
||||
if !isFinite(p.RadiusCM) || p.RadiusCM <= 0 || p.RadiusCM > maxRadiusCM {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
if !isFinite(p.XCM) || !isFinite(p.YCM) {
|
||||
return domain.ErrInvalidInput
|
||||
|
gitea-actions
commented
🟡 finalizePlanting never checks removedAt is on/after plantedAt, allowing a nonsensical removed-before-planted interval error-handling · flagged by 3 models
🪰 Gadfly · advisory 🟡 **finalizePlanting never checks removedAt is on/after plantedAt, allowing a nonsensical removed-before-planted interval**
_error-handling · flagged by 3 models_
- `internal/service/plantings.go:229-231` (`finalizePlanting`) validates `PlantedAt` and `RemovedAt` individually via `validDatePtr` but never checks `RemovedAt` is on/after `PlantedAt`. Confirmed no such comparison exists anywhere in the service or store layer (`internal/store/plantings.go` just persists both fields as given). A client can soft-remove a plop with `removedAt` earlier than `plantedAt`, silently producing a nonsensical interval.
<sub>🪰 Gadfly · advisory</sub>
|
||||
}
|
||||
// Loose bounds: the plop's CENTER must sit within the object's unrotated
|
||||
// local bounds (origin at object center); the radius may overhang.
|
||||
if checkBounds && (math.Abs(p.XCM) > o.WidthCM/2 || math.Abs(p.YCM) > o.HeightCM/2) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
if p.Count != nil && (*p.Count < 1 || *p.Count > maxExplicitCount) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
if p.Label != nil && len(*p.Label) > maxPlantingLabelLen {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
if !validDatePtr(p.PlantedAt) || !validDatePtr(p.RemovedAt) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
// A plop can't be removed before it was planted (nonsensical interval).
|
||||
|
gitea-actions
commented
🟡 trimStringPtr helper diverges from the inline strings.TrimSpace idiom used elsewhere in the service maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟡 **trimStringPtr helper diverges from the inline strings.TrimSpace idiom used elsewhere in the service**
_maintainability · flagged by 2 models_
- **`internal/service/plantings.go:245` — `trimStringPtr` diverges from the inline `strings.TrimSpace` idiom used elsewhere in the service.** Confirmed: `objects.go:194`/`:225` and `plants.go:141`/`:153`/`:159` all trim inline (`o.Name = strings.TrimSpace(*p.Name)`), whereas `plantings.go` introduces a `trimStringPtr` helper used at two call sites (`:89`, `:199`). Minor consistency nit; harmless either way.
<sub>🪰 Gadfly · advisory</sub>
|
||||
if p.PlantedAt != nil && p.RemovedAt != nil {
|
||||
planted, _ := time.Parse(dateLayout, *p.PlantedAt)
|
||||
removed, _ := time.Parse(dateLayout, *p.RemovedAt)
|
||||
if removed.Before(planted) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// validDatePtr reports whether a nil-or-'YYYY-MM-DD' date pointer is acceptable.
|
||||
func validDatePtr(s *string) bool {
|
||||
if s == nil {
|
||||
return true
|
||||
}
|
||||
_, err := time.Parse(dateLayout, *s)
|
||||
return err == nil
|
||||
}
|
||||
|
||||
// trimStringPtr trims a *string, preserving nil (distinct from empty).
|
||||
func trimStringPtr(s *string) *string {
|
||||
if s == nil {
|
||||
return nil
|
||||
}
|
||||
t := strings.TrimSpace(*s)
|
||||
return &t
|
||||
}
|
||||
@@ -0,0 +1,378 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
)
|
||||
|
||||
// seedBed creates a plantable 200×200 bed centered in a 1000×1000 garden.
|
||||
func seedBed(t *testing.T, s *Service, owner, gardenID int64) *domain.GardenObject {
|
||||
t.Helper()
|
||||
o, err := s.CreateObject(context.Background(), owner, gardenID, ObjectInput{
|
||||
Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 200, HeightCM: 200,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("seed bed: %v", err)
|
||||
}
|
||||
return o
|
||||
}
|
||||
|
||||
// seedOwnPlant creates a custom plant owned by owner with the given spacing.
|
||||
func seedOwnPlant(t *testing.T, s *Service, owner int64, spacingCM float64) *domain.Plant {
|
||||
t.Helper()
|
||||
p, err := s.CreatePlant(context.Background(), owner, PlantInput{
|
||||
Name: "Testroot", Category: domain.CategoryHerb, SpacingCM: spacingCM, Color: "#4a7c3f", Icon: "🌿",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("seed plant: %v", err)
|
||||
}
|
||||
return p
|
||||
}
|
||||
|
||||
func TestDerivedCountFormula(t *testing.T) {
|
||||
cases := []struct {
|
||||
r, sp float64
|
||||
want int
|
||||
}{
|
||||
{0.1, 10, 1}, // tiny radius floors to 1
|
||||
{0, 10, 1}, // zero radius → 1
|
||||
{10, 0, 1}, // zero spacing → 1
|
||||
{10, 10, 3}, // π·100/100 = 3.14 → 3
|
||||
{50, 10, 79}, // π·2500/100 = 78.5 → 79
|
||||
{30, 15, 13}, // π·900/225 = 12.57 → 13
|
||||
}
|
||||
for i, c := range cases {
|
||||
if got := derivedCount(c.r, c.sp); got != c.want {
|
||||
t.Errorf("case %d: derivedCount(%v, %v) = %d, want %d", i, c.r, c.sp, got, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestDerivedCountCapped(t *testing.T) {
|
||||
// A huge radius with a tiny spacing would produce an absurd (32-bit
|
||||
// overflowing) count; it's capped at the same ceiling as a manual override.
|
||||
if got := derivedCount(10_000, 0.1); got != maxExplicitCount {
|
||||
t.Errorf("derivedCount(10000, 0.1) = %d, want cap %d", got, maxExplicitCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreatePlantingDefaultsAndDerivedCount(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
bed := seedBed(t, s, owner, g.ID)
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
|
||||
pl, err := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 10, YCM: -20, RadiusCM: 10,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreatePlanting: %v", err)
|
||||
}
|
||||
if pl.Count != nil {
|
||||
t.Errorf("count = %v, want nil (derived)", *pl.Count)
|
||||
}
|
||||
if pl.DerivedCount != 3 {
|
||||
t.Errorf("derivedCount = %d, want 3", pl.DerivedCount)
|
||||
}
|
||||
if pl.PlantedAt == nil {
|
||||
t.Error("plantedAt should default to today, got nil")
|
||||
}
|
||||
if pl.Version != 1 || pl.ObjectID != bed.ID {
|
||||
t.Errorf("unexpected planting: %+v", pl)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingCountOverrideWins(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
bed := seedBed(t, s, owner, g.ID)
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
|
||||
cnt := 12
|
||||
pl, err := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 50, Count: &cnt,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreatePlanting: %v", err)
|
||||
}
|
||||
if pl.Count == nil || *pl.Count != 12 {
|
||||
t.Errorf("count override = %v, want 12", pl.Count)
|
||||
}
|
||||
// derivedCount is still computed alongside the override (79 for r=50, sp=10).
|
||||
if pl.DerivedCount != 79 {
|
||||
t.Errorf("derivedCount = %d, want 79", pl.DerivedCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdatePlantingMoveResizeAndClearCount(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
bed := seedBed(t, s, owner, g.ID)
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
|
||||
cnt := 5
|
||||
pl, _ := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 10, Count: &cnt,
|
||||
})
|
||||
|
||||
// Move + grow radius, and clear the count override back to derived.
|
||||
nx, ny, nr := 30.0, -40.0, 50.0
|
||||
updated, err := s.UpdatePlanting(context.Background(), owner, pl.ID,
|
||||
PlantingPatch{XCM: &nx, YCM: &ny, RadiusCM: &nr, SetCount: true, Count: nil}, pl.Version)
|
||||
if err != nil {
|
||||
t.Fatalf("UpdatePlanting: %v", err)
|
||||
}
|
||||
if updated.XCM != 30 || updated.YCM != -40 || updated.RadiusCM != 50 {
|
||||
t.Errorf("move/resize didn't apply: %+v", updated)
|
||||
}
|
||||
if updated.Count != nil {
|
||||
t.Errorf("count should be cleared, got %v", *updated.Count)
|
||||
}
|
||||
if updated.DerivedCount != 79 { // r=50, sp=10
|
||||
t.Errorf("derivedCount after grow = %d, want 79", updated.DerivedCount)
|
||||
}
|
||||
if updated.Version != pl.Version+1 {
|
||||
t.Errorf("version = %d, want %d", updated.Version, pl.Version+1)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingNonPlantableObjectRejected(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
|
||||
tree, _ := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
|
||||
Kind: domain.KindTree, XCM: 500, YCM: 500, WidthCM: 100, HeightCM: 100,
|
||||
})
|
||||
if _, err := s.CreatePlanting(context.Background(), owner, tree.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
}); !errors.Is(err, domain.ErrInvalidInput) {
|
||||
t.Errorf("planting into a tree err = %v, want ErrInvalidInput", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingForeignPlantRejectedBuiltinAllowed(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
bob := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
bed := seedBed(t, s, owner, g.ID)
|
||||
|
||||
// Bob's custom plant is invisible to owner → invalid reference.
|
||||
bobPlant := seedOwnPlant(t, s, bob, 10)
|
||||
if _, err := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: bobPlant.ID, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
}); !errors.Is(err, domain.ErrInvalidInput) {
|
||||
t.Errorf("foreign plant err = %v, want ErrInvalidInput", err)
|
||||
}
|
||||
// An unknown plant id is likewise invalid.
|
||||
if _, err := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: 999999, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
}); !errors.Is(err, domain.ErrInvalidInput) {
|
||||
t.Errorf("unknown plant err = %v, want ErrInvalidInput", err)
|
||||
}
|
||||
// A built-in plant is plantable by anyone.
|
||||
builtins, _ := s.ListPlants(context.Background(), owner)
|
||||
if _, err := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: builtins[0].ID, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
}); err != nil {
|
||||
t.Errorf("planting a built-in should work: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingBoundsCheck(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) // 200×200 → local bounds ±100
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
|
||||
// Center outside the object's local bounds → rejected (radius may overhang,
|
||||
// but the center may not).
|
||||
if _, err := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 150, YCM: 0, RadiusCM: 10,
|
||||
}); !errors.Is(err, domain.ErrInvalidInput) {
|
||||
t.Errorf("out-of-bounds center err = %v, want ErrInvalidInput", err)
|
||||
}
|
||||
// On the edge is allowed.
|
||||
if _, err := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 100, YCM: -100, RadiusCM: 30,
|
||||
}); err != nil {
|
||||
t.Errorf("edge-of-bounds center should be allowed: %v", err)
|
||||
}
|
||||
// Non-positive radius rejected.
|
||||
if _, err := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 0,
|
||||
}); !errors.Is(err, domain.ErrInvalidInput) {
|
||||
t.Errorf("zero radius err = %v, want ErrInvalidInput", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingSoftRemoveLeavesFull(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
bed := seedBed(t, s, owner, g.ID)
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
|
||||
pl, _ := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
})
|
||||
|
||||
// It shows in /full, with a derived count.
|
||||
full, _ := s.GardenFull(context.Background(), owner, g.ID)
|
||||
if len(full.Plantings) != 1 {
|
||||
t.Fatalf("full.Plantings = %d, want 1", len(full.Plantings))
|
||||
}
|
||||
if full.Plantings[0].DerivedCount != 3 {
|
||||
t.Errorf("full derivedCount = %d, want 3", full.Plantings[0].DerivedCount)
|
||||
}
|
||||
if len(full.Plants) != 1 {
|
||||
t.Errorf("full.Plants = %d, want 1 referenced plant", len(full.Plants))
|
||||
}
|
||||
|
||||
// Soft-remove → it leaves /full. (On/after today's default planted_at.)
|
||||
rm := "2030-06-01"
|
||||
if _, err := s.UpdatePlanting(context.Background(), owner, pl.ID,
|
||||
PlantingPatch{SetRemovedAt: true, RemovedAt: &rm}, pl.Version); err != nil {
|
||||
t.Fatalf("soft-remove: %v", err)
|
||||
}
|
||||
full, _ = s.GardenFull(context.Background(), owner, g.ID)
|
||||
if len(full.Plantings) != 0 {
|
||||
t.Errorf("removed plop still in /full: %d", len(full.Plantings))
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingRemovedBeforePlantedRejected(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
bed := seedBed(t, s, owner, g.ID)
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
|
||||
// planted_at defaults to today; a removed_at in the distant past is invalid.
|
||||
pl, _ := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
})
|
||||
past := "2000-01-01"
|
||||
if _, err := s.UpdatePlanting(context.Background(), owner, pl.ID,
|
||||
PlantingPatch{SetRemovedAt: true, RemovedAt: &past}, pl.Version); !errors.Is(err, domain.ErrInvalidInput) {
|
||||
t.Errorf("removed-before-planted err = %v, want ErrInvalidInput", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdatePlantingEditableAfterObjectShrinks(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) // 200×200 → local bounds ±100
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
|
||||
// A plop near the bed's edge.
|
||||
pl, _ := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 90, YCM: 0, RadiusCM: 10,
|
||||
})
|
||||
|
||||
// Shrink the bed to 100×100 (local bounds ±50): the plop's center (90) is now
|
||||
// outside the object.
|
||||
w, h := 100.0, 100.0
|
||||
if _, err := s.UpdateObject(context.Background(), owner, bed.ID, ObjectPatch{WidthCM: &w, HeightCM: &h}, bed.Version); err != nil {
|
||||
t.Fatalf("shrink bed: %v", err)
|
||||
}
|
||||
|
||||
// Editing the out-of-bounds plop without moving it (radius here) must still
|
||||
// work — otherwise it'd be un-removable.
|
||||
nr := 15.0
|
||||
updated, err := s.UpdatePlanting(context.Background(), owner, pl.ID, PlantingPatch{RadiusCM: &nr}, pl.Version)
|
||||
if err != nil {
|
||||
t.Fatalf("radius edit on orphaned plop should work: %v", err)
|
||||
}
|
||||
// But actually moving it still enforces the new bounds.
|
||||
outside := 90.0
|
||||
if _, err := s.UpdatePlanting(context.Background(), owner, pl.ID, PlantingPatch{XCM: &outside}, updated.Version); !errors.Is(err, domain.ErrInvalidInput) {
|
||||
t.Errorf("moving to out-of-bounds should be rejected: %v", err)
|
||||
}
|
||||
// Moving it back inside works.
|
||||
inside := 10.0
|
||||
if _, err := s.UpdatePlanting(context.Background(), owner, pl.ID, PlantingPatch{XCM: &inside}, updated.Version); err != nil {
|
||||
t.Errorf("moving in-bounds should work: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingVersionConflict(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
bed := seedBed(t, s, owner, g.ID)
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
pl, _ := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
})
|
||||
|
||||
nr := 20.0
|
||||
if _, err := s.UpdatePlanting(context.Background(), owner, pl.ID, PlantingPatch{RadiusCM: &nr}, pl.Version); err != nil {
|
||||
t.Fatalf("first update: %v", err)
|
||||
}
|
||||
current, err := s.UpdatePlanting(context.Background(), owner, pl.ID, PlantingPatch{RadiusCM: &nr}, pl.Version)
|
||||
if !errors.Is(err, domain.ErrVersionConflict) {
|
||||
t.Fatalf("stale update err = %v, want ErrVersionConflict", err)
|
||||
}
|
||||
if current == nil || current.Version != 2 {
|
||||
t.Fatalf("conflict didn't return current row: %+v", current)
|
||||
}
|
||||
if current.DerivedCount < 1 {
|
||||
t.Errorf("conflict current row should carry a derived count, got %d", current.DerivedCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlantingCrossUserIsNotFound(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
bob := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
bed := seedBed(t, s, owner, g.ID)
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
pl, _ := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
})
|
||||
|
||||
nr := 5.0
|
||||
if _, err := s.UpdatePlanting(context.Background(), bob, pl.ID, PlantingPatch{RadiusCM: &nr}, pl.Version); !errors.Is(err, domain.ErrNotFound) {
|
||||
t.Errorf("bob update err = %v, want ErrNotFound", err)
|
||||
}
|
||||
if err := s.DeletePlanting(context.Background(), bob, pl.ID); !errors.Is(err, domain.ErrNotFound) {
|
||||
t.Errorf("bob delete err = %v, want ErrNotFound", err)
|
||||
}
|
||||
// Bob can't place into owner's bed either.
|
||||
if _, err := s.CreatePlanting(context.Background(), bob, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
}); !errors.Is(err, domain.ErrNotFound) {
|
||||
t.Errorf("bob create err = %v, want ErrNotFound", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeletePlanting(t *testing.T) {
|
||||
s := newTestService(t, openConfig())
|
||||
owner := seedUser(t, s, "[email protected]")
|
||||
g := seedGarden(t, s, owner)
|
||||
bed := seedBed(t, s, owner, g.ID)
|
||||
plant := seedOwnPlant(t, s, owner, 10)
|
||||
pl, _ := s.CreatePlanting(context.Background(), owner, bed.ID, PlantingInput{
|
||||
PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 10,
|
||||
})
|
||||
|
||||
if err := s.DeletePlanting(context.Background(), owner, pl.ID); err != nil {
|
||||
t.Fatalf("DeletePlanting: %v", err)
|
||||
}
|
||||
full, _ := s.GardenFull(context.Background(), owner, g.ID)
|
||||
if len(full.Plantings) != 0 {
|
||||
t.Errorf("planting still present after delete: %d", len(full.Plantings))
|
||||
}
|
||||
}
|
||||
@@ -20,9 +20,11 @@ import (
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/store"
|
||||
)
|
||||
|
||||
// timeLayout is the ISO-8601 UTC format used for every timestamp pansy stores.
|
||||
// It matches the schema's strftime('%Y-%m-%dT%H:%M:%SZ') so string comparison
|
||||
// (e.g. session expiry) is equivalent to time comparison.
|
||||
// timeLayout is the ISO-8601 UTC format used for every full timestamp pansy
|
||||
// stores (created_at/updated_at/expires_at). It matches the schema's
|
||||
// strftime('%Y-%m-%dT%H:%M:%SZ') so string comparison (e.g. session expiry) is
|
||||
// equivalent to time comparison. Date-only fields (planting planted_at/
|
||||
// removed_at) use dateLayout instead.
|
||||
const timeLayout = "2006-01-02T15:04:05Z"
|
||||
|
||||
// sessionTTL is how long a session lives from its last use (sliding expiry).
|
||||
|
||||
@@ -2,6 +2,8 @@ package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
@@ -26,8 +28,7 @@ func scanPlanting(s scanner) (*domain.Planting, error) {
|
||||
|
||||
// ListActivePlantingsForGarden returns every currently-planted plop (removed_at
|
||||
// IS NULL) across all objects in a garden — the editor's one-shot load. Always a
|
||||
// non-nil slice (empty until plantings CRUD lands in #14). Plop CRUD itself is
|
||||
// #14; this is only the /full read side.
|
||||
// non-nil slice. The service fills each row's DerivedCount; this is the raw read.
|
||||
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
|
||||
@@ -54,3 +55,78 @@ func (d *DB) ListActivePlantingsForGarden(ctx context.Context, gardenID int64) (
|
||||
}
|
||||
return plantings, nil
|
||||
}
|
||||
|
||||
// GetPlanting returns the planting with the given id, or domain.ErrNotFound.
|
||||
func (d *DB) GetPlanting(ctx context.Context, id int64) (*domain.Planting, error) {
|
||||
p, err := scanPlanting(d.sql.QueryRowContext(ctx,
|
||||
`SELECT `+plantingColumns+` FROM plantings WHERE id = ?`, id))
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return nil, domain.ErrNotFound
|
||||
}
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: get planting: %w", err)
|
||||
}
|
||||
return p, nil
|
||||
}
|
||||
|
||||
// CreatePlanting inserts a plop (fields already validated by the service) and
|
||||
// returns the stored row. removed_at is always NULL on create — a new plop is
|
||||
// active; "clear bed" sets removed_at later via UpdatePlanting.
|
||||
func (d *DB) CreatePlanting(ctx context.Context, p *domain.Planting) (*domain.Planting, error) {
|
||||
created, err := scanPlanting(d.sql.QueryRowContext(ctx,
|
||||
`INSERT INTO plantings (object_id, plant_id, x_cm, y_cm, radius_cm, count, label, planted_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
|
||||
RETURNING `+plantingColumns,
|
||||
p.ObjectID, p.PlantID, p.XCM, p.YCM, p.RadiusCM, p.Count, p.Label, p.PlantedAt,
|
||||
))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: insert planting: %w", err)
|
||||
}
|
||||
return created, nil
|
||||
}
|
||||
|
||||
// UpdatePlanting applies a version-guarded update of all mutable columns (the
|
||||
// service merges partial patches first). Returns the updated row, or
|
||||
// (current row, ErrVersionConflict) / ErrNotFound — the same contract as the
|
||||
// other mutable resources.
|
||||
func (d *DB) UpdatePlanting(ctx context.Context, p *domain.Planting) (*domain.Planting, error) {
|
||||
updated, err := scanPlanting(d.sql.QueryRowContext(ctx,
|
||||
`UPDATE plantings
|
||||
SET plant_id = ?, x_cm = ?, y_cm = ?, radius_cm = ?, count = ?, label = ?,
|
||||
planted_at = ?, removed_at = ?,
|
||||
version = version + 1,
|
||||
updated_at = strftime('%Y-%m-%dT%H:%M:%SZ', 'now')
|
||||
WHERE id = ? AND version = ?
|
||||
RETURNING `+plantingColumns,
|
||||
p.PlantID, p.XCM, p.YCM, p.RadiusCM, p.Count, p.Label, p.PlantedAt, p.RemovedAt,
|
||||
p.ID, p.Version,
|
||||
))
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
current, gerr := d.GetPlanting(ctx, p.ID)
|
||||
if gerr != nil {
|
||||
return nil, gerr
|
||||
}
|
||||
return current, domain.ErrVersionConflict
|
||||
}
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("store: update planting: %w", err)
|
||||
}
|
||||
return updated, nil
|
||||
}
|
||||
|
||||
// DeletePlanting hard-deletes a plop (for mistakes; "removed/harvested" flows set
|
||||
// removed_at instead). Returns domain.ErrNotFound if no row was deleted.
|
||||
func (d *DB) DeletePlanting(ctx context.Context, id int64) error {
|
||||
res, err := d.sql.ExecContext(ctx, `DELETE FROM plantings WHERE id = ?`, id)
|
||||
if err != nil {
|
||||
return fmt.Errorf("store: delete planting: %w", err)
|
||||
}
|
||||
n, err := res.RowsAffected()
|
||||
if err != nil {
|
||||
return fmt.Errorf("store: planting delete rows: %w", err)
|
||||
}
|
||||
if n == 0 {
|
||||
return domain.ErrNotFound
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -26,8 +26,8 @@ func scanPlant(s scanner) (*domain.Plant, error) {
|
||||
|
||||
// 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.
|
||||
// non-nil slice (empty when the garden has no active plantings). This is the
|
||||
// /full read side; full plant CRUD/seeding lives in plants.go / plantings.go.
|
||||
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
|
||||
|
||||
🟡 ShouldBindJSON error swallowed by fixed misleading message in createPlanting
error-handling · flagged by 2 models
internal/api/plantings.go:83—createPlantingswallows the actualShouldBindJSONerror and always emits a fixed message. If the request body contains malformed JSON, wrong types (e.g.,"plantId": "abc"), or syntax errors, the client still receives "invalid planting: a plantId is required", which is misleading and makes debugging harder. Fix: inspecterr(e.g. witherrors.Asforvalidator.ValidationErrors/json.UnmarshalTypeError) and return the underlying detail, or…🪰 Gadfly · advisory