diff --git a/lib/mv_web/authorization.ex b/lib/mv_web/authorization.ex
index 18ecd70..95a8524 100644
--- a/lib/mv_web/authorization.ex
+++ b/lib/mv_web/authorization.ex
@@ -106,14 +106,18 @@ defmodule MvWeb.Authorization do
iex> can_access_page?(mitglied, "/members")
false
"""
- @spec can_access_page?(map() | nil, String.t()) :: boolean()
+ @spec can_access_page?(map() | nil, String.t() | Phoenix.VerifiedRoutes.unverified_path()) ::
+ boolean()
def can_access_page?(nil, _page_path), do: false
def can_access_page?(user, page_path) do
+ # Convert verified route to string if needed
+ page_path_str = if is_binary(page_path), do: page_path, else: to_string(page_path)
+
with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- user,
{:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name),
permissions <- PermissionSets.get_permissions(ps_atom) do
- page_matches?(permissions.pages, page_path)
+ page_matches?(permissions.pages, page_path_str)
else
_ -> false
end
diff --git a/lib/mv_web/components/layouts/navbar.ex b/lib/mv_web/components/layouts/navbar.ex
index 692f949..e3e9319 100644
--- a/lib/mv_web/components/layouts/navbar.ex
+++ b/lib/mv_web/components/layouts/navbar.ex
@@ -34,9 +34,9 @@ defmodule MvWeb.Layouts.Navbar do
<.link navigate="/settings">{gettext("Global Settings")}
- <%= if can_access_page?(@current_user, "/admin/roles") do %>
+ <%= if can_access_page?(@current_user, ~p"/admin/roles") do %>
- <.link navigate="/admin/roles">{gettext("Roles")}
+ <.link navigate={~p"/admin/roles"}>{gettext("Roles")}
<% end %>
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 0099929..9d75da6 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}
@@ -125,35 +126,33 @@ defmodule MvWeb.RoleLive.Index do
end
end
- # Loads all user counts for roles in a single query to avoid N+1 queries
- @spec load_user_counts([Mv.Authorization.Role.t()], map() | nil) :: %{Ecto.UUID.t() => non_neg_integer()}
- defp load_user_counts(roles, actor) do
+ # Loads all user counts for roles using DB-side aggregation for better performance
+ @spec load_user_counts([Mv.Authorization.Role.t()], map() | nil) :: %{
+ Ecto.UUID.t() => non_neg_integer()
+ }
+ defp load_user_counts(roles, _actor) 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
+ # Use Ecto directly for efficient GROUP BY COUNT query
+ # This is much more performant than loading all users and counting in Elixir
+ # Note: We bypass Ash here for performance, but this is a simple read-only query
+ import Ecto.Query
- users =
- case Ash.read(
- Accounts.User
- |> Ash.Query.filter(role_id in ^role_ids)
- |> Ash.Query.select([:role_id]),
- opts
- ) do
- {:ok, users_list} -> users_list
- {:error, _} -> []
- end
+ query =
+ from u in Accounts.User,
+ where: u.role_id in ^role_ids,
+ group_by: u.role_id,
+ select: {u.role_id, count(u.id)}
- # Group by role_id and count
- users
- |> Enum.group_by(& &1.role_id)
- |> Enum.map(fn {role_id, users_list} -> {role_id, length(users_list)} end)
- |> Map.new()
+ results = Mv.Repo.all(query)
+
+ results
+ |> Enum.into(%{}, fn {role_id, count} -> {role_id, count} end)
end
# Gets user count from preloaded assigns map
- @spec get_user_count(Mv.Authorization.Role.t(), %{Ecto.UUID.t() => non_neg_integer()}) :: non_neg_integer()
+ @spec get_user_count(Mv.Authorization.Role.t(), %{Ecto.UUID.t() => non_neg_integer()}) ::
+ non_neg_integer()
defp get_user_count(role, user_counts) do
Map.get(user_counts, role.id, 0)
end
@@ -161,13 +160,11 @@ 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
_ -> 0
end
end
-
end
diff --git a/lib/mv_web/live/role_live/show.ex b/lib/mv_web/live/role_live/show.ex
index 8400728..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
@@ -213,5 +213,4 @@ defmodule MvWeb.RoleLive.Show do
"""
end
-
end
diff --git a/lib/mv_web/live_helpers.ex b/lib/mv_web/live_helpers.ex
index 1835cba..f217ee2 100644
--- a/lib/mv_web/live_helpers.ex
+++ b/lib/mv_web/live_helpers.ex
@@ -49,8 +49,14 @@ defmodule MvWeb.LiveHelpers do
opts = [domain: Mv.Accounts, actor: user]
case Ash.load(user, :role, opts) do
- {:ok, loaded_user} -> loaded_user
- {:error, _} -> user
+ {:ok, loaded_user} ->
+ loaded_user
+
+ {:error, error} ->
+ # Log warning if role loading fails - this can cause authorization issues
+ require Logger
+ Logger.warning("Failed to load role for user #{user.id}: #{inspect(error)}")
+ user
end
end
end
diff --git a/lib/mv_web/router.ex b/lib/mv_web/router.ex
index e73c926..682b672 100644
--- a/lib/mv_web/router.ex
+++ b/lib/mv_web/router.ex
@@ -46,7 +46,10 @@ defmodule MvWeb.Router do
AshAuthentication-specific: We define that all routes can only be accessed when the user is signed in.
"""
ash_authentication_live_session :authentication_required,
- on_mount: {MvWeb.LiveUserAuth, :live_user_required} do
+ on_mount: [
+ {MvWeb.LiveUserAuth, :live_user_required},
+ {MvWeb.LiveHelpers, :ensure_user_role_loaded}
+ ] do
live "/", MemberLive.Index, :index
live "/members", MemberLive.Index, :index
diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po
index 5a9c32f..229b180 100644
--- a/priv/gettext/de/LC_MESSAGES/default.po
+++ b/priv/gettext/de/LC_MESSAGES/default.po
@@ -1449,9 +1449,7 @@ msgstr "Bereits bezahlte Zyklen bleiben mit dem alten Betrag."
#: lib/mv_web/live/member_live/show/membership_fees_component.ex
#: lib/mv_web/live/membership_fee_type_live/index.ex
-#: lib/mv_web/live/role_live/form.ex
-#: lib/mv_web/live/role_live/index.ex
-#: lib/mv_web/live/role_live/show.ex
+#: lib/mv_web/live/role_live/helpers.ex
#, elixir-autogen, elixir-format
msgid "An error occurred"
msgstr "Ein Fehler ist aufgetreten"
@@ -1862,7 +1860,7 @@ msgstr "System-Rolle kann nicht gelöscht werden"
#: lib/mv_web/live/role_live/index.html.heex
#, elixir-autogen, elixir-format, fuzzy
msgid "Custom"
-msgstr "Benutzerdefinierte Felder"
+msgstr "Benutzerdefiniert"
#: lib/mv_web/live/role_live/show.ex
#, elixir-autogen, elixir-format, fuzzy
@@ -1879,7 +1877,7 @@ msgstr "Rolle konnte nicht gelöscht werden: %{error}"
#: lib/mv_web/live/role_live/index.html.heex
#, elixir-autogen, elixir-format, fuzzy
msgid "Listing Roles"
-msgstr "Benutzer*innen auflisten"
+msgstr "Rollen auflisten"
#: lib/mv_web/live/role_live/index.html.heex
#, elixir-autogen, elixir-format
diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot
index cdf1895..d720511 100644
--- a/priv/gettext/default.pot
+++ b/priv/gettext/default.pot
@@ -1450,9 +1450,7 @@ msgstr ""
#: lib/mv_web/live/member_live/show/membership_fees_component.ex
#: lib/mv_web/live/membership_fee_type_live/index.ex
-#: lib/mv_web/live/role_live/form.ex
-#: lib/mv_web/live/role_live/index.ex
-#: lib/mv_web/live/role_live/show.ex
+#: lib/mv_web/live/role_live/helpers.ex
#, elixir-autogen, elixir-format
msgid "An error occurred"
msgstr ""
diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po
index 724c991..bb75e6b 100644
--- a/priv/gettext/en/LC_MESSAGES/default.po
+++ b/priv/gettext/en/LC_MESSAGES/default.po
@@ -1450,9 +1450,7 @@ msgstr ""
#: lib/mv_web/live/member_live/show/membership_fees_component.ex
#: lib/mv_web/live/membership_fee_type_live/index.ex
-#: lib/mv_web/live/role_live/form.ex
-#: lib/mv_web/live/role_live/index.ex
-#: lib/mv_web/live/role_live/show.ex
+#: lib/mv_web/live/role_live/helpers.ex
#, elixir-autogen, elixir-format
msgid "An error occurred"
msgstr ""