diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 8c4b650..0a87836 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -683,6 +683,13 @@ end - Falls back to admin user from seeds if system user doesn't exist - Should NEVER be used for user-initiated actions (only systemic operations) +**DO NOT use system actor as a fallback:** + +- **Never** fall back to `Mv.Helpers.SystemActor.get_system_actor()` when an actor is missing or nil (e.g. in validations, changes, or when reading from context). +- Fallbacks hide bugs (callers forget to pass actor) and can cause privilege escalation (unauthenticated or low-privilege paths run with system rights). +- If no actor is available, fail explicitly (validation error, Forbidden, or clear error message). Fix the caller to pass the correct actor instead of adding a fallback. +- Use system actor only where the operation is **explicitly** a systemic operation (see list above); never as a "safety net" when actor is absent. + **User Mode vs System Mode:** - **User Mode**: User-initiated actions use the actual user actor, policies are enforced @@ -1711,6 +1718,30 @@ This organization ensures fast feedback in standard CI while maintaining compreh ## 5. Security Guidelines +### 5.0 No system-actor fallbacks (mandatory) + +**Do not use the system actor as a fallback when an actor is missing.** + +Examples of forbidden patterns: + +```elixir +# ❌ FORBIDDEN - Fallback to system actor when actor is nil +actor = Map.get(changeset.context, :actor) || Mv.Helpers.SystemActor.get_system_actor() + +# ❌ FORBIDDEN - "Safety" fallback in validations, changes, or helpers +actor = opts[:actor] || Mv.Helpers.SystemActor.get_system_actor() + +# ❌ FORBIDDEN - Default actor in function options +def list_something(opts \\ []) do + actor = Keyword.get(opts, :actor) || Mv.Helpers.SystemActor.get_system_actor() + # ... +end +``` + +**Why:** Fallbacks hide missing-actor bugs and can lead to privilege escalation (e.g. a request without actor would run with system privileges). Always require the caller to pass the actor for user-facing or context-dependent operations; if none is available, return an error or fail validation instead of using the system actor. + +**Allowed:** Use the system actor only where the operation is **by design** a systemic operation (e.g. email sync, seeds, test fixtures, background jobs) and you explicitly call `SystemActor.get_system_actor()` at that call site—never as a fallback when `actor` is nil or absent. + ### 5.1 Authentication & Authorization **Use AshAuthentication:** diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index 063de32..5b930a7 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -1101,28 +1101,23 @@ end ### CustomField Resource Policies -**Location:** `lib/mv/membership/custom_field.ex` +**Location:** `lib/membership/custom_field.ex` **No Special Cases:** All users can read, only admin can write. ```elixir defmodule Mv.Membership.CustomField do - use Ash.Resource, ... - + use Ash.Resource, + domain: Mv.Membership, + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] + policies do - # All authenticated users can read custom fields (needed for forms) - # Write operations are admin-only policy action_type([:read, :create, :update, :destroy]) do description "Check permissions from user's role" authorize_if Mv.Authorization.Checks.HasPermission end - - # DEFAULT: Forbid - policy action_type([:read, :create, :update, :destroy]) do - forbid_if always() - end end - # ... end ``` diff --git a/lib/membership/custom_field.ex b/lib/membership/custom_field.ex index 94cb657..ab4ad60 100644 --- a/lib/membership/custom_field.ex +++ b/lib/membership/custom_field.ex @@ -50,7 +50,8 @@ defmodule Mv.Membership.CustomField do """ use Ash.Resource, domain: Mv.Membership, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] postgres do table "custom_fields" @@ -79,6 +80,13 @@ defmodule Mv.Membership.CustomField do end end + policies do + policy action_type([:read, :create, :update, :destroy]) do + description "Check permissions from user's role" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + attributes do uuid_primary_key :id diff --git a/lib/membership/member.ex b/lib/membership/member.ex index da61146..7b49c86 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -471,11 +471,12 @@ defmodule Mv.Membership.Member do end end - # Validate required custom fields - validate fn changeset, _ -> + # Validate required custom fields (actor from validation context only; no fallback) + validate fn changeset, context -> provided_values = provided_custom_field_values(changeset) + actor = context.actor - case Mv.Membership.list_required_custom_fields() do + case Mv.Membership.list_required_custom_fields(actor: actor) do {:ok, required_custom_fields} -> missing_fields = missing_required_fields(required_custom_fields, provided_values) @@ -485,6 +486,24 @@ defmodule Mv.Membership.Member do build_custom_field_validation_error(missing_fields) end + {:error, %Ash.Error.Forbidden{}} -> + Logger.warning( + "Required custom fields validation: actor not authorized to read CustomField" + ) + + {:error, + field: :custom_field_values, + message: + "You are not authorized to perform this action. Please sign in again or contact support."} + + {:error, :missing_actor} -> + Logger.warning("Required custom fields validation: no actor in context") + + {:error, + field: :custom_field_values, + message: + "You are not authorized to perform this action. Please sign in again or contact support."} + {:error, error} -> Logger.error( "Failed to load custom fields for validation: #{inspect(error)}. Required field validation cannot be performed." diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 47d2f0e..74735e4 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -14,7 +14,7 @@ defmodule Mv.Membership do The domain exposes these main actions: - Member CRUD: `create_member/1`, `list_members/0`, `update_member/2`, `destroy_member/1` - Custom field value management: `create_custom_field_value/1`, `list_custom_field_values/0`, etc. - - Custom field management: `create_custom_field/1`, `list_custom_fields/0`, `list_required_custom_fields/0`, etc. + - Custom field management: `create_custom_field/1`, `list_custom_fields/0`, `list_required_custom_fields/1`, etc. - Settings management: `get_settings/0`, `update_settings/2`, `update_member_field_visibility/2`, `update_single_member_field_visibility/3` - Group management: `create_group/1`, `list_groups/0`, `update_group/2`, `destroy_group/1` - Member-group associations: `create_member_group/1`, `list_member_groups/0`, `destroy_member_group/1` @@ -155,25 +155,37 @@ defmodule Mv.Membership do Lists only required custom fields. This is an optimized version that filters at the database level instead of - loading all custom fields and filtering in memory. + loading all custom fields and filtering in memory. Requires an actor for + authorization (CustomField read policy). Callers must pass `actor:`; no default. + + ## Options + + - `:actor` - Required. The actor for authorization (e.g. current user). + All roles can read CustomField; actor must have a valid role. ## Returns - `{:ok, required_custom_fields}` - List of required custom fields - - `{:error, error}` - Error reading custom fields + - `{:error, :missing_actor}` - When actor is nil (caller must pass actor) + - `{:error, error}` - Error reading custom fields (e.g. Forbidden) ## Examples - iex> {:ok, required_fields} = Mv.Membership.list_required_custom_fields() + iex> {:ok, required_fields} = Mv.Membership.list_required_custom_fields(actor: actor) iex> Enum.all?(required_fields, & &1.required) true + + iex> Mv.Membership.list_required_custom_fields(actor: nil) + {:error, :missing_actor} """ - def list_required_custom_fields do + def list_required_custom_fields(actor: actor) when not is_nil(actor) do Mv.Membership.CustomField |> Ash.Query.filter(expr(required == true)) - |> Ash.read(domain: __MODULE__) + |> Ash.read(domain: __MODULE__, actor: actor) end + def list_required_custom_fields(actor: nil), do: {:error, :missing_actor} + @doc """ Updates the member field visibility configuration. diff --git a/lib/mv_web/live/custom_field_live/form_component.ex b/lib/mv_web/live/custom_field_live/form_component.ex index 54ef4a7..b809a1a 100644 --- a/lib/mv_web/live/custom_field_live/form_component.ex +++ b/lib/mv_web/live/custom_field_live/form_component.ex @@ -91,7 +91,8 @@ defmodule MvWeb.CustomFieldLive.FormComponent do @impl true def handle_event("save", %{"custom_field" => custom_field_params}, socket) do - actor = MvWeb.LiveHelpers.current_actor(socket) + # Actor must be passed from parent (IndexComponent); component socket has no current_user + actor = socket.assigns[:actor] case MvWeb.LiveHelpers.submit_form(socket.assigns.form, custom_field_params, actor) do {:ok, custom_field} -> diff --git a/lib/mv_web/live/custom_field_live/index_component.ex b/lib/mv_web/live/custom_field_live/index_component.ex index 5f26f78..784d1ef 100644 --- a/lib/mv_web/live/custom_field_live/index_component.ex +++ b/lib/mv_web/live/custom_field_live/index_component.ex @@ -12,6 +12,8 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do """ use MvWeb, :live_component + require Logger + @impl true def render(assigns) do assigns = assign(assigns, :field_type_label, &MvWeb.Translations.FieldTypes.label/1) @@ -38,6 +40,7 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do <.live_component module={MvWeb.CustomFieldLive.FormComponent} id={@form_id} + actor={@actor} custom_field={@editing_custom_field} on_save={ fn custom_field, action -> send(self(), {:custom_field_saved, custom_field, action}) end @@ -176,6 +179,21 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do """ end + defp stream_custom_fields(actor, parent_pid) do + case Ash.read(Mv.Membership.CustomField, actor: actor) do + {:ok, custom_fields} -> + custom_fields + + {:error, error} -> + Logger.warning( + "CustomFieldLive.IndexComponent: failed to load custom fields: #{inspect(error)}" + ) + + send(parent_pid, {:custom_fields_load_error, error}) + [] + end + end + @impl true def update(assigns, socket) do # Track previous show_form state to detect when form is closed @@ -207,7 +225,7 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do |> assign_new(:show_delete_modal, fn -> false end) |> assign_new(:custom_field_to_delete, fn -> nil end) |> assign_new(:slug_confirmation, fn -> "" end) - |> stream(:custom_fields, Ash.read!(Mv.Membership.CustomField), reset: true)} + |> stream(:custom_fields, stream_custom_fields(assigns[:actor], self()), reset: true)} end @impl true @@ -226,7 +244,8 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do @impl true def handle_event("edit_custom_field", %{"id" => id}, socket) do - custom_field = Ash.get!(Mv.Membership.CustomField, id) + actor = socket.assigns[:actor] + custom_field = Ash.get!(Mv.Membership.CustomField, id, actor: actor) # Only send event if form was not already open if not socket.assigns[:show_form] do @@ -242,7 +261,13 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do @impl true def handle_event("prepare_delete", %{"id" => id}, socket) do - custom_field = Ash.get!(Mv.Membership.CustomField, id, load: [:assigned_members_count]) + actor = socket.assigns[:actor] + + custom_field = + Ash.get!(Mv.Membership.CustomField, id, + load: [:assigned_members_count], + actor: actor + ) {:noreply, socket @@ -259,9 +284,10 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do @impl true def handle_event("confirm_delete", _params, socket) do custom_field = socket.assigns.custom_field_to_delete + actor = socket.assigns[:actor] if socket.assigns.slug_confirmation == custom_field.slug do - case Ash.destroy(custom_field) do + case Ash.destroy(custom_field, actor: actor) do :ok -> send(self(), {:custom_field_deleted, custom_field}) diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index 0fbcbbe..bd0036b 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -130,6 +130,7 @@ defmodule MvWeb.GlobalSettingsLive do :if={@active_editing_section != :member_fields} module={MvWeb.CustomFieldLive.IndexComponent} id="custom-fields-component" + actor={@current_user} /> @@ -503,6 +504,15 @@ defmodule MvWeb.GlobalSettingsLive do {:noreply, put_flash(socket, :error, gettext("Slug does not match. Deletion cancelled."))} end + def handle_info({:custom_fields_load_error, _error}, socket) do + {:noreply, + put_flash( + socket, + :error, + gettext("Could not load data fields. Please check your permissions.") + )} + end + @impl true def handle_info({:editing_section_changed, section}, socket) do {:noreply, assign(socket, :active_editing_section, section)} diff --git a/lib/mv_web/live/member_live/form.ex b/lib/mv_web/live/member_live/form.ex index 2f3a0ff..b72add6 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -226,7 +226,7 @@ defmodule MvWeb.MemberLive.Form do def mount(params, _session, socket) do # current_user should be set by on_mount hooks (LiveUserAuth + LiveHelpers) actor = current_actor(socket) - {:ok, custom_fields} = Mv.Membership.list_custom_fields() + {:ok, custom_fields} = Mv.Membership.list_custom_fields(actor: actor) initial_custom_field_values = Enum.map(custom_fields, fn cf -> diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index e97950d..d650aa2 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -2262,3 +2262,13 @@ msgstr "Dieser Benutzer kann nicht bearbeitet werden." #, elixir-autogen, elixir-format msgid "This user cannot be viewed." msgstr "Dieser Benutzer kann nicht angezeigt werden." + +#: lib/mv_web/live/group_live/show.ex +#, elixir-autogen, elixir-format +msgid "Not authorized." +msgstr "Nicht berechtigt." + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Could not load data fields. Please check your permissions." +msgstr "Datenfelder konnten nicht geladen werden. Bitte überprüfen Sie Ihre Berechtigungen." diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 13b488c..98f9d7b 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -2268,3 +2268,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Not authorized." msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Could not load data fields. Please check your permissions." +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index a85d2d7..95a3c3a 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -2263,3 +2263,13 @@ msgstr "" #, elixir-autogen, elixir-format msgid "This user cannot be viewed." msgstr "" + +#: lib/mv_web/live/group_live/show.ex +#, elixir-autogen, elixir-format +msgid "Not authorized." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Could not load data fields. Please check your permissions." +msgstr "" diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 43ffcaf..4240336 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -118,10 +118,12 @@ for attrs <- [ required: false } ] do + # Bootstrap: no admin user yet; CustomField create requires admin, so skip authorization Membership.create_custom_field!( attrs, upsert?: true, - upsert_identity: :unique_name + upsert_identity: :unique_name, + authorize?: false ) end @@ -379,12 +381,16 @@ Enum.each(member_attrs_list, fn member_attrs -> final_member = if is_nil(member.membership_fee_type_id) and Map.has_key?(member_attrs_without_status, :membership_fee_type_id) do - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: member_attrs_without_status.membership_fee_type_id - }) - |> Ash.Changeset.put_context(:actor, admin_user_with_role) - |> Ash.update!(actor: admin_user_with_role) + {:ok, updated} = + Membership.update_member( + member, + %{ + membership_fee_type_id: member_attrs_without_status.membership_fee_type_id + }, + actor: admin_user_with_role + ) + + updated else member end @@ -544,9 +550,12 @@ Enum.with_index(linked_members) fee_type_index = rem(3 + index, length(all_fee_types)) fee_type = Enum.at(all_fee_types, fee_type_index) - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: admin_user_with_role) + {:ok, updated} = + Membership.update_member(member, %{membership_fee_type_id: fee_type.id}, + actor: admin_user_with_role + ) + + updated else member end @@ -594,7 +603,7 @@ end) # Create sample custom field values for some members all_members = Ash.read!(Membership.Member, actor: admin_user_with_role) -all_custom_fields = Ash.read!(Membership.CustomField) +all_custom_fields = Ash.read!(Membership.CustomField, actor: admin_user_with_role) # Helper function to find custom field by name find_field = fn name -> Enum.find(all_custom_fields, &(&1.name == name)) end diff --git a/test/membership/custom_field_deletion_test.exs b/test/membership/custom_field_deletion_test.exs index ffc7294..80f8a2b 100644 --- a/test/membership/custom_field_deletion_test.exs +++ b/test/membership/custom_field_deletion_test.exs @@ -239,13 +239,14 @@ defmodule Mv.Membership.CustomFieldDeletionTest do # Helper functions defp create_member(actor) do - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User#{System.unique_integer([:positive])}", - email: "test#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: actor) + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User#{System.unique_integer([:positive])}", + email: "test#{System.unique_integer([:positive])}@example.com" + }, + actor: actor + ) end defp create_custom_field(name, value_type, actor) do diff --git a/test/membership/custom_field_value_validation_test.exs b/test/membership/custom_field_value_validation_test.exs index d39e85c..1c237be 100644 --- a/test/membership/custom_field_value_validation_test.exs +++ b/test/membership/custom_field_value_validation_test.exs @@ -17,13 +17,14 @@ defmodule Mv.Membership.CustomFieldValueValidationTest do # Create a test member {:ok, member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test.validation@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test.validation@example.com" + }, + actor: system_actor + ) # Create custom fields for different types {:ok, string_field} = diff --git a/test/membership/member_cycle_calculations_test.exs b/test/membership/member_cycle_calculations_test.exs index ea7f378..da08d81 100644 --- a/test/membership/member_cycle_calculations_test.exs +++ b/test/membership/member_cycle_calculations_test.exs @@ -38,10 +38,8 @@ defmodule Mv.Membership.MemberCycleCalculationsTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: actor) + member end # Helper to create a cycle diff --git a/test/membership/member_search_with_custom_fields_test.exs b/test/membership/member_search_with_custom_fields_test.exs index 284fcff..62e98c3 100644 --- a/test/membership/member_search_with_custom_fields_test.exs +++ b/test/membership/member_search_with_custom_fields_test.exs @@ -14,31 +14,22 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Create test members {:ok, member1} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Alice", last_name: "Anderson", email: "alice@example.com"}, + actor: system_actor + ) {:ok, member2} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Bob", - last_name: "Brown", - email: "bob@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Bob", last_name: "Brown", email: "bob@example.com"}, + actor: system_actor + ) {:ok, member3} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Charlie", - last_name: "Clark", - email: "charlie@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Charlie", last_name: "Clark", email: "charlie@example.com"}, + actor: system_actor + ) # Create custom fields for different types {:ok, string_field} = @@ -112,9 +103,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update by reloading member {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # Search for the custom field value results = @@ -143,9 +132,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # Search for the custom field value results = @@ -174,9 +161,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # Search for partial custom field value (should work via FTS or custom field filter) results = @@ -225,9 +210,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # Search for the custom field value (date is stored as text in search_vector) results = @@ -256,9 +239,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # Search for the custom field value (boolean is stored as "true" or "false" text) results = @@ -287,9 +268,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # Update custom field value {:ok, _updated_cfv} = @@ -334,9 +313,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # Verify it's searchable results = @@ -401,9 +378,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Update member (should trigger search_vector update including custom fields) {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{notes: "Updated notes"}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{notes: "Updated notes"}, actor: system_actor) # Search should find the custom field value results = @@ -452,9 +427,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # All values should be searchable results1 = @@ -496,9 +469,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # Search for full value (should work via search_vector) results_full = @@ -541,9 +512,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do # Force search_vector update {:ok, _updated_member} = - member1 - |> Ash.Changeset.for_update(:update_member, %{}) - |> Ash.update(actor: system_actor) + Mv.Membership.update_member(member1, %{}, actor: system_actor) # Search for full phone number (should work via search_vector) results_full = diff --git a/test/membership/member_type_change_integration_test.exs b/test/membership/member_type_change_integration_test.exs index cb289be..69d722d 100644 --- a/test/membership/member_type_change_integration_test.exs +++ b/test/membership/member_type_change_integration_test.exs @@ -41,10 +41,8 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: actor) + member end # Helper to create a cycle @@ -76,10 +74,14 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Manually assign fee type (this will trigger cycle generation) member = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type1.id - }) - |> Ash.update!(actor: actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: yearly_type1.id}, + actor: actor + ) + + updated + end) # Cycle generation runs synchronously in the same transaction # No need to wait for async completion @@ -140,11 +142,9 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Change membership fee type (same interval, different amount) assert {:ok, _updated_member} = - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type2.id - }) - |> Ash.update(actor: actor) + Mv.Membership.update_member(member, %{membership_fee_type_id: yearly_type2.id}, + actor: actor + ) # Cycle regeneration runs synchronously in the same transaction # No need to wait for async completion @@ -194,10 +194,14 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Manually assign fee type (this will trigger cycle generation) member = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type1.id - }) - |> Ash.update!(actor: actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: yearly_type1.id}, + actor: actor + ) + + updated + end) # Cycle generation runs synchronously in the same transaction # No need to wait for async completion @@ -215,11 +219,9 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Change membership fee type assert {:ok, _updated_member} = - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type2.id - }) - |> Ash.update(actor: actor) + Mv.Membership.update_member(member, %{membership_fee_type_id: yearly_type2.id}, + actor: actor + ) # Cycle regeneration runs synchronously in the same transaction # No need to wait for async completion @@ -242,10 +244,14 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Manually assign fee type (this will trigger cycle generation) member = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type1.id - }) - |> Ash.update!(actor: actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: yearly_type1.id}, + actor: actor + ) + + updated + end) # Cycle generation runs synchronously in the same transaction # No need to wait for async completion @@ -263,11 +269,9 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Change membership fee type assert {:ok, _updated_member} = - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type2.id - }) - |> Ash.update(actor: actor) + Mv.Membership.update_member(member, %{membership_fee_type_id: yearly_type2.id}, + actor: actor + ) # Cycle regeneration runs synchronously in the same transaction # No need to wait for async completion @@ -290,10 +294,14 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Manually assign fee type (this will trigger cycle generation) member = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type1.id - }) - |> Ash.update!(actor: actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: yearly_type1.id}, + actor: actor + ) + + updated + end) # Cycle generation runs synchronously in the same transaction # No need to wait for async completion @@ -357,11 +365,9 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Change membership fee type assert {:ok, _updated_member} = - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type2.id - }) - |> Ash.update(actor: actor) + Mv.Membership.update_member(member, %{membership_fee_type_id: yearly_type2.id}, + actor: actor + ) # Cycle regeneration runs synchronously in the same transaction # No need to wait for async completion @@ -406,10 +412,14 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Manually assign fee type (this will trigger cycle generation) member = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type1.id - }) - |> Ash.update!(actor: actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: yearly_type1.id}, + actor: actor + ) + + updated + end) # Cycle generation runs synchronously in the same transaction # No need to wait for async completion @@ -463,11 +473,9 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do # Change membership fee type assert {:ok, updated_member} = - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type2.id - }) - |> Ash.update(actor: actor) + Mv.Membership.update_member(member, %{membership_fee_type_id: yearly_type2.id}, + actor: actor + ) # Cycle regeneration runs synchronously in the same transaction # No need to wait for async completion diff --git a/test/membership_fees/changes/set_membership_fee_start_date_test.exs b/test/membership_fees/changes/set_membership_fee_start_date_test.exs index 0f8bae9..1917fee 100644 --- a/test/membership_fees/changes/set_membership_fee_start_date_test.exs +++ b/test/membership_fees/changes/set_membership_fee_start_date_test.exs @@ -146,16 +146,17 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDateTest do |> Ash.create!(actor: actor) # Create member with join_date and fee type but no explicit start date - member = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2024-03-15], - membership_fee_type_id: fee_type.id - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2024-03-15], + membership_fee_type_id: fee_type.id + }, + actor: actor + ) # Should have auto-calculated start date (2024-01-01 for yearly with include_joining_cycle=true) assert member.membership_fee_start_date == ~D[2024-01-01] @@ -177,17 +178,18 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDateTest do # Create member with explicit start date manual_start_date = ~D[2024-07-01] - member = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2024-03-15], - membership_fee_type_id: fee_type.id, - membership_fee_start_date: manual_start_date - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2024-03-15], + membership_fee_type_id: fee_type.id, + membership_fee_start_date: manual_start_date + }, + actor: actor + ) # Should keep the manually set date assert member.membership_fee_start_date == manual_start_date @@ -207,16 +209,17 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDateTest do |> Ash.create!(actor: actor) # Create member - member = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2024-03-15], - membership_fee_type_id: fee_type.id - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2024-03-15], + membership_fee_type_id: fee_type.id + }, + actor: actor + ) # Should have next cycle start date (2025-01-01 for yearly with include_joining_cycle=false) assert member.membership_fee_start_date == ~D[2025-01-01] @@ -236,16 +239,17 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDateTest do |> Ash.create!(actor: actor) # Create member without join_date - member = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - membership_fee_type_id: fee_type.id - # No join_date - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + membership_fee_type_id: fee_type.id + # No join_date + }, + actor: actor + ) # Should not have auto-calculated start date assert is_nil(member.membership_fee_start_date) @@ -255,16 +259,17 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDateTest do setup_settings(true, actor) # Create member without fee type - member = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2024-03-15] - # No membership_fee_type_id - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2024-03-15] + # No membership_fee_type_id + }, + actor: actor + ) # Should not have auto-calculated start date assert is_nil(member.membership_fee_start_date) diff --git a/test/membership_fees/changes/validate_same_interval_test.exs b/test/membership_fees/changes/validate_same_interval_test.exs index 82fbd6b..21287dd 100644 --- a/test/membership_fees/changes/validate_same_interval_test.exs +++ b/test/membership_fees/changes/validate_same_interval_test.exs @@ -4,7 +4,6 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do """ use Mv.DataCase, async: true - alias Mv.Membership.Member alias Mv.MembershipFees.MembershipFeeType alias Mv.MembershipFees.Changes.ValidateSameInterval @@ -37,10 +36,8 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: actor) + member end describe "validate_interval_match/1" do @@ -52,9 +49,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do changeset = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type2.id - }) + |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: yearly_type2.id}, + actor: actor + ) |> ValidateSameInterval.change(%{}, %{}) assert changeset.valid? @@ -68,9 +65,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do changeset = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: monthly_type.id - }) + |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: monthly_type.id}, + actor: actor + ) |> ValidateSameInterval.change(%{}, %{}) refute changeset.valid? @@ -90,9 +87,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do changeset = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type.id - }) + |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: yearly_type.id}, + actor: actor + ) |> ValidateSameInterval.change(%{}, %{}) assert changeset.valid? @@ -104,9 +101,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do changeset = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: nil - }) + |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: nil}, actor: actor) |> ValidateSameInterval.change(%{}, %{}) refute changeset.valid? @@ -124,9 +119,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do changeset = member - |> Ash.Changeset.for_update(:update_member, %{ - first_name: "New Name" - }) + |> Ash.Changeset.for_update(:update_member, %{first_name: "New Name"}, actor: actor) |> ValidateSameInterval.change(%{}, %{}) assert changeset.valid? @@ -140,9 +133,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do changeset = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: quarterly_type.id - }) + |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: quarterly_type.id}, + actor: actor + ) |> ValidateSameInterval.change(%{}, %{}) error = Enum.find(changeset.errors, &(&1.field == :membership_fee_type_id)) @@ -179,9 +172,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do changeset = member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: type2.id - }) + |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: type2.id}, + actor: actor + ) |> ValidateSameInterval.change(%{}, %{}) refute changeset.valid?, @@ -199,11 +192,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do # Try to update member with different interval type assert {:error, %Ash.Error.Invalid{} = error} = - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: monthly_type.id - }) - |> Ash.update(actor: actor) + Mv.Membership.update_member(member, %{membership_fee_type_id: monthly_type.id}, + actor: actor + ) # Check that error is about interval mismatch error_message = extract_error_message(error) @@ -220,11 +211,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do # Update member with same-interval type assert {:ok, updated_member} = - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: yearly_type2.id - }) - |> Ash.update(actor: actor) + Mv.Membership.update_member(member, %{membership_fee_type_id: yearly_type2.id}, + actor: actor + ) assert updated_member.membership_fee_type_id == yearly_type2.id end diff --git a/test/membership_fees/member_cycle_integration_test.exs b/test/membership_fees/member_cycle_integration_test.exs index 6d5bc2e..761d249 100644 --- a/test/membership_fees/member_cycle_integration_test.exs +++ b/test/membership_fees/member_cycle_integration_test.exs @@ -52,16 +52,17 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do setup_settings(true, actor) fee_type = create_fee_type(%{interval: :yearly}, actor) - member = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2023-03-15], - membership_fee_type_id: fee_type.id - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2023-03-15], + membership_fee_type_id: fee_type.id + }, + actor: actor + ) cycles = get_member_cycles(member.id, actor) @@ -80,16 +81,16 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do test "does not create cycles when member has no fee type", %{actor: actor} do setup_settings(true, actor) - member = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2023-03-15] - # No membership_fee_type_id - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2023-03-15] + }, + actor: actor + ) cycles = get_member_cycles(member.id, actor) @@ -100,16 +101,16 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do setup_settings(true, actor) fee_type = create_fee_type(%{interval: :yearly}, actor) - member = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - membership_fee_type_id: fee_type.id - # No join_date - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + membership_fee_type_id: fee_type.id + }, + actor: actor + ) cycles = get_member_cycles(member.id, actor) @@ -123,23 +124,23 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do fee_type = create_fee_type(%{interval: :yearly}, actor) # Create member without fee type - member = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2023-03-15] - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2023-03-15] + }, + actor: actor + ) # Verify no cycles yet assert get_member_cycles(member.id, actor) == [] # Update to assign fee type - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + {:ok, member} = + Mv.Membership.update_member(member, %{membership_fee_type_id: fee_type.id}, actor: actor) cycles = get_member_cycles(member.id, actor) @@ -157,15 +158,19 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do tasks = Enum.map(1..5, fn i -> Task.async(fn -> - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test#{i}", - last_name: "User#{i}", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2023-03-15], - membership_fee_type_id: fee_type.id - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test#{i}", + last_name: "User#{i}", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2023-03-15], + membership_fee_type_id: fee_type.id + }, + actor: actor + ) + + member end) end) @@ -184,16 +189,17 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do setup_settings(true, actor) fee_type = create_fee_type(%{interval: :yearly}, actor) - member = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2023-03-15], - membership_fee_type_id: fee_type.id - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2023-03-15], + membership_fee_type_id: fee_type.id + }, + actor: actor + ) initial_cycles = get_member_cycles(member.id, actor) initial_count = length(initial_cycles) diff --git a/test/membership_fees/membership_fee_cycle_test.exs b/test/membership_fees/membership_fee_cycle_test.exs index 46d6216..2fdd009 100644 --- a/test/membership_fees/membership_fee_cycle_test.exs +++ b/test/membership_fees/membership_fee_cycle_test.exs @@ -37,10 +37,8 @@ defmodule Mv.MembershipFees.MembershipFeeCycleTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: actor) + member end # Helper to create a cycle diff --git a/test/mv/membership/custom_field_policies_test.exs b/test/mv/membership/custom_field_policies_test.exs new file mode 100644 index 0000000..1e758d1 --- /dev/null +++ b/test/mv/membership/custom_field_policies_test.exs @@ -0,0 +1,188 @@ +defmodule Mv.Membership.CustomFieldPoliciesTest do + @moduledoc """ + Tests for CustomField resource authorization policies. + + Verifies that all authenticated users with a valid role can read custom fields, + and only admin can create/update/destroy custom fields. + """ + use Mv.DataCase, async: false + + alias Mv.Membership.CustomField + alias Mv.Accounts + alias Mv.Authorization + + setup do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + %{actor: system_actor} + end + + defp create_role_with_permission_set(permission_set_name, actor) do + role_name = "Test Role #{permission_set_name} #{System.unique_integer([:positive])}" + + case Authorization.create_role( + %{ + name: role_name, + description: "Test role for #{permission_set_name}", + permission_set_name: permission_set_name + }, + actor: actor + ) do + {:ok, role} -> role + {:error, error} -> raise "Failed to create role: #{inspect(error)}" + end + end + + defp create_user_with_permission_set(permission_set_name, actor) do + role = create_role_with_permission_set(permission_set_name, actor) + + {:ok, user} = + Accounts.User + |> Ash.Changeset.for_create(:register_with_password, %{ + email: "user#{System.unique_integer([:positive])}@example.com", + password: "testpassword123" + }) + |> Ash.create(actor: actor) + + {:ok, user} = + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, role, type: :append_and_remove) + |> Ash.update(actor: actor) + + {:ok, user_with_role} = Ash.load(user, :role, domain: Mv.Accounts, actor: actor) + user_with_role + end + + defp create_custom_field(actor) do + {:ok, field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "test_field_#{System.unique_integer([:positive])}", + value_type: :string + }) + |> Ash.create(actor: actor, domain: Mv.Membership) + + field + end + + describe "read access (all roles)" do + test "user with own_data can read all custom fields", %{actor: actor} do + custom_field = create_custom_field(actor) + user = create_user_with_permission_set("own_data", actor) + + {:ok, fields} = Ash.read(CustomField, actor: user, domain: Mv.Membership) + ids = Enum.map(fields, & &1.id) + assert custom_field.id in ids + + {:ok, fetched} = Ash.get(CustomField, custom_field.id, actor: user, domain: Mv.Membership) + assert fetched.id == custom_field.id + end + + test "user with read_only can read all custom fields", %{actor: actor} do + custom_field = create_custom_field(actor) + user = create_user_with_permission_set("read_only", actor) + + {:ok, fields} = Ash.read(CustomField, actor: user, domain: Mv.Membership) + ids = Enum.map(fields, & &1.id) + assert custom_field.id in ids + + {:ok, fetched} = Ash.get(CustomField, custom_field.id, actor: user, domain: Mv.Membership) + assert fetched.id == custom_field.id + end + + test "user with normal_user can read all custom fields", %{actor: actor} do + custom_field = create_custom_field(actor) + user = create_user_with_permission_set("normal_user", actor) + + {:ok, fields} = Ash.read(CustomField, actor: user, domain: Mv.Membership) + ids = Enum.map(fields, & &1.id) + assert custom_field.id in ids + + {:ok, fetched} = Ash.get(CustomField, custom_field.id, actor: user, domain: Mv.Membership) + assert fetched.id == custom_field.id + end + + test "user with admin can read all custom fields", %{actor: actor} do + custom_field = create_custom_field(actor) + user = create_user_with_permission_set("admin", actor) + + {:ok, fields} = Ash.read(CustomField, actor: user, domain: Mv.Membership) + ids = Enum.map(fields, & &1.id) + assert custom_field.id in ids + + {:ok, fetched} = Ash.get(CustomField, custom_field.id, actor: user, domain: Mv.Membership) + assert fetched.id == custom_field.id + end + end + + describe "write access - non-admin cannot create/update/destroy" do + setup %{actor: actor} do + user = create_user_with_permission_set("normal_user", actor) + custom_field = create_custom_field(actor) + %{user: user, custom_field: custom_field} + end + + test "non-admin cannot create custom field (forbidden)", %{user: user} do + assert {:error, %Ash.Error.Forbidden{}} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "forbidden_field_#{System.unique_integer([:positive])}", + value_type: :string + }) + |> Ash.create(actor: user, domain: Mv.Membership) + end + + test "non-admin cannot update custom field (forbidden)", %{ + user: user, + custom_field: custom_field + } do + assert {:error, %Ash.Error.Forbidden{}} = + custom_field + |> Ash.Changeset.for_update(:update, %{description: "Updated"}) + |> Ash.update(actor: user, domain: Mv.Membership) + end + + test "non-admin cannot destroy custom field (forbidden)", %{ + user: user, + custom_field: custom_field + } do + assert {:error, %Ash.Error.Forbidden{}} = + Ash.destroy(custom_field, actor: user, domain: Mv.Membership) + end + end + + describe "write access - admin can create/update/destroy" do + setup %{actor: actor} do + user = create_user_with_permission_set("admin", actor) + custom_field = create_custom_field(actor) + %{user: user, custom_field: custom_field} + end + + test "admin can create custom field", %{user: user} do + name = "admin_field_#{System.unique_integer([:positive])}" + + assert {:ok, %CustomField{} = field} = + CustomField + |> Ash.Changeset.for_create(:create, %{name: name, value_type: :string}) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert field.name == name + end + + test "admin can update custom field", %{user: user, custom_field: custom_field} do + assert {:ok, updated} = + custom_field + |> Ash.Changeset.for_update(:update, %{description: "Admin updated"}) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert updated.description == "Admin updated" + end + + test "admin can destroy custom field", %{user: user, custom_field: custom_field} do + assert :ok = Ash.destroy(custom_field, actor: user, domain: Mv.Membership) + + assert {:error, _} = + Ash.get(CustomField, custom_field.id, domain: Mv.Membership, actor: user) + end + end +end diff --git a/test/mv/membership/custom_field_value_policies_test.exs b/test/mv/membership/custom_field_value_policies_test.exs index d400aec..72b6af6 100644 --- a/test/mv/membership/custom_field_value_policies_test.exs +++ b/test/mv/membership/custom_field_value_policies_test.exs @@ -9,7 +9,7 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do # async: false because we need database commits to be visible across queries use Mv.DataCase, async: false - alias Mv.Membership.{CustomField, CustomFieldValue, Member} + alias Mv.Membership.{CustomField, CustomFieldValue} alias Mv.Accounts alias Mv.Authorization @@ -62,13 +62,14 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do defp create_linked_member_for_user(user, actor) do {:ok, member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Linked", - last_name: "Member", - email: "linked#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: actor, return_notifications?: false) + Mv.Membership.create_member( + %{ + first_name: "Linked", + last_name: "Member", + email: "linked#{System.unique_integer([:positive])}@example.com" + }, + actor: actor + ) user |> Ash.Changeset.for_update(:update, %{}) @@ -80,13 +81,14 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do defp create_unlinked_member(actor) do {:ok, member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Unlinked", - last_name: "Member", - email: "unlinked#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: actor) + Mv.Membership.create_member( + %{ + first_name: "Unlinked", + last_name: "Member", + email: "unlinked#{System.unique_integer([:positive])}@example.com" + }, + actor: actor + ) member end diff --git a/test/mv/membership/member_policies_test.exs b/test/mv/membership/member_policies_test.exs index 0bbe1c1..026c3c4 100644 --- a/test/mv/membership/member_policies_test.exs +++ b/test/mv/membership/member_policies_test.exs @@ -78,13 +78,14 @@ defmodule Mv.Membership.MemberPoliciesTest do # NOTE: We need to ensure the member is actually persisted to the database # before we try to link it. Ash may delay writes, so we explicitly return the struct. {:ok, member} = - Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Linked", - last_name: "Member", - email: "linked#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: admin, return_notifications?: false) + Membership.create_member( + %{ + first_name: "Linked", + last_name: "Member", + email: "linked#{System.unique_integer([:positive])}@example.com" + }, + actor: admin + ) # Link member to user (User.member_id = member.id) # We use force_change_attribute because the member already exists and we just @@ -108,13 +109,14 @@ defmodule Mv.Membership.MemberPoliciesTest do admin = create_admin_user(actor) {:ok, member} = - Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Unlinked", - last_name: "Member", - email: "unlinked#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: admin) + Membership.create_member( + %{ + first_name: "Unlinked", + last_name: "Member", + email: "unlinked#{System.unique_integer([:positive])}@example.com" + }, + actor: admin + ) member end @@ -145,9 +147,7 @@ defmodule Mv.Membership.MemberPoliciesTest do # Update is allowed via HasPermission check with :linked scope (not via special case) # The special case policy only applies to :read actions {:ok, updated_member} = - linked_member - |> Ash.Changeset.for_update(:update_member, %{first_name: "Updated"}) - |> Ash.update(actor: user) + Membership.update_member(linked_member, %{first_name: "Updated"}, actor: user) assert updated_member.first_name == "Updated" end @@ -168,11 +168,8 @@ defmodule Mv.Membership.MemberPoliciesTest do user: user, unlinked_member: unlinked_member } do - assert_raise Ash.Error.Forbidden, fn -> - unlinked_member - |> Ash.Changeset.for_update(:update_member, %{first_name: "Updated"}) - |> Ash.update!(actor: user) - end + assert {:error, %Ash.Error.Forbidden{}} = + Membership.update_member(unlinked_member, %{first_name: "Updated"}, actor: user) end test "list members returns only linked member", %{ @@ -187,15 +184,15 @@ defmodule Mv.Membership.MemberPoliciesTest do end test "cannot create member (returns forbidden)", %{user: user} do - assert_raise Ash.Error.Forbidden, fn -> - Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "New", - last_name: "Member", - email: "new#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create!(actor: user) - end + assert {:error, %Ash.Error.Forbidden{}} = + Membership.create_member( + %{ + first_name: "New", + last_name: "Member", + email: "new#{System.unique_integer([:positive])}@example.com" + }, + actor: user + ) end test "cannot destroy member (returns forbidden)", %{ @@ -245,26 +242,23 @@ defmodule Mv.Membership.MemberPoliciesTest do end test "cannot create member (returns forbidden)", %{user: user} do - assert_raise Ash.Error.Forbidden, fn -> - Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "New", - last_name: "Member", - email: "new#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create!(actor: user) - end + assert {:error, %Ash.Error.Forbidden{}} = + Membership.create_member( + %{ + first_name: "New", + last_name: "Member", + email: "new#{System.unique_integer([:positive])}@example.com" + }, + actor: user + ) end test "cannot update any member (returns forbidden)", %{ user: user, linked_member: linked_member } do - assert_raise Ash.Error.Forbidden, fn -> - linked_member - |> Ash.Changeset.for_update(:update_member, %{first_name: "Updated"}) - |> Ash.update!(actor: user) - end + assert {:error, %Ash.Error.Forbidden{}} = + Membership.update_member(linked_member, %{first_name: "Updated"}, actor: user) end test "cannot destroy any member (returns forbidden)", %{ @@ -305,22 +299,21 @@ defmodule Mv.Membership.MemberPoliciesTest do test "can create member", %{user: user} do {:ok, member} = - Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "New", - last_name: "Member", - email: "new#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: user) + Membership.create_member( + %{ + first_name: "New", + last_name: "Member", + email: "new#{System.unique_integer([:positive])}@example.com" + }, + actor: user + ) assert member.first_name == "New" end test "can update any member", %{user: user, unlinked_member: unlinked_member} do {:ok, updated_member} = - unlinked_member - |> Ash.Changeset.for_update(:update_member, %{first_name: "Updated"}) - |> Ash.update(actor: user) + Membership.update_member(unlinked_member, %{first_name: "Updated"}, actor: user) assert updated_member.first_name == "Updated" end @@ -363,22 +356,21 @@ defmodule Mv.Membership.MemberPoliciesTest do test "can create member", %{user: user} do {:ok, member} = - Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "New", - last_name: "Member", - email: "new#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: user) + Membership.create_member( + %{ + first_name: "New", + last_name: "Member", + email: "new#{System.unique_integer([:positive])}@example.com" + }, + actor: user + ) assert member.first_name == "New" end test "can update any member", %{user: user, unlinked_member: unlinked_member} do {:ok, updated_member} = - unlinked_member - |> Ash.Changeset.for_update(:update_member, %{first_name: "Updated"}) - |> Ash.update(actor: user) + Membership.update_member(unlinked_member, %{first_name: "Updated"}, actor: user) assert updated_member.first_name == "Updated" end @@ -456,9 +448,7 @@ defmodule Mv.Membership.MemberPoliciesTest do # Should succeed via HasPermission check (not special case) {:ok, updated_member} = - linked_member - |> Ash.Changeset.for_update(:update_member, %{first_name: "Updated"}) - |> Ash.update(actor: user) + Membership.update_member(linked_member, %{first_name: "Updated"}, actor: user) assert updated_member.first_name == "Updated" end diff --git a/test/mv/membership_fees/cycle_generator_edge_cases_test.exs b/test/mv/membership_fees/cycle_generator_edge_cases_test.exs index d4899a3..fbf1740 100644 --- a/test/mv/membership_fees/cycle_generator_edge_cases_test.exs +++ b/test/mv/membership_fees/cycle_generator_edge_cases_test.exs @@ -49,10 +49,8 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: actor) + member end # Helper to create a member and explicitly generate cycles with a fixed "today" date. @@ -74,9 +72,12 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do # Assign fee type if provided (this will trigger auto-generation with real today) member = if fee_type_id do - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type_id}) - |> Ash.update!(actor: actor) + {:ok, updated} = + Mv.Membership.update_member(member, %{membership_fee_type_id: fee_type_id}, + actor: actor + ) + + updated else member end @@ -130,10 +131,8 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do ) # Assign fee type - member = - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + {:ok, member} = + Mv.Membership.update_member(member, %{membership_fee_type_id: fee_type.id}, actor: actor) # Explicitly generate cycles with fixed "today" date {:ok, _, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today) @@ -163,10 +162,8 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do ) # Assign fee type - member = - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + {:ok, member} = + Mv.Membership.update_member(member, %{membership_fee_type_id: fee_type.id}, actor: actor) # Explicitly generate cycles with fixed "today" date {:ok, _, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today) @@ -349,10 +346,8 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do |> Ash.create!(actor: actor) # Now assign fee type - member = - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + {:ok, member} = + Mv.Membership.update_member(member, %{membership_fee_type_id: fee_type.id}, actor: actor) # Explicitly generate cycles with fixed "today" date today = ~D[2024-06-15] @@ -616,13 +611,15 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do ) # Now assign fee type (simulating a retroactive assignment) - member = - member - |> Ash.Changeset.for_update(:update_member, %{ - membership_fee_type_id: fee_type.id, - membership_fee_start_date: ~D[2021-01-01] - }) - |> Ash.update!(actor: actor) + {:ok, member} = + Mv.Membership.update_member( + member, + %{ + membership_fee_type_id: fee_type.id, + membership_fee_start_date: ~D[2021-01-01] + }, + actor: actor + ) # Run batch generation with a "today" date after the member left today = ~D[2024-06-15] diff --git a/test/mv/membership_fees/cycle_generator_test.exs b/test/mv/membership_fees/cycle_generator_test.exs index 1863312..9c1fd60 100644 --- a/test/mv/membership_fees/cycle_generator_test.exs +++ b/test/mv/membership_fees/cycle_generator_test.exs @@ -40,10 +40,8 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: actor) + member end # Helper to set up settings with specific include_joining_cycle value @@ -81,8 +79,12 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do # Assign fee type member = member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: fee_type.id}, actor: actor) + + updated + end) # Explicitly generate cycles with fixed "today" date to avoid date dependency today = ~D[2024-06-15] @@ -128,8 +130,12 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do # Now assign fee type to member member = member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: fee_type.id}, actor: actor) + + updated + end) # Generate cycles with specific "today" date today = ~D[2024-06-15] @@ -237,8 +243,12 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do # start from the last existing cycle (2023) and go forward member = member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: fee_type.id}, actor: actor) + + updated + end) # Verify gap was NOT filled and new cycles were generated from last existing all_cycles = get_member_cycles(member.id, actor) diff --git a/test/mv_web/components/member_filter_component_test.exs b/test/mv_web/components/member_filter_component_test.exs index a3a3846..485475a 100644 --- a/test/mv_web/components/member_filter_component_test.exs +++ b/test/mv_web/components/member_filter_component_test.exs @@ -16,8 +16,10 @@ defmodule MvWeb.Components.MemberFilterComponentTest do alias Mv.Membership.CustomField - # Helper to create a boolean custom field + # Helper to create a boolean custom field (uses system_actor - only admin can create) defp create_boolean_custom_field(attrs \\ %{}) do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + default_attrs = %{ name: "test_boolean_#{System.unique_integer([:positive])}", value_type: :boolean @@ -27,11 +29,13 @@ defmodule MvWeb.Components.MemberFilterComponentTest do CustomField |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!() + |> Ash.create!(actor: system_actor) end - # Helper to create a non-boolean custom field + # Helper to create a non-boolean custom field (uses system_actor - only admin can create) defp create_string_custom_field(attrs \\ %{}) do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + default_attrs = %{ name: "test_string_#{System.unique_integer([:positive])}", value_type: :string @@ -41,7 +45,7 @@ defmodule MvWeb.Components.MemberFilterComponentTest do CustomField |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!() + |> Ash.create!(actor: system_actor) end describe "rendering" do diff --git a/test/mv_web/helpers/membership_fee_helpers_test.exs b/test/mv_web/helpers/membership_fee_helpers_test.exs index d5b0571..7f9afaf 100644 --- a/test/mv_web/helpers/membership_fee_helpers_test.exs +++ b/test/mv_web/helpers/membership_fee_helpers_test.exs @@ -80,21 +80,20 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do |> Ash.create!(actor: actor) # Create member without fee type first to avoid auto-generation - member = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2022-01-01] - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2022-01-01] + }, + actor: actor + ) # Assign fee type after member creation (this may generate cycles, but we'll create our own) - member = - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + {:ok, member} = + Mv.Membership.update_member(member, %{membership_fee_type_id: fee_type.id}, actor: actor) # Delete any auto-generated cycles first cycles = @@ -151,20 +150,19 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do |> Ash.create!(actor: actor) # Create member without fee type first - member = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test#{System.unique_integer([:positive])}@example.com" + }, + actor: actor + ) # Assign fee type - member = - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + {:ok, member} = + Mv.Membership.update_member(member, %{membership_fee_type_id: fee_type.id}, actor: actor) # Delete any auto-generated cycles cycles = @@ -197,21 +195,20 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do |> Ash.create!(actor: actor) # Create member without fee type first - member = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test#{System.unique_integer([:positive])}@example.com", - join_date: ~D[2023-01-01] - }) - |> Ash.create!(actor: actor) + {:ok, member} = + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test#{System.unique_integer([:positive])}@example.com", + join_date: ~D[2023-01-01] + }, + actor: actor + ) # Assign fee type - member = - member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: actor) + {:ok, member} = + Mv.Membership.update_member(member, %{membership_fee_type_id: fee_type.id}, actor: actor) # Delete any auto-generated cycles cycles = diff --git a/test/mv_web/live/custom_field_live/deletion_test.exs b/test/mv_web/live/custom_field_live/deletion_test.exs index 9610b24..36e46e2 100644 --- a/test/mv_web/live/custom_field_live/deletion_test.exs +++ b/test/mv_web/live/custom_field_live/deletion_test.exs @@ -15,13 +15,15 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do use MvWeb.ConnCase, async: true import Phoenix.LiveViewTest + require Ash.Query alias Mv.Membership.{CustomField, CustomFieldValue, Member} setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() + admin_role = Mv.Fixtures.role_fixture("admin") - # Create admin user for testing + # Create admin user for testing (must have admin role to read/manage CustomField) {:ok, user} = Mv.Accounts.User |> Ash.Changeset.for_create(:register_with_password, %{ @@ -30,8 +32,18 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do }) |> Ash.create(actor: system_actor) - conn = log_in_user(build_conn(), user) - %{conn: conn, user: user} + {:ok, user} = + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update(actor: system_actor) + + user_with_role = Ash.load!(user, :role, domain: Mv.Accounts, actor: system_actor) + conn = log_in_user(build_conn(), user_with_role) + # Use English locale so "Delete" link text matches in assertions + session = conn.private[:plug_session] || %{} + conn = Plug.Test.init_test_session(conn, Map.put(session, "locale", "en")) + %{conn: conn, user: user_with_role} end describe "delete button and modal" do @@ -107,9 +119,8 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do |> element("#delete-custom-field-modal form") |> render_change(%{"slug" => custom_field.slug}) - # Confirm button should be enabled now (no disabled attribute) - html = render(view) - refute html =~ ~r/disabled(?:=""|(?!\w))/ + # Confirm button should be enabled now (no disabled attribute on the confirm button) + refute has_element?(view, "#delete-custom-field-modal button.btn-error[disabled]") end test "delete button is disabled when slug doesn't match", %{conn: conn} do @@ -126,9 +137,8 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do |> element("#delete-custom-field-modal form") |> render_change(%{"slug" => "wrong-slug"}) - # Button should be disabled - html = render(view) - assert html =~ ~r/disabled(?:=""|(?!\w))/ + # Confirm button should be disabled + assert has_element?(view, "#delete-custom-field-modal button.btn-error[disabled]") end end @@ -186,10 +196,8 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do |> element("#delete-custom-field-modal form") |> render_change(%{"slug" => "wrong-slug"}) - # Button should be disabled and we cannot click it - # The test verifies that the button is properly disabled in the UI - html = render(view) - assert html =~ ~r/disabled(?:=""|(?!\w))/ + # Confirm button should be disabled and we cannot click it + assert has_element?(view, "#delete-custom-field-modal button.btn-error[disabled]") # Custom field should still exist since deletion couldn't proceed system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -224,17 +232,58 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do end end - # Helper functions + describe "create custom field" do + test "submitting new data field form creates custom field and shows success", %{conn: conn} do + {:ok, view, _html} = live(conn, ~p"/settings") + + # Open "New Data Field" form + view + |> element("#custom-fields-component button", "New Data Field") + |> render_click() + + # Form is visible; submit with valid data + form_params = %{ + "custom_field" => %{ + "name" => "Created via Form", + "value_type" => "string", + "description" => "", + "required" => "false", + "show_in_overview" => "true" + } + } + + view + |> form("#custom-field-form-new-form", form_params) + |> render_submit() + + # Success flash (FormComponent needs actor from parent; without it KeyError would occur) + assert render(view) =~ "successfully" + + # Custom field was created in DB + system_actor = Mv.Helpers.SystemActor.get_system_actor() + search_name = "Created via Form" + + [custom_field] = + Mv.Membership.CustomField + |> Ash.Query.filter(name == ^search_name) + |> Ash.read!(actor: system_actor) + + assert custom_field.value_type == :string + end + end + + # Helper functions (use code interface so actor is in validation context) defp create_member do system_actor = Mv.Helpers.SystemActor.get_system_actor() - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "User#{System.unique_integer([:positive])}", - email: "test#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "User#{System.unique_integer([:positive])}", + email: "test#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) end defp create_custom_field(name, value_type) do diff --git a/test/mv_web/live/membership_fee_type_live/form_test.exs b/test/mv_web/live/membership_fee_type_live/form_test.exs index 9398403..0902200 100644 --- a/test/mv_web/live/membership_fee_type_live/form_test.exs +++ b/test/mv_web/live/membership_fee_type_live/form_test.exs @@ -7,7 +7,6 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do import Phoenix.LiveViewTest alias Mv.MembershipFees.MembershipFeeType - alias Mv.Membership.Member require Ash.Query @@ -55,10 +54,8 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: system_actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: system_actor) + member end describe "create form" do diff --git a/test/mv_web/live/membership_fee_type_live/index_test.exs b/test/mv_web/live/membership_fee_type_live/index_test.exs index 302814d..38c81fb 100644 --- a/test/mv_web/live/membership_fee_type_live/index_test.exs +++ b/test/mv_web/live/membership_fee_type_live/index_test.exs @@ -7,7 +7,6 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do import Phoenix.LiveViewTest alias Mv.MembershipFees.MembershipFeeType - alias Mv.Membership.Member require Ash.Query @@ -31,7 +30,7 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do end # Helper to create a member - # Uses admin actor from global setup to ensure authorization + # Uses admin actor from global setup to ensure authorization; falls back to system_actor when nil defp create_member(attrs, actor) do default_attrs = %{ first_name: "Test", @@ -40,12 +39,9 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do } attrs = Map.merge(default_attrs, attrs) - - opts = if actor, do: [actor: actor], else: [] - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(opts) + effective_actor = actor || Mv.Helpers.SystemActor.get_system_actor() + {:ok, member} = Mv.Membership.create_member(attrs, actor: effective_actor) + member end describe "list display" do diff --git a/test/mv_web/live/user_live/show_test.exs b/test/mv_web/live/user_live/show_test.exs index fbd0407..8f7ea93 100644 --- a/test/mv_web/live/user_live/show_test.exs +++ b/test/mv_web/live/user_live/show_test.exs @@ -64,13 +64,10 @@ defmodule MvWeb.UserLive.ShowTest do # Create member {:ok, member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Smith", - email: "alice@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Alice", last_name: "Smith", email: "alice@example.com"}, + actor: system_actor + ) # Create user and link to member user = create_test_user(%{email: "user@example.com"}) diff --git a/test/mv_web/member_live/form_error_handling_test.exs b/test/mv_web/member_live/form_error_handling_test.exs index 07a3cfe..b2bf804 100644 --- a/test/mv_web/member_live/form_error_handling_test.exs +++ b/test/mv_web/member_live/form_error_handling_test.exs @@ -6,8 +6,6 @@ defmodule MvWeb.MemberLive.FormErrorHandlingTest do import Phoenix.LiveViewTest - alias Mv.Membership.Member - require Ash.Query describe "error handling - flash messages" do @@ -16,13 +14,14 @@ defmodule MvWeb.MemberLive.FormErrorHandlingTest do # Create a member with the same email to trigger uniqueness error {:ok, _existing_member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Existing", - last_name: "Member", - email: "duplicate@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Existing", + last_name: "Member", + email: "duplicate@example.com" + }, + actor: system_actor + ) conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members/new") @@ -79,23 +78,21 @@ defmodule MvWeb.MemberLive.FormErrorHandlingTest do # Create a member to edit {:ok, member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Original", - last_name: "Member", - email: "original@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Original", + last_name: "Member", + email: "original@example.com" + }, + actor: system_actor + ) # Create another member with different email {:ok, _other_member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Other", - last_name: "Member", - email: "other@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Other", last_name: "Member", email: "other@example.com"}, + actor: system_actor + ) conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members/#{member.id}/edit") 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 911a4ce..5382dfc 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 @@ -6,7 +6,6 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do import Phoenix.LiveViewTest - alias Mv.Membership.Member alias Mv.MembershipFees.MembershipFeeType require Ash.Query @@ -37,10 +36,8 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: admin_user) + {:ok, member} = Mv.Membership.create_member(attrs, actor: admin_user) + member end describe "membership fee type dropdown" do @@ -129,7 +126,7 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do # Verify member was created with fee type - use admin_user to test permissions member = - Member + Mv.Membership.Member |> Ash.Query.filter(email == ^form_data["member[email]"]) |> Ash.read_one!(actor: admin_user) @@ -177,14 +174,15 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do # Create member with fee type 1 and custom field value member = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test#{System.unique_integer([:positive])}@example.com", - membership_fee_type_id: fee_type1.id - }) - |> Ash.create!(actor: admin_user) + create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test#{System.unique_integer([:positive])}@example.com", + membership_fee_type_id: fee_type1.id + }, + admin_user + ) # Add custom field value Mv.Membership.CustomFieldValue @@ -222,14 +220,15 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do # Create member with date custom field value member = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test#{System.unique_integer([:positive])}@example.com", - membership_fee_type_id: fee_type.id - }) - |> Ash.create!(actor: admin_user) + create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test#{System.unique_integer([:positive])}@example.com", + membership_fee_type_id: fee_type.id + }, + admin_user + ) test_date = ~D[2024-01-15] @@ -269,14 +268,15 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do # Create member with custom field value member = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test#{System.unique_integer([:positive])}@example.com", - membership_fee_type_id: fee_type.id - }) - |> Ash.create!(actor: admin_user) + create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test#{System.unique_integer([:positive])}@example.com", + membership_fee_type_id: fee_type.id + }, + admin_user + ) # Add custom field value _cfv = 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 331375e..950b65f 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 @@ -39,10 +39,8 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: system_actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: system_actor) + member end # Helper to create a cycle @@ -106,8 +104,14 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do # Assign fee type member = member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: system_actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: fee_type.id}, + actor: system_actor + ) + + updated + end) # Delete any auto-generated cycles cycles = @@ -151,8 +155,14 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do # Assign fee type member = member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: system_actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: fee_type.id}, + actor: system_actor + ) + + updated + end) # Delete any auto-generated cycles cycles = @@ -192,8 +202,14 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do # Assign fee type member = member - |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) - |> Ash.update!(actor: system_actor) + |> then(fn m -> + {:ok, updated} = + Mv.Membership.update_member(m, %{membership_fee_type_id: fee_type.id}, + actor: system_actor + ) + + updated + end) # Delete any auto-generated cycles cycles = diff --git a/test/mv_web/member_live/index_custom_fields_accessibility_test.exs b/test/mv_web/member_live/index_custom_fields_accessibility_test.exs index 571555e..17e6da4 100644 --- a/test/mv_web/member_live/index_custom_fields_accessibility_test.exs +++ b/test/mv_web/member_live/index_custom_fields_accessibility_test.exs @@ -11,20 +11,17 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsAccessibilityTest do import Phoenix.LiveViewTest require Ash.Query - alias Mv.Membership.{CustomField, CustomFieldValue, Member} + alias Mv.Membership.{CustomField, CustomFieldValue} setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() # Create test member {:ok, member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Alice", last_name: "Anderson", email: "alice@example.com"}, + actor: system_actor + ) # Create custom field with show_in_overview: true {:ok, field} = diff --git a/test/mv_web/member_live/index_custom_fields_display_test.exs b/test/mv_web/member_live/index_custom_fields_display_test.exs index 04d7a8a..9ada92b 100644 --- a/test/mv_web/member_live/index_custom_fields_display_test.exs +++ b/test/mv_web/member_live/index_custom_fields_display_test.exs @@ -14,29 +14,23 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsDisplayTest do import Phoenix.LiveViewTest require Ash.Query - alias Mv.Membership.{CustomField, CustomFieldValue, Member} + alias Mv.Membership.{CustomField, CustomFieldValue} setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() # Create test members {:ok, member1} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Alice", last_name: "Anderson", email: "alice@example.com"}, + actor: system_actor + ) {:ok, member2} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Bob", - last_name: "Brown", - email: "bob@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Bob", last_name: "Brown", email: "bob@example.com"}, + actor: system_actor + ) # Create custom fields {:ok, field_show_string} = diff --git a/test/mv_web/member_live/index_custom_fields_edge_cases_test.exs b/test/mv_web/member_live/index_custom_fields_edge_cases_test.exs index 7d0ee45..cf67fc3 100644 --- a/test/mv_web/member_live/index_custom_fields_edge_cases_test.exs +++ b/test/mv_web/member_live/index_custom_fields_edge_cases_test.exs @@ -10,7 +10,7 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsEdgeCasesTest do import Phoenix.LiveViewTest require Ash.Query - alias Mv.Membership.{CustomField, Member} + alias Mv.Membership.CustomField @tag :slow test "displays custom field column even when no members have values", %{conn: conn} do @@ -18,22 +18,16 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsEdgeCasesTest do # Create test members without custom field values {:ok, _member1} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Alice", last_name: "Anderson", email: "alice@example.com"}, + actor: system_actor + ) {:ok, _member2} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Bob", - last_name: "Brown", - email: "bob@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Bob", last_name: "Brown", email: "bob@example.com"}, + actor: system_actor + ) # Create custom field with show_in_overview: true but no values {:ok, field} = @@ -58,13 +52,10 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsEdgeCasesTest do # Create test member {:ok, member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Alice", last_name: "Anderson", email: "alice@example.com"}, + actor: system_actor + ) # Create custom field {:ok, field} = @@ -102,13 +93,10 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsEdgeCasesTest do # Create test member {:ok, member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Alice", last_name: "Anderson", email: "alice@example.com"}, + actor: system_actor + ) # Create multiple custom fields with show_in_overview: true {:ok, field1} = diff --git a/test/mv_web/member_live/index_custom_fields_sorting_test.exs b/test/mv_web/member_live/index_custom_fields_sorting_test.exs index 88f225f..99e15ea 100644 --- a/test/mv_web/member_live/index_custom_fields_sorting_test.exs +++ b/test/mv_web/member_live/index_custom_fields_sorting_test.exs @@ -13,38 +13,29 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsSortingTest do import Phoenix.LiveViewTest require Ash.Query - alias Mv.Membership.{CustomField, CustomFieldValue, Member} + alias Mv.Membership.{CustomField, CustomFieldValue} setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() # Create test members {:ok, member1} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Alice", last_name: "Anderson", email: "alice@example.com"}, + actor: system_actor + ) {:ok, member2} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Bob", - last_name: "Brown", - email: "bob@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Bob", last_name: "Brown", email: "bob@example.com"}, + actor: system_actor + ) {:ok, member3} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Charlie", - last_name: "Clark", - email: "charlie@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Charlie", last_name: "Clark", email: "charlie@example.com"}, + actor: system_actor + ) # Create custom field with show_in_overview: true {:ok, field_string} = @@ -242,40 +233,28 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsSortingTest do # Create additional members with NULL and empty string values {:ok, member_with_value} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "WithValue", - last_name: "Test", - email: "withvalue@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "WithValue", last_name: "Test", email: "withvalue@example.com"}, + actor: system_actor + ) {:ok, member_with_empty} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "WithEmpty", - last_name: "Test", - email: "withempty@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "WithEmpty", last_name: "Test", email: "withempty@example.com"}, + actor: system_actor + ) {:ok, member_with_null} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "WithNull", - last_name: "Test", - email: "withnull@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "WithNull", last_name: "Test", email: "withnull@example.com"}, + actor: system_actor + ) {:ok, member_with_another_value} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "AnotherValue", - last_name: "Test", - email: "another@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "AnotherValue", last_name: "Test", email: "another@example.com"}, + actor: system_actor + ) # Create custom field {:ok, field} = @@ -355,40 +334,28 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsSortingTest do # Create additional members with NULL and empty string values {:ok, member_with_value} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "WithValue", - last_name: "Test", - email: "withvalue@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "WithValue", last_name: "Test", email: "withvalue@example.com"}, + actor: system_actor + ) {:ok, member_with_empty} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "WithEmpty", - last_name: "Test", - email: "withempty@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "WithEmpty", last_name: "Test", email: "withempty@example.com"}, + actor: system_actor + ) {:ok, member_with_null} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "WithNull", - last_name: "Test", - email: "withnull@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "WithNull", last_name: "Test", email: "withnull@example.com"}, + actor: system_actor + ) {:ok, member_with_another_value} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "AnotherValue", - last_name: "Test", - email: "another@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "AnotherValue", last_name: "Test", email: "another@example.com"}, + actor: system_actor + ) # Create custom field {:ok, field} = diff --git a/test/mv_web/member_live/index_field_visibility_test.exs b/test/mv_web/member_live/index_field_visibility_test.exs index d471a23..8de9c7e 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -16,33 +16,35 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do import Phoenix.LiveViewTest require Ash.Query - alias Mv.Membership.{CustomField, CustomFieldValue, Member} + alias Mv.Membership.{CustomField, CustomFieldValue} setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() # Create test members {:ok, member1} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com", - street: "Main St", - city: "Berlin" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Alice", + last_name: "Anderson", + email: "alice@example.com", + street: "Main St", + city: "Berlin" + }, + actor: system_actor + ) {:ok, member2} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Bob", - last_name: "Brown", - email: "bob@example.com", - street: "Second St", - city: "Hamburg" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Bob", + last_name: "Brown", + email: "bob@example.com", + street: "Second St", + city: "Hamburg" + }, + actor: system_actor + ) # Create custom field {:ok, custom_field} = diff --git a/test/mv_web/member_live/index_member_fields_display_test.exs b/test/mv_web/member_live/index_member_fields_display_test.exs index 14c8374..fe33bb4 100644 --- a/test/mv_web/member_live/index_member_fields_display_test.exs +++ b/test/mv_web/member_live/index_member_fields_display_test.exs @@ -3,33 +3,29 @@ defmodule MvWeb.MemberLive.IndexMemberFieldsDisplayTest do import Phoenix.LiveViewTest require Ash.Query - alias Mv.Membership.Member - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() {:ok, member1} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com", - street: "Main Street", - house_number: "123", - postal_code: "12345", - city: "Berlin", - join_date: ~D[2020-01-15] - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Alice", + last_name: "Anderson", + email: "alice@example.com", + street: "Main Street", + house_number: "123", + postal_code: "12345", + city: "Berlin", + join_date: ~D[2020-01-15] + }, + actor: system_actor + ) {:ok, member2} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Bob", - last_name: "Brown", - email: "bob@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Bob", last_name: "Brown", email: "bob@example.com"}, + actor: system_actor + ) %{ member1: member1, 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 f868e96..bbd9159 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 @@ -6,7 +6,6 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do import Phoenix.LiveViewTest - alias Mv.Membership.Member alias Mv.MembershipFees.MembershipFeeType alias Mv.MembershipFees.MembershipFeeCycle @@ -40,10 +39,8 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: system_actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: system_actor) + member end # Helper to create a cycle diff --git a/test/mv_web/member_live/index_test.exs b/test/mv_web/member_live/index_test.exs index 388f70d..3234761 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -531,10 +531,8 @@ defmodule MvWeb.MemberLive.IndexTest do } attrs = Map.merge(default_attrs, attrs) - - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: actor) + member end test "filter shows only members with paid status in last cycle", %{conn: conn} do @@ -750,8 +748,10 @@ defmodule MvWeb.MemberLive.IndexTest do describe "boolean custom field filters" do alias Mv.Membership.CustomField - # Helper to create a boolean custom field + # Helper to create a boolean custom field (uses system actor for authorization) defp create_boolean_custom_field(attrs \\ %{}) do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + default_attrs = %{ name: "test_boolean_#{System.unique_integer([:positive])}", value_type: :boolean @@ -761,11 +761,13 @@ defmodule MvWeb.MemberLive.IndexTest do CustomField |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!() + |> Ash.create!(actor: system_actor) end - # Helper to create a non-boolean custom field + # Helper to create a non-boolean custom field (uses system actor for authorization) defp create_string_custom_field(attrs \\ %{}) do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + default_attrs = %{ name: "test_string_#{System.unique_integer([:positive])}", value_type: :string @@ -775,7 +777,7 @@ defmodule MvWeb.MemberLive.IndexTest do CustomField |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!() + |> Ash.create!(actor: system_actor) end test "mount initializes boolean_custom_field_filters as empty map", %{conn: conn} do @@ -1016,6 +1018,7 @@ defmodule MvWeb.MemberLive.IndexTest do test "handle_params removes filter when custom field is deleted", %{conn: conn} do conn = conn_with_oidc_user(conn) + system_actor = Mv.Helpers.SystemActor.get_system_actor() boolean_field = create_boolean_custom_field() # Set up filter via URL @@ -1026,8 +1029,8 @@ defmodule MvWeb.MemberLive.IndexTest do filters_before = state_before.socket.assigns.boolean_custom_field_filters assert filters_before[boolean_field.id] == true - # Delete the custom field - Ash.destroy!(boolean_field) + # Delete the custom field (requires actor with destroy permission) + Ash.destroy!(boolean_field, actor: system_actor) # Navigate again - filter should be removed since custom field no longer exists {:ok, view2, _html} = @@ -1122,18 +1125,15 @@ defmodule MvWeb.MemberLive.IndexTest do # Helper to create a member with a boolean custom field value defp create_member_with_boolean_value(member_attrs, custom_field, value, actor) do - {:ok, member} = - Mv.Membership.Member - |> Ash.Changeset.for_create( - :create_member, - %{ - first_name: "Test", - last_name: "Member", - email: "test.member.#{System.unique_integer([:positive])}@example.com" - } - |> Map.merge(member_attrs) - ) - |> Ash.create(actor: actor) + attrs = + %{ + first_name: "Test", + last_name: "Member", + email: "test.member.#{System.unique_integer([:positive])}@example.com" + } + |> Map.merge(member_attrs) + + {:ok, member} = Mv.Membership.create_member(attrs, actor: actor) {:ok, _cfv} = Mv.Membership.CustomFieldValue @@ -1177,13 +1177,14 @@ defmodule MvWeb.MemberLive.IndexTest do boolean_field = create_boolean_custom_field() {:ok, member} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) # Create CustomFieldValue with map format (Ash expects _union_type and _union_value) {:ok, _cfv} = @@ -1210,13 +1211,14 @@ defmodule MvWeb.MemberLive.IndexTest do boolean_field = create_boolean_custom_field() {:ok, member} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) # Member has no custom field value for this field member = member |> Ash.load!(:custom_field_values, actor: system_actor) @@ -1233,13 +1235,14 @@ defmodule MvWeb.MemberLive.IndexTest do boolean_field = create_boolean_custom_field() {:ok, member} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) # Create CustomFieldValue with nil value (edge case) {:ok, _cfv} = @@ -1266,13 +1269,14 @@ defmodule MvWeb.MemberLive.IndexTest do boolean_field = create_boolean_custom_field() {:ok, member} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Test", - last_name: "Member", - email: "test.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Test", + last_name: "Member", + email: "test.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) # Create string custom field value (not boolean) {:ok, _cfv} = @@ -1315,20 +1319,21 @@ defmodule MvWeb.MemberLive.IndexTest do ) {:ok, member_without_value} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "NoValue", - last_name: "Member", - email: "novalue.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "NoValue", + last_name: "Member", + email: "novalue.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) member_without_value = member_without_value |> Ash.load!(:custom_field_values, actor: system_actor) members = [member_with_true, member_with_false, member_without_value] filters = %{to_string(boolean_field.id) => true} - all_custom_fields = Mv.Membership.CustomField |> Ash.read!() + all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor) result = MvWeb.MemberLive.Index.apply_boolean_custom_field_filters( @@ -1365,20 +1370,21 @@ defmodule MvWeb.MemberLive.IndexTest do ) {:ok, member_without_value} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "NoValue", - last_name: "Member", - email: "novalue.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "NoValue", + last_name: "Member", + email: "novalue.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) member_without_value = member_without_value |> Ash.load!(:custom_field_values, actor: system_actor) members = [member_with_true, member_with_false, member_without_value] filters = %{to_string(boolean_field.id) => false} - all_custom_fields = Mv.Membership.CustomField |> Ash.read!() + all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor) result = MvWeb.MemberLive.Index.apply_boolean_custom_field_filters( @@ -1417,7 +1423,7 @@ defmodule MvWeb.MemberLive.IndexTest do members = [member1, member2] filters = %{} - all_custom_fields = Mv.Membership.CustomField |> Ash.read!() + all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor) result = MvWeb.MemberLive.Index.apply_boolean_custom_field_filters( @@ -1442,13 +1448,14 @@ defmodule MvWeb.MemberLive.IndexTest do # Member with both fields = true {:ok, member_both_true} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "BothTrue", - last_name: "Member", - email: "bothtrue.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "BothTrue", + last_name: "Member", + email: "bothtrue.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) {:ok, _cfv1} = Mv.Membership.CustomFieldValue @@ -1472,13 +1479,14 @@ defmodule MvWeb.MemberLive.IndexTest do # Member with field1 = true, field2 = false {:ok, member_mixed} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Mixed", - last_name: "Member", - email: "mixed.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "Mixed", + last_name: "Member", + email: "mixed.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) {:ok, _cfv3} = Mv.Membership.CustomFieldValue @@ -1507,7 +1515,7 @@ defmodule MvWeb.MemberLive.IndexTest do to_string(boolean_field2.id) => true } - all_custom_fields = Mv.Membership.CustomField |> Ash.read!() + all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor) result = MvWeb.MemberLive.Index.apply_boolean_custom_field_filters( @@ -1538,7 +1546,7 @@ defmodule MvWeb.MemberLive.IndexTest do members = [member] filters = %{fake_id => true} - all_custom_fields = Mv.Membership.CustomField |> Ash.read!() + all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor) result = MvWeb.MemberLive.Index.apply_boolean_custom_field_filters( @@ -1575,13 +1583,14 @@ defmodule MvWeb.MemberLive.IndexTest do ) {:ok, _member_without_value} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "NoValue", - last_name: "Member", - email: "novalue.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "NoValue", + last_name: "Member", + email: "novalue.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) # Test true filter {:ok, _view, html_true} = @@ -1610,14 +1619,15 @@ defmodule MvWeb.MemberLive.IndexTest do # Member with true boolean value and paid status {:ok, member_paid_true} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "PaidTrue", - last_name: "Member", - email: "paidtrue.member.#{System.unique_integer([:positive])}@example.com", - membership_fee_type_id: fee_type.id - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "PaidTrue", + last_name: "Member", + email: "paidtrue.member.#{System.unique_integer([:positive])}@example.com", + membership_fee_type_id: fee_type.id + }, + actor: system_actor + ) {:ok, _cfv} = Mv.Membership.CustomFieldValue @@ -1637,14 +1647,15 @@ defmodule MvWeb.MemberLive.IndexTest do # Member with true boolean value but unpaid status {:ok, member_unpaid_true} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "UnpaidTrue", - last_name: "Member", - email: "unpaidtrue.member.#{System.unique_integer([:positive])}@example.com", - membership_fee_type_id: fee_type.id - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "UnpaidTrue", + last_name: "Member", + email: "unpaidtrue.member.#{System.unique_integer([:positive])}@example.com", + membership_fee_type_id: fee_type.id + }, + actor: system_actor + ) {:ok, _cfv2} = Mv.Membership.CustomFieldValue @@ -1725,13 +1736,14 @@ defmodule MvWeb.MemberLive.IndexTest do ) {:ok, _member_without_value} = - Mv.Membership.Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "NoValue", - last_name: "Member", - email: "novalue.member.#{System.unique_integer([:positive])}@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{ + first_name: "NoValue", + last_name: "Member", + email: "novalue.member.#{System.unique_integer([:positive])}@example.com" + }, + actor: system_actor + ) # Test that filter works even though field is not visible in overview {:ok, _view, html_true} = 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 2636419..ca9d658 100644 --- a/test/mv_web/member_live/membership_fee_integration_test.exs +++ b/test/mv_web/member_live/membership_fee_integration_test.exs @@ -6,7 +6,6 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do import Phoenix.LiveViewTest - alias Mv.Membership.Member alias Mv.MembershipFees.MembershipFeeType alias Mv.MembershipFees.MembershipFeeCycle @@ -40,10 +39,8 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: system_actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: system_actor) + member end describe "end-to-end workflows" do @@ -152,7 +149,7 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do system_actor = Mv.Helpers.SystemActor.get_system_actor() member = - Member + Mv.Membership.Member |> Ash.Query.filter(email == ^form_data["member[email]"]) |> Ash.read_one!(actor: system_actor) 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 e41f02f..20bf46d 100644 --- a/test/mv_web/member_live/show_membership_fees_test.exs +++ b/test/mv_web/member_live/show_membership_fees_test.exs @@ -6,7 +6,6 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do import Phoenix.LiveViewTest - alias Mv.Membership.Member alias Mv.MembershipFees.MembershipFeeType alias Mv.MembershipFees.MembershipFeeCycle @@ -40,10 +39,8 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do } attrs = Map.merge(default_attrs, attrs) - - Member - |> Ash.Changeset.for_create(:create_member, attrs) - |> Ash.create!(actor: system_actor) + {:ok, member} = Mv.Membership.create_member(attrs, actor: system_actor) + member end # Helper to create a cycle diff --git a/test/mv_web/member_live/show_test.exs b/test/mv_web/member_live/show_test.exs index d2c6e55..26c3f00 100644 --- a/test/mv_web/member_live/show_test.exs +++ b/test/mv_web/member_live/show_test.exs @@ -18,20 +18,17 @@ defmodule MvWeb.MemberLive.ShowTest do require Ash.Query use Gettext, backend: MvWeb.Gettext - alias Mv.Membership.{CustomField, CustomFieldValue, Member} + alias Mv.Membership.{CustomField, CustomFieldValue} setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() # Create test member {:ok, member} = - Member - |> Ash.Changeset.for_create(:create_member, %{ - first_name: "Alice", - last_name: "Anderson", - email: "alice@example.com" - }) - |> Ash.create(actor: system_actor) + Mv.Membership.create_member( + %{first_name: "Alice", last_name: "Anderson", email: "alice@example.com"}, + actor: system_actor + ) %{member: member, actor: system_actor} end