Skip to content

Conversation

@kon3ko
Copy link

@kon3ko kon3ko commented Dec 1, 2025

This pull request adds support for a new optional details field to categories, allowing for richer descriptions and improved display in forms and badges.

This addition is particularly valuable for international users who may not be fluent in English. By providing a dedicated space for detailed explanations or translations, we ensure that users in non-English speaking regions can clearly understand the category context and consistently make the correct selection without confusion.

It also cleans up a duplicate migration for import_rows and introduces minor UI and validation improvements.

Category Details Feature

  • Added a details text column to the categories table via migration, with a maximum length of 1000 characters and validation in the Category model. Blank details are stored as nil. (db/migrate/20251201160501_add_details_to_categories.rb, app/models/category.rb) [1] [2]
  • Updated strong parameters in CategoriesController to permit the new details field. (app/controllers/categories_controller.rb)
  • Added a text area for entering category details in the category form. (app/views/categories/_form.html.erb)

UI Improvements for Category Details

  • Enhanced the category badge partial to optionally show details and improved layout/styling. Details are shown in the badge and as a tooltip if present. (app/views/categories/_badge.html.erb)
  • Updated the category list and transaction form to display details alongside category names, with improved sorting and formatting for parent and child categories. (app/views/categories/_category.html.erb, app/views/transactions/_form.html.erb) [1] [2]

Sorting and Presentation

  • Changed sorting logic for subcategories so they are always listed alphabetically under their parent. (app/models/category.rb)

Database Migration Cleanup

  • Removed duplicate creation of the import_rows table from a later migration, ensuring only the correct migration is present. (db/migrate/20240630000000_create_import_rows.rb, db/migrate/20240925112218_add_import_types.rb) [1] [2]
image image image image

Summary by CodeRabbit

Release Notes

  • New Features
    • Categories now support optional details (up to 1,000 characters).
    • Category details display in relevant UI locations with intelligent truncation.
    • Transaction form category selection now shows hierarchical, alphabetically sorted display with optional category details for improved organization and readability.

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

@kon3ko kon3ko changed the title Feature/add category details Add 'details' field to Categories for improved localization and context Dec 1, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

Add a details field to categories with maximum length validation, display logic in views, and form input capability. Changes include database migration, model validation and callback, controller parameter whitelist update, and view enhancements to display and edit category details.

Changes

Cohort / File(s) Summary
Database Schema
db/migrate/20240630000000_create_import_rows.rb, db/migrate/20240925112218_add_import_types.rb, db/migrate/20251201160501_add_details_to_categories.rb
New migration for import_rows table created, then removed in subsequent migration. New migration adds details text column to categories table.
Model Layer
app/models/category.rb
Added validation constraining details to max 1000 characters with blanks allowed. Added before_save callback to coerce blank details to nil. Subcategories in nested Group class now sorted by name during initialization.
Controller
app/controllers/categories_controller.rb
Expanded category_params whitelist to permit :details alongside existing attributes.
View Layer — Category Badge
app/views/categories/_badge.html.erb
Added show_details local variable support (defaults to true). Updated layout with vertical flex container. Added title attribute and conditional rendering of details line with styling based on category color.
View Layer — Category Display & Form
app/views/categories/_category.html.erb, app/views/categories/_form.html.erb
Updated category container styling (flex-1, min-w-0, added padding). Passed show_details: true to badge partial. Added details text_area field to form with max length 1000 and "Details (optional)" label.
View Layer — Transactions
app/views/transactions/_form.html.erb
Changed category collection rendering from flat list to hierarchical, sorted display. Parent categories and child categories now formatted with optional details (truncated to 60 characters) and visual indentation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify migration creates details column with correct type and null constraints
  • Confirm show_details local variable behavior and defaults don't break existing badge usages
  • Check hierarchical category display in transactions form handles edge cases (missing details, long names, sorting)
  • Validate before_save callback interaction with validation and form submissions
  • Review styling and truncation logic in view partials for consistency

Suggested labels

enhancement

Suggested reviewers

  • sokie
  • jjmata

Poem

🐰 A details field hops into view,
Categories bloom with more to do,
Sorted subcategories dance with care,
Truncated wisely, details to share!
✨ Hop along, the form grows new~

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a 'details' field to Categories, which is reflected across the database migration, model validation, controller parameters, views, and UI components throughout the changeset.
✨ 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 (1)
app/views/transactions/_form.html.erb (1)

27-37: Consider extracting complex view logic to a helper or model method.

