From e60b5df442d214bdc69d55eeb9bc369cf0ea3c7d Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach@maybe.co>
Date: Thu, 26 Jun 2025 09:54:25 -0400
Subject: [PATCH] Handle bad API data for trade quantity signage (#2416)

---
 .../investments/transactions_processor.rb     | 21 +++++++++-
 .../transactions_processor_test.rb            | 41 ++++++++++++++++++-
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/app/models/plaid_account/investments/transactions_processor.rb b/app/models/plaid_account/investments/transactions_processor.rb
index 9dcebdb0..fdcf95ec 100644
--- a/app/models/plaid_account/investments/transactions_processor.rb
+++ b/app/models/plaid_account/investments/transactions_processor.rb
@@ -43,14 +43,14 @@ class PlaidAccount::Investments::TransactionsProcessor
       end
 
       entry.assign_attributes(
-        amount: transaction["quantity"] * transaction["price"],
+        amount: derived_qty(transaction) * transaction["price"],
         currency: transaction["iso_currency_code"],
         date: transaction["date"]
       )
 
       entry.trade.assign_attributes(
         security: resolved_security_result.security,
-        qty: transaction["quantity"],
+        qty: derived_qty(transaction),
         price: transaction["price"],
         currency: transaction["iso_currency_code"]
       )
@@ -87,4 +87,21 @@ class PlaidAccount::Investments::TransactionsProcessor
     def transactions
       plaid_account.raw_investments_payload["transactions"] || []
     end
+
+    # Plaid unfortunately returns incorrect signage on some `quantity` values. They claim all "sell" transactions
+    # are negative signage, but we have found multiple instances of production data where this is not the case.
+    #
+    # This method attempts to use several Plaid data points to derive the true quantity with the correct signage.
+    def derived_qty(transaction)
+      reported_qty = transaction["quantity"]
+      abs_qty = reported_qty.abs
+
+      if transaction["type"] == "sell" || transaction["amount"] < 0
+        -abs_qty
+      elsif transaction["type"] == "buy" || transaction["amount"] > 0
+        abs_qty
+      else
+        reported_qty
+      end
+    end
 end
diff --git a/test/models/plaid_account/investments/transactions_processor_test.rb b/test/models/plaid_account/investments/transactions_processor_test.rb
index 8a0c9efd..7cee38bd 100644
--- a/test/models/plaid_account/investments/transactions_processor_test.rb
+++ b/test/models/plaid_account/investments/transactions_processor_test.rb
@@ -6,7 +6,6 @@ class PlaidAccount::Investments::TransactionsProcessorTest < ActiveSupport::Test
     @security_resolver = PlaidAccount::Investments::SecurityResolver.new(@plaid_account)
   end
 
-
   test "creates regular trade entries" do
     test_investments_payload = {
       transactions: [
@@ -16,6 +15,7 @@ class PlaidAccount::Investments::TransactionsProcessorTest < ActiveSupport::Test
           "type" => "buy",
           "quantity" => 1, # Positive, so "buy 1 share"
           "price" => 100,
+          "amount" => 100,
           "iso_currency_code" => "USD",
           "date" => Date.current,
           "name" => "Buy 1 share of AAPL"
@@ -108,4 +108,43 @@ class PlaidAccount::Investments::TransactionsProcessorTest < ActiveSupport::Test
     assert_equal Date.current, entry.date
     assert_equal "Miscellaneous fee", entry.name
   end
+
+  test "handles bad plaid quantity signage data" do
+    test_investments_payload = {
+      transactions: [
+        {
+          "transaction_id" => "123",
+          "type" => "sell", # Correct type
+          "subtype" => "sell", # Correct subtype
+          "quantity" => 1, # ***Incorrect signage***, this should be negative
+          "price" => 100, # Correct price
+          "amount" => -100, # Correct amount
+          "iso_currency_code" => "USD",
+          "date" => Date.current,
+          "name" => "Sell 1 share of AAPL"
+        }
+      ]
+    }
+
+    @plaid_account.update!(raw_investments_payload: test_investments_payload)
+
+    @security_resolver.expects(:resolve).returns(OpenStruct.new(
+      security: securities(:aapl)
+    ))
+
+    processor = PlaidAccount::Investments::TransactionsProcessor.new(@plaid_account, security_resolver: @security_resolver)
+
+    assert_difference [ "Entry.count", "Trade.count" ], 1 do
+      processor.process
+    end
+
+    entry = Entry.order(created_at: :desc).first
+
+    assert_equal -100, entry.amount
+    assert_equal "USD", entry.currency
+    assert_equal Date.current, entry.date
+    assert_equal "Sell 1 share of AAPL", entry.name
+
+    assert_equal -1, entry.trade.qty
+  end
 end