refactor
This commit is contained in:
parent
dd68d2efbc
commit
0acdc82bcc
7 changed files with 147 additions and 126 deletions
|
|
@ -97,31 +97,16 @@ defmodule Mv.Membership.Import.HeaderMapper do
|
||||||
}
|
}
|
||||||
|
|
||||||
# Build reverse map: normalized_variant -> canonical_field
|
# Build reverse map: normalized_variant -> canonical_field
|
||||||
# Cached on first access for performance
|
# Computed on each access - the map is small enough that recomputing is fast
|
||||||
|
# This avoids Module.get_attribute issues while maintaining simplicity
|
||||||
defp normalized_to_canonical do
|
defp normalized_to_canonical do
|
||||||
cached = Process.get({__MODULE__, :normalized_to_canonical})
|
|
||||||
|
|
||||||
if cached do
|
|
||||||
cached
|
|
||||||
else
|
|
||||||
map = build_normalized_to_canonical_map()
|
|
||||||
Process.put({__MODULE__, :normalized_to_canonical}, map)
|
|
||||||
map
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
# Builds the normalized variant -> canonical field map
|
|
||||||
defp build_normalized_to_canonical_map do
|
|
||||||
@member_field_variants_raw
|
@member_field_variants_raw
|
||||||
|> Enum.flat_map(&map_variants_to_normalized/1)
|
|> Enum.flat_map(fn {canonical, variants} ->
|
||||||
|> Map.new()
|
|
||||||
end
|
|
||||||
|
|
||||||
# Maps a canonical field and its variants to normalized tuples
|
|
||||||
defp map_variants_to_normalized({canonical, variants}) do
|
|
||||||
Enum.map(variants, fn variant ->
|
Enum.map(variants, fn variant ->
|
||||||
{normalize_header(variant), canonical}
|
{normalize_header(variant), canonical}
|
||||||
end)
|
end)
|
||||||
|
end)
|
||||||
|
|> Map.new()
|
||||||
end
|
end
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
|
|
@ -139,15 +124,8 @@ defmodule Mv.Membership.Import.HeaderMapper do
|
||||||
iex> HeaderMapper.known_member_fields()
|
iex> HeaderMapper.known_member_fields()
|
||||||
#MapSet<["email", "firstname", "lastname", "street", "postalcode", "city"]>
|
#MapSet<["email", "firstname", "lastname", "street", "postalcode", "city"]>
|
||||||
"""
|
"""
|
||||||
@spec known_member_fields() :: MapSet.t(String.t())
|
# Known member fields computed at compile-time for performance and determinism
|
||||||
def known_member_fields do
|
@known_member_fields @member_field_variants_raw
|
||||||
cached = Process.get({__MODULE__, :known_member_fields})
|
|
||||||
|
|
||||||
if cached do
|
|
||||||
cached
|
|
||||||
else
|
|
||||||
fields =
|
|
||||||
@member_field_variants_raw
|
|
||||||
|> Map.keys()
|
|> Map.keys()
|
||||||
|> Enum.map(fn canonical ->
|
|> Enum.map(fn canonical ->
|
||||||
# Normalize the canonical field name (e.g., :first_name -> "firstname")
|
# Normalize the canonical field name (e.g., :first_name -> "firstname")
|
||||||
|
|
@ -158,9 +136,9 @@ defmodule Mv.Membership.Import.HeaderMapper do
|
||||||
end)
|
end)
|
||||||
|> MapSet.new()
|
|> MapSet.new()
|
||||||
|
|
||||||
Process.put({__MODULE__, :known_member_fields}, fields)
|
@spec known_member_fields() :: MapSet.t(String.t())
|
||||||
fields
|
def known_member_fields do
|
||||||
end
|
@known_member_fields
|
||||||
end
|
end
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
|
|
|
||||||
|
|
@ -48,6 +48,7 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
alias Mv.Membership
|
alias Mv.Membership
|
||||||
alias Mv.Membership.Import.MemberCSV
|
alias Mv.Membership.Import.MemberCSV
|
||||||
alias MvWeb.Authorization
|
alias MvWeb.Authorization
|
||||||
|
alias Mv.Config
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
||||||
|
|
||||||
|
|
@ -140,14 +141,6 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
"Use the custom field name as the CSV column header (same normalization as member fields applies)"
|
"Use the custom field name as the CSV column header (same normalization as member fields applies)"
|
||||||
)}
|
)}
|
||||||
</p>
|
</p>
|
||||||
<div class="mt-2">
|
|
||||||
<.link
|
|
||||||
navigate={~p"/custom_field_values"}
|
|
||||||
class="link link-primary"
|
|
||||||
>
|
|
||||||
{gettext("Manage Custom Fields")}
|
|
||||||
</.link>
|
|
||||||
</div>
|
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|
@ -209,6 +202,7 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
phx-disable-with={gettext("Starting import...")}
|
phx-disable-with={gettext("Starting import...")}
|
||||||
variant="primary"
|
variant="primary"
|
||||||
disabled={
|
disabled={
|
||||||
|
@import_status == :running or
|
||||||
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?))
|
||||||
}
|
}
|
||||||
|
|
@ -226,7 +220,7 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
class="mt-4"
|
class="mt-4"
|
||||||
data-testid="import-progress-container"
|
data-testid="import-progress-container"
|
||||||
>
|
>
|
||||||
<%= if @import_status == :running do %>
|
<%= if @import_progress.status == :running do %>
|
||||||
<p class="text-sm" data-testid="import-progress-text">
|
<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,
|
||||||
|
|
@ -235,7 +229,7 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
</p>
|
</p>
|
||||||
<% end %>
|
<% end %>
|
||||||
|
|
||||||
<%= if @import_status == :done do %>
|
<%= if @import_progress.status == :done do %>
|
||||||
<section class="space-y-4" data-testid="import-results-panel">
|
<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")}
|
||||||
|
|
@ -372,6 +366,15 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
def handle_event("start_import", _params, socket) do
|
def handle_event("start_import", _params, socket) do
|
||||||
# Server-side admin check
|
# Server-side admin check
|
||||||
if Authorization.can?(socket.assigns[:current_user], :create, Mv.Membership.Member) do
|
if Authorization.can?(socket.assigns[:current_user], :create, Mv.Membership.Member) do
|
||||||
|
# Prevent concurrent imports
|
||||||
|
if socket.assigns.import_status == :running do
|
||||||
|
{:noreply,
|
||||||
|
put_flash(
|
||||||
|
socket,
|
||||||
|
:error,
|
||||||
|
gettext("Import is already running. Please wait for it to complete.")
|
||||||
|
)}
|
||||||
|
else
|
||||||
# Check if upload is completed
|
# Check if upload is completed
|
||||||
upload_entries = socket.assigns.uploads.csv_file.entries
|
upload_entries = socket.assigns.uploads.csv_file.entries
|
||||||
|
|
||||||
|
|
@ -398,7 +401,8 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
warnings: import_state.warnings || [],
|
warnings: import_state.warnings || [],
|
||||||
status: :running,
|
status: :running,
|
||||||
current_chunk: 0,
|
current_chunk: 0,
|
||||||
total_chunks: total_chunks
|
total_chunks: total_chunks,
|
||||||
|
errors_truncated?: false
|
||||||
}
|
}
|
||||||
|
|
||||||
socket =
|
socket =
|
||||||
|
|
@ -460,6 +464,7 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
)}
|
)}
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
end
|
||||||
else
|
else
|
||||||
{:noreply,
|
{:noreply,
|
||||||
put_flash(
|
put_flash(
|
||||||
|
|
@ -576,6 +581,7 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
end
|
end
|
||||||
|
|
||||||
# Starts async task to process a chunk
|
# Starts async task to process a chunk
|
||||||
|
# In tests (SQL sandbox mode), runs synchronously to avoid Ecto Sandbox issues
|
||||||
defp start_chunk_processing_task(socket, import_state, progress, idx) do
|
defp start_chunk_processing_task(socket, import_state, progress, idx) do
|
||||||
chunk = Enum.at(import_state.chunks, idx)
|
chunk = Enum.at(import_state.chunks, idx)
|
||||||
actor = socket.assigns[:current_user]
|
actor = socket.assigns[:current_user]
|
||||||
|
|
@ -589,21 +595,33 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
actor: actor
|
actor: actor
|
||||||
]
|
]
|
||||||
|
|
||||||
# Start async task to process chunk
|
if Config.sql_sandbox?() do
|
||||||
Task.Supervisor.async_nolink(Mv.TaskSupervisor, fn ->
|
# Run synchronously in tests to avoid Ecto Sandbox issues with async tasks
|
||||||
case MemberCSV.process_chunk(
|
{:ok, chunk_result} =
|
||||||
|
MemberCSV.process_chunk(
|
||||||
chunk,
|
chunk,
|
||||||
import_state.column_map,
|
import_state.column_map,
|
||||||
import_state.custom_field_map,
|
import_state.custom_field_map,
|
||||||
opts
|
opts
|
||||||
) do
|
)
|
||||||
{:ok, chunk_result} ->
|
|
||||||
send(live_view_pid, {:chunk_done, idx, chunk_result})
|
|
||||||
|
|
||||||
{:error, reason} ->
|
# In test mode, send the message - it will be processed when render() is called
|
||||||
send(live_view_pid, {:chunk_error, idx, reason})
|
# in the test. The test helper wait_for_import_completion() handles message processing
|
||||||
end
|
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 ->
|
||||||
|
{:ok, chunk_result} =
|
||||||
|
MemberCSV.process_chunk(
|
||||||
|
chunk,
|
||||||
|
import_state.column_map,
|
||||||
|
import_state.custom_field_map,
|
||||||
|
opts
|
||||||
|
)
|
||||||
|
|
||||||
|
send(live_view_pid, {:chunk_done, idx, chunk_result})
|
||||||
end)
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
{:noreply, socket}
|
{:noreply, socket}
|
||||||
end
|
end
|
||||||
|
|
@ -670,10 +688,10 @@ defmodule MvWeb.GlobalSettingsLive do
|
||||||
end
|
end
|
||||||
end)
|
end)
|
||||||
|> case do
|
|> case do
|
||||||
[{_name, {:ok, content}}] when is_binary(content) ->
|
[{:ok, content}] when is_binary(content) ->
|
||||||
{:ok, content}
|
{:ok, content}
|
||||||
|
|
||||||
[{_name, {:error, reason}}] ->
|
[{:error, reason}] ->
|
||||||
{:error, gettext("Failed to read file: %{reason}", reason: reason)}
|
{:error, gettext("Failed to read file: %{reason}", reason: reason)}
|
||||||
|
|
||||||
[] ->
|
[] ->
|
||||||
|
|
|
||||||
5
test/fixtures/csv_with_bom_semicolon.csv
vendored
5
test/fixtures/csv_with_bom_semicolon.csv
vendored
|
|
@ -1,3 +1,8 @@
|
||||||
first_name;last_name;email;street;postal_code;city
|
first_name;last_name;email;street;postal_code;city
|
||||||
Alice;Smith;alice.smith@example.com;Main Street 1;12345;Berlin
|
Alice;Smith;alice.smith@example.com;Main Street 1;12345;Berlin
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
5
test/fixtures/csv_with_empty_lines.csv
vendored
5
test/fixtures/csv_with_empty_lines.csv
vendored
|
|
@ -4,3 +4,8 @@ Alice;Smith;alice.smith@example.com;Main Street 1;12345;Berlin
|
||||||
Bob;Johnson;invalid-email;Park Avenue 2;54321;Munich
|
Bob;Johnson;invalid-email;Park Avenue 2;54321;Munich
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
|
@ -3,3 +3,8 @@ Alice;Smith;alice.smith@example.com;Main Street 1;12345;Berlin;SomeValue
|
||||||
Bob;Johnson;bob.johnson@example.com;Park Avenue 2;54321;Munich;AnotherValue
|
Bob;Johnson;bob.johnson@example.com;Park Avenue 2;54321;Munich;AnotherValue
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
5
test/fixtures/invalid_member_import.csv
vendored
5
test/fixtures/invalid_member_import.csv
vendored
|
|
@ -3,3 +3,8 @@ Alice;Smith;invalid-email;Main Street 1;12345;Berlin
|
||||||
Bob;Johnson;;Park Avenue 2;54321;Munich
|
Bob;Johnson;;Park Avenue 2;54321;Munich
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
5
test/fixtures/valid_member_import.csv
vendored
5
test/fixtures/valid_member_import.csv
vendored
|
|
@ -3,3 +3,8 @@ Alice;Smith;alice.smith@example.com;Main Street 1;12345;Berlin
|
||||||
Bob;Johnson;bob.johnson@example.com;Park Avenue 2;54321;Munich
|
Bob;Johnson;bob.johnson@example.com;Park Avenue 2;54321;Munich
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
Loading…
Add table
Add a link
Reference in a new issue