Merge pull request 'Implements uneditable type for custom fields closes #198' (#433) from feature/198_edit_custom_fields into main

Reviewed-on: #433

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

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
   # -------------------------------------------------

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
+          <%= if @custom_field do %>
-            field={@form[:value_type]}
+            <%!-- Show value_type as read-only input when editing (matches Member Field pattern) --%>
-            type="select"
+            <div
-            label={gettext("Value type")}
+              class="tooltip tooltip-right"
-            options={
+              data-tip={gettext("Value type cannot be changed after creation")}
-              Ash.Resource.Info.attribute(Mv.Membership.CustomField, :value_type).constraints[:one_of]
+              aria-label={gettext("Value type cannot be changed after creation")}
-              |> Enum.map(fn type -> {MvWeb.Translations.FieldTypes.label(type), type} end)
+            >
-            }
+              <fieldset class="mb-2 fieldset">
-          />
+                <label>
+                  <span class="mb-1 label flex items-center gap-2">
+                    {gettext("Value type")}
+                    <.icon
+                      name="hero-information-circle"
+                      class="w-4 h-4 text-base-content/60 cursor-help"
+                      aria-hidden="true"
+                    />
+                  </span>
+                  <input
+                    type="text"
+                    name={@form[:value_type].name}
+                    id={@form[:value_type].id}
+                    value={MvWeb.Translations.FieldTypes.label(@custom_field.value_type)}
+                    disabled
+                    readonly
+                    class="w-full input"
+                  />
+                </label>
+              </fieldset>
+            </div>
+          <% 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

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

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)"

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 ""

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)"

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

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
 

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