mirror of
https://github.com/we-promise/sure.git
synced 2026-04-08 06:44:52 +00:00
* 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>
96 lines
3.6 KiB
Ruby
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
|