diff --git a/app/components/UI/account/activity_feed.html.erb b/app/components/UI/account/activity_feed.html.erb index fe5041f13..00adaf3fd 100644 --- a/app/components/UI/account/activity_feed.html.erb +++ b/app/components/UI/account/activity_feed.html.erb @@ -3,42 +3,46 @@
<%= tag.h2 t("accounts.show.activity.title"), class: "font-medium text-lg" %> - <%= render DS::Menu.new(variant: "button") do |menu| %> - <% menu.with_button(text: t("accounts.show.activity.new"), variant: "secondary", icon: "plus") %> + <%# Show menu if: manual (can add balance) OR non-crypto (can add transactions) %> + <%# This hides the menu only for linked crypto accounts who have no actions %> + <% if !account.linked? || !account.crypto? %> + <%= render DS::Menu.new(variant: "button") do |menu| %> + <% menu.with_button(text: t("accounts.show.activity.new"), variant: "secondary", icon: "plus") %> - <% unless account.linked? %> - <% menu.with_item( - variant: "link", - text: t("accounts.show.activity.new_balance"), - icon: "circle-dollar-sign", - href: new_valuation_path(account_id: account.id), - data: { turbo_frame: :modal }) %> - <% end %> - - <% unless account.crypto? %> - <% if account.investment? %> + <% unless account.linked? %> <% menu.with_item( variant: "link", - text: t("accounts.show.activity.new_activity"), - icon: "arrow-left-right", - href: new_trade_path(account_id: account.id), - data: { turbo_frame: :modal }) %> - <% else %> - <% menu.with_item( - variant: "link", - text: t("accounts.show.activity.new_transaction"), - icon: "credit-card", - href: new_transaction_path(account_id: account.id), + text: t("accounts.show.activity.new_balance"), + icon: "circle-dollar-sign", + href: new_valuation_path(account_id: account.id), data: { turbo_frame: :modal }) %> <% end %> - <% end %> - <% menu.with_item( - variant: "link", - text: t("accounts.show.activity.new_transfer"), - icon: "arrow-right-left", - href: new_transfer_path(from_account_id: account.id), - data: { turbo_frame: :modal }) %> + <% unless account.crypto? %> + <% if account.investment? %> + <% menu.with_item( + variant: "link", + text: t("accounts.show.activity.new_activity"), + icon: "arrow-left-right", + href: new_trade_path(account_id: account.id), + data: { turbo_frame: :modal }) %> + <% else %> + <% menu.with_item( + variant: "link", + text: t("accounts.show.activity.new_transaction"), + icon: "credit-card", + href: new_transaction_path(account_id: account.id), + data: { turbo_frame: :modal }) %> + <% end %> + <% end %> + + <% menu.with_item( + variant: "link", + text: t("accounts.show.activity.new_transfer"), + icon: "arrow-right-left", + href: new_transfer_path(from_account_id: account.id), + data: { turbo_frame: :modal }) %> + <% end %> <% end %>
diff --git a/app/controllers/holdings_controller.rb b/app/controllers/holdings_controller.rb index cd6d6260c..f8ba811cc 100644 --- a/app/controllers/holdings_controller.rb +++ b/app/controllers/holdings_controller.rb @@ -1,5 +1,5 @@ class HoldingsController < ApplicationController - before_action :set_holding, only: %i[show update destroy unlock_cost_basis] + before_action :set_holding, only: %i[show update destroy unlock_cost_basis remap_security reset_security] def index @account = Current.family.accounts.find(params[:account_id]) @@ -47,6 +47,48 @@ class HoldingsController < ApplicationController end end + def remap_security + # Combobox returns "TICKER|EXCHANGE" format + ticker, exchange = params[:security_id].to_s.split("|") + + # Validate ticker is present (form has required: true, but can be bypassed) + if ticker.blank? + flash[:alert] = t(".security_not_found") + redirect_to account_path(@holding.account, tab: "holdings") + return + end + + new_security = Security::Resolver.new( + ticker, + exchange_operating_mic: exchange, + country_code: Current.family.country + ).resolve + + if new_security.nil? + flash[:alert] = t(".security_not_found") + redirect_to account_path(@holding.account, tab: "holdings") + return + end + + @holding.remap_security!(new_security) + flash[:notice] = t(".success") + + respond_to do |format| + format.html { redirect_to account_path(@holding.account, tab: "holdings") } + format.turbo_stream { render turbo_stream: turbo_stream.action(:redirect, account_path(@holding.account, tab: "holdings")) } + end + end + + def reset_security + @holding.reset_security_to_provider! + flash[:notice] = t(".success") + + respond_to do |format| + format.html { redirect_to account_path(@holding.account, tab: "holdings") } + format.turbo_stream { render turbo_stream: turbo_stream.action(:redirect, account_path(@holding.account, tab: "holdings")) } + end + end + private def set_holding @holding = Current.family.holdings.find(params[:id]) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index e5efee0b0..c288c1986 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -201,12 +201,17 @@ class TransactionsController < ApplicationController # Default activity label if not provided activity_label ||= is_sell ? "Sell" : "Buy" - # Create trade entry + # Create trade entry with note about conversion + conversion_note = t("transactions.convert_to_trade.conversion_note", + original_name: @entry.name, + original_date: I18n.l(@entry.date, format: :long)) + @entry.account.entries.create!( name: params[:trade_name] || Trade.build_name(is_sell ? "sell" : "buy", qty, security.ticker), date: @entry.date, amount: signed_amount, currency: @entry.currency, + notes: conversion_note, entryable: Trade.new( security: security, qty: signed_qty, diff --git a/app/javascript/application.js b/app/javascript/application.js index 44dcb4a84..3f3b9c3a1 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -3,7 +3,8 @@ import "@hotwired/turbo-rails"; import "controllers"; Turbo.StreamActions.redirect = function () { - Turbo.visit(this.target); + // Use "replace" to avoid adding form submission to browser history + Turbo.visit(this.target, { action: "replace" }); }; // Register service worker for PWA offline support diff --git a/app/javascript/controllers/holding_security_remap_controller.js b/app/javascript/controllers/holding_security_remap_controller.js new file mode 100644 index 000000000..94c8503f4 --- /dev/null +++ b/app/javascript/controllers/holding_security_remap_controller.js @@ -0,0 +1,11 @@ +import { Controller } from "@hotwired/stimulus" + +// Handles toggling the security remap form in the holding drawer. +export default class extends Controller { + static targets = ["form"] + + toggle(event) { + event.preventDefault() + this.formTarget.classList.toggle("hidden") + } +} diff --git a/app/models/account/provider_import_adapter.rb b/app/models/account/provider_import_adapter.rb index 64ba64c81..c66fe69ef 100644 --- a/app/models/account/provider_import_adapter.rb +++ b/app/models/account/provider_import_adapter.rb @@ -323,20 +323,46 @@ class Account::ProviderImportAdapter holding = account.holdings.find_by(external_id: external_id) unless holding - # Fallback path: match by (security, date, currency) — and when provided, - # also scope by account_provider_id to avoid cross‑provider claiming. - # This keeps behavior symmetric with deletion logic below which filters - # by account_provider_id when present. - find_by_attrs = { - security: security, + # Fallback path 1a: match by provider_security (for remapped holdings) + # This allows re-matching a holding that was remapped to a different security + # Scope by account_provider_id to avoid cross-provider overwrites + fallback_1a_attrs = { + provider_security: security, date: date, currency: currency } - if account_provider_id.present? - find_by_attrs[:account_provider_id] = account_provider_id + fallback_1a_attrs[:account_provider_id] = account_provider_id if account_provider_id.present? + holding = account.holdings.find_by(fallback_1a_attrs) + + # Fallback path 1b: match by provider_security ticker (for remapped holdings when + # Security::Resolver returns a different security instance for the same ticker) + # Scope by account_provider_id to avoid cross-provider overwrites + # Skip if ticker is blank to avoid matching NULL tickers + unless holding || security.ticker.blank? + scope = account.holdings + .joins("INNER JOIN securities AS ps ON ps.id = holdings.provider_security_id") + .where(date: date, currency: currency) + .where("ps.ticker = ?", security.ticker) + scope = scope.where(account_provider_id: account_provider_id) if account_provider_id.present? + holding = scope.first end - holding = account.holdings.find_by(find_by_attrs) + # Fallback path 2: match by (security, date, currency) — and when provided, + # also scope by account_provider_id to avoid cross‑provider claiming. + # This keeps behavior symmetric with deletion logic below which filters + # by account_provider_id when present. + unless holding + find_by_attrs = { + security: security, + date: date, + currency: currency + } + if account_provider_id.present? + find_by_attrs[:account_provider_id] = account_provider_id + end + + holding = account.holdings.find_by(find_by_attrs) + end end holding ||= account.holdings.new( @@ -382,7 +408,6 @@ class Account::ProviderImportAdapter # Build base attributes attributes = { - security: security, date: date, currency: currency, qty: quantity, @@ -392,6 +417,14 @@ class Account::ProviderImportAdapter external_id: external_id } + # Only update security if not locked by user + if holding.new_record? || holding.security_replaceable_by_provider? + attributes[:security] = security + # Track the provider's original security so reset_security_to_provider! works + # Only set if not already set (preserves original if user remapped then unlocked) + attributes[:provider_security_id] = security.id if holding.provider_security_id.blank? + end + # Only update cost_basis if reconciliation says to if reconciled[:should_update] attributes[:cost_basis] = reconciled[:cost_basis] diff --git a/app/models/holding.rb b/app/models/holding.rb index 5f45aa139..2b8e73ba5 100644 --- a/app/models/holding.rb +++ b/app/models/holding.rb @@ -16,6 +16,7 @@ class Holding < ApplicationRecord belongs_to :account belongs_to :security belongs_to :account_provider, optional: true + belongs_to :provider_security, class_name: "Security", optional: true validates :qty, :currency, :date, :price, :amount, presence: true validates :qty, :price, :amount, numericality: { greater_than_or_equal_to: 0 } @@ -128,6 +129,118 @@ class Holding < ApplicationRecord update!(cost_basis_locked: false) end + # Check if this holding's security can be changed by provider sync + def security_replaceable_by_provider? + !security_locked? + end + + # Check if user has remapped this holding to a different security + # Also verifies the provider_security record still exists (FK should prevent deletion, but be safe) + def security_remapped? + provider_security_id.present? && security_id != provider_security_id && provider_security.present? + end + + # Remap this holding (and all other holdings for the same security) to a different security + # Also moves all trades for the old security to the new security + # If the target security already has holdings on some dates, merge by combining qty/amount + def remap_security!(new_security) + return if new_security.id == security_id + + old_security = security + + transaction do + # Find (date, currency) pairs where the new security already has holdings (collision keys) + # Currency must match to merge - can't combine holdings denominated in different currencies + collision_keys = account.holdings + .where(security: new_security) + .where(date: account.holdings.where(security: old_security).select(:date)) + .pluck(:date, :currency) + .to_set + + # Process each holding for the old security + account.holdings.where(security: old_security).find_each do |holding| + if collision_keys.include?([ holding.date, holding.currency ]) + # Collision: merge into existing holding for new_security (same date AND currency) + existing = account.holdings.find_by!(security: new_security, date: holding.date, currency: holding.currency) + merged_qty = existing.qty + holding.qty + merged_amount = existing.amount + holding.amount + + # Calculate weighted average cost basis if both holdings have cost_basis + merged_cost_basis = if existing.cost_basis.present? && holding.cost_basis.present? && merged_qty.positive? + ((existing.cost_basis * existing.qty) + (holding.cost_basis * holding.qty)) / merged_qty + else + existing.cost_basis # Keep existing if we can't calculate weighted average + end + + # Preserve provider tracking from the holding being destroyed + # so subsequent syncs can find the merged holding + merge_attrs = { + qty: merged_qty, + amount: merged_amount, + price: merged_qty.positive? ? merged_amount / merged_qty : 0, + cost_basis: merged_cost_basis + } + merge_attrs[:external_id] ||= holding.external_id if existing.external_id.blank? && holding.external_id.present? + merge_attrs[:provider_security_id] ||= holding.provider_security_id || old_security.id if existing.provider_security_id.blank? + merge_attrs[:account_provider_id] ||= holding.account_provider_id if existing.account_provider_id.blank? && holding.account_provider_id.present? + merge_attrs[:security_locked] = true # Lock merged holding to prevent provider overwrites + + existing.update!(merge_attrs) + holding.destroy! + else + # No collision: update to new security + holding.provider_security_id ||= old_security.id + holding.security = new_security + holding.security_locked = true + holding.save! + end + end + + # Move all trades for old security to new security + account.trades.where(security: old_security).update_all(security_id: new_security.id) + end + + # Reload self to reflect changes (may raise RecordNotFound if self was destroyed) + begin + reload + rescue ActiveRecord::RecordNotFound + nil + end + end + + # Reset security (and all related holdings) back to what the provider originally sent + # Note: This moves ALL trades for current_security back to original_security. If the user + # had legitimate trades for the target security before remapping, those would also be moved. + # In practice this is rare since SimpleFIN doesn't provide trades, and Plaid trades would + # typically be for real tickers not CUSTOM: ones. A more robust solution would track which + # trades were moved during remap, but that adds significant complexity for an edge case. + def reset_security_to_provider! + return unless provider_security_id.present? + + current_security = security + original_security = provider_security + + # Guard against deleted provider_security (shouldn't happen due to FK, but be safe) + return unless original_security.present? + + transaction do + # Move trades back (see note above about limitation) + account.trades.where(security: current_security).update_all(security_id: original_security.id) + + # Reset ALL holdings that were remapped from this provider_security + account.holdings.where(security: current_security, provider_security: original_security).find_each do |holding| + holding.update!( + security: original_security, + security_locked: false, + provider_security_id: nil + ) + end + end + + # Reload self to reflect changes + reload + end + # Check if cost_basis is known (has a source and positive value) def cost_basis_known? cost_basis.present? && cost_basis.positive? && cost_basis_source.present? diff --git a/app/models/security/provided.rb b/app/models/security/provided.rb index 9983414d4..b80046483 100644 --- a/app/models/security/provided.rb +++ b/app/models/security/provided.rb @@ -55,6 +55,9 @@ module Security::Provided return price if price.present? + # Don't fetch prices for offline securities (e.g., custom tickers from SimpleFIN) + return nil if offline? + # Make sure we have a data provider before fetching return nil unless provider.present? response = provider.fetch_security_price( diff --git a/app/models/security/resolver.rb b/app/models/security/resolver.rb index 4591e64fa..2ec7fb701 100644 --- a/app/models/security/resolver.rb +++ b/app/models/security/resolver.rb @@ -63,7 +63,7 @@ class Security::Resolver exchange_matches = s.exchange_operating_mic.upcase.to_s == exchange_operating_mic.upcase.to_s if country_code && exchange_operating_mic - ticker_matches && exchange_matches && s.country_code.upcase.to_s == country_code.upcase.to_s + ticker_matches && exchange_matches && s.country_code&.upcase.to_s == country_code.upcase.to_s else ticker_matches && exchange_matches end @@ -79,7 +79,7 @@ class Security::Resolver # If a country code is specified, we MUST find a match with the same code if country_code.present? - filtered_candidates = filtered_candidates.select { |s| s.country_code.upcase.to_s == country_code.upcase.to_s } + filtered_candidates = filtered_candidates.select { |s| s.country_code&.upcase.to_s == country_code.upcase.to_s } end # 1. Prefer exact ticker matches (MSTR before MSTRX when searching for "MSTR") diff --git a/app/views/holdings/show.html.erb b/app/views/holdings/show.html.erb index b92463f2c..66a1ca725 100644 --- a/app/views/holdings/show.html.erb +++ b/app/views/holdings/show.html.erb @@ -20,14 +20,60 @@ <% dialog.with_section(title: t(".overview"), open: true) do %>
-
-
<%= t(".ticker_label") %>
-
<%= @holding.ticker %>
+
+
+
<%= t(".security_label") %>
+
+ <%= @holding.ticker %> + <% if @holding.security_remapped? %> + + (<%= t(".originally", ticker: @holding.provider_security.ticker) %>) + + <%= icon "lock", size: "xs", class: "text-secondary" %> + <% end %> + +
+
+ +
<%= t(".current_market_price_label") %>
-
<%= @holding.security.current_price ? format_money(@holding.security.current_price) : t(".unknown") %>
+
+ <% begin %> + <%= @holding.security.current_price ? format_money(@holding.security.current_price) : t(".unknown") %> + <% rescue ActiveRecord::RecordInvalid, StandardError %> + <%= t(".unknown") %> + <% end %> +
@@ -170,15 +216,34 @@ <% else %> -