The hierarchical sorting and display logic is complex for a view template. This would be more testable and maintainable if extracted to a helper method or as a model class method.

For example, create a helper in app/helpers/categories_helper.rb:

def hierarchical_category_options(categories)
  sorted_categories = categories.select { |cat| cat.parent_id.nil? }
                                .sort_by(&:name)
                                .flat_map { |parent| [parent] + parent.subcategories.sort_by(&:name) }
  
  sorted_categories.map do |cat|
    display_name = if cat.parent_id.nil?
      format_category_with_details(cat)
    else
      "  • #{format_category_with_details(cat)}"
    end
    [display_name, cat.id]
  end
end

def format_category_with_details(category)
  category.details.present? ? "#{category.name} - #{category.details.truncate(60)}" : category.name
end

Then in the view:

<%= ef.select :category_id, hierarchical_category_options(categories), 
    { prompt: t(".category_prompt"), label: t(".category") } %>

This keeps domain logic out of view templates and makes the code easier to test and maintain.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54d041c and 34c6954.

📒 Files selected for processing (9)
  • app/controllers/categories_controller.rb (1 hunks)
  • app/models/category.rb (2 hunks)
  • app/views/categories/_badge.html.erb (1 hunks)
  • app/views/categories/_category.html.erb (1 hunks)
  • app/views/categories/_form.html.erb (1 hunks)
  • app/views/transactions/_form.html.erb (1 hunks)
  • db/migrate/20240630000000_create_import_rows.rb (1 hunks)
  • db/migrate/20240925112218_add_import_types.rb (0 hunks)
  • db/migrate/20251201160501_add_details_to_categories.rb (1 hunks)
💤 Files with no reviewable changes (1)
  • db/migrate/20240925112218_add_import_types.rb
🧰 Additional context used
📓 Path-based instructions (26)
**/*.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:

  • db/migrate/20240630000000_create_import_rows.rb
  • db/migrate/20251201160501_add_details_to_categories.rb
  • app/controllers/categories_controller.rb
  • app/models/category.rb
db/migrate/**/*.rb

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

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

