Skip to content

Conversation

@joaofrenor
Copy link

@joaofrenor joaofrenor commented Dec 5, 2025

Adds nested cash subgroups with subtype totals, exposes a preference toggle to keep a flat list, fixes subtype persistence, and adds regression tests.

image image

Summary by CodeRabbit

Release Notes

  • New Features

    • Cash accounts can now be grouped by subtype (Savings, Checking) instead of a flat list. Toggle this grouping behavior in preferences.
    • Rules can now be scheduled to run automatically on a custom schedule using standard cron expressions.
  • Updates

    • Added cash account grouping preference to the settings page.
    • Enhanced rule form to support schedule configuration options.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 5, 2025

Walkthrough

This PR introduces two features: cash account subgrouping (categorizing cash accounts by savings/checking subtypes with a user preference toggle) and cron-based rule scheduling (automating rule execution on schedules). Changes include database migrations, model logic for subgroups and cron management, controller/view updates, a Stimulus controller for scheduling UI, and comprehensive tests.

Changes

Cohort / File(s) Summary
Cash Subgroups: Models
app/models/balance_sheet/subtype_group.rb, app/models/balance_sheet/account_group.rb, app/models/balance_sheet/classification_group.rb
Introduced BalanceSheet::SubtypeGroup class for grouping accounts by subtype with monetization support. Enhanced AccountGroup with methods for subgroups, uncategorized accounts, totals, and cache-subgroup-enabled logic. Extended ClassificationGroup to store and expose family reference.
Cash Subgroups: Controllers & Params
app/controllers/depositories_controller.rb, app/controllers/users_controller.rb
Added :subtype to permitted accountable attributes in Depositories; added :cash_subgroup_enabled to permitted family attributes in Users.
Cash Subgroups: Views & Localization
app/views/accounts/_accountable_group.html.erb, app/views/depositories/_form.html.erb, app/views/settings/preferences/show.html.erb, config/locales/views/settings/en.yml
Refactored account group rendering to display subgroups and uncategorized sections when enabled. Moved subtype field into nested accountable block in Depository form. Added cash subgroup preference toggle and translations.
Cash Subgroups: Database & Helper
db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb, app/models/balance_sheet.rb, app/models/concerns/accountable.rb
Added migration for cash_subgroup_enabled column on families. Passed family object into ClassificationGroup construction. Made subtype lookup case-insensitive in Accountable concern.
Rule Scheduling: Models & Jobs
app/models/rule.rb, app/workers/rule_schedule_worker.rb
Enhanced Rule with cron validation, schedule state tracking, and Sidekiq cron job lifecycle management (create/update/destroy). Introduced RuleScheduleWorker to enqueue RuleJob when cron triggers.
Rule Scheduling: Controllers & Parameters
app/controllers/rules_controller.rb
Added :schedule_enabled and :schedule_cron to permitted rule parameters.
Rule Scheduling: UI & Stimulus
app/views/rules/_form.html.erb, app/javascript/controllers/rules_controller.js
Added "WHEN" section with schedule toggle and cron expression field. Introduced Stimulus targets (scheduleCheckbox, scheduleFields, scheduleCronInput) and toggleSchedule() method to enable/disable schedule UI fields conditionally.
Rule Scheduling: Database
db/migrate/20251205162439_add_schedule_to_rules.rb
Migration adding schedule_cron (string) and schedule_enabled (boolean, default false) columns to rules table.
Database Schema Update
db/schema.rb
Updated schema version and reflected new columns for families, rules, and rule_runs tables.
Tests
test/models/balance_sheet_test.rb, test/models/rule_test.rb, test/workers/rule_schedule_worker_test.rb, test/jobs/rule_job_test.rb, test/system/chats_test.rb
Added tests validating cash subgroup rendering behavior under various configurations. Added tests for cron validation, Sidekiq job lifecycle, and RuleScheduleWorker enqueue logic. Updated chat test for visibility scope.

Sequence Diagram(s)

sequenceDiagram
    participant User as User/Scheduler
    participant SidekiqCron as Sidekiq Cron
    participant RuleScheduleWorker as RuleScheduleWorker
    participant RuleJob as RuleJob
    participant Rule as Rule Model
    participant Entry as Entry Model
    
    User->>Rule: Create/Update rule with<br/>schedule_enabled=true
    Rule->>Rule: Validate cron expression
    Rule->>Rule: after_commit: sync_cron_schedule
    Rule->>SidekiqCron: Sidekiq::Cron::Job.create<br/>(cron_expression)
    SidekiqCron->>SidekiqCron: Schedules recurring job
    
    rect rgb(200, 220, 240)
    Note over SidekiqCron: At scheduled time (e.g., daily)
    SidekiqCron->>RuleScheduleWorker: Trigger perform(rule_id)
    end
    
    RuleScheduleWorker->>Rule: Load rule by ID
    Rule-->>RuleScheduleWorker: Return rule object
    alt Rule is active & scheduling enabled
        RuleScheduleWorker->>RuleJob: Enqueue to 'scheduled' queue<br/>(rule_id, ignore_locks)
        RuleJob->>Entry: Query matching entries
        Entry-->>RuleJob: Return entries
        RuleJob->>Rule: Apply rule conditions & actions
        Rule->>Entry: Update (e.g., set category)
        Entry-->>Rule: Persist changes
    else Rule inactive or scheduling disabled
        RuleScheduleWorker->>RuleScheduleWorker: Exit early (guard condition)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Cron job lifecycle logic in Rule model (schedule_state_changed?, sync_cron_schedule, remove_cron_schedule): verify correctness of state detection and Sidekiq integration, including error logging and edge cases.
  • Cash subgroups rendering in views (_accountable_group.html.erb): validate conditional logic for subgroups vs. uncategorized accounts and account row structure preservation.
  • Stimulus controller state management (rules_controller.js): verify toggleSchedule() properly enables/disables form fields across all browser scenarios.
  • Database migrations and schema: ensure migration order and schema consistency, particularly the new columns on families, rules, and rule_runs.
  • Test coverage for cash subgroup behavior: verify the three new balance sheet tests comprehensively cover enabled/disabled and uncategorized scenarios.

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • jjmata
  • sokie

Poem

🐰 A rabbit hops through balance sheets,
Grouping cash by savings treats,
While cron schedules tick away,
Rules run on time, night and day!
Hop, schedule, sort—what a feat!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.94% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Cash Subtype Group on View' is vague and doesn't clearly convey the main changes. While it references 'Cash' and 'View', it lacks specificity about what was added or changed, making it difficult for someone scanning history to understand the primary objectives. Consider a more descriptive title such as 'Add cash account subtype grouping UI with preference toggle' or 'Implement nested cash subtype groups with user preference' to better reflect the feature additions.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 (3)
app/models/balance_sheet/classification_group.rb (1)

6-14: The family parameter addition is correct.

The new family attribute is properly added to enable downstream cash subgroup logic. However, note that line 10 (@name = name) appears to be dead code—@name is assigned but never used since name is always computed from classification in the method on line 16-18. Consider removing this line.

 def initialize(classification:, currency:, accounts:, family:)
   @classification = normalize_classification!(classification)
-  @name = name
   @currency = currency
   @accounts = accounts
   @family = family
 end
app/views/accounts/_accountable_group.html.erb (1)

36-126: Consider extracting the repeated account row block into a partial.

The account row rendering logic (logo, name, subtype, balance, sparkline) is duplicated three times across lines 36-60, 72-96, and 101-125. Extracting this to a partial like _account_row.html.erb would reduce duplication and simplify future maintenance.

Example partial (app/views/accounts/_account_row.html.erb):

<%# locals: (account:, color:) %>
<%= link_to account_path(account),
          class: class_names(
            "block flex items-center gap-2 px-3 py-2 rounded-lg",
            page_active?(account_path(account)) ? "bg-container" : "hover:bg-surface-hover"
          ),
          title: account.name do %>
  <%= render "accounts/logo", account: account, size: "sm", color: color %>
  <div class="min-w-0 grow">
    <div class="flex items-center gap-2 mb-0.5">
      <%= tag.p account.name, class: class_names("text-sm text-primary font-medium truncate", "animate-pulse" => account.syncing?) %>
    </div>
    <%= tag.p account.short_subtype_label, class: "text-sm text-secondary truncate" %>
  </div>
  <div class="ml-auto text-right grow h-10">
    <%= tag.p format_money(account.balance_money), class: "text-sm font-medium text-primary whitespace-nowrap" %>
    <%= turbo_frame_tag dom_id(account, :sparkline), src: sparkline_account_path(account), loading: "lazy", data: { controller: "turbo-frame-timeout", turbo_frame_timeout_timeout_value: 10000 } do %>
      <div class="flex items-center w-8 h-4 ml-auto">
        <div class="w-6 h-px bg-loader"></div>
      </div>
    <% end %>
  </div>
<% end %>
app/models/balance_sheet/account_group.rb (1)

68-70: Consider using the monetize macro for consistency.

