From 43e7e35e7e6ebdba1cce09a0f0ca2690e55d8033 Mon Sep 17 00:00:00 2001 From: thomasbaker9010251 Date: Sat, 9 May 2026 02:01:23 -0700 Subject: [PATCH 1/3] fix(transactions): update dialog content class for new transaction view (#1693) * fix(transactions): update dialog content class for new transaction view * feat(credit_card): add validation for expiration date and update form to prevent past dates - Implemented a validation method to ensure the expiration date of credit cards is not in the past. - Updated the credit card form to set a minimum date for the expiration date field, preventing users from selecting past dates. * fix(credit_card): update expiration date validation error message format - Changed the error message assertion for the expiration date validation to check for the symbol :greater_than_or_equal_to instead of a specific date string. This improves the flexibility and clarity of the validation error handling. * fix(transactions): enhance dialog content class for improved overflow handling * rebase --------- Co-authored-by: ms1112 Co-authored-by: petermilord --- app/views/transactions/new.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/transactions/new.html.erb b/app/views/transactions/new.html.erb index 2e5573ac4..b2273188b 100644 --- a/app/views/transactions/new.html.erb +++ b/app/views/transactions/new.html.erb @@ -1,4 +1,4 @@ -<%= render DS::Dialog.new(scrollable: false) do |dialog| %> +<%= render DS::Dialog.new(scrollable: false, content_class: "lg:max-h-none lg:overflow-y-auto") do |dialog| %> <% dialog.with_header(title: "New transaction") %> <% dialog.with_body do %> <%= render "form", entry: @entry, categories: @categories %> From 96b1d28d5d304b17f0c8bc9aa3968b3138af64cc Mon Sep 17 00:00:00 2001 From: CrossDrain <32982516+CrossDrain@users.noreply.github.com> Date: Sat, 9 May 2026 09:56:16 +0000 Subject: [PATCH 2/3] feat(enable-banking): safe pending transaction merge with sync re-import prevention (#1709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(enable-banking): safe pending transaction merge with sync re-import prevention * preserve all merged pending IDs across syncs * fix(enable-banking): harden merge locking, safe logging, and non-blocking index * fix(enable-banking): use safe external ID in invalid currency log * refactor(models): centralize pending transaction SQL logic Move the SQL fragment used to identify pending transactions from the `Entry` model to a constant in the `Transaction` model. This improves maintainability and ensures that the logic for determining if a transaction is pending is defined in a single location. * fix(enable-banking): drop dead manual_merge index, use lateral join for excluded IDs * No net schema changes --------- Co-authored-by: Juan José Mata --- .../pending_duplicate_merges_controller.rb | 4 + app/controllers/transactions_controller.rb | 4 +- .../transactions/processor.rb | 37 +++ app/models/enable_banking_entry/processor.rb | 25 +- app/models/entry.rb | 39 +-- app/models/transaction.rb | 109 ++++++- .../transactions/processor_test.rb | 198 +++++++++++ .../transaction/merge_with_duplicate_test.rb | 308 ++++++++++++++++++ 8 files changed, 683 insertions(+), 41 deletions(-) create mode 100644 test/models/enable_banking_account/transactions/processor_test.rb create mode 100644 test/models/transaction/merge_with_duplicate_test.rb diff --git a/app/controllers/pending_duplicate_merges_controller.rb b/app/controllers/pending_duplicate_merges_controller.rb index 80c592f3b..9b690daa5 100644 --- a/app/controllers/pending_duplicate_merges_controller.rb +++ b/app/controllers/pending_duplicate_merges_controller.rb @@ -52,6 +52,10 @@ class PendingDuplicateMergesController < ApplicationController else redirect_back_or_to transactions_path, alert: "Could not merge transactions" end + rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotDestroyed, + ActiveRecord::Deadlocked, ActiveRecord::LockWaitTimeout => e + Rails.logger.error("Failed to manually merge pending transaction: #{e.message}") + redirect_back_or_to transactions_path, alert: t("transactions.merge_duplicate.failure") end private diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index ce7e32438..944744d06 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -180,7 +180,9 @@ class TransactionsController < ApplicationController end redirect_to transactions_path - rescue ActiveRecord::RecordNotDestroyed, ActiveRecord::RecordInvalid => e + rescue ActiveRecord::RecordNotFound, ActiveRecord::RecordInvalid, + ActiveRecord::RecordNotDestroyed, ActiveRecord::Deadlocked, + ActiveRecord::LockWaitTimeout => e Rails.logger.error("Failed to merge duplicate transaction #{params[:id]}: #{e.message}") flash[:alert] = t("transactions.merge_duplicate.failure") redirect_to transactions_path diff --git a/app/models/enable_banking_account/transactions/processor.rb b/app/models/enable_banking_account/transactions/processor.rb index 831539610..fb50333e0 100644 --- a/app/models/enable_banking_account/transactions/processor.rb +++ b/app/models/enable_banking_account/transactions/processor.rb @@ -15,6 +15,7 @@ class EnableBankingAccount::Transactions::Processor Rails.logger.info "EnableBankingAccount::Transactions::Processor - Processing #{total_count} transactions for enable_banking_account #{enable_banking_account.id}" imported_count = 0 + skipped_count = 0 failed_count = 0 errors = [] @@ -22,8 +23,43 @@ class EnableBankingAccount::Transactions::Processor Account::ProviderImportAdapter.new(enable_banking_account.current_account) end + # Pre-fetch external_ids that were manually merged and must not be re-imported. + # One query per sync; O(1) Set lookup per transaction — avoids N+1. + # Uses a lateral jsonb_array_elements join to extract only the ID strings in SQL, + # avoiding loading full extra blobs into Ruby. Handles both Array (current) and + # Hash (legacy) formats via jsonb_typeof. + excluded_ids = if enable_banking_account.current_account + Transaction.joins(:entry) + .where(entries: { account_id: enable_banking_account.current_account.id }) + .where("transactions.extra ? 'manual_merge'") + .joins( + Arel.sql(<<~SQL.squish) + CROSS JOIN LATERAL jsonb_array_elements( + CASE jsonb_typeof(transactions.extra->'manual_merge') + WHEN 'array' THEN transactions.extra->'manual_merge' + WHEN 'object' THEN jsonb_build_array(transactions.extra->'manual_merge') + ELSE '[]'::jsonb + END + ) AS merge_elem + SQL + ) + .pluck(Arel.sql("merge_elem->>'merged_from_external_id'")) + .compact + .to_set + else + Set.new + end + enable_banking_account.raw_transactions_payload.each_with_index do |transaction_data, index| begin + ext_id = EnableBankingEntry::Processor.compute_external_id(transaction_data) + + if ext_id && excluded_ids.include?(ext_id) + Rails.logger.info("EnableBankingAccount::Transactions::Processor - Skipping re-import of manually merged pending transaction: #{ext_id}") + skipped_count += 1 + next + end + result = EnableBankingEntry::Processor.new( transaction_data, enable_banking_account: enable_banking_account, @@ -56,6 +92,7 @@ class EnableBankingAccount::Transactions::Processor success: failed_count == 0, total: total_count, imported: imported_count, + skipped: skipped_count, failed: failed_count, errors: errors } diff --git a/app/models/enable_banking_entry/processor.rb b/app/models/enable_banking_entry/processor.rb index 1cc2309db..e08d37acc 100644 --- a/app/models/enable_banking_entry/processor.rb +++ b/app/models/enable_banking_entry/processor.rb @@ -10,6 +10,12 @@ class EnableBankingEntry::Processor # transaction_amount: { amount, currency }, # creditor_name, debtor_name, remittance_information, ... # } + def self.compute_external_id(raw_transaction_data) + data = raw_transaction_data.with_indifferent_access + id = data[:transaction_id].presence || data[:entry_reference].presence + id ? "enable_banking_#{id}" : nil + end + def initialize(enable_banking_transaction, enable_banking_account:, import_adapter: nil) @enable_banking_transaction = enable_banking_transaction @enable_banking_account = enable_banking_account @@ -17,8 +23,12 @@ class EnableBankingEntry::Processor end def process + # Cache a safe diagnostic id upfront — used in all logging paths so rescue + # blocks never call the potentially-raising private external_id method. + safe_id = self.class.compute_external_id(@enable_banking_transaction) || "unknown" + unless account.present? - Rails.logger.warn "EnableBankingEntry::Processor - No linked account for enable_banking_account #{enable_banking_account.id}, skipping transaction #{external_id}" + Rails.logger.warn "EnableBankingEntry::Processor - No linked account for enable_banking_account #{enable_banking_account.id}, skipping transaction #{safe_id}" return nil end @@ -35,13 +45,13 @@ class EnableBankingEntry::Processor extra: extra ) rescue ArgumentError => e - Rails.logger.error "EnableBankingEntry::Processor - Validation error for transaction #{external_id}: #{e.message}" + Rails.logger.error "EnableBankingEntry::Processor - Validation error for transaction #{safe_id}: #{e.message}" raise rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved => e - Rails.logger.error "EnableBankingEntry::Processor - Failed to save transaction #{external_id}: #{e.message}" + Rails.logger.error "EnableBankingEntry::Processor - Failed to save transaction #{safe_id}: #{e.message}" raise StandardError.new("Failed to import transaction: #{e.message}") rescue => e - Rails.logger.error "EnableBankingEntry::Processor - Unexpected error processing transaction #{external_id}: #{e.class} - #{e.message}" + Rails.logger.error "EnableBankingEntry::Processor - Unexpected error processing transaction #{safe_id}: #{e.class} - #{e.message}" Rails.logger.error e.backtrace.join("\n") raise StandardError.new("Unexpected error importing transaction: #{e.message}") end @@ -64,9 +74,9 @@ class EnableBankingEntry::Processor end def external_id - id = data[:transaction_id].presence || data[:entry_reference].presence + id = self.class.compute_external_id(data) raise ArgumentError, "Enable Banking transaction missing required field 'transaction_id'" unless id - "enable_banking_#{id}" + id end def name @@ -220,7 +230,8 @@ class EnableBankingEntry::Processor end def log_invalid_currency(currency_value) - Rails.logger.warn("Invalid currency code '#{currency_value}' in Enable Banking transaction #{external_id}, falling back to account currency") + safe_id = self.class.compute_external_id(data) || "unknown" + Rails.logger.warn("Invalid currency code '#{currency_value}' in Enable Banking transaction #{safe_id}, falling back to account currency") end def date diff --git a/app/models/entry.rb b/app/models/entry.rb index 042ea6957..aaf079059 100644 --- a/app/models/entry.rb +++ b/app/models/entry.rb @@ -48,27 +48,20 @@ class Entry < ApplicationRecord # Pending transaction scopes - check Transaction.extra for provider pending flags # Works with any provider that stores pending status in extra["provider_name"]["pending"] scope :pending, -> { + conditions = Transaction::PENDING_PROVIDERS.map { |p| "(transactions.extra -> '#{p}' ->> 'pending')::boolean = true" } joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'") - .where(<<~SQL.squish) - (transactions.extra -> 'simplefin' ->> 'pending')::boolean = true - OR (transactions.extra -> 'plaid' ->> 'pending')::boolean = true - OR (transactions.extra -> 'lunchflow' ->> 'pending')::boolean = true - SQL + .where(conditions.join(" OR ")) } scope :excluding_pending, -> { # For non-Transaction entries (Trade, Valuation), always include - # For Transaction entries, exclude if pending flag is true + # For Transaction entries, exclude if any provider marks it pending where(<<~SQL.squish) entries.entryable_type != 'Transaction' OR NOT EXISTS ( SELECT 1 FROM transactions t WHERE t.id = entries.entryable_id - AND ( - (t.extra -> 'simplefin' ->> 'pending')::boolean = true - OR (t.extra -> 'plaid' ->> 'pending')::boolean = true - OR (t.extra -> 'lunchflow' ->> 'pending')::boolean = true - ) + AND (#{Transaction::PENDING_CHECK_SQL}) ) SQL } @@ -148,6 +141,10 @@ class Entry < ApplicationRecord def self.reconcile_pending_duplicates(account: nil, dry_run: false, date_window: 8, amount_tolerance: 0.25) stats = { checked: 0, reconciled: 0, details: [] } + not_pending_sql = Transaction::PENDING_PROVIDERS + .map { |p| "(transactions.extra -> '#{p}' ->> 'pending')::boolean IS NOT TRUE" } + .join(" AND ") + # Get pending entries to check scope = Entry.pending.where(excluded: false) scope = scope.where(account: account) if account @@ -164,11 +161,7 @@ class Entry < ApplicationRecord .where(currency: pending_entry.currency) .where(amount: pending_entry.amount) .where(date: pending_entry.date..(pending_entry.date + date_window.days)) # Posted must be ON or AFTER pending date - .where(<<~SQL.squish) - (transactions.extra -> 'simplefin' ->> 'pending')::boolean IS NOT TRUE - AND (transactions.extra -> 'plaid' ->> 'pending')::boolean IS NOT TRUE - AND (transactions.extra -> 'lunchflow' ->> 'pending')::boolean IS NOT TRUE - SQL + .where(not_pending_sql) .limit(2) # Only need to know if 0, 1, or 2+ candidates .to_a # Load limited records to avoid COUNT(*) on .size @@ -211,11 +204,7 @@ class Entry < ApplicationRecord .where(currency: pending_entry.currency) .where(date: pending_entry.date..(pending_entry.date + fuzzy_date_window.days)) # Posted ON or AFTER pending .where("ABS(entries.amount) BETWEEN ? AND ?", min_amount, max_amount) - .where(<<~SQL.squish) - (transactions.extra -> 'simplefin' ->> 'pending')::boolean IS NOT TRUE - AND (transactions.extra -> 'plaid' ->> 'pending')::boolean IS NOT TRUE - AND (transactions.extra -> 'lunchflow' ->> 'pending')::boolean IS NOT TRUE - SQL + .where(not_pending_sql) # Match by name similarity (first 3 words) name_words = pending_entry.name.downcase.gsub(/[^a-z0-9\s]/, "").split.first(3).join(" ") @@ -253,10 +242,12 @@ class Entry < ApplicationRecord pending_transaction.update!( extra: existing_extra.merge( "potential_posted_match" => { - "entry_id" => fuzzy_match.id, - "reason" => "fuzzy_amount_match", + "entry_id" => fuzzy_match.id, + "reason" => "fuzzy_amount_match", "posted_amount" => fuzzy_match.amount.to_s, - "detected_at" => Date.current.to_s + "confidence" => "medium", + "dismissed" => false, + "detected_at" => Date.current.to_s } ) ) diff --git a/app/models/transaction.rb b/app/models/transaction.rb index e20370ead..6ebc807be 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -95,6 +95,13 @@ class Transaction < ApplicationRecord # Providers that support pending transaction flags PENDING_PROVIDERS = %w[simplefin plaid lunchflow enable_banking].freeze + # Pre-computed SQL fragment for subqueries that check if a transaction (aliased as "t") is pending. + # Stored as a constant so static analysis can verify it contains no user input. + PENDING_CHECK_SQL = PENDING_PROVIDERS + .map { |p| "(t.extra -> '#{p}' ->> 'pending')::boolean = true" } + .join(" OR ") + .freeze + # Pending transaction scopes - filter based on provider pending flags in extra JSONB # Works with any provider that stores pending status in extra["provider_name"]["pending"] scope :pending, -> { @@ -140,7 +147,7 @@ class Transaction < ApplicationRecord PENDING_PROVIDERS.any? do |provider| ActiveModel::Type::Boolean.new.cast(extra_data.dig(provider, "pending")) end - rescue + rescue StandardError false end @@ -176,22 +183,106 @@ class Transaction < ApplicationRecord potential_posted_match_data&.dig("dismissed") == true end - # Merge this pending transaction with its suggested posted match - # This DELETES the pending entry since the posted version is canonical + # Merge this pending transaction with its suggested posted match. + # The pending entry is destroyed; the posted entry survives with attributes inherited from both sides. + # Attribute inheritance: Date + Category from pending, Name + Merchant from posted (booked). def merge_with_duplicate! + return false unless pending? return false unless has_potential_duplicate? posted_entry = potential_duplicate_entry return false unless posted_entry - pending_entry_id = entry.id - pending_entry_name = entry.name + pending_entry = entry - # Delete this pending entry completely (no need to keep it around) - entry.destroy! + # Guard: cross-account merges are never valid + if posted_entry.account_id != pending_entry.account_id + Rails.logger.warn("merge_with_duplicate! rejected: posted_entry #{posted_entry.id} belongs to different account than pending entry #{pending_entry.id}") + return false + end - Rails.logger.info("User merged pending entry #{pending_entry_id} (#{pending_entry_name}) with posted entry #{posted_entry.id}") - true + pending_entry_id = pending_entry.id + merge_succeeded = false + + ApplicationRecord.transaction(requires_new: true) do + # Row-level locks prevent concurrent merges on the same pair of entries. + # If a concurrent request already destroyed the pending entry, lock! raises + # RecordNotFound — treat that as an idempotent success. + begin + pending_entry.lock! + rescue ActiveRecord::RecordNotFound + Rails.logger.info("Pending entry #{pending_entry_id} already destroyed (concurrent merge), skipping") + return true + end + + begin + posted_entry.lock! + rescue ActiveRecord::RecordNotFound + Rails.logger.info("Posted entry #{posted_entry.id} deleted concurrently; aborting merge") + raise ActiveRecord::Rollback + end + + # Capture after lock! (which reloads) to guarantee DB-fresh values and avoid + # stale in-memory cached associations (e.g., loaded via touch: true). + external_id = pending_entry.external_id + pending_entry_date = pending_entry.date + + # Batch all changes to the surviving posted Transaction into a single update! + # to avoid firing after_save callbacks twice on the same row. + # Lock the Transaction row so concurrent merges into the same posted entry + # cannot race on reading/writing extra (e.g., the manual_merge array). + posted_tx = posted_entry.entryable + posted_tx.lock! if posted_tx.is_a?(Transaction) + if posted_tx.is_a?(Transaction) + tx_attrs = {} + + # Merge metadata — always written so the sync engine can skip re-importing. + # Stored as an array so multiple pending entries merged into the same posted + # transaction each preserve their external_id for future sync exclusion. + # Legacy records written as a plain Hash are migrated to a single-element array + # on first append, maintaining backward compatibility. + if external_id.present? + new_record = { + "merged_from_entry_id" => pending_entry_id, + "merged_from_external_id" => external_id, + "merged_at" => Time.current.iso8601, + "source" => pending_entry.source + } + prior = case posted_tx.extra["manual_merge"] + when Array then posted_tx.extra["manual_merge"] + when Hash then [ posted_tx.extra["manual_merge"] ] + else [] + end + tx_attrs[:extra] = posted_tx.extra.merge("manual_merge" => prior + [ new_record ]) + end + + # Attribute inheritance — only when the posted entry is not already user-protected. + unless posted_entry.protected_from_sync? + pending_transaction = pending_entry.entryable + if pending_transaction.is_a?(Transaction) && pending_transaction.category_id.present? + tx_attrs[:category_id] = pending_transaction.category_id + end + end + + posted_tx.update!(tx_attrs) if tx_attrs.any? + end + + # Date inheritance on the Entry row — separate from the Transaction update above. + unless posted_entry.protected_from_sync? + # Date: pending dates reflect actual transaction initiation time + posted_entry.update!(date: pending_entry_date) if posted_entry.date != pending_entry_date + # Name + Merchant intentionally NOT inherited — booked values are canonical + end + + # Lock the posted entry so future syncs cannot overwrite the merged state + posted_entry.mark_user_modified! + + Rails.logger.info("User merged pending entry #{pending_entry_id} (ext: #{external_id}) into posted entry #{posted_entry.id}") + pending_entry.destroy! + merge_succeeded = true + end + + merge_succeeded end # Dismiss the duplicate suggestion - user says these are NOT the same transaction diff --git a/test/models/enable_banking_account/transactions/processor_test.rb b/test/models/enable_banking_account/transactions/processor_test.rb new file mode 100644 index 000000000..f136ab806 --- /dev/null +++ b/test/models/enable_banking_account/transactions/processor_test.rb @@ -0,0 +1,198 @@ +require "test_helper" + +class EnableBankingAccount::Transactions::ProcessorTest < ActiveSupport::TestCase + include EntriesTestHelper + + setup do + @family = families(:dylan_family) + @account = accounts(:depository) + + @enable_banking_item = EnableBankingItem.create!( + family: @family, + name: "Test EB Item", + country_code: "FR", + application_id: "app_id", + client_certificate: "cert" + ) + @enable_banking_account = EnableBankingAccount.create!( + enable_banking_item: @enable_banking_item, + name: "Compte courant", + uid: "uid_txn_proc_test", + currency: "EUR", + current_balance: 1000.00 + ) + AccountProvider.create!(account: @account, provider: @enable_banking_account) + end + + # Minimal raw transaction payload hash matching the shape EnableBankingEntry::Processor expects + def raw_pending_transaction(transaction_id:) + { + transaction_id: transaction_id, + value_date: 3.days.ago.to_date.to_s, + transaction_amount: { amount: "25.00", currency: "EUR" }, + credit_debit_indicator: "DBIT", + _pending: true + } + end + + test "does not re-import a pending transaction whose external_id was manually merged" do + pending_ext_id = "enable_banking_PDNG_MERGED" + + # Simulate a previously-merged state: a posted transaction carries the pending's external_id + # in its manual_merge metadata, which is how merge_with_duplicate! records the merge. + posted_entry = create_transaction( + account: @account, + name: "Coffee Shop", + date: 1.day.ago.to_date, + amount: 25, + currency: "EUR", + external_id: "enable_banking_BOOK_SETTLED", + source: "enable_banking" + ) + posted_entry.transaction.update!( + extra: { + "manual_merge" => { + "merged_from_entry_id" => SecureRandom.uuid, + "merged_from_external_id" => pending_ext_id, + "merged_at" => Time.current.iso8601, + "source" => "enable_banking" + } + } + ) + posted_entry.mark_user_modified! + + # Raw payload contains the pending transaction that was already merged + @enable_banking_account.update!( + raw_transactions_payload: [ + raw_pending_transaction(transaction_id: "PDNG_MERGED") + ] + ) + + assert_no_difference "@account.entries.count" do + EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process + end + end + + test "imports a pending transaction that has NOT been merged" do + @enable_banking_account.update!( + raw_transactions_payload: [ + raw_pending_transaction(transaction_id: "PDNG_NEW_UNMERGED") + ] + ) + + assert_difference "@account.entries.count", 1 do + EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process + end + end + + test "imports non-excluded transactions alongside excluded ones in the same batch" do + pending_ext_id = "enable_banking_PDNG_SKIP_ME" + + posted_entry = create_transaction( + account: @account, + name: "Already Merged", + date: 2.days.ago.to_date, + amount: 15, + currency: "EUR", + external_id: "enable_banking_BOOK_ALREADY", + source: "enable_banking" + ) + posted_entry.transaction.update!( + extra: { + "manual_merge" => { + "merged_from_external_id" => pending_ext_id, + "merged_at" => Time.current.iso8601, + "source" => "enable_banking" + } + } + ) + + @enable_banking_account.update!( + raw_transactions_payload: [ + raw_pending_transaction(transaction_id: "PDNG_SKIP_ME"), # excluded + raw_pending_transaction(transaction_id: "PDNG_BRAND_NEW_12345") # should be imported + ] + ) + + assert_difference "@account.entries.count", 1 do + EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process + end + end + + test "excludes all external_ids when multiple pending entries were merged into the same posted entry" do + pending_ext_id_1 = "enable_banking_PDNG_MULTI_1" + pending_ext_id_2 = "enable_banking_PDNG_MULTI_2" + + posted_entry = create_transaction( + account: @account, + name: "Multi Merge", + date: 2.days.ago.to_date, + amount: 30, + currency: "EUR", + external_id: "enable_banking_BOOK_MULTI", + source: "enable_banking" + ) + posted_entry.transaction.update!( + extra: { + "manual_merge" => [ + { "merged_from_external_id" => pending_ext_id_1, "merged_at" => 2.days.ago.iso8601, "source" => "enable_banking" }, + { "merged_from_external_id" => pending_ext_id_2, "merged_at" => 1.day.ago.iso8601, "source" => "enable_banking" } + ] + } + ) + + @enable_banking_account.update!( + raw_transactions_payload: [ + raw_pending_transaction(transaction_id: "PDNG_MULTI_1"), # excluded + raw_pending_transaction(transaction_id: "PDNG_MULTI_2"), # excluded + raw_pending_transaction(transaction_id: "PDNG_MULTI_NEW") # new — should import + ] + ) + + result = nil + assert_difference "@account.entries.count", 1 do + result = EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process + end + assert_equal 2, result[:skipped] + assert_equal 1, result[:imported] + end + + test "handles empty raw_transactions_payload gracefully" do + @enable_banking_account.update!(raw_transactions_payload: nil) + + result = EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process + + assert_equal true, result[:success] + assert_equal 0, result[:total] + end + + test "reports excluded transactions as skipped, not imported or failed" do + pending_ext_id = "enable_banking_PDNG_SKIP_STATS" + + posted_entry = create_transaction( + account: @account, + name: "Stats Test", + date: 2.days.ago.to_date, + amount: 50, + currency: "EUR", + external_id: "enable_banking_BOOK_STATS", + source: "enable_banking" + ) + posted_entry.transaction.update!( + extra: { "manual_merge" => { "merged_from_external_id" => pending_ext_id } } + ) + + @enable_banking_account.update!( + raw_transactions_payload: [ + raw_pending_transaction(transaction_id: "PDNG_SKIP_STATS") + ] + ) + + result = EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process + + assert_equal true, result[:success] + assert_equal 1, result[:skipped] + assert_equal 0, result[:imported] + assert_equal 0, result[:failed] + end +end diff --git a/test/models/transaction/merge_with_duplicate_test.rb b/test/models/transaction/merge_with_duplicate_test.rb new file mode 100644 index 000000000..8149d0973 --- /dev/null +++ b/test/models/transaction/merge_with_duplicate_test.rb @@ -0,0 +1,308 @@ +require "test_helper" + +class Transaction::MergeWithDuplicateTest < ActiveSupport::TestCase + include EntriesTestHelper + + setup do + @account = accounts(:depository) + @family = @account.family + + @category_a = categories(:food_and_drink) + @category_b = categories(:income) + + # Pending entry — simulates a bank-synced pending transaction + @pending_entry = create_transaction( + account: @account, + name: "Starbucks Pending", + date: 3.days.ago.to_date, + amount: 10, + currency: "USD", + external_id: "enable_banking_PDNG123", + source: "enable_banking", + category: @category_a + ) + @pending_entry.transaction.update!( + extra: { "enable_banking" => { "pending" => true } } + ) + + # Posted (booked) entry — the canonical settled transaction from the bank + @posted_entry = create_transaction( + account: @account, + name: "STARBUCKS CORP", + date: 1.day.ago.to_date, + amount: 10, + currency: "USD", + external_id: "enable_banking_BOOK456", + source: "enable_banking" + ) + + # Wire up the duplicate suggestion on the pending transaction + @pending_entry.transaction.update!( + extra: @pending_entry.transaction.extra.merge( + "potential_posted_match" => { + "entry_id" => @posted_entry.id, + "reason" => "fuzzy_amount_match", + "posted_amount" => "10.0", + "confidence" => "medium", + "detected_at" => Date.current.to_s, + "dismissed" => false + } + ) + ) + end + + test "destroys the pending entry on successful merge" do + pending_id = @pending_entry.id + assert_difference "Entry.count", -1 do + @pending_entry.transaction.merge_with_duplicate! + end + assert_nil Entry.find_by(id: pending_id) + end + + test "records merged_from_external_id on the surviving posted transaction" do + @pending_entry.transaction.merge_with_duplicate! + + posted_tx = @posted_entry.transaction.reload + assert_equal "enable_banking_PDNG123", posted_tx.extra.dig("manual_merge", 0, "merged_from_external_id") + end + + test "records merged_from_entry_id and source in manual_merge metadata" do + pending_id = @pending_entry.id + @pending_entry.transaction.merge_with_duplicate! + + merge_meta = @posted_entry.transaction.reload.extra["manual_merge"].first + assert_equal pending_id, merge_meta["merged_from_entry_id"] + assert_equal "enable_banking", merge_meta["source"] + assert merge_meta["merged_at"].present? + end + + test "appends to existing manual_merge array preserving prior merged IDs" do + # Seed a prior merge record directly so the posted entry already has one ID + prior_ext_id = "enable_banking_PDNG_PRIOR" + @posted_entry.transaction.update!( + extra: { + "manual_merge" => [ + { "merged_from_external_id" => prior_ext_id, "merged_at" => 1.day.ago.iso8601, "source" => "enable_banking" } + ] + } + ) + + @pending_entry.transaction.merge_with_duplicate! + + records = @posted_entry.transaction.reload.extra["manual_merge"] + assert_equal 2, records.size + assert_includes records.map { |r| r["merged_from_external_id"] }, prior_ext_id + assert_includes records.map { |r| r["merged_from_external_id"] }, "enable_banking_PDNG123" + end + + test "migrates legacy single-object manual_merge to array on second merge" do + # Simulate an existing record written in the old single-Hash format + @posted_entry.transaction.update!( + extra: { + "manual_merge" => { + "merged_from_external_id" => "enable_banking_LEGACY", + "merged_at" => 1.day.ago.iso8601, + "source" => "enable_banking" + } + } + ) + + @pending_entry.transaction.merge_with_duplicate! + + records = @posted_entry.transaction.reload.extra["manual_merge"] + assert_kind_of Array, records + assert_equal 2, records.size + assert_includes records.map { |r| r["merged_from_external_id"] }, "enable_banking_LEGACY" + assert_includes records.map { |r| r["merged_from_external_id"] }, "enable_banking_PDNG123" + end + + test "inherits date from pending entry onto posted entry" do + original_posted_date = @posted_entry.date + pending_date = @pending_entry.date + refute_equal original_posted_date, pending_date + + @pending_entry.transaction.merge_with_duplicate! + + assert_equal pending_date, @posted_entry.reload.date + end + + test "inherits category from pending entry onto posted entry" do + assert_nil @posted_entry.transaction.category_id + + @pending_entry.transaction.merge_with_duplicate! + + assert_equal @category_a.id, @posted_entry.transaction.reload.category_id + end + + test "overwrites existing category on posted entry with pending category" do + @posted_entry.transaction.update!(category: @category_b) + + @pending_entry.transaction.merge_with_duplicate! + + assert_equal @category_a.id, @posted_entry.transaction.reload.category_id + end + + test "does not inherit name from pending — booked name is canonical" do + @pending_entry.transaction.merge_with_duplicate! + + assert_equal "STARBUCKS CORP", @posted_entry.reload.name + end + + test "marks the posted entry as user_modified to prevent future sync overwrites" do + refute @posted_entry.user_modified? + + @pending_entry.transaction.merge_with_duplicate! + + assert @posted_entry.reload.user_modified? + end + + test "returns true on success" do + result = @pending_entry.transaction.merge_with_duplicate! + assert_equal true, result + end + + test "returns false when no potential duplicate is set" do + @pending_entry.transaction.update!(extra: {}) + result = @pending_entry.transaction.merge_with_duplicate! + assert_equal false, result + end + + test "returns false when the suggested posted entry no longer exists" do + @posted_entry.destroy! + result = @pending_entry.transaction.merge_with_duplicate! + assert_equal false, result + end + + test "returns false and does not destroy pending entry when posted entry is on a different account" do + other_account = accounts(:credit_card) + @posted_entry.update!(account: other_account) + + result = nil + assert_no_difference "Entry.count" do + result = @pending_entry.transaction.merge_with_duplicate! + end + assert_equal false, result + end + + test "does not update date or category when posted entry is already user_modified" do + # Give posted entry a category so we can assert it was preserved (avoids nil==nil comparison) + @posted_entry.transaction.update!(category: @category_b) + original_date = @posted_entry.reload.date + original_category = @posted_entry.transaction.reload.category_id + @posted_entry.mark_user_modified! + + @pending_entry.transaction.merge_with_duplicate! + + assert_equal original_date, @posted_entry.reload.date + assert_equal original_category, @posted_entry.transaction.reload.category_id + end + + test "still records merge metadata even when posted entry is already user_modified" do + @posted_entry.mark_user_modified! + + @pending_entry.transaction.merge_with_duplicate! + + assert_equal "enable_banking_PDNG123", + @posted_entry.transaction.reload.extra.dig("manual_merge", 0, "merged_from_external_id") + end + + test "is idempotent when pending entry is already destroyed (concurrent merge)" do + @pending_entry.destroy! + + result = nil + assert_no_difference "Entry.count" do + result = @pending_entry.transaction.merge_with_duplicate! + end + assert_equal true, result + end + + test "skips storing merge metadata when pending entry has no external_id" do + @pending_entry.update!(external_id: nil) + + @pending_entry.transaction.merge_with_duplicate! + + merge_meta = @posted_entry.transaction.reload.extra&.dig("manual_merge") + assert_nil merge_meta + end + + # --- C-1: concurrent deletion of posted entry --- + + test "returns false when posted entry is deleted between check and lock" do + # Simulate the race: posted_entry exists at find_by time but is gone at lock! time. + # Use a stub rather than dup so id and account_id are real values — dup gives id: nil. + ghost = stub(account_id: @posted_entry.account_id, id: @posted_entry.id) + ghost.stubs(:lock!).raises(ActiveRecord::RecordNotFound) + @pending_entry.transaction.stubs(:potential_duplicate_entry).returns(ghost) + + result = nil + assert_no_difference "Entry.count" do + result = @pending_entry.transaction.merge_with_duplicate! + end + assert_equal false, result + end + + # --- cascade: delegated_type dependent: :destroy removes the Transaction too --- + + test "destroys the pending Transaction record on successful merge" do + assert_difference "Transaction.count", -1 do + @pending_entry.transaction.merge_with_duplicate! + end + end + + # --- dismiss_duplicate_suggestion! --- + + test "dismiss_duplicate_suggestion! sets dismissed flag on the match" do + @pending_entry.transaction.dismiss_duplicate_suggestion! + assert_equal true, @pending_entry.transaction.reload.extra.dig("potential_posted_match", "dismissed") + end + + test "dismiss_duplicate_suggestion! makes has_potential_duplicate? return false" do + @pending_entry.transaction.dismiss_duplicate_suggestion! + assert_not @pending_entry.transaction.reload.has_potential_duplicate? + end + + test "dismiss_duplicate_suggestion! returns false when no suggestion is present" do + @pending_entry.transaction.update!(extra: {}) + assert_equal false, @pending_entry.transaction.dismiss_duplicate_suggestion! + end + + test "merge_with_duplicate! returns false when suggestion has been dismissed" do + @pending_entry.transaction.dismiss_duplicate_suggestion! + assert_equal false, @pending_entry.transaction.reload.merge_with_duplicate! + end + + # --- clear_duplicate_suggestion! --- + + test "clear_duplicate_suggestion! removes potential_posted_match key entirely" do + @pending_entry.transaction.clear_duplicate_suggestion! + assert_nil @pending_entry.transaction.reload.extra["potential_posted_match"] + end + + test "clear_duplicate_suggestion! returns false when no suggestion is present" do + @pending_entry.transaction.update!(extra: {}) + assert_equal false, @pending_entry.transaction.clear_duplicate_suggestion! + end + + # --- pending_duplicate_candidates --- + + test "pending_duplicate_candidates returns posted transactions from the same account" do + candidates = @pending_entry.transaction.pending_duplicate_candidates + assert_includes candidates, @posted_entry + end + + test "pending_duplicate_candidates excludes the pending entry itself" do + candidates = @pending_entry.transaction.pending_duplicate_candidates + assert_not_includes candidates, @pending_entry + end + + test "pending_duplicate_candidates excludes transactions from other accounts" do + other_entry = create_transaction(account: accounts(:credit_card), amount: 10, currency: "USD") + candidates = @pending_entry.transaction.pending_duplicate_candidates + assert_not_includes candidates, other_entry + end + + test "pending_duplicate_candidates returns Entry.none when transaction is not pending" do + @pending_entry.transaction.update!(extra: {}) + assert_equal [], @pending_entry.transaction.pending_duplicate_candidates.to_a + end +end 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 3/3] 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 %>