Polishs import UI closes #337 #398

Merged
carla merged 8 commits from feature/337_polish_import into main 2026-02-04 16:50:45 +01:00
6 changed files with 127 additions and 391 deletions
Showing only changes of commit d34ff57531 - Show all commits

View file

@ -45,6 +45,9 @@ defmodule MvWeb.ImportExportLive do
# after this limit is reached.
@max_errors 50
# Maximum length for error messages before truncation
@max_error_message_length 200
@impl true
def mount(_params, session, socket) do
# Get locale from session for translations
@ -95,11 +98,11 @@ defmodule MvWeb.ImportExportLive do
<%= if Authorization.can?(@current_user, :create, Mv.Membership.Member) do %>
<%!-- CSV Import Section --%>
<.form_section title={gettext("Import Members (CSV)")}>
<%= import_info_box(assigns) %>
<%= template_links(assigns) %>
<%= import_form(assigns) %>
{import_info_box(assigns)}
{template_links(assigns)}
{import_form(assigns)}
<%= if @import_status == :running or @import_status == :done do %>
<%= import_progress(assigns) %>
{import_progress(assigns)}
<% end %>
</.form_section>
@ -243,7 +246,7 @@ defmodule MvWeb.ImportExportLive do
<% end %>
<%= if @import_progress.status == :done do %>
<%= import_results(assigns) %>
{import_results(assigns)}
<% end %>
</div>
<% end %>
@ -487,9 +490,7 @@ defmodule MvWeb.ImportExportLive do
# Formats Ash validation errors for display
defp format_ash_error(%Ash.Error.Invalid{errors: errors}) when is_list(errors) do
errors
|> Enum.map(&format_single_error/1)
|> Enum.join(", ")
Enum.map_join(errors, ", ", &format_single_error/1)
end
defp format_ash_error(error) do
@ -498,9 +499,7 @@ defmodule MvWeb.ImportExportLive do
# Formats a list of errors into a readable string
defp format_error_list(errors) do
errors
|> Enum.map(&format_single_error/1)
|> Enum.join(", ")
Enum.map_join(errors, ", ", &format_single_error/1)
end
# Formats a single error item
@ -516,8 +515,8 @@ defmodule MvWeb.ImportExportLive do
defp format_unknown_error(other) do
error_str = inspect(other, limit: :infinity, pretty: true)
if String.length(error_str) > 200 do
String.slice(error_str, 0, 197) <> "..."
if String.length(error_str) > @max_error_message_length do
String.slice(error_str, 0, @max_error_message_length - 3) <> "..."
else
error_str
end
@ -558,6 +557,49 @@ defmodule MvWeb.ImportExportLive do
handle_chunk_error(socket, :processing_failed, idx, reason)
end
# Processes a chunk with error handling and sends result message to LiveView.
#
# Handles errors from MemberCSV.process_chunk and sends appropriate messages
# to the LiveView process for progress tracking.
@spec process_chunk_with_error_handling(
list(),
map(),
map(),
keyword(),
pid(),
non_neg_integer()
) :: :ok
defp process_chunk_with_error_handling(
chunk,
column_map,
custom_field_map,
opts,
live_view_pid,
idx
) do
result =
try do
MemberCSV.process_chunk(chunk, column_map, custom_field_map, opts)
rescue
e ->
{:error, Exception.message(e)}
catch
:exit, reason ->
{:error, inspect(reason)}
:throw, reason ->
{:error, inspect(reason)}
end
case result 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
# Starts async task to process a chunk of CSV rows.
#
# In tests (SQL sandbox mode), runs synchronously to avoid Ecto Sandbox issues.
@ -586,33 +628,16 @@ defmodule MvWeb.ImportExportLive do
if Config.sql_sandbox?() do
# Run synchronously in tests to avoid Ecto Sandbox issues with async tasks
result =
try do
MemberCSV.process_chunk(
chunk,
import_state.column_map,
import_state.custom_field_map,
opts
)
rescue
e ->
{:error, Exception.message(e)}
catch
:exit, reason ->
{:error, inspect(reason)}
:throw, reason ->
{:error, inspect(reason)}
end
case result do
{:ok, chunk_result} ->
# In test mode, send the message - it will be processed when render() is called
# in the test. The test helper wait_for_import_completion() handles message processing
send(live_view_pid, {:chunk_done, idx, chunk_result})
{:error, reason} ->
send(live_view_pid, {:chunk_error, idx, reason})
end
# In test mode, send the message - it will be processed when render() is called
# in the test. The test helper wait_for_import_completion() handles message processing
process_chunk_with_error_handling(
chunk,
import_state.column_map,
import_state.custom_field_map,
opts,
live_view_pid,
idx
)
else
# Start async task to process chunk in production
# Use start_child for fire-and-forget: no monitor, no Task messages
@ -621,31 +646,14 @@ defmodule MvWeb.ImportExportLive do
# Set locale in task process for translations
Gettext.put_locale(MvWeb.Gettext, locale)
result =
try do
MemberCSV.process_chunk(
chunk,
import_state.column_map,
import_state.custom_field_map,
opts
)
rescue
e ->
{:error, Exception.message(e)}
catch
:exit, reason ->
{:error, inspect(reason)}
:throw, reason ->
{:error, inspect(reason)}
end
case result do
{:ok, chunk_result} ->
send(live_view_pid, {:chunk_done, idx, chunk_result})
{:error, reason} ->
send(live_view_pid, {:chunk_error, idx, reason})
end
process_chunk_with_error_handling(
chunk,
import_state.column_map,
import_state.custom_field_map,
opts,
live_view_pid,
idx
)
end)
end
@ -712,8 +720,14 @@ defmodule MvWeb.ImportExportLive do
@spec consume_and_read_csv(Phoenix.LiveView.Socket.t()) ::
{:ok, String.t()} | {:error, String.t()}
defp consume_and_read_csv(socket) do
case consume_uploaded_entries(socket, :csv_file, &read_file_entry/2) do
[{:ok, content}] ->
raw = consume_uploaded_entries(socket, :csv_file, &read_file_entry/2)
case raw do
[{:ok, content}] when is_binary(content) ->
{:ok, content}
# Phoenix LiveView test (render_upload) can return raw content list when callback return is treated as value
[content] when is_binary(content) ->
{:ok, content}
[{:error, reason}] ->

