Revision history: change sets + revisions + revert — the undo substrate (#48) (#61)

Co-authored-by: Steve Dudenhoeffer <[email protected]>

internal/api/api.go

@@ -81,8 +81,9 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
 	gardens.GET("/:id", h.getGarden)
 	gardens.PATCH("/:id", h.updateGarden)
 	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.POST("/:id/copy", h.copyGarden)         // duplicate a garden the actor owns
+	gardens.GET("/:id/full", h.getGardenFull)       // one-shot editor load
+	gardens.GET("/:id/history", h.getGardenHistory) // change sets, newest first
 	gardens.POST("/:id/objects", h.createObject)
 
 	// Sharing (owner-managed; a recipient may remove their own share).
@@ -111,6 +112,11 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
 	plantings.PATCH("/:id", h.updatePlanting)
 	plantings.DELETE("/:id", h.deletePlanting)
 
+	// Undo. A change set is addressed by its own id; the service resolves the
+	// owning garden for the permission check, same as objects and plantings.
+	changeSets := v1.Group("/change-sets", h.requireAuth())
+	changeSets.POST("/:id/revert", h.revertChangeSet)
+
 	// Plant catalog: built-ins (seeded, read-only) plus the actor's own rows.
 	plants := v1.Group("/plants", h.requireAuth())
 	plants.GET("", h.listPlants)

internal/api/errors.go

@@ -69,6 +69,20 @@ func writeVersionConflict(c *gin.Context, current any) {
 	})
 }
 
