Merge pull request #556 from guarzo/guarzo/ticker2andsse

fix: sse enable checkbox, and kills ticker

lib/wanderer_app/api/map.ex

@@ -8,6 +8,8 @@ defmodule WandererApp.Api.Map do
 
   alias Ash.Resource.Change.Builtins
 
+  require Logger
+
   postgres do
     repo(WandererApp.Repo)
     table("maps_v1")
@@ -55,6 +57,7 @@ defmodule WandererApp.Api.Map do
     define(:mark_as_deleted, action: :mark_as_deleted)
     define(:update_api_key, action: :update_api_key)
     define(:toggle_webhooks, action: :toggle_webhooks)
+    define(:toggle_sse, action: :toggle_sse)
 
     define(:by_id,
       get_by: [:id],
@@ -103,7 +106,16 @@ defmodule WandererApp.Api.Map do
     end
 
     create :new do
-      accept [:name, :slug, :description, :scope, :only_tracked_characters, :owner_id, :sse_enabled]
+      accept [
+        :name,
+        :slug,
+        :description,
+        :scope,
+        :only_tracked_characters,
+        :owner_id,
+        :sse_enabled
+      ]
+
       primary?(true)
       argument :create_default_acl, :boolean, allow_nil?: true
       argument :acls, {:array, :uuid}, allow_nil?: true
@@ -188,6 +200,14 @@ defmodule WandererApp.Api.Map do
       require_atomic? false
     end
 
+    update :toggle_sse do
+      require_atomic? false
+      accept [:sse_enabled]
+
+      # Validate subscription when enabling SSE
+      validate &validate_sse_subscription/2
+    end
+
     create :duplicate do
       accept [:name, :description, :scope, :only_tracked_characters]
       argument :source_map_id, :uuid, allow_nil?: false
@@ -373,19 +393,13 @@ defmodule WandererApp.Api.Map do
     end
   end
 
-  # Private validation functions
-
-  @doc false
-  # Validates that SSE can be enabled based on subscription status.
+  # SSE Subscription Validation
   #
-  # Validation rules:
-  # 1. Skip if SSE not being enabled (no validation needed)
-  # 2. Skip during map creation (map_id is nil, subscription doesn't exist yet)
-  # 3. Skip in Community Edition mode (subscriptions disabled globally)
-  # 4. Require active subscription in Enterprise mode
-  #
-  # This ensures users cannot enable SSE without a valid subscription in Enterprise mode,
-  # while allowing SSE in Community Edition and during map creation.
+  # This validation ensures that SSE can only be enabled when:
+  # 1. SSE is being disabled (always allowed)
+  # 2. Map is being created (skip validation, will be checked on first update)
+  # 3. Community Edition mode (always allowed)
+  # 4. Enterprise mode with active subscription
   defp validate_sse_subscription(changeset, _context) do
     sse_enabled = Ash.Changeset.get_attribute(changeset, :sse_enabled)
     map_id = changeset.data.id
@@ -397,7 +411,6 @@ defmodule WandererApp.Api.Map do
         :ok
 
       # Map creation (no ID yet) - skip validation
-      # Subscription check will happen on first update if they try to enable SSE
       is_nil(map_id) ->
         :ok
 
@@ -411,7 +424,6 @@ defmodule WandererApp.Api.Map do
     end
   end
 
-  # Helper to check if map has an active subscription
   defp validate_active_subscription(map_id) do
     case WandererApp.Map.is_subscription_active?(map_id) do
       {:ok, true} ->
@@ -421,11 +433,8 @@ defmodule WandererApp.Api.Map do
         {:error, field: :sse_enabled, message: "Active subscription required to enable SSE"}
 
       {:error, reason} ->
-        require Logger
-        Logger.warning("Failed to check subscription for map #{map_id}: #{inspect(reason)}")
-        # Fail open - allow the operation but log the error
-        # This prevents database errors from blocking legitimate operations
-        :ok
+        Logger.error("Error checking subscription status: #{inspect(reason)}")
+        {:error, field: :sse_enabled, message: "Unable to verify subscription status"}
     end
   end
 end

lib/wanderer_app/api/map_webhook_subscription.ex

@@ -58,6 +58,7 @@ defmodule WandererApp.Api.MapWebhookSubscription do
         :consecutive_failures,
         :secret
       ]
+
       require_atomic? false
     end
 

lib/wanderer_app/esi/api_client.ex

@@ -463,7 +463,8 @@ defmodule WandererApp.Esi.ApiClient do
 
         {:error, reason} ->
           # Check if this is a Finch pool error
