Files
sure/app/controllers/concerns/simplefin_items/maps_helper.rb
LPW 4d3d9d10df Address remaining CodeRabbit comments from PR #267 #351 (#451)
* Address remaining CodeRabbit comments from PR #267

This commit addresses the remaining unresolved code review comments:

1. Fix down migration in drop_was_merged_from_transactions.rb
   - Add null: false, default: false constraints to match original column
   - Ensures proper rollback compatibility

2. Fix bare rescue in maps_helper.rb compute_duplicate_only_flag
   - Replace bare rescue with rescue StandardError => e
   - Add proper logging for debugging
   - Follows Ruby best practices by being explicit about exception handling

These changes improve code quality and follow Rails/Ruby best practices.

* Refactor `SimplefinItemsController` and add tests for balances sync and account relinking behavior

- Replaced direct sync execution with `SyncJob` for asynchronous handling of balances sync.
- Updated account relinking logic to prevent disabling accounts with other active provider links.
- Removed unused `compute_relink_candidates` method.
- Added tests to verify `balances` action enqueues `SyncJob` and relinking respects account-provider relationships.

* Refactor balances sync to use runtime-only `balances_only` flag

- Replaced persistent `sync_stats` usage with runtime `balances_only?` predicate via `define_singleton_method`.
- Updated `SimplefinItemsController` `balances` action to pass `balances_only` flag to `SyncJob`.
- Enhanced `SyncJob` to attach transient `balances_only?` flag for execution.
- Adjusted `SimplefinItem::Syncer` logic to rely on the runtime `balances_only?` method.
- Updated controller tests to validate runtime flag usage in `SyncJob`.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Josh Waldrep <joshua.waldrep5+github@gmail.com>
2025-12-15 09:47:16 +01:00

96 lines
3.6 KiB
Ruby

# frozen_string_literal: true
module SimplefinItems
module MapsHelper
extend ActiveSupport::Concern
# Build per-item maps consumed by the simplefin_item partial.
# Accepts a single SimplefinItem or a collection.
def build_simplefin_maps_for(items)
items = Array(items).compact
return if items.empty?
@simplefin_sync_stats_map ||= {}
@simplefin_has_unlinked_map ||= {}
@simplefin_unlinked_count_map ||= {}
@simplefin_duplicate_only_map ||= {}
@simplefin_show_relink_map ||= {}
# Batch-check if ANY family has manual accounts (same result for all items from same family)
family_ids = items.map { |i| i.family_id }.uniq
families_with_manuals = Account
.visible_manual
.where(family_id: family_ids)
.distinct
.pluck(:family_id)
.to_set
# Batch-fetch unlinked counts for all items in one query
unlinked_counts = SimplefinAccount
.where(simplefin_item_id: items.map(&:id))
.left_joins(:account, :account_provider)
.where(accounts: { id: nil }, account_providers: { id: nil })
.group(:simplefin_item_id)
.count
items.each do |item|
# Latest sync stats (avoid N+1; rely on includes(:syncs) where appropriate)
latest_sync = if item.syncs.loaded?
item.syncs.max_by(&:created_at)
else
item.syncs.ordered.first
end
stats = (latest_sync&.sync_stats || {})
@simplefin_sync_stats_map[item.id] = stats
# Whether the family has any manual accounts available to link (from batch query)
@simplefin_has_unlinked_map[item.id] = families_with_manuals.include?(item.family_id)
# Count from batch query (defaults to 0 if not found)
@simplefin_unlinked_count_map[item.id] = unlinked_counts[item.id] || 0
# Whether all reported errors for this item are duplicate-account warnings
@simplefin_duplicate_only_map[item.id] = compute_duplicate_only_flag(stats)
# Compute CTA visibility: show relink only when there are zero unlinked SFAs,
# there exist manual accounts to link, and the item has at least one SFA
begin
unlinked_count = @simplefin_unlinked_count_map[item.id] || 0
manuals_exist = @simplefin_has_unlinked_map[item.id]
sfa_any = if item.simplefin_accounts.loaded?
item.simplefin_accounts.any?
else
item.simplefin_accounts.exists?
end
@simplefin_show_relink_map[item.id] = (unlinked_count.to_i == 0 && manuals_exist && sfa_any)
rescue StandardError => e
Rails.logger.warn("SimpleFin card: CTA computation failed for item #{item.id}: #{e.class} - #{e.message}")
@simplefin_show_relink_map[item.id] = false
end
end
# Ensure maps are hashes even when items empty
@simplefin_sync_stats_map ||= {}
@simplefin_has_unlinked_map ||= {}
@simplefin_unlinked_count_map ||= {}
@simplefin_duplicate_only_map ||= {}
@simplefin_show_relink_map ||= {}
end
private
def compute_duplicate_only_flag(stats)
errs = Array(stats && stats["errors"]).map do |e|
if e.is_a?(Hash)
e["message"] || e[:message]
else
e.to_s
end
end
errs.present? && errs.all? { |m| m.to_s.downcase.include?("duplicate upstream account detected") }
rescue StandardError => e
Rails.logger.warn("SimpleFin maps: compute_duplicate_only_flag failed: #{e.class} - #{e.message}")
false
end
end
end