diff --git a/.drone.yml b/.drone.yml index 81d046f..e6770df 100644 --- a/.drone.yml +++ b/.drone.yml @@ -273,7 +273,7 @@ environment: steps: - name: renovate - image: renovate/renovate:43.25 + image: renovate/renovate:43.26 environment: RENOVATE_CONFIG_FILE: "renovate_backend_config.js" RENOVATE_TOKEN: diff --git a/lib/membership/custom_field.ex b/lib/membership/custom_field.ex index ab4ad60..411e95d 100644 --- a/lib/membership/custom_field.ex +++ b/lib/membership/custom_field.ex @@ -10,7 +10,7 @@ defmodule Mv.Membership.CustomField do ## Attributes - `name` - Unique identifier for the custom field (e.g., "phone_mobile", "birthday") - `slug` - URL-friendly, immutable identifier automatically generated from name (e.g., "phone-mobile") - - `value_type` - Data type constraint (`:string`, `:integer`, `:boolean`, `:date`, `:email`) + - `value_type` - Data type constraint (`:string`, `:integer`, `:boolean`, `:date`, `:email`). Immutable after creation. - `description` - Optional human-readable description - `required` - If true, all members must have this custom field (future feature) - `show_in_overview` - If true, this custom field will be displayed in the member overview table and can be sorted @@ -28,6 +28,7 @@ defmodule Mv.Membership.CustomField do ## Constraints - Name must be unique across all custom fields - Name maximum length: 100 characters + - `value_type` cannot be changed after creation (immutable) - Deleting a custom field will cascade delete all associated custom field values ## Calculations @@ -59,7 +60,7 @@ defmodule Mv.Membership.CustomField do end actions do - defaults [:read, :update] + defaults [:read] default_accept [:name, :value_type, :description, :required, :show_in_overview] create :create do @@ -68,6 +69,19 @@ defmodule Mv.Membership.CustomField do validate string_length(:slug, min: 1) end + update :update do + accept [:name, :description, :required, :show_in_overview] + require_atomic? false + + validate fn changeset, _context -> + if Ash.Changeset.changing_attribute?(changeset, :value_type) do + {:error, field: :value_type, message: "cannot be changed after creation"} + else + :ok + end + end + end + destroy :destroy_with_values do primary? true end diff --git a/lib/mv_web/live/components/sort_header_component.ex b/lib/mv_web/live/components/sort_header_component.ex index 3817d90..d548efa 100644 --- a/lib/mv_web/live/components/sort_header_component.ex +++ b/lib/mv_web/live/components/sort_header_component.ex @@ -26,7 +26,6 @@ defmodule MvWeb.Components.SortHeaderComponent do class="btn btn-ghost select-none" phx-click="sort" phx-value-field={@field} - phx-target={@myself} data-testid={@field} > {@label} @@ -43,12 +42,6 @@ defmodule MvWeb.Components.SortHeaderComponent do """ end - @impl true - def handle_event("sort", %{"field" => field_str}, socket) do - send(self(), {:sort, field_str}) - {:noreply, socket} - end - # ------------------------------------------------- # Hilfsfunktionen für ARIA Attribute & Icon SVG # ------------------------------------------------- diff --git a/lib/mv_web/live/custom_field_live/form_component.ex b/lib/mv_web/live/custom_field_live/form_component.ex index b809a1a..f89f767 100644 --- a/lib/mv_web/live/custom_field_live/form_component.ex +++ b/lib/mv_web/live/custom_field_live/form_component.ex @@ -5,7 +5,7 @@ defmodule MvWeb.CustomFieldLive.FormComponent do ## Features - Create new custom field definitions - Edit existing custom fields - - Select value type from supported types + - Select value type from supported types (only on create; immutable after creation) - Set required flag - Real-time validation @@ -44,15 +44,50 @@ defmodule MvWeb.CustomFieldLive.FormComponent do > <.input field={@form[:name]} type="text" label={gettext("Name")} /> - <.input - field={@form[:value_type]} - type="select" - label={gettext("Value type")} - options={ - Ash.Resource.Info.attribute(Mv.Membership.CustomField, :value_type).constraints[:one_of] - |> Enum.map(fn type -> {MvWeb.Translations.FieldTypes.label(type), type} end) - } - /> + <%= if @custom_field do %> + <%!-- Show value_type as read-only input when editing (matches Member Field pattern) --%> +
+
+ +
+
+ <% else %> + <%!-- Show value_type as select when creating --%> + <.input + field={@form[:value_type]} + type="select" + label={gettext("Value type")} + options={ + Ash.Resource.Info.attribute(Mv.Membership.CustomField, :value_type).constraints[ + :one_of + ] + |> Enum.map(fn type -> {MvWeb.Translations.FieldTypes.label(type), type} end) + } + /> + <% end %> + <.input field={@form[:description]} type="text" label={gettext("Description")} /> <.input field={@form[:required]} type="checkbox" label={gettext("Required")} /> <.input @@ -85,8 +120,16 @@ defmodule MvWeb.CustomFieldLive.FormComponent do @impl true def handle_event("validate", %{"custom_field" => custom_field_params}, socket) do + # Remove value_type from params when editing (it's immutable after creation) + cleaned_params = + if socket.assigns[:custom_field] do + Map.delete(custom_field_params, "value_type") + else + custom_field_params + end + {:noreply, - assign(socket, form: AshPhoenix.Form.validate(socket.assigns.form, custom_field_params))} + assign(socket, form: AshPhoenix.Form.validate(socket.assigns.form, cleaned_params))} end @impl true @@ -94,7 +137,15 @@ defmodule MvWeb.CustomFieldLive.FormComponent do # Actor must be passed from parent (IndexComponent); component socket has no current_user actor = socket.assigns[:actor] - case MvWeb.LiveHelpers.submit_form(socket.assigns.form, custom_field_params, actor) do + # Remove value_type from params when editing (it's immutable after creation) + cleaned_params = + if socket.assigns[:custom_field] do + Map.delete(custom_field_params, "value_type") + else + custom_field_params + end + + case MvWeb.LiveHelpers.submit_form(socket.assigns.form, cleaned_params, actor) do {:ok, custom_field} -> action = case socket.assigns.form.source.type do diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index 59ee8f9..d391cd2 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -68,18 +68,15 @@ defmodule MvWeb.MemberLive.Index do # This is appropriate for initialization errors that should be visible to the user. actor = current_actor(socket) - custom_fields_visible = - Mv.Membership.CustomField - |> Ash.Query.filter(expr(show_in_overview == true)) - |> Ash.Query.sort(name: :asc) - |> Ash.read!(actor: actor) - - # Load ALL custom fields for the dropdown (to show all available fields) all_custom_fields = Mv.Membership.CustomField |> Ash.Query.sort(name: :asc) |> Ash.read!(actor: actor) + custom_fields_visible = + all_custom_fields + |> Enum.filter(& &1.show_in_overview) + # Load boolean custom fields (filtered and sorted from all_custom_fields) boolean_custom_fields = all_custom_fields @@ -163,6 +160,7 @@ defmodule MvWeb.MemberLive.Index do - `"delete"` - Removes a member from the database - `"select_member"` - Toggles individual member selection - `"select_all"` - Toggles selection of all visible members + - `"sort"` - Sort event from SortHeaderComponent. Updates sort field/order and syncs URL """ @impl true def handle_event("delete", %{"id" => id}, socket) do @@ -305,6 +303,46 @@ defmodule MvWeb.MemberLive.Index do end end + @impl true + def handle_event("sort", %{"field" => field_str}, socket) do + # Handle both atom and string field names (for custom fields) + field = + try do + String.to_existing_atom(field_str) + rescue + ArgumentError -> field_str + end + + {new_field, new_order} = determine_new_sort(field, socket) + old_field = socket.assigns.sort_field + + socket = + socket + |> assign(:sort_field, new_field) + |> assign(:sort_order, new_order) + |> update_sort_components(old_field, new_field, new_order) + |> load_members() + |> update_selection_assigns() + + # URL sync - push_patch happens synchronously in the event handler + query_params = + build_query_params( + socket.assigns.query, + export_sort_field(socket.assigns.sort_field), + export_sort_order(socket.assigns.sort_order), + socket.assigns.cycle_status_filter, + socket.assigns[:group_filters], + socket.assigns.show_current_cycle, + socket.assigns.boolean_custom_field_filters + ) + |> maybe_add_field_selection( + socket.assigns[:user_field_selection], + socket.assigns[:fields_in_url?] || false + ) + + {:noreply, push_patch(socket, to: ~p"/members?#{query_params}", replace: true)} + end + # Helper to format errors for display defp format_error(%Ash.Error.Invalid{errors: errors}) do error_messages = @@ -329,50 +367,10 @@ defmodule MvWeb.MemberLive.Index do Handles messages from child components. ## Supported messages: - - `{:sort, field}` - Sort event from SortHeaderComponent. Updates sort field/order and syncs URL - `{:search_changed, query}` - Search event from SearchBarComponent. Filters members and syncs URL - `{:field_toggled, field, visible}` - Field toggle event from FieldVisibilityDropdownComponent - `{:fields_selected, selection}` - Select all/deselect all event from FieldVisibilityDropdownComponent """ - @impl true - def handle_info({:sort, field_str}, socket) do - # Handle both atom and string field names (for custom fields) - field = - try do - String.to_existing_atom(field_str) - rescue - ArgumentError -> field_str - end - - {new_field, new_order} = determine_new_sort(field, socket) - old_field = socket.assigns.sort_field - - socket = - socket - |> assign(:sort_field, new_field) - |> assign(:sort_order, new_order) - |> update_sort_components(old_field, new_field, new_order) - |> load_members() - |> update_selection_assigns() - - # URL sync - query_params = - build_query_params( - socket.assigns.query, - export_sort_field(socket.assigns.sort_field), - export_sort_order(socket.assigns.sort_order), - socket.assigns.cycle_status_filter, - socket.assigns[:group_filters], - socket.assigns.show_current_cycle, - socket.assigns.boolean_custom_field_filters - ) - |> maybe_add_field_selection( - socket.assigns[:user_field_selection], - socket.assigns[:fields_in_url?] || false - ) - - {:noreply, push_patch(socket, to: ~p"/members?#{query_params}", replace: true)} - end @impl true def handle_info({:search_changed, q}, socket) do diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 6dbb732..0d661cf 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -2604,6 +2604,11 @@ msgstr "PDF" msgid "Import" msgstr "Import" +#: lib/mv_web/live/custom_field_live/form_component.ex +#, elixir-autogen, elixir-format +msgid "Value type cannot be changed after creation" +msgstr "Der Wertetyp kann nach dem Erstellen nicht mehr geändert werden." + #~ #: lib/mv_web/live/import_export_live.ex #~ #, elixir-autogen, elixir-format, fuzzy #~ msgid "Export Members (CSV)" diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index df282f3..0aef1b3 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -2604,3 +2604,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Import" msgstr "" + +#: lib/mv_web/live/custom_field_live/form_component.ex +#, elixir-autogen, elixir-format +msgid "Value type cannot be changed after creation" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 56f897d..371a028 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -2605,6 +2605,11 @@ msgstr "" msgid "Import" msgstr "" +#: lib/mv_web/live/custom_field_live/form_component.ex +#, elixir-autogen, elixir-format +msgid "Value type cannot be changed after creation" +msgstr "" + #~ #: lib/mv_web/live/import_export_live.ex #~ #, elixir-autogen, elixir-format, fuzzy #~ msgid "Export Members (CSV)" diff --git a/test/membership/custom_field_validation_test.exs b/test/membership/custom_field_validation_test.exs index d0711ad..e642d82 100644 --- a/test/membership/custom_field_validation_test.exs +++ b/test/membership/custom_field_validation_test.exs @@ -8,6 +8,7 @@ defmodule Mv.Membership.CustomFieldValidationTest do - Description length validation (max 500 characters) - Description trimming - Required vs optional fields + - Value type immutability (cannot be changed after creation) """ use Mv.DataCase, async: true @@ -207,4 +208,101 @@ defmodule Mv.Membership.CustomFieldValidationTest do assert [%{field: :value_type}] = changeset.errors end end + + describe "value_type immutability" do + test "rejects attempt to change value_type after creation", %{actor: actor} do + # Create custom field with value_type :string + {:ok, custom_field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "test_field", + value_type: :string + }) + |> Ash.create(actor: actor) + + original_value_type = custom_field.value_type + assert original_value_type == :string + + # Attempt to update value_type to :integer + assert {:error, %Ash.Error.Invalid{} = error} = + custom_field + |> Ash.Changeset.for_update(:update, %{ + value_type: :integer + }) + |> Ash.update(actor: actor) + + # Verify error message contains expected text + error_message = Exception.message(error) + assert error_message =~ "cannot be changed" or error_message =~ "value_type" + + # Reload and verify value_type remained unchanged + reloaded = Ash.get!(CustomField, custom_field.id, actor: actor) + assert reloaded.value_type == original_value_type + assert reloaded.value_type == :string + end + + test "allows updating other fields while value_type remains unchanged", %{actor: actor} do + # Create custom field with value_type :string + {:ok, custom_field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "test_field", + value_type: :string, + description: "Original description" + }) + |> Ash.create(actor: actor) + + original_value_type = custom_field.value_type + assert original_value_type == :string + + # Update other fields (name, description) without touching value_type + {:ok, updated_custom_field} = + custom_field + |> Ash.Changeset.for_update(:update, %{ + name: "updated_name", + description: "Updated description" + }) + |> Ash.update(actor: actor) + + # Verify value_type remained unchanged + assert updated_custom_field.value_type == original_value_type + assert updated_custom_field.value_type == :string + # Verify other fields were updated + assert updated_custom_field.name == "updated_name" + assert updated_custom_field.description == "Updated description" + end + + test "rejects value_type change even when other fields are updated", %{actor: actor} do + # Create custom field with value_type :boolean + {:ok, custom_field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "test_field", + value_type: :boolean + }) + |> Ash.create(actor: actor) + + original_value_type = custom_field.value_type + assert original_value_type == :boolean + + # Attempt to update both name and value_type + assert {:error, %Ash.Error.Invalid{} = error} = + custom_field + |> Ash.Changeset.for_update(:update, %{ + name: "updated_name", + value_type: :date + }) + |> Ash.update(actor: actor) + + # Verify error message + error_message = Exception.message(error) + assert error_message =~ "cannot be changed" or error_message =~ "value_type" + + # Reload and verify value_type remained unchanged, but name was not updated either + reloaded = Ash.get!(CustomField, custom_field.id, actor: actor) + assert reloaded.value_type == original_value_type + assert reloaded.value_type == :boolean + assert reloaded.name == "test_field" + end + end end diff --git a/test/mv_web/components/sort_header_component_test.exs b/test/mv_web/components/sort_header_component_test.exs index 6d23ab4..bdde4ae 100644 --- a/test/mv_web/components/sort_header_component_test.exs +++ b/test/mv_web/components/sort_header_component_test.exs @@ -223,7 +223,7 @@ defmodule MvWeb.Components.SortHeaderComponentTest do end describe "component behavior" do - test "clicking sends sort message to parent", %{conn: conn} do + test "clicking triggers sort event on parent LiveView", %{conn: conn} do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") @@ -232,7 +232,7 @@ defmodule MvWeb.Components.SortHeaderComponentTest do |> element("button[phx-value-field='first_name']") |> render_click() - # The component should send a message to the parent LiveView + # The component triggers a "sort" event on the parent LiveView # This is tested indirectly through the URL change in integration tests end diff --git a/test/mv_web/member_live/index_test.exs b/test/mv_web/member_live/index_test.exs index 4f36795..53a2815 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -1,5 +1,5 @@ defmodule MvWeb.MemberLive.IndexTest do - use MvWeb.ConnCase, async: true + use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query