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, []}, diff --git a/CHANGELOG.md b/CHANGELOG.md index d08ad05..8b2a57d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,11 +24,13 @@ 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. - **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 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/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 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/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 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/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/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/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 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/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/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/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/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 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.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). 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_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. 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/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/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 4465690..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,11 +9,9 @@ defmodule Mv.Vereinfacht.Changes.SyncLinkedMemberAfterUserChange do """ use Ash.Resource.Change + alias Mv.EmailSync.Loader + require Logger - alias Mv.Helpers - alias Mv.Helpers.SystemActor - alias Mv.Membership - alias Mv.Membership.Member @impl true def change(changeset, _opts, _context) do @@ -32,7 +30,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 +53,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 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/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..f4df97f 100644 --- a/lib/mv_web/controllers/import_template_controller.ex +++ b/lib/mv_web/controllers/import_template_controller.ex @@ -13,7 +13,8 @@ defmodule MvWeb.ImportTemplateController do """ use MvWeb, :controller - alias Mv.Authorization.Actor + import MvWeb.ControllerHelpers, only: [current_actor: 1] + alias Mv.Membership.Member alias Mv.Membership.MembersCSV alias MvWeb.Authorization @@ -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..e33b211 100644 --- a/lib/mv_web/controllers/member_export_controller.ex +++ b/lib/mv_web/controllers/member_export_controller.ex @@ -6,11 +6,11 @@ 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.Authorization.Actor alias Mv.Membership.CustomField alias Mv.Membership.CustomFieldSort alias Mv.Membership.Member @@ -18,7 +18,8 @@ defmodule MvWeb.MemberExportController do alias Mv.Membership.MembersCSV alias MvWeb.MemberLive.Index.MembershipFeeStatus alias MvWeb.Translations.MemberFields - 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"] @@ -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..f5bdc6e 100644 --- a/lib/mv_web/controllers/member_pdf_export_controller.ex +++ b/lib/mv_web/controllers/member_pdf_export_controller.ex @@ -7,14 +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.Authorization.Actor alias Mv.Membership.{MemberExport, MemberExport.Build, MembersPDF} alias MvWeb.Translations.MemberFields - use Gettext, backend: MvWeb.Gettext + require Logger @payload_required_message "payload required" @invalid_json_message "invalid JSON" @@ -79,13 +79,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) 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 diff --git a/lib/mv_web/helpers/membership_fee_helpers.ex b/lib/mv_web/helpers/membership_fee_helpers.ex index e8a2ce8..1fddda8 100644 --- a/lib/mv_web/helpers/membership_fee_helpers.ex +++ b/lib/mv_web/helpers/membership_fee_helpers.ex @@ -9,8 +9,11 @@ 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 """ Formats a decimal amount as currency string. @@ -98,8 +101,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 @@ -249,8 +252,68 @@ defmodule MvWeb.Helpers.MembershipFeeHelpers do 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") + @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/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 ad24110..af93719 100644 --- a/lib/mv_web/live/group_live/show.ex +++ b/lib/mv_web/live/group_live/show.ex @@ -15,14 +15,15 @@ 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 @@ -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/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_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/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 53a5705..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() @@ -1026,8 +1027,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 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.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 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 4df6608..7ff1ec9 100644 --- a/lib/mv_web/live/membership_fee_settings_live.ex +++ b/lib/mv_web/live/membership_fee_settings_live.ex @@ -9,16 +9,15 @@ defmodule MvWeb.MembershipFeeSettingsLive do """ use MvWeb, :live_view - require Ash.Query - import MvWeb.LiveHelpers, only: [current_actor: 1] 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) @@ -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/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 1d51ce1..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) @@ -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/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 60763ab..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 @@ -43,10 +45,9 @@ 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] - import MvWeb.ErrorHelpers, only: [format_ash_error: 1] + require Jason @impl true def render(assigns) do @@ -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()} 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/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 "" 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 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_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_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/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/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 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/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/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/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 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 diff --git a/test/mv/statistics_test.exs b/test/mv/statistics_test.exs index 6b72ffb..19533e2 100644 --- a/test/mv/statistics_test.exs +++ b/test/mv/statistics_test.exs @@ -4,36 +4,21 @@ defmodule Mv.StatisticsTest do """ use Mv.DataCase, async: true - 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 + require Ash.Query + setup do actor = Mv.Helpers.SystemActor.get_system_actor() %{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 +116,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 +156,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 +192,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/components/member_filter_component_test.exs b/test/mv_web/components/member_filter_component_test.exs index fcb45f6..b93accc 100644 --- a/test/mv_web/components/member_filter_component_test.exs +++ b/test/mv_web/components/member_filter_component_test.exs @@ -8,12 +8,16 @@ 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 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/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/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 ------------------------------------------------------------- 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/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/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/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_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..74db2bf 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,13 +9,17 @@ 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 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..63dff1c 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -10,14 +10,18 @@ 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 - require Ash.Query alias Mv.Membership.{CustomField, CustomFieldValue} + require Ash.Query + setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -157,9 +161,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 +187,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 +211,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 +232,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 +254,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 +318,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 +374,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 +443,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" 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..bfe47c4 100644 --- a/test/mv_web/member_live/index_groups_accessibility_test.exs +++ b/test/mv_web/member_live/index_groups_accessibility_test.exs @@ -8,13 +8,17 @@ 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 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..ed817dd 100644 --- a/test/mv_web/member_live/index_groups_display_test.exs +++ b/test/mv_web/member_live/index_groups_display_test.exs @@ -8,13 +8,17 @@ 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 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..c8d5160 100644 --- a/test/mv_web/member_live/index_groups_filter_test.exs +++ b/test/mv_web/member_live/index_groups_filter_test.exs @@ -6,13 +6,17 @@ 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 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..cae9ce1 100644 --- a/test/mv_web/member_live/index_groups_integration_test.exs +++ b/test/mv_web/member_live/index_groups_integration_test.exs @@ -10,15 +10,19 @@ 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 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..6d05908 100644 --- a/test/mv_web/member_live/index_groups_performance_test.exs +++ b/test/mv_web/member_live/index_groups_performance_test.exs @@ -8,13 +8,17 @@ 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 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..6e19779 100644 --- a/test/mv_web/member_live/index_groups_sorting_test.exs +++ b/test/mv_web/member_live/index_groups_sorting_test.exs @@ -2,13 +2,17 @@ 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 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..8ce82aa 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,13 +9,17 @@ 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 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_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..1985534 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -1,56 +1,18 @@ 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 + import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] alias Mv.Helpers.SystemActor 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 - describe "desktop layout: scroll container and sticky table header" do @describetag :ui @@ -337,10 +299,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 @@ -433,6 +399,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() @@ -568,15 +566,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", %{ @@ -1009,26 +1005,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" @@ -1041,10 +1034,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", %{ @@ -1053,11 +1049,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", %{ @@ -1069,10 +1065,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 @@ -1110,7 +1105,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 +1122,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 +1152,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 +1169,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 +1199,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 +1216,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 +1246,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 +1263,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 ) @@ -1338,6 +1333,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 @@ -1349,6 +1347,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 @@ -1359,89 +1360,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 @@ -1452,15 +1446,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 @@ -1475,12 +1466,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", %{ @@ -1554,12 +1549,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 @@ -1580,9 +1575,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) @@ -1591,12 +1585,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 @@ -1609,12 +1602,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 @@ -1625,12 +1617,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 @@ -1645,17 +1637,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 @@ -1668,14 +1654,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 @@ -2196,7 +2180,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 +2208,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 ) @@ -2323,24 +2307,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 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_groups_display_test.exs b/test/mv_web/member_live/show_groups_display_test.exs index f8434b3..a40ed2f 100644 --- a/test/mv_web/member_live/show_groups_display_test.exs +++ b/test/mv_web/member_live/show_groups_display_test.exs @@ -10,16 +10,20 @@ 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 - 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_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/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() 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 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 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