From 433f008af8340e287c6b20e4e020a419261eb72a Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Jan 2026 16:05:09 +0100 Subject: [PATCH] refactor: Reduce function complexity and nesting depth - Extract helper functions from process_chunk to reduce nesting - Extract format_error_message from extract_changeset_error - Split extract_error_message into smaller functions to reduce complexity - Fixes Credo refactoring opportunities --- lib/mv/membership/import/member_csv.ex | 79 +++++++++++++++++------- lib/mv_web/live/member_live/form.ex | 83 ++++++++++++++++---------- 2 files changed, 107 insertions(+), 55 deletions(-) diff --git a/lib/mv/membership/import/member_csv.ex b/lib/mv/membership/import/member_csv.ex index b4f4318..d56c56e 100644 --- a/lib/mv/membership/import/member_csv.ex +++ b/lib/mv/membership/import/member_csv.ex @@ -302,28 +302,15 @@ defmodule Mv.Membership.Import.MemberCSV do max_errors = Keyword.get(opts, :max_errors, 50) {inserted, failed, errors, _collected_error_count, truncated?} = - Enum.reduce(chunk_rows_with_lines, {0, 0, [], 0, false}, fn {line_number, row_map}, - {acc_inserted, acc_failed, - acc_errors, acc_error_count, - acc_truncated?} -> - current_error_count = existing_error_count + acc_error_count + Enum.reduce(chunk_rows_with_lines, {0, 0, [], 0, false}, fn {line_number, row_map}, acc -> + current_error_count = existing_error_count + elem(acc, 3) case process_row(row_map, line_number, custom_field_lookup) do {:ok, _member} -> - {acc_inserted + 1, acc_failed, acc_errors, acc_error_count, acc_truncated?} + update_inserted(acc) {:error, error} -> - new_acc_failed = acc_failed + 1 - - # Only collect errors if under limit - {new_acc_errors, new_error_count, new_truncated?} = - if current_error_count < max_errors do - {[error | acc_errors], acc_error_count + 1, acc_truncated?} - else - {acc_errors, acc_error_count, true} - end - - {acc_inserted, new_acc_failed, new_acc_errors, new_error_count, new_truncated?} + handle_row_error(acc, error, current_error_count, max_errors) end end) @@ -397,11 +384,9 @@ defmodule Mv.Membership.Import.MemberCSV do # Extracts the first error from a changeset and converts it to a MemberCSV.Error struct defp extract_changeset_error(changeset, csv_line_number) do - case Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} -> - Enum.reduce(opts, msg, fn {key, value}, acc -> - String.replace(acc, "%{#{key}}", to_string(value)) - end) - end) do + errors = Ecto.Changeset.traverse_errors(changeset, &format_error_message/1) + + case errors do %{email: [message | _]} -> # Email-specific error %Error{ @@ -430,6 +415,56 @@ defmodule Mv.Membership.Import.MemberCSV do end end + # Helper function to update accumulator when row is successfully inserted + defp update_inserted({acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?}) do + {acc_inserted + 1, acc_failed, acc_errors, acc_error_count, acc_truncated?} + end + + # Helper function to handle row error with error count limit checking + defp handle_row_error( + {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?}, + error, + current_error_count, + max_errors + ) do + new_acc_failed = acc_failed + 1 + + {new_acc_errors, new_error_count, new_truncated?} = + collect_error_if_under_limit( + error, + acc_errors, + acc_error_count, + acc_truncated?, + current_error_count, + max_errors + ) + + {acc_inserted, new_acc_failed, new_acc_errors, new_error_count, new_truncated?} + end + + # Helper function to collect error only if under limit + defp collect_error_if_under_limit( + error, + acc_errors, + acc_error_count, + acc_truncated?, + current_error_count, + max_errors + ) do + if current_error_count < max_errors do + {[error | acc_errors], acc_error_count + 1, acc_truncated?} + else + {acc_errors, acc_error_count, true} + end + end + + # Formats error message by replacing placeholders + defp format_error_message({msg, opts}) do + Enum.reduce(opts, msg, fn {key, value}, acc -> + String.replace(acc, "%{#{key}}", to_string(value)) + end) + end + # Maps changeset error messages to appropriate Gettext messages defp gettext_error_message(message) when is_binary(message) do cond do diff --git a/lib/mv_web/live/member_live/form.ex b/lib/mv_web/live/member_live/form.ex index f07be75..395198a 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -355,48 +355,65 @@ defmodule MvWeb.MemberLive.Form do # Extracts a user-friendly error message from form errors defp extract_error_message(form) do - # Try to extract message from source errors first source_errors = get_source_errors(form) - case source_errors do - [%Ash.Error.Invalid{errors: errors} | _] when is_list(errors) -> - # Extract first error message - case List.first(errors) do - %{message: message} when is_binary(message) -> - gettext("Validation failed: %{message}", message: message) + cond do + has_invalid_error?(source_errors) -> + extract_invalid_error_message(source_errors) - %{field: field, message: message} when is_binary(message) -> - gettext("Validation failed: %{field} %{message}", field: field, message: message) + has_other_error?(source_errors) -> + extract_other_error_message(source_errors) - _ -> - gettext("Validation failed. Please check your input.") - end + has_form_errors?(form) -> + gettext("Please correct the errors in the form and try again.") - [error | _] -> - # Try to extract message from other error types - case error do - %{message: message} when is_binary(message) -> - message + true -> + gettext("Failed to save member. Please try again.") + end + end - error when is_struct(error) -> - # Try to use Ash.ErrorKind protocol if available - try do - Ash.ErrorKind.message(error) - rescue - Protocol.UndefinedError -> gettext("Failed to save member. Please try again.") - end + # Checks if source errors contain an Ash.Error.Invalid + defp has_invalid_error?([%Ash.Error.Invalid{errors: errors} | _]) when is_list(errors), do: true + defp has_invalid_error?(_), do: false - _ -> - gettext("Failed to save member. Please try again.") - end + # Extracts message from Ash.Error.Invalid + defp extract_invalid_error_message([%Ash.Error.Invalid{errors: errors} | _]) do + case List.first(errors) do + %{message: message} when is_binary(message) -> + gettext("Validation failed: %{message}", message: message) + + %{field: field, message: message} when is_binary(message) -> + gettext("Validation failed: %{field} %{message}", field: field, message: message) _ -> - # Check if there are any field errors in the form - if has_form_errors?(form) do - gettext("Please correct the errors in the form and try again.") - else - gettext("Failed to save member. Please try again.") - end + gettext("Validation failed. Please check your input.") + end + end + + # Checks if source errors contain other error types + defp has_other_error?([_ | _]), do: true + defp has_other_error?(_), do: false + + # Extracts message from other error types + defp extract_other_error_message([error | _]) do + cond do + Map.has_key?(error, :message) and is_binary(error.message) -> + error.message + + is_struct(error) -> + extract_struct_error_message(error) + + true -> + gettext("Failed to save member. Please try again.") + end + end + + # Extracts message from struct error using Ash.ErrorKind protocol + defp extract_struct_error_message(error) do + try do + Ash.ErrorKind.message(error) + rescue + Protocol.UndefinedError -> gettext("Failed to save member. Please try again.") end end