From 53f8f6b51e41fe6f72af220f0ff2e6672100ca0b Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Date: Wed, 10 Jul 2024 15:03:29 -0400
Subject: [PATCH 1/7] Add investment portfolio models

---
 app/models/account.rb                         |  2 +
 app/models/account/entryable.rb               |  2 +-
 app/models/account/holding.rb                 |  4 ++
 app/models/account/trade.rb                   | 15 +++++++
 app/models/security.rb                        |  3 ++
 app/models/security/price.rb                  |  2 +
 .../20240710182529_create_securities.rb       | 11 +++++
 .../20240710182728_create_security_prices.rb  | 12 +++++
 .../20240710184048_create_account_trades.rb   | 11 +++++
 .../20240710184249_create_account_holdings.rb | 14 ++++++
 db/schema.rb                                  | 44 ++++++++++++++++++-
 test/fixtures/account/entries.yml             |  9 ++++
 test/fixtures/account/holdings.yml            | 15 +++++++
 test/fixtures/account/trades.yml              |  4 ++
 test/fixtures/securities.yml                  |  9 ++++
 test/fixtures/security/prices.yml             | 11 +++++
 test/models/account/holding_test.rb           |  7 +++
 test/models/account/trade_test.rb             |  7 +++
 test/models/security/price_test.rb            |  7 +++
 test/models/security_test.rb                  |  7 +++
 20 files changed, 194 insertions(+), 2 deletions(-)
 create mode 100644 app/models/account/holding.rb
 create mode 100644 app/models/account/trade.rb
 create mode 100644 app/models/security.rb
 create mode 100644 app/models/security/price.rb
 create mode 100644 db/migrate/20240710182529_create_securities.rb
 create mode 100644 db/migrate/20240710182728_create_security_prices.rb
 create mode 100644 db/migrate/20240710184048_create_account_trades.rb
 create mode 100644 db/migrate/20240710184249_create_account_holdings.rb
 create mode 100644 test/fixtures/account/holdings.yml
 create mode 100644 test/fixtures/account/trades.yml
 create mode 100644 test/fixtures/securities.yml
 create mode 100644 test/fixtures/security/prices.yml
 create mode 100644 test/models/account/holding_test.rb
 create mode 100644 test/models/account/trade_test.rb
 create mode 100644 test/models/security/price_test.rb
 create mode 100644 test/models/security_test.rb

diff --git a/app/models/account.rb b/app/models/account.rb
index d1fcb02f..efc78948 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -12,6 +12,8 @@ class Account < ApplicationRecord
   has_many :entries, dependent: :destroy, class_name: "Account::Entry"
   has_many :transactions, through: :entries, source: :entryable, source_type: "Account::Transaction"
   has_many :valuations, through: :entries, source: :entryable, source_type: "Account::Valuation"
+  has_many :trades, through: :entries, source: :entryable, source_type: "Account::Trade"
+  has_many :holdings, dependent: :destroy
   has_many :balances, dependent: :destroy
   has_many :imports, dependent: :destroy
   has_many :syncs, dependent: :destroy
diff --git a/app/models/account/entryable.rb b/app/models/account/entryable.rb
index 5a23bd81..7c3b3ef9 100644
--- a/app/models/account/entryable.rb
+++ b/app/models/account/entryable.rb
@@ -1,7 +1,7 @@
 module Account::Entryable
   extend ActiveSupport::Concern
 
-  TYPES = %w[ Account::Valuation Account::Transaction ]
+  TYPES = %w[ Account::Valuation Account::Transaction Account::Trade ]
 
   def self.from_type(entryable_type)
     entryable_type.presence_in(TYPES).constantize
diff --git a/app/models/account/holding.rb b/app/models/account/holding.rb
new file mode 100644
index 00000000..626795a1
--- /dev/null
+++ b/app/models/account/holding.rb
@@ -0,0 +1,4 @@
+class Account::Holding < ApplicationRecord
+  belongs_to :account
+  belongs_to :security
+end
diff --git a/app/models/account/trade.rb b/app/models/account/trade.rb
new file mode 100644
index 00000000..52c58467
--- /dev/null
+++ b/app/models/account/trade.rb
@@ -0,0 +1,15 @@
+class Account::Trade < ApplicationRecord
+  include Account::Entryable
+
+  belongs_to :security
+
+  class << self
+    def search(_params)
+      all
+    end
+
+    def requires_search?(_params)
+      false
+    end
+  end
+end
diff --git a/app/models/security.rb b/app/models/security.rb
new file mode 100644
index 00000000..e4688f75
--- /dev/null
+++ b/app/models/security.rb
@@ -0,0 +1,3 @@
+class Security < ApplicationRecord
+  has_many :trades, dependent: :nullify, class_name: "Account::Trade"
+end
diff --git a/app/models/security/price.rb b/app/models/security/price.rb
new file mode 100644
index 00000000..aebbe237
--- /dev/null
+++ b/app/models/security/price.rb
@@ -0,0 +1,2 @@
+class Security::Price < ApplicationRecord
+end
diff --git a/db/migrate/20240710182529_create_securities.rb b/db/migrate/20240710182529_create_securities.rb
new file mode 100644
index 00000000..5076b223
--- /dev/null
+++ b/db/migrate/20240710182529_create_securities.rb
@@ -0,0 +1,11 @@
+class CreateSecurities < ActiveRecord::Migration[7.2]
+  def change
+    create_table :securities, id: :uuid do |t|
+      t.string :isin, null: false
+      t.string :symbol
+      t.string :name
+
+      t.timestamps
+    end
+  end
+end
diff --git a/db/migrate/20240710182728_create_security_prices.rb b/db/migrate/20240710182728_create_security_prices.rb
new file mode 100644
index 00000000..1069d154
--- /dev/null
+++ b/db/migrate/20240710182728_create_security_prices.rb
@@ -0,0 +1,12 @@
+class CreateSecurityPrices < ActiveRecord::Migration[7.2]
+  def change
+    create_table :security_prices, id: :uuid do |t|
+      t.string :isin
+      t.date :date
+      t.decimal :price, precision: 19, scale: 4
+      t.string :currency, default: "USD"
+
+      t.timestamps
+    end
+  end
+end
diff --git a/db/migrate/20240710184048_create_account_trades.rb b/db/migrate/20240710184048_create_account_trades.rb
new file mode 100644
index 00000000..c4b78f85
--- /dev/null
+++ b/db/migrate/20240710184048_create_account_trades.rb
@@ -0,0 +1,11 @@
+class CreateAccountTrades < ActiveRecord::Migration[7.2]
+  def change
+    create_table :account_trades, id: :uuid do |t|
+      t.references :security, null: false, foreign_key: true, type: :uuid
+      t.decimal :qty, precision: 19, scale: 4
+      t.decimal :price, precision: 19, scale: 4
+
+      t.timestamps
+    end
+  end
+end
diff --git a/db/migrate/20240710184249_create_account_holdings.rb b/db/migrate/20240710184249_create_account_holdings.rb
new file mode 100644
index 00000000..2fb1f5ff
--- /dev/null
+++ b/db/migrate/20240710184249_create_account_holdings.rb
@@ -0,0 +1,14 @@
+class CreateAccountHoldings < ActiveRecord::Migration[7.2]
+  def change
+    create_table :account_holdings, id: :uuid do |t|
+      t.references :account, null: false, foreign_key: true, type: :uuid
+      t.references :security, null: false, foreign_key: true, type: :uuid
+      t.date :date
+      t.decimal :qty, precision: 19, scale: 4
+      t.decimal :amount, precision: 19, scale: 4
+      t.string :currency
+
+      t.timestamps
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 91cbbdbd..fe7c9d9f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.2].define(version: 2024_07_09_152243) do
+ActiveRecord::Schema[7.2].define(version: 2024_07_10_184249) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -48,6 +48,19 @@ ActiveRecord::Schema[7.2].define(version: 2024_07_09_152243) do
     t.index ["transfer_id"], name: "index_account_entries_on_transfer_id"
   end
 
