QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-07-15 06:15:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix fee display consistency, derive fees from entries, clean schema churn

Browse Source 
- Show principal-only transfer amounts on both sides with separate fee and total lines (fixes inconsistent gross/net convention)
- Derive displayed fee amounts from fee_transactions entries (single source of truth) instead of stored columns
- Remove stored source_fee_amount/destination_fee_amount columns from transfers table
- Add foreign key for transactions.transfer_id -> transfers.id (replaces invalid CHECK subquery)
- Move destination fee line inside destination side div for consistent layout
- Remove orphaned view_fee_transaction locale keys from 7 locale files
- Rebuild schema.rb from origin/main to eliminate unrelated column reordering churn
This commit is contained in:
DataEnginr
2026-06-28 19:47:53 +00:00
parent 75234dab19
commit e75f2a0c78
18 changed files with 924 additions and 1049 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
app/components/DS/button.rb
View File

@@ -25,7 +25,8 @@ class DS::Button < DS::Buttonish
data = merged_opts.delete(:data) || {}
if confirm.present?
data = data.merge(turbo_confirm: confirm.to_data_attribute)
confirm_value = confirm.respond_to?(:to_data_attribute) ? confirm.to_data_attribute : confirm
data = data.merge(turbo_confirm: confirm_value)
end
if frame.present?

22
app/controllers/transfers_controller.rb
View File

@@ -197,25 +197,21 @@ class TransfersController < ApplicationController
new_source_fee = transfer_update_params[:source_fee_amount]
new_destination_fee = transfer_update_params[:destination_fee_amount]
current_source_fee = @transfer.derived_source_fee_amount
current_destination_fee = @transfer.derived_destination_fee_amount
source_fee_changed = new_source_fee.present? && new_source_fee.to_d != current_source_fee
dest_fee_changed = new_destination_fee.present? && new_destination_fee.to_d != current_destination_fee
amount_changed = new_amount.present? && new_amount.to_d != @transfer.amount.to_d
source_fee_changed = new_source_fee.present? && new_source_fee.to_d != @transfer.source_fee_amount.to_d
dest_fee_changed = new_destination_fee.present? && new_destination_fee.to_d != @transfer.destination_fee_amount.to_d
return unless amount_changed || source_fee_changed || dest_fee_changed
@transfer.amount = new_amount.to_d if amount_changed
@transfer.source_fee_amount = new_source_fee.to_d if source_fee_changed
@transfer.destination_fee_amount = new_destination_fee.to_d if dest_fee_changed
# Recompute outflow entry (always principal only)
if amount_changed
outflow_entry = @transfer.outflow_transaction.entry
outflow_entry.amount = @transfer.amount
outflow_entry.save!
end
# Recompute inflow entry (always principal converted, no fee baked in)
if amount_changed
inflow_entry = @transfer.inflow_transaction.entry
converted = Money.new(@transfer.amount, @transfer.from_account.currency)
.exchange_to(@transfer.to_account.currency, date: @transfer.date)
@@ -223,22 +219,20 @@ class TransfersController < ApplicationController
inflow_entry.save!
end
# Update source fee transaction
if source_fee_changed
update_fee_transaction(
account: @transfer.from_account,
old_fee: @transfer.source_fee_amount_before_last_save || @transfer.source_fee_amount,
new_fee: @transfer.source_fee_amount,
old_fee: current_source_fee,
new_fee: new_source_fee.to_d,
name: "Transfer fee — #{@transfer.name}"
)
end
# Update destination fee transaction
if dest_fee_changed
update_fee_transaction(
account: @transfer.to_account,
old_fee: @transfer.destination_fee_amount_before_last_save || @transfer.destination_fee_amount,
new_fee: @transfer.destination_fee_amount,
old_fee: current_destination_fee,
new_fee: new_destination_fee.to_d,
name: "Transfer fee — #{@transfer.name}"
)
end

14
app/models/transfer.rb
View File

