diff --git a/app/assets/tailwind/sure-design-system/components.css b/app/assets/tailwind/sure-design-system/components.css index 63c1229fe..7277b0b92 100644 --- a/app/assets/tailwind/sure-design-system/components.css +++ b/app/assets/tailwind/sure-design-system/components.css @@ -136,6 +136,10 @@ .checkbox--light { &[type='checkbox'] { @apply border-alpha-black-200 checked:bg-gray-900 checked:ring-gray-900 focus:ring-gray-900 focus-visible:ring-gray-900 checked:hover:bg-gray-300 hover:bg-gray-300; + /* Tailwind Forms' default :indeterminate background is currentColor (plugin + default blue) — without this it would mismatch the gray-900 :checked color + above, e.g. on a cascading parent/subcategory checkbox tree. */ + @apply indeterminate:bg-gray-900 indeterminate:ring-gray-900 indeterminate:hover:bg-gray-300; } &[type='checkbox']:disabled { diff --git a/app/controllers/transactions/categorizes_controller.rb b/app/controllers/transactions/categorizes_controller.rb index b65ebea64..f6b3c4049 100644 --- a/app/controllers/transactions/categorizes_controller.rb +++ b/app/controllers/transactions/categorizes_controller.rb @@ -17,7 +17,7 @@ class Transactions::CategorizesController < ApplicationController end @group = groups.first - @categories = Current.family.categories.alphabetically + @categories = Current.family.categories.includes(:parent).alphabetically @total_uncategorized = uncategorized_count end @@ -49,7 +49,7 @@ class Transactions::CategorizesController < ApplicationController if remaining_ids.empty? render turbo_stream: turbo_stream.action(:redirect, transactions_categorize_path(position: @position)) else - @categories = Current.family.categories.alphabetically + @categories = Current.family.categories.includes(:parent).alphabetically streams = entry_ids.map { |id| turbo_stream.remove("categorize_entry_#{id}") } remaining_entries.each do |entry| streams << turbo_stream.replace( @@ -76,7 +76,7 @@ class Transactions::CategorizesController < ApplicationController filter = params[:filter].to_s.strip transaction_type = params[:transaction_type].presence entries = filter.present? ? Entry.uncategorized_matching(Current.accessible_entries, filter, transaction_type) : [] - @categories = Current.family.categories.alphabetically + @categories = Current.family.categories.includes(:parent).alphabetically render turbo_stream: [ turbo_stream.replace("categorize_group_title", diff --git a/app/javascript/controllers/category_filter_controller.js b/app/javascript/controllers/category_filter_controller.js new file mode 100644 index 000000000..e6841f50a --- /dev/null +++ b/app/javascript/controllers/category_filter_controller.js @@ -0,0 +1,73 @@ +import { Controller } from "@hotwired/stimulus"; + +// Cascading parent/subcategory checkboxes for the transaction category filter. +// +// A parent checkbox is only ever `checked` (and therefore only ever submitted +// with the form) when *all* of its children are checked. The backend query +// (Transaction::Search#apply_category_filter) includes every subcategory +// whenever a parent category name is present in the submitted params, with +// no way to exclude an individual child. So the moment a user unchecks one +// child, the parent must be unchecked too — otherwise the parent would still +// be submitted and the backend would silently keep including the +// deselected child's transactions, ignoring what the user just did. +export default class extends Controller { + static targets = ["checkbox"]; + + connect() { + // Server-rendered `checked` state is derived independently per checkbox + // from the submitted query params, so a parent-only filter (e.g. an + // incoming link that only names the parent category) renders the parent + // checked with its children unchecked. Cascade checked parents down to + // their children first so the pass below doesn't read that as "some + // children unchecked" and clear the parent. + this.checkboxTargets.forEach((checkbox) => { + if (checkbox.checked) { + this.#childCheckboxesFor(checkbox.dataset.categoryId).forEach((child) => { + child.checked = true; + }); + } + }); + + this.checkboxTargets.forEach((checkbox) => { + if (checkbox.dataset.parentId) { + this.#syncParentState(checkbox.dataset.parentId); + } + }); + } + + toggle(event) { + const checkbox = event.target; + const categoryId = checkbox.dataset.categoryId; + + const children = this.#childCheckboxesFor(categoryId); + children.forEach((child) => { + child.checked = checkbox.checked; + child.indeterminate = false; + }); + + const parentId = checkbox.dataset.parentId; + if (parentId) { + this.#syncParentState(parentId); + } + } + + #childCheckboxesFor(parentId) { + if (!parentId) return []; + return this.checkboxTargets.filter((cb) => cb.dataset.parentId === parentId); + } + + #syncParentState(parentId) { + const parentCheckbox = this.checkboxTargets.find( + (cb) => cb.dataset.categoryId === parentId, + ); + if (!parentCheckbox) return; + + const children = this.#childCheckboxesFor(parentId); + if (children.length === 0) return; + + const checkedCount = children.filter((cb) => cb.checked).length; + + parentCheckbox.checked = checkedCount === children.length; + parentCheckbox.indeterminate = checkedCount > 0 && checkedCount < children.length; + } +} diff --git a/app/views/transactions/categorizes/show.html.erb b/app/views/transactions/categorizes/show.html.erb index f7ec1093c..5dba217e3 100644 --- a/app/views/transactions/categorizes/show.html.erb +++ b/app/views/transactions/categorizes/show.html.erb @@ -120,20 +120,23 @@ - <% @categories.each do |category| %> - + <% Category::Group.for(@categories).each do |group| %> + <% [ group.category, *group.subcategories ].each do |category| %> + <% category_label = category.parent_id.present? ? category.display_name_with_parent : category.display_name %> + + <% end %> <% end %> diff --git a/app/views/transactions/searches/filters/_category_filter.html.erb b/app/views/transactions/searches/filters/_category_filter.html.erb index 877cde855..44284d518 100644 --- a/app/views/transactions/searches/filters/_category_filter.html.erb +++ b/app/views/transactions/searches/filters/_category_filter.html.erb @@ -1,5 +1,5 @@ <%# locals: (form:) %> -
+
<%= render DS::SearchInput.new( variant: :embedded, placeholder: t(".filter_category"), @@ -17,7 +17,13 @@ { multiple: true, checked: @q[:categories]&.include?(category.name), - class: "checkbox checkbox--light" + class: "checkbox checkbox--light", + data: { + category_filter_target: "checkbox", + action: "change->category-filter#toggle", + category_id: category.id, + parent_id: category.parent_id + } }, category.name, nil %> diff --git a/test/controllers/transactions/categorizes_controller_test.rb b/test/controllers/transactions/categorizes_controller_test.rb index a1e96b250..1bd141f77 100644 --- a/test/controllers/transactions/categorizes_controller_test.rb +++ b/test/controllers/transactions/categorizes_controller_test.rb @@ -43,6 +43,26 @@ class Transactions::CategorizesControllerTest < ActionDispatch::IntegrationTest assert_equal parent_index + 1, child_index end + test "show groups subcategories immediately after their parent in the category pills" do + create_transaction(account: @account, name: "Starbucks") + get transactions_categorize_url + + assert_response :success + + doc = Nokogiri::HTML::Document.parse(response.body) + pill_values = doc.css("button[name='category_id']").map { |node| node["value"] } + + parent_index = pill_values.index(categories(:food_and_drink).id) + child_index = pill_values.index(categories(:subcategory).id) + + assert_not_nil parent_index + assert_not_nil child_index + assert_equal parent_index + 1, child_index + + child_pill = doc.css("button[name='category_id'][value='#{categories(:subcategory).id}']").first + assert_includes child_pill.text, categories(:subcategory).display_name_with_parent + end + test "show renders full dates so multi-year lists are unambiguous" do create_transaction(account: @account, name: "Starbucks", date: Date.new(2024, 7, 8)) diff --git a/test/system/category_filter_cascading_test.rb b/test/system/category_filter_cascading_test.rb new file mode 100644 index 000000000..1587b49dc --- /dev/null +++ b/test/system/category_filter_cascading_test.rb @@ -0,0 +1,101 @@ +require "application_system_test_case" + +class CategoryFilterCascadingTest < ApplicationSystemTestCase + setup do + sign_in @user = users(:family_admin) + + Entry.delete_all # clean slate + + @parent = categories(:food_and_drink) + @child_one = categories(:subcategory) # "Restaurants" + @child_two = @user.family.categories.create!(name: "Groceries", parent: @parent, color: "#4da568", lucide_icon: "shopping-bag") + + @child_one_transaction = create_transaction("restaurant purchase", 2.days.ago.to_date, 50, category: @child_one) + @child_two_transaction = create_transaction("grocery run", 1.day.ago.to_date, 30, category: @child_two) + + visit transactions_url + end + + test "checking a parent category checks all its subcategories" do + find("#transaction-filters-button").click + + within "#transaction-filters-menu" do + click_button "Category" + check(@parent.name) + + assert find_field(@child_one.name).checked? + assert find_field(@child_two.name).checked? + end + end + + test "unchecking one subcategory unchecks the parent and excludes only that subcategory from the filter" do + find("#transaction-filters-button").click + + within "#transaction-filters-menu" do + click_button "Category" + check(@parent.name) + uncheck(@child_one.name) + + assert_not find_field(@parent.name).checked? + assert find_field(@child_two.name).checked? + + click_button "Apply" + end + + # Only the still-checked subcategory's transaction should show — the + # deselected child must not sneak back in via the parent match. + assert_selector "#" + dom_id(@child_two_transaction) + assert_no_selector "#" + dom_id(@child_one_transaction) + end + + test "re-checking all subcategories re-checks the parent" do + find("#transaction-filters-button").click + + within "#transaction-filters-menu" do + click_button "Category" + check(@parent.name) + uncheck(@child_one.name) + check(@child_one.name) + + assert find_field(@parent.name).checked? + + click_button "Apply" + end + + assert_selector "#" + dom_id(@child_one_transaction) + assert_selector "#" + dom_id(@child_two_transaction) + end + + test "reopening a parent-only filter from the URL keeps it checked and active on Apply" do + visit transactions_url(q: { categories: [ @parent.name ] }) + + assert_selector "#" + dom_id(@child_one_transaction) + assert_selector "#" + dom_id(@child_two_transaction) + + find("#transaction-filters-button").click + + within "#transaction-filters-menu" do + click_button "Category" + + assert find_field(@parent.name).checked? + assert find_field(@child_one.name).checked? + assert find_field(@child_two.name).checked? + + click_button "Apply" + end + + assert_selector "#" + dom_id(@child_one_transaction) + assert_selector "#" + dom_id(@child_two_transaction) + end + + private + + def create_transaction(name, date, amount, category:) + accounts(:depository).entries.create! \ + name: name, + date: date, + amount: amount, + currency: "USD", + entryable: Transaction.new(category: category) + end +end