Docker/Maybe
Docker/Maybe
1
0
Fork 0
mirror of https://github.com/maybe-finance/maybe.git synced 2025-08-10 07:55:21 +02:00
Code Issues Releases Activity Actions

Decouple transactions list from transfer details

Browse source
This commit is contained in:
Zach Gollwitzer 2025-06-19 18:08:17 -04:00
parent b7cc49b7bf
commit 56d97a92db
23 changed files with 132 additions and 76 deletions
Download patch file Download diff file Expand all files Collapse all files

15
app/controllers/transactions_controller.rb
View file

@ -11,18 +11,13 @@ class TransactionsController < ApplicationController
def index
@q = search_params
search = Transaction::Search.new(Current.family, filters: @q)
@totals = search.totals
@search = Transaction::Search.new(Current.family, filters: @q)
per_page = params[:per_page].to_i.positive? ? params[:per_page].to_i : 50
base_scope = search.transactions_scope
base_scope = @search.transactions_scope
.reverse_chronological
.includes(
{ entry: :account },
:category, :merchant, :tags,
transfer_as_outflow: { inflow_transaction: { entry: :account } },
transfer_as_inflow: { outflow_transaction: { entry: :account } }
:category, :merchant, :tags
)
@pagy, @transactions = pagy(base_scope, limit: per_page, params: ->(p) { p.except(:focused_record_id) })
@ -118,6 +113,10 @@ class TransactionsController < ApplicationController
end
private
def per_page
params[:per_page].to_i.positive? ? params[:per_page].to_i : 50
end
def needs_rule_notification?(transaction)
return false if Current.user.rule_prompts_disabled

14
app/controllers/transfers_controller.rb
View file

@ -1,7 +1,7 @@
class TransfersController < ApplicationController
include StreamExtensions
before_action :set_transfer, only: %i[destroy show update]
before_action :set_transfer, only: %i[show destroy update]
def new
@transfer = Transfer.new
@ -50,9 +50,15 @@ class TransfersController < ApplicationController
private
def set_transfer
@transfer = Transfer.find(params[:id])
raise ActiveRecord::RecordNotFound unless @transfer.belongs_to_family?(Current.family)
if params[:transaction_id]
@transfer = Current.family.transactions.find(params[:transaction_id]).transfer
else
# Finds the transfer and ensures the family owns it
@transfer = Transfer
.where(id: params[:id])
.where(inflow_transaction_id: Current.family.transactions.select(:id))
.first
end
end
def transfer_params

8
app/helpers/application_helper.rb
View file

@ -110,7 +110,13 @@ module ApplicationHelper
private
def calculate_total(item, money_method, negate)
items = item.reject { |i| i.respond_to?(:entryable) && i.entryable.transfer.present? }
# Filter out transfer-type transactions from entries
# Only Entry objects have entryable transactions, Account objects don't
items = item.reject do |i|
i.is_a?(Entry) &&
i.entryable.is_a?(Transaction) &&
i.entryable.transfer?
end
total = items.sum(&money_method)
negate ? -total : total
end

44
app/helpers/entries_helper.rb
View file

@ -1,19 +1,35 @@
module EntriesHelper
def entries_by_date(entries, totals: false)
# Group transactions that might be transfers by their matching criteria
# Use amount, date, and currency to identify potential transfer pairs
transfer_groups = entries.group_by do |entry|
# Only check for transfer if it's a transaction
# Only check for transfer-type transactions
next nil unless entry.entryable_type == "Transaction"
entry.entryable.transfer&.id
transaction = entry.entryable
next nil unless transaction.transfer?
# Create a grouping key based on transfer matching criteria
# This groups transactions that are likely transfer pairs
[
entry.amount_money.abs, # Absolute amount
entry.currency,
entry.date
]
end
# For a more intuitive UX, we do not want to show the same transfer twice in the list
deduped_entries = transfer_groups.flat_map do |transfer_id, grouped_entries|
if transfer_id.nil? || grouped_entries.size == 1
# Keep the outflow side (positive amount) and reject the inflow side (negative amount)
deduped_entries = transfer_groups.flat_map do |group_key, grouped_entries|
if group_key.nil? || grouped_entries.size == 1
# Not a transfer or only one side found, keep all entries
grouped_entries
else
grouped_entries.reject do |e|
e.entryable_type == "Transaction" &&
e.entryable.transfer_as_inflow.present?
# Multiple entries with same amount/date/currency - likely a transfer pair
# Keep the outflow side (positive amount) and reject inflow side (negative amount)
grouped_entries.reject do |entry|
entry.entryable_type == "Transaction" &&
entry.entryable.transfer? &&
entry.amount.negative? # This is the inflow side
end
end
end
@ -37,4 +53,18 @@ module EntriesHelper
entry.name
].join(" • ")
end
# Helper method to derive transfer name without Transfer association
def transfer_name_for_transaction(transaction)
entry = transaction.entry
# For loan payments, use payment language
if transaction.loan_payment?
"Payment to #{entry.account.name}"
# For other transfer types, use transfer language
else
# Default transfer name based on the account
"Transfer involving #{entry.account.name}"
end
end
end

