QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-05-09 13:45:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(splits): add exclusion support for splits and improve rendering (#1661)

Browse Source 
* 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
This commit is contained in:
CrossDrain
2026-05-09 10:36:41 +00:00
committed by GitHub
parent 96b1d28d5d
commit 0b7fa732ae
15 changed files with 128 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
app/controllers/splits_controller.rb
View File

@@ -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

5
app/controllers/transaction_categories_controller.rb
View File

@@ -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}",

7
app/controllers/transactions_controller.rb
View File

@@ -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

4
app/helpers/transactions_helper.rb
View File

@@ -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).

6
app/models/entry.rb
View File

@@ -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<Hash>] array of { name:, amount:, category_id: } hashes
# @param splits [Array<Hash>] array of { name:, amount:, category_id:, excluded: } hashes
# @return [Array<Entry>] 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

4
app/views/categories/_menu.html.erb
View File

@@ -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 %>
<div class="p-6 flex items-center justify-center">
<p class="text-sm text-secondary animate-pulse"><%= t(".loading") %></p>
</div>

1
app/views/category/dropdowns/_row.html.erb
View File

@@ -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 }

3
app/views/category/dropdowns/show.html.erb
View File

@@ -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",

4
app/views/entries/_entry.html.erb
View File

@@ -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 %>

1
app/views/transactions/_notes.html.erb
View File

@@ -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"),

6
app/views/transactions/_transaction.html.erb
View File

@@ -19,7 +19,7 @@
} %>
<div class="flex md:hidden items-center gap-1 col-span-2 relative shrink-0">
<%= 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 %>
</div>

4
app/views/transactions/_transaction_category.html.erb
View File

@@ -1,8 +1,8 @@
<%# locals: (transaction:, variant:) %>
<%# locals: (transaction:, variant:, in_split_group: false) %>
<div id="<%= dom_id(transaction, "category_menu_#{variant}") %>" 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 %>
<div class="hidden lg:flex">
<%= render "categories/badge", category: transaction.transfer&.payment? ? payment_category : transfer_category %>

7
app/views/transactions/show.html.erb
View File

@@ -54,6 +54,7 @@
url: transaction_path(@entry),
class: "space-y-2",
data: { controller: "auto-submit-form" } do |f| %>
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
<%= f.text_field :name,
label: t(".name_label"),
disabled: @entry.split_child? || edit_locked,
@@ -95,6 +96,7 @@
url: transaction_path(@entry),
class: "space-y-2",
data: { controller: "auto-submit-form" } do |f| %>
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
<% unless @entry.transaction.transfer? %>
<%= f.select :account,
options_for_select(
@@ -262,12 +264,13 @@
<% if can_edit_entry? %>
<% dialog.with_section(title: t(".settings")) do %>
<% unless @entry.split_parent? || @entry.split_child? %>
<% unless @entry.split_parent? %>
<div class="pb-4">
<%= styled_form_with model: @entry,
url: transaction_path(@entry),
class: "p-3",
data: { controller: "auto-submit-form" } do |f| %>
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
<div class="flex cursor-pointer items-center gap-4 justify-between">
<div class="text-sm space-y-1">
<h4 class="text-primary"><%= t(".exclude") %></h4>
@@ -284,6 +287,7 @@
url: transaction_path(@entry),
class: "p-3",
data: { controller: "auto-submit-form" } do |f| %>
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
<%= f.fields_for :entryable do |ef| %>
<div class="flex cursor-pointer items-center gap-4 justify-between">
<div class="text-sm space-y-1">
@@ -309,6 +313,7 @@
url: transaction_path(@entry),
class: "p-3",
data: { controller: "auto-submit-form" } do |f| %>
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
<%= f.fields_for :entryable do |ef| %>
<div class="flex cursor-pointer items-center gap-4 justify-between">
<div class="text-sm space-y-1">

41
test/controllers/splits_controller_test.rb
View File

@@ -120,6 +120,26 @@ class SplitsControllerTest < ActionDispatch::IntegrationTest
assert_response :success
end
test "create with excluded parameter sets child as excluded" do
assert_difference "Entry.count", 2 do
post transaction_split_path(@entry), params: {
split: {
splits: [
{ name: "Groceries", amount: "-70", category_id: categories(:food_and_drink).id, excluded: "true" },
{ name: "Household", amount: "-30", category_id: "", excluded: "false" }
]
}
}
end
assert_redirected_to transactions_url
children = @entry.child_entries.order(:amount)
# Household has amount 30 (smaller), Groceries has amount 70 (larger)
# Household is NOT excluded, Groceries IS excluded
refute children.first.excluded?
assert children.last.excluded?
end
# Edit action tests
test "edit renders with existing children pre-filled" do
@entry.split!([
@@ -193,6 +213,27 @@ class SplitsControllerTest < ActionDispatch::IntegrationTest
assert_equal 2, @entry.reload.child_entries.count
end
test "update with excluded parameter sets child as excluded" do
@entry.split!([
{ name: "Groceries", amount: 70, category_id: nil },
{ name: "Household", amount: 30, category_id: nil }
])
patch transaction_split_path(@entry), params: {
split: {
splits: [
{ name: "Groceries", amount: "-70", category_id: "", excluded: "true" },
{ name: "Household", amount: "-30", category_id: "", excluded: "false" }
]
}
}
assert_redirected_to transactions_url
children = @entry.child_entries.order(:amount)
refute children.first.excluded?
assert children.last.excluded?
end
# Destroy from child tests
test "destroy from child resolves to parent and unsplits" do
@entry.split!([

47
test/models/entry_split_test.rb
View File

@@ -174,4 +174,51 @@ class EntrySplitTest < ActiveSupport::TestCase
assert children.first.split_child?
refute @entry.split_child?
end
test "split! creates child entries with excluded: true when specified" do
splits = [
{ name: "Part 1", amount: 50, category_id: nil, excluded: true },
{ name: "Part 2", amount: 50, category_id: nil, excluded: false }
]
children = @entry.split!(splits)
assert_equal 2, children.size
assert children.first.excluded?
refute children.last.excluded?
end
test "split! properly casts excluded from string values" do
splits = [
{ name: "Part 1", amount: 50, category_id: nil, excluded: "true" },
{ name: "Part 2", amount: 50, category_id: nil, excluded: "false" }
]
children = @entry.split!(splits)
assert children.first.excluded?
refute children.last.excluded?
end
test "excluded split children are excluded from balance calculations" do
@entry.split!([
{ name: "Part 1", amount: 50, category_id: nil, excluded: true },
{ name: "Part 2", amount: 50, category_id: nil, excluded: false }
])
# Parent is always excluded for splits
assert @entry.reload.excluded?
# Excluded child should be filtered out by where(excluded: false)
excluded_child = @entry.child_entries.find { |c| c.name == "Part 1" }
non_excluded_child = @entry.child_entries.find { |c| c.name == "Part 2" }
assert excluded_child.excluded?
refute non_excluded_child.excluded?
# where(excluded: false) should only include the non-excluded child
visible_entries = Entry.where(id: @entry.child_entries.map(&:id)).where(excluded: false)
assert_includes visible_entries.pluck(:id), non_excluded_child.id
refute_includes visible_entries.pluck(:id), excluded_child.id
end
end