@@ -4,6 +4,8 @@ class Transfer < ApplicationRecord
has_many :fee_transactions, class_name: "Transaction", dependent: :destroy
attr_accessor :source_fee_amount, :destination_fee_amount
enum :status, { pending: "pending", confirmed: "confirmed" }
validates :inflow_transaction_id, uniqueness: true
@@ -13,7 +15,6 @@ class Transfer < ApplicationRecord
validate :transfer_has_opposite_amounts_or_fees
validate :transfer_within_date_range
validate :transfer_has_same_family
validate :fees_must_be_non_negative
class << self
def kind_for_account(account)
@@ -48,13 +49,11 @@ class Transfer < ApplicationRecord
end
def derived_source_fee_amount
from_fee = fee_transactions.joins(:entry).where(entries: { account_id: from_account.id }).sum("entries.amount")
from_fee > 0 ? from_fee : source_fee_amount.to_d
fee_transactions.joins(:entry).where(entries: { account_id: from_account.id }).sum("entries.amount")
end
def derived_destination_fee_amount
to_fee = fee_transactions.joins(:entry).where(entries: { account_id: to_account.id }).sum("entries.amount")
to_fee > 0 ? to_fee : destination_fee_amount.to_d
fee_transactions.joins(:entry).where(entries: { account_id: to_account.id }).sum("entries.amount")
end
def amount_abs
@@ -168,11 +167,6 @@ class Transfer < ApplicationRecord
end
end
def fees_must_be_non_negative
errors.add(:source_fee_amount, :greater_than_or_equal_to, count: 0) if source_fee_amount.to_d.negative?
errors.add(:destination_fee_amount, :greater_than_or_equal_to, count: 0) if destination_fee_amount.to_d.negative?
end
def transfer_within_date_range
return unless inflow_transaction&.entry && outflow_transaction&.entry

7
app/models/transfer/creator.rb
View File

@@ -18,13 +18,14 @@ class Transfer::Creator
end
def create
raise ArgumentError, "source_fee_amount must be non-negative" if source_fee_amount.negative?
raise ArgumentError, "destination_fee_amount must be non-negative" if destination_fee_amount.negative?
transfer = Transfer.new(
inflow_transaction: inflow_transaction,
outflow_transaction: outflow_transaction,
status: "confirmed",
amount: amount,
source_fee_amount: source_fee_amount,
destination_fee_amount: destination_fee_amount
amount: amount
)
Transfer.transaction do

4
app/views/api/v1/transfers/_transfer.json.jbuilder
View File

@@ -8,9 +8,9 @@ json.amount_cents money_to_minor_units(transfer.amount_abs)
json.currency transfer.inflow_transaction.entry.currency
json.transfer_type transfer.transfer_type
json.notes transfer.notes
json.source_fee_amount transfer.source_fee_amount.to_s("F")
json.source_fee_amount transfer.derived_source_fee_amount.to_s("F")
json.source_fee_currency transfer.from_account&.currency
json.destination_fee_amount transfer.destination_fee_amount.to_s("F")
json.destination_fee_amount transfer.derived_destination_fee_amount.to_s("F")
json.destination_fee_currency transfer.to_account&.currency
json.inflow_transaction do

27
app/views/transfers/show.html.erb
View File

