Move a rule several places in one change

The Rules page has a Move… button that puts a rule before any other of
its chain, or at the end, in one move. Staged in a changeset, a move
right after a move of the same object takes its place, so pressing ↑
three times is one operation; moving it back where it was takes the
operation out.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/server/h_changesets.go

@@ -89,6 +89,17 @@ func (s *Server) stage(w http.ResponseWriter, r *http.Request, p *auth.Principal
 	if err := replay(before, cs.Ops); err != nil {
 		return conflict("changeset %d can't be applied as it is (%v); review it first", id, err)
 	}
+	// A move right after a move of the same object takes its place:
+	// moving a rule up three steps is one operation.
+	kept := cs.Ops
+	if n := len(kept); n > 0 && foldMove(before, &kept[n-1], &op) {
+		kept = kept[:n-1]
+		before = s.eng.State()
+		if err := replay(before, kept); err != nil {
+			return conflict("changeset %d can't be applied as it is (%v); review it first", id, err)
+		}
+	}
+	folded := len(kept) < len(cs.Ops)
 	after := before.Clone()
 	objID, err := applyOp(after, &op)
 	if err != nil {
@@ -98,6 +109,13 @@ func (s *Server) stage(w http.ResponseWriter, r *http.Request, p *auth.Principal
 	after.Sort()
 	changes := engine.Diff(before, after)
 	if len(changes) == 0 {
+		// Moved back where it was: the earlier move goes too.
+		if folded && !queryBool(r, "dry_run") {
+			cs.Ops = kept
+			if err := s.store.UpdateChangeset(r.Context(), cs); err != nil {
+				return err
+			}
+		}
 		writeJSON(w, http.StatusOK, api.MutationResponse{Changes: []model.Change{}, Changeset: cs})
 		return nil
 	}
@@ -108,7 +126,7 @@ func (s *Server) stage(w http.ResponseWriter, r *http.Request, p *auth.Principal
 		op.Summary = engine.UndoDescription(&h)
 	}
 	op.AddedAt = s.nowFn()
-	for _, o := range cs.Ops {
+	for _, o := range kept {
 		if o.Seq >= op.Seq {
 			op.Seq = o.Seq + 1
 		}
@@ -116,7 +134,7 @@ func (s *Server) stage(w http.ResponseWriter, r *http.Request, p *auth.Principal
 	if op.Seq == 0 {
 		op.Seq = 1
 	}
-	ops := append(append([]model.Op{}, cs.Ops...), op)
+	ops := append(append([]model.Op{}, kept...), op)
 	dry := queryBool(r, "dry_run")
 	// The whole changeset, checked as it would be applied.
 	res, err := s.eng.Apply(r.Context(), p, "", engine.Options{DryRun: true, Via: via(r, p)}, func(st *model.State, _ func() int64) error {
@@ -145,6 +163,31 @@ func (s *Server) stage(w http.ResponseWriter, r *http.Request, p *auth.Principal
 	return nil
 }
 
+// foldMove reports whether op, a move, can take the place of prev, the
+// changeset's last operation: a move of the same object. A move puts
+// the object relative to the others, which an earlier move of it
+// doesn't change, so only the chain it moved to has to be carried over.
+// st is the changeset so far.
+func foldMove(st *model.State, prev, op *model.Op) bool {
+	if prev.Op != model.OpMove || op.Op != model.OpMove || prev.Kind != op.Kind {
+		return false
+	}
+	a, err1 := findObject(st, prev.Kind, prev.Ref)
+	b, err2 := findObject(st, op.Kind, op.Ref)
+	if err1 != nil || err2 != nil || a.GetID() != b.GetID() {
+		return false
+	}
+	var was, now api.MoveInput
+	if json.Unmarshal(prev.Data, &was) != nil || json.Unmarshal(op.Data, &now) != nil {
+		return false
+	}
+	if now.Chain == "" && was.Chain != "" {
+		now.Chain = was.Chain
+		op.Data, _ = json.Marshal(now)
+	}
+	return true
+}
+
 func (s *Server) createChangeset(w http.ResponseWriter, r *http.Request, p *auth.Principal) error {
 	if err := require(p, auth.ResConfig, true); err != nil {
 		return err

internal/server/server_test.go

@@ -1336,6 +1336,53 @@ func TestCreatePlaced(t *testing.T) {
 	}
 }
 
+// TestStagedMovesFold: moving a rule several steps in a changeset stays
+// one operation, and moving it back where it was takes it out.
+func TestStagedMovesFold(t *testing.T) {
+	e := newEnv(t)
+	var ids []int64
+	for _, svc := range []string{"ssh", "dns", "http", "https"} {
+		var mr api.MutationResponse
+		var r model.Rule
+		e.call(e.token, "POST", "/rules", model.Rule{Chain: "input", Match: model.Match{Services: []string{svc}}}, &mr)
+		json.Unmarshal(mr.Object, &r)
+		ids = append(ids, r.ID)
+	}
+	var cs model.Changeset
+	e.call(e.token, "POST", "/changesets", nil, &cs)
+	q := "?changeset=" + itoa(cs.ID)
+	var mr api.MutationResponse
+	for _, before := range []int64{ids[2], ids[1], ids[0]} { // https up three steps
+		if code := e.call(e.token, "POST", "/rules/"+itoa(ids[3])+"/move"+q, api.MoveInput{Before: before}, &mr); code != 200 || !mr.Staged {
+			t.Fatalf("move before %d: %d %+v", before, code, mr)
+		}
+	}
+	if n := len(mr.Changeset.Ops); n != 1 {
+		t.Fatalf("%d operations, want 1: %+v", n, mr.Changeset.Ops)
+	}
+	// Another operation in between: the next move is its own.
+	e.call(e.token, "PATCH", "/rules/"+itoa(ids[0])+q, map[string]any{"comment": "x"}, nil)
+	e.call(e.token, "POST", "/rules/"+itoa(ids[3])+"/move"+q, api.MoveInput{After: ids[0]}, &mr)
+	if n := len(mr.Changeset.Ops); n != 3 {
+		t.Errorf("%d operations after a patch and a move, want 3", n)
+	}
+	// Back to the start: the same place as before that move, so it goes.
+	e.call(e.token, "POST", "/rules/"+itoa(ids[3])+"/move"+q, api.MoveInput{To: "start"}, &mr)
+	if n := len(mr.Changeset.Ops); n != 2 {
+		t.Errorf("%d operations after moving back, want 2", n)
+	}
+	if code := e.call(e.token, "POST", "/changesets/"+itoa(cs.ID)+"/apply", nil, nil); code != 200 {
+		t.Fatalf("apply: %d", code)
+	}
+	var out []string
+	for _, r := range e.eng.State().Rules {
+		out = append(out, strings.Join(r.Services, ""))
+	}
+	if o := strings.Join(out, ","); o != "https,ssh,dns,http" {
+		t.Errorf("order: %s", o)
+	}
+}
+
 // TestTokenForUser: an administrator makes a token for another user (the
 // web UI's New token on a user's row); that user can't make one for
 // someone else.