Account Issue Model and Resolution Flow + Troubleshooting guides (#1090)

* Rough draft of issue system

* Simplify design

* Remove stale files from merge conflicts

* STI for issues

* Cleanup

* Improve Synth api key flow

* Stub api key for test

app/controllers/issue/exchange_rate_provider_missings_controller.rb

@@ -0,0 +1,19 @@
+class Issue::ExchangeRateProviderMissingsController < ApplicationController
+  before_action :set_issue, only: :update
+
+  def update
+    Setting.synth_api_key = exchange_rate_params[:synth_api_key]
+    @issue.issuable.sync_later
+    redirect_back_or_to account_path(@issue.issuable)
+  end
+
+  private
+
+    def set_issue
+      @issue = Current.family.issues.find(params[:id])
+    end
+
+    def exchange_rate_params
+      params.require(:issue_exchange_rate_provider_missing).permit(:synth_api_key)
+    end
+end

app/controllers/issues_controller.rb

@@ -0,0 +1,13 @@
+class IssuesController < ApplicationController
+  before_action :set_issue, only: :show
+
+  def show
+    render template: "#{@issue.class.name.underscore.pluralize}/show", layout: "issues"
+  end
+
+  private
+
+    def set_issue
+      @issue = Current.family.issues.find(params[:id])
+    end
+end

app/models/account.rb

@@ -1,6 +1,5 @@
 class Account < ApplicationRecord
-  include Syncable
-  include Monetizable
+  include Syncable, Monetizable, Issuable
 
   validates :name, :balance, :currency, presence: true
 
@@ -15,6 +14,7 @@ class Account < ApplicationRecord
   has_many :balances, dependent: :destroy
   has_many :imports, dependent: :destroy
   has_many :syncs, dependent: :destroy
+  has_many :issues, as: :issuable, dependent: :destroy
 
   monetize :balance
 

app/models/account/balance/syncer.rb

@@ -1,9 +1,6 @@
 class Account::Balance::Syncer
-  attr_reader :warnings
-
   def initialize(account, start_date: nil)
     @account = account
-    @warnings = []
     @sync_start_date = calculate_sync_start_date(start_date)
   end
 
@@ -20,6 +17,8 @@ class Account::Balance::Syncer
         account.update! balance: daily_balances.select { |db| db.currency == account.currency }.last&.balance
       end
     end
+  rescue Money::ConversionError => e
+    account.observe_missing_exchange_rates(from: e.from_currency, to: e.to_currency, dates: [ e.date ])
   end
 
   private
@@ -67,20 +66,26 @@ class Account::Balance::Syncer
       from_currency = account.currency
       to_currency = account.family.currency
 
+      if ExchangeRate.exchange_rates_provider.nil?
+        account.observe_missing_exchange_rate_provider
+        return []
+      end
+
       exchange_rates = ExchangeRate.find_rates from: from_currency,
                                                to: to_currency,
                                                start_date: sync_start_date
 
+      missing_exchange_rates = balances.map(&:date) - exchange_rates.map(&:date)
+
+      if missing_exchange_rates.any?
+        account.observe_missing_exchange_rates(from: from_currency, to: to_currency, dates: missing_exchange_rates)
+        return []
+      end
+
       balances.map do |balance|
         exchange_rate = exchange_rates.find { |er| er.date == balance.date }
-
-        raise Money::ConversionError.new("missing exchange rate from #{from_currency} to #{to_currency} on date #{balance.date}") unless exchange_rate
-
         build_balance(balance.date, exchange_rate.rate * balance.balance, to_currency)
       end
-    rescue Money::ConversionError
-      @warnings << "missing exchange rates from #{from_currency} to #{to_currency}"
-      []
     end
 
     def build_balance(date, balance, currency = nil)

app/models/account/holding/syncer.rb

@@ -1,9 +1,6 @@
 class Account::Holding::Syncer
-  attr_reader :warnings
-
   def initialize(account, start_date: nil)
     @account = account
-    @warnings = []
     @sync_date_range = calculate_sync_start_date(start_date)..Date.current
     @portfolio = {}
 
@@ -69,6 +66,8 @@ class Account::Holding::Syncer
 
         price = get_cached_price(ticker, date) || trade_price
 
+        account.observe_missing_price(ticker:, date:) unless price
+
         account.holdings.build \
           date: date,
           security_id: holding[:security_id],

app/models/account/sync.rb

@@ -16,34 +16,25 @@ class Account::Sync < ApplicationRecord
   def run
     start!
 
+    account.resolve_stale_issues
+
     sync_balances
     sync_holdings
 
     complete!
   rescue StandardError => error
+    account.observe_unknown_issue(error)
     fail! error
   end
 
   private
 
     def sync_balances
-      syncer = Account::Balance::Syncer.new(account, start_date: start_date)
-
-      syncer.run
-
-      append_warnings(syncer.warnings)
+      Account::Balance::Syncer.new(account, start_date: start_date).run
     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
+      Account::Holding::Syncer.new(account, start_date: start_date).run
     end
 
     def start!
@@ -53,12 +44,17 @@ class Account::Sync < ApplicationRecord
 
     def complete!
       update! status: "completed"
-      broadcast_result type: "notice", message: "Sync complete"
+
+      if account.has_issues?
+        broadcast_result type: "alert", message: account.highest_priority_issue.title
+      else
+        broadcast_result type: "notice", message: "Sync complete"
+      end
     end
 
     def fail!(error)
       update! status: "failed", error: error.message
-      broadcast_result type: "alert", message: error.message
+      broadcast_result type: "alert", message: I18n.t("account.sync.failed")
     end
 
     def broadcast_start
@@ -78,6 +74,7 @@ class Account::Sync < ApplicationRecord
         partial: "shared/notification",
         locals: { type: type, message: message }
       )
