Add RejectedTransfer model, simplify auto matching #1690

2025-01-25T08:54:15+08:00

zachgoll commented

2025-01-25 08:54:15 +08:00

(Migrated from github.com)

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 #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.

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 #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`.

👍 1

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gavin/maybe#1690