From ef70dd293590f2221f6a8d7eade4b49390b013c3 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 15:13:27 +0200 Subject: [PATCH] refactor(settings): unify JSONB single-field update between member-field changes --- .../setting/changes/jsonb_result.ex | 98 +++++++++++++++++++ .../changes/update_single_member_field.ex | 71 +++----------- .../update_single_member_field_visibility.ex | 68 +++---------- 3 files changed, 124 insertions(+), 113 deletions(-) create mode 100644 lib/membership/setting/changes/jsonb_result.ex diff --git a/lib/membership/setting/changes/jsonb_result.ex b/lib/membership/setting/changes/jsonb_result.ex new file mode 100644 index 0000000..183efed --- /dev/null +++ b/lib/membership/setting/changes/jsonb_result.ex @@ -0,0 +1,98 @@ +defmodule Mv.Membership.Setting.Changes.JsonbResult do + @moduledoc """ + Shared normalization for the JSONB column values returned by the atomic + single-member-field settings updates. + + PostgreSQL may return a JSONB column as an already-decoded map (atom or string + keys) or as a raw JSON string depending on the driver path. This helper + normalizes either form to a string-keyed map, returning `%{}` for unexpected + or undecodable input. + """ + + alias Ash.Error.Invalid + alias Ecto.Adapters.SQL + require Logger + + @doc """ + Runs an atomic single-statement JSONB settings UPDATE inside an after_action + and maps the RETURNING row back onto the settings struct. + + Shared by the single-member-field change modules, which differ only in the + SQL statement, its parameters, the row-to-settings mapping, and the error + labels. The three-branch result handling (row found / not found / SQL error) + is identical and lives here. + + Options: + - `:sql` - The UPDATE statement with a RETURNING clause (required). + - `:params` - The full parameter list for the statement (required). + - `:on_row` - 1-arity function mapping the RETURNING row (a list of column + values) to the updated settings struct (required). + - `:error_field` - Ash error field for the not-found / failure errors. + - `:not_found_message` - Error message when no row matched. + - `:error_message` - Error message when the SQL statement failed. + - `:log_message` - Log prefix written on SQL failure. + """ + @spec run_update(keyword()) :: + {:ok, map()} | {:error, Exception.t()} + def run_update(opts) do + sql = Keyword.fetch!(opts, :sql) + params = Keyword.fetch!(opts, :params) + on_row = Keyword.fetch!(opts, :on_row) + error_field = Keyword.fetch!(opts, :error_field) + not_found_message = Keyword.fetch!(opts, :not_found_message) + error_message = Keyword.fetch!(opts, :error_message) + log_message = Keyword.fetch!(opts, :log_message) + + case SQL.query(Mv.Repo, sql, params) do + {:ok, %{rows: [row | _]}} -> + {:ok, on_row.(row)} + + {:ok, %{rows: []}} -> + {:error, + Invalid.exception( + field: error_field, + message: not_found_message + )} + + {:error, error} -> + Logger.error("#{log_message}: #{inspect(error)}") + + {:error, + Invalid.exception( + field: error_field, + message: error_message + )} + end + end + + @doc """ + Normalizes a JSONB column value to a string-keyed map. + """ + @spec normalize(map() | binary() | any()) :: map() + def normalize(updated_jsonb) do + case updated_jsonb do + map when is_map(map) -> + Enum.reduce(map, %{}, fn + {k, v}, acc when is_atom(k) -> Map.put(acc, Atom.to_string(k), v) + {k, v}, acc -> Map.put(acc, k, v) + end) + + binary when is_binary(binary) -> + case Jason.decode(binary) do + {:ok, decoded} when is_map(decoded) -> + decoded + + {:ok, _} -> + %{} + + {:error, reason} -> + Logger.warning("Failed to decode JSONB: #{inspect(reason)}") + %{} + end + + _ -> + Logger.warning("Unexpected JSONB format: #{inspect(updated_jsonb)}") + %{} + end + end +end diff --git a/lib/membership/setting/changes/update_single_member_field.ex b/lib/membership/setting/changes/update_single_member_field.ex index e24860c..cb3ce7b 100644 --- a/lib/membership/setting/changes/update_single_member_field.ex +++ b/lib/membership/setting/changes/update_single_member_field.ex @@ -19,9 +19,7 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberField do """ use Ash.Resource.Change - alias Ash.Error.Invalid - alias Ecto.Adapters.SQL - require Logger + alias Mv.Membership.Setting.Changes.JsonbResult def change(changeset, _opts, _context) do with {:ok, field} <- get_and_validate_field(changeset), @@ -118,62 +116,21 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberField do uuid_binary = Ecto.UUID.dump!(settings.id) - case SQL.query(Mv.Repo, sql, [field, show_in_overview, required, uuid_binary]) do - {:ok, %{rows: [[updated_visibility, updated_required] | _]}} -> - vis = normalize_jsonb_result(updated_visibility) - req = normalize_jsonb_result(updated_required) - - updated_settings = %{ + JsonbResult.run_update( + sql: sql, + params: [field, show_in_overview, required, uuid_binary], + on_row: fn [updated_visibility, updated_required | _] -> + %{ settings - | member_field_visibility: vis, - member_field_required: req + | member_field_visibility: JsonbResult.normalize(updated_visibility), + member_field_required: JsonbResult.normalize(updated_required) } - - {:ok, updated_settings} - - {:ok, %{rows: []}} -> - {:error, - Invalid.exception( - field: :member_field_required, - message: "Settings not found" - )} - - {:error, error} -> - Logger.error("Failed to atomically update member field settings: #{inspect(error)}") - - {:error, - Invalid.exception( - field: :member_field_required, - message: "Failed to update member field settings" - )} - end + end, + error_field: :member_field_required, + not_found_message: "Settings not found", + error_message: "Failed to update member field settings", + log_message: "Failed to atomically update member field settings" + ) end) end - - defp normalize_jsonb_result(updated_jsonb) do - case updated_jsonb do - map when is_map(map) -> - Enum.reduce(map, %{}, fn - {k, v}, acc when is_atom(k) -> Map.put(acc, Atom.to_string(k), v) - {k, v}, acc -> Map.put(acc, k, v) - end) - - binary when is_binary(binary) -> - case Jason.decode(binary) do - {:ok, decoded} when is_map(decoded) -> - decoded - - {:ok, _} -> - %{} - - {:error, reason} -> - Logger.warning("Failed to decode JSONB: #{inspect(reason)}") - %{} - end - - _ -> - Logger.warning("Unexpected JSONB format: #{inspect(updated_jsonb)}") - %{} - end - end end diff --git a/lib/membership/setting/changes/update_single_member_field_visibility.ex b/lib/membership/setting/changes/update_single_member_field_visibility.ex index e047cdf..f08bc21 100644 --- a/lib/membership/setting/changes/update_single_member_field_visibility.ex +++ b/lib/membership/setting/changes/update_single_member_field_visibility.ex @@ -19,9 +19,7 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberFieldVisibility do """ use Ash.Resource.Change - alias Ash.Error.Invalid - alias Ecto.Adapters.SQL - require Logger + alias Mv.Membership.Setting.Changes.JsonbResult def change(changeset, _opts, _context) do with {:ok, field} <- get_and_validate_field(changeset), @@ -106,59 +104,17 @@ defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberFieldVisibility do # Convert UUID string to binary for PostgreSQL uuid_binary = Ecto.UUID.dump!(settings.id) - case SQL.query(Mv.Repo, sql, [field, show_in_overview, uuid_binary]) do - {:ok, %{rows: [[updated_jsonb] | _]}} -> - updated_visibility = normalize_jsonb_result(updated_jsonb) - - # Update the settings struct with the new visibility - updated_settings = %{settings | member_field_visibility: updated_visibility} - {:ok, updated_settings} - - {:ok, %{rows: []}} -> - {:error, - Invalid.exception( - field: :member_field_visibility, - message: "Settings not found" - )} - - {:error, error} -> - Logger.error("Failed to atomically update member_field_visibility: #{inspect(error)}") - - {:error, - Invalid.exception( - field: :member_field_visibility, - message: "Failed to update visibility" - )} - end + JsonbResult.run_update( + sql: sql, + params: [field, show_in_overview, uuid_binary], + on_row: fn [updated_jsonb | _] -> + %{settings | member_field_visibility: JsonbResult.normalize(updated_jsonb)} + end, + error_field: :member_field_visibility, + not_found_message: "Settings not found", + error_message: "Failed to update visibility", + log_message: "Failed to atomically update member_field_visibility" + ) end) end - - defp normalize_jsonb_result(updated_jsonb) do - case updated_jsonb do - map when is_map(map) -> - # Convert atom keys to strings if needed - Enum.reduce(map, %{}, fn - {k, v}, acc when is_atom(k) -> Map.put(acc, Atom.to_string(k), v) - {k, v}, acc -> Map.put(acc, k, v) - end) - - binary when is_binary(binary) -> - case Jason.decode(binary) do - {:ok, decoded} when is_map(decoded) -> - decoded - - # Not a map after decode - {:ok, _} -> - %{} - - {:error, reason} -> - Logger.warning("Failed to decode JSONB: #{inspect(reason)}") - %{} - end - - _ -> - Logger.warning("Unexpected JSONB format: #{inspect(updated_jsonb)}") - %{} - end - end end