From 7f9d9646a51a0231d983101e8c7e34fd8f286888 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 14:33:39 +0200 Subject: [PATCH 01/27] fix(auth): boot AshAuthentication children under the :mv otp_app --- lib/mv/application.ex | 2 +- test/mv/application_test.exs | 58 ++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 test/mv/application_test.exs diff --git a/lib/mv/application.ex b/lib/mv/application.ex index 1b6014e..e9d62d0 100644 --- a/lib/mv/application.ex +++ b/lib/mv/application.ex @@ -32,7 +32,7 @@ defmodule Mv.Application do {Task.Supervisor, name: Mv.TaskSupervisor}, {DNSCluster, query: Application.get_env(:mv, :dns_cluster_query) || :ignore}, {Phoenix.PubSub, name: Mv.PubSub}, - {AshAuthentication.Supervisor, otp_app: :my}, + {AshAuthentication.Supervisor, otp_app: :mv}, SystemActor, # Start a worker by calling: Mv.Worker.start_link(arg) # {Mv.Worker, arg}, diff --git a/test/mv/application_test.exs b/test/mv/application_test.exs new file mode 100644 index 0000000..c8f24b0 --- /dev/null +++ b/test/mv/application_test.exs @@ -0,0 +1,58 @@ +defmodule Mv.ApplicationTest do + @moduledoc """ + Guards the AshAuthentication supervisor wiring in `Mv.Application`. + + The auth children (token Expunger, audit-log batcher/expunger) resolve their + configuration via `Spark.sparks(otp_app, Ash.Resource)`. With the wrong + `otp_app` they silently start against an empty resource set: the token + Expunger then runs against no token resources and becomes a no-op. This test + pins the corrected `:mv` wiring against the *running* application tree: the + supervisor and all three children are present, and the token Expunger booted + live and resolved the real `:mv` token resource. + + The two audit-log children legitimately report an `:undefined` pid because no + `AuditLogResource` resources exist under `:mv` (their init returns `:ignore`); + that is a successful start, not a crash, so they are only asserted present. + """ + use ExUnit.Case, async: false + + alias AshAuthentication.TokenResource.Expunger + + test "AshAuthentication children boot under the :mv otp_app and resolve real config" do + # Locate the running AshAuthentication.Supervisor inside the live app tree. + auth_sup = + Mv.Supervisor + |> Supervisor.which_children() + |> Enum.find_value(fn + {AshAuthentication.Supervisor, pid, _type, _mods} when is_pid(pid) -> pid + _ -> nil + end) + + assert is_pid(auth_sup), "AshAuthentication.Supervisor is not running in Mv.Supervisor" + + children = Supervisor.which_children(auth_sup) + child_ids = children |> Enum.map(&elem(&1, 0)) |> Enum.sort() + + # All three auth children are present in the supervision tree. + assert child_ids == + Enum.sort([ + AshAuthentication.TokenResource.Expunger, + AshAuthentication.AuditLogResource.Batcher, + AshAuthentication.AuditLogResource.Expunger + ]) + + # The token Expunger booted as a live process and resolved the real :mv + # token resource — proving the children run against non-empty :mv config + # rather than the empty set the wrong otp_app produced. + expunger_pid = + Enum.find_value(children, fn + {Expunger, pid, _, _} when is_pid(pid) -> pid + _ -> nil + end) + + assert is_pid(expunger_pid), "token Expunger did not boot as a live process under :mv" + assert Process.alive?(expunger_pid) + assert %{otp_app: :mv, resources: resources} = :sys.get_state(expunger_pid) + assert Map.has_key?(resources, Mv.Accounts.Token) + end +end From 2a3a152b13be3252a56e3f20a720b2db95fbdb2b Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 14:42:12 +0200 Subject: [PATCH 02/27] perf(member): drop per-render timing log on the member-list hot path --- lib/mv_web/live/member_live/index.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index 53a5705..1b38820 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -1026,8 +1026,7 @@ defmodule MvWeb.MemberLive.Index do # Errors in handle_params are handled by Phoenix LiveView actor = current_actor(socket) - {time_microseconds, members} = :timer.tc(fn -> Ash.read!(query, actor: actor) end) - Logger.info("Ash.read! in load_members/1 took #{time_microseconds / 1000} ms") + members = Ash.read!(query, actor: actor) # Custom field values are already filtered at the database level in load_custom_field_values/2 # No need for in-memory filtering anymore From a9932776ccddabfb0354fa11754602f9b4a3a9ea Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 14:45:36 +0200 Subject: [PATCH 03/27] chore(accounts): remove orphaned UserIdentity resource file --- lib/accounts/user_identity.exs | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 lib/accounts/user_identity.exs diff --git a/lib/accounts/user_identity.exs b/lib/accounts/user_identity.exs deleted file mode 100644 index fd8d2c9..0000000 --- a/lib/accounts/user_identity.exs +++ /dev/null @@ -1,18 +0,0 @@ -defmodule Mv.Accounts.UserIdentity do - @moduledoc """ - AshAuthentication specific ressource - """ - use Ash.Resource, - data_layer: AshPostgres.DataLayer, - extensions: [AshAuthentication.UserIdentity], - domain: Mv.Accounts - - postgres do - table "user_identities" - repo Mv.Repo - end - - user_identity do - user_resource Mv.Accounts.User - end -end From c4a695329cced362e62a59c4e6048b4d62d64204 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 14:49:06 +0200 Subject: [PATCH 04/27] refactor(member-export): remove dead fetch/2 export chain --- lib/mv/membership/member_export.ex | 273 ----------------------------- 1 file changed, 273 deletions(-) diff --git a/lib/mv/membership/member_export.ex b/lib/mv/membership/member_export.ex index a98b125..0e6793d 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -7,12 +7,6 @@ defmodule Mv.Membership.MemberExport do and sends the download. """ - require Ash.Query - import Ash.Expr - - alias Mv.Membership.CustomField - alias Mv.Membership.Member - alias Mv.Membership.MemberExportSort alias MvWeb.MemberLive.Index alias MvWeb.MemberLive.Index.MembershipFeeStatus @@ -35,261 +29,8 @@ defmodule Mv.Membership.MemberExport do ["membership_fee_type", "membership_fee_status", "groups"] @computed_export_fields ["membership_fee_status"] @computed_insert_after "membership_fee_start_date" - @custom_field_prefix Mv.Constants.custom_field_prefix() @domain_member_field_strings Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) - @doc """ - Fetches members and column specs for export. - - - `actor` - Ash actor (e.g. current user) - - `parsed` - Map from controller's parse_and_validate (selected_ids, member_fields, etc.) - - Returns `{:ok, members, column_specs}` or `{:error, :forbidden}`. - Column specs have `:kind`, `:key`, and for custom fields `:custom_field`; - the controller adds `:header` and optional computed columns to members before CSV export. - """ - @spec fetch(struct(), map()) :: - {:ok, [struct()], [map()]} | {:error, :forbidden} - def fetch(actor, parsed) do - custom_field_ids_union = - (parsed.custom_field_ids ++ Map.keys(parsed.boolean_filters || %{})) |> Enum.uniq() - - with {:ok, custom_fields_by_id} <- load_custom_fields_by_id(custom_field_ids_union, actor), - {:ok, members} <- load_members(actor, parsed, custom_fields_by_id) do - column_specs = build_column_specs(parsed, custom_fields_by_id) - {:ok, members, column_specs} - end - end - - defp load_custom_fields_by_id([], _actor), do: {:ok, %{}} - - defp load_custom_fields_by_id(custom_field_ids, actor) do - query = - CustomField - |> Ash.Query.filter(expr(id in ^custom_field_ids)) - |> Ash.Query.select([:id, :name, :value_type]) - - case Ash.read(query, actor: actor) do - {:ok, custom_fields} -> - by_id = build_custom_fields_by_id(custom_field_ids, custom_fields) - {:ok, by_id} - - {:error, %Ash.Error.Forbidden{}} -> - {:error, :forbidden} - end - end - - defp build_custom_fields_by_id(custom_field_ids, custom_fields) do - Enum.reduce(custom_field_ids, %{}, fn id, acc -> - find_and_add_custom_field(acc, id, custom_fields) - end) - end - - defp find_and_add_custom_field(acc, id, custom_fields) do - case Enum.find(custom_fields, fn cf -> to_string(cf.id) == to_string(id) end) do - nil -> acc - cf -> Map.put(acc, id, cf) - end - end - - defp build_column_specs(parsed, custom_fields_by_id) do - member_specs = build_member_column_specs(parsed) - custom_specs = build_custom_column_specs(parsed, custom_fields_by_id) - - member_specs ++ custom_specs - end - - defp build_member_column_specs(parsed) do - Enum.map(parsed.member_fields, fn f -> - build_single_member_spec(f, parsed.selectable_member_fields) - end) - |> Enum.reject(&is_nil/1) - end - - defp build_single_member_spec(field, selectable_member_fields) do - if field in selectable_member_fields do - %{kind: :member_field, key: field} - else - build_computed_spec(field) - end - end - - defp build_computed_spec(field) do - # only allow known computed export fields to avoid crashing on unknown atoms - if field in @computed_export_fields do - %{kind: :computed, key: String.to_existing_atom(field)} - else - # ignore unknown non-selectable fields defensively - nil - end - end - - defp build_custom_column_specs(parsed, custom_fields_by_id) do - parsed.custom_field_ids - |> Enum.map(fn id -> Map.get(custom_fields_by_id, id) end) - |> Enum.reject(&is_nil/1) - |> Enum.map(fn cf -> %{kind: :custom_field, key: cf.id, custom_field: cf} end) - end - - defp load_members(actor, parsed, custom_fields_by_id) do - query = build_members_query(parsed, custom_fields_by_id) - - case Ash.read(query, actor: actor) do - {:ok, members} -> - processed_members = process_loaded_members(members, parsed, custom_fields_by_id) - {:ok, processed_members} - - {:error, %Ash.Error.Forbidden{}} -> - {:error, :forbidden} - end - end - - defp build_members_query(parsed, _custom_fields_by_id) do - select_fields = - [:id] ++ Enum.map(parsed.selectable_member_fields, &String.to_existing_atom/1) - - custom_field_ids_union = parsed.custom_field_ids ++ Map.keys(parsed.boolean_filters || %{}) - - need_cycles = - parsed.show_current_cycle or parsed.cycle_status_filter != nil or - parsed.computed_fields != [] or - "membership_fee_status" in parsed.member_fields - - query = - Member - |> Ash.Query.new() - |> Ash.Query.select(select_fields) - |> load_custom_field_values_query(custom_field_ids_union) - |> maybe_load_cycles(need_cycles, parsed.show_current_cycle) - - if parsed.selected_ids != [] do - Ash.Query.filter(query, expr(id in ^parsed.selected_ids)) - else - query - |> apply_search(parsed.query) - |> then(fn q -> - {q, _sort_after_load} = maybe_sort(q, parsed.sort_field, parsed.sort_order) - q - end) - end - end - - defp process_loaded_members(members, parsed, custom_fields_by_id) do - members - |> apply_post_load_filters(parsed, custom_fields_by_id) - |> apply_post_load_sorting(parsed, custom_fields_by_id) - |> add_computed_fields(parsed.computed_fields, parsed.show_current_cycle) - end - - defp apply_post_load_filters(members, parsed, custom_fields_by_id) do - if parsed.selected_ids == [] do - members - |> apply_cycle_status_filter(parsed.cycle_status_filter, parsed.show_current_cycle) - |> Index.apply_boolean_custom_field_filters( - parsed.boolean_filters || %{}, - Map.values(custom_fields_by_id) - ) - else - members - end - end - - defp apply_post_load_sorting(members, parsed, custom_fields_by_id) do - if parsed.selected_ids == [] and sort_after_load?(parsed.sort_field) do - sort_members_by_custom_field( - members, - parsed.sort_field, - parsed.sort_order, - Map.values(custom_fields_by_id) - ) - else - members - end - end - - defp load_custom_field_values_query(query, []), do: query - - defp load_custom_field_values_query(query, custom_field_ids) do - cfv_query = - Mv.Membership.CustomFieldValue - |> Ash.Query.filter(expr(custom_field_id in ^custom_field_ids)) - |> Ash.Query.load(custom_field: [:id, :name, :value_type]) - - Ash.Query.load(query, custom_field_values: cfv_query) - end - - defp apply_search(query, nil), do: query - defp apply_search(query, ""), do: query - - defp apply_search(query, q) when is_binary(q) do - if String.trim(q) != "" do - Member.fuzzy_search(query, %{query: q}) - else - query - end - end - - defp maybe_sort(query, nil, _order), do: {query, false} - defp maybe_sort(query, _field, nil), do: {query, false} - - defp maybe_sort(query, field, order) when is_binary(field) do - if custom_field_sort?(field) do - {query, true} - else - field_atom = String.to_existing_atom(field) - - if field_atom in (Mv.Constants.member_fields() -- [:notes]) do - {Ash.Query.sort(query, [{field_atom, String.to_existing_atom(order)}]), false} - else - {query, false} - end - end - rescue - ArgumentError -> {query, false} - end - - defp sort_after_load?(field) when is_binary(field), - do: String.starts_with?(field, @custom_field_prefix) - - defp sort_after_load?(_), do: false - - defp sort_members_by_custom_field(members, _field, _order, _custom_fields) when members == [], - do: [] - - defp sort_members_by_custom_field(members, field, order, custom_fields) when is_binary(field) do - id_str = String.trim_leading(field, @custom_field_prefix) - custom_field = Enum.find(custom_fields, fn cf -> to_string(cf.id) == id_str end) - if is_nil(custom_field), do: members - - key_fn = fn member -> - cfv = find_cfv(member, custom_field) - raw = if cfv, do: cfv.value, else: nil - MemberExportSort.custom_field_sort_key(custom_field.value_type, raw) - end - - members - |> Enum.map(fn m -> {m, key_fn.(m)} end) - |> Enum.sort(fn {_, ka}, {_, kb} -> MemberExportSort.key_lt(ka, kb, order) end) - |> Enum.map(fn {m, _} -> m end) - end - - defp find_cfv(member, custom_field) do - (member.custom_field_values || []) - |> Enum.find(fn cfv -> - to_string(cfv.custom_field_id) == to_string(custom_field.id) or - (Map.get(cfv, :custom_field) && - to_string(cfv.custom_field.id) == to_string(custom_field.id)) - end) - end - - defp custom_field_sort?(field), do: String.starts_with?(field, @custom_field_prefix) - - defp maybe_load_cycles(query, false, _show_current), do: query - - defp maybe_load_cycles(query, true, show_current) do - MembershipFeeStatus.load_cycles_for_members(query, show_current) - end - defp apply_cycle_status_filter(members, nil, _show_current), do: members defp apply_cycle_status_filter(members, status, show_current) when status in [:paid, :unpaid] do @@ -298,20 +39,6 @@ defmodule Mv.Membership.MemberExport do defp apply_cycle_status_filter(members, _status, _show_current), do: members - defp add_computed_fields(members, computed_fields, show_current_cycle) do - computed_fields = computed_fields || [] - - if "membership_fee_status" in computed_fields do - Enum.map(members, fn member -> - status = MembershipFeeStatus.get_cycle_status_for_member(member, show_current_cycle) - # <= Atom rein - Map.put(member, :membership_fee_status, status) - end) - else - members - end - end - # Called by controller to build parsed map from raw params (kept here so controller stays thin) @doc """ Parses and validates export params (from JSON payload). From 924dbd3bb81d5b1e2055f2a9c52a44a63981961d Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 14:52:34 +0200 Subject: [PATCH 05/27] refactor(oidc): drop OidcRoleSyncConfig passthrough and use Mv.Config directly --- docs/admin-bootstrap-and-oidc-role-sync.md | 2 +- lib/mv/oidc_role_sync.ex | 8 +-- lib/mv/oidc_role_sync_config.ex | 20 -------- test/mv/oidc_role_sync_config_test.exs | 59 ---------------------- 4 files changed, 5 insertions(+), 84 deletions(-) delete mode 100644 lib/mv/oidc_role_sync_config.ex delete mode 100644 test/mv/oidc_role_sync_config_test.exs diff --git a/docs/admin-bootstrap-and-oidc-role-sync.md b/docs/admin-bootstrap-and-oidc-role-sync.md index ee78069..de34d47 100644 --- a/docs/admin-bootstrap-and-oidc-role-sync.md +++ b/docs/admin-bootstrap-and-oidc-role-sync.md @@ -34,7 +34,7 @@ - `OIDC_ADMIN_GROUP_NAME` – OIDC group name that maps to the Admin role. If unset, no role sync. - `OIDC_GROUPS_CLAIM` – JWT claim name for group list (default "groups"). -- Module: Mv.OidcRoleSyncConfig (oidc_admin_group_name/0, oidc_groups_claim/0). +- Module: Mv.Config (oidc_admin_group_name/0, oidc_groups_claim/0). ### Sign-in page (OIDC-only mode) diff --git a/lib/mv/oidc_role_sync.ex b/lib/mv/oidc_role_sync.ex index 0f6467c..7461a32 100644 --- a/lib/mv/oidc_role_sync.ex +++ b/lib/mv/oidc_role_sync.ex @@ -4,7 +4,7 @@ defmodule Mv.OidcRoleSync do Used after OIDC registration (register_with_oidc) and on sign-in so that users in the configured admin group get the Admin role; others get Mitglied. - Configure via OIDC_ADMIN_GROUP_NAME and OIDC_GROUPS_CLAIM (see OidcRoleSyncConfig). + Configure via OIDC_ADMIN_GROUP_NAME and OIDC_GROUPS_CLAIM (see Mv.Config). Groups are read from user_info (ID token claims) first; if missing or empty, the access_token from oauth_tokens is decoded as JWT and the groups claim is @@ -23,7 +23,7 @@ defmodule Mv.OidcRoleSync do """ alias Mv.Accounts.User alias Mv.Authorization.Role - alias Mv.OidcRoleSyncConfig + alias Mv.Config @doc """ Applies Admin or Mitglied role to the user based on OIDC groups claim. @@ -38,12 +38,12 @@ defmodule Mv.OidcRoleSync do @spec apply_admin_role_from_user_info(User.t(), map(), map() | nil) :: :ok def apply_admin_role_from_user_info(user, user_info, oauth_tokens \\ nil) when is_map(user_info) do - admin_group = OidcRoleSyncConfig.oidc_admin_group_name() + admin_group = Config.oidc_admin_group_name() if is_nil(admin_group) or admin_group == "" do :ok else - claim = OidcRoleSyncConfig.oidc_groups_claim() + claim = Config.oidc_groups_claim() groups = groups_from_user_info(user_info, claim) groups = diff --git a/lib/mv/oidc_role_sync_config.ex b/lib/mv/oidc_role_sync_config.ex deleted file mode 100644 index bbb5770..0000000 --- a/lib/mv/oidc_role_sync_config.ex +++ /dev/null @@ -1,20 +0,0 @@ -defmodule Mv.OidcRoleSyncConfig do - @moduledoc """ - Runtime configuration for OIDC group → role sync (e.g. admin group → Admin role). - - Reads from Mv.Config (ENV first, then Settings): - - `oidc_admin_group_name/0` – OIDC group name that maps to Admin role (optional; when nil, no sync). - - `oidc_groups_claim/0` – JWT/user_info claim name for groups (default: `"groups"`). - - Set via ENV: OIDC_ADMIN_GROUP_NAME, OIDC_GROUPS_CLAIM; or via Settings (Basic settings → OIDC). - """ - @doc "Returns the OIDC group name that maps to Admin role, or nil if not configured." - def oidc_admin_group_name do - Mv.Config.oidc_admin_group_name() - end - - @doc "Returns the JWT/user_info claim name for groups; defaults to \"groups\"." - def oidc_groups_claim do - Mv.Config.oidc_groups_claim() - end -end diff --git a/test/mv/oidc_role_sync_config_test.exs b/test/mv/oidc_role_sync_config_test.exs deleted file mode 100644 index 4b77378..0000000 --- a/test/mv/oidc_role_sync_config_test.exs +++ /dev/null @@ -1,59 +0,0 @@ -defmodule Mv.OidcRoleSyncConfigTest do - @moduledoc """ - Tests for OIDC role sync configuration (OIDC_ADMIN_GROUP_NAME, OIDC_GROUPS_CLAIM). - Reads via Mv.Config (ENV first, then Settings). - """ - use Mv.DataCase, async: false - - alias Mv.OidcRoleSyncConfig - - describe "oidc_admin_group_name/0" do - test "returns nil when OIDC_ADMIN_GROUP_NAME is not configured" do - restore = clear_env("OIDC_ADMIN_GROUP_NAME") - on_exit(restore) - - assert OidcRoleSyncConfig.oidc_admin_group_name() == nil - end - - test "returns configured admin group name when set via ENV" do - restore = set_env("OIDC_ADMIN_GROUP_NAME", "mila-admin") - on_exit(restore) - - assert OidcRoleSyncConfig.oidc_admin_group_name() == "mila-admin" - end - end - - describe "oidc_groups_claim/0" do - test "returns default \"groups\" when OIDC_GROUPS_CLAIM is not configured" do - restore = clear_env("OIDC_GROUPS_CLAIM") - on_exit(restore) - - assert OidcRoleSyncConfig.oidc_groups_claim() == "groups" - end - - test "returns configured claim name when OIDC_GROUPS_CLAIM is set via ENV" do - restore = set_env("OIDC_GROUPS_CLAIM", "ak_groups") - on_exit(restore) - - assert OidcRoleSyncConfig.oidc_groups_claim() == "ak_groups" - end - end - - defp set_env(key, value) do - previous = System.get_env(key) - System.put_env(key, value) - - fn -> - if previous, do: System.put_env(key, previous), else: System.delete_env(key) - end - end - - defp clear_env(key) do - previous = System.get_env(key) - System.delete_env(key) - - fn -> - if previous, do: System.put_env(key, previous) - end - end -end From 164826d3aa5949bc0f5657a24d0eaf2591f1653a Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 14:56:07 +0200 Subject: [PATCH 06/27] refactor(authorization): unify own_data read check across linked resources --- lib/membership/member_group.ex | 2 +- lib/membership_fees/membership_fee_cycle.ex | 2 +- .../member_group_read_linked_for_own_data.ex | 63 ---------------- ...ship_fee_cycle_read_linked_for_own_data.ex | 62 ---------------- .../checks/read_linked_for_own_data.ex | 74 +++++++++++++++++++ 5 files changed, 76 insertions(+), 127 deletions(-) delete mode 100644 lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex delete mode 100644 lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex create mode 100644 lib/mv/authorization/checks/read_linked_for_own_data.ex diff --git a/lib/membership/member_group.ex b/lib/membership/member_group.ex index 22a1f70..d09f0e5 100644 --- a/lib/membership/member_group.ex +++ b/lib/membership/member_group.ex @@ -63,7 +63,7 @@ defmodule Mv.Membership.MemberGroup do policies do bypass action_type(:read) do description "own_data: read only member_groups where member_id == actor.member_id" - authorize_if Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData + authorize_if {Mv.Authorization.Checks.ReadLinkedForOwnData, member_id_field: :member_id} end policy action_type(:read) do diff --git a/lib/membership_fees/membership_fee_cycle.ex b/lib/membership_fees/membership_fee_cycle.ex index f0dd1a7..cd62887 100644 --- a/lib/membership_fees/membership_fee_cycle.ex +++ b/lib/membership_fees/membership_fee_cycle.ex @@ -88,7 +88,7 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do policies do bypass action_type(:read) do description "own_data: read only cycles where member_id == actor.member_id" - authorize_if Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData + authorize_if {Mv.Authorization.Checks.ReadLinkedForOwnData, member_id_field: :member_id} end policy action_type([:read, :create, :update, :destroy]) do diff --git a/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex b/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex deleted file mode 100644 index a553fde..0000000 --- a/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex +++ /dev/null @@ -1,63 +0,0 @@ -defmodule Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData do - @moduledoc """ - Policy check for MemberGroup read: true only when actor has permission set "own_data" - AND record.member_id == actor.member_id. - - Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries), - while admin with member_id does not match and gets :all from HasPermission. - - - With a record (e.g. get by id): returns true only when own_data and member_id match. - - Without a record (list query): strict_check returns false; auto_filter adds filter when own_data. - """ - use Ash.Policy.Check - - alias Mv.Authorization.Checks.ActorPermissionSetIs - - @impl true - def type, do: :filter - - @impl true - def describe(_opts), - do: "own_data can read only member_groups where member_id == actor.member_id" - - @impl true - def strict_check(actor, authorizer, _opts) do - record = get_record_from_authorizer(authorizer) - is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") - - cond do - # List query + own_data: return :unknown so authorizer applies auto_filter (keyword list) - is_nil(record) and is_own_data -> - {:ok, :unknown} - - is_nil(record) -> - {:ok, false} - - not is_own_data -> - {:ok, false} - - record.member_id == actor.member_id -> - {:ok, true} - - true -> - {:ok, false} - end - end - - @impl true - def auto_filter(actor, _authorizer, _opts) do - if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && - Map.get(actor, :member_id) do - [member_id: actor.member_id] - else - [] - end - end - - defp get_record_from_authorizer(authorizer) do - case authorizer.subject do - %{data: data} when not is_nil(data) -> data - _ -> nil - end - end -end diff --git a/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex b/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex deleted file mode 100644 index 092558c..0000000 --- a/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex +++ /dev/null @@ -1,62 +0,0 @@ -defmodule Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData do - @moduledoc """ - Policy check for MembershipFeeCycle read: true only when actor has permission set "own_data" - AND record.member_id == actor.member_id. - - Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries), - while admin with member_id does not match and gets :all from HasPermission. - - - With a record (e.g. get by id): returns true only when own_data and member_id match. - - Without a record (list query): return :unknown so authorizer applies auto_filter. - """ - use Ash.Policy.Check - - alias Mv.Authorization.Checks.ActorPermissionSetIs - - @impl true - def type, do: :filter - - @impl true - def describe(_opts), - do: "own_data can read only membership_fee_cycles where member_id == actor.member_id" - - @impl true - def strict_check(actor, authorizer, _opts) do - record = get_record_from_authorizer(authorizer) - is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") - - cond do - is_nil(record) and is_own_data -> - {:ok, :unknown} - - is_nil(record) -> - {:ok, false} - - not is_own_data -> - {:ok, false} - - record.member_id == actor.member_id -> - {:ok, true} - - true -> - {:ok, false} - end - end - - @impl true - def auto_filter(actor, _authorizer, _opts) do - if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && - Map.get(actor, :member_id) do - [member_id: actor.member_id] - else - [] - end - end - - defp get_record_from_authorizer(authorizer) do - case authorizer.subject do - %{data: data} when not is_nil(data) -> data - _ -> nil - end - end -end diff --git a/lib/mv/authorization/checks/read_linked_for_own_data.ex b/lib/mv/authorization/checks/read_linked_for_own_data.ex new file mode 100644 index 0000000..eafeb8b --- /dev/null +++ b/lib/mv/authorization/checks/read_linked_for_own_data.ex @@ -0,0 +1,74 @@ +defmodule Mv.Authorization.Checks.ReadLinkedForOwnData do + @moduledoc """ + Generic policy check for resources that link to a member via a member-id + attribute: read is allowed only when the actor has the "own_data" permission + set AND `record. == actor.member_id`. + + Used in a read bypass so that own_data gets the linked filter (via auto_filter + for list queries), while admin with a member_id does not match and falls + through to `HasPermission` for `:all`. + + - With a record (e.g. get by id): returns true only when own_data and the + member ids match. + - Without a record (list query) + own_data: returns `:unknown` so the + authorizer applies `auto_filter`. + + ## Options + - `:member_id_field` - the attribute on the resource holding the member id. + Defaults to `:member_id`. + """ + use Ash.Policy.Check + + alias Mv.Authorization.Checks.ActorPermissionSetIs + + @impl true + def type, do: :filter + + @impl true + def describe(opts) do + "own_data can read only records where #{member_id_field(opts)} == actor.member_id" + end + + @impl true + def strict_check(actor, authorizer, opts) do + field = member_id_field(opts) + record = get_record_from_authorizer(authorizer) + is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") + + cond do + is_nil(record) and is_own_data -> + {:ok, :unknown} + + is_nil(record) -> + {:ok, false} + + not is_own_data -> + {:ok, false} + + Map.get(record, field) == actor.member_id -> + {:ok, true} + + true -> + {:ok, false} + end + end + + @impl true + def auto_filter(actor, _authorizer, opts) do + if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && + Map.get(actor, :member_id) do + [{member_id_field(opts), actor.member_id}] + else + [] + end + end + + defp member_id_field(opts), do: Keyword.get(opts, :member_id_field, :member_id) + + defp get_record_from_authorizer(authorizer) do + case authorizer.subject do + %{data: data} when not is_nil(data) -> data + _ -> nil + end + end +end From 561779e7048fada8a7a0fa0baa405866f410b340 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:03:05 +0200 Subject: [PATCH 07/27] refactor(web): share member-dropdown keyboard navigation between LiveViews --- lib/mv_web/live/group_live/show.ex | 61 +------------------ lib/mv_web/live/member_dropdown_nav.ex | 83 ++++++++++++++++++++++++++ lib/mv_web/live/user_live/form.ex | 64 +------------------- 3 files changed, 89 insertions(+), 119 deletions(-) create mode 100644 lib/mv_web/live/member_dropdown_nav.ex diff --git a/lib/mv_web/live/group_live/show.ex b/lib/mv_web/live/group_live/show.ex index ad24110..9e5bdf6 100644 --- a/lib/mv_web/live/group_live/show.ex +++ b/lib/mv_web/live/group_live/show.ex @@ -23,6 +23,7 @@ defmodule MvWeb.GroupLive.Show do alias Mv.Membership alias MvWeb.Helpers.MemberHelpers, as: MemberHelpers + alias MvWeb.Live.MemberDropdownNav @impl true def mount(_params, _session, socket) do @@ -566,56 +567,8 @@ defmodule MvWeb.GroupLive.Show do end @impl true - def handle_event("member_dropdown_keydown", %{"key" => "ArrowDown"}, socket) do - return_if_dropdown_closed(socket, fn -> - max_index = length(socket.assigns.available_members) - 1 - current = socket.assigns.focused_member_index - - new_index = - case current do - nil -> 0 - index when index < max_index -> index + 1 - _ -> current - end - - {:noreply, assign(socket, focused_member_index: new_index)} - end) - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "ArrowUp"}, socket) do - return_if_dropdown_closed(socket, fn -> - current = socket.assigns.focused_member_index - - new_index = - case current do - nil -> 0 - 0 -> 0 - index -> index - 1 - end - - {:noreply, assign(socket, focused_member_index: new_index)} - end) - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "Enter"}, socket) do - return_if_dropdown_closed(socket, fn -> - select_focused_member(socket) - end) - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "Escape"}, socket) do - return_if_dropdown_closed(socket, fn -> - {:noreply, assign(socket, show_member_dropdown: false, focused_member_index: nil)} - end) - end - - @impl true - def handle_event("member_dropdown_keydown", _params, socket) do - # Ignore other keys - {:noreply, socket} + def handle_event("member_dropdown_keydown", params, socket) do + MemberDropdownNav.handle_keydown(params, socket, fn -> select_focused_member(socket) end) end @impl true @@ -705,14 +658,6 @@ defmodule MvWeb.GroupLive.Show do end # Helper functions - defp return_if_dropdown_closed(socket, fun) do - if socket.assigns.show_member_dropdown do - fun.() - else - {:noreply, socket} - end - end - defp select_focused_member(socket) do case socket.assigns.focused_member_index do nil -> diff --git a/lib/mv_web/live/member_dropdown_nav.ex b/lib/mv_web/live/member_dropdown_nav.ex new file mode 100644 index 0000000..1a5d4f2 --- /dev/null +++ b/lib/mv_web/live/member_dropdown_nav.ex @@ -0,0 +1,83 @@ +defmodule MvWeb.Live.MemberDropdownNav do + @moduledoc """ + Shared keyboard-navigation logic for the member-search dropdown used by the + user form and the group show LiveViews. + + Both views keep their own `member_dropdown_keydown` event entry point and their + own `select_focused_member/1` (the selection effect differs per view), but the + arrow/enter/escape navigation over `:focused_member_index` and the + dropdown-open guard are identical and live here. + + The caller passes a zero-arity `select_focused` callback that performs the + view-specific selection of the currently focused entry. + """ + + import Phoenix.Component, only: [assign: 2] + + @type socket :: Phoenix.LiveView.Socket.t() + @type result :: {:noreply, socket} + + @doc """ + Handles a `member_dropdown_keydown` event for the shared dropdown. + + Navigation keys move `:focused_member_index` within + `[0, length(available_members) - 1]`; Enter invokes the view-specific + `select_focused` callback; Escape closes the dropdown; any other key is a + no-op. All key handling is guarded so that keystrokes while the dropdown is + closed are ignored. + """ + @spec handle_keydown(map(), socket, (-> result)) :: result + def handle_keydown(%{"key" => "ArrowDown"}, socket, _select_focused) do + return_if_dropdown_closed(socket, fn -> + max_index = length(socket.assigns.available_members) - 1 + current = socket.assigns.focused_member_index + + new_index = + case current do + nil -> 0 + index when index < max_index -> index + 1 + _ -> current + end + + {:noreply, assign(socket, focused_member_index: new_index)} + end) + end + + def handle_keydown(%{"key" => "ArrowUp"}, socket, _select_focused) do + return_if_dropdown_closed(socket, fn -> + current = socket.assigns.focused_member_index + + new_index = + case current do + nil -> 0 + 0 -> 0 + index -> index - 1 + end + + {:noreply, assign(socket, focused_member_index: new_index)} + end) + end + + def handle_keydown(%{"key" => "Enter"}, socket, select_focused) do + return_if_dropdown_closed(socket, select_focused) + end + + def handle_keydown(%{"key" => "Escape"}, socket, _select_focused) do + return_if_dropdown_closed(socket, fn -> + {:noreply, assign(socket, show_member_dropdown: false, focused_member_index: nil)} + end) + end + + def handle_keydown(_params, socket, _select_focused) do + # Ignore other keys + {:noreply, socket} + end + + defp return_if_dropdown_closed(socket, func) do + if socket.assigns.show_member_dropdown do + func.() + else + {:noreply, socket} + end + end +end diff --git a/lib/mv_web/live/user_live/form.ex b/lib/mv_web/live/user_live/form.ex index 60763ab..297f1f6 100644 --- a/lib/mv_web/live/user_live/form.ex +++ b/lib/mv_web/live/user_live/form.ex @@ -43,6 +43,7 @@ defmodule MvWeb.UserLive.Form do alias Mv.Membership alias Mv.Membership.Member, as: MemberResource alias MvWeb.Helpers.MemberHelpers + alias MvWeb.Live.MemberDropdownNav import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3] import MvWeb.Authorization, only: [can?: 3] @@ -571,56 +572,8 @@ defmodule MvWeb.UserLive.Form do end @impl true - def handle_event("member_dropdown_keydown", %{"key" => "ArrowDown"}, socket) do - return_if_dropdown_closed(socket, fn -> - max_index = length(socket.assigns.available_members) - 1 - current = socket.assigns.focused_member_index - - new_index = - case current do - nil -> 0 - index when index < max_index -> index + 1 - _ -> current - end - - {:noreply, assign(socket, focused_member_index: new_index)} - end) - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "ArrowUp"}, socket) do - return_if_dropdown_closed(socket, fn -> - current = socket.assigns.focused_member_index - - new_index = - case current do - nil -> 0 - 0 -> 0 - index -> index - 1 - end - - {:noreply, assign(socket, focused_member_index: new_index)} - end) - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "Enter"}, socket) do - return_if_dropdown_closed(socket, fn -> - select_focused_member(socket) - end) - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "Escape"}, socket) do - return_if_dropdown_closed(socket, fn -> - {:noreply, assign(socket, show_member_dropdown: false, focused_member_index: nil)} - end) - end - - @impl true - def handle_event("member_dropdown_keydown", _params, socket) do - # Ignore other keys - {:noreply, socket} + def handle_event("member_dropdown_keydown", params, socket) do + MemberDropdownNav.handle_keydown(params, socket, fn -> select_focused_member(socket) end) end @impl true @@ -778,17 +731,6 @@ defmodule MvWeb.UserLive.Form do @spec notify_parent(any()) :: {module(), any()} defp notify_parent(msg), do: send(self(), {__MODULE__, msg}) - # Helper to ignore keyboard events when dropdown is closed - @spec return_if_dropdown_closed(Phoenix.LiveView.Socket.t(), function()) :: - {:noreply, Phoenix.LiveView.Socket.t()} - defp return_if_dropdown_closed(socket, func) do - if socket.assigns.show_member_dropdown do - func.() - else - {:noreply, socket} - end - end - # Select the currently focused member from the dropdown @spec select_focused_member(Phoenix.LiveView.Socket.t()) :: {:noreply, Phoenix.LiveView.Socket.t()} From 1adf6aa6643d85d784b673d143a570fd20ec05d0 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:06:34 +0200 Subject: [PATCH 08/27] refactor(web): extract shared current_actor controller helper --- lib/mv_web/controllers/controller_helpers.ex | 22 +++++++++++++++++++ .../controllers/import_template_controller.ex | 8 ++----- .../controllers/member_export_controller.ex | 8 ++----- .../member_pdf_export_controller.ex | 10 ++------- 4 files changed, 28 insertions(+), 20 deletions(-) create mode 100644 lib/mv_web/controllers/controller_helpers.ex diff --git a/lib/mv_web/controllers/controller_helpers.ex b/lib/mv_web/controllers/controller_helpers.ex new file mode 100644 index 0000000..04087c0 --- /dev/null +++ b/lib/mv_web/controllers/controller_helpers.ex @@ -0,0 +1,22 @@ +defmodule MvWeb.ControllerHelpers do + @moduledoc """ + Shared helpers for plug-based controllers. + + The LiveView equivalent lives in `MvWeb.LiveHelpers`; this module is the + controller-side counterpart that works on a `Plug.Conn`. + """ + + alias Mv.Authorization.Actor + + @doc """ + Returns the request actor for a controller, loaded for authorization. + + Reads `:current_user` from the connection assigns and ensures it is loaded via + `Mv.Authorization.Actor.ensure_loaded/1`. + """ + @spec current_actor(Plug.Conn.t()) :: Mv.Accounts.User.t() | nil + def current_actor(conn) do + conn.assigns[:current_user] + |> Actor.ensure_loaded() + end +end diff --git a/lib/mv_web/controllers/import_template_controller.ex b/lib/mv_web/controllers/import_template_controller.ex index f040c7a..877697d 100644 --- a/lib/mv_web/controllers/import_template_controller.ex +++ b/lib/mv_web/controllers/import_template_controller.ex @@ -13,11 +13,12 @@ defmodule MvWeb.ImportTemplateController do """ use MvWeb, :controller - alias Mv.Authorization.Actor alias Mv.Membership.Member alias Mv.Membership.MembersCSV alias MvWeb.Authorization + import MvWeb.ControllerHelpers, only: [current_actor: 1] + # Standard member columns in template order, with their English and German headers # and a placeholder example value. Groups and fee type are importable extras. @columns [ @@ -105,11 +106,6 @@ defmodule MvWeb.ImportTemplateController do end end - defp current_actor(conn) do - conn.assigns[:current_user] - |> Actor.ensure_loaded() - end - defp return_forbidden(conn) do conn |> put_status(403) diff --git a/lib/mv_web/controllers/member_export_controller.ex b/lib/mv_web/controllers/member_export_controller.ex index 1f70a18..240db34 100644 --- a/lib/mv_web/controllers/member_export_controller.ex +++ b/lib/mv_web/controllers/member_export_controller.ex @@ -10,7 +10,6 @@ defmodule MvWeb.MemberExportController do require Ash.Query import Ash.Expr - alias Mv.Authorization.Actor alias Mv.Membership.CustomField alias Mv.Membership.CustomFieldSort alias Mv.Membership.Member @@ -18,6 +17,8 @@ defmodule MvWeb.MemberExportController do alias Mv.Membership.MembersCSV alias MvWeb.MemberLive.Index.MembershipFeeStatus alias MvWeb.Translations.MemberFields + + import MvWeb.ControllerHelpers, only: [current_actor: 1] use Gettext, backend: MvWeb.Gettext @member_fields_allowlist (Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)) ++ @@ -53,11 +54,6 @@ defmodule MvWeb.MemberExportController do |> json(%{error: message}) end - defp current_actor(conn) do - conn.assigns[:current_user] - |> Actor.ensure_loaded() - end - defp return_forbidden(conn) do conn |> put_status(403) diff --git a/lib/mv_web/controllers/member_pdf_export_controller.ex b/lib/mv_web/controllers/member_pdf_export_controller.ex index f00c0d1..1e82898 100644 --- a/lib/mv_web/controllers/member_pdf_export_controller.ex +++ b/lib/mv_web/controllers/member_pdf_export_controller.ex @@ -10,10 +10,11 @@ defmodule MvWeb.MemberPdfExportController do require Logger - alias Mv.Authorization.Actor alias Mv.Membership.{MemberExport, MemberExport.Build, MembersPDF} alias MvWeb.Translations.MemberFields + import MvWeb.ControllerHelpers, only: [current_actor: 1] + use Gettext, backend: MvWeb.Gettext @payload_required_message "payload required" @@ -79,13 +80,6 @@ defmodule MvWeb.MemberPdfExportController do bad_request(conn, @payload_required_message) end - # --- Actor / auth --- - - defp current_actor(conn) do - conn.assigns[:current_user] - |> Actor.ensure_loaded() - end - defp forbidden(conn) do conn |> put_status(:forbidden) From e66fb5d3d90f320deaa7bb1c8ddd3023bb7516e6 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:10:03 +0200 Subject: [PATCH 09/27] refactor(email): share build/deliver skeleton across join emails --- .../emails/join_already_member_email.ex | 24 +-------- .../emails/join_already_pending_email.ex | 24 +-------- lib/mv_web/emails/join_confirmation_email.ex | 24 ++------- lib/mv_web/emails/join_email.ex | 54 +++++++++++++++++++ 4 files changed, 61 insertions(+), 65 deletions(-) create mode 100644 lib/mv_web/emails/join_email.ex diff --git a/lib/mv_web/emails/join_already_member_email.ex b/lib/mv_web/emails/join_already_member_email.ex index fa309d8..a7ea54f 100644 --- a/lib/mv_web/emails/join_already_member_email.ex +++ b/lib/mv_web/emails/join_already_member_email.ex @@ -5,15 +5,9 @@ defmodule MvWeb.Emails.JoinAlreadyMemberEmail do Used for anti-enumeration: the UI shows the same success message; only the email informs the recipient. Uses the unified email layout. """ - use Phoenix.Swoosh, - view: MvWeb.EmailsView, - layout: {MvWeb.EmailLayoutView, "layout.html"} - - use MvWeb, :verified_routes - import Swoosh.Email use Gettext, backend: MvWeb.Gettext, otp_app: :mv - alias Mv.Mailer + alias MvWeb.Emails.JoinEmail @doc """ Sends the "already a member" notice to the given address. @@ -23,20 +17,6 @@ defmodule MvWeb.Emails.JoinAlreadyMemberEmail do def send(email_address) when is_binary(email_address) do subject = gettext("Membership application – already a member") - assigns = %{ - subject: subject, - app_name: Mailer.mail_from() |> elem(0), - locale: Gettext.get_locale(MvWeb.Gettext) - } - - email = - new() - |> from(Mailer.mail_from()) - |> to(email_address) - |> subject(subject) - |> put_view(MvWeb.EmailsView) - |> render_body("join_already_member.html", assigns) - - Mailer.deliver(email, Mailer.smtp_config()) + JoinEmail.deliver(email_address, "join_already_member.html", subject) end end diff --git a/lib/mv_web/emails/join_already_pending_email.ex b/lib/mv_web/emails/join_already_pending_email.ex index 17dc487..8d0c680 100644 --- a/lib/mv_web/emails/join_already_pending_email.ex +++ b/lib/mv_web/emails/join_already_pending_email.ex @@ -6,15 +6,9 @@ defmodule MvWeb.Emails.JoinAlreadyPendingEmail do Used for anti-enumeration: the UI shows the same success message; only the email informs the recipient. Uses the unified email layout. """ - use Phoenix.Swoosh, - view: MvWeb.EmailsView, - layout: {MvWeb.EmailLayoutView, "layout.html"} - - use MvWeb, :verified_routes - import Swoosh.Email use Gettext, backend: MvWeb.Gettext, otp_app: :mv - alias Mv.Mailer + alias MvWeb.Emails.JoinEmail @doc """ Sends the "application already under review" notice to the given address. @@ -24,20 +18,6 @@ defmodule MvWeb.Emails.JoinAlreadyPendingEmail do def send(email_address) when is_binary(email_address) do subject = gettext("Membership application – already under review") - assigns = %{ - subject: subject, - app_name: Mailer.mail_from() |> elem(0), - locale: Gettext.get_locale(MvWeb.Gettext) - } - - email = - new() - |> from(Mailer.mail_from()) - |> to(email_address) - |> subject(subject) - |> put_view(MvWeb.EmailsView) - |> render_body("join_already_pending.html", assigns) - - Mailer.deliver(email, Mailer.smtp_config()) + JoinEmail.deliver(email_address, "join_already_pending.html", subject) end end diff --git a/lib/mv_web/emails/join_confirmation_email.ex b/lib/mv_web/emails/join_confirmation_email.ex index 08f4ad3..5578d16 100644 --- a/lib/mv_web/emails/join_confirmation_email.ex +++ b/lib/mv_web/emails/join_confirmation_email.ex @@ -2,15 +2,10 @@ defmodule MvWeb.Emails.JoinConfirmationEmail do @moduledoc """ Sends the join request confirmation email (double opt-in) using the unified email layout. """ - use Phoenix.Swoosh, - view: MvWeb.EmailsView, - layout: {MvWeb.EmailLayoutView, "layout.html"} - use MvWeb, :verified_routes - import Swoosh.Email use Gettext, backend: MvWeb.Gettext, otp_app: :mv - alias Mv.Mailer + alias MvWeb.Emails.JoinEmail @doc """ Sends the join confirmation email to the given address with the confirmation link. @@ -31,22 +26,9 @@ defmodule MvWeb.Emails.JoinConfirmationEmail do confirm_url = url(~p"/confirm_join/#{token}") subject = gettext("Confirm your membership request") - assigns = %{ + JoinEmail.deliver(email_address, "join_confirmation.html", subject, %{ confirm_url: confirm_url, - subject: subject, - app_name: Mailer.mail_from() |> elem(0), - locale: Gettext.get_locale(MvWeb.Gettext), resend: Keyword.get(opts, :resend, false) - } - - email = - new() - |> from(Mailer.mail_from()) - |> to(email_address) - |> subject(subject) - |> put_view(MvWeb.EmailsView) - |> render_body("join_confirmation.html", assigns) - - Mailer.deliver(email, Mailer.smtp_config()) + }) end end diff --git a/lib/mv_web/emails/join_email.ex b/lib/mv_web/emails/join_email.ex new file mode 100644 index 0000000..c837be5 --- /dev/null +++ b/lib/mv_web/emails/join_email.ex @@ -0,0 +1,54 @@ +defmodule MvWeb.Emails.JoinEmail do + @moduledoc """ + Shared build/deliver skeleton for the join-flow emails (confirmation, + already-a-member, already-pending). + + Each concrete join email supplies only its subject, template, and any + email-specific assigns; this module builds the Swoosh email with the unified + layout and delivers it via `Mailer.deliver/2` with `Mailer.smtp_config/0`, + preserving the `{:ok, email}` / `{:error, reason}` contract. + """ + use Phoenix.Swoosh, + view: MvWeb.EmailsView, + layout: {MvWeb.EmailLayoutView, "layout.html"} + + import Swoosh.Email + + alias Mv.Mailer + + @doc """ + Builds and delivers a join-flow email. + + - `email_address` - recipient address + - `template` - the EmailsView template to render (e.g. `"join_confirmation.html"`) + - `subject` - already-translated subject line + - `extra_assigns` - email-specific assigns merged on top of the common ones + (`subject`, `app_name`, `locale`) + + Returns `{:ok, email}` on success, `{:error, reason}` on delivery failure. + """ + @spec deliver(String.t(), String.t(), String.t(), map()) :: + {:ok, Swoosh.Email.t()} | {:error, term()} + def deliver(email_address, template, subject, extra_assigns \\ %{}) + when is_binary(email_address) and is_binary(template) and is_binary(subject) do + assigns = + Map.merge( + %{ + subject: subject, + app_name: Mailer.mail_from() |> elem(0), + locale: Gettext.get_locale(MvWeb.Gettext) + }, + extra_assigns + ) + + email = + new() + |> from(Mailer.mail_from()) + |> to(email_address) + |> subject(subject) + |> put_view(MvWeb.EmailsView) + |> render_body(template, assigns) + + Mailer.deliver(email, Mailer.smtp_config()) + end +end From ef70dd293590f2221f6a8d7eade4b49390b013c3 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:13:27 +0200 Subject: [PATCH 10/27] refactor(settings): unify JSONB single-field update between member-field changes --- .../setting/changes/jsonb_result.ex | 98 +++++++++++++++++++ .../changes/update_single_member_field.ex | 71 +++----------- .../update_single_member_field_visibility.ex | 68 +++---------- 3 files changed, 124 insertions(+), 113 deletions(-) create mode 100644 lib/membership/setting/changes/jsonb_result.ex diff --git a/lib/membership/setting/changes/jsonb_result.ex b/lib/membership/setting/changes/jsonb_result.ex new file mode 100644 index 0000000..183efed --- /dev/null +++ b/lib/membership/setting/changes/jsonb_result.ex @@ -0,0 +1,98 @@ +defmodule Mv.Membership.Setting.Changes.JsonbResult do + @moduledoc """ + Shared normalization for the JSONB column values returned by the atomic + single-member-field settings updates. + + PostgreSQL may return a JSONB column as an already-decoded map (atom or string + keys) or as a raw JSON string depending on the driver path. This helper + normalizes either form to a string-keyed map, returning `%{}` for unexpected + or undecodable input. + """ + + alias Ash.Error.Invalid + alias Ecto.Adapters.SQL + require Logger + + @doc """ + Runs an atomic single-statement JSONB settings UPDATE inside an after_action + and maps the RETURNING row back onto the settings struct. + + Shared by the single-member-field change modules, which differ only in the + SQL statement, its parameters, the row-to-settings mapping, and the error + labels. The three-branch result handling (row found / not found / SQL error) + is identical and lives here. + + Options: + - `:sql` - The UPDATE statement with a RETURNING clause (required). + - `:params` - The full parameter list for the statement (required). + - `:on_row` - 1-arity function mapping the RETURNING row (a list of column + values) to the updated settings struct (required). + - `:error_field` - Ash error field for the not-found / failure errors. + - `:not_found_message` - Error message when no row matched. + - `:error_message` - Error message when the SQL statement failed. + - `:log_message` - Log prefix written on SQL failure. + """ + @spec run_update(keyword()) :: + {:ok, map()} | {:error, Exception.t()} + def run_update(opts) do + sql = Keyword.fetch!(opts, :sql) + params = Keyword.fetch!(opts, :params) + on_row = Keyword.fetch!(opts, :on_row) + error_field = Keyword.fetch!(opts, :error_field) + not_found_message = Keyword.fetch!(opts, :not_found_message) + error_message = Keyword.fetch!(opts, :error_message) + log_message = Keyword.fetch!(opts, :log_message) + + case SQL.query(Mv.Repo, sql, params) do + {:ok, %{rows: [row | _]}} -> + {:ok, on_row.(row)} + + {:ok, %{rows: []}} -> + {:error, + Invalid.exception( + field: error_field, + message: not_found_message + )} + + {:error, error} -> + Logger.error("#{log_message}: #{inspect(error)}") + + {:error, + Invalid.exception( + field: error_field, + message: error_message + )} + end + end + + @doc """ + Normalizes a JSONB column value to a string-keyed map. + """ + @spec normalize(map() | binary() | any()) :: map() + def normalize(updated_jsonb) do + case updated_jsonb do + map when is_map(map) -> + Enum.reduce(map, %{}, fn + {k, v}, acc when is_atom(k) -> Map.put(acc, Atom.to_string(k), v) + {k, v}, acc -> Map.put(acc, k, v) + end) + + binary when is_binary(binary) -> + case Jason.decode(binary) do + {:ok, decoded} when is_map(decoded) -> + decoded + + {:ok, _} -> + %{} + + {:error, reason} -> + Logger.warning("Failed to decode JSONB: #{inspect(reason)}") + %{} + end + + _ -> + Logger.warning("Unexpected JSONB format: #{inspect(updated_jsonb)}") + %{} + end + end +end diff --git a/lib/membership/setting/changes/update_single_member_field.ex b/lib/membership/setting/changes/update_single_member_field.ex index e24860c..cb3ce7b 100644 --- a/lib/membership/setting/changes/update_single_member_field.ex +++ b/lib/membership/setting/changes/update_single_member_field.ex @@ -19,9 +19,7 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberField do """ use Ash.Resource.Change - alias Ash.Error.Invalid - alias Ecto.Adapters.SQL - require Logger + alias Mv.Membership.Setting.Changes.JsonbResult def change(changeset, _opts, _context) do with {:ok, field} <- get_and_validate_field(changeset), @@ -118,62 +116,21 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberField do uuid_binary = Ecto.UUID.dump!(settings.id) - case SQL.query(Mv.Repo, sql, [field, show_in_overview, required, uuid_binary]) do - {:ok, %{rows: [[updated_visibility, updated_required] | _]}} -> - vis = normalize_jsonb_result(updated_visibility) - req = normalize_jsonb_result(updated_required) - - updated_settings = %{ + JsonbResult.run_update( + sql: sql, + params: [field, show_in_overview, required, uuid_binary], + on_row: fn [updated_visibility, updated_required | _] -> + %{ settings - | member_field_visibility: vis, - member_field_required: req + | member_field_visibility: JsonbResult.normalize(updated_visibility), + member_field_required: JsonbResult.normalize(updated_required) } - - {:ok, updated_settings} - - {:ok, %{rows: []}} -> - {:error, - Invalid.exception( - field: :member_field_required, - message: "Settings not found" - )} - - {:error, error} -> - Logger.error("Failed to atomically update member field settings: #{inspect(error)}") - - {:error, - Invalid.exception( - field: :member_field_required, - message: "Failed to update member field settings" - )} - end + end, + error_field: :member_field_required, + not_found_message: "Settings not found", + error_message: "Failed to update member field settings", + log_message: "Failed to atomically update member field settings" + ) end) end - - defp normalize_jsonb_result(updated_jsonb) do - case updated_jsonb do - map when is_map(map) -> - Enum.reduce(map, %{}, fn - {k, v}, acc when is_atom(k) -> Map.put(acc, Atom.to_string(k), v) - {k, v}, acc -> Map.put(acc, k, v) - end) - - binary when is_binary(binary) -> - case Jason.decode(binary) do - {:ok, decoded} when is_map(decoded) -> - decoded - - {:ok, _} -> - %{} - - {:error, reason} -> - Logger.warning("Failed to decode JSONB: #{inspect(reason)}") - %{} - end - - _ -> - Logger.warning("Unexpected JSONB format: #{inspect(updated_jsonb)}") - %{} - end - end end diff --git a/lib/membership/setting/changes/update_single_member_field_visibility.ex b/lib/membership/setting/changes/update_single_member_field_visibility.ex index e047cdf..f08bc21 100644 --- a/lib/membership/setting/changes/update_single_member_field_visibility.ex +++ b/lib/membership/setting/changes/update_single_member_field_visibility.ex @@ -19,9 +19,7 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberFieldVisibility do """ use Ash.Resource.Change - alias Ash.Error.Invalid - alias Ecto.Adapters.SQL - require Logger + alias Mv.Membership.Setting.Changes.JsonbResult def change(changeset, _opts, _context) do with {:ok, field} <- get_and_validate_field(changeset), @@ -106,59 +104,17 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberFieldVisibility do # Convert UUID string to binary for PostgreSQL uuid_binary = Ecto.UUID.dump!(settings.id) - case SQL.query(Mv.Repo, sql, [field, show_in_overview, uuid_binary]) do - {:ok, %{rows: [[updated_jsonb] | _]}} -> - updated_visibility = normalize_jsonb_result(updated_jsonb) - - # Update the settings struct with the new visibility - updated_settings = %{settings | member_field_visibility: updated_visibility} - {:ok, updated_settings} - - {:ok, %{rows: []}} -> - {:error, - Invalid.exception( - field: :member_field_visibility, - message: "Settings not found" - )} - - {:error, error} -> - Logger.error("Failed to atomically update member_field_visibility: #{inspect(error)}") - - {:error, - Invalid.exception( - field: :member_field_visibility, - message: "Failed to update visibility" - )} - end + JsonbResult.run_update( + sql: sql, + params: [field, show_in_overview, uuid_binary], + on_row: fn [updated_jsonb | _] -> + %{settings | member_field_visibility: JsonbResult.normalize(updated_jsonb)} + end, + error_field: :member_field_visibility, + not_found_message: "Settings not found", + error_message: "Failed to update visibility", + log_message: "Failed to atomically update member_field_visibility" + ) end) end - - defp normalize_jsonb_result(updated_jsonb) do - case updated_jsonb do - map when is_map(map) -> - # Convert atom keys to strings if needed - Enum.reduce(map, %{}, fn - {k, v}, acc when is_atom(k) -> Map.put(acc, Atom.to_string(k), v) - {k, v}, acc -> Map.put(acc, k, v) - end) - - binary when is_binary(binary) -> - case Jason.decode(binary) do - {:ok, decoded} when is_map(decoded) -> - decoded - - # Not a map after decode - {:ok, _} -> - %{} - - {:error, reason} -> - Logger.warning("Failed to decode JSONB: #{inspect(reason)}") - %{} - end - - _ -> - Logger.warning("Unexpected JSONB format: #{inspect(updated_jsonb)}") - %{} - end - end end From 0cf27c95ca39584b243ba2e0654f488cd5350eb7 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:16:54 +0200 Subject: [PATCH 11/27] refactor(membership-fees): fold cycle-generation run/0 into run/1 --- .../membership_fees/cycle_generation_job.ex | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/lib/mv/membership_fees/cycle_generation_job.ex b/lib/mv/membership_fees/cycle_generation_job.ex index b38886c..1c5551f 100644 --- a/lib/mv/membership_fees/cycle_generation_job.ex +++ b/lib/mv/membership_fees/cycle_generation_job.ex @@ -59,27 +59,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do """ @spec run() :: {:ok, CycleGenerator.results_summary()} | {:error, Ash.Error.t()} - def run do - Logger.info("Starting membership fee cycle generation job") - start_time = System.monotonic_time(:millisecond) - - result = CycleGenerator.generate_cycles_for_all_members() - - elapsed = System.monotonic_time(:millisecond) - start_time - - case result do - {:ok, stats} -> - Logger.info( - "Cycle generation completed in #{elapsed}ms: #{stats.success} success, #{stats.failed} failed, #{stats.total} total" - ) - - result - - {:error, reason} -> - Logger.error("Cycle generation failed: #{inspect(reason)}") - result - end - end + def run, do: run([]) @doc """ Runs cycle generation with custom options. From ea105186a5f64cc08df3e05edc9430a9e6892cbc Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:20:18 +0200 Subject: [PATCH 12/27] refactor(vereinfacht): reuse EmailSync.Loader for linked-member lookup --- .../sync_linked_member_after_user_change.ex | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex b/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex index 4465690..79f8c47 100644 --- a/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex +++ b/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex @@ -10,10 +10,7 @@ defmodule Mv.Vereinfacht.Changes.SyncLinkedMemberAfterUserChange do use Ash.Resource.Change require Logger - alias Mv.Helpers - alias Mv.Helpers.SystemActor - alias Mv.Membership - alias Mv.Membership.Member + alias Mv.EmailSync.Loader @impl true def change(changeset, _opts, _context) do @@ -32,7 +29,7 @@ defmodule Mv.Vereinfacht.Changes.SyncLinkedMemberAfterUserChange do end defp sync_linked_member_after_transaction(_changeset, {:ok, user}) do - case load_linked_member(user) do + case Loader.get_linked_member(user) do nil -> {:ok, user} @@ -55,17 +52,4 @@ defmodule Mv.Vereinfacht.Changes.SyncLinkedMemberAfterUserChange do end defp sync_linked_member_after_transaction(_changeset, result), do: result - - defp load_linked_member(%{member_id: nil}), do: nil - defp load_linked_member(%{member_id: ""}), do: nil - - defp load_linked_member(user) do - actor = SystemActor.get_system_actor() - opts = Helpers.ash_actor_opts(actor) - - case Ash.get(Member, user.member_id, [domain: Membership] ++ opts) do - {:ok, %Member{} = member} -> member - _ -> nil - end - end end From 18bf4dab2ba639032daa7186ffef9dc200499812 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:23:52 +0200 Subject: [PATCH 13/27] refactor(web): use canonical DateFormatter for all date display --- lib/mv_web/helpers/membership_fee_helpers.ex | 10 +++------- lib/mv_web/live/member_live/show.ex | 15 ++++----------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/lib/mv_web/helpers/membership_fee_helpers.ex b/lib/mv_web/helpers/membership_fee_helpers.ex index e8a2ce8..b9ebd71 100644 --- a/lib/mv_web/helpers/membership_fee_helpers.ex +++ b/lib/mv_web/helpers/membership_fee_helpers.ex @@ -11,6 +11,7 @@ defmodule MvWeb.Helpers.MembershipFeeHelpers do alias Mv.Membership.Member alias Mv.MembershipFees.CalendarCycles alias Mv.MembershipFees.MembershipFeeCycle + alias MvWeb.Helpers.DateFormatter @doc """ Formats a decimal amount as currency string. @@ -98,8 +99,8 @@ defmodule MvWeb.Helpers.MembershipFeeHelpers do @spec format_cycle_range(Date.t(), :monthly | :quarterly | :half_yearly | :yearly) :: String.t() def format_cycle_range(cycle_start, interval) do cycle_end = CalendarCycles.calculate_cycle_end(cycle_start, interval) - start_str = format_date(cycle_start) - end_str = format_date(cycle_end) + start_str = DateFormatter.format_date(cycle_start) + end_str = DateFormatter.format_date(cycle_end) "#{start_str} - #{end_str}" end @@ -248,9 +249,4 @@ defmodule MvWeb.Helpers.MembershipFeeHelpers do def status_icon(:paid), do: "hero-check-circle" def status_icon(:unpaid), do: "hero-x-circle" def status_icon(:suspended), do: "hero-pause-circle" - - # Private helper function for date formatting - defp format_date(%Date{} = date) do - Calendar.strftime(date, "%d.%m.%Y") - end end diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index 521bb38..f174859 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -28,6 +28,7 @@ defmodule MvWeb.MemberLive.Show do alias Mv.Membership.CustomFieldValue alias Mv.Membership.Member, as: MemberResource alias Mv.Vereinfacht.Client, as: VereinfachtClient + alias MvWeb.Helpers.DateFormatter alias MvWeb.Helpers.MemberHelpers alias MvWeb.Helpers.MembershipFeeHelpers alias Phoenix.HTML.Engine, as: HTMLEngine @@ -159,12 +160,12 @@ defmodule MvWeb.MemberLive.Show do
<.data_field label={gettext("Join Date")} - value={format_date(@member.join_date)} + value={DateFormatter.format_date(@member.join_date)} class="w-28" /> <.data_field label={gettext("Exit Date")} - value={format_date(@member.exit_date)} + value={DateFormatter.format_date(@member.exit_date)} class="w-28" />
@@ -719,14 +720,6 @@ defmodule MvWeb.MemberLive.Show do end end - defp format_date(nil), do: nil - - defp format_date(%Date{} = date) do - Calendar.strftime(date, "%d.%m.%Y") - end - - defp format_date(date), do: to_string(date) - # Finds custom field value for a given custom field id # Returns the value (not the CustomFieldValue struct) or nil defp find_custom_field_value(nil, _custom_field_id), do: nil @@ -760,7 +753,7 @@ defmodule MvWeb.MemberLive.Show do end defp format_custom_field_value(%Date{} = date, :date) do - Calendar.strftime(date, "%d.%m.%Y") + DateFormatter.format_date(date) end defp format_custom_field_value(value, :email) when is_binary(value) do From 3f073d43659165851400e7d8262c9f2926c0be09 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:30:56 +0200 Subject: [PATCH 14/27] refactor(membership-fees): share fee-type delete handling between LiveViews --- lib/mv_web/helpers/membership_fee_helpers.ex | 67 +++++++++++++++++++ .../live/membership_fee_settings_live.ex | 49 +------------- .../live/membership_fee_type_live/index.ex | 48 +------------ priv/gettext/default.pot | 15 ++--- 4 files changed, 74 insertions(+), 105 deletions(-) diff --git a/lib/mv_web/helpers/membership_fee_helpers.ex b/lib/mv_web/helpers/membership_fee_helpers.ex index b9ebd71..1fddda8 100644 --- a/lib/mv_web/helpers/membership_fee_helpers.ex +++ b/lib/mv_web/helpers/membership_fee_helpers.ex @@ -9,8 +9,10 @@ defmodule MvWeb.Helpers.MembershipFeeHelpers do use Gettext, backend: MvWeb.Gettext alias Mv.Membership.Member + alias Mv.MembershipFees alias Mv.MembershipFees.CalendarCycles alias Mv.MembershipFees.MembershipFeeCycle + alias Mv.MembershipFees.MembershipFeeType alias MvWeb.Helpers.DateFormatter @doc """ @@ -249,4 +251,69 @@ defmodule MvWeb.Helpers.MembershipFeeHelpers do def status_icon(:paid), do: "hero-check-circle" def status_icon(:unpaid), do: "hero-x-circle" def status_icon(:suspended), do: "hero-pause-circle" + + @doc """ + Handles a membership-fee-type "delete" event for the fee-type list and the + fee-settings LiveViews. + + Loads the fee type, attempts to destroy it, and returns the updated socket + with the matching flash. On success the deleted type and its member count are + dropped from the `:membership_fee_types` and `:member_counts` assigns. The + NotFound, Forbidden (delete and access), and generic error branches preserve + the exact messages both views used before they shared this block. + """ + @spec delete_fee_type(Phoenix.LiveView.Socket.t(), String.t(), term()) :: + {:noreply, Phoenix.LiveView.Socket.t()} + def delete_fee_type(socket, id, actor) do + case Ash.get(MembershipFeeType, id, domain: MembershipFees, actor: actor) do + {:ok, fee_type} -> + destroy_fee_type(socket, fee_type, id, actor) + + {:error, %Ash.Error.Query.NotFound{}} -> + {:noreply, + Phoenix.LiveView.put_flash(socket, :error, gettext("Membership fee type not found"))} + + {:error, %Ash.Error.Forbidden{}} -> + {:noreply, + Phoenix.LiveView.put_flash( + socket, + :error, + gettext("You do not have permission to access this membership fee type") + )} + + {:error, error} -> + {:noreply, Phoenix.LiveView.put_flash(socket, :error, fee_error_message(error))} + end + end + + defp destroy_fee_type(socket, fee_type, id, actor) do + case Ash.destroy(fee_type, domain: MembershipFees, actor: actor) do + :ok -> + updated_types = Enum.reject(socket.assigns.membership_fee_types, &(&1.id == id)) + updated_counts = Map.delete(socket.assigns.member_counts, id) + + {:noreply, + socket + |> Phoenix.Component.assign(:membership_fee_types, updated_types) + |> Phoenix.Component.assign(:member_counts, updated_counts) + |> Phoenix.LiveView.put_flash(:success, gettext("Membership fee type deleted"))} + + {:error, %Ash.Error.Forbidden{}} -> + {:noreply, + Phoenix.LiveView.put_flash( + socket, + :error, + gettext("You do not have permission to delete this membership fee type") + )} + + {:error, error} -> + {:noreply, Phoenix.LiveView.put_flash(socket, :error, fee_error_message(error))} + end + end + + defp fee_error_message(%Ash.Error.Invalid{} = error) do + Enum.map_join(error.errors, ", ", fn e -> e.message end) + end + + defp fee_error_message(_error), do: gettext("An error occurred") end diff --git a/lib/mv_web/live/membership_fee_settings_live.ex b/lib/mv_web/live/membership_fee_settings_live.ex index 4df6608..56de3b5 100644 --- a/lib/mv_web/live/membership_fee_settings_live.ex +++ b/lib/mv_web/live/membership_fee_settings_live.ex @@ -15,7 +15,6 @@ defmodule MvWeb.MembershipFeeSettingsLive do alias Mv.Membership alias Mv.Membership.Member - alias Mv.MembershipFees alias Mv.MembershipFees.MembershipFeeType alias MvWeb.Helpers.MembershipFeeHelpers @@ -92,47 +91,7 @@ defmodule MvWeb.MembershipFeeSettingsLive do @impl true def handle_event("delete", %{"id" => id}, socket) do - actor = current_actor(socket) - - case Ash.get(MembershipFeeType, id, domain: MembershipFees, actor: actor) do - {:ok, fee_type} -> - case Ash.destroy(fee_type, domain: MembershipFees, actor: actor) do - :ok -> - updated_types = Enum.reject(socket.assigns.membership_fee_types, &(&1.id == id)) - updated_counts = Map.delete(socket.assigns.member_counts, id) - - {:noreply, - socket - |> assign(:membership_fee_types, updated_types) - |> assign(:member_counts, updated_counts) - |> put_flash(:success, gettext("Membership fee type deleted"))} - - {:error, %Ash.Error.Forbidden{}} -> - {:noreply, - put_flash( - socket, - :error, - gettext("You do not have permission to delete this membership fee type") - )} - - {:error, error} -> - {:noreply, put_flash(socket, :error, format_error(error))} - end - - {:error, %Ash.Error.Query.NotFound{}} -> - {:noreply, put_flash(socket, :error, gettext("Membership fee type not found"))} - - {:error, %Ash.Error.Forbidden{}} -> - {:noreply, - put_flash( - socket, - :error, - gettext("You do not have permission to access this membership fee type") - )} - - {:error, error} -> - {:noreply, put_flash(socket, :error, format_error(error))} - end + MembershipFeeHelpers.delete_fee_type(socket, id, current_actor(socket)) end @impl true @@ -465,12 +424,6 @@ defmodule MvWeb.MembershipFeeSettingsLive do Map.get(member_counts, fee_type.id, 0) end - defp format_error(%Ash.Error.Invalid{} = error) do - Enum.map_join(error.errors, ", ", fn e -> e.message end) - end - - defp format_error(_error), do: gettext("An error occurred") - defp assign_form(%{assigns: %{settings: settings}} = socket) do form = AshPhoenix.Form.for_update( diff --git a/lib/mv_web/live/membership_fee_type_live/index.ex b/lib/mv_web/live/membership_fee_type_live/index.ex index 1d51ce1..b6290e2 100644 --- a/lib/mv_web/live/membership_fee_type_live/index.ex +++ b/lib/mv_web/live/membership_fee_type_live/index.ex @@ -141,47 +141,7 @@ defmodule MvWeb.MembershipFeeTypeLive.Index do @impl true def handle_event("delete", %{"id" => id}, socket) do - actor = current_actor(socket) - - case Ash.get(MembershipFeeType, id, domain: MembershipFees, actor: actor) do - {:ok, fee_type} -> - case Ash.destroy(fee_type, domain: MembershipFees, actor: actor) do - :ok -> - updated_types = Enum.reject(socket.assigns.membership_fee_types, &(&1.id == id)) - updated_counts = Map.delete(socket.assigns.member_counts, id) - - {:noreply, - socket - |> assign(:membership_fee_types, updated_types) - |> assign(:member_counts, updated_counts) - |> put_flash(:success, gettext("Membership fee type deleted"))} - - {:error, %Ash.Error.Forbidden{}} -> - {:noreply, - put_flash( - socket, - :error, - gettext("You do not have permission to delete this membership fee type") - )} - - {:error, error} -> - {:noreply, put_flash(socket, :error, format_error(error))} - end - - {:error, %Ash.Error.Query.NotFound{}} -> - {:noreply, put_flash(socket, :error, gettext("Membership fee type not found"))} - - {:error, %Ash.Error.Forbidden{} = _error} -> - {:noreply, - put_flash( - socket, - :error, - gettext("You do not have permission to access this membership fee type") - )} - - {:error, error} -> - {:noreply, put_flash(socket, :error, format_error(error))} - end + MembershipFeeHelpers.delete_fee_type(socket, id, current_actor(socket)) end # Helper functions @@ -215,12 +175,6 @@ defmodule MvWeb.MembershipFeeTypeLive.Index do Map.get(member_counts, fee_type.id, 0) end - defp format_error(%Ash.Error.Invalid{} = error) do - Enum.map_join(error.errors, ", ", fn e -> e.message end) - end - - defp format_error(_error), do: gettext("An error occurred") - # Info card explaining the membership fee type concept defp info_card(assigns) do ~H""" diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 3c21f67..4c9a10a 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -193,8 +193,7 @@ msgid "An account with this email already exists. Please verify your password to msgstr "" #: lib/mv_web/helpers/ash_error_helpers.ex -#: lib/mv_web/live/membership_fee_settings_live.ex -#: lib/mv_web/live/membership_fee_type_live/index.ex +#: lib/mv_web/helpers/membership_fee_helpers.ex #: lib/mv_web/live/role_live/helpers.ex #, elixir-autogen, elixir-format msgid "An error occurred" @@ -2289,14 +2288,12 @@ msgstr "" msgid "Membership fee start" msgstr "" -#: lib/mv_web/live/membership_fee_settings_live.ex -#: lib/mv_web/live/membership_fee_type_live/index.ex +#: lib/mv_web/helpers/membership_fee_helpers.ex #, elixir-autogen, elixir-format msgid "Membership fee type deleted" msgstr "" -#: lib/mv_web/live/membership_fee_settings_live.ex -#: lib/mv_web/live/membership_fee_type_live/index.ex +#: lib/mv_web/helpers/membership_fee_helpers.ex #, elixir-autogen, elixir-format msgid "Membership fee type not found" msgstr "" @@ -3956,8 +3953,7 @@ msgstr "" msgid "You do not have permission to %{action} members." msgstr "" -#: lib/mv_web/live/membership_fee_settings_live.ex -#: lib/mv_web/live/membership_fee_type_live/index.ex +#: lib/mv_web/helpers/membership_fee_helpers.ex #, elixir-autogen, elixir-format msgid "You do not have permission to access this membership fee type" msgstr "" @@ -3973,8 +3969,7 @@ msgstr "" msgid "You do not have permission to delete this member" msgstr "" -#: lib/mv_web/live/membership_fee_settings_live.ex -#: lib/mv_web/live/membership_fee_type_live/index.ex +#: lib/mv_web/helpers/membership_fee_helpers.ex #, elixir-autogen, elixir-format msgid "You do not have permission to delete this membership fee type" msgstr "" From fe534319ee6a8d443c1d4f21563cdb3145ae714d Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:34:35 +0200 Subject: [PATCH 15/27] style: order module directives per StrictModuleLayout --- lib/accounts/user.ex | 3 ++- .../user/validations/oidc_email_collision.ex | 3 ++- lib/membership/email.ex | 4 ++++ lib/membership/group.ex | 3 ++- lib/membership/member.ex | 5 +++-- lib/membership/membership.ex | 4 +++- lib/membership/setting.ex | 4 ++-- .../tasks/join_requests.cleanup_expired.ex | 7 +++---- .../send_new_user_confirmation_email.ex | 5 +++-- .../user/senders/send_password_reset_email.ex | 5 +++-- lib/mv/authorization/actor.ex | 4 ++-- lib/mv/authorization/checks/has_permission.ex | 5 ++++- lib/mv/email_sync/helpers.ex | 3 ++- lib/mv/helpers/system_actor.ex | 4 ++-- lib/mv/mailer.ex | 5 +++-- lib/mv/membership/import/column_resolver.ex | 4 ++-- lib/mv/membership/import/member_csv.ex | 19 ++++++++----------- lib/mv/membership/member_export/build.ex | 3 ++- lib/mv/membership/members_pdf.ex | 4 ++-- lib/mv/membership_fees/cycle_generator.ex | 14 +++++++------- lib/mv/release.ex | 4 ++-- lib/mv/statistics.ex | 6 +++--- .../sync_linked_member_after_user_change.ex | 3 ++- lib/mv/vereinfacht/vereinfacht.ex | 4 +++- lib/mv_web/components/table_components.ex | 3 ++- .../controllers/import_template_controller.ex | 4 ++-- .../controllers/member_export_controller.ex | 6 +++--- .../member_pdf_export_controller.ex | 7 +++---- .../live/auth/link_oidc_account_live.ex | 5 +++-- lib/mv_web/live/global_settings_live.ex | 3 ++- lib/mv_web/live/group_live/show.ex | 6 +++--- lib/mv_web/live/import_live/components.ex | 4 ++-- lib/mv_web/live/join_request_live/index.ex | 6 +++--- lib/mv_web/live/join_request_live/show.ex | 6 +++--- lib/mv_web/live/member_live/form.ex | 3 ++- lib/mv_web/live/member_live/index.ex | 5 +++-- .../live/member_live/index/date_filter.ex | 3 ++- .../show/membership_fees_component.ex | 5 +++-- .../live/membership_fee_settings_live.ex | 4 ++-- .../live/membership_fee_type_live/form.ex | 4 ++-- .../live/membership_fee_type_live/index.ex | 4 ++-- lib/mv_web/live/role_live/form.ex | 4 ++-- lib/mv_web/live/role_live/index.ex | 4 ++-- lib/mv_web/live/role_live/show.ex | 6 +++--- lib/mv_web/live/statistics_live.ex | 5 +++-- lib/mv_web/live/user_live/form.ex | 8 ++++---- lib/mv_web/live_user_auth.ex | 3 ++- .../group_database_constraints_test.exs | 3 ++- test/membership/group_test.exs | 3 ++- .../join_request_approval_domain_test.exs | 3 ++- test/membership/join_request_test.exs | 3 ++- test/membership/member_group_test.exs | 3 ++- .../member_groups_relationship_test.exs | 3 ++- .../has_permission_fail_closed_test.exs | 4 ++-- test/mv/statistics_test.exs | 3 ++- .../member_filter_component_test.exs | 3 ++- .../helpers/membership_fee_helpers_test.exs | 4 ++-- .../live/custom_field_live/deletion_test.exs | 3 ++- .../live/custom_field_live/form_test.exs | 3 ++- test/mv_web/live/group_live/form_test.exs | 3 ++- test/mv_web/live/group_live/index_test.exs | 3 ++- .../live/group_live/integration_test.exs | 8 +++++--- .../group_live/show_accessibility_test.exs | 3 ++- .../live/group_live/show_add_member_test.exs | 5 +++-- .../show_add_remove_members_test.exs | 3 ++- .../group_live/show_authorization_test.exs | 3 ++- .../live/group_live/show_integration_test.exs | 3 ++- .../group_live/show_member_search_test.exs | 3 ++- .../group_live/show_remove_member_test.exs | 3 ++- test/mv_web/live/group_live/show_test.exs | 6 ++++-- .../live/member_live/deactivate_test.exs | 3 ++- test/mv_web/live/role_live/show_test.exs | 6 ++++-- test/mv_web/live/user_live/show_test.exs | 6 ++++-- ...index_custom_fields_accessibility_test.exs | 3 ++- .../index_custom_fields_display_test.exs | 3 ++- .../index_custom_fields_edge_cases_test.exs | 3 ++- .../index_custom_fields_sorting_test.exs | 3 ++- .../index_field_visibility_test.exs | 3 ++- .../index_groups_accessibility_test.exs | 3 ++- .../member_live/index_groups_display_test.exs | 3 ++- .../member_live/index_groups_filter_test.exs | 3 ++- .../index_groups_integration_test.exs | 3 ++- .../index_groups_performance_test.exs | 3 ++- .../member_live/index_groups_sorting_test.exs | 3 ++- .../index_groups_url_params_test.exs | 3 ++- test/mv_web/member_live/index_test.exs | 3 ++- .../member_live/show_groups_display_test.exs | 6 ++++-- test/mv_web/member_live/show_test.exs | 6 ++++-- 88 files changed, 225 insertions(+), 155 deletions(-) diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index 0127796..f7f59ff 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -8,7 +8,6 @@ defmodule Mv.Accounts.User do extensions: [AshAuthentication], authorizers: [Ash.Policy.Authorizer] - require Ash.Query import Ash.Expr alias Ash.Resource.Preparation.Builtins @@ -16,6 +15,8 @@ defmodule Mv.Accounts.User do alias Mv.Helpers.SystemActor alias Mv.OidcRoleSync + require Ash.Query + postgres do table "users" repo Mv.Repo diff --git a/lib/accounts/user/validations/oidc_email_collision.ex b/lib/accounts/user/validations/oidc_email_collision.ex index 7ae8510..87e8250 100644 --- a/lib/accounts/user/validations/oidc_email_collision.ex +++ b/lib/accounts/user/validations/oidc_email_collision.ex @@ -24,12 +24,13 @@ defmodule Mv.Accounts.User.Validations.OidcEmailCollision do - Allow (new user will be created) """ use Ash.Resource.Validation - require Logger alias Mv.Accounts.User alias Mv.Accounts.User.Errors.PasswordVerificationRequired alias Mv.Helpers.SystemActor + require Logger + @impl true def init(opts), do: {:ok, opts} diff --git a/lib/membership/email.ex b/lib/membership/email.ex index 730ccd7..4dfbcc8 100644 --- a/lib/membership/email.ex +++ b/lib/membership/email.ex @@ -38,6 +38,10 @@ defmodule Mv.Membership.Email do @min_length 5 @max_length 254 + # These compile-time constants are referenced by the `use` options below, so + # they must be declared first; StrictModuleLayout cannot be satisfied by + # reordering here without breaking the macro expansion. + # credo:disable-for-next-line Credo.Check.Readability.StrictModuleLayout use Ash.Type.NewType, subtype_of: :string, constraints: [ diff --git a/lib/membership/group.ex b/lib/membership/group.ex index d468166..fcfe077 100644 --- a/lib/membership/group.ex +++ b/lib/membership/group.ex @@ -39,9 +39,10 @@ defmodule Mv.Membership.Group do data_layer: AshPostgres.DataLayer, authorizers: [Ash.Policy.Authorizer] - require Ash.Query alias Mv.Helpers alias Mv.Helpers.SystemActor + + require Ash.Query require Logger postgres do diff --git a/lib/membership/member.ex b/lib/membership/member.ex index cddc23f..0f67a00 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -37,9 +37,9 @@ defmodule Mv.Membership.Member do data_layer: AshPostgres.DataLayer, authorizers: [Ash.Policy.Authorizer] - import Bitwise - require Ash.Query import Ash.Expr + import Bitwise + alias Ecto.Adapters.SQL, as: EctoSQL alias Mv.Helpers alias Mv.Helpers.SystemActor @@ -49,6 +49,7 @@ defmodule Mv.Membership.Member do alias Mv.MembershipFees.MembershipFeeCycle alias Mv.Repo + require Ash.Query require Logger @typedoc "An `Mv.Membership.Member` resource record." diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 72be69b..2a5b17f 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -26,13 +26,15 @@ defmodule Mv.Membership do use Ash.Domain, extensions: [AshAdmin.Domain, AshPhoenix] - require Ash.Query import Ash.Expr + alias Ash.Error.Query.NotFound, as: NotFoundError alias Mv.Helpers.SystemActor alias Mv.Membership.JoinRequest alias Mv.Membership.Member alias Mv.Membership.SettingsCache + + require Ash.Query require Logger admin do diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index 83c5c8b..f319685 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -65,12 +65,12 @@ defmodule Mv.Membership.Setting do data_layer: AshPostgres.DataLayer, primary_read_warning?: false + alias Ash.Resource.Info, as: ResourceInfo + # Used in join_form_field_ids validation (compile-time to avoid recompiling regex and list on every validation) @uuid_pattern ~r/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i @valid_join_form_member_fields Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) - alias Ash.Resource.Info, as: ResourceInfo - postgres do table "settings" repo Mv.Repo diff --git a/lib/mix/tasks/join_requests.cleanup_expired.ex b/lib/mix/tasks/join_requests.cleanup_expired.ex index 0d1823c..0cc8a8b 100644 --- a/lib/mix/tasks/join_requests.cleanup_expired.ex +++ b/lib/mix/tasks/join_requests.cleanup_expired.ex @@ -1,4 +1,5 @@ defmodule Mix.Tasks.JoinRequests.CleanupExpired do + @shortdoc "Deletes join requests in pending_confirmation with expired confirmation token" @moduledoc """ Hard-deletes JoinRequests in status `pending_confirmation` whose confirmation link has expired. @@ -16,12 +17,10 @@ defmodule Mix.Tasks.JoinRequests.CleanupExpired do """ use Mix.Task - require Ash.Query - require Logger - alias Mv.Membership.JoinRequest - @shortdoc "Deletes join requests in pending_confirmation with expired confirmation token" + require Ash.Query + require Logger @impl Mix.Task def run(_args) do diff --git a/lib/mv/accounts/user/senders/send_new_user_confirmation_email.ex b/lib/mv/accounts/user/senders/send_new_user_confirmation_email.ex index 7312b91..30dd381 100644 --- a/lib/mv/accounts/user/senders/send_new_user_confirmation_email.ex +++ b/lib/mv/accounts/user/senders/send_new_user_confirmation_email.ex @@ -13,13 +13,14 @@ defmodule Mv.Accounts.User.Senders.SendNewUserConfirmationEmail do layout: {MvWeb.EmailLayoutView, "layout.html"} use MvWeb, :verified_routes - import Swoosh.Email use Gettext, backend: MvWeb.Gettext, otp_app: :mv - require Logger + import Swoosh.Email alias Mv.Mailer + require Logger + @doc """ Sends a confirmation email to a new user. diff --git a/lib/mv/accounts/user/senders/send_password_reset_email.ex b/lib/mv/accounts/user/senders/send_password_reset_email.ex index e276e20..8527f04 100644 --- a/lib/mv/accounts/user/senders/send_password_reset_email.ex +++ b/lib/mv/accounts/user/senders/send_password_reset_email.ex @@ -13,13 +13,14 @@ defmodule Mv.Accounts.User.Senders.SendPasswordResetEmail do layout: {MvWeb.EmailLayoutView, "layout.html"} use MvWeb, :verified_routes - import Swoosh.Email use Gettext, backend: MvWeb.Gettext, otp_app: :mv - require Logger + import Swoosh.Email alias Mv.Mailer + require Logger + @doc """ Sends a password reset email to a user. diff --git a/lib/mv/authorization/actor.ex b/lib/mv/authorization/actor.ex index edc6b8b..b7f8844 100644 --- a/lib/mv/authorization/actor.ex +++ b/lib/mv/authorization/actor.ex @@ -49,10 +49,10 @@ defmodule Mv.Authorization.Actor do adds complexity and potential for inconsistency. """ - require Logger - alias Mv.Helpers.SystemActor + require Logger + @doc """ Ensures the actor (User) has their `:role` relationship loaded. diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index a11bf2e..94981f7 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -79,10 +79,13 @@ defmodule Mv.Authorization.Checks.HasPermission do """ use Ash.Policy.Check - require Ash.Query + import Ash.Expr + alias Mv.Authorization.Actor alias Mv.Authorization.PermissionSets + + require Ash.Query require Logger @impl true diff --git a/lib/mv/email_sync/helpers.ex b/lib/mv/email_sync/helpers.ex index 7feaf57..72f586a 100644 --- a/lib/mv/email_sync/helpers.ex +++ b/lib/mv/email_sync/helpers.ex @@ -6,9 +6,10 @@ defmodule Mv.EmailSync.Helpers do provides clean abstractions for email updates within transactions. """ - require Logger import Ecto.Changeset + require Logger + @doc """ Extracts the record from an Ash action result. diff --git a/lib/mv/helpers/system_actor.ex b/lib/mv/helpers/system_actor.ex index 7b86a3c..8070138 100644 --- a/lib/mv/helpers/system_actor.ex +++ b/lib/mv/helpers/system_actor.ex @@ -48,10 +48,10 @@ defmodule Mv.Helpers.SystemActor do use Agent - require Ash.Query - alias Mv.Config + require Ash.Query + @doc """ Starts the SystemActor Agent. diff --git a/lib/mv/mailer.ex b/lib/mv/mailer.ex index 1e55b6e..98697ff 100644 --- a/lib/mv/mailer.ex +++ b/lib/mv/mailer.ex @@ -27,11 +27,12 @@ defmodule Mv.Mailer do ENV takes priority over Settings (same pattern as OIDC and Vereinfacht). """ use Swoosh.Mailer, otp_app: :mv - - import Swoosh.Email use Gettext, backend: MvWeb.Gettext, otp_app: :mv + import Swoosh.Email + alias Mv.Smtp.ConfigBuilder + require Logger # Simple format check for test-email recipient only (e.g. allows a@b.c). Not for strict RFC validation. diff --git a/lib/mv/membership/import/column_resolver.ex b/lib/mv/membership/import/column_resolver.ex index 2edb540..f034b7a 100644 --- a/lib/mv/membership/import/column_resolver.ex +++ b/lib/mv/membership/import/column_resolver.ex @@ -15,10 +15,10 @@ defmodule Mv.Membership.Import.ColumnResolver do This module has no Phoenix or web dependencies. """ - require Logger - alias Mv.Membership.Import.HeaderMapper + require Logger + @preview_row_limit 3 @type numbered_row :: {pos_integer(), [String.t()]} diff --git a/lib/mv/membership/import/member_csv.ex b/lib/mv/membership/import/member_csv.ex index 31dea59..c666458 100644 --- a/lib/mv/membership/import/member_csv.ex +++ b/lib/mv/membership/import/member_csv.ex @@ -53,6 +53,14 @@ defmodule Mv.Membership.Import.MemberCSV do MemberCSV.process_chunk(chunk, import_state.column_map, import_state.custom_field_map, []) """ + use Gettext, backend: MvWeb.Gettext + + alias Mv.Helpers.SystemActor + alias Mv.Membership.Import.ColumnResolver + alias Mv.Membership.Import.CsvParser + alias Mv.Membership.Import.HeaderMapper + alias MvWeb.Translations.FieldTypes + defmodule Error do @moduledoc """ Error struct for CSV import errors. @@ -101,17 +109,6 @@ defmodule Mv.Membership.Import.MemberCSV do groups_found: list(Mv.Membership.Group.t() | %{id: String.t(), name: String.t()}) } - alias Mv.Membership.Import.ColumnResolver - alias Mv.Membership.Import.CsvParser - alias Mv.Membership.Import.HeaderMapper - - use Gettext, backend: MvWeb.Gettext - - alias Mv.Helpers.SystemActor - - # Import FieldTypes for human-readable type labels - alias MvWeb.Translations.FieldTypes - # Configuration constants @default_max_errors 50 @default_chunk_size 200 diff --git a/lib/mv/membership/member_export/build.ex b/lib/mv/membership/member_export/build.ex index 7159679..9f848a3 100644 --- a/lib/mv/membership/member_export/build.ex +++ b/lib/mv/membership/member_export/build.ex @@ -17,13 +17,14 @@ defmodule Mv.Membership.MemberExport.Build do No translations/Gettext in this module - labels come from the web layer via a function. """ - require Ash.Query import Ash.Expr alias Mv.Membership.{CustomField, CustomFieldValueFormatter, Member, MemberExportSort} alias MvWeb.MemberLive.Index alias MvWeb.MemberLive.Index.MembershipFeeStatus + require Ash.Query + @custom_field_prefix Mv.Constants.custom_field_prefix() @doc """ diff --git a/lib/mv/membership/members_pdf.ex b/lib/mv/membership/members_pdf.ex index a1c8418..f4bd353 100644 --- a/lib/mv/membership/members_pdf.ex +++ b/lib/mv/membership/members_pdf.ex @@ -12,12 +12,12 @@ defmodule Mv.Membership.MembersPDF do to avoid symlink issues and ensure isolation. """ - require Logger - use Gettext, backend: MvWeb.Gettext alias Mv.Config + require Logger + @template_filename "members_export.typ" @doc """ diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 189f40a..a6c2d30 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -1,11 +1,4 @@ defmodule Mv.MembershipFees.CycleGenerator do - @typedoc "Aggregate counts returned by a batch cycle-generation run." - @type results_summary :: %{ - success: non_neg_integer(), - failed: non_neg_integer(), - total: non_neg_integer() - } - @moduledoc """ Module for generating membership fee cycles for members. @@ -66,6 +59,13 @@ defmodule Mv.MembershipFees.CycleGenerator do require Ash.Query require Logger + @typedoc "Aggregate counts returned by a batch cycle-generation run." + @type results_summary :: %{ + success: non_neg_integer(), + failed: non_neg_integer(), + total: non_neg_integer() + } + @type generate_result :: {:ok, [MembershipFeeCycle.t()], [Ash.Notifier.Notification.t()]} | {:error, term()} diff --git a/lib/mv/release.ex b/lib/mv/release.ex index 5db4751..107b096 100644 --- a/lib/mv/release.ex +++ b/lib/mv/release.ex @@ -12,8 +12,6 @@ defmodule Mv.Release do or ADMIN_PASSWORD_FILE). Idempotent; can be run on every deployment or via shell to update the admin password without redeploying. """ - @app :mv - alias Mv.Accounts alias Mv.Accounts.User alias Mv.Authorization.Role @@ -21,6 +19,8 @@ defmodule Mv.Release do require Ash.Query require Logger + @app :mv + def migrate do _ = load_app() diff --git a/lib/mv/statistics.ex b/lib/mv/statistics.ex index b3c54c0..dbec91f 100644 --- a/lib/mv/statistics.ex +++ b/lib/mv/statistics.ex @@ -7,15 +7,15 @@ defmodule Mv.Statistics do to Ash reads so that policies are enforced. """ - require Ash.Query import Ash.Expr - require Logger - alias Mv.Membership.Member alias Mv.MembershipFees alias Mv.MembershipFees.MembershipFeeCycle + require Ash.Query + require Logger + @doc """ Returns the earliest year in which any member has a join_date. diff --git a/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex b/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex index 79f8c47..5f08606 100644 --- a/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex +++ b/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex @@ -9,9 +9,10 @@ defmodule Mv.Vereinfacht.Changes.SyncLinkedMemberAfterUserChange do """ use Ash.Resource.Change - require Logger alias Mv.EmailSync.Loader + require Logger + @impl true def change(changeset, _opts, _context) do if Mv.Config.vereinfacht_configured?() and relevant_change?(changeset) do diff --git a/lib/mv/vereinfacht/vereinfacht.ex b/lib/mv/vereinfacht/vereinfacht.ex index 4d58f8d..de1ed61 100644 --- a/lib/mv/vereinfacht/vereinfacht.ex +++ b/lib/mv/vereinfacht/vereinfacht.ex @@ -7,13 +7,15 @@ defmodule Mv.Vereinfacht do the linked member's email via Ecto (e.g. user email change). - `sync_members_without_contact/0` – Bulk sync of members without a contact ID. """ - require Ash.Query import Ash.Expr + alias Mv.Helpers alias Mv.Helpers.SystemActor alias Mv.Membership.Member alias Mv.Vereinfacht.Client + require Ash.Query + @doc """ Tests the connection to the Vereinfacht API using the current configuration. diff --git a/lib/mv_web/components/table_components.ex b/lib/mv_web/components/table_components.ex index 6b3c060..9d083dd 100644 --- a/lib/mv_web/components/table_components.ex +++ b/lib/mv_web/components/table_components.ex @@ -3,9 +3,10 @@ defmodule MvWeb.TableComponents do TableComponents that can be used in tables as components (like a button for sorting, a filter...) """ use Phoenix.Component - import MvWeb.CoreComponents use Gettext, backend: MvWeb.Gettext + import MvWeb.CoreComponents + attr :field, :atom, required: true attr :label, :string, required: true attr :sort_field, :atom, default: nil diff --git a/lib/mv_web/controllers/import_template_controller.ex b/lib/mv_web/controllers/import_template_controller.ex index 877697d..f4df97f 100644 --- a/lib/mv_web/controllers/import_template_controller.ex +++ b/lib/mv_web/controllers/import_template_controller.ex @@ -13,12 +13,12 @@ defmodule MvWeb.ImportTemplateController do """ use MvWeb, :controller + import MvWeb.ControllerHelpers, only: [current_actor: 1] + alias Mv.Membership.Member alias Mv.Membership.MembersCSV alias MvWeb.Authorization - import MvWeb.ControllerHelpers, only: [current_actor: 1] - # Standard member columns in template order, with their English and German headers # and a placeholder example value. Groups and fee type are importable extras. @columns [ diff --git a/lib/mv_web/controllers/member_export_controller.ex b/lib/mv_web/controllers/member_export_controller.ex index 240db34..e33b211 100644 --- a/lib/mv_web/controllers/member_export_controller.ex +++ b/lib/mv_web/controllers/member_export_controller.ex @@ -6,9 +6,10 @@ defmodule MvWeb.MemberExportController do Same permission and actor context as the member overview; 403 if unauthorized. """ use MvWeb, :controller + use Gettext, backend: MvWeb.Gettext - require Ash.Query import Ash.Expr + import MvWeb.ControllerHelpers, only: [current_actor: 1] alias Mv.Membership.CustomField alias Mv.Membership.CustomFieldSort @@ -18,8 +19,7 @@ defmodule MvWeb.MemberExportController do alias MvWeb.MemberLive.Index.MembershipFeeStatus alias MvWeb.Translations.MemberFields - import MvWeb.ControllerHelpers, only: [current_actor: 1] - use Gettext, backend: MvWeb.Gettext + require Ash.Query @member_fields_allowlist (Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)) ++ ["membership_fee_type", "groups"] diff --git a/lib/mv_web/controllers/member_pdf_export_controller.ex b/lib/mv_web/controllers/member_pdf_export_controller.ex index 1e82898..f5bdc6e 100644 --- a/lib/mv_web/controllers/member_pdf_export_controller.ex +++ b/lib/mv_web/controllers/member_pdf_export_controller.ex @@ -7,15 +7,14 @@ defmodule MvWeb.MemberPdfExportController do """ use MvWeb, :controller + use Gettext, backend: MvWeb.Gettext - require Logger + import MvWeb.ControllerHelpers, only: [current_actor: 1] alias Mv.Membership.{MemberExport, MemberExport.Build, MembersPDF} alias MvWeb.Translations.MemberFields - import MvWeb.ControllerHelpers, only: [current_actor: 1] - - use Gettext, backend: MvWeb.Gettext + require Logger @payload_required_message "payload required" @invalid_json_message "invalid JSON" diff --git a/lib/mv_web/live/auth/link_oidc_account_live.ex b/lib/mv_web/live/auth/link_oidc_account_live.ex index c4b3ab0..c0ab0e5 100644 --- a/lib/mv_web/live/auth/link_oidc_account_live.ex +++ b/lib/mv_web/live/auth/link_oidc_account_live.ex @@ -15,13 +15,14 @@ defmodule MvWeb.LinkOidcAccountLive do 6. User is redirected to complete OIDC login """ use MvWeb, :live_view - require Ash.Query - require Logger alias AshAuthentication.Strategy.Password.Actions, as: PasswordActions alias Mv.Accounts.User, as: UserResource alias Mv.Helpers.SystemActor + require Ash.Query + require Logger + @impl true def mount(_params, session, socket) do # Use SystemActor for authorization during OIDC linking (user is not yet logged in) diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index 735c165..42311df 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -34,7 +34,6 @@ defmodule MvWeb.GlobalSettingsLive do """ use MvWeb, :live_view - require Ash.Query import Ash.Expr alias Mv.Helpers @@ -44,6 +43,8 @@ defmodule MvWeb.GlobalSettingsLive do alias MvWeb.Helpers.MemberHelpers alias MvWeb.Translations.MemberFields + require Ash.Query + on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded} @impl true diff --git a/lib/mv_web/live/group_live/show.ex b/lib/mv_web/live/group_live/show.ex index 9e5bdf6..af93719 100644 --- a/lib/mv_web/live/group_live/show.ex +++ b/lib/mv_web/live/group_live/show.ex @@ -15,16 +15,16 @@ defmodule MvWeb.GroupLive.Show do """ use MvWeb, :live_view - require Logger - import Ash.Expr - import MvWeb.LiveHelpers, only: [current_actor: 1] import MvWeb.Authorization + import MvWeb.LiveHelpers, only: [current_actor: 1] alias Mv.Membership alias MvWeb.Helpers.MemberHelpers, as: MemberHelpers alias MvWeb.Live.MemberDropdownNav + require Logger + @impl true def mount(_params, _session, socket) do {:ok, diff --git a/lib/mv_web/live/import_live/components.ex b/lib/mv_web/live/import_live/components.ex index c317b87..4058bc8 100644 --- a/lib/mv_web/live/import_live/components.ex +++ b/lib/mv_web/live/import_live/components.ex @@ -7,13 +7,13 @@ defmodule MvWeb.ImportLive.Components do use Phoenix.Component use Gettext, backend: MvWeb.Gettext - import MvWeb.CoreComponents - use Phoenix.VerifiedRoutes, endpoint: MvWeb.Endpoint, router: MvWeb.Router, statics: MvWeb.static_paths() + import MvWeb.CoreComponents + @doc """ Renders the info box explaining that data fields must exist before import and linking to Manage Member Data (custom fields). diff --git a/lib/mv_web/live/join_request_live/index.ex b/lib/mv_web/live/join_request_live/index.ex index 533f0ce..4326416 100644 --- a/lib/mv_web/live/join_request_live/index.ex +++ b/lib/mv_web/live/join_request_live/index.ex @@ -13,15 +13,15 @@ defmodule MvWeb.JoinRequestLive.Index do """ use MvWeb, :live_view - require Logger - - import MvWeb.LiveHelpers, only: [current_actor: 1] import MvWeb.Authorization + import MvWeb.LiveHelpers, only: [current_actor: 1] alias Mv.Membership alias MvWeb.Helpers.DateFormatter alias MvWeb.JoinRequestLive.Helpers, as: JoinRequestHelpers + require Logger + @impl true def mount(_params, _session, socket) do actor = current_actor(socket) diff --git a/lib/mv_web/live/join_request_live/show.ex b/lib/mv_web/live/join_request_live/show.ex index d634f53..e02c64d 100644 --- a/lib/mv_web/live/join_request_live/show.ex +++ b/lib/mv_web/live/join_request_live/show.ex @@ -14,10 +14,8 @@ defmodule MvWeb.JoinRequestLive.Show do """ use MvWeb, :live_view - require Logger - - import MvWeb.LiveHelpers, only: [current_actor: 1] import MvWeb.Authorization + import MvWeb.LiveHelpers, only: [current_actor: 1] alias Mv.Constants alias Mv.Membership @@ -26,6 +24,8 @@ defmodule MvWeb.JoinRequestLive.Show do alias MvWeb.JoinRequestLive.Helpers, as: JoinRequestHelpers alias MvWeb.Translations.MemberFields, as: MemberFieldsTranslations + require Logger + @impl true def mount(_params, _session, socket) do if Membership.join_form_enabled?() do diff --git a/lib/mv_web/live/member_live/form.ex b/lib/mv_web/live/member_live/form.ex index 7770e90..dc79c9c 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -20,7 +20,6 @@ defmodule MvWeb.MemberLive.Form do """ use MvWeb, :live_view - require Logger import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3] alias Mv.Membership @@ -32,6 +31,8 @@ defmodule MvWeb.MemberLive.Form do alias MvWeb.Helpers.MemberHelpers alias MvWeb.Helpers.MembershipFeeHelpers + require Logger + @impl true def render(assigns) do # Sort custom fields by name for display only diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index 1b38820..c991f7f 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -26,8 +26,6 @@ defmodule MvWeb.MemberLive.Index do """ use MvWeb, :live_view - require Ash.Query - require Logger import Ash.Expr import MvWeb.LiveHelpers, only: [current_actor: 1] @@ -45,6 +43,9 @@ defmodule MvWeb.MemberLive.Index do alias MvWeb.MemberLive.Index.Formatter alias MvWeb.MemberLive.Index.MembershipFeeStatus + require Ash.Query + require Logger + @custom_field_prefix Mv.Constants.custom_field_prefix() @boolean_filter_prefix Mv.Constants.boolean_filter_prefix() @group_filter_prefix Mv.Constants.group_filter_prefix() diff --git a/lib/mv_web/live/member_live/index/date_filter.ex b/lib/mv_web/live/member_live/index/date_filter.ex index 162524a..14d1aef 100644 --- a/lib/mv_web/live/member_live/index/date_filter.ex +++ b/lib/mv_web/live/member_live/index/date_filter.ex @@ -28,12 +28,13 @@ defmodule MvWeb.MemberLive.Index.DateFilter do `exit_date IS NULL OR exit_date > today` — a member who left today is hidden. """ - require Ash.Query import Ash.Expr alias MvWeb.MemberLive.Index.CustomFieldValueLookup alias MvWeb.MemberLive.Index.FilterParams + require Ash.Query + @join_date_from_param Mv.Constants.join_date_from_param() @join_date_to_param Mv.Constants.join_date_to_param() @exit_date_mode_param Mv.Constants.exit_date_mode_param() diff --git a/lib/mv_web/live/member_live/show/membership_fees_component.ex b/lib/mv_web/live/member_live/show/membership_fees_component.ex index 16ee5dc..3a5a976 100644 --- a/lib/mv_web/live/member_live/show/membership_fees_component.ex +++ b/lib/mv_web/live/member_live/show/membership_fees_component.ex @@ -12,10 +12,9 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do """ use MvWeb, :live_component - require Ash.Query - import MvWeb.LiveHelpers, only: [current_actor: 1] import MvWeb.Authorization, only: [can?: 3] import MvWeb.Helpers.AshErrorHelpers, only: [format_error: 1] + import MvWeb.LiveHelpers, only: [current_actor: 1] alias Mv.Membership alias Mv.MembershipFees @@ -25,6 +24,8 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do alias Mv.MembershipFees.MembershipFeeType alias MvWeb.Helpers.MembershipFeeHelpers + require Ash.Query + @impl true def render(assigns) do ~H""" diff --git a/lib/mv_web/live/membership_fee_settings_live.ex b/lib/mv_web/live/membership_fee_settings_live.ex index 56de3b5..7ff1ec9 100644 --- a/lib/mv_web/live/membership_fee_settings_live.ex +++ b/lib/mv_web/live/membership_fee_settings_live.ex @@ -9,8 +9,6 @@ defmodule MvWeb.MembershipFeeSettingsLive do """ use MvWeb, :live_view - require Ash.Query - import MvWeb.LiveHelpers, only: [current_actor: 1] alias Mv.Membership @@ -18,6 +16,8 @@ defmodule MvWeb.MembershipFeeSettingsLive do alias Mv.MembershipFees.MembershipFeeType alias MvWeb.Helpers.MembershipFeeHelpers + require Ash.Query + @impl true def mount(_params, _session, socket) do actor = current_actor(socket) diff --git a/lib/mv_web/live/membership_fee_type_live/form.ex b/lib/mv_web/live/membership_fee_type_live/form.ex index a4d8506..6b726c2 100644 --- a/lib/mv_web/live/membership_fee_type_live/form.ex +++ b/lib/mv_web/live/membership_fee_type_live/form.ex @@ -15,13 +15,13 @@ defmodule MvWeb.MembershipFeeTypeLive.Form do import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3] - require Ash.Query - alias Mv.Membership.Member alias Mv.MembershipFees alias Mv.MembershipFees.MembershipFeeType alias MvWeb.Helpers.MembershipFeeHelpers + require Ash.Query + @impl true def render(assigns) do ~H""" diff --git a/lib/mv_web/live/membership_fee_type_live/index.ex b/lib/mv_web/live/membership_fee_type_live/index.ex index b6290e2..6efdad3 100644 --- a/lib/mv_web/live/membership_fee_type_live/index.ex +++ b/lib/mv_web/live/membership_fee_type_live/index.ex @@ -16,14 +16,14 @@ defmodule MvWeb.MembershipFeeTypeLive.Index do import MvWeb.LiveHelpers, only: [current_actor: 1] - require Ash.Query - alias Mv.Membership alias Mv.Membership.Member alias Mv.MembershipFees alias Mv.MembershipFees.MembershipFeeType alias MvWeb.Helpers.MembershipFeeHelpers + require Ash.Query + @impl true def mount(_params, _session, socket) do actor = current_actor(socket) diff --git a/lib/mv_web/live/role_live/form.ex b/lib/mv_web/live/role_live/form.ex index 2e315b9..bf61230 100644 --- a/lib/mv_web/live/role_live/form.ex +++ b/lib/mv_web/live/role_live/form.ex @@ -13,10 +13,10 @@ defmodule MvWeb.RoleLive.Form do """ use MvWeb, :live_view - alias Mv.Authorization.PermissionSets - import MvWeb.RoleLive.Helpers, only: [format_error: 1] + alias Mv.Authorization.PermissionSets + @impl true def render(assigns) do ~H""" diff --git a/lib/mv_web/live/role_live/index.ex b/lib/mv_web/live/role_live/index.ex index 5e210b5..188754f 100644 --- a/lib/mv_web/live/role_live/index.ex +++ b/lib/mv_web/live/role_live/index.ex @@ -13,13 +13,13 @@ defmodule MvWeb.RoleLive.Index do """ use MvWeb, :live_view + import MvWeb.RoleLive.Helpers, only: [permission_set_badge_variant: 1] + alias Mv.Accounts alias Mv.Authorization require Ash.Query - import MvWeb.RoleLive.Helpers, only: [permission_set_badge_variant: 1] - @impl true def mount(_params, _session, socket) do actor = socket.assigns[:current_user] diff --git a/lib/mv_web/live/role_live/show.ex b/lib/mv_web/live/role_live/show.ex index d8153be..2aed457 100644 --- a/lib/mv_web/live/role_live/show.ex +++ b/lib/mv_web/live/role_live/show.ex @@ -12,13 +12,13 @@ defmodule MvWeb.RoleLive.Show do """ use MvWeb, :live_view + import MvWeb.RoleLive.Helpers, + only: [format_error: 1, permission_set_badge_variant: 1, opts_with_actor: 3] + alias Mv.Accounts require Ash.Query - import MvWeb.RoleLive.Helpers, - only: [format_error: 1, permission_set_badge_variant: 1, opts_with_actor: 3] - @impl true def mount(%{"id" => id}, _session, socket) do case Ash.get( diff --git a/lib/mv_web/live/statistics_live.ex b/lib/mv_web/live/statistics_live.ex index bdaae01..8a9b5cd 100644 --- a/lib/mv_web/live/statistics_live.ex +++ b/lib/mv_web/live/statistics_live.ex @@ -6,13 +6,14 @@ defmodule MvWeb.StatisticsLive do """ use MvWeb, :live_view - require Logger - import MvWeb.LiveHelpers, only: [current_actor: 1] + alias Mv.MembershipFees.MembershipFeeType alias Mv.Statistics alias MvWeb.Helpers.MembershipFeeHelpers + require Logger + @impl true def mount(_params, _session, socket) do # Only static assigns and fee types here; load_statistics runs once in handle_params diff --git a/lib/mv_web/live/user_live/form.ex b/lib/mv_web/live/user_live/form.ex index 297f1f6..2d29592 100644 --- a/lib/mv_web/live/user_live/form.ex +++ b/lib/mv_web/live/user_live/form.ex @@ -33,7 +33,9 @@ defmodule MvWeb.UserLive.Form do """ use MvWeb, :live_view - require Jason + import MvWeb.Authorization, only: [can?: 3] + import MvWeb.ErrorHelpers, only: [format_ash_error: 1] + import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3] alias Mv.Accounts alias Mv.Accounts.User, as: UserResource @@ -45,9 +47,7 @@ defmodule MvWeb.UserLive.Form do alias MvWeb.Helpers.MemberHelpers alias MvWeb.Live.MemberDropdownNav - import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3] - import MvWeb.Authorization, only: [can?: 3] - import MvWeb.ErrorHelpers, only: [format_ash_error: 1] + require Jason @impl true def render(assigns) do diff --git a/lib/mv_web/live_user_auth.ex b/lib/mv_web/live_user_auth.ex index 617b079..a5b3f6f 100644 --- a/lib/mv_web/live_user_auth.ex +++ b/lib/mv_web/live_user_auth.ex @@ -3,9 +3,10 @@ defmodule MvWeb.LiveUserAuth do Helpers for authenticating users in LiveViews. """ - import Phoenix.Component use MvWeb, :verified_routes + import Phoenix.Component + alias AshAuthentication.Phoenix.LiveSession alias Phoenix.LiveView diff --git a/test/membership/group_database_constraints_test.exs b/test/membership/group_database_constraints_test.exs index 4418b33..25f0f90 100644 --- a/test/membership/group_database_constraints_test.exs +++ b/test/membership/group_database_constraints_test.exs @@ -5,10 +5,11 @@ defmodule Mv.Membership.GroupDatabaseConstraintsTest do """ use Mv.DataCase, async: false + import Ash.Expr + alias Mv.Membership require Ash.Query - import Ash.Expr setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/membership/group_test.exs b/test/membership/group_test.exs index 72db874..a7befb9 100644 --- a/test/membership/group_test.exs +++ b/test/membership/group_test.exs @@ -5,10 +5,11 @@ defmodule Mv.Membership.GroupTest do """ use Mv.DataCase, async: true + import Ash.Expr + alias Mv.Membership require Ash.Query - import Ash.Expr setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/membership/join_request_approval_domain_test.exs b/test/membership/join_request_approval_domain_test.exs index 15f5636..fece3d8 100644 --- a/test/membership/join_request_approval_domain_test.exs +++ b/test/membership/join_request_approval_domain_test.exs @@ -8,13 +8,14 @@ defmodule Mv.Membership.JoinRequestApprovalDomainTest do use Mv.DataCase, async: true import Ash.Expr - require Ash.Query alias Mv.Fixtures alias Mv.Helpers.SystemActor alias Mv.Membership alias Mv.Membership.Member + require Ash.Query + defp member_count do actor = SystemActor.get_system_actor() {:ok, members} = Membership.list_members(actor: actor) diff --git a/test/membership/join_request_test.exs b/test/membership/join_request_test.exs index 5f0ae83..9915d4a 100644 --- a/test/membership/join_request_test.exs +++ b/test/membership/join_request_test.exs @@ -12,13 +12,14 @@ defmodule Mv.Membership.JoinRequestTest do """ use Mv.DataCase, async: true - require Ash.Query import Ash.Expr alias Mv.Fixtures alias Mv.Membership alias Mv.Membership.JoinRequest + require Ash.Query + # Valid minimal attributes for submit (email required; confirmation_token optional for tests) @valid_submit_attrs %{ email: "join#{System.unique_integer([:positive])}@example.com" diff --git a/test/membership/member_group_test.exs b/test/membership/member_group_test.exs index 430ae7b..f7afaa1 100644 --- a/test/membership/member_group_test.exs +++ b/test/membership/member_group_test.exs @@ -5,10 +5,11 @@ defmodule Mv.Membership.MemberGroupTest do """ use Mv.DataCase, async: true + import Ash.Expr + alias Mv.Membership require Ash.Query - import Ash.Expr setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/membership/member_groups_relationship_test.exs b/test/membership/member_groups_relationship_test.exs index 5ecddbd..188f71c 100644 --- a/test/membership/member_groups_relationship_test.exs +++ b/test/membership/member_groups_relationship_test.exs @@ -4,10 +4,11 @@ defmodule Mv.Membership.MemberGroupsRelationshipTest do """ use Mv.DataCase, async: true + import Ash.Expr + alias Mv.Membership require Ash.Query - import Ash.Expr setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv/authorization/checks/has_permission_fail_closed_test.exs b/test/mv/authorization/checks/has_permission_fail_closed_test.exs index 36ddbd2..940da35 100644 --- a/test/mv/authorization/checks/has_permission_fail_closed_test.exs +++ b/test/mv/authorization/checks/has_permission_fail_closed_test.exs @@ -12,10 +12,10 @@ defmodule Mv.Authorization.Checks.HasPermissionFailClosedTest do """ use Mv.DataCase, async: true - alias Mv.Authorization.Checks.HasPermission - import Mv.Fixtures + alias Mv.Authorization.Checks.HasPermission + test "auto_filter deny-filter matches no records (regression for NOT IN [] allow-all bug)" do # Arrange: create some members in DB _m1 = member_fixture() diff --git a/test/mv/statistics_test.exs b/test/mv/statistics_test.exs index 6b72ffb..874681f 100644 --- a/test/mv/statistics_test.exs +++ b/test/mv/statistics_test.exs @@ -4,7 +4,6 @@ defmodule Mv.StatisticsTest do """ use Mv.DataCase, async: true - require Ash.Query import Ash.Expr alias Mv.Membership.Member @@ -13,6 +12,8 @@ defmodule Mv.StatisticsTest do alias Mv.MembershipFees.MembershipFeeType alias Mv.Statistics + require Ash.Query + setup do actor = Mv.Helpers.SystemActor.get_system_actor() %{actor: actor} diff --git a/test/mv_web/components/member_filter_component_test.exs b/test/mv_web/components/member_filter_component_test.exs index fcb45f6..9364eaa 100644 --- a/test/mv_web/components/member_filter_component_test.exs +++ b/test/mv_web/components/member_filter_component_test.exs @@ -11,9 +11,10 @@ defmodule MvWeb.Components.MemberFilterComponentTest do # async: false to prevent PostgreSQL deadlocks when running LiveView tests against DB use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Membership.CustomField # Helper to create a boolean custom field (uses system_actor - only admin can create) diff --git a/test/mv_web/helpers/membership_fee_helpers_test.exs b/test/mv_web/helpers/membership_fee_helpers_test.exs index 773bd26..c143b34 100644 --- a/test/mv_web/helpers/membership_fee_helpers_test.exs +++ b/test/mv_web/helpers/membership_fee_helpers_test.exs @@ -4,11 +4,11 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do """ use Mv.DataCase, async: true - require Ash.Query - alias Mv.MembershipFees.CalendarCycles alias MvWeb.Helpers.MembershipFeeHelpers + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() %{actor: system_actor} 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 7b0953a..2e600d0 100644 --- a/test/mv_web/live/custom_field_live/deletion_test.exs +++ b/test/mv_web/live/custom_field_live/deletion_test.exs @@ -15,10 +15,11 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do use MvWeb.ConnCase, async: true import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{CustomField, CustomFieldValue, Member} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() admin_role = Mv.Fixtures.role_fixture("admin") diff --git a/test/mv_web/live/custom_field_live/form_test.exs b/test/mv_web/live/custom_field_live/form_test.exs index 4f7c8ec..bd5c60c 100644 --- a/test/mv_web/live/custom_field_live/form_test.exs +++ b/test/mv_web/live/custom_field_live/form_test.exs @@ -8,10 +8,11 @@ defmodule MvWeb.CustomFieldLive.FormTest do use MvWeb.ConnCase, async: true import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.CustomField + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() admin_role = Mv.Fixtures.role_fixture("admin") diff --git a/test/mv_web/live/group_live/form_test.exs b/test/mv_web/live/group_live/form_test.exs index 8934e85..d0e73e4 100644 --- a/test/mv_web/live/group_live/form_test.exs +++ b/test/mv_web/live/group_live/form_test.exs @@ -10,9 +10,10 @@ defmodule MvWeb.GroupLive.FormTest do - Security """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures describe "create form" do diff --git a/test/mv_web/live/group_live/index_test.exs b/test/mv_web/live/group_live/index_test.exs index 5adfe56..aa1d23d 100644 --- a/test/mv_web/live/group_live/index_test.exs +++ b/test/mv_web/live/group_live/index_test.exs @@ -10,9 +10,10 @@ defmodule MvWeb.GroupLive.IndexTest do - Edge cases """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership diff --git a/test/mv_web/live/group_live/integration_test.exs b/test/mv_web/live/group_live/integration_test.exs index 937ed1e..5cbf373 100644 --- a/test/mv_web/live/group_live/integration_test.exs +++ b/test/mv_web/live/group_live/integration_test.exs @@ -9,14 +9,16 @@ defmodule MvWeb.GroupLive.IntegrationTest do - URL persistence """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest - require Ash.Query - import Ash.Expr use Gettext, backend: MvWeb.Gettext + import Ash.Expr + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership + require Ash.Query + describe "complete workflow" do test "create → view via slug → edit → view via slug (slug unchanged)", %{ conn: conn, diff --git a/test/mv_web/live/group_live/show_accessibility_test.exs b/test/mv_web/live/group_live/show_accessibility_test.exs index 14866ef..cbec37d 100644 --- a/test/mv_web/live/group_live/show_accessibility_test.exs +++ b/test/mv_web/live/group_live/show_accessibility_test.exs @@ -5,9 +5,10 @@ defmodule MvWeb.GroupLive.ShowAccessibilityTest do """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership diff --git a/test/mv_web/live/group_live/show_add_member_test.exs b/test/mv_web/live/group_live/show_add_member_test.exs index 761dc83..fbc3bac 100644 --- a/test/mv_web/live/group_live/show_add_member_test.exs +++ b/test/mv_web/live/group_live/show_add_member_test.exs @@ -5,10 +5,11 @@ defmodule MvWeb.GroupLive.ShowAddMemberTest do """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest - import MvWeb.GroupLiveHelpers use Gettext, backend: MvWeb.Gettext + import MvWeb.GroupLiveHelpers + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership diff --git a/test/mv_web/live/group_live/show_add_remove_members_test.exs b/test/mv_web/live/group_live/show_add_remove_members_test.exs index ede25fd..6737b91 100644 --- a/test/mv_web/live/group_live/show_add_remove_members_test.exs +++ b/test/mv_web/live/group_live/show_add_remove_members_test.exs @@ -5,9 +5,10 @@ defmodule MvWeb.GroupLive.ShowAddRemoveMembersTest do """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership diff --git a/test/mv_web/live/group_live/show_authorization_test.exs b/test/mv_web/live/group_live/show_authorization_test.exs index c75e623..f7b5e33 100644 --- a/test/mv_web/live/group_live/show_authorization_test.exs +++ b/test/mv_web/live/group_live/show_authorization_test.exs @@ -5,9 +5,10 @@ defmodule MvWeb.GroupLive.ShowAuthorizationTest do """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership diff --git a/test/mv_web/live/group_live/show_integration_test.exs b/test/mv_web/live/group_live/show_integration_test.exs index 407ed1e..519e6ca 100644 --- a/test/mv_web/live/group_live/show_integration_test.exs +++ b/test/mv_web/live/group_live/show_integration_test.exs @@ -5,9 +5,10 @@ defmodule MvWeb.GroupLive.ShowIntegrationTest do """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership diff --git a/test/mv_web/live/group_live/show_member_search_test.exs b/test/mv_web/live/group_live/show_member_search_test.exs index 8a3d191..a855804 100644 --- a/test/mv_web/live/group_live/show_member_search_test.exs +++ b/test/mv_web/live/group_live/show_member_search_test.exs @@ -5,9 +5,10 @@ defmodule MvWeb.GroupLive.ShowMemberSearchTest do """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership diff --git a/test/mv_web/live/group_live/show_remove_member_test.exs b/test/mv_web/live/group_live/show_remove_member_test.exs index 1dfd290..125a169 100644 --- a/test/mv_web/live/group_live/show_remove_member_test.exs +++ b/test/mv_web/live/group_live/show_remove_member_test.exs @@ -5,9 +5,10 @@ defmodule MvWeb.GroupLive.ShowRemoveMemberTest do """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership diff --git a/test/mv_web/live/group_live/show_test.exs b/test/mv_web/live/group_live/show_test.exs index ee30991..0a82c15 100644 --- a/test/mv_web/live/group_live/show_test.exs +++ b/test/mv_web/live/group_live/show_test.exs @@ -11,13 +11,15 @@ defmodule MvWeb.GroupLive.ShowTest do - Delete functionality """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest - require Ash.Query use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures alias Mv.Membership + require Ash.Query + describe "mount and display" do test "page renders successfully", %{conn: conn} do group = Fixtures.group_fixture() diff --git a/test/mv_web/live/member_live/deactivate_test.exs b/test/mv_web/live/member_live/deactivate_test.exs index 2e7a6a7..367c387 100644 --- a/test/mv_web/live/member_live/deactivate_test.exs +++ b/test/mv_web/live/member_live/deactivate_test.exs @@ -4,9 +4,10 @@ defmodule MvWeb.MemberLive.DeactivateTest do driven through the parent LiveView (the DeactivateComponent is stateful). """ use MvWeb.ConnCase, async: true - import Phoenix.LiveViewTest use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Fixtures defp reload_member(member) do diff --git a/test/mv_web/live/role_live/show_test.exs b/test/mv_web/live/role_live/show_test.exs index fe5c48d..857f54b 100644 --- a/test/mv_web/live/role_live/show_test.exs +++ b/test/mv_web/live/role_live/show_test.exs @@ -11,13 +11,15 @@ defmodule MvWeb.RoleLive.ShowTest do - Delete functionality """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest - require Ash.Query use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Authorization alias Mv.Authorization.Role + require Ash.Query + # Helper to create a role (authorize?: false for test data setup) defp create_role(attrs \\ %{}) do default_attrs = %{ diff --git a/test/mv_web/live/user_live/show_test.exs b/test/mv_web/live/user_live/show_test.exs index 084e346..dbfa0c2 100644 --- a/test/mv_web/live/user_live/show_test.exs +++ b/test/mv_web/live/user_live/show_test.exs @@ -10,10 +10,12 @@ defmodule MvWeb.UserLive.ShowTest do - Error handling """ use MvWeb.ConnCase, async: true - import Phoenix.LiveViewTest - require Ash.Query use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + + require Ash.Query + setup do # Create test user user = create_test_user(%{email: "test@example.com", oidc_id: "test123"}) 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 17e6da4..c33210e 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 @@ -9,10 +9,11 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsAccessibilityTest do """ use MvWeb.ConnCase, async: true import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{CustomField, CustomFieldValue} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() 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 9ada92b..e4e1824 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 @@ -12,10 +12,11 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsDisplayTest do # async: false to prevent PostgreSQL deadlocks when creating members and custom fields use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{CustomField, CustomFieldValue} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() 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 cf67fc3..288ea0a 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 @@ -8,10 +8,11 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsEdgeCasesTest do """ use MvWeb.ConnCase, async: true import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.CustomField + require Ash.Query + @tag :slow test "displays custom field column even when no members have values", %{conn: conn} do system_actor = Mv.Helpers.SystemActor.get_system_actor() 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 afce67b..4119205 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 @@ -11,10 +11,11 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsSortingTest do """ use MvWeb.ConnCase, async: true import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{CustomField, CustomFieldValue} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() 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 79d078b..21c75ae 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -14,10 +14,11 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{CustomField, CustomFieldValue} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_accessibility_test.exs b/test/mv_web/member_live/index_groups_accessibility_test.exs index d14cd9f..1644d5f 100644 --- a/test/mv_web/member_live/index_groups_accessibility_test.exs +++ b/test/mv_web/member_live/index_groups_accessibility_test.exs @@ -11,10 +11,11 @@ defmodule MvWeb.MemberLive.IndexGroupsAccessibilityTest do # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{Group, MemberGroup} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_display_test.exs b/test/mv_web/member_live/index_groups_display_test.exs index 263ac2a..4521942 100644 --- a/test/mv_web/member_live/index_groups_display_test.exs +++ b/test/mv_web/member_live/index_groups_display_test.exs @@ -11,10 +11,11 @@ defmodule MvWeb.MemberLive.IndexGroupsDisplayTest do # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{Group, MemberGroup} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_filter_test.exs b/test/mv_web/member_live/index_groups_filter_test.exs index d32b17f..48a335a 100644 --- a/test/mv_web/member_live/index_groups_filter_test.exs +++ b/test/mv_web/member_live/index_groups_filter_test.exs @@ -9,10 +9,11 @@ defmodule MvWeb.MemberLive.IndexGroupsFilterTest do # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{Group, MemberGroup} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_integration_test.exs b/test/mv_web/member_live/index_groups_integration_test.exs index 86738da..3c32f4c 100644 --- a/test/mv_web/member_live/index_groups_integration_test.exs +++ b/test/mv_web/member_live/index_groups_integration_test.exs @@ -13,12 +13,13 @@ defmodule MvWeb.MemberLive.IndexGroupsIntegrationTest do # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Helpers.SystemActor alias Mv.Membership.{CustomField, CustomFieldValue, Group, MemberGroup} alias Mv.MembershipFees.{MembershipFeeCycle, MembershipFeeType} + require Ash.Query + setup do system_actor = SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_performance_test.exs b/test/mv_web/member_live/index_groups_performance_test.exs index 761c4eb..c5f170a 100644 --- a/test/mv_web/member_live/index_groups_performance_test.exs +++ b/test/mv_web/member_live/index_groups_performance_test.exs @@ -11,10 +11,11 @@ defmodule MvWeb.MemberLive.IndexGroupsPerformanceTest do # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{Group, MemberGroup} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_sorting_test.exs b/test/mv_web/member_live/index_groups_sorting_test.exs index 068152c..0828a33 100644 --- a/test/mv_web/member_live/index_groups_sorting_test.exs +++ b/test/mv_web/member_live/index_groups_sorting_test.exs @@ -5,10 +5,11 @@ defmodule MvWeb.MemberLive.IndexGroupsSortingTest do # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{Group, MemberGroup} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_url_params_test.exs b/test/mv_web/member_live/index_groups_url_params_test.exs index 469b010..d2f3b7b 100644 --- a/test/mv_web/member_live/index_groups_url_params_test.exs +++ b/test/mv_web/member_live/index_groups_url_params_test.exs @@ -12,10 +12,11 @@ defmodule MvWeb.MemberLive.IndexGroupsUrlParamsTest do # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Membership.{Group, MemberGroup} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_test.exs b/test/mv_web/member_live/index_test.exs index 0cde8fd..d664a5c 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -1,7 +1,6 @@ defmodule MvWeb.MemberLive.IndexTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - require Ash.Query alias Mv.Helpers.SystemActor alias Mv.Membership @@ -11,6 +10,8 @@ defmodule MvWeb.MemberLive.IndexTest do alias Mv.MembershipFees.MembershipFeeType alias MvWeb.MemberLive.Index, as: MemberIndex + require Ash.Query + # Helper to create a membership fee type (shared across all tests) defp create_fee_type(attrs, actor) do default_attrs = %{ diff --git a/test/mv_web/member_live/show_groups_display_test.exs b/test/mv_web/member_live/show_groups_display_test.exs index f8434b3..ea9cf52 100644 --- a/test/mv_web/member_live/show_groups_display_test.exs +++ b/test/mv_web/member_live/show_groups_display_test.exs @@ -14,12 +14,14 @@ defmodule MvWeb.MemberLive.ShowGroupsDisplayTest do in the same test run (same as IndexGroupsDisplayTest). """ use MvWeb.ConnCase, async: false - import Phoenix.LiveViewTest - require Ash.Query use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Membership.{Group, MemberGroup} + require Ash.Query + describe "groups section" do setup do actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/show_test.exs b/test/mv_web/member_live/show_test.exs index 2ec5a68..c6f26a8 100644 --- a/test/mv_web/member_live/show_test.exs +++ b/test/mv_web/member_live/show_test.exs @@ -14,12 +14,14 @@ defmodule MvWeb.MemberLive.ShowTest do - Custom field cleanup in tests ensures no interference between tests """ use MvWeb.ConnCase, async: true - import Phoenix.LiveViewTest - require Ash.Query use Gettext, backend: MvWeb.Gettext + import Phoenix.LiveViewTest + alias Mv.Membership.{CustomField, CustomFieldValue} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() From a12fedcb5b04ec2ca863b7948b944be0b864301d Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:38:02 +0200 Subject: [PATCH 16/27] ci(credo): enforce StrictModuleLayout and additional zero-violation checks --- .credo.exs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/.credo.exs b/.credo.exs index 3a4f8dc..a94fead 100644 --- a/.credo.exs +++ b/.credo.exs @@ -114,6 +114,7 @@ {Credo.Check.Readability.RedundantBlankLines, []}, {Credo.Check.Readability.Semicolons, []}, {Credo.Check.Readability.SpaceAfterCommas, []}, + {Credo.Check.Readability.StrictModuleLayout, []}, {Credo.Check.Readability.StringSigils, []}, {Credo.Check.Readability.TrailingBlankLine, []}, {Credo.Check.Readability.TrailingWhiteSpace, []}, @@ -166,13 +167,19 @@ {Credo.Check.Warning.UnusedTupleOperation, []}, {Credo.Check.Warning.WrongTestFileExtension, []}, # Module documentation check (enabled after adding @moduledoc to all modules) - {Credo.Check.Readability.ModuleDoc, []} + {Credo.Check.Readability.ModuleDoc, []}, + + # Promoted in the cleanup ratchet (each currently at zero violations): + {Credo.Check.Refactor.DoubleBooleanNegation, []}, + {Credo.Check.Refactor.FilterReject, []}, + {Credo.Check.Refactor.RejectFilter, []}, + {Credo.Check.Refactor.UtcNowTruncate, []}, + {Credo.Check.Warning.LazyLogging, []}, + {Credo.Check.Warning.LeakyEnvironment, []}, + {Credo.Check.Warning.MapGetUnsafePass, []}, + {Credo.Check.Warning.MixEnv, []} ], disabled: [ - # - # Checks scheduled for next check update (opt-in for now) - {Credo.Check.Refactor.UtcNowTruncate, []}, - # # Controversial and experimental checks (opt-in, just move the check to `:enabled` # and be sure to use `mix credo --strict` to see low priority checks) @@ -183,6 +190,7 @@ {Credo.Check.Design.SkipTestWithoutComment, []}, {Credo.Check.Readability.AliasAs, []}, {Credo.Check.Readability.BlockPipe, []}, + # ImplTrue: ~269 violations; deferred to a follow-up. {Credo.Check.Readability.ImplTrue, []}, {Credo.Check.Readability.MultiAlias, []}, {Credo.Check.Readability.NestedFunctionCalls, []}, @@ -192,24 +200,20 @@ {Credo.Check.Readability.SingleFunctionToBlockPipe, []}, {Credo.Check.Readability.SinglePipe, []}, {Credo.Check.Readability.Specs, []}, - {Credo.Check.Readability.StrictModuleLayout, []}, {Credo.Check.Readability.WithCustomTaggedTuple, []}, {Credo.Check.Refactor.ABCSize, []}, + # AppendSingleItem: ~10 violations (mostly tests); deferred to a follow-up. {Credo.Check.Refactor.AppendSingleItem, []}, - {Credo.Check.Refactor.DoubleBooleanNegation, []}, - {Credo.Check.Refactor.FilterReject, []}, + # IoPuts: 3 violations in Mv.Release seed output; deferred to a follow-up. {Credo.Check.Refactor.IoPuts, []}, + # MapMap: ~8 violations; deferred to a follow-up. {Credo.Check.Refactor.MapMap, []}, {Credo.Check.Refactor.ModuleDependencies, []}, + # NegatedIsNil: ~63 violations; deferred to a follow-up. {Credo.Check.Refactor.NegatedIsNil, []}, {Credo.Check.Refactor.PassAsyncInTestCases, []}, {Credo.Check.Refactor.PipeChainStart, []}, - {Credo.Check.Refactor.RejectFilter, []}, {Credo.Check.Refactor.VariableRebinding, []}, - {Credo.Check.Warning.LazyLogging, []}, - {Credo.Check.Warning.LeakyEnvironment, []}, - {Credo.Check.Warning.MapGetUnsafePass, []}, - {Credo.Check.Warning.MixEnv, []}, {Credo.Check.Warning.UnsafeToAtom, []} # {Credo.Check.Refactor.MapInto, []}, From 4f3050cc3575164ed7cf7fcbed3fc7ceae9b24b8 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:38:02 +0200 Subject: [PATCH 17/27] docs(changelog): record cleanup and quick fixes under Unreleased --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d08ad05..ba4e05f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Default GDPR custom field** – The seeded GDPR field was shortened from "Datenschutzerklärung akzeptiert" to "DSGVO" and now ships with a default join-form description (with a placeholder link to replace). ### Fixed +- **Authentication background workers start correctly** – The token-cleanup (Expunger) and audit-log batching workers now boot under the application's real configuration instead of an unused OTP app, so they run as intended in production rather than silently doing nothing. - **CSV date round-trip** – Date custom-field values are now exported as ISO-8601 (`YYYY-MM-DD`), so an exported CSV can be re-imported without date-parsing errors. - **CSV import – fee-status columns ignored** – Columns such as `Bezahlstatus` / `Membership Fee Status` are always ignored on import and never stored as a custom-field value, even when a custom field of the same name exists. - **Column-header tooltips clipped** – Tooltips on the members-overview column headers are no longer clipped by the sticky table header. From 18fb954f73ca8edf6d0d914bcf7eceb1a4518f58 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:49:50 +0200 Subject: [PATCH 18/27] test(membership-fees): share create_fee_type and create_cycle fixtures Replace the create_fee_type/create_cycle helpers duplicated across 18/8 membership-fee test files with a single shared definition in Mv.Fixtures, reconciling the divergent local signatures (including the reversed argument order) into one superset so behavior is unchanged. --- .../member_cycle_calculations_test.exs | 36 +---- .../member_type_change_integration_test.exs | 35 +---- .../changes/validate_same_interval_test.exs | 18 +-- .../member_cycle_integration_test.exs | 18 +-- .../membership_fee_cycle_test.exs | 34 +---- .../membership_fee_type_integration_test.exs | 17 +-- .../cycle_generator_edge_cases_test.exs | 18 +-- .../membership_fees/cycle_generator_test.exs | 18 +-- .../membership_fee_cycle_policies_test.exs | 56 +++----- test/mv/statistics_test.exs | 26 +--- .../membership_fee_type_live/form_test.exs | 18 +-- .../membership_fee_type_live/index_test.exs | 17 +-- .../form_membership_fee_type_test.exs | 19 +-- .../index/membership_fee_status_test.exs | 42 +----- .../index_membership_fee_status_test.exs | 136 ++++++++++-------- test/mv_web/member_live/index_test.exs | 62 ++------ .../membership_fee_integration_test.exs | 19 +-- .../member_live/show_membership_fees_test.exs | 125 ++++++++-------- test/support/fixtures.ex | 78 ++++++++++ 19 files changed, 279 insertions(+), 513 deletions(-) diff --git a/test/membership/member_cycle_calculations_test.exs b/test/membership/member_cycle_calculations_test.exs index 9be1272..860d784 100644 --- a/test/membership/member_cycle_calculations_test.exs +++ b/test/membership/member_cycle_calculations_test.exs @@ -4,30 +4,15 @@ defmodule Mv.Membership.MemberCycleCalculationsTest do """ use Mv.DataCase, async: true + import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] + alias Mv.MembershipFees.CalendarCycles - alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member defp create_member(attrs, actor) do default_attrs = %{ @@ -41,23 +26,6 @@ defmodule Mv.Membership.MemberCycleCalculationsTest do member end - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs, actor) do - default_attrs = %{ - cycle_start: ~D[2024-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - describe "current_cycle_status" do test "returns status of current cycle for member with active cycle", %{actor: actor} do fee_type = create_fee_type(%{interval: :yearly}, actor) diff --git a/test/membership/member_type_change_integration_test.exs b/test/membership/member_type_change_integration_test.exs index 35b3137..a2baf78 100644 --- a/test/membership/member_type_change_integration_test.exs +++ b/test/membership/member_type_change_integration_test.exs @@ -4,9 +4,10 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do """ use Mv.DataCase, async: true + import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] + alias Mv.MembershipFees.CalendarCycles alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query @@ -15,21 +16,6 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member defp create_member(attrs, actor) do default_attrs = %{ @@ -44,23 +30,6 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do member end - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs, actor) do - default_attrs = %{ - cycle_start: ~D[2024-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - describe "type change cycle regeneration" do test "future unpaid cycles are regenerated with new amount", %{actor: actor} do today = Date.utc_today() diff --git a/test/membership_fees/changes/validate_same_interval_test.exs b/test/membership_fees/changes/validate_same_interval_test.exs index 31c2847..21e0a55 100644 --- a/test/membership_fees/changes/validate_same_interval_test.exs +++ b/test/membership_fees/changes/validate_same_interval_test.exs @@ -4,29 +4,15 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do """ use Mv.DataCase, async: true + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.MembershipFees.Changes.ValidateSameInterval - alias Mv.MembershipFees.MembershipFeeType setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member defp create_member(attrs, actor) do default_attrs = %{ diff --git a/test/membership_fees/member_cycle_integration_test.exs b/test/membership_fees/member_cycle_integration_test.exs index 76f4d08..c6a1486 100644 --- a/test/membership_fees/member_cycle_integration_test.exs +++ b/test/membership_fees/member_cycle_integration_test.exs @@ -4,8 +4,9 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query @@ -14,21 +15,6 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to set up settings defp setup_settings(include_joining_cycle, actor) do {:ok, settings} = Mv.Membership.get_settings() diff --git a/test/membership_fees/membership_fee_cycle_test.exs b/test/membership_fees/membership_fee_cycle_test.exs index 8770134..f4f89af 100644 --- a/test/membership_fees/membership_fee_cycle_test.exs +++ b/test/membership_fees/membership_fee_cycle_test.exs @@ -4,29 +4,15 @@ defmodule Mv.MembershipFees.MembershipFeeCycleTest do """ use Mv.DataCase, async: true + import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] + alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member defp create_member(attrs, actor) do default_attrs = %{ @@ -40,22 +26,6 @@ defmodule Mv.MembershipFees.MembershipFeeCycleTest do member end - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs, actor) do - default_attrs = %{ - cycle_start: ~D[2024-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - describe "status defaults" do test "status defaults to :unpaid when creating a cycle", %{actor: actor} do fee_type = create_fee_type(%{interval: :yearly}, actor) diff --git a/test/membership_fees/membership_fee_type_integration_test.exs b/test/membership_fees/membership_fee_type_integration_test.exs index f834094..86ef284 100644 --- a/test/membership_fees/membership_fee_type_integration_test.exs +++ b/test/membership_fees/membership_fee_type_integration_test.exs @@ -4,6 +4,8 @@ defmodule Mv.MembershipFees.MembershipFeeTypeIntegrationTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.Membership.Member alias Mv.MembershipFees.MembershipFeeCycle alias Mv.MembershipFees.MembershipFeeType @@ -15,21 +17,6 @@ defmodule Mv.MembershipFees.MembershipFeeTypeIntegrationTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - describe "admin can create membership fee type" do test "creates type with all fields", %{actor: actor} do attrs = %{ 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 a9e3316..eb9da26 100644 --- a/test/mv/membership_fees/cycle_generator_edge_cases_test.exs +++ b/test/mv/membership_fees/cycle_generator_edge_cases_test.exs @@ -12,9 +12,10 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.MembershipFees.CycleGenerator alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query @@ -23,21 +24,6 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member. Note: If membership_fee_type_id is provided, # cycles will be auto-generated during creation in test environment. defp create_member(attrs, actor) do diff --git a/test/mv/membership_fees/cycle_generator_test.exs b/test/mv/membership_fees/cycle_generator_test.exs index f193903..a715ca9 100644 --- a/test/mv/membership_fees/cycle_generator_test.exs +++ b/test/mv/membership_fees/cycle_generator_test.exs @@ -4,9 +4,10 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.MembershipFees.CycleGenerator alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query @@ -15,21 +16,6 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member without triggering cycle generation defp create_member_without_cycles(attrs, actor) do default_attrs = %{ diff --git a/test/mv/membership_fees/membership_fee_cycle_policies_test.exs b/test/mv/membership_fees/membership_fee_cycle_policies_test.exs index cf7e889..e3e37ba 100644 --- a/test/mv/membership_fees/membership_fee_cycle_policies_test.exs +++ b/test/mv/membership_fees/membership_fee_cycle_policies_test.exs @@ -8,6 +8,8 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 1, create_cycle: 3] + alias Mv.Membership alias Mv.MembershipFees @@ -32,41 +34,15 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do member end - defp create_fee_type_fixture do - admin = Mv.Fixtures.user_with_role_fixture("admin") - - {:ok, fee_type} = - MembershipFees.create_membership_fee_type( - %{ - name: "Test Fee #{System.unique_integer([:positive])}", - amount: Decimal.new("10.00"), - interval: :yearly, - description: "Test" - }, - actor: admin - ) - - fee_type + defp fee_type_fixture do + create_fee_type(%{amount: Decimal.new("10.00"), description: "Test"}) end - defp create_cycle_fixture do - admin = Mv.Fixtures.user_with_role_fixture("admin") - member = create_member_fixture() - fee_type = create_fee_type_fixture() - - {:ok, cycle} = - MembershipFees.create_membership_fee_cycle( - %{ - member_id: member.id, - membership_fee_type_id: fee_type.id, - cycle_start: Date.utc_today(), - amount: Decimal.new("10.00"), - status: :unpaid - }, - actor: admin - ) - - cycle + defp cycle_fixture do + create_cycle(create_member_fixture(), fee_type_fixture(), %{ + cycle_start: Date.utc_today(), + amount: Decimal.new("10.00") + }) end describe "own_data permission set" do @@ -74,7 +50,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do user = Mv.Fixtures.user_with_role_fixture("own_data") linked_member = create_member_fixture() other_member = create_member_fixture() - fee_type = create_fee_type_fixture() + fee_type = fee_type_fixture() admin = Mv.Fixtures.user_with_role_fixture("admin") user = @@ -130,7 +106,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do describe "read_only permission set" do setup %{actor: actor} do user = Mv.Fixtures.user_with_role_fixture("read_only") - cycle = create_cycle_fixture() + cycle = cycle_fixture() %{actor: actor, user: user, cycle: cycle} end @@ -156,7 +132,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do test "cannot create cycle (returns forbidden)", %{user: user, actor: _actor} do member = create_member_fixture() - fee_type = create_fee_type_fixture() + fee_type = fee_type_fixture() assert {:error, %Ash.Error.Forbidden{}} = MembershipFees.create_membership_fee_cycle( @@ -180,7 +156,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do describe "normal_user permission set" do setup %{actor: actor} do user = Mv.Fixtures.user_with_role_fixture("normal_user") - cycle = create_cycle_fixture() + cycle = cycle_fixture() %{actor: actor, user: user, cycle: cycle} end @@ -210,7 +186,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do test "can create cycle", %{user: user, actor: _actor} do member = create_member_fixture() - fee_type = create_fee_type_fixture() + fee_type = fee_type_fixture() assert {:ok, created} = MembershipFees.create_membership_fee_cycle( @@ -235,7 +211,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do describe "admin permission set" do setup %{actor: actor} do user = Mv.Fixtures.user_with_role_fixture("admin") - cycle = create_cycle_fixture() + cycle = cycle_fixture() %{actor: actor, user: user, cycle: cycle} end @@ -270,7 +246,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do test "can create cycle", %{user: user, actor: _actor} do member = create_member_fixture() - fee_type = create_fee_type_fixture() + fee_type = fee_type_fixture() assert {:ok, created} = MembershipFees.create_membership_fee_cycle( diff --git a/test/mv/statistics_test.exs b/test/mv/statistics_test.exs index 6b72ffb..e016a10 100644 --- a/test/mv/statistics_test.exs +++ b/test/mv/statistics_test.exs @@ -6,11 +6,11 @@ defmodule Mv.StatisticsTest do require Ash.Query import Ash.Expr + import Mv.Fixtures, only: [create_fee_type: 2] alias Mv.Membership.Member alias Mv.MembershipFees alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType alias Mv.Statistics setup do @@ -18,22 +18,6 @@ defmodule Mv.StatisticsTest do %{actor: actor} end - defp create_fee_type(actor, attrs) do - MembershipFeeType - |> Ash.Changeset.for_create( - :create, - Map.merge( - %{ - name: "Test Fee #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - }, - attrs - ) - ) - |> Ash.create!(actor: actor) - end - describe "first_join_year/1" do test "returns the year of the earliest join_date", %{actor: actor} do Mv.Fixtures.member_fixture(%{join_date: ~D[2019-03-15]}) @@ -131,7 +115,7 @@ defmodule Mv.StatisticsTest do end test "returns totals by status for cycles in that year", %{actor: actor} do - fee_type = create_fee_type(actor, %{amount: Decimal.new("50.00")}) + fee_type = create_fee_type(%{amount: Decimal.new("50.00")}, actor) # Creating members with fee type triggers cycle generation (2020..today). We use 2024 cycles. _member1 = @@ -171,8 +155,8 @@ defmodule Mv.StatisticsTest do test "when fee_type_id is passed in opts, returns only cycles of that fee type", %{ actor: actor } do - fee_type_a = create_fee_type(actor, %{amount: Decimal.new("30.00")}) - fee_type_b = create_fee_type(actor, %{amount: Decimal.new("70.00")}) + fee_type_a = create_fee_type(%{amount: Decimal.new("30.00")}, actor) + fee_type_b = create_fee_type(%{amount: Decimal.new("70.00")}, actor) _m1 = Mv.Fixtures.member_fixture(%{ @@ -207,7 +191,7 @@ defmodule Mv.StatisticsTest do end test "returns sum of amount for all unpaid cycles", %{actor: actor} do - fee_type = create_fee_type(actor, %{amount: Decimal.new("50.00")}) + fee_type = create_fee_type(%{amount: Decimal.new("50.00")}, actor) _member = Mv.Fixtures.member_fixture(%{ 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 a836c4d..998fb65 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 @@ -5,6 +5,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 1] alias Mv.MembershipFees.MembershipFeeType @@ -17,23 +18,6 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do %{conn: authenticated_conn, user: user} end - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - # Helper to create a member defp create_member(attrs) do system_actor = Mv.Helpers.SystemActor.get_system_actor() 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 c65341f..8c99d90 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 @@ -5,6 +5,7 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 2] alias Mv.MembershipFees.MembershipFeeType @@ -13,22 +14,6 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do # Use global setup from ConnCase which provides admin user with role # No custom setup needed - # Helper to create a membership fee type - # Uses admin_user to test permissions (UI-/Permissions-nah) - defp create_fee_type(attrs, admin_user) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: admin_user) - end - # Helper to create a member. Requires actor (e.g. admin_user from setup); no fallback so # missing-actor bugs are not masked in tests. defp create_member(attrs, actor) do 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 5382dfc..01812df 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 @@ -5,27 +5,10 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - - alias Mv.MembershipFees.MembershipFeeType + import Mv.Fixtures, only: [create_fee_type: 2] require Ash.Query - # Helper to create a membership fee type - # Uses admin_user to test permissions (UI-/Permissions-nah) - defp create_fee_type(attrs, admin_user) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: admin_user) - end - # Helper to create a member # Uses admin_user to test permissions (UI-/Permissions-nah) defp create_member(attrs, admin_user) do 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 5d96e68..8412f61 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 @@ -4,30 +4,13 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 1, create_cycle: 3] + alias Mv.Membership.Member - alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType alias MvWeb.MemberLive.Index.MembershipFeeStatus require Ash.Query - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - # Helper to create a member defp create_member(attrs) do system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -43,27 +26,6 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do member end - # Helper to create a cycle - # Note: Does not delete existing cycles - tests should manage their own test data - # If cleanup is needed, it should be done in setup or explicitly in the test - defp create_cycle(member, fee_type, attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - cycle_start: ~D[2023-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - describe "load_cycles_for_members/2" do test "efficiently loads cycles for members" do fee_type = create_fee_type(%{interval: :yearly}) 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 ce6bee8..2efe055 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 @@ -5,29 +5,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 1, create_cycle: 3] alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - # Helper to create a member defp create_member(attrs) do system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -43,38 +26,16 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do member end - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - # Delete any auto-generated cycles first to avoid conflicts - existing_cycles = - MembershipFeeCycle - |> Ash.Query.filter(member_id == ^member.id) - |> Ash.read!(actor: system_actor) - - Enum.each(existing_cycles, fn cycle -> Ash.destroy!(cycle, actor: system_actor) end) - - default_attrs = %{ - cycle_start: ~D[2023-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - describe "status column display" do test "shows status column in member list", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) {:ok, _view, html} = live(conn, "/members") @@ -85,8 +46,18 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do test "shows last completed cycle status by default", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2022-01-01], status: :paid}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2022-01-01], + status: :paid, + replace_existing: true + }) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -102,8 +73,17 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do today = Date.utc_today() current_year_start = %{today | month: 1, day: 1} - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) - create_cycle(member, fee_type, %{cycle_start: current_year_start, status: :suspended}) + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) + + create_cycle(member, fee_type, %{ + cycle_start: current_year_start, + status: :suspended, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -120,7 +100,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do test "shows correct color coding for paid status", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -131,7 +116,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do test "shows correct color coding for unpaid status", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -142,7 +132,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do test "shows correct color coding for suspended status", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :suspended}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :suspended, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -169,11 +164,21 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do # Member with unpaid last cycle member1 = create_member(%{first_name: "UnpaidMember", membership_fee_type_id: fee_type.id}) - create_cycle(member1, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + create_cycle(member1, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) # Member with paid last cycle member2 = create_member(%{first_name: "PaidMember", membership_fee_type_id: fee_type.id}) - create_cycle(member2, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + + create_cycle(member2, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -205,11 +210,21 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do # Member with unpaid current cycle member1 = create_member(%{first_name: "UnpaidCurrent", membership_fee_type_id: fee_type.id}) - create_cycle(member1, fee_type, %{cycle_start: current_year_start, status: :unpaid}) + + create_cycle(member1, fee_type, %{ + cycle_start: current_year_start, + status: :unpaid, + replace_existing: true + }) # Member with paid current cycle member2 = create_member(%{first_name: "PaidCurrent", membership_fee_type_id: fee_type.id}) - create_cycle(member2, fee_type, %{cycle_start: current_year_start, status: :paid}) + + create_cycle(member2, fee_type, %{ + cycle_start: current_year_start, + status: :paid, + replace_existing: true + }) system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -243,7 +258,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do # Create multiple members with cycles Enum.each(1..5, fn _ -> member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) end) {:ok, _view, html} = live(conn, "/members") diff --git a/test/mv_web/member_live/index_test.exs b/test/mv_web/member_live/index_test.exs index 0cde8fd..be61c9c 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -7,49 +7,9 @@ defmodule MvWeb.MemberLive.IndexTest do alias Mv.Membership alias Mv.Membership.CustomField alias Mv.Membership.CustomFieldValue - alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType alias MvWeb.MemberLive.Index, as: MemberIndex - # Helper to create a membership fee type (shared across all tests) - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - - # Helper to create a cycle (shared across all tests) - defp create_cycle(member, fee_type, attrs, actor) do - # Delete any auto-generated cycles first to avoid conflicts - existing_cycles = - MembershipFeeCycle - |> Ash.Query.filter(member_id == ^member.id) - |> Ash.read!(actor: actor) - - Enum.each(existing_cycles, fn cycle -> Ash.destroy!(cycle, actor: actor) end) - - default_attrs = %{ - cycle_start: ~D[2023-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end + import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] describe "desktop layout: scroll container and sticky table header" do @describetag :ui @@ -1110,7 +1070,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: last_year_start, status: :paid}, + %{cycle_start: last_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -1127,7 +1087,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: last_year_start, status: :unpaid}, + %{cycle_start: last_year_start, status: :unpaid, replace_existing: true}, system_actor ) @@ -1157,7 +1117,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: last_year_start, status: :paid}, + %{cycle_start: last_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -1174,7 +1134,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: last_year_start, status: :unpaid}, + %{cycle_start: last_year_start, status: :unpaid, replace_existing: true}, system_actor ) @@ -1204,7 +1164,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: current_year_start, status: :paid}, + %{cycle_start: current_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -1221,7 +1181,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: current_year_start, status: :unpaid}, + %{cycle_start: current_year_start, status: :unpaid, replace_existing: true}, system_actor ) @@ -1251,7 +1211,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: current_year_start, status: :paid}, + %{cycle_start: current_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -1268,7 +1228,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: current_year_start, status: :unpaid}, + %{cycle_start: current_year_start, status: :unpaid, replace_existing: true}, system_actor ) @@ -2196,7 +2156,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( member_paid_true, fee_type, - %{cycle_start: last_year_start, status: :paid}, + %{cycle_start: last_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -2224,7 +2184,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( member_unpaid_true, fee_type, - %{cycle_start: last_year_start, status: :unpaid}, + %{cycle_start: last_year_start, status: :unpaid, replace_existing: true}, system_actor ) 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 ac60220..16f1064 100644 --- a/test/mv_web/member_live/membership_fee_integration_test.exs +++ b/test/mv_web/member_live/membership_fee_integration_test.exs @@ -5,29 +5,12 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 1] alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - # Helper to create a member defp create_member(attrs) do system_actor = Mv.Helpers.SystemActor.get_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 394d743..9c1b73f 100644 --- a/test/mv_web/member_live/show_membership_fees_test.exs +++ b/test/mv_web/member_live/show_membership_fees_test.exs @@ -5,56 +5,12 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 1, create_cycle: 3] alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - # Delete any auto-generated cycles first to avoid conflicts - existing_cycles = - MembershipFeeCycle - |> Ash.Query.filter(member_id == ^member.id) - |> Ash.read!(actor: system_actor) - - Enum.each(existing_cycles, fn cycle -> Ash.destroy!(cycle, actor: system_actor) end) - - default_attrs = %{ - cycle_start: ~D[2023-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - describe "cycle-regeneration control tooltip (§3.5 icon/tooltip audit)" do test "the regenerate_cycles control carries a tooltip and accessible label", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) @@ -76,8 +32,19 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - _cycle1 = create_cycle(member, fee_type, %{cycle_start: ~D[2022-01-01], status: :paid}) - _cycle2 = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + _cycle1 = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2022-01-01], + status: :paid, + replace_existing: true + }) + + _cycle2 = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -101,7 +68,8 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do create_cycle(member, fee_type, %{ cycle_start: ~D[2023-01-01], amount: Decimal.new("60.00"), - status: :paid + status: :paid, + replace_existing: true }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -158,7 +126,12 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -189,7 +162,12 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -220,7 +198,12 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -316,7 +299,13 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do } do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - _cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + _cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -335,7 +324,13 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do } do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -361,7 +356,13 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do # This test verifies that Ash.destroy(cycle, actor: read_only_user) returns Forbidden. fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) assert {:error, %Ash.Error.Forbidden{}} = Ash.destroy(cycle, domain: Mv.MembershipFees, actor: read_only_user) @@ -389,8 +390,20 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - _c1 = create_cycle(member, fee_type, %{cycle_start: ~D[2022-01-01], status: :paid}) - _c2 = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + _c1 = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2022-01-01], + status: :paid, + replace_existing: true + }) + + _c2 = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") diff --git a/test/support/fixtures.ex b/test/support/fixtures.ex index 73bf12a..e022600 100644 --- a/test/support/fixtures.ex +++ b/test/support/fixtures.ex @@ -335,4 +335,82 @@ defmodule Mv.Fixtures do {:ok, request} = Membership.confirm_join_request(token, actor: nil) request end + + @doc """ + Creates a membership fee type with default or custom attributes. + + Defaults: a unique `name`, `amount` 50.00, `interval` :yearly. + + ## Parameters + - `attrs` - Map of attributes to override defaults (e.g. `%{interval: :monthly}`). + - `actor` - the authorization actor; defaults to the system actor when omitted/nil. + + ## Returns + - MembershipFeeType struct. + + ## Examples + + iex> create_fee_type(%{interval: :monthly}) + %Mv.MembershipFees.MembershipFeeType{interval: :monthly, ...} + + iex> create_fee_type(%{amount: Decimal.new("10.00")}, admin) + %Mv.MembershipFees.MembershipFeeType{...} + """ + def create_fee_type(attrs \\ %{}, actor \\ nil) do + actor = actor || SystemActor.get_system_actor() + + default_attrs = %{ + name: "Test Fee Type #{System.unique_integer([:positive])}", + amount: Decimal.new("50.00"), + interval: :yearly + } + + Mv.MembershipFees.MembershipFeeType + |> Ash.Changeset.for_create(:create, Map.merge(default_attrs, attrs)) + |> Ash.create!(actor: actor) + end + + @doc """ + Creates a membership fee cycle for the given member and fee type. + + Defaults: `cycle_start` ~D[2024-01-01], `amount` 50.00, `status` :unpaid, + with `member_id`/`membership_fee_type_id` derived from the passed structs. + + ## Parameters + - `member` - the Member struct the cycle belongs to. + - `fee_type` - the MembershipFeeType struct the cycle references. + - `attrs` - Map overriding the cycle defaults (e.g. `%{cycle_start: ~D[2023-01-01], status: :paid}`). + A reserved `:replace_existing` key (truthy) deletes any pre-existing cycles for the member + before creating the new one (used where auto-generated cycles would otherwise conflict); + it is stripped from the attrs and never passed to the create action. Defaults to absent/false. + - `actor` - the authorization actor; defaults to the system actor when omitted/nil. + + ## Returns + - MembershipFeeCycle struct. + """ + def create_cycle(member, fee_type, attrs \\ %{}, actor \\ nil) do + actor = actor || SystemActor.get_system_actor() + {replace_existing, attrs} = Map.pop(attrs, :replace_existing, false) + + if replace_existing do + require Ash.Query + + Mv.MembershipFees.MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member.id) + |> Ash.read!(actor: actor) + |> Enum.each(&Ash.destroy!(&1, actor: actor)) + end + + default_attrs = %{ + cycle_start: ~D[2024-01-01], + amount: Decimal.new("50.00"), + member_id: member.id, + membership_fee_type_id: fee_type.id, + status: :unpaid + } + + Mv.MembershipFees.MembershipFeeCycle + |> Ash.Changeset.for_create(:create, Map.merge(default_attrs, attrs)) + |> Ash.create!(actor: actor) + end end From 3bd55fbfec75f394271f393643b3447ec4f16b24 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:50:24 +0200 Subject: [PATCH 19/27] test(seeds): drop the dead per-process seeds-run guard --- test/seeds_test.exs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/test/seeds_test.exs b/test/seeds_test.exs index c729dbb..2721d13 100644 --- a/test/seeds_test.exs +++ b/test/seeds_test.exs @@ -24,21 +24,11 @@ defmodule Mv.SeedsTest do require Ash.Query - # Module attribute to track if seeds have been run - # This allows us to run seeds once per test module while respecting sandbox isolation - @seeds_run_key :seeds_test_run - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() - # Run seeds once per test process (due to sandbox isolation, each test runs in its own process) - # Process.get/put ensures seeds are only executed once per test process, not once per test - # Note: With async: false and sandbox isolation, this effectively runs seeds once per test, - # but the guard prevents multiple executions within the same test process if setup is called multiple times - unless Process.get(@seeds_run_key) do - Code.eval_file("priv/repo/seeds.exs") - Process.put(@seeds_run_key, true) - end + # Each test runs in its own sandbox-owned process, so seeds are loaded once per test. + Code.eval_file("priv/repo/seeds.exs") %{actor: system_actor} end From ccd1f81e3ec50ce1fc07e132e9c316a0b85b5f87 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:50:57 +0200 Subject: [PATCH 20/27] test(member-live): assert rendered behavior instead of socket internals in the index view Replace :sys.get_state assertions on the LiveView socket with assertions on rendered output, so the tests pin user-visible behavior rather than internal state; the few sites with no observable equivalent are kept and annotated. --- test/mv_web/member_live/index_test.exs | 316 +++++++++++++------------ 1 file changed, 169 insertions(+), 147 deletions(-) diff --git a/test/mv_web/member_live/index_test.exs b/test/mv_web/member_live/index_test.exs index be61c9c..cd166cc 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -1,4 +1,8 @@ defmodule MvWeb.MemberLive.IndexTest do + # async: false on purpose: the @slow "boolean filter performance with 150 members" + # test asserts a wall-clock budget (duration < 1000ms). Running this module in + # parallel with others adds CPU contention that inflates that measurement and makes + # the timing assertion flaky, so this module stays synchronous. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query @@ -297,10 +301,14 @@ defmodule MvWeb.MemberLive.IndexTest do send(view.pid, {:search_changed, "Friedrich"}) - state = :sys.get_state(view.pid) - - assert state.socket.assigns.query == "Friedrich" - assert is_list(state.socket.assigns.members) + # Rationale: this exercises the handle_info(:search_changed) callback in isolation. + # The search box value is owned by SearchBarComponent (assign_new), and scope is + # recomputed on handle_params rather than this handle_info, so the updated :query + # and :members assigns have no faithful rendered proxy here. The assigns are + # asserted on internal state to preserve the original coverage of the callback. + assigns = :sys.get_state(view.pid).socket.assigns + assert assigns.query == "Friedrich" + assert is_list(assigns.members) end @tag :ui @@ -393,6 +401,38 @@ defmodule MvWeb.MemberLive.IndexTest do |> LazyHTML.query(~s([data-testid="bulk-actions-scope-badge"])) end + # Opens the bulk-actions dropdown and returns the mailto link's BCC payload + # (everything after "mailto:?bcc="). This is the observable carrier of the + # recipient set / recipient_count, replacing direct socket-assign inspection. + defp mailto_bcc(view) do + view |> element(~s([data-testid="bulk-actions-button"])) |> render_click() + + href = + render(view) + |> LazyHTML.from_fragment() + |> LazyHTML.query(~s([data-testid="bulk-actions-mailto"])) + |> LazyHTML.attribute("href") + |> List.first() + + case href do + "mailto:?bcc=" <> bcc -> bcc + other -> other || "" + end + end + + # Opens the member-filter dropdown so its boolean filter controls are rendered. + defp open_member_filter(view) do + view + |> element(~s(button[phx-click="toggle_dropdown"][aria-label="Filter members"])) + |> render_click() + end + + # Returns the rendered HTML of the member-filter dropdown (with it open). + defp member_filter_html(view) do + open_member_filter(view) + render(view) + end + describe "copy_emails feature" do setup do system_actor = SystemActor.get_system_actor() @@ -528,15 +568,13 @@ defmodule MvWeb.MemberLive.IndexTest do # Select two members by sending the select_member event directly render_click(view, "select_member", %{"id" => member1.id}) - render_click(view, "select_member", %{"id" => member2.id}) + html = render_click(view, "select_member", %{"id" => member2.id}) - # Get the socket state to verify the formatted email string - state = :sys.get_state(view.pid) - selected_members = state.socket.assigns.selected_members - - # Verify MapSet is used - assert %MapSet{} = selected_members - assert MapSet.size(selected_members) == 2 + # Both selected members are observably reflected: their row checkboxes are + # checked and the scope badge shows the selection count ("2"). + assert has_element?(view, ~s(input[role="checkbox"][name="#{member1.id}"][checked])) + assert has_element?(view, ~s(input[role="checkbox"][name="#{member2.id}"][checked])) + assert scope_badge(html) |> LazyHTML.text() |> String.trim() == "2" end test "email format is 'First Last ' with comma separator", %{ @@ -969,26 +1007,23 @@ defmodule MvWeb.MemberLive.IndexTest do test "scope is :all when nothing selected and no filter", %{conn: conn} do conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/members") + {:ok, _view, html} = live(conn, "/members") - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.scope == :all + # :all scope renders the muted "all" badge. + assert scope_badge(html) |> LazyHTML.text() |> String.trim() == "all" end test "scope is :filtered when a search term is active", %{conn: conn} do conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/members?query=Scope") + {:ok, _view, html} = live(conn, "/members?query=Scope") - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.scope == :filtered + # An active search narrows the list, so the scope badge reads "filtered". + assert scope_badge(html) |> LazyHTML.text() |> String.trim() == "filtered" end test "scope is :filtered when a non-search filter is active", %{conn: conn} do conn = conn_with_oidc_user(conn) - {:ok, view, html} = live(conn, "/members?cycle_status_filter=paid") - - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.scope == :filtered + {:ok, _view, html} = live(conn, "/members?cycle_status_filter=paid") badge = scope_badge(html) assert badge |> LazyHTML.text() |> String.trim() == "filtered" @@ -1001,10 +1036,13 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") - render_click(view, "select_member", %{"id" => member1.id}) + html = render_click(view, "select_member", %{"id" => member1.id}) - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.scope == :selection + # A selection switches the badge to the emphasized (primary) variant whose + # label is the selected count ("1"), which is the observable proxy for scope == :selection. + badge = scope_badge(html) + assert badge |> LazyHTML.text() |> String.trim() == "1" + assert badge |> LazyHTML.attribute("class") |> List.first() =~ "badge-primary" end test "with no selection, recipient_count and mailto_bcc cover all members", %{ @@ -1013,11 +1051,11 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") - assigns = :sys.get_state(view.pid).socket.assigns + # The mailto link's BCC is the observable carrier of recipient_count/mailto_bcc. + bcc = mailto_bcc(view) # Both seeded members have an email, so the no-selection scope covers both. - assert assigns.recipient_count == 2 - assert assigns.mailto_bcc =~ "scope1%40example.com" - assert assigns.mailto_bcc =~ "scope2%40example.com" + assert bcc =~ "scope1%40example.com" + assert bcc =~ "scope2%40example.com" end test "with a selection, recipient_count and mailto_bcc cover only the selection", %{ @@ -1029,10 +1067,9 @@ defmodule MvWeb.MemberLive.IndexTest do render_click(view, "select_member", %{"id" => member1.id}) - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.recipient_count == 1 - assert assigns.mailto_bcc =~ "scope1%40example.com" - refute assigns.mailto_bcc =~ "scope2%40example.com" + bcc = mailto_bcc(view) + assert bcc =~ "scope1%40example.com" + refute bcc =~ "scope2%40example.com" end end @@ -1298,6 +1335,9 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") + # Rationale: with no boolean fields and no active filter there is no rendered + # filter control or active-count badge to observe, so the empty initial filter + # map is asserted on internal state directly. state = :sys.get_state(view.pid) assert state.socket.assigns.boolean_custom_field_filters == %{} end @@ -1309,6 +1349,9 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") + # Rationale: the absence of boolean fields means the filter dropdown renders + # no boolean fieldsets; "empty list" has no positive rendered signal, so it is + # asserted on internal state directly. state = :sys.get_state(view.pid) assert state.socket.assigns.boolean_custom_fields == [] end @@ -1319,89 +1362,82 @@ defmodule MvWeb.MemberLive.IndexTest do # Create boolean and non-boolean custom fields boolean_field1 = create_boolean_custom_field(%{name: "Active Member"}) boolean_field2 = create_boolean_custom_field(%{name: "Newsletter Subscription"}) - _string_field = create_string_custom_field(%{name: "Phone Number"}) + string_field = create_string_custom_field(%{name: "Phone Number"}) {:ok, view, _html} = live(conn, "/members") + open_member_filter(view) - state = :sys.get_state(view.pid) - boolean_custom_fields = state.socket.assigns.boolean_custom_fields - - # Should only contain boolean fields - assert length(boolean_custom_fields) == 2 - assert Enum.all?(boolean_custom_fields, &(&1.value_type == :boolean)) - assert Enum.any?(boolean_custom_fields, &(&1.id == boolean_field1.id)) - assert Enum.any?(boolean_custom_fields, &(&1.id == boolean_field2.id)) + # Only the boolean fields render a tri-state filter control; the string field does not. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field1.id}-all"}") + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field2.id}-all"}") + refute has_element?(view, "##{"custom-boolean-filter-#{string_field.id}-all"}") end test "mount sorts boolean custom fields by name ascending", %{conn: conn} do conn = conn_with_oidc_user(conn) # Create boolean fields with specific names to test sorting - _boolean_field_z = create_boolean_custom_field(%{name: "Zebra Field"}) - _boolean_field_a = create_boolean_custom_field(%{name: "Alpha Field"}) - _boolean_field_m = create_boolean_custom_field(%{name: "Middle Field"}) + field_z = create_boolean_custom_field(%{name: "Zebra Field"}) + field_a = create_boolean_custom_field(%{name: "Alpha Field"}) + field_m = create_boolean_custom_field(%{name: "Middle Field"}) {:ok, view, _html} = live(conn, "/members") + html = member_filter_html(view) - state = :sys.get_state(view.pid) - boolean_custom_fields = state.socket.assigns.boolean_custom_fields + # The rendered boolean filter controls appear in name-ascending order. + rendered_order = + html + |> LazyHTML.from_fragment() + |> LazyHTML.query(~s(input[id$="-all"][name^="custom_boolean"])) + |> LazyHTML.attribute("id") + |> Enum.map( + &(&1 + |> String.replace_prefix("custom-boolean-filter-", "") + |> String.replace_suffix("-all", "")) + ) - # Should be sorted by name ascending - names = Enum.map(boolean_custom_fields, & &1.name) - assert names == ["Alpha Field", "Middle Field", "Zebra Field"] + assert rendered_order == [field_a.id, field_m.id, field_z.id] end test "handle_params parses bf_ values correctly", %{conn: conn} do conn = conn_with_oidc_user(conn) boolean_field = create_boolean_custom_field() - # Test true value - {:ok, view1, _html} = - live(conn, "/members?bf_#{boolean_field.id}=true") + # Test true value: the "Yes" radio is checked (the boolean true, not the string "true"). + {:ok, view1, _html} = live(conn, "/members?bf_#{boolean_field.id}=true") + open_member_filter(view1) + assert has_element?(view1, "##{"custom-boolean-filter-#{boolean_field.id}-true"}[checked]") + refute has_element?(view1, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]") - state1 = :sys.get_state(view1.pid) - filters1 = state1.socket.assigns.boolean_custom_field_filters - assert filters1[boolean_field.id] == true - refute filters1[boolean_field.id] == "true" - - # Test false value - {:ok, view2, _html} = - live(conn, "/members?bf_#{boolean_field.id}=false") - - state2 = :sys.get_state(view2.pid) - filters2 = state2.socket.assigns.boolean_custom_field_filters - assert filters2[boolean_field.id] == false - refute filters2[boolean_field.id] == "false" + # Test false value: the "No" radio is checked. + {:ok, view2, _html} = live(conn, "/members?bf_#{boolean_field.id}=false") + open_member_filter(view2) + assert has_element?(view2, "##{"custom-boolean-filter-#{boolean_field.id}-false"}[checked]") + refute has_element?(view2, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]") end test "handle_params ignores non-existent custom field IDs", %{conn: conn} do conn = conn_with_oidc_user(conn) fake_id = Ecto.UUID.generate() - {:ok, view, _html} = - live(conn, "/members?bf_#{fake_id}=true") + {:ok, view, _html} = live(conn, "/members?bf_#{fake_id}=true") + open_member_filter(view) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters - - # Filter should not be added for non-existent custom field - refute Map.has_key?(filters, fake_id) - assert filters == %{} + # No filter control exists for a non-existent field, and no active-filter badge appears. + refute has_element?(view, "##{"custom-boolean-filter-#{fake_id}-true"}") + refute has_element?(view, ~s(button[aria-label="Filter members"].btn-active)) end test "handle_params ignores non-boolean custom fields", %{conn: conn} do conn = conn_with_oidc_user(conn) string_field = create_string_custom_field() - {:ok, view, _html} = - live(conn, "/members?bf_#{string_field.id}=true") + {:ok, view, _html} = live(conn, "/members?bf_#{string_field.id}=true") + open_member_filter(view) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters - - # Filter should not be added for non-boolean custom field - refute Map.has_key?(filters, string_field.id) - assert filters == %{} + # A string field is never rendered as a boolean filter, and no filter becomes active. + refute has_element?(view, "##{"custom-boolean-filter-#{string_field.id}-true"}") + refute has_element?(view, ~s(button[aria-label="Filter members"].btn-active)) end test "handle_params ignores invalid filter values", %{conn: conn} do @@ -1412,15 +1448,12 @@ defmodule MvWeb.MemberLive.IndexTest do invalid_values = ["1", "0", "yes", "no", "True", "False", "", "invalid", "null"] for invalid_value <- invalid_values do - {:ok, view, _html} = - live(conn, "/members?bf_#{boolean_field.id}=#{invalid_value}") + {:ok, view, _html} = live(conn, "/members?bf_#{boolean_field.id}=#{invalid_value}") + open_member_filter(view) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters - - # Invalid values should not be added to filters - refute Map.has_key?(filters, boolean_field.id), - "Invalid value '#{invalid_value}' should not be added to filters" + # An invalid value leaves the field's filter at "All" (no filter applied). + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]"), + "Invalid value '#{invalid_value}' should leave the filter at All" end end @@ -1435,12 +1468,16 @@ defmodule MvWeb.MemberLive.IndexTest do "/members?bf_#{boolean_field1.id}=true&bf_#{boolean_field2.id}=false" ) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - assert filters[boolean_field1.id] == true - assert filters[boolean_field2.id] == false - assert map_size(filters) == 2 + # Both filters are reflected: field1 at "Yes", field2 at "No", and the + # active-filter count badge shows 2. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field1.id}-true"}[checked]") + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field2.id}-false"}[checked]") + + assert view + |> element(~s(button[aria-label="Filter members"] .badge), "2") + |> has_element?() end test "build_query_params includes active boolean filters and excludes nil filters", %{ @@ -1514,12 +1551,12 @@ defmodule MvWeb.MemberLive.IndexTest do "/members?cycle_status_filter=paid&bf_#{boolean_field.id}=true" ) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - # Both filters should be set - assert filters[boolean_field.id] == true - assert state.socket.assigns.cycle_status_filter == :paid + # Both filters are reflected in the rendered controls: the boolean field at + # "Yes" and the payment-status filter at "paid". + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-true"}[checked]") + assert has_element?(view, "#payment-filter-paid[checked]") # Both should be in URL when triggering search view @@ -1540,9 +1577,8 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_#{boolean_field.id}=true") - state_before = :sys.get_state(view.pid) - filters_before = state_before.socket.assigns.boolean_custom_field_filters - assert filters_before[boolean_field.id] == true + open_member_filter(view) + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-true"}[checked]") # Delete the custom field (requires actor with destroy permission) Ash.destroy!(boolean_field, actor: system_actor) @@ -1551,12 +1587,11 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view2, _html} = live(conn, "/members?bf_#{boolean_field.id}=true") - state_after = :sys.get_state(view2.pid) - filters_after = state_after.socket.assigns.boolean_custom_field_filters + open_member_filter(view2) - # Filter should not be present for deleted custom field - refute Map.has_key?(filters_after, boolean_field.id) - assert filters_after == %{} + # The deleted field renders no filter control and no filter is active. + refute has_element?(view2, "##{"custom-boolean-filter-#{boolean_field.id}-true"}") + refute has_element?(view2, ~s(button[aria-label="Filter members"].btn-active)) end test "handle_params handles URL-encoded custom field IDs correctly", %{conn: conn} do @@ -1569,12 +1604,11 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_#{encoded_id}=true") - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - # Filter should work with URL-encoded ID - # Phoenix should decode it automatically, so we check with original ID - assert filters[boolean_field.id] == true + # Phoenix decodes the param, so the filter applies under the original ID: + # the "Yes" radio for the field is checked. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-true"}[checked]") end test "handle_params ignores malformed prefix (bf_bf_)", %{conn: conn} do @@ -1585,12 +1619,12 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_bf_#{boolean_field.id}=true") - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - # Should not parse as valid filter (UUID validation should fail) - refute Map.has_key?(filters, boolean_field.id) - assert filters == %{} + # The double-prefixed param is not a valid filter: the field stays at "All" + # and no filter is active. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]") + refute has_element?(view, ~s(button[aria-label="Filter members"].btn-active)) end test "handle_params limits number of boolean filters to prevent DoS", %{conn: conn} do @@ -1605,17 +1639,11 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?#{filter_params}") - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters - - # Should limit to maximum 50 filters - assert map_size(filters) <= 50 - # All filters in the result should be valid - Enum.each(filters, fn {id, value} -> - assert value in [true, false] - # Verify the ID corresponds to one of our boolean fields - assert id in Enum.map(boolean_fields, &to_string(&1.id)) - end) + # The active-filter count badge is the observable carrier of the filter count. + # With 60 requested filters, the DoS cap clamps it to exactly 50. + assert view + |> element(~s(button[aria-label="Filter members"] .badge), "50") + |> has_element?() end test "handle_params ignores extremely long custom field IDs", %{conn: conn} do @@ -1628,14 +1656,12 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_#{fake_long_id}=true") - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - # Should not accept the extremely long ID - refute Map.has_key?(filters, fake_long_id) - # Valid boolean field should still work - refute Map.has_key?(filters, boolean_field.id) - assert filters == %{} + # The over-long ID is rejected: the real field stays at "All" and no filter + # is active. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]") + refute has_element?(view, ~s(button[aria-label="Filter members"].btn-active)) end # Helper to create a member with a boolean custom field value @@ -2283,24 +2309,20 @@ defmodule MvWeb.MemberLive.IndexTest do # Start with no boolean custom fields {:ok, view, _html} = live(conn, "/members") + open_member_filter(view) - state_before = :sys.get_state(view.pid) - boolean_fields_before = state_before.socket.assigns.boolean_custom_fields - assert boolean_fields_before == [] + # No boolean field control is rendered yet. + refute has_element?(view, ~s(input[name^="custom_boolean"])) # Create a new boolean custom field new_boolean_field = create_boolean_custom_field(%{name: "Newly Added Field"}) - # Navigate again - the new field should appear + # Navigate again - the new field should appear in the filter dropdown. {:ok, view2, _html} = live(conn, "/members") + html_after = member_filter_html(view2) - state_after = :sys.get_state(view2.pid) - boolean_fields_after = state_after.socket.assigns.boolean_custom_fields - - # New boolean field should be present - assert length(boolean_fields_after) == 1 - assert Enum.any?(boolean_fields_after, &(&1.id == new_boolean_field.id)) - assert Enum.any?(boolean_fields_after, &(&1.name == "Newly Added Field")) + assert has_element?(view2, "##{"custom-boolean-filter-#{new_boolean_field.id}-all"}") + assert html_after =~ "Newly Added Field" end @tag :slow From 655fd80524f8da2f3781224d89a46aa5ca12a385 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:51:43 +0200 Subject: [PATCH 21/27] test: wait on observable state instead of blind sleeps Replace the fixed Process.sleep waits in the import, members-PDF and field-visibility tests with event-based / bounded-poll waits on the observable condition, removing a known flakiness vector. --- test/mv/membership/members_pdf_test.exs | 31 ++++++++++++--- test/mv_web/live/import_live_test.exs | 38 ++++++++++++++----- .../index_field_visibility_test.exs | 22 ----------- 3 files changed, 55 insertions(+), 36 deletions(-) diff --git a/test/mv/membership/members_pdf_test.exs b/test/mv/membership/members_pdf_test.exs index 2b83e3b..3ae18cf 100644 --- a/test/mv/membership/members_pdf_test.exs +++ b/test/mv/membership/members_pdf_test.exs @@ -274,17 +274,38 @@ defmodule Mv.Membership.MembersPDFTest do assert {:ok, _pdf_binary} = result - # Wait a bit for cleanup (async cleanup might take a moment) - Process.sleep(100) - - # Count temp directories after - after_count = + count_export_dirs = fn -> temp_base |> File.ls!() |> Enum.count(fn name -> String.starts_with?(name, "mv_pdf_export_") end) + end + + # Poll the observable cleanup condition (temp-dir count returns to the baseline) + # with a bounded deadline instead of a fixed sleep, so the test waits no longer + # than the cleanup actually needs and still fails if cleanup never runs. + after_count = poll_until_cleaned(count_export_dirs, before_count, 100) # Should have same or fewer temp dirs (cleanup should have run) assert after_count <= before_count + 1 end end + + # Bounded poll: returns the export-dir count once it drops back to the baseline + # (cleanup done), or the last observed count when the attempt budget is exhausted + # (so the caller's assertion reports the real state on a genuine cleanup stall). + defp poll_until_cleaned(count_fun, baseline, attempts_left) do + current = count_fun.() + + cond do + current <= baseline -> + current + + attempts_left <= 0 -> + current + + true -> + Process.sleep(10) + poll_until_cleaned(count_fun, baseline, attempts_left - 1) + end + end end diff --git a/test/mv_web/live/import_live_test.exs b/test/mv_web/live/import_live_test.exs index bd5cdec..8837fc6 100644 --- a/test/mv_web/live/import_live_test.exs +++ b/test/mv_web/live/import_live_test.exs @@ -37,7 +37,27 @@ defmodule MvWeb.ImportLiveTest do confirm_import(view) end - defp wait_for_import_completion, do: Process.sleep(1000) + # Waits for the asynchronous chunk-import to finish by polling the rendered + # results panel, instead of a fixed sleep. In the test sandbox the chunks run + # in-process and signal completion via self-messages; each render/2 forces the + # LiveView to drain its mailbox before replying, so the panel appears once all + # chunk messages have been processed. Bounded so a genuine stall still fails. + defp wait_for_import_completion(view) do + wait_for_import_completion(view, 200) + end + + defp wait_for_import_completion(_view, 0) do + flunk("import did not complete: results panel never rendered") + end + + defp wait_for_import_completion(view, attempts_left) do + if has_element?(view, "[data-testid='import-results-panel']") do + :ok + else + Process.sleep(10) + wait_for_import_completion(view, attempts_left - 1) + end + end # ---------- Business logic: Authorization ---------- describe "Authorization" do @@ -67,7 +87,7 @@ defmodule MvWeb.ImportLiveTest do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content) - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-summary']") @@ -131,7 +151,7 @@ defmodule MvWeb.ImportLiveTest do } do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content, "invalid_import.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-error-list']") @@ -150,7 +170,7 @@ defmodule MvWeb.ImportLiveTest do for i <- 1..100, do: "Row#{i};Last#{i};;Country#{i};City#{i};Street#{i};12345\n" run_full_import(view, header <> Enum.join(invalid_rows), "large_invalid.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-error-list']") @@ -182,7 +202,7 @@ defmodule MvWeb.ImportLiveTest do |> File.read!() run_full_import(view, csv_content, "bom_import.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") html = render(view) @@ -200,7 +220,7 @@ defmodule MvWeb.ImportLiveTest do |> File.read!() run_full_import(view, csv_content, "empty_lines.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-error-list']") html = render(view) @@ -214,7 +234,7 @@ defmodule MvWeb.ImportLiveTest do } do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content, "unknown_custom.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-warnings']") @@ -279,7 +299,7 @@ defmodule MvWeb.ImportLiveTest do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content) - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-progress-container']") html = render(view) assert html =~ "aria-live" @@ -342,7 +362,7 @@ defmodule MvWeb.ImportLiveTest do } do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content) - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") 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 79d078b..007939d 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -157,9 +157,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_click() - # Wait for update - :timer.sleep(100) - # Email should no longer be visible html = render(view) refute html =~ "alice@example.com" @@ -186,9 +183,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='#{custom_field_string}']") |> render_click() - # Wait for update - :timer.sleep(100) - # Custom field should no longer be visible html = render(view) refute html =~ "M001" @@ -213,9 +207,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_all']") |> render_click() - # Wait for update - :timer.sleep(100) - # All fields should be visible html = render(view) assert html =~ "alice@example.com" @@ -237,9 +228,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_none']") |> render_click() - # Wait for update - :timer.sleep(100) - # Only first_name should be visible (it's always shown) html = render(view) # Email and street should be hidden @@ -262,9 +250,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_click() - # Wait for URL update - :timer.sleep(100) - # Check that URL contains fields parameter # Note: In LiveView tests, we check the rendered HTML for the updated state # The actual URL update happens via push_patch @@ -329,8 +314,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_click() - :timer.sleep(100) - html = render(view) refute html =~ "alice@example.com" end @@ -387,8 +370,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do view |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_click() - - :timer.sleep(50) end # Should still work correctly @@ -458,9 +439,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_keydown(%{key: "Enter"}) - # Wait for update - :timer.sleep(100) - # Email should no longer be visible html = render(view) refute html =~ "alice@example.com" From cb54c2c46efc6e04244b07fbba6974b0e59cb746 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:52:17 +0200 Subject: [PATCH 22/27] test(member-live): build date-filter property bounds without a reject-filter The bound-pair generator filtered out ~1/4 of generated values, so unlucky seeds hit StreamData's FilterTooNarrowError under full property runs. Construct an at-least-one-bound-set pair directly instead, preserving the exact domain with no rejection. --- .../member_live/date_filter_property_test.exs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/test/mv_web/live/member_live/date_filter_property_test.exs b/test/mv_web/live/member_live/date_filter_property_test.exs index fcce2a3..8844f91 100644 --- a/test/mv_web/live/member_live/date_filter_property_test.exs +++ b/test/mv_web/live/member_live/date_filter_property_test.exs @@ -20,11 +20,12 @@ defmodule MvWeb.MemberLive.Index.DateFilterPropertyTest do # Generators ----------------------------------------------------------- + defp date_gen do + map(integer(-3650..3650), &Date.add(~D[2000-01-01], &1)) + end + defp optional_date_gen do - one_of([ - constant(nil), - map(integer(-3650..3650), &Date.add(~D[2000-01-01], &1)) - ]) + one_of([constant(nil), date_gen()]) end defp exit_date_mode_gen do @@ -57,19 +58,25 @@ defmodule MvWeb.MemberLive.Index.DateFilterPropertyTest do # Property ------------------------------------------------------------- + # Generates a {from, to} pair with at least one bound set. Built by construction + # (pick which bounds are set, then generate the required dates) rather than by + # filtering out the both-nil case, so StreamData never rejects values and cannot + # raise FilterTooNarrowError on unlucky seeds. defp bound_pair_with_at_least_one_set_gen do gen all( - from <- optional_date_gen(), - to <- optional_date_gen(), - from != nil or to != nil + which <- member_of([:from_only, :to_only, :both]), + date_a <- date_gen(), + date_b <- date_gen() ) do - {from, to} + case which do + :from_only -> {date_a, nil} + :to_only -> {nil, date_a} + :both -> {date_a, date_b} + end end end - defp value_date_gen do - map(integer(-3650..3650), &Date.add(~D[2000-01-01], &1)) - end + defp value_date_gen, do: date_gen() # Property ------------------------------------------------------------- From ef94d2ef105f69674d6d4a127b97b3b2c5361790 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:52:51 +0200 Subject: [PATCH 23/27] fix(repo): make member/user foreign keys deferrable to avoid create_member deadlock Concurrent create_member transactions took FK FOR KEY SHARE (MultiXact) locks on shared rows across members/users/membership_fee_types and could form a cross-transaction cycle, producing intermittent PostgreSQL deadlocks (40P01) under load. Making the three foreign keys DEFERRABLE INITIALLY DEFERRED moves the check to commit time and breaks the cycle, without weakening integrity (NOT NULL and ON DELETE RESTRICT are unaffected). --- ...165309_make_member_user_fks_deferrable.exs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 priv/repo/migrations/20260616165309_make_member_user_fks_deferrable.exs diff --git a/priv/repo/migrations/20260616165309_make_member_user_fks_deferrable.exs b/priv/repo/migrations/20260616165309_make_member_user_fks_deferrable.exs new file mode 100644 index 0000000..2fa45fb --- /dev/null +++ b/priv/repo/migrations/20260616165309_make_member_user_fks_deferrable.exs @@ -0,0 +1,32 @@ +defmodule Mv.Repo.Migrations.MakeMemberUserFksDeferrable do + @moduledoc """ + Makes the members/users foreign keys DEFERRABLE INITIALLY DEFERRED. + + Concurrent `create_member` transactions take FK `FOR KEY SHARE` (MultiXact) + locks on these foreign keys at statement time and can form a cross-transaction + lock cycle, producing a PostgreSQL `deadlock_detected` (40P01). Deferring the + FK checks to commit time breaks the cycle. + + Constraint deferrability is not tracked by AshPostgres resource snapshots, so + this is a hand-written migration with raw `execute/2`. Do not regenerate it + via `mix ash_postgres.generate_migrations`. + """ + use Ecto.Migration + + def change do + execute( + "ALTER TABLE users ALTER CONSTRAINT users_member_id_fkey DEFERRABLE INITIALLY DEFERRED", + "ALTER TABLE users ALTER CONSTRAINT users_member_id_fkey NOT DEFERRABLE" + ) + + execute( + "ALTER TABLE users ALTER CONSTRAINT users_role_id_fkey DEFERRABLE INITIALLY DEFERRED", + "ALTER TABLE users ALTER CONSTRAINT users_role_id_fkey NOT DEFERRABLE" + ) + + execute( + "ALTER TABLE members ALTER CONSTRAINT members_membership_fee_type_id_fkey DEFERRABLE INITIALLY DEFERRED", + "ALTER TABLE members ALTER CONSTRAINT members_membership_fee_type_id_fkey NOT DEFERRABLE" + ) + end +end From 5e84c342b7c54eceff25445292f9c3a90fed4e3a Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:53:25 +0200 Subject: [PATCH 24/27] test(repo): assert member/user foreign keys are deferrable --- test/mv/repo/deferrable_fk_test.exs | 37 +++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 test/mv/repo/deferrable_fk_test.exs diff --git a/test/mv/repo/deferrable_fk_test.exs b/test/mv/repo/deferrable_fk_test.exs new file mode 100644 index 0000000..4127082 --- /dev/null +++ b/test/mv/repo/deferrable_fk_test.exs @@ -0,0 +1,37 @@ +defmodule Mv.DeferrableFkTest do + @moduledoc """ + Regression guard for the deferrable-FK migration. + + Asserts the schema property directly (`condeferred = true`) for the three + members/users foreign keys. A multi-connection deadlock reproduction cannot + be made deterministic under the Ecto sandbox (ownership serializes + connections), so the structural assertion is the guard here; see the + migration moduledoc for the full FOR-KEY-SHARE/MultiXact deadlock rationale. + """ + use Mv.DataCase, async: true + + @deferrable_fks ~w( + users_member_id_fkey + users_role_id_fkey + members_membership_fee_type_id_fkey + ) + + test "member/user foreign keys are DEFERRABLE INITIALLY DEFERRED" do + query = """ + SELECT conname, condeferrable, condeferred + FROM pg_constraint + WHERE conname = ANY($1) + """ + + {:ok, %{rows: rows}} = Mv.Repo.query(query, [@deferrable_fks]) + + found = Map.new(rows, fn [name, deferrable, deferred] -> {name, {deferrable, deferred}} end) + + for fk <- @deferrable_fks do + assert Map.has_key?(found, fk), "expected foreign key #{fk} to exist" + + assert found[fk] == {true, true}, + "expected #{fk} to be DEFERRABLE INITIALLY DEFERRED, got #{inspect(found[fk])}" + end + end +end From c0f40a13cee7341fdc8e2865f966f09f9badc90b Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:53:59 +0200 Subject: [PATCH 25/27] test(member-live): keep deadlock-prone member tests synchronous These member/group/custom-field LiveView tests stay async: false. With the foreign keys now deferrable the create_member deadlock no longer forces it, so the rationale is updated: they remain synchronous as a deferred scope decision, and index_groups_url_params/member_filter_component additionally have separate async-isolation issues that must be fixed before they can run in parallel. --- test/mv_web/components/member_filter_component_test.exs | 5 ++++- .../mv_web/member_live/index_custom_fields_display_test.exs | 5 ++++- test/mv_web/member_live/index_field_visibility_test.exs | 5 ++++- test/mv_web/member_live/index_groups_accessibility_test.exs | 5 ++++- test/mv_web/member_live/index_groups_display_test.exs | 5 ++++- test/mv_web/member_live/index_groups_filter_test.exs | 5 ++++- test/mv_web/member_live/index_groups_integration_test.exs | 5 ++++- test/mv_web/member_live/index_groups_performance_test.exs | 5 ++++- test/mv_web/member_live/index_groups_sorting_test.exs | 5 ++++- test/mv_web/member_live/index_groups_url_params_test.exs | 5 ++++- test/mv_web/member_live/show_groups_display_test.exs | 6 ++++-- test/mv_web/user_live/form_test.exs | 5 ++++- 12 files changed, 48 insertions(+), 13 deletions(-) diff --git a/test/mv_web/components/member_filter_component_test.exs b/test/mv_web/components/member_filter_component_test.exs index fcb45f6..58aee87 100644 --- a/test/mv_web/components/member_filter_component_test.exs +++ b/test/mv_web/components/member_filter_component_test.exs @@ -8,7 +8,10 @@ defmodule MvWeb.Components.MemberFilterComponentTest do - Button label and badge logic - Filtering to show only boolean custom fields """ - # async: false to prevent PostgreSQL deadlocks when running LiveView tests against DB + # Kept async: false. The deferrable-FK migration removed the concurrent + # create_member deadlock, but this file additionally showed an async-isolation + # failure under load (filtered members from a parallel test leaking in), so it + # is not trivially async-safe; resolving that is a separate follow-up. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest 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 9ada92b..bcc9913 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 @@ -9,7 +9,10 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsDisplayTest do - Custom field values are correctly formatted for different types - Members without custom field values show empty cell or "-" """ - # async: false to prevent PostgreSQL deadlocks when creating members and custom fields + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/custom-fields suite to async is a possible follow-up + # rather than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query 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 007939d..b992ddd 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -10,7 +10,10 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do - Integration with member list display - Custom fields visibility """ - # async: false to prevent PostgreSQL deadlocks when creating members and custom fields + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/custom-fields suite to async is a possible follow-up + # rather than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest diff --git a/test/mv_web/member_live/index_groups_accessibility_test.exs b/test/mv_web/member_live/index_groups_accessibility_test.exs index d14cd9f..e54a0ba 100644 --- a/test/mv_web/member_live/index_groups_accessibility_test.exs +++ b/test/mv_web/member_live/index_groups_accessibility_test.exs @@ -8,7 +8,10 @@ defmodule MvWeb.MemberLive.IndexGroupsAccessibilityTest do - Sort header has aria-label for screen reader - Keyboard navigation works (Tab through filter, sort header) """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_display_test.exs b/test/mv_web/member_live/index_groups_display_test.exs index 263ac2a..bc39967 100644 --- a/test/mv_web/member_live/index_groups_display_test.exs +++ b/test/mv_web/member_live/index_groups_display_test.exs @@ -8,7 +8,10 @@ defmodule MvWeb.MemberLive.IndexGroupsDisplayTest do - No badge for members without groups - Badge shows group name correctly """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_filter_test.exs b/test/mv_web/member_live/index_groups_filter_test.exs index d32b17f..e0c7b22 100644 --- a/test/mv_web/member_live/index_groups_filter_test.exs +++ b/test/mv_web/member_live/index_groups_filter_test.exs @@ -6,7 +6,10 @@ defmodule MvWeb.MemberLive.IndexGroupsFilterTest do All / Yes / No (per group). Multiple active group filters combine with AND (member must match all selected group conditions). """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_integration_test.exs b/test/mv_web/member_live/index_groups_integration_test.exs index 86738da..0832faa 100644 --- a/test/mv_web/member_live/index_groups_integration_test.exs +++ b/test/mv_web/member_live/index_groups_integration_test.exs @@ -10,7 +10,10 @@ defmodule MvWeb.MemberLive.IndexGroupsIntegrationTest do - Groups work with existing search (but not testing search integration itself) - Member index search by group name returns members in that group (Issue #375) """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_performance_test.exs b/test/mv_web/member_live/index_groups_performance_test.exs index 761c4eb..6eb3311 100644 --- a/test/mv_web/member_live/index_groups_performance_test.exs +++ b/test/mv_web/member_live/index_groups_performance_test.exs @@ -8,7 +8,10 @@ defmodule MvWeb.MemberLive.IndexGroupsPerformanceTest do - Filter works at database level (not in-memory) - Sort runs in-memory but uses preloaded group data (no extra DB queries) """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_sorting_test.exs b/test/mv_web/member_live/index_groups_sorting_test.exs index 068152c..9427419 100644 --- a/test/mv_web/member_live/index_groups_sorting_test.exs +++ b/test/mv_web/member_live/index_groups_sorting_test.exs @@ -2,7 +2,10 @@ defmodule MvWeb.MemberLive.IndexGroupsSortingTest do @moduledoc """ Tests for sorting by groups in the member overview. """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_url_params_test.exs b/test/mv_web/member_live/index_groups_url_params_test.exs index 469b010..66bc186 100644 --- a/test/mv_web/member_live/index_groups_url_params_test.exs +++ b/test/mv_web/member_live/index_groups_url_params_test.exs @@ -9,7 +9,10 @@ defmodule MvWeb.MemberLive.IndexGroupsUrlParamsTest do - URL parameters work with other parameters (query, sort_field, etc.) - URL is bookmarkable (filter/sorting persist) """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false. The deferrable-FK migration removed the concurrent + # create_member deadlock, but this file additionally showed an async-isolation + # failure under load (filtered members from a parallel test leaking in), so it + # is not trivially async-safe; resolving that is a separate follow-up. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/show_groups_display_test.exs b/test/mv_web/member_live/show_groups_display_test.exs index f8434b3..913cf76 100644 --- a/test/mv_web/member_live/show_groups_display_test.exs +++ b/test/mv_web/member_live/show_groups_display_test.exs @@ -10,8 +10,10 @@ defmodule MvWeb.MemberLive.ShowGroupsDisplayTest do - Accessibility: group links have aria-label for screen readers ## Note on async - async: false to avoid PostgreSQL deadlocks when creating members and groups - in the same test run (same as IndexGroupsDisplayTest). + Kept async: false as a deferred scope decision. The deferrable-FK migration + removed the concurrent-create_member deadlock that previously forced this, so + re-flipping this members/groups suite to async is a possible follow-up rather + than part of the original change. """ use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest diff --git a/test/mv_web/user_live/form_test.exs b/test/mv_web/user_live/form_test.exs index a22c230..63649e4 100644 --- a/test/mv_web/user_live/form_test.exs +++ b/test/mv_web/user_live/form_test.exs @@ -1,5 +1,8 @@ defmodule MvWeb.UserLive.FormTest do - # async: false to prevent PostgreSQL deadlocks when creating members and users + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/users suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest From 3d792e8b047fdca7ec18dd07791dadf40651e896 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:54:32 +0200 Subject: [PATCH 26/27] docs(testing): document create_member deadlock fix and async-test-safety --- docs/test-performance-optimization.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/test-performance-optimization.md b/docs/test-performance-optimization.md index b45c9fc..0a243c3 100644 --- a/docs/test-performance-optimization.md +++ b/docs/test-performance-optimization.md @@ -90,6 +90,29 @@ test/ --- +## 5. Concurrent `create_member` deadlock and deferrable FKs + +A class of intermittent failures (PostgreSQL `deadlock_detected`, SQLSTATE `40P01`) was traced to **concurrent `create_member` transactions**, not to any single test. It surfaced as a `MatchError` on `{:ok, member} = ...` in member-heavy LiveView tests (e.g. `FormMemberSelectionTest`) and reproduced only under CPU contention (≈1 in 12 full-fast-suite runs at high `async: true` concurrency; effectively never on an idle machine). + +**Root cause.** `create_member` writes a cascade in one transaction (member row, `custom_field_values`, the `user` link, fee-type defaulting, cycle generation). Concurrent inserts take FK `FOR KEY SHARE` (MultiXact) locks on shared parent rows across `members` / `users` / `membership_fee_types`; under contention these can form a cross-transaction lock cycle that Postgres resolves by aborting one transaction. It is a product-level concurrency property, **not** test-data contention, so it is not fixable by test-state isolation. + +**Fix.** Migration `…_make_member_user_fks_deferrable.exs` makes the three FKs (`users.member_id`, `users.role_id`, `members.membership_fee_type_id`) `DEFERRABLE INITIALLY DEFERRED`, moving the FK check (and its lock) to commit time and breaking the cycle. Verified: **0 deadlocks in 15 full-suite runs under maximum CPU contention**, versus 1/12 before. This does **not** weaken integrity — `NOT NULL` is independent of FK deferral, a real dangling reference still aborts the commit, and `ON DELETE RESTRICT` (e.g. `users.role_id`) stays immediate regardless of deferrability. `Mv.DeferrableFkTest` asserts the constraint state as a regression guard (a deterministic in-process concurrent reproduction is infeasible under the Ecto sandbox, which serializes connections by ownership). + +This deadlock is also a latent **production** risk under concurrent sign-ups; the deferrable-FK fix addresses both. + +### Async-test-safety checklist (members/groups/custom fields) + +Several member-creating test files historically used `async: false` with a "prevent PostgreSQL deadlocks" comment. With the deferrable-FK migration in place those files are deadlock-safe, but before flipping any such file to `async: true`: + +- **Prove isolation under load, not just one green run.** Re-run the file (and the full suite) under varying `--seed` **and** CPU contention; a single green run is not evidence (the deadlock and the isolation flakes below are load-dependent). +- **Watch for separate async-isolation issues beyond the deadlock.** `index_groups_url_params_test.exs` and `member_filter_component_test.exs` showed filtered-member-leak failures (`refute html =~ name`) under concurrency that are independent of the FK deadlock — these need their own per-file isolation fix before they can run async. + +### StreamData generator pitfall + +`FilterTooNarrowError` appeared on unlucky seeds (e.g. 222) in a property test that built a value with a reject-filter (`StreamData.filter` discarding ~1/4 of generated pairs). Under full property-run counts this hits too many consecutive rejections. Fix: **construct the desired value directly** instead of generating-then-filtering (preserves the exact domain, no rejection). Prefer constructive generators over reject-filters in property tests. + +--- + ## References - Testing Standards: `CODE_GUIDELINES.md` §4 From 6d7ece20a8c151e07cb47fe1a686c27c01464ac4 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:55:24 +0200 Subject: [PATCH 27/27] docs(changelog): record member-creation deadlock fix under Unreleased --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 293d07c..f1df7ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Column-header tooltips clipped** – Tooltips on the members-overview column headers are no longer clipped by the sticky table header. - **Text selection opens member** – Dragging to select text in a members-overview row (for example to copy an email) no longer opens the member details; a plain click still opens them. - **Sort by custom date** – Sorting the member list or member export by a custom date field now orders rows chronologically instead of like text, so e.g. 29.01.1981 correctly comes before 01.03.1982. +- **Concurrent member creation no longer deadlocks** – Creating members in parallel (e.g. simultaneous sign-ups, or batch operations) could intermittently fail with a PostgreSQL deadlock; the affected foreign keys are now deferrable, so concurrent member creation succeeds reliably. ## [1.2.0] - 2026-05-08