+
       account.family.broadcast_refresh
     end
 end

app/models/concerns/issuable.rb

@@ -0,0 +1,58 @@
+module Issuable
+  extend ActiveSupport::Concern
+
+  included do
+    has_many :issues, dependent: :destroy, as: :issuable
+  end
+
+  def has_issues?
+    issues.active.any?
+  end
+
+  def resolve_stale_issues
+    issues.active.each do |issue|
+      issue.resolve! if issue.stale?
+    end
+  end
+
+  def observe_unknown_issue(error)
+    observe_issue(
+      Issue::Unknown.new(data: { error: error.message })
+    )
+  end
+
+  def observe_missing_exchange_rates(from:, to:, dates:)
+    observe_issue(
+      Issue::ExchangeRatesMissing.new(data: { from_currency: from, to_currency: to, dates: dates })
+    )
+  end
+
+  def observe_missing_exchange_rate_provider
+    observe_issue(
+      Issue::ExchangeRateProviderMissing.new
+    )
+  end
+
+  def observe_missing_price(ticker:, date:)
+    issue = issues.find_or_create_by(type: Issue::PricesMissing.name, resolved_at: nil)
+    issue.append_missing_price(ticker, date)
+    issue.save!
+  end
+
+  def highest_priority_issue
+    issues.active.ordered.first
+  end
+
+  private
+
+    def observe_issue(new_issue)
+      existing_issue = issues.find_by(type: new_issue.type, resolved_at: nil)
+
+      if existing_issue
+        existing_issue.update!(last_observed_at: Time.current, data: new_issue.data)
+      else
+        new_issue.issuable = self
+        new_issue.save!
+      end
+    end
+end

app/models/concerns/providable.rb

@@ -21,10 +21,12 @@ module Providable
     private
 
       def synth_provider
