Address PR #1749 review M5 — Backpex.InitAssigns test coverage

Restores test coverage for the on_mount hook that replaced the deleted
ThemeSelectorPlug. Covers the happy path, malformed-session fallbacks,
and adapter-driven overrides for `global.theme`.

Also fixes a latent bug in `Backpex.Preferences.Adapters.Session.root/1`
uncovered while writing the malformed-session tests: a host app that
stomps on the session key with a non-map (binary/nil/other) caused
`get_in/2` to crash. `root/1` now coerces any non-map value to `%{}`.

Author: Flo0807

lib/backpex/preferences/adapters/session.ex

@@ -66,7 +66,17 @@ defmodule Backpex.Preferences.Adapters.Session do
     {:error, :requires_http}
   end
 
-  defp root(session) when is_map(session), do: Map.get(session, @session_key) || %{}
+  # The session key is expected to hold a map, but a misbehaving host app (or
+  # a session rewrite by another plug) can stomp on it with a non-map. Coerce
+  # any non-map value to `%{}` here so `get_in/2` upstream can't crash on a
+  # binary/number/etc.
+  defp root(session) when is_map(session) do
+    case Map.get(session, @session_key) do
+      map when is_map(map) -> map
+      _other -> %{}
+    end
+  end
+
   defp root(_other), do: %{}
 
   defp deep_put(map, [k], value), do: Map.put(map, k, value)

test/init_assigns_test.exs

