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

Add RejectedTransfer model, simplify auto matching #1690

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zachgoll
Copy link
Collaborator

@zachgoll zachgoll commented Jan 25, 2025

This PR addresses and fixes #1630 and #1661 along with a few other simplifications.

Rejected transfers

Previously, the Transfer model had 3 possible status codes: approved, pending, and rejected.

The rejected status introduced a large group of edge cases that made the UI, auto matching logic, and overall transfer domain more confusing:

  • A transaction could theoretically be part of multiple Transfer records since a user could reject one and then add the same transaction to another. This made it so transaction.transfer? needed to query a has_many relationship to decide whether a transaction was part of a confirmed transfer.
  • During auto-matching, the logic required to filter out transactions that "belonged to existing transfers, but NOT rejected ones" was confusing and error prone
  • The concept of a "Rejected transfer" is confusing in general because it doesn't really exist. A "rejected transfer" is equivalent to "no transfer".

The sole reason we had this status was to keep track of transaction pairs that the user had explicitly rejected to avoid auto-matching them again. Adding RejectedTransfer drastically simplifies the required domain logic while giving us a simple way to "remember" which transfers have been rejected by the user already.

Auto matching validation updates

  • Per Allow auto-matched transfers to have an inflow that occurs before outflow #1661, auto matches will now include any transactions within 4 days of each other to account for scenarios where loan providers post inflow payments before receiving the funds from the user's auto-payment account (i.e. Checking)
  • Auto-matching now deduplicates transfers and prioritizes based on date proximity. Previously, auto matching would create 2 Transfer records with the same transaction in both, leaving the user with a confusing matching approve/reject scenario

Consolidation of auto-matching logic

Previously, there were 2 places in the codebase we were duplicating the "auto matching" conditions:

  • Transfer.auto_match_for_account
  • entry.transfer_match_candidates

This created a brittle scenario where a user was able to manually match transactions based on a different criteria than we were matching during account syncs. This logic is now consolidated into a scope of account.transfer_match_candidates which is consumed by both account.auto_match_transfers! and entry.transfer_match_candidates.

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