From 7994303166cd605ba2965970be09dc9491a18a6d Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 12 Dec 2025 20:08:03 +0100 Subject: [PATCH] feat: add validation for same-interval membership fee type changes --- lib/membership/member.ex | 5 + .../changes/validate_same_interval.ex | 119 ++++++++++ mix.lock | 6 +- .../changes/validate_same_interval_test.exs | 220 ++++++++++++++++++ 4 files changed, 347 insertions(+), 3 deletions(-) create mode 100644 lib/membership_fees/changes/validate_same_interval.ex create mode 100644 test/membership_fees/changes/validate_same_interval_test.exs diff --git a/lib/membership/member.ex b/lib/membership/member.ex index b76fb64..4c90e05 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -178,6 +178,11 @@ defmodule Mv.Membership.Member do where [changing(:user)] end + # Validate that membership fee type changes only allow same-interval types + change Mv.MembershipFees.Changes.ValidateSameInterval do + where [changing(:membership_fee_type_id)] + end + # Auto-calculate membership_fee_start_date when membership_fee_type_id is set # and membership_fee_start_date is not already set change Mv.MembershipFees.Changes.SetMembershipFeeStartDate do diff --git a/lib/membership_fees/changes/validate_same_interval.ex b/lib/membership_fees/changes/validate_same_interval.ex new file mode 100644 index 0000000..0d067ce --- /dev/null +++ b/lib/membership_fees/changes/validate_same_interval.ex @@ -0,0 +1,119 @@ +defmodule Mv.MembershipFees.Changes.ValidateSameInterval do + @moduledoc """ + Validates that membership fee type changes only allow same-interval types. + + Prevents changing from yearly to monthly, etc. (MVP constraint). + + ## Usage + + In a Member action: + + update :update_member do + # ... + change Mv.MembershipFees.Changes.ValidateSameInterval + end + + The change module only executes when `membership_fee_type_id` is being changed. + If the new type has a different interval than the current type, a validation error is returned. + """ + use Ash.Resource.Change + + @impl true + def change(changeset, _opts, _context) do + if changing_membership_fee_type?(changeset) do + validate_interval_match(changeset) + else + changeset + end + end + + # Check if membership_fee_type_id is being changed + defp changing_membership_fee_type?(changeset) do + Ash.Changeset.changing_attribute?(changeset, :membership_fee_type_id) + end + + # Validate that the new type has the same interval as the current type + defp validate_interval_match(changeset) do + current_type_id = get_current_type_id(changeset) + new_type_id = get_new_type_id(changeset) + + # If no current type, allow any change (first assignment) + if is_nil(current_type_id) do + changeset + else + # If new type is nil, that's allowed (removing type) + if is_nil(new_type_id) do + changeset + else + # Both types exist - validate intervals match + case get_intervals(current_type_id, new_type_id) do + {:ok, current_interval, new_interval} -> + if current_interval == new_interval do + changeset + else + add_interval_mismatch_error(changeset, current_interval, new_interval) + end + + {:error, _reason} -> + # If we can't load the types, allow the change (fail open) + # The database constraint will catch invalid foreign keys + changeset + end + end + end + end + + # Get current type ID from changeset data + defp get_current_type_id(changeset) do + case changeset.data do + %{membership_fee_type_id: type_id} -> type_id + _ -> nil + end + end + + # Get new type ID from changeset + defp get_new_type_id(changeset) do + case Ash.Changeset.fetch_change(changeset, :membership_fee_type_id) do + {:ok, type_id} -> type_id + :error -> nil + end + end + + # Get intervals for both types + defp get_intervals(current_type_id, new_type_id) do + alias Mv.MembershipFees.MembershipFeeType + + case {Ash.get(MembershipFeeType, current_type_id), + Ash.get(MembershipFeeType, new_type_id)} do + {{:ok, current_type}, {:ok, new_type}} -> + {:ok, current_type.interval, new_type.interval} + + _ -> + {:error, :type_not_found} + end + end + + # Add validation error for interval mismatch + defp add_interval_mismatch_error(changeset, current_interval, new_interval) do + current_interval_name = format_interval(current_interval) + new_interval_name = format_interval(new_interval) + + message = + "Cannot change membership fee type: current type uses #{current_interval_name} interval, " <> + "new type uses #{new_interval_name} interval. Only same-interval changes are allowed." + + Ash.Changeset.add_error( + changeset, + field: :membership_fee_type_id, + message: message + ) + end + + # Format interval atom to human-readable string + defp format_interval(:monthly), do: "monthly" + defp format_interval(:quarterly), do: "quarterly" + defp format_interval(:half_yearly), do: "half-yearly" + defp format_interval(:yearly), do: "yearly" + defp format_interval(interval), do: to_string(interval) +end + diff --git a/mix.lock b/mix.lock index 44dffbf..1dd3d48 100644 --- a/mix.lock +++ b/mix.lock @@ -26,7 +26,7 @@ "esbuild": {:hex, :esbuild, "0.10.0", "b0aa3388a1c23e727c5a3e7427c932d89ee791746b0081bbe56103e9ef3d291f", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "468489cda427b974a7cc9f03ace55368a83e1a7be12fba7e30969af78e5f8c70"}, "ets": {:hex, :ets, "0.9.0", "79c6a6c205436780486f72d84230c6cba2f8a9920456750ddd1e47389107d5fd", [:mix], [], "hexpm", "2861fdfb04bcaeff370f1a5904eec864f0a56dcfebe5921ea9aadf2a481c822b"}, "ex_phone_number": {:hex, :ex_phone_number, "0.4.8", "c1c5e6f0673822a2a7b439b43af7d3eb1a5c19ae582b772b8b8d12625dd51ec1", [:mix], [{:sweet_xml, "~> 0.7", [hex: :sweet_xml, repo: "hexpm", optional: false]}], "hexpm", "43e2357c6b8cfe556bcd417f4ce9aaef267a786e31a2938902daaa0d36f69757"}, - "expo": {:hex, :expo, "1.1.1", "4202e1d2ca6e2b3b63e02f69cfe0a404f77702b041d02b58597c00992b601db5", [], [], "hexpm", "5fb308b9cb359ae200b7e23d37c76978673aa1b06e2b3075d814ce12c5811640"}, + "expo": {:hex, :expo, "1.1.1", "4202e1d2ca6e2b3b63e02f69cfe0a404f77702b041d02b58597c00992b601db5", [:mix], [], "hexpm", "5fb308b9cb359ae200b7e23d37c76978673aa1b06e2b3075d814ce12c5811640"}, "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"}, "finch": {:hex, :finch, "0.20.0", "5330aefb6b010f424dcbbc4615d914e9e3deae40095e73ab0c1bb0968933cadf", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.6.2 or ~> 1.7", [hex: :mint, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.4 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:nimble_pool, "~> 1.1", [hex: :nimble_pool, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "2658131a74d051aabfcba936093c903b8e89da9a1b63e430bee62045fa9b2ee2"}, "fine": {:hex, :fine, "0.1.4", "b19a89c1476c7c57afb5f9314aed5960b5bc95d5277de4cb5ee8e1d1616ce379", [:mix], [], "hexpm", "be3324cc454a42d80951cf6023b9954e9ff27c6daa255483b3e8d608670303f5"}, @@ -39,7 +39,7 @@ "iterex": {:hex, :iterex, "0.1.2", "58f9b9b9a22a55cbfc7b5234a9c9c63eaac26d276b3db80936c0e1c60355a5a6", [:mix], [], "hexpm", "2e103b8bcc81757a9af121f6dc0df312c9a17220f302b1193ef720460d03029d"}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, "joken": {:hex, :joken, "2.6.2", "5daaf82259ca603af4f0b065475099ada1b2b849ff140ccd37f4b6828ca6892a", [:mix], [{:jose, "~> 1.11.10", [hex: :jose, repo: "hexpm", optional: false]}], "hexpm", "5134b5b0a6e37494e46dbf9e4dad53808e5e787904b7c73972651b51cce3d72b"}, - "jose": {:hex, :jose, "1.11.12", "06e62b467b61d3726cbc19e9b5489f7549c37993de846dfb3ee8259f9ed208b3", [], [], "hexpm", "31e92b653e9210b696765cdd885437457de1add2a9011d92f8cf63e4641bab7b"}, + "jose": {:hex, :jose, "1.11.12", "06e62b467b61d3726cbc19e9b5489f7549c37993de846dfb3ee8259f9ed208b3", [:mix, :rebar3], [], "hexpm", "31e92b653e9210b696765cdd885437457de1add2a9011d92f8cf63e4641bab7b"}, "lazy_html": {:hex, :lazy_html, "0.1.8", "677a8642e644eef8de98f3040e2520d42d0f0f8bd6c5cd49db36504e34dffe91", [:make, :mix], [{:cc_precompiler, "~> 0.1", [hex: :cc_precompiler, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.9.0", [hex: :elixir_make, repo: "hexpm", optional: false]}, {:fine, "~> 0.1.0", [hex: :fine, repo: "hexpm", optional: false]}], "hexpm", "0d8167d930b704feb94b41414ca7f5779dff9bca7fcf619fcef18de138f08736"}, "libgraph": {:hex, :libgraph, "0.16.0", "3936f3eca6ef826e08880230f806bfea13193e49bf153f93edcf0239d4fd1d07", [:mix], [], "hexpm", "41ca92240e8a4138c30a7e06466acc709b0cbb795c643e9e17174a178982d6bf"}, "live_debugger": {:hex, :live_debugger, "0.5.0", "95e0f7727d61010f7e9053923fb2a9416904a7533c2dfb36120e7684cba4c0af", [:mix], [{:igniter, ">= 0.5.40 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:phoenix, "~> 1.7", [hex: :phoenix, repo: "hexpm", optional: false]}, {:phoenix_live_view, "~> 0.20.8 or ~> 1.0", [hex: :phoenix_live_view, repo: "hexpm", optional: false]}], "hexpm", "73ebe95118d22aa402675f677abd731cb16b136d1b6ae5f4010441fb50753b14"}, @@ -80,7 +80,7 @@ "telemetry_metrics": {:hex, :telemetry_metrics, "1.1.0", "5bd5f3b5637e0abea0426b947e3ce5dd304f8b3bc6617039e2b5a008adc02f8f", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "e7b79e8ddfde70adb6db8a6623d1778ec66401f366e9a8f5dd0955c56bc8ce67"}, "telemetry_poller": {:hex, :telemetry_poller, "1.3.0", "d5c46420126b5ac2d72bc6580fb4f537d35e851cc0f8dbd571acf6d6e10f5ec7", [:rebar3], [{:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "51f18bed7128544a50f75897db9974436ea9bfba560420b646af27a9a9b35211"}, "text_diff": {:hex, :text_diff, "0.1.0", "1caf3175e11a53a9a139bc9339bd607c47b9e376b073d4571c031913317fecaa", [:mix], [], "hexpm", "d1ffaaecab338e49357b6daa82e435f877e0649041ace7755583a0ea3362dbd7"}, - "thousand_island": {:hex, :thousand_island, "1.4.2", "735fa783005d1703359bbd2d3a5a3a398075ba4456e5afe3c5b7cf4666303d36", [], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "1c7637f16558fc1c35746d5ee0e83b18b8e59e18d28affd1f2fa1645f8bc7473"}, + "thousand_island": {:hex, :thousand_island, "1.4.2", "735fa783005d1703359bbd2d3a5a3a398075ba4456e5afe3c5b7cf4666303d36", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "1c7637f16558fc1c35746d5ee0e83b18b8e59e18d28affd1f2fa1645f8bc7473"}, "tidewave": {:hex, :tidewave, "0.5.2", "f549acffe9daeed8b6b547c232c60de987770da7f827f9b3300140dfc465b102", [:mix], [{:circular_buffer, "~> 0.4 or ~> 1.0", [hex: :circular_buffer, repo: "hexpm", optional: false]}, {:igniter, "~> 0.6", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:phoenix_live_reload, ">= 1.6.1", [hex: :phoenix_live_reload, repo: "hexpm", optional: true]}, {:plug, "~> 1.17", [hex: :plug, repo: "hexpm", optional: false]}, {:req, "~> 0.5", [hex: :req, repo: "hexpm", optional: false]}], "hexpm", "34ab3ffee7e402f05cd1eae68d0e77ed0e0d1925677971ef83634247553e8afd"}, "unicode_util_compat": {:hex, :unicode_util_compat, "0.7.1", "a48703a25c170eedadca83b11e88985af08d35f37c6f664d6dcfb106a97782fc", [:rebar3], [], "hexpm", "b3a917854ce3ae233619744ad1e0102e05673136776fb2fa76234f3e03b23642"}, "websock": {:hex, :websock, "0.5.3", "2f69a6ebe810328555b6fe5c831a851f485e303a7c8ce6c5f675abeb20ebdadc", [:mix], [], "hexpm", "6105453d7fac22c712ad66fab1d45abdf049868f253cf719b625151460b8b453"}, diff --git a/test/membership_fees/changes/validate_same_interval_test.exs b/test/membership_fees/changes/validate_same_interval_test.exs new file mode 100644 index 0000000..7b7a433 --- /dev/null +++ b/test/membership_fees/changes/validate_same_interval_test.exs @@ -0,0 +1,220 @@ +defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do + @moduledoc """ + Tests for ValidateSameInterval change module. + """ + use Mv.DataCase, async: true + + alias Mv.Membership.Member + alias Mv.MembershipFees.MembershipFeeType + alias Mv.MembershipFees.Changes.ValidateSameInterval + + # Helper to create a membership fee type + defp create_fee_type(attrs) 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!() + end + + # Helper to create a member + defp create_member(attrs) do + default_attrs = %{ + first_name: "Test", + last_name: "Member", + email: "test.member.#{System.unique_integer([:positive])}@example.com" + } + + attrs = Map.merge(default_attrs, attrs) + + Member + |> Ash.Changeset.for_create(:create_member, attrs) + |> Ash.create!() + end + + describe "validate_interval_match/1" do + test "allows change to type with same interval" do + yearly_type1 = create_fee_type(%{interval: :yearly, name: "Yearly Type 1"}) + yearly_type2 = create_fee_type(%{interval: :yearly, name: "Yearly Type 2"}) + + member = create_member(%{membership_fee_type_id: yearly_type1.id}) + + changeset = + member + |> Ash.Changeset.for_update(:update_member, %{ + membership_fee_type_id: yearly_type2.id + }) + |> ValidateSameInterval.change(%{}, %{}) + + assert changeset.valid? + end + + test "prevents change to type with different interval" do + yearly_type = create_fee_type(%{interval: :yearly}) + monthly_type = create_fee_type(%{interval: :monthly}) + + member = create_member(%{membership_fee_type_id: yearly_type.id}) + + changeset = + member + |> Ash.Changeset.for_update(:update_member, %{ + membership_fee_type_id: monthly_type.id + }) + |> ValidateSameInterval.change(%{}, %{}) + + refute changeset.valid? + assert %{errors: errors} = changeset + assert Enum.any?(errors, fn error -> + error.field == :membership_fee_type_id and + error.message =~ "yearly" and + error.message =~ "monthly" + end) + end + + test "allows first assignment of membership fee type" do + yearly_type = create_fee_type(%{interval: :yearly}) + member = create_member(%{}) # No fee type assigned + + changeset = + member + |> Ash.Changeset.for_update(:update_member, %{ + membership_fee_type_id: yearly_type.id + }) + |> ValidateSameInterval.change(%{}, %{}) + + assert changeset.valid? + end + + test "allows removal of membership fee type" do + yearly_type = create_fee_type(%{interval: :yearly}) + member = create_member(%{membership_fee_type_id: yearly_type.id}) + + changeset = + member + |> Ash.Changeset.for_update(:update_member, %{ + membership_fee_type_id: nil + }) + |> ValidateSameInterval.change(%{}, %{}) + + assert changeset.valid? + end + + test "does nothing when membership_fee_type_id is not changed" do + yearly_type = create_fee_type(%{interval: :yearly}) + member = create_member(%{membership_fee_type_id: yearly_type.id}) + + changeset = + member + |> Ash.Changeset.for_update(:update_member, %{ + first_name: "New Name" + }) + |> ValidateSameInterval.change(%{}, %{}) + + assert changeset.valid? + end + + test "error message is clear and helpful" do + yearly_type = create_fee_type(%{interval: :yearly}) + quarterly_type = create_fee_type(%{interval: :quarterly}) + + member = create_member(%{membership_fee_type_id: yearly_type.id}) + + changeset = + member + |> Ash.Changeset.for_update(:update_member, %{ + membership_fee_type_id: quarterly_type.id + }) + |> ValidateSameInterval.change(%{}, %{}) + + error = Enum.find(changeset.errors, &(&1.field == :membership_fee_type_id)) + assert error.message =~ "yearly" + assert error.message =~ "quarterly" + assert error.message =~ "same-interval" + end + + test "handles all interval types correctly" do + intervals = [:monthly, :quarterly, :half_yearly, :yearly] + + for interval1 <- intervals, + interval2 <- intervals, + interval1 != interval2 do + type1 = + create_fee_type(%{ + interval: interval1, + name: "Type #{interval1} #{System.unique_integer([:positive])}" + }) + + type2 = + create_fee_type(%{ + interval: interval2, + name: "Type #{interval2} #{System.unique_integer([:positive])}" + }) + + member = create_member(%{membership_fee_type_id: type1.id}) + + changeset = + member + |> Ash.Changeset.for_update(:update_member, %{ + membership_fee_type_id: type2.id + }) + |> ValidateSameInterval.change(%{}, %{}) + + refute changeset.valid?, + "Should prevent change from #{interval1} to #{interval2}" + end + end + end + + describe "integration with update_member action" do + test "validation works when updating member via update_member action" do + yearly_type = create_fee_type(%{interval: :yearly}) + monthly_type = create_fee_type(%{interval: :monthly}) + + member = create_member(%{membership_fee_type_id: yearly_type.id}) + + # Try to update member with different interval type + assert {:error, %Ash.Error.Invalid{} = error} = + member + |> Ash.Changeset.for_update(:update_member, %{ + membership_fee_type_id: monthly_type.id + }) + |> Ash.update() + + # Check that error is about interval mismatch + error_message = extract_error_message(error) + assert error_message =~ "yearly" + assert error_message =~ "monthly" + assert error_message =~ "same-interval" + end + + test "allows update when interval matches" do + yearly_type1 = create_fee_type(%{interval: :yearly, name: "Yearly Type 1"}) + yearly_type2 = create_fee_type(%{interval: :yearly, name: "Yearly Type 2"}) + + member = create_member(%{membership_fee_type_id: yearly_type1.id}) + + # Update member with same-interval type + assert {:ok, updated_member} = + member + |> Ash.Changeset.for_update(:update_member, %{ + membership_fee_type_id: yearly_type2.id + }) + |> Ash.update() + + assert updated_member.membership_fee_type_id == yearly_type2.id + end + + defp extract_error_message(%Ash.Error.Invalid{errors: errors}) do + errors + |> Enum.filter(&(&1.field == :membership_fee_type_id)) + |> Enum.map(& &1.message) + |> Enum.join(" ") + end + end +end