adam/jarl
1
0 Fork 0
Code Issues 4 Pull Requests Actions Packages Projects Releases 34 Wiki Activity

Changesets: staged deletes of gone records count as done, name stale ops

Browse Source 
A web changeset with a staged record delete could not be applied after
the record was deleted elsewhere (e.g. with the CLI), and the error only
said "record N does not exist", so it was hard to find the operation to
remove. A delete of a record that is already gone is now a no-op with a
warning. Other operations that no longer apply fail with their number and
summary (op_summary in the API), and the Changes page marks that row and
offers to remove it from the error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
adamandClaude Opus 5.5 committed 2026-10-11 22:59:19 +01:00
1 parent c37de31f4c
commit 49b1c872c9
7 files changed
+113 -17
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/engine/errors.go
+12 -6
View File
@@ -25,15 +25,21 @@ const (
// Error is an engine error with details for the caller.
type Error struct {
Kind ErrorKind `json:"code"`
Message string `json:"message"`
OpSeq int `json:"op,omitempty"` // the operation that failed
Problems []validate.Problem `json:"problems,omitempty"`
Check []CheckMessage `json:"check,omitempty"`
Diff string `json:"diff,omitempty"`
Kind ErrorKind `json:"code"`
Message string `json:"message"`
OpSeq int `json:"op,omitempty"` // the operation that failed
// OpSummary describes that operation, so people can find it in the
// changeset (record IDs mean little to them).
OpSummary string `json:"op_summary,omitempty"`
Problems []validate.Problem `json:"problems,omitempty"`
Check []CheckMessage `json:"check,omitempty"`
Diff string `json:"diff,omitempty"`
}
func (e *Error) Error() string {
if e.OpSeq > 0 && e.OpSummary != "" {
return fmt.Sprintf("operation %d (%s): %s", e.OpSeq, e.OpSummary, e.Message)
}
if e.OpSeq > 0 {
return fmt.Sprintf("operation %d: %s", e.OpSeq, e.Message)
}
internal/engine/ops.go
+6 -3
View File
@@ -120,7 +120,7 @@ func applyOp(st *model.State, op model.Op, c *applyCtx) error {
err = errf(KindBadRequest, "unknown operation %q", op.Kind)
}
if e, ok := AsError(err); ok && e.OpSeq == 0 {
e.OpSeq = op.Seq
e.OpSeq, e.OpSummary = op.Seq, op.Summary
}
return err
}
@@ -457,7 +457,7 @@ func applyRecordCreate(st *model.State, op model.Op, c *applyCtx) error {
func applyRecordUpdate(st *model.State, op model.Op, c *applyCtx) error {
r := st.Records[op.ID]
if r == nil {
return errf(KindNotFound, "record %d does not exist", op.ID)
return errf(KindConflict, "the record (ID %d) no longer exists: it was deleted after this change was staged; remove this operation from the changeset", op.ID)
}
if err := checkVersion(op, r.Version, recordLabel(r)); err != nil {
return err
@@ -533,7 +533,10 @@ func applyRecordDelete(st *model.State, op model.Op, c *applyCtx) error {
if op.ID != 0 {
r := st.Records[op.ID]
if r == nil {
return errf(KindNotFound, "record %d does not exist", op.ID)
// Deleted since this was staged (by someone else, or the
// CLI): what the operation wanted is done.
c.warn("%s: the record was already deleted outside this changeset, so there was nothing to delete", strings.TrimPrefix(op.Summary, "delete "))
return nil
}
if err := checkVersion(op, r.Version, recordLabel(r)); err != nil {
return err
internal/engine/stale_test.go
+51
View File
@@ -0,0 +1,51 @@
package engine
import (
"strings"
"testing"
"jarl/internal/model"
)
// Records deleted or changed after a change to them was staged (in the
// CLI while a web UI changeset is open, say).
func TestStaleChangeset(t *testing.T) {
env := newEnv(t)
setupZone(env)
env.submit(env.admin, rec("example.com", "one", "A", "192.0.2.51"), rec("example.com", "two", "A", "192.0.2.52"), rec("example.com", "three", "A", "192.0.2.53"))
st := env.state()
one, two, three := findRec(st, "one.example.com.", "A"), findRec(st, "two.example.com.", "A"), findRec(st, "three.example.com.", "A")
// Two deletes staged; one of the records is deleted elsewhere first.
cs, err := env.e.CreateChangeset(env.ctx, env.admin, "web", "")
if err != nil {
t.Fatal(err)
}
if _, err := env.e.Stage(env.ctx, cs.ID, []Request{NewRequest(OpRecordDelete, one.ID, nil), NewRequest(OpRecordDelete, two.ID, nil)}, env.admin); err != nil {
t.Fatal(err)
}
env.submit(env.admin, NewRequest(OpRecordDelete, one.ID, nil))
res, err := env.e.Apply(env.ctx, cs.ID, env.admin, ApplyOptions{})
if err != nil {
t.Fatalf("apply after the record was deleted elsewhere: %v", err)
}
if len(res.Warnings) != 1 || !strings.Contains(res.Warnings[0], "one.example.com. A 192.0.2.51: the record was already deleted") {
t.Errorf("warnings = %q", res.Warnings)
}
if st := env.state(); st.Records[two.ID] != nil {
t.Error("two was not deleted")
}
// A staged edit of a record deleted elsewhere names the operation.
cs, _ = env.e.CreateChangeset(env.ctx, env.admin, "web", "")
env.e.Stage(env.ctx, cs.ID, []Request{NewRequest(OpRecordUpdate, three.ID, RecordInput{Data: model.Ptr("192.0.2.54")})}, env.admin)
env.submit(env.admin, NewRequest(OpRecordDelete, three.ID, nil))
p, err := env.e.Preview(env.ctx, cs.ID, env.admin)
if err != nil || p.Error == nil || p.Error.OpSeq != 1 || !strings.HasPrefix(p.Error.OpSummary, "change three.example.com. A 192.0.2.53") {
t.Fatalf("preview = %+v, %v", p, err)
}
_, err = env.e.Apply(env.ctx, cs.ID, env.admin, ApplyOptions{})
if ee, ok := AsError(err); !ok || ee.Kind != KindConflict || !strings.Contains(err.Error(), "operation 1 (change three.example.com. A") || !strings.Contains(err.Error(), "remove this operation") {
t.Errorf("apply = %v", err)
}
}
internal/web/static/assets/pages/changes.js
+23 -8
View File
@@ -44,18 +44,33 @@ async function draftPanel(d) {
const applyBtn = h("button", { class: "primary", disabled: true }, "Apply changes");
const forceBox = h("label", { class: "check small hidden" }, h("input", { type: "checkbox" }), "Overwrite files changed outside Jarl");
const ops = d.ops || [];
const removeOp = async (seq) => {
try {
const cs = await api("DELETE", "/changesets/" + d.id + "/ops/" + seq);
useDraft(cs);
rerender();
} catch (e) { toast(e.message, "err", 8000); }
};
const opsTable = table([
{ label: "#", render: (o) => String(o.seq) },
{ label: "Change", render: (o) => o.summary || o.op, tdClass: "mono small" },
{ label: "Added", render: (o) => time(o.added_at) },
{ label: "", render: (o) => h("button", { class: "small", title: "Remove this operation from the changeset", onclick: async () => {
try {
const cs = await api("DELETE", "/changesets/" + d.id + "/ops/" + o.seq);
useDraft(cs);
rerender();
} catch (e) { toast(e.message, "err", 8000); }
} }, "Remove"), tdClass: "actions-cell" },
{ label: "", render: (o) => h("button", { class: "small", title: "Remove this operation from the changeset", onclick: () => removeOp(o.seq) }, "Remove"), tdClass: "actions-cell" },
], ops);
// failedOp marks the operation that no longer applies and says how to
// get past it.
const failedOp = (err) => {
const i = ops.findIndex((o) => o.seq === err.op);
const rows = opsTable.querySelectorAll("tbody tr");
if (i < 0 || !rows[i]) return errorBox({ message: err.message, details: err });
rows[i].classList.add("op-failed");
return h("div", { class: "msg err" },
h("div", null, h("strong", null, "Operation " + err.op + " can't be applied: "), err.message),
h("div", { class: "mono small" }, ops[i].summary || ops[i].op),
h("div", { style: { marginTop: "8px" } },
h("button", { class: "small", onclick: () => removeOp(err.op) }, "Remove operation " + err.op),
" ", h("span", { class: "muted small" }, "It is marked in the list above. Then check the rest again.")));
};
const runPreview = async () => {
try {
const p = await get("/changesets/" + d.id + "/preview");
@@ -64,7 +79,7 @@ async function draftPanel(d) {
const drift = (p.drift || []);
forceBox.classList.toggle("hidden", drift.length === 0);
mount(preview,
p.error ? errorBox({ message: p.error.message, details: p.error }) : null,
p.error ? (p.error.op ? failedOp(p.error) : errorBox({ message: p.error.message, details: p.error })) : null,
valid ? h("div", { class: "msg ok" }, "Valid. named-checkconf accepts the new configuration.", (p.serials || []).length ? " Serials: " + p.serials.map((s) => s.zone.replace(/\.$/, "") + " " + s.old + "→" + s.new).join(", ") : "") : null,
!p.error && !valid ? h("div", { class: "msg err" }, "This changeset can't be applied yet:") : null,
problemsList(p.problems),
internal/web/static/assets/style.css
+2
View File
@@ -154,6 +154,8 @@ td.actions-cell { white-space: nowrap; text-align: right; width: 1%; }
tr.editing td { background: var(--accent-soft); }
tr.pending-add td { background: var(--add); }
tr.pending-change td, tr.pending-disable td, tr.pending-enable td { background: var(--warn-soft); }
tr.op-failed td { background: var(--err-soft); }
tr.op-failed td:first-child { box-shadow: inset 3px 0 0 var(--err); }
tr.pending-delete td { background: var(--del); text-decoration: line-through; color: var(--muted); }
tr.disabled td:not(.actions-cell), tr.disabled td strong { color: var(--muted); }
/* Disabled records: hatched, so they stand out from live and pending rows. */