-          if is_exception(reason) and Exception.message(reason) =~ "unable to provide a connection" do
+          if is_exception(reason) and
+               Exception.message(reason) =~ "unable to provide a connection" do
             :telemetry.execute(
               [:wanderer_app, :finch, :pool_exhausted],
               %{count: 1},
@@ -677,7 +678,8 @@ defmodule WandererApp.Esi.ApiClient do
 
         {:error, reason} ->
           # Check if this is a Finch pool error
-          if is_exception(reason) and Exception.message(reason) =~ "unable to provide a connection" do
+          if is_exception(reason) and
+               Exception.message(reason) =~ "unable to provide a connection" do
             :telemetry.execute(
               [:wanderer_app, :finch, :pool_exhausted],
               %{count: 1},

lib/wanderer_app/kills/message_handler.ex

@@ -403,10 +403,24 @@ defmodule WandererApp.Kills.MessageHandler do
 
   defp extract_field(_data, _field_names), do: nil
 
-  # Specific field extractors using the generic function
+  # Generic nested field extraction - tries flat keys first, then nested object
+  @spec extract_nested_field(map(), list(String.t()), String.t(), String.t()) :: String.t() | nil
+  defp extract_nested_field(data, flat_keys, nested_key, field) when is_map(data) do
+    case extract_field(data, flat_keys) do
+      nil ->
+        case data[nested_key] do
+          %{^field => value} when is_binary(value) and value != "" -> value
+          _ -> nil
+        end
+
+      value ->
+        value
+    end
+  end
+
+  # Specific field extractors using the generic functions
   @spec get_character_name(map() | any()) :: String.t() | nil
   defp get_character_name(data) when is_map(data) do
-    # Try multiple possible field names
     field_names = ["attacker_name", "victim_name", "character_name", "name"]
 
     extract_field(data, field_names) ||
@@ -419,30 +433,26 @@ defmodule WandererApp.Kills.MessageHandler do
   defp get_character_name(_), do: nil
 
   @spec get_corp_ticker(map() | any()) :: String.t() | nil
-  defp get_corp_ticker(data) when is_map(data) do
-    extract_field(data, ["corporation_ticker", "corp_ticker"])
-  end
+  defp get_corp_ticker(data) when is_map(data),
+    do: extract_nested_field(data, ["corporation_ticker", "corp_ticker"], "corporation", "ticker")
 
   defp get_corp_ticker(_), do: nil
 
   @spec get_corp_name(map() | any()) :: String.t() | nil
-  defp get_corp_name(data) when is_map(data) do
-    extract_field(data, ["corporation_name", "corp_name"])
-  end
+  defp get_corp_name(data) when is_map(data),
+    do: extract_nested_field(data, ["corporation_name", "corp_name"], "corporation", "name")
 
   defp get_corp_name(_), do: nil
 
   @spec get_alliance_ticker(map() | any()) :: String.t() | nil
-  defp get_alliance_ticker(data) when is_map(data) do
-    extract_field(data, ["alliance_ticker"])
-  end
+  defp get_alliance_ticker(data) when is_map(data),
+    do: extract_nested_field(data, ["alliance_ticker"], "alliance", "ticker")
 
   defp get_alliance_ticker(_), do: nil
 
   @spec get_alliance_name(map() | any()) :: String.t() | nil
-  defp get_alliance_name(data) when is_map(data) do
-    extract_field(data, ["alliance_name"])
-  end
+  defp get_alliance_name(data) when is_map(data),
+    do: extract_nested_field(data, ["alliance_name"], "alliance", "name")
 
   defp get_alliance_name(_), do: nil
 

lib/wanderer_app_web/live/admin/admin_live.html.heex

@@ -336,8 +336,8 @@
         label="Valid"
         options={Enum.map(@valid_types, fn valid_type -> {valid_type.label, valid_type.id} end)}
       />
-
-      <!-- Modal action buttons -->
+      
+<!-- Modal action buttons -->
       <div class="modal-action">
         <.button class="mt-2" type="submit" phx-disable-with="Saving...">
           {(@live_action == :add_invite_link && "Add") || "Save"}

lib/wanderer_app_web/live/maps/maps_live.ex

@@ -163,6 +163,7 @@ defmodule WandererAppWeb.MapsLive do
         |> assign(:map_slug, map_slug)
         |> assign(:map_id, map.id)
         |> assign(:public_api_key, map.public_api_key)
+        |> assign(:sse_enabled, map.sse_enabled)
         |> assign(:map, map)
         |> assign(
           export_settings: export_settings |> _get_export_map_data(),
@@ -232,6 +233,27 @@ defmodule WandererAppWeb.MapsLive do
     {:noreply, assign(socket, public_api_key: new_api_key)}
   end
 
+  def handle_event("toggle-sse", _params, socket) do
+    new_sse_enabled = not socket.assigns.sse_enabled
+    map = socket.assigns.map
+
+    case WandererApp.Api.Map.toggle_sse(map, %{sse_enabled: new_sse_enabled}) do
+      {:ok, updated_map} ->
+        {:noreply, assign(socket, sse_enabled: new_sse_enabled, map: updated_map)}
+
+      {:error, %Ash.Error.Invalid{errors: errors}} ->
+        error_message =
+          errors
+          |> Enum.map(fn error -> Map.get(error, :message, "Unknown error") end)
+          |> Enum.join(", ")
+
+        {:noreply, put_flash(socket, :error, error_message)}
+
+      {:error, _} ->
+        {:noreply, put_flash(socket, :error, "Failed to update SSE setting")}
+    end
+  end
+
   @impl true
   def handle_event(
         "live_select_change",

lib/wanderer_app_web/live/maps/maps_live.html.heex

@@ -540,6 +540,24 @@
                     </.button>
                   </div>
                 </div>
+
+                <div class="border-t border-stone-700 mt-4 pt-4">
+                  <h3 class="text-md font-semibold mb-3">Server-Sent Events (SSE)</h3>
+                  <div class="flex items-center gap-3">
+                    <label class="flex items-center gap-2 cursor-pointer">
+                      <input
+                        type="checkbox"
+                        class="checkbox checkbox-primary"
+                        checked={@sse_enabled}
+                        phx-click="toggle-sse"
+                      />
+                      <span>Enable SSE for this map</span>
+                    </label>
+                  </div>
+                  <p class="text-sm text-stone-400 mt-2">
+                    When enabled, external clients can subscribe to real-time map events via SSE.
+                  </p>
+                </div>
               </div>
 
               <.live_component

priv/repo/migrations/20251118202140_add_public_api_key_unique_index.exs

@@ -52,25 +52,24 @@ defmodule WandererApp.Repo.Migrations.AddPublicApiKeyUniqueIndex do
   end
 
   defp create_backup_table do
-    repo().query!("""
-    CREATE TABLE IF NOT EXISTS maps_v1_api_key_backup (
-      id UUID PRIMARY KEY,
-      map_id UUID NOT NULL,
-      old_public_api_key TEXT NOT NULL,
-      reason TEXT NOT NULL,
-      backed_up_at TIMESTAMP NOT NULL DEFAULT NOW()
+    repo().query!(
+      """
+      CREATE TABLE IF NOT EXISTS maps_v1_api_key_backup (
+        id UUID PRIMARY KEY,
+        map_id UUID NOT NULL,
+        old_public_api_key TEXT NOT NULL,
+        reason TEXT NOT NULL,
+        backed_up_at TIMESTAMP NOT NULL DEFAULT NOW()
+      )
+      """,
+      []
     )
-    """, [])
 
     IO.puts("Created backup table maps_v1_api_key_backup")
   end
 
   def down do
-    drop_if_exists(
-      index(:maps_v1, [:public_api_key],
-        name: :maps_v1_unique_public_api_key_index
-      )
-    )
+    drop_if_exists(index(:maps_v1, [:public_api_key], name: :maps_v1_unique_public_api_key_index))
 
     IO.puts("Dropped unique index on maps_v1.public_api_key")
 
@@ -119,6 +118,7 @@ defmodule WandererApp.Repo.Migrations.AddPublicApiKeyUniqueIndex do
                 INSERT INTO maps_v1_api_key_backup (id, map_id, old_public_api_key, reason)
                 VALUES (gen_random_uuid(), $1::uuid, $2, 'duplicate_api_key_cleared_for_unique_index')
                 """
+
                 repo().query!(backup_query, [id, api_key])
 
                 # Clear the duplicate

test/integration/api/v1/map_system_api_v1_test.exs

@@ -247,9 +247,10 @@ defmodule WandererAppWeb.Api.V1.MapSystemApiV1Test do
       payload = %{
         "data" => %{
           "type" => "map_systems",
-          "attributes" => %{
-            # Missing solar_system_id - JSON:API returns 400 for schema validation
-          }
+          "attributes" =>
+            %{
+              # Missing solar_system_id - JSON:API returns 400 for schema validation
+            }
         }
       }
 

test/integration/map/character_location_tracking_test.exs

@@ -47,24 +47,26 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
     user = create_user(%{name: "Test User", hash: "test_hash_#{:rand.uniform(1_000_000)}"})
 
     # Create test character with location tracking scopes
-    character = create_character(%{
-      eve_id: "#{@test_character_eve_id}",
-      name: "Test Character",
-      user_id: user.id,
-      scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1",
-      tracking_pool: "default"
-    })
+    character =
+      create_character(%{
+        eve_id: "#{@test_character_eve_id}",
+        name: "Test Character",
+        user_id: user.id,
+        scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1",
+        tracking_pool: "default"
+      })
 
     # Create test map
     # Note: scope: :all is used because :none prevents system addition
     # (is_connection_valid returns false for :none scope)
-    map = create_map(%{
-      name: "Test Char Track",
-      slug: "test-char-tracking-#{:rand.uniform(1_000_000)}",
-      owner_id: character.id,
-      scope: :all,
-      only_tracked_characters: false
-    })
+    map =
+      create_map(%{
+        name: "Test Char Track",
+        slug: "test-char-tracking-#{:rand.uniform(1_000_000)}",
+        owner_id: character.id,
+        scope: :all,
+        only_tracked_characters: false
+      })
 
     on_exit(fn ->
       cleanup_test_data(map.id)
@@ -150,6 +152,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Setup: Character starts at Jita
       set_character_location(character.id, @system_jita)
+
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
         @system_jita
@@ -157,6 +160,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # First update - start system is intentionally NOT added yet
       CharactersImpl.update_characters(map.id)
+
       refute system_on_map?(map.id, @system_jita),
              "Start system should not be added until character moves"
 
@@ -167,8 +171,11 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       CharactersImpl.update_characters(map.id)
 
       # Verify: Both systems should be on map after character moves
-      assert wait_for_system_on_map(map.id, @system_jita), "Jita should be added after character moves"
-      assert wait_for_system_on_map(map.id, @system_amarr), "Amarr should be added as the new location"
+      assert wait_for_system_on_map(map.id, @system_jita),
+             "Jita should be added after character moves"
+
+      assert wait_for_system_on_map(map.id, @system_amarr),
+             "Amarr should be added as the new location"
     end
   end
 
@@ -185,6 +192,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Character starts at Jita
       set_character_location(character.id, @system_jita)
+
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
         @system_jita
@@ -192,6 +200,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # First update - start system is intentionally NOT added yet
       CharactersImpl.update_characters(map.id)
+
       refute system_on_map?(map.id, @system_jita),
              "Start system should not be added until character moves"
 
@@ -202,7 +211,9 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       CharactersImpl.update_characters(map.id)
 
       # Verify both Jita and Amarr are now on map
-      assert wait_for_system_on_map(map.id, @system_jita), "Jita (start) should be on map after movement"
+      assert wait_for_system_on_map(map.id, @system_jita),
+             "Jita (start) should be on map after movement"
+
       assert wait_for_system_on_map(map.id, @system_amarr), "Amarr should be on map"
 
       # Rapid jump to Dodixie before next update cycle
@@ -213,7 +224,10 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Verify: All three systems should be on map
       assert wait_for_system_on_map(map.id, @system_jita), "Jita (start) should still be on map"
-      assert wait_for_system_on_map(map.id, @system_amarr), "Amarr (intermediate) should still be on map - this is the critical test"
+
+      assert wait_for_system_on_map(map.id, @system_amarr),
+             "Amarr (intermediate) should still be on map - this is the critical test"
+
       assert wait_for_system_on_map(map.id, @system_dodixie), "Dodixie (end) should be on map"
     end
 
@@ -230,6 +244,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Start at Jita
       set_character_location(character.id, @system_jita)
+
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
         @system_jita
@@ -284,9 +299,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Verify start_solar_system_id still exists after first update
       {:ok, start_system} =
-        WandererApp.Cache.lookup(
-          "map:#{map.id}:character:#{character.id}:start_solar_system_id"
-        )
+        WandererApp.Cache.lookup("map:#{map.id}:character:#{character.id}:start_solar_system_id")
 
       assert start_system == @system_jita,
              "start_solar_system_id should persist after first update (not be taken/removed)"
@@ -369,6 +382,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Set character at Jita and set start location
       set_character_location(character.id, @system_jita)
+
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
         @system_jita
@@ -401,6 +415,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Set up character location
       set_character_location(character.id, @system_jita)
+
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
         @system_jita
@@ -424,19 +439,22 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Create a second character
       user2 = create_user(%{name: "Test User 2", hash: "test_hash_#{:rand.uniform(1_000_000)}"})
-      character2 = create_character(%{
-        eve_id: "#{@test_character_eve_id + 1}",
-        name: "Test Character 2",
-        user_id: user2.id,
-        scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1",
-        tracking_pool: "default"
-      })
+
+      character2 =
+        create_character(%{
+          eve_id: "#{@test_character_eve_id + 1}",
+          name: "Test Character 2",
+          user_id: user2.id,
+          scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1",
+          tracking_pool: "default"
+        })
 
       # Add both characters to map presence
       add_character_to_map_presence(map.id, character2.id)
 
       # Set locations for both characters
       set_character_location(character2.id, @system_amarr)
+
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character2.id}:start_solar_system_id",
         @system_amarr
@@ -464,6 +482,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Set up character with location
       set_character_location(character.id, @system_jita)
+
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
         @system_jita
@@ -491,32 +510,38 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       ensure_map_started(map.id)
 
       # Create multiple characters for concurrent processing
-      characters = for i <- 1..5 do
-        user = create_user(%{
-          name: "Test User #{i}",
-          hash: "test_hash_#{:rand.uniform(1_000_000)}"
-        })
+      characters =
+        for i <- 1..5 do
+          user =
+            create_user(%{
+              name: "Test User #{i}",
+              hash: "test_hash_#{:rand.uniform(1_000_000)}"
+            })
 
-        character = create_character(%{
-          eve_id: "#{@test_character_eve_id + i}",
-          name: "Test Character #{i}",
-          user_id: user.id,
-          scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1",
-          tracking_pool: "default"
-        })
+          character =
+            create_character(%{
+              eve_id: "#{@test_character_eve_id + i}",
+              name: "Test Character #{i}",
+              user_id: user.id,
+              scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1",
+              tracking_pool: "default"
+            })
 
-        # Add character to presence and set location
-        add_character_to_map_presence(map.id, character.id)
+          # Add character to presence and set location
+          add_character_to_map_presence(map.id, character.id)
 
-        solar_system_id = Enum.at([@system_jita, @system_amarr, @system_dodixie, @system_rens], rem(i, 4))
-        set_character_location(character.id, solar_system_id)
-        WandererApp.Cache.insert(
-          "map:#{map.id}:character:#{character.id}:start_solar_system_id",
-          solar_system_id
-        )
+          solar_system_id =
+            Enum.at([@system_jita, @system_amarr, @system_dodixie, @system_rens], rem(i, 4))
 
-        character
-      end
+          set_character_location(character.id, solar_system_id)
+
+          WandererApp.Cache.insert(
+            "map:#{map.id}:character:#{character.id}:start_solar_system_id",
+            solar_system_id
+          )
+
+          character
+        end
 
       # Run update_characters - should handle all characters concurrently
       result = CharactersImpl.update_characters(map.id)
@@ -563,7 +588,8 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
       CharactersImpl.update_characters(map.id)
 
       # Should receive start and complete events (or error event if something failed)
-      assert_receive {:telemetry_event, [:wanderer_app, :map, :update_characters, :start], _, _}, 1000
+      assert_receive {:telemetry_event, [:wanderer_app, :map, :update_characters, :start], _, _},
+                     1000
 
       # Should receive either complete or error event
       receive do
@@ -593,6 +619,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Set location in character cache
       set_character_location(character.id, @system_jita)
+
       WandererApp.Cache.insert(
         "map:#{map.id}:character:#{character.id}:start_solar_system_id",
         @system_jita
@@ -613,10 +640,12 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do
 
       # Verify both caches updated
       {:ok, character_data} = Cachex.get(:character_cache, character.id)
+
       {:ok, map_cached_location} =
         WandererApp.Cache.lookup("map:#{map.id}:character:#{character.id}:solar_system_id")
 
       assert character_data.solar_system_id == @system_amarr
+
       assert map_cached_location == @system_amarr,
              "Both caches should be consistent after update"
     end

test/support/map_test_helpers.ex

@@ -356,7 +356,8 @@ defmodule WandererApp.MapTestHelpers do
   def set_character_location(character_id, solar_system_id, opts \\ []) do
     structure_id = opts[:structure_id]
     station_id = opts[:station_id]
-    ship = opts[:ship] || 670  # Capsule
+    # Capsule
+    ship = opts[:ship] || 670
 
     # First get the existing character from cache or database to maintain all fields
     {:ok, existing_character} = WandererApp.Character.get_character(character_id)