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

fix: Transfers were not syncing between accounts (#987)

Browse Source 
* fix: Include investment_contribution in transfer? check and protect transfer entries from sync

Transfer transactions with kind "investment_contribution" were not recognized
as transfers by the UI, causing missing +/- indicators, "Transfer" labels,
and showing regular transaction forms instead of transfer details.

Also adds user_modified: true to entries created via TransferMatchesController
and SetAsTransferOrPayment rule action to protect them from provider sync
overwrites, matching the existing behavior in Transfer::Creator.

https://claude.ai/code/session_019BZ5Z1aqKSK3cRdR81P5Jg

* fix: Centralize transfer/budget kind constants for consistent investment_contribution handling

Define TRANSFER_KINDS and BUDGET_EXCLUDED_KINDS on Transaction to eliminate
hard-coded kind lists scattered across filters, rules, and analytics code.

investment_contribution is now consistently treated as a transfer in search
filters, rule conditions, and UI display (via TRANSFER_KINDS), while budget
analytics correctly continue treating it as an expense (via BUDGET_EXCLUDED_KINDS).

https://claude.ai/code/session_019BZ5Z1aqKSK3cRdR81P5Jg

* fix: Update tests for consistent investment_contribution as transfer kind

- search_test: loan_payment is now in TRANSFER_KINDS, so uncategorized
  filter correctly excludes it (same as funds_movement/cc_payment)
- condition_test: investment_contribution is now a transfer kind, so it
  matches the transfer filter rather than expense filter

https://claude.ai/code/session_019BZ5Z1aqKSK3cRdR81P5Jg

* fix: Eliminate SQL injection warnings in Transaction::Search

Replace string-interpolated SQL with parameterized queries:
- totals: use sanitize_sql_array with ? placeholders
- apply_category_filter: pass TRANSFER_KINDS as bind parameter
- apply_type_filter: use where(kind:)/where.not(kind:) and
  parameterized IN (?) for compound OR conditions
- Remove unused transfer_kinds_sql helper

https://claude.ai/code/session_019BZ5Z1aqKSK3cRdR81P5Jg

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Juan José Mata
2026-02-16 13:50:06 +01:00
committed by GitHub
parent 23087c1e98
commit b48cec3a2e
14 changed files with 146 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
app/controllers/reports_controller.rb
View File

@@ -350,7 +350,7 @@ class ReportsController < ApplicationController
.joins(entry: :account)
.where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] })
.where(entries: { entryable_type: "Transaction", excluded: false, date: @period.date_range })
.where.not(kind: [ "funds_movement", "one_time", "cc_payment" ])
.where.not(kind: Transaction::BUDGET_EXCLUDED_KINDS)
.includes(entry: :account, category: :parent)
# Apply filters
@@ -611,7 +611,7 @@ class ReportsController < ApplicationController
.joins(entry: :account)
.where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] })
.where(entries: { entryable_type: "Transaction", excluded: false, date: @period.date_range })
.where.not(kind: [ "funds_movement", "one_time", "cc_payment" ])
.where.not(kind: Transaction::BUDGET_EXCLUDED_KINDS)
.includes(entry: :account, category: [])
transactions = apply_transaction_filters(transactions)
@@ -648,7 +648,7 @@ class ReportsController < ApplicationController
.joins(entry: :account)
.where(accounts: { family_id: Current.family.id, status: [ "draft", "active" ] })
.where(entries: { entryable_type: "Transaction", excluded: false, date: @period.date_range })
.where.not(kind: [ "funds_movement", "one_time", "cc_payment" ])
.where.not(kind: Transaction::BUDGET_EXCLUDED_KINDS)
.includes(entry: :account, category: [])
transactions = apply_transaction_filters(transactions)

1
app/controllers/transfer_matches_controller.rb
View File

@@ -49,6 +49,7 @@ class TransferMatchesController < ApplicationController
currency: @entry.currency,
date: @entry.date,
name: "Transfer to #{@entry.amount.negative? ? @entry.account.name : target_account.name}",
user_modified: true,
)
)

6
app/models/income_statement/category_stats.rb
View File