2
app/models/assistant/function/get_transactions.rb
View file

@ -163,7 +163,7 @@ class Assistant::Function::GetTransactions < Assistant::Function
category: txn.category&.name,
merchant: txn.merchant&.name,
tags: txn.tags.map(&:name),
is_transfer: txn.transfer.present?
is_transfer: txn.transfer?
}
end

2
app/models/income_statement/category_stats.rb
View file

@ -47,7 +47,7 @@ class IncomeStatement::CategoryStats
er.to_currency = :target_currency
)
WHERE a.family_id = :family_id
AND t.kind NOT IN ('transfer', 'one_time', 'payment')
AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
GROUP BY c.id, period, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END
)
SELECT

2
app/models/income_statement/family_stats.rb
View file

@ -44,7 +44,7 @@ class IncomeStatement::FamilyStats
er.to_currency = :target_currency
)
WHERE a.family_id = :family_id
AND t.kind NOT IN ('transfer', 'one_time', 'payment')
AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
GROUP BY period, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END
)
SELECT

2
app/models/income_statement/totals.rb
View file

@ -44,7 +44,7 @@ class IncomeStatement::Totals
er.from_currency = ae.currency AND
er.to_currency = :target_currency
)
WHERE at.kind NOT IN ('transfer', 'one_time', 'payment')
WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
GROUP BY c.id, c.parent_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END;
SQL
end

9
app/models/transaction.rb
View file

@ -11,12 +11,17 @@ class Transaction < ApplicationRecord
enum :kind, {
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)
funds_movement: "funds_movement", # Movement of funds between accounts, excluded from budget analytics
cc_payment: "cc_payment", # A CC 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
}
# Overarching grouping method for all transfer-type transactions
def transfer?
funds_movement? || cc_payment? || loan_payment?
end
def set_category!(category)
if category.is_a?(String)
category = entry.account.family.categories.find_or_create_by!(

4
app/models/transaction/search.rb
View file

@ -102,7 +102,7 @@ class Transaction::Search
query = query.left_joins(:category).where(
"categories.name IN (?) OR (
categories.id IS NULL AND (transactions.kind NOT IN ('transfer', 'payment'))
categories.id IS NULL AND (transactions.kind NOT IN ('funds_movement', 'cc_payment'))
)",
categories
)
@ -118,7 +118,7 @@ class Transaction::Search
return query unless types.present?
return query if types.sort == [ "expense", "income", "transfer" ]
transfer_condition = "transactions.kind IN ('transfer', 'payment', 'loan_payment')"
transfer_condition = "transactions.kind IN ('funds_movement', 'cc_payment', 'loan_payment')"
expense_condition = "entries.amount >= 0"
income_condition = "entries.amount <= 0"

8
app/models/transfer.rb
View file

@ -32,10 +32,6 @@ class Transfer < ApplicationRecord
outflow_transaction&.entry&.sync_account_later
end
def belongs_to_family?(family)
family.transactions.include?(inflow_transaction)
end
def to_account
inflow_transaction&.entry&.account
end
@ -66,11 +62,11 @@ class Transfer < ApplicationRecord
end
def liability_payment?
outflow_transaction&.kind == "payment"
outflow_transaction&.kind == "cc_payment"
end
def regular_transfer?
outflow_transaction&.kind == "transfer"
outflow_transaction&.kind == "funds_movement"
end
def transfer_type

6
app/models/transfer/creator.rb
View file

