From 12cbab035c20c4ea36cb38380064c216cb869d5d Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 16 Jun 2025 18:30:34 -0400 Subject: [PATCH 01/26] add kind to transaction model --- .gitignore | 1 + app/models/transaction.rb | 7 ++++ ...20250616183654_add_kind_to_transactions.rb | 33 +++++++++++++++++++ db/schema.rb | 2 +- 4 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20250616183654_add_kind_to_transactions.rb diff --git a/.gitignore b/.gitignore index a37966ee..3d5baf07 100644 --- a/.gitignore +++ b/.gitignore @@ -100,6 +100,7 @@ node_modules/ tasks.json .taskmaster/tasks/ .taskmaster/reports/ +.taskmaster/state.json *.mcp.json scripts/ .cursor/mcp.json diff --git a/app/models/transaction.rb b/app/models/transaction.rb index d2a8abdc..c2e20148 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -9,6 +9,13 @@ class Transaction < ApplicationRecord accepts_nested_attributes_for :taggings, allow_destroy: true + enum :kind, { + standard: "standard", + transfer: "transfer", + loan_payment: "loan_payment", + one_time: "one_time" + } + class << self def search(params) Search.new(params).build_query(all) diff --git a/db/migrate/20250616183654_add_kind_to_transactions.rb b/db/migrate/20250616183654_add_kind_to_transactions.rb new file mode 100644 index 00000000..fd6e3a8f --- /dev/null +++ b/db/migrate/20250616183654_add_kind_to_transactions.rb @@ -0,0 +1,33 @@ +class AddKindToTransactions < ActiveRecord::Migration[7.2] + def change + add_column :transactions, :kind, :string, null: false, default: "standard" + add_index :transactions, :kind + + reversible do |dir| + dir.up do + # Update transaction kinds based on transfer relationships + execute <<~SQL + UPDATE transactions + SET kind = CASE + WHEN loan_accounts.accountable_type = 'Loan' + AND entries.amount > 0 + THEN 'loan_payment' + ELSE 'transfer' + END + FROM transfers t + JOIN entries ON ( + entries.entryable_id = t.inflow_transaction_id OR + entries.entryable_id = t.outflow_transaction_id + ) + LEFT JOIN entries inflow_entries ON ( + inflow_entries.entryable_id = t.inflow_transaction_id + AND inflow_entries.entryable_type = 'Transaction' + ) + LEFT JOIN accounts loan_accounts ON loan_accounts.id = inflow_entries.account_id + WHERE transactions.id = entries.entryable_id + AND entries.entryable_type = 'Transaction' + SQL + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 33074c58..9c10112c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -30,7 +30,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_06_10_181219) do t.decimal "balance", precision: 19, scale: 4 t.string "currency" t.boolean "is_active", default: true, null: false - t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true + t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true t.uuid "import_id" t.uuid "plaid_account_id" t.boolean "scheduled_for_deletion", default: false -- 2.53.0 From ae4a4e22b288b996bb97ec2d96f8b0eaa44bed13 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 17 Jun 2025 11:03:16 -0400 Subject: [PATCH 02/26] Basic transfer creator --- app/controllers/transfers_controller.rb | 26 ++-- app/models/trade/creator.rb | 12 ++ app/models/transaction.rb | 9 +- app/models/transfer.rb | 28 ++++ app/models/transfer/creator.rb | 85 ++++++++++++ db/schema.rb | 4 +- test/fixtures/transactions.yml | 6 +- test/models/transfer/creator_test.rb | 166 ++++++++++++++++++++++++ test/system/transactions_test.rb | 1 + 9 files changed, 315 insertions(+), 22 deletions(-) create mode 100644 app/models/trade/creator.rb create mode 100644 app/models/transfer/creator.rb create mode 100644 test/models/transfer/creator_test.rb diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index 895dcfa2..48556f09 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -1,4 +1,6 @@ class TransfersController < ApplicationController + include StreamExtensions + before_action :set_transfer, only: %i[destroy show update] def new @@ -10,25 +12,19 @@ class TransfersController < ApplicationController end def create - from_account = Current.family.accounts.find(transfer_params[:from_account_id]) - to_account = Current.family.accounts.find(transfer_params[:to_account_id]) - - @transfer = Transfer.from_accounts( - from_account: from_account, - to_account: to_account, + @transfer = Transfer::Creator.new( + family: Current.family, + source_account_id: transfer_params[:from_account_id], + destination_account_id: transfer_params[:to_account_id], date: transfer_params[:date], amount: transfer_params[:amount].to_d - ) - - if @transfer.save - @transfer.sync_account_later - - flash[:notice] = t(".success") + ).create + if @transfer.persisted? + success_message = "Transfer created" respond_to do |format| - format.html { redirect_back_or_to transactions_path } - redirect_target_url = request.referer || transactions_path - format.turbo_stream { render turbo_stream: turbo_stream.action(:redirect, redirect_target_url) } + format.html { redirect_back_or_to transactions_path, notice: success_message } + format.turbo_stream { stream_redirect_back_or_to transactions_path, notice: success_message } end else render :new, status: :unprocessable_entity diff --git a/app/models/trade/creator.rb b/app/models/trade/creator.rb new file mode 100644 index 00000000..224f3576 --- /dev/null +++ b/app/models/trade/creator.rb @@ -0,0 +1,12 @@ +class Trade::Creator + def initialize(attrs) + @attrs = attrs + end + + def create + # TODO + end + + private + attr_reader :attrs +end \ No newline at end of file diff --git a/app/models/transaction.rb b/app/models/transaction.rb index c2e20148..0967aac1 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -10,10 +10,11 @@ class Transaction < ApplicationRecord accepts_nested_attributes_for :taggings, allow_destroy: true enum :kind, { - standard: "standard", - transfer: "transfer", - loan_payment: "loan_payment", - one_time: "one_time" + standard: "standard", # A regular transaction, included in budget analytics + transfer: "transfer", # Movement of funds, excluded from budget analytics + payment: "payment", # A CC or Other payment, excluded from budget analytics (CC payments offset the sum of expense transactions) + loan_payment: "loan_payment", # A payment to a Loan account, treated as an expense in budgets + one_time: "one_time" # A one-time expense/income, excluded from budget analytics } class << self diff --git a/app/models/transfer.rb b/app/models/transfer.rb index 20804ddd..7177a0dd 100644 --- a/app/models/transfer.rb +++ b/app/models/transfer.rb @@ -22,8 +22,17 @@ class Transfer < ApplicationRecord Money.new(amount.abs, from_account.currency) end + outflow_kind = if to_account&.accountable_type == "Loan" + "loan_payment" + elsif to_account&.liability? + "payment" + else + "transfer" + end + new( inflow_transaction: Transaction.new( + kind: "transfer", entry: to_account.entries.build( amount: converted_amount.amount.abs * -1, currency: converted_amount.currency.iso_code, @@ -32,6 +41,7 @@ class Transfer < ApplicationRecord ) ), outflow_transaction: Transaction.new( + kind: outflow_kind, entry: from_account.entries.build( amount: amount.abs, currency: from_account.currency, @@ -89,6 +99,24 @@ class Transfer < ApplicationRecord to_account&.liability? end + def loan_payment? + outflow_transaction&.kind == "loan_payment" + end + + def liability_payment? + outflow_transaction&.kind == "payment" + end + + def regular_transfer? + outflow_transaction&.kind == "transfer" + end + + def transfer_type + return "loan_payment" if loan_payment? + return "liability_payment" if liability_payment? + "transfer" + end + def categorizable? to_account&.accountable_type == "Loan" end diff --git a/app/models/transfer/creator.rb b/app/models/transfer/creator.rb new file mode 100644 index 00000000..b713bafa --- /dev/null +++ b/app/models/transfer/creator.rb @@ -0,0 +1,85 @@ +class Transfer::Creator + def initialize(family:, source_account_id:, destination_account_id:, date:, amount:) + @family = family + @source_account = family.accounts.find(source_account_id) # early throw if not found + @destination_account = family.accounts.find(destination_account_id) # early throw if not found + @date = date + @amount = amount + end + + def create + transfer = Transfer.new( + inflow_transaction: inflow_transaction, + outflow_transaction: outflow_transaction, + status: "confirmed" + ) + + if transfer.save + source_account.sync_later + destination_account.sync_later + end + + transfer + end + + private + attr_reader :family, :source_account, :destination_account, :date, :amount + + def outflow_transaction + name = "#{name_prefix} to #{destination_account.name}" + + Transaction.new( + kind: outflow_transaction_kind, + entry: source_account.entries.build( + amount: amount.abs, + currency: source_account.currency, + date: date, + name: name, + ) + ) + end + + def inflow_transaction + name = "#{name_prefix} from #{source_account.name}" + + Transaction.new( + kind: "transfer", + entry: destination_account.entries.build( + amount: inflow_converted_money.amount.abs * -1, + currency: destination_account.currency, + date: date, + name: name, + ) + ) + end + + # If destination account has different currency, its transaction should show up as converted + # Future improvement: instead of a 1:1 conversion fallback, add a UI/UX flow for missing rates + def inflow_converted_money + Money.new(amount.abs, source_account.currency) + .exchange_to( + destination_account.currency, + date: date, + fallback_rate: 1.0 + ) + end + + # The "expense" side of a transfer is treated different in analytics based on where it goes. + def outflow_transaction_kind + if destination_account.loan? + "loan_payment" + elsif destination_account.liability? + "payment" + else + "transfer" + end + end + + def name_prefix + if destination_account.liability? + "Payment" + else + "Transfer" + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 9c10112c..f4d6b2a9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_06_10_181219) do +ActiveRecord::Schema[7.2].define(version: 2025_06_16_183654) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -648,7 +648,9 @@ ActiveRecord::Schema[7.2].define(version: 2025_06_10_181219) do t.uuid "category_id" t.uuid "merchant_id" t.jsonb "locked_attributes", default: {} + t.string "kind", default: "standard", null: false t.index ["category_id"], name: "index_transactions_on_category_id" + t.index ["kind"], name: "index_transactions_on_kind" t.index ["merchant_id"], name: "index_transactions_on_merchant_id" end diff --git a/test/fixtures/transactions.yml b/test/fixtures/transactions.yml index 426d7d58..7420c310 100644 --- a/test/fixtures/transactions.yml +++ b/test/fixtures/transactions.yml @@ -2,5 +2,7 @@ one: category: food_and_drink merchant: amazon -transfer_out: { } -transfer_in: { } \ No newline at end of file +transfer_out: + kind: payment +transfer_in: + kind: transfer \ No newline at end of file diff --git a/test/models/transfer/creator_test.rb b/test/models/transfer/creator_test.rb new file mode 100644 index 00000000..6328a1b4 --- /dev/null +++ b/test/models/transfer/creator_test.rb @@ -0,0 +1,166 @@ +require "test_helper" + +class Transfer::CreatorTest < ActiveSupport::TestCase + setup do + @family = families(:dylan_family) + @source_account = accounts(:depository) + @destination_account = accounts(:investment) + @date = Date.current + @amount = 100 + end + + test "creates basic transfer" do + creator = Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: @destination_account.id, + date: @date, + amount: @amount + ) + + transfer = creator.create + + assert transfer.persisted? + assert_equal "confirmed", transfer.status + assert transfer.regular_transfer? + assert_equal "transfer", transfer.transfer_type + + # Verify outflow transaction (from source account) + outflow = transfer.outflow_transaction + assert_equal "transfer", outflow.kind + assert_equal @amount, outflow.entry.amount + assert_equal @source_account.currency, outflow.entry.currency + assert_equal "Transfer to #{@destination_account.name}", outflow.entry.name + + # Verify inflow transaction (to destination account) + inflow = transfer.inflow_transaction + assert_equal "transfer", inflow.kind + assert_equal(@amount * -1, inflow.entry.amount) + assert_equal @destination_account.currency, inflow.entry.currency + assert_equal "Transfer from #{@source_account.name}", inflow.entry.name + end + + test "creates multi-currency transfer" do + # Use crypto account which has USD currency but different from source + crypto_account = accounts(:crypto) + + creator = Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: crypto_account.id, + date: @date, + amount: @amount + ) + + transfer = creator.create + + assert transfer.persisted? + assert transfer.regular_transfer? + assert_equal "transfer", transfer.transfer_type + + # Verify outflow transaction + outflow = transfer.outflow_transaction + assert_equal "transfer", outflow.kind + assert_equal "Transfer to #{crypto_account.name}", outflow.entry.name + + # Verify inflow transaction with currency handling + inflow = transfer.inflow_transaction + assert_equal "transfer", inflow.kind + assert_equal "Transfer from #{@source_account.name}", inflow.entry.name + assert_equal crypto_account.currency, inflow.entry.currency + end + + test "creates loan payment" do + loan_account = accounts(:loan) + + creator = Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: loan_account.id, + date: @date, + amount: @amount + ) + + transfer = creator.create + + assert transfer.persisted? + assert transfer.loan_payment? + assert_equal "loan_payment", transfer.transfer_type + + # Verify outflow transaction is marked as loan payment + outflow = transfer.outflow_transaction + assert_equal "loan_payment", outflow.kind + assert_equal "Payment to #{loan_account.name}", outflow.entry.name + + # Verify inflow transaction + inflow = transfer.inflow_transaction + assert_equal "transfer", inflow.kind + assert_equal "Payment from #{@source_account.name}", inflow.entry.name + end + + test "creates credit card payment" do + credit_card_account = accounts(:credit_card) + + creator = Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: credit_card_account.id, + date: @date, + amount: @amount + ) + + transfer = creator.create + + assert transfer.persisted? + assert transfer.liability_payment? + assert_equal "liability_payment", transfer.transfer_type + + # Verify outflow transaction is marked as payment for liability + outflow = transfer.outflow_transaction + assert_equal "payment", outflow.kind + assert_equal "Payment to #{credit_card_account.name}", outflow.entry.name + + # Verify inflow transaction + inflow = transfer.inflow_transaction + assert_equal "transfer", inflow.kind + assert_equal "Payment from #{@source_account.name}", inflow.entry.name + end + + test "raises error when source account ID is invalid" do + assert_raises(ActiveRecord::RecordNotFound) do + Transfer::Creator.new( + family: @family, + source_account_id: 99999, + destination_account_id: @destination_account.id, + date: @date, + amount: @amount + ) + end + end + + test "raises error when destination account ID is invalid" do + assert_raises(ActiveRecord::RecordNotFound) do + Transfer::Creator.new( + family: @family, + source_account_id: @source_account.id, + destination_account_id: 99999, + date: @date, + amount: @amount + ) + end + end + + test "raises error when source account belongs to different family" do + other_family = families(:empty) + + assert_raises(ActiveRecord::RecordNotFound) do + Transfer::Creator.new( + family: other_family, + source_account_id: @source_account.id, + destination_account_id: @destination_account.id, + date: @date, + amount: @amount + ) + end + end +end diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb index 4b3bcae8..8b292c8e 100644 --- a/test/system/transactions_test.rb +++ b/test/system/transactions_test.rb @@ -203,6 +203,7 @@ class TransactionsTest < ApplicationSystemTestCase inflow_entry = create_transaction("inflow", 1.day.ago.to_date, -500, account: investment_account) @user.family.auto_match_transfers! visit transactions_url + within "#entry-group-" + Date.current.to_s + "-totals" do assert_text "-$100.00" # transaction eleven from setup end -- 2.53.0 From b049f23cad26276b19a3703ccd081a345ee1d172 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 17 Jun 2025 11:31:46 -0400 Subject: [PATCH 03/26] Fix method naming conflict --- app/helpers/application_helper.rb | 2 +- app/models/transaction/transferable.rb | 4 - app/views/category/dropdowns/show.html.erb | 92 ++++++++++---------- app/views/transactions/_header.html.erb | 2 +- app/views/transactions/_transaction.html.erb | 12 +-- app/views/transactions/show.html.erb | 4 +- 6 files changed, 56 insertions(+), 60 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index a86e374e..1602931c 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -110,7 +110,7 @@ module ApplicationHelper private def calculate_total(item, money_method, negate) - items = item.reject { |i| i.respond_to?(:entryable) && i.entryable.transfer? } + items = item.reject { |i| i.respond_to?(:entryable) && i.entryable.transfer.present? } total = items.sum(&money_method) negate ? -total : total end diff --git a/app/models/transaction/transferable.rb b/app/models/transaction/transferable.rb index d839047a..cea7978b 100644 --- a/app/models/transaction/transferable.rb +++ b/app/models/transaction/transferable.rb @@ -14,10 +14,6 @@ module Transaction::Transferable transfer_as_inflow || transfer_as_outflow end - def transfer? - transfer.present? - end - def transfer_match_candidates candidates_scope = if self.entry.amount.negative? family_matches_scope.where("inflow_candidates.entryable_id = ?", self.id) diff --git a/app/views/category/dropdowns/show.html.erb b/app/views/category/dropdowns/show.html.erb index 4329918f..6af8ba45 100644 --- a/app/views/category/dropdowns/show.html.erb +++ b/app/views/category/dropdowns/show.html.erb @@ -9,81 +9,81 @@ class="bg-container placeholder:text-sm placeholder:text-secondary font-normal h-10 relative pl-10 w-full border-none rounded-lg focus:outline-hidden focus:ring-0" data-list-filter-target="input" data-action="list-filter#filter"> - <%= icon("search", class: "absolute inset-0 ml-2 transform top-1/2 -translate-y-1/2") %> + <%= icon("search", class: "absolute inset-0 ml-2 transform top-1/2 -translate-y-1/2") %> + - -
- - <% if @categories.any? %> - <% Category::Group.for(@categories).each do |group| %> - <%= render "category/dropdowns/row", category: group.category %> +
+ + <% if @categories.any? %> + <% Category::Group.for(@categories).each do |group| %> + <%= render "category/dropdowns/row", category: group.category %> - <% group.subcategories.each do |category| %> - <%= render "category/dropdowns/row", category: category %> + <% group.subcategories.each do |category| %> + <%= render "category/dropdowns/row", category: category %> + <% end %> <% end %> - <% end %> - <% else %> -
-
-