+  create_table "account_holdings", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+    t.uuid "account_id", null: false
+    t.uuid "security_id", null: false
+    t.date "date"
+    t.decimal "qty", precision: 19, scale: 4
+    t.decimal "amount", precision: 19, scale: 4
+    t.string "currency"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["account_id"], name: "index_account_holdings_on_account_id"
+    t.index ["security_id"], name: "index_account_holdings_on_security_id"
+  end
+
   create_table "account_syncs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.uuid "account_id", null: false
     t.string "status", default: "pending", null: false
@@ -60,6 +73,15 @@ ActiveRecord::Schema[7.2].define(version: 2024_07_09_152243) do
     t.index ["account_id"], name: "index_account_syncs_on_account_id"
   end
 
+  create_table "account_trades", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+    t.uuid "security_id", null: false
+    t.decimal "qty", precision: 19, scale: 4
+    t.decimal "price", precision: 19, scale: 4
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["security_id"], name: "index_account_trades_on_security_id"
+  end
+
   create_table "account_transactions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
@@ -321,6 +343,23 @@ ActiveRecord::Schema[7.2].define(version: 2024_07_09_152243) do
     t.datetime "updated_at", null: false
   end
 
+  create_table "securities", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+    t.string "isin", null: false
+    t.string "symbol"
+    t.string "name"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+  end
+
+  create_table "security_prices", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+    t.string "isin"
+    t.date "date"
+    t.decimal "price", precision: 19, scale: 4
+    t.string "currency", default: "USD"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+  end
+
   create_table "settings", force: :cascade do |t|
     t.string "var", null: false
     t.text "value"
@@ -373,7 +412,10 @@ ActiveRecord::Schema[7.2].define(version: 2024_07_09_152243) do
   add_foreign_key "account_balances", "accounts", on_delete: :cascade
   add_foreign_key "account_entries", "account_transfers", column: "transfer_id"
   add_foreign_key "account_entries", "accounts"
+  add_foreign_key "account_holdings", "accounts"
+  add_foreign_key "account_holdings", "securities"
   add_foreign_key "account_syncs", "accounts"
+  add_foreign_key "account_trades", "securities"
   add_foreign_key "account_transactions", "categories", on_delete: :nullify
   add_foreign_key "account_transactions", "merchants"
   add_foreign_key "accounts", "families"
diff --git a/test/fixtures/account/entries.yml b/test/fixtures/account/entries.yml
index 315867aa..680710be 100644
--- a/test/fixtures/account/entries.yml
+++ b/test/fixtures/account/entries.yml
@@ -7,6 +7,15 @@ valuation:
   entryable_type: Account::Valuation
   entryable: one
 
+trade:
+  name: Purchase 10 shares of AAPL
+  date: <%= 1.day.ago.to_date %>
+  amount: 2140 # 10 shares * $214 per share
+  currency: USD
+  account: investment
+  entryable_type: Account::Trade
+  entryable: one
+
 transaction:
   name: Starbucks
   date: <%= 1.day.ago.to_date %>
diff --git a/test/fixtures/account/holdings.yml b/test/fixtures/account/holdings.yml
new file mode 100644
index 00000000..1fa5f3b9
--- /dev/null
+++ b/test/fixtures/account/holdings.yml
@@ -0,0 +1,15 @@
+one:
+  account: investment
+  security: one
+  date: <%= Date.current %>
+  qty: 10
+  amount: 2150 # 10 * $215
+  currency: USD
+
+two:
+  account: investment
+  security: one
+  date: <%= 1.day.ago.to_date %>
+  qty: 10
+  amount: 2140 # 10 * $214
+  currency: USD
diff --git a/test/fixtures/account/trades.yml b/test/fixtures/account/trades.yml
new file mode 100644
index 00000000..35c7ed0c
--- /dev/null
+++ b/test/fixtures/account/trades.yml
@@ -0,0 +1,4 @@
+one:
+  security: one
+  qty: 10
+  price: 214
diff --git a/test/fixtures/securities.yml b/test/fixtures/securities.yml
new file mode 100644
index 00000000..9d34aa4e
--- /dev/null
+++ b/test/fixtures/securities.yml
@@ -0,0 +1,9 @@
+one:
+  isin: US0378331005
+  symbol: aapl
+  name: Apple
+
+two:
+  isin: US5949181045
+  symbol: msft
+  name: Microsoft
diff --git a/test/fixtures/security/prices.yml b/test/fixtures/security/prices.yml
new file mode 100644
index 00000000..b78fe85a
--- /dev/null
+++ b/test/fixtures/security/prices.yml
@@ -0,0 +1,11 @@
+one:
+  isin: US0378331005 # AAPL
+  date: <%= Date.current %>
+  price: 215
+  currency: USD
+
+two:
+  isin: US0378331005 # AAPL
+  date: <%= 1.day.ago.to_date %>
+  price: 214
+  currency: USD
diff --git a/test/models/account/holding_test.rb b/test/models/account/holding_test.rb
new file mode 100644
index 00000000..ebc93ee6
--- /dev/null
+++ b/test/models/account/holding_test.rb
@@ -0,0 +1,7 @@
+require "test_helper"
+
+class Account::HoldingTest < ActiveSupport::TestCase
+  # test "the truth" do
+  #   assert true
+  # end
+end
diff --git a/test/models/account/trade_test.rb b/test/models/account/trade_test.rb
new file mode 100644
index 00000000..eae6ac60
--- /dev/null
+++ b/test/models/account/trade_test.rb
@@ -0,0 +1,7 @@
+require "test_helper"
+
+class Account::TradeTest < ActiveSupport::TestCase
+  # test "the truth" do
+  #   assert true
+  # end
+end
diff --git a/test/models/security/price_test.rb b/test/models/security/price_test.rb
new file mode 100644
index 00000000..a86705dc
--- /dev/null
+++ b/test/models/security/price_test.rb
@@ -0,0 +1,7 @@
+require "test_helper"
+
+class Security::PriceTest < ActiveSupport::TestCase
+  # test "the truth" do
+  #   assert true
+  # end
+end
diff --git a/test/models/security_test.rb b/test/models/security_test.rb
new file mode 100644
index 00000000..8e82099f
--- /dev/null
+++ b/test/models/security_test.rb
@@ -0,0 +1,7 @@
+require "test_helper"
+
+class SecurityTest < ActiveSupport::TestCase
+  # test "the truth" do
+  #   assert true
+  # end
+end
-- 
2.53.0


From 7d2edebd64e38ac2d73335134fcf3fba91b6737a Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Date: Thu, 11 Jul 2024 12:51:07 -0400
Subject: [PATCH 2/7] Add portfolio to demo data

---
 app/models/demo/generator.rb | 84 +++++++++++++++++++++++++++---------
 1 file changed, 64 insertions(+), 20 deletions(-)

