diff --git a/lib/mv/membership/import/member_csv.ex b/lib/mv/membership/import/member_csv.ex index 319ff81..4222fc3 100644 --- a/lib/mv/membership/import/member_csv.ex +++ b/lib/mv/membership/import/member_csv.ex @@ -310,10 +310,17 @@ defmodule Mv.Membership.Import.MemberCSV do case process_row(row_map, line_number, custom_field_lookup, actor) do {:ok, _member} -> - update_inserted({acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?}) + update_inserted( + {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?} + ) {:error, error} -> - handle_row_error({acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?}, error, current_error_count, max_errors) + handle_row_error( + {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?}, + error, + current_error_count, + max_errors + ) end end) diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index c9cb8ac..b633c48 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -364,114 +364,97 @@ defmodule MvWeb.GlobalSettingsLive do @impl true def handle_event("start_import", _params, socket) do - # Server-side admin check - if Authorization.can?(socket.assigns[:current_user], :create, Mv.Membership.Member) do - # Prevent concurrent imports - if socket.assigns.import_status == :running do + case check_import_prerequisites(socket) do + {:error, message} -> + {:noreply, put_flash(socket, :error, message)} + + :ok -> + process_csv_upload(socket) + end + end + + # Checks if import can be started (admin permission, status, upload ready) + defp check_import_prerequisites(socket) do + cond do + not Authorization.can?(socket.assigns[:current_user], :create, Mv.Membership.Member) -> + {:error, gettext("Only administrators can import members from CSV files.")} + + socket.assigns.import_status == :running -> + {:error, gettext("Import is already running. Please wait for it to complete.")} + + Enum.empty?(socket.assigns.uploads.csv_file.entries) -> + {:error, gettext("Please select a CSV file to import.")} + + not List.first(socket.assigns.uploads.csv_file.entries).done? -> + {:error, + gettext("Please wait for the file upload to complete before starting the import.")} + + true -> + :ok + end + end + + # Processes CSV upload and starts import + defp process_csv_upload(socket) do + with {:ok, content} <- consume_and_read_csv(socket), + {:ok, import_state} <- MemberCSV.prepare(content) do + start_import(socket, import_state) + else + {:error, reason} when is_binary(reason) -> {:noreply, put_flash( socket, :error, - gettext("Import is already running. Please wait for it to complete.") + gettext("Failed to prepare CSV import: %{reason}", reason: reason) )} - else - # Check if upload is completed - upload_entries = socket.assigns.uploads.csv_file.entries - if Enum.empty?(upload_entries) do - {:noreply, - put_flash( - socket, - :error, - gettext("Please select a CSV file to import.") - )} - else - entry = List.first(upload_entries) + {:error, error} -> + error_message = format_error_message(error) + {:noreply, + put_flash( + socket, + :error, + gettext("Failed to prepare CSV import: %{error}", error: error_message) + )} + end + end - if entry.done? do - with {:ok, content} <- consume_and_read_csv(socket) do - case MemberCSV.prepare(content) do - {:ok, import_state} -> - total_chunks = length(import_state.chunks) + # Starts the import process + defp start_import(socket, import_state) do + progress = initialize_import_progress(import_state) - progress = %{ - inserted: 0, - failed: 0, - errors: [], - warnings: import_state.warnings || [], - status: :running, - current_chunk: 0, - total_chunks: total_chunks, - errors_truncated?: false - } + socket = + socket + |> assign(:import_state, import_state) + |> assign(:import_progress, progress) + |> assign(:import_status, :running) - socket = - socket - |> assign(:import_state, import_state) - |> assign(:import_progress, progress) - |> assign(:import_status, :running) + send(self(), {:process_chunk, 0}) - send(self(), {:process_chunk, 0}) + {:noreply, socket} + end - {:noreply, socket} + # Initializes import progress structure + defp initialize_import_progress(import_state) do + %{ + inserted: 0, + failed: 0, + errors: [], + warnings: import_state.warnings || [], + status: :running, + current_chunk: 0, + total_chunks: length(import_state.chunks), + errors_truncated?: false + } + end - {:error, error} -> - error_message = - case error do - %{message: msg} when is_binary(msg) -> msg - %{errors: errors} when is_list(errors) -> inspect(errors) - reason when is_binary(reason) -> reason - other -> inspect(other) - end - - {:noreply, - put_flash( - socket, - :error, - gettext("Failed to prepare CSV import: %{error}", error: error_message) - )} - end - else - {:error, reason} when is_binary(reason) -> - {:noreply, - put_flash( - socket, - :error, - gettext("Failed to prepare CSV import: %{reason}", reason: reason) - )} - - {:error, error} -> - error_message = - case error do - %{message: msg} when is_binary(msg) -> msg - %{errors: errors} when is_list(errors) -> inspect(errors) - other -> inspect(other) - end - - {:noreply, - put_flash( - socket, - :error, - gettext("Failed to prepare CSV import: %{error}", error: error_message) - )} - end - else - {:noreply, - put_flash( - socket, - :error, - gettext("Please wait for the file upload to complete before starting the import.") - )} - end - end - end - else - {:noreply, - put_flash( - socket, - :error, - gettext("Only administrators can import members from CSV files.") - )} + # Formats error messages for display + defp format_error_message(error) do + case error do + %{message: msg} when is_binary(msg) -> msg + %{errors: errors} when is_list(errors) -> inspect(errors) + reason when is_binary(reason) -> reason + other -> inspect(other) end end @@ -610,7 +593,9 @@ defmodule MvWeb.GlobalSettingsLive do send(live_view_pid, {:chunk_done, idx, chunk_result}) else # Start async task to process chunk in production - Task.Supervisor.async_nolink(Mv.TaskSupervisor, fn -> + # Use start_child for fire-and-forget: no monitor, no Task messages + # We only use our own send/2 messages for communication + Task.Supervisor.start_child(Mv.TaskSupervisor, fn -> {:ok, chunk_result} = MemberCSV.process_chunk( chunk, diff --git a/test/mv_web/live/global_settings_live_test.exs b/test/mv_web/live/global_settings_live_test.exs index 1756dc1..a627fee 100644 --- a/test/mv_web/live/global_settings_live_test.exs +++ b/test/mv_web/live/global_settings_live_test.exs @@ -28,17 +28,21 @@ defmodule MvWeb.GlobalSettingsLiveTest do if has_element?(view, "[data-testid='import-results-panel']") do {:halt, html} else - if attempt < max_attempts do - # Process any pending messages - :timer.sleep(50) - {:cont, nil} - else - {:halt, html} - end + check_attempt_limit(attempt, max_attempts, html) end end) end + # Checks if we should continue or halt based on attempt limit + defp check_attempt_limit(attempt, max_attempts, html) do + if attempt < max_attempts do + :timer.sleep(50) + {:cont, nil} + else + {:halt, html} + end + end + describe "Global Settings LiveView" do setup %{conn: conn} do user = create_test_user(%{email: "admin@example.com"})