From 856ce53295e025ff3d627d9e1739a269c95ec822 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 26 Dec 2025 21:41:05 +0100 Subject: [PATCH] fix: improve MembershipFeesComponent state management and error handling Replace assign_new with assign for cycles and available_fee_types. Set regenerating flag at event start. Fix create_cycle parsing with explicit error handling. Use atomic bulk delete for all cycles. Improve delete confirmation robustness. Fix unless/else pattern for Credo compliance. --- .../show/membership_fees_component.ex | 132 +++++++++++------- 1 file changed, 79 insertions(+), 53 deletions(-) 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 cf46dc7..f96fd73 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 @@ -200,11 +200,12 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do {gettext("Amount")} String.replace(".", ",")} class="input input-bordered w-full" required /> @@ -294,7 +295,8 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do phx-target={@myself} class="btn btn-error" disabled={ - @delete_all_confirmation != gettext("Yes") && @delete_all_confirmation != "Yes" + String.trim(String.downcase(@delete_all_confirmation)) != + String.downcase(gettext("Yes")) } > {gettext("Delete All")} @@ -351,12 +353,15 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do {gettext("Amount")} String.replace(".", ",") + } class="input input-bordered w-full" required aria-label={gettext("Amount")} @@ -403,8 +408,8 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do {:ok, socket |> assign(assigns) - |> assign_new(:cycles, fn -> cycles end) - |> assign_new(:available_fee_types, fn -> available_fee_types end) + |> assign(:cycles, cycles) + |> assign(:available_fee_types, available_fee_types) |> assign_new(:interval_warning, fn -> nil end) |> assign_new(:editing_cycle, fn -> nil end) |> assign_new(:deleting_cycle, fn -> nil end) @@ -438,7 +443,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do def handle_event("change_membership_fee_type", %{"value" => fee_type_id}, socket) do member = socket.assigns.member - new_fee_type = Ash.get!(MembershipFeeType, fee_type_id) + new_fee_type = Ash.get!(MembershipFeeType, fee_type_id, domain: MembershipFees) # Check if interval matches interval_warning = @@ -500,7 +505,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do :suspended -> :mark_as_suspended end - case Ash.update(cycle, action: action) do + case Ash.update(cycle, action: action, domain: MembershipFees) do {:ok, updated_cycle} -> updated_cycles = replace_cycle(socket.assigns.cycles, updated_cycle) @@ -528,6 +533,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do end def handle_event("regenerate_cycles", _params, socket) do + socket = assign(socket, :regenerating, true) member = socket.assigns.member case CycleGenerator.generate_cycles_for_member(member.id) do @@ -580,11 +586,14 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do def handle_event("save_cycle_amount", %{"cycle_id" => cycle_id, "amount" => amount_str}, socket) do cycle = find_cycle(socket.assigns.cycles, cycle_id) - case Decimal.parse(amount_str) do + # Normalize comma to dot for decimal parsing (German locale support) + normalized_amount_str = String.replace(amount_str, ",", ".") + + case Decimal.parse(normalized_amount_str) do {amount, _} when is_struct(amount, Decimal) -> case cycle |> Ash.Changeset.for_update(:update, %{amount: amount}) - |> Ash.update() do + |> Ash.update(domain: MembershipFees) do {:ok, updated_cycle} -> updated_cycles = replace_cycle(socket.assigns.cycles, updated_cycle) @@ -621,7 +630,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do def handle_event("confirm_delete_cycle", %{"cycle_id" => cycle_id}, socket) do cycle = find_cycle(socket.assigns.cycles, cycle_id) - case Ash.destroy(cycle) do + case Ash.destroy(cycle, domain: MembershipFees) do :ok -> updated_cycles = Enum.reject(socket.assigns.cycles, &(&1.id == cycle_id)) @@ -668,52 +677,60 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do end def handle_event("confirm_delete_all_cycles", _params, socket) do - member = socket.assigns.member - cycles = socket.assigns.cycles + # Validate confirmation (case-insensitive, trimmed) + confirmation = String.trim(String.downcase(socket.assigns.delete_all_confirmation)) + expected = String.downcase(gettext("Yes")) - # Delete all cycles - results = - Enum.map(cycles, fn cycle -> - Ash.destroy(cycle) - end) + if confirmation != expected do + {:noreply, + socket + |> assign(:deleting_all_cycles, false) + |> assign(:delete_all_confirmation, "") + |> put_flash(:error, gettext("Confirmation text does not match"))} + else + member = socket.assigns.member - # Check if all deletions were successful - errors = Enum.filter(results, &match?({:error, _}, &1)) + # Delete all cycles atomically using Ecto query + import Ecto.Query - if Enum.empty?(errors) do - # Reload member to get updated cycles - updated_member = - member - |> Ash.load!([ - :membership_fee_type, - membership_fee_cycles: [:membership_fee_type] - ]) - - updated_cycles = - Enum.sort_by( - updated_member.membership_fee_cycles || [], - & &1.cycle_start, - {:desc, Date} + deleted_count = + Mv.Repo.delete_all( + from c in Mv.MembershipFees.MembershipFeeCycle, + where: c.member_id == ^member.id ) - send(self(), {:member_updated, updated_member}) + if deleted_count > 0 do + # Reload member to get updated cycles + updated_member = + member + |> Ash.load!([ + :membership_fee_type, + membership_fee_cycles: [:membership_fee_type] + ]) - {:noreply, - socket - |> assign(:member, updated_member) - |> assign(:cycles, updated_cycles) - |> assign(:deleting_all_cycles, false) - |> assign(:delete_all_confirmation, "") - |> put_flash(:info, gettext("All cycles deleted"))} - else - error_msg = - Enum.map_join(errors, ", ", fn {:error, error} -> format_error(error) end) + updated_cycles = + Enum.sort_by( + updated_member.membership_fee_cycles || [], + & &1.cycle_start, + {:desc, Date} + ) - {:noreply, - socket - |> assign(:deleting_all_cycles, false) - |> assign(:delete_all_confirmation, "") - |> put_flash(:error, gettext("Failed to delete some cycles: %{errors}", errors: error_msg))} + send(self(), {:member_updated, updated_member}) + + {:noreply, + socket + |> assign(:member, updated_member) + |> assign(:cycles, updated_cycles) + |> assign(:deleting_all_cycles, false) + |> assign(:delete_all_confirmation, "") + |> put_flash(:info, gettext("All cycles deleted"))} + else + {:noreply, + socket + |> assign(:deleting_all_cycles, false) + |> assign(:delete_all_confirmation, "") + |> put_flash(:info, gettext("No cycles to delete"))} + end end end @@ -749,8 +766,17 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do def handle_event("create_cycle", %{"date" => date_str, "amount" => amount_str}, socket) do member = socket.assigns.member + # Normalize comma to dot for decimal parsing (German locale support) + normalized_amount_str = String.replace(amount_str, ",", ".") + + amount = + case Decimal.parse(normalized_amount_str) do + {d, _} when is_struct(d, Decimal) -> {:ok, d} + :error -> {:error, :invalid_amount} + end + with {:ok, date} <- Date.from_iso8601(date_str), - {amount, _} when is_struct(amount, Decimal) <- Decimal.parse(amount_str), + {:ok, amount} <- amount, cycle_start <- CalendarCycles.calculate_cycle_start(date, member.membership_fee_type.interval), :ok <- validate_cycle_not_exists(socket.assigns.cycles, cycle_start) do @@ -762,7 +788,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do membership_fee_type_id: member.membership_fee_type_id } - case Ash.create(MembershipFeeCycle, attrs) do + case Ash.create(MembershipFeeCycle, attrs, domain: MembershipFees) do {:ok, _new_cycle} -> # Reload member with cycles updated_member =