Generate error message on missing exchange rates while converting bal…

…ances

app/models/account/balance/calculator.rb

@@ -1,11 +1,10 @@
 class Account::Balance::Calculator
     attr_reader :daily_balances, :errors, :warnings
 
-    @daily_balances = []
-    @errors = []
-    @warnings = []
-
     def initialize(account, options = {})
+      @daily_balances = []
+      @errors = []
+      @warnings = []
       @account = account
       @calc_start_date = [ options[:calc_start_date], @account.effective_start_date ].compact.max
     end
@@ -49,6 +48,12 @@ def convert_balances_to_family_currency
           @calc_start_date..Date.current
         ).to_a
 
+        # Abort conversion if some required rates are missing
+        if rates.length != @daily_balances.length
+          @errors << :missing_rates
+          return []
+        end
+
         @daily_balances.map do |balance|
           rate = rates.find { |rate| rate.date == balance[:date] }
           raise "Rate for #{@account.currency} to #{@account.family.currency} on #{balance[:date]} not found" if rate.nil?

app/models/account/syncable.rb

@@ -8,12 +8,6 @@ def sync_later(start_date = nil)
   def sync(start_date = nil)
     update!(status: "syncing")
 
-    if self.foreign_currency? && !ExchangeRate.exchange_rates_provider.initialized?
-      logger.error("Cancelling sync of foreign account #{id}: No exchange rate provider ready")
-      update!(status: "error")
-      return
-    end
-
     sync_exchange_rates
 
     calc_start_date = start_date - 1.day if start_date.present? && self.balance_on(start_date - 1.day).present?
@@ -24,14 +18,13 @@ def sync(start_date = nil)
     self.balances.where("date < ?", effective_start_date).delete_all
     new_balance = calculator.daily_balances.select { |b| b[:currency] == self.currency }.last[:balance]
 
-    update!(status: "ok", last_sync_date: Date.today, balance: new_balance)
+    update!(status: "ok", last_sync_date: Date.today, balance: new_balance, sync_errors: calculator.errors, sync_warnings: calculator.warnings)
   rescue => e
     update!(status: "error")
     logger.error("Failed to sync account #{id}: #{e.message}")
   end
 
   def can_sync?
-    return false if self.foreign_currency? && !ExchangeRate.exchange_rates_provider.initialized?
     # Skip account sync if account is not active or the sync process is already running
     return false unless is_active
     return false if syncing?

app/models/concerns/providable.rb

@@ -7,7 +7,7 @@ module Providable
 
   class_methods do
     def exchange_rates_provider
-      Provider::Synth
+      Provider::Synth.new
     end
 
     def git_repository_provider

app/models/exchange_rate.rb

@@ -12,7 +12,7 @@ def find_rate(from:, to:, date:)
     end
 
     def find_rate_or_fetch(from:, to:, date:)
-      find_rate(from:, to:, date:) || fetch_rate_from_provider(from:, to:, date:).tap(&:save!)
+      find_rate(from:, to:, date:) || fetch_rate_from_provider(from:, to:, date:)&.tap(&:save!)
     end
 
     def get_rate_series(from, to, date_range)

app/models/exchange_rate/provided.rb

@@ -5,6 +5,8 @@ module ExchangeRate::Provided
   class_methods do
     private
       def fetch_rate_from_provider(from:, to:, date:)
+        return nil unless exchange_rates_provider.configured?
+
         response = exchange_rates_provider.fetch_exchange_rate \
           from: Money::Currency.new(from).iso_code,
           to: Money::Currency.new(to).iso_code,

app/models/provider/synth.rb

@@ -1,62 +1,55 @@
 class Provider::Synth
-  class << self
-    include Retryable
+  include Retryable
 
-    attr_accessor :configuration
-
-    def initialized?
-      configuration.api_key.present?
-    end
+  def initialize(api_key = ENV["SYNTH_API_KEY"])
+    @api_key = api_key || ENV["SYNTH_API_KEY"]
+  end
 
-    def configure
-      self.configuration ||= Configuration.new
-      yield(configuration)
-    end
+  def configured?
+    @api_key.present?
+  end
 
