Basic trade and holdings view (#1271)

* Add trade view

* Lint fix

* Fix stale placeholder variable

* Add holding view

app/controllers/account/entries_controller.rb

@@ -25,7 +25,7 @@ class Account::EntriesController < ApplicationController
   def destroy
     @entry.destroy!
     @entry.sync_account_later
-    redirect_back_or_to account_url(@entry.account), notice: t(".success")
+    redirect_to account_url(@entry.account), notice: t(".success")
   end
 
   private

app/controllers/account/holdings_controller.rb

@@ -2,7 +2,7 @@ class Account::HoldingsController < ApplicationController
   layout :with_sidebar
 
   before_action :set_account
-  before_action :set_holding, only: :show
+  before_action :set_holding, only: %i[show destroy]
 
   def index
     @holdings = @account.holdings.current
@@ -11,6 +11,11 @@ class Account::HoldingsController < ApplicationController
   def show
   end
 
+  def destroy
+    @holding.destroy_holding_and_entries!
+    redirect_back_or_to account_holdings_path(@account)
+  end
+
   private
 
     def set_account

app/controllers/account/trades_controller.rb

@@ -2,6 +2,7 @@ class Account::TradesController < ApplicationController
   layout :with_sidebar
 
   before_action :set_account
+  before_action :set_entry, only: :update
 
   def new
     @entry = @account.entries.account_trades.new(entryable_attributes: {})
@@ -23,15 +24,36 @@ class Account::TradesController < ApplicationController
     end
   end
 
+  def update
+    @entry.update!(entry_params)
+
+    respond_to do |format|
+      format.html { redirect_to account_entry_path(@account, @entry), notice: t(".success") }
+      format.turbo_stream { render turbo_stream: turbo_stream.replace(@entry) }
+    end
+  end
+
   private
 
     def set_account
       @account = Current.family.accounts.find(params[:account_id])
     end
 
+    def set_entry
+      @entry = @account.entries.find(params[:id])
+    end
+
     def entry_params
       params.require(:account_entry)
-            .permit(:type, :date, :qty, :ticker, :price, :amount, :currency, :transfer_account_id)
+            .permit(
+              :type, :date, :qty, :ticker, :price, :amount, :notes, :excluded, :currency, :transfer_account_id, :entryable_type,
+              entryable_attributes: [
+                :id,
+                :qty,
+                :ticker,
+                :price
+              ]
+            )
             .merge(account: @account)
     end
 end

app/controllers/account/transactions_controller.rb

@@ -33,11 +33,9 @@ class Account::TransactionsController < ApplicationController
     def entry_params
       params.require(:account_entry)
             .permit(
-              :name, :date, :amount, :currency, :entryable_type, :nature,
+              :name, :date, :amount, :currency, :excluded, :notes, :entryable_type, :nature,
               entryable_attributes: [
                 :id,
-                :notes,
-                :excluded,
                 :category_id,
                 :merchant_id,
                 { tag_ids: [] }

app/helpers/styled_form_builder.rb

@@ -39,11 +39,11 @@ class StyledFormBuilder < ActionView::Helpers::FormBuilder
     build_styled_field(label, field, options, remove_padding_right: true)
   end
 
-  def money_field(amount_method, currency_method, options = {})
+  def money_field(amount_method, options = {})
     @template.render partial: "shared/money_field", locals: {
       form: self,
       amount_method:,
-      currency_method:,
+      currency_method: options[:currency_method] || :currency,
       **options
     }
   end

app/models/account/entry.rb

@@ -109,8 +109,8 @@ class Account::Entry < ApplicationRecord
     def bulk_update!(bulk_update_params)
       bulk_attributes = {
         date: bulk_update_params[:date],
-        entryable_attributes: {
         notes: bulk_update_params[:notes],
+        entryable_attributes: {
           category_id: bulk_update_params[:category_id],
           merchant_id: bulk_update_params[:merchant_id]
         }.compact_blank

app/models/account/holding.rb

@@ -37,6 +37,19 @@ class Account::Holding < ApplicationRecord
     @trend ||= calculate_trend
   end
 
+  def trades
+    account.entries.where(entryable: account.trades.where(security: security)).reverse_chronological
+  end
+
+  def destroy_holding_and_entries!
+    transaction do
+      account.entries.where(entryable: account.trades.where(security: security)).destroy_all
+      destroy
+    end
+
+    account.sync_later
+  end
+
   private
 
     def calculate_trend

app/models/account/trade.rb

@@ -25,4 +25,15 @@ class Account::Trade < ApplicationRecord
   def buy?
     qty > 0
   end
+
+  def unrealized_gain_loss
+    return nil if sell?
+    current_price = security.current_price
+    return nil if current_price.nil?
+
+    current_value = current_price * qty.abs
+    cost_basis = price_money * qty.abs
+
+    TimeSeries::Trend.new(current: current_value, previous: cost_basis)
+  end
 end

app/models/mint_import.rb

@@ -34,7 +34,8 @@ class MintImport < Import
           amount: row.signed_amount,
           name: row.name,
           currency: row.currency,
-          entryable: Account::Transaction.new(category: category, tags: tags, notes: row.notes),
+          notes: row.notes,
+          entryable: Account::Transaction.new(category: category, tags: tags),
           import: self
 
         entry.save!

app/models/security.rb

@@ -5,6 +5,12 @@ class Security < ApplicationRecord
 
   validates :ticker, presence: true, uniqueness: { case_sensitive: false }
 
+  def current_price
+    @current_price ||= Security::Price.find_price(ticker:, date: Date.current)
+    return nil if @current_price.nil?
+    Money.new(@current_price.price, @current_price.currency)
+  end
+
   private
 
     def upcase_ticker

app/models/transaction_import.rb

@@ -13,7 +13,8 @@ class TransactionImport < Import
           amount: row.signed_amount,
           name: row.name,
           currency: row.currency,
-          entryable: Account::Transaction.new(category: category, tags: tags, notes: row.notes),
+          notes: row.notes,
+          entryable: Account::Transaction.new(category: category, tags: tags),
           import: self
 
         entry.save!

app/views/account/holdings/show.html.erb

@@ -9,36 +9,103 @@
       <%= render "shared/circle_logo", name: @holding.name %>
     </header>
 
-    <details class="group space-y-2">
+    <details class="group space-y-2" open>
       <summary class="flex list-none items-center justify-between rounded-xl px-3 py-2 text-xs font-medium uppercase text-gray-500 bg-gray-25 focus-visible:outline-none">
         <h4><%= t(".overview") %></h4>
         <%= lucide_icon "chevron-down", class: "group-open:transform group-open:rotate-180 text-gray-500 w-5" %>
       </summary>
 
-      <div>
-        <p class="pl-4 text-gray-500">Coming soon...</p>
+      <div class="pb-4">
+        <dl class="space-y-3 px-3 py-2">
+          <div class="flex items-center justify-between text-sm">
+            <dt class="text-gray-500"><%= t(".ticker_label") %></dt>
+            <dd class="text-gray-900"><%= @holding.ticker %></dd>
+          </div>
+
+          <div class="flex items-center justify-between text-sm">
+            <dt class="text-gray-500"><%= t(".current_market_price_label") %></dt>
+            <dd class="text-gray-900"><%= @holding.security.current_price ? format_money(@holding.security.current_price) : t(".unknown") %></dd>
+          </div>
+
+          <div class="flex items-center justify-between text-sm">
+            <dt class="text-gray-500"><%= t(".portfolio_weight_label") %></dt>
+            <dd class="text-gray-900"><%= @holding.weight ? number_to_percentage(@holding.weight, precision: 2) : t(".unknown") %></dd>
+          </div>
+
+          <div class="flex items-center justify-between text-sm">
+            <dt class="text-gray-500"><%= t(".avg_cost_label") %></dt>
+            <dd class="text-gray-900"><%= @holding.avg_cost ? format_money(@holding.avg_cost) : t(".unknown") %></dd>
+          </div>
+
+          <div class="flex items-center justify-between text-sm">
+            <dt class="text-gray-500"><%= t(".trend_label") %></dt>
+            <dd style="color: <%= @holding.trend&.color %>;">
+              <%= @holding.trend ? render("shared/trend_change", trend: @holding.trend) : t(".unknown") %>
+            </dd>
+          </div>
+        </dl>
       </div>
     </details>
 
-    <details class="group space-y-2">
+    <details class="group space-y-2" open>
       <summary class="flex list-none items-center justify-between rounded-xl px-3 py-2 text-xs font-medium uppercase text-gray-500 bg-gray-25 focus-visible:outline-none">
         <h4><%= t(".history") %></h4>
         <%= lucide_icon "chevron-down", class: "group-open:transform group-open:rotate-180 text-gray-500 w-5" %>
       </summary>
 
+      <div class="space-y-2">
+        <div class="px-3 py-4">
+          <% if @holding.trades.any? %>
+            <ul class="space-y-2">
+              <% @holding.trades.each_with_index do |trade_entry, index| %>
+                <li class="flex gap-4 text-sm space-y-1">
+                  <div class="flex flex-col items-center gap-1.5 pt-2">
+                    <div class="rounded-full h-1.5 w-1.5 bg-gray-300"></div>
+                    <% unless index == @holding.trades.length - 1 %>
+                      <div class="h-12 w-px bg-alpha-black-200"></div>
+                    <% end %>
+                  </div>
+
                   <div>
-        <p class="pl-4 text-gray-500">Coming soon...</p>
+                    <p class="text-gray-500 text-xs uppercase"><%= l(trade_entry.date, format: :long) %></p>
+
+                    <p><%= t(
+                      ".trade_history_entry",
+                      qty: trade_entry.account_trade.qty,
+                      security: trade_entry.account_trade.security.ticker,
+                      price: format_money(trade_entry.account_trade.price)
+                    ) %></p>
+                  </div>
+                </li>
+              <% end %>
+            </ul>
+
+          <% else %>
+            <p class="text-gray-500">No trade history available for this holding.</p>
+          <% end %>
+        </div>
       </div>
     </details>
 
-    <details class="group space-y-2">
+    <details class="group space-y-2" open>
       <summary class="flex list-none items-center justify-between rounded-xl px-3 py-2 text-xs font-medium uppercase text-gray-500 bg-gray-25 focus-visible:outline-none">
         <h4><%= t(".settings") %></h4>
         <%= lucide_icon "chevron-down", class: "group-open:transform group-open:rotate-180 text-gray-500 w-5" %>
       </summary>
 
-      <div>
-        <p class="pl-4 text-gray-500">Coming soon...</p>
+      <div class="pb-4">
+        <div class="flex items-center justify-between gap-2 p-3">
+          <div class="text-sm space-y-1">
+            <h4 class="text-gray-900"><%= t(".delete_title") %></h4>
+            <p class="text-gray-500"><%= t(".delete_subtitle") %></p>
+          </div>
+
+          <%= button_to t(".delete"),
+                account_holding_path(@holding.account, @holding),
+                method: :delete,
+                class: "rounded-lg px-3 py-2 text-red-500 text-sm font-medium border border-alpha-black-200",
+                data: { turbo_confirm: true, turbo_frame: "_top" } %>
+        </div>
       </div>
     </details>
   </div>

app/views/account/trades/_form.html.erb

@@ -13,7 +13,7 @@
       <%= form.date_field :date, label: true %>
 
       <div data-trade-form-target="amountInput" hidden>
-        <%= form.money_field :amount, :currency, label: t(".amount"), disable_currency: true %>
+        <%= form.money_field :amount, label: t(".amount"), disable_currency: true %>
       </div>
 
       <div data-trade-form-target="transferAccountInput" hidden>
@@ -25,7 +25,7 @@
       </div>
 
       <div data-trade-form-target="priceInput">
-        <%= form.money_field :price, :currency, label: t(".price"), disable_currency: true %>
+        <%= form.money_field :price, label: t(".price"), disable_currency: true %>
       </div>
     </div>
 

app/views/account/trades/show.html.erb

@@ -1,29 +1,146 @@
-<% entry = @entry %>
+<% entry, trade, account = @entry, @entry.account_trade, @entry.account %>
 
 <%= drawer do %>
-  <div>
   <header class="mb-4 space-y-1">
     <div class="flex items-center gap-4">
       <h3 class="font-medium">
-          <span class="text-2xl"><%= format_money -entry.amount_money %></span>
-          <span class="text-lg text-gray-500"><%= entry.currency %></span>
+        <span class="text-2xl">
+          <%= format_money -entry.amount_money %>
+        </span>
+
+        <span class="text-lg text-gray-500">
+          <%= entry.currency %>
+        </span>
       </h3>
     </div>
 
-      <span class="text-sm text-gray-500"><%= entry.date.strftime("%A %d %B") %></span>
+    <span class="text-sm text-gray-500">
+      <%= I18n.l(entry.date, format: :long) %>
+    </span>
   </header>
 
   <div class="space-y-2">
-      <details class="group space-y-2" open>
-        <summary class="flex list-none items-center justify-between rounded-xl px-3 py-2 text-xs font-medium uppercase text-gray-500 bg-gray-25 focus-visible:outline-none">
-          <h4><%= t(".overview") %></h4>
-          <%= lucide_icon "chevron-down", class: "group-open:transform group-open:rotate-180 text-gray-500 w-5" %>
-        </summary>
+    <!-- Overview Section -->
+    <%= disclosure t(".overview") do %>
+      <div class="pb-4">
+        <dl class="space-y-3 px-3 py-2">
+          <div class="flex items-center justify-between text-sm">
+            <dt class="text-gray-500"><%= t(".symbol_label") %></dt>
+            <dd class="text-gray-900"><%= trade.security.ticker %></dd>
+          </div>
 
-        <div class="pb-6 pl-4 text-gray-500">
-          <p>Details coming soon...</p>
+          <% if trade.buy? %>
+            <div class="flex items-center justify-between text-sm">
+              <dt class="text-gray-500"><%= t(".purchase_qty_label") %></dt>
+              <dd class="text-gray-900"><%= trade.qty.abs %></dd>
             </div>
-      </details>
+
+            <div class="flex items-center justify-between text-sm">
+              <dt class="text-gray-500"><%= t(".purchase_price_label") %></dt>
+              <dd class="text-gray-900"><%= format_money trade.price_money %></dd>
             </div>
+          <% end %>
+
+          <div class="flex items-center justify-between text-sm">
+            <dt class="text-gray-500"><%= t(".current_market_price_label") %></dt>
+            <dd class="text-gray-900"><%= format_money trade.security.current_price %></dd>
+          </div>
+
+          <% if trade.buy? %>
+            <div class="flex items-center justify-between text-sm">
+              <dt class="text-gray-500"><%= t(".total_return_label") %></dt>
+              <dd style="color: <%= trade.unrealized_gain_loss.color %>;">
+                <%= render "shared/trend_change", trend: trade.unrealized_gain_loss %>
+              </dd>
+            </div>
+          <% end %>
+        </dl>
+      </div>
+    <% end %>
+
+    <!-- Details Section -->
+    <%= disclosure t(".details") do %>
+      <div class="pb-4">
+        <%= styled_form_with model: [account, entry],
+              url: account_trade_path(account, entry),
+              class: "space-y-2",
+              data: { controller: "auto-submit-form" } do |f| %>
+          <%= f.date_field :date,
+                label: t(".date_label"),
+                max: Date.current,
+                "data-auto-submit-form-target": "auto" %>
+
+          <%= f.fields_for :entryable do |ef| %>
+            <%= ef.number_field :qty,
+                  label: t(".quantity_label"),
+                  step: "any",
+                  "data-auto-submit-form-target": "auto" %>
+
+            <%= ef.money_field :price,
+                  label: t(".cost_per_share_label"),
+                  disable_currency: true,
+                  auto_submit: true,
+                  min: 0 %>
+          <% end %>
+        <% end %>
+      </div>
+    <% end %>
+
+    <!-- Additional Section -->
+    <%= disclosure t(".additional") do %>
+      <div class="pb-4">
+        <%= styled_form_with model: [account, entry],
+              url: account_trade_path(account, entry),
+              class: "space-y-2",
+              data: { controller: "auto-submit-form" } do |f| %>
+          <%= f.text_area :notes,
+                label: t(".note_label"),
+                placeholder: t(".note_placeholder"),
+                rows: 5,
+                "data-auto-submit-form-target": "auto" %>
+        <% end %>
+      </div>
+    <% end %>
+
+    <!-- Settings Section -->
+    <%= disclosure t(".settings") do %>
+      <div class="pb-4">
+        <!-- Exclude Trade Form -->
+        <%= styled_form_with model: [account, entry],
+              url: account_trade_path(account, entry),
+              class: "p-3",
+              data: { controller: "auto-submit-form" } do |f| %>
+          <div class="flex cursor-pointer items-center gap-2 justify-between">
+            <div class="text-sm space-y-1">
+              <h4 class="text-gray-900"><%= t(".exclude_title") %></h4>
+              <p class="text-gray-500"><%= t(".exclude_subtitle") %></p>
+            </div>
+
+            <div class="relative inline-block select-none">
+              <%= f.check_box :excluded,
+                      class: "sr-only peer",
+                      "data-auto-submit-form-target": "auto" %>
+              <label for="account_entry_excluded"
+                         class="maybe-switch"></label>
+            </div>
+          </div>
+        <% end %>
+
+        <!-- Delete Trade Form -->
+        <div class="flex items-center justify-between gap-2 p-3">
+          <div class="text-sm space-y-1">
+            <h4 class="text-gray-900"><%= t(".delete_title") %></h4>
+            <p class="text-gray-500"><%= t(".delete_subtitle") %></p>
+          </div>
+
+          <%= button_to t(".delete"),
+                account_entry_path(account, entry),
+                method: :delete,
+                class:  "rounded-lg px-3 py-2 text-red-500 text-sm
+                         font-medium border border-alpha-black-200",
+                data:   { turbo_confirm: true, turbo_frame: "_top" } %>
+        </div>
+      </div>
+    <% end %>
   </div>
 <% end %>

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

@@ -47,7 +47,7 @@
                     { container_class: "w-1/3", label: t(".nature"), selected: entry.amount.negative? ? "income" : "expense" },
                     { data: { "auto-submit-form-target": "auto" } } %>
 
-              <%= f.money_field :amount, :currency, label: t(".amount"),
+              <%= f.money_field :amount, label: t(".amount"),
                     container_class: "w-2/3",
                     auto_submit: true,
                     min: 0,
@@ -104,7 +104,13 @@
                   },
                   { "data-auto-submit-form-target": "auto" } %>
 
-            <%= ef.text_area :notes,
+          <% end %>
+
+          <%= styled_form_with model: [account, entry],
+              url: account_transaction_path(account, entry),
+              class: "space-y-2",
+              data: { controller: "auto-submit-form" } do |f| %>
+            <%= f.text_area :notes,
                   label: t(".note_label"),
                   placeholder: t(".note_placeholder"),
                   rows: 5,
@@ -122,7 +128,6 @@
               url: account_transaction_path(account, entry),
               class: "p-3",
               data: { controller: "auto-submit-form" } do |f| %>
-          <%= f.fields_for :entryable do |ef| %>
           <div class="flex cursor-pointer items-center gap-2 justify-between">
             <div class="text-sm space-y-1">
               <h4 class="text-gray-900"><%= t(".exclude_title") %></h4>
@@ -130,7 +135,7 @@
             </div>
 
             <div class="relative inline-block select-none">
-                <%= ef.check_box :excluded,
+              <%= f.check_box :excluded,
                       class: "sr-only peer",
                       "data-auto-submit-form-target": "auto" %>
               <label for="account_entry_entryable_attributes_excluded"
@@ -138,7 +143,6 @@
             </div>
           </div>
         <% end %>
-        <% end %>
 
         <!-- Delete Transaction Form -->
         <% unless entry.marked_as_transfer? %>

app/views/account/transfers/_form.html.erb

@@ -29,7 +29,7 @@
     <%= f.text_field :name, value: transfer.name, label: t(".description"), placeholder: t(".description_placeholder"), required: true %>
     <%= f.collection_select :from_account_id, Current.family.accounts.alphabetically, :id, :name, { prompt: t(".select_account"), label: t(".from") }, required: true %>
     <%= f.collection_select :to_account_id, Current.family.accounts.alphabetically, :id, :name, { prompt: t(".select_account"), label: t(".to") }, required: true %>
-    <%= f.money_field :amount, :currency, label: t(".amount"), required: true, hide_currency: true %>
+    <%= f.money_field :amount, label: t(".amount"), required: true, hide_currency: true %>
     <%= f.date_field :date, value: transfer.date, label: t(".date"), required: true, max: Date.current %>
   </section>
 

app/views/accounts/_form.html.erb

@@ -5,7 +5,7 @@
     <%= f.hidden_field :accountable_type %>
     <%= f.text_field :name, placeholder: t(".name_placeholder"), required: "required", label: t(".name_label"), autofocus: true %>
     <%= f.collection_select :institution_id, Current.family.institutions.alphabetically, :id, :name, { include_blank: t(".ungrouped"), label: t(".institution") } %>
-    <%= f.money_field :balance, :currency, label: t(".balance"), required: true, default_currency: Current.family.currency %>
+    <%= f.money_field :balance, label: t(".balance"), required: true, default_currency: Current.family.currency %>
 
     <% if account.new_record? %>
       <div class="flex items-center gap-2 mt-3 mb-6">

app/views/transactions/_form.html.erb

@@ -13,7 +13,7 @@
   <section class="space-y-2">
     <%= f.text_field :name, label: t(".description"), placeholder: t(".description_placeholder"), required: true %>
     <%= f.collection_select :account_id, Current.family.accounts.alphabetically, :id, :name, { prompt: t(".account_prompt"), label: t(".account") }, required: true %>
-    <%= f.money_field :amount, :currency, label: t(".amount"), required: true %>
+    <%= f.money_field :amount, label: t(".amount"), required: true %>
     <%= f.hidden_field :entryable_type, value: "Account::Transaction" %>
     <%= f.fields_for :entryable do |ef| %>
       <%= ef.collection_select :category_id, Current.family.categories.alphabetically, :id, :name, { prompt: t(".category_prompt"), label: t(".category") } %>

config/locales/views/account/holdings/en.yml

@@ -20,6 +20,17 @@ en:
           its returns or value.
         missing_data: Missing data
       show:
+        avg_cost_label: Average Cost
+        current_market_price_label: Current Market Price
+        delete: Delete
+        delete_subtitle: This will delete the holding and all your associated trades
+          on this account.  This action cannot be undone.
+        delete_title: Delete holding
         history: History
         overview: Overview
+        portfolio_weight_label: Portfolio Weight
         settings: Settings
+        ticker_label: Ticker
+        trade_history_entry: "%{qty} shares of %{security} at %{price}"
+        trend_label: Trend
+        unknown: Unknown

config/locales/views/account/trades/en.yml

@@ -25,7 +25,25 @@ en:
       new:
         title: New transaction
       show:
+        additional: Additional
+        cost_per_share_label: Cost per Share
+        current_market_price_label: Current Market Price
+        date_label: Date
+        delete: Delete
+        delete_subtitle: This action cannot be undone
+        delete_title: Delete Trade
+        details: Details
+        exclude_subtitle: This trade will not be included in reports and calculations
+        exclude_title: Exclude from analytics
+        note_label: Note
+        note_placeholder: Add any additional notes here...
         overview: Overview
+        purchase_price_label: Purchase Price
+        purchase_qty_label: Purchase Quantity
+        quantity_label: Quantity
+        settings: Settings
+        symbol_label: Symbol
+        total_return_label: Unrealized gain/loss
       trade:
         buy: Buy
         deposit: Deposit
@@ -33,3 +51,5 @@ en:
         outflow: Outflow
         sell: Sell
         withdrawal: Withdrawal
+      update:
+        success: Trade updated successfully.

config/routes.rb

@@ -63,12 +63,12 @@ Rails.application.routes.draw do
     scope module: :account do
       resource :logo, only: :show
 
-      resources :holdings, only: %i[index new show]
+      resources :holdings, only: %i[index new show destroy]
       resources :cashes, only: :index
 
       resources :transactions, only: %i[index update]
       resources :valuations, only: %i[index new create]
-      resources :trades, only: %i[index new create]
+      resources :trades, only: %i[index new create update]
 
       resources :entries, only: %i[edit update show destroy]
     end

db/migrate/20241009132959_add_notes_to_entry.rb

@@ -0,0 +1,33 @@
+class AddNotesToEntry < ActiveRecord::Migration[7.2]
+  def change
+    add_column :account_entries, :notes, :text
+    add_column :account_entries, :excluded, :boolean, default: false
+
+    reversible do |dir|
+      dir.up do
+        execute <<-SQL
+          UPDATE account_entries
+          SET notes = account_transactions.notes,
+              excluded = account_transactions.excluded
+          FROM account_transactions
+          WHERE account_entries.entryable_type = 'Account::Transaction'
+            AND account_entries.entryable_id = account_transactions.id
+        SQL
+      end
+
+      dir.down do
+        execute <<-SQL
+          UPDATE account_transactions
+          SET notes = account_entries.notes,
+              excluded = account_entries.excluded
+          FROM account_entries
+          WHERE account_entries.entryable_type = 'Account::Transaction'
+            AND account_entries.entryable_id = account_transactions.id
+        SQL
+      end
+    end
+
+    remove_column :account_transactions, :notes, :text
+    remove_column :account_transactions, :excluded, :boolean
+  end
+end

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_10_08_122449) do
+ActiveRecord::Schema[7.2].define(version: 2024_10_09_132959) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -45,6 +45,8 @@ ActiveRecord::Schema[7.2].define(version: 2024_10_08_122449) do
     t.uuid "transfer_id"
     t.boolean "marked_as_transfer", default: false, null: false
     t.uuid "import_id"
+    t.text "notes"
+    t.boolean "excluded", default: false
     t.index ["account_id"], name: "index_account_entries_on_account_id"
     t.index ["import_id"], name: "index_account_entries_on_import_id"
     t.index ["transfer_id"], name: "index_account_entries_on_transfer_id"
@@ -90,8 +92,6 @@ ActiveRecord::Schema[7.2].define(version: 2024_10_08_122449) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.uuid "category_id"
-    t.boolean "excluded", default: false
-    t.text "notes"
     t.uuid "merchant_id"
     t.index ["category_id"], name: "index_account_transactions_on_category_id"
     t.index ["merchant_id"], name: "index_account_transactions_on_merchant_id"
@@ -120,7 +120,7 @@ ActiveRecord::Schema[7.2].define(version: 2024_10_08_122449) do
     t.boolean "is_active", default: true, null: false
     t.date "last_sync_date"
     t.uuid "institution_id"
-    t.virtual "classification", type: :string, as: "\nCASE\n    WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n    ELSE 'asset'::text\nEND", stored: true
+    t.virtual "classification", type: :string, as: "\nCASE\n    WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n    ELSE 'asset'::text\nEND", stored: true
     t.uuid "import_id"
     t.index ["accountable_type"], name: "index_accounts_on_accountable_type"
     t.index ["family_id"], name: "index_accounts_on_family_id"

test/controllers/account/holdings_controller_test.rb

@@ -17,4 +17,14 @@ class Account::HoldingsControllerTest < ActionDispatch::IntegrationTest
 
     assert_response :success
   end
+
+  test "destroys holding and associated entries" do
+    assert_difference -> { Account::Holding.count } => -1,
+                      -> { Account::Entry.count } => -1 do
+      delete account_holding_path(@account, @holding)
+    end
+
+    assert_redirected_to account_holdings_path(@account)
+    assert_empty @account.entries.where(entryable: @account.trades.where(security: @holding.security))
+  end
 end

test/controllers/transactions_controller_test.rb

@@ -174,7 +174,7 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
       assert_equal 1.day.ago.to_date, transaction.date
       assert_equal Category.second, transaction.account_transaction.category
       assert_equal Merchant.second, transaction.account_transaction.merchant
-      assert_equal "Updated note", transaction.account_transaction.notes
+      assert_equal "Updated note", transaction.notes
     end
   end
 end