chore(balance): make balance error an embedded schema (#352)

apps/app/lib/app/balance.ex

@@ -7,48 +7,42 @@ defmodule App.Balance do
 
   alias App.Repo
 
+  alias App.Balance.BalanceError
   alias App.Books.BookMember
   alias App.Transfers
   alias App.Transfers.Peer
 
-  @type error_reasons :: [%{uniq_hash: String.t(), kind: atom(), extra: map()}]
-
   @doc """
   Compute the `:balance` field of book members.
   """
   def fill_members_balance(members) do
     transfers =
       members
       |> Transfers.list_transfers_of_members()
-      |> preload_peers(members)
+      # Ensure the peers are always returned in the same order,
+      # a different order would result in a different repartition
+      # of the transfers amount.
+      |> Repo.preload(peers: order_by(Peer, asc: :id))
       |> compute_total_peer_weight()
 
     members
     |> reset_members_balance()
     |> adjust_balance_from_transfers(transfers)
+    |> fill_members_balance_errors()
   end
 
-  # Preload peers of transfers and fill their `:member` field.
-  # The member of the peer can be used to create more precise
-  # errors if the computation of the balance is not possible.
-  defp preload_peers(transfers, members) when is_list(transfers) do
-    members_by_id = Map.new(members, &{&1.id, &1})
+  defp fill_members_balance_errors(members) do
+    member_nicknames_by_id = Map.new(members, &{&1.id, &1.nickname})
 
-    transfers
-    # Ensure the peers are always returned in the same order,
-    # a different order would result in a different repartition
-    # of the transfers amount.
-    |> Repo.preload(peers: order_by(Peer, asc: :id))
-    |> Enum.map(&fill_peers_members(&1, members_by_id))
-  end
+    for member <- members do
+      balance_errors =
+        Enum.map(member.balance_errors, fn %{kind: :revenues_missing} = balance_error ->
+          member_nickname = Map.fetch!(member_nicknames_by_id, balance_error.extra.member_id)
+          put_in(balance_error.private[:member_nickname], member_nickname)
+        end)
 
-  defp fill_peers_members(transfer, members_by_id) do
-    Map.update!(transfer, :peers, fn peers ->
-      Enum.map(peers, fn peer ->
-        member = Map.fetch!(members_by_id, peer.member_id)
-        %{peer | member: member}
-      end)
-    end)
+      %{member | balance_errors: balance_errors}
+    end
   end
 
   defp compute_total_peer_weight(transfers) when is_list(transfers) do
@@ -121,13 +115,9 @@ defmodule App.Balance do
   defp maybe_set_weight_by_income_total_weight(transfer, peers_without_revenues) do
     error_reasons =
       Enum.map(peers_without_revenues, fn peer ->
-        %{
-          uniq_hash: "revenues_missing_#{peer.member_id}",
-          kind: :revenues_missing,
-          extra: %{
-            member: peer.member
-          }
-        }
+        BalanceError.new(:revenues_missing, %{
+          member_id: peer.member_id
+        })
       end)
 
     {:error, error_reasons, transfer}
@@ -286,7 +276,7 @@ defmodule App.Balance do
 
   The total sum of balanced money must be equal to 0, otherwise the function will crash.
   """
-  @spec transactions([BookMember.t()]) :: {:ok, [transaction()]} | {:error, error_reasons()}
+  @spec transactions([BookMember.t()]) :: {:ok, [transaction()]} | {:error, [BalanceError.t()]}
   def transactions(members) do
     error_reasons =
       Enum.find_value(members, fn member ->

apps/app/lib/app/balance/balance_error.ex

@@ -0,0 +1,36 @@
+defmodule App.Balance.BalanceError do
+  @moduledoc """
+  Balance errors are instantiated when the balance of a book cannot be computed,
+  and are used to store the kind of error along with some extra information.
+  """
+  use Ecto.Schema
+
+  @type t :: %__MODULE__{
+          kind: atom(),
+          extra: map(),
+          uniq_hash: String.t(),
+          private: map()
+        }
+
+  @primary_key false
+  embedded_schema do
+    field :kind, Ecto.Enum, values: [:missing_revenues]
+    field :extra, :map
+
+    field :uniq_hash, :string, virtual: true
+    field :private, :map, virtual: true, default: %{}
+  end
+
+  @spec new(kind :: atom(), extra :: map()) :: t()
+  def new(kind, extra) when is_atom(kind) and is_map(extra) do
+    %__MODULE__{
+      kind: kind,
+      extra: extra,
+      uniq_hash: uniq_hash(kind, extra)
+    }
+  end
+
+  defp uniq_hash(:revenues_missing, extra) do
+    "revenues_missing_#{extra.member_id}"
+  end
+end

apps/app/test/app/balance_test.exs

@@ -7,6 +7,7 @@ defmodule App.BalanceTest do
   import App.TransfersFixtures
 
   alias App.Balance
+  alias App.Balance.BalanceError
 
   describe "fill_members_balance/1" do
     setup do
@@ -298,21 +299,23 @@ defmodule App.BalanceTest do
       member1_id = member1.id
       expected_hash = "revenues_missing_#{member1_id}"
 
-      assert [
-               %{
+      assert member1.balance_errors == [
+               %BalanceError{
                  kind: :revenues_missing,
-                 uniq_hash: ^expected_hash,
-                 extra: %{member: %{id: ^member1_id}}
+                 uniq_hash: expected_hash,
+                 extra: %{member_id: member1_id},
+                 private: %{member_nickname: "member1"}
                }
-             ] = member1.balance_errors
+             ]
 
-      assert [
-               %{
+      assert member2.balance_errors == [
+               %BalanceError{
                  kind: :revenues_missing,
-                 uniq_hash: ^expected_hash,
-                 extra: %{member: %{id: ^member1_id}}
+                 uniq_hash: expected_hash,
+                 extra: %{member_id: member1_id},
+                 private: %{member_nickname: "member1"}
                }
-             ] = member2.balance_errors
+             ]
 
       assert Money.equal?(member3.balance, Money.new!(:EUR, -20))
     end

apps/app_web/lib/app_web/live/books/book_balance_live.ex

@@ -4,6 +4,7 @@ defmodule AppWeb.BookBalanceLive do
   import AppWeb.BooksComponents, only: [balance_card: 1]
 
   alias App.Balance
+  alias App.Balance.BalanceError
   alias App.Books.Book
   alias App.Books.BookMember
   alias App.Books.Members
@@ -33,15 +34,15 @@ defmodule AppWeb.BookBalanceLive do
         </.link>
       </.card_grid>
 
-      <%= if @transaction_errors != nil do %>
+      <%= if @balance_errors != nil do %>
         <section class="space-y-4" id="transaction-errors">
           <.alert kind={:error}>
             {gettext("Some information is missing to balance the book")}
           </.alert>
-          <.transaction_error_tile
-            :for={transaction_error <- @transaction_errors}
+          <.balance_error_tile
+            :for={balance_error <- @balance_errors}
             book={@book}
-            {transaction_error}
+            balance_error={balance_error}
           />
         </section>
       <% else %>
@@ -75,23 +76,25 @@ defmodule AppWeb.BookBalanceLive do
   end
 
   attr :book, Book, required: true
-  attr :kind, :atom, required: true
-  attr :extra, :map, required: true
-
-  defp transaction_error_tile(%{kind: :revenues_missing} = assigns) do
-    ~H"""
-    <.link navigate={~p"/books/#{@book}/members/#{@extra.member}"} class="block">
-      <.tile class="justify-between">
-        <div class="truncate">
-          <span class="label">{@extra.member.nickname}</span>
-          <span class="font-normal">did not set their revenues.</span>
-        </div>
-        <.button kind={:ghost}>
-          {gettext("Fix it")} <.icon name={:chevron_right} />
-        </.button>
-      </.tile>
-    </.link>
-    """
+  attr :balance_error, BalanceError, required: true
+
+  defp balance_error_tile(assigns) do
+    case assigns.balance_error.kind do
+      :revenues_missing ->
+        ~H"""
+        <.link navigate={~p"/books/#{@book}/members/#{@balance_error.extra.member_id}"} class="block">
+          <.tile class="justify-between">
+            <div class="truncate">
+              <span class="label">{@balance_error.private.member_nickname}</span>
+              <span class="font-normal">did not set their revenues.</span>
+            </div>
+            <.button kind={:ghost}>
+              {gettext("Fix it")} <.icon name={:chevron_right} />
+            </.button>
+          </.tile>
+        </.link>
+        """
+    end
   end
 
   # Highlight the nickname of the current member
@@ -130,18 +133,18 @@ defmodule AppWeb.BookBalanceLive do
       )
       |> assign_transactions(members)
 
-    {:ok, socket, temporary_assigns: [transaction_errors: []]}
+    {:ok, socket, temporary_assigns: [balance_errors: []]}
   end
 
   defp assign_transactions(socket, members) do
     case Balance.transactions(members) do
       {:ok, transactions} ->
         socket
-        |> assign(transaction_errors: nil)
+        |> assign(balance_errors: nil)
         |> stream(:transactions, transactions)
 
-      {:error, reasons} ->
-        assign(socket, transaction_errors: reasons)
+      {:error, balance_errors} ->
+        assign(socket, balance_errors: balance_errors)
     end
   end
 end