From 075a06ba6f2105cac8ee5d5267dc76e6b7d11807 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 9 Jan 2026 02:23:29 +0100 Subject: [PATCH] Refactor test setup: use global setup and fix MembershipFees domain alias - Remove redundant setup blocks from member_live tests - Add build_unauthenticated_conn helper for AuthController tests - Add global setup in conn_case.ex --- lib/mv_web/live/member_live/form.ex | 2 +- .../show/membership_fees_component.ex | 17 +-- .../controllers/auth_controller_test.exs | 89 +++++++++++++--- .../form_membership_fee_type_test.exs | 14 --- .../index_membership_fee_status_test.exs | 14 --- .../membership_fee_integration_test.exs | 14 --- .../member_live/show_membership_fees_test.exs | 14 --- test/support/conn_case.ex | 33 +++++- test/support/fixtures.ex | 100 ++++++++++++++++++ 9 files changed, 216 insertions(+), 81 deletions(-) diff --git a/lib/mv_web/live/member_live/form.ex b/lib/mv_web/live/member_live/form.ex index c7c718e..42d0da2 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -339,7 +339,7 @@ defmodule MvWeb.MemberLive.Form do form = if member do - {:ok, member} = Ash.load(member, custom_field_values: [:custom_field], actor: actor) + {:ok, member} = Ash.load(member, [custom_field_values: [:custom_field]], actor: actor) existing_custom_field_values = member.custom_field_values diff --git a/lib/mv_web/live/member_live/show/membership_fees_component.ex b/lib/mv_web/live/member_live/show/membership_fees_component.ex index 0b32efe..864889a 100644 --- a/lib/mv_web/live/member_live/show/membership_fees_component.ex +++ b/lib/mv_web/live/member_live/show/membership_fees_component.ex @@ -15,10 +15,11 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do require Ash.Query alias Mv.Membership - alias Mv.MembershipFees.CalendarCycles - alias Mv.MembershipFees.CycleGenerator - alias Mv.MembershipFees.MembershipFeeCycle + alias Mv.MembershipFees alias Mv.MembershipFees.MembershipFeeType + alias Mv.MembershipFees.MembershipFeeCycle + alias Mv.MembershipFees.CycleGenerator + alias Mv.MembershipFees.CalendarCycles alias MvWeb.Helpers.MembershipFeeHelpers @impl true @@ -63,7 +64,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do phx-click="delete_all_cycles" phx-target={@myself} class="btn btn-sm btn-error btn-outline" - title={gettext("Delete All Cycles")} + title={gettext("Delete all cycles")} > <.icon name="hero-trash" class="size-4" /> {gettext("Delete All Cycles")} @@ -168,7 +169,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do phx-value-cycle_id={cycle.id} phx-target={@myself} class="btn btn-sm btn-error btn-outline" - title={gettext("Delete Cycle")} + title={gettext("Delete cycle")} > <.icon name="hero-trash" class="size-4" /> {gettext("Delete")} @@ -329,14 +330,16 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do /> <%= if @create_cycle_date do %>
{format_create_cycle_period( diff --git a/test/mv_web/controllers/auth_controller_test.exs b/test/mv_web/controllers/auth_controller_test.exs index 3c3956d..444571b 100644 --- a/test/mv_web/controllers/auth_controller_test.exs +++ b/test/mv_web/controllers/auth_controller_test.exs @@ -1,21 +1,42 @@ defmodule MvWeb.AuthControllerTest do use MvWeb.ConnCase, async: true import Phoenix.LiveViewTest + import Phoenix.ConnTest + + # Helper to create an unauthenticated conn (preserves sandbox metadata) + defp build_unauthenticated_conn(authenticated_conn) do + # Create new conn but preserve sandbox metadata for database access + new_conn = build_conn() + + # Copy sandbox metadata from authenticated conn + if authenticated_conn.private[:ecto_sandbox] do + Plug.Conn.put_private(new_conn, :ecto_sandbox, authenticated_conn.private[:ecto_sandbox]) + else + new_conn + end + end # Basic UI tests - test "GET /sign-in shows sign in form", %{conn: conn} do + test "GET /sign-in shows sign in form", %{conn: authenticated_conn} do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) conn = get(conn, ~p"/sign-in") assert html_response(conn, 200) =~ "Sign in" end - test "GET /sign-out redirects to home", %{conn: conn} do - conn = conn_with_oidc_user(conn) + test "GET /sign-out redirects to home", %{conn: authenticated_conn} do + conn = conn_with_oidc_user(authenticated_conn) conn = get(conn, ~p"/sign-out") assert redirected_to(conn) == ~p"/" end # Password authentication (LiveView) - test "password user can sign in with valid credentials via LiveView", %{conn: conn} do + test "password user can sign in with valid credentials via LiveView", %{ + conn: authenticated_conn + } do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) + _user = create_test_user(%{ email: "password@example.com", @@ -35,7 +56,12 @@ defmodule MvWeb.AuthControllerTest do assert to =~ "/auth/user/password/sign_in_with_token" end - test "password user with invalid credentials shows error via LiveView", %{conn: conn} do + test "password user with invalid credentials shows error via LiveView", %{ + conn: authenticated_conn + } do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) + _user = create_test_user(%{ email: "test@example.com", @@ -55,7 +81,12 @@ defmodule MvWeb.AuthControllerTest do assert html =~ "Email or password was incorrect" end - test "password user with non-existent email shows error via LiveView", %{conn: conn} do + test "password user with non-existent email shows error via LiveView", %{ + conn: authenticated_conn + } do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) + {:ok, view, _html} = live(conn, "/sign-in") html = @@ -69,7 +100,10 @@ defmodule MvWeb.AuthControllerTest do end # Registration (LiveView) - test "user can register with valid credentials via LiveView", %{conn: conn} do + test "user can register with valid credentials via LiveView", %{conn: authenticated_conn} do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) + {:ok, view, _html} = live(conn, "/register") {:error, {:redirect, %{to: to}}} = @@ -82,7 +116,10 @@ defmodule MvWeb.AuthControllerTest do assert to =~ "/auth/user/password/sign_in_with_token" end - test "registration with existing email shows error via LiveView", %{conn: conn} do + test "registration with existing email shows error via LiveView", %{conn: authenticated_conn} do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) + _user = create_test_user(%{ email: "existing@example.com", @@ -102,7 +139,10 @@ defmodule MvWeb.AuthControllerTest do assert html =~ "has already been taken" end - test "registration with weak password shows error via LiveView", %{conn: conn} do + test "registration with weak password shows error via LiveView", %{conn: authenticated_conn} do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) + {:ok, view, _html} = live(conn, "/register") html = @@ -116,18 +156,27 @@ defmodule MvWeb.AuthControllerTest do end # Access control - test "unauthenticated user accessing protected route gets redirected to sign-in", %{conn: conn} do + test "unauthenticated user accessing protected route gets redirected to sign-in", %{ + conn: authenticated_conn + } do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) conn = get(conn, ~p"/members") assert redirected_to(conn) == ~p"/sign-in" end - test "authenticated user can access protected route", %{conn: conn} do - conn = conn_with_oidc_user(conn) + test "authenticated user can access protected route", %{conn: authenticated_conn} do + conn = conn_with_oidc_user(authenticated_conn) conn = get(conn, ~p"/members") assert conn.status == 200 end - test "password authenticated user can access protected route via LiveView", %{conn: conn} do + test "password authenticated user can access protected route via LiveView", %{ + conn: authenticated_conn + } do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) + _user = create_test_user(%{ email: "auth@example.com", @@ -150,7 +199,12 @@ defmodule MvWeb.AuthControllerTest do end # Edge cases - test "user with nil oidc_id can still sign in with password via LiveView", %{conn: conn} do + test "user with nil oidc_id can still sign in with password via LiveView", %{ + conn: authenticated_conn + } do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) + _user = create_test_user(%{ email: "nil_oidc@example.com", @@ -170,7 +224,12 @@ defmodule MvWeb.AuthControllerTest do assert to =~ "/auth/user/password/sign_in_with_token" end - test "user with empty string oidc_id is handled correctly via LiveView", %{conn: conn} do + test "user with empty string oidc_id is handled correctly via LiveView", %{ + conn: authenticated_conn + } do + # Create unauthenticated conn for this test + conn = build_unauthenticated_conn(authenticated_conn) + _user = create_test_user(%{ email: "empty_oidc@example.com", diff --git a/test/mv_web/member_live/form_membership_fee_type_test.exs b/test/mv_web/member_live/form_membership_fee_type_test.exs index cc4388f..63c5a0d 100644 --- a/test/mv_web/member_live/form_membership_fee_type_test.exs +++ b/test/mv_web/member_live/form_membership_fee_type_test.exs @@ -11,20 +11,6 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do require Ash.Query - setup %{conn: conn} do - # Create admin 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() - - authenticated_conn = conn_with_password_user(conn, user) - %{conn: authenticated_conn, user: user} - end - # Helper to create a membership fee type defp create_fee_type(attrs) do default_attrs = %{ diff --git a/test/mv_web/member_live/index_membership_fee_status_test.exs b/test/mv_web/member_live/index_membership_fee_status_test.exs index baa5f67..a189873 100644 --- a/test/mv_web/member_live/index_membership_fee_status_test.exs +++ b/test/mv_web/member_live/index_membership_fee_status_test.exs @@ -12,20 +12,6 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do require Ash.Query - setup %{conn: conn} do - # Create admin 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() - - conn = conn_with_password_user(conn, user) - %{conn: conn, user: user} - end - # Helper to create a membership fee type defp create_fee_type(attrs) do default_attrs = %{ diff --git a/test/mv_web/member_live/membership_fee_integration_test.exs b/test/mv_web/member_live/membership_fee_integration_test.exs index e76e422..9358c70 100644 --- a/test/mv_web/member_live/membership_fee_integration_test.exs +++ b/test/mv_web/member_live/membership_fee_integration_test.exs @@ -12,20 +12,6 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do require Ash.Query - setup do - # Create admin 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() - - conn = conn_with_password_user(build_conn(), user) - %{conn: conn, user: user} - end - # Helper to create a membership fee type defp create_fee_type(attrs) do default_attrs = %{ diff --git a/test/mv_web/member_live/show_membership_fees_test.exs b/test/mv_web/member_live/show_membership_fees_test.exs index 1f68244..d0402c3 100644 --- a/test/mv_web/member_live/show_membership_fees_test.exs +++ b/test/mv_web/member_live/show_membership_fees_test.exs @@ -12,20 +12,6 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do require Ash.Query - setup %{conn: conn} do - # Create admin 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() - - conn = conn_with_password_user(conn, user) - %{conn: conn, user: user} - end - # Helper to create a membership fee type defp create_fee_type(attrs) do default_attrs = %{ diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index 853a326..8f943d9 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -99,6 +99,7 @@ defmodule MvWeb.ConnCase do @doc """ Signs in a user via OIDC and returns a connection with the user authenticated. By default creates a user with "user@example.com" for consistency. + The user will have an admin role for authorization. """ def conn_with_oidc_user(conn, user_attrs \\ %{}) do # Ensure unique email for OIDC users @@ -109,8 +110,22 @@ defmodule MvWeb.ConnCase do oidc_id: "oidc_#{unique_id}" } + # Create user using Ash.Seed (supports oidc_id) user = create_test_user(Map.merge(default_attrs, user_attrs)) - sign_in_user_via_oidc(conn, user) + + # Create admin role and assign it + admin_role = Mv.Fixtures.role_fixture("admin") + + {: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 + user_with_role = Ash.load!(user, :role, domain: Mv.Accounts) + + sign_in_user_via_oidc(conn, user_with_role) end @doc """ @@ -122,6 +137,15 @@ defmodule MvWeb.ConnCase do |> AshAuthentication.Plug.Helpers.store_in_session(user) end + @doc """ + Creates a connection with an authenticated user that has an admin role. + This is useful for tests that need full access to resources. + """ + def conn_with_admin_user(conn) do + admin_user = Mv.Fixtures.user_with_role_fixture("admin") + conn_with_password_user(conn, admin_user) + end + setup tags do pid = Mv.DataCase.setup_sandbox(tags) @@ -130,6 +154,11 @@ defmodule MvWeb.ConnCase do # to share the test's database connection in async tests conn = Plug.Conn.put_private(conn, :ecto_sandbox, pid) - {:ok, conn: conn} + # Create admin user with role for all tests (unless test overrides with its own user) + # This ensures all tests have an authenticated user with proper authorization + admin_user = Mv.Fixtures.user_with_role_fixture("admin") + authenticated_conn = conn_with_password_user(conn, admin_user) + + {:ok, conn: authenticated_conn, current_user: admin_user} end end diff --git a/test/support/fixtures.ex b/test/support/fixtures.ex index 5dd14a9..d474764 100644 --- a/test/support/fixtures.ex +++ b/test/support/fixtures.ex @@ -93,4 +93,104 @@ defmodule Mv.Fixtures do {user, member} end + + @doc """ + Creates a role with a specific permission set. + + ## Parameters + - `permission_set_name` - The permission set name (e.g., "admin", "read_only", "normal_user", "own_data") + + ## Returns + - Role struct + + ## Examples + + iex> role_fixture("admin") + %Mv.Authorization.Role{permission_set_name: "admin", ...} + + """ + def role_fixture(permission_set_name) do + role_name = "Test Role #{permission_set_name} #{System.unique_integer([:positive])}" + + case Mv.Authorization.create_role(%{ + name: role_name, + description: "Test role for #{permission_set_name}", + permission_set_name: permission_set_name + }) do + {:ok, role} -> role + {:error, error} -> raise "Failed to create role: #{inspect(error)}" + end + end + + @doc """ + Creates a user with a specific permission set (role). + + ## Parameters + - `permission_set_name` - The permission set name (e.g., "admin", "read_only", "normal_user", "own_data") + - `user_attrs` - Optional user attributes + + ## Returns + - User struct with role preloaded + + ## Examples + + iex> admin_user = user_with_role_fixture("admin") + iex> admin_user.role.permission_set_name + "admin" + + """ + def user_with_role_fixture(permission_set_name \\ "admin", user_attrs \\ %{}) do + # Create role with permission set + role = role_fixture(permission_set_name) + + # Create user + {:ok, user} = + user_attrs + |> Enum.into(%{ + email: "user#{System.unique_integer([:positive])}@example.com" + }) + |> Mv.Accounts.create_user() + + # Assign role to user + {:ok, user} = + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, role, type: :append_and_remove) + |> Ash.update() + + # Reload user with role preloaded (critical for authorization!) + {:ok, user_with_role} = Ash.load(user, :role, domain: Mv.Accounts) + user_with_role + end + + @doc """ + Creates a member with an actor (for use in tests with policies). + + ## Parameters + - `attrs` - Map or keyword list of attributes to override defaults + - `actor` - The actor (user) to use for authorization + + ## Returns + - Member struct + + ## Examples + + iex> admin = user_with_role_fixture("admin") + iex> member_fixture_with_actor(%{first_name: "Alice"}, admin) + %Mv.Membership.Member{first_name: "Alice", ...} + + """ + def member_fixture_with_actor(attrs \\ %{}, actor) do + attrs + |> Enum.into(%{ + first_name: "Test", + last_name: "Member", + email: "test#{System.unique_integer([:positive])}@example.com" + }) + |> Mv.Membership.create_member(actor: actor) + |> case do + {:ok, member} -> member + {:error, error} -> raise "Failed to create member: #{inspect(error)}" + end + end end