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

feat(up): flag internal transfers and round-ups as funds_movement (#2460)

Browse Source 
* feat(up): flag internal transfers and round-ups as funds_movement

Up populates relationships.transferAccount on transactions that move money
between the user's own accounts (including round-ups swept into a Saver), but
flatten_transaction dropped it, so these imported as ordinary income/expense
and distorted budgets and cashflow.

- Provider::Up#flatten_transaction: lift transfer_account_id from
  relationships.transferAccount.data.id.
- UpEntry::Processor: import transfers as funds_movement and persist
  transfer_account_id in extra["up"].
- Account::ProviderImportAdapter#import_transaction: optional kind: param; an
  explicit provider kind takes precedence over account-type auto-detection and
  is applied after the sync-protection check, so user re-categorisations
  survive re-sync.

Complementary to Family#auto_match_transfers!: two-sided transfers between
linked accounts are still paired into a Transfer (the matcher does not filter
on kind); one-sided movements and round-ups, which the matcher cannot pair,
are the cases this fixes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(up): account-type kind wins over provider transfer hint

Codex review caught that Up HOME_LOAN accounts map to a Loan account, so a
repayment carrying transferAccount would be reclassified from loan_payment to
funds_movement (budget-excluded). Make the provider kind: a fallback:
activity-label and account-type classification now take precedence, so
loan_payment and cc_payment survive. Adds a regression test (loan repayment
stays loan_payment), a depository-applies test, and a note that the up_test
stub ignores query: intentionally.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Gavin Matthews <matthews.gav@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
threatsurfer
2026-06-30 16:19:25 +10:00
committed by GitHub
parent a14e1a1321
commit ab32388326
6 changed files with 144 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
app/models/account/provider_import_adapter.rb
View File

@@ -26,7 +26,7 @@ class Account::ProviderImportAdapter
# @param extra [Hash, nil] Optional provider-specific metadata to merge into transaction.extra
# @param investment_activity_label [String, nil] Optional activity type label (e.g., "Buy", "Dividend")
# @return [Entry] The created or updated entry
def import_transaction(external_id:, amount:, currency:, date:, name:, source:, category_id: nil, merchant: nil, notes: nil, pending_transaction_id: nil, extra: nil, investment_activity_label: nil)
def import_transaction(external_id:, amount:, currency:, date:, name:, source:, category_id: nil, kind: nil, merchant: nil, notes: nil, pending_transaction_id: nil, extra: nil, investment_activity_label: nil)
raise ArgumentError, "external_id is required" if external_id.blank?
raise ArgumentError, "source is required" if source.blank?
@@ -208,7 +208,13 @@ class Account::ProviderImportAdapter
detected_label = detect_activity_label(name, amount)
end
# Auto-set kind for internal movements and contributions
# Determine the transaction kind. Activity-label and account-type classification
# take precedence; an explicit kind supplied by the provider is used as a fallback
# for the standard case. A provider such as Up flags internal transfers and
# round-ups (via relationships.transferAccount) and passes funds_movement, but a
# repayment imported onto a linked Loan/CreditCard account must stay
# loan_payment/cc_payment (a budgeted expense) rather than being reclassified, so
# the account-type branches below win over the provider hint.
auto_kind = nil
auto_category = nil
if Transaction::INTERNAL_MOVEMENT_LABELS.include?(detected_label)
@@ -221,6 +227,7 @@ class Account::ProviderImportAdapter
elsif account.accountable_type == "CreditCard" && amount.negative?
auto_kind = "cc_payment"
end
auto_kind ||= kind.presence
# Set investment activity label, kind, and category if detected
if entry.entryable.is_a?(Transaction)

8
app/models/provider/up.rb
View File

@@ -92,6 +92,11 @@ class Provider::Up
# Flattens a JSON:API transaction resource, lifting attributes to the top level and
# extracting the related account/category ids from relationships.
#
# transfer_account_id is the other side of an internal money movement: Up populates
# relationships.transferAccount on any transaction that moves funds between the
# user's own accounts (including round-ups swept into a Saver). It is nil for
# ordinary income/expense.
def flatten_transaction(resource)
data = resource.with_indifferent_access
attributes = data[:attributes].is_a?(Hash) ? data[:attributes] : {}
@@ -99,7 +104,8 @@ class Provider::Up
attributes.merge(
id: data[:id],
account_id: data.dig(:relationships, :account, :data, :id),
category_id: data.dig(:relationships, :category, :data, :id)
category_id: data.dig(:relationships, :category, :data, :id),
transfer_account_id: data.dig(:relationships, :transferAccount, :data, :id)
).with_indifferent_access
end

18
app/models/up_entry/processor.rb
View File

@@ -53,6 +53,7 @@ class UpEntry::Processor
date: date,
name: name,
source: "up",
kind: kind,
merchant: merchant,
notes: notes,
extra: extra_metadata
@@ -98,6 +99,22 @@ class UpEntry::Processor
data[:description].presence || I18n.t("transactions.unknown_name")
end
# The id of the other account in an internal money movement, if any (see
# Provider::Up#flatten_transaction). Present for transfers between the user's own
# accounts and for round-ups swept into a Saver; nil for ordinary income/expense.
def transfer_account_id
data[:transfer_account_id].presence
end
# Mark internal movements as funds_movement so they are excluded from income,
# expense, and budget analytics. Two-sided transfers between two linked accounts are
# additionally paired into a Transfer by Family#auto_match_transfers!; one-sided moves
# (counterpart not linked in Sure) and round-ups rely on this flag, since the matcher
# has no opposing entry to pair them with.
def kind
transfer_account_id ? "funds_movement" : nil
end
# Optional user-entered message attached to the transaction.
def notes
data[:message].presence
@@ -170,6 +187,7 @@ class UpEntry::Processor
"pending" => pending?,
"status" => data[:status],
"category_id" => data[:category_id],
"transfer_account_id" => transfer_account_id,
"raw_text" => data[:rawText],
"fx_from" => foreign_amount_data[:currencyCode],
"fx_amount" => foreign_amount_data[:value]

31
test/models/account/provider_import_adapter_test.rb
View File

@@ -55,6 +55,37 @@ class Account::ProviderImportAdapterTest < ActiveSupport::TestCase
end
end
test "applies an explicit provider kind on a depository account" do
entry = @adapter.import_transaction(
external_id: "up_transfer_1",
amount: -50.00,
currency: "USD",
date: Date.today,
name: "Transfer to Savings",
source: "up",
kind: "funds_movement"
)
assert_equal "funds_movement", entry.transaction.kind
end
test "account-type kind takes precedence over an explicit provider kind" do
loan_adapter = Account::ProviderImportAdapter.new(accounts(:loan))
entry = loan_adapter.import_transaction(
external_id: "up_loan_repayment_1",
amount: -200.00,
currency: "USD",
date: Date.today,
name: "Home Loan Repayment",
source: "up",
kind: "funds_movement"
)
assert_equal "loan_payment", entry.transaction.kind,
"a repayment on a Loan account must stay loan_payment, not the provider's funds_movement"
end
test "updates existing transaction instead of creating duplicate" do
# Create initial transaction
entry = @adapter.import_transaction(

41
test/models/provider/up_test.rb
View File

@@ -138,6 +138,47 @@ class Provider::UpTest < ActiveSupport::TestCase
end
end
test "flattens transaction relationships including transferAccount" do
response = FakeResponse.new(
code: 200,
message: "OK",
body: {
data: [
{
type: "transactions", id: "tx_xfer",
attributes: { status: "SETTLED", description: "Transfer to Savings" },
relationships: {
account: { data: { id: "acc_123" } },
category: { data: nil },
transferAccount: { data: { id: "acc_saver" } }
}
},
{
type: "transactions", id: "tx_plain",
attributes: { status: "SETTLED", description: "Coffee" },
relationships: {
account: { data: { id: "acc_123" } },
category: { data: { id: "restaurants-and-cafes" } },
transferAccount: { data: nil }
}
}
],
links: { prev: nil, next: nil }
}.to_json
)
# The stub deliberately ignores the query: keyword: this test exercises only
# response flattening, not the request params (pagination/date filters), which
# are covered by the pagination tests above.
Provider::Up.stub(:get, ->(_url, headers:, query: nil) { response }) do
transactions = Provider::Up.new("up-access-token").get_account_transactions(account_id: "acc_123")
assert_equal "acc_saver", transactions.first[:transfer_account_id]
assert_equal "restaurants-and-cafes", transactions.second[:category_id]
assert_nil transactions.second[:transfer_account_id]
end
end
test "raises typed errors for unauthorized responses" do
response = FakeResponse.new(code: 401, message: "Unauthorized", body: "{}")

38
test/models/up_entry/processor_test.rb
View File

@@ -113,4 +113,42 @@ class UpEntry::ProcessorTest < ActiveSupport::TestCase
assert_equal BigDecimal("-2500.0"), entry.amount
end
test "marks internal transfers (transferAccount present) as funds_movement" do
entry = UpEntry::Processor.new(
{
id: "tx_transfer_1",
account_id: "acc_123",
status: "SETTLED",
description: "Transfer to 2Up Spending",
amount: { currencyCode: "AUD", value: "-500.00", valueInBaseUnits: -50000 },
settledAt: "2026-01-22T00:00:00+11:00",
createdAt: "2026-01-22T00:00:00+11:00",
transfer_account_id: "acc_other"
},
up_account: @up_account
).process
transaction = entry.entryable
assert_equal "funds_movement", transaction.kind
assert_equal "acc_other", transaction.extra.dig("up", "transfer_account_id")
end
test "ordinary transactions (no transferAccount) keep the standard kind" do
entry = UpEntry::Processor.new(
{
id: "tx_standard_1",
account_id: "acc_123",
status: "SETTLED",
description: "Coffee Shop",
amount: { currencyCode: "AUD", value: "-4.50", valueInBaseUnits: -450 },
settledAt: "2026-01-22T00:00:00+11:00",
createdAt: "2026-01-22T00:00:00+11:00"
},
up_account: @up_account
).process
assert_equal "standard", entry.entryable.kind
assert_nil entry.entryable.extra.dig("up", "transfer_account_id")
end
end