Simplefin sync improvements #240
Conversation
WalkthroughAdds pre-discovery account fetching and rate-limit detection to SimpleFin syncs, surfaces a rate-limited message in the UI, delegates institution metadata to account-level data, relaxes SimpleFin item institution column constraints and indexes, wraps dashboard broadcasts in rescue blocks, and adds a debug rake task. Changes
Sequence Diagram(s)sequenceDiagram
actor Sync as Sync Process
participant Importer as SimpleFin::Importer
participant API as SimpleFin API
participant Handler as Error Handler
participant DB as Local DB
Sync->>Importer: import_with_chunked_history / import_regular_sync
Note over Importer: Pre-discovery phase (start_date: nil)
Importer->>API: fetch_accounts_data(start_date: nil)
API-->>Importer: accounts (empty / populated)
alt No accounts found
Importer->>API: fetch_accounts_data(start_date: nil, pending: true)
API-->>Importer: pending accounts
end
alt Discovery yields accounts
Importer->>DB: upsert_simplefin_snapshot!
Importer->>Importer: import accounts
Note over Importer: Main sync (transactions/holdings)
Importer->>API: fetch_accounts_data(start_date: determined_date)
API-->>Importer: transactions/holdings
Importer->>DB: import data
else Rate limit / API error
Importer->>Handler: handle_errors()
Handler-->>Importer: raise RateLimitedError / other error
end
sequenceDiagram
participant Event as sync_complete_event
participant Dashboard as ActionCable / View Renderer
participant Logger as Rails.logger
Event->>Dashboard: broadcast_replace(:balance_sheet)
alt Success
Dashboard-->>Event: rendered
else Rendering fails
Dashboard-->>Logger: error logged
Logger-->>Event: suppressed (rescue)
end
Event->>Dashboard: broadcast_replace(:net_worth_chart)
alt Success
Dashboard-->>Event: rendered
else Rendering fails
Dashboard-->>Logger: error logged
Logger-->>Event: suppressed (rescue)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/models/family/sync_complete_event.rb (1)
9-30: LGTM! Consider logging the full backtrace for debugging.The defensive error handling correctly prevents UI rendering issues from failing the overall sync job, which aligns with the PR objectives. The error messages provide clear context about which broadcast failed.
To improve debugging in production, consider including the backtrace in the error logs:
rescue => e - Rails.logger.error("Family::SyncCompleteEvent balance_sheet broadcast failed: #{e.message}") + Rails.logger.error("Family::SyncCompleteEvent balance_sheet broadcast failed: #{e.message}\n#{e.backtrace.join("\n")}") end begin family.broadcast_replace( target: "net-worth-chart", partial: "pages/dashboard/net_worth_chart", locals: { balance_sheet: family.balance_sheet, period: Period.last_30_days } ) rescue => e - Rails.logger.error("Family::SyncCompleteEvent net_worth_chart broadcast failed: #{e.message}") + Rails.logger.error("Family::SyncCompleteEvent net_worth_chart broadcast failed: #{e.message}\n#{e.backtrace.join("\n")}") end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/assets/images/logomark-color.svgis excluded by!**/*.svg
📒 Files selected for processing (12)
app/controllers/oidc_accounts_controller.rb(3 hunks)app/models/family/sync_complete_event.rb(1 hunks)app/models/family/syncer.rb(1 hunks)app/models/oidc_identity.rb(1 hunks)app/models/simplefin_item.rb(2 hunks)app/models/simplefin_item/importer.rb(5 hunks)app/views/oidc_accounts/link.html.erb(1 hunks)app/views/simplefin_items/_simplefin_item.html.erb(1 hunks)db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb(1 hunks)db/schema.rb(3 hunks)lib/tasks/simplefin.rake(1 hunks)test/models/oidc_identity_test.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (27)
app/models/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-design.mdc)
Domain models should not call Provider::Registry directly; use a Provided concern within the model’s namespace to select providers and expose convenience methods
Use ActiveRecord validations for forms and complex domain constraints.
app/models/**/*.rb: Place business logic in POROs and model classes under app/models
Models should answer questions about themselves (e.g., prefer account.balance_series over service objects)
Implement complex validations and business logic with ActiveRecord validations
Model-level validations may mirror DB constraints but are not strictly required
Files:
app/models/family/syncer.rbapp/models/oidc_identity.rbapp/models/simplefin_item/importer.rbapp/models/simplefin_item.rbapp/models/family/sync_complete_event.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/models/family/syncer.rbapp/models/oidc_identity.rbapp/controllers/oidc_accounts_controller.rbapp/models/simplefin_item/importer.rbapp/models/simplefin_item.rbapp/models/family/sync_complete_event.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/models/family/syncer.rbapp/models/oidc_identity.rbapp/controllers/oidc_accounts_controller.rbapp/models/simplefin_item/importer.rbtest/models/oidc_identity_test.rbdb/migrate/20251025095800_relax_simplefin_item_institution_constraints.rbdb/schema.rbapp/models/simplefin_item.rbapp/models/family/sync_complete_event.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
app/models/family/syncer.rbapp/models/oidc_identity.rbapp/controllers/oidc_accounts_controller.rbapp/models/simplefin_item/importer.rbtest/models/oidc_identity_test.rbdb/migrate/20251025095800_relax_simplefin_item_institution_constraints.rbdb/schema.rbapp/models/simplefin_item.rbapp/models/family/sync_complete_event.rb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/models/family/syncer.rbapp/models/oidc_identity.rbapp/controllers/oidc_accounts_controller.rbapp/models/simplefin_item/importer.rbapp/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erbapp/models/simplefin_item.rbapp/models/family/sync_complete_event.rb
**/app/models/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Business logic should primarily reside in models; use concerns and POROs for organization.
Files:
app/models/family/syncer.rbapp/models/oidc_identity.rbapp/models/simplefin_item/importer.rbapp/models/simplefin_item.rbapp/models/family/sync_complete_event.rb
**/*.{rb,erb,haml,slim}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb,haml,slim}: UseCurrent.userfor the current user; do not usecurrent_user
UseCurrent.familyfor the current family; do not usecurrent_family
Ignore i18n methods; hardcode strings in English for now (do not useI18n.t,t, or similar)
Files:
app/models/family/syncer.rbapp/models/oidc_identity.rbapp/controllers/oidc_accounts_controller.rbapp/models/simplefin_item/importer.rbtest/models/oidc_identity_test.rbdb/migrate/20251025095800_relax_simplefin_item_institution_constraints.rbdb/schema.rbapp/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erbapp/models/simplefin_item.rbapp/models/family/sync_complete_event.rb
app/{helpers,models}/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, and dates on the server side (Ruby) before sending to the client
Files:
app/models/family/syncer.rbapp/models/oidc_identity.rbapp/models/simplefin_item/importer.rbapp/models/simplefin_item.rbapp/models/family/sync_complete_event.rb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/models/family/syncer.rbapp/models/oidc_identity.rbapp/controllers/oidc_accounts_controller.rbapp/models/simplefin_item/importer.rbapp/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erbapp/models/simplefin_item.rbapp/models/family/sync_complete_event.rb
**/app/controllers/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Controllers should be skinny; business logic should go in models (
app/models/), not inapp/services/.
Files:
app/controllers/oidc_accounts_controller.rb
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
Name Minitest files with *_test.rb and mirror the app/ structure under test/
test/**/*_test.rb: ALWAYS use Minitest + fixtures + Mocha for tests; NEVER RSpec or FactoryBot.
Use Mocha for mocking in tests when necessary.
Use VCR for external API tests.
test/**/*_test.rb: Always use Minitest for tests; do not use RSpec
Do not use factories (e.g., FactoryBot) in tests; rely on fixtures instead
For tests needing many records, use Rails helpers to construct data and inline the creation in the test
Only write tests for critical and important code paths
Avoid tests that merely verify framework/ActiveRecord behavior
Test boundaries correctly: for queries, assert returned values; for commands, assert collaborators are called with correct params
Never test the implementation details of one class in another class’s test suite
Use the mocha gem for stubs and mocks
Prefer OpenStruct for mock instances; use a mock class for complex cases
Only mock what’s necessary; don’t mock return values unless they are under test
Files:
test/models/oidc_identity_test.rb
**/test/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/test/**/*.rb: ALWAYS use Minitest and fixtures for Ruby tests; NEVER RSpec or factories.
Only test critical and important code paths; system tests should be used sparingly for critical user flows.
Files:
test/models/oidc_identity_test.rb
db/migrate/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place database migrations under db/migrate/
Simple validations like null checks and unique constraints should be handled by database constraints in migration files.
ActiveRecord migrations must inherit from
ActiveRecord::Migration[7.2]; do not use version 8.0
Files:
db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb
db/migrate/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Enforce simple validations in the database (NULL constraints, unique indexes, etc.)
Files:
db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb
db/schema.rb
📄 CodeRabbit inference engine (.cursor/rules/project-design.mdc)
Treat db/schema.rb as the authoritative source of truth for model relationships
Files:
db/schema.rb
app/views/**/*.erb
📄 CodeRabbit inference engine (AGENTS.md)
app/views/**/*.erb: Keep heavy logic out of ERB views; prefer helpers/components instead
ERB templates are linted by erb-lint per .erb_lint.ymlAlways use the icon helper (icon) for icons; never call lucide_icon directly
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
app/views/**/*.html.*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.html.*: Use partials only for simple, context-specific, mostly static HTML content.
Prefer semantic HTML; use Turbo Frames where possible instead of client-side solutions.
Use query params for state, not localStorage or sessionStorage.
Always perform server-side formatting for currencies, numbers, and dates.
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
**/*.{html,erb,slim,js,jsx,ts,tsx,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use functional design tokens (e.g., text-primary, bg-container) from the design system; do not use raw colors or ad-hoc classes.
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
**/app/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Always use the
iconhelper for icons in templates; never uselucide_icondirectly.
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
**/app/views/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/views/**/*.erb: Keep domain logic OUT of view templates; logic belongs in component files, not templates.
Use partials only for primarily static, simple HTML with minimal logic, and only when not likely to be reused.
Prefer native HTML over JavaScript components for UI elements (e.g., use<dialog>,<details><summary>).
Leverage Turbo frames for page sections over client-side JavaScript solutions.
Use query params (not localStorage/sessions) for client state management.
Perform server-side formatting for currencies, numbers, and dates in templates.
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
app/views/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/views/**/*.html.erb: Prefer native HTML elements (e.g., ,) over JS-based components
Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Prefer native client-side form validation when possible
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
{app/javascript/controllers/**/*.{js,ts},app/views/**/*.erb,app/components/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Use declarative Stimulus actions in ERB (data-action) instead of imperative event listeners in controller lifecycle (e.g., avoid addEventListener in connect); controllers should just respond to actions
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
{app/components/**/*.{js,ts,erb},app/views/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Component-scoped Stimulus controllers in app/components must be used only within their component views, not in app/views
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Style UI using TailwindCSS v4.x with the custom design system defined in app/assets/tailwind/maybe-design-system.css
Always start by referencing app/assets/tailwind/maybe-design-system.css to understand available primitives, functional tokens, and component tokens before styling
Prefer functional tokens from the design system over raw Tailwind values (e.g., use text-primary, bg-container, border border-primary instead of text-white, bg-white, border-gray-200)
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
{app/views,app/components}/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
{app/views,app/components}/**/*.html.erb: Keep domain logic out of ERB templates; compute values in component/controller code and reference them in the template
Integrate Stimulus declaratively in ERB: templates declare data-controller/data-action; controllers respond to those declarations
Pass data from Rails to Stimulus via data-*-value attributes, not inline JavaScript
Files:
app/views/simplefin_items/_simplefin_item.html.erbapp/views/oidc_accounts/link.html.erb
app/views/**/_*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Name partials with a leading underscore (e.g., _trend_change.html.erb, _form_errors.html.erb)
Files:
app/views/simplefin_items/_simplefin_item.html.erb
🧠 Learnings (3)
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-09-16T13:15:56.406Z
Learning: Applies to app/controllers/concerns/auto_sync.rb : Run daily Family syncs via the AutoSync concern; Family sync orchestrates Account and Plaid Item syncs
Applied to files:
app/models/family/syncer.rb
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-09-16T13:15:56.406Z
Learning: Applies to app/models/account.rb : Account syncs are orchestrated by Account#sync_data (auto-match transfers, compute balances from start_date..today, enrich transactions) and should run whenever an Entry is updated
Applied to files:
app/models/simplefin_item/importer.rb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to db/migrate/**/*.rb : Enforce simple validations in the database (NULL constraints, unique indexes, etc.)
Applied to files:
db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb
🧬 Code graph analysis (6)
app/models/family/syncer.rb (1)
app/models/sync.rb (1)
family(192-198)
app/controllers/oidc_accounts_controller.rb (2)
app/models/oidc_identity.rb (1)
create_from_omniauth(14-27)test/controllers/oidc_accounts_controller_test.rb (1)
pending_auth(8-17)
app/models/simplefin_item/importer.rb (4)
app/models/simplefin_item.rb (1)
upsert_simplefin_snapshot!(52-61)app/models/simplefin_account.rb (1)
upsert_simplefin_snapshot!(9-27)app/models/simplefin_item/provided.rb (1)
simplefin_provider(4-6)app/models/provider/simplefin.rb (1)
get_accounts(27-74)
db/schema.rb (2)
app/controllers/simplefin_items_controller.rb (1)
index(4-7)app/controllers/accounts_controller.rb (1)
index(5-11)
app/models/simplefin_item.rb (3)
app/models/concerns/syncable.rb (1)
latest_sync(67-69)db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb (1)
down(9-19)db/migrate/20250813144520_add_institution_fields_to_simplefin_items.rb (1)
down(19-32)
app/models/family/sync_complete_event.rb (1)
app/models/family.rb (1)
balance_sheet(63-65)
🔇 Additional comments (21)
app/models/family/syncer.rb (1)
29-29: LGTM! SimpleFin items now included in family sync orchestration.The addition of
family.simplefin_items.activeappropriately extends the family sync to include active SimpleFin items alongside Plaid items and manual accounts, aligning with the PR objective to enable SimpleFin syncing.app/models/oidc_identity.rb (1)
24-25: LGTM! Settinglast_authenticated_atat creation time.The addition ensures that
last_authenticated_atis initialized when creating an identity from OmniAuth, eliminating the need for a separate update call and ensuring consistency.app/views/oidc_accounts/link.html.erb (1)
45-73: LGTM! Updated to use design system components and functional tokens.The view now correctly uses:
- DS::Button and DS::Link components instead of plain HTML helpers
- Functional tokens (bg-container, text-primary, border-secondary) instead of raw color classes
These changes align with the coding guidelines for design system usage.
As per coding guidelines
db/migrate/20251025095800_relax_simplefin_item_institution_constraints.rb (1)
1-20: LGTM! Cautious migration approach to relax constraints.The migration correctly:
- Uses ActiveRecord::Migration[7.2] as required
- Relaxes NULL constraints for institution fields that SimpleFin may not provide initially
- Implements conservative rollback logic that only restores NOT NULL if no existing NULLs would violate the constraint
The approach prevents migration failures on rollback while maintaining data integrity.
Based on learnings
db/schema.rb (2)
13-13: Schema version updated correctly.The version bump reflects the new migration that relaxes SimpleFin institution constraints.
757-759: LGTM! New indexes for institution fields.The three new indexes on
simplefin_items(institution_domain, institution_id, institution_name) will optimize queries filtering or grouping SimpleFin items by institution metadata.app/models/simplefin_item/importer.rb (6)
2-2: LGTM! New error type for rate-limit scenarios.The nested
RateLimitedErrorallows callers to distinguish rate-limit errors from other SimpleFin API errors, enabling specific handling and user-facing messaging.
48-62: LGTM! Pre-discovery ensures all accounts are visible.The unbounded discovery fetch before chunked sync ensures newly added accounts without recent transactions aren't filtered out by date windows. The fallback to
pending=1is a good defensive strategy for varying server behaviors.
120-143: LGTM! Regular sync now includes discovery step.Consistent with the chunked approach, regular sync now performs unbounded discovery first to capture all accounts, then fetches transactions within the window. This prevents newly added accounts from being missed.
159-168: LGTM! Enhanced logging and pending parameter support.The updated signature accepts the
pendingparameter, and the enhanced logging with formatted dates and day counts will aid in debugging API interactions.
193-197: LGTM! Handles top-level error responses.The additional check for
accounts_data[:error]handles SimpleFin servers that return errors in a different format, ensuring consistent error handling.
282-287: LGTM! Specific rate-limit detection and error surfacing.The logic detects common rate-limit messages and raises
RateLimitedErrorwith a user-friendly message, enabling distinct handling in the UI layer.app/views/simplefin_items/_simplefin_item.html.erb (1)
41-45: LGTM! User-friendly rate-limit messaging.The new conditional branch displays rate-limit guidance using appropriate visual cues (clock icon, warning color) and relies on the model's
rate_limited_messagemethod. The placement in the status hierarchy (after auth errors, before generic errors) is logical.test/models/oidc_identity_test.rb (1)
80-80: LGTM! Test verifieslast_authenticated_atis set.The assertion confirms that
last_authenticated_atis properly initialized when creating an identity from OmniAuth, validating the production code change.app/models/simplefin_item.rb (2)
57-58: Clear documentation of design decision.The comment appropriately explains why item-level institution fields are no longer populated, aligning with the migration that relaxed NOT NULL constraints on these fields.
151-169: Good defensive implementation for rate-limit detection.The method appropriately guards against missing columns and provides user-friendly messaging. The string matching approach (line 164) is pragmatic for this use case, though it may require updates if SimpleFin changes their error message format in the future.
app/controllers/oidc_accounts_controller.rb (1)
112-121: Well-structured helper method for OmniAuth compatibility.The
build_auth_hashmethod correctly transforms the session-stored pending_auth hash into an OmniAuth-compatible structure. The use ofOpenStructallows the model to access fields using the same dot notation as actual OmniAuth responses.lib/tasks/simplefin.rake (4)
1-10: Good argument validation and error messaging.The rake task properly validates the required
item_idargument and provides clear usage instructions in JSON format, consistent with the task's output format.
15-16: Correct implementation for snapshot and unlinked account queries.The safe navigation and
digusage on Line 15 properly handles potentially missing data. The left join query on Line 16 correctly identifies unlinkedsimplefin_accounts.
30-32: Good error handling for debugging utility.The rescue block appropriately catches errors, formats them as JSON with useful debugging information (class, message, limited backtrace), and exits with a non-zero status code.
18-27: The review comment is based on an incorrect assumption about what "unlinked" means.The
unlinkedquery filters for SimplefinAccounts where the associated Account record is nil:left_joins(:account).where(accounts: { id: nil }). This means the SimplefinAccount has not been linked to the application's Account model, but theaccount_idfield (which stores the SimpleFin API identifier fromsnapshot[:id]) will still be populated.The code correctly uses
upstream_id: sfa.account_idto output the SimpleFin API account ID for these unlinked accounts. This is exactly what "upstream_id" should contain.Likely an incorrect or invalid review comment.
- Keep SimpleFin institution metadata strictly per account (`simplefin_accounts.org_data`). - Relax `simplefin_items` institution constraints to allow creating items before org data exists. - Remove code that copied the first account’s `org` onto `simplefin_items`.
• SimpleFin: family “Sync” includes SimpleFin items; importer does unbounded discovery (with pending=1 fallback) before windowed fetch, for both regular and first syncs. • Stop populating item‑level institution fields; keep institution metadata per account. • Relax NOT NULL on item institution fields. • Post‑sync dashboard broadcasts are now guarded (UI cannot fail the job). • Show a friendly “daily refresh limit” banner on the SimpleFin card when the latest sync is rate‑limited. • Add bin/rails sure:simplefin:debug[ITEM_ID] to print latest sync, snapshot account count, simplefin_accounts count, and unlinked list.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/models/simplefin_item/importer.rb (1)
257-259: Remove the redundant defensive account_id assignment at lines 257–259.The
find_or_initialize_by(account_id: account_id)call on lines 220–222 guaranteesaccount_idis set on the record. Theassign_attributes(attrs)call on line 254 cannot clear it becauseaccount_idis not included in theattrshash. The early return on line 218 ensuresaccount_idis never blank at that point. The SimplefinAccount model has no callbacks or setters that would clear this field. This defensive check is logically redundant and can be safely removed.
🧹 Nitpick comments (1)
app/models/simplefin_item/importer.rb (1)
48-62: Extract duplicated discovery logic.The unbounded discovery sequence (fetch without dates, fallback to pending=true, upsert snapshot, import accounts) is duplicated between
import_with_chunked_historyandimport_regular_sync. Consider extracting this into a helper method likeperform_account_discoveryto improve maintainability and ensure both paths remain synchronized.Example helper:
def perform_account_discovery discovery_data = fetch_accounts_data(start_date: nil) discovered_count = discovery_data&.dig(:accounts)&.size.to_i Rails.logger.info "SimpleFin discovery (no params) returned #{discovered_count} accounts" if discovered_count.zero? discovery_data = fetch_accounts_data(start_date: nil, pending: true) discovered_count = discovery_data&.dig(:accounts)&.size.to_i Rails.logger.info "SimpleFin discovery (pending=1) returned #{discovered_count} accounts" end if discovery_data && discovered_count > 0 simplefin_item.upsert_simplefin_snapshot!(discovery_data) discovery_data[:accounts]&.each { |account_data| import_account(account_data) } end endAlso applies to: 120-143
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/models/simplefin_item/importer.rb(5 hunks)lib/tasks/simplefin.rake(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
app/models/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-design.mdc)
Domain models should not call Provider::Registry directly; use a Provided concern within the model’s namespace to select providers and expose convenience methods
Use ActiveRecord validations for forms and complex domain constraints.
app/models/**/*.rb: Place business logic in POROs and model classes under app/models
Models should answer questions about themselves (e.g., prefer account.balance_series over service objects)
Implement complex validations and business logic with ActiveRecord validations
Model-level validations may mirror DB constraints but are not strictly required
Files:
app/models/simplefin_item/importer.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/models/simplefin_item/importer.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/models/simplefin_item/importer.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
app/models/simplefin_item/importer.rb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/models/simplefin_item/importer.rb
**/app/models/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Business logic should primarily reside in models; use concerns and POROs for organization.
Files:
app/models/simplefin_item/importer.rb
**/*.{rb,erb,haml,slim}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb,haml,slim}: UseCurrent.userfor the current user; do not usecurrent_user
UseCurrent.familyfor the current family; do not usecurrent_family
Ignore i18n methods; hardcode strings in English for now (do not useI18n.t,t, or similar)
Files:
app/models/simplefin_item/importer.rb
app/{helpers,models}/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, and dates on the server side (Ruby) before sending to the client
Files:
app/models/simplefin_item/importer.rb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/models/simplefin_item/importer.rb
🧠 Learnings (1)
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-09-16T13:15:56.406Z
Learning: Applies to app/models/account.rb : Account syncs are orchestrated by Account#sync_data (auto-match transfers, compute balances from start_date..today, enrich transactions) and should run whenever an Entry is updated
Applied to files:
app/models/simplefin_item/importer.rb
🧬 Code graph analysis (1)
app/models/simplefin_item/importer.rb (4)
app/models/simplefin_item.rb (1)
upsert_simplefin_snapshot!(52-61)app/models/simplefin_account.rb (1)
upsert_simplefin_snapshot!(9-27)app/models/simplefin_item/provided.rb (1)
simplefin_provider(4-6)app/models/provider/simplefin.rb (1)
get_accounts(27-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (5)
lib/tasks/simplefin.rake (2)
13-13: Good use of the model helper method.This correctly uses
item.latest_syncfrom the Syncable concern, addressing the previous review feedback.
1-35: Debug task implementation looks solid.The task provides comprehensive debug information with proper error handling and JSON output. The structure is clean and the safe navigation operators prevent nil errors.
app/models/simplefin_item/importer.rb (3)
2-2: Good addition of specific error type.The
RateLimitedErrorprovides clear signal for rate-limit conditions and enables specific handling downstream.
159-200: Enhanced error handling and logging look good.The addition of the
pendingparameter, improved date-range logging, and top-levelerrorhandling (in addition toerrorsarray) make this more robust. The authentication error handling and rate-limit detection are well implemented.
282-287: Rate-limit detection is well-designed.The specific pattern matching for rate-limit messages and raising a distinct
RateLimitedErrorenables clear user communication and appropriate handling in the UI layer.
luckyPipewrench
force-pushed
the
simplefin-sync-improvements
branch
from
ab234e3 to
b26dae2
Compare
- SimpleFin discovery logic deduplicated fixed - app/models/simplefin_item/importer.rb --Added a concise docstring for perform_account_discovery describing purpose, steps, and side‑effects. --Added a docstring for fetch_accounts_data describing params and return value.
| locals: { balance_sheet: family.balance_sheet } | ||
| ) | ||
| rescue => e | ||
| Rails.logger.error("Family::SyncCompleteEvent balance_sheet broadcast failed: #{e.message}\n#{e.backtrace&.join("\n")}") |
There was a problem hiding this comment.
Thanks for the logging, @luckyPipewrench! Really helps later when supporting users.
jjmata
left a comment
jjmata
left a comment
There was a problem hiding this comment.
Looks great, with nice logging additions, rake tasks and overall attention to error conditions/corner cases.
|
Still running into issues with accounts SimpleFIN Bridge themselves are stuck handing: Want to take care of those before we merge? I can also create an alpha build just with these changes, @luckyPipewrench! |
|
Also, there might be a better place to display the error message from SimpleFIN that the hover I stuck it inside of! :-) |
|
I haven't noticed the stuck accounts but I'll see about cleaning those things up also. Alpha may be good, im working on adding encryption for the stored SimpleFin access URLs in the database and some other simplefin features. |
|
Once I cleared the SimpleFIN Bridge error with Apple Card (their problem) things worked very nicely on the Sure side. Would be nice to fail gracefully and leave that account out of the sync set, but this is already a marked improvement and stabilizes the great work @Himmelschmidt ... so cutting an alpha build next! |
commit 9fefe57 Author: João Felipe <[email protected]> Date: Wed Oct 29 02:15:14 2025 +0400 Feature/yahoo finance (we-promise#123) * Implement Yahoo Finance * Added tests * Updated hosting controller to check for managed app_mode instead of env_override * Suggestions from CodeRabbit and Fixes on tests * Remove Css changes * Fix yahoo finance impl and i18n * Updated view to use healthy method * remove usage * Updated env example * keep usage on class just to keep same format * Ci test * Remove some useless validations * Remove logs * Linter fixes * Broke this in my conflict merge * Wrong indentation level --------- Signed-off-by: Juan José Mata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit 3910116 Author: Albert Solà <[email protected]> Date: Tue Oct 28 20:23:20 2025 +0000 Update pages/dashboard locales (we-promise#255) commit 4fb0a38 Author: soky srm <[email protected]> Date: Tue Oct 28 19:32:27 2025 +0100 Providers factory (we-promise#250) * Implement providers factory * Multiple providers sync support - Proper Multi-Provider Syncing: When you click sync on an account with multiple providers (e.g., both Plaid and SimpleFin), all provider items are synced - Better API: The existing account.providers method already returns all providers, and account.provider returns the first one for backward compatibility - Correct Holdings Deletion Logic: Holdings can only be deleted if ALL providers allow it, preventing accidental deletions that would be recreated on next sync TODO: validate this is the way we want to go? We would need to check holdings belong to which account, and then check provider allows deletion. More complex - Database Constraints: The existing validations ensure an account can have at most one provider of each type (one PlaidAccount, one SimplefinAccount, etc.) * Add generic provider_import_adapter * Finish unified import strategy * Update app/models/plaid_account.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <[email protected]> * Update app/models/provider/factory.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <[email protected]> * Fix account linked by plaid_id instead of external_id * Parse numerics to BigDecimal Parse numerics to BigDecimal before computing amount; guard nils. Avoid String * String and float drift; also normalize date. * Fix incorrect usage of assert_raises. * Fix linter * Fix processor test. * Update current_balance_manager.rb * Test fixes * Fix plaid linked account test * Add support for holding per account_provider * Fix proper account access Also fix account deletion for simpefin too * FIX match tests for consistency * Some more factory updates * Fix account schema for multipe providers Can do: - Account #1 → PlaidAccount + SimplefinAccount (multiple different providers) - Account #2 → PlaidAccount only - Account #3 → SimplefinAccount only Cannot do: - Account #1 → PlaidAccount + PlaidAccount (duplicate provider type) - PlaidAccount we-promise#123 → Account #1 + Account #2 (provider linked to multiple accounts) * Fix account setup - An account CAN have multiple providers (the schema shows account_providers with unique index on [account_id, provider_type]) - Each provider should maintain its own separate entries - We should NOT update one provider's entry when another provider syncs * Fix linter and guard migration * FIX linter issues. * Fixes - Remove duplicated index - Pass account_provider_id - Guard holdings call to avoid NoMethodError * Update schema and provider import fix * Plaid doesn't allow holdings deletion * Use ClimateControl for proper env setup * No need for this in .git --------- Signed-off-by: soky srm <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Juan José Mata <[email protected]> commit 72e7d77 Author: Juan José Mata <[email protected]> Date: Mon Oct 27 21:52:37 2025 +0100 Add onboarding state selector for self-hosted signup (we-promise#251) * Add onboarding modes to self-hosted signup * Style form consistently * Configure ONBOARDING_STATE via ENV commit dcb6748 Author: ampersandru <[email protected]> Date: Mon Oct 27 04:18:45 2025 -0700 Added a clickable drop down arrow that reveals list of instructions for Brand Fetch Client ID (we-promise#246) * Added a clickable drop down arrow that reveals list of instructions for Brand Fetch Client ID Signed-off-by: ampersandru <[email protected]> * Updated html code based on recommendations Signed-off-by: ampersandru <[email protected]> * Update _brand_fetch_settings.html.erb Signed-off-by: ampersandru <[email protected]> * Small edit for consistency with Twelve Data settings flow * HTML linting * Missed an extra closing tag. --------- Signed-off-by: ampersandru <[email protected]> Signed-off-by: Juan José Mata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit cca14c9 Author: ampersandru <[email protected]> Date: Mon Oct 27 03:45:01 2025 -0700 Added a clickable drop down arrow that reveals list of instructions for Twelve Data Secret Key (we-promise#247) * Added a clickable drop down arrow that reveals list of instructions for Twelve Data Secret Key Signed-off-by: ampersandru <[email protected]> * Fixed html spacing Signed-off-by: ampersandru <[email protected]> * Make display even more compact * Remove <u> and use CSS classes instead --------- Signed-off-by: ampersandru <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit 0fc70e9 Author: Juan José Mata <[email protected]> Date: Sun Oct 26 23:16:25 2025 +0100 Add runServices for db and redis in devcontainer Signed-off-by: Juan José Mata <[email protected]> commit d3d91f4 Author: Juan José Mata <[email protected]> Date: Sun Oct 26 20:53:18 2025 +0100 Add PikaPods One-click installs Signed-off-by: Juan José Mata <[email protected]> commit 53eb90f Author: ampersandru <[email protected]> Date: Sun Oct 26 09:58:18 2025 -0700 Added instructions on where and how to obtain API key for Twelve Data (we-promise#235) * Added instructions on where and how to obtain API key for Twelve Data Signed-off-by: ampersandru <[email protected]> * added rel="noopener noreferrer" to links Signed-off-by: ampersandru <[email protected]> --------- Signed-off-by: ampersandru <[email protected]> commit a35a4ac Author: Juan José Mata <[email protected]> Date: Sun Oct 26 17:51:31 2025 +0100 Update version to 0.6.5-alpha.3 Prepare for alpha.3 Signed-off-by: Juan José Mata <[email protected]> commit f42e6e3 Author: Albert Solà <[email protected]> Date: Sun Oct 26 16:50:31 2025 +0000 Added translations for ca - Catalan (we-promise#238) * Add CA locales for models * Add CA locales for views * Use translations in activity feed * Additional CA locales * Fix typo --------- Co-authored-by: Juan José Mata <[email protected]> commit db19c95 Author: ampersandru <[email protected]> Date: Sun Oct 26 09:47:29 2025 -0700 Add instructions on how to obtain Brand Logo Client ID API (we-promise#234) * Add instructions on how to obtain Brand Logo Client ID API Signed-off-by: ampersandru <[email protected]> * updated html based on recommendations Signed-off-by: ampersandru <[email protected]> * added rel="noopener noreferrer" to links Signed-off-by: ampersandru <[email protected]> --------- Signed-off-by: ampersandru <[email protected]> commit 0b393a0 Author: OrangeDrangon <[email protected]> Date: Sun Oct 26 11:58:26 2025 -0400 add custom s3 support storage config option (we-promise#239) Options are documented here including an example on how to set a custom endpoint: https://guides.rubyonrails.org/active_storage_overview.html#s3-service-amazon-s3-and-s3-compatible-apis commit fea228d Author: luckyPipewrench <[email protected]> Date: Sun Oct 26 10:50:45 2025 -0400 Simplefin sync improvements (we-promise#240) * Fix syncing issues with new connections and accounts.. - Keep SimpleFin institution metadata strictly per account (`simplefin_accounts.org_data`). - Relax `simplefin_items` institution constraints to allow creating items before org data exists. - Remove code that copied the first account’s `org` onto `simplefin_items`. * Improve Simplefin Sync • SimpleFin: family “Sync” includes SimpleFin items; importer does unbounded discovery (with pending=1 fallback) before windowed fetch, for both regular and first syncs. • Stop populating item‑level institution fields; keep institution metadata per account. • Relax NOT NULL on item institution fields. • Post‑sync dashboard broadcasts are now guarded (UI cannot fail the job). • Show a friendly “daily refresh limit” banner on the SimpleFin card when the latest sync is rate‑limited. • Add bin/rails sure:simplefin:debug[ITEM_ID] to print latest sync, snapshot account count, simplefin_accounts count, and unlinked list. * Fixed double‑quoted strings, spacing around array brackets and commas * chore: ignore local .junie files * - Broadcast error logs now include full backtraces - SimpleFin discovery logic deduplicated fixed - app/models/simplefin_item/importer.rb --Added a concise docstring for perform_account_discovery describing purpose, steps, and side‑effects. --Added a docstring for fetch_accounts_data describing params and return value. commit f9f6274 Author: Juan José Mata <[email protected]> Date: Sat Oct 25 22:57:49 2025 +0200 Make logo work in dark mode Signed-off-by: Juan José Mata <[email protected]> commit 1ee20ab Author: Copilot <[email protected]> Date: Sat Oct 25 22:23:28 2025 +0200 Eliminate code duplication in OIDC identity creation (we-promise#230) * Eliminate duplication by using create_from_omniauth method - Updated OidcIdentity.create_from_omniauth to set last_authenticated_at - Refactored OidcAccountsController to use create_from_omniauth instead of direct create! calls - Updated test to verify last_authenticated_at is set by create_from_omniauth Co-authored-by: jjmata <[email protected]> * Extract auth hash building into private helper method - Added build_auth_hash helper method to eliminate OpenStruct creation duplication - Both create_link and create_user actions now use the same helper Co-authored-by: jjmata <[email protected]> * Linter fix * Fix button style on OIDC link step * Fix dark mode styles --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jjmata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit b24b102 Author: luckyPipewrench <[email protected]> Date: Fri Oct 24 14:41:33 2025 -0400 Fix rounding issue (we-promise#226) Remove rounding from transactions. commit 03cac95 Author: soky srm <[email protected]> Date: Fri Oct 24 19:39:42 2025 +0200 Category improvements (we-promise#232) * Category improvements - Updated default categories to a more inclusive set - Updated default icon set for categories - Updated default categories colors to better separate then. * FIX tests * Better color pallettes for a few commit 962ddd1 Author: Juan José Mata <[email protected]> Date: Fri Oct 24 19:25:36 2025 +0200 Refresh README with new logo and LLM conversation in screenshot Signed-off-by: Juan José Mata <[email protected]> commit b001d1c Author: Marc <[email protected]> Date: Fri Oct 24 19:02:54 2025 +0200 I have created a one click deploy button for railway users 🧙♂️ (we-promise#228) * Add Railway deployment button to README Signed-off-by: Marc <[email protected]> commit 4ba8f32 Author: Juan José Mata <[email protected]> Date: Fri Oct 24 18:11:31 2025 +0200 Fix production OIDC regression Signed-off-by: Juan José Mata <[email protected]> commit 768e85c Author: Juan José Mata <[email protected]> Date: Fri Oct 24 16:07:45 2025 +0200 Add OpenID Connect login support (we-promise#77) * Add OpenID Connect login support * Add docs for OIDC config with Google Auth * Use Google styles for log in - Add support for linking existing account - Force users to sign-in with passoword first, when linking existing accounts - Add support to create new user when using OIDC - Add identities to user to prevent account take-ver - Make tests mocking instead of being integration tests - Manage session handling correctly - use OmniAuth.config.mock_auth instead of passing auth data via request env * Conditionally render Oauth button - Set a config item `configuration.x.auth.oidc_enabled` - Hide button if disabled --------- Signed-off-by: Juan José Mata <[email protected]> Signed-off-by: soky srm <[email protected]> Co-authored-by: sokie <[email protected]> commit d51ba51 Author: Copilot <[email protected]> Date: Fri Oct 24 13:33:06 2025 +0200 Fix Twelve Data API parsing errors causing "Current Market Price Unknown" (we-promise#224) * Add tests and fix for Twelve Data API parsing errors * Fix search_securities to handle nil data key --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jjmata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit a8f318c Author: Copilot <[email protected]> Date: Fri Oct 24 12:04:19 2025 +0200 Fix "Messages is invalid" error for Ollama/custom LLM providers and add comprehensive AI documentation (we-promise#225) * Add comprehensive AI/LLM configuration documentation * Fix Chat.start! to use default model when model is nil or empty * Ensure all controllers use Chat.default_model for consistency * Move AI doc inside `hosting/` * Probably too much error handling --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jjmata <[email protected]> Co-authored-by: Juan José Mata <[email protected]> commit 4f44630 Author: Juan José Mata <[email protected]> Date: Fri Oct 24 00:16:29 2025 +0200 Bump version to 0.6.5-alpha.1 Signed-off-by: Juan José Mata <[email protected]> commit bb364fa Author: soky srm <[email protected]> Date: Fri Oct 24 00:08:59 2025 +0200 LLM cost estimation (we-promise#223) * Password reset back button also after confirmation Signed-off-by: Juan José Mata <[email protected]> * Implement a filter for category (we-promise#215) - Also implement an is empty/is null condition. * Implement an LLM cost estimation page Track costs across all the cost categories: auto categorization, auto merchant detection and chat. Show warning with estimated cost when running a rule that contains AI. * Update pricing * Add google pricing and fix inferred model everywhere. * Update app/models/llm_usage.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <[email protected]> * FIX address review * Linter * Address review - Lowered log level - extracted the duplicated record_usage method into a shared concern * Update app/controllers/settings/llm_usages_controller.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <[email protected]> * Moved attr_reader out of private --------- Signed-off-by: Juan José Mata <[email protected]> Signed-off-by: soky srm <[email protected]> Co-authored-by: Juan José Mata <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Fix syncing issues with new connections and accounts.. - Keep SimpleFin institution metadata strictly per account (`simplefin_accounts.org_data`). - Relax `simplefin_items` institution constraints to allow creating items before org data exists. - Remove code that copied the first account’s `org` onto `simplefin_items`. * Improve Simplefin Sync • SimpleFin: family “Sync” includes SimpleFin items; importer does unbounded discovery (with pending=1 fallback) before windowed fetch, for both regular and first syncs. • Stop populating item‑level institution fields; keep institution metadata per account. • Relax NOT NULL on item institution fields. • Post‑sync dashboard broadcasts are now guarded (UI cannot fail the job). • Show a friendly “daily refresh limit” banner on the SimpleFin card when the latest sync is rate‑limited. • Add bin/rails sure:simplefin:debug[ITEM_ID] to print latest sync, snapshot account count, simplefin_accounts count, and unlinked list. * Fixed double‑quoted strings, spacing around array brackets and commas * chore: ignore local .junie files * - Broadcast error logs now include full backtraces - SimpleFin discovery logic deduplicated fixed - app/models/simplefin_item/importer.rb --Added a concise docstring for perform_account_discovery describing purpose, steps, and side‑effects. --Added a docstring for fetch_accounts_data describing params and return value.
* Fix syncing issues with new connections and accounts.. - Keep SimpleFin institution metadata strictly per account (`simplefin_accounts.org_data`). - Relax `simplefin_items` institution constraints to allow creating items before org data exists. - Remove code that copied the first account’s `org` onto `simplefin_items`. * Improve Simplefin Sync • SimpleFin: family “Sync” includes SimpleFin items; importer does unbounded discovery (with pending=1 fallback) before windowed fetch, for both regular and first syncs. • Stop populating item‑level institution fields; keep institution metadata per account. • Relax NOT NULL on item institution fields. • Post‑sync dashboard broadcasts are now guarded (UI cannot fail the job). • Show a friendly “daily refresh limit” banner on the SimpleFin card when the latest sync is rate‑limited. • Add bin/rails sure:simplefin:debug[ITEM_ID] to print latest sync, snapshot account count, simplefin_accounts count, and unlinked list. * Fixed double‑quoted strings, spacing around array brackets and commas * chore: ignore local .junie files * - Broadcast error logs now include full backtraces - SimpleFin discovery logic deduplicated fixed - app/models/simplefin_item/importer.rb --Added a concise docstring for perform_account_discovery describing purpose, steps, and side‑effects. --Added a docstring for fetch_accounts_data describing params and return value.
2 participants
Enhanced SimpleFin to be easily usable.
Fixes new SimpleFin connection creation after initial connection is deleted then receated.
Newly added accounts now sync with button press.
SimpleFin: family “Sync” includes SimpleFin items; importer does unbounded discovery (with pending=1 fallback) before windowed fetch, for both regular and first syncs.
Stop populating item‑level institution fields; keep institution metadata per account.
Relax NOT NULL on item institution fields.
Post‑sync dashboard broadcasts are now guarded (UI cannot fail the job).
Show a friendly “daily refresh limit” banner on the SimpleFin card when the latest sync is rate‑limited.
Add bin/rails sure:simplefin:debug[ITEM_ID] to print latest sync, snapshot account count, simplefin_accounts count, and unlinked list.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
This PR may address #110