-        @synth_provider ||= begin
-                              api_key = ENV["SYNTH_API_KEY"]
-                              api_key.present? ? Provider::Synth.new(api_key) : nil
-                            end
+        api_key = self_hosted? ? Setting.synth_api_key : ENV["SYNTH_API_KEY"]
+        api_key.present? ? Provider::Synth.new(api_key) : nil
+      end
+
+      def self_hosted?
+        Rails.application.config.app_mode.self_hosted?
       end
   end
 end

app/models/exchange_rate/provided.rb

@@ -4,6 +4,10 @@ module ExchangeRate::Provided
   include Providable
 
   class_methods do
+    def provider_healthy?
+      exchange_rates_provider.present? && exchange_rates_provider.healthy?
+    end
+
     private
 
       def fetch_rates_from_provider(from:, to:, start_date:, end_date: Date.current, cache: false)

app/models/family.rb

@@ -8,6 +8,7 @@ class Family < ApplicationRecord
   has_many :imports, through: :accounts
   has_many :categories, dependent: :destroy
   has_many :merchants, dependent: :destroy
+  has_many :issues, through: :accounts
 
   def snapshot(period = Period.all)
     query = accounts.active.joins(:balances)

app/models/help/article.rb

@@ -1,54 +0,0 @@
-class Help::Article
-  attr_reader :frontmatter, :content
-
-  def initialize(frontmatter:, content:)
-    @frontmatter = frontmatter
-    @content = content
-  end
-
-  def title
-    frontmatter["title"]
-  end
-
-  def html
-    render_markdown(content)
-  end
-
-  class << self
-    def root_path
-      Rails.root.join("docs", "help")
-    end
-
-    def find(slug)
-      Dir.glob(File.join(root_path, "*.md")).each do |file_path|
-        file_content = File.read(file_path)
-        frontmatter, markdown_content = parse_frontmatter(file_content)
-
-        return new(frontmatter:, content: markdown_content) if frontmatter["slug"] == slug
-      end
-
-      nil
-    end
-
-    private
-
-      def parse_frontmatter(content)
-        if content =~ /\A---(.+?)---/m
-          frontmatter = YAML.safe_load($1)
-          markdown_content = content[($~.end(0))..-1].strip
-        else
-          frontmatter = {}
-          markdown_content = content
-        end
-
-        [ frontmatter, markdown_content ]
-      end
-  end
-
-  private
-
-    def render_markdown(content)
-      markdown = Redcarpet::Markdown.new(Redcarpet::Render::HTML)
-      markdown.render(content)
-    end
-end

app/models/issue.rb

@@ -0,0 +1,35 @@
+class Issue < ApplicationRecord
+  belongs_to :issuable, polymorphic: true
+
+  after_initialize :set_default_severity
+
+  enum :severity, { critical: 1, error: 2, warning: 3, info: 4 }
+
+  validates :severity, presence: true
+
+  scope :active, -> { where(resolved_at: nil) }
+  scope :ordered, -> { order(:severity) }
+
+  def title
+    model_name.human
+  end
+
+  # The conditions that must be met for an issue to be fixed
+  def stale?
+    raise NotImplementedError, "#{self.class} must implement #{__method__}"
+  end
+
+  def resolve!
+    update!(resolved_at: Time.current)
+  end
+
+  def default_severity
+    :warning
+  end
+
+  private
+
+    def set_default_severity
+      self.severity ||= default_severity
+    end
+end

app/models/issue/exchange_rate_provider_missing.rb

@@ -0,0 +1,9 @@
+class Issue::ExchangeRateProviderMissing < Issue
+  def default_severity
+    :error
+  end
+
+  def stale?
+    ExchangeRate.provider_healthy?
+  end
+end

app/models/issue/exchange_rates_missing.rb

@@ -0,0 +1,15 @@
+class Issue::ExchangeRatesMissing < Issue
+  store_accessor :data, :from_currency, :to_currency, :dates
+
+  validates :from_currency, :to_currency, :dates, presence: true
+
+  def stale?
+    if dates.length == 1
+      ExchangeRate.find_rate(from: from_currency, to: to_currency, date: dates.first).present?
+    else
+      sorted_dates = dates.sort
+      rates = ExchangeRate.find_rates(from: from_currency, to: to_currency, start_date: sorted_dates.first, end_date: sorted_dates.last)
+      rates.length == dates.length
+    end
+  end
+end