diff --git a/app/models/demo/generator.rb b/app/models/demo/generator.rb
index 5a1c1a9e..9df6bed4 100644
--- a/app/models/demo/generator.rb
+++ b/app/models/demo/generator.rb
@@ -13,31 +13,33 @@ class Demo::Generator
   end
 
   def reset_data!
-    clear_data!
-    create_user!
+    Family.transaction do
+      clear_data!
+      create_user!
 
-    puts "user reset"
+      puts "user reset"
 
-    create_tags!
-    create_categories!
-    create_merchants!
+      create_tags!
+      create_categories!
+      create_merchants!
 
-    puts "tags, categories, merchants created"
+      puts "tags, categories, merchants created"
 
-    create_credit_card_account!
-    create_checking_account!
-    create_savings_account!
+      create_credit_card_account!
+      create_checking_account!
+      create_savings_account!
 
-    create_investment_account!
-    create_house_and_mortgage!
-    create_car_and_loan!
+      create_investment_account!
+      create_house_and_mortgage!
+      create_car_and_loan!
 
-    puts "accounts created"
+      puts "accounts created"
 
-    family.sync
+      family.sync
 
-    puts "balances synced"
-    puts "Demo data loaded successfully!"
+      puts "balances synced"
+      puts "Demo data loaded successfully!"
+    end
   end
 
   private
@@ -55,6 +57,8 @@ class Demo::Generator
 
     def clear_data!
       ExchangeRate.destroy_all
+      Security.destroy_all
+      Security::Price.destroy_all
     end
 
     def create_user!
@@ -161,16 +165,52 @@ class Demo::Generator
       end
     end
 
+    def load_securities!
+      securities = [
+        { isin: "US0378331005", symbol: "AAPL", name: "Apple Inc.", reference_price: 210 },
+        { isin: "JP3633400001", symbol: "TM", name: "Toyota Motor Corporation", reference_price: 202 },
+        { isin: "US5949181045", symbol: "MSFT", name: "Microsoft Corporation", reference_price: 455 }
+      ]
+
+      securities.each do |security_attributes|
+        security = Security.create! security_attributes.except(:reference_price)
+
+        # Load prices for last 2 years
+        (730.days.ago.to_date..Date.current).each do |date|
+          reference = security_attributes[:reference_price]
+          low_price = reference - 20
+          high_price = reference + 20
+          Security::Price.create! \
+            isin: security.isin,
+            date: date,
+            price: Faker::Number.positive(from: low_price, to: high_price)
+        end
+      end
+    end
+
     def create_investment_account!
+      load_securities!
+
       account = family.accounts.create! \
         accountable: Investment.new,
         name: "Robinhood",
         balance: 100000,
         institution: family.institutions.find_or_create_by(name: "Robinhood")
 
-      create_valuation!(account, 2.years.ago.to_date, 60000)
-      create_valuation!(account, 1.year.ago.to_date, 70000)
-      create_valuation!(account, 3.months.ago.to_date, 92000)
+      15.times do
+        date = Faker::Number.positive(to: 730).days.ago.to_date
+        security = securities.sample
+        qty = Faker::Number.between(from: -10, to: 10)
+        price = Security::Price.find_by!(isin: security.isin, date: date).price
+        name_prefix = qty < 0 ? "Sell " : "Buy "
+
+        account.entries.create! \
+          date: date,
+          amount: qty * price,
+          currency: "USD",
+          name: name_prefix + "#{qty} shares of #{security.symbol}",
+          entryable: Account::Trade.new(qty: qty, price: price, security: security)
+      end
     end
 
     def create_house_and_mortgage!
@@ -262,6 +302,10 @@ class Demo::Generator
       tag_from_merchant || tags.find { |t| t.name == "Demo Tag" }
     end
 
+    def securities
+      @securities ||= Security.all.to_a
+    end
+
     def merchants
       @merchants ||= family.merchants
     end
-- 
2.53.0


From ae067b07f8703e8c22975e24e4580746e2280d2a Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Date: Thu, 11 Jul 2024 15:23:09 -0400
Subject: [PATCH 3/7] Setup initial tests

---
 app/models/account/balance/syncer.rb          |  8 +-
 app/models/account/holding/syncer.rb          | 42 +++++++++
 app/models/security.rb                        | 11 +++
 .../20240710184249_create_account_holdings.rb |  3 +
 db/schema.rb                                  |  2 +
 test/fixtures/account/holdings.yml            |  4 +-
 test/fixtures/account/trades.yml              |  2 +-
 test/fixtures/securities.yml                  |  4 +-
 test/models/account/holding/syncer_test.rb    | 87 +++++++++++++++++++
 test/models/account/trade_test.rb             |  6 +-
 10 files changed, 155 insertions(+), 14 deletions(-)
 create mode 100644 app/models/account/holding/syncer.rb
 create mode 100644 test/models/account/holding/syncer_test.rb

diff --git a/app/models/account/balance/syncer.rb b/app/models/account/balance/syncer.rb
index 690c52b2..2af85166 100644
--- a/app/models/account/balance/syncer.rb
+++ b/app/models/account/balance/syncer.rb
@@ -22,13 +22,9 @@ class Account::Balance::Syncer
     attr_reader :sync_start_date, :account
 
     def upsert_balances!(balances)
+      current_time = Time.now
       balances_to_upsert = balances.map do |balance|
-        {
-          date: balance.date,
-          balance: balance.balance,
-          currency: balance.currency,
-          updated_at: Time.now
-        }
+        balance.attributes.slice("date", "balance", "currency").merge("updated_at" => current_time)
       end
 
       account.balances.upsert_all(balances_to_upsert, unique_by: %i[account_id date currency])
diff --git a/app/models/account/holding/syncer.rb b/app/models/account/holding/syncer.rb
new file mode 100644
index 00000000..a85313eb
--- /dev/null
+++ b/app/models/account/holding/syncer.rb
@@ -0,0 +1,42 @@
+class Account::Holding::Syncer
+  attr_reader :warnings
+
+  def initialize(account, start_date: nil)
+    @account = account
+    @warnings = []
+    @sync_start_date = calculate_sync_start_date(start_date)
+  end
+
+  def run
+    daily_holdings = calculate_daily_holdings
+
+    Account::Holding.transaction do
+      upsert_holdings! daily_holdings
+    end
+  end
+
+  private
+
+    attr_reader :account, :sync_start_date
+
+    def calculate_daily_holdings
+      trades = account.entries.account_trades.where("date >= ?", sync_start_date).to_a
+
+      [] if trades.empty?
+    end
+
+    def upsert_holdings!(holdings)
+      current_time = Time.now
+      holdings_to_upsert = holdings.map do |holding|
+        holding.attributes
+               .slice("date", "currency", "qty", "price", "amount", "security_id")
+               .merge("updated_at" => current_time)
+      end
+
+      account.holdings.upsert_all(holdings_to_upsert, unique_by: %i[account_id security_id date currency])
+    end
+
+    def calculate_sync_start_date(start_date)
+      start_date
+    end
+end
diff --git a/app/models/security.rb b/app/models/security.rb
index e4688f75..106925bf 100644
--- a/app/models/security.rb
+++ b/app/models/security.rb
@@ -1,3 +1,14 @@
 class Security < ApplicationRecord
+  before_save :normalize_identifiers
+
   has_many :trades, dependent: :nullify, class_name: "Account::Trade"
+
+  validates :isin, presence: true, uniqueness: { case_sensitive: false }
+
+  private
+
+    def normalize_identifiers
+      self.isin = isin.upcase
+      self.symbol = symbol.upcase
+    end
 end
