API-level tests for the /seed-lots route group #88
@@ -0,0 +1,264 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
func seedLotPath(id int64) string {
|
||||
return "/api/v1/seed-lots/" + strconv.FormatInt(id, 10)
|
||||
}
|
||||
|
||||
// decodeList decodes a bare JSON array body. Seed lot listing returns the array
|
||||
// directly rather than wrapping it (unlike /journal's {"entries": …}), so a
|
||||
// helper that assumed an object would quietly read nothing.
|
||||
func decodeList(t *testing.T, body []byte) []any {
|
||||
|
|
||||
t.Helper()
|
||||
var out []any
|
||||
if err := json.Unmarshal(body, &out); err != nil {
|
||||
t.Fatalf("decode list: %v (%s)", err, body)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// createPlantAPI makes a custom plant and returns its id.
|
||||
func createPlantAPI(t *testing.T, r *gin.Engine, cookie *http.Cookie, name string, spacing float64) int64 {
|
||||
|
gitea-actions
commented
🟡 createPlantAPI plant-creation helper is declared in seed_lots_test.go instead of plants_test.go, where custom-plant creation is currently inlined maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **createPlantAPI plant-creation helper is declared in seed_lots_test.go instead of plants_test.go, where custom-plant creation is currently inlined**
_maintainability · flagged by 1 model_
- **`internal/api/seed_lots_test.go:29` — `createPlantAPI` lives in the wrong file.** A plant-creation helper is plant-domain code, but it's declared in `seed_lots_test.go`. `plants_test.go` currently inlines custom-plant creation (`plants_test.go:46-47` and `85-86`) with a near-identical payload rather than calling a shared helper, so this PR introduces a second, parallel pattern instead of promoting a shared one. Grep confirms no `createPlant`/`createPlantAPI` helper exists elsewhere; `plants_…
<sub>🪰 Gadfly · advisory</sub>
|
||||
t.Helper()
|
||||
w := doJSON(t, r, http.MethodPost, "/api/v1/plants", map[string]any{
|
||||
"name": name, "category": "vegetable", "spacingCm": spacing, "color": "#4a7c3f", "icon": "🌱",
|
||||
}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("create plant %q: status %d, body %s", name, w.Code, w.Body.String())
|
||||
}
|
||||
return int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
|
||||
}
|
||||
|
||||
// TestSeedLotCrudAPI walks the whole seed lot lifecycle over HTTP.
|
||||
//
|
||||
// It exists for the reason CLAUDE.md gives: service tests cannot see a route
|
||||
// that was never registered, or one registered with the wrong :param name. Every
|
||||
// other handler file had a sibling API test; this group did not, which is the
|
||||
// state PATCH/DELETE /journal/:id shipped in — implemented, unit-tested, and
|
||||
// completely unreachable.
|
||||
func TestSeedLotCrudAPI(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
plantID := createPlantAPI(t, r, cookie, "Music Garlic", 15)
|
||||
|
||||
// Create.
|
||||
w := doJSON(t, r, http.MethodPost, "/api/v1/seed-lots", map[string]any{
|
||||
"plantId": plantID, "vendor": "Johnny's", "sku": "2761",
|
||||
"quantity": 100, "unit": "seeds", "packedForYear": 2026, "costCents": 495,
|
||||
}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("create: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
lot := decodeMap(t, w.Body.Bytes())
|
||||
id := int64(lot["id"].(float64))
|
||||
if lot["vendor"] != "Johnny's" || lot["unit"] != "seeds" {
|
||||
t.Errorf("unexpected lot: %+v", lot)
|
||||
}
|
||||
|
||||
// GET by id — the route most likely to be missing or mis-registered.
|
||||
w = doJSON(t, r, http.MethodGet, seedLotPath(id), nil, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("get: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if got := decodeMap(t, w.Body.Bytes()); int64(got["id"].(float64)) != id {
|
||||
t.Errorf("get returned id %v, want %d", got["id"], id)
|
||||
}
|
||||
|
||||
// List, and the ?plantId= filter.
|
||||
w = doJSON(t, r, http.MethodGet, "/api/v1/seed-lots", nil, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("list: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if n := len(decodeList(t, w.Body.Bytes())); n != 1 {
|
||||
t.Fatalf("list returned %d lots, want 1: %s", n, w.Body.String())
|
||||
}
|
||||
|
||||
other := createPlantAPI(t, r, cookie, "Cherokee Purple", 45)
|
||||
w = doJSON(t, r, http.MethodGet, "/api/v1/seed-lots?plantId="+strconv.FormatInt(other, 10), nil, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("filtered list: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if n := len(decodeList(t, w.Body.Bytes())); n != 0 {
|
||||
t.Errorf("filter by a plant with no lots returned %d, want 0", n)
|
||||
}
|
||||
// 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.
|
||||
w = doJSON(t, r, http.MethodPatch, seedLotPath(id), map[string]any{
|
||||
"vendor": "Fedco", "version": lot["version"],
|
||||
}, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("patch: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
updated := decodeMap(t, w.Body.Bytes())
|
||||
if updated["vendor"] != "Fedco" {
|
||||
t.Errorf("vendor = %v, want Fedco", updated["vendor"])
|
||||
}
|
||||
if updated["version"].(float64) != lot["version"].(float64)+1 {
|
||||
t.Errorf("patch didn't bump version: %v -> %v", lot["version"], updated["version"])
|
||||
}
|
||||
|
||||
// A stale version conflicts and carries the current row back, so the client
|
||||
// can rebase without a second request.
|
||||
w = doJSON(t, r, http.MethodPatch, seedLotPath(id), map[string]any{
|
||||
"vendor": "stale", "version": lot["version"],
|
||||
}, cookie)
|
||||
if w.Code != http.StatusConflict {
|
||||
t.Fatalf("stale patch: status %d, want 409", w.Code)
|
||||
}
|
||||
if cur, ok := decodeMap(t, w.Body.Bytes())["current"].(map[string]any); !ok || cur["vendor"] != "Fedco" {
|
||||
t.Errorf("409 body missing the current row: %s", w.Body.String())
|
||||
}
|
||||
|
||||
// DELETE.
|
||||
if w := doJSON(t, r, http.MethodDelete, seedLotPath(id), nil, cookie); w.Code != http.StatusNoContent {
|
||||
t.Fatalf("delete: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodGet, seedLotPath(id), nil, cookie); w.Code != http.StatusNotFound {
|
||||
t.Errorf("get after delete = %d, want 404", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeedLotRemainingIsDerivedAPI checks that `remaining` reflects plantings
|
||||
// through the HTTP surface, not just in the service.
|
||||
//
|
||||
// DESIGN.md makes derivation load-bearing — "a decremented column drifts the
|
||||
// moment a planting is edited behind its back" — so a route that returned a
|
||||
// stored or stale figure would break the invariant silently, and the number is
|
||||
// the whole reason anyone opens the seed shelf.
|
||||
func TestSeedLotRemainingIsDerivedAPI(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
plantID := createPlantAPI(t, r, cookie, "Beans", 10)
|
||||
|
||||
w := doJSON(t, r, http.MethodPost, "/api/v1/seed-lots", map[string]any{
|
||||
"plantId": plantID, "quantity": 50, "unit": "seeds",
|
||||
}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("create lot: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
lotID := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
|
||||
|
||||
gid := createGardenAPI(t, r, cookie, "G")
|
||||
w = doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{
|
||||
"kind": "bed", "widthCm": 100, "heightCm": 100, "plantable": true,
|
||||
}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("create object: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
oid := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
|
||||
|
||||
// Plant 12 of them against the lot.
|
||||
w = doJSON(t, r, http.MethodPost, objectPlantingsPath(oid), map[string]any{
|
||||
"plantId": plantID, "xCm": 0, "yCm": 0, "radiusCm": 20, "count": 12, "seedLotId": lotID,
|
||||
}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("create planting: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
w = doJSON(t, r, http.MethodGet, seedLotPath(lotID), nil, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("get lot: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
got := decodeMap(t, w.Body.Bytes())
|
||||
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, objectPlantingsPath(oid), 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)
|
||||
|
gitea-actions
commented
⚪ Missing status-code check before decoding response body after over-planting, masking the real error on failure error-handling · flagged by 1 model
🪰 Gadfly · advisory ⚪ **Missing status-code check before decoding response body after over-planting, masking the real error on failure**
_error-handling · flagged by 1 model_
- `internal/api/seed_lots_test.go:192-195` — The final GET in `TestSeedLotRemainingIsDerivedAPI` decodes the response body without first checking `w.Code == http.StatusOK`, unlike every other GET/decode pair in this file (e.g. lines 173-176, 160-162). If this request fails (e.g. a 404/500 due to a regression), `decodeMap` (internal/api/gardens_test.go:26-34) will still unmarshal successfully since it only fails on invalid JSON, not missing keys — `["remaining"]` will be absent, the assertion fal…
<sub>🪰 Gadfly · advisory</sub>
|
||||
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.
|
||||
//
|
||||
// Lots are private to the buyer and deliberately never travel with a shared
|
||||
// garden, so another user must not be able to read or edit one. Per the
|
||||
// project's convention, no-access is ErrNotFound rather than ErrForbidden —
|
||||
// existence is masked — so every one of these is a 404, not a 403.
|
||||
func TestSeedLotsArePrivateAPI(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
alice := registerAndCookie(t, r, "[email protected]")
|
||||
bob := registerAndCookie(t, r, "[email protected]")
|
||||
|
||||
plantID := createPlantAPI(t, r, alice, "Alice's garlic", 15)
|
||||
w := doJSON(t, r, http.MethodPost, "/api/v1/seed-lots", map[string]any{
|
||||
"plantId": plantID, "vendor": "Secret Vendor", "quantity": 10, "unit": "bulbs",
|
||||
}, alice)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("alice create: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
lotID := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
method string
|
||||
body any
|
||||
}{
|
||||
{"get", http.MethodGet, nil},
|
||||
{"patch", http.MethodPatch, map[string]any{"vendor": "hijacked", "version": 1}},
|
||||
{"delete", http.MethodDelete, nil},
|
||||
} {
|
||||
if w := doJSON(t, r, tc.method, seedLotPath(lotID), tc.body, bob); w.Code != http.StatusNotFound {
|
||||
t.Errorf("bob %s = %d, want 404 (existence is masked)", tc.name, w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// Bob's own listing must not include it either.
|
||||
w = doJSON(t, r, http.MethodGet, "/api/v1/seed-lots", nil, bob)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("bob list: status %d", w.Code)
|
||||
}
|
||||
if n := len(decodeList(t, w.Body.Bytes())); n != 0 {
|
||||
t.Errorf("bob sees %d of alice's lots, want 0: %s", n, w.Body.String())
|
||||
}
|
||||
|
||||
// And it's still intact for alice.
|
||||
if w := doJSON(t, r, http.MethodGet, seedLotPath(lotID), nil, alice); w.Code != http.StatusOK {
|
||||
t.Errorf("alice lost access to her own lot: %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeedLotsRequireAuthAPI: the group is behind requireAuth, and an
|
||||
// unauthenticated caller gets 401 rather than an empty list.
|
||||
func TestSeedLotsRequireAuthAPI(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
for _, tc := range []struct {
|
||||
method, path string
|
||||
}{
|
||||
{http.MethodGet, "/api/v1/seed-lots"},
|
||||
{http.MethodPost, "/api/v1/seed-lots"},
|
||||
{http.MethodGet, seedLotPath(1)},
|
||||
{http.MethodPatch, seedLotPath(1)},
|
||||
{http.MethodDelete, seedLotPath(1)},
|
||||
} {
|
||||
if w := doJSON(t, r, tc.method, tc.path, nil, nil); w.Code != http.StatusUnauthorized {
|
||||
t.Errorf("%s %s = %d, want 401", tc.method, tc.path, w.Code)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user
⚪ decodeList returns []any, diverging from the package's []map[string]any list-decoding convention
maintainability · flagged by 1 model
internal/api/seed_lots_test.go:19—decodeListdiverges from the established list-decoding convention. The package's other list-decoding sites decode into[]map[string]anyinline:gardens_test.go:56-58,plants_test.go:18, andshares_test.go:41-42. The newdecodeListhelper returns[]anyinstead. It is only used forlen()checks in this file, so it works, but it's a slightly leakier type and breaks the visual pattern a reader of this package expects. Either return `[]map[s…🪰 Gadfly · advisory