@@ -42,11 +42,6 @@
<dt class="text-secondary"><%= t(".source_fee") %></dt>
<dd class="font-medium text-destructive privacy-sensitive"><%= format_money Money.new(@transfer.derived_source_fee_amount, @transfer.from_account.currency) * -1 %></dd>
</dl>
<div class="text-xs text-secondary text-right">
<% @transfer.fee_transactions.select { |t| t.entry.account_id == @transfer.from_account.id }.each do |fee_tx| %>
<%= link_to t(".view_fee_transaction"), transaction_path(fee_tx), data: { turbo_frame: "_top" }, class: "underline" %>
<% end %>
</div>
<dl class="flex items-center gap-2 justify-between pt-1 border-t border-alpha-black-50">
<dt class="text-secondary font-medium"><%= t(".total") %></dt>
<dd class="font-medium text-destructive privacy-sensitive">
@@ -55,18 +50,6 @@
</dl>
<% end %>
</div>
<% if @transfer.has_destination_fee? %>
<dl class="flex items-center gap-2 justify-between">
<dt class="text-secondary"><%= t(".destination_fee") %></dt>
<dd class="font-medium text-destructive privacy-sensitive"><%= format_money Money.new(@transfer.derived_destination_fee_amount, @transfer.to_account.currency) * -1 %></dd>
</dl>
<div class="text-xs text-secondary text-right">
<% @transfer.fee_transactions.select { |t| t.entry.account_id == @transfer.to_account.id }.each do |fee_tx| %>
<%= link_to t(".view_fee_transaction"), transaction_path(fee_tx), data: { turbo_frame: "_top" }, class: "underline" %>
<% end %>
</div>
<% end %>
<%= render "shared/ruler", classes: "my-2" %>
<div class="space-y-3">
<dl class="flex items-center gap-2 justify-between">
@@ -85,6 +68,10 @@
<dd class="font-medium text-success privacy-sensitive">+<%= format_money @transfer.inflow_transaction.entry.amount_money * -1 %></dd>
</dl>
<% if @transfer.has_destination_fee? %>
<dl class="flex items-center gap-2 justify-between">
<dt class="text-secondary"><%= t(".destination_fee") %></dt>
<dd class="font-medium text-destructive privacy-sensitive"><%= format_money Money.new(@transfer.derived_destination_fee_amount, @transfer.to_account.currency) * -1 %></dd>
</dl>
<dl class="flex items-center gap-2 justify-between pt-1 border-t border-alpha-black-50">
<dt class="text-secondary font-medium"><%= t(".total") %></dt>
<dd class="font-medium text-success privacy-sensitive">+<%= format_money (@transfer.inflow_transaction.entry.amount_money * -1) - Money.new(@transfer.derived_destination_fee_amount, @transfer.to_account.currency) %></dd>
@@ -135,7 +122,11 @@
size: :md,
href: transfer_path(@transfer),
method: :delete,
confirm: true,
confirm: CustomConfirm.new(
title: t(".delete_title"),
body: t(".delete_subtitle"),
destructive: true
),
frame: "_top"
) %>
</div>

1
config/locales/views/transfers/ca.yml
View File

@@ -43,6 +43,5 @@ ca:
total: Total
transfer_amount: Import de la transferència
uncategorized: Sense categoria
view_fee_transaction: Veure transacció de comissió
update:
success: Transferència actualitzada

1
config/locales/views/transfers/en.yml
View File

@@ -48,7 +48,6 @@ en:
bank_charges: Bank Charges
source_fee: Source fee
destination_fee: Destination fee
view_fee_transaction: View fee transaction
category: Category
uncategorized: Uncategorized
update:

1
config/locales/views/transfers/es.yml
View File

@@ -32,6 +32,5 @@ es:
settings: Configuración
total: Total
transfer_amount: Importe de la transferencia
view_fee_transaction: Ver transacción de comisión
update:
success: Transferencia actualizada

1
config/locales/views/transfers/fr.yml
View File

@@ -32,6 +32,5 @@ fr:
settings: Paramètres
total: Total
transfer_amount: Montant du transfert
view_fee_transaction: Voir la transaction de frais
update:
success: Transfert mis à jour

1
config/locales/views/transfers/hu.yml
View File

@@ -41,6 +41,5 @@ hu:
uncategorized: Kategorizálatlan
total: '[Total]'
transfer_amount: '[Transfer amount]'
view_fee_transaction: '[View fee transaction]'
update:
success: Átutalás frissítve

1
config/locales/views/transfers/vi.yml
View File

@@ -41,6 +41,5 @@ vi:
uncategorized: Chưa phân loại
total: '[Total]'
transfer_amount: '[Transfer amount]'
view_fee_transaction: '[View fee transaction]'
update:
success: Chuyển khoản đã được cập nhật

1
config/locales/views/transfers/zh-CN.yml
View File

@@ -45,6 +45,5 @@ zh-CN:
uncategorized: 未分类
total: 总计
transfer_amount: 转账金额
view_fee_transaction: 查看费用交易
update:
success: 转账已更新

8
db/migrate/20260628200000_remove_fee_amounts_from_transfers.rb Normal file
View File

@@ -0,0 +1,8 @@
class RemoveFeeAmountsFromTransfers < ActiveRecord::Migration[7.2]
def change
remove_check_constraint :transfers, name: "check_source_fee_non_negative"
remove_check_constraint :transfers, name: "check_destination_fee_non_negative"
remove_column :transfers, :source_fee_amount, :decimal
remove_column :transfers, :destination_fee_amount, :decimal
end
end

1745
db/schema.rb generated
View File

File diff suppressed because it is too large Load Diff

25
test/controllers/transfers_controller_test.rb
View File