diff --git a/db/migrate/20240710184249_create_account_holdings.rb b/db/migrate/20240710184249_create_account_holdings.rb
index 2fb1f5ff..33ccb4ac 100644
--- a/db/migrate/20240710184249_create_account_holdings.rb
+++ b/db/migrate/20240710184249_create_account_holdings.rb
@@ -5,10 +5,13 @@ class CreateAccountHoldings < ActiveRecord::Migration[7.2]
       t.references :security, null: false, foreign_key: true, type: :uuid
       t.date :date
       t.decimal :qty, precision: 19, scale: 4
+      t.decimal :price, precision: 19, scale: 4
       t.decimal :amount, precision: 19, scale: 4
       t.string :currency
 
       t.timestamps
     end
+
+    add_index :account_holdings, %i[account_id security_id date currency], unique: true
   end
 end
diff --git a/db/schema.rb b/db/schema.rb
index fe7c9d9f..eebb6a59 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -53,10 +53,12 @@ ActiveRecord::Schema[7.2].define(version: 2024_07_10_184249) do
     t.uuid "security_id", null: false
     t.date "date"
     t.decimal "qty", precision: 19, scale: 4
+    t.decimal "price", precision: 19, scale: 4
     t.decimal "amount", precision: 19, scale: 4
     t.string "currency"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.index ["account_id", "security_id", "date", "currency"], name: "idx_on_account_id_security_id_date_currency_234024c8e3", unique: true
     t.index ["account_id"], name: "index_account_holdings_on_account_id"
     t.index ["security_id"], name: "index_account_holdings_on_security_id"
   end
diff --git a/test/fixtures/account/holdings.yml b/test/fixtures/account/holdings.yml
index 1fa5f3b9..fabc6453 100644
--- a/test/fixtures/account/holdings.yml
+++ b/test/fixtures/account/holdings.yml
@@ -1,6 +1,6 @@
 one:
   account: investment
-  security: one
+  security: aapl
   date: <%= Date.current %>
   qty: 10
   amount: 2150 # 10 * $215
@@ -8,7 +8,7 @@ one:
 
 two:
   account: investment
-  security: one
+  security: aapl
   date: <%= 1.day.ago.to_date %>
   qty: 10
   amount: 2140 # 10 * $214
diff --git a/test/fixtures/account/trades.yml b/test/fixtures/account/trades.yml
index 35c7ed0c..b782ec63 100644
--- a/test/fixtures/account/trades.yml
+++ b/test/fixtures/account/trades.yml
@@ -1,4 +1,4 @@
 one:
-  security: one
+  security: aapl
   qty: 10
   price: 214
diff --git a/test/fixtures/securities.yml b/test/fixtures/securities.yml
index 9d34aa4e..790b4631 100644
--- a/test/fixtures/securities.yml
+++ b/test/fixtures/securities.yml
@@ -1,9 +1,9 @@
-one:
+aapl:
   isin: US0378331005
   symbol: aapl
   name: Apple
 
-two:
+msft:
   isin: US5949181045
   symbol: msft
   name: Microsoft
diff --git a/test/models/account/holding/syncer_test.rb b/test/models/account/holding/syncer_test.rb
new file mode 100644
index 00000000..2286fd5f
--- /dev/null
+++ b/test/models/account/holding/syncer_test.rb
@@ -0,0 +1,87 @@
+require "test_helper"
+
+class Account::Holding::SyncerTest < ActiveSupport::TestCase
+  include Account::EntriesTestHelper
+
+  setup do
+    @account = families(:empty).accounts.create!(name: "Test Brokerage", balance: 20000, currency: "USD", accountable: Investment.new)
+  end
+
+  test "account with no trades has no holdings" do
+    run_sync_for(@account)
+
+    assert_equal [], @account.holdings
+  end
+
+  test "can buy and sell securities" do
+    run_sync_for(@account)
+
+    security = create_security("AMZN", prices: [
+      { date: 2.days.ago.to_date, price: 214 },
+      { date: 1.day.ago.to_date, price: 215 },
+      { date: Date.current, price: 216 }
+    ])
+
+    create_trade(security, qty: 10, date: 2.days.ago.to_date)
+    create_trade(security, qty: 2, date: 1.day.ago.to_date)
+    create_trade(security, qty: -10, date: Date.current)
+
+    expected = [
+      { symbol: "AMZN", qty: 10, price: 214, amount: 10 * 214, date: 1.day.ago.to_date },
+      { symbol: "AMZN", qty: 12, price: 215, amount: 12 * 215, date: 1.day.ago.to_date },
+      { symbol: "AMZN", qty: 2, price: 216, amount: 2 * 216, date: Date.current }
+    ]
+
+    assert_holdings(expected)
+  end
+
+  private
+
+    def assert_holdings(expected_holdings)
+      actual = @account.holdings.map do |holding|
+        {
+          symbol: holding.security.symbol,
+          qty: holding.qty,
+          price: holding.price,
+          amount: holding.amount,
+          date: holding.date
+        }
+      end
+
+      assert_equal expected_holdings, actual
+    end
+
+    def create_security(symbol, prices:)
+      isin_codes = {
+        "AMZN" => "US0231351067"
+      }
+
+      isin = isin_codes[symbol]
+
+      prices.each do |price|
+        Security::Price.create! isin: isin, date: price[:date], price: price[:price]
+      end
+
+      Security.create! isin: isin, symbol: symbol
+    end
+
+    def create_trade(security, qty:, date:)
+      price = Security::Price.find_by!(isin: security.isin, date: date).price
+
+      trade = Account::Trade.new \
+        qty: qty,
+        security: security,
+        price: price
+
+      @account.entries.create! \
+        name: "Trade",
+        date: date,
+        amount: qty * price,
+        currency: "USD",
+        entryable: trade
+    end
+
+    def run_sync_for(account)
+      Account::Holding::Syncer.new(account).run
+    end
+end
diff --git a/test/models/account/trade_test.rb b/test/models/account/trade_test.rb
index eae6ac60..41fa603a 100644
--- a/test/models/account/trade_test.rb
+++ b/test/models/account/trade_test.rb
@@ -1,7 +1,7 @@
 require "test_helper"
 
 class Account::TradeTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  test "cannot sell more shares than are owned" do
+    flunk
+  end
 end
-- 
2.53.0


From 07c299555d3b469626d13ebd36034d933e7c4063 Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Date: Thu, 11 Jul 2024 18:43:35 -0400
Subject: [PATCH 4/7] Rough sketch of sync logic

---
 app/models/account/holding.rb              |  2 +
 app/models/account/holding/syncer.rb       | 60 ++++++++++++++++++++--
 app/models/account/portfolio.rb            |  9 ++++
 test/models/account/holding/syncer_test.rb | 52 ++++++++++++-------
 4 files changed, 101 insertions(+), 22 deletions(-)
 create mode 100644 app/models/account/portfolio.rb

diff --git a/app/models/account/holding.rb b/app/models/account/holding.rb
index 626795a1..b5a63248 100644
--- a/app/models/account/holding.rb
+++ b/app/models/account/holding.rb
@@ -1,4 +1,6 @@
 class Account::Holding < ApplicationRecord
   belongs_to :account
   belongs_to :security
+
+  scope :chronological, -> { order(:date) }
 end
diff --git a/app/models/account/holding/syncer.rb b/app/models/account/holding/syncer.rb
index a85313eb..09601136 100644
--- a/app/models/account/holding/syncer.rb
+++ b/app/models/account/holding/syncer.rb
@@ -20,9 +20,45 @@ class Account::Holding::Syncer
     attr_reader :account, :sync_start_date
 
     def calculate_daily_holdings
