diff --git a/internal/web/service/node.go b/internal/web/service/node.go index 206b0939e..870547f37 100644 --- a/internal/web/service/node.go +++ b/internal/web/service/node.go @@ -780,12 +780,19 @@ func (s *NodeService) NodeSyncState(id int) (enabled bool, status string, dirty return row.Enable, row.Status, row.ConfigDirty, row.ConfigDirtyAt, nil } +// IsNodePending reports whether a save targeting this node was deferred because +// the node is unreachable right now — offline or disabled — so the edit only +// reaches it on the next reconcile. It deliberately ignores config_dirty: that +// flag is set on EVERY node-backed edit as the reconcile self-heal marker, +// including edits pushed live to an online node, so keying the user-facing +// "saved, node offline, will sync" toast off it fired the warning on every save +// to a perfectly healthy online node. func (s *NodeService) IsNodePending(id int) bool { - enabled, status, dirty, _, err := s.NodeSyncState(id) + enabled, status, _, _, err := s.NodeSyncState(id) if err != nil { return false } - return !enabled || status != "online" || dirty + return !enabled || status != "online" } func nodeMetricKey(id int, metric string) string { diff --git a/internal/web/service/node_dirty_test.go b/internal/web/service/node_dirty_test.go index 2d299a359..0de264114 100644 --- a/internal/web/service/node_dirty_test.go +++ b/internal/web/service/node_dirty_test.go @@ -110,6 +110,59 @@ func TestDelInboundClientByEmail_DisabledNodeClientMarksDirty(t *testing.T) { } } +// An online, enabled node that is merely config-dirty must NOT be reported as +// pending: every node-backed edit marks the node dirty as the reconcile +// self-heal marker, so keying the "saved, node offline, will sync" toast off +// the dirty flag fired it on every save to a healthy online node. +func TestIsNodePending_OnlineDirtyNodeIsNotPending(t *testing.T) { + setupConflictDB(t) + db := database.GetDB() + + node := &model.Node{Name: "n1", Address: "127.0.0.1", Port: 2096, ApiToken: "tok", Enable: true, Status: "online"} + if err := db.Create(node).Error; err != nil { + t.Fatalf("create node: %v", err) + } + + nodeSvc := NodeService{} + if nodeSvc.IsNodePending(node.Id) { + t.Fatal("a clean online node must not be pending") + } + if err := nodeSvc.MarkNodeDirty(node.Id); err != nil { + t.Fatalf("MarkNodeDirty: %v", err) + } + if nodeSvc.IsNodePending(node.Id) { + t.Fatal("an online, enabled node must not be pending just because it is config-dirty") + } +} + +// Offline or disabled nodes are genuinely deferred and must report pending so +// the "saved, node offline, will sync" toast still surfaces for them. +func TestIsNodePending_OfflineOrDisabledIsPending(t *testing.T) { + setupConflictDB(t) + db := database.GetDB() + + offline := &model.Node{Name: "off", Address: "127.0.0.1", Port: 2096, ApiToken: "tok", Enable: true, Status: "offline"} + disabled := &model.Node{Name: "dis", Address: "127.0.0.1", Port: 2097, ApiToken: "tok", Enable: false, Status: "online"} + for _, n := range []*model.Node{offline, disabled} { + if err := db.Create(n).Error; err != nil { + t.Fatalf("create node %s: %v", n.Name, err) + } + } + // Node.Enable carries gorm default:true, so Create({Enable:false}) persists + // TRUE — force the column off to actually exercise the disabled path. + if err := db.Model(&model.Node{}).Where("id = ?", disabled.Id).Update("enable", false).Error; err != nil { + t.Fatalf("force-disable node: %v", err) + } + + nodeSvc := NodeService{} + if !nodeSvc.IsNodePending(offline.Id) { + t.Fatal("an offline node must be pending") + } + if !nodeSvc.IsNodePending(disabled.Id) { + t.Fatal("a disabled node must be pending") + } +} + // ClearNodeDirty must be a compare-and-swap on config_dirty_at so a concurrent // edit that re-dirties the node during a reconcile is not silently cleared. func TestNodeDirty_ClearIsCASOnDirtyAt(t *testing.T) {