@@ -164,9 +164,9 @@ class TransfersControllerTest < ActionDispatch::IntegrationTest
end
transfer = Transfer.order(created_at: :desc).first
assert_equal 3, transfer.source_fee_amount
assert_equal 0, transfer.destination_fee_amount
assert_equal 100, transfer.amount
assert_equal 3, transfer.derived_source_fee_amount
assert_equal 0, transfer.derived_destination_fee_amount
# Outflow should be principal only (no fee baked in)
assert_equal 100, transfer.outflow_transaction.entry.amount
# Inflow should be -(converted_principal)
@@ -177,9 +177,6 @@ class TransfersControllerTest < ActionDispatch::IntegrationTest
assert_equal "standard", fee_tx.kind
assert_equal 3, fee_tx.entry.amount
assert_equal accounts(:depository).id, fee_tx.entry.account_id
# Derived fee methods match stored amounts
assert_equal 3, transfer.derived_source_fee_amount
assert_equal 0, transfer.derived_destination_fee_amount
assert transfer.has_source_fee?
assert_not transfer.has_destination_fee?
end
@@ -198,9 +195,9 @@ class TransfersControllerTest < ActionDispatch::IntegrationTest
end
transfer = Transfer.order(created_at: :desc).first
assert_equal 0, transfer.source_fee_amount
assert_equal 3, transfer.destination_fee_amount
assert_equal 100, transfer.amount
assert_equal 0, transfer.derived_source_fee_amount
assert_equal 3, transfer.derived_destination_fee_amount
# Outflow should be principal only
assert_equal 100, transfer.outflow_transaction.entry.amount
# Inflow should be -(converted_principal)
@@ -211,9 +208,6 @@ class TransfersControllerTest < ActionDispatch::IntegrationTest
assert_equal "standard", fee_tx.kind
assert_equal 3, fee_tx.entry.amount
assert_equal accounts(:credit_card).id, fee_tx.entry.account_id
# Derived fee methods match stored amounts
assert_equal 0, transfer.derived_source_fee_amount
assert_equal 3, transfer.derived_destination_fee_amount
assert_not transfer.has_source_fee?
assert transfer.has_destination_fee?
end
@@ -233,9 +227,9 @@ class TransfersControllerTest < ActionDispatch::IntegrationTest
end
transfer = Transfer.order(created_at: :desc).first
assert_equal 2, transfer.source_fee_amount
assert_equal 3, transfer.destination_fee_amount
assert_equal 100, transfer.amount
assert_equal 2, transfer.derived_source_fee_amount
assert_equal 3, transfer.derived_destination_fee_amount
# Outflow should be principal only
assert_equal 100, transfer.outflow_transaction.entry.amount
# Inflow should be -(converted_principal)
@@ -246,9 +240,6 @@ class TransfersControllerTest < ActionDispatch::IntegrationTest
dest_fee_tx = transfer.fee_transactions.find { |t| t.entry.account_id == accounts(:credit_card).id }
assert_equal 2, source_fee_tx.entry.amount
assert_equal 3, dest_fee_tx.entry.amount
# Derived fee methods match stored amounts
assert_equal 2, transfer.derived_source_fee_amount
assert_equal 3, transfer.derived_destination_fee_amount
assert transfer.has_fees?
end
@@ -270,11 +261,9 @@ class TransfersControllerTest < ActionDispatch::IntegrationTest
fee_tx = transfer.fee_transactions.first
fee_tx.entry.update!(amount: 5)
# Derived fee should reflect the updated entry, not the stored column
# Derived fee should reflect the updated entry
transfer.reload
assert_equal 5, transfer.derived_source_fee_amount
# Stored column remains unchanged
assert_equal 3, transfer.source_fee_amount
assert transfer.has_source_fee?
end

107
test/models/transfer_test.rb
View File