@@ -38,6 +38,10 @@ class IncomeStatement::CategoryStats
params
end
def budget_excluded_kinds_sql
@budget_excluded_kinds_sql ||= Transaction::BUDGET_EXCLUDED_KINDS.map { |k| "'#{k}'" }.join(", ")
end
def exclude_tax_advantaged_sql
ids = @family.tax_advantaged_account_ids
return "" if ids.empty?
@@ -62,7 +66,7 @@ class IncomeStatement::CategoryStats
er.to_currency = :target_currency
)
WHERE a.family_id = :family_id
AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
AND t.kind NOT IN (#{budget_excluded_kinds_sql})
AND ae.excluded = false
AND (t.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true
AND (t.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true

6
app/models/income_statement/family_stats.rb
View File

@@ -37,6 +37,10 @@ class IncomeStatement::FamilyStats
params
end
def budget_excluded_kinds_sql
@budget_excluded_kinds_sql ||= Transaction::BUDGET_EXCLUDED_KINDS.map { |k| "'#{k}'" }.join(", ")
end
def exclude_tax_advantaged_sql
ids = @family.tax_advantaged_account_ids
return "" if ids.empty?
@@ -59,7 +63,7 @@ class IncomeStatement::FamilyStats
er.to_currency = :target_currency
)
WHERE a.family_id = :family_id
AND t.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
AND t.kind NOT IN (#{budget_excluded_kinds_sql})
AND ae.excluded = false
AND (t.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true
AND (t.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true

8
app/models/income_statement/totals.rb
View File

@@ -69,7 +69,7 @@ class IncomeStatement::Totals
er.from_currency = ae.currency AND
er.to_currency = :target_currency
)
WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
WHERE at.kind NOT IN (#{budget_excluded_kinds_sql})
AND ae.excluded = false
AND a.family_id = :family_id
AND a.status IN ('draft', 'active')
@@ -96,7 +96,7 @@ class IncomeStatement::Totals
er.from_currency = ae.currency AND
er.to_currency = :target_currency
)
WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
WHERE at.kind NOT IN (#{budget_excluded_kinds_sql})
AND (
at.investment_activity_label IS NULL
OR at.investment_activity_label NOT IN ('Transfer', 'Sweep In', 'Sweep Out', 'Exchange')
@@ -144,6 +144,10 @@ class IncomeStatement::Totals
"AND a.id NOT IN (:tax_advantaged_account_ids)"
end
def budget_excluded_kinds_sql
@budget_excluded_kinds_sql ||= Transaction::BUDGET_EXCLUDED_KINDS.map { |k| "'#{k}'" }.join(", ")
end
def validate_date_range!
unless @date_range.is_a?(Range)
raise ArgumentError, "date_range must be a Range, got #{@date_range.class}"

1
app/models/rule/action_executor/set_as_transfer_or_payment.rb
View File

@@ -46,6 +46,7 @@ class Rule::ActionExecutor::SetAsTransferOrPayment < Rule::ActionExecutor
currency: entry.currency,
date: entry.date,
name: "#{target_account.liability? ? "Payment" : "Transfer"} #{entry.amount.negative? ? "to #{target_account.name}" : "from #{entry.account.name}"}",
user_modified: true,
)
)

13
app/models/rule/condition_filter/transaction_type.rb
View File

@@ -1,7 +1,4 @@
class Rule::ConditionFilter::TransactionType < Rule::ConditionFilter
# Transfer kinds matching Transaction#transfer? method
TRANSFER_KINDS = %w[funds_movement cc_payment loan_payment].freeze
def type
"select"
end
@@ -26,15 +23,13 @@ class Rule::ConditionFilter::TransactionType < Rule::ConditionFilter
# Logic matches Transaction::Search#apply_type_filter for consistency
case value
when "income"
# Negative amounts, excluding transfers and investment_contribution
scope.where("entries.amount < 0")
.where.not(kind: TRANSFER_KINDS + %w[investment_contribution])
.where.not(kind: Transaction::TRANSFER_KINDS)
when "expense"
# Positive amounts OR investment_contribution (regardless of sign), excluding transfers
scope.where("entries.amount >= 0 OR transactions.kind = 'investment_contribution'")
.where.not(kind: TRANSFER_KINDS)
scope.where("entries.amount >= 0")
.where.not(kind: Transaction::TRANSFER_KINDS)
when "transfer"
scope.where(kind: TRANSFER_KINDS)
scope.where(kind: Transaction::TRANSFER_KINDS)
else
scope
end

13
app/models/transaction.rb
View File

@@ -17,9 +17,18 @@ class Transaction < ApplicationRecord
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
investment_contribution: "investment_contribution" # Transfer to investment/crypto account, excluded from budget analytics
investment_contribution: "investment_contribution" # Transfer to investment/crypto account, treated as an expense in budgets
}
# All kinds where money moves between accounts (transfer? returns true).
# Used for search filters, rule conditions, and UI display.
TRANSFER_KINDS = %w[funds_movement cc_payment loan_payment investment_contribution].freeze
# Kinds excluded from budget/income-statement analytics.
# loan_payment and investment_contribution are intentionally NOT here —
# they represent real cash outflow from a budgeting perspective.
BUDGET_EXCLUDED_KINDS = %w[funds_movement one_time cc_payment].freeze
# All valid investment activity labels (for UI dropdown)
ACTIVITY_LABELS = [
"Buy", "Sell", "Sweep In", "Sweep Out", "Dividend", "Reinvestment",
@@ -54,7 +63,7 @@ class Transaction < ApplicationRecord
# Overarching grouping method for all transfer-type transactions
def transfer?
funds_movement? || cc_payment? || loan_payment?
TRANSFER_KINDS.include?(kind)
end
def set_category!(category)

45
app/models/transaction/search.rb
View File

@@ -57,8 +57,14 @@ class Transaction::Search
result = scope
.select(
"COALESCE(SUM(CASE WHEN transactions.kind = 'investment_contribution' THEN ABS(entries.amount * COALESCE(er.rate, 1)) WHEN entries.amount >= 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment') THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as expense_total",
"COALESCE(SUM(CASE WHEN entries.amount < 0 AND transactions.kind NOT IN ('funds_movement', 'cc_payment', 'investment_contribution') THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as income_total",
ActiveRecord::Base.sanitize_sql_array([
"COALESCE(SUM(CASE WHEN entries.amount >= 0 AND transactions.kind NOT IN (?) THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as expense_total",
Transaction::TRANSFER_KINDS
]),
ActiveRecord::Base.sanitize_sql_array([
"COALESCE(SUM(CASE WHEN entries.amount < 0 AND transactions.kind NOT IN (?) THEN ABS(entries.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as income_total",
Transaction::TRANSFER_KINDS
]),
"COUNT(entries.id) as transactions_count"
)
.joins(
@@ -110,14 +116,14 @@ class Transaction::Search
# Get parent category IDs for the given category names
parent_category_ids = family.categories.where(name: real_categories).pluck(:id)
uncategorized_condition = "(categories.id IS NULL AND transactions.kind NOT IN ('funds_movement', 'cc_payment'))"
uncategorized_condition = "categories.id IS NULL AND transactions.kind NOT IN (?)"
# Build condition based on whether parent_category_ids is empty
if parent_category_ids.empty?
if include_uncategorized
query = query.left_joins(:category).where(
"categories.name IN (?) OR #{uncategorized_condition}",
real_categories.presence || []
"categories.name IN (?) OR (#{uncategorized_condition})",
real_categories.presence || [], Transaction::TRANSFER_KINDS
)
else
query = query.left_joins(:category).where(categories: { name: real_categories })
@@ -125,8 +131,8 @@ class Transaction::Search
else
if include_uncategorized
query = query.left_joins(:category).where(
"categories.name IN (?) OR categories.parent_id IN (?) OR #{uncategorized_condition}",
real_categories, parent_category_ids
"categories.name IN (?) OR categories.parent_id IN (?) OR (#{uncategorized_condition})",
real_categories, parent_category_ids, Transaction::TRANSFER_KINDS
)
else
query = query.left_joins(:category).where(
@@ -143,29 +149,22 @@ class Transaction::Search
return query unless types.present?
return query if types.sort == [ "expense", "income", "transfer" ]
transfer_condition = "transactions.kind IN ('funds_movement', 'cc_payment', 'loan_payment')"
# investment_contribution is always an expense regardless of amount sign
# (handles both manual outflows and provider-imported inflows like 401k contributions)
investment_contribution_condition = "transactions.kind = 'investment_contribution'"
expense_condition = "(entries.amount >= 0 OR #{investment_contribution_condition})"
income_condition = "(entries.amount <= 0 AND NOT #{investment_contribution_condition})"
condition = case types.sort
case types.sort
when [ "transfer" ]
transfer_condition
query.where(kind: Transaction::TRANSFER_KINDS)
when [ "expense" ]
Arel.sql("#{expense_condition} AND NOT (#{transfer_condition})")
query.where("entries.amount >= 0").where.not(kind: Transaction::TRANSFER_KINDS)
when [ "income" ]
Arel.sql("#{income_condition} AND NOT (#{transfer_condition})")
query.where("entries.amount < 0").where.not(kind: Transaction::TRANSFER_KINDS)
when [ "expense", "transfer" ]
Arel.sql("#{expense_condition} OR #{transfer_condition}")
query.where("entries.amount >= 0 OR transactions.kind IN (?)", Transaction::TRANSFER_KINDS)
when [ "income", "transfer" ]
Arel.sql("#{income_condition} OR #{transfer_condition}")
query.where("entries.amount < 0 OR transactions.kind IN (?)", Transaction::TRANSFER_KINDS)
when [ "expense", "income" ]
Arel.sql("NOT (#{transfer_condition})")
query.where.not(kind: Transaction::TRANSFER_KINDS)
else
query
end
query.where(condition)
end
def apply_merchant_filter(query, merchants)