Bug 25/26: geleerte Felder blieben bestehen, Route-Bearbeiten scheiterte an nur-lesbarem Feld
Beide Bugs in ExpertViewModel.pendingCommand, betreffen Experte-Tab direkt (nicht nur den neuen Übersicht-Bearbeiten-Weg): - Bug 25: ein geleertes Textfeld (z.B. Kommentar löschen) wurde beim Speichern aus den Argumenten gefiltert statt explizit als "" gesendet — RouterOS' `set` ändert nur übergebene Parameter, ein weggelassener bleibt unangetastet statt geleert. Fix: Feld bleibt im Argument-Set, wenn es vorher einen Wert hatte; RouterOSCommand's SSH-Zeilen-Rendering gibt einen leeren Wert jetzt als `""` statt als nacktes `feld=` aus. - Bug 26: eine Route bearbeiten (z.B. nur Kommentar ändern) scheiterte mit "bad parameter immediate-gw" — dieses von RouterOS mitgelieferte, nur lesbare/berechnete Feld landete unkuratiert in den freien "Weiteren Parametern" und wurde bei jedem Speichern blind mitgeschickt. Fix: ein unkuratiertes Feld wird nur noch gesendet, wenn sein Wert sich gegenüber dem ursprünglich geladenen Item tatsächlich geändert hat. 54 Unit-Tests grün (neue ExpertViewModelTests + eine Ergänzung in RouterOSCommandBuilderTests). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YDmUd93KxsYGr2kLTotWnG
This commit is contained in:
Kay
+14
@@ -765,3 +765,17 @@ Test, WLAN/Bonding/PPPoE-Live-Tests).
|
||||
einer Firewall-Regel geändert, "funktioniert". 59 Unit-Tests grün,
|
||||
HANDOFF.md/README.md aktualisiert (u.a. M11s veraltete "rein lesend"-
|
||||
Aussage korrigiert), Commit + Push.
|
||||
- "nachdem entfernen des Kommentars... bleibt dieser bestehen. ...Route
|
||||
... Fehler: bad parameter immediate-gw" → zwei Bugs in
|
||||
`ExpertViewModel.pendingCommand` gefunden (betreffen Experte-Tab UND
|
||||
Übersicht-Tab): geleerte Felder wurden aus den Argumenten gefiltert
|
||||
statt explizit als "" gesendet (RouterOS' `set` ändert nur übergebene
|
||||
Parameter); unkuratierte "Weitere Parameter" wurden bei jedem Speichern
|
||||
blind mitgeschickt, auch nur-lesbare/berechnete Felder wie einer
|
||||
Route's `immediate-gw` — RouterOS lehnte das ab und brach die ganze
|
||||
Änderung ab. Bug 25/26, beide gefixt (Feld bleibt bei vorherigem Wert
|
||||
erhalten zum expliziten Leeren + SSH-Rendering zeigt leeren Wert jetzt
|
||||
als `""`; unkuratiertes Feld nur noch gesendet wenn tatsächlich
|
||||
geändert). 54 Unit-Tests grün (neue `ExpertViewModelTests` +
|
||||
`RouterOSCommandBuilderTests`-Ergänzung), HANDOFF.md aktualisiert,
|
||||
Commit + Push.
|
||||
|
||||
+33
-6
@@ -528,7 +528,26 @@ selbst als ungültig ("in/out-interface matcher not possible when
|
||||
interface is slave - use master instead"), live an `ether4` bestätigt;
|
||||
`DhcpServerCommandBuilder` stellt seitdem ein `/interface bridge port
|
||||
remove [find interface=<name>]` voran (übersprungen für "bridge"
|
||||
selbst).
|
||||
selbst). **Ein geleertes Textfeld (z.B. Kommentar) blieb nach dem
|
||||
Speichern unverändert bestehen** (Bug 25) — `pendingCommand` filterte
|
||||
alle leeren `formValues` grundsätzlich aus den Argumenten heraus, RouterOS'
|
||||
`set` ändert aber nur explizit übergebene Parameter, ein weggelassener
|
||||
bleibt unangetastet statt geleert. Fix: ein Feld bleibt im Argument-Set,
|
||||
wenn es vorher einen Wert hatte (jetzt als `feld=""` explizit geleert);
|
||||
zusätzlich musste `RouterOSCommand`s SSH-Zeilen-Rendering einen leeren
|
||||
Wert als `""` statt als nacktes `feld=` ausgeben. **Bearbeiten einer
|
||||
Route (z.B. nur den Kommentar ändern) scheiterte mit "bad parameter
|
||||
immediate-gw"** (Bug 26) — RouterOS liefert bei `/ip route print` u.a.
|
||||
das berechnete, nur lesbare Feld `immediate-gw` mit; da dieses Feld im
|
||||
Route-Schema nicht kuratiert ist, landete es unverändert in den freien
|
||||
"Weitere Parameter" und wurde bei *jedem* Speichern blind mitgeschickt —
|
||||
RouterOS lehnt `immediate-gw` beim `set` als ungültigen Parameter ab und
|
||||
bricht dadurch die komplette Änderung ab, auch wenn nur ein unrelated
|
||||
Feld wie der Kommentar geändert wurde. Fix: ein unkuratiertes Feld wird
|
||||
nur noch mitgeschickt, wenn sein Wert sich gegenüber dem ursprünglich
|
||||
geladenen Item tatsächlich geändert hat (oder neu hinzugefügt wurde) —
|
||||
betrifft nicht nur Routen, sondern jedes Menü mit berechneten/nur
|
||||
lesbaren Feldern im generischen Lese-Pfad.
|
||||
|
||||
## `xcodebuild test` hängt — Gatekeeper, kein Code-Bug
|
||||
|
||||
@@ -812,8 +831,13 @@ Bezug zu einem Router-Item).
|
||||
|
||||
**Live gegen Hardware verifiziert** — Nutzer bestätigte Kommentar-
|
||||
Änderung an einer Firewall-Regel über den neuen Bearbeiten-Weg
|
||||
("funktioniert"). Alle 59 Unit-Tests grün, inkl. neuer Tests für
|
||||
`editTarget` (`OverviewGraphTests.
|
||||
("funktioniert"). Direkt danach beim Weitertesten zwei echte Bugs in der
|
||||
gemeinsamen Bearbeiten-Logik gefunden (Bug 25, 26 — siehe Bug-Liste
|
||||
oben): geleertes Textfeld blieb bestehen, Route-Bearbeiten scheiterte an
|
||||
einem nur-lesbaren Feld (`immediate-gw`). Beide betreffen `ExpertViewModel.
|
||||
pendingCommand` und damit **auch den Experte-Tab direkt**, nicht nur den
|
||||
neuen Übersicht-Weg — gefixt, alle 54 Unit-Tests grün (inkl. neuer
|
||||
`ExpertViewModelTests` und `OverviewGraphTests.
|
||||
testEditableNodesCarryTheirRouterOSMenuAndItemID`).
|
||||
|
||||
## Stand der Milestones
|
||||
@@ -915,9 +939,12 @@ testEditableNodesCarryTheirRouterOSMenuAndItemID`).
|
||||
verdrängt) plus neue Fähigkeit: Knoten (IP-Adresse, Pool, DHCP-Server/
|
||||
-Netzwerk/-Client, Route, Firewall-Filter-/NAT-Regel, WireGuard-Peer)
|
||||
direkt über dieselbe Sheet wie im Experte-Tab bearbeiten und
|
||||
zurückschreiben. **Live gegen Hardware verifiziert** — Nutzer
|
||||
bestätigte Kommentar-Änderung an einer Firewall-Regel über den neuen
|
||||
Weg ("funktioniert").
|
||||
zurückschreiben. Dabei zwei Bugs in der gemeinsamen Bearbeiten-Logik
|
||||
gefunden+gefixt (Bug 25, 26 — betreffen auch den Experte-Tab direkt):
|
||||
geleertes Feld blieb bestehen statt geleert zu werden, Route-Bearbeiten
|
||||
scheiterte an einem nur-lesbaren Feld (`immediate-gw`). **Live gegen
|
||||
Hardware verifiziert** — Nutzer bestätigte Kommentar-Änderung an einer
|
||||
Firewall-Regel über den neuen Weg ("funktioniert").
|
||||
|
||||
## Nächste Schritte
|
||||
|
||||
|
||||
@@ -135,6 +135,9 @@ struct RouterOSCommand: Equatable, Identifiable {
|
||||
}
|
||||
|
||||
private static func quoteIfNeeded(_ value: String) -> String {
|
||||
value.contains(" ") ? "\"\(value)\"" : value
|
||||
// An empty value must render as an explicit `""`, not a bare `key=` with nothing after
|
||||
// the `=` — that's how RouterOS' CLI represents "clear this field" versus a syntax error.
|
||||
if value.isEmpty { return "\"\"" }
|
||||
return value.contains(" ") ? "\"\(value)\"" : value
|
||||
}
|
||||
}
|
||||
|
||||
@@ -150,7 +150,13 @@ final class ExpertViewModel: ObservableObject {
|
||||
/// actually executes, consistent with the rest of the app never applying silently.
|
||||
var pendingCommand: RouterOSCommand? {
|
||||
guard let schema = selectedSchema, let editingItem else { return nil }
|
||||
var arguments = formValues.filter { !$0.value.isEmpty }
|
||||
// Keep a curated field even when now empty if it previously held a value — RouterOS'
|
||||
// `set` only touches parameters it's given, so simply omitting a cleared field (the
|
||||
// naive "drop empty values" rule) leaves the old value in place instead of clearing it.
|
||||
// Confirmed live (2026-09-15): deleting a comment's text and saving left the old comment
|
||||
// on the router. Still drop fields that were already empty/unset, same as before — no
|
||||
// point sending e.g. an untouched optional field as "" on every save.
|
||||
var arguments = formValues.filter { !$0.value.isEmpty || !(editingItem.fields[$0.key] ?? "").isEmpty }
|
||||
// RouterOS' CLI parser rejects "true"/"false" for boolean parameters — it only accepts
|
||||
// "yes"/"no" (confirmed live: "disabled=false" on "/interface vlan add" produced
|
||||
// "syntax error (line 1 column 30)"; "disabled=no" succeeded). Schema defaults are
|
||||
@@ -161,8 +167,18 @@ final class ExpertViewModel: ObservableObject {
|
||||
if value == "true" { arguments[field.key] = "yes" }
|
||||
else if value == "false" { arguments[field.key] = "no" }
|
||||
}
|
||||
// Only resend an uncurated ("Weitere Parameter") field if the user actually changed it
|
||||
// (or added a brand-new one) — RouterOS' `print`/REST GET returns some fields that are
|
||||
// computed/read-only and rejected on `set` (confirmed live: a route's "immediate-gw",
|
||||
// "bad parameter immediate-gw"). Since `startEditing` pre-fills every uncurated field
|
||||
// from the live item for visibility, blindly resending all of them on every save — even
|
||||
// to change one unrelated curated field like a comment — resent that computed value
|
||||
// unchanged and broke the whole edit. Comparing against `editingItem.fields` (the
|
||||
// untouched original) tells "user touched this" apart from "just showing what's there".
|
||||
for extra in extraFields where !extra.key.isEmpty {
|
||||
arguments[extra.key] = extra.value
|
||||
if extra.value != editingItem.fields[extra.key] {
|
||||
arguments[extra.key] = extra.value
|
||||
}
|
||||
}
|
||||
let isNew = editingItem.id.isEmpty
|
||||
let summary = "\(isNew ? "Neu anlegen" : "Ändern") unter \(schema.menuPath)"
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
import XCTest
|
||||
@testable import RouterOSAssistant
|
||||
|
||||
@MainActor
|
||||
final class ExpertViewModelTests: XCTestCase {
|
||||
private func makeSchema() -> RouterOSMenuSchema {
|
||||
RouterOSMenuSchema(
|
||||
menuPath: "/ip route", restPath: "ip/route", category: .routing,
|
||||
displayName: "Test", summary: "", explanation: "",
|
||||
fields: [
|
||||
RouterOSFieldSchema(key: "comment", label: "Kommentar", kind: .text, help: "")
|
||||
]
|
||||
)
|
||||
}
|
||||
|
||||
func testClearingACuratedFieldSendsExplicitEmptyValue() {
|
||||
let viewModel = ExpertViewModel(connectionService: ConnectionService())
|
||||
viewModel.selectedSchema = makeSchema()
|
||||
viewModel.startEditing(RouterOSMenuItem(id: "*1", fields: ["comment": "bridge"]))
|
||||
viewModel.formValues["comment"] = ""
|
||||
|
||||
XCTAssertEqual(viewModel.pendingCommand?.arguments["comment"], "")
|
||||
}
|
||||
|
||||
func testUnchangedUncuratedFieldIsNotResent() {
|
||||
let viewModel = ExpertViewModel(connectionService: ConnectionService())
|
||||
viewModel.selectedSchema = makeSchema()
|
||||
viewModel.startEditing(RouterOSMenuItem(id: "*1", fields: ["comment": "", "immediate-gw": "192.168.88.1"]))
|
||||
viewModel.formValues["comment"] = "bridge"
|
||||
|
||||
XCTAssertNil(viewModel.pendingCommand?.arguments["immediate-gw"])
|
||||
XCTAssertEqual(viewModel.pendingCommand?.arguments["comment"], "bridge")
|
||||
}
|
||||
|
||||
func testChangedUncuratedFieldIsSent() {
|
||||
let viewModel = ExpertViewModel(connectionService: ConnectionService())
|
||||
viewModel.selectedSchema = makeSchema()
|
||||
viewModel.startEditing(RouterOSMenuItem(id: "*1", fields: ["comment": "", "note": "old"]))
|
||||
viewModel.extraFields = [.init(key: "note", value: "new")]
|
||||
|
||||
XCTAssertEqual(viewModel.pendingCommand?.arguments["note"], "new")
|
||||
}
|
||||
}
|
||||
@@ -67,6 +67,19 @@ final class RouterOSCommandBuilderTests: XCTestCase {
|
||||
XCTAssertEqual(commands[1].menuPath, "/ip address")
|
||||
}
|
||||
|
||||
func testCliLineRendersEmptyValueAsExplicitEmptyQuotes() {
|
||||
let command = RouterOSCommand.set(
|
||||
menuPath: "/ip route",
|
||||
restPath: "ip/route",
|
||||
matchField: ".id",
|
||||
matchValue: "*1",
|
||||
arguments: ["comment": ""],
|
||||
summary: "test"
|
||||
)
|
||||
|
||||
XCTAssertEqual(command.cliLine, "/ip route set [find .id=*1] comment=\"\"")
|
||||
}
|
||||
|
||||
func testCliLineRendersSortedQuotedArgumentsForAdd() {
|
||||
let command = RouterOSCommand.add(
|
||||
menuPath: "/interface pppoe-client",
|
||||
|
||||
Reference in New Issue
Block a user