-      trades = account.entries.account_trades.where("date >= ?", sync_start_date).to_a
+      all_trade_entries = account.entries
+                                 .account_trades
+                                 .includes(entryable: :security)
+                                 .where("date >= ?", sync_start_date)
+                                 .order(:date)
+      holdings = []
+      portfolio = get_prior_portfolio
 
-      [] if trades.empty?
+      (sync_start_date..Date.current).each do |date|
+        trade_entries = all_trade_entries.select { |trade| trade.date == date }
+
+        trade_entries.each do |entry|
+          trade = entry.account_trade
+          prior_qty = portfolio.dig(trade.security.isin, :qty) || 0
+          new_qty = prior_qty + trade.qty
+
+          portfolio[trade.security.isin] = {
+            qty: new_qty,
+            price: trade.price,
+            amount: new_qty * trade.price,
+            security_id: trade.security_id
+          }
+        end
+
+        portfolio.each do |isin, holding|
+          price = Security::Price.find_by!(date: date, isin: isin).price
+
+          holding = account.holdings.build \
+            date: date,
+            security_id: holding[:security_id],
+            qty: holding[:qty],
+            price: price,
+            amount: price * holding[:qty]
+
+          holdings << holding
+        end
+      end
+
+      holdings
     end
 
     def upsert_holdings!(holdings)
@@ -36,7 +72,25 @@ class Account::Holding::Syncer
       account.holdings.upsert_all(holdings_to_upsert, unique_by: %i[account_id security_id date currency])
     end
 
+    def get_prior_portfolio
+      portfolio = {}
+      prior_day_holdings = account.holdings.where(date: sync_start_date - 1.day)
+
+      if prior_day_holdings.any?
+        prior_day_holdings.each do |holding|
+          portfolio[holding.security.isin] = {
+            qty: holding.qty,
+            price: holding.price,
+            amount: holding.amount,
+            security_id: holding.security_id
+          }
+        end
+      end
+
+      portfolio
+    end
+
     def calculate_sync_start_date(start_date)
-      start_date
+      start_date || account.entries.account_trades.order(:date).first.try(:date) || Date.current
     end
 end
diff --git a/app/models/account/portfolio.rb b/app/models/account/portfolio.rb
new file mode 100644
index 00000000..a9454af3
--- /dev/null
+++ b/app/models/account/portfolio.rb
@@ -0,0 +1,9 @@
+class Account::Portfolio
+  class << self
+    def from_holdings(account, date: Date.current)
+    end
+  end
+
+  def initialize
+  end
+end
diff --git a/test/models/account/holding/syncer_test.rb b/test/models/account/holding/syncer_test.rb
index 2286fd5f..8a84295b 100644
--- a/test/models/account/holding/syncer_test.rb
+++ b/test/models/account/holding/syncer_test.rb
@@ -14,46 +14,60 @@ class Account::Holding::SyncerTest < ActiveSupport::TestCase
   end
 
   test "can buy and sell securities" do
