fix(lunchflow): mark sync unhealthy when importer reports fetch failures (#1796) (#1873)

* fix(lunchflow): mark sync unhealthy when importer reports fetch failures (#1796)

`LunchflowItem::Syncer#perform_sync` called
`lunchflow_item.import_latest_lunchflow_data` but threw the result away
and then ran `collect_health_stats(sync, errors: nil)`. The importer
already catches per-account 429/500 fetch errors, bumps a
`transactions_failed` counter, and returns `success: false` — but the
syncer never inspected the return value, so the parent sync was marked
completed/green even when zero transactions were imported because every
fetch had been rate-limited.

Capture the importer result and translate any
`accounts_failed` / `transactions_failed` / `error` fields into the
`{ message:, category: }` error shape `collect_health_stats` expects.
The exception-path `rescue` branch is unchanged.

Closes #1796

* test(lunchflow): i18n the new health messages + add syncer invariant tests (#1796)

Two pieces of follow-up feedback:

- @coderabbitai + @JSONbored: the three new operator-facing strings should
  go through I18n.t. Add keys under provider_warnings.lunchflow_*
  (matching the existing provider_warnings.limited_investment_data
  shape) and use Rails pluralization for the count-bearing entries.
  Other locales follow the repo's normal translation flow.

- @jjmata + @JSONbored: add tests for the invariants. New
  LunchflowItem::SyncerTest covers:
    * successful import → sync healthy
    * accounts_failed positive → sync unhealthy with localized message
    * transactions_failed positive → sync unhealthy with localized message
    * both counters positive → both error entries recorded in order
    * sync raises → sync_error category + reraise (existing rescue branch)

@jjmata also asked to confirm the importer contract: LunchflowItem::Importer#import
returns 'success: accounts_failed == 0 && transactions_failed == 0' (see
importer.rb), so the early 'return [] if import_result[:success]' guard
is safe — success is never true while either counter is positive.

---------

Co-authored-by: jeffrey701 <jeffrey701@users.noreply.github.com>

app/models/lunchflow_item/syncer.rb

@@ -10,7 +10,7 @@ class LunchflowItem::Syncer
   def perform_sync(sync)
     # Phase 1: Import data from Lunchflow API
     sync.update!(status_text: "Importing accounts from Lunchflow...") if sync.respond_to?(:status_text)
-    lunchflow_item.import_latest_lunchflow_data
+    import_result = lunchflow_item.import_latest_lunchflow_data
 
     # Phase 2: Collect setup statistics using shared concern
     sync.update!(status_text: "Checking account configuration...") if sync.respond_to?(:status_text)
@@ -54,8 +54,10 @@ class LunchflowItem::Syncer
       Rails.logger.info "LunchflowItem::Syncer - No linked accounts to process"
     end
 
-    # Mark sync health
-    collect_health_stats(sync, errors: nil)
+    # Mark sync health — surface importer failures so the sync isn't reported
+    # as completed when the upstream Lunchflow API rejected fetches (e.g.
+    # 429 rate-limit responses wrapped as 500s, transient network errors).
+    collect_health_stats(sync, errors: import_failures_as_errors(import_result).presence)
   rescue => e
     collect_health_stats(sync, errors: [ { message: e.message, category: "sync_error" } ])
     raise
@@ -67,6 +69,37 @@ class LunchflowItem::Syncer
 
   private
 
+    # Translate the LunchflowItem::Importer result hash into the error-shape
+    # collect_health_stats expects. Returns [] for a successful import.
+    def import_failures_as_errors(import_result)
+      return [] unless import_result.is_a?(Hash)
+      return [] if import_result[:success]
+
+      errors = []
+      accounts_failed = import_result[:accounts_failed].to_i
+      transactions_failed = import_result[:transactions_failed].to_i
+
+      if accounts_failed.positive?
+        errors << {
+          message: I18n.t("provider_warnings.lunchflow_accounts_failed", count: accounts_failed),
+          category: "lunchflow_import"
+        }
+      end
+      if transactions_failed.positive?
+        errors << {
+          message: I18n.t("provider_warnings.lunchflow_transactions_failed", count: transactions_failed),
+          category: "lunchflow_import"
+        }
+      end
+      if errors.empty? && import_result[:error].present?
+        errors << {
+          message: I18n.t("provider_warnings.lunchflow_import_error", error: import_result[:error]),
+          category: "lunchflow_import"
+        }
+      end
+      errors
+    end
+
     # Collects a data quality warning if any linked accounts are investment or crypto accounts.
     # Lunchflow cannot provide activity labels (Buy, Sell, Dividend, etc.) for investment transactions,
     # which may affect budget accuracy.

config/locales/models/provider_warnings/en.yml

@@ -2,3 +2,10 @@
 en:
   provider_warnings:
     limited_investment_data: "Investment data from this provider is limited. Activity labels (Buy, Sell, Dividend) are not available, which may affect budget accuracy. Consider creating rules to exclude or categorize investment transactions."
+    lunchflow_accounts_failed:
+      one: "Lunchflow import: %{count} account failed to update"
+      other: "Lunchflow import: %{count} accounts failed to update"
+    lunchflow_transactions_failed:
+      one: "Lunchflow import: transaction fetch failed for %{count} account"
+      other: "Lunchflow import: transaction fetch failed for %{count} accounts"
+    lunchflow_import_error: "Lunchflow import: %{error}"

test/models/lunchflow_item/syncer_test.rb

@@ -0,0 +1,115 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class LunchflowItem::SyncerTest < ActiveSupport::TestCase
+  setup do
+    @lunchflow_item = lunchflow_items(:one)
+    @syncer = LunchflowItem::Syncer.new(@lunchflow_item)
+  end
+
+  test "marks sync healthy when import succeeds with no failures" do
+    sync = recording_sync
+    @lunchflow_item.expects(:import_latest_lunchflow_data).returns(
+      success: true,
+      accounts_failed: 0,
+      transactions_failed: 0
+    )
+
+    @syncer.perform_sync(sync)
+
+    assert_equal 0, sync.sync_stats["total_errors"]
+    assert_nil sync.sync_stats["errors"]
+  end
+
+  test "marks sync unhealthy when importer reports accounts_failed" do
+    sync = recording_sync
+    @lunchflow_item.expects(:import_latest_lunchflow_data).returns(
+      success: false,
+      accounts_failed: 2,
+      transactions_failed: 0
+    )
+
+    @syncer.perform_sync(sync)
+
+    assert_equal 1, sync.sync_stats["total_errors"]
+    assert_equal(
+      [ I18n.t("provider_warnings.lunchflow_accounts_failed", count: 2) ],
+      sync.sync_stats["errors"].map { |e| e["message"] }
+    )
+    assert_equal "lunchflow_import", sync.sync_stats["errors"].first["category"]
+  end
+
+  test "marks sync unhealthy when importer reports transactions_failed" do
+    sync = recording_sync
+    @lunchflow_item.expects(:import_latest_lunchflow_data).returns(
+      success: false,
+      accounts_failed: 0,
+      transactions_failed: 3
+    )
+
+    @syncer.perform_sync(sync)
+
+    assert_equal 1, sync.sync_stats["total_errors"]
+    assert_equal(
+      [ I18n.t("provider_warnings.lunchflow_transactions_failed", count: 3) ],
+      sync.sync_stats["errors"].map { |e| e["message"] }
+    )
+  end
+
+  test "records both failure categories when accounts and transactions both fail" do
+    sync = recording_sync
+    @lunchflow_item.expects(:import_latest_lunchflow_data).returns(
+      success: false,
+      accounts_failed: 1,
+      transactions_failed: 4
+    )
+
+    @syncer.perform_sync(sync)
+
+    assert_equal 2, sync.sync_stats["total_errors"]
+    assert_equal(
+      [
+        I18n.t("provider_warnings.lunchflow_accounts_failed", count: 1),
+        I18n.t("provider_warnings.lunchflow_transactions_failed", count: 4)
+      ],
+      sync.sync_stats["errors"].map { |e| e["message"] }
+    )
+  end
+
+  test "captures sync_error category and reraises when import raises" do
+    sync = recording_sync
+    @lunchflow_item.expects(:import_latest_lunchflow_data).raises(StandardError, "boom")
+
+    assert_raises(StandardError) do
+      @syncer.perform_sync(sync)
+    end
+
+    assert_equal 1, sync.sync_stats["total_errors"]
+    error = sync.sync_stats["errors"].first
+    assert_equal "boom", error["message"]
+    assert_equal "sync_error", error["category"]
+  end
+
+  private
+
+    def recording_sync
+      Class.new do
+        attr_accessor :sync_stats, :status_text
+        attr_reader :updates, :window_start_date, :window_end_date
+
+        def initialize
+          @sync_stats = {}
+          @updates = []
+          @window_start_date = nil
+          @window_end_date = nil
+        end
+
+        def update!(attributes)
+          @updates << attributes
+          self.sync_stats = attributes[:sync_stats] if attributes.key?(:sync_stats)
+          self.status_text = attributes[:status_text] if attributes.key?(:status_text)
+        end
+      end.new
+    end
+end