refactor: add opts_with_actor helper and improve error formatting
Add opts_with_actor helper function to reduce duplication when building Ash options with actor and domain. Improve format_error documentation and ensure consistent error message formatting.
This commit is contained in:
parent
34afe798ec
commit
5ac9ab7ff9
4 changed files with 26 additions and 9 deletions
|
|
@ -15,7 +15,7 @@ defmodule MvWeb.RoleLive.Form do
|
||||||
|
|
||||||
alias Mv.Authorization.PermissionSets
|
alias Mv.Authorization.PermissionSets
|
||||||
|
|
||||||
import MvWeb.RoleLive.Helpers
|
import MvWeb.RoleLive.Helpers, only: [format_error: 1]
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,7 @@ defmodule MvWeb.RoleLive.Helpers do
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Formats an error for display to the user.
|
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()
|
@spec format_error(Ash.Error.Invalid.t() | String.t() | any()) :: String.t()
|
||||||
def format_error(%Ash.Error.Invalid{} = error) do
|
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("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("admin"), do: "badge badge-error badge-sm"
|
||||||
def permission_set_badge_class(_), do: "badge badge-ghost 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
|
end
|
||||||
|
|
|
||||||
|
|
@ -21,7 +21,8 @@ defmodule MvWeb.RoleLive.Index do
|
||||||
|
|
||||||
require Ash.Query
|
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}
|
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
||||||
|
|
||||||
|
|
@ -126,6 +127,7 @@ defmodule MvWeb.RoleLive.Index do
|
||||||
end
|
end
|
||||||
|
|
||||||
# Loads all user counts for roles in a single query to avoid N+1 queries
|
# 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) :: %{
|
@spec load_user_counts([Mv.Authorization.Role.t()], map() | nil) :: %{
|
||||||
Ecto.UUID.t() => non_neg_integer()
|
Ecto.UUID.t() => non_neg_integer()
|
||||||
}
|
}
|
||||||
|
|
@ -133,8 +135,7 @@ defmodule MvWeb.RoleLive.Index do
|
||||||
role_ids = Enum.map(roles, & &1.id)
|
role_ids = Enum.map(roles, & &1.id)
|
||||||
|
|
||||||
# Load all users with role_id in a single query
|
# Load all users with role_id in a single query
|
||||||
opts = [domain: Mv.Accounts]
|
opts = opts_with_actor([], actor, Mv.Accounts)
|
||||||
opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts
|
|
||||||
|
|
||||||
users =
|
users =
|
||||||
case Ash.read(
|
case Ash.read(
|
||||||
|
|
@ -164,8 +165,7 @@ defmodule MvWeb.RoleLive.Index do
|
||||||
# Recalculates user count for a specific role (used before deletion)
|
# Recalculates user count for a specific role (used before deletion)
|
||||||
@spec recalculate_user_count(Mv.Authorization.Role.t(), map() | nil) :: non_neg_integer()
|
@spec recalculate_user_count(Mv.Authorization.Role.t(), map() | nil) :: non_neg_integer()
|
||||||
defp recalculate_user_count(role, actor) do
|
defp recalculate_user_count(role, actor) do
|
||||||
opts = [domain: Mv.Accounts]
|
opts = opts_with_actor([], actor, Mv.Accounts)
|
||||||
opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts
|
|
||||||
|
|
||||||
case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do
|
case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do
|
||||||
{:ok, count} -> count
|
{:ok, count} -> count
|
||||||
|
|
|
||||||
|
|
@ -16,7 +16,8 @@ defmodule MvWeb.RoleLive.Show do
|
||||||
|
|
||||||
require Ash.Query
|
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}
|
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)
|
# Recalculates user count for a specific role (used before deletion)
|
||||||
@spec recalculate_user_count(Mv.Authorization.Role.t(), map() | nil) :: non_neg_integer()
|
@spec recalculate_user_count(Mv.Authorization.Role.t(), map() | nil) :: non_neg_integer()
|
||||||
defp recalculate_user_count(role, actor) do
|
defp recalculate_user_count(role, actor) do
|
||||||
opts = [domain: Mv.Accounts]
|
opts = opts_with_actor([], actor, Mv.Accounts)
|
||||||
opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts
|
|
||||||
|
|
||||||
case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do
|
case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do
|
||||||
{:ok, count} -> count
|
{:ok, count} -> count
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue