From 5bdaf218287dbd7204e3a79a3a4ee1a1c8bff00a Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Tue, 21 Jul 2026 20:39:43 -0400 Subject: [PATCH] Address Gadfly findings on #88 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cover the plantId filter's out-of-range branch (0, -1), not just the non-numeric one — the handler rejects id < 1. - Add the documented negative-remaining case: over-planting a lot (57 against 50 bought) drives remaining to -7 through the HTTP surface. The number is a derived truth about what's committed, not a floor at zero, and that's the signal a gardener wants. - Rename objID → oid in the remaining test to match the package convention (shares_test etc.). Not taken: relocating createPlantAPI to plants_test.go — it's shared with #89's branch and the move is cleanest once both land (consolidating with that branch's makeFillPlant), noted on both PRs. The 409 "current" assertion style is deliberate and reads clearly; matching journal_test.go's exact phrasing isn't worth a divergence churn. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ --- internal/api/seed_lots_test.go | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/internal/api/seed_lots_test.go b/internal/api/seed_lots_test.go index 8260717..332a353 100644 --- a/internal/api/seed_lots_test.go +++ b/internal/api/seed_lots_test.go @@ -89,8 +89,12 @@ func TestSeedLotCrudAPI(t *testing.T) { if n := len(decodeList(t, w.Body.Bytes())); n != 0 { t.Errorf("filter by a plant with no lots returned %d, want 0", n) } - if w := doJSON(t, r, http.MethodGet, "/api/v1/seed-lots?plantId=nope", nil, cookie); w.Code != http.StatusBadRequest { - t.Errorf("bad plantId = %d, want 400", w.Code) + // A bad plantId filter is 400, whether non-numeric or out of range — the + // handler rejects id < 1, not just unparseable strings. + for _, bad := range []string{"nope", "0", "-1"} { + if w := doJSON(t, r, http.MethodGet, "/api/v1/seed-lots?plantId="+bad, nil, cookie); w.Code != http.StatusBadRequest { + t.Errorf("plantId=%q filter = %d, want 400", bad, w.Code) + } } // PATCH with the current version. @@ -156,10 +160,10 @@ func TestSeedLotRemainingIsDerivedAPI(t *testing.T) { if w.Code != http.StatusCreated { t.Fatalf("create object: status %d, body %s", w.Code, w.Body.String()) } - objID := int64(decodeMap(t, w.Body.Bytes())["id"].(float64)) + oid := int64(decodeMap(t, w.Body.Bytes())["id"].(float64)) // Plant 12 of them against the lot. - w = doJSON(t, r, http.MethodPost, objectPath(objID)+"/plantings", map[string]any{ + w = doJSON(t, r, http.MethodPost, objectPath(oid)+"/plantings", map[string]any{ "plantId": plantID, "xCm": 0, "yCm": 0, "radiusCm": 20, "count": 12, "seedLotId": lotID, }, cookie) if w.Code != http.StatusCreated { @@ -174,6 +178,21 @@ func TestSeedLotRemainingIsDerivedAPI(t *testing.T) { if rem, ok := got["remaining"].(float64); !ok || rem != 38 { t.Errorf("remaining = %v, want 38 (50 bought - 12 planted): %s", got["remaining"], w.Body.String()) } + + // Over-plant it: 45 more (57 total against 50 bought) drives remaining + // NEGATIVE. That's deliberate — the number is a derived truth about what + // you've committed, not a floor clamped at zero, and "you've planted more than + // you bought" is exactly the signal a gardener wants rather than a hidden -7. + w = doJSON(t, r, http.MethodPost, objectPath(oid)+"/plantings", map[string]any{ + "plantId": plantID, "xCm": 40, "yCm": 40, "radiusCm": 20, "count": 45, "seedLotId": lotID, + }, cookie) + if w.Code != http.StatusCreated { + t.Fatalf("over-plant: status %d, body %s", w.Code, w.Body.String()) + } + w = doJSON(t, r, http.MethodGet, seedLotPath(lotID), nil, cookie) + if rem, ok := decodeMap(t, w.Body.Bytes())["remaining"].(float64); !ok || rem != -7 { + t.Errorf("remaining after over-planting = %v, want -7 (50 - 57)", rem) + } } // TestSeedLotsArePrivateAPI checks the ACL through the router.