The manual Money construction here is inconsistent with how SubtypeGroup (lines 4, 22-24 in subtype_group.rb) handles the exact same value type. Both uncategorized_total and SubtypeGroup#total sum converted_balance across accounts, yet one uses manual construction and the other uses the monetize macro.

Consider using the monetize macro pattern for consistency:

+  monetize :uncategorized_total, as: :uncategorized_total_money
+
-  def uncategorized_total_money
-    Money.new((uncategorized_total * 100).to_i, currency)
-  end

If the manual construction with * 100 and .to_i is necessary due to how Money objects are constructed in your codebase, please verify whether SubtypeGroup#total_money has the same issue.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a91a439 and f5a8d98.

📒 Files selected for processing (15)
  • app/controllers/depositories_controller.rb (1 hunks)
  • app/controllers/users_controller.rb (1 hunks)
  • app/models/balance_sheet.rb (1 hunks)
  • app/models/balance_sheet/account_group.rb (2 hunks)
  • app/models/balance_sheet/classification_group.rb (1 hunks)
  • app/models/balance_sheet/subtype_group.rb (1 hunks)
  • app/models/concerns/accountable.rb (1 hunks)
  • app/views/accounts/_accountable_group.html.erb (1 hunks)
  • app/views/depositories/_form.html.erb (1 hunks)
  • app/views/settings/preferences/show.html.erb (1 hunks)
  • config/locales/views/settings/en.yml (1 hunks)
  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb (1 hunks)
  • db/schema.rb (2 hunks)
  • test/models/balance_sheet_test.rb (1 hunks)
  • test/system/chats_test.rb (1 hunks)
🧰 Additional context used
📓 Path-based instructions (31)
**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins

**/*.rb: Use Rails pluralization in i18n: t("transactions.count", count: @transactions.count).
Run bin/rubocop -f github -a for Ruby linting with auto-correct before pull requests.

**/*.rb: Ruby code should use 2-space indent, snake_case for methods and variables, and CamelCase for classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration

Files:

  • test/system/chats_test.rb
  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
  • app/controllers/users_controller.rb
  • app/models/balance_sheet/classification_group.rb
  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
  • test/models/balance_sheet_test.rb
  • db/schema.rb
test/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

test/**/*.rb: Use Minitest with fixtures (never RSpec or FactoryBot) for testing
Use mocha gem for mocks and VCR for external API tests
Use fixtures for base cases in tests (2-3 per model) and create edge cases on-the-fly within test context
Test critical code paths only: test Commands for correct params, Queries for output

test/**/*.rb: Use Rails' built-in Minitest framework with fixtures for test data. Avoid RSpec and FactoryBot.
Use VCR for external API testing.
Use mocha gem for stubs and mocks. Prefer OpenStruct for mock instances. Only mock what's necessary.

Write tests using Minitest and fixtures as the testing framework

test/**/*.rb: Always use Minitest and fixtures for testing, NEVER rspec or factories
For tests that require a large number of fixture records, use Rails helpers to help create the records needed for the test, then inline the creation instead of defining them in fixtures
Distinguish between commands and query methods. Test output of query methods; test that commands were called with the correct params.
Never test the implementation details of one class in another class's test suite
Use the mocha gem for stubs and mocks
Always prefer OpenStruct when creating mock instances, or in complex cases, a mock class
Only mock what's necessary. If you're not testing return values, don't mock a return value.

Files:

  • test/system/chats_test.rb
  • test/models/balance_sheet_test.rb
**/*.{rb,erb,html.erb}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{rb,erb,html.erb}: Use Current.user for the current user context in controllers and views. Do NOT use current_user.
Use Current.family for the current family context. Do NOT use current_family.

Files:

  • test/system/chats_test.rb
  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
  • app/controllers/users_controller.rb
  • app/models/balance_sheet/classification_group.rb
  • app/views/settings/preferences/show.html.erb
  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
  • app/views/depositories/_form.html.erb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
  • app/views/accounts/_accountable_group.html.erb
  • test/models/balance_sheet_test.rb
  • db/schema.rb
**/*.{erb,html.erb,rb}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via the t() helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.

Files:

  • test/system/chats_test.rb
  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
  • app/controllers/users_controller.rb
  • app/models/balance_sheet/classification_group.rb
  • app/views/settings/preferences/show.html.erb
  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
  • app/views/depositories/_form.html.erb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
  • app/views/accounts/_accountable_group.html.erb
  • test/models/balance_sheet_test.rb
  • db/schema.rb
test/system/**/*.rb

📄 CodeRabbit inference engine (CLAUDE.md)

Use system tests sparingly for critical user flows. Keep system tests minimal and effective.

Use system tests sparingly as they increase the time to complete the test suite

Files:

  • test/system/chats_test.rb
**/*.{rb,erb}

📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)

**/*.{rb,erb}: Use Current.user for accessing the current user. Do NOT use current_user
Use Current.family for accessing the current family. Do NOT use current_family

Files:

  • test/system/chats_test.rb
  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
  • app/controllers/users_controller.rb
  • app/models/balance_sheet/classification_group.rb
  • app/views/settings/preferences/show.html.erb
  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
  • app/views/depositories/_form.html.erb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
  • app/views/accounts/_accountable_group.html.erb
  • test/models/balance_sheet_test.rb
  • db/schema.rb
