Fix transfer matching logic (#1625)

* Fix transfer matching logic

* Fix tests

app/models/transfer.rb

@@ -7,6 +7,8 @@ class Transfer < ApplicationRecord
   validate :transfer_has_different_accounts
   validate :transfer_has_opposite_amounts
   validate :transfer_within_date_range
+  validate :transfer_has_same_family
+  validate :inflow_on_or_after_outflow
 
   class << self
     def from_accounts(from_account:, to_account:, date:, amount:)
@@ -42,7 +44,10 @@ class Transfer < ApplicationRecord
     end
 
     def auto_match_for_account(account)
-      matches = Account::Entry.from("account_entries inflow_candidates")
+      matches = Account::Entry.select([
+        "inflow_candidates.entryable_id as inflow_transaction_id",
+        "outflow_candidates.entryable_id as outflow_transaction_id"
+      ]).from("account_entries inflow_candidates")
         .joins("
           JOIN account_entries outflow_candidates ON (
             inflow_candidates.amount < 0 AND
@@ -55,21 +60,21 @@ class Transfer < ApplicationRecord
           )
         ").joins("
           LEFT JOIN transfers existing_transfers ON (
-            (existing_transfers.inflow_transaction_id = inflow_candidates.entryable_id AND
-             existing_transfers.outflow_transaction_id = outflow_candidates.entryable_id) OR
-            (existing_transfers.inflow_transaction_id = inflow_candidates.entryable_id) OR
-            (existing_transfers.outflow_transaction_id = outflow_candidates.entryable_id)
+            existing_transfers.inflow_transaction_id = inflow_candidates.entryable_id OR
+            existing_transfers.outflow_transaction_id = outflow_candidates.entryable_id
           )
         ")
+        .joins("JOIN accounts inflow_accounts ON inflow_accounts.id = inflow_candidates.account_id")
+        .joins("JOIN accounts outflow_accounts ON outflow_accounts.id = outflow_candidates.account_id")
+        .where("inflow_accounts.family_id = ? AND outflow_accounts.family_id = ?", account.family_id, account.family_id)
+        .where("inflow_candidates.entryable_type = 'Account::Transaction' AND outflow_candidates.entryable_type = 'Account::Transaction'")
         .where(existing_transfers: { id: nil })
-        .where("inflow_candidates.account_id = ? AND outflow_candidates.account_id = ?", account.id, account.id)
-        .pluck(:inflow_transaction_id, :outflow_transaction_id)
 
       Transfer.transaction do
-        matches.each do |inflow_transaction_id, outflow_transaction_id|
+        matches.each do |match|
           Transfer.create!(
-            inflow_transaction_id: inflow_transaction_id,
-            outflow_transaction_id: outflow_transaction_id,
+            inflow_transaction_id: match.inflow_transaction_id,
+            outflow_transaction_id: match.outflow_transaction_id,
           )
         end
       end
@@ -114,11 +119,21 @@ class Transfer < ApplicationRecord
   end
 
   private
+    def inflow_on_or_after_outflow
+      return unless inflow_transaction.present? && outflow_transaction.present?
+      errors.add(:base, :inflow_must_be_on_or_after_outflow) if inflow_transaction.entry.date < outflow_transaction.entry.date
+    end
+
     def transfer_has_different_accounts
       return unless inflow_transaction.present? && outflow_transaction.present?
       errors.add(:base, :must_be_from_different_accounts) if inflow_transaction.entry.account == outflow_transaction.entry.account
     end
 
+    def transfer_has_same_family
+      return unless inflow_transaction.present? && outflow_transaction.present?
+      errors.add(:base, :must_be_from_same_family) unless inflow_transaction.entry.account.family == outflow_transaction.entry.account.family
+    end
+
     def transfer_has_opposite_amounts
       return unless inflow_transaction.present? && outflow_transaction.present?
 

app/views/account/transactions/_transaction_category.html.erb

@@ -1,7 +1,7 @@
 <%# locals: (entry:) %>
 
 <div id="<%= dom_id(entry, "category_menu") %>">
-  <% if entry.account_transaction.transfer&.categorizable? || entry.account_transaction.transfer.nil? %>
+  <% if entry.account_transaction.transfer&.categorizable? || entry.account_transaction.transfer.nil? || entry.account_transaction.transfer&.rejected? %>
     <%= render "categories/menu", transaction: entry.account_transaction %>
   <% else %>
     <%= render "categories/badge", category: entry.account_transaction.transfer&.payment? ? payment_category :  transfer_category %>

config/locales/models/transfer/en.yml

@@ -12,6 +12,9 @@ en:
               must_have_opposite_amounts: Transfer transactions must have opposite
                 amounts
               must_have_single_currency: Transfer must have a single currency
+              must_be_from_same_family: Transfer must be from the same family
+              inflow_must_be_on_or_after_outflow: Inflow transaction must be on or after outflow transaction
   transfer:
     name: Transfer to %{to_account}
     payment_name: Payment to %{to_account}
+

test/models/transfer_test.rb

@@ -8,9 +8,18 @@ class TransferTest < ActiveSupport::TestCase
     @inflow = account_transactions(:transfer_in)
   end
 
+  test "auto matches transfers" do
+    outflow_entry = create_transaction(date: 1.day.ago.to_date, account: accounts(:depository), amount: 500)
+    inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -500)
+
+    assert_difference -> { Transfer.count } => 1 do
+      Transfer.auto_match_for_account(accounts(:depository))
+    end
+  end
+
   test "transfer has different accounts, opposing amounts, and within 4 days of each other" do
-    outflow_entry = create_transaction(date: Date.current, account: accounts(:depository), amount: 500)
-    inflow_entry = create_transaction(date: 1.day.ago.to_date, account: accounts(:credit_card), amount: -500)
+    outflow_entry = create_transaction(date: 1.day.ago.to_date, account: accounts(:depository), amount: 500)
+    inflow_entry = create_transaction(date: Date.current, account: accounts(:credit_card), amount: -500)
 
     assert_difference -> { Transfer.count } => 1 do
       Transfer.create!(
@@ -68,6 +77,25 @@ class TransferTest < ActiveSupport::TestCase
     assert_equal "Transfer transaction dates must be within 4 days of each other", transfer.errors.full_messages.first
   end
 
+  test "transfer must be from the same family" do
+    family1 = families(:empty)
+    family2 = families(:dylan_family)
+
+    family1_account = family1.accounts.create!(name: "Family 1 Account", balance: 5000, currency: "USD", accountable: Depository.new)
+    family2_account = family2.accounts.create!(name: "Family 2 Account", balance: 5000, currency: "USD", accountable: Depository.new)
+
+    outflow_txn = create_transaction(date: Date.current, account: family1_account, amount: 500)
+    inflow_txn = create_transaction(date: Date.current, account: family2_account, amount: -500)
+
+    transfer = Transfer.new(
+      inflow_transaction: inflow_txn.account_transaction,
+      outflow_transaction: outflow_txn.account_transaction,
+    )
+
+    assert transfer.invalid?
+    assert_equal "Transfer must be from the same family", transfer.errors.full_messages.first
+  end
+
   test "from_accounts converts amounts to the to_account's currency" do
     accounts(:depository).update!(currency: "EUR")