<%= t(".empty") %>

+ <% else %> +
+
+

<%= t(".empty") %>

- <%= render ButtonComponent.new( + <%= render ButtonComponent.new( text: t(".bootstrap"), variant: "outline", href: bootstrap_categories_path, method: :post, data: { turbo_frame: :_top }) %> +
-
- <% end %> -
+ <% end %> +
- <%= render "shared/ruler", classes: "my-2" %> + <%= render "shared/ruler", classes: "my-2" %> -
- <% if @transaction.category %> - <%= button_to transaction_path(@transaction.entry), +
+ <% if @transaction.category %> + <%= button_to transaction_path(@transaction.entry), method: :patch, data: { turbo_frame: dom_id(@transaction.entry) }, params: { entry: { entryable_type: "Transaction", entryable_attributes: { id: @transaction.id, category_id: nil } } }, class: "flex text-sm font-medium items-center gap-2 text-secondary w-full rounded-lg p-2 hover:bg-container-inset-hover" do %> - <%= icon("minus") %> + <%= icon("minus") %> - <%= t(".clear") %> + <%= t(".clear") %> + <% end %> <% end %> - <% end %> - <% unless @transaction.transfer? %> - <%= link_to new_transaction_transfer_match_path(@transaction.entry), + <% unless @transaction.transfer.present? %> + <%= link_to new_transaction_transfer_match_path(@transaction.entry), class: "flex text-sm font-medium items-center gap-2 text-secondary w-full rounded-lg p-2 hover:bg-container-inset-hover", data: { turbo_frame: "modal" } do %> - <%= icon("refresh-cw") %> + <%= icon("refresh-cw") %> -

Match transfer/payment

+

Match transfer/payment

+ <% end %> <% end %> - <% end %> -
-
- <%= form_with url: transaction_path(@transaction.entry), +
+
+ <%= form_with url: transaction_path(@transaction.entry), method: :patch, data: { controller: "auto-submit-form" } do |f| %> - <%= f.hidden_field "entry[excluded]", value: !@transaction.entry.excluded %> - <%= f.check_box "entry[excluded]", + <%= f.hidden_field "entry[excluded]", value: !@transaction.entry.excluded %> + <%= f.check_box "entry[excluded]", checked: @transaction.entry.excluded, class: "checkbox checkbox--light", data: { auto_submit_form_target: "auto", autosubmit_trigger_event: "change" } %> - <% end %> + <% end %> +
+ +

One-time <%= @transaction.entry.amount.negative? ? "income" : "expense" %>

+ + + <%= icon("asterisk", color: "current") %> +
- -

One-time <%= @transaction.entry.amount.negative? ? "income" : "expense" %>

- - - <%= icon("asterisk", color: "current") %> -
-
-<% end %> + <% end %> diff --git a/app/views/transactions/_header.html.erb b/app/views/transactions/_header.html.erb index 6c7b1615..e56d2270 100644 --- a/app/views/transactions/_header.html.erb +++ b/app/views/transactions/_header.html.erb @@ -12,7 +12,7 @@ - <% if entry.transaction.transfer? %> + <% if entry.transaction.transfer.present? %> <%= icon "arrow-left-right", class: "mt-1" %> <% end %> diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index a046915e..ebca40de 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -10,7 +10,7 @@
<%= check_box_tag dom_id(entry, "selection"), - disabled: transaction.transfer?, + disabled: transaction.transfer.present?, class: "checkbox checkbox--light", data: { id: entry.id, @@ -37,8 +37,8 @@
<%= link_to( - transaction.transfer? ? transaction.transfer.name : entry.name, - transaction.transfer? ? transfer_path(transaction.transfer) : entry_path(entry), + transaction.transfer.present? ? transaction.transfer.name : entry.name, + transaction.transfer.present? ? transfer_path(transaction.transfer) : entry_path(entry), data: { turbo_frame: "drawer", turbo_prefetch: false @@ -52,13 +52,13 @@ <% end %> - <% if transaction.transfer? %> + <% if transaction.transfer.present? %> <%= render "transactions/transfer_match", transaction: transaction %> <% end %>