From 0b7fa732ae5969746f14f3dbf8d439c42ae701cf Mon Sep 17 00:00:00 2001 From: CrossDrain <32982516+CrossDrain@users.noreply.github.com> Date: Sat, 9 May 2026 10:36:41 +0000 Subject: [PATCH] feat(splits): add exclusion support for splits and improve rendering (#1661) * feat(splits): add excluded attribute support for split children and improve rendering of split transactions * address coderabbitai suggestions to improve code quality * Fix split excluded coercion, DRY helpers, and clean up view partials Fix boolean coercion bug where string "false" from form params was truthy in Ruby, causing all split children to be marked excluded. Use ActiveModel::Type::Boolean for explicit casting in Entry#split!. Additional changes addressing code review feedback: - Extract duplicated in_split_group logic from TransactionsController and TransactionCategoriesController into TransactionsHelper - Remove redundant local_assigns.fetch calls in partials that already declare defaults via the Rails 7.1 locals: magic comment - Simplify ternary in _transaction.html.erb to pass grouped directly - Guard hidden_field_tag :grouped to only emit when value is "true" - Add model tests for excluded on split children (boolean and string) - Add controller test for excluded param through full HTTP stack - Add test confirming excluded children are dropped from balance queries * fix(splits): simplify excluded attribute boolean check * refactor(splits): extract truthy values constant for excluded check Extract the array of truthy values used for excluded attribute check into a private constant to improve code maintainability and avoid duplication of the magic array. * refactor: simplify split grouping link generation and add test coverage for excluded split parameters --- app/controllers/splits_controller.rb | 6 +-- .../transaction_categories_controller.rb | 5 +- app/controllers/transactions_controller.rb | 7 ++- app/helpers/transactions_helper.rb | 4 ++ app/models/entry.rb | 6 ++- app/views/categories/_menu.html.erb | 4 +- app/views/category/dropdowns/_row.html.erb | 1 + app/views/category/dropdowns/show.html.erb | 3 +- app/views/entries/_entry.html.erb | 4 +- app/views/transactions/_notes.html.erb | 1 + app/views/transactions/_transaction.html.erb | 6 +-- .../_transaction_category.html.erb | 4 +- app/views/transactions/show.html.erb | 7 ++- test/controllers/splits_controller_test.rb | 41 ++++++++++++++++ test/models/entry_split_test.rb | 47 +++++++++++++++++++ 15 files changed, 128 insertions(+), 18 deletions(-) diff --git a/app/controllers/splits_controller.rb b/app/controllers/splits_controller.rb index a62540e9d..e31b64596 100644 --- a/app/controllers/splits_controller.rb +++ b/app/controllers/splits_controller.rb @@ -16,7 +16,7 @@ class SplitsController < ApplicationController raw_splits = raw_splits.values if raw_splits.respond_to?(:values) splits = raw_splits.map do |s| - { name: s[:name], amount: s[:amount].to_d * -1, category_id: s[:category_id].presence } + { name: s[:name], amount: s[:amount].to_d * -1, category_id: s[:category_id].presence, excluded: s[:excluded] } end @entry.split!(splits) @@ -51,7 +51,7 @@ class SplitsController < ApplicationController raw_splits = raw_splits.values if raw_splits.respond_to?(:values) splits = raw_splits.map do |s| - { name: s[:name], amount: s[:amount].to_d * -1, category_id: s[:category_id].presence } + { name: s[:name], amount: s[:amount].to_d * -1, category_id: s[:category_id].presence, excluded: s[:excluded] } end Entry.transaction do @@ -95,6 +95,6 @@ class SplitsController < ApplicationController end def split_params - params.require(:split).permit(splits: [ :name, :amount, :category_id ]) + params.require(:split).permit(splits: [ :name, :amount, :category_id, :excluded ]) end end diff --git a/app/controllers/transaction_categories_controller.rb b/app/controllers/transaction_categories_controller.rb index 6a8ff353a..d9daf436a 100644 --- a/app/controllers/transaction_categories_controller.rb +++ b/app/controllers/transaction_categories_controller.rb @@ -21,6 +21,7 @@ class TransactionCategoriesController < ApplicationController transaction.lock_saved_attributes! @entry.lock_saved_attributes! + in_split_group = helpers.in_split_group?(@entry, params[:grouped]) respond_to do |format| format.html { redirect_back_or_to transaction_path(@entry) } format.turbo_stream do @@ -28,12 +29,12 @@ class TransactionCategoriesController < ApplicationController turbo_stream.replace( dom_id(transaction, "category_menu_mobile"), partial: "transactions/transaction_category", - locals: { transaction: transaction, variant: "mobile" } + locals: { transaction: transaction, variant: "mobile", in_split_group: in_split_group } ), turbo_stream.replace( dom_id(transaction, "category_menu_desktop"), partial: "transactions/transaction_category", - locals: { transaction: transaction, variant: "desktop" } + locals: { transaction: transaction, variant: "desktop", in_split_group: in_split_group } ), turbo_stream.replace( "category_name_mobile_#{transaction.id}", diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 944744d06..d88ee1bcf 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -142,6 +142,7 @@ class TransactionsController < ApplicationController respond_to do |format| format.html { redirect_back_or_to account_path(@entry.account), notice: "Transaction updated" } format.turbo_stream do + in_split_group = helpers.in_split_group?(@entry, params[:grouped]) render turbo_stream: [ turbo_stream.replace( dom_id(@entry, :header), @@ -158,7 +159,11 @@ class TransactionsController < ApplicationController partial: "transactions/notes", locals: { entry: @entry, can_annotate: can_annotate_entry? } ) if params[:entry]&.key?(:notes) && notes_changed), - turbo_stream.replace(@entry), + turbo_stream.replace( + dom_id(@entry), + partial: "entries/entry", + locals: { entry: @entry, in_split_group: in_split_group } + ), *flash_notification_stream_items ].compact end diff --git a/app/helpers/transactions_helper.rb b/app/helpers/transactions_helper.rb index 92ce26c81..b9ea06d35 100644 --- a/app/helpers/transactions_helper.rb +++ b/app/helpers/transactions_helper.rb @@ -20,6 +20,10 @@ module TransactionsHelper transaction_search_filters[0] end + def in_split_group?(entry, params_grouped) + entry.split_child? && Current.user.show_split_grouped? && params_grouped == "true" + end + # ---- Transaction extra details helpers ---- # Returns a structured hash describing extra details for a transaction. # Input can be a Transaction or an Entry (responds_to :transaction). diff --git a/app/models/entry.rb b/app/models/entry.rb index aaf079059..5dbcf2090 100644 --- a/app/models/entry.rb +++ b/app/models/entry.rb @@ -1,6 +1,9 @@ class Entry < ApplicationRecord include Monetizable, Enrichable + TRUTHY_VALUES = [ true, "true", "1", 1 ].freeze + private_constant :TRUTHY_VALUES + attr_accessor :unsplitting monetize :amount @@ -361,7 +364,7 @@ class Entry < ApplicationRecord # Splits this entry into child entries. Marks parent as excluded. # - # @param splits [Array] array of { name:, amount:, category_id: } hashes + # @param splits [Array] array of { name:, amount:, category_id:, excluded: } hashes # @return [Array] the created child entries def split!(splits) total = splits.sum { |s| s[:amount].to_d } @@ -383,6 +386,7 @@ class Entry < ApplicationRecord name: split_attrs[:name], amount: split_attrs[:amount], currency: currency, + excluded: TRUTHY_VALUES.include?(split_attrs[:excluded]), entryable: child_transaction ) end diff --git a/app/views/categories/_menu.html.erb b/app/views/categories/_menu.html.erb index 6af5a8ce8..e3f8598b6 100644 --- a/app/views/categories/_menu.html.erb +++ b/app/views/categories/_menu.html.erb @@ -1,4 +1,4 @@ -<%# locals: (transaction:) %> +<%# locals: (transaction:, in_split_group: false) %> <%= render DS::Menu.new(variant: "button") do |menu| %> <% menu.with_button(class: "block w-full overflow-hidden") do %> @@ -11,7 +11,7 @@ <% end %> <% menu.with_custom_content do %> - <%= turbo_frame_tag "category_dropdown", src: category_dropdown_path(category_id: transaction.category_id, transaction_id: transaction.id), loading: :lazy do %> + <%= turbo_frame_tag "category_dropdown", src: category_dropdown_path(category_id: transaction.category_id, transaction_id: transaction.id, grouped: in_split_group), loading: :lazy do %>

<%= t(".loading") %>

diff --git a/app/views/category/dropdowns/_row.html.erb b/app/views/category/dropdowns/_row.html.erb index fbf6cf421..d9fab35e5 100644 --- a/app/views/category/dropdowns/_row.html.erb +++ b/app/views/category/dropdowns/_row.html.erb @@ -10,6 +10,7 @@ data: { filter_name: category.name } do %> <%= button_to transaction_category_path( @transaction.entry, + grouped: params[:grouped], entry: { entryable_type: "Transaction", entryable_attributes: { id: @transaction.id, category_id: category.id } diff --git a/app/views/category/dropdowns/show.html.erb b/app/views/category/dropdowns/show.html.erb index 234c31463..91ada562e 100644 --- a/app/views/category/dropdowns/show.html.erb +++ b/app/views/category/dropdowns/show.html.erb @@ -51,7 +51,7 @@ <%= button_to transaction_path(@transaction.entry), method: :patch, data: { turbo_frame: dom_id(@transaction.entry) }, - params: { entry: { entryable_type: "Transaction", entryable_attributes: { id: @transaction.id, category_id: nil } } }, + params: { grouped: params[:grouped], entry: { entryable_type: "Transaction", entryable_attributes: { id: @transaction.id, category_id: nil } } }, class: "flex text-sm font-medium items-center gap-2 text-secondary w-full rounded-lg p-2 hover:bg-container-inset-hover" do %> <%= icon("minus") %> @@ -85,6 +85,7 @@ method: :patch, data: { controller: "auto-submit-form" } do |f| %> <%= f.hidden_field "entry[excluded]", value: !@transaction.entry.excluded %> + <%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %> <%= f.check_box "entry[excluded]", checked: @transaction.entry.excluded, class: "checkbox checkbox--light", diff --git a/app/views/entries/_entry.html.erb b/app/views/entries/_entry.html.erb index 3c3f83fa3..5ba08622e 100644 --- a/app/views/entries/_entry.html.erb +++ b/app/views/entries/_entry.html.erb @@ -1,6 +1,6 @@ -<%# locals: (entry:, balance_trend: nil, view_ctx: "global") %> +<%# locals: (entry:, balance_trend: nil, view_ctx: "global", in_split_group: false) %> <% if entry.entryable.present? %> <%= render partial: entry.entryable.to_partial_path, - locals: { entry: entry, balance_trend: balance_trend, view_ctx: view_ctx } %> + locals: { entry: entry, balance_trend: balance_trend, view_ctx: view_ctx, in_split_group: in_split_group } %> <% end %> diff --git a/app/views/transactions/_notes.html.erb b/app/views/transactions/_notes.html.erb index 2d6114c0b..7e32b9156 100644 --- a/app/views/transactions/_notes.html.erb +++ b/app/views/transactions/_notes.html.erb @@ -3,6 +3,7 @@ <%= styled_form_with model: entry, url: transaction_path(entry), data: { controller: "auto-submit-form" } do |f| %> + <%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %> <%= f.text_area :notes, label: t("transactions.show.note_label"), placeholder: t("transactions.show.note_placeholder"), diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index 551c56138..8ee5761c1 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -19,7 +19,7 @@ } %>
- <%= render "transactions/transaction_category", transaction: transaction, variant: "mobile" %> + <%= render "transactions/transaction_category", transaction: transaction, variant: "mobile", in_split_group: in_split_group %> <% if transaction.merchant&.logo_url.present? %> <%= image_tag Setting.transform_brand_fetch_url(transaction.merchant.logo_url), class: "w-5 h-5 rounded-full absolute -bottom-1 -right-1 border border-secondary pointer-events-none", @@ -70,7 +70,7 @@ <% else %> <%= link_to( entry.name, - entry_path(entry), + in_split_group ? entry_path(entry, grouped: true) : entry_path(entry), data: { turbo_frame: "drawer", turbo_prefetch: false @@ -163,7 +163,7 @@ <%# For investment accounts, show activity label instead of category %> <%= render "investment_activity/quick_edit_badge", entry: entry, entryable: transaction %> <% else %> - <%= render "transactions/transaction_category", transaction: transaction, variant: "desktop" %> + <%= render "transactions/transaction_category", transaction: transaction, variant: "desktop", in_split_group: in_split_group %> <% end %>
diff --git a/app/views/transactions/_transaction_category.html.erb b/app/views/transactions/_transaction_category.html.erb index 434e422af..0d7301633 100644 --- a/app/views/transactions/_transaction_category.html.erb +++ b/app/views/transactions/_transaction_category.html.erb @@ -1,8 +1,8 @@ -<%# locals: (transaction:, variant:) %> +<%# locals: (transaction:, variant:, in_split_group: false) %>
" class="min-w-0 overflow-hidden"> <% if transaction.transfer&.categorizable? || transaction.transfer.nil? %> - <%= render "categories/menu", transaction: transaction %> + <%= render "categories/menu", transaction: transaction, in_split_group: in_split_group %> <% else %>