diff --git a/.credo.exs b/.credo.exs index a94fead..3a4f8dc 100644 --- a/.credo.exs +++ b/.credo.exs @@ -114,7 +114,6 @@ {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, []}, @@ -167,19 +166,13 @@ {Credo.Check.Warning.UnusedTupleOperation, []}, {Credo.Check.Warning.WrongTestFileExtension, []}, # Module documentation check (enabled after adding @moduledoc to all modules) - {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, []} + {Credo.Check.Readability.ModuleDoc, []} ], 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) @@ -190,7 +183,6 @@ {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, []}, @@ -200,20 +192,24 @@ {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, []}, - # IoPuts: 3 violations in Mv.Release seed output; deferred to a follow-up. + {Credo.Check.Refactor.DoubleBooleanNegation, []}, + {Credo.Check.Refactor.FilterReject, []}, {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 8b2a57d..d08ad05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,13 +24,11 @@ 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 de34d47..ee78069 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.Config (oidc_admin_group_name/0, oidc_groups_claim/0). +- Module: Mv.OidcRoleSyncConfig (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 0a243c3..b45c9fc 100644 --- a/docs/test-performance-optimization.md +++ b/docs/test-performance-optimization.md @@ -90,29 +90,6 @@ 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 f7f59ff..0127796 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -8,6 +8,7 @@ defmodule Mv.Accounts.User do extensions: [AshAuthentication], authorizers: [Ash.Policy.Authorizer] + require Ash.Query import Ash.Expr alias Ash.Resource.Preparation.Builtins @@ -15,8 +16,6 @@ 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 87e8250..7ae8510 100644 --- a/lib/accounts/user/validations/oidc_email_collision.ex +++ b/lib/accounts/user/validations/oidc_email_collision.ex @@ -24,13 +24,12 @@ 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 new file mode 100644 index 0000000..fd8d2c9 --- /dev/null +++ b/lib/accounts/user_identity.exs @@ -0,0 +1,18 @@ +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 4dfbcc8..730ccd7 100644 --- a/lib/membership/email.ex +++ b/lib/membership/email.ex @@ -38,10 +38,6 @@ 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 fcfe077..d468166 100644 --- a/lib/membership/group.ex +++ b/lib/membership/group.ex @@ -39,10 +39,9 @@ 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 0f67a00..cddc23f 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 Ash.Expr import Bitwise - + require Ash.Query + import Ash.Expr alias Ecto.Adapters.SQL, as: EctoSQL alias Mv.Helpers alias Mv.Helpers.SystemActor @@ -49,7 +49,6 @@ 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 d09f0e5..22a1f70 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.ReadLinkedForOwnData, member_id_field: :member_id} + authorize_if Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData end policy action_type(:read) do diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 2a5b17f..72be69b 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -26,15 +26,13 @@ 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 f319685..83c5c8b 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 deleted file mode 100644 index 183efed..0000000 --- a/lib/membership/setting/changes/jsonb_result.ex +++ /dev/null @@ -1,98 +0,0 @@ -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 cb3ce7b..e24860c 100644 --- a/lib/membership/setting/changes/update_single_member_field.ex +++ b/lib/membership/setting/changes/update_single_member_field.ex @@ -19,7 +19,9 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberField do """ use Ash.Resource.Change - alias Mv.Membership.Setting.Changes.JsonbResult + alias Ash.Error.Invalid + alias Ecto.Adapters.SQL + require Logger def change(changeset, _opts, _context) do with {:ok, field} <- get_and_validate_field(changeset), @@ -116,21 +118,62 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberField do uuid_binary = Ecto.UUID.dump!(settings.id) - JsonbResult.run_update( - sql: sql, - params: [field, show_in_overview, required, uuid_binary], - on_row: fn [updated_visibility, updated_required | _] -> - %{ + 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 = %{ settings - | member_field_visibility: JsonbResult.normalize(updated_visibility), - member_field_required: JsonbResult.normalize(updated_required) + | member_field_visibility: vis, + member_field_required: req } - 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" - ) + + {: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) 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 f08bc21..e047cdf 100644 --- a/lib/membership/setting/changes/update_single_member_field_visibility.ex +++ b/lib/membership/setting/changes/update_single_member_field_visibility.ex @@ -19,7 +19,9 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberFieldVisibility do """ use Ash.Resource.Change - alias Mv.Membership.Setting.Changes.JsonbResult + alias Ash.Error.Invalid + alias Ecto.Adapters.SQL + require Logger def change(changeset, _opts, _context) do with {:ok, field} <- get_and_validate_field(changeset), @@ -104,17 +106,59 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberFieldVisibility do # Convert UUID string to binary for PostgreSQL uuid_binary = Ecto.UUID.dump!(settings.id) - 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" - ) + 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 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 cd62887..f0dd1a7 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.ReadLinkedForOwnData, member_id_field: :member_id} + authorize_if Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData 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 0cc8a8b..0d1823c 100644 --- a/lib/mix/tasks/join_requests.cleanup_expired.ex +++ b/lib/mix/tasks/join_requests.cleanup_expired.ex @@ -1,5 +1,4 @@ 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. @@ -17,11 +16,13 @@ defmodule Mix.Tasks.JoinRequests.CleanupExpired do """ use Mix.Task - alias Mv.Membership.JoinRequest - require Ash.Query require Logger + alias Mv.Membership.JoinRequest + + @shortdoc "Deletes join requests in pending_confirmation with expired confirmation token" + @impl Mix.Task def run(_args) do Mix.Task.run("app.start") 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 30dd381..7312b91 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,14 +13,13 @@ 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 - import Swoosh.Email + require Logger 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 8527f04..e276e20 100644 --- a/lib/mv/accounts/user/senders/send_password_reset_email.ex +++ b/lib/mv/accounts/user/senders/send_password_reset_email.ex @@ -13,14 +13,13 @@ 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 - import Swoosh.Email + require Logger 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 e9d62d0..1b6014e 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: :mv}, + {AshAuthentication.Supervisor, otp_app: :my}, 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 b7f8844..edc6b8b 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. """ - alias Mv.Helpers.SystemActor - require Logger + alias Mv.Helpers.SystemActor + @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 94981f7..a11bf2e 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -79,13 +79,10 @@ 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 new file mode 100644 index 0000000..a553fde --- /dev/null +++ b/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex @@ -0,0 +1,63 @@ +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 new file mode 100644 index 0000000..092558c --- /dev/null +++ b/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex @@ -0,0 +1,62 @@ +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 deleted file mode 100644 index eafeb8b..0000000 --- a/lib/mv/authorization/checks/read_linked_for_own_data.ex +++ /dev/null @@ -1,74 +0,0 @@ -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 72f586a..7feaf57 100644 --- a/lib/mv/email_sync/helpers.ex +++ b/lib/mv/email_sync/helpers.ex @@ -6,9 +6,8 @@ defmodule Mv.EmailSync.Helpers do provides clean abstractions for email updates within transactions. """ - import Ecto.Changeset - require Logger + import Ecto.Changeset @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 8070138..7b86a3c 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 - alias Mv.Config - require Ash.Query + alias Mv.Config + @doc """ Starts the SystemActor Agent. diff --git a/lib/mv/mailer.ex b/lib/mv/mailer.ex index 98697ff..1e55b6e 100644 --- a/lib/mv/mailer.ex +++ b/lib/mv/mailer.ex @@ -27,12 +27,11 @@ defmodule Mv.Mailer do ENV takes priority over Settings (same pattern as OIDC and Vereinfacht). """ use Swoosh.Mailer, otp_app: :mv - use Gettext, backend: MvWeb.Gettext, otp_app: :mv import Swoosh.Email + use Gettext, backend: MvWeb.Gettext, otp_app: :mv 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 f034b7a..2edb540 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. """ - alias Mv.Membership.Import.HeaderMapper - require Logger + alias Mv.Membership.Import.HeaderMapper + @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 c666458..31dea59 100644 --- a/lib/mv/membership/import/member_csv.ex +++ b/lib/mv/membership/import/member_csv.ex @@ -53,14 +53,6 @@ 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. @@ -109,6 +101,17 @@ 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 0e6793d..a98b125 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -7,6 +7,12 @@ 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 @@ -29,8 +35,261 @@ 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 @@ -39,6 +298,20 @@ 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 9f848a3..7159679 100644 --- a/lib/mv/membership/member_export/build.ex +++ b/lib/mv/membership/member_export/build.ex @@ -17,14 +17,13 @@ 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 f4bd353..a1c8418 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 1c5551f..b38886c 100644 --- a/lib/mv/membership_fees/cycle_generation_job.ex +++ b/lib/mv/membership_fees/cycle_generation_job.ex @@ -59,7 +59,27 @@ defmodule Mv.MembershipFees.CycleGenerationJob do """ @spec run() :: {:ok, CycleGenerator.results_summary()} | {:error, Ash.Error.t()} - def run, do: run([]) + 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 @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 a6c2d30..189f40a 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -1,4 +1,11 @@ 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. @@ -59,13 +66,6 @@ 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 7461a32..0f6467c 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 Mv.Config). + Configure via OIDC_ADMIN_GROUP_NAME and OIDC_GROUPS_CLAIM (see OidcRoleSyncConfig). 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.Config + alias Mv.OidcRoleSyncConfig @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 = Config.oidc_admin_group_name() + admin_group = OidcRoleSyncConfig.oidc_admin_group_name() if is_nil(admin_group) or admin_group == "" do :ok else - claim = Config.oidc_groups_claim() + claim = OidcRoleSyncConfig.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 new file mode 100644 index 0000000..bbb5770 --- /dev/null +++ b/lib/mv/oidc_role_sync_config.ex @@ -0,0 +1,20 @@ +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 107b096..5db4751 100644 --- a/lib/mv/release.ex +++ b/lib/mv/release.ex @@ -12,6 +12,8 @@ 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 @@ -19,8 +21,6 @@ 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 dbec91f..b3c54c0 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 5f08606..4465690 100644 --- a/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex +++ b/lib/mv/vereinfacht/changes/sync_linked_member_after_user_change.ex @@ -9,9 +9,11 @@ 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 @@ -30,7 +32,7 @@ defmodule Mv.Vereinfacht.Changes.SyncLinkedMemberAfterUserChange do end defp sync_linked_member_after_transaction(_changeset, {:ok, user}) do - case Loader.get_linked_member(user) do + case load_linked_member(user) do nil -> {:ok, user} @@ -53,4 +55,17 @@ 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 de1ed61..4d58f8d 100644 --- a/lib/mv/vereinfacht/vereinfacht.ex +++ b/lib/mv/vereinfacht/vereinfacht.ex @@ -7,15 +7,13 @@ 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 9d083dd..6b3c060 100644 --- a/lib/mv_web/components/table_components.ex +++ b/lib/mv_web/components/table_components.ex @@ -3,9 +3,8 @@ defmodule MvWeb.TableComponents do TableComponents that can be used in tables as components (like a button for sorting, a filter...) """ use Phoenix.Component - use Gettext, backend: MvWeb.Gettext - import MvWeb.CoreComponents + use Gettext, backend: MvWeb.Gettext attr :field, :atom, required: true attr :label, :string, required: true diff --git a/lib/mv_web/controllers/controller_helpers.ex b/lib/mv_web/controllers/controller_helpers.ex deleted file mode 100644 index 04087c0..0000000 --- a/lib/mv_web/controllers/controller_helpers.ex +++ /dev/null @@ -1,22 +0,0 @@ -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 f4df97f..f040c7a 100644 --- a/lib/mv_web/controllers/import_template_controller.ex +++ b/lib/mv_web/controllers/import_template_controller.ex @@ -13,8 +13,7 @@ defmodule MvWeb.ImportTemplateController do """ use MvWeb, :controller - import MvWeb.ControllerHelpers, only: [current_actor: 1] - + alias Mv.Authorization.Actor alias Mv.Membership.Member alias Mv.Membership.MembersCSV alias MvWeb.Authorization @@ -106,6 +105,11 @@ 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 e33b211..1f70a18 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,8 +18,7 @@ defmodule MvWeb.MemberExportController do alias Mv.Membership.MembersCSV alias MvWeb.MemberLive.Index.MembershipFeeStatus alias MvWeb.Translations.MemberFields - - require Ash.Query + use Gettext, backend: MvWeb.Gettext @member_fields_allowlist (Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)) ++ ["membership_fee_type", "groups"] @@ -54,6 +53,11 @@ 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 f5bdc6e..f00c0d1 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 - import MvWeb.ControllerHelpers, only: [current_actor: 1] + require Logger + alias Mv.Authorization.Actor alias Mv.Membership.{MemberExport, MemberExport.Build, MembersPDF} alias MvWeb.Translations.MemberFields - require Logger + use Gettext, backend: MvWeb.Gettext @payload_required_message "payload required" @invalid_json_message "invalid JSON" @@ -79,6 +79,13 @@ 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 a7ea54f..fa309d8 100644 --- a/lib/mv_web/emails/join_already_member_email.ex +++ b/lib/mv_web/emails/join_already_member_email.ex @@ -5,9 +5,15 @@ 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 MvWeb.Emails.JoinEmail + alias Mv.Mailer @doc """ Sends the "already a member" notice to the given address. @@ -17,6 +23,20 @@ defmodule MvWeb.Emails.JoinAlreadyMemberEmail do def send(email_address) when is_binary(email_address) do subject = gettext("Membership application – already a member") - JoinEmail.deliver(email_address, "join_already_member.html", subject) + 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()) 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 8d0c680..17dc487 100644 --- a/lib/mv_web/emails/join_already_pending_email.ex +++ b/lib/mv_web/emails/join_already_pending_email.ex @@ -6,9 +6,15 @@ 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 MvWeb.Emails.JoinEmail + alias Mv.Mailer @doc """ Sends the "application already under review" notice to the given address. @@ -18,6 +24,20 @@ defmodule MvWeb.Emails.JoinAlreadyPendingEmail do def send(email_address) when is_binary(email_address) do subject = gettext("Membership application – already under review") - JoinEmail.deliver(email_address, "join_already_pending.html", subject) + 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()) end end diff --git a/lib/mv_web/emails/join_confirmation_email.ex b/lib/mv_web/emails/join_confirmation_email.ex index 5578d16..08f4ad3 100644 --- a/lib/mv_web/emails/join_confirmation_email.ex +++ b/lib/mv_web/emails/join_confirmation_email.ex @@ -2,10 +2,15 @@ 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 MvWeb.Emails.JoinEmail + alias Mv.Mailer @doc """ Sends the join confirmation email to the given address with the confirmation link. @@ -26,9 +31,22 @@ defmodule MvWeb.Emails.JoinConfirmationEmail do confirm_url = url(~p"/confirm_join/#{token}") subject = gettext("Confirm your membership request") - JoinEmail.deliver(email_address, "join_confirmation.html", subject, %{ + assigns = %{ 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 deleted file mode 100644 index c837be5..0000000 --- a/lib/mv_web/emails/join_email.ex +++ /dev/null @@ -1,54 +0,0 @@ -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 1fddda8..e8a2ce8 100644 --- a/lib/mv_web/helpers/membership_fee_helpers.ex +++ b/lib/mv_web/helpers/membership_fee_helpers.ex @@ -9,11 +9,8 @@ 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. @@ -101,8 +98,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 = DateFormatter.format_date(cycle_start) - end_str = DateFormatter.format_date(cycle_end) + start_str = format_date(cycle_start) + end_str = format_date(cycle_end) "#{start_str} - #{end_str}" end @@ -252,68 +249,8 @@ defmodule MvWeb.Helpers.MembershipFeeHelpers do def status_icon(:unpaid), do: "hero-x-circle" def status_icon(:suspended), do: "hero-pause-circle" - @doc """ - Handles a membership-fee-type "delete" event for the fee-type list and the - fee-settings LiveViews. - - Loads the fee type, attempts to destroy it, and returns the updated socket - with the matching flash. On success the deleted type and its member count are - dropped from the `:membership_fee_types` and `:member_counts` assigns. The - NotFound, Forbidden (delete and access), and generic error branches preserve - the exact messages both views used before they shared this block. - """ - @spec delete_fee_type(Phoenix.LiveView.Socket.t(), String.t(), term()) :: - {:noreply, Phoenix.LiveView.Socket.t()} - def delete_fee_type(socket, id, actor) do - case Ash.get(MembershipFeeType, id, domain: MembershipFees, actor: actor) do - {:ok, fee_type} -> - destroy_fee_type(socket, fee_type, id, actor) - - {:error, %Ash.Error.Query.NotFound{}} -> - {:noreply, - Phoenix.LiveView.put_flash(socket, :error, gettext("Membership fee type not found"))} - - {:error, %Ash.Error.Forbidden{}} -> - {:noreply, - Phoenix.LiveView.put_flash( - socket, - :error, - gettext("You do not have permission to access this membership fee type") - )} - - {:error, error} -> - {:noreply, Phoenix.LiveView.put_flash(socket, :error, fee_error_message(error))} - end + # Private helper function for date formatting + defp format_date(%Date{} = date) do + Calendar.strftime(date, "%d.%m.%Y") 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 c0ab0e5..c4b3ab0 100644 --- a/lib/mv_web/live/auth/link_oidc_account_live.ex +++ b/lib/mv_web/live/auth/link_oidc_account_live.ex @@ -15,14 +15,13 @@ 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 42311df..735c165 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -34,6 +34,7 @@ defmodule MvWeb.GlobalSettingsLive do """ use MvWeb, :live_view + require Ash.Query import Ash.Expr alias Mv.Helpers @@ -43,8 +44,6 @@ 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 af93719..ad24110 100644 --- a/lib/mv_web/live/group_live/show.ex +++ b/lib/mv_web/live/group_live/show.ex @@ -15,15 +15,14 @@ defmodule MvWeb.GroupLive.Show do """ use MvWeb, :live_view + require Logger + import Ash.Expr - import MvWeb.Authorization import MvWeb.LiveHelpers, only: [current_actor: 1] + import MvWeb.Authorization alias Mv.Membership alias MvWeb.Helpers.MemberHelpers, as: MemberHelpers - alias MvWeb.Live.MemberDropdownNav - - require Logger @impl true def mount(_params, _session, socket) do @@ -567,8 +566,56 @@ defmodule MvWeb.GroupLive.Show do end @impl true - def handle_event("member_dropdown_keydown", params, socket) do - MemberDropdownNav.handle_keydown(params, socket, fn -> select_focused_member(socket) end) + 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} end @impl true @@ -658,6 +705,14 @@ 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 4058bc8..c317b87 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 4326416..533f0ce 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 - import MvWeb.Authorization + require Logger + import MvWeb.LiveHelpers, only: [current_actor: 1] + import MvWeb.Authorization 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 e02c64d..d634f53 100644 --- a/lib/mv_web/live/join_request_live/show.ex +++ b/lib/mv_web/live/join_request_live/show.ex @@ -14,8 +14,10 @@ defmodule MvWeb.JoinRequestLive.Show do """ use MvWeb, :live_view - import MvWeb.Authorization + require Logger + import MvWeb.LiveHelpers, only: [current_actor: 1] + import MvWeb.Authorization alias Mv.Constants alias Mv.Membership @@ -24,8 +26,6 @@ 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 deleted file mode 100644 index 1a5d4f2..0000000 --- a/lib/mv_web/live/member_dropdown_nav.ex +++ /dev/null @@ -1,83 +0,0 @@ -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 dc79c9c..7770e90 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -20,6 +20,7 @@ defmodule MvWeb.MemberLive.Form do """ use MvWeb, :live_view + require Logger import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3] alias Mv.Membership @@ -31,8 +32,6 @@ 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 c991f7f..53a5705 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -26,6 +26,8 @@ defmodule MvWeb.MemberLive.Index do """ use MvWeb, :live_view + require Ash.Query + require Logger import Ash.Expr import MvWeb.LiveHelpers, only: [current_actor: 1] @@ -43,9 +45,6 @@ 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() @@ -1027,7 +1026,8 @@ defmodule MvWeb.MemberLive.Index do # Errors in handle_params are handled by Phoenix LiveView actor = current_actor(socket) - members = Ash.read!(query, actor: actor) + {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") # 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 14d1aef..162524a 100644 --- a/lib/mv_web/live/member_live/index/date_filter.ex +++ b/lib/mv_web/live/member_live/index/date_filter.ex @@ -28,13 +28,12 @@ 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 f174859..521bb38 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -28,7 +28,6 @@ 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 @@ -160,12 +159,12 @@ defmodule MvWeb.MemberLive.Show do
<.data_field label={gettext("Join Date")} - value={DateFormatter.format_date(@member.join_date)} + value={format_date(@member.join_date)} class="w-28" /> <.data_field label={gettext("Exit Date")} - value={DateFormatter.format_date(@member.exit_date)} + value={format_date(@member.exit_date)} class="w-28" />
@@ -720,6 +719,14 @@ 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 @@ -753,7 +760,7 @@ defmodule MvWeb.MemberLive.Show do end defp format_custom_field_value(%Date{} = date, :date) do - DateFormatter.format_date(date) + Calendar.strftime(date, "%d.%m.%Y") 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 3a5a976..16ee5dc 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,9 +12,10 @@ 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 @@ -24,8 +25,6 @@ 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 7ff1ec9..4df6608 100644 --- a/lib/mv_web/live/membership_fee_settings_live.ex +++ b/lib/mv_web/live/membership_fee_settings_live.ex @@ -9,15 +9,16 @@ 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) @@ -91,7 +92,47 @@ defmodule MvWeb.MembershipFeeSettingsLive do @impl true def handle_event("delete", %{"id" => id}, socket) do - MembershipFeeHelpers.delete_fee_type(socket, id, current_actor(socket)) + 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 end @impl true @@ -424,6 +465,12 @@ 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 6b726c2..a4d8506 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 6efdad3..1d51ce1 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,7 +141,47 @@ defmodule MvWeb.MembershipFeeTypeLive.Index do @impl true def handle_event("delete", %{"id" => id}, socket) do - MembershipFeeHelpers.delete_fee_type(socket, id, current_actor(socket)) + 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 end # Helper functions @@ -175,6 +215,12 @@ 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 bf61230..2e315b9 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 - import MvWeb.RoleLive.Helpers, only: [format_error: 1] - alias Mv.Authorization.PermissionSets + import MvWeb.RoleLive.Helpers, only: [format_error: 1] + @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 188754f..5e210b5 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 2aed457..d8153be 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 8a9b5cd..bdaae01 100644 --- a/lib/mv_web/live/statistics_live.ex +++ b/lib/mv_web/live/statistics_live.ex @@ -6,14 +6,13 @@ defmodule MvWeb.StatisticsLive do """ use MvWeb, :live_view - import MvWeb.LiveHelpers, only: [current_actor: 1] + 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 2d29592..60763ab 100644 --- a/lib/mv_web/live/user_live/form.ex +++ b/lib/mv_web/live/user_live/form.ex @@ -33,9 +33,7 @@ defmodule MvWeb.UserLive.Form do """ use MvWeb, :live_view - import MvWeb.Authorization, only: [can?: 3] - import MvWeb.ErrorHelpers, only: [format_ash_error: 1] - import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3] + require Jason alias Mv.Accounts alias Mv.Accounts.User, as: UserResource @@ -45,9 +43,10 @@ defmodule MvWeb.UserLive.Form do alias Mv.Membership alias Mv.Membership.Member, as: MemberResource alias MvWeb.Helpers.MemberHelpers - alias MvWeb.Live.MemberDropdownNav - require Jason + import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3] + import MvWeb.Authorization, only: [can?: 3] + import MvWeb.ErrorHelpers, only: [format_ash_error: 1] @impl true def render(assigns) do @@ -572,8 +571,56 @@ defmodule MvWeb.UserLive.Form do end @impl true - def handle_event("member_dropdown_keydown", params, socket) do - MemberDropdownNav.handle_keydown(params, socket, fn -> select_focused_member(socket) end) + 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} end @impl true @@ -731,6 +778,17 @@ 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 a5b3f6f..617b079 100644 --- a/lib/mv_web/live_user_auth.ex +++ b/lib/mv_web/live_user_auth.ex @@ -3,9 +3,8 @@ defmodule MvWeb.LiveUserAuth do Helpers for authenticating users in LiveViews. """ - use MvWeb, :verified_routes - import Phoenix.Component + use MvWeb, :verified_routes alias AshAuthentication.Phoenix.LiveSession alias Phoenix.LiveView diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 4c9a10a..3c21f67 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -193,7 +193,8 @@ 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/helpers/membership_fee_helpers.ex +#: lib/mv_web/live/membership_fee_settings_live.ex +#: lib/mv_web/live/membership_fee_type_live/index.ex #: lib/mv_web/live/role_live/helpers.ex #, elixir-autogen, elixir-format msgid "An error occurred" @@ -2288,12 +2289,14 @@ msgstr "" msgid "Membership fee start" msgstr "" -#: lib/mv_web/helpers/membership_fee_helpers.ex +#: lib/mv_web/live/membership_fee_settings_live.ex +#: lib/mv_web/live/membership_fee_type_live/index.ex #, elixir-autogen, elixir-format msgid "Membership fee type deleted" msgstr "" -#: lib/mv_web/helpers/membership_fee_helpers.ex +#: lib/mv_web/live/membership_fee_settings_live.ex +#: lib/mv_web/live/membership_fee_type_live/index.ex #, elixir-autogen, elixir-format msgid "Membership fee type not found" msgstr "" @@ -3953,7 +3956,8 @@ msgstr "" msgid "You do not have permission to %{action} members." msgstr "" -#: lib/mv_web/helpers/membership_fee_helpers.ex +#: lib/mv_web/live/membership_fee_settings_live.ex +#: lib/mv_web/live/membership_fee_type_live/index.ex #, elixir-autogen, elixir-format msgid "You do not have permission to access this membership fee type" msgstr "" @@ -3969,7 +3973,8 @@ msgstr "" msgid "You do not have permission to delete this member" msgstr "" -#: lib/mv_web/helpers/membership_fee_helpers.ex +#: lib/mv_web/live/membership_fee_settings_live.ex +#: lib/mv_web/live/membership_fee_type_live/index.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 deleted file mode 100644 index 2fa45fb..0000000 --- a/priv/repo/migrations/20260616165309_make_member_user_fks_deferrable.exs +++ /dev/null @@ -1,32 +0,0 @@ -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 25f0f90..4418b33 100644 --- a/test/membership/group_database_constraints_test.exs +++ b/test/membership/group_database_constraints_test.exs @@ -5,11 +5,10 @@ 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 a7befb9..72db874 100644 --- a/test/membership/group_test.exs +++ b/test/membership/group_test.exs @@ -5,11 +5,10 @@ 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 fece3d8..15f5636 100644 --- a/test/membership/join_request_approval_domain_test.exs +++ b/test/membership/join_request_approval_domain_test.exs @@ -8,14 +8,13 @@ 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 9915d4a..5f0ae83 100644 --- a/test/membership/join_request_test.exs +++ b/test/membership/join_request_test.exs @@ -12,14 +12,13 @@ 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 860d784..9be1272 100644 --- a/test/membership/member_cycle_calculations_test.exs +++ b/test/membership/member_cycle_calculations_test.exs @@ -4,15 +4,30 @@ 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 = %{ @@ -26,6 +41,23 @@ 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 f7afaa1..430ae7b 100644 --- a/test/membership/member_group_test.exs +++ b/test/membership/member_group_test.exs @@ -5,11 +5,10 @@ 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 188f71c..5ecddbd 100644 --- a/test/membership/member_groups_relationship_test.exs +++ b/test/membership/member_groups_relationship_test.exs @@ -4,11 +4,10 @@ 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 a2baf78..35b3137 100644 --- a/test/membership/member_type_change_integration_test.exs +++ b/test/membership/member_type_change_integration_test.exs @@ -4,10 +4,9 @@ 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 @@ -16,6 +15,21 @@ 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 = %{ @@ -30,6 +44,23 @@ 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 21e0a55..31c2847 100644 --- a/test/membership_fees/changes/validate_same_interval_test.exs +++ b/test/membership_fees/changes/validate_same_interval_test.exs @@ -4,15 +4,29 @@ 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 c6a1486..76f4d08 100644 --- a/test/membership_fees/member_cycle_integration_test.exs +++ b/test/membership_fees/member_cycle_integration_test.exs @@ -4,9 +4,8 @@ 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 @@ -15,6 +14,21 @@ 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 f4f89af..8770134 100644 --- a/test/membership_fees/membership_fee_cycle_test.exs +++ b/test/membership_fees/membership_fee_cycle_test.exs @@ -4,15 +4,29 @@ 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 = %{ @@ -26,6 +40,22 @@ 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 86ef284..f834094 100644 --- a/test/membership_fees/membership_fee_type_integration_test.exs +++ b/test/membership_fees/membership_fee_type_integration_test.exs @@ -4,8 +4,6 @@ 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 @@ -17,6 +15,21 @@ 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 deleted file mode 100644 index c8f24b0..0000000 --- a/test/mv/application_test.exs +++ /dev/null @@ -1,58 +0,0 @@ -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 940da35..36ddbd2 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 - import Mv.Fixtures - alias Mv.Authorization.Checks.HasPermission + import Mv.Fixtures + 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 3ae18cf..2b83e3b 100644 --- a/test/mv/membership/members_pdf_test.exs +++ b/test/mv/membership/members_pdf_test.exs @@ -274,38 +274,17 @@ defmodule Mv.Membership.MembersPDFTest do assert {:ok, _pdf_binary} = result - count_export_dirs = fn -> + # Wait a bit for cleanup (async cleanup might take a moment) + Process.sleep(100) + + # Count temp directories after + after_count = 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 eb9da26..a9e3316 100644 --- a/test/mv/membership_fees/cycle_generator_edge_cases_test.exs +++ b/test/mv/membership_fees/cycle_generator_edge_cases_test.exs @@ -12,10 +12,9 @@ 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 @@ -24,6 +23,21 @@ 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 a715ca9..f193903 100644 --- a/test/mv/membership_fees/cycle_generator_test.exs +++ b/test/mv/membership_fees/cycle_generator_test.exs @@ -4,10 +4,9 @@ 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 @@ -16,6 +15,21 @@ 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 e3e37ba..cf7e889 100644 --- a/test/mv/membership_fees/membership_fee_cycle_policies_test.exs +++ b/test/mv/membership_fees/membership_fee_cycle_policies_test.exs @@ -8,8 +8,6 @@ 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 @@ -34,15 +32,41 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do member end - defp fee_type_fixture do - create_fee_type(%{amount: Decimal.new("10.00"), description: "Test"}) + 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 end - defp cycle_fixture do - create_cycle(create_member_fixture(), fee_type_fixture(), %{ - cycle_start: Date.utc_today(), - amount: Decimal.new("10.00") - }) + 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 end describe "own_data permission set" do @@ -50,7 +74,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 = fee_type_fixture() + fee_type = create_fee_type_fixture() admin = Mv.Fixtures.user_with_role_fixture("admin") user = @@ -106,7 +130,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 = cycle_fixture() + cycle = create_cycle_fixture() %{actor: actor, user: user, cycle: cycle} end @@ -132,7 +156,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do test "cannot create cycle (returns forbidden)", %{user: user, actor: _actor} do member = create_member_fixture() - fee_type = fee_type_fixture() + fee_type = create_fee_type_fixture() assert {:error, %Ash.Error.Forbidden{}} = MembershipFees.create_membership_fee_cycle( @@ -156,7 +180,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 = cycle_fixture() + cycle = create_cycle_fixture() %{actor: actor, user: user, cycle: cycle} end @@ -186,7 +210,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do test "can create cycle", %{user: user, actor: _actor} do member = create_member_fixture() - fee_type = fee_type_fixture() + fee_type = create_fee_type_fixture() assert {:ok, created} = MembershipFees.create_membership_fee_cycle( @@ -211,7 +235,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do describe "admin permission set" do setup %{actor: actor} do user = Mv.Fixtures.user_with_role_fixture("admin") - cycle = cycle_fixture() + cycle = create_cycle_fixture() %{actor: actor, user: user, cycle: cycle} end @@ -246,7 +270,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do test "can create cycle", %{user: user, actor: _actor} do member = create_member_fixture() - fee_type = fee_type_fixture() + fee_type = create_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 new file mode 100644 index 0000000..4b77378 --- /dev/null +++ b/test/mv/oidc_role_sync_config_test.exs @@ -0,0 +1,59 @@ +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 deleted file mode 100644 index 4127082..0000000 --- a/test/mv/repo/deferrable_fk_test.exs +++ /dev/null @@ -1,37 +0,0 @@ -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 19533e2..6b72ffb 100644 --- a/test/mv/statistics_test.exs +++ b/test/mv/statistics_test.exs @@ -4,21 +4,36 @@ 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]}) @@ -116,7 +131,7 @@ defmodule Mv.StatisticsTest do end test "returns totals by status for cycles in that year", %{actor: actor} do - fee_type = create_fee_type(%{amount: Decimal.new("50.00")}, actor) + fee_type = create_fee_type(actor, %{amount: Decimal.new("50.00")}) # Creating members with fee type triggers cycle generation (2020..today). We use 2024 cycles. _member1 = @@ -156,8 +171,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(%{amount: Decimal.new("30.00")}, actor) - fee_type_b = create_fee_type(%{amount: Decimal.new("70.00")}, actor) + fee_type_a = create_fee_type(actor, %{amount: Decimal.new("30.00")}) + fee_type_b = create_fee_type(actor, %{amount: Decimal.new("70.00")}) _m1 = Mv.Fixtures.member_fixture(%{ @@ -192,7 +207,7 @@ defmodule Mv.StatisticsTest do end test "returns sum of amount for all unpaid cycles", %{actor: actor} do - fee_type = create_fee_type(%{amount: Decimal.new("50.00")}, actor) + fee_type = create_fee_type(actor, %{amount: Decimal.new("50.00")}) _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 b93accc..fcb45f6 100644 --- a/test/mv_web/components/member_filter_component_test.exs +++ b/test/mv_web/components/member_filter_component_test.exs @@ -8,15 +8,11 @@ defmodule MvWeb.Components.MemberFilterComponentTest do - Button label and badge logic - Filtering to show only boolean custom fields """ - # 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. + # async: false to prevent PostgreSQL deadlocks when running LiveView tests against DB use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext alias Mv.Membership.CustomField diff --git a/test/mv_web/helpers/membership_fee_helpers_test.exs b/test/mv_web/helpers/membership_fee_helpers_test.exs index c143b34..773bd26 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 2e600d0..7b0953a 100644 --- a/test/mv_web/live/custom_field_live/deletion_test.exs +++ b/test/mv_web/live/custom_field_live/deletion_test.exs @@ -15,11 +15,10 @@ 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 bd5c60c..4f7c8ec 100644 --- a/test/mv_web/live/custom_field_live/form_test.exs +++ b/test/mv_web/live/custom_field_live/form_test.exs @@ -8,11 +8,10 @@ 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 d0e73e4..8934e85 100644 --- a/test/mv_web/live/group_live/form_test.exs +++ b/test/mv_web/live/group_live/form_test.exs @@ -10,9 +10,8 @@ defmodule MvWeb.GroupLive.FormTest do - Security """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext alias Mv.Fixtures diff --git a/test/mv_web/live/group_live/index_test.exs b/test/mv_web/live/group_live/index_test.exs index aa1d23d..5adfe56 100644 --- a/test/mv_web/live/group_live/index_test.exs +++ b/test/mv_web/live/group_live/index_test.exs @@ -10,9 +10,8 @@ defmodule MvWeb.GroupLive.IndexTest do - Edge cases """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext 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 5cbf373..937ed1e 100644 --- a/test/mv_web/live/group_live/integration_test.exs +++ b/test/mv_web/live/group_live/integration_test.exs @@ -9,16 +9,14 @@ defmodule MvWeb.GroupLive.IntegrationTest do - URL persistence """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - - import Ash.Expr import Phoenix.LiveViewTest + require Ash.Query + import Ash.Expr + use Gettext, backend: MvWeb.Gettext 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 cbec37d..14866ef 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,8 @@ defmodule MvWeb.GroupLive.ShowAccessibilityTest do """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext 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 fbc3bac..761dc83 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,9 @@ defmodule MvWeb.GroupLive.ShowAddMemberTest do """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - - import MvWeb.GroupLiveHelpers import Phoenix.LiveViewTest + import MvWeb.GroupLiveHelpers + use Gettext, backend: MvWeb.Gettext 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 6737b91..ede25fd 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,8 @@ defmodule MvWeb.GroupLive.ShowAddRemoveMembersTest do """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext 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 f7b5e33..c75e623 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,8 @@ defmodule MvWeb.GroupLive.ShowAuthorizationTest do """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext 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 519e6ca..407ed1e 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,8 @@ defmodule MvWeb.GroupLive.ShowIntegrationTest do """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext 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 a855804..8a3d191 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,8 @@ defmodule MvWeb.GroupLive.ShowMemberSearchTest do """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext 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 125a169..1dfd290 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,8 @@ defmodule MvWeb.GroupLive.ShowRemoveMemberTest do """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext 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 0a82c15..ee30991 100644 --- a/test/mv_web/live/group_live/show_test.exs +++ b/test/mv_web/live/group_live/show_test.exs @@ -11,15 +11,13 @@ defmodule MvWeb.GroupLive.ShowTest do - Delete functionality """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + require Ash.Query + use Gettext, backend: MvWeb.Gettext 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 8837fc6..bd5cdec 100644 --- a/test/mv_web/live/import_live_test.exs +++ b/test/mv_web/live/import_live_test.exs @@ -37,27 +37,7 @@ defmodule MvWeb.ImportLiveTest do confirm_import(view) end - # 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 + defp wait_for_import_completion, do: Process.sleep(1000) # ---------- Business logic: Authorization ---------- describe "Authorization" do @@ -87,7 +67,7 @@ defmodule MvWeb.ImportLiveTest do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content) - wait_for_import_completion(view) + wait_for_import_completion() assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-summary']") @@ -151,7 +131,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(view) + wait_for_import_completion() assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-error-list']") @@ -170,7 +150,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(view) + wait_for_import_completion() assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-error-list']") @@ -202,7 +182,7 @@ defmodule MvWeb.ImportLiveTest do |> File.read!() run_full_import(view, csv_content, "bom_import.csv") - wait_for_import_completion(view) + wait_for_import_completion() assert has_element?(view, "[data-testid='import-results-panel']") html = render(view) @@ -220,7 +200,7 @@ defmodule MvWeb.ImportLiveTest do |> File.read!() run_full_import(view, csv_content, "empty_lines.csv") - wait_for_import_completion(view) + wait_for_import_completion() assert has_element?(view, "[data-testid='import-error-list']") html = render(view) @@ -234,7 +214,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(view) + wait_for_import_completion() assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-warnings']") @@ -299,7 +279,7 @@ defmodule MvWeb.ImportLiveTest do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content) - wait_for_import_completion(view) + wait_for_import_completion() assert has_element?(view, "[data-testid='import-progress-container']") html = render(view) assert html =~ "aria-live" @@ -362,7 +342,7 @@ defmodule MvWeb.ImportLiveTest do } do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content) - wait_for_import_completion(view) + wait_for_import_completion() 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 8844f91..fcce2a3 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,12 +20,11 @@ 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), date_gen()]) + one_of([ + constant(nil), + map(integer(-3650..3650), &Date.add(~D[2000-01-01], &1)) + ]) end defp exit_date_mode_gen do @@ -58,25 +57,19 @@ 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( - which <- member_of([:from_only, :to_only, :both]), - date_a <- date_gen(), - date_b <- date_gen() + from <- optional_date_gen(), + to <- optional_date_gen(), + from != nil or to != nil ) do - case which do - :from_only -> {date_a, nil} - :to_only -> {nil, date_a} - :both -> {date_a, date_b} - end + {from, to} end end - defp value_date_gen, do: date_gen() + defp value_date_gen do + map(integer(-3650..3650), &Date.add(~D[2000-01-01], &1)) + end # Property ------------------------------------------------------------- diff --git a/test/mv_web/live/member_live/deactivate_test.exs b/test/mv_web/live/member_live/deactivate_test.exs index 367c387..2e7a6a7 100644 --- a/test/mv_web/live/member_live/deactivate_test.exs +++ b/test/mv_web/live/member_live/deactivate_test.exs @@ -4,9 +4,8 @@ defmodule MvWeb.MemberLive.DeactivateTest do driven through the parent LiveView (the DeactivateComponent is stateful). """ use MvWeb.ConnCase, async: true - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + use Gettext, backend: MvWeb.Gettext alias Mv.Fixtures 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 998fb65..a836c4d 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,7 +5,6 @@ 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 @@ -18,6 +17,23 @@ 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 8c99d90..c65341f 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,7 +5,6 @@ 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 @@ -14,6 +13,22 @@ 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 857f54b..fe5c48d 100644 --- a/test/mv_web/live/role_live/show_test.exs +++ b/test/mv_web/live/role_live/show_test.exs @@ -11,15 +11,13 @@ defmodule MvWeb.RoleLive.ShowTest do - Delete functionality """ use MvWeb.ConnCase, async: false - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest + require Ash.Query + use Gettext, backend: MvWeb.Gettext 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 dbfa0c2..084e346 100644 --- a/test/mv_web/live/user_live/show_test.exs +++ b/test/mv_web/live/user_live/show_test.exs @@ -10,11 +10,9 @@ defmodule MvWeb.UserLive.ShowTest do - Error handling """ use MvWeb.ConnCase, async: true - use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest - require Ash.Query + use Gettext, backend: MvWeb.Gettext setup do # Create test user 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 01812df..5382dfc 100644 --- a/test/mv_web/member_live/form_membership_fee_type_test.exs +++ b/test/mv_web/member_live/form_membership_fee_type_test.exs @@ -5,10 +5,27 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - import Mv.Fixtures, only: [create_fee_type: 2] + + alias Mv.MembershipFees.MembershipFeeType 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 8412f61..5d96e68 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,13 +4,30 @@ 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() @@ -26,6 +43,27 @@ 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 c33210e..17e6da4 100644 --- a/test/mv_web/member_live/index_custom_fields_accessibility_test.exs +++ b/test/mv_web/member_live/index_custom_fields_accessibility_test.exs @@ -9,11 +9,10 @@ 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 74db2bf..9ada92b 100644 --- a/test/mv_web/member_live/index_custom_fields_display_test.exs +++ b/test/mv_web/member_live/index_custom_fields_display_test.exs @@ -9,17 +9,13 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsDisplayTest do - Custom field values are correctly formatted for different types - Members without custom field values show empty cell or "-" """ - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and custom fields use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + require Ash.Query alias Mv.Membership.{CustomField, CustomFieldValue} - require Ash.Query - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_custom_fields_edge_cases_test.exs b/test/mv_web/member_live/index_custom_fields_edge_cases_test.exs index 288ea0a..cf67fc3 100644 --- a/test/mv_web/member_live/index_custom_fields_edge_cases_test.exs +++ b/test/mv_web/member_live/index_custom_fields_edge_cases_test.exs @@ -8,11 +8,10 @@ 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 4119205..afce67b 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,11 +11,10 @@ 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 63dff1c..79d078b 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -10,18 +10,14 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do - Integration with member list display - Custom fields visibility """ - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and custom fields use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + require Ash.Query alias Mv.Membership.{CustomField, CustomFieldValue} - require Ash.Query - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -161,6 +157,9 @@ 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" @@ -187,6 +186,9 @@ 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" @@ -211,6 +213,9 @@ 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" @@ -232,6 +237,9 @@ 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 @@ -254,6 +262,9 @@ 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 @@ -318,6 +329,8 @@ 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 @@ -374,6 +387,8 @@ 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 @@ -443,6 +458,9 @@ 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 bfe47c4..d14cd9f 100644 --- a/test/mv_web/member_live/index_groups_accessibility_test.exs +++ b/test/mv_web/member_live/index_groups_accessibility_test.exs @@ -8,17 +8,13 @@ defmodule MvWeb.MemberLive.IndexGroupsAccessibilityTest do - Sort header has aria-label for screen reader - Keyboard navigation works (Tab through filter, sort header) """ - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + require Ash.Query alias Mv.Membership.{Group, MemberGroup} - require Ash.Query - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_display_test.exs b/test/mv_web/member_live/index_groups_display_test.exs index ed817dd..263ac2a 100644 --- a/test/mv_web/member_live/index_groups_display_test.exs +++ b/test/mv_web/member_live/index_groups_display_test.exs @@ -8,17 +8,13 @@ defmodule MvWeb.MemberLive.IndexGroupsDisplayTest do - No badge for members without groups - Badge shows group name correctly """ - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + require Ash.Query alias Mv.Membership.{Group, MemberGroup} - require Ash.Query - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_filter_test.exs b/test/mv_web/member_live/index_groups_filter_test.exs index c8d5160..d32b17f 100644 --- a/test/mv_web/member_live/index_groups_filter_test.exs +++ b/test/mv_web/member_live/index_groups_filter_test.exs @@ -6,17 +6,13 @@ defmodule MvWeb.MemberLive.IndexGroupsFilterTest do All / Yes / No (per group). Multiple active group filters combine with AND (member must match all selected group conditions). """ - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + require Ash.Query alias Mv.Membership.{Group, MemberGroup} - require Ash.Query - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_integration_test.exs b/test/mv_web/member_live/index_groups_integration_test.exs index cae9ce1..86738da 100644 --- a/test/mv_web/member_live/index_groups_integration_test.exs +++ b/test/mv_web/member_live/index_groups_integration_test.exs @@ -10,19 +10,15 @@ 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) """ - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + require Ash.Query alias Mv.Helpers.SystemActor alias Mv.Membership.{CustomField, CustomFieldValue, Group, MemberGroup} alias Mv.MembershipFees.{MembershipFeeCycle, MembershipFeeType} - require Ash.Query - setup do system_actor = SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_performance_test.exs b/test/mv_web/member_live/index_groups_performance_test.exs index 6d05908..761c4eb 100644 --- a/test/mv_web/member_live/index_groups_performance_test.exs +++ b/test/mv_web/member_live/index_groups_performance_test.exs @@ -8,17 +8,13 @@ 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) """ - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + require Ash.Query alias Mv.Membership.{Group, MemberGroup} - require Ash.Query - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_sorting_test.exs b/test/mv_web/member_live/index_groups_sorting_test.exs index 6e19779..068152c 100644 --- a/test/mv_web/member_live/index_groups_sorting_test.exs +++ b/test/mv_web/member_live/index_groups_sorting_test.exs @@ -2,17 +2,13 @@ defmodule MvWeb.MemberLive.IndexGroupsSortingTest do @moduledoc """ Tests for sorting by groups in the member overview. """ - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + require Ash.Query alias Mv.Membership.{Group, MemberGroup} - require Ash.Query - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_groups_url_params_test.exs b/test/mv_web/member_live/index_groups_url_params_test.exs index 8ce82aa..469b010 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,17 +9,13 @@ defmodule MvWeb.MemberLive.IndexGroupsUrlParamsTest do - URL parameters work with other parameters (query, sort_field, etc.) - URL is bookmarkable (filter/sorting persist) """ - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and groups use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + require Ash.Query alias Mv.Membership.{Group, MemberGroup} - require Ash.Query - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/index_membership_fee_status_test.exs b/test/mv_web/member_live/index_membership_fee_status_test.exs index 2efe055..ce6bee8 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,12 +5,29 @@ 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() @@ -26,16 +43,38 @@ 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, - replace_existing: true - }) + create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) {:ok, _view, html} = live(conn, "/members") @@ -46,18 +85,8 @@ 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, - replace_existing: true - }) - - create_cycle(member, fee_type, %{ - cycle_start: ~D[2023-01-01], - status: :unpaid, - replace_existing: true - }) + 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}) {:ok, view, _html} = live(conn, "/members") @@ -73,17 +102,8 @@ 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, - replace_existing: true - }) - - create_cycle(member, fee_type, %{ - cycle_start: current_year_start, - status: :suspended, - replace_existing: true - }) + 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}) {:ok, view, _html} = live(conn, "/members") @@ -100,12 +120,7 @@ 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, - replace_existing: true - }) + create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) {:ok, view, _html} = live(conn, "/members") @@ -116,12 +131,7 @@ 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, - replace_existing: true - }) + create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) {:ok, view, _html} = live(conn, "/members") @@ -132,12 +142,7 @@ 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, - replace_existing: true - }) + create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :suspended}) {:ok, view, _html} = live(conn, "/members") @@ -164,21 +169,11 @@ 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, - replace_existing: true - }) + create_cycle(member1, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) # 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, - replace_existing: true - }) + create_cycle(member2, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -210,21 +205,11 @@ 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, - replace_existing: true - }) + create_cycle(member1, fee_type, %{cycle_start: current_year_start, status: :unpaid}) # 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, - replace_existing: true - }) + create_cycle(member2, fee_type, %{cycle_start: current_year_start, status: :paid}) system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -258,12 +243,7 @@ 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, - replace_existing: true - }) + create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) 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 1985534..0cde8fd 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -1,18 +1,56 @@ 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 - import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] + require Ash.Query 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 @@ -299,14 +337,10 @@ defmodule MvWeb.MemberLive.IndexTest do send(view.pid, {:search_changed, "Friedrich"}) - # 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) + state = :sys.get_state(view.pid) + + assert state.socket.assigns.query == "Friedrich" + assert is_list(state.socket.assigns.members) end @tag :ui @@ -399,38 +433,6 @@ 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() @@ -566,13 +568,15 @@ defmodule MvWeb.MemberLive.IndexTest do # Select two members by sending the select_member event directly render_click(view, "select_member", %{"id" => member1.id}) - html = render_click(view, "select_member", %{"id" => member2.id}) + render_click(view, "select_member", %{"id" => member2.id}) - # 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" + # 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 end test "email format is 'First Last ' with comma separator", %{ @@ -1005,23 +1009,26 @@ 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") - # :all scope renders the muted "all" badge. - assert scope_badge(html) |> LazyHTML.text() |> String.trim() == "all" + assigns = :sys.get_state(view.pid).socket.assigns + assert assigns.scope == :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") - # An active search narrows the list, so the scope badge reads "filtered". - assert scope_badge(html) |> LazyHTML.text() |> String.trim() == "filtered" + assigns = :sys.get_state(view.pid).socket.assigns + assert assigns.scope == :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") + {:ok, view, html} = live(conn, "/members?cycle_status_filter=paid") + + assigns = :sys.get_state(view.pid).socket.assigns + assert assigns.scope == :filtered badge = scope_badge(html) assert badge |> LazyHTML.text() |> String.trim() == "filtered" @@ -1034,13 +1041,10 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") - html = render_click(view, "select_member", %{"id" => member1.id}) + render_click(view, "select_member", %{"id" => member1.id}) - # 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" + assigns = :sys.get_state(view.pid).socket.assigns + assert assigns.scope == :selection end test "with no selection, recipient_count and mailto_bcc cover all members", %{ @@ -1049,11 +1053,11 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") - # The mailto link's BCC is the observable carrier of recipient_count/mailto_bcc. - bcc = mailto_bcc(view) + assigns = :sys.get_state(view.pid).socket.assigns # Both seeded members have an email, so the no-selection scope covers both. - assert bcc =~ "scope1%40example.com" - assert bcc =~ "scope2%40example.com" + assert assigns.recipient_count == 2 + assert assigns.mailto_bcc =~ "scope1%40example.com" + assert assigns.mailto_bcc =~ "scope2%40example.com" end test "with a selection, recipient_count and mailto_bcc cover only the selection", %{ @@ -1065,9 +1069,10 @@ defmodule MvWeb.MemberLive.IndexTest do render_click(view, "select_member", %{"id" => member1.id}) - bcc = mailto_bcc(view) - assert bcc =~ "scope1%40example.com" - refute bcc =~ "scope2%40example.com" + 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" end end @@ -1105,7 +1110,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: last_year_start, status: :paid, replace_existing: true}, + %{cycle_start: last_year_start, status: :paid}, system_actor ) @@ -1122,7 +1127,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: last_year_start, status: :unpaid, replace_existing: true}, + %{cycle_start: last_year_start, status: :unpaid}, system_actor ) @@ -1152,7 +1157,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: last_year_start, status: :paid, replace_existing: true}, + %{cycle_start: last_year_start, status: :paid}, system_actor ) @@ -1169,7 +1174,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: last_year_start, status: :unpaid, replace_existing: true}, + %{cycle_start: last_year_start, status: :unpaid}, system_actor ) @@ -1199,7 +1204,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: current_year_start, status: :paid, replace_existing: true}, + %{cycle_start: current_year_start, status: :paid}, system_actor ) @@ -1216,7 +1221,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: current_year_start, status: :unpaid, replace_existing: true}, + %{cycle_start: current_year_start, status: :unpaid}, system_actor ) @@ -1246,7 +1251,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: current_year_start, status: :paid, replace_existing: true}, + %{cycle_start: current_year_start, status: :paid}, system_actor ) @@ -1263,7 +1268,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: current_year_start, status: :unpaid, replace_existing: true}, + %{cycle_start: current_year_start, status: :unpaid}, system_actor ) @@ -1333,9 +1338,6 @@ 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 @@ -1347,9 +1349,6 @@ 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 @@ -1360,82 +1359,89 @@ 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) - # 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"}") + 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)) 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 - 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"}) + _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"}) {:ok, view, _html} = live(conn, "/members") - html = member_filter_html(view) - # 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", "")) - ) + state = :sys.get_state(view.pid) + boolean_custom_fields = state.socket.assigns.boolean_custom_fields - assert rendered_order == [field_a.id, field_m.id, field_z.id] + # Should be sorted by name ascending + names = Enum.map(boolean_custom_fields, & &1.name) + assert names == ["Alpha Field", "Middle Field", "Zebra Field"] 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: 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]") + # Test true value + {:ok, view1, _html} = + live(conn, "/members?bf_#{boolean_field.id}=true") - # 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]") + 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" 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") - open_member_filter(view) + {:ok, view, _html} = + live(conn, "/members?bf_#{fake_id}=true") - # 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)) + 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 == %{} 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") - open_member_filter(view) + {:ok, view, _html} = + live(conn, "/members?bf_#{string_field.id}=true") - # 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)) + 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 == %{} end test "handle_params ignores invalid filter values", %{conn: conn} do @@ -1446,12 +1452,15 @@ 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}") - open_member_filter(view) + {:ok, view, _html} = + live(conn, "/members?bf_#{boolean_field.id}=#{invalid_value}") - # 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" + 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" end end @@ -1466,16 +1475,12 @@ defmodule MvWeb.MemberLive.IndexTest do "/members?bf_#{boolean_field1.id}=true&bf_#{boolean_field2.id}=false" ) - open_member_filter(view) + state = :sys.get_state(view.pid) + filters = state.socket.assigns.boolean_custom_field_filters - # 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?() + assert filters[boolean_field1.id] == true + assert filters[boolean_field2.id] == false + assert map_size(filters) == 2 end test "build_query_params includes active boolean filters and excludes nil filters", %{ @@ -1549,12 +1554,12 @@ defmodule MvWeb.MemberLive.IndexTest do "/members?cycle_status_filter=paid&bf_#{boolean_field.id}=true" ) - open_member_filter(view) + state = :sys.get_state(view.pid) + filters = state.socket.assigns.boolean_custom_field_filters - # 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 filters should be set + assert filters[boolean_field.id] == true + assert state.socket.assigns.cycle_status_filter == :paid # Both should be in URL when triggering search view @@ -1575,8 +1580,9 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_#{boolean_field.id}=true") - open_member_filter(view) - assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-true"}[checked]") + state_before = :sys.get_state(view.pid) + filters_before = state_before.socket.assigns.boolean_custom_field_filters + assert filters_before[boolean_field.id] == true # Delete the custom field (requires actor with destroy permission) Ash.destroy!(boolean_field, actor: system_actor) @@ -1585,11 +1591,12 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view2, _html} = live(conn, "/members?bf_#{boolean_field.id}=true") - open_member_filter(view2) + state_after = :sys.get_state(view2.pid) + filters_after = state_after.socket.assigns.boolean_custom_field_filters - # 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)) + # Filter should not be present for deleted custom field + refute Map.has_key?(filters_after, boolean_field.id) + assert filters_after == %{} end test "handle_params handles URL-encoded custom field IDs correctly", %{conn: conn} do @@ -1602,11 +1609,12 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_#{encoded_id}=true") - open_member_filter(view) + state = :sys.get_state(view.pid) + filters = state.socket.assigns.boolean_custom_field_filters - # 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]") + # Filter should work with URL-encoded ID + # Phoenix should decode it automatically, so we check with original ID + assert filters[boolean_field.id] == true end test "handle_params ignores malformed prefix (bf_bf_)", %{conn: conn} do @@ -1617,12 +1625,12 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_bf_#{boolean_field.id}=true") - open_member_filter(view) + state = :sys.get_state(view.pid) + filters = state.socket.assigns.boolean_custom_field_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)) + # Should not parse as valid filter (UUID validation should fail) + refute Map.has_key?(filters, boolean_field.id) + assert filters == %{} end test "handle_params limits number of boolean filters to prevent DoS", %{conn: conn} do @@ -1637,11 +1645,17 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?#{filter_params}") - # 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?() + 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) end test "handle_params ignores extremely long custom field IDs", %{conn: conn} do @@ -1654,12 +1668,14 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_#{fake_long_id}=true") - open_member_filter(view) + state = :sys.get_state(view.pid) + filters = state.socket.assigns.boolean_custom_field_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)) + # 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 == %{} end # Helper to create a member with a boolean custom field value @@ -2180,7 +2196,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( member_paid_true, fee_type, - %{cycle_start: last_year_start, status: :paid, replace_existing: true}, + %{cycle_start: last_year_start, status: :paid}, system_actor ) @@ -2208,7 +2224,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( member_unpaid_true, fee_type, - %{cycle_start: last_year_start, status: :unpaid, replace_existing: true}, + %{cycle_start: last_year_start, status: :unpaid}, system_actor ) @@ -2307,20 +2323,24 @@ defmodule MvWeb.MemberLive.IndexTest do # Start with no boolean custom fields {:ok, view, _html} = live(conn, "/members") - open_member_filter(view) - # No boolean field control is rendered yet. - refute has_element?(view, ~s(input[name^="custom_boolean"])) + state_before = :sys.get_state(view.pid) + boolean_fields_before = state_before.socket.assigns.boolean_custom_fields + assert boolean_fields_before == [] # Create a new boolean custom field new_boolean_field = create_boolean_custom_field(%{name: "Newly Added Field"}) - # Navigate again - the new field should appear in the filter dropdown. + # Navigate again - the new field should appear {:ok, view2, _html} = live(conn, "/members") - html_after = member_filter_html(view2) - assert has_element?(view2, "##{"custom-boolean-filter-#{new_boolean_field.id}-all"}") - assert html_after =~ "Newly Added Field" + 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")) 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 16f1064..ac60220 100644 --- a/test/mv_web/member_live/membership_fee_integration_test.exs +++ b/test/mv_web/member_live/membership_fee_integration_test.exs @@ -5,12 +5,29 @@ 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 a40ed2f..f8434b3 100644 --- a/test/mv_web/member_live/show_groups_display_test.exs +++ b/test/mv_web/member_live/show_groups_display_test.exs @@ -10,20 +10,16 @@ defmodule MvWeb.MemberLive.ShowGroupsDisplayTest do - Accessibility: group links have aria-label for screen readers ## Note on async - 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. + async: false to avoid PostgreSQL deadlocks when creating members and groups + in the same test run (same as IndexGroupsDisplayTest). """ use MvWeb.ConnCase, async: false + import Phoenix.LiveViewTest + require Ash.Query use Gettext, backend: MvWeb.Gettext - import Phoenix.LiveViewTest - alias Mv.Membership.{Group, MemberGroup} - require Ash.Query - describe "groups section" do setup do actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/show_membership_fees_test.exs b/test/mv_web/member_live/show_membership_fees_test.exs index 9c1b73f..394d743 100644 --- a/test/mv_web/member_live/show_membership_fees_test.exs +++ b/test/mv_web/member_live/show_membership_fees_test.exs @@ -5,12 +5,56 @@ 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}) @@ -32,19 +76,8 @@ 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, - replace_existing: true - }) - - _cycle2 = - create_cycle(member, fee_type, %{ - cycle_start: ~D[2023-01-01], - status: :unpaid, - replace_existing: true - }) + _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}) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -68,8 +101,7 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do create_cycle(member, fee_type, %{ cycle_start: ~D[2023-01-01], amount: Decimal.new("60.00"), - status: :paid, - replace_existing: true + status: :paid }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -126,12 +158,7 @@ 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, - replace_existing: true - }) + cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -162,12 +189,7 @@ 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, - replace_existing: true - }) + cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -198,12 +220,7 @@ 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, - replace_existing: true - }) + cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -299,13 +316,7 @@ 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, - replace_existing: true - }) + _cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -324,13 +335,7 @@ 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, - replace_existing: true - }) + cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -356,13 +361,7 @@ 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, - replace_existing: true - }) + cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) assert {:error, %Ash.Error.Forbidden{}} = Ash.destroy(cycle, domain: Mv.MembershipFees, actor: read_only_user) @@ -390,20 +389,8 @@ 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, - replace_existing: true - }) - - _c2 = - create_cycle(member, fee_type, %{ - cycle_start: ~D[2023-01-01], - status: :unpaid, - replace_existing: true - }) + _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}) {: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 c6f26a8..2ec5a68 100644 --- a/test/mv_web/member_live/show_test.exs +++ b/test/mv_web/member_live/show_test.exs @@ -14,14 +14,12 @@ 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 63649e4..a22c230 100644 --- a/test/mv_web/user_live/form_test.exs +++ b/test/mv_web/user_live/form_test.exs @@ -1,8 +1,5 @@ defmodule MvWeb.UserLive.FormTest do - # 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. + # async: false to prevent PostgreSQL deadlocks when creating members and users use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest diff --git a/test/seeds_test.exs b/test/seeds_test.exs index 2721d13..c729dbb 100644 --- a/test/seeds_test.exs +++ b/test/seeds_test.exs @@ -24,11 +24,21 @@ 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() - # Each test runs in its own sandbox-owned process, so seeds are loaded once per test. - Code.eval_file("priv/repo/seeds.exs") + # 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 %{actor: system_actor} end diff --git a/test/support/fixtures.ex b/test/support/fixtures.ex index e022600..73bf12a 100644 --- a/test/support/fixtures.ex +++ b/test/support/fixtures.ex @@ -335,82 +335,4 @@ 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