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": "2026-07-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"`
|
||||
}
|
||||
|
||||
@@ -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,251 @@
|
||||
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 radius
|
||||
// or spacing collapses to the floor of 1.
|
||||
func derivedCount(radiusCM, spacingCM float64) int {
|
||||
|
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>
|
||||
if radiusCM <= 0 || spacingCM <= 0 || math.IsNaN(radiusCM) || math.IsNaN(spacingCM) {
|
||||
return 1
|
||||
}
|
||||
area := math.Pi * radiusCM * radiusCM
|
||||
n := int(math.Round(area / (spacingCM * spacingCM)))
|
||||
|
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>
|
||||
if n < 1 {
|
||||
return 1
|
||||
}
|
||||
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); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
created, err := s.store.CreatePlanting(ctx, p)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
created.DerivedCount = derivedCount(created.RadiusCM, plant.SpacingCM)
|
||||
return created, nil
|
||||
}
|
||||
|
||||
// UpdatePlanting applies a partial, version-guarded patch to a plop in an object
|
||||
|
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>
|
||||
// the actor can edit. On a version mismatch it returns (current, ErrVersionConflict).
|
||||
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
|
||||
}
|
||||
|
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>
|
||||
o, _, err := s.objectForRole(ctx, actorID, pl.ObjectID, roleEditor)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
applyPlantingPatch(pl, patch)
|
||||
|
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>
|
||||
// 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
|
||||
}
|
||||
if err := finalizePlanting(pl, o); 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)
|
||||
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
|
||||
}
|
||||
|
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>
|
||||
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
|
||||
}
|
||||
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) {
|
||||
|
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>
|
||||
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 {
|
||||
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.
|
||||
func finalizePlanting(p *domain.Planting, o *domain.GardenObject) error {
|
||||
|
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>
|
||||
if !isFinite(p.RadiusCM) || p.RadiusCM <= 0 || p.RadiusCM > maxRadiusCM {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
if !isFinite(p.XCM) || !isFinite(p.YCM) {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
// Loose bounds: the plop's CENTER must sit within the object's unrotated
|
||||
// local bounds (origin at object center); the radius may overhang.
|
||||
if 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) {
|
||||
|
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>
|
||||
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 {
|
||||
|
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 s == nil {
|
||||
return nil
|
||||
}
|
||||
t := strings.TrimSpace(*s)
|
||||
return &t
|
||||
}
|
||||
@@ -0,0 +1,314 @@
|
||||
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 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.
|
||||
rm := "2026-07-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 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))
|
||||
}
|
||||
}
|
||||
@@ -2,6 +2,8 @@ package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
|
||||
@@ -54,3 +56,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
|
||||
}
|
||||
|
||||
🟡 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