-    def fetch_exchange_rate(from:, to:, date:)
-      retrying Provider::Base.known_transient_errors do |on_last_attempt|
-        response = Faraday.get("#{base_url}/rates/historical") do |req|
-          req.headers["Authorization"] = "Bearer #{configuration.api_key}"
-          req.params["date"] = date.to_s
-          req.params["from"] = from
-          req.params["to"] = to
-        end
+  def fetch_exchange_rate(from:, to:, date:)
+    retrying Provider::Base.known_transient_errors do |on_last_attempt|
+      response = Faraday.get("#{base_url}/rates/historical") do |req|
+        req.headers["Authorization"] = "Bearer #{api_key}"
+        req.params["date"] = date.to_s
+        req.params["from"] = from
+        req.params["to"] = to
+      end
 
-        if response.success?
+      if response.success?
+        ExchangeRateResponse.new \
+          rate: JSON.parse(response.body).dig("data", "rates", to),
+          success?: true,
+          raw_response: response
+      else
+        if on_last_attempt
           ExchangeRateResponse.new \
-            rate: JSON.parse(response.body).dig("data", "rates", to),
-            success?: true,
+            success?: false,
+            error: build_error(response),
             raw_response: response
         else
-          if on_last_attempt
-            ExchangeRateResponse.new \
-              success?: false,
-              error: build_error(response),
-              raw_response: response
-          else
-            raise build_error(response)
-          end
+          raise build_error(response)
         end
       end
     end
+  end
 
-    private
-      class Configuration
-        attr_accessor :api_key
-      end
+  private
+    attr_reader :api_key
 
-      ExchangeRateResponse = Struct.new :rate, :success?, :error, :raw_response, keyword_init: true
+    ExchangeRateResponse = Struct.new :rate, :success?, :error, :raw_response, keyword_init: true
 
-      def base_url
-        "https://api.synthfinance.com"
-      end
+    def base_url
+      "https://api.synthfinance.com"
+    end
 
-      def build_error(response)
-        Provider::Base::ProviderError.new(<<~ERROR)
-          Failed to fetch exchange rate from #{self}
-            Status: #{response.status}
-            Body: #{response.body.inspect}
-        ERROR
-      end
-  end
+    def build_error(response)
+      Provider::Base::ProviderError.new(<<~ERROR)
+        Failed to fetch exchange rate from #{self.class}
+          Status: #{response.status}
+          Body: #{response.body.inspect}
+      ERROR
+    end
 end

app/views/accounts/show.html.erb

@@ -41,8 +41,11 @@
   <%= turbo_frame_tag "sync_message" do %>
     <%= render partial: "accounts/sync_message", locals: { is_syncing: @account.syncing? } %>
   <% end %>
-  <% if @account.foreign_currency? && !ExchangeRate.exchange_rates_provider.initialized? %>
-    <%= render partial: "shared/alert", locals: { type: "error", content: t(".no_exchange_rate_provider") } %>
+  <% @account.sync_errors.each do |message| %>
+    <%= render partial: "shared/alert", locals: { type: "error", content: t("." + message) } %>
+  <% end %>
+  <% @account.sync_warnings.each do |message| %>
+    <%= render partial: "shared/alert", locals: { type: "warning", content: t("." + message) } %>
   <% end %>
   <div class="bg-white shadow-xs rounded-xl border border-alpha-black-25 rounded-lg">
     <div class="p-4 flex justify-between">

app/views/shared/_alert.html.erb

@@ -1,5 +1,5 @@
 <%# locals: (type: "error", content: "") -%>
-<% color = type == "error" ? "red" : "blue" %>
+<% color = type == "error" ? "red" : "yellow" %>
 <%= content_tag :div,
   class: "flex justify-between bg-#{color}-50 rounded-xl p-3",
   data: {controller: "element-removal" },

config/locales/views/account/en.yml

@@ -35,8 +35,7 @@ en:
         /> <p>After deletion, there is no way you'll be able to restore the account
         information because you'll need to add it as a new account.</p>"
       confirm_title: Delete account?