View file

@ -41,18 +41,6 @@ defmodule Mv.Accounts.UserAuthenticationTest do
assert is_nil(found_user.oidc_id)
end
@tag :test_proposal
test "password authentication uses email as identity_field" do
# Verify the configuration: password strategy should use email as identity_field
# This test checks the AshAuthentication configuration
strategies = AshAuthentication.Info.authentication_strategies(Mv.Accounts.User)
password_strategy = Enum.find(strategies, fn s -> s.name == :password end)
assert password_strategy != nil
assert password_strategy.identity_field == :email
end
@tag :test_proposal
test "multiple users can exist with different emails" do
user1 =

View file

@ -1,13 +1,14 @@
defmodule Mv.Membership.CustomFieldSlugTest do
@moduledoc """
Tests for automatic slug generation on CustomField resource.
Tests for CustomField slug business rules only.
This test suite verifies:
1. Slugs are automatically generated from the name attribute
2. Slugs are unique (cannot have duplicates)
3. Slugs are immutable (don't change when name changes)
4. Slugs handle various edge cases (unicode, special chars, etc.)
5. Slugs can be used for lookups
We test our business logic, not Ash/slugify implementation details:
- Slug is generated from name on create (one smoke test)
- Slug is unique (business rule)
- Slug is immutable (does not change when name is updated; cannot be set manually)
- Slug cannot be empty (rejects name with only special characters)
We do not test: slugify edge cases (umlauts, truncation, etc.) or Ash/Ecto struct/load behavior.
"""
use Mv.DataCase, async: true
@ -18,8 +19,8 @@ defmodule Mv.Membership.CustomFieldSlugTest do
%{actor: system_actor}
end
describe "automatic slug generation on create" do
test "generates slug from name with simple ASCII text", %{actor: actor} do
describe "slug generation (business rule)" do
test "slug is generated from name on create", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
@ -30,78 +31,6 @@ defmodule Mv.Membership.CustomFieldSlugTest do
assert custom_field.slug == "mobile-phone"
end
test "generates slug from name with German umlauts", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "Café Müller",
value_type: :string
})
|> Ash.create(actor: actor)
assert custom_field.slug == "cafe-muller"
end
test "generates slug with lowercase conversion", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "TEST NAME",
value_type: :string
})
|> Ash.create(actor: actor)
assert custom_field.slug == "test-name"
end
test "generates slug by removing special characters", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "E-Mail & Address!",
value_type: :string
})
|> Ash.create(actor: actor)
assert custom_field.slug == "e-mail-address"
end
test "generates slug by replacing multiple spaces with single hyphen", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "Multiple Spaces",
value_type: :string
})
|> Ash.create(actor: actor)
assert custom_field.slug == "multiple-spaces"
end
test "trims leading and trailing hyphens", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "-Test-",
value_type: :string
})
|> Ash.create(actor: actor)
assert custom_field.slug == "test"
end
test "handles unicode characters properly (ß becomes ss)", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "Straße",
value_type: :string
})
|> Ash.create(actor: actor)
assert custom_field.slug == "strasse"
end
end
describe "slug uniqueness" do
@ -248,29 +177,8 @@ defmodule Mv.Membership.CustomFieldSlugTest do
end
end
describe "slug edge cases" do
test "handles very long names by truncating slug", %{actor: actor} do
# Create a name at the maximum length (100 chars)
long_name = String.duplicate("abcdefghij", 10)
# 100 characters exactly
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: long_name,
value_type: :string
})
|> Ash.create(actor: actor)
# Slug should be truncated to maximum 100 characters
assert String.length(custom_field.slug) <= 100
# Should be the full slugified version since name is exactly 100 chars
assert custom_field.slug == long_name
end
describe "slug cannot be empty (business rule)" do
test "rejects name with only special characters", %{actor: actor} do
# When name contains only special characters, slug would be empty
# This should fail validation
assert {:error, %Ash.Error.Invalid{} = error} =
CustomField
|> Ash.Changeset.for_create(:create, %{
@ -279,107 +187,9 @@ defmodule Mv.Membership.CustomFieldSlugTest do
})
|> Ash.create(actor: actor)
# Should fail because slug would be empty
error_message = Exception.message(error)
assert error_message =~ "Slug cannot be empty" or error_message =~ "is required"
end
test "handles mixed special characters and text", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "Test@#$%Name",
value_type: :string
})
|> Ash.create(actor: actor)
# slugify keeps the hyphen between words
assert custom_field.slug == "test-name"
end
test "handles numbers in name", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "Field 123 Test",
value_type: :string
})
|> Ash.create(actor: actor)
assert custom_field.slug == "field-123-test"
end
test "handles consecutive hyphens in name", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "Test---Name",
value_type: :string
})
|> Ash.create(actor: actor)
# Should reduce multiple hyphens to single hyphen
assert custom_field.slug == "test-name"
end
test "handles name with dots and underscores", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "test.field_name",
value_type: :string
})
|> Ash.create(actor: actor)
# Dots and underscores should be handled (either kept or converted)
assert custom_field.slug =~ ~r/^[a-z0-9-]+$/
end
end
describe "slug in queries and responses" do
test "slug is included in struct after create", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "Test",
value_type: :string
})
|> Ash.create(actor: actor)
# Slug should be present in the struct
assert Map.has_key?(custom_field, :slug)
assert custom_field.slug != nil
end
test "can load custom field and slug is present", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "Test",
value_type: :string
})
|> Ash.create(actor: actor)
# Load it back
loaded_custom_field = Ash.get!(CustomField, custom_field.id, actor: actor)
assert loaded_custom_field.slug == "test"
end
test "slug is returned in list queries", %{actor: actor} do
{:ok, custom_field} =
CustomField
|> Ash.Changeset.for_create(:create, %{
name: "Test",
value_type: :string
})
|> Ash.create(actor: actor)
custom_fields = Ash.read!(CustomField, actor: actor)
found = Enum.find(custom_fields, &(&1.id == custom_field.id))
assert found.slug == "test"
end
end
describe "slug-based lookup (future feature)" do

