Improved UI warning states for holdings with missing data (#1098)

* Fix security price issue flow * Fix tooltip positioning and add tooltip for missing holding data * Fix tooltip controller error with stale arrow target * Lint fixes
maybe-finance · Aug 16, 2024 · 1b6ce6a · 1b6ce6a
1 parent 4527482
commit 1b6ce6a
Show file tree

Hide file tree

Showing 16 changed files with 139 additions and 73 deletions.
diff --git a/app/javascript/controllers/tooltip_controller.js b/app/javascript/controllers/tooltip_controller.js
@@ -4,11 +4,11 @@ import {
   flip,
   shift,
   offset,
-  arrow
+  autoUpdate
 } from '@floating-ui/dom';
 
 export default class extends Controller {
-  static targets = ["arrow", "tooltip"];
+  static targets = ["tooltip"];
   static values = {
     placement: { type: String, default: "top" },
     offset: { type: Number, default: 10 },
@@ -17,58 +17,67 @@ export default class extends Controller {
   };
 
   connect() {
-    this.element.addEventListener("mouseenter", this.showTooltip);
-    this.element.addEventListener("mouseleave", this.hideTooltip);
-    this.element.addEventListener("focus", this.showTooltip);
-    this.element.addEventListener("blur", this.hideTooltip);
-  };
+    this._cleanup = null;
+    this.boundUpdate = this.update.bind(this);
+    this.startAutoUpdate();
+    this.addEventListeners();
+  }
+
+  disconnect() {
+    this.removeEventListeners();
+    this.stopAutoUpdate();
+  }
+
+  addEventListeners() {
+    this.element.addEventListener("mouseenter", this.show);
+    this.element.addEventListener("mouseleave", this.hide);
+  }
 
-  showTooltip = () => {
+  removeEventListeners() {
+    this.element.removeEventListener("mouseenter", this.show);
+    this.element.removeEventListener("mouseleave", this.hide);
+  }
+
+  show = () => {
     this.tooltipTarget.style.display = 'block';
-    this.#update();
-  };
+    this.update(); // Ensure immediate update when shown
+  }
 
-  hideTooltip = () => {
-    this.tooltipTarget.style.display = '';
-  };
+  hide = () => {
+    this.tooltipTarget.style.display = 'none';
+  }
 
-  disconnect() {
-    this.element.removeEventListener("mouseenter", this.showTooltip);
-    this.element.removeEventListener("mouseleave", this.hideTooltip);
-    this.element.removeEventListener("focus", this.showTooltip);
-    this.element.removeEventListener("blur", this.hideTooltip);
-  };
+  startAutoUpdate() {
+    if (!this._cleanup) {
+      this._cleanup = autoUpdate(
+        this.element,
+        this.tooltipTarget,
+        this.boundUpdate
+      );
+    }
+  }
 
-  #update() {
+  stopAutoUpdate() {
+    if (this._cleanup) {
+      this._cleanup();
+      this._cleanup = null;
+    }
+  }
+
+  update() {
+    // Update position even if not visible, to ensure correct positioning when shown
     computePosition(this.element, this.tooltipTarget, {
       placement: this.placementValue,
       middleware: [
         offset({ mainAxis: this.offsetValue, crossAxis: this.crossAxisValue, alignmentAxis: this.alignmentAxisValue }),
         flip(),
-        shift({ padding: 5 }),
-        arrow({ element: this.arrowTarget }),
+        shift({ padding: 5 })
       ],
     }).then(({ x, y, placement, middlewareData }) => {
       Object.assign(this.tooltipTarget.style, {
         left: `${x}px`,
         top: `${y}px`,
       });
-
-      const { x: arrowX, y: arrowY } = middlewareData.arrow;
-      const staticSide = {
-        top: 'bottom',
-        right: 'left',
-        bottom: 'top',
-        left: 'right',
-      }[placement.split('-')[0]];
-
-      Object.assign(this.arrowTarget.style, {
-        left: arrowX != null ? `${arrowX}px` : '',
-        top: arrowY != null ? `${arrowY}px` : '',
-        right: '',
-        bottom: '',
-        [staticSide]: '-4px',
-      });
     });
-  };
-}
+  }
+}
diff --git a/app/models/account.rb b/app/models/account.rb
@@ -75,9 +75,11 @@ def create_with_optional_start_balance!(attributes:, start_date: nil, start_bala
     end
   end
 
-  def alert
-    latest_sync = syncs.latest
-    [ latest_sync&.error, *latest_sync&.warnings ].compact.first
+  def owns_ticker?(ticker)
+    security_id = Security.find_by(ticker: ticker)&.id
+    entries.account_trades
+           .joins("JOIN account_trades ON account_entries.entryable_id = account_trades.id")
+           .where(account_trades: { security_id: security_id }).any?
   end
 
   def favorable_direction

diff --git a/app/models/account/sync.rb b/app/models/account/sync.rb
@@ -25,6 +25,8 @@ def run
   rescue StandardError => error
     account.observe_unknown_issue(error)
     fail! error
+
+    raise error if Rails.env.development?
   end
 
   private

diff --git a/app/models/issue/prices_missing.rb b/app/models/issue/prices_missing.rb
@@ -1,6 +1,8 @@
 class Issue::PricesMissing < Issue
   store_accessor :data, :missing_prices
 
+  after_initialize :initialize_missing_prices
+
   validates :missing_prices, presence: true
 
   def append_missing_price(ticker, date)
@@ -10,7 +12,10 @@ def append_missing_price(ticker, date)
 
   def stale?
     stale = true
+
     missing_prices.each do |ticker, dates|
+      next unless issuable.owns_ticker?(ticker)
+
       oldest_date = dates.min
       expected_price_count = (oldest_date..Date.current).count
       prices = Security::Price.find_prices(ticker: ticker, start_date: oldest_date)
@@ -19,4 +24,10 @@ def stale?
 
     stale
   end
+
+  private
+
+    def initialize_missing_prices
+      self.missing_prices ||= {}
+    end
 end
diff --git a/app/views/account/holdings/_holding.html.erb b/app/views/account/holdings/_holding.html.erb
@@ -4,9 +4,13 @@
   <div class="grid grid-cols-12 items-center text-gray-900 text-sm font-medium p-4">
     <div class="col-span-4 flex items-center gap-4">
       <%= render "shared/circle_logo", name: holding.name || "H" %>
-      <div>
+      <div class="space-y-0.5">
         <%= link_to holding.name || holding.ticker, account_holding_path(holding.account, holding), data: { turbo_frame: :drawer }, class: "hover:underline" %>
-        <%= tag.p holding.ticker, class: "text-gray-500 text-xs uppercase" %>
+        <% if holding.amount %>
+          <%= tag.p holding.ticker, class: "text-gray-500 text-xs uppercase" %>
+        <% else %>
+          <%= render "missing_price_tooltip" %>
+        <% end %>
       </div>
     </div>
 
@@ -15,7 +19,7 @@
         <%= render "shared/progress_circle", progress: holding.weight, text_class: "text-blue-500" %>
         <%= tag.p number_to_percentage(holding.weight, precision: 1) %>
       <% else %>
-        <%= tag.p "?", class: "text-gray-500" %>
+        <%= tag.p "--", class: "text-gray-500 mb-5" %>
       <% end %>
     </div>
 
@@ -28,7 +32,7 @@
       <% if holding.amount_money %>
         <%= tag.p format_money holding.amount_money %>
       <% else %>
-        <%= tag.p "?", class: "text-gray-500" %>
+        <%= tag.p "--", class: "text-gray-500" %>
       <% end %>
       <%= tag.p t(".shares", qty: number_with_precision(holding.qty, precision: 1)), class: "font-normal text-gray-500" %>
     </div>
@@ -38,7 +42,7 @@
         <%= tag.p format_money(holding.trend.value), style: "color: #{holding.trend.color};" %>
         <%= tag.p "(#{number_to_percentage(holding.trend.percent, precision: 1)})", style: "color: #{holding.trend.color};" %>
       <% else %>
-        <%= tag.p "?", class: "text-gray-500" %>
+        <%= tag.p "--", class: "text-gray-500 mb-4" %>
       <% end %>
     </div>
   </div>

diff --git a/app/views/account/holdings/_missing_price_tooltip.html.erb b/app/views/account/holdings/_missing_price_tooltip.html.erb
@@ -0,0 +1,11 @@
+<div data-controller="tooltip" data-tooltip-cross-axis-value="50">
+  <div class="flex items-center gap-1 text-warning">
+    <%= lucide_icon "info", class: "w-4 h-4 shrink-0" %>
+    <%= tag.span t(".missing_data"), class: "font-normal text-xs" %>
+  </div>
+  <div role="tooltip" data-tooltip-target="tooltip" class="tooltip bg-gray-700 text-sm p-2 rounded w-64">
+    <div class="text-white">
+      <%= t(".description") %>
+    </div>
+  </div>
+</div>
diff --git a/app/views/accounts/_tooltip.html.erb b/app/views/accounts/_tooltip.html.erb
@@ -1,7 +1,7 @@
 <%# locals: (account:) -%>
-<div data-controller="tooltip" data-tooltip-target="element" data-tooltip-placement-value="right" data-tooltip-offset-value=10 data-tooltip-cross-axis-value=50>
+<div data-controller="tooltip" data-tooltip-placement-value="right" data-tooltip-offset-value=10 data-tooltip-cross-axis-value=50>
   <%= lucide_icon("info", class: "w-4 h-4 shrink-0 text-gray-500") %>
-  <div id="tooltip" role="tooltip" data-tooltip-target="tooltip" class="tooltip bg-gray-700 text-sm p-2 rounded w-64">
+  <div role="tooltip" data-tooltip-target="tooltip" class="tooltip bg-gray-700 text-sm p-2 rounded w-64">
     <div class="text-white">
       <%= t(".total_value_tooltip") %>
     </div>
@@ -22,5 +22,4 @@
       </div>
     </div>
   </div>
-  <div data-tooltip-target="arrow"></div>
 </div>
diff --git a/app/views/issue/_request_synth_data_action.html.erb b/app/views/issue/_request_synth_data_action.html.erb
@@ -0,0 +1,12 @@
+<p>The Synth data provider could not find the requested data.</p>
+
+<p>We are actively developing Synth to be a low cost and easy to use data provider. You can help us improve Synth by
+  requesting the data you need.</p>
+
+<p>Please post in our <%= link_to "Discord server", "https://link.maybe.co/discord", target: "_blank" %> with the
+  following information:</p>
+
+<ul>
+  <li>What type of data is missing?</li>
+  <li>Any other information you think might be helpful</li>
+</ul>
diff --git a/app/views/issue/exchange_rates_missings/show.html.erb b/app/views/issue/exchange_rates_missings/show.html.erb
@@ -7,16 +7,5 @@
 <% end %>
 
 <%= content_for :action do %>
-  <p>The Synth data provider could not find the requested data.</p>
-
-  <p>We are actively developing Synth to be a low cost and easy to use data provider. You can help us improve Synth by
-    requesting the data you need.</p>
-
-  <p>Please post in our <%= link_to "Discord server", "https://link.maybe.co/discord", target: "_blank" %> with the
-    following information:</p>
-
-  <ul>
-    <li>What type of data is missing?</li>
-    <li>Any other information you think might be helpful</li>
-  </ul>
+  <%= render "issue/request_synth_data_action" %>
 <% end %>
diff --git a/app/views/issue/prices_missings/show.html.erb b/app/views/issue/prices_missings/show.html.erb
@@ -0,0 +1,11 @@
+<%= content_for :title, @issue.title %>
+
+<%= content_for :description do %>
+  <p>Some stock prices are missing for this account.</p>
+
+  <pre><code><%= JSON.pretty_generate(@issue.data) %></code></pre>
+<% end %>
+
+<%= content_for :action do %>
+  <%= render "issue/request_synth_data_action" %>
+<% end %>
diff --git a/app/views/issues/_issue.html.erb b/app/views/issues/_issue.html.erb
@@ -1,14 +1,15 @@
 <%# locals: (issue:) %>
 
 <% priority_class = issue.critical? || issue.error? ? "bg-error/5" : "bg-warning/5" %>
+<% text_class = issue.critical? || issue.error? ? "text-error" : "text-warning" %>
 
 <%= tag.div class: "flex gap-6 items-center rounded-xl px-4 py-3 #{priority_class}" do %>
-  <div class="flex gap-3 items-center text-red-500 grow overflow-x-scroll">
+  <div class="flex gap-3 items-center grow overflow-x-scroll <%= text_class %>">
     <%= lucide_icon("alert-octagon", class: "w-5 h-5 shrink-0") %>
     <p class="text-sm whitespace-nowrap"><%= issue.title %></p>
   </div>
 
   <div class="flex items-center gap-4 ml-auto">
-    <%= link_to "Troubleshoot", issue_path(issue), class: "text-red-500 font-medium hover:underline", data: { turbo_frame: :drawer } %>
+    <%= link_to "Troubleshoot", issue_path(issue), class: "#{text_class} font-medium hover:underline", data: { turbo_frame: :drawer } %>
   </div>
 <% end %>
diff --git a/app/views/shared/_drawer.html.erb b/app/views/shared/_drawer.html.erb
@@ -1,12 +1,12 @@
 <%= turbo_frame_tag "drawer" do %>
-  <dialog class="bg-white border border-alpha-black-25 rounded-2xl max-h-[calc(100vh-32px)] max-w-[480px] w-full shadow-xs h-full mt-4 mr-4 focus-visible:outline-none" data-controller="modal" data-action="click->modal#clickOutside">
-    <div class="flex flex-col h-full p-4">
-      <div class="flex justify-end items-center pb-4">
+  <dialog class="bg-white border border-alpha-black-25 rounded-2xl max-w-[480px] w-full shadow-xs h-full mt-4 mr-4 focus-visible:outline-none flex flex-col" data-controller="modal" data-action="click->modal#clickOutside">
+    <div class="flex flex-col h-full">
+      <div class="flex justify-end items-center p-4">
         <div data-action="click->modal#close" class="cursor-pointer p-2">
           <%= lucide_icon("x", class: "w-5 h-5 shrink-0") %>
         </div>
       </div>
-      <div class="flex flex-col overflow-scroll">
+      <div class="flex-1 overflow-y-auto px-4 pb-4">
         <%= content %>
       </div>
     </div>

diff --git a/config/locales/models/issue/en.yml b/config/locales/models/issue/en.yml
@@ -4,4 +4,5 @@ en:
     models:
       issue/exchange_rate_provider_missing: Exchange rate provider missing
       issue/exchange_rates_missing: Exchange rates missing
+      issue/missing_prices: Missing prices
       issue/unknown: Unknown issue occurred
diff --git a/config/locales/views/account/holdings/en.yml b/config/locales/views/account/holdings/en.yml
@@ -15,6 +15,10 @@ en:
         no_holdings: No holdings to show.
         return: total return
         weight: weight
+      missing_price_tooltip:
+        description: This investment has missing values and we could not calculate
+          its returns or value.
+        missing_data: Missing data
       show:
         history: History
         overview: Overview

diff --git a/test/models/account_test.rb b/test/models/account_test.rb
@@ -89,4 +89,12 @@ class AccountTest < ActiveSupport::TestCase
     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
+
+  test "can observe missing price" do
+    account = accounts(:investment)
+
+    assert_difference -> { account.issues.count } do
+      account.observe_missing_price(ticker: "AAPL", date: Date.current)
+    end
+  end
 end
diff --git a/test/system/tooltips_test.rb b/test/system/tooltips_test.rb
@@ -11,16 +11,18 @@ class TooltipsTest < ApplicationSystemTestCase
 
   test "can see account information tooltip" do
     visit account_path(@account)
-    find('[data-controller="tooltip"]').hover
-    assert find("#tooltip", visible: true)
-    within "#tooltip" do
+    tooltip_element = find('[data-controller="tooltip"]')
+    tooltip_element.hover
+    tooltip_contents = find('[data-tooltip-target="tooltip"]')
+    assert tooltip_contents.visible?
+    within tooltip_contents do
       assert_text I18n.t("accounts.tooltip.total_value_tooltip")
       assert_text I18n.t("accounts.tooltip.holdings")
       assert_text format_money(@account.investment.holdings_value, precision: 0)
       assert_text I18n.t("accounts.tooltip.cash")
       assert_text format_money(@account.balance_money, precision: 0)
     end
     find("body").click
-    assert find("#tooltip", visible: false)
+    assert find('[data-tooltip-target="tooltip"]', visible: false)
   end
 end