Copy a garden (deep duplicate) from the gardens list #46

Merged
steve merged 2 commits from feat/copy-garden into main 2026-07-21 03:58:40 +00:00
14 changed files with 632 additions and 29 deletions
Showing only changes of commit 5dd35c3800 - Show all commits
+1
View File
@@ -52,6 +52,7 @@ POST /auth/register | /auth/login | /auth/logout GET /auth/me GET /auth
GET /auth/oidc/login GET /auth/oidc/callback (authorization-code + PKCE) GET /auth/oidc/login GET /auth/oidc/callback (authorization-code + PKCE)
GET,POST /gardens GET,PATCH,DELETE /gardens/:id GET,POST /gardens GET,PATCH,DELETE /gardens/:id
GET /gardens/:id/full ← one-shot editor load: garden + objects + plantings + referenced plants GET /gardens/:id/full ← one-shot editor load: garden + objects + plantings + referenced plants
POST /gardens/:id/copy ← deep-copy a garden you own (objects + active plops; not shares/link)
POST /gardens/:id/objects PATCH,DELETE /objects/:id POST /gardens/:id/objects PATCH,DELETE /objects/:id
POST /objects/:id/plantings PATCH,DELETE /plantings/:id POST /objects/:id/plantings PATCH,DELETE /plantings/:id
GET,POST /plants PATCH,DELETE /plants/:id (own plants only) GET,POST /plants PATCH,DELETE /plants/:id (own plants only)
+1
View File
@@ -81,6 +81,7 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
gardens.GET("/:id", h.getGarden) gardens.GET("/:id", h.getGarden)
gardens.PATCH("/:id", h.updateGarden) gardens.PATCH("/:id", h.updateGarden)
gardens.DELETE("/:id", h.deleteGarden) gardens.DELETE("/:id", h.deleteGarden)
gardens.POST("/:id/copy", h.copyGarden) // duplicate a garden the actor owns
gardens.GET("/:id/full", h.getGardenFull) // one-shot editor load gardens.GET("/:id/full", h.getGardenFull) // one-shot editor load
gardens.POST("/:id/objects", h.createObject) gardens.POST("/:id/objects", h.createObject)
+28
View File
@@ -103,6 +103,34 @@ func (h *handlers) updateGarden(c *gin.Context) {
c.JSON(http.StatusOK, g) c.JSON(http.StatusOK, g)
} }
// gardenCopyRequest is the (optional) body of POST /gardens/:id/copy. A blank or
// absent name lets the service derive "<source> (copy)".
type gardenCopyRequest struct {
Name string `json:"name"`
}
func (h *handlers) copyGarden(c *gin.Context) {
id, ok := parseIDParam(c, "id")
if !ok {
return
}
// The body is optional — POST with no body means "copy under the default
// name" — so only decode when one was actually sent.
var req gardenCopyRequest
if c.Request.ContentLength != 0 {
Outdated
Review

🟠 Inconsistent optional-body pattern uses ContentLength instead of io.EOF

error-handling, maintainability · flagged by 4 models

  • internal/api/gardens.go:120 — The copyGarden handler checks c.Request.ContentLength != 0 to decide whether to parse an optional JSON body, but the codebase already has an established pattern for this in internal/api/public.go:56: try c.ShouldBindJSON(&req) and ignore io.EOF. The ContentLength guard is both inconsistent with surrounding code and less robust (e.g., chunked encoding or HTTP/2 can leave ContentLength == -1 even with no body, which would cause the handler to incorre…

🪰 Gadfly · advisory

🟠 **Inconsistent optional-body pattern uses ContentLength instead of io.EOF** _error-handling, maintainability · flagged by 4 models_ - `internal/api/gardens.go:120` — The `copyGarden` handler checks `c.Request.ContentLength != 0` to decide whether to parse an optional JSON body, but the codebase already has an established pattern for this in `internal/api/public.go:56`: try `c.ShouldBindJSON(&req)` and ignore `io.EOF`. The `ContentLength` guard is both inconsistent with surrounding code and less robust (e.g., chunked encoding or HTTP/2 can leave `ContentLength == -1` even with no body, which would cause the handler to incorre… <sub>🪰 Gadfly · advisory</sub>
if err := c.ShouldBindJSON(&req); err != nil {
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "name must be a string")
return
}
}
g, err := h.svc.CopyGarden(c.Request.Context(), mustActor(c).ID, id, req.Name)
if err != nil {
writeServiceError(c, err)
return
}
c.JSON(http.StatusCreated, g)
}
func (h *handlers) deleteGarden(c *gin.Context) { func (h *handlers) deleteGarden(c *gin.Context) {
id, ok := parseIDParam(c, "id") id, ok := parseIDParam(c, "id")
if !ok { if !ok {
+66
View File
@@ -181,3 +181,69 @@ func TestGardenUpdateRejectsBadVersion(t *testing.T) {
func gardenPath(id int64) string { func gardenPath(id int64) string {
return "/api/v1/gardens/" + strconv.FormatInt(id, 10) return "/api/v1/gardens/" + strconv.FormatInt(id, 10)
} }
func TestCopyGardenEndpoint(t *testing.T) {
r := authEngine(t, localCfg())
cookie := registerAndCookie(t, r, "[email protected]")
w := doJSON(t, r, http.MethodPost, "/api/v1/gardens",
map[string]any{"name": "Home", "widthCm": 400, "heightCm": 400}, cookie)
if w.Code != http.StatusCreated {
t.Fatalf("create status = %d, body %s", w.Code, w.Body.String())
}
src := decodeMap(t, w.Body.Bytes())
id := int64(src["id"].(float64))
path := "/api/v1/gardens/" + strconv.FormatInt(id, 10) + "/copy"
// No body at all → 201 with the derived name.
w = doJSON(t, r, http.MethodPost, path, nil, cookie)
if w.Code != http.StatusCreated {
t.Fatalf("copy status = %d, body %s", w.Code, w.Body.String())
}
dup := decodeMap(t, w.Body.Bytes())
if dup["name"] != "Home (copy)" {
t.Errorf("name = %v, want 'Home (copy)'", dup["name"])
}
if int64(dup["id"].(float64)) == id {
t.Error("copy reused the source id")
}
if dup["myRole"] != "owner" {
t.Errorf("myRole = %v, want owner", dup["myRole"])
}
// An explicit name is honored.
w = doJSON(t, r, http.MethodPost, path, map[string]any{"name": "Plan B"}, cookie)
if w.Code != http.StatusCreated {
t.Fatalf("named copy status = %d, body %s", w.Code, w.Body.String())
}
if got := decodeMap(t, w.Body.Bytes())["name"]; got != "Plan B" {
t.Errorf("name = %v, want 'Plan B'", got)
}
// Both copies plus the source show up in the list.
w = doJSON(t, r, http.MethodGet, "/api/v1/gardens", nil, cookie)
var list []map[string]any
if err := json.Unmarshal(w.Body.Bytes(), &list); err != nil {
t.Fatalf("decode list: %v", err)
}
if len(list) != 3 {
t.Errorf("list has %d gardens, want 3", len(list))
}
// Unauthenticated → 401; someone else's garden → 404.
if w := doJSON(t, r, http.MethodPost, path, nil, nil); w.Code != http.StatusUnauthorized {
t.Errorf("anon copy status = %d, want 401", w.Code)
}
other := registerAndCookie(t, r, "[email protected]")
if w := doJSON(t, r, http.MethodPost, path, nil, other); w.Code != http.StatusNotFound {
t.Errorf("stranger copy status = %d, want 404", w.Code)
}
// A non-numeric id is a 400, and a malformed body is rejected.
if w := doJSON(t, r, http.MethodPost, "/api/v1/gardens/not-a-number/copy", nil, cookie); w.Code != http.StatusBadRequest {
t.Errorf("bad id status = %d, want 400", w.Code)
}
if w := doJSON(t, r, http.MethodPost, path, map[string]any{"name": 42}, cookie); w.Code != http.StatusBadRequest {
t.Errorf("bad body status = %d, want 400", w.Code)
}
}
+42
View File
@@ -156,6 +156,48 @@ func (s *Service) UpdateGarden(ctx context.Context, actorID, gardenID int64, in
return updated, err return updated, err
} }
// CopyGarden duplicates a garden into a new one owned by the actor, carrying
// over its dimensions, grid settings, objects and currently-planted plops. A
// blank name defaults to "<source> (copy)".
//
// Owner-only, deliberately: a copy's plops keep pointing at the SOURCE's
// plant_ids, and a custom plant is owned by one user. Letting a viewer copy
// someone else's garden would hand them a garden referencing plants that aren't
// in their catalog — and would block the original owner from ever deleting those
// plants (plantings.plant_id is ON DELETE RESTRICT). Copying a shared garden
// needs a plant-cloning policy first; until then the owner's own copy is the
// case that's unambiguously correct.
func (s *Service) CopyGarden(ctx context.Context, actorID, gardenID int64, name string) (*domain.Garden, error) {
src, err := s.requireGardenRole(ctx, actorID, gardenID, roleOwner)
if err != nil {
return nil, err
}
name = strings.TrimSpace(name)
if name == "" {
name = copyName(src.Name)
}
if len(name) > maxGardenNameLen {
return nil, domain.ErrInvalidInput
}
created, err := s.store.CopyGarden(ctx, gardenID, actorID, name)
if err != nil {
return nil, err
}
created.MyRole = roleOwner.String() // the copier owns the copy
return created, nil
}
// copyName derives the default name for a copy. A name already at the length cap
// is trimmed to make room for the suffix, on a rune boundary so the result stays
// valid UTF-8.
func copyName(src string) string {
const suffix = " (copy)"
if room := maxGardenNameLen - len(suffix); len(src) > room {
src = strings.TrimSpace(strings.ToValidUTF8(src[:room], ""))
}
return src + suffix
}
// DeleteGarden removes a garden; only the owner may. // DeleteGarden removes a garden; only the owner may.
func (s *Service) DeleteGarden(ctx context.Context, actorID, gardenID int64) error { func (s *Service) DeleteGarden(ctx context.Context, actorID, gardenID int64) error {
if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleOwner); err != nil { if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleOwner); err != nil {
+281
View File
@@ -6,6 +6,7 @@ import (
"math" "math"
"strings" "strings"
"testing" "testing"
"unicode/utf8"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
) )
@@ -249,3 +250,283 @@ func TestDeleteGarden(t *testing.T) {
t.Errorf("re-delete err = %v, want ErrNotFound", err) t.Errorf("re-delete err = %v, want ErrNotFound", err)
} }
} }
func TestCopyGardenDuplicatesContents(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
src, err := s.CreateGarden(ctx, owner, GardenInput{
Name: "Home", WidthCM: 732, HeightCM: 732, UnitPref: domain.UnitImperial,
Notes: "the real one", GridSizeCM: 30, SnapToGrid: true,
})
if err != nil {
t.Fatalf("CreateGarden: %v", err)
}
bed := seedBed(t, s, owner, src.ID)
plant := seedOwnPlant(t, s, owner, 20)
plop, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{PlantID: plant.ID, XCM: 10, YCM: -20, RadiusCM: 15})
if err != nil {
t.Fatalf("CreatePlanting: %v", err)
}
dup, err := s.CopyGarden(ctx, owner, src.ID, "")
if err != nil {
t.Fatalf("CopyGarden: %v", err)
}
if dup.ID == src.ID {
t.Fatal("copy reused the source id")
}
if dup.Name != "Home (copy)" {
t.Errorf("name = %q, want 'Home (copy)'", dup.Name)
}
if dup.OwnerID != owner {
t.Errorf("owner = %d, want %d", dup.OwnerID, owner)
}
if dup.Version != 1 {
t.Errorf("version = %d, want a fresh 1", dup.Version)
}
if dup.WidthCM != src.WidthCM || dup.HeightCM != src.HeightCM ||
dup.UnitPref != src.UnitPref || dup.Notes != src.Notes ||
dup.GridSizeCM != src.GridSizeCM || dup.SnapToGrid != src.SnapToGrid {
t.Errorf("settings not carried over: %+v vs source %+v", dup, src)
}
full, err := s.GardenFull(ctx, owner, dup.ID)
if err != nil {
t.Fatalf("GardenFull(copy): %v", err)
}
if len(full.Objects) != 1 {
t.Fatalf("copy has %d objects, want 1", len(full.Objects))
}
got := full.Objects[0]
if got.ID == bed.ID {
t.Error("copied object reused the source object id")
}
if got.GardenID != dup.ID {
t.Errorf("copied object garden = %d, want the copy %d", got.GardenID, dup.ID)
}
if got.Kind != bed.Kind || got.XCM != bed.XCM || got.YCM != bed.YCM ||
got.WidthCM != bed.WidthCM || got.HeightCM != bed.HeightCM || got.Plantable != bed.Plantable {
t.Errorf("copied object differs: %+v vs source %+v", got, bed)
}
if len(full.Plantings) != 1 {
t.Fatalf("copy has %d plantings, want 1", len(full.Plantings))
}
gotPlop := full.Plantings[0]
if gotPlop.ID == plop.ID {
t.Error("copied planting reused the source planting id")
}
if gotPlop.ObjectID != got.ID {
t.Errorf("copied planting parent = %d, want the copied object %d", gotPlop.ObjectID, got.ID)
}
if gotPlop.PlantID != plop.PlantID || gotPlop.XCM != plop.XCM || gotPlop.YCM != plop.YCM ||
gotPlop.RadiusCM != plop.RadiusCM {
t.Errorf("copied planting differs: %+v vs source %+v", gotPlop, plop)
}
// The source is untouched by the copy.
srcFull, err := s.GardenFull(ctx, owner, src.ID)
if err != nil {
t.Fatalf("GardenFull(source): %v", err)
}
if len(srcFull.Objects) != 1 || len(srcFull.Plantings) != 1 {
t.Errorf("source changed: %d objects, %d plantings", len(srcFull.Objects), len(srcFull.Plantings))
}
}
func TestCopyGardenSkipsRemovedPlantings(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
src := seedGarden(t, s, owner)
bed := seedBed(t, s, owner, src.ID)
plant := seedOwnPlant(t, s, owner, 20)
kept, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{PlantID: plant.ID, RadiusCM: 10})
Outdated
Review

🟡 Misleadingly-named dead variable kept in TestCopyGardenSkipsRemovedPlantings — it is actually cleared, not kept

maintainability · flagged by 1 model

  • internal/service/gardens_test.go:348 (TestCopyGardenSkipsRemovedPlantings) — the local var kept (declared line 348, silenced via _ = kept at line 364) is misleadingly named and is dead code. It's created, never asserted on, and then discarded just to silence "declared and not used." Worse, the name actively misleads: ClearObjectPlantings (internal/store/plantings.go:76-92), invoked via ClearObject (internal/service/ops.go:221-227), soft-removes every active plop on the object…

🪰 Gadfly · advisory

🟡 **Misleadingly-named dead variable `kept` in TestCopyGardenSkipsRemovedPlantings — it is actually cleared, not kept** _maintainability · flagged by 1 model_ - `internal/service/gardens_test.go:348` (`TestCopyGardenSkipsRemovedPlantings`) — the local var `kept` (declared line 348, silenced via `_ = kept` at line 364) is misleadingly named and is dead code. It's created, never asserted on, and then discarded just to silence "declared and not used." Worse, the name actively misleads: `ClearObjectPlantings` (`internal/store/plantings.go:76-92`), invoked via `ClearObject` (`internal/service/ops.go:221-227`), soft-removes *every* active plop on the object… <sub>🪰 Gadfly · advisory</sub>
if err != nil {
t.Fatalf("CreatePlanting(kept): %v", err)
}
if _, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{PlantID: plant.ID, XCM: 50, RadiusCM: 10}); err != nil {
t.Fatalf("CreatePlanting(cleared): %v", err)
}
// Clear the bed, then re-plant one plop: the copy should carry the active plop
// only, not the soft-removed history.
if _, err := s.ClearObject(ctx, owner, bed.ID); err != nil {
t.Fatalf("ClearObject: %v", err)
}
replanted, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{PlantID: plant.ID, XCM: 25, RadiusCM: 10})
if err != nil {
t.Fatalf("CreatePlanting(replanted): %v", err)
}
_ = kept
Review