**/*.{rb,js,erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only

Files:

  • test/system/chats_test.rb
  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
  • app/controllers/users_controller.rb
  • app/models/balance_sheet/classification_group.rb
  • app/views/settings/preferences/show.html.erb
  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
  • app/views/depositories/_form.html.erb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
  • app/views/accounts/_accountable_group.html.erb
  • test/models/balance_sheet_test.rb
  • db/schema.rb
**/*.{rb,html.erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible

Files:

  • test/system/chats_test.rb
  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
  • app/controllers/users_controller.rb
  • app/models/balance_sheet/classification_group.rb
  • app/views/settings/preferences/show.html.erb
  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
  • app/views/depositories/_form.html.erb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
  • app/views/accounts/_accountable_group.html.erb
  • test/models/balance_sheet_test.rb
  • db/schema.rb
test/**/*_test.rb

📄 CodeRabbit inference engine (AGENTS.md)

test/**/*_test.rb: Test files should use *_test.rb naming convention and mirror app/ structure (e.g., test/models/*_test.rb)
Use Minitest framework for tests with names *_test.rb, run via bin/rails test, and use test/fixtures and VCR cassettes for HTTP testing

Files:

  • test/system/chats_test.rb
  • test/models/balance_sheet_test.rb
config/locales/**/*.yml

📄 CodeRabbit inference engine (CLAUDE.md)

Use hierarchical i18n keys by feature: accounts.index.title, transactions.form.amount_label. Use t() helper for all user-facing strings with interpolation for dynamic content.

Files:

  • config/locales/views/settings/en.yml
config/**/*.{rb,yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

Configuration files should be placed in config/ directory, with environment examples in .env.local.example and .env.test.example

Files:

  • config/locales/views/settings/en.yml
db/migrate/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use simple validations (null checks, unique indexes) in database layer

Files:

  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
{app/models/**/*.rb,db/migrate/**/*.rb}

📄 CodeRabbit inference engine (CLAUDE.md)

Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.

Files:

  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
  • app/models/balance_sheet/classification_group.rb
  • app/models/balance_sheet/subtype_group.rb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
db/migrate/*.rb

📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)

ActiveRecord migrations must inherit from ActiveRecord::Migration[7.2]. Do NOT use version 8.0 yet

Enforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL

Files:

  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
db/**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Data migrations should be placed in db/ directory with test fixtures in test/fixtures/

Files:

  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
  • db/schema.rb
app/controllers/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Rails strong parameters and CSRF protection throughout the application

app/controllers/**/*.rb: Use strong parameters and CSRF protection throughout the application.
Use query params for state over localStorage/sessions.

Files:

  • app/controllers/users_controller.rb
  • app/controllers/depositories_controller.rb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Code should be organized in app/ directory (Rails MVC, services, jobs, mailers, components), with JS in app/javascript/, and styles/assets in app/assets/ (Tailwind, images, fonts)

Files:

  • app/controllers/users_controller.rb
  • app/models/balance_sheet/classification_group.rb
  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
app/**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Run bin/brakeman security scan before major PRs to check for static analysis of common Rails issues

Files:

  • app/controllers/users_controller.rb
  • app/models/balance_sheet/classification_group.rb
  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
app/models/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder

app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). Use Money objects for currency conversion and formatting.
Place business logic in app/models/ folder. Avoid creating app/services/ folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: use account.balance_series not AccountSeries.new(account).call.

Files:

  • app/models/balance_sheet/classification_group.rb
  • app/models/balance_sheet/subtype_group.rb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
{app/**/*.{erb,html},app/javascript/**/*.js}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
app/views/**/*.erb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

app/views/**/*.erb: Always use the icon helper for icons in views, never use lucide_icon directly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-side

Views should use ERB and avoid heavy logic; prefer helpers and components instead (ERB checked by erb-lint per .erb_lint.yml)

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
**/*.{css,erb}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Tailwind CSS v4.x with custom design system for styling

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
**/*.{erb,html,vue,jsx,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{erb,html,vue,jsx,tsx}: Use Tailwind CSS v4.x with custom design system defined in app/assets/tailwind/maybe-design-system.css. Always use functional tokens (e.g., text-primary not text-white).
Prefer semantic HTML elements over JavaScript components (e.g., use <dialog> for modals, <details><summary> for disclosures).

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
**/*.{erb,html.erb}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{erb,html.erb}: Use icon helper for icons, never use lucide_icon directly.
Leverage Turbo frames for page sections over client-side solutions.
Pass data to Stimulus controllers via data-*-value attributes, not inline JavaScript.

**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
{app/components/**/*.html.erb,app/views/**/*.{erb,html.erb}}

📄 CodeRabbit inference engine (CLAUDE.md)

Keep domain logic out of view templates. Logic belongs in component files, not template files.

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
**/*.erb

📄 CodeRabbit inference engine (CLAUDE.md)

Run bundle exec erb_lint ./app/**/*.erb -a for ERB linting with auto-correct before pull requests.

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
**/*.html.erb

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,

for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use the icon helper from application_helper.rb for icons. Never use lucide_icon helper directly

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
{app/views/**,app/helpers/**,app/javascript/controllers/**}

📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)

{app/views/**,app/helpers/**,app/javascript/controllers/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
{app/views/**,app/helpers/**}

📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)

Always generate semantic HTML

Files:

  • app/views/settings/preferences/show.html.erb
  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
app/views/**/_*.html.erb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use partials only for simple, static HTML with minimal logic in specific contexts

app/views/**/_*.html.erb: Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Place context-specific partials in relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)

Files:

  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
app/views/**/_*.{erb,html.erb}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Partials when element is primarily static HTML with minimal logic, used in only one or few contexts, is simple template content, or doesn't need variants/sizes/complex configuration.

Files:

  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.html.erb
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Account model is a Rails delegated type with subtypes: Depository, Investment, Crypto, Property, Vehicle, OtherAsset (assets) and CreditCard, Loan, OtherLiability (liabilities)
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to config/locales/**/*.yml : Use hierarchical i18n keys by feature: `accounts.index.title`, `transactions.form.amount_label`. Use `t()` helper for all user-facing strings with interpolation for dynamic content.

Applied to files:

  • config/locales/views/settings/en.yml
  • app/views/settings/preferences/show.html.erb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{erb,html.erb,rb} : All user-facing strings must use localization (i18n) via the `t()` helper. Update locale files for each new or changed element.

Applied to files:

  • config/locales/views/settings/en.yml
  • app/views/settings/preferences/show.html.erb
📚 Learning: 2025-11-24T16:55:59.156Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/general-rules.mdc:0-0
Timestamp: 2025-11-24T16:55:59.156Z
Learning: Applies to db/migrate/*.rb : ActiveRecord migrations must inherit from `ActiveRecord::Migration[7.2]`. Do NOT use version 8.0 yet

Applied to files:

  • db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb
  • db/schema.rb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to app/controllers/**/*.rb : Use strong parameters and CSRF protection throughout the application.

Applied to files:

  • app/controllers/users_controller.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to **/*.rb : Use Current.user and Current.family instead of current_user / current_family for authentication context

Applied to files:

  • app/controllers/users_controller.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/controllers/**/*.rb : Use Rails strong parameters and CSRF protection throughout the application

Applied to files:

  • app/controllers/users_controller.rb
📚 Learning: 2025-11-24T16:56:30.669Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Account model is a Rails delegated type with subtypes: Depository, Investment, Crypto, Property, Vehicle, OtherAsset (assets) and CreditCard, Loan, OtherLiability (liabilities)

Applied to files:

  • app/controllers/users_controller.rb
  • app/models/balance_sheet/classification_group.rb
  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
  • app/views/depositories/_form.html.erb
  • app/models/balance_sheet.rb
  • app/models/concerns/accountable.rb
  • app/models/balance_sheet/account_group.rb
  • test/models/balance_sheet_test.rb
📚 Learning: 2025-11-24T16:55:59.156Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/general-rules.mdc:0-0
Timestamp: 2025-11-24T16:55:59.156Z
Learning: Applies to **/*.{rb,erb} : Use `Current.family` for accessing the current family. Do NOT use `current_family`

Applied to files:

  • app/models/balance_sheet/classification_group.rb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to app/models/**/*.rb : All monetary values stored in base currency (user's primary currency). Use `Money` objects for currency conversion and formatting.

Applied to files:

  • app/models/balance_sheet/classification_group.rb
📚 Learning: 2025-11-24T16:56:30.669Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Entry model is a Rails delegated type with three subtypes: Valuation (absolute account value), Transaction (balance modification), and Trade (buy/sell for investment accounts)

Applied to files:

  • app/models/balance_sheet/subtype_group.rb
  • app/controllers/depositories_controller.rb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Use Rails concerns for code organization around model 'traits' and shared functionality, not just for moving code to another location. Prefer models answering questions about themselves with methods like account.balance_series over service-like patterns

Applied to files:

  • app/controllers/depositories_controller.rb
  • app/models/concerns/accountable.rb
📚 Learning: 2025-11-24T16:57:20.318Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.318Z
Learning: Applies to app/views/**/_*.html.erb : Place context-specific partials in relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)

Applied to files:

  • app/views/depositories/_form.html.erb
  • app/views/accounts/_accountable_group.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 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/balance_sheet.rb
  • app/models/balance_sheet/account_group.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/balance_sheet.rb
  • app/models/balance_sheet/account_group.rb
  • test/models/balance_sheet_test.rb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to **/*.html.erb : Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions

Applied to files:

  • app/views/accounts/_accountable_group.html.erb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to test/**/*.rb : Use fixtures for base cases in tests (2-3 per model) and create edge cases on-the-fly within test context

Applied to files:

  • test/models/balance_sheet_test.rb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to db/migrate/*.rb : Enforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL

Applied to files:

  • db/schema.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to db/migrate/**/*.rb : Use simple validations (null checks, unique indexes) in database layer

Applied to files:

  • db/schema.rb
📚 Learning: 2025-11-24T16:56:30.669Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Syncable models (Account, PlaidItem, Family) must implement sync operations tracked via Sync records that log status, errors, and maintain an audit trail

Applied to files:

  • db/schema.rb
🧬 Code graph analysis (4)
app/models/balance_sheet/subtype_group.rb (3)
app/models/balance_sheet/account_group.rb (2)
  • key (32-34)
  • currency (87-89)
app/models/concerns/monetizable.rb (1)
  • monetize (5-15)
app/models/concerns/accountable.rb (1)
  • short_subtype_label_for (43-45)
app/controllers/depositories_controller.rb (1)
app/controllers/concerns/accountable_resource.rb (1)
  • permitted_accountable_attributes (12-15)
app/models/balance_sheet/account_group.rb (3)
app/controllers/concerns/accountable_resource.rb (1)
  • accountable_type (78-80)
app/models/balance_sheet/subtype_group.rb (2)
  • name (14-16)
  • currency (26-28)
app/models/balance_sheet.rb (1)
  • currency (46-48)
test/models/balance_sheet_test.rb (4)
app/models/balance_sheet.rb (2)
  • assets (12-19)
  • account_groups (34-36)
app/models/balance_sheet/classification_group.rb (2)
  • account_groups (33-53)
  • name (16-18)
app/models/balance_sheet/subtype_group.rb (1)
  • name (14-16)
app/models/balance_sheet/account_group.rb (3)
  • subgroups (40-56)
  • uncategorized_accounts (58-62)
  • uncategorized_total_money (68-70)
⏰ 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 (22)
app/controllers/depositories_controller.rb (1)

4-4: LGTM!

The permitted_accountable_attributes :subtype configuration correctly allows the subtype attribute to be submitted through the nested accountable form fields. This aligns with the form changes in app/views/depositories/_form.html.erb.

app/models/balance_sheet.rb (1)

16-17: LGTM!

Passing the family object to ClassificationGroup enables family-scoped behavior for cash subgrouping. The implementation is clean and consistent for both assets and liabilities.

config/locales/views/settings/en.yml (1)

33-34: LGTM!

The localization keys follow the hierarchical structure and provide clear, concise descriptions of the cash subgrouping feature. The hint text effectively explains the toggle's behavior.

app/views/settings/preferences/show.html.erb (2)

43-43: LGTM!

Adding a link to the I18n tracking issue improves user experience by providing transparency about translation progress.


45-54: LGTM!

The cash subgroup toggle implementation follows the existing pattern on this page, with proper localization, auto-submit behavior, and clear label/hint structure. The UI integration is consistent and clean.

db/schema.rb (1)

337-337: LGTM!

The cash_subgroup_enabled column is properly defined with default: true and null: false, ensuring the feature is enabled by default for all families with no null values. This is a safe, well-designed schema change.

app/views/depositories/_form.html.erb (1)

4-8: Verify nested attributes configuration for accountable association.

The form uses fields_for :accountable to nest the subtype field. Ensure the Account model has accepts_nested_attributes_for :accountable configured to properly handle this nested form structure, and that strong parameters in the controller permit accountable_attributes.

app/models/concerns/accountable.rb (1)

38-39: Verify SUBTYPES keys are lowercase across all accountable types.

The case-insensitive lookup at line 38-39 downcases the input subtype, which assumes all SUBTYPES hash keys are lowercase. Confirm that Depository::SUBTYPES, Investment::SUBTYPES, Crypto::SUBTYPES, Property::SUBTYPES, Vehicle::SUBTYPES, OtherAsset::SUBTYPES, CreditCard::SUBTYPES, Loan::SUBTYPES, and OtherLiability::SUBTYPES all use lowercase keys consistently.

app/controllers/users_controller.rb (1)

109-109: LGTM!

The new :cash_subgroup_enabled attribute is correctly added to the permitted family_attributes. This follows Rails strong parameters conventions and enables the family-level preference toggle to be saved through user updates.

db/migrate/20251205120000_add_cash_subgroup_enabled_to_families.rb (1)

1-5: LGTM!

The migration correctly inherits from ActiveRecord::Migration[7.2] as per guidelines, and properly adds a non-nullable boolean column with a sensible default of true to enable cash subgrouping by default for all families.

test/models/balance_sheet_test.rb (2)

78-100: Good test coverage for cash subgroup scenarios.

The tests appropriately cover the three key scenarios: enabled with known subtypes, disabled (flattened), and unknown subtypes as uncategorized. The approach of using fixtures for base cases and creating accounts inline follows the testing guidelines.


78-110: Verify the expected money values in assertions.

The tests assert uncategorized_total_money.to_f equals 20000.0 for a balance of 200 (line 86) and 15000.0 for a balance of 150 (line 109). This suggests a 100x multiplier. Confirm whether:

  1. converted_balance returns values in dollar units or cents
  2. Money#to_f returns the underlying numeric value (which would be 20000 if Money.new(20000, currency) was called with the multiplied value)
  3. The multiplication in Money.new((uncategorized_total * 100).to_i, currency) is necessary or represents a double-conversion bug
app/models/balance_sheet/account_group.rb (5)

58-62: LGTM!

The uncategorized_accounts method correctly complements subgroups by selecting accounts with nil normalized_subtype. The guard clauses are consistent with the subgrouping logic.


64-66: LGTM!

The aggregation logic is correct and consistent with the existing total method pattern.


91-93: LGTM!

The != false check correctly treats nil as enabled (defaulting to true), which aligns with the default behavior for the cash_subgroup_enabled preference.


96-101: LGTM!

The normalization logic is defensive and thorough, handling nil, blank, and invalid subtypes with case-insensitive validation. This ensures consistent subtype values throughout the grouping logic.


40-56: Verify Depository::SUBTYPES constant and query optimization.

The subgrouping logic appears well-structured with proper guard clauses and filtering. However, the accounts.group_by call should be verified to ensure it doesn't introduce N+1 queries—confirm that accounts is properly preloaded in the calling context. Additionally, verify that Depository::SUBTYPES is defined with the expected subtype keys and is accessible from this context.

app/models/balance_sheet/subtype_group.rb (5)

1-12: LGTM!

The class setup is clean and consistent with AccountGroup. The use of keyword arguments and Monetizable concern follows established patterns in the codebase.


14-16: LGTM!

The name delegation with fallback is well-designed. It correctly retrieves the localized subtype label from the accountable type, with a sensible fallback to the account group name.


18-20: LGTM!

The key method provides a stable identifier with a sensible "other" fallback for nil subtypes, suitable for DOM id generation.


22-24: LGTM!

The aggregation logic is consistent with AccountGroup#total and correctly provides the value for monetization via the monetize macro.


26-28: LGTM!

The currency delegation is correct and ensures consistent currency usage across the grouping hierarchy for monetization.

Comment on lines +31 to +33
<div class="flex items-center px-3 pt-2 text-xs font-medium text-secondary">
<p>Cash</p>
<p class="ml-auto text-sm text-primary whitespace-nowrap"><%= format_money(account_group.uncategorized_total_money) %></p>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hardcoded "Cash" string should use i18n.

Per coding guidelines, all user-facing strings must use localization via the t() helper.

 <div class="flex items-center px-3 pt-2 text-xs font-medium text-secondary">
-  <p>Cash</p>
+  <p><%= t("accounts.cash") %></p>
   <p class="ml-auto text-sm text-primary whitespace-nowrap"><%= format_money(account_group.uncategorized_total_money) %></p>
 </div>
🤖 Prompt for AI Agents
In app/views/accounts/_accountable_group.html.erb around lines 31 to 33, the
literal "Cash" must be localized; replace the hardcoded string with the Rails
i18n helper (e.g. t('accounts.accountable_group.cash') or a view-scoped key like
t('.cash')) and add the corresponding key and translation to the appropriate
locale YAML (e.g. config/locales/en.yml) so the view uses t('...') and has an
English entry for the "Cash" label.

# After page refresh, we're still on the last chat we were viewing
within "#chat-container" do
assert_selector "h1", text: @user.chats.first.title
assert_selector "h1", text: @user.chats.first.title, visible: :all
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unrelated change: chat test modification in cash subgroup PR.

This change to the chat test visibility assertion appears unrelated to the PR's stated objectives of adding cash subgroup functionality. Consider moving unrelated test fixes to a separate commit or PR to maintain clear change history.

🤖 Prompt for AI Agents
In test/system/chats_test.rb around line 45, the visibility change to
assert_selector "h1", text: @user.chats.first.title, visible: :all is unrelated
to the cash subgroup work; revert this change or remove it from this branch and
move it into a separate commit/PR dedicated to test fixes. Restore the assertion
to its original form (undo the visible: :all modification) in this PR, or create
a new commit containing only the test adjustment and reference it separately so
the cash subgroup PR only contains scope-relevant changes.

Add scheduled execution for rules
@jjmata jjmata self-requested a review December 5, 2025 21:44
@jjmata jjmata added this to the v0.6.6 milestone Dec 5, 2025
@jjmata
Copy link
Collaborator

jjmata commented Dec 5, 2025

Love this concept, but let me give it one more twist ... what if instead of having it be "static" SUBTYPE groups and just for cash accounts we take it one level above/more abstract and let people use tags in accounts and then group by tag? 🤔

My number 1 usage pattern for grouping would be business vs. personal accounts, and this approach would allow me to have both if I wanted since I could have any tag configuration I want.

@jjmata jjmata moved this to In progress in Next release (v0.6.6) Dec 8, 2025
Resolved conflicts in:
- app/jobs/rule_job.rb: Adopted enhanced version with RuleRun tracking
- db/schema.rb: Updated to version 2025_12_06_131244 and removed duplicate rule_runs table definition
Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (6)
app/workers/rule_schedule_worker.rb (1)

1-11: Worker behavior is correct; optional extra guard

The worker correctly:

  • Runs on the "scheduled" queue.
  • Reloads the rule by id and bails out if missing or not both scheduled and active.
  • Enqueues RuleJob onto the "scheduled" queue with ignore_attribute_locks: false.

If you want an extra bit of defensive safety against any manually-created cron jobs, you could also require a non-blank cron here:

-    return unless rule&.schedule_enabled? && rule.active?
+    return unless rule&.schedule_enabled? && rule.active? && rule.schedule_cron.present?

Not strictly necessary given how sync_cron_schedule is written, but it makes the guard condition self-contained.

test/models/rule_test.rb (1)

78-127: Good coverage of cron validation and lifecycle

The new tests do a solid job of pinning:

  • Validation of invalid cron strings when schedule_enabled is true.
  • Cron job creation with the expected class/queue/args when enabling an active rule.
  • Cron job destruction when turning scheduling off.

If you want to harden coverage further, consider an extra case asserting the "can't be blank when scheduling is enabled" error when schedule_enabled: true and schedule_cron: "", but what you have is already quite serviceable for this feature.

test/workers/rule_schedule_worker_test.rb (1)

1-45: Worker tests match scheduling semantics

These tests nicely verify the two critical behaviors:

  • When a rule is active and scheduling is enabled, RuleScheduleWorker#perform enqueues RuleJob on the "scheduled" queue.
  • When the rule is inactive or scheduling is disabled, nothing is enqueued.

If you ever see flakiness around deleted rules, you might add a small test for RuleScheduleWorker.new.perform(nonexistent_id) asserting no jobs are enqueued, but the current suite already covers the main paths.

test/jobs/rule_job_test.rb (1)

1-29: RuleJob id-path behavior is well covered

This test correctly exercises the “job receives an id” code path and asserts that the rule still applies as expected, which is the important regression risk after changing perform to accept either a Rule or an id.

If you want symmetry, you could add a second test calling RuleJob.perform_later(rule) (with a Rule instance) to lock in that behavior too, but not strictly required.

app/javascript/controllers/rules_controller.js (1)

5-15: Stimulus scheduling UI wiring looks clean

The added scheduling targets and toggleSchedule() behavior are straightforward and align with the Stimulus conventions:

  • New targets (scheduleCheckbox, scheduleFields, scheduleCronInput) keep responsibilities localized.
  • connect() calling this.toggleSchedule() ensures the initial UI state matches the current checkbox value on both new and edit forms.
  • toggleSchedule() safely no-ops if schedule elements are missing, dims the fields via "opacity-50" when disabled, and toggles disabled on the cron input to avoid posting it accidentally when scheduling is off.

Just make sure the corresponding ERB form has data-rules-target and data-action="rules#toggleSchedule" (or equivalent) wired up to the checkbox so this method is invoked on change as intended.

Also applies to: 17-23, 49-58

app/jobs/rule_job.rb (1)

4-7: LGTM! Good Sidekiq best practice.

The change to accept either a Rule object or ID is well-implemented and follows Sidekiq best practices by allowing callers to pass IDs instead of Active Record objects. Using find_by with the guard clause safely handles the case when a rule doesn't exist.

Optionally, consider adding logging when a rule is not found to aid debugging:

 def perform(rule_or_id, ignore_attribute_locks: false)
   rule = rule_or_id.is_a?(Rule) ? rule_or_id : Rule.find_by(id: rule_or_id)
-  return unless rule
+  return Rails.logger.warn("RuleJob: Rule not found (ID: #{rule_or_id})") unless rule

   rule.apply(ignore_attribute_locks: ignore_attribute_locks)
 end
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5a8d98 and 2a29c7e.

📒 Files selected for processing (11)
  • app/controllers/rules_controller.rb (1 hunks)
  • app/javascript/controllers/rules_controller.js (2 hunks)
  • app/jobs/rule_job.rb (1 hunks)
  • app/models/rule.rb (3 hunks)
  • app/views/rules/_form.html.erb (1 hunks)
  • app/workers/rule_schedule_worker.rb (1 hunks)
  • db/migrate/20251205162439_add_schedule_to_rules.rb (1 hunks)
  • db/schema.rb (3 hunks)
  • test/jobs/rule_job_test.rb (1 hunks)
  • test/models/rule_test.rb (1 hunks)
  • test/workers/rule_schedule_worker_test.rb (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • db/schema.rb
🧰 Additional context used
📓 Path-based instructions (39)
**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins

**/*.rb: Use Rails pluralization in i18n: t("transactions.count", count: @transactions.count).
Run bin/rubocop -f github -a for Ruby linting with auto-correct before pull requests.

**/*.rb: Ruby code should use 2-space indent, snake_case for methods and variables, and CamelCase for classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration

Files:

  • app/controllers/rules_controller.rb
  • test/jobs/rule_job_test.rb
  • test/models/rule_test.rb
  • app/jobs/rule_job.rb
  • test/workers/rule_schedule_worker_test.rb
  • db/migrate/20251205162439_add_schedule_to_rules.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
app/controllers/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Rails strong parameters and CSRF protection throughout the application

app/controllers/**/*.rb: Use strong parameters and CSRF protection throughout the application.
Use query params for state over localStorage/sessions.

Files:

  • app/controllers/rules_controller.rb
**/*.{rb,erb,html.erb}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{rb,erb,html.erb}: Use Current.user for the current user context in controllers and views. Do NOT use current_user.
Use Current.family for the current family context. Do NOT use current_family.

Files:

  • app/controllers/rules_controller.rb
  • test/jobs/rule_job_test.rb
  • test/models/rule_test.rb
  • app/jobs/rule_job.rb
  • test/workers/rule_schedule_worker_test.rb
  • db/migrate/20251205162439_add_schedule_to_rules.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
  • app/views/rules/_form.html.erb
**/*.{erb,html.erb,rb}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via the t() helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.

Files:

  • app/controllers/rules_controller.rb
  • test/jobs/rule_job_test.rb
  • test/models/rule_test.rb
  • app/jobs/rule_job.rb
  • test/workers/rule_schedule_worker_test.rb
  • db/migrate/20251205162439_add_schedule_to_rules.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
  • app/views/rules/_form.html.erb
**/*.{rb,erb}

📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)

**/*.{rb,erb}: Use Current.user for accessing the current user. Do NOT use current_user
Use Current.family for accessing the current family. Do NOT use current_family

Files:

  • app/controllers/rules_controller.rb
  • test/jobs/rule_job_test.rb
  • test/models/rule_test.rb
  • app/jobs/rule_job.rb
  • test/workers/rule_schedule_worker_test.rb
  • db/migrate/20251205162439_add_schedule_to_rules.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
  • app/views/rules/_form.html.erb
**/*.{rb,js,erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only

Files:

  • app/controllers/rules_controller.rb
  • test/jobs/rule_job_test.rb
  • test/models/rule_test.rb
  • app/jobs/rule_job.rb
  • test/workers/rule_schedule_worker_test.rb
  • app/javascript/controllers/rules_controller.js
  • db/migrate/20251205162439_add_schedule_to_rules.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
  • app/views/rules/_form.html.erb
**/*.{rb,html.erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible

Files:

  • app/controllers/rules_controller.rb
  • test/jobs/rule_job_test.rb
  • test/models/rule_test.rb
  • app/jobs/rule_job.rb
  • test/workers/rule_schedule_worker_test.rb
  • db/migrate/20251205162439_add_schedule_to_rules.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
  • app/views/rules/_form.html.erb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Code should be organized in app/ directory (Rails MVC, services, jobs, mailers, components), with JS in app/javascript/, and styles/assets in app/assets/ (Tailwind, images, fonts)

Files:

  • app/controllers/rules_controller.rb
  • app/jobs/rule_job.rb
  • app/javascript/controllers/rules_controller.js
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
app/**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Run bin/brakeman security scan before major PRs to check for static analysis of common Rails issues

Files:

  • app/controllers/rules_controller.rb
  • app/jobs/rule_job.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
test/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

test/**/*.rb: Use Minitest with fixtures (never RSpec or FactoryBot) for testing
Use mocha gem for mocks and VCR for external API tests
Use fixtures for base cases in tests (2-3 per model) and create edge cases on-the-fly within test context
Test critical code paths only: test Commands for correct params, Queries for output

test/**/*.rb: Use Rails' built-in Minitest framework with fixtures for test data. Avoid RSpec and FactoryBot.
Use VCR for external API testing.
Use mocha gem for stubs and mocks. Prefer OpenStruct for mock instances. Only mock what's necessary.

Write tests using Minitest and fixtures as the testing framework

test/**/*.rb: Always use Minitest and fixtures for testing, NEVER rspec or factories
For tests that require a large number of fixture records, use Rails helpers to help create the records needed for the test, then inline the creation instead of defining them in fixtures
Distinguish between commands and query methods. Test output of query methods; test that commands were called with the correct params.
Never test the implementation details of one class in another class's test suite
Use the mocha gem for stubs and mocks
Always prefer OpenStruct when creating mock instances, or in complex cases, a mock class
Only mock what's necessary. If you're not testing return values, don't mock a return value.

Files:

  • test/jobs/rule_job_test.rb
  • test/models/rule_test.rb
  • test/workers/rule_schedule_worker_test.rb
test/**/*_test.rb

📄 CodeRabbit inference engine (AGENTS.md)

test/**/*_test.rb: Test files should use *_test.rb naming convention and mirror app/ structure (e.g., test/models/*_test.rb)
Use Minitest framework for tests with names *_test.rb, run via bin/rails test, and use test/fixtures and VCR cassettes for HTTP testing

Files:

  • test/jobs/rule_job_test.rb
  • test/models/rule_test.rb
  • test/workers/rule_schedule_worker_test.rb
app/jobs/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

app/jobs/**/*.rb: Use Sidekiq for background jobs (e.g., SyncJob, ImportJob, AssistantResponseJob)
Handle account syncing, import processing, and AI responses via background jobs (Sidekiq)
Use background jobs (Sidekiq) for heavy operations instead of synchronous processing

Files:

  • app/jobs/rule_job.rb
{app/**/*.{erb,html},app/javascript/**/*.js}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript

Files:

  • app/javascript/controllers/rules_controller.js
  • app/views/rules/_form.html.erb
app/javascript/controllers/**/*_controller.js

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

app/javascript/controllers/**/*_controller.js: Keep Stimulus controllers lightweight and simple with fewer than 7 targets
Keep Stimulus controllers small with private methods and clear public API

Files:

  • app/javascript/controllers/rules_controller.js
app/javascript/**/*.js

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use D3.js for financial visualizations (time series, donut, sankey charts)

Files:

  • app/javascript/controllers/rules_controller.js
{app/components/**/*.js,app/components/**/*.ts,app/javascript/controllers/**/*.{js,ts}}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Stimulus Controllers for interactivity, organized alongside components in app/components/ or globally in app/javascript/controllers/.

Files:

  • app/javascript/controllers/rules_controller.js
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use D3.js for financial visualizations including time series, donut, and sankey charts.

Files:

  • app/javascript/controllers/rules_controller.js
{app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts},app/components/**/*.html.erb}

📄 CodeRabbit inference engine (CLAUDE.md)

Use declarative Stimulus actions in HTML with data-action and data-*-target attributes. Keep controllers lightweight and simple (< 7 targets).

Files:

  • app/javascript/controllers/rules_controller.js
{app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts}}

📄 CodeRabbit inference engine (CLAUDE.md)

{app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts}}: Use private methods in Stimulus controllers and expose a clear public API.
Component Stimulus controllers should stay in component directory (app/components/). Global controllers should be in app/javascript/controllers/.

Files:

  • app/javascript/controllers/rules_controller.js
**/*_controller.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)

**/*_controller.{js,ts}: Use declarative actions in Stimulus controllers, not imperative event listeners - define actions in ERB templates using data-action attributes instead of binding events in the controller's connect() method
Keep Stimulus controllers lightweight and simple - aim for fewer than 7 controller targets as a sign of excessive complexity
Use private methods in Stimulus controllers and expose a clear public API
Keep Stimulus controllers focused on single or highly related responsibilities - domain logic should not belong in Stimulus controllers

Files:

  • app/javascript/controllers/rules_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/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values

Files:

  • app/javascript/controllers/rules_controller.js
  • app/views/rules/_form.html.erb
app/javascript/controllers/**/*.js

📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)

