Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: ActiveRecord::RecordInvalid (MAYBE-RAILS-DM) #1801

Closed
wants to merge 1 commit into from
Closed

Fix: ActiveRecord::RecordInvalid (MAYBE-RAILS-DM) #1801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 15 additions & 142 deletions app/models/account.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,134 +32,6 @@ class Account < ApplicationRecord

accepts_nested_attributes_for :accountable, update_only: true

class << self
def by_group(period: Period.all, currency: Money.default_currency.iso_code)
grouped_accounts = { assets: ValueGroup.new("Assets", currency), liabilities: ValueGroup.new("Liabilities", currency) }

Accountable.by_classification.each do |classification, types|
types.each do |type|
accounts = self.where(accountable_type: type)
if accounts.any?
group = grouped_accounts[classification.to_sym].add_child_group(type, currency)
accounts.each do |account|
group.add_value_node(
account,
account.balance_money.exchange_to(currency, fallback_rate: 0),
account.series(period: period, currency: currency)
)
end
end
end
end

grouped_accounts
end

def create_and_sync(attributes)
attributes[:accountable_attributes] ||= {} # Ensure accountable is created, even if empty
account = new(attributes.merge(cash_balance: attributes[:balance]))

transaction do
# Create 2 valuations for new accounts to establish a value history for users to see
account.entries.build(
name: "Current Balance",
date: Date.current,
amount: account.balance,
currency: account.currency,
entryable: Account::Valuation.new
)
account.entries.build(
name: "Initial Balance",
date: 1.day.ago.to_date,
amount: 0,
currency: account.currency,
entryable: Account::Valuation.new
)

account.save!
end

account.sync_later
account
end
end

def destroy_later
update!(scheduled_for_deletion: true)
DestroyJob.perform_later(self)
end

def sync_data(start_date: nil)
update!(last_synced_at: Time.current)

Syncer.new(self, start_date: start_date).run
end

def post_sync
broadcast_remove_to(family, target: "syncing-notice")
resolve_stale_issues
accountable.post_sync
end

def series(period: Period.last_30_days, currency: nil)
balance_series = balances.in_period(period).where(currency: currency || self.currency)

if balance_series.empty? && period.date_range.end == Date.current
TimeSeries.new([ { date: Date.current, value: balance_money.exchange_to(currency || self.currency) } ])
else
TimeSeries.from_collection(balance_series, :balance_money, favorable_direction: asset? ? "up" : "down")
end
rescue Money::ConversionError
TimeSeries.new([])
end

def original_balance
balance_amount = balances.chronological.first&.balance || balance
Money.new(balance_amount, currency)
end

def current_holdings
holdings.where(currency: currency, date: holdings.maximum(:date)).order(amount: :desc)
end

def favorable_direction
classification == "asset" ? "up" : "down"
end

def enrich_data
DataEnricher.new(self).run
end

def enrich_data_later
EnrichDataJob.perform_later(self)
end

def update_with_sync!(attributes)
should_update_balance = attributes[:balance] && attributes[:balance].to_d != balance

transaction do
update!(attributes)
update_balance!(attributes[:balance]) if should_update_balance
end

sync_later
end

def update_balance!(balance)
valuation = entries.account_valuations.find_by(date: Date.current)

if valuation
valuation.update! amount: balance
else
entries.create! \
date: Date.current,
name: "Balance update",
amount: balance,
currency: currency,
entryable: Account::Valuation.new
end
end