@@ -0,0 +1,234 @@
+defmodule Backpex.InitAssignsTest do
+  use ExUnit.Case, async: false
+
+  alias Backpex.InitAssigns
+  alias Phoenix.LiveView.Lifecycle
+  alias Phoenix.LiveView.Socket
+
+  # --- test-only adapter --------------------------------------------------
+
+  defmodule StubAdapter do
+    @moduledoc false
+    # Returns fixed per-key values regardless of the context. Configure via
+    # application env; the `fetch/1` and `fetch_map/1` functions look the key
+    # up in a lookup map set by the test.
+    @behaviour Backpex.Preferences.Adapter
+
+    @table_env_key :backpex_init_assigns_test_stub_adapter
+
+    def set(values) when is_map(values) do
+      Application.put_env(:backpex, @table_env_key, values)
+    end
+
+    def clear, do: Application.delete_env(:backpex, @table_env_key)
+
+    defp values, do: Application.get_env(:backpex, @table_env_key, %{})
+
+    @impl Backpex.Preferences.Adapter
+    def get(_ctx, key, _opts) do
+      case Map.fetch(values(), key) do
+        {:ok, value} -> {:ok, value}
+        :error -> {:ok, :not_found}
+      end
+    end
+
+    @impl Backpex.Preferences.Adapter
+    def get_map(_ctx, prefix, _opts) do
+      map =
+        values()
+        |> Enum.flat_map(fn {k, v} ->
+          case maybe_strip(k, prefix <> ".") do
+            nil -> []
+            rest -> [{rest, v}]
+          end
+        end)
+        |> Map.new()
+
+      {:ok, map}
+    end
+
+    @impl Backpex.Preferences.Adapter
+    def put(_ctx, _key, _value, _opts), do: {:ok, [:noop]}
+
+    defp maybe_strip(key, prefix) do
+      case String.split(key, prefix, parts: 2) do
+        ["", rest] -> rest
+        _other -> nil
+      end
+    end
+  end
+
+  # --- setup --------------------------------------------------------------
+
+  setup do
+    on_exit(fn ->
+      StubAdapter.clear()
+      Application.delete_env(:backpex, Backpex.Preferences)
+    end)
+
+    :ok
+  end
+
+  # --- helpers ------------------------------------------------------------
+
+  # Builds a socket compatible with `Phoenix.LiveView.attach_hook/4`.
+  #
+  # `attach_hook(..., :handle_params, ...)` refuses a socket with `router: nil`,
+  # and both `attach_hook` and `assign/3` touch `socket.private` — so the
+  # private map must carry a `:lifecycle` struct and a `:live_temp` map.
+  defp build_socket do
+    %Socket{
+      endpoint: __MODULE__.Endpoint,
+      router: __MODULE__.Router,
+      assigns: %{__changed__: %{}},
+      private: %{
+        connect_info: %{},
+        lifecycle: %Lifecycle{},
+        live_temp: %{}
+      }
+    }
+  end
+
+  defp mount(session, socket \\ build_socket()) do
+    {:cont, socket} = InitAssigns.on_mount(:default, %{}, session, socket)
+    socket
+  end
+
+  # --- tests --------------------------------------------------------------
+
+  describe "on_mount/4 with an empty session" do
+    test "assigns the documented defaults (theme nil, sidebar open, no sections)" do
+      socket = mount(%{})
+
+      assert socket.assigns.current_theme == nil
+      assert socket.assigns.sidebar_open == true
+      assert socket.assigns.sidebar_section_states == %{}
+    end
+
+    test "treats a session that only has the preferences key present as empty" do
+      socket = mount(%{"backpex_preferences" => %{}})
+
+      assert socket.assigns.current_theme == nil
+      assert socket.assigns.sidebar_open == true
+      assert socket.assigns.sidebar_section_states == %{}
+    end
+
+    test "returns `{:cont, socket}` so subsequent hooks still run" do
+      # The public contract of a LiveView `on_mount` hook is the `{:cont | :halt,
+      # socket}` tuple. Pin the shape here so refactors cannot silently change
+      # semantics (e.g. to `{:halt, ...}`).
+      assert {:cont, %Socket{}} = InitAssigns.on_mount(:default, %{}, %{}, build_socket())
+    end
+  end
+
+  describe "on_mount/4 with preference values present in the session" do
+    test "mirrors the stored theme, sidebar_open, and sidebar_section_states" do
+      session = %{
+        "backpex_preferences" => %{
+          "global" => %{
+            "theme" => "dark",
+            "sidebar_open" => false,
+            "sidebar_section" => %{"users" => true, "blog" => false}
+          }
+        }
+      }
+
+      socket = mount(session)
+
+      assert socket.assigns.current_theme == "dark"
+      assert socket.assigns.sidebar_open == false
+      assert socket.assigns.sidebar_section_states == %{"users" => true, "blog" => false}
+    end
+
+    test "accepts non-boolean sidebar_open as-is (adapter has no schema enforcement)" do
+      # The Session adapter is a dumb key/value store — it returns whatever is
+      # stored. Document the current behavior: `sidebar_open` can hold any term
+      # if the host app writes one. Layout components are responsible for
+      # coercing.
+      session = %{"backpex_preferences" => %{"global" => %{"sidebar_open" => "nope"}}}
+      socket = mount(session)
+      assert socket.assigns.sidebar_open == "nope"
+    end
+  end
+
+  describe "on_mount/4 with a malformed session" do
+    test "falls back to defaults when `backpex_preferences` is a binary" do
+      # Pathological but possible: a host app stomps on the session key with a
+      # non-map. The Session adapter's `root/1` guards against this and the
+      # on_mount hook must not crash.
+      socket = mount(%{"backpex_preferences" => "oops"})
+
+      assert socket.assigns.current_theme == nil
+      assert socket.assigns.sidebar_open == true
+      assert socket.assigns.sidebar_section_states == %{}
+    end
+
+    test "falls back to defaults when `backpex_preferences` is explicitly nil" do
+      socket = mount(%{"backpex_preferences" => nil})
+
+      assert socket.assigns.current_theme == nil
+      assert socket.assigns.sidebar_open == true
+      assert socket.assigns.sidebar_section_states == %{}
+    end
+
+    test "returns the stored value unchanged when the theme slot holds a non-string" do
+      # The Session adapter doesn't type-check; surfacing the raw value lets
+      # layout code decide whether to coerce. Pin current behavior so a silent
+      # change to coerce-at-read surfaces here.
+      session = %{"backpex_preferences" => %{"global" => %{"theme" => 42}}}
+      socket = mount(session)
+      assert socket.assigns.current_theme == 42
+    end
+
+    test "sidebar_section_states falls back to %{} when the stored subtree is not a map" do
+      # `Preferences.get_map/3` must degrade to `%{}` rather than returning a
+      # scalar for the sidebar_section sub-tree — otherwise callers that
+      # pattern-match on a map in the layout crash.
+      session = %{
+        "backpex_preferences" => %{"global" => %{"sidebar_section" => "not-a-map"}}
+      }
+
+      socket = mount(session)
+      assert socket.assigns.sidebar_section_states == %{}
+    end
+  end
+
+  describe "on_mount/4 with a custom Preferences adapter" do
+    test "the adapter's value for global.theme wins over anything in the session" do
+      # Route every key through StubAdapter and seed a value for `global.theme`.
+      # Despite the session also holding a theme, the adapter result must win —
+      # this is the seam that lets a DB-backed adapter override session state.
+      Application.put_env(:backpex, Backpex.Preferences, adapters: [{:default, StubAdapter, []}])
+
+      StubAdapter.set(%{"global.theme" => "cupcake"})
+
+      session = %{"backpex_preferences" => %{"global" => %{"theme" => "dark"}}}
+      socket = mount(session)
+
+      assert socket.assigns.current_theme == "cupcake"
+    end
+
+    test "falls back to the :default option when the adapter reports `:not_found` for sidebar_open" do
+      # StubAdapter returns `:not_found` for unknown keys. Verify the caller's
+      # `default: true` option reaches the value.
+      Application.put_env(:backpex, Backpex.Preferences, adapters: [{:default, StubAdapter, []}])
+
+      StubAdapter.set(%{})
+
+      socket = mount(%{})
+      assert socket.assigns.sidebar_open == true
+    end
+  end
+
+  describe "on_mount/4 hooks the current URL into :handle_params" do
+    test "attaches a handle_params hook that stores the URL on :current_url" do
+      # The hook itself runs on each `handle_params`. We can't drive the real
+      # lifecycle without a mounted LiveView, but we can verify the attachment
+      # happened — future changes that drop the hook break this test.
+      socket = mount(%{})
+
+      hooks = socket.private.lifecycle.handle_params
+      assert Enum.any?(hooks, fn hook -> hook.id == :current_url end)
+    end
+  end
+end