@ -43,7 +43,7 @@ class Transfer::Creator
name = "#{name_prefix} from #{source_account.name}"
Transaction.new(
kind: "transfer",
kind: "funds_movement",
entry: destination_account.entries.build(
amount: inflow_converted_money.amount.abs * -1,
currency: destination_account.currency,
@ -69,9 +69,9 @@ class Transfer::Creator
if destination_account.loan?
"loan_payment"
elsif destination_account.liability?
"payment"
"cc_payment"
else
"transfer"
"funds_movement"
end
end

2
app/views/category/dropdowns/show.html.erb
View file

@ -55,7 +55,7 @@
<% end %>
<% end %>
<% unless @transaction.transfer.present? %>
<% unless @transaction.transfer? %>
<%= 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 %>

2
app/views/transactions/_header.html.erb
View file

@ -12,7 +12,7 @@
</span>
</h3>
<% if entry.transaction.transfer.present? %>
<% if entry.transaction.transfer? %>
<%= icon "arrow-left-right", class: "mt-1" %>
<% end %>

44
app/views/transactions/_transaction.html.erb
View file

@ -10,7 +10,7 @@
<div class="pr-4 lg:pr-10 flex items-center gap-3 lg:gap-4 col-span-8 lg:col-span-6">
<%= check_box_tag dom_id(entry, "selection"),
disabled: transaction.transfer.present?,
disabled: transaction.transfer?,
class: "checkbox checkbox--light",
data: {
id: entry.id,
@ -36,15 +36,27 @@
<div class="truncate">
<div class="space-y-0.5">
<div class="flex items-center gap-1">
<%= link_to(
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
},
class: "hover:underline"
) %>
<% if transaction.transfer? %>
<%= link_to(
entry.name,
transaction_transfer_path(transaction),
data: {
turbo_frame: "drawer",
turbo_prefetch: false
},
class: "hover:underline"
) %>
<% else %>
<%= link_to(
entry.name,
entry_path(entry),
data: {
turbo_frame: "drawer",
turbo_prefetch: false
},
class: "hover:underline"
) %>
<% end %>
<% if entry.excluded %>
<span class="text-orange-500" title="One-time <%= entry.amount.negative? ? "income" : "expense" %> (excluded from averages)">
@ -52,16 +64,16 @@
</span>
<% end %>
<% if transaction.transfer.present? %>
<% if transaction.transfer? %>
<%= render "transactions/transfer_match", transaction: transaction %>
<% end %>
</div>
<div class="text-secondary text-xs font-normal hidden lg:block">
<% if transaction.transfer.present? %>
<%= render "transfers/account_links",
transfer: transaction.transfer,
is_inflow: transaction.transfer_as_inflow.present? %>
<% if transaction.transfer? %>
<span class="text-secondary">
<%= transaction.loan_payment? ? "Loan Payment" : "Transfer" %> • <%= entry.account.name %>
</span>
<% else %>
<%= link_to entry.account.name,
account_path(entry.account, tab: "transactions", focused_record_id: entry.id),
@ -81,7 +93,7 @@
<div class="col-span-4 lg:col-span-2 ml-auto text-right">
<%= content_tag :p,
transaction.transfer.present? && view_ctx == "global" ? "+/- #{format_money(entry.amount_money.abs)}" : format_money(-entry.amount_money),
transaction.transfer? && view_ctx == "global" ? "+/- #{format_money(entry.amount_money.abs)}" : format_money(-entry.amount_money),
class: ["text-green-600": entry.amount.negative?] %>
</div>

2
app/views/transactions/index.html.erb
View file

@ -43,7 +43,7 @@
</div>
</header>
<%= render "summary", totals: @totals %>
<%= render "summary", totals: @search.totals %>
<div id="transactions"
data-controller="bulk-select"

4
app/views/transactions/show.html.erb
View file

@ -21,7 +21,7 @@
disabled: @entry.linked?,
"data-auto-submit-form-target": "auto" %>
<% unless @entry.transaction.transfer.present? %>
<% unless @entry.transaction.transfer? %>
<div class="flex items-center gap-2">
<%= f.select :nature,
[["Expense", "outflow"], ["Income", "inflow"]],
@ -57,7 +57,7 @@
url: transaction_path(@entry),
class: "space-y-2",
data: { controller: "auto-submit-form" } do |f| %>
<% unless @entry.transaction.transfer.present? %>
<% unless @entry.transaction.transfer? %>
<%= f.select :account,
options_for_select(
Current.family.accounts.alphabetically.pluck(:name, :id),

1
config/routes.rb
View file

@ -124,6 +124,7 @@ Rails.application.routes.draw do
end
resources :transactions, only: %i[index new create show update destroy] do
resource :transfer, only: :show
resource :transfer_match, only: %i[new create]
resource :category, only: :update, controller: :transaction_categories

4
db/migrate/20250616183654_add_kind_to_transactions.rb
View file

@ -10,8 +10,8 @@ class AddKindToTransactions < ActiveRecord::Migration[7.2]
UPDATE transactions
SET kind = CASE
WHEN destination_accounts.accountable_type = 'Loan' AND entries.amount > 0 THEN 'loan_payment'
WHEN destination_accounts.accountable_type IN ('CreditCard', 'OtherLiability') AND entries.amount > 0 THEN 'payment'
ELSE 'transfer'
WHEN destination_accounts.accountable_type = 'CreditCard' AND entries.amount > 0 THEN 'cc_payment'
ELSE 'funds_movement'
END
FROM transfers t
JOIN entries ON (

3
test/controllers/transactions_controller_test.rb
View file

@ -162,7 +162,8 @@ end
income_money: Money.new(0, "USD")
)
Transaction::Search.expects(:new).with(family, filters: {}).returns(search)
expected_filters = { "start_date" => 30.days.ago.to_date }
Transaction::Search.expects(:new).with(family, filters: expected_filters).returns(search)
search.expects(:totals).once.returns(totals)
get transactions_url

6
test/models/income_statement_test.rb
View file

@ -153,8 +153,8 @@ class IncomeStatementTest < ActiveSupport::TestCase
# NOTE: These tests now pass because kind filtering is working after the refactoring!
test "excludes regular transfers from income statement calculations" do
# Create a regular transfer between accounts
outflow_transaction = create_transaction(account: @checking_account, amount: 500, kind: "transfer")
inflow_transaction = create_transaction(account: @credit_card_account, amount: -500, kind: "transfer")
outflow_transaction = create_transaction(account: @checking_account, amount: 500, kind: "funds_movement")
inflow_transaction = create_transaction(account: @credit_card_account, amount: -500, kind: "funds_movement")
income_statement = IncomeStatement.new(@family)
totals = income_statement.totals
@ -193,7 +193,7 @@ class IncomeStatementTest < ActiveSupport::TestCase
test "excludes payment transactions from income statement calculations" do
# Create a payment transaction (credit card payment)
payment_transaction = create_transaction(account: @checking_account, amount: 300, category: nil, kind: "payment")
payment_transaction = create_transaction(account: @checking_account, amount: 300, category: nil, kind: "cc_payment")
income_statement = IncomeStatement.new(@family)
totals = income_statement.totals

10
test/models/transaction/search_test.rb
View file

@ -25,13 +25,13 @@ class Transaction::SearchTest < ActiveSupport::TestCase
transfer_entry = create_transaction(
account: @checking_account,
amount: 200,
kind: "transfer"
kind: "funds_movement"
)
payment_entry = create_transaction(
account: @credit_card_account,
amount: -300,
kind: "payment"
kind: "cc_payment"
)
loan_payment_entry = create_transaction(
@ -104,7 +104,7 @@ class Transaction::SearchTest < ActiveSupport::TestCase
uncategorized_transfer = create_transaction(
account: @checking_account,
amount: 200,
kind: "transfer"
kind: "funds_movement"
)
uncategorized_loan_payment = create_transaction(
@ -138,7 +138,7 @@ class Transaction::SearchTest < ActiveSupport::TestCase
transaction2 = create_transaction(
account: @checking_account,
amount: 200,
kind: "transfer"
kind: "funds_movement"
)
# Test new family-based API
@ -153,7 +153,7 @@ class Transaction::SearchTest < ActiveSupport::TestCase
# Test that the relation builds from family.transactions correctly
assert_equal @family.transactions.joins(entry: :account).where(
"entries.amount >= 0 AND NOT (transactions.kind IN ('transfer', 'payment', 'loan_payment'))"
"entries.amount >= 0 AND NOT (transactions.kind IN ('funds_movement', 'cc_payment', 'loan_payment'))"
).count, results.count
end

14
test/models/transfer/creator_test.rb
View file

@ -27,14 +27,14 @@ class Transfer::CreatorTest < ActiveSupport::TestCase
# Verify outflow transaction (from source account)
outflow = transfer.outflow_transaction
assert_equal "transfer", outflow.kind
assert_equal "funds_movement", 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 "funds_movement", 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
@ -60,12 +60,12 @@ class Transfer::CreatorTest < ActiveSupport::TestCase
# Verify outflow transaction
outflow = transfer.outflow_transaction
assert_equal "transfer", outflow.kind
assert_equal "funds_movement", 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 "funds_movement", inflow.kind
assert_equal "Transfer from #{@source_account.name}", inflow.entry.name
assert_equal crypto_account.currency, inflow.entry.currency
end
@ -94,7 +94,7 @@ class Transfer::CreatorTest < ActiveSupport::TestCase
# Verify inflow transaction
inflow = transfer.inflow_transaction
assert_equal "transfer", inflow.kind
assert_equal "funds_movement", inflow.kind
assert_equal "Payment from #{@source_account.name}", inflow.entry.name
end
@ -117,12 +117,12 @@ class Transfer::CreatorTest < ActiveSupport::TestCase
# Verify outflow transaction is marked as payment for liability
outflow = transfer.outflow_transaction
assert_equal "payment", outflow.kind
assert_equal "cc_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 "funds_movement", inflow.kind
assert_equal "Payment from #{@source_account.name}", inflow.entry.name
end