app/models/issue/prices_missing.rb

@@ -0,0 +1,22 @@
+class Issue::PricesMissing < Issue
+  store_accessor :data, :missing_prices
+
+  validates :missing_prices, presence: true
+
+  def append_missing_price(ticker, date)
+    missing_prices[ticker] ||= []
+    missing_prices[ticker] << date
+  end
+
+  def stale?
+    stale = true
+    missing_prices.each do |ticker, dates|
+      oldest_date = dates.min
+      expected_price_count = (oldest_date..Date.current).count
+      prices = Security::Price.find_prices(ticker: ticker, start_date: oldest_date)
+      stale = false if prices.count < expected_price_count
+    end
+
+    stale
+  end
+end

app/models/issue/unknown.rb

@@ -0,0 +1,11 @@
+class Issue::Unknown < Issue
+  def default_severity
+    :warning
+  end
+
+  # Unknown issues are always stale because we only want to show them
+  # to the user once.  If the same error occurs again, we'll create a new instance.
+  def stale?
+    true
+  end
+end

app/models/provider/synth.rb

@@ -5,6 +5,11 @@ class Provider::Synth
     @api_key = api_key
   end
 
+  def healthy?
+    response = client.get("#{base_url}/user")
+    JSON.parse(response.body).dig("id").present?
+  end
+
   def fetch_security_prices(ticker:, start_date:, end_date:)
     prices = paginate(
       "#{base_url}/tickers/#{ticker}/open-close",

app/models/setting.rb

@@ -20,6 +20,8 @@ class Setting < RailsSettings::Base
   field :app_domain, type: :string, default: ENV["APP_DOMAIN"]
   field :email_sender, type: :string, default: ENV["EMAIL_SENDER"]
 
+  field :synth_api_key, type: :string, default: ENV["SYNTH_API_KEY"]
+
   scope :smtp_settings do
     field :smtp_host, type: :string, read_only: true, default: ENV["SMTP_ADDRESS"]
     field :smtp_port, type: :string, read_only: true, default: ENV["SMTP_PORT"]

app/views/accounts/_alert.html.erb

@@ -1,19 +0,0 @@
-<%# locals: (message:, help_path: nil) -%>
-<%= tag.div class: "flex gap-6 items-center rounded-xl px-4 py-3 bg-error/5",
-            data: { controller: "element-removal" },
-            role: "alert" do %>
-  <div class="flex gap-3 items-center text-red-500 grow overflow-x-scroll">
-    <%= lucide_icon("alert-octagon", class: "w-5 h-5 shrink-0") %>
-    <p class="text-sm whitespace-nowrap"><%= message %></p>
-  </div>
-
-  <div class="flex items-center gap-4 ml-auto">
-    <% if help_path %>
-      <%= link_to "Troubleshoot", help_path, class: "text-red-500 font-medium hover:underline", data: { turbo_frame: :drawer } %>
-    <% end %>
-
-    <%= tag.button data: { action: "click->element-removal#remove" } do %>
-      <%= lucide_icon("x", class: "w-5 h-5 shrink-0 text-red-500") %>
-    <% end %>
-  </div>
-<% end %>

app/views/accounts/show.html.erb

@@ -45,8 +45,8 @@
     </div>
   </div>
 
-  <% if @account.alert %>
-    <%= render "alert", message: @account.alert, help_path: help_article_path("troubleshooting") %>
+  <% if @account.highest_priority_issue %>
+    <%= render partial: "issues/issue", locals: { issue: @account.highest_priority_issue } %>
   <% end %>
 
   <div class="bg-white shadow-xs rounded-xl border border-alpha-black-25 rounded-lg">

app/views/help/articles/show.html.erb

@@ -1,7 +0,0 @@
-<%= drawer do %>
-  <div class="prose">
-    <%= tag.h1 @article.title %>
-
-    <%= sanitize(@article.html).html_safe %>
-  </div>
-<% end %>

app/views/issue/exchange_rate_provider_missings/show.html.erb

@@ -0,0 +1,28 @@
+<%= content_for :title, @issue.title %>
+
+<%= content_for :description do %>
+  <p>You have set your family currency preference to <%= Current.family.currency %>.  <%= @issue.issuable.name %> has
+    entries in another currency, which means we have to fetch exchange rates from a data provider to accurately show
+    historical results.</p>
+
+  <p>We have detected that your exchange rates provider is not configured yet.</p>
+<% end %>
+
+<%= content_for :action do %>
+  <% if self_hosted? %>
+    <p>To fix this issue, you need to provide an API key for your exchange rate provider.</p>
+
+    <p>Currently, we support <%= link_to "Synth Finance", "https://synthfinance.com", target: "_blank" %>, so you need
+      to
+      get a free API key from the link provided.</p>
+
+    <p>Once you have your API key, paste it below to configure it.</p>
+
+    <%= styled_form_with model: @issue, url: issue_exchange_rate_provider_missing_path(@issue), method: :patch, class: "space-y-3" do |form| %>
+      <%= form.text_field :synth_api_key, label: "Synth API Key", placeholder: "Synth API Key", type: "password", class: "w-full", value: Setting.synth_api_key %>
+      <%= form.submit "Save and Re-Sync Account", class: "btn-primary" %>
+    <% end %>
+  <% else %>
+    <p>Please contact the Maybe team.</p>
+  <% end %>
+<% end %>

app/views/issue/exchange_rates_missings/show.html.erb

@@ -0,0 +1,22 @@
+<%= content_for :title, @issue.title %>
+
+<%= content_for :description do %>
+  <p>Some exchange rates are missing for this account.</p>
+
+  <pre><code><%= JSON.pretty_generate(@issue.data) %></code></pre>
+<% 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>
+<% end %>

app/views/issue/unknowns/show.html.erb

@@ -0,0 +1,23 @@
+<%= content_for :title, @issue.title %>
+
+<%= content_for :description do %>
+  <p>An unknown issue has occurred.</p>
+
+  <pre><code><%= JSON.pretty_generate(@issue.data || "No data provided for this issue") %></code></pre>
+<% end %>
+
+<%= content_for :action do %>
+  <p>There is no fix for this issue yet.</p>
+
+  <p>Maybe is in active development and we value your feedback. There are a couple ways you can report this issue to
+    help us make Maybe better:</p>
+
+  <ul>
+    <li>Post in our <%= link_to "Discord server", "https://link.maybe.co/discord", target: "_blank" %></li>
+    <li>Open an issue on
+      our <%= link_to "Github repository", "https://github.com/maybe-finance/maybe/issues", target: "_blank" %></li>
+  </ul>
+
+  <p>If there is data shown in the code block above that you think might be helpful, please include it in your
+    report.</p>
+<% end %>

app/views/issues/_issue.html.erb

@@ -0,0 +1,14 @@
+<%# locals: (issue:) %>
+
+<% priority_class = issue.critical? || issue.error? ? "bg-error/5" : "bg-warning/5" %>
+
+<%= 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">
+    <%= 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 } %>
+  </div>
+<% end %>

app/views/layouts/issues.html.erb

@@ -0,0 +1,15 @@
+<%= drawer do %>
+  <article class="prose">
+    <%= tag.h2 do %>
+      <%= yield :title %>
+    <% end %>
+
+    <%= tag.h3 t(".description") %>
+    <%= yield :description %>
+
+    <%= tag.h3 t(".action") %>
+    <%= yield :action %>
+  </article>
+<% end %>
+
+<%= render template: "layouts/application" %>