View file

@ -232,23 +232,7 @@ defmodule Mv.Membership.GroupTest do
end
describe "Relationships & Deletion" do
test "group has many_to_many members relationship (load with preloading)", %{actor: actor} do
{:ok, group} = Membership.create_group(%{name: "Test Group"}, actor: actor)
{:ok, member} = Membership.create_member(%{email: "test@test.com"}, actor: actor)
{:ok, _mg} =
Membership.create_member_group(%{member_id: member.id, group_id: group.id},
actor: actor
)
# Load group with members
{:ok, group_with_members} =
Ash.load(group, :members, actor: actor, domain: Mv.Membership)
assert length(group_with_members.members) == 1
assert hd(group_with_members.members).id == member.id
end
# We test business/data rules (CASCADE), not Ash relationship loading (framework).
test "delete group cascades to member_groups (members remain intact)", %{actor: actor} do
{:ok, group} = Membership.create_group(%{name: "Test Group"}, actor: actor)
{:ok, member} = Membership.create_member(%{email: "test@test.com"}, actor: actor)

View file

@ -1,6 +1,10 @@
defmodule Mv.MembershipFees.MembershipFeeTypeTest do
@moduledoc """
Tests for MembershipFeeType resource.
Tests for MembershipFeeType business rules only.
We test: required fields, allowed interval values, uniqueness, amount constraints,
interval immutability, and referential integrity (cannot delete when in use).
We do not test: standard CRUD (create/update/delete when no constraints apply).
"""
use Mv.DataCase, async: true
@ -11,34 +15,7 @@ defmodule Mv.MembershipFees.MembershipFeeTypeTest do
%{actor: system_actor}
end
describe "create MembershipFeeType" do
test "can create membership fee type with valid attributes", %{actor: actor} do
attrs = %{
name: "Standard Membership",
amount: Decimal.new("120.00"),
interval: :yearly,
description: "Standard yearly membership fee"
}
assert {:ok, %MembershipFeeType{} = fee_type} =
Ash.create(MembershipFeeType, attrs, actor: actor)
assert fee_type.name == "Standard Membership"
assert Decimal.equal?(fee_type.amount, Decimal.new("120.00"))
assert fee_type.interval == :yearly
assert fee_type.description == "Standard yearly membership fee"
end
test "can create membership fee type without description", %{actor: actor} do
attrs = %{
name: "Basic",
amount: Decimal.new("60.00"),
interval: :monthly
}
assert {:ok, %MembershipFeeType{}} = Ash.create(MembershipFeeType, attrs, actor: actor)
end
describe "create MembershipFeeType - business rules" do
test "requires name", %{actor: actor} do
attrs = %{
amount: Decimal.new("100.00"),
@ -69,28 +46,24 @@ defmodule Mv.MembershipFees.MembershipFeeTypeTest do
assert error_on_field?(error, :interval)
end
test "validates interval enum values - monthly", %{actor: actor} do
attrs = %{name: "Monthly", amount: Decimal.new("10.00"), interval: :monthly}
assert {:ok, fee_type} = Ash.create(MembershipFeeType, attrs, actor: actor)
assert fee_type.interval == :monthly
end
test "accepts valid interval values (monthly, quarterly, half_yearly, yearly)", %{
actor: actor
} do
for {interval, name} <- [
monthly: "Monthly",
quarterly: "Quarterly",
half_yearly: "Half Yearly",
yearly: "Yearly"
] do
attrs = %{
name: "#{name} #{System.unique_integer([:positive])}",
amount: Decimal.new("10.00"),
interval: interval
}
test "validates interval enum values - quarterly", %{actor: actor} do
attrs = %{name: "Quarterly", amount: Decimal.new("30.00"), interval: :quarterly}
assert {:ok, fee_type} = Ash.create(MembershipFeeType, attrs, actor: actor)
assert fee_type.interval == :quarterly
end
test "validates interval enum values - half_yearly", %{actor: actor} do
attrs = %{name: "Half Yearly", amount: Decimal.new("60.00"), interval: :half_yearly}
assert {:ok, fee_type} = Ash.create(MembershipFeeType, attrs, actor: actor)
assert fee_type.interval == :half_yearly
end
test "validates interval enum values - yearly", %{actor: actor} do
attrs = %{name: "Yearly", amount: Decimal.new("120.00"), interval: :yearly}
assert {:ok, fee_type} = Ash.create(MembershipFeeType, attrs, actor: actor)
assert fee_type.interval == :yearly
assert {:ok, fee_type} = Ash.create(MembershipFeeType, attrs, actor: actor)
assert fee_type.interval == interval
end
end
test "rejects invalid interval values", %{actor: actor} do
@ -128,13 +101,13 @@ defmodule Mv.MembershipFees.MembershipFeeTypeTest do
end
end
describe "update MembershipFeeType" do
describe "update MembershipFeeType - business rules" do
setup %{actor: actor} do
{:ok, fee_type} =
Ash.create(
MembershipFeeType,
%{
name: "Original Name",
name: "Original Name #{System.unique_integer([:positive])}",
amount: Decimal.new("100.00"),
interval: :yearly,
description: "Original description"
@ -145,28 +118,6 @@ defmodule Mv.MembershipFees.MembershipFeeTypeTest do
%{fee_type: fee_type}
end
test "can update name", %{actor: actor, fee_type: fee_type} do
assert {:ok, updated} = Ash.update(fee_type, %{name: "Updated Name"}, actor: actor)
assert updated.name == "Updated Name"
end
test "can update amount", %{actor: actor, fee_type: fee_type} do
assert {:ok, updated} = Ash.update(fee_type, %{amount: Decimal.new("150.00")}, actor: actor)
assert Decimal.equal?(updated.amount, Decimal.new("150.00"))
end
test "can update description", %{actor: actor, fee_type: fee_type} do
assert {:ok, updated} =
Ash.update(fee_type, %{description: "Updated description"}, actor: actor)
assert updated.description == "Updated description"
end
test "can clear description", %{actor: actor, fee_type: fee_type} do
assert {:ok, updated} = Ash.update(fee_type, %{description: nil}, actor: actor)
assert updated.description == nil
end
test "interval immutability: update fails when interval is changed", %{
actor: actor,
fee_type: fee_type
@ -179,7 +130,7 @@ defmodule Mv.MembershipFees.MembershipFeeTypeTest do
end
end
describe "delete MembershipFeeType" do
describe "delete MembershipFeeType - business rules (referential integrity)" do
setup %{actor: actor} do
{:ok, fee_type} =
Ash.create(
@ -195,12 +146,6 @@ defmodule Mv.MembershipFees.MembershipFeeTypeTest do
%{fee_type: fee_type}
end
test "can delete when not in use", %{actor: actor, fee_type: fee_type} do
result = Ash.destroy(fee_type, actor: actor)
# Ash.destroy returns :ok or {:ok, _} depending on version
assert result == :ok or match?({:ok, _}, result)
end
test "cannot delete when members are assigned", %{actor: actor, fee_type: fee_type} do
alias Mv.Membership.Member

View file

@ -380,18 +380,14 @@ defmodule MvWeb.ImportExportLiveTest do
|> form("#csv-upload-form", %{})
|> render_submit()
# Wait a bit for processing to start
Process.sleep(200)
# In test mode chunks run synchronously, so we may already be :done when we check.
# Accept either progress container (if we caught :running) or results panel (if already :done).
_html = render(view)
# Check that import-progress-container exists (with aria-live for accessibility)
assert has_element?(view, "[data-testid='import-progress-container']")
assert has_element?(view, "[data-testid='import-progress-container']") or
has_element?(view, "[data-testid='import-results-panel']")
# Check that progress text is shown when running
html = render(view)
assert has_element?(view, "[data-testid='import-progress-text']") or
html =~ "Processing chunk"
# Final state should show import-results-panel
# Wait for final state and assert results panel is shown
Process.sleep(500)
assert has_element?(view, "[data-testid='import-results-panel']")
end
@ -552,9 +548,8 @@ defmodule MvWeb.ImportExportLiveTest do
assert html =~ "English Template" or html =~ "German Template" or
html =~ "English" or html =~ "German"
# Custom Fields section should have descriptive text (Data Field button)
# The component uses "New Data Field" button, not a link
assert html =~ "Data Field" or html =~ "New Data Field" or html =~ "Manage Memberdata"
# Import page has link "Manage Member Data" and info text about "data field"
assert html =~ "Manage Member Data" or html =~ "data field" or html =~ "Data field"
end
end