Initial OFX Support #560
Initial OFX Support #560
Conversation
📝 WalkthroughWalkthroughAdds OFX import support: new OfxImport model and OFX parsing, controller/view updates for OFX upload flow, migration/schema change for import_rows.external_id, enum/type registrations, OFX initializer patch, Gemfile additions, and i18n keys. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller as Import::UploadsController
participant Model as OfxImport
participant Parser as OFX::Parser
participant Adapter as Account::ProviderImportAdapter
participant DB as Database
User->>Controller: POST upload (ofx_file)
Controller->>Model: build OfxImport (params, ofx_str)
Controller->>Parser: validate/parse OFX (via ofx_valid?)
Parser-->>Model: statements
alt no valid statement
Model-->>Controller: validation error
Controller-->>User: render error
else statement found
Model->>Model: select matching_account (last digits, currency)
Model->>Adapter: map transactions -> import_rows (include external_id)
Adapter->>DB: persist import_rows
Model->>DB: save import record
Controller-->>User: redirect on success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 5
🤖 Fix all issues with AI Agents
In @app/views/import/uploads/show.html.erb:
- Line 89: Replace the hardcoded button label in the form.submit call so it uses
the i18n helper t(), e.g. change the label "Upload CSV" to a translated key
(like t('imports.upload_csv')) while preserving the disabled: @import.complete?
option; also update the other hardcoded button at the other submit call (the one
referenced at line 107) to use a corresponding t(...) key so both buttons are
internationalized.
- Line 104: Replace the hardcoded placeholder string "Paste your CSV file
contents here" with a translatable helper call; update the view where the
placeholder option is set (the form/input config in show.html.erb) to use
placeholder: t('imports.uploads.csv_placeholder') (or your chosen i18n key), add
the corresponding entry to the locale files (e.g. en.yml) under that key, and
ensure any JS-inserted copies of this text also use the I18n lookup so the
string is fully localizable.
- Line 66: The form.select call in the view is using hardcoded English for the
label and include_blank options; replace those literals with I18n translation
lookups (t(...)) so the label and placeholder are localized, e.g. change the
:label and :include_blank values in the form.select invocation to use t('...')
keys (create sensible keys like 'imports.account_label' and
'imports.multi_account_import' in your locale files) and ensure the selected:
@import.account_id remains unchanged.
- Around line 57-58: Replace the hardcoded tab labels in the calls to
nav.with_btn (ids "csv-upload" and "csv-paste") with i18n lookups using the t()
helper (e.g. nav.with_btn(id: "csv-upload", label: t('import.upload_csv')) and
nav.with_btn(id: "csv-paste", label: t('import.copy_paste'))), and add
corresponding keys to your locale files for the desired translations.
- Around line 73-75: Replace the hardcoded "Browse" string inside the span with
the i18n helper: locate the span element with class "font-medium text-primary"
in show.html.erb and change its content to use t('...') (e.g.
t('import.uploads.browse') or a suitable key), and add that translation key to
your locale yml so the label is localized.
🧹 Nitpick comments (3)
db/schema.rb (1)
508-508: Consider adding an index on external_id for idempotency lookups.The new
external_idcolumn supports idempotency for OFX imports. However, there's no index defined, which may impact lookup performance when checking for duplicate import rows. Consider whether a unique constraint or index is needed, similar to the entries table pattern at line 306.#!/bin/bash # Check if external_id is used for lookups in OFX import logic echo "=== Searching for external_id usage in import-related files ===" rg -n --type=rb -C3 'external_id' -g '*import*.rb' -g '*ofx*.rb' echo -e "\n=== Check if duplicate checking is implemented ===" rg -n --type=rb 'find_by.*external_id|where.*external_id' -g '*import*.rb' -g '*ofx*.rb'app/controllers/import/uploads_controller.rb (1)
1-2: Consider movingrequirestatements.While functional, placing
require "ofx"andrequire "stringio"at the controller level is less conventional. These could be moved toapp/models/ofx_import.rb(which already requires them) or an initializer for cleaner organization. Not blocking.app/models/ofx_import.rb (1)
38-62: Method namegenerate_rows_from_csvis inherited but misleading for OFX.The method overrides the parent's CSV-based implementation to handle OFX data instead. While this works for polymorphism, consider renaming the parent method to
generate_rowsif feasible, or add a clarifying comment.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
Gemfileapp/controllers/import/uploads_controller.rbapp/helpers/imports_helper.rbapp/models/data_enrichment.rbapp/models/import.rbapp/models/ofx_import.rbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erbapp/views/imports/new.html.erbconfig/locales/views/imports/en.ymldb/migrate/20251218120000_add_external_id_to_import_rows.rbdb/schema.rb
🧰 Additional context used
📓 Path-based instructions (25)
{app/**/*.{erb,html},app/javascript/**/*.js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erb
app/views/**/*.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.erb: Always use theiconhelper for icons in views, never uselucide_icondirectly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-side
app/views/**/*.erb: Avoid heavy logic in ERB views; prefer helpers and components instead
Ensure ERB views passerb-lintchecks as configured in.erb_lint.yml
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erb
**/*.{css,erb}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Tailwind CSS v4.x with custom design system for styling
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erb
**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb}: UseCurrent.userfor accessing the current user. Do NOT usecurrent_user
UseCurrent.familyfor accessing the current family. Do NOT usecurrent_family
Files:
app/views/imports/new.html.erbapp/models/import.rbapp/models/data_enrichment.rbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erbapp/helpers/imports_helper.rbapp/models/ofx_import.rbdb/schema.rbdb/migrate/20251218120000_add_external_id_to_import_rows.rbapp/controllers/import/uploads_controller.rb
**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use theiconhelper from application_helper.rb for icons. Never uselucide_iconhelper directly
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erb
**/*.{rb,js,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Files:
app/views/imports/new.html.erbapp/models/import.rbapp/models/data_enrichment.rbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erbapp/helpers/imports_helper.rbapp/models/ofx_import.rbdb/schema.rbdb/migrate/20251218120000_add_external_id_to_import_rows.rbapp/controllers/import/uploads_controller.rb
**/*.{rb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible
**/*.{rb,html.erb}: UseCurrent.userfor the current user, NOTcurrent_user
UseCurrent.familyfor the current family, NOTcurrent_family
Files:
app/views/imports/new.html.erbapp/models/import.rbapp/models/data_enrichment.rbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erbapp/helpers/imports_helper.rbapp/models/ofx_import.rbdb/schema.rbdb/migrate/20251218120000_add_external_id_to_import_rows.rbapp/controllers/import/uploads_controller.rb
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erbapp/helpers/imports_helper.rb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erbapp/helpers/imports_helper.rb
**/*.{erb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erb
**/*.{html.erb,erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{html.erb,erb}: Use<dialog>for modals and<details><summary>for disclosures instead of JavaScript components
Use Turbo frames for page sections instead of client-side solutions
Use theiconhelper for icons, NEVER uselucide_icondirectly
Keep domain logic OUT of view templates; logic belongs in component files, not template files
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erb
**/*.{js,ts,html.erb,erb}
📄 CodeRabbit inference engine (CLAUDE.md)
Use query params for state instead of localStorage/sessions
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erb
**/*.{html.erb,erb,css}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{html.erb,erb,css}: Always use functional tokens from the design system (e.g.,text-primaryinstead oftext-white,bg-containerinstead ofbg-white)
Referenceapp/assets/tailwind/maybe-design-system.cssfor primitives and tokens; never create new styles without permission
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erb
**/*.{html.erb,erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
All user-facing strings must use the
t()helper for localization (i18n); updateconfig/locales/en.ymlfor new strings
Files:
app/views/imports/new.html.erbapp/models/import.rbapp/models/data_enrichment.rbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erbapp/helpers/imports_helper.rbapp/models/ofx_import.rbdb/schema.rbdb/migrate/20251218120000_add_external_id_to_import_rows.rbapp/controllers/import/uploads_controller.rb
**/*.{rb,html.erb,erb}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Rails pluralization for i18n:
t('transactions.count', count: @transactions.count)
Files:
app/views/imports/new.html.erbapp/models/import.rbapp/models/data_enrichment.rbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erbapp/helpers/imports_helper.rbapp/models/ofx_import.rbdb/schema.rbdb/migrate/20251218120000_add_external_id_to_import_rows.rbapp/controllers/import/uploads_controller.rb
**/*.{html.erb,erb,js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{html.erb,erb,js,ts}: Stimulus controllers should use declarative actions in HTML (data-action attributes) rather than inline JavaScript
Pass data to Stimulus controllers viadata-*-valueattributes, not inline JavaScript
Files:
app/views/imports/new.html.erbapp/views/import/configurations/_ofx_import.html.erbapp/views/import/uploads/show.html.erb
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Place configuration files in
config/directory
Files:
config/locales/views/imports/en.yml
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Place business logic inapp/models/folder, avoid usingapp/services/
Use Rails concerns and POROs for organization; models should answer questions about themselves (e.g.,account.balance_seriesnotAccountSeries.new(account).call)
The codebase runs in two distinct modes: 'managed' (team-operated servers) and 'self_hosted' (user-hosted infrastructure); checkRails.application.config.app_mode
Use Money objects for all monetary values; store in base currency (user's primary currency) for accurate conversion and formatting
**/*.rb: Use 2-space indentation for Ruby code
Usesnake_casefor method and variable names in Ruby
UseCamelCasefor class and module names in Ruby
Runbin/rubocopbefore pushing code; use-Aflag to auto-correct safe style violations
Files:
app/models/import.rbapp/models/data_enrichment.rbapp/helpers/imports_helper.rbapp/models/ofx_import.rbdb/schema.rbdb/migrate/20251218120000_add_external_id_to_import_rows.rbapp/controllers/import/uploads_controller.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder
app/models/**/*.rb: Simple validations (null checks, unique indexes) belong in the database; ActiveRecord validations for convenience in forms; complex validations in ActiveRecord
Optimize database queries with proper indexes, prevent N+1 queries via includes/joins
SimpleFIN: detect pending viapending: trueorpostedblank/0 +transacted_at; Plaid: detect pending viapending: trueatextra['plaid']['pending']
Store pending transaction metadata from providers onTransaction#extra(e.g.,extra['simplefin']['pending']); FX usesfx_from,fx_date
Files:
app/models/import.rbapp/models/data_enrichment.rbapp/models/ofx_import.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Follow Rails conventions for folder and file names in the app directory
Files:
app/models/import.rbapp/models/data_enrichment.rbapp/helpers/imports_helper.rbapp/models/ofx_import.rbapp/controllers/import/uploads_controller.rb
app/views/**/_*.html.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use partials only for simple, static HTML with minimal logic in specific contexts
app/views/**/_*.html.erb: Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Place context-specific partials in relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)Use partials when element is static HTML with minimal logic, used in only one/few contexts, is simple template content, or doesn't need variants
Files:
app/views/import/configurations/_ofx_import.html.erb
db/migrate/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use simple validations (null checks, unique indexes) in database layer
Rails database migrations should follow Rails migration conventions and be stored in
db/migrate/
Files:
db/migrate/20251218120000_add_external_id_to_import_rows.rb
db/migrate/*.rb
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
ActiveRecord migrations must inherit from
ActiveRecord::Migration[7.2]. Do NOT use version 8.0 yetEnforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL
Files:
db/migrate/20251218120000_add_external_id_to_import_rows.rb
{Gemfile,*.gemspec}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never add new dependencies without strong technical/business justification; push Rails to its limits first
Files:
Gemfile
app/controllers/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Rails strong parameters and CSRF protection throughout the application
Files:
app/controllers/import/uploads_controller.rb
🧠 Learnings (15)
📚 Learning: 2025-12-19T22:25:35.646Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T22:25:35.646Z
Learning: Applies to config/locales/en.yml : Use hierarchical i18n keys by feature (e.g., `accounts.index.title`, `transactions.form.amount_label`)
Applied to files:
config/locales/views/imports/en.yml
📚 Learning: 2025-12-19T22:25:35.646Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T22:25:35.646Z
Learning: Applies to **/*.{html.erb,erb,rb} : All user-facing strings must use the `t()` helper for localization (i18n); update `config/locales/en.yml` for new strings
Applied to files:
config/locales/views/imports/en.yml
📚 Learning: 2025-12-18T07:48:15.175Z
Learnt from: jjmata
Repo: we-promise/sure PR: 464
File: config/locales/views/simplefin_items/zh-CN.yml:36-36
Timestamp: 2025-12-18T07:48:15.175Z
Learning: Enforce the exact branding: always use 'SimpleFIN' (FIN in all caps) and never 'SimpleFin' when referring to the Simple Financial Interchange protocol or SimpleFIN Bridge service across all locale files under config/locales/views. This applies to code, documentation, and translations. Update zh-CN.yml and other locale files as needed; perform a project-wide search/replace to fix inconsistent references and add a review check to catch any future deviations.
Applied to files:
config/locales/views/imports/en.yml
📚 Learning: 2025-12-18T08:32:32.373Z
Learnt from: jjmata
Repo: we-promise/sure PR: 464
File: config/locales/views/simplefin_items/zh-CN.yml:36-36
Timestamp: 2025-12-18T08:32:32.373Z
Learning: In YAML locale files, ensure vendor branding (SimpleFIN) appears only in end-user visible strings (UI text, translations, user-facing messages). Internal keys such as logs, class names, and variable identifiers should not include the vendor branding and can use simpler naming like SimpleFin or simplefin. Apply this broadly to all locale files under config/locales.
Applied to files:
config/locales/views/imports/en.yml
📚 Learning: 2025-11-14T14:22:37.076Z
Learnt from: jjmata
Repo: we-promise/sure PR: 326
File: app/models/category_import.rb:10-10
Timestamp: 2025-11-14T14:22:37.076Z
Learning: In the CategoryImport model (app/models/category_import.rb), the import process implements a "last write wins" strategy where importing a category.csv file completely overrides existing category structures, including parent relationships. Setting category.parent = nil in the first pass is intentional behavior to ensure the CSV becomes the source of truth.
Applied to files:
app/models/import.rbapp/models/ofx_import.rbapp/controllers/import/uploads_controller.rb
📚 Learning: 2025-11-24T16:56:30.669Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Entry model is a Rails delegated type with three subtypes: Valuation (absolute account value), Transaction (balance modification), and Trade (buy/sell for investment accounts)
Applied to files:
app/models/import.rb
📚 Learning: 2025-11-24T16:56:30.669Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Account model is a Rails delegated type with subtypes: Depository, Investment, Crypto, Property, Vehicle, OtherAsset (assets) and CreditCard, Loan, OtherLiability (liabilities)
Applied to files:
app/models/data_enrichment.rb
📚 Learning: 2025-12-19T22:25:35.646Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T22:25:35.646Z
Learning: Applies to app/models/**/*.rb : SimpleFIN: detect pending via `pending: true` or `posted` blank/0 + `transacted_at`; Plaid: detect pending via `pending: true` at `extra['plaid']['pending']`
Applied to files:
app/models/data_enrichment.rb
📚 Learning: 2025-11-24T16:57:20.318Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.318Z
Learning: Applies to app/views/**/_*.html.erb : Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Applied to files:
app/views/import/configurations/_ofx_import.html.erb
📚 Learning: 2025-11-24T16:56:30.669Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Account sync must perform transfer auto-matching, calculate daily balance records from start_date to Date.current, calculate holdings, and optionally enrich transaction data
Applied to files:
app/models/ofx_import.rb
📚 Learning: 2025-11-24T16:55:59.156Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/general-rules.mdc:0-0
Timestamp: 2025-11-24T16:55:59.156Z
Learning: Applies to db/migrate/*.rb : ActiveRecord migrations must inherit from `ActiveRecord::Migration[7.2]`. Do NOT use version 8.0 yet
Applied to files:
db/schema.rbdb/migrate/20251218120000_add_external_id_to_import_rows.rb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to db/migrate/*.rb : Enforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL
Applied to files:
db/schema.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to db/migrate/**/*.rb : Use simple validations (null checks, unique indexes) in database layer
Applied to files:
db/schema.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to {Gemfile,*.gemspec} : Never add new dependencies without strong technical/business justification; push Rails to its limits first
Applied to files:
Gemfile
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to test/**/*.rb : Use mocha gem for mocks and VCR for external API tests
Applied to files:
Gemfile
🧬 Code graph analysis (3)
app/models/data_enrichment.rb (1)
app/models/provider_merchant.rb (1)
enum(1-6)
app/models/ofx_import.rb (3)
app/models/import.rb (6)
import!(238-240)new(1-291)required_column_keys(125-127)column_keys(129-131)mapping_steps(175-177)generate_rows_from_csv(133-155)app/models/account/provider_import_adapter.rb (1)
import_transaction(21-83)app/models/import/row.rb (2)
signed_amount(25-31)date_iso(21-23)
app/controllers/import/uploads_controller.rb (1)
app/models/ofx_import.rb (1)
matching_account(64-77)
⏰ 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 (14)
config/locales/views/imports/en.yml (1)
18-20: LGTM! Localization keys follow conventions.The OFX-related i18n keys follow the hierarchical structure and align with existing patterns in the locale file. The use of "OFX" in all-caps is appropriate as it's a standard format name.
Also applies to: 64-72, 107-107
db/schema.rb (1)
13-13: LGTM! Schema version updated correctly.The schema version bump to 2025_12_18_120000 follows the timestamp-based naming convention and reflects the external_id column addition.
Gemfile (2)
79-79: Clarify the necessity of the nkf dependency.The
nkf(Network Kanji Filter) gem is primarily used for Japanese text encoding conversion. Please verify whether this is required for OFX file processing or if Ruby's built-inEncodingclass would suffice. Per project policy, new dependencies require strong technical or business justification.
78-78: The OFX gem version 0.3.8 is current and secure.Version 0.3.8 is the latest stable release on RubyGems (released June 19, 2025) and has no known CVEs. While upstream libOFX projects have had historical vulnerabilities (CVE-2017-2816, CVE-2019-9656), these do not apply to this Ruby gem wrapper.
app/models/import.rb (1)
5-5: OfxImport class implementation is correct. It properly inherits from Import and implements all required methods (import!,required_column_keys,column_keys, andmapping_steps). The addition ofOfxImportto the TYPES constant follows the existing pattern.app/models/data_enrichment.rb (1)
4-4: OFX does not require addition to ProviderMerchant sources.OFX is a one-time import mechanism (similar to CSV), not a connected provider account like Plaid or SimpleFin. The ProviderMerchant enum is for providers that establish ongoing connections and require merchant mapping, whereas OFX transactions are imported via the Import framework without needing ProviderMerchant records. Adding OFX to DataEnrichment is appropriate for transaction enrichment tracking, but ProviderMerchant does not need this addition.
app/views/import/configurations/_ofx_import.html.erb (1)
1-7: LGTM!The partial correctly uses localization helpers (
t(".description"),t(".button_label")), functional design tokens (bg-container-inset,text-secondary), and follows the underscore naming convention for partials. The form structure is clean and appropriate for the configuration step.app/helpers/imports_helper.rb (1)
69-71: LGTM!The addition of
ofx_importfollows the existing pattern and aligns with theOfxImporttype added toImport::TYPES.app/views/imports/new.html.erb (1)
48-66: LGTM!The OFX import option follows the established UI pattern. The omission of the
Current.family.accounts.any?guard (unlikeTransactionImportandTradeImport) appears intentional since the OFX upload flow handles account selection/creation at the upload stage.db/migrate/20251218120000_add_external_id_to_import_rows.rb (1)
1-5: LGTM!Migration correctly inherits from
ActiveRecord::Migration[7.2]as required. Theexternal_idcolumn stores the OFXfit_idfor reference during import; since lookups happen on theentriestable (viaProviderImportAdapter), an index here is not necessary.app/controllers/import/uploads_controller.rb (1)
81-100: LGTM!The OFX upload handler correctly validates the file, falls back to
matching_accountwhen no account is selected, ensures an account exists before proceeding, and provides clear error feedback.app/models/ofx_import.rb (3)
8-24: LGTM!The
import!method correctly integrates withAccount::ProviderImportAdapter, validates account presence, and passes appropriate parameters including the OFX source identifier.
121-125: Good defensive fallback forexternal_id.The SHA256 hash fallback ensures deduplication works even if the OFX
fit_idis missing, using a composite key of date, amount, currency, and name.
64-77: Account matching logic is reasonable but relies on naming convention.The matching heuristic uses the last 4 digits of the OFX account ID and currency to find a matching account. This works well when users include account numbers in their account names, but may not match otherwise.
Consider adding a note in the UI or documentation that account names should include the last 4 digits for auto-matching to work.
| <% nav.with_btn(id: "csv-upload", label: "Upload CSV") %> | ||
| <% nav.with_btn(id: "csv-paste", label: "Copy & Paste") %> |
There was a problem hiding this comment.
Hardcoded strings should use t() helper for localization.
Per coding guidelines, all user-facing strings must use the t() helper. The tab labels "Upload CSV" and "Copy & Paste" should be localized.
Suggested fix
- <% nav.with_btn(id: "csv-upload", label: "Upload CSV") %>
- <% nav.with_btn(id: "csv-paste", label: "Copy & Paste") %>
+ <% nav.with_btn(id: "csv-upload", label: t(".tab_upload_csv")) %>
+ <% nav.with_btn(id: "csv-paste", label: t(".tab_copy_paste")) %>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <% nav.with_btn(id: "csv-upload", label: "Upload CSV") %> | |
| <% nav.with_btn(id: "csv-paste", label: "Copy & Paste") %> | |
| <% nav.with_btn(id: "csv-upload", label: t(".tab_upload_csv")) %> | |
| <% nav.with_btn(id: "csv-paste", label: t(".tab_copy_paste")) %> |
🤖 Prompt for AI Agents
In @app/views/import/uploads/show.html.erb around lines 57 - 58, Replace the
hardcoded tab labels in the calls to nav.with_btn (ids "csv-upload" and
"csv-paste") with i18n lookups using the t() helper (e.g. nav.with_btn(id:
"csv-upload", label: t('import.upload_csv')) and nav.with_btn(id: "csv-paste",
label: t('import.copy_paste'))), and add corresponding keys to your locale files
for the desired translations.
| <%= form.select :account_id, @import.family.accounts.visible.pluck(:name, :id), { label: "Account (optional)", include_blank: "Multi-account import", selected: @import.account_id } %> | ||
| <% end %> | ||
| <% if @import.type == "TransactionImport" || @import.type == "TradeImport" %> | ||
| <%= form.select :account_id, @import.family.accounts.visible.pluck(:name, :id), { label: "Account (optional)", include_blank: "Multi-account import", selected: @import.account_id } %> |
There was a problem hiding this comment.
Hardcoded label and placeholder strings should use t() helper.
The label: and include_blank: options contain hardcoded English strings that should be localized.
Suggested fix
- <%= form.select :account_id, @import.family.accounts.visible.pluck(:name, :id), { label: "Account (optional)", include_blank: "Multi-account import", selected: @import.account_id } %>
+ <%= form.select :account_id, @import.family.accounts.visible.pluck(:name, :id), { label: t(".account_label"), include_blank: t(".multi_account_import"), selected: @import.account_id } %>🤖 Prompt for AI Agents
In @app/views/import/uploads/show.html.erb at line 66, The form.select call in
the view is using hardcoded English for the label and include_blank options;
replace those literals with I18n translation lookups (t(...)) so the label and
placeholder are localized, e.g. change the :label and :include_blank values in
the form.select invocation to use t('...') keys (create sensible keys like
'imports.account_label' and 'imports.multi_account_import' in your locale files)
and ensure the selected: @import.account_id remains unchanged.
| <p class="mb-2 text-md text-gray text-center"> | ||
| <span class="font-medium text-primary">Browse</span> to add your CSV file here | ||
| </p> |
There was a problem hiding this comment.
Hardcoded browse text should use t() helper.
Suggested fix
- <span class="font-medium text-primary">Browse</span> to add your CSV file here
+ <span class="font-medium text-primary"><%= t(".browse") %></span> <%= t(".browse_csv_suffix") %>🤖 Prompt for AI Agents
In @app/views/import/uploads/show.html.erb around lines 73 - 75, Replace the
hardcoded "Browse" string inside the span with the i18n helper: locate the span
element with class "font-medium text-primary" in show.html.erb and change its
content to use t('...') (e.g. t('import.uploads.browse') or a suitable key), and
add that translation key to your locale yml so the label is localized.
| </div> | ||
|
|
||
| <%= form.submit "Upload CSV", disabled: @import.complete? %> | ||
| <%= form.submit "Upload CSV", disabled: @import.complete? %> |
There was a problem hiding this comment.
Hardcoded button text should use t() helper.
Suggested fix
- <%= form.submit "Upload CSV", disabled: @import.complete? %>
+ <%= form.submit t(".upload_csv"), disabled: @import.complete? %>Apply similar fix to Line 107.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <%= form.submit "Upload CSV", disabled: @import.complete? %> | |
| <%= form.submit t(".upload_csv"), disabled: @import.complete? %> |
🤖 Prompt for AI Agents
In @app/views/import/uploads/show.html.erb at line 89, Replace the hardcoded
button label in the form.submit call so it uses the i18n helper t(), e.g. change
the label "Upload CSV" to a translated key (like t('imports.upload_csv')) while
preserving the disabled: @import.complete? option; also update the other
hardcoded button at the other submit call (the one referenced at line 107) to
use a corresponding t(...) key so both buttons are internationalized.
| <%= form.text_area :raw_file_str, | ||
| rows: 10, | ||
| required: true, | ||
| placeholder: "Paste your CSV file contents here", |
There was a problem hiding this comment.
Hardcoded placeholder should use t() helper.
Suggested fix
- placeholder: "Paste your CSV file contents here",
+ placeholder: t(".paste_placeholder"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| placeholder: "Paste your CSV file contents here", | |
| placeholder: t(".paste_placeholder"), |
🤖 Prompt for AI Agents
In @app/views/import/uploads/show.html.erb at line 104, Replace the hardcoded
placeholder string "Paste your CSV file contents here" with a translatable
helper call; update the view where the placeholder option is set (the form/input
config in show.html.erb) to use placeholder:
t('imports.uploads.csv_placeholder') (or your chosen i18n key), add the
corresponding entry to the locale files (e.g. en.yml) under that key, and ensure
any JS-inserted copies of this text also use the I18n lookup so the string is
fully localizable.
OFX gem crashes with empty date tags.
|
@DeathCamel58 I've pushed a fix for ofx gem, it crashes when tags have empty dates. |
sokie
left a comment
sokie
left a comment
There was a problem hiding this comment.
For me this is good to go, but would love you to fix translation strings.
Also the Import OFX icon looks not good on dark theme, maybe you can reuse the CSV icon?
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @app/views/imports/new.html.erb:
- Around line 48-66: The OfxImport option is shown even when no accounts exist,
causing a runtime error in OfxImport (raises "Account is required for OFX
imports."); modify the conditional that wraps the OfxImport button (the if
checking params[:type] and the button_to with imports_path(import: { type:
"OfxImport" })) to also require Current.family.accounts.any? like the other
import options do, so the OfxImport UI is only rendered when accounts exist.
🧹 Nitpick comments (1)
app/views/imports/new.html.erb (1)
52-54: Consider using a distinct icon/color for OfxImport.The OfxImport option uses the same icon (
file-spreadsheet) and color scheme (indigo-500) as TransactionImport (lines 32-34). Using a distinct visual identifier would help users differentiate between CSV transaction imports and OFX file imports at a glance.💡 Example alternative icons
Consider one of these alternatives:
file-codeorfile-texticon to suggest a different file format- A different color like
purple-500orteal-500to distinguish it visually- <div class="bg-indigo-500/5 rounded-md w-8 h-8 flex items-center justify-center"> - <span class="text-indigo-500"> - <%= icon("file-spreadsheet", color: "current") %> + <div class="bg-purple-500/5 rounded-md w-8 h-8 flex items-center justify-center"> + <span class="text-purple-500"> + <%= icon("file-code", color: "current") %> </span>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/views/imports/new.html.erb
🧰 Additional context used
📓 Path-based instructions (16)
{app/**/*.{erb,html},app/javascript/**/*.js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript
Files:
app/views/imports/new.html.erb
app/views/**/*.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.erb: Always use theiconhelper for icons in views, never uselucide_icondirectly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-side
app/views/**/*.erb: Avoid heavy logic in ERB views; prefer helpers and components instead
Ensure ERB views passerb-lintchecks as configured in.erb_lint.yml
Files:
app/views/imports/new.html.erb
**/*.{css,erb}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Tailwind CSS v4.x with custom design system for styling
Files:
app/views/imports/new.html.erb
**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb}: UseCurrent.userfor accessing the current user. Do NOT usecurrent_user
UseCurrent.familyfor accessing the current family. Do NOT usecurrent_family
Files:
app/views/imports/new.html.erb
**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use theiconhelper from application_helper.rb for icons. Never uselucide_iconhelper directly
Files:
app/views/imports/new.html.erb
**/*.{rb,js,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Files:
app/views/imports/new.html.erb
**/*.{rb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible
**/*.{rb,html.erb}: UseCurrent.userfor the current user, NOTcurrent_user
UseCurrent.familyfor the current family, NOTcurrent_family
Files:
app/views/imports/new.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/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values
Files:
app/views/imports/new.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/imports/new.html.erb
**/*.{erb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript
Files:
app/views/imports/new.html.erb
**/*.{html.erb,erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{html.erb,erb}: Use<dialog>for modals and<details><summary>for disclosures instead of JavaScript components
Use Turbo frames for page sections instead of client-side solutions
Use theiconhelper for icons, NEVER uselucide_icondirectly
Keep domain logic OUT of view templates; logic belongs in component files, not template files
Files:
app/views/imports/new.html.erb
**/*.{js,ts,html.erb,erb}
📄 CodeRabbit inference engine (CLAUDE.md)
Use query params for state instead of localStorage/sessions
Files:
app/views/imports/new.html.erb
**/*.{html.erb,erb,css}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{html.erb,erb,css}: Always use functional tokens from the design system (e.g.,text-primaryinstead oftext-white,bg-containerinstead ofbg-white)
Referenceapp/assets/tailwind/maybe-design-system.cssfor primitives and tokens; never create new styles without permission
Files:
app/views/imports/new.html.erb
**/*.{html.erb,erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
All user-facing strings must use the
t()helper for localization (i18n); updateconfig/locales/en.ymlfor new strings
Files:
app/views/imports/new.html.erb
**/*.{rb,html.erb,erb}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Rails pluralization for i18n:
t('transactions.count', count: @transactions.count)
Files:
app/views/imports/new.html.erb
**/*.{html.erb,erb,js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{html.erb,erb,js,ts}: Stimulus controllers should use declarative actions in HTML (data-action attributes) rather than inline JavaScript
Pass data to Stimulus controllers viadata-*-valueattributes, not inline JavaScript
Files:
app/views/imports/new.html.erb
⏰ 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
| <% if params[:type].nil? || params[:type] == "OfxImport" %> | ||
| <li> | ||
| <%= button_to imports_path(import: { type: "OfxImport" }), class: "flex items-center justify-between p-4 group cursor-pointer w-full", data: { turbo: false } do %> | ||
| <div class="flex items-center gap-2"> | ||
| <div class="bg-indigo-500/5 rounded-md w-8 h-8 flex items-center justify-center"> | ||
| <span class="text-indigo-500"> | ||
| <%= icon("file-spreadsheet", color: "current") %> | ||
| </span> | ||
| </div> | ||
| <span class="text-sm text-primary group-hover:text-secondary"> | ||
| <%= t(".import_ofx") %> | ||
| </span> | ||
| </div> | ||
| <%= icon("chevron-right") %> | ||
| <% end %> | ||
|
|
||
| <%= render "shared/ruler" %> | ||
| </li> | ||
| <% end %> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find OfxImport model definition
echo "=== OfxImport model definition ==="
fd -type f -name "*ofx*import*.rb" app/models/Repository: we-promise/sure
Length of output: 281
🏁 Script executed:
#!/bin/bash
# Search for OfxImport class and account associations
echo "=== Searching for OfxImport class ==="
rg -n "class OfxImport" app/models/ -A 20Repository: we-promise/sure
Length of output: 1233
🏁 Script executed:
#!/bin/bash
# Check if OfxImport has belongs_to :account
echo "=== OfxImport associations and validations ==="
rg -n "belongs_to.*account|has_many.*ofx" app/models/ -i -B 2 -A 10Repository: we-promise/sure
Length of output: 5113
🏁 Script executed:
#!/bin/bash
# Check imports controller for account validation in OFX flow
echo "=== Checking imports controller ==="
cat -n app/controllers/imports_controller.rb | head -50Repository: we-promise/sure
Length of output: 1973
🏁 Script executed:
#!/bin/bash
# Search for any account requirement in create/new actions
echo "=== Searching for account checks in import views ==="
rg -n "accounts.any" app/views/imports/ -B 2 -A 2Repository: we-promise/sure
Length of output: 1615
Add account availability check for OfxImport option.
OfxImport explicitly requires an account (line 9 of app/models/ofx_import.rb raises "Account is required for OFX imports." if account is nil), but the view allows the option to display without accounts. This is inconsistent with TransactionImport (line 28), TradeImport (line 68), and MintImport (line 148), which all check Current.family.accounts.any?. Users without accounts will see the OfxImport option, click it, complete the upload, and receive an error when publishing instead of being prevented upfront.
Proposed fix
- <% if params[:type].nil? || params[:type] == "OfxImport" %>
+ <% if Current.family.accounts.any? && (params[:type].nil? || params[:type] == "OfxImport") %>
<li>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <% if params[:type].nil? || params[:type] == "OfxImport" %> | |
| <li> | |
| <%= button_to imports_path(import: { type: "OfxImport" }), class: "flex items-center justify-between p-4 group cursor-pointer w-full", data: { turbo: false } do %> | |
| <div class="flex items-center gap-2"> | |
| <div class="bg-indigo-500/5 rounded-md w-8 h-8 flex items-center justify-center"> | |
| <span class="text-indigo-500"> | |
| <%= icon("file-spreadsheet", color: "current") %> | |
| </span> | |
| </div> | |
| <span class="text-sm text-primary group-hover:text-secondary"> | |
| <%= t(".import_ofx") %> | |
| </span> | |
| </div> | |
| <%= icon("chevron-right") %> | |
| <% end %> | |
| <%= render "shared/ruler" %> | |
| </li> | |
| <% end %> | |
| <% if Current.family.accounts.any? && (params[:type].nil? || params[:type] == "OfxImport") %> | |
| <li> | |
| <%= button_to imports_path(import: { type: "OfxImport" }), class: "flex items-center justify-between p-4 group cursor-pointer w-full", data: { turbo: false } do %> | |
| <div class="flex items-center gap-2"> | |
| <div class="bg-indigo-500/5 rounded-md w-8 h-8 flex items-center justify-center"> | |
| <span class="text-indigo-500"> | |
| <%= icon("file-spreadsheet", color: "current") %> | |
| </span> | |
| </div> | |
| <span class="text-sm text-primary group-hover:text-secondary"> | |
| <%= t(".import_ofx") %> | |
| </span> | |
| </div> | |
| <%= icon("chevron-right") %> | |
| <% end %> | |
| <%= render "shared/ruler" %> | |
| </li> | |
| <% end %> |
🤖 Prompt for AI Agents
In @app/views/imports/new.html.erb around lines 48 - 66, The OfxImport option is
shown even when no accounts exist, causing a runtime error in OfxImport (raises
"Account is required for OFX imports."); modify the conditional that wraps the
OfxImport button (the if checking params[:type] and the button_to with
imports_path(import: { type: "OfxImport" })) to also require
Current.family.accounts.any? like the other import options do, so the OfxImport
UI is only rendered when accounts exist.
| email_validator (2.2.4) | ||
| activemodel | ||
| erb (5.0.1) | ||
| erb (4.0.4) |
There was a problem hiding this comment.
This bumps it lower, can you fix/retest @DeathCamel58?
| racc (~> 1.4) | ||
| nokogiri (1.18.9-x86_64-linux-musl) | ||
| nkf (0.2.0) | ||
| nokogiri (1.15.7) |
|
Glad you noticed the dependency issue @jjmata. Turns out There's a PR (annacruz/ofx#124) for them to update the nokogiri dependency, then I should swing back around to this PR. Marking this as a draft awaiting their merge. |
It's been 7 months since that OFX repo had a commit, should we fork it ourselves and take the PR in? |
@jjmata tbh i already monkey patched a bug in that library ( see my commit here ). |
Here is our fork, in case you want to move it forward that way,@DeathCamel58. |
Yeah, that's the other reason I was suggesting a fork for now. I don't see anybody taking the time to do our own OFX implementation in the meantime, and it's better if we could be "better stewards" of that Gem ourselves. Anyway, it's @DeathCamel58's PR ... he can decide. :-) |
|
About this statement :
OFX files have an ID file to deduplicate imported line : <FITID>1905720436I don't know if there is a deduplicate macanism, but with this ID, it is possible to manage that. |
Sure already has built in dedup, and it will be handled since ids are built properly already |
4 participants
Add support for importing transactions for an account via OFX files.
Note: This import method does not attempt any sort of de-duplication. Most banks will let the user specify from and to dates, and they can use that for keeping duplicates from occuring.
There's some import page wording weirdness. This is due to re-using the standard import page templates. We can circle back around to work on the wording of these pages later.
Screenshots:
Summary by CodeRabbit
New Features
Data
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.