From ae41b3de46fc07ac91101b4600247352967ebe6f Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Wed, 30 Apr 2025 14:20:09 -0500 Subject: [PATCH 1/3] Refactor Transfer model to use safe navigation for entry associations and improve error messages for account validation --- app/models/transfer.rb | 46 +++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/app/models/transfer.rb b/app/models/transfer.rb index 601966bb..20804ddd 100644 --- a/app/models/transfer.rb +++ b/app/models/transfer.rb @@ -56,8 +56,8 @@ class Transfer < ApplicationRecord end def sync_account_later - inflow_transaction.entry.sync_account_later - outflow_transaction.entry.sync_account_later + inflow_transaction&.entry&.sync_account_later + outflow_transaction&.entry&.sync_account_later end def belongs_to_family?(family) @@ -65,63 +65,67 @@ class Transfer < ApplicationRecord end def to_account - inflow_transaction.entry.account + inflow_transaction&.entry&.account end def from_account - outflow_transaction.entry.account + outflow_transaction&.entry&.account end def amount_abs - inflow_transaction.entry.amount_money.abs + inflow_transaction&.entry&.amount_money&.abs end def name + acc = to_account if payment? - I18n.t("transfer.payment_name", to_account: to_account.name) + acc ? "Payment to #{acc.name}" : "Payment" else - I18n.t("transfer.name", to_account: to_account.name) + acc ? "Transfer to #{acc.name}" : "Transfer" end end def payment? - to_account.liability? + to_account&.liability? end def categorizable? - to_account.accountable_type == "Loan" + to_account&.accountable_type == "Loan" end private def transfer_has_different_accounts - return unless inflow_transaction.present? && outflow_transaction.present? - errors.add(:base, :must_be_from_different_accounts) if inflow_transaction.entry.account == outflow_transaction.entry.account + return unless inflow_transaction&.entry && outflow_transaction&.entry + errors.add(:base, "Must be from different accounts") if to_account == from_account end def transfer_has_same_family - return unless inflow_transaction.present? && outflow_transaction.present? - errors.add(:base, :must_be_from_same_family) unless inflow_transaction.entry.account.family == outflow_transaction.entry.account.family + return unless inflow_transaction&.entry && outflow_transaction&.entry + errors.add(:base, "Must be from same family") unless to_account&.family == from_account&.family end def transfer_has_opposite_amounts - return unless inflow_transaction.present? && outflow_transaction.present? + return unless inflow_transaction&.entry && outflow_transaction&.entry - inflow_amount = inflow_transaction.entry.amount - outflow_amount = outflow_transaction.entry.amount + inflow_entry = inflow_transaction.entry + outflow_entry = outflow_transaction.entry - if inflow_transaction.entry.currency == outflow_transaction.entry.currency + inflow_amount = inflow_entry.amount + outflow_amount = outflow_entry.amount + + if inflow_entry.currency == outflow_entry.currency # For same currency, amounts must be exactly opposite - errors.add(:base, :must_have_opposite_amounts) if inflow_amount + outflow_amount != 0 + errors.add(:base, "Must have opposite amounts") if inflow_amount + outflow_amount != 0 else # For different currencies, just check the signs are opposite - errors.add(:base, :must_have_opposite_amounts) unless inflow_amount.negative? && outflow_amount.positive? + errors.add(:base, "Must have opposite amounts") unless inflow_amount.negative? && outflow_amount.positive? end end def transfer_within_date_range - return unless inflow_transaction.present? && outflow_transaction.present? + return unless inflow_transaction&.entry && outflow_transaction&.entry date_diff = (inflow_transaction.entry.date - outflow_transaction.entry.date).abs - errors.add(:base, :must_be_within_date_range) if date_diff > 4 + errors.add(:base, "Must be within 4 days") if date_diff > 4 end end From 47017a64326354a1e5f817f9ac65c159b489f0de Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Wed, 30 Apr 2025 14:29:33 -0500 Subject: [PATCH 2/3] Enhance account links in transfers view to handle missing accounts gracefully. Added conditional checks to display a warning message when account data is unavailable. --- app/views/transfers/_account_links.html.erb | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/app/views/transfers/_account_links.html.erb b/app/views/transfers/_account_links.html.erb index a71b7de0..f7ba4365 100644 --- a/app/views/transfers/_account_links.html.erb +++ b/app/views/transfers/_account_links.html.erb @@ -1,7 +1,24 @@ <%# locals: (transfer:, is_inflow: false) %>
<% first_account, second_account = is_inflow ? [transfer.to_account, transfer.from_account] : [transfer.from_account, transfer.to_account] %> - <%= link_to first_account.name, account_path(first_account, tab: "activity"), class: "hover:underline", data: { turbo_frame: "_top" } %> + + <%# Check if first_account exists before creating link %> + <% if first_account %> + <%= link_to first_account.name, account_path(first_account, tab: "activity"), class: "hover:underline", data: { turbo_frame: "_top" } %> + <% else %> + + Data Error: Missing account + + <% end %> + <%= lucide_icon is_inflow ? "arrow-left" : "arrow-right", class: "w-4 h-4 shrink-0" %> - <%= link_to second_account.name, account_path(second_account, tab: "activity"), class: "hover:underline", data: { turbo_frame: "_top" } %> + + <%# Check if second_account exists before creating link %> + <% if second_account %> + <%= link_to second_account.name, account_path(second_account, tab: "activity"), class: "hover:underline", data: { turbo_frame: "_top" } %> + <% else %> + + Data Error: Missing account + + <% end %>
From 1aafed5f8b85eae5eceb3685b3bbe62fb6d853c6 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Wed, 30 Apr 2025 14:45:39 -0500 Subject: [PATCH 3/3] Update error messages in Transfer model tests for clarity and conciseness --- test/models/transfer_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/models/transfer_test.rb b/test/models/transfer_test.rb index 2bc190f0..36dc56d4 100644 --- a/test/models/transfer_test.rb +++ b/test/models/transfer_test.rb @@ -39,7 +39,7 @@ class TransferTest < ActiveSupport::TestCase transfer.save end - assert_equal "Transfer must have different accounts", transfer.errors.full_messages.first + assert_equal "Must be from different accounts", transfer.errors.full_messages.first end test "Transfer transactions must have opposite amounts" do @@ -55,7 +55,7 @@ class TransferTest < ActiveSupport::TestCase transfer.save end - assert_equal "Transfer transactions must have opposite amounts", transfer.errors.full_messages.first + assert_equal "Must have opposite amounts", transfer.errors.full_messages.first end test "transfer dates must be within 4 days of each other" do @@ -71,7 +71,7 @@ class TransferTest < ActiveSupport::TestCase transfer.save end - assert_equal "Transfer transaction dates must be within 4 days of each other", transfer.errors.full_messages.first + assert_equal "Must be within 4 days", transfer.errors.full_messages.first end test "transfer must be from the same family" do @@ -90,7 +90,7 @@ class TransferTest < ActiveSupport::TestCase ) assert transfer.invalid? - assert_equal "Transfer must be from the same family", transfer.errors.full_messages.first + assert_equal "Must be from same family", transfer.errors.full_messages.first end test "from_accounts converts amounts to the to_account's currency" do