From ad5b0b8b7d1e6bf354be0111f992b6948cd80dac Mon Sep 17 00:00:00 2001
From: Julien Bertazzo Lambert <42924425+JLambertazzo@users.noreply.github.com>
Date: Thu, 30 Jan 2025 16:49:31 -0500
Subject: [PATCH] Ensure Consistent Category Colors (#1722)

* feat: add validation to require consistent category color

* feat: reflect color requirement in new category form

* refactor: move logic inline over shared component

* rubocop

* tests: fix breaking and add case for new validation

* feat: hide color selector when parent category selected

* feat: override color with parent color in model

* tests: remove case for unnecessary validation

---------

Signed-off-by: Julien Bertazzo Lambert <42924425+JLambertazzo@users.noreply.github.com>
---
 app/javascript/controllers/color_avatar_controller.js | 8 +++++++-
 app/models/category.rb                                | 8 ++++++++
 app/views/categories/_form.html.erb                   | 5 +++--
 test/models/category_test.rb                          | 2 +-
 4 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/app/javascript/controllers/color_avatar_controller.js b/app/javascript/controllers/color_avatar_controller.js
index 276ae023..41b7568b 100644
--- a/app/javascript/controllers/color_avatar_controller.js
+++ b/app/javascript/controllers/color_avatar_controller.js
@@ -3,7 +3,7 @@ import { Controller } from "@hotwired/stimulus";
 // Connects to data-controller="color-avatar"
 // Used by the transaction merchant form to show a preview of what the avatar will look like
 export default class extends Controller {
-  static targets = ["name", "avatar"];
+  static targets = ["name", "avatar", "selection"];
 
   connect() {
     this.nameTarget.addEventListener("input", this.handleNameChange);
@@ -25,4 +25,10 @@ export default class extends Controller {
     this.avatarTarget.style.borderColor = `color-mix(in srgb, ${color} 10%, white)`;
     this.avatarTarget.style.color = color;
   }
+
+  handleParentChange(e) {
+    const parent = e.currentTarget.value;
+    const visibility = typeof parent === "string" && parent !== "" ? "hidden" : "visible"
+    this.selectionTarget.style.visibility = visibility
+  }
 }
diff --git a/app/models/category.rb b/app/models/category.rb
index d0ccaa60..76a83866 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -14,6 +14,8 @@ class Category < ApplicationRecord
   validate :category_level_limit
   validate :nested_category_matches_parent_classification
 
+  before_create :inherit_color_from_parent
+
   scope :alphabetically, -> { order(:name) }
   scope :roots, -> { where(parent_id: nil) }
   scope :incomes, -> { where(classification: "income") }
@@ -85,6 +87,12 @@ class Category < ApplicationRecord
       end
   end
 
+  def inherit_color_from_parent
+    if subcategory?
+      self.color = parent.color
+    end
+  end
+
   def replace_and_destroy!(replacement)
     transaction do
       transactions.update_all category_id: replacement&.id
diff --git a/app/views/categories/_form.html.erb b/app/views/categories/_form.html.erb
index 884d5509..fd9a8513 100644
--- a/app/views/categories/_form.html.erb
+++ b/app/views/categories/_form.html.erb
@@ -7,7 +7,7 @@
         <%= render partial: "shared/color_avatar", locals: { name: category.name, color: category.color } %>
       </div>
 
-      <div class="flex gap-2 items-center justify-center">
+      <div class="flex gap-2 items-center justify-center" data-color-avatar-target="selection">
         <% Category::COLORS.each do |color| %>
           <label class="relative">
             <%= f.radio_button :color, color, class: "sr-only peer", data: { action: "change->color-avatar#handleColorChange" } %>
@@ -21,6 +21,7 @@
       <% end %>
 
       <div class="flex flex-wrap gap-2 justify-center mb-4">
+
         <% Category.icon_codes.each do |icon| %>
           <label class="relative">
             <%= f.radio_button :lucide_icon, icon, class: "sr-only peer" %>
@@ -35,7 +36,7 @@
         <%= f.select :classification, [["Income", "income"], ["Expense", "expense"]], { label: "Classification" }, required: true %>
         <%= f.text_field :name, placeholder: t(".placeholder"), required: true, autofocus: true, label: "Name", data: { color_avatar_target: "name" } %>
         <% unless category.parent? %>
-          <%= f.select :parent_id, categories.pluck(:name, :id), { include_blank: "(unassigned)", label: "Parent category (optional)" }, disabled: category.parent? %>
+          <%= f.select :parent_id, categories.pluck(:name, :id), { include_blank: "(unassigned)", label: "Parent category (optional)" }, disabled: category.parent?, data: { action: "change->color-avatar#handleParentChange" } %>
         <% end %>
       </div>
     </section>
diff --git a/test/models/category_test.rb b/test/models/category_test.rb
index 0f29843d..e6dccd84 100644
--- a/test/models/category_test.rb
+++ b/test/models/category_test.rb
@@ -25,7 +25,7 @@ class CategoryTest < ActiveSupport::TestCase
     category = categories(:subcategory)
 
     error = assert_raises(ActiveRecord::RecordInvalid) do
-      category.subcategories.create!(name: "Invalid category", color: "#000", family: @family)
+      category.subcategories.create!(name: "Invalid category", family: @family)
     end
 
     assert_equal "Validation failed: Parent can't have more than 2 levels of subcategories", error.message