🟡 Unused variable suppressed with _ = kept instead of using _ in original assignment

maintainability · flagged by 3 models

  • internal/service/gardens_test.go:364 (in TestCopyGardenSkipsRemovedPlantings) — The test assigns a planting to kept only to suppress it with _ = kept. Using _, err := s.CreatePlanting(...) on that line avoids the unused-variable workaround.

🪰 Gadfly · advisory

🟡 **Unused variable suppressed with _ = kept instead of using _ in original assignment** _maintainability · flagged by 3 models_ - `internal/service/gardens_test.go:364` (in `TestCopyGardenSkipsRemovedPlantings`) — The test assigns a planting to `kept` only to suppress it with `_ = kept`. Using `_, err := s.CreatePlanting(...)` on that line avoids the unused-variable workaround. <sub>🪰 Gadfly · advisory</sub>
dup, err := s.CopyGarden(ctx, owner, src.ID, "")
if err != nil {
t.Fatalf("CopyGarden: %v", err)
}
full, err := s.GardenFull(ctx, owner, dup.ID)
if err != nil {
t.Fatalf("GardenFull: %v", err)
}
if len(full.Plantings) != 1 {
t.Fatalf("copy has %d plantings, want only the 1 active one", len(full.Plantings))
}
if full.Plantings[0].XCM != replanted.XCM {
t.Errorf("copied plop x = %v, want the active %v", full.Plantings[0].XCM, replanted.XCM)
}
}
func TestCopyGardenNaming(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
g, err := s.CreateGarden(ctx, owner, GardenInput{Name: "Yard"})
if err != nil {
t.Fatalf("CreateGarden: %v", err)
}
// An explicit name wins, and is trimmed like any other garden name.
dup, err := s.CopyGarden(ctx, owner, g.ID, " Experiment ")
if err != nil {
t.Fatalf("CopyGarden: %v", err)
}
if dup.Name != "Experiment" {
t.Errorf("name = %q, want trimmed 'Experiment'", dup.Name)
}
// An over-long explicit name is rejected rather than silently truncated.
if _, err := s.CopyGarden(ctx, owner, g.ID, strings.Repeat("a", maxGardenNameLen+1)); !errors.Is(err, domain.ErrInvalidInput) {
t.Errorf("long name err = %v, want ErrInvalidInput", err)
}
// A source already at the cap still yields a valid derived name.
long, err := s.CreateGarden(ctx, owner, GardenInput{Name: strings.Repeat("b", maxGardenNameLen)})
if err != nil {
t.Fatalf("CreateGarden(long): %v", err)
}
dupLong, err := s.CopyGarden(ctx, owner, long.ID, "")
if err != nil {
t.Fatalf("CopyGarden(long): %v", err)
}
if len(dupLong.Name) > maxGardenNameLen {
t.Errorf("derived name is %d bytes, over the %d cap", len(dupLong.Name), maxGardenNameLen)
}
if !strings.HasSuffix(dupLong.Name, " (copy)") {
t.Errorf("derived name = %q, want a ' (copy)' suffix", dupLong.Name)
}
}
func TestCopyGardenNameRuneBoundary(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
// A name at the cap made of multi-byte runes: truncating for the suffix must
// not split one and leave invalid UTF-8.
name := strings.Repeat("é", maxGardenNameLen/2) // 2 bytes each == the cap
g, err := s.CreateGarden(ctx, owner, GardenInput{Name: name})
if err != nil {
t.Fatalf("CreateGarden: %v", err)
}
dup, err := s.CopyGarden(ctx, owner, g.ID, "")
if err != nil {
t.Fatalf("CopyGarden: %v", err)
}
if !utf8.ValidString(dup.Name) {
t.Errorf("derived name %q is not valid UTF-8", dup.Name)
}
if len(dup.Name) > maxGardenNameLen {
t.Errorf("derived name is %d bytes, over the %d cap", len(dup.Name), maxGardenNameLen)
}
}
func TestCopyGardenPermissions(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
alice := seedUser(t, s, "[email protected]")
bob := seedUser(t, s, "[email protected]")
g := seedGarden(t, s, alice)
// A stranger can't see the garden at all, let alone copy it.
if _, err := s.CopyGarden(ctx, bob, g.ID, ""); !errors.Is(err, domain.ErrNotFound) {
t.Errorf("stranger copy err = %v, want ErrNotFound", err)
}
// An editor may edit contents but not duplicate the garden (see CopyGarden's
// note on cross-owner plant references).
if _, err := s.AddShare(ctx, alice, g.ID, "[email protected]", domain.RoleEditor); err != nil {
t.Fatalf("AddShare: %v", err)
}
if _, err := s.CopyGarden(ctx, bob, g.ID, ""); !errors.Is(err, domain.ErrForbidden) {
t.Errorf("editor copy err = %v, want ErrForbidden", err)
}
// A missing garden is ErrNotFound.
if _, err := s.CopyGarden(ctx, alice, g.ID+9999, ""); !errors.Is(err, domain.ErrNotFound) {
t.Errorf("missing garden err = %v, want ErrNotFound", err)
}
}
func TestCopyGardenDoesNotInheritPublicLink(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
g := seedGarden(t, s, owner)
link, err := s.EnablePublicShareLink(ctx, owner, g.ID, false)
if err != nil {
t.Fatalf("EnablePublicShareLink: %v", err)
}
if link.Token == "" {
t.Fatal("no token issued")
}
dup, err := s.CopyGarden(ctx, owner, g.ID, "")
if err != nil {
t.Fatalf("CopyGarden: %v", err)
}
// The copy must start private: no token of its own...
dupLink, err := s.GetPublicShareLink(ctx, owner, dup.ID)
if err != nil {
t.Fatalf("GetPublicShareLink(copy): %v", err)
}
if dupLink.Token != "" {
t.Errorf("copy inherited a public token %q", dupLink.Token)
}
// ...and the source's token still resolves to the source, not the copy.
pub, err := s.PublicGarden(ctx, link.Token)
if err != nil {
t.Fatalf("PublicGarden: %v", err)
}
if pub.Garden.ID != g.ID {
t.Errorf("source token resolves to garden %d, want the source %d", pub.Garden.ID, g.ID)
}
}
func TestCopyGardenDoesNotInheritShares(t *testing.T) {
ctx := context.Background()
s := newTestService(t, openConfig())
alice := seedUser(t, s, "[email protected]")
bob := seedUser(t, s, "[email protected]")
g := seedGarden(t, s, alice)
if _, err := s.AddShare(ctx, alice, g.ID, "[email protected]", domain.RoleEditor); err != nil {
t.Fatalf("AddShare: %v", err)
}
dup, err := s.CopyGarden(ctx, alice, g.ID, "")
if err != nil {
t.Fatalf("CopyGarden: %v", err)
}
// Bob keeps his access to the original but gets none to the copy.
if _, err := s.GetGarden(ctx, bob, g.ID); err != nil {
t.Errorf("bob lost access to the source: %v", err)
}
if _, err := s.GetGarden(ctx, bob, dup.ID); !errors.Is(err, domain.ErrNotFound) {
t.Errorf("bob can see the copy: err = %v, want ErrNotFound", err)
}
}
+95
View File
@@ -163,6 +163,101 @@ func (d *DB) DeleteGarden(ctx context.Context, id int64) error {
return nil return nil
} }
// CopyGarden deep-copies garden srcID into a new garden owned by ownerID and
// named name, returning the new row. Objects are copied with their geometry,
// grid settings and z-order, and each object's ACTIVE plantings (removed_at IS
// NULL) are re-parented onto the copied object.
//
// Deliberately not copied:
// - public_token — the share link is a capability granted to the original, so a
// copy must not silently inherit a live public URL;
// - garden_shares — a copy is private to whoever made it;
// - removed plantings — the copy is a fresh layout, not the original's history.
//
// The whole copy runs in one transaction, so a failure part-way leaves no
// half-populated garden behind. Returns domain.ErrNotFound if srcID is gone.
func (d *DB) CopyGarden(ctx context.Context, srcID, ownerID int64, name string) (*domain.Garden, error) {
tx, err := d.sql.BeginTx(ctx, nil)
if err != nil {
return nil, fmt.Errorf("store: begin copy garden tx: %w", err)
}
defer tx.Rollback() //nolint:errcheck // no-op after a successful commit
// INSERT ... SELECT copies the source's own columns; owner and name are
// supplied. If the source is gone the SELECT is empty, nothing is inserted,
// and RETURNING yields no rows.
created, err := scanGarden(tx.QueryRowContext(ctx,
`INSERT INTO gardens (owner_id, name, width_cm, height_cm, unit_pref, notes, grid_size_cm, snap_to_grid)
SELECT ?, ?, width_cm, height_cm, unit_pref, notes, grid_size_cm, snap_to_grid
FROM gardens WHERE id = ?
RETURNING `+gardenColumns,
ownerID, name, srcID,
))
if errors.Is(err, sql.ErrNoRows) {
return nil, domain.ErrNotFound
}
if err != nil {
return nil, fmt.Errorf("store: copy garden: %w", err)
}
// Read the source's objects fully before inserting: SQLite can't have an open
// cursor and a write on the same connection, and the pool is pinned to one
// connection for in-memory DBs.
srcObjects, err := queryObjects(ctx, tx,
`SELECT `+objectColumns+` FROM garden_objects WHERE garden_id = ? ORDER BY id`, srcID)
if err != nil {
return nil, err
}
// objectIDs maps a source object id to its copy, so plantings can be
// re-parented onto the right new object.
objectIDs := make(map[int64]int64, len(srcObjects))
for _, o := range srcObjects {
copied, err := scanObject(tx.QueryRowContext(ctx,
Outdated
Review

🟠 CopyGarden inserts objects/plantings one row at a time inside a single write transaction, serializing all other writers under WAL for O(n) round-trips

maintainability, performance · flagged by 2 models

  • internal/store/gardens.go:216-253CopyGarden executes one INSERT ... RETURNING per source object and one INSERT per active planting, sequentially, inside a single write transaction. There is no cap on how many objects/plantings a garden can hold (maxGardensListed at internal/store/gardens.go:50 only bounds the list endpoint). The DB is opened in WAL mode with busy_timeout(5000) (internal/store/sqlite.go:20-21), so only one writer can proceed app-wide; every other user's wri…

🪰 Gadfly · advisory

🟠 **CopyGarden inserts objects/plantings one row at a time inside a single write transaction, serializing all other writers under WAL for O(n) round-trips** _maintainability, performance · flagged by 2 models_ - `internal/store/gardens.go:216-253` — `CopyGarden` executes one `INSERT ... RETURNING` per source object and one `INSERT` per active planting, sequentially, inside a single write transaction. There is no cap on how many objects/plantings a garden can hold (`maxGardensListed` at `internal/store/gardens.go:50` only bounds the *list* endpoint). The DB is opened in WAL mode with `busy_timeout(5000)` (`internal/store/sqlite.go:20-21`), so only one writer can proceed app-wide; every other user's wri… <sub>🪰 Gadfly · advisory</sub>
`INSERT INTO garden_objects
(garden_id, kind, name, shape, points, x_cm, y_cm, width_cm, height_cm,
rotation_deg, z_index, plantable, color, props, grid_size_cm, snap_to_grid, notes)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
RETURNING `+objectColumns,
created.ID, o.Kind, o.Name, o.Shape, o.Points, o.XCM, o.YCM, o.WidthCM, o.HeightCM,
o.RotationDeg, o.ZIndex, boolToInt(o.Plantable), o.Color, o.Props,
o.GridSizeCM, boolToInt(o.SnapToGrid), o.Notes,
))
if err != nil {
return nil, fmt.Errorf("store: copy object: %w", err)
}
objectIDs[o.ID] = copied.ID
}
srcPlantings, err := queryPlantings(ctx, tx,
`SELECT `+qualifyColumns("pl", plantingColumns)+` FROM plantings pl
JOIN garden_objects o ON o.id = pl.object_id
WHERE o.garden_id = ? AND pl.removed_at IS NULL
ORDER BY pl.id`, srcID)
if err != nil {
return nil, err
}
for _, p := range srcPlantings {
objectID, ok := objectIDs[p.ObjectID]
if !ok {
// Unreachable: every active planting joins an object we just copied.
return nil, fmt.Errorf("store: copy planting %d: source object %d not copied", p.ID, p.ObjectID)
}
if _, err := tx.ExecContext(ctx,
`INSERT INTO plantings (object_id, plant_id, x_cm, y_cm, radius_cm, count, label, planted_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
objectID, p.PlantID, p.XCM, p.YCM, p.RadiusCM, p.Count, p.Label, p.PlantedAt,
); err != nil {
return nil, fmt.Errorf("store: copy planting: %w", err)
}
}
if err := tx.Commit(); err != nil {
return nil, fmt.Errorf("store: commit garden copy: %w", err)
}
return created, nil
}
// GetGardenByPublicToken returns the garden whose public_token matches, or // GetGardenByPublicToken returns the garden whose public_token matches, or
// domain.ErrNotFound. Possession of the token is the capability, so there is no // domain.ErrNotFound. Possession of the token is the capability, so there is no
// owner/actor check here — the service exposes this only for the unauthenticated // owner/actor check here — the service exposes this only for the unauthenticated
+10 -3
View File
@@ -66,10 +66,17 @@ func (d *DB) GetObject(ctx context.Context, id int64) (*domain.GardenObject, err
// ListObjectsForGarden returns a garden's objects, bottom-to-top (z_index then // ListObjectsForGarden returns a garden's objects, bottom-to-top (z_index then
// id). Always a non-nil slice. // id). Always a non-nil slice.
func (d *DB) ListObjectsForGarden(ctx context.Context, gardenID int64) ([]domain.GardenObject, error) { func (d *DB) ListObjectsForGarden(ctx context.Context, gardenID int64) ([]domain.GardenObject, error) {
rows, err := d.sql.QueryContext(ctx, return queryObjects(ctx, d.sql,
`SELECT `+objectColumns+` FROM garden_objects WHERE garden_id = ? ORDER BY z_index, id`, `SELECT `+objectColumns+` FROM garden_objects WHERE garden_id = ? ORDER BY z_index, id`,
gardenID, gardenID)
) }
// queryObjects runs an object query and scans every row. The result set is fully
// drained and the cursor closed before returning, so a caller inside a
// transaction may write on the same connection afterwards. Always a non-nil
// slice.
func queryObjects(ctx context.Context, q queryer, query string, args ...any) ([]domain.GardenObject, error) {
rows, err := q.QueryContext(ctx, query, args...)
if err != nil { if err != nil {
return nil, fmt.Errorf("store: list objects: %w", err) return nil, fmt.Errorf("store: list objects: %w", err)
} }
+11 -23
View File
@@ -30,13 +30,19 @@ func scanPlanting(s scanner) (*domain.Planting, error) {
// IS NULL) across all objects in a garden — the editor's one-shot load. Always a // IS NULL) across all objects in a garden — the editor's one-shot load. Always a
// non-nil slice. The service fills each row's DerivedCount; this is the raw read. // 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) { func (d *DB) ListActivePlantingsForGarden(ctx context.Context, gardenID int64) ([]domain.Planting, error) {
rows, err := d.sql.QueryContext(ctx, return queryPlantings(ctx, d.sql,
`SELECT `+qualifyColumns("pl", plantingColumns)+` FROM plantings pl `SELECT `+qualifyColumns("pl", plantingColumns)+` FROM plantings pl
JOIN garden_objects o ON o.id = pl.object_id JOIN garden_objects o ON o.id = pl.object_id
WHERE o.garden_id = ? AND pl.removed_at IS NULL WHERE o.garden_id = ? AND pl.removed_at IS NULL
ORDER BY pl.id`, ORDER BY pl.id`,
gardenID, gardenID)
) }
// queryPlantings runs a planting query and scans every row. Like queryObjects it
// drains and closes the cursor before returning, so a transaction caller may
// write afterwards. Always a non-nil slice.
func queryPlantings(ctx context.Context, q queryer, query string, args ...any) ([]domain.Planting, error) {
rows, err := q.QueryContext(ctx, query, args...)
if err != nil { if err != nil {
return nil, fmt.Errorf("store: list plantings: %w", err) return nil, fmt.Errorf("store: list plantings: %w", err)
} }
@@ -60,27 +66,9 @@ func (d *DB) ListActivePlantingsForGarden(ctx context.Context, gardenID int64) (
// (removed_at IS NULL). Always a non-nil slice. Used by FillRegion to avoid // (removed_at IS NULL). Always a non-nil slice. Used by FillRegion to avoid
// stacking new plops inside existing ones. // stacking new plops inside existing ones.
func (d *DB) ListActivePlantingsForObject(ctx context.Context, objectID int64) ([]domain.Planting, error) { func (d *DB) ListActivePlantingsForObject(ctx context.Context, objectID int64) ([]domain.Planting, error) {
rows, err := d.sql.QueryContext(ctx, return queryPlantings(ctx, d.sql,
`SELECT `+plantingColumns+` FROM plantings WHERE object_id = ? AND removed_at IS NULL ORDER BY id`, `SELECT `+plantingColumns+` FROM plantings WHERE object_id = ? AND removed_at IS NULL ORDER BY id`,
objectID, objectID)
)
if err != nil {
return nil, fmt.Errorf("store: list object plantings: %w", err)
}
defer rows.Close()
plantings := []domain.Planting{}
for rows.Next() {
p, err := scanPlanting(rows)
if err != nil {
return nil, fmt.Errorf("store: scan planting: %w", err)
}
plantings = append(plantings, *p)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("store: iterate plantings: %w", err)
}
return plantings, nil
} }
// ClearObjectPlantings soft-removes every active plop in an object in one UPDATE // ClearObjectPlantings soft-removes every active plop in an object in one UPDATE
+6
View File
@@ -19,6 +19,12 @@ type scanner interface {
Scan(dest ...any) error Scan(dest ...any) error
} }
// queryer is the read surface shared by *sql.DB and *sql.Tx, so a list helper can
Outdated
Review

Generic queryer interface declared in users.go rather than a shared store file

maintainability · flagged by 1 model

  • internal/store/gardens.go:217 and internal/store/plantings.go:112/:135 — the INSERT INTO garden_objects … and INSERT INTO plantings … column lists are duplicated verbatim between CreateObject/CreatePlanting and the in-transaction copy in CopyGarden. The copy can't call CreateObject/CreatePlanting directly because those bind to d.sql rather than tx, so the duplication is a deliberate tradeoff, but it means a future schema/column change must be edited in two (objects) / t…

🪰 Gadfly · advisory

⚪ **Generic queryer interface declared in users.go rather than a shared store file** _maintainability · flagged by 1 model_ - `internal/store/gardens.go:217` and `internal/store/plantings.go:112`/`:135` — the `INSERT INTO garden_objects …` and `INSERT INTO plantings …` column lists are duplicated verbatim between `CreateObject`/`CreatePlanting` and the in-transaction copy in `CopyGarden`. The copy can't call `CreateObject`/`CreatePlanting` directly because those bind to `d.sql` rather than `tx`, so the duplication is a deliberate tradeoff, but it means a future schema/column change must be edited in two (objects) / t… <sub>🪰 Gadfly · advisory</sub>
// serve both a standalone read and one inside a transaction.
type queryer interface {
QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error)
}
// scanUser reads one users row. is_admin is stored as INTEGER 0/1 (the driver // scanUser reads one users row. is_admin is stored as INTEGER 0/1 (the driver
// returns it as int64, which does not convert straight to bool), so it is read // returns it as int64, which does not convert straight to bool), so it is read
// into an int64 and mapped. // into an int64 and mapped.
@@ -0,0 +1,64 @@
import { useState } from 'react'
import { useNavigate } from '@tanstack/react-router'
import { Alert } from '@/components/ui/Alert'
import { Button } from '@/components/ui/Button'
import { Modal } from '@/components/ui/Modal'
import { TextField } from '@/components/ui/TextField'
import { toast } from '@/components/ui/toast'
import { errorMessage } from '@/lib/api'
import { useCopyGarden, type Garden } from '@/lib/gardens'
/**
* Duplicate a garden under a new name. The name is prefilled with the server's
* own default so what you see is what you get; the beds and everything currently
* planted in them come along, while the source's share link and shares do not.
* On success we land in the copy — the point of copying is to start editing it.
*/
export function CopyGardenModal({ garden, onClose }: { garden: Garden; onClose: () => void }) {
const copy = useCopyGarden()
const navigate = useNavigate()
const [name, setName] = useState(`${garden.name} (copy)`)
Outdated
Review

🟠 Client-side copy-name derivation diverges from server truncation logic

correctness, error-handling, maintainability · flagged by 3 models

  • web/src/components/gardens/CopyGardenModal.tsx:20 — The modal pre-derives the copy name with `${garden.name} (copy)`, duplicating the server's copyName() logic but without the byte-cap truncation. If the source name is long, the prefilled default can exceed maxGardenNameLen and will be rejected by the service when submitted unchanged. The client and server derivations are out of sync. Fix: either share the truncation logic (e.g., a common util) or prefill with a name capped the…

🪰 Gadfly · advisory

🟠 **Client-side copy-name derivation diverges from server truncation logic** _correctness, error-handling, maintainability · flagged by 3 models_ - `web/src/components/gardens/CopyGardenModal.tsx:20` — The modal pre-derives the copy name with `` `${garden.name} (copy)` ``, duplicating the server's `copyName()` logic but without the byte-cap truncation. If the source name is long, the prefilled default can exceed `maxGardenNameLen` and will be rejected by the service when submitted unchanged. The client and server derivations are out of sync. **Fix:** either share the truncation logic (e.g., a common util) or prefill with a name capped the… <sub>🪰 Gadfly · advisory</sub>
const [error, setError] = useState<string | null>(null)
async function onSubmit(e: React.FormEvent) {
e.preventDefault()
setError(null)
try {
const created = await copy.mutateAsync({ id: garden.id, name: name.trim() })
toast.info(`Copied to “${created.name}”.`)
onClose()
navigate({ to: '/gardens/$gardenId', params: { gardenId: String(created.id) } })
} catch (err) {
setError(errorMessage(err, 'Could not copy the garden.'))
}
}
return (
<Modal title="Copy garden" onClose={onClose} busy={copy.isPending}>
<form onSubmit={onSubmit} className="flex flex-col gap-4">
<p className="text-sm text-muted">
Make a copy of <span className="font-medium text-fg">{garden.name}</span> with its beds and
everything planted in them. The copy is private the original's share link and people you've
shared it with aren't carried over.
</p>
<TextField
label="Name"
name="name"
required
autoFocus
value={name}
onChange={(e) => setName(e.target.value)}
/>
{error && <Alert>{error}</Alert>}
<div className="flex justify-end gap-2">
<Button type="button" variant="ghost" onClick={onClose} disabled={copy.isPending}>
Cancel
</Button>
<Button type="submit" disabled={copy.isPending || name.trim() === ''}>
{copy.isPending ? 'Copying' : 'Copy garden'}
</Button>
</div>
</form>
</Modal>
)
}
+8 -2
View File
@@ -5,14 +5,16 @@ import { cardActionClass, cardDangerClass } from '@/components/ui/cardActions'
/** /**
* One garden as a card: the body links into the editor. The footer differs by * One garden as a card: the body links into the editor. The footer differs by
* role — the owner gets Share / Edit / Delete; a recipient sees a "shared · role" * role — the owner gets Share / Copy / Edit / Delete; a recipient sees a
* badge and a Leave action (garden metadata edit + sharing are owner-only). * "shared · role" badge and a Leave action (garden metadata edit, sharing and
* copying are owner-only).
* Ownership is the authoritative ownerId==me check, not the my_role hint. * Ownership is the authoritative ownerId==me check, not the my_role hint.
*/ */
export function GardenCard({ export function GardenCard({
garden, garden,
currentUserId, currentUserId,
onShare, onShare,
onCopy,
onEdit, onEdit,
onDelete, onDelete,
onLeave, onLeave,
@@ -20,6 +22,7 @@ export function GardenCard({
garden: Garden garden: Garden
currentUserId?: number currentUserId?: number
onShare: () => void onShare: () => void
onCopy: () => void
onEdit: () => void onEdit: () => void
onDelete: () => void onDelete: () => void
onLeave: () => void onLeave: () => void
@@ -51,6 +54,9 @@ export function GardenCard({
<button type="button" onClick={onShare} className={cardActionClass}> <button type="button" onClick={onShare} className={cardActionClass}>
Share Share
</button> </button>
<button type="button" onClick={onCopy} className={cardActionClass}>
Copy
</button>
<button type="button" onClick={onEdit} className={cardActionClass}> <button type="button" onClick={onEdit} className={cardActionClass}>
Edit Edit
</button> </button>
+14
View File
@@ -86,6 +86,20 @@ export function useUpdateGarden() {
}) })
} }
/**
* Duplicate a garden the actor owns, including its objects and current plantings.
* An omitted/blank name lets the server derive "<source> (copy)". The copy does
* not inherit the source's public link or shares.
*/
export function useCopyGarden() {
const qc = useQueryClient()
return useMutation({
mutationFn: async ({ id, name }: { id: number; name?: string }): Promise<Garden> =>
gardenSchema.parse(await api.post(`/gardens/${id}/copy`, { name: name ?? '' })),
onSuccess: () => qc.invalidateQueries({ queryKey: gardensKey }),
})
}
export function useDeleteGarden() { export function useDeleteGarden() {
const qc = useQueryClient() const qc = useQueryClient()
return useMutation({ return useMutation({
+5 -1
View File
@@ -1,6 +1,7 @@
import { useState } from 'react' import { useState } from 'react'
import { Alert } from '@/components/ui/Alert' import { Alert } from '@/components/ui/Alert'
import { Button } from '@/components/ui/Button' import { Button } from '@/components/ui/Button'
import { CopyGardenModal } from '@/components/gardens/CopyGardenModal'
import { DeleteGardenModal } from '@/components/gardens/DeleteGardenModal' import { DeleteGardenModal } from '@/components/gardens/DeleteGardenModal'
import { GardenCard } from '@/components/gardens/GardenCard' import { GardenCard } from '@/components/gardens/GardenCard'
import { GardenFormModal } from '@/components/gardens/GardenFormModal' import { GardenFormModal } from '@/components/gardens/GardenFormModal'
@@ -10,12 +11,13 @@ import { useMe } from '@/lib/auth'
import { useGardens, type Garden } from '@/lib/gardens' import { useGardens, type Garden } from '@/lib/gardens'
import { usePageTitle } from '@/lib/usePageTitle' import { usePageTitle } from '@/lib/usePageTitle'
// Which modal is open, if any. edit/delete/share/leave carry the target garden. // Which modal is open, if any. edit/delete/share/copy/leave carry the target garden.
type Dialog = type Dialog =
| { kind: 'create' } | { kind: 'create' }
| { kind: 'edit'; garden: Garden } | { kind: 'edit'; garden: Garden }
| { kind: 'delete'; garden: Garden } | { kind: 'delete'; garden: Garden }
| { kind: 'share'; garden: Garden } | { kind: 'share'; garden: Garden }
| { kind: 'copy'; garden: Garden }
| { kind: 'leave'; garden: Garden } | { kind: 'leave'; garden: Garden }
| null | null
@@ -56,6 +58,7 @@ export function GardensPage() {
garden={g} garden={g}
currentUserId={me.data?.id} currentUserId={me.data?.id}
onShare={() => setDialog({ kind: 'share', garden: g })} onShare={() => setDialog({ kind: 'share', garden: g })}
onCopy={() => setDialog({ kind: 'copy', garden: g })}
onEdit={() => setDialog({ kind: 'edit', garden: g })} onEdit={() => setDialog({ kind: 'edit', garden: g })}
onDelete={() => setDialog({ kind: 'delete', garden: g })} onDelete={() => setDialog({ kind: 'delete', garden: g })}
onLeave={() => setDialog({ kind: 'leave', garden: g })} onLeave={() => setDialog({ kind: 'leave', garden: g })}
@@ -69,6 +72,7 @@ export function GardensPage() {
{dialog?.kind === 'edit' && <GardenFormModal garden={dialog.garden} onClose={close} />} {dialog?.kind === 'edit' && <GardenFormModal garden={dialog.garden} onClose={close} />}
{dialog?.kind === 'delete' && <DeleteGardenModal garden={dialog.garden} onClose={close} />} {dialog?.kind === 'delete' && <DeleteGardenModal garden={dialog.garden} onClose={close} />}
{dialog?.kind === 'share' && <ShareGardenModal garden={dialog.garden} onClose={close} />} {dialog?.kind === 'share' && <ShareGardenModal garden={dialog.garden} onClose={close} />}
{dialog?.kind === 'copy' && <CopyGardenModal garden={dialog.garden} onClose={close} />}
{dialog?.kind === 'leave' && <LeaveGardenModal garden={dialog.garden} onClose={close} />} {dialog?.kind === 'leave' && <LeaveGardenModal garden={dialog.garden} onClose={close} />}
</section> </section>
) )