Merge remote-tracking branch 'origin/main' into sidebar

test/mv_web/authorization_test.exs

@@ -0,0 +1,219 @@
+defmodule MvWeb.AuthorizationTest do
+  @moduledoc """
+  Tests for UI-level authorization helpers.
+  """
+  use ExUnit.Case, async: true
+
+  alias MvWeb.Authorization
+  alias Mv.Membership.Member
+  alias Mv.Accounts.User
+
+  describe "can?/3 with resource atom" do
+    test "returns true when user has permission for resource+action" do
+      admin = %{
+        id: "admin-123",
+        role: %{permission_set_name: "admin"}
+      }
+
+      assert Authorization.can?(admin, :create, Mv.Membership.Member) == true
+      assert Authorization.can?(admin, :read, Mv.Membership.Member) == true
+      assert Authorization.can?(admin, :update, Mv.Membership.Member) == true
+      assert Authorization.can?(admin, :destroy, Mv.Membership.Member) == true
+    end
+
+    test "returns false when user lacks permission" do
+      read_only_user = %{
+        id: "read-only-123",
+        role: %{permission_set_name: "read_only"}
+      }
+
+      assert Authorization.can?(read_only_user, :create, Mv.Membership.Member) == false
+      assert Authorization.can?(read_only_user, :read, Mv.Membership.Member) == true
+      assert Authorization.can?(read_only_user, :update, Mv.Membership.Member) == false
+      assert Authorization.can?(read_only_user, :destroy, Mv.Membership.Member) == false
+    end
+
+    test "returns false for nil user" do
+      assert Authorization.can?(nil, :create, Mv.Membership.Member) == false
+      assert Authorization.can?(nil, :read, Mv.Membership.Member) == false
+    end
+
+    test "admin can manage roles" do
+      admin = %{
+        id: "admin-123",
+        role: %{permission_set_name: "admin"}
+      }
+
+      assert Authorization.can?(admin, :create, Mv.Authorization.Role) == true
+      assert Authorization.can?(admin, :read, Mv.Authorization.Role) == true
+      assert Authorization.can?(admin, :update, Mv.Authorization.Role) == true
+      assert Authorization.can?(admin, :destroy, Mv.Authorization.Role) == true
+    end
+
+    test "non-admin cannot manage roles" do
+      normal_user = %{
+        id: "normal-123",
+        role: %{permission_set_name: "normal_user"}
+      }
+
+      assert Authorization.can?(normal_user, :create, Mv.Authorization.Role) == false
+      assert Authorization.can?(normal_user, :read, Mv.Authorization.Role) == false
+      assert Authorization.can?(normal_user, :update, Mv.Authorization.Role) == false
+      assert Authorization.can?(normal_user, :destroy, Mv.Authorization.Role) == false
+    end
+  end
+
+  describe "can?/3 with record struct - scope :all" do
+    test "admin can update any member" do
+      admin = %{
+        id: "admin-123",
+        role: %{permission_set_name: "admin"}
+      }
+
+      member1 = %Member{id: "member-1", user: %User{id: "other-user"}}
+      member2 = %Member{id: "member-2", user: %User{id: "another-user"}}
+
+      assert Authorization.can?(admin, :update, member1) == true
+      assert Authorization.can?(admin, :update, member2) == true
+    end
+
+    test "normal_user can update any member" do
+      normal_user = %{
+        id: "normal-123",
+        role: %{permission_set_name: "normal_user"}
+      }
+
+      member = %Member{id: "member-1", user: %User{id: "other-user"}}
+
+      assert Authorization.can?(normal_user, :update, member) == true
+    end
+  end
+
+  describe "can?/3 with record struct - scope :own" do
+    test "user can update own User record" do
+      user = %{
+        id: "user-123",
+        role: %{permission_set_name: "own_data"}
+      }
+
+      own_user_record = %User{id: "user-123"}
+      other_user_record = %User{id: "other-user"}
+
+      assert Authorization.can?(user, :update, own_user_record) == true
+      assert Authorization.can?(user, :update, other_user_record) == false
+    end
+  end
+
+  describe "can?/3 with record struct - scope :linked" do
+    test "user can update linked member" do
+      user = %{
+        id: "user-123",
+        role: %{permission_set_name: "own_data"}
+      }
+
+      # Member has_one :user (inverse relationship)
+      linked_member = %Member{id: "member-1", user: %User{id: "user-123"}}
+      unlinked_member = %Member{id: "member-2", user: nil}
+      unlinked_member_other = %Member{id: "member-3", user: %User{id: "other-user"}}
+
+      assert Authorization.can?(user, :update, linked_member) == true
+      assert Authorization.can?(user, :update, unlinked_member) == false
+      assert Authorization.can?(user, :update, unlinked_member_other) == false
+    end
+
+    test "user can update CustomFieldValue of linked member" do
+      user = %{
+        id: "user-123",
+        role: %{permission_set_name: "own_data"}
+      }
+
+      linked_cfv = %Mv.Membership.CustomFieldValue{
+        id: "cfv-1",
+        member: %Member{id: "member-1", user: %User{id: "user-123"}}
+      }
+
+      unlinked_cfv = %Mv.Membership.CustomFieldValue{
+        id: "cfv-2",
+        member: %Member{id: "member-2", user: nil}
+      }
+
+      unlinked_cfv_other = %Mv.Membership.CustomFieldValue{
+        id: "cfv-3",
+        member: %Member{id: "member-3", user: %User{id: "other-user"}}
+      }
+
+      assert Authorization.can?(user, :update, linked_cfv) == true
+      assert Authorization.can?(user, :update, unlinked_cfv) == false
+      assert Authorization.can?(user, :update, unlinked_cfv_other) == false
+    end
+  end
+
+  describe "can_access_page?/2" do
+    test "admin can access all pages via wildcard" do
+      admin = %{
+        id: "admin-123",
+        role: %{permission_set_name: "admin"}
+      }
+
+      assert Authorization.can_access_page?(admin, "/admin/roles") == true
+      assert Authorization.can_access_page?(admin, "/members") == true
+      assert Authorization.can_access_page?(admin, "/any/page") == true
+    end
+
+    test "read_only user can access allowed pages" do
+      read_only_user = %{
+        id: "read-only-123",
+        role: %{permission_set_name: "read_only"}
+      }
+
+      assert Authorization.can_access_page?(read_only_user, "/") == true
+      assert Authorization.can_access_page?(read_only_user, "/members") == true
+      assert Authorization.can_access_page?(read_only_user, "/members/123") == true
+      assert Authorization.can_access_page?(read_only_user, "/admin/roles") == false
+    end
+
+    test "matches dynamic routes correctly" do
+      read_only_user = %{
+        id: "read-only-123",
+        role: %{permission_set_name: "read_only"}
+      }
+
+      assert Authorization.can_access_page?(read_only_user, "/members/123") == true
+      assert Authorization.can_access_page?(read_only_user, "/members/abc") == true
+      assert Authorization.can_access_page?(read_only_user, "/members/123/edit") == false
+    end
+
+    test "returns false for nil user" do
+      assert Authorization.can_access_page?(nil, "/members") == false
+      assert Authorization.can_access_page?(nil, "/admin/roles") == false
+    end
+  end
+
+  describe "error handling" do
+    test "user without role returns false" do
+      user_without_role = %{id: "user-123", role: nil}
+
+      assert Authorization.can?(user_without_role, :create, Mv.Membership.Member) == false
+      assert Authorization.can_access_page?(user_without_role, "/members") == false
+    end
+
+    test "user with invalid permission_set_name returns false" do
+      user_with_invalid_permission = %{
+        id: "user-123",
+        role: %{permission_set_name: "invalid_set"}
+      }
+
+      assert Authorization.can?(user_with_invalid_permission, :create, Mv.Membership.Member) ==
+               false
+
+      assert Authorization.can_access_page?(user_with_invalid_permission, "/members") == false
+    end
+
+    test "handles missing fields gracefully" do
+      user_missing_role = %{id: "user-123"}
+
+      assert Authorization.can?(user_missing_role, :create, Mv.Membership.Member) == false
+      assert Authorization.can_access_page?(user_missing_role, "/members") == false
+    end
+  end
+end