Use Stimulus targets to reference DOM elements, not manual getElementById calls

Files:

  • app/javascript/controllers/rules_controller.js
app/javascript/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

JavaScript code should use lowerCamelCase for variables and functions, and PascalCase for classes and components

Files:

  • app/javascript/controllers/rules_controller.js
app/**/*.{js,ts,jsx,tsx,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

JavaScript and CSS code should be formatted using Biome via npm run lint and npm run format

Files:

  • app/javascript/controllers/rules_controller.js
db/migrate/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use simple validations (null checks, unique indexes) in database layer

Files:

  • db/migrate/20251205162439_add_schedule_to_rules.rb
{app/models/**/*.rb,db/migrate/**/*.rb}

📄 CodeRabbit inference engine (CLAUDE.md)

Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.

Files:

  • db/migrate/20251205162439_add_schedule_to_rules.rb
  • app/models/rule.rb
db/migrate/*.rb

📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)

ActiveRecord migrations must inherit from ActiveRecord::Migration[7.2]. Do NOT use version 8.0 yet

Enforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL

Files:

  • db/migrate/20251205162439_add_schedule_to_rules.rb
db/**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Data migrations should be placed in db/ directory with test fixtures in test/fixtures/

Files:

  • db/migrate/20251205162439_add_schedule_to_rules.rb
app/models/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder

app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). Use Money objects for currency conversion and formatting.
Place business logic in app/models/ folder. Avoid creating app/services/ folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: use account.balance_series not AccountSeries.new(account).call.

Files:

  • app/models/rule.rb
app/views/**/_*.html.erb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use partials only for simple, static HTML with minimal logic in specific contexts

app/views/**/_*.html.erb: Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Place context-specific partials in relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)

