Fix foreign account sync crash (#794)

* Fix foreign account sync crash

* Refactor synth provider and show UI error if not configured

* Generate error message on missing exchange rates while converting balances

* Ignore sync messaged in i18n-tasks unused

* Generate missing exchange rate error during entry normalization

* Update alert classes

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
@@ -43,34 +42,50 @@ class Account::Balance::Calculator
 
     private
       def convert_balances_to_family_currency
-        rates = ExchangeRate.get_rate_series(
+        rates = ExchangeRate.get_rates(
           @account.currency,
           @account.family.currency,
           @calc_start_date..Date.current
         ).to_a
 
-        @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?
-          converted_balance = balance[:balance] * rate.rate
+        # Abort conversion if some required rates are missing
+        if rates.length != @daily_balances.length
+          @errors << :sync_message_missing_rates
+          return []
+        end
+
+        @daily_balances.map.with_index do |balance, index|
+          converted_balance = balance[:balance] * rates[index].rate
           { date: balance[:date], balance: converted_balance, currency: @account.family.currency, updated_at: Time.current }
         end
       end
 
       # For calculation, all transactions and valuations need to be normalized to the same currency (the account's primary currency)
       def normalize_entries_to_account_currency(entries, value_key)
-        entries.map do |entry|
-          currency = entry.currency
-          date = entry.date
-          value = entry.send(value_key)
+        grouped_entries = entries.group_by(&:currency)
+        normalized_entries = []
 
+        grouped_entries.each do |currency, entries|
           if currency != @account.currency
-            value = ExchangeRate.convert(value:, from: currency, to: @account.currency, date:)
-            currency = @account.currency
+            dates = entries.map(&:date).uniq
+            rates = ExchangeRate.get_rates(currency, @account.currency, dates).to_a
+            if rates.length != dates.length
+              @errors << :sync_message_missing_rates
+            else
+              entries.each do |entry|
+                ## There can be several entries on the same date so we cannot rely on indeces
+                rate = rates.find { |rate| rate.date == entry.date }
+                value = entry.send(value_key)
+                value *= rate.rate
+                normalized_entries << entry.attributes.merge(value_key.to_s => value, "currency" => currency)
+              end
+            end
+          else
+            normalized_entries.concat(entries)
           end
-
-          entry.attributes.merge(value_key.to_s => value, "currency" => currency)
         end
+
+        normalized_entries
       end
 
       def normalized_valuations
@@ -92,8 +107,8 @@ class Account::Balance::Calculator
           return @account.balance_on(@calc_start_date)
         end
 
-        oldest_valuation_date = normalized_valuations.first&.dig("date")
-        oldest_transaction_date = normalized_transactions.first&.dig("date")
+        oldest_valuation_date = normalized_valuations.first&.date
+        oldest_transaction_date = normalized_transactions.first&.date
         oldest_entry_date = [ oldest_valuation_date, oldest_transaction_date ].compact.min
 
         if oldest_entry_date.present? && oldest_entry_date == oldest_valuation_date

app/models/account/syncable.rb

@@ -18,10 +18,10 @@ module Account::Syncable
     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")
-    Rails.logger.error("Failed to sync account #{id}: #{e.message}")
+    update!(status: "error", sync_errors: [ :sync_message_unknown_error ])
+    logger.error("Failed to sync account #{id}: #{e.message}")
   end
 
   def can_sync?
@@ -77,8 +77,7 @@ module Account::Syncable
       next if existing_rates_set.include?([ rc_from, rc_to, rc_date.to_s ])
 
       logger.info "Fetching exchange rate from provider for account #{self.name}: #{self.id} (#{rc_from} to #{rc_to} on #{rc_date})"
-      rate = ExchangeRate.find_rate_or_fetch from: rc_from, to: rc_to, date: rc_date
-      ExchangeRate.create! base_currency: rc_from, converted_currency: rc_to, date: rc_date, rate: rate if rate
+      ExchangeRate.find_rate_or_fetch from: rc_from, to: rc_to, date: rc_date
     end
 
     nil

app/models/exchange_rate.rb

@@ -12,11 +12,11 @@ class ExchangeRate < ApplicationRecord
     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)
-      where(base_currency: from, converted_currency: to, date: date_range).order(:date)
+    def get_rates(from, to, dates)
+      where(base_currency: from, converted_currency: to, date: dates).order(:date)
     end
 
     def convert(value:, from:, to:, date:)

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

@@ -5,6 +5,10 @@ class Provider::Synth
     @api_key = api_key || ENV["SYNTH_API_KEY"]
   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|