Files:

  • db/migrate/20240630000000_create_import_rows.rb
  • db/migrate/20251201160501_add_details_to_categories.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:

  • db/migrate/20240630000000_create_import_rows.rb
  • app/views/categories/_form.html.erb
  • db/migrate/20251201160501_add_details_to_categories.rb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/controllers/categories_controller.rb
  • app/models/category.rb
  • app/views/categories/_category.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:

  • db/migrate/20240630000000_create_import_rows.rb
  • app/views/categories/_form.html.erb
  • db/migrate/20251201160501_add_details_to_categories.rb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/controllers/categories_controller.rb
  • app/models/category.rb
  • app/views/categories/_category.html.erb
{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/20240630000000_create_import_rows.rb
  • db/migrate/20251201160501_add_details_to_categories.rb
  • app/models/category.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:

  • db/migrate/20240630000000_create_import_rows.rb
  • app/views/categories/_form.html.erb
  • db/migrate/20251201160501_add_details_to_categories.rb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/controllers/categories_controller.rb
  • app/models/category.rb
  • app/views/categories/_category.html.erb
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/20240630000000_create_import_rows.rb
  • db/migrate/20251201160501_add_details_to_categories.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:

  • db/migrate/20240630000000_create_import_rows.rb
  • app/views/categories/_form.html.erb
  • db/migrate/20251201160501_add_details_to_categories.rb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/controllers/categories_controller.rb
  • app/models/category.rb
  • app/views/categories/_category.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:

  • db/migrate/20240630000000_create_import_rows.rb
  • app/views/categories/_form.html.erb
  • db/migrate/20251201160501_add_details_to_categories.rb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/controllers/categories_controller.rb
  • app/models/category.rb
  • app/views/categories/_category.html.erb
db/**/*.rb

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • db/migrate/20240630000000_create_import_rows.rb
  • db/migrate/20251201160501_add_details_to_categories.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.html.erb
{app/views/**,app/helpers/**}

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

Always generate semantic HTML

Files:

  • app/views/categories/_form.html.erb
  • app/views/transactions/_form.html.erb
  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.html.erb
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/categories_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/categories_controller.rb
  • app/models/category.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/categories_controller.rb
  • app/models/category.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/category.rb
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:57:34.452Z
Learning: Pull requests should have clear description, linked issues, screenshots for UI changes, migration notes if applicable, and ensure CI passes with tests added/updated and `rubocop`/Biome clean
📚 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/20240630000000_create_import_rows.rb
  • db/migrate/20251201160501_add_details_to_categories.rb
📚 Learning: 2025-11-14T14:22:37.076Z
Learnt from: jjmata
Repo: we-promise/sure PR: 326
File: app/models/category_import.rb:10-10
Timestamp: 2025-11-14T14:22:37.076Z
Learning: In the CategoryImport model (app/models/category_import.rb), the import process implements a "last write wins" strategy where importing a category.csv file completely overrides existing category structures, including parent relationships. Setting category.parent = nil in the first pass is intentional behavior to ensure the CSV becomes the source of truth.

Applied to files:

  • db/migrate/20240630000000_create_import_rows.rb
  • app/views/transactions/_form.html.erb
  • app/controllers/categories_controller.rb
  • app/models/category.rb
  • app/views/categories/_category.html.erb
📚 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/migrate/20240630000000_create_import_rows.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: CSV import is managed via `Import` model with support for transaction and balance imports, custom field mapping, and transformation rules.

Applied to files:

  • db/migrate/20240630000000_create_import_rows.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 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:

  • app/views/transactions/_form.html.erb
📚 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 **/*.{rb,js,erb} : Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only

Applied to files:

  • app/views/transactions/_form.html.erb
📚 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 : Prefer native HTML elements over JavaScript-based components. Use semantic HTML: <dialog> for modals, <details><summary> for disclosures

Applied to files:

  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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 app/views/**/*.erb : Prefer semantic HTML elements (dialog, details, summary) over JavaScript components

Applied to files:

  • app/views/categories/_badge.html.erb
  • app/views/categories/_category.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/categories_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/categories_controller.rb
🔇 Additional comments (8)
db/migrate/20240630000000_create_import_rows.rb (1)

1-20: LGTM!

The migration follows Rails 7.2 conventions correctly with UUID primary key and proper foreign key reference.

db/migrate/20251201160501_add_details_to_categories.rb (1)

1-5: LGTM!

The migration correctly adds a text column for details without a null constraint, which aligns with the model validation that allows blank values.

app/controllers/categories_controller.rb (1)

90-90: LGTM!

The :details parameter is correctly added to the strong parameters whitelist.

app/models/category.rb (3)

14-14: LGTM!

The validation correctly enforces the 1000 character maximum and allows blank values, which aligns with the optional nature of the field.


20-20: LGTM!

Normalizing blank details to nil is good practice for consistent data storage and prevents empty strings in the database.


47-47: LGTM!

Sorting subcategories by name improves consistency in the UI display and aligns with the hierarchical category rendering in forms.

app/views/categories/_badge.html.erb (1)

5-21: LGTM!

The layout changes and conditional details rendering work well. The title attribute provides a tooltip for the full details, and the conditional display logic is clean.

app/views/categories/_category.html.erb (1)

4-4: LGTM!

The layout improvements (flex-1, min-w-0, shrink-0) prevent overflow issues and improve the responsive behavior. Passing show_details: true appropriately enables details display in category lists.

Also applies to: 11-11, 14-14

Comment on lines +1 to +3
<%# locals: (category:, show_details: false) %>
<% category ||= Category.uncategorized %>
<% show_details = true if show_details.nil? %>
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

Clarify the default value for show_details.

There's an inconsistency between the locals declaration (which suggests false as the default) and line 3 (which sets it to true when nil). This can cause confusion.

Choose one approach:

Option 1: Make the default truly true:

-<%# locals: (category:, show_details: false) %>
+<%# locals: (category:, show_details: true) %>
 <% category ||= Category.uncategorized %>
-<% show_details = true if show_details.nil? %>

Option 2: Make the default truly false:

 <%# locals: (category:, show_details: false) %>
 <% category ||= Category.uncategorized %>
-<% show_details = true if show_details.nil? %>
+<% show_details ||= false %>

Based on the usage in _category.html.erb where show_details: true is explicitly passed, Option 1 seems more aligned with the intended behavior.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<%# locals: (category:, show_details: false) %>
<% category ||= Category.uncategorized %>
<% show_details = true if show_details.nil? %>
<%# locals: (category:, show_details: true) %>
<% category ||= Category.uncategorized %>
🤖 Prompt for AI Agents
In app/views/categories/_badge.html.erb around lines 1 to 3, the locals comment
declares show_details default as false but the code sets show_details to true
when nil, causing inconsistency; choose one approach and make it consistent:
either change the locals line to indicate show_details: true (Option 1) or
change the nil-coercion to set show_details = false (Option 2). Since
_category.html.erb passes show_details: true, update the locals declaration to
show_details: true and keep the current nil handling, or if you prefer
false-by-default change the nil-handling line to set false instead; update only
the locals comment or the nil assignment so both match.

<% unless category.parent? %>
<%= f.select :parent_id, categories.pluck(:name, :id), { include_blank: "(unassigned)", label: "Parent category (optional)" }, disabled: category.parent?, data: { action: "change->category#handleParentChange" } %>
<% end %>
<%= f.text_area :details, label: "Details (optional)", placeholder: "Details...", rows: 3, maxlength: 1000 %>
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

Use i18n for user-facing strings.

The label and placeholder text should use the t() helper for localization.

Apply this diff:

-        <%= f.text_area :details, label: "Details (optional)", placeholder: "Details...", rows: 3, maxlength: 1000 %>
+        <%= f.text_area :details, label: t(".details_label"), placeholder: t(".details_placeholder"), rows: 3, maxlength: 1000 %>

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

en:
  categories:
    form:
      details_label: "Details (optional)"
      details_placeholder: "Details..."

As per coding guidelines: "All user-facing strings must use localization (i18n) via the t() helper."

🤖 Prompt for AI Agents
In app/views/categories/_form.html.erb around line 70, replace the hard-coded
label and placeholder strings with i18n lookups (e.g., use
t('categories.form.details_label') for the label and
t('categories.form.details_placeholder') for the placeholder) and update the
view accordingly; then add the provided keys under config/locales (for example
in config/locales/en.yml) with the entries categories.form.details_label:
"Details (optional)" and categories.form.details_placeholder: "Details..." so
all user-facing copy uses the t() helper.

@jjmata jjmata marked this pull request as draft December 1, 2025 19:27
@kon3ko
Copy link
Author

kon3ko commented Dec 1, 2025

@jjmata #403 (comment)

Thanks for the feedback! I'd like to share a few points to clarify:

  1. It actually fixes the layout: The previous layout had an issue where long category names would push the edit/delete buttons off-screen. I fixed this by allowing the text to wrap properly, so the menu is always accessible now.
  2. Limited Scope: This description text only appears on the Category Settings page. In other places (like the Transaction list), it's hidden and only shows as a tooltip on hover, so it won't break mobile or table layouts.
  3. Why I need this: As a non-native English speaker (Thai), I spend a lot of time hesitating over English category names. Being able to add descriptions in my own language helps me categorize transactions much faster.
  4. Totally Optional: If you are happy with the default English categories and leave this field blank, the UI looks exactly the same as before. Nothing changes for you.
image

@kon3ko
Copy link
Author

kon3ko commented Dec 1, 2025

@jjmata

I'd like to add more context on why this is necessary:

To accurately describe categories in my native language, the names often need to be much longer than the concise English defaults. While English names are short, they aren't intuitive for me, and I often forget the specific context or meaning of each one.

If I were to put the full Thai description into the "Name" field, it would cause significant layout overflow (as shown in the example). Also, when adding a new transaction, having to scroll through and interpret a list of English terms makes it very confusing and slow to find the right one. The details field solves this by keeping the name short while preserving the context I need.

image image image

@jjmata
Copy link
Collaborator

jjmata commented Dec 2, 2025

Like I said in the first PR submitted, this is an interesting idea, but one that seemed to break layout in places for very little advantage. I know understand better your goal, and if we can keep this making no impact to UI/UX except for those that decide to populate the detailed description ... we could consider it!

If you want to add this, @kon3ko ... please provide descriptions for each current default category (see #341 for what is coming) and leave the display out of all the places that break layout - users can go to category details if they want to see the description.

Thank you!

@jjmata
Copy link
Collaborator

jjmata commented Dec 2, 2025

Also, please fix tests! ;-)

@kon3ko
Copy link
Author

kon3ko commented Dec 9, 2025

@jjmata What are your thoughts on implementing a custom Select component instead of using the native one? My goal is to create a design that is simpler and more visually appealing. I intend for this to supersede the draft in #341

@jjmata
Copy link
Collaborator

jjmata commented Dec 10, 2025

@jjmata What are your thoughts on implementing a custom Select component instead of using the native one? My goal is to create a design that is simpler and more visually appealing. I intend for this to supersede the draft in #341

Not against it, but seems like a lot of work ... what do you have in mind? Would it "travel well" over to mobile/PWA? What is the main problem you would be solving?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants