diff --git a/internal/api/ops.go b/internal/api/ops.go index a59c464..97e0cba 100644 --- a/internal/api/ops.go +++ b/internal/api/ops.go @@ -19,6 +19,24 @@ import ( // service methods `internal/agent/tools.go` calls, so the permission checks and // the one-change-set-per-operation guarantee come along unchanged. +// fillRect is an explicit rectangle in the object's local frame, the alternative +// to a compass name. A named type (not an inline anonymous struct) to match the +// rest of internal/api and so it can carry its own validity check. +type fillRect struct { + MinX float64 `json:"minXCm"` + MinY float64 `json:"minYCm"` + MaxX float64 `json:"maxXCm"` + MaxY float64 `json:"maxYCm"` +} + +// degenerate reports whether the rect encloses no area. Such a rect (including +// the all-zeros an empty `"rect": {}` decodes to) would otherwise slip through +// and plant a single plop at the object's centre — a surprising result for what +// is really malformed input. +func (r fillRect) degenerate() bool { + return r.MaxX <= r.MinX || r.MaxY <= r.MinY +} + // fillRequest is the body for POST /objects/:id/fill. // // A region is given EITHER by compass name ("ne", "south half", "all") or as an @@ -27,14 +45,9 @@ import ( // Exactly one must be supplied — accepting both and silently preferring one // would make a client bug look like a geometry bug. type fillRequest struct { - PlantID int64 `json:"plantId" binding:"required"` - Region string `json:"region"` - Rect *struct { - MinX float64 `json:"minXCm"` - MinY float64 `json:"minYCm"` - MaxX float64 `json:"maxXCm"` - MaxY float64 `json:"maxYCm"` - } `json:"rect"` + PlantID int64 `json:"plantId" binding:"required"` + Region string `json:"region"` + Rect *fillRect `json:"rect"` // SpacingOverrideCM plants tighter or looser than the plant's mature spacing // without editing the catalog entry. SpacingOverrideCM *float64 `json:"spacingOverrideCm"` @@ -62,11 +75,18 @@ func (h *handlers) fillObject(c *gin.Context) { created []domain.Planting err error ) - if named { - created, err = h.svc.FillNamedRegion(c.Request.Context(), actor, id, req.Region, req.PlantID, req.SpacingOverrideCM) - } else { - region := service.Region{MinX: req.Rect.MinX, MinY: req.Rect.MinY, MaxX: req.Rect.MaxX, MaxY: req.Rect.MaxY} + if rect := req.Rect; rect != nil { + // Reject a zero-area rect here rather than let it plant one stray plop. + // (Binding the pointer to `rect` also keeps the deref visibly guarded, + // instead of reading req.Rect.MinX under an invariant from a line above.) + if rect.degenerate() { + writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "rect must enclose a positive area") + return + } + region := service.Region{MinX: rect.MinX, MinY: rect.MinY, MaxX: rect.MaxX, MaxY: rect.MaxY} created, err = h.svc.FillRegion(c.Request.Context(), actor, id, region, req.PlantID, req.SpacingOverrideCM) + } else { + created, err = h.svc.FillNamedRegion(c.Request.Context(), actor, id, req.Region, req.PlantID, req.SpacingOverrideCM) } if err != nil { writeServiceError(c, err) diff --git a/internal/api/ops_test.go b/internal/api/ops_test.go index a505016..2bcd06f 100644 --- a/internal/api/ops_test.go +++ b/internal/api/ops_test.go @@ -61,7 +61,7 @@ func countChangeSets(t *testing.T, r *gin.Engine, cookie *http.Cookie, gardenID func TestFillAndClearAPI(t *testing.T) { r := authEngine(t, localCfg()) cookie := registerAndCookie(t, r, "fill@example.com") - gid, objID, plantID := seedFillableBed(t, r, cookie, 200, 200, 20) + _, objID, plantID := seedFillableBed(t, r, cookie, 200, 200, 20) // Fill by compass name. w := doJSON(t, r, http.MethodPost, fillPath(objID), map[string]any{ @@ -96,7 +96,6 @@ func TestFillAndClearAPI(t *testing.T) { if n := int(decodeMap(t, w.Body.Bytes())["cleared"].(float64)); n != 0 { t.Errorf("second clear removed %d, want 0", n) } - _ = gid } // TestFillRegionSelectionAPI: exactly one of region/rect, and a rect fills only @@ -125,6 +124,20 @@ func TestFillRegionSelectionAPI(t *testing.T) { t.Errorf("bad region name = %d, want 400", w.Code) } + // A zero-area rect is malformed input, not "plant one at the centre". An empty + // `"rect": {}` decodes to all-zeros and must be caught the same way. + for _, rect := range []map[string]any{ + {}, // {} → 0,0,0,0 + {"minXCm": 10, "minYCm": 10, "maxXCm": 10, "maxYCm": 50}, // zero width + {"minXCm": 10, "minYCm": 50, "maxXCm": 50, "maxYCm": 50}, // zero height + {"minXCm": 50, "minYCm": 50, "maxXCm": 10, "maxYCm": 10}, // inverted + } { + if w := doJSON(t, r, http.MethodPost, fillPath(objID), + map[string]any{"plantId": plantID, "rect": rect}, cookie); w.Code != http.StatusBadRequest { + t.Errorf("degenerate rect %v = %d, want 400", rect, w.Code) + } + } + // A rect confined to the NE corner produces plops only there. Local frame: // +x east, -y north. w := doJSON(t, r, http.MethodPost, fillPath(objID), map[string]any{ diff --git a/web/src/editor/ClearBedModal.tsx b/web/src/editor/ClearBedModal.tsx index 81b8e9d..bfc4b7f 100644 --- a/web/src/editor/ClearBedModal.tsx +++ b/web/src/editor/ClearBedModal.tsx @@ -18,12 +18,12 @@ export function ClearBedModal({ onClose: () => void }) { const clear = useClearObject(gardenId) - const n = plopCount return (

- Remove all {n} {n === 1 ? 'plant' : 'plants'} from{' '} + Remove all {plopCount}{' '} + {plopCount === 1 ? 'plant' : 'plants'} from{' '} {objectName}? They're marked removed but kept in history.

@@ -33,7 +33,7 @@ export function ClearBedModal({