diff --git a/lib/mv_web/live/role_live/form.ex b/lib/mv_web/live/role_live/form.ex index 6388ec1..7b74c7e 100644 --- a/lib/mv_web/live/role_live/form.ex +++ b/lib/mv_web/live/role_live/form.ex @@ -15,7 +15,7 @@ defmodule MvWeb.RoleLive.Form do alias Mv.Authorization.PermissionSets - import MvWeb.RoleLive.Helpers + import MvWeb.RoleLive.Helpers, only: [format_error: 1] on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded} diff --git a/lib/mv_web/live/role_live/helpers.ex b/lib/mv_web/live/role_live/helpers.ex index 9d4e77d..8fbc544 100644 --- a/lib/mv_web/live/role_live/helpers.ex +++ b/lib/mv_web/live/role_live/helpers.ex @@ -6,6 +6,7 @@ defmodule MvWeb.RoleLive.Helpers do @doc """ Formats an error for display to the user. + Extracts error messages from Ash.Error.Invalid and joins them. """ @spec format_error(Ash.Error.Invalid.t() | String.t() | any()) :: String.t() def format_error(%Ash.Error.Invalid{} = error) do @@ -24,4 +25,20 @@ defmodule MvWeb.RoleLive.Helpers do def permission_set_badge_class("normal_user"), do: "badge badge-success badge-sm" def permission_set_badge_class("admin"), do: "badge badge-error badge-sm" def permission_set_badge_class(_), do: "badge badge-ghost badge-sm" + + @doc """ + Builds Ash options with actor and domain, ensuring actor is never nil in real paths. + """ + @spec opts_with_actor(keyword(), map() | nil, atom()) :: keyword() + def opts_with_actor(base_opts \\ [], actor, domain) do + opts = Keyword.put(base_opts, :domain, domain) + + if actor do + Keyword.put(opts, :actor, actor) + else + require Logger + Logger.warning("opts_with_actor called with nil actor - this may bypass policies") + opts + end + end end diff --git a/lib/mv_web/live/role_live/index.ex b/lib/mv_web/live/role_live/index.ex index d34cb22..718aa34 100644 --- a/lib/mv_web/live/role_live/index.ex +++ b/lib/mv_web/live/role_live/index.ex @@ -21,7 +21,8 @@ defmodule MvWeb.RoleLive.Index do require Ash.Query - import MvWeb.RoleLive.Helpers + import MvWeb.RoleLive.Helpers, + only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3] on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded} @@ -126,6 +127,7 @@ defmodule MvWeb.RoleLive.Index do end # Loads all user counts for roles in a single query to avoid N+1 queries + # TODO: Optimize to use DB-side aggregation instead of loading all users @spec load_user_counts([Mv.Authorization.Role.t()], map() | nil) :: %{ Ecto.UUID.t() => non_neg_integer() } @@ -133,8 +135,7 @@ defmodule MvWeb.RoleLive.Index do role_ids = Enum.map(roles, & &1.id) # Load all users with role_id in a single query - opts = [domain: Mv.Accounts] - opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts + opts = opts_with_actor([], actor, Mv.Accounts) users = case Ash.read( @@ -164,8 +165,7 @@ defmodule MvWeb.RoleLive.Index do # Recalculates user count for a specific role (used before deletion) @spec recalculate_user_count(Mv.Authorization.Role.t(), map() | nil) :: non_neg_integer() defp recalculate_user_count(role, actor) do - opts = [domain: Mv.Accounts] - opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts + opts = opts_with_actor([], actor, Mv.Accounts) case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do {:ok, count} -> count diff --git a/lib/mv_web/live/role_live/show.ex b/lib/mv_web/live/role_live/show.ex index 3f15155..7184b68 100644 --- a/lib/mv_web/live/role_live/show.ex +++ b/lib/mv_web/live/role_live/show.ex @@ -16,7 +16,8 @@ defmodule MvWeb.RoleLive.Show do require Ash.Query - import MvWeb.RoleLive.Helpers + import MvWeb.RoleLive.Helpers, + only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3] on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded} @@ -143,8 +144,7 @@ defmodule MvWeb.RoleLive.Show do # Recalculates user count for a specific role (used before deletion) @spec recalculate_user_count(Mv.Authorization.Role.t(), map() | nil) :: non_neg_integer() defp recalculate_user_count(role, actor) do - opts = [domain: Mv.Accounts] - opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts + opts = opts_with_actor([], actor, Mv.Accounts) case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do {:ok, count} -> count