-    run_sync_for(@account)
-
-    security = create_security("AMZN", prices: [
+    security1 = create_security("AMZN", prices: [
       { date: 2.days.ago.to_date, price: 214 },
       { date: 1.day.ago.to_date, price: 215 },
       { date: Date.current, price: 216 }
     ])
 
-    create_trade(security, qty: 10, date: 2.days.ago.to_date)
-    create_trade(security, qty: 2, date: 1.day.ago.to_date)
-    create_trade(security, qty: -10, date: Date.current)
+    security2 = create_security("NVDA", prices: [
+      { date: 1.day.ago.to_date, price: 122 },
+      { date: Date.current, price: 124 }
+    ])
+
+    create_trade(security1, qty: 10, date: 2.days.ago.to_date) # buy 10 shares of AMZN
+
+    create_trade(security1, qty: 2, date: 1.day.ago.to_date) # buy 2 shares of AMZN
+    create_trade(security2, qty: 20, date: 1.day.ago.to_date) # buy 20 shares of NVDA
+
+    create_trade(security1, qty: -10, date: Date.current) # sell 10 shares of AMZN
 
     expected = [
-      { symbol: "AMZN", qty: 10, price: 214, amount: 10 * 214, date: 1.day.ago.to_date },
+      { symbol: "AMZN", qty: 10, price: 214, amount: 10 * 214, date: 2.days.ago.to_date },
       { symbol: "AMZN", qty: 12, price: 215, amount: 12 * 215, date: 1.day.ago.to_date },
-      { symbol: "AMZN", qty: 2, price: 216, amount: 2 * 216, date: Date.current }
+      { symbol: "AMZN", qty: 2, price: 216, amount: 2 * 216, date: Date.current },
+      { symbol: "NVDA", qty: 20, price: 122, amount: 20 * 122, date: 1.day.ago.to_date },
+      { symbol: "NVDA", qty: 20, price: 124, amount: 20 * 124, date: Date.current }
     ]
 
+    run_sync_for(@account)
+
     assert_holdings(expected)
   end
 
   private
 
     def assert_holdings(expected_holdings)
-      actual = @account.holdings.map do |holding|
-        {
-          symbol: holding.security.symbol,
-          qty: holding.qty,
-          price: holding.price,
-          amount: holding.amount,
-          date: holding.date
-        }
-      end
+      holdings = @account.holdings.includes(:security).to_a
+      expected_holdings.each do |expected_holding|
+        actual_holding = holdings.find { |holding| holding.security.symbol == expected_holding[:symbol] && holding.date == expected_holding[:date] }
+        date = expected_holding[:date]
+        expected_price = expected_holding[:price]
+        expected_qty = expected_holding[:qty]
+        expected_amount = expected_holding[:amount]
+        symbol = expected_holding[:symbol]
 
-      assert_equal expected_holdings, actual
+        assert actual_holding, "expected #{symbol} holding on date: #{date}"
+        assert_equal expected_holding[:qty], actual_holding.qty, "expected #{expected_qty} qty for holding #{symbol} on date: #{date}"
+        assert_equal expected_holding[:amount], actual_holding.amount, "expected #{expected_amount} amount for holding #{symbol} on date: #{date}"
+        assert_equal expected_holding[:price], actual_holding.price, "expected #{expected_price} price for holding #{symbol} on date: #{date}"
+      end
     end
 
     def create_security(symbol, prices:)
       isin_codes = {
-        "AMZN" => "US0231351067"
+        "AMZN" => "US0231351067",
+        "NVDA" => "US67066G1040"
       }
 
       isin = isin_codes[symbol]
-- 
2.53.0


From 89157d08266f935822c7e4ccae2beb39580315f5 Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Date: Fri, 12 Jul 2024 19:03:33 -0400
Subject: [PATCH 5/7] Clean up trade sync logic

---
 app/models/account/holding/syncer.rb | 120 +++++++++++++--------------
 app/models/account/portfolio.rb      |   9 --
 test/models/account/trade_test.rb    |   3 -
 3 files changed, 60 insertions(+), 72 deletions(-)
 delete mode 100644 app/models/account/portfolio.rb

diff --git a/app/models/account/holding/syncer.rb b/app/models/account/holding/syncer.rb
index 09601136..3f2af7a7 100644
--- a/app/models/account/holding/syncer.rb
+++ b/app/models/account/holding/syncer.rb
@@ -4,64 +4,69 @@ class Account::Holding::Syncer
   def initialize(account, start_date: nil)
     @account = account
     @warnings = []
-    @sync_start_date = calculate_sync_start_date(start_date)
+    @sync_date_range = calculate_sync_start_date(start_date)..Date.current
+    @portfolio = {}
+
+    load_prior_portfolio if start_date
   end
 
   def run
-    daily_holdings = calculate_daily_holdings
+    holdings = []
 
-    Account::Holding.transaction do
-      upsert_holdings! daily_holdings
+    sync_date_range.each do |date|
+      holdings += build_holdings_for_date(date)
     end
+
+    upsert_holdings holdings
   end
 
   private
 
-    attr_reader :account, :sync_start_date
+    attr_reader :account, :sync_date_range
 
-    def calculate_daily_holdings
-      all_trade_entries = account.entries
-                                 .account_trades
-                                 .includes(entryable: :security)
-                                 .where("date >= ?", sync_start_date)
-                                 .order(:date)
-      holdings = []
-      portfolio = get_prior_portfolio
-
-      (sync_start_date..Date.current).each do |date|
-        trade_entries = all_trade_entries.select { |trade| trade.date == date }
-
-        trade_entries.each do |entry|
-          trade = entry.account_trade
-          prior_qty = portfolio.dig(trade.security.isin, :qty) || 0
-          new_qty = prior_qty + trade.qty
-
-          portfolio[trade.security.isin] = {
-            qty: new_qty,
-            price: trade.price,
-            amount: new_qty * trade.price,
-            security_id: trade.security_id
-          }
-        end
-
-        portfolio.each do |isin, holding|
-          price = Security::Price.find_by!(date: date, isin: isin).price
-
-          holding = account.holdings.build \
-            date: date,
-            security_id: holding[:security_id],
-            qty: holding[:qty],
-            price: price,
-            amount: price * holding[:qty]
-
-          holdings << holding
-        end
-      end
-
-      holdings
+    def sync_entries
+      @sync_entries ||= account.entries
+                               .account_trades
+                               .includes(entryable: :security)
+                               .where("date >= ?", sync_date_range.begin)
+                               .order(:date)
     end
 
-    def upsert_holdings!(holdings)
+    def build_holdings_for_date(date)
+      trades = sync_entries.select { |trade| trade.date == date }
+
+      @portfolio = generate_next_portfolio(@portfolio, trades)
+
+      @portfolio.map do |isin, holding|
+        price = Security::Price.find_by!(date: date, isin: isin).price
+
+        account.holdings.build \
+          date: date,
+          security_id: holding[:security_id],
+          qty: holding[:qty],
+          price: price,
+          amount: price * holding[:qty]
+      end
+    end
+
+    def generate_next_portfolio(prior_portfolio, trade_entries)
+      trade_entries.each_with_object(prior_portfolio) do |entry, new_portfolio|
+        trade = entry.account_trade
+
+        price = trade.price
+        prior_qty = prior_portfolio.dig(trade.security.isin, :qty) || 0
+        new_qty = prior_qty + trade.qty
+
+        new_portfolio[trade.security.isin] = {
+          qty: new_qty,
+          price: price,
+          amount: new_qty * price,
+          security_id: trade.security_id
+        }
+      end
+    end
+
+    def upsert_holdings(holdings)
       current_time = Time.now
       holdings_to_upsert = holdings.map do |holding|
         holding.attributes
@@ -72,22 +77,17 @@ class Account::Holding::Syncer
       account.holdings.upsert_all(holdings_to_upsert, unique_by: %i[account_id security_id date currency])
     end
 
-    def get_prior_portfolio
-      portfolio = {}
-      prior_day_holdings = account.holdings.where(date: sync_start_date - 1.day)
+    def load_prior_portfolio
+      prior_day_holdings = account.holdings.where(date: sync_date_range.begin - 1.day)
 
-      if prior_day_holdings.any?
-        prior_day_holdings.each do |holding|
-          portfolio[holding.security.isin] = {
-            qty: holding.qty,
-            price: holding.price,
-            amount: holding.amount,
-            security_id: holding.security_id
-          }
-        end
+      prior_day_holdings.each do |holding|
+        @portfolio[holding.security.isin] = {
+          qty: holding.qty,
+          price: holding.price,
+          amount: holding.amount,
+          security_id: holding.security_id
+        }
       end
-
-      portfolio
     end
 
     def calculate_sync_start_date(start_date)
diff --git a/app/models/account/portfolio.rb b/app/models/account/portfolio.rb
deleted file mode 100644
index a9454af3..00000000
--- a/app/models/account/portfolio.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-class Account::Portfolio
-  class << self
-    def from_holdings(account, date: Date.current)
-    end
-  end
-
-  def initialize
-  end
-end
diff --git a/test/models/account/trade_test.rb b/test/models/account/trade_test.rb
index 41fa603a..b571a70b 100644
--- a/test/models/account/trade_test.rb
+++ b/test/models/account/trade_test.rb
@@ -1,7 +1,4 @@
 require "test_helper"
 
 class Account::TradeTest < ActiveSupport::TestCase
-  test "cannot sell more shares than are owned" do
-    flunk
-  end
 end
-- 
2.53.0


From cb0a977f6c0127ceec7072ebc6e5d6e96ee7a078 Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Date: Sat, 13 Jul 2024 10:12:03 -0400
Subject: [PATCH 6/7] Add trade validation

---
 app/models/account.rb             |  8 ++++++++
 app/models/account/entry.rb       | 13 ++++++++++++-
 app/models/account/trade.rb       | 11 +++++++++++
 test/models/account/entry_test.rb | 17 +++++++++++++++++
 test/models/account_test.rb       |  9 +++++++++
 5 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/app/models/account.rb b/app/models/account.rb
index 25625e46..98a9f843 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -109,4 +109,12 @@ class Account < ApplicationRecord
         entryable: Account::Valuation.new
     end
   end
+
+  def holding_qty(security, date: Date.current)
+    entries.account_trades
+           .joins("JOIN account_trades ON account_entries.entryable_id = account_trades.id")
+           .where(account_trades: { security_id: security.id })
+           .where("account_entries.date <= ?", date)
+           .sum("account_trades.qty")
+  end
 end
diff --git a/app/models/account/entry.rb b/app/models/account/entry.rb
index cdc6410c..2fd84e64 100644
--- a/app/models/account/entry.rb
+++ b/app/models/account/entry.rb
@@ -11,6 +11,7 @@ class Account::Entry < ApplicationRecord
 
   validates :date, :amount, :currency, presence: true
   validates :date, uniqueness: { scope: [ :account_id, :entryable_type ] }, if: -> { account_valuation? }
+  validate :trade_valid?, if: -> { account_trade? }
 
   scope :chronological, -> { order(:date, :created_at) }
   scope :reverse_chronological, -> { order(date: :desc, created_at: :desc) }
@@ -123,7 +124,7 @@ class Account::Entry < ApplicationRecord
 
     def income_total(currency = "USD")
       without_transfers.account_transactions.includes(:entryable)
-                          .where("account_entries.amount <= 0")
+        .where("account_entries.amount <= 0")
                        .map { |e| e.amount_money.exchange_to(currency, date: e.date, fallback_rate: 0) }
                        .sum
     end
@@ -191,4 +192,14 @@ class Account::Entry < ApplicationRecord
         previous: previous_entry&.amount_money,
         favorable_direction: account.favorable_direction
     end
+
+    def trade_valid?
+      if account_trade.sell?
+        current_qty = account.holding_qty(account_trade.security)
+
+        if current_qty < account_trade.qty.abs
+          errors.add(:base, "cannot sell #{account_trade.qty.abs} shares of #{account_trade.security.symbol} because you only own #{current_qty} shares")
+        end
+      end
+    end
 end
diff --git a/app/models/account/trade.rb b/app/models/account/trade.rb
index 52c58467..35cafe19 100644
--- a/app/models/account/trade.rb
+++ b/app/models/account/trade.rb
@@ -3,6 +3,9 @@ class Account::Trade < ApplicationRecord
 
   belongs_to :security
 
+  validates :qty, presence: true, numericality: { other_than: 0 }
+  validates :price, presence: true
+
   class << self
     def search(_params)
       all
@@ -12,4 +15,12 @@ class Account::Trade < ApplicationRecord
       false
     end
   end
+
+  def sell?
+    qty < 0
+  end
+
+  def buy?
+    qty > 0
+  end
 end
diff --git a/test/models/account/entry_test.rb b/test/models/account/entry_test.rb
index edea8464..c0a10529 100644
--- a/test/models/account/entry_test.rb
+++ b/test/models/account/entry_test.rb
@@ -12,6 +12,7 @@ class Account::EntryTest < ActiveSupport::TestCase
 
     new_valuation = Account::Entry.new \
       entryable: Account::Valuation.new,
+      account: existing_valuation.account,
       date: existing_valuation.date, # invalid
       currency: existing_valuation.currency,
       amount: existing_valuation.amount
@@ -89,4 +90,20 @@ class Account::EntryTest < ActiveSupport::TestCase
     assert create_transaction(amount: -10).inflow?
     assert create_transaction(amount: 10).outflow?
   end
+
+  test "cannot sell more shares of stock than owned" do
+    account = families(:empty).accounts.create! name: "Test", balance: 0, accountable: Investment.new
+    security = securities(:aapl)
+
+    error = assert_raises ActiveRecord::RecordInvalid do
+      account.entries.create! \
+        date: Date.current,
+        amount: 100,
+        currency: "USD",
+        name: "Sell 10 shares of AMZN",
+        entryable: Account::Trade.new(qty: -10, price: 200, security: security)
+    end
+
+    assert_match /cannot sell 10.0 shares of aapl because you only own 0.0 shares/, error.message
+  end
 end
diff --git a/test/models/account_test.rb b/test/models/account_test.rb
index b0c45a7d..ddafdc78 100644
--- a/test/models/account_test.rb
+++ b/test/models/account_test.rb
@@ -74,4 +74,13 @@ class AccountTest < ActiveSupport::TestCase
   test "generates empty series if no balances and no exchange rate" do
     assert_equal 0, @account.series(currency: "NZD").values.count
   end
+
+  test "calculates shares owned of holding for date" do
+    account = accounts(:investment)
+    security = securities(:aapl)
+
+    assert_equal 10, account.holding_qty(security, date: Date.current)
+    assert_equal 10, account.holding_qty(security, date: 1.day.ago.to_date)
+    assert_equal 0, account.holding_qty(security, date: 2.days.ago.to_date)
+  end
 end
-- 
2.53.0


From e7c6912551927ceb1c0eb5e596d8f5dcfed0f6d6 Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Date: Sat, 13 Jul 2024 12:40:53 -0400
Subject: [PATCH 7/7] Integrate trades into sync process

---
 app/models/account/balance/syncer.rb        | 14 ++++----
 app/models/account/sync.rb                  |  9 +++++
 test/models/account/balance/syncer_test.rb  | 39 ++++++++++++---------
 test/models/account/sync_test.rb            | 16 +++++++--
 test/support/account/entries_test_helper.rb |  9 +++++
 test/system/transfers_test.rb               |  2 +-
 6 files changed, 62 insertions(+), 27 deletions(-)

diff --git a/app/models/account/balance/syncer.rb b/app/models/account/balance/syncer.rb
index 601875e8..1756edc3 100644
--- a/app/models/account/balance/syncer.rb
+++ b/app/models/account/balance/syncer.rb
@@ -45,9 +45,9 @@ class Account::Balance::Syncer
       return valuation.amount if valuation
       return derived_sync_start_balance(entries) unless prior_balance
 
-      transactions = entries.select { |e| e.date == date && e.account_transaction? }
+      entries = entries.select { |e| e.date == date }
 
-      prior_balance - net_transaction_flows(transactions)
+      prior_balance - net_entry_flows(entries)
     end
 
     def calculate_daily_balances
@@ -91,19 +91,19 @@ class Account::Balance::Syncer
     end
 
     def derived_sync_start_balance(entries)
-      transactions = entries.select { |e| e.account_transaction? && e.date > sync_start_date }
+      transactions_and_trades = entries.reject { |e| e.account_valuation? }.select { |e| e.date > sync_start_date }
 
-      account.balance + net_transaction_flows(transactions)
+      account.balance + net_entry_flows(transactions_and_trades)
     end
 
     def find_prior_balance
       account.balances.where("date < ?", sync_start_date).order(date: :desc).first&.balance
     end
 
-    def net_transaction_flows(transactions, target_currency = account.currency)
-      converted_transaction_amounts = transactions.map { |t| t.amount_money.exchange_to(target_currency, date: t.date) }
+    def net_entry_flows(entries, target_currency = account.currency)
+      converted_entry_amounts = entries.map { |t| t.amount_money.exchange_to(target_currency, date: t.date) }
 
-      flows = converted_transaction_amounts.sum(&:amount)
+      flows = converted_entry_amounts.sum(&:amount)
 
       account.liability? ? flows * -1 : flows
     end
diff --git a/app/models/account/sync.rb b/app/models/account/sync.rb
index 90e18ebc..0f8a1c9a 100644
--- a/app/models/account/sync.rb
+++ b/app/models/account/sync.rb
@@ -17,6 +17,7 @@ class Account::Sync < ApplicationRecord
     start!
 
     sync_balances
+    sync_holdings
 
     complete!
   rescue StandardError => error
@@ -33,6 +34,14 @@ class Account::Sync < ApplicationRecord
       append_warnings(syncer.warnings)
     end
 
+    def sync_holdings
+      syncer = Account::Holding::Syncer.new(account, start_date: start_date)
+
+      syncer.run
+
+      append_warnings(syncer.warnings)
+    end
+
     def append_warnings(new_warnings)
       update! warnings: warnings + new_warnings
     end
diff --git a/test/models/account/balance/syncer_test.rb b/test/models/account/balance/syncer_test.rb
index 6d1e3b10..4f548168 100644
--- a/test/models/account/balance/syncer_test.rb
+++ b/test/models/account/balance/syncer_test.rb
@@ -5,13 +5,13 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase
 
   setup do
     @account = families(:empty).accounts.create!(name: "Test", balance: 20000, currency: "USD", accountable: Depository.new)
+    @investment_account = families(:empty).accounts.create!(name: "Test Investment", balance: 50000, currency: "USD", accountable: Investment.new)
   end
 
   test "syncs account with no entries" do
     assert_equal 0, @account.balances.count
 
-    syncer = Account::Balance::Syncer.new(@account)
-    syncer.run
+    run_sync_for @account
 
     assert_equal [ @account.balance ], @account.balances.chronological.map(&:balance)
   end
@@ -19,8 +19,7 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase
   test "syncs account with valuations only" do
     create_valuation(account: @account, date: 2.days.ago.to_date, amount: 22000)
 
-    syncer = Account::Balance::Syncer.new(@account)
-    syncer.run
+    run_sync_for @account
 
     assert_equal 22000, @account.balance
     assert_equal [ 22000, 22000, 22000 ], @account.balances.chronological.map(&:balance)
@@ -30,21 +29,28 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase
     create_transaction(account: @account, date: 4.days.ago.to_date, amount: 100)
     create_transaction(account: @account, date: 2.days.ago.to_date, amount: -500)
 
-    syncer = Account::Balance::Syncer.new(@account)
-    syncer.run
+    run_sync_for @account
 
     assert_equal 20000, @account.balance
     assert_equal [ 19600, 19500, 19500, 20000, 20000, 20000 ], @account.balances.chronological.map(&:balance)
   end
 
+  test "syncs account with trades only" do
+    aapl = securities(:aapl)
+    create_trade(account: @investment_account, date: 1.day.ago.to_date, security: aapl, qty: 10, price: 200)
+
+    run_sync_for @investment_account
+
+    assert_equal [ 52000, 50000, 50000 ], @investment_account.balances.chronological.map(&:balance)
+  end
+
   test "syncs account with valuations and transactions" do
     create_valuation(account: @account, date: 5.days.ago.to_date, amount: 20000)
     create_transaction(account: @account, date: 3.days.ago.to_date, amount: -500)
     create_transaction(account: @account, date: 2.days.ago.to_date, amount: 100)
     create_valuation(account: @account, date: 1.day.ago.to_date, amount: 25000)
 
-    syncer = Account::Balance::Syncer.new(@account)
-    syncer.run
+    run_sync_for(@account)
 
     assert_equal 25000, @account.balance
     assert_equal [ 20000, 20000, 20500, 20400, 25000, 25000 ], @account.balances.chronological.map(&:balance)
@@ -57,8 +63,7 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase
     create_transaction(account: @account, date: 2.days.ago.to_date, amount: 300, currency: "USD")
     create_transaction(account: @account, date: 1.day.ago.to_date, amount: 500, currency: "EUR") # €500 * 1.2 = $600
 
-    syncer = Account::Balance::Syncer.new(@account)
-    syncer.run
+    run_sync_for(@account)
 
     assert_equal 20000, @account.balance
     assert_equal [ 21000, 20900, 20600, 20000, 20000 ], @account.balances.chronological.map(&:balance)
@@ -73,8 +78,7 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase
     create_exchange_rate(1.day.ago.to_date, from: "EUR", to: "USD", rate: 2)
     create_exchange_rate(Date.current, from: "EUR", to: "USD", rate: 2)
 
-    syncer = Account::Balance::Syncer.new(@account)
-    syncer.run
+    run_sync_for(@account)
 
     usd_balances = @account.balances.where(currency: "USD").chronological.map(&:balance)
     eur_balances = @account.balances.where(currency: "EUR").chronological.map(&:balance)
@@ -113,8 +117,7 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase
 
     assert_equal 2, @account.balances.size
 
-    syncer = Account::Balance::Syncer.new(@account)
-    syncer.run
+    run_sync_for(@account)
 
     assert_equal [ @account.balance ], @account.balances.chronological.map(&:balance)
   end
@@ -124,14 +127,18 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase
 
     transaction = create_transaction(account: @account, date: 1.day.ago.to_date, amount: 100, currency: "USD")
 
-    syncer = Account::Balance::Syncer.new(@account, start_date: 1.day.ago.to_date)
-    syncer.run
+    run_sync_for(@account, start_date: 1.day.ago.to_date)
 
     assert_equal [ existing_balance.balance, existing_balance.balance - transaction.amount, @account.balance ], @account.balances.chronological.map(&:balance)
   end
 
   private
 
+    def run_sync_for(account, start_date: nil)
+      syncer = Account::Balance::Syncer.new(account, start_date: start_date)
+      syncer.run
+    end
+
     def create_exchange_rate(date, from:, to:, rate:)
       ExchangeRate.create! date: date, from_currency: from, to_currency: to, rate: rate
     end
diff --git a/test/models/account/sync_test.rb b/test/models/account/sync_test.rb
index 335dabe9..564889ce 100644
--- a/test/models/account/sync_test.rb
+++ b/test/models/account/sync_test.rb
@@ -5,13 +5,20 @@ class Account::SyncTest < ActiveSupport::TestCase
     @account = accounts(:depository)
 
     @sync = Account::Sync.for(@account)
+
     @balance_syncer = mock("Account::Balance::Syncer")
-    Account::Balance::Syncer.expects(:new).with(@account, start_date: nil).returns(@balance_syncer).once
+    @holding_syncer = mock("Account::Holding::Syncer")
   end
 
   test "runs sync" do
+    Account::Balance::Syncer.expects(:new).with(@account, start_date: nil).returns(@balance_syncer).once
+    Account::Holding::Syncer.expects(:new).with(@account, start_date: nil).returns(@holding_syncer).once
+
     @balance_syncer.expects(:run).once
-    @balance_syncer.expects(:warnings).returns([ "test sync warning" ]).once
+    @balance_syncer.expects(:warnings).returns([ "test balance sync warning" ]).once
+
+    @holding_syncer.expects(:run).once
+    @holding_syncer.expects(:warnings).returns([ "test holding sync warning" ]).once
 
     assert_equal "pending", @sync.status
     assert_equal [], @sync.warnings
@@ -20,11 +27,14 @@ class Account::SyncTest < ActiveSupport::TestCase
     @sync.run
 
     assert_equal "completed", @sync.status
-    assert_equal [ "test sync warning" ], @sync.warnings
+    assert_equal [ "test balance sync warning", "test holding sync warning" ], @sync.warnings
     assert @sync.last_ran_at
   end
 
   test "handles sync errors" do
+    Account::Balance::Syncer.expects(:new).with(@account, start_date: nil).returns(@balance_syncer).once
+    Account::Holding::Syncer.expects(:new).with(@account, start_date: nil).returns(@holding_syncer).never # error from balance sync halts entire sync
+
     @balance_syncer.expects(:run).raises(StandardError.new("test sync error"))
 
     @sync.run
diff --git a/test/support/account/entries_test_helper.rb b/test/support/account/entries_test_helper.rb
index 4a4dd339..6afc40b1 100644
--- a/test/support/account/entries_test_helper.rb
+++ b/test/support/account/entries_test_helper.rb
@@ -27,4 +27,13 @@ module Account::EntriesTestHelper
 
     Account::Entry.create! entry_defaults.merge(attributes)
   end
+
+  def create_trade(account:, security:, qty:, price:, date:)
+    account.entries.create! \
+      date: date,
+      amount: qty * price,
+      currency: "USD",
+      name: "Trade",
+      entryable: Account::Trade.new(qty: qty, price: price, security: security)
+  end
 end
diff --git a/test/system/transfers_test.rb b/test/system/transfers_test.rb
index 702ca81f..228c3ab2 100644
--- a/test/system/transfers_test.rb
+++ b/test/system/transfers_test.rb
@@ -61,7 +61,7 @@ class TransfersTest < ApplicationSystemTestCase
   end
 
   test "can mark a single transaction as a transfer" do
-    txn = @user.family.entries.reverse_chronological.first
+    txn = @user.family.entries.account_transactions.reverse_chronological.first
 
     within "#" + dom_id(txn) do
       assert_text txn.account_transaction.category.name || "Uncategorized"
-- 
2.53.0