@@ -125,70 +125,18 @@ class TransferTest < ActiveSupport::TestCase
assert_equal "funds_movement", Transfer.kind_for_account(accounts(:depository))
end
test "transfer with source fee adjusts validation" do
outflow_entry = create_transaction(date: Date.current, account: accounts(:depository), amount: 100)
inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -100)
transfer = Transfer.new(
inflow_transaction: inflow_entry.transaction,
outflow_transaction: outflow_entry.transaction,
source_fee_amount: 3
)
assert transfer.valid?
end
test "transfer with destination fee adjusts validation" do
outflow_entry = create_transaction(date: Date.current, account: accounts(:depository), amount: 100)
inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -100)
transfer = Transfer.new(
inflow_transaction: inflow_entry.transaction,
outflow_transaction: outflow_entry.transaction,
destination_fee_amount: 3
)
assert transfer.valid?
end
test "transfer with both source and destination fees adjusts validation" do
outflow_entry = create_transaction(date: Date.current, account: accounts(:depository), amount: 100)
inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -100)
transfer = Transfer.new(
inflow_transaction: inflow_entry.transaction,
outflow_transaction: outflow_entry.transaction,
source_fee_amount: 3,
destination_fee_amount: 6
)
assert transfer.valid?
end
test "transfer with non-opposite entries fails validation" do
outflow_entry = create_transaction(date: Date.current, account: accounts(:depository), amount: 100)
inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -95)
transfer = Transfer.new(
inflow_transaction: inflow_entry.transaction,
outflow_transaction: outflow_entry.transaction,
source_fee_amount: 3
)
assert transfer.invalid?
assert_equal "Must have opposite amounts", transfer.errors.full_messages.first
end
test "has_source_fee? returns true when source fee present" do
transfer = transfers(:one)
transfer.update_column(:source_fee_amount, 5)
entry = accounts(:depository).entries.create!(name: "Fee", date: Date.current, amount: 5, currency: "USD", entryable: Transaction.new(kind: "standard"))
transfer.fee_transactions << entry.entryable
assert transfer.has_source_fee?
assert transfer.has_fees?
end
test "has_destination_fee? returns true when destination fee present" do
transfer = transfers(:one)
transfer.update_column(:destination_fee_amount, 5)
entry = accounts(:credit_card).entries.create!(name: "Fee", date: Date.current, amount: 5, currency: "USD", entryable: Transaction.new(kind: "standard"))
transfer.fee_transactions << entry.entryable
assert transfer.has_destination_fee?
assert transfer.has_fees?
end
@@ -200,50 +148,9 @@ class TransferTest < ActiveSupport::TestCase
test "total_fee sums source and destination fees" do
transfer = transfers(:one)
transfer.update_columns(source_fee_amount: 3, destination_fee_amount: 2)
entry1 = accounts(:depository).entries.create!(name: "Fee", date: Date.current, amount: 3, currency: "USD", entryable: Transaction.new(kind: "standard"))
entry2 = accounts(:credit_card).entries.create!(name: "Fee", date: Date.current, amount: 2, currency: "USD", entryable: Transaction.new(kind: "standard"))
transfer.fee_transactions << entry1.entryable << entry2.entryable
assert_equal 5, transfer.total_fee
end
test "destination fee larger than amount inverts inflow sign and fails validation" do
outflow_entry = create_transaction(date: Date.current, account: accounts(:depository), amount: 100)
inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: 50)
transfer = Transfer.new(
inflow_transaction: inflow_entry.transaction,
outflow_transaction: outflow_entry.transaction,
destination_fee_amount: 150
)
# inflow amount (50) is positive, which means destination is also outflowing
assert transfer.invalid?
assert_equal "Must have opposite amounts", transfer.errors.full_messages.first
end
test "negative source fee is rejected" do
outflow_entry = create_transaction(date: Date.current, account: accounts(:depository), amount: 500)
inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -500)
transfer = Transfer.new(
inflow_transaction: inflow_entry.transaction,
outflow_transaction: outflow_entry.transaction,
source_fee_amount: -5
)
assert transfer.invalid?
assert_includes transfer.errors.full_messages, "Source fee amount must be greater than or equal to 0"
end
test "negative destination fee is rejected" do
outflow_entry = create_transaction(date: Date.current, account: accounts(:depository), amount: 500)
inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -500)
transfer = Transfer.new(
inflow_transaction: inflow_entry.transaction,
outflow_transaction: outflow_entry.transaction,
destination_fee_amount: -5
)
assert transfer.invalid?
assert_includes transfer.errors.full_messages, "Destination fee amount must be greater than or equal to 0"
end
end

4
test/support/entries_test_helper.rb
View File

@@ -59,9 +59,7 @@ module EntriesTestHelper
transfer = Transfer.create!(
outflow_transaction: outflow_transaction,
inflow_transaction: inflow_transaction,
amount: amount.abs,
source_fee_amount: source_fee_amount,
destination_fee_amount: destination_fee_amount
amount: amount.abs
)
from_account.entries.create!(