Address Gadfly review on #10: clearable color/props, column lists, dedup
Build image / build-and-push (push) Successful in 7s
Build image / build-and-push (push) Successful in 7s
Fixes from the PR #28 adversarial review (considered; not graded). Correctness / API - PATCH /objects/:id can now clear nullable color/props back to NULL: the request takes them as json.RawMessage, and ObjectPatch carries an explicit Set flag so an explicit `null` (clear) is distinguished from an absent field (unchanged) — the strongest cross-model finding (6 hits). New test. Maintainability - store/plantings.go + plants.go use explicit qualified column lists (qualifyColumns helper) instead of SELECT *, matching gardens/objects and surviving a future column add. - Consolidated objectKinds + plantableByDefault into one kind→traits map. - objectForRole factors the fetch-then-authorize shared by UpdateObject and DeleteObject; dropped the redundant kind check in CreateObject (finalizeObject is the single validation point). - Request→service mapping via toInput()/toPatch() methods (matches gardens). - Renamed handler gardenFull → getGardenFull (verbNoun); test helper decodeGarden → decodeMap; generic bind-error messages. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi
This commit is contained in:
+41
-28
@@ -15,17 +15,17 @@ const (
|
||||
maxObjectPropsLen = 20_000
|
||||
)
|
||||
|
||||
// objectKinds is the set of valid garden_objects.kind values.
|
||||
var objectKinds = map[string]bool{
|
||||
domain.KindBed: true, domain.KindGrowBag: true, domain.KindContainer: true,
|
||||
domain.KindInGround: true, domain.KindTree: true, domain.KindPath: true, domain.KindStructure: true,
|
||||
}
|
||||
|
||||
// plantableByDefault: beds/bags/containers/in-ground hold plants; trees, paths,
|
||||
// and structures don't. Overridable per object.
|
||||
var plantableByDefault = map[string]bool{
|
||||
domain.KindBed: true, domain.KindGrowBag: true, domain.KindContainer: true, domain.KindInGround: true,
|
||||
domain.KindTree: false, domain.KindPath: false, domain.KindStructure: false,
|
||||
// objectKinds maps each valid garden_objects.kind to its traits. plantable is
|
||||
// the default (beds/bags/containers/in-ground hold plants; trees/paths/
|
||||
// structures don't) and is overridable per object.
|
||||
var objectKinds = map[string]struct{ plantable bool }{
|
||||
domain.KindBed: {plantable: true},
|
||||
domain.KindGrowBag: {plantable: true},
|
||||
domain.KindContainer: {plantable: true},
|
||||
domain.KindInGround: {plantable: true},
|
||||
domain.KindTree: {plantable: false},
|
||||
domain.KindPath: {plantable: false},
|
||||
domain.KindStructure: {plantable: false},
|
||||
}
|
||||
|
||||
// ObjectInput is the payload for creating a garden object. Plantable nil means
|
||||
@@ -48,6 +48,10 @@ type ObjectInput struct {
|
||||
|
||||
// ObjectPatch is a partial update: every field is optional (nil = leave as-is).
|
||||
// Kind and shape are intentionally immutable after creation.
|
||||
//
|
||||
// Color and Props are nullable in the DB, so they use an explicit "set" flag to
|
||||
// tell "clear to NULL" (Set*=true, value nil) apart from "leave unchanged"
|
||||
// (Set*=false) — a plain nil pointer can't express both.
|
||||
type ObjectPatch struct {
|
||||
Name *string
|
||||
XCM *float64
|
||||
@@ -57,7 +61,9 @@ type ObjectPatch struct {
|
||||
RotationDeg *float64
|
||||
ZIndex *int
|
||||
Plantable *bool
|
||||
SetColor bool
|
||||
Color *string
|
||||
SetProps bool
|
||||
Props *string
|
||||
Notes *string
|
||||
}
|
||||
@@ -79,14 +85,13 @@ func (s *Service) CreateObject(ctx context.Context, actorID, gardenID int64, in
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if !objectKinds[in.Kind] {
|
||||
return nil, domain.ErrInvalidInput
|
||||
}
|
||||
// finalizeObject is the single validation point (it rejects an unknown kind),
|
||||
// so an unknown kind here just yields plantable=false before being rejected.
|
||||
shape := in.Shape
|
||||
if shape == "" {
|
||||
shape = domain.ShapeRect
|
||||
}
|
||||
plantable := plantableByDefault[in.Kind]
|
||||
plantable := objectKinds[in.Kind].plantable
|
||||
if in.Plantable != nil {
|
||||
plantable = *in.Plantable
|
||||
}
|
||||
@@ -113,14 +118,25 @@ func (s *Service) CreateObject(ctx context.Context, actorID, gardenID int64, in
|
||||
return s.store.CreateObject(ctx, o)
|
||||
}
|
||||
|
||||
// objectForRole loads an object and enforces that the actor holds at least min
|
||||
// on its owning garden, returning both. A missing object — or one in a garden
|
||||
// the actor can't see — is ErrNotFound (existence masked, same as gardens).
|
||||
func (s *Service) objectForRole(ctx context.Context, actorID, objectID int64, min gardenRole) (*domain.GardenObject, *domain.Garden, error) {
|
||||
o, err := s.store.GetObject(ctx, objectID)
|
||||
if err != nil {
|
||||
return nil, nil, err // ErrNotFound
|
||||
}
|
||||
g, err := s.requireGardenRole(ctx, actorID, o.GardenID, min)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
return o, g, nil
|
||||
}
|
||||
|
||||
// UpdateObject applies a partial, version-guarded patch to an object in a garden
|
||||
// the actor can edit. On a version mismatch it returns (current, ErrVersionConflict).
|
||||
func (s *Service) UpdateObject(ctx context.Context, actorID, objectID int64, patch ObjectPatch, version int64) (*domain.GardenObject, error) {
|
||||
o, err := s.store.GetObject(ctx, objectID)
|
||||
if err != nil {
|
||||
return nil, err // ErrNotFound
|
||||
}
|
||||
g, err := s.requireGardenRole(ctx, actorID, o.GardenID, roleEditor)
|
||||
o, g, err := s.objectForRole(ctx, actorID, objectID, roleEditor)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -136,11 +152,7 @@ func (s *Service) UpdateObject(ctx context.Context, actorID, objectID int64, pat
|
||||
// DeleteObject removes an object (its plantings cascade) from a garden the actor
|
||||
// can edit.
|
||||
func (s *Service) DeleteObject(ctx context.Context, actorID, objectID int64) error {
|
||||
o, err := s.store.GetObject(ctx, objectID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if _, err := s.requireGardenRole(ctx, actorID, o.GardenID, roleEditor); err != nil {
|
||||
if _, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor); err != nil {
|
||||
return err
|
||||
}
|
||||
return s.store.DeleteObject(ctx, objectID)
|
||||
@@ -193,10 +205,11 @@ func applyObjectPatch(o *domain.GardenObject, p ObjectPatch) {
|
||||
if p.Plantable != nil {
|
||||
o.Plantable = *p.Plantable
|
||||
}
|
||||
if p.Color != nil {
|
||||
// Color/Props: Set* distinguishes "clear to NULL" (value nil) from unchanged.
|
||||
if p.SetColor {
|
||||
o.Color = p.Color
|
||||
}
|
||||
if p.Props != nil {
|
||||
if p.SetProps {
|
||||
o.Props = p.Props
|
||||
}
|
||||
if p.Notes != nil {
|
||||
@@ -208,7 +221,7 @@ func applyObjectPatch(o *domain.GardenObject, p ObjectPatch) {
|
||||
// object against its garden. Shared by create and update so both enforce the
|
||||
// same invariants.
|
||||
func finalizeObject(o *domain.GardenObject, g *domain.Garden) error {
|
||||
if !objectKinds[o.Kind] {
|
||||
if _, ok := objectKinds[o.Kind]; !ok {
|
||||
return domain.ErrInvalidInput
|
||||
}
|
||||
// rect/circle only; polygon is reserved in the schema but not supported yet.
|
||||
|
||||
Reference in New Issue
Block a user