test/mv_web/components/sort_header_component_test.exs

@@ -24,7 +24,6 @@ defmodule MvWeb.Components.SortHeaderComponentTest do
         :house_number,
         :postal_code,
         :city,
-        :phone_number,
         :join_date
       ]
 
@@ -101,7 +100,6 @@ defmodule MvWeb.Components.SortHeaderComponentTest do
       assert has_element?(view, "[data-testid='street'] .opacity-40")
       assert has_element?(view, "[data-testid='house_number'] .opacity-40")
       assert has_element?(view, "[data-testid='postal_code'] .opacity-40")
-      assert has_element?(view, "[data-testid='phone_number'] .opacity-40")
       assert has_element?(view, "[data-testid='join_date'] .opacity-40")
     end
 

test/mv_web/helpers/member_helpers_test.exs

@@ -0,0 +1,141 @@
+defmodule MvWeb.Helpers.MemberHelpersTest do
+  @moduledoc """
+  Tests for the display_name/1 helper function in MemberHelpers.
+  """
+  use Mv.DataCase, async: true
+
+  alias Mv.Membership.Member
+  alias MvWeb.Helpers.MemberHelpers
+
+  describe "display_name/1" do
+    test "returns full name when both first_name and last_name are present" do
+      member = %Member{
+        first_name: "John",
+        last_name: "Doe",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John Doe"
+    end
+
+    test "returns email when both first_name and last_name are nil" do
+      member = %Member{
+        first_name: nil,
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "returns first_name only when last_name is nil" do
+      member = %Member{
+        first_name: "John",
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John"
+    end
+
+    test "returns last_name only when first_name is nil" do
+      member = %Member{
+        first_name: nil,
+        last_name: "Doe",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "Doe"
+    end
+
+    test "returns email when first_name and last_name are empty strings" do
+      member = %Member{
+        first_name: "",
+        last_name: "",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "returns email when first_name and last_name are whitespace only" do
+      member = %Member{
+        first_name: "   ",
+        last_name: "  \t  ",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "trims whitespace from name parts" do
+      member = %Member{
+        first_name: "  John  ",
+        last_name: "  Doe  ",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John Doe"
+    end
+
+    test "handles one empty string and one nil" do
+      member = %Member{
+        first_name: "",
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "handles one nil and one empty string" do
+      member = %Member{
+        first_name: nil,
+        last_name: "",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "handles one whitespace and one nil" do
+      member = %Member{
+        first_name: "   ",
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "handles one valid name and one whitespace" do
+      member = %Member{
+        first_name: "John",
+        last_name: "   ",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John"
+    end
+
+    test "handles member with only first_name containing whitespace" do
+      member = %Member{
+        first_name: "  John  ",
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John"
+    end
+
+    test "handles member with only last_name containing whitespace" do
+      member = %Member{
+        first_name: nil,
+        last_name: "  Doe  ",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "Doe"
+    end
+  end
+end

test/mv_web/live/custom_field_live/deletion_test.exs

@@ -154,7 +154,7 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do
       |> render_click()
 
       # Should show success message
-      assert render(view) =~ "Custom field deleted successfully"
+      assert render(view) =~ "Data field deleted successfully"
 
       # Custom field should be gone from database
       assert {:error, _} = Ash.get(CustomField, custom_field.id)

test/mv_web/live/global_settings_live_test.exs

@@ -64,5 +64,21 @@ defmodule MvWeb.GlobalSettingsLiveTest do
 
       assert html =~ "must be present"
     end
+
+    test "displays Memberdata section", %{conn: conn} do
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      assert html =~ "Memberdata" or html =~ "Member Data"
+    end
+
+    test "displays flash message after member field visibility update", %{conn: conn} do
+      {:ok, view, _html} = live(conn, ~p"/settings")
+
+      # Simulate member field visibility update
+      send(view.pid, {:member_field_visibility_updated})
+
+      # Check for flash message
+      assert render(view) =~ "updated" or render(view) =~ "success"
+    end
   end
 end

test/mv_web/live/member_field_live/index_component_test.exs

@@ -0,0 +1,124 @@
+defmodule MvWeb.MemberFieldLive.IndexComponentTest do
+  @moduledoc """
+  Tests for MemberFieldLive.IndexComponent.
+
+  Tests cover:
+  - Rendering all member fields from Mv.Constants.member_fields()
+  - Displaying show_in_overview status as badge (Yes/No)
+  - Displaying required status for required fields (first_name, last_name, email)
+  - Current status is displayed based on settings.member_field_visibility
+  - Default status is "Yes" (visible) when not configured in settings
+  """
+  use MvWeb.ConnCase, async: false
+
+  import Phoenix.LiveViewTest
+
+  alias Mv.Membership
+
+  setup %{conn: conn} do
+    user = create_test_user(%{email: "admin@example.com"})
+    conn = conn_with_oidc_user(conn, user)
+    {:ok, conn: conn, user: user}
+  end
+
+  describe "rendering" do
+    test "renders all member fields from Constants", %{conn: conn} do
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      # Check that all member fields are displayed
+      member_fields = Mv.Constants.member_fields()
+
+      for field <- member_fields do
+        field_name = String.replace(Atom.to_string(field), "_", " ") |> String.capitalize()
+        # Field name should appear in the table (either as label or in some form)
+        assert html =~ field_name or html =~ Atom.to_string(field)
+      end
+    end
+
+    test "displays show_in_overview status as badge", %{conn: conn} do
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      # Should have "Show in overview" column header
+      assert html =~ "Show in overview" or html =~ "Show in Overview"
+
+      # Should have badge elements (Yes/No)
+      assert html =~ "badge" or html =~ "Yes" or html =~ "No"
+    end
+
+    test "displays required status for required fields", %{conn: conn} do
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      # Required fields: first_name, last_name, email
+      # Should have "Required" column or indicator
+      assert html =~ "Required" or html =~ "required"
+    end
+
+    test "shows default status as Yes when not configured", %{conn: conn} do
+      # Ensure settings have no member_field_visibility configured
+      {:ok, settings} = Membership.get_settings()
+
+      {:ok, _updated} =
+        Membership.update_settings(settings, %{member_field_visibility: %{}})
+
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      # All fields should show as visible (Yes) by default
+      # Check for "Yes" badge or similar indicator
+      assert html =~ "Yes" or html =~ "badge-success"
+    end
+
+    test "shows configured visibility status from settings", %{conn: conn} do
+      # Configure some fields as hidden
+      {:ok, settings} = Membership.get_settings()
+      visibility_config = %{"street" => false, "house_number" => false}
+
+      {:ok, _updated} =
+        Membership.update_member_field_visibility(settings, visibility_config)
+
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      # Street and house_number should show as hidden (No)
+      # Other fields should show as visible (Yes)
+      assert html =~ "street" or html =~ "Street"
+      assert html =~ "house_number" or html =~ "House number"
+    end
+  end
+
+  describe "required fields" do
+    test "marks first_name as required", %{conn: conn} do
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      # first_name should be marked as required
+      assert html =~ "first_name" or html =~ "First name"
+      # Should have required indicator
+      assert html =~ "required" or html =~ "Required"
+    end
+
+    test "marks last_name as required", %{conn: conn} do
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      # last_name should be marked as required
+      assert html =~ "last_name" or html =~ "Last name"
+      # Should have required indicator
+      assert html =~ "required" or html =~ "Required"
+    end
+
+    test "marks email as required", %{conn: conn} do
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      # email should be marked as required
+      assert html =~ "email" or html =~ "Email"
+      # Should have required indicator
+      assert html =~ "required" or html =~ "Required"
+    end
+
+    test "does not mark optional fields as required", %{conn: conn} do
+      {:ok, _view, html} = live(conn, ~p"/settings")
+
+      # Optional fields should not have required indicator
+      # Check that street (optional) doesn't have required badge
+      # This test verifies that only required fields show the indicator
+      assert html =~ "street" or html =~ "Street"
+    end
+  end
+end

test/mv_web/live/role_live_test.exs

@@ -0,0 +1,452 @@
+defmodule MvWeb.RoleLiveTest do
+  @moduledoc """
+  Tests for role management LiveViews.
+  """
+  use MvWeb.ConnCase, async: false
+
+  import Phoenix.LiveViewTest
+
+  alias Mv.Authorization
+  alias Mv.Authorization.Role
+
+  # Helper to create a role
+  defp create_role(attrs \\ %{}) do
+    default_attrs = %{
+      name: "Test Role #{System.unique_integer([:positive])}",
+      description: "Test description",
+      permission_set_name: "read_only"
+    }
+
+    attrs = Map.merge(default_attrs, attrs)
+
+    case Authorization.create_role(attrs) do
+      {:ok, role} -> role
+      {:error, error} -> raise "Failed to create role: #{inspect(error)}"
+    end
+  end
+
+  # Helper to create admin user with admin role
+  defp create_admin_user(conn) do
+    # Create admin role
+    admin_role =
+      case Authorization.list_roles() do
+        {:ok, roles} ->
+          case Enum.find(roles, &(&1.name == "Admin")) do
+            nil ->
+              # Create admin role if it doesn't exist
+              create_role(%{
+                name: "Admin",
+                description: "Administrator with full access",
+                permission_set_name: "admin"
+              })
+
+            role ->
+              role
+          end
+
+        _ ->
+          # Create admin role if list_roles fails
+          create_role(%{
+            name: "Admin",
+            description: "Administrator with full access",
+            permission_set_name: "admin"
+          })
+      end
+
+    # Create user
+    {:ok, user} =
+      Mv.Accounts.User
+      |> Ash.Changeset.for_create(:register_with_password, %{
+        email: "admin#{System.unique_integer([:positive])}@mv.local",
+        password: "testpassword123"
+      })
+      |> Ash.create()
+
+    # Assign admin role using manage_relationship
+    {:ok, user} =
+      user
+      |> Ash.Changeset.for_update(:update, %{})
+      |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
+      |> Ash.update()
+
+    # Load role for authorization checks (must be loaded for can?/3 to work)
+    user_with_role = Ash.load!(user, :role, domain: Mv.Accounts)
+
+    # Store user with role in session for LiveView
+    conn = conn_with_password_user(conn, user_with_role)
+    {conn, user_with_role, admin_role}
+  end
+
+  # Helper to create non-admin user
+  defp create_non_admin_user(conn) do
+    {:ok, user} =
+      Mv.Accounts.User
+      |> Ash.Changeset.for_create(:register_with_password, %{
+        email: "user#{System.unique_integer([:positive])}@mv.local",
+        password: "testpassword123"
+      })
+      |> Ash.create()
+
+    conn = conn_with_password_user(conn, user)
+    {conn, user}
+  end
+
+  describe "index page" do
+    setup %{conn: conn} do
+      {conn, user, _admin_role} = create_admin_user(conn)
+      %{conn: conn, user: user}
+    end
+
+    test "mounts successfully", %{conn: conn} do
+      {:ok, _view, _html} = live(conn, "/admin/roles")
+    end
+
+    test "loads all roles from database", %{conn: conn} do
+      role1 = create_role(%{name: "Role 1"})
+      role2 = create_role(%{name: "Role 2"})
+
+      {:ok, _view, html} = live(conn, "/admin/roles")
+
+      assert html =~ role1.name
+      assert html =~ role2.name
+    end
+
+    test "shows table with role names", %{conn: conn} do
+      role = create_role(%{name: "Test Role"})
+
+      {:ok, _view, html} = live(conn, "/admin/roles")
+
+      assert html =~ role.name
+      assert html =~ role.description
+      assert html =~ role.permission_set_name
+    end
+
+    test "shows system role badge", %{conn: conn} do
+      _system_role =
+        Role
+        |> Ash.Changeset.for_create(:create_role, %{
+          name: "System Role",
+          permission_set_name: "own_data"
+        })
+        |> Ash.Changeset.force_change_attribute(:is_system_role, true)
+        |> Ash.create!()
+
+      {:ok, _view, html} = live(conn, "/admin/roles")
+
+      assert html =~ "System Role" || html =~ "system"
+    end
+
+    test "delete button disabled for system roles", %{conn: conn} do
+      system_role =
+        Role
+        |> Ash.Changeset.for_create(:create_role, %{
+          name: "System Role",
+          permission_set_name: "own_data"
+        })
+        |> Ash.Changeset.force_change_attribute(:is_system_role, true)
+        |> Ash.create!()
+
+      {:ok, view, _html} = live(conn, "/admin/roles")
+
+      assert has_element?(
+               view,
+               "button[phx-click='delete'][phx-value-id='#{system_role.id}'][disabled]"
+             ) ||
+               not has_element?(
+                 view,
+                 "button[phx-click='delete'][phx-value-id='#{system_role.id}']"
+               )
+    end
+
+    test "delete button enabled for non-system roles", %{conn: conn} do
+      role = create_role()
+
+      {:ok, view, html} = live(conn, "/admin/roles")
+
+      # Delete is a link with phx-click containing delete event
+      # Check if delete link exists in HTML (phx-click contains delete and role id)
+      assert (html =~ "phx-click" && html =~ "delete" && html =~ role.id) ||
+               has_element?(view, "a[phx-click*='delete'][phx-value-id='#{role.id}']") ||
+               has_element?(view, "a[aria-label='Delete role']")
+    end
+
+    test "new role button navigates to form", %{conn: conn} do
+      {:ok, view, html} = live(conn, "/admin/roles")
+
+      # Check if button exists (admin should see it)
+      if html =~ "New Role" do
+        {:error, {:live_redirect, %{to: to}}} =
+          view
+          |> element("a[href='/admin/roles/new'], button[href='/admin/roles/new']")
+          |> render_click()
+
+        assert to == "/admin/roles/new"
+      else
+        # If button not visible, user doesn't have permission (expected for non-admin)
+        # This test assumes admin user, so button should be visible
+        flunk("New Role button not found - user may not have admin role loaded")
+      end
+    end
+  end
+
+  describe "show page" do
+    setup %{conn: conn} do
+      {conn, user, _admin_role} = create_admin_user(conn)
+      %{conn: conn, user: user}
+    end
+
+    test "mounts with valid role ID", %{conn: conn} do
+      role = create_role()
+
+      {:ok, _view, html} = live(conn, "/admin/roles/#{role.id}")
+
+      assert html =~ role.name
+      assert html =~ role.description
+      assert html =~ role.permission_set_name
+    end
+
+    test "returns 404 for invalid role ID", %{conn: conn} do
+      invalid_id = Ecto.UUID.generate()
+
+      # Should redirect to index with error message
+      # redirect in mount returns {:error, {:redirect, ...}}
+      result = live(conn, "/admin/roles/#{invalid_id}")
+
+      assert match?({:error, {:redirect, %{to: "/admin/roles"}}}, result)
+    end
+
+    test "shows system role badge if is_system_role is true", %{conn: conn} do
+      system_role =
+        Role
+        |> Ash.Changeset.for_create(:create_role, %{
+          name: "System Role",
+          permission_set_name: "own_data"
+        })
+        |> Ash.Changeset.force_change_attribute(:is_system_role, true)
+        |> Ash.create!()
+
+      {:ok, _view, html} = live(conn, "/admin/roles/#{system_role.id}")
+
+      assert html =~ "System Role" || html =~ "system"
+    end
+  end
+
+  describe "form - create" do
+    setup %{conn: conn} do
+      {conn, user, _admin_role} = create_admin_user(conn)
+      %{conn: conn, user: user}
+    end
+
+    test "mounts successfully", %{conn: conn} do
+      {:ok, _view, _html} = live(conn, "/admin/roles/new")
+    end
+
+    test "form dropdown shows all 4 permission sets", %{conn: conn} do
+      {:ok, _view, html} = live(conn, "/admin/roles/new")
+
+      assert html =~ "own_data"
+      assert html =~ "read_only"
+      assert html =~ "normal_user"
+      assert html =~ "admin"
+    end
+
+    test "creates new role with valid data", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/admin/roles/new")
+
+      attrs = %{
+        "name" => "New Role",
+        "description" => "New description",
+        "permission_set_name" => "read_only"
+      }
+
+      view
+      |> form("#role-form", role: attrs)
+      |> render_submit()
+
+      # Should redirect to index or show page
+      assert_redirect(view, "/admin/roles")
+    end
+
+    test "shows error with invalid permission_set_name", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/admin/roles/new")
+
+      # Try to submit with empty permission_set_name (invalid)
+      attrs = %{
+        "name" => "New Role",
+        "description" => "New description",
+        "permission_set_name" => ""
+      }
+
+      view
+      |> form("#role-form", role: attrs)
+      |> render_submit()
+
+      # Should show validation error
+      html = render(view)
+      assert html =~ "error" || html =~ "required" || html =~ "Permission Set"
+    end
+
+    test "shows flash message after successful creation", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/admin/roles/new")
+
+      attrs = %{
+        "name" => "New Role #{System.unique_integer([:positive])}",
+        "description" => "New description",
+        "permission_set_name" => "read_only"
+      }
+
+      view
+      |> form("#role-form", role: attrs)
+      |> render_submit()
+
+      # Should redirect to index
+      assert_redirect(view, "/admin/roles")
+    end
+  end
+
+  describe "form - edit" do
+    setup %{conn: conn} do
+      {conn, user, _admin_role} = create_admin_user(conn)
+      role = create_role()
+      %{conn: conn, user: user, role: role}
+    end
+
+    test "mounts with valid role ID", %{conn: conn, role: role} do
+      {:ok, _view, html} = live(conn, "/admin/roles/#{role.id}/edit")
+
+      assert html =~ role.name
+    end
+
+    test "returns 404 for invalid role ID in edit", %{conn: conn} do
+      invalid_id = Ecto.UUID.generate()
+
+      # Should redirect to index with error message
+      # redirect in mount returns {:error, {:redirect, ...}}
+      result = live(conn, "/admin/roles/#{invalid_id}/edit")
+
+      assert match?({:error, {:redirect, %{to: "/admin/roles"}}}, result)
+    end
+
+    test "updates role name", %{conn: conn, role: role} do
+      {:ok, view, _html} = live(conn, "/admin/roles/#{role.id}/edit?return_to=show")
+
+      attrs = %{
+        "name" => "Updated Role Name",
+        "description" => role.description,
+        "permission_set_name" => role.permission_set_name
+      }
+
+      view
+      |> form("#role-form", role: attrs)
+      |> render_submit()
+
+      assert_redirect(view, "/admin/roles/#{role.id}")
+
+      # Verify update
+      {:ok, updated_role} = Authorization.get_role(role.id)
+      assert updated_role.name == "Updated Role Name"
+    end
+
+    test "updates system role's permission_set_name", %{conn: conn} do
+      system_role =
+        Role
+        |> Ash.Changeset.for_create(:create_role, %{
+          name: "System Role",
+          permission_set_name: "own_data"
+        })
+        |> Ash.Changeset.force_change_attribute(:is_system_role, true)
+        |> Ash.create!()
+
+      {:ok, view, _html} = live(conn, "/admin/roles/#{system_role.id}/edit?return_to=show")
+
+      attrs = %{
+        "name" => system_role.name,
+        "description" => system_role.description,
+        "permission_set_name" => "read_only"
+      }
+
+      view
+      |> form("#role-form", role: attrs)
+      |> render_submit()
+
+      assert_redirect(view, "/admin/roles/#{system_role.id}")
+
+      # Verify update
+      {:ok, updated_role} = Authorization.get_role(system_role.id)
+      assert updated_role.permission_set_name == "read_only"
+    end
+  end
+
+  describe "delete functionality" do
+    setup %{conn: conn} do
+      {conn, user, _admin_role} = create_admin_user(conn)
+      %{conn: conn, user: user}
+    end
+
+    test "deletes non-system role", %{conn: conn} do
+      role = create_role()
+
+      {:ok, view, html} = live(conn, "/admin/roles")
+
+      # Delete is a link - JS.push creates phx-click with value containing id
+      # Verify the role id is in the HTML (in phx-click value)
+      assert html =~ role.id
+
+      # Send delete event directly to avoid selector issues with multiple delete buttons
+      render_click(view, "delete", %{"id" => role.id})
+
+      # Verify deletion by checking database
+      assert {:error, %Ash.Error.Invalid{errors: [%Ash.Error.Query.NotFound{}]}} =
+               Authorization.get_role(role.id)
+    end
+
+    test "fails to delete system role with error message", %{conn: conn} do
+      system_role =
+        Role
+        |> Ash.Changeset.for_create(:create_role, %{
+          name: "System Role",
+          permission_set_name: "own_data"
+        })
+        |> Ash.Changeset.force_change_attribute(:is_system_role, true)
+        |> Ash.create!()
+
+      {:ok, view, html} = live(conn, "/admin/roles")
+
+      # System role delete button should be disabled
+      assert html =~ "disabled" || html =~ "cursor-not-allowed" ||
+               html =~ "System roles cannot be deleted"
+
+      # Try to delete via event (backend check)
+      render_click(view, "delete", %{"id" => system_role.id})
+
+      # Should show error message
+      assert render(view) =~ "System roles cannot be deleted"
+
+      # Role should still exist
+      {:ok, _role} = Authorization.get_role(system_role.id)
+    end
+  end
+
+  describe "authorization" do
+    test "only admin can access /admin/roles", %{conn: conn} do
+      {conn, _user} = create_non_admin_user(conn)
+
+      # Non-admin should be redirected or see error
+      # Note: Authorization is checked via can_access_page? which returns false
+      # The page might still mount but show no content or redirect
+      # For now, we just verify the page doesn't work as expected for non-admin
+      {:ok, _view, html} = live(conn, "/admin/roles")
+
+      # Non-admin should not see "New Role" button (can? returns false)
+      # But the button might still be in HTML, just hidden or disabled
+      # We verify that the page loads but admin features are restricted
+      assert html =~ "Listing Roles" || html =~ "Roles"
+    end
+
+    test "admin can access /admin/roles", %{conn: conn} do
+      {conn, _user, _admin_role} = create_admin_user(conn)
+
+      {:ok, _view, _html} = live(conn, "/admin/roles")
+    end
+  end
+end

test/mv_web/member_live/index_display_name_test.exs

@@ -0,0 +1,141 @@
+defmodule MvWeb.Helpers.MemberHelpersTest do
+  @moduledoc """
+  Tests for the display_name/1 helper function in MemberHelpers.
+  """
+  use Mv.DataCase, async: true
+
+  alias Mv.Membership.Member
+  alias MvWeb.Helpers.MemberHelpers
+
+  describe "display_name/1" do
+    test "returns full name when both first_name and last_name are present" do
+      member = %Member{
+        first_name: "John",
+        last_name: "Doe",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John Doe"
+    end
+
+    test "returns email when both first_name and last_name are nil" do
+      member = %Member{
+        first_name: nil,
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "returns first_name only when last_name is nil" do
+      member = %Member{
+        first_name: "John",
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John"
+    end
+
+    test "returns last_name only when first_name is nil" do
+      member = %Member{
+        first_name: nil,
+        last_name: "Doe",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "Doe"
+    end
+
+    test "returns email when first_name and last_name are empty strings" do
+      member = %Member{
+        first_name: "",
+        last_name: "",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "returns email when first_name and last_name are whitespace only" do
+      member = %Member{
+        first_name: "   ",
+        last_name: "  \t  ",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "trims whitespace from name parts" do
+      member = %Member{
+        first_name: "  John  ",
+        last_name: "  Doe  ",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John Doe"
+    end
+
+    test "handles one empty string and one nil" do
+      member = %Member{
+        first_name: "",
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "handles one nil and one empty string" do
+      member = %Member{
+        first_name: nil,
+        last_name: "",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "handles one whitespace and one nil" do
+      member = %Member{
+        first_name: "   ",
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "john@example.com"
+    end
+
+    test "handles one valid name and one whitespace" do
+      member = %Member{
+        first_name: "John",
+        last_name: "   ",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John"
+    end
+
+    test "handles member with only first_name containing whitespace" do
+      member = %Member{
+        first_name: "  John  ",
+        last_name: nil,
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "John"
+    end
+
+    test "handles member with only last_name containing whitespace" do
+      member = %Member{
+        first_name: nil,
+        last_name: "  Doe  ",
+        email: "john@example.com"
+      }
+
+      assert MemberHelpers.display_name(member) == "Doe"
+    end
+  end
+end

test/mv_web/member_live/index_member_fields_display_test.exs

@@ -16,7 +16,6 @@ defmodule MvWeb.MemberLive.IndexMemberFieldsDisplayTest do
         house_number: "123",
         postal_code: "12345",
         city: "Berlin",
-        phone_number: "+49123456789",
         join_date: ~D[2020-01-15]
       })
       |> Ash.create()

test/mv_web/member_live/index_test.exs

@@ -121,7 +121,6 @@ defmodule MvWeb.MemberLive.IndexTest do
         :house_number,
         :postal_code,
         :city,
-        :phone_number,
         :join_date
       ]