From 0f6dd536df84efd1767cf3dec96df8e6e3701fab Mon Sep 17 00:00:00 2001 From: LPW Date: Sat, 17 Jan 2026 16:46:15 -0500 Subject: [PATCH] Enhance ticker search and validation in "Convert to Trade" form (#688) - Updated resolution logic to support combobox-based ticker selection and validation. - Added market price display with validation against entered prices to detect significant mismatches. - Improved messaging and UI for custom ticker input and market price warnings. Co-authored-by: luckyPipewrench --- app/controllers/transactions_controller.rb | 32 +++++-- .../convert_to_trade_controller.js | 78 +++++++++++++++- app/models/security/resolver.rb | 8 +- .../transactions/convert_to_trade.html.erb | 90 +++++++++++++++---- config/locales/views/transactions/en.yml | 8 +- 5 files changed, 184 insertions(+), 32 deletions(-) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 8c8ef6a21..c97482c90 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -366,17 +366,25 @@ class TransactionsController < ApplicationController def resolve_security_for_conversion if params[:security_id] == "__custom__" - ticker = params[:custom_ticker].presence - unless ticker.present? + # User selected "Enter custom ticker" - check for combobox selection or manual entry + if params[:ticker].present? + # Combobox selection: format is "SYMBOL|EXCHANGE" + ticker_symbol, exchange_operating_mic = params[:ticker].split("|") + Security::Resolver.new( + ticker_symbol.strip, + exchange_operating_mic: exchange_operating_mic.presence || params[:exchange_operating_mic].presence + ).resolve + elsif params[:custom_ticker].present? + # Manual entry from combobox's name_when_new or fallback text field + Security::Resolver.new( + params[:custom_ticker].strip, + exchange_operating_mic: params[:exchange_operating_mic].presence + ).resolve + else flash[:alert] = t("transactions.convert_to_trade.errors.enter_ticker") redirect_back_or_to transactions_path return nil end - - Security::Resolver.new( - ticker.strip, - exchange_operating_mic: params[:exchange_operating_mic].presence - ).resolve elsif params[:security_id].present? found = Security.find_by(id: params[:security_id]) unless found @@ -386,8 +394,16 @@ class TransactionsController < ApplicationController end found elsif params[:ticker].present? + # Direct combobox (no existing holdings) - format is "SYMBOL|EXCHANGE" + ticker_symbol, exchange_operating_mic = params[:ticker].split("|") Security::Resolver.new( - params[:ticker].strip, + ticker_symbol.strip, + exchange_operating_mic: exchange_operating_mic.presence || params[:exchange_operating_mic].presence + ).resolve + elsif params[:custom_ticker].present? + # Manual entry from combobox's name_when_new (no existing holdings path) + Security::Resolver.new( + params[:custom_ticker].strip, exchange_operating_mic: params[:exchange_operating_mic].presence ).resolve end.tap do |security| diff --git a/app/javascript/controllers/convert_to_trade_controller.js b/app/javascript/controllers/convert_to_trade_controller.js index c0e71663b..7b92abcca 100644 --- a/app/javascript/controllers/convert_to_trade_controller.js +++ b/app/javascript/controllers/convert_to_trade_controller.js @@ -1,7 +1,11 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { - static targets = ["customWrapper", "customField", "tickerSelect"] + static targets = ["customWrapper", "customField", "tickerSelect", "qtyField", "priceField", "priceWarning", "priceWarningMessage"] + static values = { + amountCents: Number, + currency: String + } toggleCustomTicker(event) { const value = event.target.value @@ -18,4 +22,76 @@ export default class extends Controller { this.customFieldTarget.value = "" } } + + validatePrice() { + // Get the selected security's market price (in cents) + const selectedOption = this.tickerSelectTarget.selectedOptions[0] + if (!selectedOption || selectedOption.value === "" || selectedOption.value === "__custom__") { + this.hidePriceWarning() + return + } + + const marketPriceCents = selectedOption.dataset.priceCents + const ticker = selectedOption.dataset.ticker + + // If no market price data, can't validate + if (!marketPriceCents || marketPriceCents === "null") { + this.hidePriceWarning() + return + } + + // Calculate the implied/entered price + let enteredPriceCents = null + const qty = Number.parseFloat(this.qtyFieldTarget?.value) + const enteredPrice = Number.parseFloat(this.priceFieldTarget?.value) + + if (enteredPrice && enteredPrice > 0) { + // User entered a price directly + enteredPriceCents = enteredPrice * 100 + } else if (qty && qty > 0 && this.amountCentsValue > 0) { + // Calculate price from amount / qty + enteredPriceCents = this.amountCentsValue / qty + } + + if (!enteredPriceCents || enteredPriceCents <= 0) { + this.hidePriceWarning() + return + } + + // Compare prices - warn if they differ by more than 50% + const marketPrice = Number.parseFloat(marketPriceCents) + const ratio = enteredPriceCents / marketPrice + + if (ratio < 0.5 || ratio > 2.0) { + this.showPriceWarning(ticker, enteredPriceCents, marketPrice) + } else { + this.hidePriceWarning() + } + } + + showPriceWarning(ticker, enteredPriceCents, marketPriceCents) { + if (!this.hasPriceWarningTarget) return + + const enteredPrice = this.formatMoney(enteredPriceCents) + const marketPrice = this.formatMoney(marketPriceCents) + + // Build warning message + const message = `Your price (${enteredPrice}/share) differs significantly from ${ticker}'s current market price (${marketPrice}). If this seems wrong, you may have selected the wrong security — try using "Enter custom ticker" to specify the correct one.` + + this.priceWarningMessageTarget.textContent = message + this.priceWarningTarget.classList.remove("hidden") + } + + hidePriceWarning() { + if (!this.hasPriceWarningTarget) return + this.priceWarningTarget.classList.add("hidden") + } + + formatMoney(cents) { + const dollars = cents / 100 + return new Intl.NumberFormat('en-US', { + style: 'currency', + currency: this.currencyValue || 'USD' + }).format(dollars) + } } diff --git a/app/models/security/resolver.rb b/app/models/security/resolver.rb index 2b0e13bc9..c8f403d4b 100644 --- a/app/models/security/resolver.rb +++ b/app/models/security/resolver.rb @@ -82,11 +82,13 @@ class Security::Resolver filtered_candidates = filtered_candidates.select { |s| s.country_code.upcase.to_s == country_code.upcase.to_s } end - # 1. Prefer exact exchange_operating_mic matches (if one was provided) - # 2. Rank by country relevance (lower index in the list is more relevant) - # 3. Rank by exchange_operating_mic relevance (lower index in the list is more relevant) + # 1. Prefer exact ticker matches (MSTR before MSTRX when searching for "MSTR") + # 2. Prefer exact exchange_operating_mic matches (if one was provided) + # 3. Rank by country relevance (lower index in the list is more relevant) + # 4. Rank by exchange_operating_mic relevance (lower index in the list is more relevant) sorted_candidates = filtered_candidates.sort_by do |s| [ + s.ticker.upcase.to_s == symbol.upcase.to_s ? 0 : 1, exchange_operating_mic.present? && s.exchange_operating_mic.upcase.to_s == exchange_operating_mic.upcase.to_s ? 0 : 1, sorted_country_codes_by_relevance.index(s.country_code&.upcase.to_s) || sorted_country_codes_by_relevance.length, sorted_exchange_operating_mics_by_relevance.index(s.exchange_operating_mic&.upcase.to_s) || sorted_exchange_operating_mics_by_relevance.length diff --git a/app/views/transactions/convert_to_trade.html.erb b/app/views/transactions/convert_to_trade.html.erb index 4c26097bc..32c3f172d 100644 --- a/app/views/transactions/convert_to_trade.html.erb +++ b/app/views/transactions/convert_to_trade.html.erb @@ -5,12 +5,24 @@ default_label = params[:activity_label].presence || (@entry.amount > 0 ? "Sell" : "Buy") # Get unique securities from account holdings for ticker suggestions + # Include current price for display and validation account_securities = @entry.account.holdings .includes(:security) .where.not(security: nil) .map(&:security) .uniq(&:ticker) .sort_by(&:ticker) + + # Build options with price data for validation + security_options_with_prices = account_securities.map do |s| + price = s.current_price + price_display = price ? " (#{format_money(price)})" : "" + label = "#{s.ticker}#{price_display}" + label += " - #{s.name.truncate(25)}" if s.name.present? + # Store price as cents for JS comparison (amount is BigDecimal, multiply by 100 for cents) + price_cents = price ? (price.amount * 100).to_i : nil + [label, s.id, { "data-price-cents" => price_cents, "data-ticker" => s.ticker }] + end %> <%= render DS::Dialog.new(variant: "modal", reload_on_close: true) do |dialog| %> @@ -20,7 +32,7 @@ <% end %> <% dialog.with_body do %> - <%= form_with url: create_trade_from_transaction_transaction_path(@transaction), method: :post, class: "space-y-4", data: { controller: "convert-to-trade", turbo_frame: "_top" } do |f| %> + <%= form_with url: create_trade_from_transaction_transaction_path(@transaction), method: :post, class: "space-y-4", data: { controller: "convert-to-trade", turbo_frame: "_top", convert_to_trade_amount_cents_value: (@entry.amount_money.abs.amount * 100).to_i, convert_to_trade_currency_value: @entry.currency } do |f| %>
@@ -45,7 +57,7 @@ <%= f.select :security_id, options_for_select( [[t(".security_prompt"), ""]] + - account_securities.map { |s| ["#{s.ticker}#{s.name.present? ? " - #{s.name.truncate(30)}" : ""}", s.id] } + + security_options_with_prices + [[t(".security_custom"), "__custom__"]], nil ), @@ -53,27 +65,50 @@ { class: "form-field__input border border-secondary rounded-lg px-3 py-2 w-full text-primary bg-container", data: { - action: "change->convert-to-trade#toggleCustomTicker", + action: "change->convert-to-trade#toggleCustomTicker change->convert-to-trade#validatePrice", convert_to_trade_target: "tickerSelect" } } %> +

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

+ <% else %> + <% if Security.provider.present? %> +
+ <%= f.combobox :ticker, + securities_path(country_code: Current.family.country), + name_when_new: "custom_ticker", + placeholder: t(".ticker_search_placeholder"), + required: true %> +
+

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

+ <% else %> + <%= f.text_field :ticker, placeholder: t(".ticker_placeholder"), + required: true, class: "form-field__input border border-secondary rounded-lg px-3 py-2 w-full text-primary bg-container", pattern: "[A-Za-z0-9.:-]{1,20}", - title: t(".ticker_hint"), - data: { convert_to_trade_target: "customField" } %> -
-

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

- <% else %> - <%= f.text_field :ticker, - placeholder: t(".ticker_placeholder"), - required: true, - class: "form-field__input border border-secondary rounded-lg px-3 py-2 w-full text-primary bg-container", - pattern: "[A-Za-z0-9.:-]{1,20}", - title: t(".ticker_hint") %> -

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

+ title: t(".ticker_hint") %> +

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

+ <% end %> <% end %>
@@ -83,7 +118,11 @@ <%= f.number_field :qty, step: "any", placeholder: t(".quantity_placeholder"), - class: "form-field__input border border-secondary rounded-lg px-3 py-2 w-full text-primary bg-container" %> + class: "form-field__input border border-secondary rounded-lg px-3 py-2 w-full text-primary bg-container", + data: { + action: "input->convert-to-trade#validatePrice", + convert_to_trade_target: "qtyField" + } %>

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

@@ -93,11 +132,26 @@ step: "0.0001", min: "0", placeholder: t(".price_placeholder"), - class: "form-field__input border border-secondary rounded-lg px-3 py-2 w-full text-primary bg-container" %> + class: "form-field__input border border-secondary rounded-lg px-3 py-2 w-full text-primary bg-container", + data: { + action: "input->convert-to-trade#validatePrice", + convert_to_trade_target: "priceField" + } %>

<%= t(".price_hint", currency: @entry.currency) %>

+ + +

<%= icon "info", size: "xs", class: "inline-block mr-1" %> <%= t(".qty_or_price_hint", amount: format_money(@entry.amount_money.abs)) %> diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml index 35eb2308e..6f4a78ba1 100644 --- a/config/locales/views/transactions/en.yml +++ b/config/locales/views/transactions/en.yml @@ -115,9 +115,13 @@ en: security_label: Security security_prompt: Select a security... security_custom: "+ Enter custom ticker" - security_hint: Select from your holdings or enter a custom ticker + security_not_listed_hint: Don't see your security? Select "Enter custom ticker" at the bottom of the list. ticker_placeholder: AAPL - ticker_hint: Enter the stock/ETF ticker symbol + ticker_hint: Enter the stock/ETF ticker symbol (e.g., AAPL, MSFT) + ticker_search_placeholder: Search for a ticker... + ticker_search_hint: Search by ticker symbol or company name, or type a custom ticker + price_mismatch_title: Price may not match + price_mismatch_message: "Your price (%{entered_price}/share) differs significantly from %{ticker}'s current market price (%{market_price}). If this seems wrong, you may have selected the wrong security — try using \"Enter custom ticker\" to specify the correct one." quantity_label: Quantity (Shares) quantity_placeholder: e.g. 20 quantity_hint: Number of shares traded