From 5676abcde6f301f2b02306db8cc200adc9a96115 Mon Sep 17 00:00:00 2001 From: Enzo Date: Sun, 12 Jan 2025 18:40:07 +0100 Subject: [PATCH 1/4] chore(transfers): replace unused `belongs_to :book` association by a simple field --- apps/app/lib/app/transfers/money_transfer.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/app/lib/app/transfers/money_transfer.ex b/apps/app/lib/app/transfers/money_transfer.ex index 29a0070d..4ce5bad1 100644 --- a/apps/app/lib/app/transfers/money_transfer.ex +++ b/apps/app/lib/app/transfers/money_transfer.ex @@ -21,8 +21,9 @@ defmodule App.Transfers.MoneyTransfer do amount: Money.t(), type: type(), date: Date.t(), - book: Book.t(), + book_id: Book.id(), tenant: BookMember.t(), + tenant_id: BookMember.id(), balance_params: TransferParams.t(), peers: [Peer.t()], total_peer_weight: Decimal.t(), @@ -41,7 +42,7 @@ defmodule App.Transfers.MoneyTransfer do field :type, Ecto.Enum, values: @transfer_types field :date, :date, read_after_writes: true - belongs_to :book, Book + field :book_id, :integer belongs_to :tenant, BookMember # balance From 7dd52a796bf6bf5d34ca4e4c81266a44ffc8fa70 Mon Sep 17 00:00:00 2001 From: Enzo Date: Sun, 12 Jan 2025 18:50:43 +0100 Subject: [PATCH 2/4] feat(transfers): add creator id to money transfers --- apps/app/lib/app/transfers.ex | 10 ++++-- apps/app/lib/app/transfers/money_transfer.ex | 2 ++ ...2174553_add_money_transfers_creator_id.exs | 9 +++++ apps/app/test/app/transfers_test.exs | 33 ++++++++++++++++--- .../live/books/book_transfer_form_live.ex | 4 +-- 5 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 apps/app/priv/repo/migrations/20250112174553_add_money_transfers_creator_id.exs diff --git a/apps/app/lib/app/transfers.ex b/apps/app/lib/app/transfers.ex index 5a872dc1..db9c2ad4 100644 --- a/apps/app/lib/app/transfers.ex +++ b/apps/app/lib/app/transfers.ex @@ -197,12 +197,16 @@ defmodule App.Transfers do @doc """ Creates a money_transfer. """ - @spec create_money_transfer(Book.t(), MoneyTransfer.type(), map()) :: + @spec create_money_transfer(Book.t(), BookMember.t(), MoneyTransfer.type(), map()) :: {:ok, MoneyTransfer.t()} | {:error, Ecto.Changeset.t()} - def create_money_transfer(%Book{} = book, type, attrs) + def create_money_transfer(%Book{} = book, %BookMember{} = creator, type, attrs) when is_map(attrs) and type in [:payment, :income] do changeset = - %MoneyTransfer{book_id: book.id, type: type} + %MoneyTransfer{ + book_id: book.id, + creator_id: creator.id, + type: type + } |> MoneyTransfer.changeset(attrs) result = diff --git a/apps/app/lib/app/transfers/money_transfer.ex b/apps/app/lib/app/transfers/money_transfer.ex index 4ce5bad1..e6071d1c 100644 --- a/apps/app/lib/app/transfers/money_transfer.ex +++ b/apps/app/lib/app/transfers/money_transfer.ex @@ -22,6 +22,7 @@ defmodule App.Transfers.MoneyTransfer do type: type(), date: Date.t(), book_id: Book.id(), + creator_id: BookMember.id(), tenant: BookMember.t(), tenant_id: BookMember.id(), balance_params: TransferParams.t(), @@ -43,6 +44,7 @@ defmodule App.Transfers.MoneyTransfer do field :date, :date, read_after_writes: true field :book_id, :integer + field :creator_id, :integer belongs_to :tenant, BookMember # balance diff --git a/apps/app/priv/repo/migrations/20250112174553_add_money_transfers_creator_id.exs b/apps/app/priv/repo/migrations/20250112174553_add_money_transfers_creator_id.exs new file mode 100644 index 00000000..77e95906 --- /dev/null +++ b/apps/app/priv/repo/migrations/20250112174553_add_money_transfers_creator_id.exs @@ -0,0 +1,9 @@ +defmodule App.Repo.Migrations.AddMoneyTransfersCreatorId do + use Ecto.Migration + + def change do + alter table("money_transfers") do + add :creator_id, references("book_members", on_delete: :nilify_all) + end + end +end diff --git a/apps/app/test/app/transfers_test.exs b/apps/app/test/app/transfers_test.exs index 36bb185f..fe79861e 100644 --- a/apps/app/test/app/transfers_test.exs +++ b/apps/app/test/app/transfers_test.exs @@ -12,6 +12,7 @@ defmodule App.TransfersTest do alias App.Balance.TransferParams alias App.Books.Book + alias App.Books.BookMember alias App.Transfers alias App.Transfers.MoneyTransfer alias App.Transfers.Peer @@ -203,13 +204,14 @@ defmodule App.TransfersTest do end end - describe "create_money_transfer/1" do + describe "create_money_transfer/4" do setup :book_with_member_context test "creates a money transfer", %{book: book, member: member} do assert {:ok, transfer} = Transfers.create_money_transfer( book, + member, :payment, money_transfer_attributes( tenant_id: member.id, @@ -224,6 +226,7 @@ defmodule App.TransfersTest do assert transfer.type == :payment assert transfer.date == ~D[2022-06-23] assert transfer.balance_params == struct!(TransferParams, transfer_params_attributes()) + assert transfer.creator_id == member.id transfer = Repo.preload(transfer, :peers) assert Enum.empty?(transfer.peers) @@ -233,6 +236,7 @@ defmodule App.TransfersTest do assert {:ok, transfer} = Transfers.create_money_transfer( book, + member, :payment, money_transfer_attributes( tenant_id: member.id, @@ -254,6 +258,7 @@ defmodule App.TransfersTest do assert {:ok, transfer} = Transfers.create_money_transfer( book, + member, :payment, money_transfer_attributes( tenant_id: member.id, @@ -278,6 +283,7 @@ defmodule App.TransfersTest do assert_raise Ecto.ConstraintError, ~r/transfers_peers_transfer_id_member_id_index/, fn -> Transfers.create_money_transfer( book, + member, :payment, money_transfer_attributes( tenant_id: member.id, @@ -291,6 +297,18 @@ defmodule App.TransfersTest do assert_raise Ecto.ConstraintError, ~r/money_transfers_book_id_fkey/, fn -> Transfers.create_money_transfer( %Book{id: 0}, + member, + :payment, + money_transfer_attributes(tenant_id: member.id) + ) + end + end + + test "fails with invalid creator_id", %{book: book, member: member} do + assert_raise Ecto.ConstraintError, ~r/money_transfers_creator_id_fkey/, fn -> + Transfers.create_money_transfer( + book, + %BookMember{id: 0}, :payment, money_transfer_attributes(tenant_id: member.id) ) @@ -301,23 +319,30 @@ defmodule App.TransfersTest do assert_raise FunctionClauseError, fn -> Transfers.create_money_transfer( book, + member, :reimbursement, money_transfer_attributes(tenant_id: member.id) ) end end - test "fails with missing tenant_id", %{book: book} do + test "fails with missing tenant_id", %{book: book, member: member} do assert {:error, changeset} = - Transfers.create_money_transfer(book, :payment, money_transfer_attributes()) + Transfers.create_money_transfer( + book, + member, + :payment, + money_transfer_attributes() + ) assert errors_on(changeset) == %{tenant_id: ["can't be blank"]} end - test "fails with invalid tenant_id", %{book: book} do + test "fails with invalid tenant_id", %{book: book, member: member} do assert {:error, changeset} = Transfers.create_money_transfer( book, + member, :payment, money_transfer_attributes(tenant_id: 0) ) diff --git a/apps/app_web/lib/app_web/live/books/book_transfer_form_live.ex b/apps/app_web/lib/app_web/live/books/book_transfer_form_live.ex index 4cb436aa..e44a8755 100644 --- a/apps/app_web/lib/app_web/live/books/book_transfer_form_live.ex +++ b/apps/app_web/lib/app_web/live/books/book_transfer_form_live.ex @@ -425,9 +425,9 @@ defmodule AppWeb.BookTransferFormLive do end defp submit_form(socket, :new, money_transfer_params) do - %{book: book, type: type} = socket.assigns + %{book: book, current_member: current_member, type: type} = socket.assigns - case Transfers.create_money_transfer(book, type, money_transfer_params) do + case Transfers.create_money_transfer(book, current_member, type, money_transfer_params) do {:ok, _money_transfer} -> {:noreply, push_navigate(socket, to: ~p"/books/#{book}/transfers")} From 8665ff992c334bd2d15c0fb92cc7611a15a41ff5 Mon Sep 17 00:00:00 2001 From: Enzo Date: Sun, 12 Jan 2025 19:02:22 +0100 Subject: [PATCH 3/4] chore(transfers): add data migrations to backfill column --- ...08_backfill_money_transfers_creator_id.exs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 apps/app/priv/repo/data_migrations/20250112175408_backfill_money_transfers_creator_id.exs diff --git a/apps/app/priv/repo/data_migrations/20250112175408_backfill_money_transfers_creator_id.exs b/apps/app/priv/repo/data_migrations/20250112175408_backfill_money_transfers_creator_id.exs new file mode 100644 index 00000000..f2760f07 --- /dev/null +++ b/apps/app/priv/repo/data_migrations/20250112175408_backfill_money_transfers_creator_id.exs @@ -0,0 +1,44 @@ +defmodule App.Repo.Migrations.BackfillMoneyTransfersCreatorId do + use Ecto.Migration + + import Ecto.Query + + @disable_ddl_transaction true + @disable_migration_lock true + @batch_size 50 + @throttle_ms 500 + + def up do + throttle_change_in_batches(&page_query/0, &do_change/1) + end + + def down, do: :ok + + defp do_change(batch_of_ids) do + from(t in "money_transfers", + where: t.id in ^batch_of_ids, + update: [set: [creator_id: t.tenant_id]] + ) + |> repo().update_all([], log: :info, timeout: :infinity) + end + + defp page_query do + from t in "money_transfers", + where: is_nil(t.creator_id), + order_by: [asc: t.id], + limit: @batch_size, + select: t.id + end + + defp throttle_change_in_batches(query_fun, change_fun) do + case repo().all(query_fun.(), log: :info, timeout: :infinity) do + [] -> + :ok + + ids -> + change_fun.(ids) + Process.sleep(@throttle_ms) + throttle_change_in_batches(query_fun, change_fun) + end + end +end From 170e79d3d367e787f7893afd859b62361695d5c9 Mon Sep 17 00:00:00 2001 From: Enzo Date: Sun, 12 Jan 2025 19:17:01 +0100 Subject: [PATCH 4/4] feat(transfers): allow filtering by creator id --- apps/app/lib/app/transfers.ex | 42 ++++++++++++------- apps/app/test/app/transfers_test.exs | 17 ++++++++ .../app_web/live/books/book_transfers_live.ex | 19 +++++++++ apps/app_web/priv/gettext/default.pot | 4 ++ .../priv/gettext/en/LC_MESSAGES/default.po | 4 ++ .../priv/gettext/fr/LC_MESSAGES/default.po | 4 ++ 6 files changed, 74 insertions(+), 16 deletions(-) diff --git a/apps/app/lib/app/transfers.ex b/apps/app/lib/app/transfers.ex index db9c2ad4..ee449ec2 100644 --- a/apps/app/lib/app/transfers.ex +++ b/apps/app/lib/app/transfers.ex @@ -77,16 +77,18 @@ defmodule App.Transfers do ## Filters @filters_default %{ - sort_by: :most_recent, - tenanted_by: nil + tenanted_by: nil, + created_by: nil, + sort_by: :most_recent } @filters_types %{ + # Values are `nil`, `member_id` or `{:not, member_id}` + tenanted_by: :any, + created_by: {:array, :integer}, sort_by: Ecto.ParameterizedType.init(Ecto.Enum, values: [:most_recent, :oldest, :last_created, :first_created] - ), - # Values are `nil`, `member_id` or `{:not, member_id}` - tenanted_by: :any + ) } defp filter_money_transfers_query(query, raw_filters) do @@ -96,10 +98,28 @@ defmodule App.Transfers do |> Ecto.Changeset.apply_changes() query - |> sort_money_transfers_by(filters[:sort_by]) |> filter_money_transfers_by_tenancy(filters[:tenanted_by]) + |> filter_money_transfers_by_creator(filters[:created_by]) + |> sort_money_transfers_by(filters[:sort_by]) + end + + defp filter_money_transfers_by_tenancy(query, {:not, member_id}) when is_integer(member_id) do + from [money_transfer: money_transfer] in query, where: money_transfer.tenant_id != ^member_id + end + + defp filter_money_transfers_by_tenancy(query, member_id) when is_integer(member_id) do + from [money_transfer: money_transfer] in query, where: money_transfer.tenant_id == ^member_id + end + + defp filter_money_transfers_by_tenancy(query, nil), do: query + + defp filter_money_transfers_by_creator(query, creator_ids) when is_list(creator_ids) do + from [money_transfer: money_transfer] in query, + where: money_transfer.creator_id in ^creator_ids end + defp filter_money_transfers_by_creator(query, nil), do: query + defp sort_money_transfers_by(query, :most_recent) do from [money_transfer: money_transfer] in query, order_by: [desc: money_transfer.date] end @@ -116,16 +136,6 @@ defmodule App.Transfers do from [money_transfer: money_transfer] in query, order_by: [asc: money_transfer.inserted_at] end - defp filter_money_transfers_by_tenancy(query, {:not, member_id}) when is_integer(member_id) do - from [money_transfer: money_transfer] in query, where: money_transfer.tenant_id != ^member_id - end - - defp filter_money_transfers_by_tenancy(query, member_id) when is_integer(member_id) do - from [money_transfer: money_transfer] in query, where: money_transfer.tenant_id == ^member_id - end - - defp filter_money_transfers_by_tenancy(query, nil), do: query - ## Pagination defp paginate_query(query, offset, limit) do diff --git a/apps/app/test/app/transfers_test.exs b/apps/app/test/app/transfers_test.exs index fe79861e..2ad59f10 100644 --- a/apps/app/test/app/transfers_test.exs +++ b/apps/app/test/app/transfers_test.exs @@ -99,6 +99,23 @@ defmodule App.TransfersTest do |> Enum.map(& &1.id) == [transfer2.id] end + test "filters by creator", %{book: book} do + member1 = book_member_fixture(book) + member2 = book_member_fixture(book) + member3 = book_member_fixture(book) + + transfer1 = money_transfer_fixture(book, tenant_id: member1.id, creator_id: member1.id) + transfer2 = money_transfer_fixture(book, tenant_id: member1.id, creator_id: member2.id) + + assert Transfers.list_transfers_of_book(book, filters: %{created_by: [member1.id]}) + |> Enum.map(& &1.id) == [transfer1.id] + + assert Transfers.list_transfers_of_book(book, filters: %{created_by: [member2.id]}) + |> Enum.map(& &1.id) == [transfer2.id] + + assert Transfers.list_transfers_of_book(book, filters: %{created_by: [member3.id]}) == [] + end + test "paginates results", %{book: book, member: member} do transfer1 = money_transfer_fixture(book, tenant_id: member.id, date: ~D[2020-06-29]) transfer2 = money_transfer_fixture(book, tenant_id: member.id, date: ~D[2020-06-30]) diff --git a/apps/app_web/lib/app_web/live/books/book_transfers_live.ex b/apps/app_web/lib/app_web/live/books/book_transfers_live.ex index 7d03635d..6142fe86 100644 --- a/apps/app_web/lib/app_web/live/books/book_transfers_live.ex +++ b/apps/app_web/lib/app_web/live/books/book_transfers_live.ex @@ -56,6 +56,11 @@ defmodule AppWeb.BookTransfersLive do others: gettext("Others") ] ), + multi_select( + name: "created_by", + label: gettext("Created by"), + options: @book_members_options + ), sort_by( options: [ most_recent: gettext("Most recent"), @@ -122,6 +127,7 @@ defmodule AppWeb.BookTransfersLive do page: 1, per_page: 25 ) + |> assign_book_members_options() |> assign_filters(%{"sort_by" => "most_recent"}) |> paginate_transfers(1) @@ -209,6 +215,19 @@ defmodule AppWeb.BookTransfersLive do {:noreply, socket} end + defp assign_book_members_options(socket) do + book = socket.assigns.book + + book_members_options = + from(book_member in BookMember.book_query(book), + order_by: [asc: book_member.nickname], + select: {book_member.id, book_member.nickname} + ) + |> Repo.all() + + assign(socket, :book_members_options, book_members_options) + end + defp assign_filters(socket, filters) do current_member = socket.assigns.current_member diff --git a/apps/app_web/priv/gettext/default.pot b/apps/app_web/priv/gettext/default.pot index 0701c17d..77e3e955 100644 --- a/apps/app_web/priv/gettext/default.pot +++ b/apps/app_web/priv/gettext/default.pot @@ -150,6 +150,10 @@ msgstr "" msgid "Create reimbursement" msgstr "" +#, elixir-autogen, elixir-format +msgid "Created by" +msgstr "" + #, elixir-autogen, elixir-format msgid "Current password" msgstr "" diff --git a/apps/app_web/priv/gettext/en/LC_MESSAGES/default.po b/apps/app_web/priv/gettext/en/LC_MESSAGES/default.po index 2983bd02..fec3b915 100644 --- a/apps/app_web/priv/gettext/en/LC_MESSAGES/default.po +++ b/apps/app_web/priv/gettext/en/LC_MESSAGES/default.po @@ -151,6 +151,10 @@ msgstr "Create manually" msgid "Create reimbursement" msgstr "Create reimbursement" +#, elixir-autogen, elixir-format +msgid "Created by" +msgstr "Created by" + #, elixir-autogen, elixir-format msgid "Current password" msgstr "Current password" diff --git a/apps/app_web/priv/gettext/fr/LC_MESSAGES/default.po b/apps/app_web/priv/gettext/fr/LC_MESSAGES/default.po index 99eac633..4317fc80 100644 --- a/apps/app_web/priv/gettext/fr/LC_MESSAGES/default.po +++ b/apps/app_web/priv/gettext/fr/LC_MESSAGES/default.po @@ -151,6 +151,10 @@ msgstr "Créer manuellement" msgid "Create reimbursement" msgstr "Créer le remboursement" +#, elixir-autogen, elixir-format +msgid "Created by" +msgstr "Créé par" + #, elixir-autogen, elixir-format msgid "Current password" msgstr "Mot de passe actuel"