fix/test-failures (#112)

Reviewed-on: graham/towerops-web#112

lib/towerops/agents/agent_assignment.ex

@@ -48,7 +48,7 @@ defmodule Towerops.Agents.AgentAssignment do
     assignment
     |> cast(attrs, [:agent_token_id, :device_id, :enabled])
     |> validate_required([:agent_token_id, :device_id])
-    |> unique_constraint([:device_id])
+    |> unique_constraint([:device_id], name: "agent_assignments_equipment_id_index")
     |> foreign_key_constraint(:agent_token_id)
     |> foreign_key_constraint(:device_id)
   end

lib/towerops/settings.ex

@@ -44,7 +44,7 @@ defmodule Towerops.Settings do
   end
 
   @doc """
-  Updates a setting value.
+  Updates a setting value, creating it if it doesn't exist.
 
   ## Examples
 
@@ -57,7 +57,16 @@ defmodule Towerops.Settings do
   def update_setting(key, value) when is_binary(key) do
     case get_setting_record(key) do
       nil ->
-        {:error, :setting_not_found}
+        # Create the setting if it doesn't exist
+        attrs = %{
+          key: key,
+          value: to_string_or_nil(value),
+          value_type: infer_value_type(key)
+        }
+
+        %ApplicationSetting{}
+        |> ApplicationSetting.changeset(attrs)
+        |> Repo.insert()
 
       setting ->
         setting
@@ -114,4 +123,8 @@ defmodule Towerops.Settings do
 
   defp to_string_or_nil(nil), do: nil
   defp to_string_or_nil(value), do: to_string(value)
+
+  # Infer the value type based on the setting key
+  defp infer_value_type("global_default_cloud_poller_id"), do: "uuid"
+  defp infer_value_type(_key), do: "string"
 end

lib/towerops/snmp/deferred_discovery.ex

@@ -60,11 +60,13 @@ defmodule Towerops.Snmp.DeferredDiscovery do
 
     task = Task.async(fn -> check_fn.() end)
 
-    case Task.yield(task, timeout) || Task.shutdown(task, :brutal_kill) do
+    case Task.yield(task, timeout) do
       {:ok, result} ->
         result
 
       nil ->
+        # Task timed out - kill it and use default
+        Task.shutdown(task, :brutal_kill)
         Logger.warning("Slow check timed out after #{timeout}ms, using default value")
         {:ok, default_value}
     end

priv/repo/migrations/20260322175649_remove_account_count_default.exs

@@ -0,0 +1,17 @@
+defmodule Towerops.Repo.Migrations.RemoveAccountCountDefault do
+  use Ecto.Migration
+
+  def up do
+    # Remove default value from account_count
+    alter table(:gaiia_network_sites) do
+      modify :account_count, :integer, default: nil
+    end
+  end
+
+  def down do
+    # Restore default value
+    alter table(:gaiia_network_sites) do
+      modify :account_count, :integer, default: 0
+    end
+  end
+end

priv/repo/structure.sql

@@ -2,7 +2,7 @@
 -- PostgreSQL database dump
 --
 
-\restrict 32QbDTL5DhFj1lcei3mnfG1cknYpwB9C9Oj4tDgcCDVv7jE32mmRzIFwys9vq6R
+\restrict RRAhb3L0Us7vE3B3YUZQWFpUy9oe9JDkh3ZeMLm2wvwJPMjBbIXH8zFu6qq0U19
 
 -- Dumped from database version 17.9 (Homebrew)
 -- Dumped by pg_dump version 17.9 (Homebrew)
@@ -4399,7 +4399,7 @@ CREATE TABLE public.gaiia_network_sites (
     name character varying(255),
     address jsonb,
     ip_blocks character varying(255)[] DEFAULT ARRAY[]::character varying[],
-    account_count integer DEFAULT 0,
+    account_count integer,
     total_mrr numeric,
     site_id uuid,
     raw_data jsonb,
@@ -12801,7 +12801,7 @@ ALTER TABLE ONLY public.wireless_clients
 -- PostgreSQL database dump complete
 --
 
-\unrestrict 32QbDTL5DhFj1lcei3mnfG1cknYpwB9C9Oj4tDgcCDVv7jE32mmRzIFwys9vq6R
+\unrestrict RRAhb3L0Us7vE3B3YUZQWFpUy9oe9JDkh3ZeMLm2wvwJPMjBbIXH8zFu6qq0U19
 
 INSERT INTO public."schema_migrations" (version) VALUES (20251221192340);
 INSERT INTO public."schema_migrations" (version) VALUES (20251221192454);
@@ -12995,3 +12995,4 @@ INSERT INTO public."schema_migrations" (version) VALUES (20260317224403);
 INSERT INTO public."schema_migrations" (version) VALUES (20260322152809);
 INSERT INTO public."schema_migrations" (version) VALUES (20260322161037);
 INSERT INTO public."schema_migrations" (version) VALUES (20260322161750);
+INSERT INTO public."schema_migrations" (version) VALUES (20260322175649);

test/towerops/settings_test.exs

@@ -6,7 +6,7 @@ defmodule Towerops.SettingsTest do
 
   describe "get_setting/1" do
     test "returns parsed value for existing setting" do
-      # The migration already created the global_default_cloud_poller_id setting
+      {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid")
       assert Settings.get_setting("global_default_cloud_poller_id") == nil
     end
 
@@ -75,6 +75,7 @@ defmodule Towerops.SettingsTest do
 
   describe "get_setting_record/1" do
     test "returns the raw ApplicationSetting record" do
+      {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid")
       record = Settings.get_setting_record("global_default_cloud_poller_id")
       assert %ApplicationSetting{} = record
       assert record.key == "global_default_cloud_poller_id"
@@ -88,6 +89,7 @@ defmodule Towerops.SettingsTest do
 
   describe "update_setting/2" do
     test "updates an existing setting value" do
+      {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid")
       agent_token_id = Ecto.UUID.generate()
       {:ok, updated} = Settings.update_setting("global_default_cloud_poller_id", agent_token_id)
 
@@ -96,6 +98,7 @@ defmodule Towerops.SettingsTest do
     end
 
     test "updates setting to nil" do
+      {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid")
       agent_token_id = Ecto.UUID.generate()
       {:ok, _} = Settings.update_setting("global_default_cloud_poller_id", agent_token_id)
       {:ok, updated} = Settings.update_setting("global_default_cloud_poller_id", nil)
@@ -104,9 +107,11 @@ defmodule Towerops.SettingsTest do
       assert Settings.get_setting("global_default_cloud_poller_id") == nil
     end
 
-    test "returns error for non-existent setting" do
-      assert {:error, :setting_not_found} =
-               Settings.update_setting("nonexistent_key", "value")
+    test "creates setting if it doesn't exist" do
+      assert {:ok, setting} = Settings.update_setting("nonexistent_key", "value")
+      assert setting.key == "nonexistent_key"
+      assert setting.value == "value"
+      assert setting.value_type == "string"
     end
 
     test "converts non-string values to strings" do
@@ -152,11 +157,14 @@ defmodule Towerops.SettingsTest do
 
   describe "list_all_settings/0" do
     test "returns all application settings" do
+      # Create a test setting first
+      {:ok, _} = create_setting("test_setting", "test_value", "string")
+
       settings = Settings.list_all_settings()
 
       assert is_list(settings)
       refute Enum.empty?(settings)
-      assert Enum.any?(settings, &(&1.key == "global_default_cloud_poller_id"))
+      assert Enum.any?(settings, &(&1.key == "test_setting"))
     end
   end