No trade history available for this holding.

+

<%= t(".no_trade_history") %>

<% end %>
<% end %> - <% if @holding.cost_basis_locked? || @holding.account.can_delete_holdings? %> + <% if @holding.cost_basis_locked? || @holding.security_remapped? || @holding.account.can_delete_holdings? %> <% dialog.with_section(title: t(".settings"), open: true) do %>
+ <% if @holding.security_remapped? %> +
+
+

<%= t(".security_remapped_label") %>

+

<%= t(".provider_sent", ticker: @holding.provider_security.ticker) %>

+
+ + <%= button_to t(".reset_to_provider"), + reset_security_holding_path(@holding), + method: :post, + class: "inline-flex items-center gap-1 px-3 py-2 rounded-lg text-sm font-medium text-primary bg-gray-200 hover:bg-gray-300 theme-dark:bg-gray-700 theme-dark:hover:bg-gray-600", + form: { data: { turbo: false } }, + data: { turbo_confirm: { + title: t(".reset_confirm_title"), + body: t(".reset_confirm_body", current: @holding.security.ticker, original: @holding.provider_security.ticker) + } } %> +
+ <% end %> + <% if @holding.cost_basis_locked? %>
diff --git a/config/locales/views/holdings/en.yml b/config/locales/views/holdings/en.yml index 0e6d588d4..a14efe9b6 100644 --- a/config/locales/views/holdings/en.yml +++ b/config/locales/views/holdings/en.yml @@ -10,6 +10,13 @@ en: error: Invalid cost basis value. unlock_cost_basis: success: Cost basis unlocked. It may be updated on next sync. + remap_security: + success: Security updated successfully. + security_not_found: Could not find the selected security. + reset_security: + success: Security reset to provider value. + errors: + security_collision: "Cannot remap: you already have a holding for %{ticker} on %{date}." cost_basis_sources: manual: User set calculated: From trades @@ -33,7 +40,7 @@ en: average_cost: Average cost holdings: Holdings name: Name - new_holding: New transaction + new_holding: New activity no_holdings: No holdings to show. return: Total return weight: Weight @@ -48,10 +55,24 @@ en: delete_subtitle: This will delete the holding and all your associated trades on this account. This action cannot be undone. delete_title: Delete holding + edit_security: Edit security history: History + no_trade_history: No trade history available for this holding. overview: Overview portfolio_weight_label: Portfolio Weight settings: Settings + security_label: Security + originally: "was %{ticker}" + search_security: Search security + search_security_placeholder: Search by ticker or name + cancel: Cancel + remap_security: Save + no_security_provider: Security provider not configured. Cannot search for securities. + security_remapped_label: Security remapped + provider_sent: "Provider sent: %{ticker}" + reset_to_provider: Reset to provider + reset_confirm_title: Reset security to provider? + reset_confirm_body: "This will change the security from %{current} back to %{original} and move all associated trades." ticker_label: Ticker trade_history_entry: "%{qty} shares of %{security} at %{price}" total_return_label: Total Return diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml index 6f4a78ba1..e7849fd41 100644 --- a/config/locales/views/transactions/en.yml +++ b/config/locales/views/transactions/en.yml @@ -137,6 +137,7 @@ en: cancel: Cancel submit: Convert to Trade success: Transaction converted to trade + conversion_note: "Converted from transaction: %{original_name} (%{original_date})" errors: not_investment_account: Only transactions in investment accounts can be converted to trades already_converted: This transaction has already been converted or excluded diff --git a/config/routes.rb b/config/routes.rb index 69e1f4fc1..96ad6a28d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -227,6 +227,8 @@ Rails.application.routes.draw do resources :holdings, only: %i[index new show update destroy] do member do post :unlock_cost_basis + patch :remap_security + post :reset_security end end resources :trades, only: %i[show new create update destroy] diff --git a/db/migrate/20260119000001_add_provider_security_tracking_to_holdings.rb b/db/migrate/20260119000001_add_provider_security_tracking_to_holdings.rb new file mode 100644 index 000000000..17a63395c --- /dev/null +++ b/db/migrate/20260119000001_add_provider_security_tracking_to_holdings.rb @@ -0,0 +1,6 @@ +class AddProviderSecurityTrackingToHoldings < ActiveRecord::Migration[7.2] + def change + add_reference :holdings, :provider_security, type: :uuid, null: true, foreign_key: { to_table: :securities } + add_column :holdings, :security_locked, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 4a1957462..a2f020b1d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -551,10 +551,13 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_22_160000) do t.uuid "account_provider_id" t.string "cost_basis_source" t.boolean "cost_basis_locked", default: false, null: false + t.uuid "provider_security_id" + t.boolean "security_locked", default: false, null: false t.index ["account_id", "external_id"], name: "idx_holdings_on_account_id_external_id_unique", unique: true, where: "(external_id IS NOT NULL)" t.index ["account_id", "security_id", "date", "currency"], name: "idx_on_account_id_security_id_date_currency_5323e39f8b", unique: true t.index ["account_id"], name: "index_holdings_on_account_id" t.index ["account_provider_id"], name: "index_holdings_on_account_provider_id" + t.index ["provider_security_id"], name: "index_holdings_on_provider_security_id" t.index ["security_id"], name: "index_holdings_on_security_id" end @@ -1449,6 +1452,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_22_160000) do add_foreign_key "holdings", "account_providers" add_foreign_key "holdings", "accounts", on_delete: :cascade add_foreign_key "holdings", "securities" + add_foreign_key "holdings", "securities", column: "provider_security_id" add_foreign_key "impersonation_session_logs", "impersonation_sessions" add_foreign_key "impersonation_sessions", "users", column: "impersonated_id" add_foreign_key "impersonation_sessions", "users", column: "impersonator_id" diff --git a/test/models/holding_test.rb b/test/models/holding_test.rb index 1a384bdd1..7f6985767 100644 --- a/test/models/holding_test.rb +++ b/test/models/holding_test.rb @@ -245,6 +245,164 @@ class HoldingTest < ActiveSupport::TestCase assert_not @amzn.cost_basis_replaceable_by?("manual") end + # Security remapping tests + + test "security_replaceable_by_provider? returns false when locked" do + @amzn.update!(security_locked: true) + assert_not @amzn.security_replaceable_by_provider? + end + + test "security_replaceable_by_provider? returns true when not locked" do + @amzn.update!(security_locked: false) + assert @amzn.security_replaceable_by_provider? + end + + test "security_remapped? returns true when provider_security differs from security" do + other_security = create_security("GOOG", prices: [ { date: Date.current, price: 100.00 } ]) + @amzn.update!(provider_security: other_security) + assert @amzn.security_remapped? + end + + test "security_remapped? returns false when provider_security is nil" do + assert_nil @amzn.provider_security_id + assert_not @amzn.security_remapped? + end + + test "security_remapped? returns false when provider_security equals security" do + @amzn.update!(provider_security: @amzn.security) + assert_not @amzn.security_remapped? + end + + test "remap_security! changes holding security and locks it" do + old_security = @amzn.security + new_security = create_security("GOOG", prices: [ { date: Date.current, price: 100.00 } ]) + + @amzn.remap_security!(new_security) + + assert_equal new_security, @amzn.security + assert @amzn.security_locked? + assert_equal old_security, @amzn.provider_security + end + + test "remap_security! updates all holdings for the same security" do + old_security = @amzn.security + new_security = create_security("GOOG", prices: [ { date: Date.current, price: 100.00 } ]) + + # There are 2 AMZN holdings (from load_holdings) - yesterday and today + amzn_holdings_count = @account.holdings.where(security: old_security).count + assert_equal 2, amzn_holdings_count + + @amzn.remap_security!(new_security) + + # All holdings should now be for the new security + assert_equal 0, @account.holdings.where(security: old_security).count + assert_equal 2, @account.holdings.where(security: new_security).count + + # All should be locked with provider_security set + @account.holdings.where(security: new_security).each do |h| + assert h.security_locked? + assert_equal old_security, h.provider_security + end + end + + test "remap_security! moves trades to new security" do + old_security = @amzn.security + new_security = create_security("GOOG", prices: [ { date: Date.current, price: 100.00 } ]) + + # Create a trade for the old security + create_trade(old_security, account: @account, qty: 5, price: 100.00, date: Date.current) + assert_equal 1, @account.trades.where(security: old_security).count + + @amzn.remap_security!(new_security) + + # Trade should have moved to the new security + assert_equal 0, @account.trades.where(security: old_security).count + assert_equal 1, @account.trades.where(security: new_security).count + end + + test "remap_security! does nothing when security is same" do + current_security = @amzn.security + + @amzn.remap_security!(current_security) + + assert_equal current_security, @amzn.security + assert_not @amzn.security_locked? + assert_nil @amzn.provider_security_id + end + + test "remap_security! merges holdings on collision by combining qty and amount" do + new_security = create_security("GOOG", prices: [ { date: Date.current, price: 100.00 } ]) + + # Create an existing holding for the new security on the same date + existing_goog = @account.holdings.create!( + date: @amzn.date, + security: new_security, + qty: 5, + price: 100, + amount: 500, + currency: "USD" + ) + + amzn_security = @amzn.security + amzn_qty = @amzn.qty + amzn_amount = @amzn.amount + initial_count = @account.holdings.count + + # Remap should merge by combining qty and amount + @amzn.remap_security!(new_security) + + # The AMZN holding on collision date should be deleted, merged into GOOG + assert_equal initial_count - 1, @account.holdings.count + + # The existing GOOG holding should have merged values + existing_goog.reload + assert_equal 5 + amzn_qty, existing_goog.qty + assert_equal 500 + amzn_amount, existing_goog.amount + + # Merged holding should be locked to prevent provider overwrites + assert existing_goog.security_locked, "Merged holding should be locked" + + # No holdings should remain for the old AMZN security + assert_equal 0, @account.holdings.where(security: amzn_security).count + end + + test "reset_security_to_provider! restores original security" do + old_security = @amzn.security + new_security = create_security("GOOG", prices: [ { date: Date.current, price: 100.00 } ]) + + @amzn.remap_security!(new_security) + assert_equal new_security, @amzn.security + assert @amzn.security_locked? + + @amzn.reset_security_to_provider! + + assert_equal old_security, @amzn.security + assert_not @amzn.security_locked? + assert_nil @amzn.provider_security_id + end + + test "reset_security_to_provider! moves trades back" do + old_security = @amzn.security + new_security = create_security("GOOG", prices: [ { date: Date.current, price: 100.00 } ]) + + create_trade(old_security, account: @account, qty: 5, price: 100.00, date: Date.current) + + @amzn.remap_security!(new_security) + assert_equal 1, @account.trades.where(security: new_security).count + + @amzn.reset_security_to_provider! + assert_equal 0, @account.trades.where(security: new_security).count + assert_equal 1, @account.trades.where(security: old_security).count + end + + test "reset_security_to_provider! does nothing if not remapped" do + old_security = @amzn.security + @amzn.reset_security_to_provider! + + assert_equal old_security, @amzn.security + assert_nil @amzn.provider_security_id + end + private def load_holdings diff --git a/test/system/trades_test.rb b/test/system/trades_test.rb index 7d0183c3a..8065beab7 100644 --- a/test/system/trades_test.rb +++ b/test/system/trades_test.rb @@ -58,7 +58,7 @@ class TradesTest < ApplicationSystemTestCase private def open_new_trade_modal - click_on "New transaction" + click_on "New activity" end def within_trades(&block)