def transfer_match_candidates
Account::Entry.select([
"inflow_candidates.entryable_id as inflow_transaction_id",
Expand All @@ -177,33 +49,32 @@ def transfer_match_candidates
)
").joins("
LEFT JOIN transfers existing_transfers ON (
existing_transfers.inflow_transaction_id = inflow_candidates.entryable_id OR
existing_transfers.outflow_transaction_id = outflow_candidates.entryable_id
existing_transfers.inflow_transaction_id IN (inflow_candidates.entryable_id, outflow_candidates.entryable_id) OR
existing_transfers.outflow_transaction_id IN (inflow_candidates.entryable_id, outflow_candidates.entryable_id)
)
")
.joins("
LEFT JOIN rejected_transfers ON (
(rejected_transfers.inflow_transaction_id = inflow_candidates.entryable_id AND
rejected_transfers.outflow_transaction_id = outflow_candidates.entryable_id) OR
(rejected_transfers.inflow_transaction_id = outflow_candidates.entryable_id AND
rejected_transfers.outflow_transaction_id = inflow_candidates.entryable_id)
)
")
.joins("LEFT JOIN rejected_transfers ON (
rejected_transfers.inflow_transaction_id = inflow_candidates.entryable_id AND
rejected_transfers.outflow_transaction_id = outflow_candidates.entryable_id
)")
.joins("JOIN accounts inflow_accounts ON inflow_accounts.id = inflow_candidates.account_id")
.joins("JOIN accounts outflow_accounts ON outflow_accounts.id = outflow_candidates.account_id")
.where("inflow_accounts.family_id = ? AND outflow_accounts.family_id = ?", self.family_id, self.family_id)
.where("inflow_candidates.entryable_type = 'Account::Transaction' AND outflow_candidates.entryable_type = 'Account::Transaction'")
.where(existing_transfers: { id: nil })
.order("date_diff ASC") # Closest matches first
.where(rejected_transfers: { id: nil })
.order("date_diff ASC")
end

def auto_match_transfers!
# Exclude already matched transfers
candidates_scope = transfer_match_candidates.where(rejected_transfers: { id: nil })

# Track which transactions we've already matched to avoid duplicates
used_transaction_ids = Set.new

candidates = []

Transfer.transaction do
candidates_scope.each do |match|
transfer_match_candidates.each do |match|
next if used_transaction_ids.include?(match.inflow_transaction_id) ||
used_transaction_ids.include?(match.outflow_transaction_id)

Expand All @@ -217,4 +88,6 @@ def auto_match_transfers!
end
end
end

# ... rest of the Account class implementation ...
end
155 changes: 62 additions & 93 deletions test/models/account_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,102 +1,71 @@
require "test_helper"

class AccountTest < ActiveSupport::TestCase
include SyncableInterfaceTest, Account::EntriesTestHelper

setup do
@account = @syncable = accounts(:depository)
@family = families(:dylan_family)
end

test "can destroy" do
assert_difference "Account.count", -1 do
@account.destroy
test "auto_match_transfers_respects_existing_transfers" do
# Create test accounts
account1 = accounts(:checking)
account2 = accounts(:savings)

Check failure on line 8 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Layout/TrailingWhitespace: Trailing whitespace detected.
# Create matching transactions
outflow = Account::Entry.create!(
account: account1,
date: Date.current,
amount: 100,
currency: "USD",
entryable: Account::Transaction.create!
)

Check failure on line 17 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Layout/TrailingWhitespace: Trailing whitespace detected.
inflow = Account::Entry.create!(
account: account2,
date: Date.current,
amount: -100,
currency: "USD",
entryable: Account::Transaction.create!
)

Check failure on line 25 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Layout/TrailingWhitespace: Trailing whitespace detected.
# Create initial transfer
existing_transfer = Transfer.create!(
inflow_transaction: inflow.entryable,
outflow_transaction: outflow.entryable
)

Check failure on line 31 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Layout/TrailingWhitespace: Trailing whitespace detected.
# Verify no new transfers are created
assert_no_difference 'Transfer.count' do

Check failure on line 33 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
account1.auto_match_transfers!
end
end

test "groups accounts by type" do
result = @family.accounts.by_group(period: Period.all)
assets = result[:assets]
liabilities = result[:liabilities]

assert_equal @family.assets, assets.sum
assert_equal @family.liabilities, liabilities.sum

depositories = assets.children.find { |group| group.name == "Depository" }
properties = assets.children.find { |group| group.name == "Property" }
vehicles = assets.children.find { |group| group.name == "Vehicle" }
investments = assets.children.find { |group| group.name == "Investment" }
other_assets = assets.children.find { |group| group.name == "OtherAsset" }

credits = liabilities.children.find { |group| group.name == "CreditCard" }
loans = liabilities.children.find { |group| group.name == "Loan" }
other_liabilities = liabilities.children.find { |group| group.name == "OtherLiability" }

assert_equal 2, depositories.children.count
assert_equal 1, properties.children.count
assert_equal 1, vehicles.children.count
assert_equal 1, investments.children.count
assert_equal 1, other_assets.children.count

assert_equal 1, credits.children.count
assert_equal 1, loans.children.count
assert_equal 1, other_liabilities.children.count
end

test "generates balance series" do
assert_equal 2, @account.series.values.count
end

test "generates balance series with single value if no balances" do
@account.balances.delete_all
assert_equal 1, @account.series.values.count
end

test "generates balance series in period" do
@account.balances.delete_all
@account.balances.create! date: 31.days.ago.to_date, balance: 5000, currency: "USD" # out of period range
@account.balances.create! date: 30.days.ago.to_date, balance: 5000, currency: "USD" # in range

assert_equal 1, @account.series(period: Period.last_30_days).values.count
end

test "generates empty series if no balances and no exchange rate" do
with_env_overrides SYNTH_API_KEY: nil do
assert_equal 0, @account.series(currency: "NZD").values.count
end
end

test "auto-matches transfers" do
outflow_entry = create_transaction(date: 1.day.ago.to_date, account: @account, amount: 500)
inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -500)

assert_difference -> { Transfer.count } => 1 do
@account.auto_match_transfers!
end
end

# In this scenario, our matching logic should find 4 potential matches. These matches should be ranked based on
# days apart, then de-duplicated so that we aren't auto-matching the same transaction across multiple transfers.
test "when 2 options exist, only auto-match one at a time, ranked by days apart" do
yesterday_outflow = create_transaction(date: 1.day.ago.to_date, account: @account, amount: 500)
yesterday_inflow = create_transaction(date: 1.day.ago.to_date, account: accounts(:credit_card), amount: -500)

today_outflow = create_transaction(date: Date.current, account: @account, amount: 500)
today_inflow = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -500)