Files:

  • app/views/rules/_form.html.erb
app/views/**/*.erb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

app/views/**/*.erb: Always use the icon helper for icons in views, never use lucide_icon directly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-side

Views should use ERB and avoid heavy logic; prefer helpers and components instead (ERB checked by erb-lint per .erb_lint.yml)

Files:

  • app/views/rules/_form.html.erb
**/*.{css,erb}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Tailwind CSS v4.x with custom design system for styling

Files:

  • app/views/rules/_form.html.erb
**/*.{erb,html,vue,jsx,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{erb,html,vue,jsx,tsx}: Use Tailwind CSS v4.x with custom design system defined in app/assets/tailwind/maybe-design-system.css. Always use functional tokens (e.g., text-primary not text-white).
Prefer semantic HTML elements over JavaScript components (e.g., use <dialog> for modals, <details><summary> for disclosures).

Files:

  • app/views/rules/_form.html.erb
**/*.{erb,html.erb}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{erb,html.erb}: Use icon helper for icons, never use lucide_icon directly.
Leverage Turbo frames for page sections over client-side solutions.
Pass data to Stimulus controllers via data-*-value attributes, not inline JavaScript.

**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript

Files:

  • app/views/rules/_form.html.erb
app/views/**/_*.{erb,html.erb}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Partials when element is primarily static HTML with minimal logic, used in only one or few contexts, is simple template content, or doesn't need variants/sizes/complex configuration.

Files:

  • app/views/rules/_form.html.erb
{app/components/**/*.html.erb,app/views/**/*.{erb,html.erb}}

📄 CodeRabbit inference engine (CLAUDE.md)

Keep domain logic out of view templates. Logic belongs in component files, not template files.

Files:

  • app/views/rules/_form.html.erb
**/*.erb

📄 CodeRabbit inference engine (CLAUDE.md)

Run bundle exec erb_lint ./app/**/*.erb -a for ERB linting with auto-correct before pull requests.

Files:

  • app/views/rules/_form.html.erb
**/*.html.erb

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,

for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use the icon helper from application_helper.rb for icons. Never use lucide_icon helper directly

Files:

  • app/views/rules/_form.html.erb
{app/views/**,app/helpers/**}

📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)

Always generate semantic HTML

Files:

  • app/views/rules/_form.html.erb
🧠 Learnings (26)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Account model is a Rails delegated type with subtypes: Depository, Investment, Crypto, Property, Vehicle, OtherAsset (assets) and CreditCard, Loan, OtherLiability (liabilities)
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Core domain model: User → Accounts → Transactions. Accounts support types: checking, savings, credit cards, investments, crypto, loans, properties. Transactions belong to Categories and may have Tags and Rules.
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Document breaking changes in rules

Applied to files:

  • app/controllers/rules_controller.rb
  • db/migrate/20251205162439_add_schedule_to_rules.rb
  • app/models/rule.rb
  • app/views/rules/_form.html.erb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Modify Existing Rules When: Implementation details have changed

Applied to files:

  • app/controllers/rules_controller.rb
  • app/views/rules/_form.html.erb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to app/controllers/**/*.rb : Use strong parameters and CSRF protection throughout the application.

Applied to files:

  • app/controllers/rules_controller.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/controllers/**/*.rb : Use Rails strong parameters and CSRF protection throughout the application

Applied to files:

  • app/controllers/rules_controller.rb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Modify Existing Rules When: Better examples exist in the codebase

Applied to files:

  • app/controllers/rules_controller.rb
  • app/views/rules/_form.html.erb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Update rules after major refactors

Applied to files:

  • app/controllers/rules_controller.rb
  • app/views/rules/_form.html.erb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Modify Existing Rules When: Related rules have been updated

Applied to files:

  • app/controllers/rules_controller.rb
  • app/views/rules/_form.html.erb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Modify Existing Rules When: Additional edge cases are discovered

Applied to files:

  • app/controllers/rules_controller.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to test/**/*.rb : Use fixtures for base cases in tests (2-3 per model) and create edge cases on-the-fly within test context

Applied to files:

  • test/jobs/rule_job_test.rb
  • test/models/rule_test.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to test/**/*.rb : Test critical code paths only: test Commands for correct params, Queries for output

Applied to files:

  • test/models/rule_test.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Use background jobs (Sidekiq) for heavy operations instead of synchronous processing

Applied to files:

  • app/jobs/rule_job.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Use Sidekiq for background jobs (e.g., SyncJob, ImportJob, AssistantResponseJob)

Applied to files:

  • app/jobs/rule_job.rb
  • test/workers/rule_schedule_worker_test.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Handle account syncing, import processing, and AI responses via background jobs (Sidekiq)

Applied to files:

  • app/jobs/rule_job.rb
  • test/workers/rule_schedule_worker_test.rb
  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to {app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts},app/components/**/*.html.erb} : Use declarative Stimulus actions in HTML with `data-action` and `data-*-target` attributes. Keep controllers lightweight and simple (< 7 targets).

Applied to files:

  • app/javascript/controllers/rules_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/rules_controller.js
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to {app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts}} : Use private methods in Stimulus controllers and expose a clear public API.

Applied to files:

  • app/javascript/controllers/rules_controller.js
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/javascript/controllers/**/*_controller.js : Keep Stimulus controllers lightweight and simple with fewer than 7 targets

Applied to files:

  • app/javascript/controllers/rules_controller.js
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/javascript/controllers/**/*_controller.js : Keep Stimulus controllers small with private methods and clear public API

Applied to files:

  • app/javascript/controllers/rules_controller.js
📚 Learning: 2025-11-24T16:56:51.982Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:51.982Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers lightweight and simple - aim for fewer than 7 controller targets as a sign of excessive complexity

Applied to files:

  • app/javascript/controllers/rules_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/rules_controller.js
📚 Learning: 2025-11-24T16:56:51.982Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:51.982Z
Learning: Applies to **/*_controller.{js,ts} : Use private methods in Stimulus controllers and expose a clear public API

Applied to files:

  • app/javascript/controllers/rules_controller.js
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to {app/components/**/*.js,app/components/**/*.ts,app/javascript/controllers/**/*.{js,ts}} : Use Stimulus Controllers for interactivity, organized alongside components in `app/components/` or globally in `app/javascript/controllers/`.

Applied to files:

  • app/javascript/controllers/rules_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/rules_controller.js
📚 Learning: 2025-11-24T16:55:59.156Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/general-rules.mdc:0-0
Timestamp: 2025-11-24T16:55:59.156Z
Learning: Applies to db/migrate/*.rb : ActiveRecord migrations must inherit from `ActiveRecord::Migration[7.2]`. Do NOT use version 8.0 yet

Applied to files:

  • db/migrate/20251205162439_add_schedule_to_rules.rb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Use Sidekiq for background processing including account syncing, import processing, AI chat responses, and scheduled maintenance via sidekiq-cron.

Applied to files:

  • app/models/rule.rb
  • app/workers/rule_schedule_worker.rb
🧬 Code graph analysis (5)
test/jobs/rule_job_test.rb (5)
app/workers/rule_schedule_worker.rb (1)
  • include (1-12)
test/models/rule_test.rb (1)
  • include (3-128)
test/workers/rule_schedule_worker_test.rb (1)
  • include (3-46)
app/models/rule.rb (1)
  • new (4-178)
app/models/rule/condition.rb (1)
  • rule (14-16)
test/models/rule_test.rb (2)
app/models/rule/condition.rb (1)
  • rule (14-16)
app/models/rule.rb (1)
  • new (4-178)
app/jobs/rule_job.rb (2)
app/workers/rule_schedule_worker.rb (1)
  • perform (6-11)
app/models/rule/condition.rb (1)
  • rule (14-16)
test/workers/rule_schedule_worker_test.rb (3)
app/workers/rule_schedule_worker.rb (2)
  • include (1-12)
  • perform (6-11)
test/jobs/rule_job_test.rb (1)
  • include (3-29)
app/jobs/rule_job.rb (1)
  • perform (4-9)
app/workers/rule_schedule_worker.rb (4)
test/jobs/rule_job_test.rb (1)
  • include (3-29)
test/workers/rule_schedule_worker_test.rb (1)
  • include (3-46)
app/jobs/rule_job.rb (1)
  • perform (4-9)
app/models/rule/condition.rb (1)
  • rule (14-16)
🔇 Additional comments (3)
db/migrate/20251205162439_add_schedule_to_rules.rb (1)

1-5: Migration shape looks good

Using ActiveRecord::Migration[7.2] with schedule_enabled constrained to null: false, default: false is consistent with the migration guidelines; schedule_cron being nullable works with the model validation that only enforces presence when scheduling is enabled.

app/controllers/rules_controller.rb (1)

86-97: Strong params correctly extended for scheduling

Allowing :schedule_enabled and :schedule_cron through rule_params is exactly what’s needed to wire the new scheduling UI into the model; no issues from an auth/strong-params perspective.

app/models/rule.rb (1)

1-2: Cron scheduling logic is sound; verify dependencies and document behavior

The new scheduling block in Rule looks well thought out:

  • valid_schedule_when_enabled:
    • Only runs when schedule_enabled? is true.
    • Enforces presence of schedule_cron and validates it via Fugit::Cron.parse, rescuing parse errors into a clean "is invalid" message.
  • State tracking:
    • schedule_state_changed? keys only off schedule_cron, schedule_enabled, and active, which are exactly the attributes that affect scheduling.
    • scheduled_before_change? carefully reconstructs the "previous" scheduled state using *_before_last_save when available, so you only call destroy on cron jobs that actually existed.
  • Callbacks:
    • after_commit :sync_cron_schedule on create/update creates or updates a Sidekiq::Cron::Job when schedule_enabled? && active? && schedule_cron.present?, and otherwise tears it down if it was previously scheduled.
    • after_commit :remove_cron_schedule on destroy gives you a final cleanup pass for any remaining cron job tied to this rule id.
  • Operationally, failures in cron synchronization are logged but do not break saves, which is the right tradeoff.

Two follow-ups worth considering:

  1. Verify Gem dependencies are present
    Since the model now requires "fugit" and "sidekiq/cron/job", ensure both gems are in your Gemfile and loaded in all environments that instantiate Rule (including test).

  2. Document the new scheduling semantics for rules
    Since rules can now be scheduled via cron and must satisfy the new validation when schedule_enabled is true, update your rules-related documentation to describe:

    • The meaning of schedule_enabled + schedule_cron.
    • That only active rules with a non-blank, valid cron expression are scheduled.
    • That cron jobs are automatically created/updated/destroyed as these attributes change.

Comment on lines +104 to +133
<section class="space-y-4">
<div class="flex items-center gap-1 text-secondary">
<h3 class="text-sm font-medium text-primary">WHEN</h3>
</div>

<div class="ml-6 space-y-3">
<div class="flex items-center gap-3 text-sm text-primary">
<%= render DS::Toggle.new(
id: dom_id(rule, :schedule_toggle),
name: "rule[schedule_enabled]",
checked: rule.schedule_enabled?,
disabled: false,
data: { rules_target: "scheduleCheckbox", action: "change->rules#toggleSchedule" }
) %>
<label for="<%= dom_id(rule, :schedule_toggle) %>" class="cursor-pointer">Run this rule on a schedule</label>
</div>

<div class="space-y-2 transition-opacity" data-rules-target="scheduleFields">
<div class="flex items-center gap-2 text-sm text-secondary">
<%= f.label :schedule_cron, "Cron expression" %>
<%= icon "info", size: "xs", class: "text-secondary", title: "Use standard cron syntax (min hour day month weekday)" %>
</div>
<%= f.text_field :schedule_cron,
placeholder: "0 0 * * *",
class: "form-field__input w-full",
data: { rules_target: "scheduleCronInput" } %>
<p class="text-xs text-secondary">Example: 0 0 * * * to run every day at midnight.</p>
</div>
</div>
</section>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Missing i18n for all user-facing strings.

All user-facing strings in this new section are hardcoded in English, violating the coding guidelines requirement that "All user-facing strings must use localization (i18n) via the t() helper."

The following strings need to be localized:

  • Line 106: "WHEN"
  • Line 118: "Run this rule on a schedule"
  • Line 123: "Cron expression"
  • Line 124: Info icon title text
  • Line 130: "Example: 0 0 * * * to run every day at midnight."

Apply this approach to fix the i18n violations:

     <div class="flex items-center gap-1 text-secondary">
-      <h3 class="text-sm font-medium text-primary">WHEN</h3>
+      <h3 class="text-sm font-medium text-primary"><%= t("rules.form.when") %></h3>
     </div>

     <div class="ml-6 space-y-3">
       <div class="flex items-center gap-3 text-sm text-primary">
         <%= render DS::Toggle.new(
               id: dom_id(rule, :schedule_toggle),
               name: "rule[schedule_enabled]",
               checked: rule.schedule_enabled?,
               disabled: false,
               data: { rules_target: "scheduleCheckbox", action: "change->rules#toggleSchedule" }
             ) %>
-        <label for="<%= dom_id(rule, :schedule_toggle) %>" class="cursor-pointer">Run this rule on a schedule</label>
+        <label for="<%= dom_id(rule, :schedule_toggle) %>" class="cursor-pointer"><%= t("rules.form.schedule.label") %></label>
       </div>

       <div class="space-y-2 transition-opacity" data-rules-target="scheduleFields">
         <div class="flex items-center gap-2 text-sm text-secondary">
-          <%= f.label :schedule_cron, "Cron expression" %>
-          <%= icon "info", size: "xs", class: "text-secondary", title: "Use standard cron syntax (min hour day month weekday)" %>
+          <%= f.label :schedule_cron, t("rules.form.schedule.cron_label") %>
+          <%= icon "info", size: "xs", class: "text-secondary", title: t("rules.form.schedule.cron_help") %>
         </div>
         <%= f.text_field :schedule_cron,
               placeholder: "0 0 * * *",
               class: "form-field__input w-full",
               data: { rules_target: "scheduleCronInput" } %>
-        <p class="text-xs text-secondary">Example: 0 0 * * * to run every day at midnight.</p>
+        <p class="text-xs text-secondary"><%= t("rules.form.schedule.cron_example") %></p>
       </div>
     </div>

Then add corresponding entries to the locale files (e.g., config/locales/en.yml):

en:
  rules:
    form:
      when: "WHEN"
      schedule:
        label: "Run this rule on a schedule"
        cron_label: "Cron expression"
        cron_help: "Use standard cron syntax (min hour day month weekday)"
        cron_example: "Example: 0 0 * * * to run every day at midnight."
🤖 Prompt for AI Agents
In app/views/rules/_form.html.erb around lines 104 to 133, several user-facing
strings are hardcoded; replace each with t() lookups using keys under rules.form
(e.g. t('rules.form.when') for "WHEN", t('rules.form.schedule.label') for "Run
this rule on a schedule", t('rules.form.schedule.cron_label') for "Cron
expression", pass the info icon title via t('rules.form.schedule.cron_help'),
and t('rules.form.schedule.cron_example') for the example text; then add
matching entries to config/locales/en.yml under en: rules: form: ... with the
strings provided in the review comment.

Copy link

@coderabbitai coderabbitai bot left a 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 (2)
app/models/rule.rb (2)

157-172: Localize new validation messages for schedule errors

valid_schedule_when_enabled introduces user-facing error strings directly in the model:

  • "can't be blank when scheduling is enabled"
  • "is invalid"

Per the project guidelines, these should go through i18n rather than hard-coded strings.

As per coding guidelines, consider adding translation keys (e.g., errors.messages.rule.schedule_cron_blank_when_enabled, errors.messages.rule.schedule_cron_invalid) and switching to:

- errors.add(:schedule_cron, "can't be blank when scheduling is enabled")
+ errors.add(:schedule_cron, :blank_when_scheduling_enabled)

- errors.add(:schedule_cron, "is invalid") unless parsed_cron
+ errors.add(:schedule_cron, :invalid_schedule_cron) unless parsed_cron

with the corresponding entries in your locale files.


27-27: Optionally simplify cron cleanup on destroy

Right now remove_cron_schedule on destroy is guarded by if: :scheduled_before_change?. Since remove_cron_schedule already guards on id and rescues errors, it should be safe and cheap to always attempt cleanup on destroy, which also covers any prior inconsistencies between DB state and Sidekiq cron.

You could simplify to:

-  after_commit :remove_cron_schedule, on: :destroy, if: :scheduled_before_change?
+  after_commit :remove_cron_schedule, on: :destroy

leaving remove_cron_schedule unchanged.

Also applies to: 195-201

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a29c7e and d7fa758.

📒 Files selected for processing (3)
  • app/controllers/rules_controller.rb (1 hunks)
  • app/models/rule.rb (3 hunks)
  • db/schema.rb (3 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins

**/*.rb: Use Rails pluralization in i18n: t("transactions.count", count: @transactions.count).
Run bin/rubocop -f github -a for Ruby linting with auto-correct before pull requests.

**/*.rb: Ruby code should use 2-space indent, snake_case for methods and variables, and CamelCase for classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration

Files:

  • app/models/rule.rb
  • app/controllers/rules_controller.rb
  • db/schema.rb
app/models/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder

app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). Use Money objects for currency conversion and formatting.
Place business logic in app/models/ folder. Avoid creating app/services/ folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: use account.balance_series not AccountSeries.new(account).call.

Files:

  • app/models/rule.rb
**/*.{rb,erb,html.erb}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{rb,erb,html.erb}: Use Current.user for the current user context in controllers and views. Do NOT use current_user.
Use Current.family for the current family context. Do NOT use current_family.

Files:

  • app/models/rule.rb
  • app/controllers/rules_controller.rb
  • db/schema.rb
**/*.{erb,html.erb,rb}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via the t() helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.

Files:

  • app/models/rule.rb
  • app/controllers/rules_controller.rb
  • db/schema.rb
{app/models/**/*.rb,db/migrate/**/*.rb}

📄 CodeRabbit inference engine (CLAUDE.md)

Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.

Files:

  • app/models/rule.rb
**/*.{rb,erb}

📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)

**/*.{rb,erb}: Use Current.user for accessing the current user. Do NOT use current_user
Use Current.family for accessing the current family. Do NOT use current_family

Files:

  • app/models/rule.rb
  • app/controllers/rules_controller.rb
  • db/schema.rb
**/*.{rb,js,erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only

Files:

  • app/models/rule.rb
  • app/controllers/rules_controller.rb
  • db/schema.rb
**/*.{rb,html.erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible

Files:

  • app/models/rule.rb
  • app/controllers/rules_controller.rb
  • db/schema.rb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Code should be organized in app/ directory (Rails MVC, services, jobs, mailers, components), with JS in app/javascript/, and styles/assets in app/assets/ (Tailwind, images, fonts)

Files:

  • app/models/rule.rb
  • app/controllers/rules_controller.rb
app/**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Run bin/brakeman security scan before major PRs to check for static analysis of common Rails issues

Files:

  • app/models/rule.rb
  • app/controllers/rules_controller.rb
app/controllers/**/*.rb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Rails strong parameters and CSRF protection throughout the application

app/controllers/**/*.rb: Use strong parameters and CSRF protection throughout the application.
Use query params for state over localStorage/sessions.

Files:

  • app/controllers/rules_controller.rb
db/**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

Data migrations should be placed in db/ directory with test fixtures in test/fixtures/

Files:

  • db/schema.rb
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Core domain model: User → Accounts → Transactions. Accounts support types: checking, savings, credit cards, investments, crypto, loans, properties. Transactions belong to Categories and may have Tags and Rules.
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Use Sidekiq for background processing including account syncing, import processing, AI chat responses, and scheduled maintenance via sidekiq-cron.

Applied to files:

  • app/models/rule.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Handle account syncing, import processing, and AI responses via background jobs (Sidekiq)

Applied to files:

  • app/models/rule.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Use Sidekiq for background jobs (e.g., SyncJob, ImportJob, AssistantResponseJob)

Applied to files:

  • app/models/rule.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Use background jobs (Sidekiq) for heavy operations instead of synchronous processing

Applied to files:

  • app/models/rule.rb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to app/controllers/**/*.rb : Use strong parameters and CSRF protection throughout the application.

Applied to files:

  • app/controllers/rules_controller.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/controllers/**/*.rb : Use Rails strong parameters and CSRF protection throughout the application

Applied to files:

  • app/controllers/rules_controller.rb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Modify Existing Rules When: Implementation details have changed

Applied to files:

  • app/controllers/rules_controller.rb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Document breaking changes in rules

Applied to files:

  • app/controllers/rules_controller.rb
  • db/schema.rb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Modify Existing Rules When: Better examples exist in the codebase

Applied to files:

  • app/controllers/rules_controller.rb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Modify Existing Rules When: Additional edge cases are discovered

Applied to files:

  • app/controllers/rules_controller.rb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Update rules after major refactors

Applied to files:

  • db/schema.rb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to db/migrate/*.rb : Enforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL

Applied to files:

  • db/schema.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to db/migrate/**/*.rb : Use simple validations (null checks, unique indexes) in database layer

Applied to files:

  • db/schema.rb
📚 Learning: 2025-11-24T16:56:30.669Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Syncable models (Account, PlaidItem, Family) must implement sync operations tracked via Sync records that log status, errors, and maintain an audit trail

Applied to files:

  • db/schema.rb
🧬 Code graph analysis (1)
app/models/rule.rb (2)
app/models/provider/configurable.rb (2)
  • validate (238-254)
  • present? (226-228)
app/controllers/rules_controller.rb (2)
  • create (34-42)
  • destroy (81-84)
🔇 Additional comments (3)
app/controllers/rules_controller.rb (1)

96-107: Strong params for scheduling look correct

Adding :schedule_enabled and :schedule_cron to rule_params is consistent with the new Rule scheduling attributes and keeps parameter whitelisting tight.

app/models/rule.rb (1)

1-2: Cron scheduling integration is coherent with Rule lifecycle

The new cron scheduling wiring (validation, schedule_state_changed?, scheduled_before_change?, and the after_commit hooks that call sync_cron_schedule / remove_cron_schedule) matches the Rule lifecycle and should keep Sidekiq cron jobs in sync when rules are created, updated, activated/deactivated, or destroyed. Error handling via Rails.logger.error in both sync/remove paths is also appropriate.

Please verify in your environment that Sidekiq::Cron::Job.create with a fixed name updates the existing job rather than leaving duplicates, and that your tests cover the key transitions (enable/disable schedule, change cron, deactivate rule, destroy rule).

Also applies to: 20-27, 141-201

db/schema.rb (1)

396-412: New schema columns and constraints look consistent with usage

The added columns:

  • families.cash_subgroup_enabled (default true, null: false)
  • rule_runs.rule_name, transactions_queued, pending_jobs_count (integer counters with default: 0, null: false)
  • rules.schedule_cron and rules.schedule_enabled (boolean with default: false, null: false)

follow the project’s pattern of enforcing simple constraints at the DB layer and provide sensible defaults for existing data.

Also applies to: 867-897

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

2 participants