-
Notifications
You must be signed in to change notification settings - Fork 95
Lunchflow fix #307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lunchflow fix #307
Conversation
- Proper closing of modal on cancel - Preload on new account already
WalkthroughAdds asynchronous Lunch Flow account preloading: new preload endpoint and route, a Stimulus preload controller and view hooks to fetch/cache accounts and toggle spinner/link UI, removes synchronous preloading from the accountable concern, adds per-account blank-name validation across controller/views, and updates importer/syncer filtering to visible/existing accounts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Page as Accounts Page
participant Stimulus as LunchflowPreloadCtrl
participant Backend as LunchflowItemsController#preload_accounts
participant Cache as Cache (5m)
participant API as Provider::LunchflowAdapter / Lunch Flow API
User->>Browser: open accounts/new
Browser->>Page: render (controller attached if `@show_lunchflow_link`)
Page->>Stimulus: connect()
activate Stimulus
Stimulus->>Stimulus: showLoading() (spinner, disable link)
Stimulus->>Backend: GET /lunchflow_items/preload_accounts (accountable_type, return_to)
deactivate Stimulus
activate Backend
Backend->>Cache: lookup accounts
alt cache hit
Cache-->>Backend: accounts
else cache miss
Backend->>API: fetch accounts
API-->>Backend: accounts or error
Backend->>Cache: store accounts (5m) on success
end
Backend-->>Stimulus: JSON { success, has_accounts, cached, error? }
deactivate Backend
activate Stimulus
Stimulus->>Stimulus: hideLoading()
alt has_accounts == true
Stimulus->>Page: keep link enabled
else has_accounts == false
Stimulus->>Page: hide link
end
deactivate Stimulus
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/controllers/lunchflow_items_controller.rb (1)
12-47: Well-structured preload action with good error handling.The caching strategy and error handling are appropriate for a background preload operation.
Consider adding rate limiting to prevent excessive API calls if a user repeatedly refreshes the page:
def preload_accounts # Optional: Rate limit using family-scoped throttle throttle_key = "lunchflow_preload_#{Current.family.id}" if Rails.cache.read(throttle_key) render json: { success: false, error: "rate_limited", has_accounts: false } return end begin cache_key = "lunchflow_accounts_#{Current.family.id}" # ... existing implementation # Set throttle for 10 seconds Rails.cache.write(throttle_key, true, expires_in: 10.seconds) # ... rest of implementation endAlso, consider how the cache should be invalidated when Lunchflow accounts change (e.g., after a sync or when accounts are linked).
app/javascript/controllers/lunchflow_preload_controller.js (1)
48-52: Consider using a CSS class instead of direct style manipulation.While
style.display = "none"works, using a utility class likehiddenwould be more consistent with the rest of the codebase that uses Tailwind CSS.} else if (!data.has_accounts) { // No accounts available, hide the link entirely if (this.hasLinkTarget) { - this.linkTarget.style.display = "none"; + this.linkTarget.classList.add("hidden"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/controllers/accounts_controller.rb(1 hunks)app/controllers/concerns/accountable_resource.rb(0 hunks)app/controllers/lunchflow_items_controller.rb(1 hunks)app/javascript/controllers/lunchflow_preload_controller.js(1 hunks)app/views/accounts/new.html.erb(1 hunks)app/views/accounts/new/_method_selector.html.erb(2 hunks)app/views/lunchflow_items/select_accounts.html.erb(1 hunks)app/views/lunchflow_items/select_existing_account.html.erb(1 hunks)config/routes.rb(1 hunks)
💤 Files with no reviewable changes (1)
- app/controllers/concerns/accountable_resource.rb
🧰 Additional context used
📓 Path-based instructions (21)
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/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.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/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.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/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/javascript/controllers/lunchflow_preload_controller.jsapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.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/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.html.erb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/views/lunchflow_items/select_accounts.html.erbapp/controllers/accounts_controller.rbapp/views/accounts/new/_method_selector.html.erbapp/controllers/lunchflow_items_controller.rbapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.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/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/javascript/controllers/lunchflow_preload_controller.jsapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.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/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/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/**}: 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/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/javascript/controllers/lunchflow_preload_controller.jsapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.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/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.html.erb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/controllers/accounts_controller.rbapp/controllers/lunchflow_items_controller.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/controllers/accounts_controller.rbapp/controllers/lunchflow_items_controller.rbconfig/routes.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/controllers/accounts_controller.rbapp/controllers/lunchflow_items_controller.rbconfig/routes.rbapp/javascript/controllers/lunchflow_preload_controller.js
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/accounts/new/_method_selector.html.erb
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/routes.rb
app/javascript/**/*.{js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
app/javascript/**/*.{js,jsx}: Place JavaScript code under app/javascript/
JavaScript naming: lowerCamelCase for variables/functions; PascalCase for classes/components
Let Biome format and lint JavaScript code (npm run lint/format)
Files:
app/javascript/controllers/lunchflow_preload_controller.js
app/javascript/controllers/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/javascript/controllers/**/*.js: Stimulus controllers should remain lightweight, have fewer than 7 targets, use private methods and expose a clear public API.
Pass data viadata-*-valueattributes, not with inline JavaScript.
Component-specific controllers should stay near components; global controllers go inapp/javascript/controllers/.
Files:
app/javascript/controllers/lunchflow_preload_controller.js
app/javascript/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use URL query params for state over localStorage/sessionStorage; only persist to DB if absolutely necessary
Files:
app/javascript/controllers/lunchflow_preload_controller.js
app/javascript/controllers/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/javascript/controllers/**/*.{js,ts}: Stimulus controllers should display preformatted values; avoid implementing formatting logic client-side
Keep client-side code for interactions where it truly shines (e.g., bulk selection); avoid unnecessary client-only solutionsPlace global Stimulus controllers under app/javascript/controllers so they can be used across any view
Files:
app/javascript/controllers/lunchflow_preload_controller.js
{app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
{app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}}: Keep Stimulus controllers lightweight: aim for fewer than 7 targets; use private methods and expose a clear public API
Keep Stimulus controllers focused: no domain logic; favor single responsibility; leverage Stimulus callbacks, actions, targets, values, and classes
Files:
app/javascript/controllers/lunchflow_preload_controller.js
{app/javascript/controllers,app/components}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Use Stimulus targets for DOM access instead of manual document.getElementById calls
Files:
app/javascript/controllers/lunchflow_preload_controller.js
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 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
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}} : Keep Stimulus controllers focused: no domain logic; favor single responsibility; leverage Stimulus callbacks, actions, targets, values, and classes
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/**/*.html.erb : Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Applied to files:
app/views/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/views/accounts/new.html.erb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/layouts/application.html.erb : Use Turbo frames in the application layout to load controller actions as demonstrated
Applied to files:
app/views/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/views/accounts/new.html.erb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/**/*.html.erb : Prefer native HTML elements (e.g., <dialog>, <details><summary>) over JS-based components
Applied to files:
app/views/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.html.erb
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to app/{views,components}/**/*.html.erb : In ERB views, wire Stimulus interactions declaratively using data-action to call controller methods
Applied to files:
app/views/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/javascript/controllers/lunchflow_preload_controller.jsapp/views/lunchflow_items/select_existing_account.html.erbapp/views/accounts/new.html.erb
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Applies to {app/views,app/components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/data-action; controllers respond to those declarations
Applied to files:
app/views/lunchflow_items/select_accounts.html.erbapp/views/accounts/new/_method_selector.html.erbapp/javascript/controllers/lunchflow_preload_controller.jsapp/views/accounts/new.html.erb
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
Repo: we-promise/sure PR: 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/controllers/accounts_controller.rb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/{views,components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/actions/targets; controllers respond to those declarations
Applied to files:
app/views/accounts/new/_method_selector.html.erbapp/javascript/controllers/lunchflow_preload_controller.jsapp/views/accounts/new.html.erb
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Place context-specific partials within the relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)
Applied to files:
app/views/accounts/new/_method_selector.html.erbapp/views/accounts/new.html.erb
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
Repo: we-promise/sure PR: 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/controllers/lunchflow_items_controller.rb
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}} : Keep Stimulus controllers lightweight: aim for fewer than 7 targets; use private methods and expose a clear public API
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}} : Keep Stimulus controllers focused: no domain logic; favor single responsibility; leverage Stimulus callbacks, actions, targets, values, and classes
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to app/javascript/controllers/**/*.js : Stimulus controllers should remain lightweight, have fewer than 7 targets, use private methods and expose a clear public API.
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/views/**/*.erb,app/components/**/*.erb} : 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
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.jsapp/views/accounts/new.html.erb
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers lightweight: aim for fewer than 7 targets and expose a small public API with other logic in private methods
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Applies to app/javascript/controllers/**/*.{js,ts} : Place global Stimulus controllers under app/javascript/controllers so they can be used across any view
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers focused on UI behavior; avoid domain/business logic and prefer single, closely related responsibilities; use Stimulus callbacks, actions, targets, values, and classes
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/javascript/controllers/**/*.js : Global Stimulus controllers under app/javascript/controllers/ can be used across any view
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : In Stimulus controllers, do not imperatively bind DOM events (e.g., addEventListener in connect); expose methods and let views trigger them via actions
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Applies to {app/views,app/components}/**/*.html.erb : Pass data from Rails to Stimulus via data-*-value attributes, not inline JavaScript
Applied to files:
app/views/accounts/new.html.erb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/{views,components}/**/*.html.erb : Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript
Applied to files:
app/views/accounts/new.html.erb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/layouts/**/*.erb : Be mindful of performance in global layouts (e.g., avoid loading large data payloads)
Applied to files:
app/views/accounts/new.html.erb
🧬 Code graph analysis (2)
app/controllers/accounts_controller.rb (1)
app/models/family/lunchflow_connectable.rb (1)
can_connect_lunchflow?(8-11)
app/controllers/lunchflow_items_controller.rb (3)
app/controllers/accounts_controller.rb (1)
family(82-84)app/models/provider/lunchflow_adapter.rb (1)
build_provider(39-45)app/models/provider/lunchflow.rb (1)
get_accounts(16-29)
⏰ 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 (7)
app/views/lunchflow_items/select_existing_account.html.erb (1)
35-35: LGTM!The addition of the dialog close action improves UX by ensuring the dialog closes when the user cancels.
app/views/lunchflow_items/select_accounts.html.erb (1)
35-35: LGTM!Consistent with the same improvement in
select_existing_account.html.erb.app/controllers/accounts_controller.rb (1)
14-16: LGTM!Clean implementation that delegates the feature check to the model layer.
config/routes.rb (1)
295-295: LGTM!The route is correctly placed as a collection route and follows RESTful conventions.
app/views/accounts/new/_method_selector.html.erb (2)
43-43: Verify thatturbo_action: "advance"is the intended behavior.This will push a new history entry when clicking the Lunchflow link. For a modal dialog, this may not be the desired behavior since users can't navigate back to the modal state via browser back button once it's dismissed.
Consider whether the modal should appear in browser history. If not, remove the
turbo_action: "advance"line.
49-54: Nice UX improvement with the loading spinner!The spinner provides clear visual feedback during the preload operation.
app/javascript/controllers/lunchflow_preload_controller.js (1)
11-13: Well-structured Stimulus controller following conventions.The controller is lightweight, uses proper lifecycle callbacks, and stays focused on UI behavior without domain logic. Good use of async/await for the background preload.
Based on learnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/javascript/controllers/lunchflow_preload_controller.js (1)
34-41: CSRF token handling is improved but consider logging when missing.The null-safety check for the CSRF token addresses the previous concern. However, while GET requests typically don't require CSRF tokens, adding a console warning when the token is missing would aid debugging in production environments where it might be unexpectedly absent.
Consider adding a warning:
const csrfToken = document.querySelector('[name="csrf-token"]'); const headers = { Accept: "application/json", }; if (csrfToken) { headers["X-CSRF-Token"] = csrfToken.content; +} else { + console.warn("CSRF token not found in page"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/javascript/controllers/lunchflow_preload_controller.js(1 hunks)app/views/accounts/new.html.erb(1 hunks)app/views/accounts/new/_method_selector.html.erb(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/views/accounts/new/_method_selector.html.erb
- app/views/accounts/new.html.erb
🧰 Additional context used
📓 Path-based instructions (10)
app/javascript/**/*.{js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
app/javascript/**/*.{js,jsx}: Place JavaScript code under app/javascript/
JavaScript naming: lowerCamelCase for variables/functions; PascalCase for classes/components
Let Biome format and lint JavaScript code (npm run lint/format)
Files:
app/javascript/controllers/lunchflow_preload_controller.js
app/javascript/controllers/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/javascript/controllers/**/*.js: Stimulus controllers should remain lightweight, have fewer than 7 targets, use private methods and expose a clear public API.
Pass data viadata-*-valueattributes, not with inline JavaScript.
Component-specific controllers should stay near components; global controllers go inapp/javascript/controllers/.
Files:
app/javascript/controllers/lunchflow_preload_controller.js
**/*.{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/javascript/controllers/lunchflow_preload_controller.js
**/*.{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/javascript/controllers/lunchflow_preload_controller.js
app/javascript/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use URL query params for state over localStorage/sessionStorage; only persist to DB if absolutely necessary
Files:
app/javascript/controllers/lunchflow_preload_controller.js
app/javascript/controllers/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/javascript/controllers/**/*.{js,ts}: Stimulus controllers should display preformatted values; avoid implementing formatting logic client-side
Keep client-side code for interactions where it truly shines (e.g., bulk selection); avoid unnecessary client-only solutionsPlace global Stimulus controllers under app/javascript/controllers so they can be used across any view
Files:
app/javascript/controllers/lunchflow_preload_controller.js
{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/javascript/controllers/lunchflow_preload_controller.js
{app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
{app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}}: Keep Stimulus controllers lightweight: aim for fewer than 7 targets; use private methods and expose a clear public API
Keep Stimulus controllers focused: no domain logic; favor single responsibility; leverage Stimulus callbacks, actions, targets, values, and classes
Files:
app/javascript/controllers/lunchflow_preload_controller.js
{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/javascript/controllers/lunchflow_preload_controller.js
{app/javascript/controllers,app/components}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Use Stimulus targets for DOM access instead of manual document.getElementById calls
Files:
app/javascript/controllers/lunchflow_preload_controller.js
🧠 Learnings (12)
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}} : Keep Stimulus controllers lightweight: aim for fewer than 7 targets; use private methods and expose a clear public API
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to app/javascript/controllers/**/*.js : Stimulus controllers should remain lightweight, have fewer than 7 targets, use private methods and expose a clear public API.
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}} : Keep Stimulus controllers focused: no domain logic; favor single responsibility; leverage Stimulus callbacks, actions, targets, values, and classes
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers lightweight: aim for fewer than 7 targets and expose a small public API with other logic in private methods
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/views/**/*.erb,app/components/**/*.erb} : 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
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/{views,components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/actions/targets; controllers respond to those declarations
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Applies to app/javascript/controllers/**/*.{js,ts} : Place global Stimulus controllers under app/javascript/controllers so they can be used across any view
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers focused on UI behavior; avoid domain/business logic and prefer single, closely related responsibilities; use Stimulus callbacks, actions, targets, values, and classes
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Applies to {app/views,app/components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/data-action; controllers respond to those declarations
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/javascript/controllers/**/*.js : Global Stimulus controllers under app/javascript/controllers/ can be used across any view
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : In Stimulus controllers, do not imperatively bind DOM events (e.g., addEventListener in connect); expose methods and let views trigger them via actions
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to app/{views,components}/**/*.html.erb : In ERB views, wire Stimulus interactions declaratively using data-action to call controller methods
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
⏰ 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 (2)
app/javascript/controllers/lunchflow_preload_controller.js (2)
1-9: LGTM - Clean controller structure.The controller follows Stimulus conventions with a clear, minimal public API: 2 targets and 2 values, well under the 7-target guideline.
11-13: LGTM - Appropriate use of connect lifecycle.Invoking preloadAccounts on connect is a standard Stimulus pattern for initialization logic that should run automatically when the controller attaches.
Signed-off-by: soky srm <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/javascript/controllers/lunchflow_preload_controller.js (1)
76-88: Use design system tokens instead of raw Tailwind utilities.Lines 77 and 84 use the raw
opacity-50class, which violates the coding guideline requiring functional design tokens from the design system. Reviewapp/assets/tailwind/maybe-design-system.cssfor an appropriate disabled/loading state token.As per coding guidelines.
🧹 Nitpick comments (1)
app/javascript/controllers/lunchflow_preload_controller.js (1)
55-59: Prefer CSS class toggling over inline styles.Line 58 uses
style.display = "none"while the rest of the controller usesclassListfor UI state. This inconsistency makes the code harder to maintain.Apply this diff to use class-based hiding:
- this.linkTarget.style.display = "none"; + this.linkTarget.classList.add("hidden");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/javascript/controllers/lunchflow_preload_controller.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
app/javascript/**/*.{js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
app/javascript/**/*.{js,jsx}: Place JavaScript code under app/javascript/
JavaScript naming: lowerCamelCase for variables/functions; PascalCase for classes/components
Let Biome format and lint JavaScript code (npm run lint/format)
Files:
app/javascript/controllers/lunchflow_preload_controller.js
app/javascript/controllers/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/javascript/controllers/**/*.js: Stimulus controllers should remain lightweight, have fewer than 7 targets, use private methods and expose a clear public API.
Pass data viadata-*-valueattributes, not with inline JavaScript.
Component-specific controllers should stay near components; global controllers go inapp/javascript/controllers/.
Files:
app/javascript/controllers/lunchflow_preload_controller.js
**/*.{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/javascript/controllers/lunchflow_preload_controller.js
**/*.{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/javascript/controllers/lunchflow_preload_controller.js
app/javascript/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use URL query params for state over localStorage/sessionStorage; only persist to DB if absolutely necessary
Files:
app/javascript/controllers/lunchflow_preload_controller.js
app/javascript/controllers/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/javascript/controllers/**/*.{js,ts}: Stimulus controllers should display preformatted values; avoid implementing formatting logic client-side
Keep client-side code for interactions where it truly shines (e.g., bulk selection); avoid unnecessary client-only solutionsPlace global Stimulus controllers under app/javascript/controllers so they can be used across any view
Files:
app/javascript/controllers/lunchflow_preload_controller.js
{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/javascript/controllers/lunchflow_preload_controller.js
{app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
{app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}}: Keep Stimulus controllers lightweight: aim for fewer than 7 targets; use private methods and expose a clear public API
Keep Stimulus controllers focused: no domain logic; favor single responsibility; leverage Stimulus callbacks, actions, targets, values, and classes
Files:
app/javascript/controllers/lunchflow_preload_controller.js
{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/javascript/controllers/lunchflow_preload_controller.js
{app/javascript/controllers,app/components}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Use Stimulus targets for DOM access instead of manual document.getElementById calls
Files:
app/javascript/controllers/lunchflow_preload_controller.js
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 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
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}} : Keep Stimulus controllers lightweight: aim for fewer than 7 targets; use private methods and expose a clear public API
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/components/**/*.{js,ts}} : Keep Stimulus controllers focused: no domain logic; favor single responsibility; leverage Stimulus callbacks, actions, targets, values, and classes
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to app/javascript/controllers/**/*.js : Stimulus controllers should remain lightweight, have fewer than 7 targets, use private methods and expose a clear public API.
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:07.608Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:07.608Z
Learning: Applies to {app/javascript/controllers/**/*.{js,ts},app/views/**/*.erb,app/components/**/*.erb} : 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
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers lightweight: aim for fewer than 7 targets and expose a small public API with other logic in private methods
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/{views,components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/actions/targets; controllers respond to those declarations
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers focused on UI behavior; avoid domain/business logic and prefer single, closely related responsibilities; use Stimulus callbacks, actions, targets, values, and classes
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Applies to app/javascript/controllers/**/*.{js,ts} : Place global Stimulus controllers under app/javascript/controllers so they can be used across any view
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/javascript/controllers/**/*.js : Global Stimulus controllers under app/javascript/controllers/ can be used across any view
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Applies to {app/views,app/components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/data-action; controllers respond to those declarations
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:23:17.441Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-09-23T22:23:17.441Z
Learning: Applies to {app/views/**,app/helpers/**,app/javascript/controllers/**} : 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)
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to **/*.{html,erb,slim,js,jsx,ts,tsx,css,scss} : Always use functional design tokens (e.g., text-primary, bg-container) from the design system; do not use raw colors or ad-hoc classes.
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to app/assets/tailwind/maybe-design-system.css : Always reference app/assets/tailwind/maybe-design-system.css for design tokens and primitives; use only defined tokens.
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-08-22T18:30:26.758Z
Learnt from: jjmata
Repo: we-promise/sure PR: 116
File: app/views/family_exports/_list.html.erb:16-28
Timestamp: 2025-08-22T18:30:26.758Z
Learning: In the family exports UI (app/views/family_exports/_list.html.erb), raw color utilities like bg-green-500/5, text-green-500, bg-red-500/5, text-red-500 are preferred over design system tokens for status indicators to maintain UI legibility and consistency with the existing import UI patterns. The user has indicated that design token replacements would make the UI illegible in this context.
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:23:17.441Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-09-23T22:23:17.441Z
Learning: Applies to {app/views/**,app/helpers/**,app/javascript/controllers/**} : Always start by referencing app/assets/tailwind/maybe-design-system.css to understand available primitives, functional tokens, and component tokens before styling
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:23:17.441Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-09-23T22:23:17.441Z
Learning: Applies to {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
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-10-24T12:37:31.878Z
Learnt from: jjmata
Repo: we-promise/sure PR: 77
File: app/assets/tailwind/google-sign-in.css:1-106
Timestamp: 2025-10-24T12:37:31.878Z
Learning: In app/assets/tailwind/google-sign-in.css: Google OAuth button styles (gsi-material-button classes) should be kept with their original hardcoded colors and fonts as provided by Google, even though they don't use design system tokens. This is a brand compliance exception.
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to **/*.{rb,js,jsx,ts,tsx} : Make changes atomic, testable, and explain their impact briefly in code suggestions.
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to app/assets/tailwind/maybe-design-system.css : NEVER add new styles to the design system CSS file without permission.
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:23:17.441Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-09-23T22:23:17.441Z
Learning: Applies to app/assets/tailwind/{maybe-design-system.css,application.css} : Do not create new styles in app/assets/tailwind/maybe-design-system.css or app/assets/tailwind/application.css without explicit permission
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : In Stimulus controllers, do not imperatively bind DOM events (e.g., addEventListener in connect); expose methods and let views trigger them via actions
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to app/{views,components}/**/*.html.erb : In ERB views, wire Stimulus interactions declaratively using data-action to call controller methods
Applied to files:
app/javascript/controllers/lunchflow_preload_controller.js
⏰ 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 (2)
app/javascript/controllers/lunchflow_preload_controller.js (2)
1-13: LGTM! Clean controller setup.The controller structure follows Stimulus conventions with appropriate targets (2, well under the guideline limit of 7), values for parameterization, and uses the connect lifecycle hook declaratively.
15-48: LGTM! Previous review concerns addressed.The CSRF token is now safely retrieved with null checking (lines 34-40), and the HTTP response status is validated before parsing JSON (lines 44-46). These improvements prevent crashes when the CSRF meta tag is missing or when the server returns non-2xx responses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/models/provider/lunchflow.rb(5 hunks)app/models/provider/lunchflow_adapter.rb(5 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
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/provider/lunchflow_adapter.rbapp/models/provider/lunchflow.rb
app/models/provider/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-design.mdc)
Concrete provider classes must be under the Provider:: namespace, inherit from Provider, wrap calls with with_provider_response, and raise on invalid/unavailable data
Files:
app/models/provider/lunchflow_adapter.rbapp/models/provider/lunchflow.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/models/provider/lunchflow_adapter.rbapp/models/provider/lunchflow.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/models/provider/lunchflow_adapter.rbapp/models/provider/lunchflow.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/provider/lunchflow_adapter.rbapp/models/provider/lunchflow.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/provider/lunchflow_adapter.rbapp/models/provider/lunchflow.rb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/models/provider/lunchflow_adapter.rbapp/models/provider/lunchflow.rb
🧬 Code graph analysis (1)
app/models/provider/lunchflow_adapter.rb (1)
app/models/provider/configurable.rb (1)
configure(55-59)
⏰ 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 (4)
app/models/provider/lunchflow.rb (1)
24-29: LGTM: Consistent branding update.The log messages have been properly updated to use "Lunch Flow API" for consistency.
app/models/provider/lunchflow_adapter.rb (3)
9-31: LGTM: Consistent branding updates in configuration.All user-facing strings in the configuration block have been properly updated to "Lunch Flow" for consistency. The code identifiers and external URLs are appropriately unchanged.
37-53: LGTM: Comment updates align with branding.Comments have been consistently updated to reference "Lunch Flow" instead of "Lunchflow".
68-84: LGTM: Consistent naming in institution metadata handling.The comments and log messages have been properly updated to use "Lunch Flow" terminology.
| Rails.logger.error "Lunch Flow API: Bad request - #{response.body}" | ||
| raise LunchflowError.new("Bad request to Lunchflow API: #{response.body}", :bad_request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent naming: one instance of "Lunchflow API" remains.
Line 96 was updated to "Lunch Flow API", but line 97 still contains "Lunchflow API" in the error message. This creates inconsistency with the rest of the branding updates in this PR.
Apply this diff to complete the branding update:
- raise LunchflowError.new("Bad request to Lunchflow API: #{response.body}", :bad_request)
+ raise LunchflowError.new("Bad request to Lunch Flow API: #{response.body}", :bad_request)🤖 Prompt for AI Agents
In app/models/provider/lunchflow.rb around lines 96 to 97, the log message was
updated to "Lunch Flow API" but the raised error message still uses "Lunchflow
API"; update the raise to use the same branding ("Lunch Flow API") so both lines
are consistent, i.e., change the error message string passed to LunchflowError
to read "Bad request to Lunch Flow API: #{response.body}".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/models/lunchflow_item.rb(2 hunks)app/models/lunchflow_item/importer.rb(3 hunks)app/models/lunchflow_item/syncer.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
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/lunchflow_item/syncer.rbapp/models/lunchflow_item.rbapp/models/lunchflow_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/lunchflow_item/syncer.rbapp/models/lunchflow_item.rbapp/models/lunchflow_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/lunchflow_item/syncer.rbapp/models/lunchflow_item.rbapp/models/lunchflow_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/lunchflow_item/syncer.rbapp/models/lunchflow_item.rbapp/models/lunchflow_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/lunchflow_item/syncer.rbapp/models/lunchflow_item.rbapp/models/lunchflow_item/importer.rb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/models/lunchflow_item/syncer.rbapp/models/lunchflow_item.rbapp/models/lunchflow_item/importer.rb
🧠 Learnings (4)
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
Repo: we-promise/sure PR: 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/lunchflow_item/syncer.rbapp/models/lunchflow_item.rbapp/models/lunchflow_item/importer.rb
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
Repo: we-promise/sure PR: 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/lunchflow_item/syncer.rbapp/models/lunchflow_item.rbapp/models/lunchflow_item/importer.rb
📚 Learning: 2025-09-13T11:36:45.479Z
Learnt from: alessiocappa
Repo: we-promise/sure PR: 141
File: app/models/enable_banking_account/processor.rb:72-79
Timestamp: 2025-09-13T11:36:45.479Z
Learning: The balance_calculator method pattern is consistent across banking integrations in the codebase. Both PlaidAccount::Processor and EnableBankingAccount::Processor use the same approach: `balance = account.current_balance || account.available_balance || 0` followed by `OpenStruct.new(balance: balance, cash_balance: balance)` without explicit requires, and include identical comments about not distinguishing cash vs non-cash balances for non-investment accounts.
Applied to files:
app/models/lunchflow_item/importer.rb
📚 Learning: 2025-09-13T11:27:55.271Z
Learnt from: alessiocappa
Repo: we-promise/sure PR: 141
File: app/controllers/enable_banking_items_controller.rb:48-62
Timestamp: 2025-09-13T11:27:55.271Z
Learning: The `create_enable_banking_item!` method in `Family::EnableBankingConnectable` uses `find_or_create_by(id: enable_banking_id)` to either find an existing EnableBankingItem or create a new one, so it automatically handles updates to existing items without creating duplicates.
Applied to files:
app/models/lunchflow_item/importer.rb
⏰ 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/views/lunchflow_items/select_accounts.html.erb (1)
18-40: Duplicate of logic in select_existing_account.html.erb.This block is nearly identical to lines 18-40 in
app/views/lunchflow_items/select_existing_account.html.erb, with only the input type (checkbox vs radio) differing. Refer to the review comment in that file for the refactoring suggestion to extract this into a shared partial.
🧹 Nitpick comments (2)
app/views/lunchflow_items/select_existing_account.html.erb (1)
35-39: Consider adding programmatic association for screen reader accessibility.The error message is visually displayed but not programmatically associated with the radio button. Consider adding an
idto the error div and anaria-describedbyattribute to the radio button for better screen reader support.Example:
-<%= radio_button_tag "lunchflow_account_id", account[:id], false, disabled: has_blank_name, class: "mt-1" %> +<%= radio_button_tag "lunchflow_account_id", account[:id], false, disabled: has_blank_name, class: "mt-1", aria: { describedby: has_blank_name ? "error-#{account[:id]}" : nil } %> ... <% if has_blank_name %> - <div class="text-xs text-error mt-1"> + <div id="error-<%= account[:id] %>" class="text-xs text-error mt-1"> <%= t(".configure_name_in_lunchflow") %> </div> <% end %>app/views/lunchflow_items/select_accounts.html.erb (1)
35-39: Consider adding programmatic association for screen reader accessibility.Same accessibility enhancement applies here as in
select_existing_account.html.erb: addidto the error div andaria-describedbyto the checkbox for better screen reader support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/controllers/lunchflow_items_controller.rb(4 hunks)app/views/lunchflow_items/select_accounts.html.erb(2 hunks)app/views/lunchflow_items/select_existing_account.html.erb(2 hunks)config/locales/views/lunchflow_items/en.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/controllers/lunchflow_items_controller.rb
🧰 Additional context used
📓 Path-based instructions (11)
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/locales/views/lunchflow_items/en.yml
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/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.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/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.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/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.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/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.html.erb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/views/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.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/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.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/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.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/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.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/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.html.erb
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 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
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/**/*.html.erb : Prefer native client-side form validation when possible
Applied to files:
app/views/lunchflow_items/select_accounts.html.erbapp/views/lunchflow_items/select_existing_account.html.erb
📚 Learning: 2025-09-13T11:36:45.479Z
Learnt from: alessiocappa
Repo: we-promise/sure PR: 141
File: app/models/enable_banking_account/processor.rb:72-79
Timestamp: 2025-09-13T11:36:45.479Z
Learning: The balance_calculator method pattern is consistent across banking integrations in the codebase. Both PlaidAccount::Processor and EnableBankingAccount::Processor use the same approach: `balance = account.current_balance || account.available_balance || 0` followed by `OpenStruct.new(balance: balance, cash_balance: balance)` without explicit requires, and include identical comments about not distinguishing cash vs non-cash balances for non-investment accounts.
Applied to files:
app/views/lunchflow_items/select_accounts.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
🔇 Additional comments (3)
app/views/lunchflow_items/select_existing_account.html.erb (1)
48-48: Good enhancement to cancel behavior.Adding the
DS--dialog#closeaction alongside the Turbo Frame navigation improves the UX by properly closing the dialog component.config/locales/views/lunchflow_items/en.yml (1)
18-64: Translation keys look good.The new translation keys are well-structured, follow existing patterns, and properly implement pluralization. The messages are clear and user-friendly.
Note: The duplication of
configure_name_in_lunchflowandno_name_placeholderkeys acrossselect_accountsandselect_existing_accountsections could be consolidated into a shared namespace (e.g.,lunchflow_items.shared) if the views are refactored to use a shared partial as suggested in the view file reviews.app/views/lunchflow_items/select_accounts.html.erb (1)
48-48: Good enhancement to cancel behavior.The addition of
DS--dialog#closeaction improves the dialog UX consistently across both selection views.
| <% has_blank_name = account[:name].blank? %> | ||
| <label class="flex items-start gap-3 p-3 border <%= has_blank_name ? 'border-error bg-error/5' : 'border-primary' %> rounded-lg <%= has_blank_name ? 'cursor-not-allowed opacity-60' : 'hover:bg-subtle cursor-pointer' %> transition-colors"> | ||
| <%= radio_button_tag "lunchflow_account_id", account[:id], false, disabled: has_blank_name, class: "mt-1" %> | ||
| <div class="flex-1"> | ||
| <div class="font-medium text-sm text-primary"> | ||
| <%= account[:name] %> | ||
| <div class="font-medium text-sm <%= has_blank_name ? 'text-error' : 'text-primary' %>"> | ||
| <% if has_blank_name %> | ||
| <%= t(".no_name_placeholder") %> | ||
| <% else %> | ||
| <%= account[:name] %> | ||
| <% end %> | ||
| </div> | ||
| <div class="text-xs text-secondary mt-1"> | ||
| <% if account[:iban].present? %> | ||
| <%= account[:iban] %> • | ||
| <% end %> | ||
| <%= account[:institution_name] %> • <%= account[:currency] %> • <%= account[:status] %> | ||
| </div> | ||
| <% if has_blank_name %> | ||
| <div class="text-xs text-error mt-1"> | ||
| <%= t(".configure_name_in_lunchflow") %> | ||
| </div> | ||
| <% end %> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated blank name validation logic into a shared partial.
This exact block (lines 18-40) is duplicated in app/views/lunchflow_items/select_accounts.html.erb. Extract the account rendering logic into a partial (e.g., _lunchflow_account_item.html.erb) that accepts the account data and input type (radio/checkbox) as parameters.
Example partial structure:
<%# app/views/lunchflow_items/_lunchflow_account_item.html.erb %>
<%# locals: account:, input_type: :radio/:checkbox, input_name:, disabled_class: "mt-1" %>
<% has_blank_name = account[:name].blank? %>
<label class="flex items-start gap-3 p-3 border <%= has_blank_name ? 'border-error bg-error/5' : 'border-primary' %> rounded-lg <%= has_blank_name ? 'cursor-not-allowed opacity-60' : 'hover:bg-subtle cursor-pointer' %> transition-colors">
<% if input_type == :radio %>
<%= radio_button_tag input_name, account[:id], false, disabled: has_blank_name, class: disabled_class %>
<% else %>
<%= check_box_tag input_name, account[:id], false, disabled: has_blank_name, class: disabled_class %>
<% end %>
<div class="flex-1">
<!-- rest of the account display logic -->
</div>
</label>Then use: <%= render "lunchflow_account_item", account: account, input_type: :radio, input_name: "lunchflow_account_id" %>
🤖 Prompt for AI Agents
In app/views/lunchflow_items/select_existing_account.html.erb around lines 18-40
(duplicate of the block in app/views/lunchflow_items/select_accounts.html.erb),
extract the account rendering block into a partial (e.g.,
app/views/lunchflow_items/_lunchflow_account_item.html.erb) that receives
locals: account, input_type(:radio or :checkbox), input_name (string) and an
optional input_class; move the existing markup into that partial, compute
has_blank_name = account[:name].blank? inside it, render either radio_button_tag
or check_box_tag based on input_type with disabled set to has_blank_name and
class set to input_class, keep the rest of the DOM/conditional text unchanged,
then replace the duplicated blocks in both select_existing_account.html.erb and
select_accounts.html.erb with a render call passing the proper locals (e.g.,
render "lunchflow_account_item", account: account, input_type: :radio,
input_name: "lunchflow_account_id", input_class: "mt-1").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/models/lunchflow_item/importer.rb (1)
32-33: Type mismatch resolved with string normalization.The
.map(&:to_s)correctly addresses the type mismatch issue from the previous review where integer account_ids from the database weren't matching string IDs from the API.Consider filtering out nil account_ids before mapping for extra clarity:
- existing_account_ids = lunchflow_item.lunchflow_accounts.pluck(:account_id).map(&:to_s) + existing_account_ids = lunchflow_item.lunchflow_accounts + .where.not(account_id: nil) + .pluck(:account_id) + .map(&:to_s)While not critical (line 37 checks presence anyway), this makes the intent explicit and prevents potential edge cases where nil converts to an empty string.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/models/lunchflow_item/importer.rb(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
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/lunchflow_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/lunchflow_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/lunchflow_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/lunchflow_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/lunchflow_item/importer.rb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/models/lunchflow_item/importer.rb
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 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
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
Repo: we-promise/sure PR: 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/lunchflow_item/importer.rb
📚 Learning: 2025-09-13T11:40:26.913Z
Learnt from: alessiocappa
Repo: we-promise/sure PR: 141
File: app/models/enable_banking_entry/processor.rb:10-12
Timestamp: 2025-09-13T11:40:26.913Z
Learning: The plaid_id field is used consistently across multiple banking integrations (Plaid, SimpleFin, and Enable Banking) as a common external identifier field. A comprehensive refactor to use a proper external_id field should be done across all integrations in a separate PR to maintain consistency and avoid scope creep.
Applied to files:
app/models/lunchflow_item/importer.rb
📚 Learning: 2025-09-13T11:27:55.271Z
Learnt from: alessiocappa
Repo: we-promise/sure PR: 141
File: app/controllers/enable_banking_items_controller.rb:48-62
Timestamp: 2025-09-13T11:27:55.271Z
Learning: The `create_enable_banking_item!` method in `Family::EnableBankingConnectable` uses `find_or_create_by(id: enable_banking_id)` to either find an existing EnableBankingItem or create a new one, so it automatically handles updates to existing items without creating duplicates.
Applied to files:
app/models/lunchflow_item/importer.rb
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
Repo: we-promise/sure PR: 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/lunchflow_item/importer.rb
⏰ 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 (3)
app/models/lunchflow_item/importer.rb (3)
36-40: LGTM: Correctly filters to existing accounts only.The filtering logic properly converts account IDs to strings and checks membership before processing, aligning with the PR's intent to update only pre-selected accounts without creating new ones during synchronization.
59-59: LGTM: Efficient filtering with Account.visible scope.The join with
Account.visiblecorrectly scopes transaction fetching to active accounts without introducing N+1 queries, and aligns with the PR's objective to process only linked accounts with active status.
141-149: LGTM: Defensive guard prevents unintended account creation.Changing from
find_or_initialize_bytofind_bywith an early return correctly enforces the "no new accounts during sync" policy. While this check is redundant with the filtering at lines 39-40, it provides defense against race conditions and makesimport_accountself-contained.
jjmata
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love these improvements, LuchFlow is now as seamless as SimpleFIN!
* Fix lunch flow pre-loading and UX * Small UX fixes - Proper closing of modal on cancel - Preload on new account already * Review comments * Fix json error * Delete .claude/settings.local.json Signed-off-by: soky srm <[email protected]> * Lunch Flow brand (again :-) * FIX process only linked accounts * FIX disable accounts with no name * Fix string normalization --------- Signed-off-by: soky srm <[email protected]> Co-authored-by: Juan José Mata <[email protected]>
Summary by CodeRabbit
New Features
Bug Fixes
Style / Text