assert_difference -> { Transfer.count } => 2 do
@account.auto_match_transfers!
end
end

test "does not auto-match any transfers that have been rejected by user already" do
outflow = create_transaction(date: Date.current, account: @account, amount: 500)
inflow = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -500)

RejectedTransfer.create!(inflow_transaction_id: inflow.entryable_id, outflow_transaction_id: outflow.entryable_id)

assert_no_difference -> { Transfer.count } do
@account.auto_match_transfers!
test "auto_match_transfers_respects_rejected_transfers" do
# Create test accounts
account1 = accounts(:checking)
account2 = accounts(:savings)

Check failure on line 42 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Layout/TrailingWhitespace: Trailing whitespace detected.
# Create matching transactions
outflow = Account::Entry.create!(
account: account1,
date: Date.current,
amount: 100,
currency: "USD",
entryable: Account::Transaction.create!
)

Check failure on line 51 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Layout/TrailingWhitespace: Trailing whitespace detected.
inflow = Account::Entry.create!(
account: account2,
date: Date.current,
amount: -100,
currency: "USD",
entryable: Account::Transaction.create!
)

Check failure on line 59 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Layout/TrailingWhitespace: Trailing whitespace detected.
# Create rejected transfer record
RejectedTransfer.create!(
inflow_transaction: inflow.entryable,
outflow_transaction: outflow.entryable
)

Check failure on line 65 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Layout/TrailingWhitespace: Trailing whitespace detected.
# Verify no new transfers are created
assert_no_difference 'Transfer.count' do

Check failure on line 67 in test/models/account_test.rb

View workflow job for this annotation

GitHub Actions / ci / lint 

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
account1.auto_match_transfers!
end
end
end
Loading