-      no_exchange_rate_provider: This account cannot be synced at the moment as there
-        is no exchange rate provider configured.
+      missing_rates: Since exchange rates haven't been synced, balance graphs may not reflect accurate values.
     summary:
       new: New account
     sync:

db/migrate/20240524203959_change_account_error_columns_default.rb

@@ -0,0 +1,8 @@
+class ChangeAccountErrorColumnsDefault < ActiveRecord::Migration[7.2]
+  def up
+    change_column_default :accounts, :sync_warnings, from: "[]", to: []
+    change_column_default :accounts, :sync_errors, from: "[]", to: []
+    Account.update_all(sync_warnings: [])
+    Account.update_all(sync_errors: [])
+  end
+end

test/models/exchange_rate_test.rb

@@ -7,36 +7,44 @@ class ExchangeRateTest < ActiveSupport::TestCase
   end
 
   test "fetch rate from provider when it's not found in db" do
-    ExchangeRate
-      .expects(:fetch_rate_from_provider)
-      .returns(ExchangeRate.new(base_currency: "USD", converted_currency: "MXN", rate: 1.0, date: Date.current))
+    with_env_overrides SYNTH_API_KEY: "true" do
+      ExchangeRate
+        .expects(:fetch_rate_from_provider)
+        .returns(ExchangeRate.new(base_currency: "USD", converted_currency: "MXN", rate: 1.0, date: Date.current))
 
-    ExchangeRate.find_rate_or_fetch from: "USD", to: "MXN", date: Date.current
+      ExchangeRate.find_rate_or_fetch from: "USD", to: "MXN", date: Date.current
+    end
   end
 
   test "provided rates are saved to the db" do
-    VCR.use_cassette "synth_exchange_rate" do
-      assert_difference "ExchangeRate.count", 1 do
-        ExchangeRate.find_rate_or_fetch from: "USD", to: "MXN", date: Date.current
+    with_env_overrides SYNTH_API_KEY: "true" do
+      VCR.use_cassette "synth_exchange_rate" do
+        assert_difference "ExchangeRate.count", 1 do
+          ExchangeRate.find_rate_or_fetch from: "USD", to: "MXN", date: Date.current
+        end
       end
     end
   end
 
   test "retrying, then raising on provider error" do
-    Faraday.expects(:get).returns(OpenStruct.new(success?: false)).times(3)
+    with_env_overrides SYNTH_API_KEY: "true" do
+      Faraday.expects(:get).returns(OpenStruct.new(success?: false)).times(3)
 
-    error = assert_raises Provider::Base::ProviderError do
-      ExchangeRate.find_rate_or_fetch from: "USD", to: "MXN", date: Date.current
-    end
+      error = assert_raises Provider::Base::ProviderError do
+        ExchangeRate.find_rate_or_fetch from: "USD", to: "MXN", date: Date.current
+      end
 
-    assert_match "Failed to fetch exchange rate from Provider::Synth", error.message
+      assert_match "Failed to fetch exchange rate from Provider::Synth", error.message
+    end
   end
 
   test "retrying, then raising on network error" do
-    Faraday.expects(:get).raises(Faraday::TimeoutError).times(3)
+    with_env_overrides SYNTH_API_KEY: "true" do
+      Faraday.expects(:get).raises(Faraday::TimeoutError).times(3)
 
-    assert_raises Faraday::TimeoutError do
-      ExchangeRate.find_rate_or_fetch from: "USD", to: "MXN", date: Date.current
+      assert_raises Faraday::TimeoutError do
+        ExchangeRate.find_rate_or_fetch from: "USD", to: "MXN", date: Date.current
+      end
     end
   end
 end

test/models/provider/synth_test.rb

@@ -4,6 +4,6 @@ class Provider::SynthTest < ActiveSupport::TestCase
   include ExchangeRateProviderInterfaceTest
 
   setup do
-    @subject = Provider::Synth
+    @subject = Provider::Synth.new
   end
 end

test/test_helper.rb

@@ -46,10 +46,6 @@ class TestCase
       end
     end
 
-    Provider::Synth.configure do |config|
-      config.api_key = "api_key"
-    end
-
     # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.
     fixtures :all