fix: cascade parent category color changes to subcategories#2820
fix: cascade parent category color changes to subcategories#2820RealDiligent wants to merge 1 commit into
Conversation
Subcategories inherit their parent's color via a before_save hook, but that only fires when the subcategory itself is saved. Changing a parent category's color saved only the parent, leaving each subcategory's stored color at the old value — so the subcategory color visibly mismatched its parent and could not be corrected from the UI. Add an after_update hook on Category that, when a category's color changes, cascades the new color to its subcategories (skipping rows that already match). Leaf categories have no subcategories, so this is a no-op for them. Fixes we-promise#2816 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughCategory color updates now trigger synchronization of differing stored subcategory colors. Tests cover propagation to subcategories and successful updates for categories without subcategories. ChangesCategory color synchronization
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/models/category.rb`:
- Line 22: Update the instruction text in the category update behavior within
update_category so it states that parent color changes cascade to subcategories,
removing the outdated claim that they do not. Keep the surrounding assistant
behavior guidance unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3deed5ea-e4c0-42c0-a42d-5fd25fc3284e
📒 Files selected for processing (2)
app/models/category.rbtest/models/category_test.rb
| validate :category_level_limit | ||
|
|
||
| before_save :inherit_color_from_parent | ||
| after_update :sync_subcategory_colors, if: :saved_change_to_color? |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update the assistant behavior contract.
app/models/assistant/function/update_category.rb lines 8-14 still says parent color changes do not cascade, which is now false. Update that instruction so the assistant does not give callers incorrect behavior expectations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/models/category.rb` at line 22, Update the instruction text in the
category update behavior within update_category so it states that parent color
changes cascade to subcategories, removing the outdated claim that they do not.
Keep the surrounding assistant behavior guidance unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 954fc4aff1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # a parent's color must also cascade to already-saved subcategories, or their | ||
| # stored color goes stale and mismatches the parent (#2816). | ||
| def sync_subcategory_colors | ||
| subcategories.where.not(color: color).update_all(color: color) |
There was a problem hiding this comment.
Index parent_id before cascading colors
On installations with many families, every parent color edit now runs an UPDATE filtered by categories.parent_id, but the categories table has no index on parent_id (db/schema.rb only indexes family_id). PostgreSQL therefore scans the entire cross-tenant categories table while the parent transaction remains open, even when the category has no children. Add a parent_id index or scope the update by family_id so the existing index can bound the scan.
Useful? React with 👍 / 👎.
jjmata
left a comment
There was a problem hiding this comment.
Small, well-targeted fix. The root-cause analysis is correct: the existing before_save :inherit_color_from_parent only fires when the subcategory itself is saved, so a parent-only color change never touched the child row. The new after_update :sync_subcategory_colors, if: :saved_change_to_color? closes that gap with a single update_all, which correctly bypasses callbacks/validations and skips already-in-sync rows via where.not(color: color).
Since category_level_limit caps the hierarchy at parent → subcategory (no grandchildren), one level of cascade is sufficient here — worth confirming that assumption still holds if the depth limit ever changes, but not a concern today. Tests cover both the cascade and the no-subcategories no-op case. No issues found.
Generated by Claude Code
Fixes #2816
Problem
A subcategory's color does not follow its parent. After you change a parent category's color, the subcategory keeps its old color, so the two visibly mismatch and there's no way to correct the subcategory from the UI (subcategory colors are driven by the parent).
Root cause
Categoryalready forces a subcategory to adopt its parent's color, but only via abefore_savehook on the subcategory:Changing a parent's color saves only the parent record — the subcategories are never re-saved, so their stored
colorcolumn stays at the old value.Fix
Add an
after_updatehook so a parent's color change cascades to its subcategories:coloractually changed (saved_change_to_color?).where.not(color: color)skips rows already in sync, so it's a no-op when nothing changes.update_allis a single UPDATE — no per-row callbacks or N extra saves.Combined with the existing
before_save, subcategory colors now stay consistent with their parent in both directions.Tests
Added to
test/models/category_test.rb:Impact / risk
Category(+1 hook, +1 method, +2 tests).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests