-
Notifications
You must be signed in to change notification settings - Fork 96
More rebranding changes #159
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
Conversation
WalkthroughRename/branding swap from "Maybe" → "Sure" across configs, views, models, tests, seeds and docs; update defaults (DB/Sidekiq/cable), OAuth redirect URIs, Plaid client_name, demo/test data, TOTP issuer, release module name, and remove a negation in SimpleFin amount parsing. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant R as Rails Router
participant A as App (controllers)
Note over R,A: /privacy and /terms redirects changed to internal about:blank
U->>R: GET /privacy
R->>A: route matched
A-->>U: 302 Redirect to about:blank
U->>R: GET /terms
R->>A: route matched
A-->>U: 302 Redirect to about:blank
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (7)test/fixtures/**/*.yml📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
Files:
test/fixtures/**/*📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/test/fixtures/**/*.yml📄 CodeRabbit inference engine (CLAUDE.md)
Files:
test/**/*_test.rb📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
Files:
**/*.rb📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rb,js,jsx,ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/test/**/*.rb📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ 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)
🔇 Additional comments (4)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/views/onboardings/trial.html.erb (1)
19-21: Replace remaining "Maybe" strings in views with "Sure" (preserve historical "Maybe Finance" attribution in footer)rg found these view files still referencing "Maybe" — update the UI copy accordingly:
- app/views/onboardings/trial.html.erb (19-21, 27-29, 33-38, 76-79, 108-111): replace "Maybe" → "Sure"; change "Maybe AI" → "AI" where appropriate.
- app/views/layouts/doorkeeper/application.html.erb: "Maybe Authorization" → "Sure Authorization".
- app/views/subscriptions/upgrade.html.erb: replace UI copy ("Maybe" gradient, "To continue using Maybe…", "Subscribe and unlock Maybe") → "Sure".
- app/views/settings/api_keys/new.html.erb: "access your Maybe data…" → "access your Sure data…".
- app/views/pages/redis_configuration_error.html.erb: title/body references to "Maybe" → "Sure" (self‑hosted Sure installation).
- app/views/impersonation_sessions/_approval_bar.html.erb: replace "Maybe support staff"/"Maybe Finance team" → "Sure support staff"/"Sure team".
- app/views/import/configurations/_trade_import.html.erb: "Maybe will not backfill price history." → "Sure will not backfill price history."
- app/views/layouts/shared/_footer.html.erb: keep "(community fork of Maybe Finance)" as historical attribution — do NOT replace the "Maybe Finance" text.
🧹 Nitpick comments (9)
app/views/family_exports/new.html.erb (1)
28-31: Prefer design-system tokens over raw amber colors.For consistency with the rest of the app (e.g., text-positive, bg-container), consider replacing raw amber utility classes with design tokens (e.g., a warning/info callout style) if available in maybe-design-system.css.
docs/hosting/docker.md (1)
170-174: Service name mismatch (“app”) when restarting.The compose defines “web” and “worker,” not “app.”
Use:
-docker compose up --no-deps -d app # This restarts the app using the newest version +docker compose up --no-deps -d web worker # This restarts the app using the newest versionapp/models/provider/plaid.rb (1)
4-4: Rename MAYBE_SUPPORTED_PLAID_PRODUCTS for consistency.Branding nit: rename to a neutral constant name.
Apply this diff:
- MAYBE_SUPPORTED_PLAID_PRODUCTS = %w[transactions investments liabilities].freeze + SUPPORTED_PLAID_PRODUCTS = %w[transactions investments liabilities].freeze @@ - MAYBE_SUPPORTED_PLAID_PRODUCTS - [ get_primary_product(accountable_type) ] + SUPPORTED_PLAID_PRODUCTS - [ get_primary_product(accountable_type) ]Also applies to: 198-199
app/models/user.rb (1)
196-196: TOTP issuer updated — LGTM.Consider centralizing "Sure Finances" in a single constant to avoid drift with Plaid client_name.
config/initializers/version.rb (1)
1-1: Namespace rename to Sure — LGTM.No code-level references to
Maybe::orMaybe.were found; literal "Maybe" strings remain in views/locales/README/tests (e.g. app/views/onboardings/trial.html.erb, app/views/subscriptions/upgrade.html.erb, config/locales/, README.md, test/, public/422.html). Update user-facing/branding strings if you intend a full rebrand.app/models/demo/generator.rb (1)
136-140: Demo password change — tests/docs checked; centralize demo password (optional)
- Verified: app/models/demo/generator.rb (create_family_and_users!) sets password "Password!1"; ripgrep found no other "Password!1" occurrences.
- Tests use user_password_test in test/test_helper.rb (value "maybetestpassword817983172") so test logins won’t break.
- Optional: centralize demo password, e.g. DEMO_PASSWORD = ENV.fetch("DEMO_PASSWORD","Password!1") and replace password: "Password!1" with password: DEMO_PASSWORD.
config/application.rb (1)
9-9: App namespace renamed to Sure — no code-level Maybe constants found; update remaining UI/locales/docs if you want a full rename.
- Verified: no "module Maybe" or "Maybe::" usages in code.
- Remaining textual occurrences (examples): app/views/onboardings/trial.html.erb, app/views/layouts/shared/_footer.html.erb, config/locales/views/, public/422.html, README.md, test/
db/seeds/oauth_applications.rb (1)
4-9: Make seeds idempotent and handle rename from "Maybe iOS" to "Sure iOS" to avoid duplicate OAuth apps.If an older "Maybe iOS" app exists, this will create a second app instead of renaming/updating. Prefer updating-or-renaming the existing record.
Apply this diff:
-# Sure iOS App -ios_app = Doorkeeper::Application.find_or_create_by(name: "Sure iOS") do |app| - app.redirect_uri = "sureapp://oauth/callback" - app.scopes = "read_accounts read_transactions read_balances" - app.confidential = false # Public client (mobile app) -end +# Sure iOS App +ios_app = Doorkeeper::Application.find_by(name: "Sure iOS") || + Doorkeeper::Application.find_by(name: "Maybe iOS") || + Doorkeeper::Application.new + +ios_app.name = "Sure iOS" +ios_app.redirect_uri = "sureapp://oauth/callback" +ios_app.scopes = "read_accounts read_transactions read_balances" +ios_app.confidential = false # Public client (mobile app) +ios_app.save!app/views/users/_user_menu.html.erb (1)
23-26: Add rel="noopener noreferrer" to external links with target="_blank".Prevents reverse‑tabnabbing; cheap hardening.
Apply this diff:
- <%= link_to Sure.version.to_release_tag, "https://github.com/we-promise/sure/releases/tag/#{Sure.version.to_release_tag}", target: "_blank", class: "hover:underline" %> + <%= link_to Sure.version.to_release_tag, "https://github.com/we-promise/sure/releases/tag/#{Sure.version.to_release_tag}", target: "_blank", rel: "noopener noreferrer", class: "hover:underline" %> @@ - (<%= link_to Sure.commit_sha.first(7), "https://github.com/we-promise/sure/commit/#{Sure.commit_sha}", target: "_blank", class: "hover:underline" %>) + (<%= link_to Sure.commit_sha.first(7), "https://github.com/we-promise/sure/commit/#{Sure.commit_sha}", target: "_blank", rel: "noopener noreferrer", class: "hover:underline" %>)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
app/controllers/subscriptions_controller.rb(2 hunks)app/models/demo/generator.rb(1 hunks)app/models/provider/plaid.rb(1 hunks)app/models/user.rb(1 hunks)app/views/chats/_ai_consent.html.erb(2 hunks)app/views/chats/_ai_greeting.html.erb(1 hunks)app/views/family_exports/new.html.erb(1 hunks)app/views/onboardings/goals.html.erb(1 hunks)app/views/onboardings/trial.html.erb(1 hunks)app/views/shared/_app_version.html.erb(1 hunks)app/views/users/_user_menu.html.erb(1 hunks)compose.example.yml(1 hunks)config/application.rb(1 hunks)config/cable.yml(1 hunks)config/database.yml(1 hunks)config/initializers/version.rb(1 hunks)db/seeds/oauth_applications.rb(1 hunks)docs/api/chats.md(1 hunks)docs/hosting/docker.md(1 hunks)lib/tasks/benchmarking.rake(1 hunks)test/components/previews/dialog_component_preview.rb(1 hunks)test/controllers/onboardings_controller_test.rb(1 hunks)test/controllers/subscriptions_controller_test.rb(2 hunks)test/integration/oauth_mobile_test.rb(1 hunks)test/models/user_test.rb(2 hunks)test/system/chats_test.rb(1 hunks)test/system/onboardings_test.rb(2 hunks)
🧰 Additional context used
📓 Path-based instructions (30)
{app/**/*.rb,app/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
{app/**/*.rb,app/**/*.erb}: UseCurrent.userfor the current user; do not usecurrent_userin Rails code and templates
UseCurrent.familyfor the current family; do not usecurrent_familyin Rails code and templates
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/models/provider/plaid.rbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/models/user.rbapp/views/users/_user_menu.html.erbapp/models/demo/generator.rbapp/controllers/subscriptions_controller.rbapp/views/shared/_app_version.html.erb
{app/javascript/controllers/**/*.@(js|ts),app/components/**/*.@(js|ts),app/views/**/*.html.erb,app/components/**/*.html.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Use declarative Stimulus actions in ERB (data-action) instead of imperative event listeners in controllers (e.g., avoid addEventListener in connect/initializers).
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
{app/components/**/*.@(js|ts),app/views/**/*.html.erb,app/components/**/*.html.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Component Stimulus controllers under app/components must only be used within their component views, not in app/views.
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.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/**}: Author styles assuming TailwindCSS v4.x is in use across the app
Always start by referencing app/assets/tailwind/maybe-design-system.css to understand the base primitives, functional tokens, and component tokens
Prefer functional tokens from maybe-design-system.css over raw colors/utilities (e.g., usetext-primaryovertext-white,bg-containeroverbg-white,border border-primaryoverborder border-gray-200)
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
app/views/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
app/views/**/*.html.erb: Use ViewComponents when the element has complex logic/styling, is reused across contexts, needs variants/sizes or slots, requires interactivity/Stimulus, or needs accessibility/ARIA support
Use partials when the element is primarily static HTML with minimal logic, used in few contexts, simple template content, has no variants/configuration, and is mainly for content organization
Prefer components over partials: if a suitable component exists in app/components, use it; otherwise consider a partial or create a component based on the criteria
Integrate Stimulus declaratively in views: HTML declares actions/targets; controllers respond (avoid imperative JS in templates)
app/views/**/*.html.erb: Prefer native semantic HTML elements (e.g., ,/) over custom JS components
Use Turbo Frames to segment pages instead of JS-driven client-side solutions where possible
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
{app/views/**/*.html.erb,app/components/**/*.html.erb}
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
{app/views/**/*.html.erb,app/components/**/*.html.erb}: Keep domain logic out of views; compute classes and logic in component/view helpers, not inside ERB templates
Pass data from Rails to Stimulus via data-*-value attributes; do not embed inline JavaScript in templates
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
{app/helpers/**/*.rb,app/views/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Always use the icon helper from application_helper.rb; never call lucide_icon directly
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
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.yml
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.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/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.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/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
**/app/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Always use the
iconhelper for icons in templates; never uselucide_icondirectly.
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
**/app/views/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/views/**/*.erb: Keep domain logic OUT of view templates; logic belongs in component files, not templates.
Use partials only for primarily static, simple HTML with minimal logic, and only when not likely to be reused.
Prefer native HTML over JavaScript components for UI elements (e.g., use<dialog>,<details><summary>).
Leverage Turbo frames for page sections over client-side JavaScript solutions.
Use query params (not localStorage/sessions) for client state management.
Perform server-side formatting for currencies, numbers, and dates in templates.
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/views/onboardings/goals.html.erbapp/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/models/provider/plaid.rbapp/views/family_exports/new.html.erbapp/views/onboardings/trial.html.erbapp/models/user.rbapp/views/users/_user_menu.html.erbapp/models/demo/generator.rbapp/controllers/subscriptions_controller.rbapp/views/shared/_app_version.html.erb
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/application.rbconfig/cable.ymlconfig/database.ymlconfig/initializers/version.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
config/application.rbtest/components/previews/dialog_component_preview.rbtest/controllers/onboardings_controller_test.rbapp/models/provider/plaid.rbapp/models/user.rbtest/system/onboardings_test.rbtest/controllers/subscriptions_controller_test.rbtest/system/chats_test.rbconfig/initializers/version.rbtest/integration/oauth_mobile_test.rbapp/models/demo/generator.rbdb/seeds/oauth_applications.rbtest/models/user_test.rbapp/controllers/subscriptions_controller.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:
config/application.rbtest/components/previews/dialog_component_preview.rbtest/controllers/onboardings_controller_test.rbapp/models/provider/plaid.rbapp/models/user.rbtest/system/onboardings_test.rbtest/controllers/subscriptions_controller_test.rbtest/system/chats_test.rbconfig/initializers/version.rbtest/integration/oauth_mobile_test.rbapp/models/demo/generator.rbdb/seeds/oauth_applications.rbtest/models/user_test.rbapp/controllers/subscriptions_controller.rb
**/test/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/test/**/*.rb: ALWAYS use Minitest and fixtures for Ruby tests; NEVER RSpec or factories.
Only test critical and important code paths; system tests should be used sparingly for critical user flows.
Files:
test/components/previews/dialog_component_preview.rbtest/controllers/onboardings_controller_test.rbtest/system/onboardings_test.rbtest/controllers/subscriptions_controller_test.rbtest/system/chats_test.rbtest/integration/oauth_mobile_test.rbtest/models/user_test.rb
app/views/**/_*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Name partials with an underscore prefix (e.g., _trend_change.html.erb, _form_errors.html.erb)
Files:
app/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/users/_user_menu.html.erbapp/views/shared/_app_version.html.erb
app/views/!(shared)/**/_*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Place context-specific partials in the relevant controller's view directory (not in shared/)
Files:
app/views/chats/_ai_greeting.html.erbapp/views/chats/_ai_consent.html.erbapp/views/users/_user_menu.html.erb
test/**/*_test.rb
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
test/**/*_test.rb: Use Minitest for all tests (do not write RSpec tests)
When many records are needed, use Rails test helpers to generate them and inline creation within the test
Test boundaries correctly: for query methods, assert outputs; for commands, assert that the command was called with correct params
Never test the implementation details of one class in another class’s test suite
Use the mocha gem for stubs and mocks
Prefer OpenStruct (or a small mock class) when creating mock instances
Only mock what’s necessary; don’t stub return values unless they are assertedName Minitest files with *_test.rb and mirror the app/ structure under test/
test/**/*_test.rb: ALWAYS use Minitest + fixtures + Mocha for tests; NEVER RSpec or FactoryBot.
Use Mocha for mocking in tests when necessary.
Use VCR for external API tests.
Files:
test/controllers/onboardings_controller_test.rbtest/system/onboardings_test.rbtest/controllers/subscriptions_controller_test.rbtest/system/chats_test.rbtest/integration/oauth_mobile_test.rbtest/models/user_test.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/models/**/*.rb: Place most business logic in app/models as POROs or model code rather than separate service objects
Prefer model methods that answer questions about themselves (e.g., Account#balance_series) over service-style objects
ActiveRecord validations may mirror DB-level constraints for UX but are not strictly required
Keep complex validations and business logic in ActiveRecord models rather than the databaseDomain 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.
Files:
app/models/provider/plaid.rbapp/models/user.rbapp/models/demo/generator.rb
app/{models,controllers}/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries; only optimize performance in critical/global paths
Files:
app/models/provider/plaid.rbapp/models/user.rbapp/models/demo/generator.rbapp/controllers/subscriptions_controller.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/plaid.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/plaid.rbapp/models/user.rbapp/models/demo/generator.rbapp/controllers/subscriptions_controller.rb
**/app/models/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Business logic should primarily reside in models; use concerns and POROs for organization.
Files:
app/models/provider/plaid.rbapp/models/user.rbapp/models/demo/generator.rb
test/system/**/*_test.rb
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
Use system tests sparingly
Files:
test/system/onboardings_test.rbtest/system/chats_test.rb
{app/controllers/**/*.rb,app/javascript/**/*.js}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Prefer URL query params for state over localStorage or sessions; use DB only if persistence is necessary
Files:
app/controllers/subscriptions_controller.rb
**/app/controllers/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Controllers should be skinny; business logic should go in models (
app/models/), not inapp/services/.
Files:
app/controllers/subscriptions_controller.rb
app/views/shared/**/_*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Place shared partials under app/views/shared/
Files:
app/views/shared/_app_version.html.erb
🧠 Learnings (2)
📓 Common learnings
Learnt from: alessiocappa
PR: we-promise/sure#141
File: app/views/settings/hostings/_enable_banking_settings.html.erb:32-38
Timestamp: 2025-09-13T12:16:48.300Z
Learning: In the Sure project settings forms, the user prefers to maintain consistency with existing patterns for credential handling, even when security improvements are identified, and address such concerns comprehensively in separate PRs rather than making isolated changes.
📚 Learning: 2025-09-13T11:40:26.913Z
Learnt from: alessiocappa
PR: we-promise/sure#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/provider/plaid.rb
🧬 Code graph analysis (1)
test/models/user_test.rb (1)
app/models/user.rb (2)
totp(195-197)provisioning_uri(154-157)
⏰ 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 (25)
lib/tasks/benchmarking.rake (2)
3-4: DB name consistent — no leftover 'maybe'/'possibly' benchmarking names foundSearched repo for sure_benchmarking / maybe_benchmarking / possibly_benchmarking and variants — only occurrence is the comment in lib/tasks/benchmarking.rake (lines 3–4); no config or docs references remain.
11-13: Require benchmark/ips in the rake task.Add
require "benchmark/ips"at the top of lib/tasks/benchmarking.rake (after the header comment) to ensure Benchmark.ips is available when running the task — repository search produced no evidence it's required elsewhere, so add this require or confirm it's loaded by other boot code.Suggested patch:
+# Ensure Benchmark.ips is available when running this task directly +require "benchmark/ips"app/views/onboardings/goals.html.erb (1)
18-19: LGTM on rebranding copy.Copy updates read well and align with the design tokens already in use.
config/cable.yml (1)
10-10: Redis channel prefix updated — looks good.This cleanly isolates production channels under the new brand.
If you have existing Redis keys from previous deployments, verify no tooling depends on the old "maybe_production" prefix.
app/views/onboardings/trial.html.erb (1)
65-69: LGTM on updated “Today” copy.Matches the project-wide rebrand.
config/database.yml (1)
12-20: DB defaults rebranded — good alignment.Defaults now match compose/example docs and other config.
Confirm rollout plan for existing self-hosted users who may have DBs named maybe_* (migration/backups, env overrides).
compose.example.yml (1)
31-33: DB env defaults updated — consistent with rebrand.Matches config/database.yml and docs changes.
test/system/chats_test.rb (1)
15-15: Test expectation updated — good.Aligns with the new “Enable AI Chats” copy in the consent view.
Double-check app/views/chats/_ai_consent.html.erb and any related partials also use “AI Chats” (no “Maybe AI” remnants).
app/models/provider/plaid.rb (1)
48-48: Plaid Link client_name updated — LGTM.Change is correct and consistent with the rebrand.
Please confirm the Plaid Dashboard app “Display name” matches “Sure Finances” across envs to avoid user-visible inconsistencies during Link.
docs/api/chats.md (1)
3-3: Rebrand copy update — LGTM.test/system/onboardings_test.rb (1)
41-41: Updated expectations to “Sure” — LGTM.Also applies to: 140-140
test/components/previews/dialog_component_preview.rb (1)
8-8: Preview copy updated — LGTM.test/controllers/onboardings_controller_test.rb (1)
85-85: Heading expectation updated — LGTM.Regex without the question mark is fine and resilient to copy tweaks.
test/integration/oauth_mobile_test.rb (1)
11-11: Doorkeeper app name updated — LGTM.Matches the new custom scheme and aligns with seed data.
test/controllers/subscriptions_controller_test.rb (1)
29-29: Flash message expectations updated correctly.Matches controller notice changes and keeps tests aligned with branding.
Also applies to: 74-74
app/views/shared/_app_version.html.erb (1)
2-2: Version source rename looks good.Using Sure.version aligns with the new namespace and related view updates.
test/models/user_test.rb (2)
109-109: Issuer updated in TOTP setup matches app code.
139-139: Provisioning URI expectation updated appropriately.Loose match on issuer avoids URL-encoding flakiness.
app/views/chats/_ai_consent.html.erb (3)
6-6: Heading copy update looks good.
10-10: Body copy aligns with the new branding and remains clear.
21-21: Button label updated consistently with heading.app/controllers/subscriptions_controller.rb (2)
33-33: Trial creation notice updated correctly.
55-55: Checkout success notice updated correctly.app/views/chats/_ai_greeting.html.erb (1)
5-5: Greeting copy updated appropriately; no logic impact.db/seeds/oauth_applications.rb (1)
6-6: Verify iOS URL scheme registration.Confirm the iOS app Info.plist has CFBundleURLSchemes including "sureapp" to match the new redirect URI.
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/models/simplefin_account/processor.rb (1)
46-65: Add a unique DB index on entries.plaid_id to enforce idempotencydb/schema.rb shows entries has a plaid_id column but no unique index (plaid_accounts/plaid_items do). Add a migration: add_index :entries, :plaid_id, unique: true, and guard the create in app/models/simplefin_account/processor.rb (lines ~46–65) against races (rely on DB uniqueness + rescue ActiveRecord::RecordNotUnique or use an upsert/find_or_create_by).
perf.rake (1)
9-11: Update stale brand reference in comment.Change “Maybe’s” to “Sure’s” to match the rebranding.
Apply this diff:
-# Custom auth helper for Maybe's session-based authentication +# Custom auth helper for Sure's session-based authentication
🧹 Nitpick comments (9)
app/models/simplefin_account/processor.rb (1)
71-79: Minor: mark the unusedcurrencyparam intentionally unused.Avoids “unused param” noise while preserving the signature.
- def parse_amount(amount_value, currency) + def parse_amount(amount_value, _currency)test/vcr_cassettes/plaid/link_token.yml (1)
8-8: Rebrand looks good; VCR won't fail on JSON body — address leftover "Maybe Finance" text
- app/models/provider/plaid.rb and test/vcr_cassettes/plaid/link_token.yml (plus tests/views) use "Sure Finances"; VCR is configured in test/test_helper.rb with no :body matcher found (VCR defaults to [:method, :uri]), so JSON ordering/whitespace in cassette bodies won't cause mismatches.
- Lingering "Maybe Finance" occurrences found in README.md and app/views/layouts/shared/_footer.html.erb — update if unintentional.
perf.rake (1)
17-17: Make benchmark user email configurable and improve error message.Use an ENV override with a clear failure message to avoid local edits when changing the user.
Apply this diff:
- user = User.find_by!(email: "[email protected]") + email = ENV.fetch("BENCH_USER_EMAIL", "[email protected]") + user = User.find_by(email: email) or raise( + ActiveRecord::RecordNotFound, + "Benchmark user not found: #{email}. Create it or set BENCH_USER_EMAIL." + )app/models/mobile_device.rb (1)
24-29: Avoid hardcoding redirect URI; centralize or allow ENV override.This eases future branding/app-id changes without code edits.
Apply this diff:
- app = Doorkeeper::Application.create!( - name: "Mobile App - #{device_id}", - redirect_uri: "sureapp://oauth/callback", # Custom scheme for mobile + redirect = ENV.fetch("MOBILE_OAUTH_REDIRECT_URI", "sureapp://oauth/callback") + app = Doorkeeper::Application.create!( + name: "Mobile App - #{device_id}", + redirect_uri: redirect, # Custom scheme for mobile scopes: "read_write", # Use the configured scope confidential: false # Public client for mobile )lib/tasks/data_migration.rake (1)
10-18: Optional: Skip updates when already set to reduce API calls and rate‑limit risk.Check the current webhook and no‑op if it matches. Falls back to update if the lookup fails.
eu_items.find_each do |item| - request = Plaid::ItemWebhookUpdateRequest.new( + webhook_url = ENV.fetch("PLAID_EU_WEBHOOK_URL", "https://app.sure.am/webhooks/plaid_eu") + + # Idempotency: skip if already pointed at the target URL + begin + current_webhook = provider.client.item_get( + Plaid::ItemGetRequest.new(access_token: item.access_token) + ).item.webhook + if current_webhook == webhook_url + puts "Skipping Plaid item #{item.plaid_id} (already set)" + next + end + rescue => _e + # Proceed with update if lookup fails + end + + request = Plaid::ItemWebhookUpdateRequest.new( access_token: item.access_token, - webhook: "https://app.sure.am/webhooks/plaid_eu" + webhook: webhook_url )app/views/layouts/doorkeeper/application.html.erb (2)
23-23: Consider centralizing brand name to avoid future rebranding churnIf a brand helper/constant exists, prefer it over hard-coding “Sure” (e.g., I18n key or Brand.name) for maintainability.
19-19: Add alt text to logo for accessibilityProvide descriptive alt text for the brand mark.
-<%= image_tag "logo-color.png", class: "w-16 mb-6" %> +<%= image_tag "logo-color.png", alt: "Sure", class: "w-16 mb-6" %>config/environments/test.rb (1)
27-27: Don’t overwrite CI/environment-provided EMAIL_SENDER in testsUse ||= to set a default without clobbering existing env, keeping tests configurable.
-ENV["EMAIL_SENDER"] = "[email protected]" +ENV["EMAIL_SENDER"] ||= "[email protected]"app/views/onboardings/trial.html.erb (1)
28-28: Fix double space in sentenceMinor microcopy polish.
- Starting the trial activates your account for Sure. You won't need to enter payment details. + Starting the trial activates your account for Sure. You won't need to enter payment details.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
README.md(1 hunks)app/controllers/concerns/self_hostable.rb(1 hunks)app/models/account.rb(1 hunks)app/models/assistant/configurable.rb(2 hunks)app/models/demo/generator.rb(3 hunks)app/models/family_export.rb(1 hunks)app/models/import/row.rb(1 hunks)app/models/mobile_device.rb(1 hunks)app/models/plaid_account/investments/balance_calculator.rb(1 hunks)app/models/plaid_account/investments/security_resolver.rb(1 hunks)app/models/plaid_account/transactions/category_matcher.rb(1 hunks)app/models/provider/plaid.rb(3 hunks)app/models/security/health_checker.rb(1 hunks)app/models/simplefin_account/processor.rb(1 hunks)app/views/impersonation_sessions/_approval_bar.html.erb(1 hunks)app/views/import/configurations/_trade_import.html.erb(1 hunks)app/views/layouts/doorkeeper/application.html.erb(1 hunks)app/views/onboardings/trial.html.erb(4 hunks)app/views/pages/redis_configuration_error.html.erb(4 hunks)app/views/settings/api_keys/new.html.erb(1 hunks)app/views/settings/preferences/show.html.erb(1 hunks)app/views/subscriptions/upgrade.html.erb(2 hunks)config/environments/test.rb(1 hunks)config/initializers/sidekiq.rb(1 hunks)config/routes.rb(1 hunks)docs/hosting/docker.md(2 hunks)lib/tasks/benchmarking.rake(2 hunks)lib/tasks/data_migration.rake(1 hunks)perf.rake(1 hunks)test/controllers/impersonation_sessions_controller_test.rb(5 hunks)test/controllers/mfa_controller_test.rb(2 hunks)test/fixtures/impersonation_sessions.yml(1 hunks)test/fixtures/users.yml(1 hunks)test/mailers/email_confirmation_mailer_test.rb(1 hunks)test/models/family_export_test.rb(1 hunks)test/models/impersonation_session_test.rb(2 hunks)test/test_helper.rb(1 hunks)test/vcr_cassettes/plaid/link_token.yml(1 hunks)test/vcr_cassettes/stripe/checkout_session.yml(1 hunks)
✅ Files skipped from review due to trivial changes (20)
- test/controllers/mfa_controller_test.rb
- README.md
- app/views/settings/api_keys/new.html.erb
- test/vcr_cassettes/stripe/checkout_session.yml
- app/models/assistant/configurable.rb
- app/models/account.rb
- app/views/pages/redis_configuration_error.html.erb
- app/models/import/row.rb
- app/controllers/concerns/self_hostable.rb
- app/views/import/configurations/_trade_import.html.erb
- app/models/plaid_account/transactions/category_matcher.rb
- test/fixtures/impersonation_sessions.yml
- app/models/plaid_account/investments/security_resolver.rb
- app/models/plaid_account/investments/balance_calculator.rb
- app/views/subscriptions/upgrade.html.erb
- config/routes.rb
- app/models/security/health_checker.rb
- app/views/settings/preferences/show.html.erb
- app/models/family_export.rb
- test/mailers/email_confirmation_mailer_test.rb
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/hosting/docker.md
- app/models/provider/plaid.rb
- app/models/demo/generator.rb
- lib/tasks/benchmarking.rake
🧰 Additional context used
📓 Path-based instructions (29)
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/environments/test.rbconfig/initializers/sidekiq.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
config/environments/test.rbtest/models/impersonation_session_test.rbtest/controllers/impersonation_sessions_controller_test.rbapp/models/mobile_device.rbconfig/initializers/sidekiq.rbtest/models/family_export_test.rbtest/test_helper.rbapp/models/simplefin_account/processor.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:
config/environments/test.rbtest/models/impersonation_session_test.rbtest/controllers/impersonation_sessions_controller_test.rbapp/models/mobile_device.rbconfig/initializers/sidekiq.rbtest/models/family_export_test.rbtest/test_helper.rbapp/models/simplefin_account/processor.rb
test/**/*_test.rb
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
test/**/*_test.rb: Use Minitest for all tests (do not write RSpec tests)
When many records are needed, use Rails test helpers to generate them and inline creation within the test
Test boundaries correctly: for query methods, assert outputs; for commands, assert that the command was called with correct params
Never test the implementation details of one class in another class’s test suite
Use the mocha gem for stubs and mocks
Prefer OpenStruct (or a small mock class) when creating mock instances
Only mock what’s necessary; don’t stub return values unless they are assertedName Minitest files with *_test.rb and mirror the app/ structure under test/
test/**/*_test.rb: ALWAYS use Minitest + fixtures + Mocha for tests; NEVER RSpec or FactoryBot.
Use Mocha for mocking in tests when necessary.
Use VCR for external API tests.
Files:
test/models/impersonation_session_test.rbtest/controllers/impersonation_sessions_controller_test.rbtest/models/family_export_test.rb
**/test/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/test/**/*.rb: ALWAYS use Minitest and fixtures for Ruby tests; NEVER RSpec or factories.
Only test critical and important code paths; system tests should be used sparingly for critical user flows.
Files:
test/models/impersonation_session_test.rbtest/controllers/impersonation_sessions_controller_test.rbtest/models/family_export_test.rbtest/test_helper.rb
{app/**/*.rb,app/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
{app/**/*.rb,app/**/*.erb}: UseCurrent.userfor the current user; do not usecurrent_userin Rails code and templates
UseCurrent.familyfor the current family; do not usecurrent_familyin Rails code and templates
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/models/mobile_device.rbapp/views/layouts/doorkeeper/application.html.erbapp/models/simplefin_account/processor.rb
{app/javascript/controllers/**/*.@(js|ts),app/components/**/*.@(js|ts),app/views/**/*.html.erb,app/components/**/*.html.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Use declarative Stimulus actions in ERB (data-action) instead of imperative event listeners in controllers (e.g., avoid addEventListener in connect/initializers).
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.html.erb
{app/components/**/*.@(js|ts),app/views/**/*.html.erb,app/components/**/*.html.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Component Stimulus controllers under app/components must only be used within their component views, not in app/views.
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.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/**}: Author styles assuming TailwindCSS v4.x is in use across the app
Always start by referencing app/assets/tailwind/maybe-design-system.css to understand the base primitives, functional tokens, and component tokens
Prefer functional tokens from maybe-design-system.css over raw colors/utilities (e.g., usetext-primaryovertext-white,bg-containeroverbg-white,border border-primaryoverborder border-gray-200)
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.html.erb
app/views/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
app/views/**/*.html.erb: Use ViewComponents when the element has complex logic/styling, is reused across contexts, needs variants/sizes or slots, requires interactivity/Stimulus, or needs accessibility/ARIA support
Use partials when the element is primarily static HTML with minimal logic, used in few contexts, simple template content, has no variants/configuration, and is mainly for content organization
Prefer components over partials: if a suitable component exists in app/components, use it; otherwise consider a partial or create a component based on the criteria
Integrate Stimulus declaratively in views: HTML declares actions/targets; controllers respond (avoid imperative JS in templates)
app/views/**/*.html.erb: Prefer native semantic HTML elements (e.g., ,/) over custom JS components
Use Turbo Frames to segment pages instead of JS-driven client-side solutions where possible
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.html.erb
{app/views/**/*.html.erb,app/components/**/*.html.erb}
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
{app/views/**/*.html.erb,app/components/**/*.html.erb}: Keep domain logic out of views; compute classes and logic in component/view helpers, not inside ERB templates
Pass data from Rails to Stimulus via data-*-value attributes; do not embed inline JavaScript in templates
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.html.erb
app/views/**/_*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Name partials with an underscore prefix (e.g., _trend_change.html.erb, _form_errors.html.erb)
Files:
app/views/impersonation_sessions/_approval_bar.html.erb
app/views/!(shared)/**/_*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Place context-specific partials in the relevant controller's view directory (not in shared/)
Files:
app/views/impersonation_sessions/_approval_bar.html.erb
{app/helpers/**/*.rb,app/views/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Always use the icon helper from application_helper.rb; never call lucide_icon directly
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.html.erb
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.yml
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.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/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.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/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.html.erb
**/app/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Always use the
iconhelper for icons in templates; never uselucide_icondirectly.
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.html.erb
**/app/views/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/views/**/*.erb: Keep domain logic OUT of view templates; logic belongs in component files, not templates.
Use partials only for primarily static, simple HTML with minimal logic, and only when not likely to be reused.
Prefer native HTML over JavaScript components for UI elements (e.g., use<dialog>,<details><summary>).
Leverage Turbo frames for page sections over client-side JavaScript solutions.
Use query params (not localStorage/sessions) for client state management.
Perform server-side formatting for currencies, numbers, and dates in templates.
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/views/layouts/doorkeeper/application.html.erb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/views/impersonation_sessions/_approval_bar.html.erbapp/views/onboardings/trial.html.erbapp/models/mobile_device.rbapp/views/layouts/doorkeeper/application.html.erbapp/models/simplefin_account/processor.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/models/**/*.rb: Place most business logic in app/models as POROs or model code rather than separate service objects
Prefer model methods that answer questions about themselves (e.g., Account#balance_series) over service-style objects
ActiveRecord validations may mirror DB-level constraints for UX but are not strictly required
Keep complex validations and business logic in ActiveRecord models rather than the databaseDomain 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.
Files:
app/models/mobile_device.rbapp/models/simplefin_account/processor.rb
app/{models,controllers}/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries; only optimize performance in critical/global paths
Files:
app/models/mobile_device.rbapp/models/simplefin_account/processor.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/models/mobile_device.rbapp/models/simplefin_account/processor.rb
**/app/models/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Business logic should primarily reside in models; use concerns and POROs for organization.
Files:
app/models/mobile_device.rbapp/models/simplefin_account/processor.rb
test/fixtures/**/*.yml
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
test/fixtures/**/*.yml: Use Rails fixtures for test data
Keep fixtures to a minimum (2–3 base-case records per model; create edge cases inline in tests)Keep fixtures minimal: 2–3 per model for base cases.
Files:
test/fixtures/users.yml
test/fixtures/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Use test/fixtures for fixtures (and existing VCR cassettes if present)
Files:
test/fixtures/users.yml
**/test/fixtures/**/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Keep fixtures minimal—2-3 per model for base cases; create edge cases in test context.
Files:
test/fixtures/users.yml
app/views/layouts/**/*.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Be mindful of loading large data payloads in global layouts
Files:
app/views/layouts/doorkeeper/application.html.erb
🧠 Learnings (4)
📓 Common learnings
Learnt from: alessiocappa
PR: we-promise/sure#141
File: app/views/settings/hostings/_enable_banking_settings.html.erb:32-38
Timestamp: 2025-09-13T12:16:48.300Z
Learning: In the Sure project settings forms, the user prefers to maintain consistency with existing patterns for credential handling, even when security improvements are identified, and address such concerns comprehensively in separate PRs rather than making isolated changes.
📚 Learning: 2025-08-12T01:01:56.633Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/general-rules.mdc:0-0
Timestamp: 2025-08-12T01:01:56.633Z
Learning: Applies to {app/**/*.rb,app/**/*.erb} : Use `Current.user` for the current user; do not use `current_user` in Rails code and templates
Applied to files:
perf.rake
📚 Learning: 2025-09-20T08:42:15.420Z
Learnt from: CR
PR: we-promise/sure#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-20T08:42:15.420Z
Learning: Applies to **/test/fixtures/**/*.yml : Keep fixtures minimal—2-3 per model for base cases; create edge cases in test context.
Applied to files:
test/fixtures/users.yml
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-09-16T13:15:56.406Z
Learning: Applies to app/models/{entry,valuation,transaction,trade}.rb : Entry amounts are signed: negative = inflow, positive = outflow (applies to Entry and its types: Valuation, Transaction, Trade)
Applied to files:
app/models/simplefin_account/processor.rb
🧬 Code graph analysis (2)
test/models/impersonation_session_test.rb (1)
test/controllers/impersonation_sessions_controller_test.rb (1)
test(3-112)
test/controllers/impersonation_sessions_controller_test.rb (1)
test/test_helper.rb (1)
sign_in(59-61)
🔇 Additional comments (19)
app/models/simplefin_account/processor.rb (1)
82-84: Sign convention is correct; negation matches Sure’s domain rule.Negating SimpleFin amounts aligns with the project rule (negative = inflow, positive = outflow). I cross-checked with the retrieved learning about signed amounts.
app/models/mobile_device.rb (2)
26-26: LGTM: redirect URI updated to sureapp scheme.Change aligns with rebranding and expected mobile scheme.
Please confirm the mobile apps (iOS/Android) register the sureapp:// scheme and tests/seed data were updated accordingly.
24-29: Scopes OK — 'read_write' is configured as an optional scope.
config/initializers/doorkeeper.rb sets default_scopes :read and optional_scopes :read_write, so creating the app with scopes: "read_write" is valid.perf.rake (1)
28-28: Cookie name confirmed: app reads cookies.signed[:session_token]Found in app/controllers/concerns/authentication.rb (lines ~31,42) and config/initializers/doorkeeper.rb (lines ~12,32) — no change required.
lib/tasks/data_migration.rake (1)
11-14: Make Plaid webhook URL env‑aware (avoid hard‑coding prod host)Source the webhook from ENV (defaulting to https://app.sure.am/webhooks/plaid_eu) so the rake task is safe in dev/staging and re-runnable.
- eu_items.find_each do |item| - request = Plaid::ItemWebhookUpdateRequest.new( - access_token: item.access_token, - webhook: "https://app.sure.am/webhooks/plaid_eu" - ) + eu_items.find_each do |item| + webhook_url = ENV.fetch("PLAID_EU_WEBHOOK_URL", "https://app.sure.am/webhooks/plaid_eu") + request = Plaid::ItemWebhookUpdateRequest.new( + access_token: item.access_token, + webhook: webhook_url + )Re-check for leftover old domains (run locally):
rg -n -C2 -P 'maybefinance\.com'app/views/layouts/doorkeeper/application.html.erb (1)
23-23: LGTM: branding string updated correctlyHeader copy aligned to “Sure Authorization.”
test/models/family_export_test.rb (1)
32-33: LGTM: filename expectation matches rebrandExpectation updated to “sure_export_…”. Test remains deterministic with travel_to.
app/views/onboardings/trial.html.erb (2)
20-35: LGTM: onboarding trial copy updated to “Sure” consistentlyCTA and headline/subtext align with the rebrand.
111-111: Microcopy consistency: “Sure AI”If there’s a canonical product name (e.g., “SureAI”), align this string to it to keep brand consistent across the app.
test/controllers/impersonation_sessions_controller_test.rb (5)
5-6: LGTM: fixture reference updated to sure_support_staffReferences now match the renamed fixture key.
19-20: LGTMConsistent use of sure_support_staff across tests.
28-29: LGTMUpdates align with fixture rename.
55-56: LGTMConsistent rename applied.
81-81: LGTMAll occurrences appear updated in this test file.
test/fixtures/users.yml (1)
11-16: No stale support fixture/email references found — migration verifiedSearched for maybe_support_staff and old support emails; none found. New fixture sure_support_staff (email [email protected]) is defined in test/fixtures/users.yml and referenced in test/fixtures/impersonation_sessions.yml and tests: test/models/impersonation_session_test.rb (lines 12, 18, 31) and test/controllers/impersonation_sessions_controller_test.rb (lines 5, 19, 28, 55, 80).
test/test_helper.rb (1)
72-74: Confirm fixture password digests match the new test passwordtest/fixtures/users.yml contains the bcrypt digest $2a$12$XoNBo/cMCyzpYtvhrPAhsubG21mELX48RAcjSVCRctW8dG8wrDIla (repeated). Sandbox couldn’t verify the hash (bcrypt not available) — locally run:
ruby -r bcrypt -e 'puts BCrypt::Password.new("$2a$12$XoNBo/cMCyzpYtvhrPAhsubG21mELX48RAcjSVCRctW8dG8wrDIla") == "testpassword817983172"'
If false, regenerate/update the fixture password_digest values to match the new test password.
test/models/impersonation_session_test.rb (3)
18-18: LGTM: super_admin derived from sure_support_staff.
Consistent with rebranding and assertions.
31-31: LGTM: self‑impersonation guard uses sure_support_staff consistently.
No logic change; naming aligned.
12-12: Fixture reference validated — approve changes.
test/fixtures/users.yml definessure_support_staff(line 11); test/fixtures/impersonation_sessions.yml references it (line 2); nomaybe_support_staffoccurrences found.
| configured_username = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_USERNAME", "sure")) | ||
| configured_password = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_PASSWORD", "sure")) |
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.
🧩 Analysis chain
Avoid shipping known default creds in production; require explicit SIDEKIQ_WEB_ or fail fast.*
Using "sure" as a production fallback still leaves Sidekiq Web protected by guessable defaults if env vars are missing. Remove the defaults so boot fails unless credentials are set.
Apply this diff:
- configured_username = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_USERNAME", "sure"))
- configured_password = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_PASSWORD", "sure"))
+ configured_username = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_USERNAME"))
+ configured_password = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_PASSWORD"))Optionally, if you prefer not to crash on boot, deny access when vars are missing (follow‑up PR).
To verify and update docs/configs, run:
I can open a follow‑up issue and PR to harden this without disrupting the rebranding change—want me to proceed?
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check for places to document/set SIDEKIQ_WEB_*:"
rg -n -C2 -P 'SIDEKIQ_WEB_(USERNAME|PASSWORD)'
echo
echo "Residual references to old default 'maybe' (should be none for Sidekiq creds):"
rg -n -C2 -i '\bmaybe\b' | rg -n -v 'CHANGELOG|LICENSE'Length of output: 36856
Fail boot if SIDEKIQ_WEB_USERNAME/PASSWORD are unset — remove the sure fallbacks
Current code hashes the literal "sure" when env vars are missing, leaving predictable Sidekiq Web credentials; require ENV.fetch without defaults so the app fails to boot when creds aren't provided.
File: config/initializers/sidekiq.rb — lines 5–6
- configured_username = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_USERNAME", "sure"))
- configured_password = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_PASSWORD", "sure"))
+ configured_username = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_USERNAME"))
+ configured_password = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_PASSWORD"))📝 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.
| configured_username = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_USERNAME", "sure")) | |
| configured_password = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_PASSWORD", "sure")) | |
| configured_username = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_USERNAME")) | |
| configured_password = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_PASSWORD")) |
🤖 Prompt for AI Agents
In config/initializers/sidekiq.rb around lines 5 to 6, the initializer currently
provides fallback defaults ("sure") for SIDEKIQ_WEB_USERNAME and
SIDEKIQ_WEB_PASSWORD which are then hashed, creating predictable credentials;
update both lines to call ENV.fetch("SIDEKIQ_WEB_USERNAME") and
ENV.fetch("SIDEKIQ_WEB_PASSWORD") without default values so the process raises
on missing environment variables and prevents the app from booting with insecure
fallback credentials.
e565b92 to
b81fcf6
Compare
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: 2
🧹 Nitpick comments (4)
app/views/pages/redis_configuration_error.html.erb (2)
8-9: Replace raw Tailwind colors with design tokens.Per design-system guidance, avoid raw classes like bg-red-100, text-red-600, bg-amber-50, border-amber-200, text-amber-600, text-amber-800. Use functional tokens from app/assets/tailwind/maybe-design-system.css (e.g., text-primary/text-danger, bg-container/bg-warning-subtle, border-primary/border-warning).
Also applies to: 17-21
47-54: Avoid inline JS in templates; use declarative alternative.Inline onclick violates view guidelines. Prefer a link refresh or a Stimulus action.
Apply one of these:
- Option A (simple): make it a link to the current path.
- <%= render DS::Button.new( - text: "Refresh Page", - variant: "secondary", - icon: "refresh-cw", - type: "button", - full_width: true, - onclick: "window.location.reload()" - ) %> + <%= render DS::Link.new( + text: "Refresh Page", + href: request.fullpath, + variant: "secondary", + icon: "refresh-cw", + full_width: true + ) %>
- Option B (Stimulus): remove onclick and add a data-action (assuming a reload controller exists).
- <%= render DS::Button.new( + <%= render DS::Button.new( text: "Refresh Page", variant: "secondary", icon: "refresh-cw", - type: "button", - full_width: true, - onclick: "window.location.reload()" - ) %> + type: "button", + full_width: true, + data: { action: "click->reload#now" } + ) %>app/views/settings/api_keys/new.html.erb (2)
10-31: Use fieldset/legend for the radio group (semantic HTML + a11y).Radio groups should be wrapped in a fieldset with a legend, not a standalone label.
Apply:
- <div> - <%= form.label :scopes, "Permissions", class: "block text-sm font-medium text-primary mb-2" %> + <fieldset> + <legend class="block text-sm font-medium text-primary mb-2">Permissions</legend> <p class="text-sm text-secondary mb-3">Select the permissions this API key should have:</p> @@ - </div> + </fieldset>
21-22: Prefer form-bound radio helpers for consistency and error handling.Bind radios to the form object for consistent params and validation styling.
- <%= radio_button_tag "api_key[scopes]", value, (@api_key&.scopes || []).include?(value), - class: "mt-1" %> + <%= form.radio_button :scopes, value, + checked: Array(@api_key&.scopes).include?(value), + class: "mt-1" %>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app/controllers/concerns/self_hostable.rb(1 hunks)app/models/account.rb(1 hunks)app/models/assistant/configurable.rb(2 hunks)app/models/family_export.rb(1 hunks)app/models/import/row.rb(1 hunks)app/models/plaid_account/investments/balance_calculator.rb(1 hunks)app/models/plaid_account/investments/security_resolver.rb(1 hunks)app/models/plaid_account/transactions/category_matcher.rb(1 hunks)app/models/provider/plaid.rb(3 hunks)app/models/security/health_checker.rb(1 hunks)app/views/layouts/doorkeeper/application.html.erb(1 hunks)app/views/onboardings/trial.html.erb(4 hunks)app/views/pages/redis_configuration_error.html.erb(4 hunks)app/views/settings/api_keys/new.html.erb(1 hunks)config/initializers/sidekiq.rb(1 hunks)config/routes.rb(1 hunks)docs/hosting/docker.md(2 hunks)lib/tasks/data_migration.rake(1 hunks)test/models/family_export_test.rb(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- app/models/assistant/configurable.rb
- app/models/account.rb
- app/views/layouts/doorkeeper/application.html.erb
- app/models/plaid_account/investments/balance_calculator.rb
🚧 Files skipped from review as they are similar to previous changes (12)
- config/initializers/sidekiq.rb
- lib/tasks/data_migration.rake
- app/models/import/row.rb
- app/views/onboardings/trial.html.erb
- app/models/provider/plaid.rb
- app/models/family_export.rb
- app/models/plaid_account/transactions/category_matcher.rb
- docs/hosting/docker.md
- app/controllers/concerns/self_hostable.rb
- test/models/family_export_test.rb
- app/models/plaid_account/investments/security_resolver.rb
- app/models/security/health_checker.rb
🧰 Additional context used
📓 Path-based instructions (17)
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/routes.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
config/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:
config/routes.rb
{app/**/*.rb,app/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
{app/**/*.rb,app/**/*.erb}: UseCurrent.userfor the current user; do not usecurrent_userin Rails code and templates
UseCurrent.familyfor the current family; do not usecurrent_familyin Rails code and templates
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
{app/javascript/controllers/**/*.@(js|ts),app/components/**/*.@(js|ts),app/views/**/*.html.erb,app/components/**/*.html.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Use declarative Stimulus actions in ERB (data-action) instead of imperative event listeners in controllers (e.g., avoid addEventListener in connect/initializers).
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
{app/components/**/*.@(js|ts),app/views/**/*.html.erb,app/components/**/*.html.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Component Stimulus controllers under app/components must only be used within their component views, not in app/views.
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.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/**}: Author styles assuming TailwindCSS v4.x is in use across the app
Always start by referencing app/assets/tailwind/maybe-design-system.css to understand the base primitives, functional tokens, and component tokens
Prefer functional tokens from maybe-design-system.css over raw colors/utilities (e.g., usetext-primaryovertext-white,bg-containeroverbg-white,border border-primaryoverborder border-gray-200)
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
app/views/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
app/views/**/*.html.erb: Use ViewComponents when the element has complex logic/styling, is reused across contexts, needs variants/sizes or slots, requires interactivity/Stimulus, or needs accessibility/ARIA support
Use partials when the element is primarily static HTML with minimal logic, used in few contexts, simple template content, has no variants/configuration, and is mainly for content organization
Prefer components over partials: if a suitable component exists in app/components, use it; otherwise consider a partial or create a component based on the criteria
Integrate Stimulus declaratively in views: HTML declares actions/targets; controllers respond (avoid imperative JS in templates)
app/views/**/*.html.erb: Prefer native semantic HTML elements (e.g., ,/) over custom JS components
Use Turbo Frames to segment pages instead of JS-driven client-side solutions where possible
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
{app/views/**/*.html.erb,app/components/**/*.html.erb}
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
{app/views/**/*.html.erb,app/components/**/*.html.erb}: Keep domain logic out of views; compute classes and logic in component/view helpers, not inside ERB templates
Pass data from Rails to Stimulus via data-*-value attributes; do not embed inline JavaScript in templates
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
{app/helpers/**/*.rb,app/views/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Always use the icon helper from application_helper.rb; never call lucide_icon directly
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
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.yml
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.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/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.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/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
**/app/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Always use the
iconhelper for icons in templates; never uselucide_icondirectly.
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
**/app/views/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/views/**/*.erb: Keep domain logic OUT of view templates; logic belongs in component files, not templates.
Use partials only for primarily static, simple HTML with minimal logic, and only when not likely to be reused.
Prefer native HTML over JavaScript components for UI elements (e.g., use<dialog>,<details><summary>).
Leverage Turbo frames for page sections over client-side JavaScript solutions.
Use query params (not localStorage/sessions) for client state management.
Perform server-side formatting for currencies, numbers, and dates in templates.
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.html.erb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/views/settings/api_keys/new.html.erbapp/views/pages/redis_configuration_error.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 (4)
app/views/pages/redis_configuration_error.html.erb (4)
1-1: Page title rebrand LGTM.
12-12: Copy update LGTM (brand).
22-22: Copy update LGTM (brand + context).
31-31: Docs URL resolves (HTTP 200). The link is valid and points to the guide at the new repo path.
| <%= content_for :page_title, "Create New API Key" %> | ||
|
|
||
| <%= settings_section title: nil, subtitle: "Generate a new API key to access your Maybe data programmatically." do %> | ||
| <%= settings_section title: nil, subtitle: "Generate a new API key to access your Sure data programmatically." do %> |
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
🧩 Analysis chain
Brand update LGTM.
Copy change to “Sure data” looks good.
Run to catch any remaining “Maybe” references:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for remaining 'Maybe' references..."
rg -n -C2 -S '\bMaybe\b|Maybe data' || trueLength of output: 19585
Fix remaining "Maybe" references
rg found leftover "Maybe" occurrences in user-facing strings, locales, docs, tests and fixtures — update them to "Sure" or appropriate fork wording.
Key files to change (not exhaustive):
- README.md
- public/422.html
- config/locales/views/** (e.g. views/settings/api_keys/{en,tr,nb}.yml, views/registrations/*.yml)
- app/views/subscriptions/upgrade.html.erb
- app/views/layouts/shared/_footer.html.erb
- app/views/import/configurations/_trade_import.html.erb
- app/views/impersonation_sessions/_approval_bar.html.erb
- test/controllers/mfa_controller_test.rb
- test/vcr_cassettes/plaid/link_token.yml
- test/fixtures/accounts.yml
- perf.rake
🤖 Prompt for AI Agents
In app/views/settings/api_keys/new.html.erb around line 3, and across the repo,
there are leftover user-facing strings and locale keys still using "Maybe"
instead of the product name ("Sure") or appropriate fork wording; search the
codebase for occurrences of the string "Maybe" (including locales, views, docs,
tests and fixtures listed in the review), replace them with "Sure" or the
correct project-specific wording, update corresponding locale files (YAML
keys/translations) and any tests/fixtures that assert on the old text, run the
test suite and locale linting to ensure no regressions, and commit the changes
with a short message like "Replace remaining 'Maybe' references with 'Sure'".
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 (8)
app/models/simplefin_account/processor.rb (1)
71-71: Mark unused parameter to signal intent
currencyisn’t used. Rename to_currencyto make the intent explicit and quiet linters.Apply:
-def parse_amount(amount_value, currency) +def parse_amount(amount_value, _currency)app/models/demo/generator.rb (4)
41-41: DRY the demo email literal via a constant.Avoid repeating "[email protected]" in multiple places.
Apply this diff:
- create_family_and_users!("Demo Family", "[email protected]", onboarded: true, subscribed: true) + create_family_and_users!("Demo Family", DEFAULT_DEMO_EMAIL, onboarded: true, subscribed: true)Add this near the top of the class:
DEFAULT_DEMO_EMAIL = "[email protected]".freeze
56-56: Use the shared DEFAULT_DEMO_EMAIL constant here as well.- create_family_and_users!("Demo Family", "[email protected]", onboarded: false, subscribed: false) + create_family_and_users!("Demo Family", DEFAULT_DEMO_EMAIL, onboarded: false, subscribed: false)
136-151: Demo credentials: restrict usage to dev/test and consider fictional names.Static password is fine for seeds, but ensure this cannot run in production.
- Gate demo generation by environment (add once, covers all entry points):
Add to the class:
def ensure_demo_env! unless Rails.env.development? || Rails.env.test? raise "Demo data generation is disabled outside development/test" end endCall at the start of create_family_and_users!:
ensure_demo_env!
- Optional: Replace real-person-like names with fictional ones (e.g., "Alex"/"Eve Sure") to avoid brand/legal confusion.
63-63: Default param change — safe, consider centralizing demo emailgenerate_default_data! is only defined in app/models/demo/generator.rb (no external call sites) and there are no occurrences of
skip_clear: true— uniqueness conflicts unlikely. Unrelated hard-coded[email protected]remains in perf.rake, lib/tasks/benchmarking.rake, and test/vcr_cassettes/stripe/checkout_session.yml — consider centralizing the demo email into a constant (e.g. DEFAULT_DEMO_EMAIL) and replacing literals.app/models/mobile_device.rb (3)
21-34: Make creation atomic and centralize redirect URIAvoid orphaning Doorkeeper::Application if the association update fails; also centralize the redirect URI to prevent future search/replace rebrands.
Apply:
def create_oauth_application! return oauth_application if oauth_application.present? - app = Doorkeeper::Application.create!( - name: "Mobile App - #{device_id}", - redirect_uri: "sureapp://oauth/callback", # Custom scheme for mobile - scopes: "read_write", # Use the configured scope - confidential: false # Public client for mobile - ) - - # Store the association - update!(oauth_application: app) - app + app = nil + ApplicationRecord.transaction do + app = Doorkeeper::Application.create!( + name: "Mobile App - #{device_id}", + redirect_uri: MOBILE_OAUTH_REDIRECT_URI, # Custom scheme for mobile + scopes: "read_write", # Use the configured scope + confidential: false # Public client for mobile + ) + update!(oauth_application: app) + end + app endAdd near the top of the class:
MOBILE_OAUTH_REDIRECT_URI = ENV.fetch("MOBILE_OAUTH_REDIRECT_URI", "sureapp://oauth/callback")
13-15: Guard against nil last_seen_atPrevent a possible NoMethodError if legacy rows have null last_seen_at.
- def active? - last_seen_at > 90.days.ago - end + def active? + last_seen_at.present? && last_seen_at > 90.days.ago + end
24-29: Custom scheme can be hijacked; consider Universal Links/App LinksCustom URL schemes are vulnerable to scheme hijacking by other apps. Prefer HTTPS redirect URIs with iOS Universal Links and Android App Links. If staying with a custom scheme, ensure strict state validation and PKCE.
Can you confirm Universal Links/App Links are planned, or document mitigations for the custom scheme?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.md(1 hunks)app/models/demo/generator.rb(3 hunks)app/models/mobile_device.rb(1 hunks)app/models/simplefin_account/processor.rb(1 hunks)app/views/impersonation_sessions/_approval_bar.html.erb(1 hunks)app/views/import/configurations/_trade_import.html.erb(1 hunks)app/views/settings/preferences/show.html.erb(1 hunks)app/views/subscriptions/upgrade.html.erb(2 hunks)
✅ Files skipped from review due to trivial changes (5)
- app/views/subscriptions/upgrade.html.erb
- app/views/impersonation_sessions/_approval_bar.html.erb
- README.md
- app/views/import/configurations/_trade_import.html.erb
- app/views/settings/preferences/show.html.erb
🧰 Additional context used
📓 Path-based instructions (8)
{app/**/*.rb,app/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
{app/**/*.rb,app/**/*.erb}: UseCurrent.userfor the current user; do not usecurrent_userin Rails code and templates
UseCurrent.familyfor the current family; do not usecurrent_familyin Rails code and templates
Files:
app/models/demo/generator.rbapp/models/mobile_device.rbapp/models/simplefin_account/processor.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/models/**/*.rb: Place most business logic in app/models as POROs or model code rather than separate service objects
Prefer model methods that answer questions about themselves (e.g., Account#balance_series) over service-style objects
ActiveRecord validations may mirror DB-level constraints for UX but are not strictly required
Keep complex validations and business logic in ActiveRecord models rather than the databaseDomain 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.
Files:
app/models/demo/generator.rbapp/models/mobile_device.rbapp/models/simplefin_account/processor.rb
app/{models,controllers}/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries; only optimize performance in critical/global paths
Files:
app/models/demo/generator.rbapp/models/mobile_device.rbapp/models/simplefin_account/processor.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/models/demo/generator.rbapp/models/mobile_device.rbapp/models/simplefin_account/processor.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/models/demo/generator.rbapp/models/mobile_device.rbapp/models/simplefin_account/processor.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/demo/generator.rbapp/models/mobile_device.rbapp/models/simplefin_account/processor.rb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/models/demo/generator.rbapp/models/mobile_device.rbapp/models/simplefin_account/processor.rb
**/app/models/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Business logic should primarily reside in models; use concerns and POROs for organization.
Files:
app/models/demo/generator.rbapp/models/mobile_device.rbapp/models/simplefin_account/processor.rb
🧠 Learnings (3)
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-09-16T13:15:56.406Z
Learning: Applies to app/models/{entry,valuation,transaction,trade}.rb : Entry amounts are signed: negative = inflow, positive = outflow (applies to Entry and its types: Valuation, Transaction, Trade)
Applied to files:
app/models/simplefin_account/processor.rb
📚 Learning: 2025-09-13T11:36:45.479Z
Learnt from: alessiocappa
PR: we-promise/sure#141
File: app/models/enable_banking_account/processor.rb:72-79
Timestamp: 2025-09-13T11:36:45.479Z
Learning: The balance_calculator method in EnableBankingAccount::Processor follows the same pattern as the Plaid integration, using OpenStruct without explicit requires and returning balance values as-is without BigDecimal coercion for consistency across banking integrations.
Applied to files:
app/models/simplefin_account/processor.rb
📚 Learning: 2025-09-13T11:36:45.479Z
Learnt from: alessiocappa
PR: we-promise/sure#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/simplefin_account/processor.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 (4)
app/models/simplefin_account/processor.rb (2)
82-84: AI summary mismatch: negation was not removedThe enriched summary says negation was removed, but Line 84 still negates the amount. Please confirm the intended behavior.
71-88: Approve — sign conversion matches domain rule; confirm other providersNegating parsed_amount converts SimpleFin's convention (expenses negative, income positive) into Sure's convention (expenses positive, income negative) and aligns with Entry amounts: negative=inflow, positive=outflow.
- Confirm other banking processors normalize sign the same way; search returned parse_amount only in app/models/simplefin_account/processor.rb.
app/models/mobile_device.rb (2)
26-26: Rebrand redirect URI LGTM — Doorkeeper PKCE & redirect settings verified; confirm mobile apps handle sureapp://
- force_pkce is enabled (config/initializers/doorkeeper.rb) — PKCE required for non-confidential/native clients.
- force_ssl_in_redirect_uri is false (config/initializers/doorkeeper.rb) — custom-scheme/non-HTTPS redirects allowed.
- default_scopes :read and optional_scopes :read_write are configured.
- No mobile client code found here — confirm iOS/Android apps are updated to handle sureapp://.
39-44: Postgres-specific expiry SQL — PostgreSQL adapter confirmedconfig/database.yml uses adapter: postgresql and Gemfile includes gem "pg", so the interval '1 second' expression is supported.
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 (3)
test/controllers/mfa_controller_test.rb (2)
34-34: Use @user.totp to avoid issuer string drift.Keeps tests aligned with the model and future rebranding changes.
- totp = ROTP::TOTP.new(@user.otp_secret, issuer: "Sure Finances") + totp = @user.totp
73-73: DRY up TOTP creation by delegating to the model.Reduces duplication and removes hardcoded issuer from the test.
- totp = ROTP::TOTP.new(@user.otp_secret, issuer: "Sure Finances") + totp = @user.totpconfig/environments/test.rb (1)
26-27: Avoid unconditionally mutating ENV in tests; use ||= or configure Action Mailer default.app/mailers/application_mailer.rb uses ENV.fetch("EMAIL_SENDER", "[email protected]") — the assignment in config/environments/test.rb will override any CI-provided EMAIL_SENDER. Use one of:
Minimal, keep ENV-based usage:
-ENV["EMAIL_SENDER"] = "[email protected]" +ENV["EMAIL_SENDER"] ||= "[email protected]"Better, avoid mutating ENV (works if mailers read Rails config):
-ENV["EMAIL_SENDER"] = "[email protected]" +config.action_mailer.default_options = { from: ENV.fetch("EMAIL_SENDER", "[email protected]") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
config/environments/test.rb(1 hunks)lib/tasks/benchmarking.rake(2 hunks)perf.rake(1 hunks)test/controllers/mfa_controller_test.rb(2 hunks)test/mailers/email_confirmation_mailer_test.rb(1 hunks)test/vcr_cassettes/plaid/link_token.yml(1 hunks)test/vcr_cassettes/stripe/checkout_session.yml(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- perf.rake
- test/mailers/email_confirmation_mailer_test.rb
- test/vcr_cassettes/plaid/link_token.yml
- test/vcr_cassettes/stripe/checkout_session.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/tasks/benchmarking.rake
🧰 Additional context used
📓 Path-based instructions (5)
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/environments/test.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
config/environments/test.rbtest/controllers/mfa_controller_test.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:
config/environments/test.rbtest/controllers/mfa_controller_test.rb
test/**/*_test.rb
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
test/**/*_test.rb: Use Minitest for all tests (do not write RSpec tests)
When many records are needed, use Rails test helpers to generate them and inline creation within the test
Test boundaries correctly: for query methods, assert outputs; for commands, assert that the command was called with correct params
Never test the implementation details of one class in another class’s test suite
Use the mocha gem for stubs and mocks
Prefer OpenStruct (or a small mock class) when creating mock instances
Only mock what’s necessary; don’t stub return values unless they are assertedName Minitest files with *_test.rb and mirror the app/ structure under test/
test/**/*_test.rb: ALWAYS use Minitest + fixtures + Mocha for tests; NEVER RSpec or FactoryBot.
Use Mocha for mocking in tests when necessary.
Use VCR for external API tests.
Files:
test/controllers/mfa_controller_test.rb
**/test/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/test/**/*.rb: ALWAYS use Minitest and fixtures for Ruby tests; NEVER RSpec or factories.
Only test critical and important code paths; system tests should be used sparingly for critical user flows.
Files:
test/controllers/mfa_controller_test.rb
🧬 Code graph analysis (1)
test/controllers/mfa_controller_test.rb (2)
app/models/user.rb (1)
totp(195-197)app/controllers/mfa_controller.rb (1)
new(5-8)
⏰ 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
Summary by CodeRabbit
Improvements
Changes
Documentation
Tests