web: fix node disappearance role reset (#707)

* web: fix node disappearance role reset

* web: address review comments

* web: address review comments

* web: address review comments

web/spec/data_processing_spec.rb

@@ -136,14 +136,10 @@ RSpec.describe PotatoMesh::App::DataProcessing do
     end
   end
 
-  # ---------------------------------------------------------------------------
-  # insert_telemetry (telemetry_type validation path)
-  # ---------------------------------------------------------------------------
-  describe "#insert_telemetry" do
+  shared_context "with isolated db" do
     around do |example|
       Dir.mktmpdir("dp-spec-") do |dir|
         db_path = File.join(dir, "mesh.db")
-
         RSpec::Mocks.with_temporary_scope do
           allow(PotatoMesh::Config).to receive(:db_path).and_return(db_path)
           allow(PotatoMesh::Config).to receive(:db_busy_timeout_ms).and_return(5000)
@@ -164,7 +160,35 @@ RSpec.describe PotatoMesh::App::DataProcessing do
       db
     end
 
+    # Return the full node row for the canonical test node ID.
+    def read_node(db)
+      db.execute("SELECT * FROM nodes WHERE node_id = '!aabbccdd'").first
+    end
+
+    # Insert the canonical test node with full user info and CLIENT_BASE role.
+    def seed_node(db)
+      dp.upsert_node(db, "!aabbccdd", {
+        "lastHeard" => now - 100,
+        "num" => 0xaabbccdd,
+        "user" => {
+          "role" => "CLIENT_BASE",
+          "longName" => "Real Long Name",
+          "shortName" => "RLN",
+          "macaddr" => "aa:bb:cc:dd:ee:ff",
+          "hwModel" => "TBEAM",
+          "publicKey" => "abc123",
+        },
+      })
+    end
+
     let(:now) { Time.now.to_i }
+  end
+
+  # ---------------------------------------------------------------------------
+  # insert_telemetry (telemetry_type validation path)
+  # ---------------------------------------------------------------------------
+  describe "#insert_telemetry" do
+    include_context "with isolated db"
 
     let(:valid_payload) do
       {
@@ -209,4 +233,143 @@ RSpec.describe PotatoMesh::App::DataProcessing do
       expect(result).to be_nil
     end
   end
+
+  # ---------------------------------------------------------------------------
+  # upsert_node — Bug 1: lastHeard = 0 must not be stored as 0
+  # ---------------------------------------------------------------------------
+  describe "#upsert_node — last_heard zero handling" do
+    include_context "with isolated db"
+
+    it "treats lastHeard=0 as absent, storing approximately now instead" do
+      db = open_db
+      dp.upsert_node(db, "!aabbccdd", { "lastHeard" => 0, "num" => 0xaabbccdd })
+      lh = read_node(db)["last_heard"]
+      db.close
+      expect(lh).not_to eq(0)
+      expect(lh).to be_within(5).of(now)
+    end
+
+    it "uses position time as last_heard fallback when lastHeard is 0" do
+      pt = now - 300
+      db = open_db
+      dp.upsert_node(db, "!aabbccdd", {
+        "lastHeard" => 0,
+        "num" => 0xaabbccdd,
+        "position" => { "time" => pt, "latitude" => 1.0, "longitude" => 2.0 },
+      })
+      expect(read_node(db)["last_heard"]).to eq(pt)
+      db.close
+    end
+
+    it "stores a positive lastHeard value as-is" do
+      lh = now - 120
+      db = open_db
+      dp.upsert_node(db, "!aabbccdd", { "lastHeard" => lh, "num" => 0xaabbccdd })
+      expect(read_node(db)["last_heard"]).to eq(lh)
+      db.close
+    end
+
+    it "includes the node in query_nodes results after lastHeard=0 fix" do
+      db = open_db
+      dp.upsert_node(db, "!aabbccdd", { "lastHeard" => 0, "num" => 0xaabbccdd })
+      db.close
+      # query_nodes applies a 7-day floor; the node must appear
+      expect(dp.query_nodes(100).map { |n| n["node_id"] }).to include("!aabbccdd")
+    end
+  end
+
+  # ---------------------------------------------------------------------------
+  # upsert_node — Bug 2: role must not be reset by no-user packets
+  # ---------------------------------------------------------------------------
+  describe "#upsert_node — role preservation" do
+    include_context "with isolated db"
+
+    it "preserves CLIENT_BASE role when a no-user packet arrives" do
+      db = open_db
+      seed_node(db)
+      dp.upsert_node(db, "!aabbccdd", { "lastHeard" => now, "num" => 0xaabbccdd })
+      expect(read_node(db)["role"]).to eq("CLIENT_BASE")
+      db.close
+    end
+
+    it "updates role when an explicit role is supplied" do
+      db = open_db
+      seed_node(db)
+      dp.upsert_node(db, "!aabbccdd", {
+        "lastHeard" => now,
+        "num" => 0xaabbccdd,
+        "user" => { "role" => "CLIENT", "longName" => "Real Long Name", "shortName" => "RLN" },
+      })
+      expect(read_node(db)["role"]).to eq("CLIENT")
+      db.close
+    end
+
+    it "stores NULL role for new nodes without user info (display layer supplies CLIENT)" do
+      db = open_db
+      dp.upsert_node(db, "!aabbccdd", { "lastHeard" => now, "num" => 0xaabbccdd })
+      # NULL is stored; query_nodes applies r["role"] ||= "CLIENT" for display
+      expect(read_node(db)["role"]).to be_nil
+      db.close
+    end
+  end
+
+  # ---------------------------------------------------------------------------
+  # upsert_node — Bug 3: identity fields must not be overwritten with NULL
+  # ---------------------------------------------------------------------------
+  describe "#upsert_node — identity field preservation" do
+    include_context "with isolated db"
+
+    it "preserves all identity fields when a no-user packet arrives" do
+      db = open_db
+      seed_node(db)
+      dp.upsert_node(db, "!aabbccdd", { "lastHeard" => now, "num" => 0xaabbccdd })
+      row = read_node(db)
+      db.close
+      expect(row["short_name"]).to eq("RLN")
+      expect(row["long_name"]).to eq("Real Long Name")
+      expect(row["macaddr"]).to eq("aa:bb:cc:dd:ee:ff")
+      expect(row["hw_model"]).to eq("TBEAM")
+      expect(row["public_key"]).to eq("abc123")
+    end
+
+    it "updates short_name when a real value is supplied" do
+      db = open_db
+      seed_node(db)
+      dp.upsert_node(db, "!aabbccdd", {
+        "lastHeard" => now,
+        "num" => 0xaabbccdd,
+        "user" => { "shortName" => "NEW", "longName" => "New Long Name" },
+      })
+      expect(read_node(db)["short_name"]).to eq("NEW")
+      db.close
+    end
+
+    it "does not overwrite a real long_name with a generic placeholder" do
+      db = open_db
+      seed_node(db)
+      dp.upsert_node(db, "!aabbccdd", {
+        "lastHeard" => now,
+        "num" => 0xaabbccdd,
+        "user" => { "longName" => "Meshtastic CCDD", "shortName" => "RLN" },
+      })
+      expect(read_node(db)["long_name"]).to eq("Real Long Name")
+      db.close
+    end
+
+    it "overwrites a generic placeholder long_name with a real long_name" do
+      db = open_db
+      dp.upsert_node(db, "!aabbccdd", {
+        "lastHeard" => now - 100,
+        "num" => 0xaabbccdd,
+        "user" => { "longName" => "Meshtastic CCDD", "shortName" => "CCDD" },
+      })
+      dp.upsert_node(db, "!aabbccdd", {
+        "lastHeard" => now,
+        "num" => 0xaabbccdd,
+        "user" => { "longName" => "Real Long Name", "shortName" => "RLN" },
+      })
+      expect(read_node(db)["long_name"]).to eq("Real Long Name")
+      db.close
+    end
+  end
 end