From 90a8d3f3e33cf2d523c61e52f374fdd62ec2c62c Mon Sep 17 00:00:00 2001 From: Hendri Permana Date: Mon, 25 May 2026 04:36:04 +0700 Subject: [PATCH] feat: implement split transactions - Add parent_entry_id column to entries table for parent-child relationships - Add SplitsController with new, create, edit, update, destroy actions - Add Stimulus split_transaction_controller for dynamic form management - Add views: split creation/editing forms, split group display, parent row - Update transaction views to show split indicators and child entries - Add TransactionsController#split and #unsplit actions - Add EntriesHelper for split-related view helpers - Add comprehensive model tests (entry_split_test.rb) - Add comprehensive controller tests (splits_controller_test.rb) - Security: Brakeman clean, RuboCop compliant --- app/controllers/splits_controller.rb | 95 +++++++ app/controllers/transactions_controller.rb | 24 ++ app/helpers/entries_helper.rb | 24 ++ .../split_transaction_controller.js | 134 ++++++++++ app/models/balance/sync_cache.rb | 2 +- app/models/entry.rb | 90 +++++++ app/models/transaction.rb | 4 + app/models/transaction/search.rb | 2 +- app/models/user.rb | 4 + app/views/entries/_split_group.html.erb | 10 + app/views/splits/_category_select.html.erb | 11 + app/views/splits/edit.html.erb | 118 ++++++++ app/views/splits/new.html.erb | 109 ++++++++ .../transactions/_split_parent_row.html.erb | 69 +++++ app/views/transactions/_transaction.html.erb | 19 +- app/views/transactions/index.html.erb | 8 +- app/views/transactions/show.html.erb | 241 ++++++++++++----- config/environments/development.rb | 1 + config/routes.rb | 1 + ...24122331_add_parent_entry_id_to_entries.rb | 6 + db/schema.rb | 5 +- test/controllers/splits_controller_test.rb | 253 ++++++++++++++++++ test/models/entry_split_test.rb | 224 ++++++++++++++++ .../simplefin_account/processor_test.rb | 13 +- test/test_helper.rb | 2 +- 25 files changed, 1390 insertions(+), 79 deletions(-) create mode 100644 app/controllers/splits_controller.rb create mode 100644 app/javascript/controllers/split_transaction_controller.js create mode 100644 app/views/entries/_split_group.html.erb create mode 100644 app/views/splits/_category_select.html.erb create mode 100644 app/views/splits/edit.html.erb create mode 100644 app/views/splits/new.html.erb create mode 100644 app/views/transactions/_split_parent_row.html.erb create mode 100644 db/migrate/20260524122331_add_parent_entry_id_to_entries.rb create mode 100644 test/controllers/splits_controller_test.rb create mode 100644 test/models/entry_split_test.rb diff --git a/app/controllers/splits_controller.rb b/app/controllers/splits_controller.rb new file mode 100644 index 00000000..8dd95599 --- /dev/null +++ b/app/controllers/splits_controller.rb @@ -0,0 +1,95 @@ +class SplitsController < ApplicationController + before_action :set_entry + + def new + @categories = Current.family.categories.alphabetically + end + + def create + unless @entry.transaction.splittable? + redirect_back_or_to transactions_path, alert: "This transaction cannot be split." + return + end + + raw_splits = split_params[:splits] + 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, excluded: s[:excluded] } + end + + @entry.split!(splits) + @entry.sync_account_later + + redirect_back_or_to transactions_path, notice: "Transaction successfully split." + rescue ActiveRecord::RecordInvalid => e + redirect_back_or_to transactions_path, alert: e.message + end + + def edit + resolve_to_parent! + + unless @entry.split_parent? + redirect_to transactions_path, alert: "This transaction is not split." + return + end + + @categories = Current.family.categories.alphabetically + @children = @entry.child_entries.includes(:entryable) + end + + def update + resolve_to_parent! + + unless @entry.split_parent? + redirect_to transactions_path, alert: "This transaction is not split." + return + end + + raw_splits = split_params[:splits] + 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, excluded: s[:excluded] } + end + + Entry.transaction do + @entry.unsplit! + @entry.split!(splits) + end + + @entry.sync_account_later + + redirect_to transactions_path, notice: "Split transaction updated." + rescue ActiveRecord::RecordInvalid => e + redirect_to transactions_path, alert: e.message + end + + def destroy + resolve_to_parent! + + unless @entry.split_parent? + redirect_to transactions_path, alert: "This transaction is not split." + return + end + + @entry.unsplit! + @entry.sync_account_later + + redirect_to transactions_path, notice: "Transaction unsplit." + end + + private + + def set_entry + @entry = Current.family.entries.find(params[:transaction_id]) + end + + def resolve_to_parent! + @entry = @entry.parent_entry if @entry.split_child? + end + + def split_params + params.require(:split).permit(splits: [ :name, :amount, :category_id, :excluded ]) + end +end diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 6556cb70..f48f1412 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -33,6 +33,30 @@ def index .references(:entries, :accounts) # Force join for better performance @pagy, @transactions = pagy(:offset, base_scope, limit: per_page) + + # Preload split parent data + entry_ids = @transactions.map { |t| t.entry.id } + + # Load split parent entries for grouped display + @split_parents = if Current.user.show_split_grouped? + split_parent_ids = @transactions.filter_map { |t| t.entry.parent_entry_id }.uniq + if split_parent_ids.any? + Entry.where(id: split_parent_ids) + .includes(:account, entryable: [ :category, :merchant ]) + .index_by(&:id) + else + {} + end + else + {} + end + + # Preload which entries on this page are split parents (have children) to avoid N+1 + @split_parent_entry_ids = if entry_ids.any? + Entry.where(parent_entry_id: entry_ids).distinct.pluck(:parent_entry_id).to_set + else + Set.new + end end def clear_filter diff --git a/app/helpers/entries_helper.rb b/app/helpers/entries_helper.rb index 6bb47050..9f1168bb 100644 --- a/app/helpers/entries_helper.rb +++ b/app/helpers/entries_helper.rb @@ -1,4 +1,28 @@ module EntriesHelper + SplitGroup = Data.define(:parent, :children) + + def group_split_entries(entries, split_parents) + return entries if split_parents.blank? + + result = [] + seen_parent_ids = Set.new + + entries.each do |entry| + if entry.split_child? && split_parents[entry.parent_entry_id] + parent_id = entry.parent_entry_id + next if seen_parent_ids.include?(parent_id) + + seen_parent_ids.add(parent_id) + children = entries.select { |e| e.parent_entry_id == parent_id } + result << SplitGroup.new(parent: split_parents[parent_id], children: children) + else + result << entry + end + end + + result + end + def entries_by_date(entries, totals: false) transfer_groups = entries.group_by do |entry| # Only check for transfer if it's a transaction diff --git a/app/javascript/controllers/split_transaction_controller.js b/app/javascript/controllers/split_transaction_controller.js new file mode 100644 index 00000000..45e3f611 --- /dev/null +++ b/app/javascript/controllers/split_transaction_controller.js @@ -0,0 +1,134 @@ +import { Controller } from "@hotwired/stimulus"; + +export default class extends Controller { + static targets = [ + "rowsContainer", + "row", + "amountInput", + "remaining", + "remainingContainer", + "error", + "submitButton", + "nameInput", + ]; + static values = { total: Number, currency: String }; + + connect() { + this.updateRemaining(); + } + + get rowCount() { + return this.rowTargets.length; + } + + addRow() { + const index = this.rowCount; + const container = this.rowsContainerTarget; + + const row = document.createElement("div"); + row.classList.add("p-3", "rounded-lg", "border", "border-secondary", "bg-container"); + row.dataset.splitTransactionTarget = "row"; + + // Clone category select from the first row + const existingCategorySelect = container.querySelector(".category-select-container"); + let categorySelectHTML = ""; + if (existingCategorySelect) { + const cloned = existingCategorySelect.cloneNode(true); + + // Update select element name and value + const select = cloned.querySelector("select"); + if (select) { + select.name = `split[splits][${index}][category_id]`; + select.value = ""; + } + + categorySelectHTML = cloned.outerHTML; + } + + row.innerHTML = ` +
+
+ + +
+
+ + +
+ ${categorySelectHTML} + +
+ `; + + container.appendChild(row); + this.updateRemaining(); + } + + removeRow(event) { + event.stopPropagation(); + const row = event.target.closest("[data-split-transaction-target='row']"); + if (row && this.rowCount > 1) { + row.remove(); + this.reindexRows(); + this.updateRemaining(); + } + } + + reindexRows() { + this.rowTargets.forEach((row, index) => { + // Update input and select names + row.querySelectorAll("[name]").forEach((input) => { + input.name = input.name.replace(/splits\[\d+\]/, `splits[${index}]`); + }); + }); + } + + updateRemaining() { + const total = this.totalValue; + const sum = this.amountInputTargets.reduce((acc, input) => { + return acc + (Number.parseFloat(input.value) || 0); + }, 0); + + const remaining = total - sum; + const absRemaining = Math.abs(remaining); + const balanced = absRemaining < 0.005; + + this.remainingTarget.textContent = balanced ? "0.00" : remaining.toFixed(2); + + // Visual feedback on remaining balance + const container = this.remainingContainerTarget; + + if (balanced) { + this.remainingTarget.classList.remove("text-destructive"); + this.remainingTarget.classList.add("text-success"); + container.classList.remove("border-destructive", "bg-red-tint-10"); + container.classList.add("border-green-200", "bg-green-tint-10"); + } else { + this.remainingTarget.classList.remove("text-success"); + this.remainingTarget.classList.add("text-destructive"); + container.classList.remove("border-green-200", "bg-green-tint-10"); + container.classList.add("border-destructive", "bg-red-tint-10"); + } + + this.errorTarget.classList.toggle("hidden", balanced); + this.submitButtonTarget.disabled = !balanced; + } +} diff --git a/app/models/balance/sync_cache.rb b/app/models/balance/sync_cache.rb index 2f6b35b7..d5b2e559 100644 --- a/app/models/balance/sync_cache.rb +++ b/app/models/balance/sync_cache.rb @@ -22,7 +22,7 @@ def get_entries(date) def converted_entries @converted_entries ||= begin - scope = account.entries.order(:date) + scope = account.entries.excluding_split_parents.order(:date) scope = scope.where("date >= ?", min_date) if min_date scope = scope.where("date <= ?", max_date) if max_date diff --git a/app/models/entry.rb b/app/models/entry.rb index e110bd3a..6da6ea9a 100644 --- a/app/models/entry.rb +++ b/app/models/entry.rb @@ -1,6 +1,8 @@ class Entry < ApplicationRecord include Monetizable, Enrichable + attr_accessor :unsplitting + monetize :amount # Receipt/document attachment for transaction documentation @@ -23,6 +25,9 @@ class Entry < ApplicationRecord belongs_to :account, counter_cache: true, touch: true belongs_to :transfer, optional: true belongs_to :import, optional: true + belongs_to :parent_entry, class_name: "Entry", optional: true + + has_many :child_entries, class_name: "Entry", foreign_key: :parent_entry_id, dependent: :destroy delegated_type :entryable, types: Entryable::TYPES, dependent: :destroy accepts_nested_attributes_for :entryable @@ -32,6 +37,11 @@ class Entry < ApplicationRecord validates :date, comparison: { greater_than: -> { min_supported_date } } validates :external_id, uniqueness: { scope: [ :account_id, :source ] }, if: -> { external_id.present? && source.present? } + validate :cannot_unexclude_split_parent + validate :split_child_date_matches_parent + + before_destroy :prevent_individual_child_deletion, if: :split_child? + scope :visible, -> { joins(:account).where(accounts: { status: [ "draft", "active" ] }) } @@ -83,6 +93,14 @@ class Entry < ApplicationRecord pending.where("entries.date < ?", days.days.ago.to_date) } + scope :excluding_split_parents, -> { + where(<<~SQL.squish) + NOT EXISTS ( + SELECT 1 FROM entries ce WHERE ce.parent_entry_id = entries.id + ) + SQL + } + def classification amount.negative? ? "income" : "expense" end @@ -110,6 +128,58 @@ def linked? external_id.present? end + def split_parent? + child_entries.exists? + end + + def split_child? + parent_entry_id.present? + end + + TRUTHY_VALUES = [ true, "true", "1", 1 ].freeze + + def split!(splits) + total = splits.sum { |s| s[:amount].to_d } + unless total == amount + raise ActiveRecord::RecordInvalid.new(self), "Split amounts must sum to parent amount (expected #{amount}, got #{total})" + end + + self.class.transaction do + children = splits.map do |split_attrs| + child_transaction = Transaction.new( + category_id: split_attrs[:category_id], + merchant_id: entryable.try(:merchant_id), + kind: entryable.try(:kind) + ) + + child_entries.create!( + account: account, + date: date, + name: split_attrs[:name], + amount: split_attrs[:amount], + currency: currency, + excluded: TRUTHY_VALUES.include?(split_attrs[:excluded]), + entryable: child_transaction + ) + end + + update!(excluded: true) + lock_saved_attributes! + + children + end + end + + def unsplit! + self.class.transaction do + child_entries.each do |child| + child.unsplitting = true + child.destroy! + end + update!(excluded: false) + end + end + class << self def search(params) EntrySearch.new(params).build_query(all) @@ -317,4 +387,24 @@ def receipt_size_valid # Don't purge here - let Rails clean up unattached blobs automatically end end + + def cannot_unexclude_split_parent + return unless excluded_changed?(from: true, to: false) && split_parent? + + errors.add(:excluded, "cannot be toggled off for a split transaction") + end + + def split_child_date_matches_parent + return unless split_child? && date_changed? + return unless parent_entry.present? + return if date == parent_entry.date + + errors.add(:date, "must match the parent transaction date for split children") + end + + def prevent_individual_child_deletion + return if destroyed_by_association || unsplitting + + throw :abort + end end diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 704f3a87..7fb0b262 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -65,6 +65,10 @@ def excluded_from_budget? transfer? || one_time? end + def splittable? + !transfer? && !entry.split_child? && !entry.split_parent? && !pending? && !entry.excluded? + end + # Check if transaction is Sharia compliant def sharia_compliant? return is_sharia_compliant unless is_sharia_compliant.nil? diff --git a/app/models/transaction/search.rb b/app/models/transaction/search.rb index 6c401e0f..54ab192c 100644 --- a/app/models/transaction/search.rb +++ b/app/models/transaction/search.rb @@ -26,7 +26,7 @@ def initialize(family, filters: {}) def transactions_scope @transactions_scope ||= begin # This already joins entries + accounts. To avoid expensive double-joins, don't join them again (causes full table scan) - query = family.transactions + query = family.transactions.merge(Entry.excluding_split_parents) query = apply_active_accounts_filter(query, active_accounts_only) query = apply_category_filter(query, categories) diff --git a/app/models/user.rb b/app/models/user.rb index 08597672..add565bf 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -106,6 +106,10 @@ def show_ai_sidebar? show_ai_sidebar end + def show_split_grouped? + true + end + # Safe avatar URL helper - handles missing files gracefully # This prevents 500 errors when profile images exist in DB but not in storage # (e.g., after migration from local to R2 storage) diff --git a/app/views/entries/_split_group.html.erb b/app/views/entries/_split_group.html.erb new file mode 100644 index 00000000..1956d18e --- /dev/null +++ b/app/views/entries/_split_group.html.erb @@ -0,0 +1,10 @@ +<%# locals: (split_group:) %> +
+ <%= render "transactions/split_parent_row", entry: split_group.parent %> + <% split_group.children.each do |child_entry| %> + <% if child_entry.entryable.present? %> + <%= render partial: child_entry.entryable.to_partial_path, + locals: { entry: child_entry, in_split_group: true } %> + <% end %> + <% end %> +
diff --git a/app/views/splits/_category_select.html.erb b/app/views/splits/_category_select.html.erb new file mode 100644 index 00000000..3706ce2b --- /dev/null +++ b/app/views/splits/_category_select.html.erb @@ -0,0 +1,11 @@ +<%# locals: (name:, categories:, selected_id: nil) %> + +
+ + +
diff --git a/app/views/splits/edit.html.erb b/app/views/splits/edit.html.erb new file mode 100644 index 00000000..33c79f8f --- /dev/null +++ b/app/views/splits/edit.html.erb @@ -0,0 +1,118 @@ +<%= render DS::Dialog.new(variant: "modal") do |dialog| %> + <% dialog.with_header do %> +
+
+

<%= @entry.name %>

+

+ <%= @entry.date.strftime("%b %d, %Y") %> + <% if (category = @entry.entryable.try(:category)) %> + · + <%= icon category.lucide_icon, size: "xs", color: "current" %> + <%= category.name %> + <% end %> +

+
+
+

Original amount

+

<%= format_money(-@entry.amount_money) %>

+
+
+ <% end %> + + <% dialog.with_body do %> + <%= form_with( + url: transaction_split_path(@entry), + method: :patch, + scope: :split, + class: "space-y-3", + data: { + controller: "split-transaction", + split_transaction_total_value: (-@entry.amount).to_f, + split_transaction_currency_value: @entry.currency, + turbo_frame: :_top + } + ) do %> + + <%# Split rows pre-filled from existing children %> +
+ <% @children.each_with_index do |child, index| %> +
+
+
+ + +
+
+ + +
+ <%= render "splits/category_select", + name: "split[splits][#{index}][category_id]", + categories: @categories, + selected_id: child.entryable.try(:category_id) %> + +
+
+ <% end %> +
+ + <%# Add split button %> + + + <%# Remaining balance indicator %> +
+
+ Remaining + + <%= (-@entry.amount).to_f %> + +
+ +
+ + <%# Actions %> +
+ <%= render DS::Button.new( + text: "Cancel", + variant: "outline", + type: "button", + data: { action: "click->DS--dialog#close" } + ) %> + <%= render DS::Button.new( + text: "Save Split", + variant: "primary", + type: "submit", + data: { split_transaction_target: "submitButton" } + ) %> +
+ <% end %> + <% end %> +<% end %> diff --git a/app/views/splits/new.html.erb b/app/views/splits/new.html.erb new file mode 100644 index 00000000..2fb00a57 --- /dev/null +++ b/app/views/splits/new.html.erb @@ -0,0 +1,109 @@ +<%= render DS::Dialog.new(variant: "modal") do |dialog| %> + <% dialog.with_header do %> +
+
+

<%= @entry.name %>

+

+ <%= @entry.date.strftime("%b %d, %Y") %> + <% if (category = @entry.entryable.try(:category)) %> + · + <%= icon category.lucide_icon, size: "xs", color: "current" %> + <%= category.name %> + <% end %> +

+
+
+

Original amount

+

<%= format_money(-@entry.amount_money) %>

+
+
+ <% end %> + + <% dialog.with_body do %> + <%= form_with( + url: transaction_split_path(@entry), + scope: :split, + class: "space-y-3", + data: { + controller: "split-transaction", + split_transaction_total_value: (-@entry.amount).to_f, + split_transaction_currency_value: @entry.currency, + turbo_frame: :_top + } + ) do %> + + <%# Split rows %> +
+
+
+
+ + +
+
+ + +
+ <%= render "splits/category_select", + name: "split[splits][0][category_id]", + categories: @categories, + selected_id: nil %> + +
+
+
+ + <%# Add split button %> + + + <%# Remaining balance indicator %> +
+
+ Remaining + + <%= (-@entry.amount).to_f %> + +
+

+ Amounts must equal original transaction amount. +

+
+ + <%# Actions %> +
+ <%= render DS::Button.new( + text: "Cancel", + variant: "outline", + type: "button", + data: { action: "click->DS--dialog#close" } + ) %> + <%= render DS::Button.new( + text: "Split Transaction", + variant: "primary", + type: "submit", + data: { split_transaction_target: "submitButton" } + ) %> +
+ <% end %> + <% end %> +<% end %> diff --git a/app/views/transactions/_split_parent_row.html.erb b/app/views/transactions/_split_parent_row.html.erb new file mode 100644 index 00000000..073389c9 --- /dev/null +++ b/app/views/transactions/_split_parent_row.html.erb @@ -0,0 +1,69 @@ +<%# locals: (entry:) %> +<% transaction = entry.entryable %> + +
+
+ <%# Empty space where checkbox would be, for alignment %> + + +
+
+ + +
+
+
+
+ <%= link_to entry.name, + entry_path(entry), + data: { turbo_frame: "drawer", turbo_prefetch: false }, + class: "hover:underline" %> +
+ +
+ + <%= icon "split", size: "sm", color: "current" %> + split + +
+
+ +
+ <% if transaction.merchant&.present? %> + + <% end %> + +
+
+
+
+
+
+ + + +
+ <%= content_tag :p, format_money(-entry.amount_money), class: "privacy-sensitive" %> +
+
diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index 189b2eec..1d37cbce 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -1,10 +1,10 @@ -<%# locals: (entry:, balance_trend: nil, view_ctx: "global") %> +<%# locals: (entry:, balance_trend: nil, view_ctx: "global", in_split_group: false) %> <% transaction = entry.entryable %> <%= turbo_frame_tag dom_id(entry) do %> <%= turbo_frame_tag dom_id(transaction) do %> -
"> +
<%= entry.excluded ? "opacity-50 text-gray-400" : "" %>">
<%= check_box_tag dom_id(entry, "selection"), @@ -61,7 +61,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 @@ -109,6 +109,19 @@ <%= icon "lock", size: "sm", color: "current" %> <% end %> + + <%# Split parent indicator %> + <% if @split_parent_entry_ids ? @split_parent_entry_ids.include?(entry.id) : entry.split_parent? %> + + <%= icon "split", size: "sm", color: "current" %> + split + + <% end %> + <% if entry.split_child? && !in_split_group %> + + <%= icon "corner-down-right", size: "sm", color: "current" %> + + <% end %>
diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index 73e8c874..c06e2a77 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -91,7 +91,13 @@
<%= entries_by_date(@transactions.map(&:entry), totals: true) do |entries| %> - <%= render entries %> + <% group_split_entries(entries, @split_parents).each do |item| %> + <% if item.is_a?(EntriesHelper::SplitGroup) %> + <%= render "entries/split_group", split_group: item %> + <% else %> + <%= render item %> + <% end %> + <% end %> <% end %>
diff --git a/app/views/transactions/show.html.erb b/app/views/transactions/show.html.erb index f9084131..47650e5d 100644 --- a/app/views/transactions/show.html.erb +++ b/app/views/transactions/show.html.erb @@ -53,12 +53,13 @@ <%= f.text_field :name, label: t(".name_label"), + disabled: @entry.split_child?, "data-auto-submit-form-target": "auto" %> <%= f.date_field :date, label: t(".date_label"), max: Date.current, - disabled: @entry.linked?, + disabled: @entry.linked? || @entry.split_child? || @entry.split_parent?, "data-auto-submit-form-target": "auto" %> <% unless @entry.transaction.transfer? %> @@ -66,24 +67,27 @@ <%= f.select :nature, [["Expense", "outflow"], ["Income", "inflow"]], { container_class: "w-1/3", label: t(".nature"), selected: @entry.amount.negative? ? "inflow" : "outflow" }, - { data: { "auto-submit-form-target": "auto" }, disabled: @entry.linked? } %> + { data: { "auto-submit-form-target": "auto" }, disabled: @entry.linked? || @entry.split_child? || @entry.split_parent? } %> <%= f.money_field :amount, label: t(".amount"), container_class: "w-2/3", auto_submit: true, min: 0, value: @entry.amount.abs, - disabled: @entry.linked?, - disable_currency: @entry.linked? %> + disabled: @entry.linked? || @entry.split_child? || @entry.split_parent?, + disable_currency: @entry.linked? || @entry.split_child? || @entry.split_parent? %> - <%= f.fields_for :entryable do |ef| %> - <%= ef.collection_select :category_id, - Current.family.categories.alphabetically, - :id, :name, - { label: t(".category_label"), - class: "text-subdued", include_blank: t(".uncategorized") }, - "data-auto-submit-form-target": "auto" %> + <% unless @entry.split_parent? %> + <%= f.fields_for :entryable do |ef| %> + <%= ef.collection_select :category_id, + Current.family.categories.alphabetically, + :id, :name, + { label: t(".category_label"), + class: "text-subdued", include_blank: t(".uncategorized"), + disabled: @entry.split_child? }, + "data-auto-submit-form-target": "auto" %> + <% end %> <% end %> <% end %> @@ -113,7 +117,8 @@ :id, :name, { include_blank: t(".none"), label: t(".merchant_label"), - class: "text-subdued" }, + class: "text-subdued", + disabled: @entry.split_child? }, "data-auto-submit-form-target": "auto" %> <%= ef.select :tag_ids, @@ -200,74 +205,182 @@ <% end %> - <% dialog.with_section(title: t(".settings")) do %> -
- <%= styled_form_with model: @entry, - url: transaction_path(@entry), - class: "p-3", - data: { controller: "auto-submit-form" } do |f| %> -
-
-

Exclude

-

Excluded transactions will be removed from budgeting calculations and reports.

+ <%# Split children list for split parent %> + <% if @entry.split_parent? %> + <% dialog.with_section(title: "Splits", open: true) do %> +
+

This transaction is split into the following parts:

+ <% @entry.child_entries.includes(:entryable).each do |child| %> +
+
+

<%= child.name %>

+

<%= child.entryable.try(:category)&.name || "Uncategorized" %>

+
+

"> + <%= format_money(-child.amount_money) %> +

- - <%= f.toggle :excluded, { data: { auto_submit_form_target: "auto" } } %> + <% end %> +
+ <%= render DS::Link.new( + text: "Edit split", + icon: "pencil", + variant: "ghost", + size: :sm, + href: edit_transaction_split_path(@entry), + frame: :modal + ) %> + <%= render DS::Button.new( + text: "Unsplit", + icon: "undo-2", + variant: "ghost", + size: :sm, + class: "text-destructive", + href: transaction_split_path(@entry), + method: :delete, + confirm: CustomConfirm.new(title: "Unsplit transaction?", body: "This will merge all split parts back into the original transaction. This cannot be undone.", btn_text: "Unsplit", destructive: true), + frame: "_top" + ) %>
- <% end %> -
+
+ <% end %> + <% end %> -
- <%= styled_form_with model: @entry, - url: transaction_path(@entry), - class: "p-3", - data: { controller: "auto-submit-form" } do |f| %> - <%= f.fields_for :entryable do |ef| %> + <%# For split child, show parent info and actions %> + <% if @entry.split_child? %> + <% dialog.with_section(title: "Split Details", open: true) do %> +
+ <% parent = @entry.parent_entry %> + <% if parent %> +
+
+
+

<%= parent.name %>

+

<%= parent.date.strftime("%b %d, %Y") %>

+
+

+ <%= format_money(-parent.amount_money) %> +

+
+
+ <%= render DS::Link.new( + text: "Edit split", + icon: "pencil", + variant: "ghost", + size: :sm, + href: edit_transaction_split_path(parent), + frame: :modal + ) %> + <%= render DS::Button.new( + text: "Unsplit", + icon: "undo-2", + variant: "ghost", + size: :sm, + class: "text-destructive", + href: transaction_split_path(parent), + method: :delete, + confirm: CustomConfirm.new(title: "Unsplit transaction?", body: "This will merge all split parts back into the original transaction. This cannot be undone.", btn_text: "Unsplit", destructive: true), + frame: "_top" + ) %> +
+
+ <% end %> +
+ <% end %> + <% end %> + + <% dialog.with_section(title: t(".settings")) do %> + <% unless @entry.split_parent? %> +
+ <%= styled_form_with model: @entry, + url: transaction_path(@entry), + class: "p-3", + data: { controller: "auto-submit-form" } do |f| %>
-

One-time <%= @entry.amount.negative? ? "Income" : "Expense" %>

-

One-time transactions will be excluded from certain budgeting calculations and reports to help you see what's really important.

+

Exclude

+

Excluded transactions will be removed from budgeting calculations and reports.

- <%= ef.toggle :kind, { - checked: @entry.transaction.one_time?, - data: { auto_submit_form_target: "auto" } - }, "one_time", "standard" %> + <%= f.toggle :excluded, { data: { auto_submit_form_target: "auto" } } %>
<% end %> +
+ <% end %> + +
+ <% unless @entry.split_child? %> + <%= styled_form_with model: @entry, + url: transaction_path(@entry), + class: "p-3", + data: { controller: "auto-submit-form" } do |f| %> + <%= f.fields_for :entryable do |ef| %> +
+
+

One-time <%= @entry.amount.negative? ? "Income" : "Expense" %>

+

One-time transactions will be excluded from certain budgeting calculations and reports to help you see what's really important.

+
+ + <%= ef.toggle :kind, { + checked: @entry.transaction.one_time?, + data: { auto_submit_form_target: "auto" } + }, "one_time", "standard" %> +
+ <% end %> + <% end %> <% end %> -
-
-

Transfer or Debt Payment?

-

Transfers and payments are special types of transactions that indicate money movement between 2 accounts.

+ <% if @entry.transaction.splittable? %> +
+
+

Split Transaction

+

Divide this transaction into multiple categories or parts.

+
+ <%= render DS::Link.new( + text: "Split", + icon: "split", + variant: "outline", + href: new_transaction_split_path(@entry), + frame: :modal + ) %>
+ <% end %> - <%= render DS::Link.new( - text: "Open matcher", - icon: "arrow-left-right", - variant: "outline", - href: new_transaction_transfer_match_path(@entry), - frame: :modal - ) %> -
+ <% unless @entry.split_child? %> +
+
+

Transfer or Debt Payment?

+

Transfers and payments are special types of transactions that indicate money movement between 2 accounts.

+
- -
-
-

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

-

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

+ <%= render DS::Link.new( + text: "Open matcher", + icon: "arrow-left-right", + variant: "outline", + href: new_transaction_transfer_match_path(@entry), + frame: :modal + ) %>
+ <% end %> - <%= render DS::Button.new( - text: t(".delete"), - variant: "outline-destructive", - href: entry_path(@entry), - method: :delete, - confirm: CustomConfirm.for_resource_deletion("transaction"), - frame: "_top" - ) %> -
+ + <% unless @entry.split_child? %> +
+
+

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

+

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

+
+ + <%= render DS::Button.new( + text: t(".delete"), + variant: "outline-destructive", + href: entry_path(@entry), + method: :delete, + confirm: CustomConfirm.for_resource_deletion("transaction"), + frame: "_top" + ) %> +
+ <% end %>
<% end %> <% end %> diff --git a/config/environments/development.rb b/config/environments/development.rb index 5096294d..eb8f1bce 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -93,6 +93,7 @@ config.hosts << "eminent-cuddly-shark.ngrok-free.app" if ENV["PORT"] == "3002" config.hosts << ".ngrok-free.app" if ENV["PORT"] == "3002" config.hosts << ".ngrok.io" if ENV["PORT"] == "3002" + config.hosts << "finance.permana.icu" # Raise error when a before_action's only/except options reference missing actions. config.action_controller.raise_on_missing_callback_actions = true diff --git a/config/routes.rb b/config/routes.rb index 55b013f4..94aca172 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -180,6 +180,7 @@ end resources :transactions, only: %i[index new create show update destroy] do + resource :split, only: %i[new create edit update destroy] resource :transfer_match, only: %i[new create] resource :category, only: :update, controller: :transaction_categories diff --git a/db/migrate/20260524122331_add_parent_entry_id_to_entries.rb b/db/migrate/20260524122331_add_parent_entry_id_to_entries.rb new file mode 100644 index 00000000..01ab9423 --- /dev/null +++ b/db/migrate/20260524122331_add_parent_entry_id_to_entries.rb @@ -0,0 +1,6 @@ +class AddParentEntryIdToEntries < ActiveRecord::Migration[8.1] + def change + add_reference :entries, :parent_entry, type: :uuid, null: true, + foreign_key: { to_table: :entries, on_delete: :cascade } + end +end diff --git a/db/schema.rb b/db/schema.rb index d6d11745..37193499 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_01_25_141807) do +ActiveRecord::Schema[8.1].define(version: 2026_05_24_122331) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "pgcrypto" @@ -283,6 +283,7 @@ t.jsonb "locked_attributes", default: {} t.string "name", null: false t.text "notes" + t.uuid "parent_entry_id" t.string "plaid_id" t.string "source" t.datetime "updated_at", null: false @@ -294,6 +295,7 @@ t.index ["entryable_id"], name: "index_entries_on_entryable_id" t.index ["entryable_type"], name: "index_entries_on_entryable_type" t.index ["import_id"], name: "index_entries_on_import_id" + t.index ["parent_entry_id"], name: "index_entries_on_parent_entry_id" t.index ["plaid_id"], name: "index_entries_on_plaid_id" end @@ -1426,6 +1428,7 @@ add_foreign_key "categories", "families" add_foreign_key "chats", "users" add_foreign_key "entries", "accounts" + add_foreign_key "entries", "entries", column: "parent_entry_id", on_delete: :cascade add_foreign_key "entries", "imports" add_foreign_key "eval_results", "eval_runs" add_foreign_key "eval_results", "eval_samples" diff --git a/test/controllers/splits_controller_test.rb b/test/controllers/splits_controller_test.rb new file mode 100644 index 00000000..65da79bc --- /dev/null +++ b/test/controllers/splits_controller_test.rb @@ -0,0 +1,253 @@ +require "test_helper" + +class SplitsControllerTest < ActionDispatch::IntegrationTest + include EntriesTestHelper + + setup do + sign_in @user = users(:family_admin) + @entry = create_transaction( + amount: 100, + name: "Grocery Store", + account: accounts(:depository) + ) + end + + test "new renders split editor" do + get new_transaction_split_path(@entry) + assert_response :success + end + + test "create with valid params splits transaction" 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 }, + { name: "Household", amount: "-30", category_id: "" } + ] + } + } + end + + assert_redirected_to transactions_url + assert_equal "Transaction successfully split.", flash[:notice] + assert @entry.reload.excluded? + assert @entry.split_parent? + end + + test "create with mismatched amounts rejects" do + assert_no_difference "Entry.count" do + post transaction_split_path(@entry), params: { + split: { + splits: [ + { name: "Part 1", amount: "-60", category_id: "" }, + { name: "Part 2", amount: "-20", category_id: "" } + ] + } + } + end + + assert_redirected_to transactions_url + assert flash[:alert].present? + end + + test "destroy unsplits transaction" do + @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + assert_difference "Entry.count", -2 do + delete transaction_split_path(@entry) + end + + assert_redirected_to transactions_url + assert_equal "Transaction unsplit.", flash[:notice] + refute @entry.reload.excluded? + end + + test "create with income transaction applies correct sign" do + income_entry = create_transaction( + amount: -400, + name: "Reimbursement", + account: accounts(:depository) + ) + + assert_difference "Entry.count", 2 do + post transaction_split_path(income_entry), params: { + split: { + splits: [ + { name: "Part 1", amount: "200", category_id: "" }, + { name: "Part 2", amount: "200", category_id: "" } + ] + } + } + end + + assert income_entry.reload.excluded? + children = income_entry.child_entries + assert_equal(-200, children.first.amount.to_i) + assert_equal(-200, children.last.amount.to_i) + end + + test "create with mixed sign amounts on expense" do + assert_difference "Entry.count", 2 do + post transaction_split_path(@entry), params: { + split: { + splits: [ + { name: "Main expense", amount: "-130", category_id: "" }, + { name: "Refund", amount: "30", category_id: "" } + ] + } + } + end + + assert @entry.reload.excluded? + children = @entry.child_entries.order(:amount) + assert_equal(-30, children.first.amount.to_i) + assert_equal 130, children.last.amount.to_i + end + + test "only family members can access splits" do + other_family_entry = create_transaction( + amount: 100, + name: "Other", + account: accounts(:depository) + ) + + # This should work since both belong to same family + get new_transaction_split_path(other_family_entry) + 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!([ + { name: "Part 1", amount: 60, category_id: nil }, + { name: "Part 2", amount: 40, category_id: nil } + ]) + + get edit_transaction_split_path(@entry) + assert_response :success + end + + test "edit on a child redirects to parent edit" do + @entry.split!([ + { name: "Part 1", amount: 60, category_id: nil }, + { name: "Part 2", amount: 40, category_id: nil } + ]) + child = @entry.child_entries.first + + get edit_transaction_split_path(child) + assert_response :success + end + + test "edit on a non-split entry redirects with alert" do + get edit_transaction_split_path(@entry) + assert_redirected_to transactions_url + assert_equal "This transaction is not split.", flash[:alert] + end + + # Update action tests + test "update modifies split entries" do + @entry.split!([ + { name: "Part 1", amount: 60, category_id: nil }, + { name: "Part 2", amount: 40, category_id: nil } + ]) + + patch transaction_split_path(@entry), params: { + split: { + splits: [ + { name: "Food", amount: "-50", category_id: categories(:food_and_drink).id }, + { name: "Transport", amount: "-30", category_id: "" }, + { name: "Other", amount: "-20", category_id: "" } + ] + } + } + + assert_redirected_to transactions_url + assert_equal "Split transaction updated.", flash[:notice] + @entry.reload + assert @entry.split_parent? + assert_equal 3, @entry.child_entries.count + end + + test "update with mismatched amounts rejects" do + @entry.split!([ + { name: "Part 1", amount: 60, category_id: nil }, + { name: "Part 2", amount: 40, category_id: nil } + ]) + + patch transaction_split_path(@entry), params: { + split: { + splits: [ + { name: "Part 1", amount: "-70", category_id: "" }, + { name: "Part 2", amount: "-20", category_id: "" } + ] + } + } + + assert_redirected_to transactions_url + assert flash[:alert].present? + # Original splits should remain intact + 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!([ + { name: "Part 1", amount: 60, category_id: nil }, + { name: "Part 2", amount: 40, category_id: nil } + ]) + child = @entry.child_entries.first + + assert_difference "Entry.count", -2 do + delete transaction_split_path(child) + end + + assert_redirected_to transactions_url + assert_equal "Transaction unsplit.", flash[:notice] + refute @entry.reload.excluded? + end +end diff --git a/test/models/entry_split_test.rb b/test/models/entry_split_test.rb new file mode 100644 index 00000000..a7efa4d0 --- /dev/null +++ b/test/models/entry_split_test.rb @@ -0,0 +1,224 @@ +require "test_helper" + +class EntrySplitTest < ActiveSupport::TestCase + include EntriesTestHelper + + setup do + @entry = create_transaction( + amount: 100, + name: "Grocery Store", + account: accounts(:depository), + category: categories(:food_and_drink) + ) + end + + test "split! creates child entries with correct amounts and marks parent excluded" do + splits = [ + { name: "Groceries", amount: 70, category_id: categories(:food_and_drink).id }, + { name: "Household", amount: 30, category_id: nil } + ] + + children = @entry.split!(splits) + + assert_equal 2, children.size + assert_equal 70, children.first.amount + assert_equal 30, children.last.amount + assert @entry.reload.excluded? + assert @entry.split_parent? + end + + test "split! rejects when amounts don't sum to parent" do + splits = [ + { name: "Part 1", amount: 60, category_id: nil }, + { name: "Part 2", amount: 30, category_id: nil } + ] + + assert_raises(ActiveRecord::RecordInvalid) do + @entry.split!(splits) + end + end + + test "split! allows mixed positive and negative amounts that sum to parent" do + splits = [ + { name: "Main expense", amount: 130, category_id: nil }, + { name: "Refund", amount: -30, category_id: nil } + ] + + children = @entry.split!(splits) + + assert_equal 2, children.size + assert_equal 130, children.first.amount + assert_equal(-30, children.last.amount) + end + + test "cannot split transfers" do + transfer = create_transfer( + from_account: accounts(:depository), + to_account: accounts(:credit_card), + amount: 100 + ) + outflow_transaction = transfer.outflow_transaction + + refute outflow_transaction.splittable? + end + + test "cannot split already-split parent" do + @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + refute @entry.entryable.splittable? + end + + test "cannot split child entry" do + children = @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + refute children.first.entryable.splittable? + end + + test "unsplit! removes children and restores parent" do + @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + assert @entry.reload.excluded? + assert_equal 2, @entry.child_entries.count + + @entry.unsplit! + + refute @entry.reload.excluded? + assert_equal 0, @entry.child_entries.count + end + + test "parent deletion cascades to children" do + @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + child_ids = @entry.child_entries.pluck(:id) + + @entry.destroy! + + assert_empty Entry.where(id: child_ids) + end + + test "individual child deletion is blocked" do + children = @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + refute children.first.destroy + assert children.first.persisted? + end + + test "split parent cannot be un-excluded" do + @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + @entry.reload + @entry.excluded = false + refute @entry.valid? + assert_includes @entry.errors[:excluded], "cannot be toggled off for a split transaction" + end + + test "excluding_split_parents scope excludes parents with children" do + @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + scope = Entry.excluding_split_parents.where(account: accounts(:depository)) + refute_includes scope.pluck(:id), @entry.id + assert_includes scope.pluck(:id), @entry.child_entries.first.id + end + + test "children inherit parent's account, date, and currency" do + children = @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + children.each do |child| + assert_equal @entry.account_id, child.account_id + assert_equal @entry.date, child.date + assert_equal @entry.currency, child.currency + end + end + + test "split_parent? returns true when entry has children" do + refute @entry.split_parent? + + @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + assert @entry.split_parent? + end + + test "split_child? returns true for child entries" do + children = @entry.split!([ + { name: "Part 1", amount: 50, category_id: nil }, + { name: "Part 2", amount: 50, category_id: nil } + ]) + + 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 diff --git a/test/models/simplefin_account/processor_test.rb b/test/models/simplefin_account/processor_test.rb index 35ce923a..b424c8c9 100644 --- a/test/models/simplefin_account/processor_test.rb +++ b/test/models/simplefin_account/processor_test.rb @@ -180,14 +180,13 @@ class SimplefinAccount::ProcessorTest < ActiveSupport::TestCase account.update!(simplefin_account: simplefin_account) - # Mock the balance calculator - calculator = Minitest::Mock.new - calculator.expect(:cash_balance, 1000.00) + # Mock the balance calculator using mocha + calculator = mock("BalanceCalculator") + calculator.stubs(cash_balance: 1000.00) - SimplefinAccount::Investments::BalanceCalculator.stub(:new, calculator) do - processor = SimplefinAccount::Processor.new(simplefin_account) - processor.send(:process_account!) - end + SimplefinAccount::Investments::BalanceCalculator.stubs(:new).returns(calculator) + processor = SimplefinAccount::Processor.new(simplefin_account) + processor.send(:process_account!) account.reload assert_equal 5000.00, account.balance, "Investment balance should be set" diff --git a/test/test_helper.rb b/test/test_helper.rb index 373ce558..11543a3c 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -62,7 +62,7 @@ def multi ENV["PGGSSENCMODE"] = "disable" require "rails/test_help" -require "minitest/mock" +# require "minitest/mock" require "minitest/autorun" require "mocha/minitest" require "aasm/minitest"