refactor
This commit is contained in:
parent
092fd99d48
commit
04b0916c1e
4 changed files with 182 additions and 95 deletions
|
|
@ -124,6 +124,45 @@ defmodule Mv.Membership.Import.HeaderMapper do
|
||||||
end)
|
end)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Returns a MapSet of normalized member field names.
|
||||||
|
|
||||||
|
This is the single source of truth for known member fields.
|
||||||
|
Used to distinguish between member fields and custom fields.
|
||||||
|
|
||||||
|
## Returns
|
||||||
|
|
||||||
|
- `MapSet.t(String.t())` - Set of normalized member field names
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
iex> HeaderMapper.known_member_fields()
|
||||||
|
#MapSet<["email", "firstname", "lastname", "street", "postalcode", "city"]>
|
||||||
|
"""
|
||||||
|
@spec known_member_fields() :: MapSet.t(String.t())
|
||||||
|
def known_member_fields do
|
||||||
|
cached = Process.get({__MODULE__, :known_member_fields})
|
||||||
|
|
||||||
|
if cached do
|
||||||
|
cached
|
||||||
|
else
|
||||||
|
fields =
|
||||||
|
@member_field_variants_raw
|
||||||
|
|> Map.keys()
|
||||||
|
|> Enum.map(fn canonical ->
|
||||||
|
# Normalize the canonical field name (e.g., :first_name -> "firstname")
|
||||||
|
canonical
|
||||||
|
|> Atom.to_string()
|
||||||
|
|> String.replace("_", "")
|
||||||
|
|> String.downcase()
|
||||||
|
end)
|
||||||
|
|> MapSet.new()
|
||||||
|
|
||||||
|
Process.put({__MODULE__, :known_member_fields}, fields)
|
||||||
|
fields
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Normalizes a CSV header string for comparison.
|
Normalizes a CSV header string for comparison.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -84,17 +84,6 @@ defmodule Mv.Membership.Import.MemberCSV do
|
||||||
@default_chunk_size 200
|
@default_chunk_size 200
|
||||||
@default_max_rows 1000
|
@default_max_rows 1000
|
||||||
|
|
||||||
# Known member field names (normalized) for efficient lookup
|
|
||||||
# These match the canonical fields in HeaderMapper
|
|
||||||
@known_member_fields [
|
|
||||||
"email",
|
|
||||||
"firstname",
|
|
||||||
"lastname",
|
|
||||||
"street",
|
|
||||||
"postalcode",
|
|
||||||
"city"
|
|
||||||
]
|
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Prepares CSV content for import by parsing, mapping headers, and validating limits.
|
Prepares CSV content for import by parsing, mapping headers, and validating limits.
|
||||||
|
|
||||||
|
|
@ -205,9 +194,9 @@ defmodule Mv.Membership.Import.MemberCSV do
|
||||||
end
|
end
|
||||||
|
|
||||||
# Checks if a normalized header matches a member field
|
# Checks if a normalized header matches a member field
|
||||||
# Uses direct lookup for better performance (avoids calling build_maps/2)
|
# Uses HeaderMapper.known_member_fields/0 as single source of truth
|
||||||
defp member_field?(normalized) when is_binary(normalized) do
|
defp member_field?(normalized) when is_binary(normalized) do
|
||||||
normalized in @known_member_fields
|
MapSet.member?(HeaderMapper.known_member_fields(), normalized)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp member_field?(_), do: false
|
defp member_field?(_), do: false
|
||||||
|
|
|
||||||
|
|
@ -52,7 +52,8 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
||||||
|
|
||||||
# CSV Import configuration constants
|
# CSV Import configuration constants
|
||||||
@max_file_size_bytes 10_485_760 # 10 MB
|
# 10 MB
|
||||||
|
@max_file_size_bytes 10_485_760
|
||||||
@max_errors 50
|
@max_errors 50
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
|
|
@ -182,6 +183,7 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
multipart={true}
|
multipart={true}
|
||||||
phx-change="validate_csv_upload"
|
phx-change="validate_csv_upload"
|
||||||
phx-submit="start_import"
|
phx-submit="start_import"
|
||||||
|
data-testid="csv-upload-form"
|
||||||
>
|
>
|
||||||
<div class="form-control">
|
<div class="form-control">
|
||||||
<label for="csv_file" class="label">
|
<label for="csv_file" class="label">
|
||||||
|
|
@ -204,13 +206,13 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
|
|
||||||
<.button
|
<.button
|
||||||
type="submit"
|
type="submit"
|
||||||
phx-click="start_import"
|
|
||||||
phx-disable-with={gettext("Starting import...")}
|
phx-disable-with={gettext("Starting import...")}
|
||||||
variant="primary"
|
variant="primary"
|
||||||
disabled={
|
disabled={
|
||||||
Enum.empty?(@uploads.csv_file.entries) or
|
Enum.empty?(@uploads.csv_file.entries) or
|
||||||
@uploads.csv_file.entries |> List.first() |> then(&(&1 && not &1.done?))
|
@uploads.csv_file.entries |> List.first() |> then(&(&1 && not &1.done?))
|
||||||
}
|
}
|
||||||
|
data-testid="start-import-button"
|
||||||
>
|
>
|
||||||
{gettext("Start Import")}
|
{gettext("Start Import")}
|
||||||
</.button>
|
</.button>
|
||||||
|
|
@ -218,9 +220,14 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
|
|
||||||
<%= if @import_status == :running or @import_status == :done do %>
|
<%= if @import_status == :running or @import_status == :done do %>
|
||||||
<%= if @import_progress do %>
|
<%= if @import_progress do %>
|
||||||
<div role="status" aria-live="polite" class="mt-4">
|
<div
|
||||||
|
role="status"
|
||||||
|
aria-live="polite"
|
||||||
|
class="mt-4"
|
||||||
|
data-testid="import-progress-container"
|
||||||
|
>
|
||||||
<%= if @import_status == :running do %>
|
<%= if @import_status == :running do %>
|
||||||
<p class="text-sm">
|
<p class="text-sm" data-testid="import-progress-text">
|
||||||
{gettext("Processing chunk %{current} of %{total}...",
|
{gettext("Processing chunk %{current} of %{total}...",
|
||||||
current: @import_progress.current_chunk,
|
current: @import_progress.current_chunk,
|
||||||
total: @import_progress.total_chunks
|
total: @import_progress.total_chunks
|
||||||
|
|
@ -229,7 +236,7 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
<% end %>
|
<% end %>
|
||||||
|
|
||||||
<%= if @import_status == :done do %>
|
<%= if @import_status == :done do %>
|
||||||
<section class="space-y-4">
|
<section class="space-y-4" data-testid="import-results-panel">
|
||||||
<h2 class="text-lg font-semibold">
|
<h2 class="text-lg font-semibold">
|
||||||
{gettext("Import Results")}
|
{gettext("Import Results")}
|
||||||
</h2>
|
</h2>
|
||||||
|
|
@ -267,14 +274,16 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
class="size-4 inline mr-1"
|
class="size-4 inline mr-1"
|
||||||
aria-hidden="true"
|
aria-hidden="true"
|
||||||
/>
|
/>
|
||||||
{gettext("Error list truncated to 50 entries")}
|
{gettext("Error list truncated to %{count} entries",
|
||||||
|
count: @max_errors
|
||||||
|
)}
|
||||||
</p>
|
</p>
|
||||||
<% end %>
|
<% end %>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<%= if length(@import_progress.errors) > 0 do %>
|
<%= if length(@import_progress.errors) > 0 do %>
|
||||||
<div>
|
<div data-testid="import-error-list">
|
||||||
<h3 class="text-sm font-semibold mb-2">
|
<h3 class="text-sm font-semibold mb-2">
|
||||||
<.icon
|
<.icon
|
||||||
name="hero-exclamation-circle"
|
name="hero-exclamation-circle"
|
||||||
|
|
@ -402,8 +411,21 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
|
|
||||||
{:noreply, socket}
|
{:noreply, socket}
|
||||||
|
|
||||||
{:error, _error} = error_result ->
|
{:error, error} ->
|
||||||
error_result
|
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
|
end
|
||||||
else
|
else
|
||||||
{:error, reason} when is_binary(reason) ->
|
{:error, reason} when is_binary(reason) ->
|
||||||
|
|
@ -524,7 +546,7 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
%{import_state: import_state, import_progress: progress}
|
%{import_state: import_state, import_progress: progress}
|
||||||
when is_map(import_state) and is_map(progress) ->
|
when is_map(import_state) and is_map(progress) ->
|
||||||
if idx >= 0 and idx < length(import_state.chunks) do
|
if idx >= 0 and idx < length(import_state.chunks) do
|
||||||
process_chunk_and_schedule_next(socket, import_state, progress, idx)
|
start_chunk_processing_task(socket, import_state, progress, idx)
|
||||||
else
|
else
|
||||||
handle_chunk_error(socket, :invalid_index, idx)
|
handle_chunk_error(socket, :invalid_index, idx)
|
||||||
end
|
end
|
||||||
|
|
@ -535,44 +557,73 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Processes a chunk and schedules the next one
|
@impl true
|
||||||
defp process_chunk_and_schedule_next(socket, import_state, progress, idx) do
|
def handle_info({:chunk_done, idx, result}, socket) do
|
||||||
chunk = Enum.at(import_state.chunks, idx)
|
case socket.assigns do
|
||||||
|
%{import_state: import_state, import_progress: progress}
|
||||||
|
when is_map(import_state) and is_map(progress) ->
|
||||||
|
handle_chunk_result(socket, import_state, progress, idx, result)
|
||||||
|
|
||||||
# Process chunk with existing error count for capping
|
_ ->
|
||||||
opts = [
|
# Missing required assigns - mark as error
|
||||||
custom_field_lookup: import_state.custom_field_lookup,
|
handle_chunk_error(socket, :missing_state, idx)
|
||||||
existing_error_count: length(progress.errors),
|
|
||||||
max_errors: @max_errors,
|
|
||||||
actor: socket.assigns[:current_user]
|
|
||||||
]
|
|
||||||
|
|
||||||
case MemberCSV.process_chunk(
|
|
||||||
chunk,
|
|
||||||
import_state.column_map,
|
|
||||||
import_state.custom_field_map,
|
|
||||||
opts
|
|
||||||
) do
|
|
||||||
{:ok, chunk_result} ->
|
|
||||||
# Merge progress
|
|
||||||
new_progress = merge_progress(progress, chunk_result, idx)
|
|
||||||
|
|
||||||
socket =
|
|
||||||
socket
|
|
||||||
|> assign(:import_progress, new_progress)
|
|
||||||
|> assign(:import_status, new_progress.status)
|
|
||||||
|
|
||||||
# Schedule next chunk or mark as done
|
|
||||||
socket = schedule_next_chunk(socket, idx, length(import_state.chunks))
|
|
||||||
|
|
||||||
{:noreply, socket}
|
|
||||||
|
|
||||||
{:error, reason} ->
|
|
||||||
# Chunk processing failed - mark as error
|
|
||||||
handle_chunk_error(socket, :processing_failed, idx, reason)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@impl true
|
||||||
|
def handle_info({:chunk_error, idx, reason}, socket) do
|
||||||
|
handle_chunk_error(socket, :processing_failed, idx, reason)
|
||||||
|
end
|
||||||
|
|
||||||
|
# Starts async task to process a chunk
|
||||||
|
defp start_chunk_processing_task(socket, import_state, progress, idx) do
|
||||||
|
chunk = Enum.at(import_state.chunks, idx)
|
||||||
|
actor = socket.assigns[:current_user]
|
||||||
|
live_view_pid = self()
|
||||||
|
|
||||||
|
# Process chunk with existing error count for capping
|
||||||
|
opts = [
|
||||||
|
custom_field_lookup: import_state.custom_field_lookup,
|
||||||
|
existing_error_count: length(progress.errors),
|
||||||
|
max_errors: @max_errors,
|
||||||
|
actor: actor
|
||||||
|
]
|
||||||
|
|
||||||
|
# Start async task to process chunk
|
||||||
|
Task.Supervisor.async_nolink(Mv.TaskSupervisor, fn ->
|
||||||
|
case MemberCSV.process_chunk(
|
||||||
|
chunk,
|
||||||
|
import_state.column_map,
|
||||||
|
import_state.custom_field_map,
|
||||||
|
opts
|
||||||
|
) do
|
||||||
|
{:ok, chunk_result} ->
|
||||||
|
send(live_view_pid, {:chunk_done, idx, chunk_result})
|
||||||
|
|
||||||
|
{:error, reason} ->
|
||||||
|
send(live_view_pid, {:chunk_error, idx, reason})
|
||||||
|
end
|
||||||
|
end)
|
||||||
|
|
||||||
|
{:noreply, socket}
|
||||||
|
end
|
||||||
|
|
||||||
|
# Handles chunk processing result from async task
|
||||||
|
defp handle_chunk_result(socket, import_state, progress, idx, chunk_result) do
|
||||||
|
# Merge progress
|
||||||
|
new_progress = merge_progress(progress, chunk_result, idx)
|
||||||
|
|
||||||
|
socket =
|
||||||
|
socket
|
||||||
|
|> assign(:import_progress, new_progress)
|
||||||
|
|> assign(:import_status, new_progress.status)
|
||||||
|
|
||||||
|
# Schedule next chunk or mark as done
|
||||||
|
socket = schedule_next_chunk(socket, idx, length(import_state.chunks))
|
||||||
|
|
||||||
|
{:noreply, socket}
|
||||||
|
end
|
||||||
|
|
||||||
# Handles chunk processing errors
|
# Handles chunk processing errors
|
||||||
defp handle_chunk_error(socket, error_type, idx, reason \\ nil) do
|
defp handle_chunk_error(socket, error_type, idx, reason \\ nil) do
|
||||||
error_message =
|
error_message =
|
||||||
|
|
@ -612,39 +663,32 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
end
|
end
|
||||||
|
|
||||||
defp consume_and_read_csv(socket) do
|
defp consume_and_read_csv(socket) do
|
||||||
result =
|
consume_uploaded_entries(socket, :csv_file, fn %{path: path}, _entry ->
|
||||||
case consume_uploaded_entries(socket, :csv_file, fn %{path: path}, _entry ->
|
case File.read(path) do
|
||||||
File.read!(path)
|
{:ok, content} -> {:ok, content}
|
||||||
end) do
|
{:error, reason} -> {:error, Exception.message(reason)}
|
||||||
[{_name, content}] when is_binary(content) ->
|
|
||||||
{:ok, content}
|
|
||||||
|
|
||||||
[] ->
|
|
||||||
{:error, gettext("No file was uploaded")}
|
|
||||||
|
|
||||||
[{_name, {:ok, content}}] when is_binary(content) ->
|
|
||||||
# Handle case where callback returns {:ok, content}
|
|
||||||
{:ok, content}
|
|
||||||
|
|
||||||
[content] when is_binary(content) ->
|
|
||||||
# Handle case where consume_uploaded_entries returns a list with the content directly
|
|
||||||
{:ok, content}
|
|
||||||
|
|
||||||
_other ->
|
|
||||||
{:error, gettext("Failed to read uploaded file")}
|
|
||||||
end
|
end
|
||||||
|
end)
|
||||||
|
|> case do
|
||||||
|
[{_name, {:ok, content}}] when is_binary(content) ->
|
||||||
|
{:ok, content}
|
||||||
|
|
||||||
result
|
[{_name, {:error, reason}}] ->
|
||||||
rescue
|
{:error, gettext("Failed to read file: %{reason}", reason: reason)}
|
||||||
e in File.Error ->
|
|
||||||
{:error, gettext("Failed to read file: %{reason}", reason: Exception.message(e))}
|
[] ->
|
||||||
|
{:error, gettext("No file was uploaded")}
|
||||||
|
|
||||||
|
_other ->
|
||||||
|
{:error, gettext("Failed to read uploaded file")}
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp merge_progress(progress, chunk_result, current_chunk_idx) do
|
defp merge_progress(progress, chunk_result, current_chunk_idx) do
|
||||||
# Merge errors with cap of 50 overall
|
# Merge errors with cap of @max_errors overall
|
||||||
all_errors = progress.errors ++ chunk_result.errors
|
all_errors = progress.errors ++ chunk_result.errors
|
||||||
new_errors = Enum.take(all_errors, 50)
|
new_errors = Enum.take(all_errors, @max_errors)
|
||||||
errors_truncated? = length(all_errors) > 50
|
errors_truncated? = length(all_errors) > @max_errors
|
||||||
|
|
||||||
# Merge warnings (optional dedupe - simple append for now)
|
# Merge warnings (optional dedupe - simple append for now)
|
||||||
new_warnings = progress.warnings ++ Map.get(chunk_result, :warnings, [])
|
new_warnings = progress.warnings ++ Map.get(chunk_result, :warnings, [])
|
||||||
|
|
|
||||||
|
|
@ -19,6 +19,26 @@ defmodule MvWeb.GlobalSettingsLiveTest do
|
||||||
|> render_upload(filename)
|
|> render_upload(filename)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Helper function to wait for import completion by checking for results panel
|
||||||
|
# Uses deterministic checks instead of Process.sleep/1
|
||||||
|
defp wait_for_import_completion(view, max_attempts \\ 10) do
|
||||||
|
Enum.reduce_while(1..max_attempts, nil, fn attempt, _acc ->
|
||||||
|
html = render(view)
|
||||||
|
|
||||||
|
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
|
||||||
|
end
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
describe "Global Settings LiveView" do
|
describe "Global Settings LiveView" do
|
||||||
setup %{conn: conn} do
|
setup %{conn: conn} do
|
||||||
user = create_test_user(%{email: "admin@example.com"})
|
user = create_test_user(%{email: "admin@example.com"})
|
||||||
|
|
@ -333,18 +353,13 @@ defmodule MvWeb.GlobalSettingsLiveTest do
|
||||||
|> form("#csv-upload-form", %{})
|
|> form("#csv-upload-form", %{})
|
||||||
|> render_submit()
|
|> render_submit()
|
||||||
|
|
||||||
# Wait for chunk processing to complete
|
# Wait for import completion deterministically
|
||||||
# Process all chunks by waiting for final state
|
html = wait_for_import_completion(view)
|
||||||
Process.sleep(500)
|
|
||||||
|
|
||||||
# Check final status
|
# Check final status using data-testid
|
||||||
html = render(view)
|
assert has_element?(view, "[data-testid='import-results-panel']")
|
||||||
# Should show done status or success message
|
# Should show success count
|
||||||
assert html =~ "done" or html =~ "complete" or html =~ "success" or
|
assert html =~ "Successfully inserted"
|
||||||
html =~ "finished" or html =~ "imported" or html =~ "Inserted"
|
|
||||||
|
|
||||||
# Should show success count > 0
|
|
||||||
assert html =~ "2" or html =~ "inserted" or html =~ "success"
|
|
||||||
end
|
end
|
||||||
|
|
||||||
test "error handling: invalid CSV shows errors with line numbers", %{
|
test "error handling: invalid CSV shows errors with line numbers", %{
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue