From dbe05e3795905b410d6e8895c0acb2413f04d76e Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Tue, 17 Jun 2025 11:36:05 -0500 Subject: [PATCH] Fix rubocop offenses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix indentation and spacing issues - Convert single quotes to double quotes - Add spaces inside array brackets - Fix comment alignment - Add missing trailing newlines - Correct else/end alignment 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- CLAUDE.md | 172 ++++++++++++++++++ app/controllers/api/v1/accounts_controller.rb | 4 +- app/controllers/api/v1/base_controller.rb | 38 ++-- .../api/v1/transactions_controller.rb | 42 ++--- .../settings/api_keys_controller.rb | 2 +- app/services/api_rate_limiter.rb | 4 +- .../api/v1/base_controller_test.rb | 2 +- .../api/v1/transactions_controller_test.rb | 42 ++--- .../api/v1/usage_controller_test.rb | 6 +- test/services/api_rate_limiter_test.rb | 6 +- 10 files changed, 245 insertions(+), 73 deletions(-) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..daf4f17c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,172 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Common Development Commands + +### Development Server +- `bin/dev` - Start development server (Rails, Sidekiq, Tailwind CSS watcher) +- `bin/rails server` - Start Rails server only +- `bin/rails console` - Open Rails console + +### Testing +- `bin/rails test` - Run all tests +- `bin/rails test:db` - Run tests with database reset +- `bin/rails test:system` - Run system tests only +- `bin/rails test test/models/account_test.rb` - Run specific test file +- `bin/rails test test/models/account_test.rb:42` - Run specific test at line + +### Linting & Formatting +- `bin/rubocop` - Run Ruby linter +- `npm run lint` - Check JavaScript/TypeScript code +- `npm run lint:fix` - Fix JavaScript/TypeScript issues +- `npm run format` - Format JavaScript/TypeScript code +- `bin/brakeman` - Run security analysis + +### Database +- `bin/rails db:prepare` - Create and migrate database +- `bin/rails db:migrate` - Run pending migrations +- `bin/rails db:rollback` - Rollback last migration +- `bin/rails db:seed` - Load seed data + +### Setup +- `bin/setup` - Initial project setup (installs dependencies, prepares database) + +## General Development Rules + +### Authentication Context +- Use `Current.user` for the current user. Do NOT use `current_user`. +- Use `Current.family` for the current family. Do NOT use `current_family`. + +### Development Guidelines +- Prior to generating any code, carefully read the project conventions and guidelines +- Ignore i18n methods and files. Hardcode strings in English for now to optimize speed of development +- Do not run `rails server` in your responses +- Do not run `touch tmp/restart.txt` +- Do not run `rails credentials` +- Do not automatically run migrations + +## High-Level Architecture + +### Application Modes +The Maybe app runs in two distinct modes: +- **Managed**: The Maybe team operates and manages servers for users (Rails.application.config.app_mode = "managed") +- **Self Hosted**: Users host the Maybe app on their own infrastructure, typically through Docker Compose (Rails.application.config.app_mode = "self_hosted") + +### Core Domain Model +The application is built around financial data management with these key relationships: +- **User** → has many **Accounts** → has many **Transactions** +- **Account** types: checking, savings, credit cards, investments, crypto, loans, properties +- **Transaction** → belongs to **Category**, can have **Tags** and **Rules** +- **Investment accounts** → have **Holdings** → track **Securities** via **Trades** + +### API Architecture +The application provides both internal and external APIs: +- Internal API: Controllers serve JSON via Turbo for SPA-like interactions +- External API: `/api/v1/` namespace with Doorkeeper OAuth and API key authentication +- API responses use Jbuilder templates for JSON rendering +- Rate limiting via Rack Attack with configurable limits per API key + +### Sync & Import System +Two primary data ingestion methods: +1. **Plaid Integration**: Real-time bank account syncing + - `PlaidItem` manages connections + - `Sync` tracks sync operations + - Background jobs handle data updates +2. **CSV Import**: Manual data import with mapping + - `Import` manages import sessions + - Supports transaction and balance imports + - Custom field mapping with transformation rules + +### Background Processing +Sidekiq handles asynchronous tasks: +- Account syncing (`SyncAccountsJob`) +- Import processing (`ImportDataJob`) +- AI chat responses (`CreateChatResponseJob`) +- Scheduled maintenance via sidekiq-cron + +### Frontend Architecture +- **Hotwire Stack**: Turbo + Stimulus for reactive UI without heavy JavaScript +- **ViewComponents**: Reusable UI components in `app/components/` +- **Stimulus Controllers**: Handle interactivity, organized alongside components +- **Charts**: D3.js for financial visualizations (time series, donut, sankey) +- **Styling**: Tailwind CSS v4.x with custom design system + - 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 JS components + - Use `icon` helper for icons, never `lucide_icon` directly + +### Multi-Currency Support +- All monetary values stored in base currency (user's primary currency) +- Exchange rates fetched from Synth API +- `Money` objects handle currency conversion and formatting +- Historical exchange rates for accurate reporting + +### Security & Authentication +- Session-based auth for web users +- API authentication via: + - OAuth2 (Doorkeeper) for third-party apps + - API keys with JWT tokens for direct API access +- Scoped permissions system for API access +- Strong parameters and CSRF protection throughout + +### Key Service Objects & Patterns +- **Query Objects**: Complex database queries isolated in `app/queries/` +- **Service Objects**: Business logic in `app/services/` +- **Form Objects**: Complex forms with validation +- **Concerns**: Shared functionality across models/controllers +- **Jobs**: Background processing logic + +### Testing Philosophy +- Comprehensive test coverage using Rails' built-in Minitest +- Fixtures for test data (avoid FactoryBot) +- Keep fixtures minimal (2-3 per model for base cases) +- VCR for external API testing +- System tests for critical user flows (use sparingly) +- Test helpers in `test/support/` for common scenarios +- Only test critical code paths that significantly increase confidence +- Write tests as you go, when required + +### Performance Considerations +- Database queries optimized with proper indexes +- N+1 queries prevented via includes/joins +- Background jobs for heavy operations +- Caching strategies for expensive calculations +- Turbo Frames for partial page updates + +### Development Workflow +- Feature branches merged to `main` +- Docker support for consistent environments +- Environment variables via `.env` files +- Lookbook for component development (`/lookbook`) +- Letter Opener for email preview in development + +## Project Conventions + +### Convention 1: Minimize Dependencies +- Push Rails to its limits before adding new dependencies +- When adding dependencies, favor old and reliable over new and flashy +- Strong technical or business reason required for new dependencies + +### Convention 2: POROs and Concerns over Service Objects +- "Skinny controller, fat models" convention +- Everything in `app/models/` folder, avoid separate folders like `app/services/` +- Use Rails concerns for better organization (can be one-off concerns) +- Models should answer questions about themselves (e.g., `account.balance_series`) + +### Convention 3: Leverage Hotwire and Server-Side Solutions +- Native HTML preferred over JS components (e.g., ``, `
`) +- Use Turbo frames to break up pages +- Leverage query params for state over local storage +- Format values server-side, pass to Stimulus for display only +- Client-side code only where it truly shines (e.g., bulk selections) + +### Convention 4: Optimize for Simplicity and Clarity +- Prioritize good OOP domain design over performance +- Only focus on performance in critical/global areas +- Be mindful of N+1 queries and large data payloads + +### Convention 5: ActiveRecord for Complex Validations, DB for Simple Ones +- Enforce null checks, unique indexes in the DB +- ActiveRecord validations for convenience in forms +- Complex validations and business logic in ActiveRecord \ No newline at end of file diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index ce12cc76..98a49fef 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -6,7 +6,7 @@ class Api::V1::AccountsController < Api::V1::BaseController # Ensure proper scope authorization for read access before_action :ensure_read_scope - def index + def index # Test with Pagy pagination family = current_resource_owner.family accounts_query = family.accounts.active.alphabetically @@ -30,7 +30,7 @@ class Api::V1::AccountsController < Api::V1::BaseController error: "internal_server_error", message: "Error: #{e.message}" }, status: :internal_server_error - end +end private diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index 76883150..adced683 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -9,7 +9,7 @@ class Api::V1::BaseController < ApplicationController # Skip onboarding requirements for API endpoints skip_before_action :require_onboarding_and_upgrade - # Force JSON format for all API requests + # Force JSON format for all API requests before_action :force_json_format # Use our custom authentication that supports both OAuth and API keys before_action :authenticate_request! @@ -35,7 +35,7 @@ class Api::V1::BaseController < ApplicationController request.format = :json end - # Authenticate using either OAuth or API key + # Authenticate using either OAuth or API key def authenticate_request! return if authenticate_oauth return if authenticate_api_key @@ -46,12 +46,12 @@ class Api::V1::BaseController < ApplicationController def authenticate_oauth return false unless request.headers["Authorization"].present? - # Manually verify the token (bypassing doorkeeper_authorize! which had scope issues) - token_string = request.authorization&.split(' ')&.last + # Manually verify the token (bypassing doorkeeper_authorize! which had scope issues) + token_string = request.authorization&.split(" ")&.last access_token = Doorkeeper::AccessToken.by_token(token_string) # Check token validity and scope (read_write includes read access) - has_sufficient_scope = access_token&.scopes&.include?('read') || access_token&.scopes&.include?('read_write') + has_sufficient_scope = access_token&.scopes&.include?("read") || access_token&.scopes&.include?("read_write") unless access_token && !access_token.expired? && has_sufficient_scope render_json({ error: "unauthorized", message: "Access token is invalid, expired, or missing required scope" }, status: :unauthorized) @@ -61,24 +61,24 @@ class Api::V1::BaseController < ApplicationController # Set the doorkeeper_token for compatibility @_doorkeeper_token = access_token - if doorkeeper_token&.resource_owner_id - @current_user = User.find_by(id: doorkeeper_token.resource_owner_id) + if doorkeeper_token&.resource_owner_id + @current_user = User.find_by(id: doorkeeper_token.resource_owner_id) - # If user doesn't exist, the token is invalid (user was deleted) - unless @current_user - Rails.logger.warn "API OAuth Token Invalid: Access token resource_owner_id #{doorkeeper_token.resource_owner_id} does not exist" - render_json({ error: "unauthorized", message: "Access token is invalid - user not found" }, status: :unauthorized) - return false - end + # If user doesn't exist, the token is invalid (user was deleted) + unless @current_user + Rails.logger.warn "API OAuth Token Invalid: Access token resource_owner_id #{doorkeeper_token.resource_owner_id} does not exist" + render_json({ error: "unauthorized", message: "Access token is invalid - user not found" }, status: :unauthorized) + return false + end else Rails.logger.warn "API OAuth Token Invalid: Access token missing resource_owner_id" render_json({ error: "unauthorized", message: "Access token is invalid - missing resource owner" }, status: :unauthorized) return false end - @authentication_method = :oauth - setup_current_context_for_api - true + @authentication_method = :oauth + setup_current_context_for_api + true rescue Doorkeeper::Errors::DoorkeeperError => e Rails.logger.warn "API OAuth Error: #{e.message}" false @@ -228,7 +228,7 @@ class Api::V1::BaseController < ApplicationController Rails.logger.info "API Request: #{request.method} #{request.path} - User: #{current_resource_owner.email} (Family: #{current_resource_owner.family_id}) - Auth: #{auth_info}" end - # Family-based access control helper (to be used by subcontrollers) + # Family-based access control helper (to be used by subcontrollers) def ensure_current_family_access(resource) return unless resource.respond_to?(:family_id) @@ -241,12 +241,12 @@ class Api::V1::BaseController < ApplicationController true end - # Manual doorkeeper_token accessor for compatibility with manual token verification + # Manual doorkeeper_token accessor for compatibility with manual token verification def doorkeeper_token @_doorkeeper_token end - # Set up Current context for API requests since we don't use session-based auth + # Set up Current context for API requests since we don't use session-based auth def setup_current_context_for_api # For API requests, we need to create a minimal session-like object # or find/create an actual session for this user to make Current.user work diff --git a/app/controllers/api/v1/transactions_controller.rb b/app/controllers/api/v1/transactions_controller.rb index 2dedf281..b2e483bc 100644 --- a/app/controllers/api/v1/transactions_controller.rb +++ b/app/controllers/api/v1/transactions_controller.rb @@ -4,9 +4,9 @@ class Api::V1::TransactionsController < Api::V1::BaseController include Pagy::Backend # Ensure proper scope authorization for read vs write access - before_action :ensure_read_scope, only: [:index, :show] - before_action :ensure_write_scope, only: [:create, :update, :destroy] - before_action :set_transaction, only: [:show, :update, :destroy] + before_action :ensure_read_scope, only: [ :index, :show ] + before_action :ensure_write_scope, only: [ :create, :update, :destroy ] + before_action :set_transaction, only: [ :show, :update, :destroy ] def index family = current_resource_owner.family @@ -63,7 +63,7 @@ class Api::V1::TransactionsController < Api::V1::BaseController }, status: :internal_server_error end - def create + def create family = current_resource_owner.family # Validate account_id is present @@ -71,7 +71,7 @@ class Api::V1::TransactionsController < Api::V1::BaseController render json: { error: "validation_failed", message: "Account ID is required", - errors: ["Account ID is required"] + errors: [ "Account ID is required" ] }, status: :unprocessable_entity return end @@ -102,7 +102,7 @@ class Api::V1::TransactionsController < Api::V1::BaseController error: "internal_server_error", message: "Error: #{e.message}" }, status: :internal_server_error - end +end def update if @entry.update(entry_params_for_update) @@ -202,22 +202,22 @@ class Api::V1::TransactionsController < Api::V1::BaseController # Date range filtering if params[:start_date].present? - query = query.joins(:entry).where('entries.date >= ?', Date.parse(params[:start_date])) + query = query.joins(:entry).where("entries.date >= ?", Date.parse(params[:start_date])) end if params[:end_date].present? - query = query.joins(:entry).where('entries.date <= ?', Date.parse(params[:end_date])) + query = query.joins(:entry).where("entries.date <= ?", Date.parse(params[:end_date])) end # Amount filtering if params[:min_amount].present? min_amount = params[:min_amount].to_f - query = query.joins(:entry).where('entries.amount >= ?', min_amount) + query = query.joins(:entry).where("entries.amount >= ?", min_amount) end if params[:max_amount].present? max_amount = params[:max_amount].to_f - query = query.joins(:entry).where('entries.amount <= ?', max_amount) + query = query.joins(:entry).where("entries.amount <= ?", max_amount) end # Tag filtering @@ -229,26 +229,26 @@ class Api::V1::TransactionsController < Api::V1::BaseController # Transaction type filtering (income/expense) if params[:type].present? case params[:type].downcase - when 'income' - query = query.joins(:entry).where('entries.amount < 0') - when 'expense' - query = query.joins(:entry).where('entries.amount > 0') + when "income" + query = query.joins(:entry).where("entries.amount < 0") + when "expense" + query = query.joins(:entry).where("entries.amount > 0") end end query end - def apply_search(query) + def apply_search(query) search_term = "%#{params[:search]}%" query.joins(:entry) .left_joins(:merchant) .where( - 'entries.name ILIKE ? OR entries.notes ILIKE ? OR merchants.name ILIKE ?', + "entries.name ILIKE ? OR entries.notes ILIKE ? OR merchants.name ILIKE ?", search_term, search_term, search_term ) - end +end def transaction_params params.require(:transaction).permit( @@ -264,7 +264,7 @@ class Api::V1::TransactionsController < Api::V1::BaseController amount: calculate_signed_amount, currency: transaction_params[:currency] || current_resource_owner.family.currency, notes: transaction_params[:notes], - entryable_type: 'Transaction', + entryable_type: "Transaction", entryable_attributes: { category_id: transaction_params[:category_id], merchant_id: transaction_params[:merchant_id], @@ -301,9 +301,9 @@ class Api::V1::TransactionsController < Api::V1::BaseController nature = transaction_params[:nature] case nature&.downcase - when 'income', 'inflow' + when "income", "inflow" -amount.abs # Income is negative - when 'expense', 'outflow' + when "expense", "outflow" amount.abs # Expense is positive else amount # Use as provided @@ -324,4 +324,4 @@ class Api::V1::TransactionsController < Api::V1::BaseController 25 # Default end end -end \ No newline at end of file +end diff --git a/app/controllers/settings/api_keys_controller.rb b/app/controllers/settings/api_keys_controller.rb index ce7e951b..4804cdf5 100644 --- a/app/controllers/settings/api_keys_controller.rb +++ b/app/controllers/settings/api_keys_controller.rb @@ -54,7 +54,7 @@ class Settings::ApiKeysController < ApplicationController # Convert single scope value to array for storage permitted_params = params.require(:api_key).permit(:name, :scopes) if permitted_params[:scopes].present? - permitted_params[:scopes] = [permitted_params[:scopes]] + permitted_params[:scopes] = [ permitted_params[:scopes] ] end permitted_params end diff --git a/app/services/api_rate_limiter.rb b/app/services/api_rate_limiter.rb index ab4a5fad..9ceb9e79 100644 --- a/app/services/api_rate_limiter.rb +++ b/app/services/api_rate_limiter.rb @@ -59,7 +59,7 @@ class ApiRateLimiter { current_count: current_count, rate_limit: rate_limit, - remaining: [rate_limit - current_count, 0].max, + remaining: [ rate_limit - current_count, 0 ].max, reset_time: reset_time, tier: determine_tier } @@ -82,4 +82,4 @@ class ApiRateLimiter # or API key configuration DEFAULT_TIER end -end \ No newline at end of file +end diff --git a/test/controllers/api/v1/base_controller_test.rb b/test/controllers/api/v1/base_controller_test.rb index db777326..9479e70c 100644 --- a/test/controllers/api/v1/base_controller_test.rb +++ b/test/controllers/api/v1/base_controller_test.rb @@ -403,7 +403,7 @@ class Api::V1::BaseControllerTest < ActionDispatch::IntegrationTest other_api_key = ApiKey.create!( user: @user, name: "Other Test API Key", - scopes: ["read"], + scopes: [ "read" ], key: "other_test_key_for_rate_limiting" ) diff --git a/test/controllers/api/v1/transactions_controller_test.rb b/test/controllers/api/v1/transactions_controller_test.rb index 2d65340a..92e4f953 100644 --- a/test/controllers/api/v1/transactions_controller_test.rb +++ b/test/controllers/api/v1/transactions_controller_test.rb @@ -141,7 +141,7 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest } } - assert_difference('@account.entries.count', 1) do + assert_difference("@account.entries.count", 1) do post api_v1_transactions_url, params: transaction_params, headers: api_headers(@api_key) @@ -232,25 +232,25 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest end # DESTROY action tests - test "should destroy transaction" do - entry_to_delete = @account.entries.create!( - name: "Transaction to Delete", - amount: 10.00, - currency: "USD", - date: Date.current, - entryable: Transaction.new - ) - transaction_to_delete = entry_to_delete.transaction + test "should destroy transaction" do + entry_to_delete = @account.entries.create!( + name: "Transaction to Delete", + amount: 10.00, + currency: "USD", + date: Date.current, + entryable: Transaction.new + ) + transaction_to_delete = entry_to_delete.transaction - assert_difference('@account.entries.count', -1) do - delete api_v1_transaction_url(transaction_to_delete), headers: api_headers(@api_key) - end - - assert_response :success - response_data = JSON.parse(response.body) - assert response_data.key?("message") + assert_difference("@account.entries.count", -1) do + delete api_v1_transaction_url(transaction_to_delete), headers: api_headers(@api_key) end + assert_response :success + response_data = JSON.parse(response.body) + assert response_data.key?("message") +end + test "should reject destroy with read-only API key" do delete api_v1_transaction_url(@transaction), headers: api_headers(@read_only_api_key) assert_response :forbidden @@ -334,7 +334,7 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest private - def api_headers(api_key) - { "X-Api-Key" => api_key.plain_key } - end -end \ No newline at end of file + def api_headers(api_key) + { "X-Api-Key" => api_key.plain_key } + end +end diff --git a/test/controllers/api/v1/usage_controller_test.rb b/test/controllers/api/v1/usage_controller_test.rb index 70808f29..22ebe98e 100644 --- a/test/controllers/api/v1/usage_controller_test.rb +++ b/test/controllers/api/v1/usage_controller_test.rb @@ -6,7 +6,7 @@ class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest @api_key = ApiKey.create!( user: @user, name: "Test API Key", - scopes: ["read"], + scopes: [ "read" ], key: "test_key_for_usage" ) @@ -34,7 +34,7 @@ class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest # Check API key information assert_equal "Test API Key", response_body["api_key"]["name"] - assert_equal ["read"], response_body["api_key"]["scopes"] + assert_equal [ "read" ], response_body["api_key"]["scopes"] assert_not_nil response_body["api_key"]["last_used_at"] assert_not_nil response_body["api_key"]["created_at"] @@ -137,4 +137,4 @@ class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest # Mock fixture - in real tests this would reference actual fixtures OpenStruct.new(id: 1, email: "test@example.com", family_id: 1) end -end \ No newline at end of file +end diff --git a/test/services/api_rate_limiter_test.rb b/test/services/api_rate_limiter_test.rb index b4c8e751..3ea3e5b8 100644 --- a/test/services/api_rate_limiter_test.rb +++ b/test/services/api_rate_limiter_test.rb @@ -6,7 +6,7 @@ class ApiRateLimiterTest < ActiveSupport::TestCase @api_key = ApiKey.create!( user: @user, name: "Test API Key", - scopes: ["read"], + scopes: [ "read" ], key: "test_key_123" ) @rate_limiter = ApiRateLimiter.new(@api_key) @@ -99,7 +99,7 @@ class ApiRateLimiterTest < ActiveSupport::TestCase other_api_key = ApiKey.create!( user: @user, name: "Other API Key", - scopes: ["read_write"], + scopes: [ "read_write" ], key: "other_key_456" ) @@ -137,4 +137,4 @@ class ApiRateLimiterTest < ActiveSupport::TestCase # Mock fixture - in real tests this would reference actual fixtures OpenStruct.new(id: 1, email: "test@example.com", family_id: 1) end -end \ No newline at end of file +end