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.

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

Transfer and Payment auto-matching, model and UI improvements #1585

Conversation

zachgoll
Copy link
Collaborator

@zachgoll zachgoll commented Dec 31, 2024

This PR is an overhaul of Transfer model and the UI flows for identifying, marking, and viewing transfers.

Demo

CleanShot.2025-01-06.at.17.57.45.mp4

Changes

Association updates

Previously, Account::Transfer had a has_many :entries relationship. This posed a few problems when trying to easily validate both sides of the transfer. It was cumbersome to identify the "inflow" transaction and the "outflow" transaction. Furthermore, this was set at the Account::Entry level, while transfers are not possible for other entry types like Account::Valuation and Account::Trade.

The new Transfer model is more reflective of reality, which enables easier access patterns such as entry.account_transaction.transfer? to determine if the transaction is one side of a transfer.

class Transfer < ApplicationRecord
  belongs_to :inflow_transaction, class_name: "Account::Transaction"
  belongs_to :outflow_transaction, class_name: "Account::Transaction"
end

Validation and data integrity updates

Previously, the system had a marked_as_transfer boolean field on Account::Entry, which was in place to identify "1-sided transfers" (i.e. transfers where one of the accounts was not in the app). This created confusing logic where we needed to update/query 2 separate places (transfer_id and marked_as_transfer) to identify a one-way or two-way transfer.

In the new system all transfers are 2-way, which reflects the domain a lot more closely. In future PRs, I'll be adding additional ways to exclude transactions from budgeting calculations while keeping this "transfer" concept intact.

Auto matching

I have also removed the "matching" UI, disabled selections of transfer transactions, and created a much more seamless "auto-matching" background process that will intelligently match transfers for the user and ask them to confirm the match. Our app can identify 90% of transfers by looking at the amount, account, and date. Two transactions with opposite amounts, within a few days of each other from different accounts give high confidence that we're dealing with a transfer.

Transfers and Payments overview

One of the major goals of this app is to treat "transfers" and "payments" as first-class citizens rather than a categorical afterthought. Most personal finance apps leave it to the user to identify transfers and payments, which generally ends in the user asking, "How do I incorporate transfers in my budget and goals?". This leads to inaccurate calculations of spending, income, and overall budgets because the app does not give the user a clear way to separate these concepts from expenses and income (the primary purpose of budgeting). Our goal is to give transfers/payments a clear "home" in the app and assist the user in identifying them. Below are some technical definitions of each:

  • Transfer - any transaction that is simply a compositional shift of wealth. A transfer does NOT affect a users net worth because it is simply a movement from one of their accounts to another.
    • Example: User moves $5,000 from a checking account (asset) to a Brokerage account (asset), which does not affect net worth, but changes the composition of the user's balance sheet.
  • Payment - a payment is a specific type of transfer where a user makes a transfer from an asset account to a liability account, therefore reducing debt.
    • Example: User moves $2,000 from a checking account (asset) to a Credit card account (liability), which reduces both account values, but does NOT change net worth.

Transfer/Payment treatment

Transfers and payments are incorporated into a user's budget based on the classification of account in which they correspond to.

  • All transfers are excluded from budgets since they are neither "spending" nor "income"
  • Payments made to/from credit card accounts are excluded from budgets to avoid "double counting"
    • Since all the credit card account transactions will be categorized and summed up, we need to exclude the monthly payment made from the user's Checking account to avoid double counting. A credit card is a "Suspense" or "Clearing" account that acts as a temporary holding area for categorized transactions that will eventually be cleared by the debt servicing payment.
  • Payments made to loan accounts are included in budgets, because this is a monthly "expense" that a user must budget for

@zachgoll zachgoll marked this pull request as ready for review January 6, 2025 22:02
@zachgoll zachgoll merged commit 307a368 into main Jan 7, 2025
5 checks passed
@zachgoll zachgoll deleted the zachgoll/transfer-and-payment-auto-identification-and-improvements branch January 7, 2025 14:41
@nikhilbadyal
Copy link
Contributor

nikhilbadyal commented Jan 7, 2025

@zachgoll Auto Matching behaving weirdly. Auto Match should atleast try to match on same dates. Auto Matching with past dates sound strange.

Ex -
image

From date - 7th
To Date- 5th

@zachgoll
Copy link
Collaborator Author

zachgoll commented Jan 7, 2025

@nikhilbadyal I'm not sure I understand what you're referring to here. Can you explain this a bit more? Is the screenshot shown a valid transfer for you?

Currently, the matching window "tolerance" is 4 days to account for cases where the outgoing transfer was sent on a Friday, there was a 3 day holiday weekend, and the incoming settled the following Tuesday.

@nikhilbadyal
Copy link
Contributor

nikhilbadyal commented Jan 7, 2025

It's not a valid detection.

Also I got the tolerance but Receiver transaction date will always be greater (or euqal) than Sender Transaction date right ?

In above screenshot.

Money was sent Transferred from ICICI account on 7th Jan 2025 to SBI account on 3Jan 2025 . This look like a transfer from Future to past. Is it clear now ?

@zachgoll
Copy link
Collaborator Author

zachgoll commented Jan 7, 2025

@nikhilbadyal yep, makes sense! I agree that the receiver should always be equal or later date to the sender.

All that said, you should have the option to "Reject" the transfer in the UI, which was put in for incorrect detections just like this. Are you seeing that and is it working?

@nikhilbadyal
Copy link
Contributor

nikhilbadyal commented Jan 7, 2025

Yes, removed the transfer by that button. Found a few more such instances as well. I'll probably find all and revert manually or fire a query to revert instead of checking one by one.
Thanks for this cool feature though. It's awesome 💯

@zachgoll
Copy link
Collaborator Author

zachgoll commented Jan 7, 2025

@nikhilbadyal nice, yeah you'll have to do a manual rejection of these, but I've opened #1602 so future auto-detects aren't wrong like this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants