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.
This commit is contained in:
Moritz 2025-12-26 21:41:05 +01:00
parent 3afc20c2e2
commit 856ce53295

View file

@ -200,11 +200,12 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
<span class="label-text">{gettext("Amount")}</span> <span class="label-text">{gettext("Amount")}</span>
</label> </label>
<input <input
type="number" type="text"
inputmode="decimal"
name="amount" name="amount"
step="0.01" step="0.01"
min="0" min="0"
value={Decimal.to_string(@editing_cycle.amount)} value={Decimal.to_string(@editing_cycle.amount) |> String.replace(".", ",")}
class="input input-bordered w-full" class="input input-bordered w-full"
required required
/> />
@ -294,7 +295,8 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
phx-target={@myself} phx-target={@myself}
class="btn btn-error" class="btn btn-error"
disabled={ disabled={
@delete_all_confirmation != gettext("Yes") && @delete_all_confirmation != "Yes" String.trim(String.downcase(@delete_all_confirmation)) !=
String.downcase(gettext("Yes"))
} }
> >
{gettext("Delete All")} {gettext("Delete All")}
@ -351,12 +353,15 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
<span class="label-text">{gettext("Amount")}</span> <span class="label-text">{gettext("Amount")}</span>
</label> </label>
<input <input
type="number" type="text"
inputmode="decimal"
id="create-cycle-amount" id="create-cycle-amount"
name="amount" name="amount"
step="0.01" step="0.01"
min="0" min="0"
value={Decimal.to_string(@member.membership_fee_type.amount)} value={
Decimal.to_string(@member.membership_fee_type.amount) |> String.replace(".", ",")
}
class="input input-bordered w-full" class="input input-bordered w-full"
required required
aria-label={gettext("Amount")} aria-label={gettext("Amount")}
@ -403,8 +408,8 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
{:ok, {:ok,
socket socket
|> assign(assigns) |> assign(assigns)
|> assign_new(:cycles, fn -> cycles end) |> assign(:cycles, cycles)
|> assign_new(:available_fee_types, fn -> available_fee_types end) |> assign(:available_fee_types, available_fee_types)
|> assign_new(:interval_warning, fn -> nil end) |> assign_new(:interval_warning, fn -> nil end)
|> assign_new(:editing_cycle, fn -> nil end) |> assign_new(:editing_cycle, fn -> nil end)
|> assign_new(:deleting_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 def handle_event("change_membership_fee_type", %{"value" => fee_type_id}, socket) do
member = socket.assigns.member 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 # Check if interval matches
interval_warning = interval_warning =
@ -500,7 +505,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
:suspended -> :mark_as_suspended :suspended -> :mark_as_suspended
end end
case Ash.update(cycle, action: action) do case Ash.update(cycle, action: action, domain: MembershipFees) do
{:ok, updated_cycle} -> {:ok, updated_cycle} ->
updated_cycles = replace_cycle(socket.assigns.cycles, updated_cycle) updated_cycles = replace_cycle(socket.assigns.cycles, updated_cycle)
@ -528,6 +533,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
end end
def handle_event("regenerate_cycles", _params, socket) do def handle_event("regenerate_cycles", _params, socket) do
socket = assign(socket, :regenerating, true)
member = socket.assigns.member member = socket.assigns.member
case CycleGenerator.generate_cycles_for_member(member.id) do 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 def handle_event("save_cycle_amount", %{"cycle_id" => cycle_id, "amount" => amount_str}, socket) do
cycle = find_cycle(socket.assigns.cycles, cycle_id) 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) -> {amount, _} when is_struct(amount, Decimal) ->
case cycle case cycle
|> Ash.Changeset.for_update(:update, %{amount: amount}) |> Ash.Changeset.for_update(:update, %{amount: amount})
|> Ash.update() do |> Ash.update(domain: MembershipFees) do
{:ok, updated_cycle} -> {:ok, updated_cycle} ->
updated_cycles = replace_cycle(socket.assigns.cycles, 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 def handle_event("confirm_delete_cycle", %{"cycle_id" => cycle_id}, socket) do
cycle = find_cycle(socket.assigns.cycles, cycle_id) cycle = find_cycle(socket.assigns.cycles, cycle_id)
case Ash.destroy(cycle) do case Ash.destroy(cycle, domain: MembershipFees) do
:ok -> :ok ->
updated_cycles = Enum.reject(socket.assigns.cycles, &(&1.id == cycle_id)) updated_cycles = Enum.reject(socket.assigns.cycles, &(&1.id == cycle_id))
@ -668,52 +677,60 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
end end
def handle_event("confirm_delete_all_cycles", _params, socket) do def handle_event("confirm_delete_all_cycles", _params, socket) do
member = socket.assigns.member # Validate confirmation (case-insensitive, trimmed)
cycles = socket.assigns.cycles confirmation = String.trim(String.downcase(socket.assigns.delete_all_confirmation))
expected = String.downcase(gettext("Yes"))
# Delete all cycles if confirmation != expected do
results = {:noreply,
Enum.map(cycles, fn cycle -> socket
Ash.destroy(cycle) |> assign(:deleting_all_cycles, false)
end) |> assign(:delete_all_confirmation, "")
|> put_flash(:error, gettext("Confirmation text does not match"))}
else
member = socket.assigns.member
# Check if all deletions were successful # Delete all cycles atomically using Ecto query
errors = Enum.filter(results, &match?({:error, _}, &1)) import Ecto.Query
if Enum.empty?(errors) do deleted_count =
# Reload member to get updated cycles Mv.Repo.delete_all(
updated_member = from c in Mv.MembershipFees.MembershipFeeCycle,
member where: c.member_id == ^member.id
|> 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}
) )
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, updated_cycles =
socket Enum.sort_by(
|> assign(:member, updated_member) updated_member.membership_fee_cycles || [],
|> assign(:cycles, updated_cycles) & &1.cycle_start,
|> assign(:deleting_all_cycles, false) {:desc, Date}
|> 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)
{:noreply, send(self(), {:member_updated, updated_member})
socket
|> assign(:deleting_all_cycles, false) {:noreply,
|> assign(:delete_all_confirmation, "") socket
|> put_flash(:error, gettext("Failed to delete some cycles: %{errors}", errors: error_msg))} |> 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
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 def handle_event("create_cycle", %{"date" => date_str, "amount" => amount_str}, socket) do
member = socket.assigns.member 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), with {:ok, date} <- Date.from_iso8601(date_str),
{amount, _} when is_struct(amount, Decimal) <- Decimal.parse(amount_str), {:ok, amount} <- amount,
cycle_start <- cycle_start <-
CalendarCycles.calculate_cycle_start(date, member.membership_fee_type.interval), CalendarCycles.calculate_cycle_start(date, member.membership_fee_type.interval),
:ok <- validate_cycle_not_exists(socket.assigns.cycles, cycle_start) do :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 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} -> {:ok, _new_cycle} ->
# Reload member with cycles # Reload member with cycles
updated_member = updated_member =