+// intQuery reads a non-negative integer query parameter, falling back to def on
+// an absent or malformed value.
+func intQuery(c *gin.Context, name string, def int) int {
+	raw := c.Query(name)
+	if raw == "" {
+		return def
+	}
+	v, err := strconv.Atoi(raw)
+	if err != nil || v < 0 {
+		return def
+	}
+	return v
+}
+
 // parseIDParam reads a positive int64 path parameter, writing a 400 and
 // returning ok=false on a malformed value.
 func parseIDParam(c *gin.Context, name string) (int64, bool) {

internal/api/history.go

@@ -0,0 +1,80 @@
+package api
+
+import (
+	"net/http"
+
+	"github.com/gin-gonic/gin"
+
+	"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
+)
+
+// Change history and undo (#48). Two endpoints: read a garden's change sets, and
+// revert one. Both encode domain types directly, like the rest of the package —
+// domain.ChangeSet already carries the actor name, revert linkage and per-op
+// counts the history list renders, and its Revisions field is omitempty so the
+// JSON snapshots never ride along on a list response.
+
+// historyResponse is the body of GET /gardens/:id/history. hasMore lets the
+// client page without a separate count query over a table that only grows.
+type historyResponse struct {
+	ChangeSets []domain.ChangeSet `json:"changeSets"`
+	HasMore    bool               `json:"hasMore"`
+}
+
+// revertResponse is the body of POST /change-sets/:id/revert on every path.
+// Carrying both fields regardless is what lets the UI say "2 of 3 changes undone;
+// the north bed was edited since and was left alone" instead of a generic
+// failure — a partial revert really did change things, and pretending otherwise
+// would be a lie. ChangeSet is null when nothing needed reverting.
+type revertResponse struct {
+	ChangeSet *domain.ChangeSet       `json:"changeSet"`
+	Conflicts []domain.RevertConflict `json:"conflicts"`
+}
+
+func (h *handlers) getGardenHistory(c *gin.Context) {
+	gardenID, ok := parseIDParam(c, "id")
+	if !ok {
+		return
+	}
+	// 0 means "the service's default"; it clamps the upper bound too.
+	limit := intQuery(c, "limit", 0)
+	offset := intQuery(c, "offset", 0)
+
+	sets, hasMore, err := h.svc.GardenHistory(c.Request.Context(), mustActor(c).ID, gardenID, limit, offset)
+	if err != nil {
+		writeServiceError(c, err)
+		return
+	}
+	c.JSON(http.StatusOK, historyResponse{ChangeSets: sets, HasMore: hasMore})
+}
+
+func (h *handlers) revertChangeSet(c *gin.Context) {
+	id, ok := parseIDParam(c, "id")
+	if !ok {
+		return
+	}
+	// A revert through the REST API is a person clicking undo. The agent reverts
+	// its own work through the service directly and stamps SourceAgent, so the
+	// history badge can tell the two apart.
+	cs, conflicts, err := h.svc.RevertChangeSet(c.Request.Context(), mustActor(c).ID, id, domain.SourceUI)
+	if err != nil {
+		writeServiceError(c, err)
+		return
+	}
+	if conflicts == nil {
+		conflicts = []domain.RevertConflict{}
+	}
+	body := revertResponse{ChangeSet: cs, Conflicts: conflicts}
+	switch {
+	case len(conflicts) > 0:
+		// 409 even when part of the revert applied: something the caller asked for
+		// did not happen, and the body says exactly what.
+		c.JSON(http.StatusConflict, body)
+	case cs == nil:
+		// Every revision resolved to a no-op (already undone by hand, say). Nothing
+		// was created, so 200 rather than a 201 pointing at nothing.
+		c.JSON(http.StatusOK, body)
+	default:
+		c.JSON(http.StatusCreated, body)
+	}
+}

internal/api/history_test.go

@@ -0,0 +1,153 @@
+package api
+
+import (
+	"net/http"
+	"strconv"
+	"testing"
+)
+
+func historyPath(gardenID int64) string {
+	return "/api/v1/gardens/" + strconv.FormatInt(gardenID, 10) + "/history"
+}
+
+func revertPath(changeSetID int64) string {
+	return "/api/v1/change-sets/" + strconv.FormatInt(changeSetID, 10) + "/revert"
+}
+
+// TestHistoryAndRevertAPI walks the whole loop over HTTP: a mutation shows up in
+// history without anyone asking for it, reverting it answers 201 with the new
+// change set, and the change is actually gone from /full.
+func TestHistoryAndRevertAPI(t *testing.T) {
+	r := authEngine(t, localCfg())
+	cookie := registerAndCookie(t, r, "[email protected]")
+	gid := createGardenAPI(t, r, cookie, "G")
+
+	w := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{
+		"kind": "bed", "name": "North Bed", "xCm": 100, "yCm": 100, "widthCm": 100, "heightCm": 100,
+	}, cookie)
+	if w.Code != http.StatusCreated {
+		t.Fatalf("create object: status %d, body %s", w.Code, w.Body.String())
+	}
+	// The create landed in history with no explicit change set anywhere.
+	w = doJSON(t, r, http.MethodGet, historyPath(gid), nil, cookie)
+	if w.Code != http.StatusOK {
+		t.Fatalf("history: status %d, body %s", w.Code, w.Body.String())
+	}
+	body := decodeMap(t, w.Body.Bytes())
+	sets, _ := body["changeSets"].([]any)
+	if len(sets) != 1 {
+		t.Fatalf("got %d change sets, want 1: %s", len(sets), w.Body.String())
+	}
+	if body["hasMore"].(bool) {
+		t.Error("hasMore should be false for a single-entry history")
+	}
+	entry := sets[0].(map[string]any)
+	if entry["summary"] != "Added North Bed" {
+		t.Errorf("summary = %v", entry["summary"])
+	}
+	if entry["source"] != "ui" || entry["actorName"] == "" {
+		t.Errorf("unexpected entry: %+v", entry)
+	}
+	counts, _ := entry["counts"].([]any)
+	if len(counts) != 1 {
+		t.Fatalf("counts = %+v", entry["counts"])
+	}
+
+	// Revert it: 201, and the new change set points back at the original.
+	csID := int64(entry["id"].(float64))
+	w = doJSON(t, r, http.MethodPost, revertPath(csID), nil, cookie)
+	if w.Code != http.StatusCreated {
+		t.Fatalf("revert: status %d, body %s", w.Code, w.Body.String())
+	}
+	rev := decodeMap(t, w.Body.Bytes())
+	cs := rev["changeSet"].(map[string]any)
+	if int64(cs["revertsId"].(float64)) != csID {
+		t.Errorf("revertsId = %v, want %d", cs["revertsId"], csID)
+	}
+	if conflicts, _ := rev["conflicts"].([]any); len(conflicts) != 0 {
+		t.Errorf("unexpected conflicts: %+v", conflicts)
+	}
+
+	// The object is gone from the editor payload.
+	w = doJSON(t, r, http.MethodGet, fullPath(gid), nil, cookie)
+	full := decodeMap(t, w.Body.Bytes())
+	if objects, _ := full["objects"].([]any); len(objects) != 0 {
+		t.Errorf("%d objects survived the revert", len(objects))
+	}
+}
+
+// TestRevertConflictAPI: a partial revert answers 409 and names what it skipped,
+// because "2 of 3 undone, the north bed was edited since" is the only useful
+// thing to say — a bare failure would be a lie about what happened.
+func TestRevertConflictAPI(t *testing.T) {
+	r := authEngine(t, localCfg())
+	cookie := registerAndCookie(t, r, "[email protected]")
+	gid := createGardenAPI(t, r, cookie, "G")
+
+	w := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{
+		"kind": "bed", "name": "Bed", "xCm": 100, "yCm": 100, "widthCm": 100, "heightCm": 100,
+	}, cookie)
+	obj := decodeMap(t, w.Body.Bytes())
+	objectID := int64(obj["id"].(float64))
+	version := int64(obj["version"].(float64))
+
+	// Move it (change set #2), then edit it again (change set #3).
+	w = doJSON(t, r, http.MethodPatch, objectPath(objectID), map[string]any{
+		"xCm": 300, "version": version,
+	}, cookie)
+	if w.Code != http.StatusOK {
+		t.Fatalf("move: status %d, body %s", w.Code, w.Body.String())
+	}
+	version = int64(decodeMap(t, w.Body.Bytes())["version"].(float64))
+	w = doJSON(t, r, http.MethodPatch, objectPath(objectID), map[string]any{
+		"name": "Renamed", "version": version,
+	}, cookie)
+	if w.Code != http.StatusOK {
+		t.Fatalf("rename: status %d, body %s", w.Code, w.Body.String())
+	}
+
+	// Undoing the move now conflicts: the row changed after it.
+	w = doJSON(t, r, http.MethodGet, historyPath(gid), nil, cookie)
+	sets := decodeMap(t, w.Body.Bytes())["changeSets"].([]any)
+	moveID := int64(sets[1].(map[string]any)["id"].(float64)) // newest first: rename, move, create
+
+	w = doJSON(t, r, http.MethodPost, revertPath(moveID), nil, cookie)
+	if w.Code != http.StatusConflict {
+		t.Fatalf("revert: status %d, want 409, body %s", w.Code, w.Body.String())
+	}
+	conflicts := decodeMap(t, w.Body.Bytes())["conflicts"].([]any)
+	if len(conflicts) != 1 {
+		t.Fatalf("conflicts = %+v", conflicts)
+	}
+	c := conflicts[0].(map[string]any)
+	if c["reason"] != "changed" || c["name"] != "Renamed" {
+		t.Errorf("conflict = %+v", c)
+	}
+
+	// The object was left exactly alone.
+	w = doJSON(t, r, http.MethodGet, fullPath(gid), nil, cookie)
+	objects := decodeMap(t, w.Body.Bytes())["objects"].([]any)
+	o := objects[0].(map[string]any)
+	if o["xCm"].(float64) != 300 || o["name"] != "Renamed" {
+		t.Errorf("conflicted object was modified: %+v", o)
+	}
+}
+
+// TestHistoryRequiresAccess — an unauthenticated caller can't read history, and a
+// stranger gets 404 rather than a hint that the garden exists.
+func TestHistoryRequiresAccess(t *testing.T) {
+	r := authEngine(t, localCfg())
+	owner := registerAndCookie(t, r, "[email protected]")
+	gid := createGardenAPI(t, r, owner, "G")
+	stranger := registerAndCookie(t, r, "[email protected]")
+
+	if w := doJSON(t, r, http.MethodGet, historyPath(gid), nil, nil); w.Code != http.StatusUnauthorized {
+		t.Errorf("anonymous history status = %d, want 401", w.Code)
+	}
+	if w := doJSON(t, r, http.MethodGet, historyPath(gid), nil, stranger); w.Code != http.StatusNotFound {
+		t.Errorf("stranger history status = %d, want 404", w.Code)
+	}
+	if w := doJSON(t, r, http.MethodPost, revertPath(1), nil, stranger); w.Code != http.StatusNotFound {
+		t.Errorf("stranger revert status = %d, want 404", w.Code)
+	}
+}