feat: add pending transaction manual merging tool (#1088)

* refactor: use a map of providers that support pending transactions

* feat: add pending transaction manual merging tool

* fix(coderabbit): validate posted_entry_id against eligible posted candidates server-side

* fix(coderabbit): validate offset for negative numbers

* fix(coderabbit): check if pending_duplicate_candidates has_more in one transaction

* refactor: use list of radio buttons for better pagination

* chore: show current transaction range in paginated view

* chore: whitespace

chore: whitespace
This commit is contained in:
AdamWHY2K
2026-03-14 19:32:13 +00:00
committed by GitHub
parent 57199d6eb9
commit 3a869c760e
8 changed files with 456 additions and 13 deletions

View File

@@ -0,0 +1,82 @@
class PendingDuplicateMergesController < ApplicationController
before_action :set_transaction
def new
@limit = 10
# Ensure offset is non-negative to prevent abuse
@offset = [ (params[:offset] || 0).to_i, 0 ].max
# Fetch one extra to determine if there are more results
candidates = @transaction.pending_duplicate_candidates(limit: @limit + 1, offset: @offset).to_a
@has_more = candidates.size > @limit
@potential_duplicates = candidates.first(@limit)
# Calculate range for display (e.g., "1-10", "11-20")
@range_start = @offset + 1
@range_end = @offset + @potential_duplicates.count
end
def create
# Manually merge the pending transaction with the selected posted transaction
unless merge_params[:posted_entry_id].present?
redirect_back_or_to transactions_path, alert: "Please select a posted transaction to merge with"
return
end
# Validate the posted entry is an eligible candidate (same account, currency, not pending)
posted_entry = find_eligible_posted_entry(merge_params[:posted_entry_id])
unless posted_entry
redirect_back_or_to transactions_path, alert: "Invalid transaction selected for merge"
return
end
# Store the merge suggestion and immediately execute it
@transaction.update!(
extra: (@transaction.extra || {}).merge(
"potential_posted_match" => {
"entry_id" => posted_entry.id,
"reason" => "manual_match",
"posted_amount" => posted_entry.amount.to_s,
"confidence" => "high", # Manual matches are high confidence
"detected_at" => Date.current.to_s
}
)
)
# Immediately merge
if @transaction.merge_with_duplicate!
redirect_back_or_to transactions_path, notice: "Pending transaction merged with posted transaction"
else
redirect_back_or_to transactions_path, alert: "Could not merge transactions"
end
end
private
def set_transaction
entry = Current.family.entries.find(params[:transaction_id])
@transaction = entry.entryable
unless @transaction.is_a?(Transaction) && @transaction.pending?
redirect_to transactions_path, alert: "This feature is only available for pending transactions"
end
end
def find_eligible_posted_entry(entry_id)
# Constrain to same account, currency, and ensure it's a posted transaction
# Use the same logic as pending_duplicate_candidates to ensure consistency
conditions = Transaction::PENDING_PROVIDERS.map { |provider| "(transactions.extra -> '#{provider}' ->> 'pending')::boolean IS NOT TRUE" }
@transaction.entry.account.entries
.joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'")
.where(id: entry_id)
.where(currency: @transaction.entry.currency)
.where.not(id: @transaction.entry.id)
.where(conditions.join(" AND "))
.first
end
def merge_params
params.require(:pending_duplicate_merges).permit(:posted_entry_id)
end
end

View File

@@ -38,22 +38,19 @@ class Transaction < ApplicationRecord
# Internal movement labels that should be excluded from budget (auto cash management)
INTERNAL_MOVEMENT_LABELS = [ "Transfer", "Sweep In", "Sweep Out", "Exchange" ].freeze
# Providers that support pending transaction flags
PENDING_PROVIDERS = %w[simplefin plaid lunchflow].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, -> {
where(<<~SQL.squish)
(transactions.extra -> 'simplefin' ->> 'pending')::boolean = true
OR (transactions.extra -> 'plaid' ->> 'pending')::boolean = true
OR (transactions.extra -> 'lunchflow' ->> 'pending')::boolean = true
SQL
conditions = PENDING_PROVIDERS.map { |provider| "(transactions.extra -> '#{provider}' ->> 'pending')::boolean = true" }
where(conditions.join(" OR "))
}
scope :excluding_pending, -> {
where(<<~SQL.squish)
(transactions.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true
AND (transactions.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true
AND (transactions.extra -> 'lunchflow' ->> 'pending')::boolean IS DISTINCT FROM true
SQL
conditions = PENDING_PROVIDERS.map { |provider| "(transactions.extra -> '#{provider}' ->> 'pending')::boolean IS DISTINCT FROM true" }
where(conditions.join(" AND "))
}
# Family-scoped query for Enrichable#clear_ai_cache
@@ -78,9 +75,9 @@ class Transaction < ApplicationRecord
def pending?
extra_data = extra.is_a?(Hash) ? extra : {}
ActiveModel::Type::Boolean.new.cast(extra_data.dig("simplefin", "pending")) ||
ActiveModel::Type::Boolean.new.cast(extra_data.dig("plaid", "pending")) ||
ActiveModel::Type::Boolean.new.cast(extra_data.dig("lunchflow", "pending"))
PENDING_PROVIDERS.any? do |provider|
ActiveModel::Type::Boolean.new.cast(extra_data.dig(provider, "pending"))
end
rescue
false
end
@@ -157,6 +154,28 @@ class Transaction < ApplicationRecord
true
end
# Find potential posted transactions that might be duplicates of this pending transaction
# Returns entries (not transactions) for UI consistency with transfer matcher
# Lists recent posted transactions from the same account for manual merging
def pending_duplicate_candidates(limit: 20, offset: 0)
return Entry.none unless pending? && entry.present?
account = entry.account
currency = entry.currency
# Find recent posted transactions from the same account
conditions = PENDING_PROVIDERS.map { |provider| "(transactions.extra -> '#{provider}' ->> 'pending')::boolean IS NOT TRUE" }
account.entries
.joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'")
.where.not(id: entry.id)
.where(currency: currency)
.where(conditions.join(" AND "))
.order(date: :desc, created_at: :desc)
.limit(limit)
.offset(offset)
end
private
def potential_posted_match_data

View File

@@ -0,0 +1,96 @@
<%= render DS::Dialog.new do |dialog| %>
<% dialog.with_header(title: t(".title")) %>
<% dialog.with_body do %>
<div class="space-y-6">
<div class="bg-warning/10 border border-warning/20 rounded-lg p-4">
<div class="flex items-start gap-3">
<%= icon "alert-triangle", size: "md", class: "text-warning flex-shrink-0 mt-0.5" %>
<div class="text-sm space-y-2">
<p class="text-primary font-medium"><%= t(".warning_title") %></p>
<p class="text-secondary"><%= t(".warning_description") %></p>
</div>
</div>
</div>
<%= styled_form_with(
url: transaction_pending_duplicate_merges_path(@transaction.entry),
scope: :pending_duplicate_merges,
class: "space-y-6",
data: { turbo_frame: :_top }
) do |f| %>
<!-- Current Pending Transaction -->
<section class="space-y-3">
<h2 class="inline-flex items-center gap-1 ml-2 text-sm font-medium text-primary"><%= icon "clock", size: "sm" %> <%= t(".pending_transaction") %></h2>
<div class="bg-surface-inset border border-secondary rounded-lg p-4">
<div class="flex items-center justify-between">
<div class="space-y-1">
<p class="text-primary font-medium"><%= @transaction.entry.name %></p>
<p class="text-xs text-secondary">
<%= @transaction.entry.account.name %>
• <%= I18n.l(@transaction.entry.date, format: :long) %>
• <%= number_to_currency(@transaction.entry.amount.abs, unit: Money::Currency.new(@transaction.entry.currency).symbol) %>
</p>
</div>
</div>
</div>
</section>
<div class="flex justify-center">
<%= icon "arrow-down", class: "text-secondary" %>
</div>
<!-- Select Posted Transaction -->
<section class="space-y-3">
<h2 class="text-sm font-medium text-primary"><%= t(".select_posted") %></h2>
<%= turbo_frame_tag "posted_transaction_candidates" do %>
<% if @potential_duplicates.any? %>
<div class="space-y-2 max-h-96 overflow-y-auto">
<% @potential_duplicates.each do |entry| %>
<label class="flex items-center gap-3 p-3 border border-secondary rounded-lg cursor-pointer hover:bg-surface-inset transition-colors">
<%= f.radio_button :posted_entry_id, entry.id, class: "form-radio" %>
<div class="flex-1 text-sm">
<div class="font-medium text-primary"><%= entry.name %></div>
<div class="text-xs text-secondary">
<%= I18n.l(entry.date, format: :short) %> •
<%= number_to_currency(entry.amount.abs, unit: Money::Currency.new(entry.currency).symbol) %>
</div>
</div>
</label>
<% end %>
</div>
<div class="flex items-center justify-between mt-3">
<p class="text-xs text-secondary">
<%= t(".showing_range", start: @range_start, end: @range_end) %>
</p>
<div class="flex gap-2">
<% if @offset > 0 %>
<%= link_to t(".previous"),
new_transaction_pending_duplicate_merges_path(@transaction.entry, offset: [@offset - 10, 0].max),
class: "text-xs text-link hover:underline",
data: { turbo_frame: "posted_transaction_candidates" } %>
<% end %>
<% if @has_more %>
<%= link_to t(".next"),
new_transaction_pending_duplicate_merges_path(@transaction.entry, offset: @offset + 10),
class: "text-xs text-link hover:underline",
data: { turbo_frame: "posted_transaction_candidates" } %>
<% end %>
</div>
</div>
<% else %>
<div class="bg-surface-inset border border-secondary rounded-lg p-4 text-center">
<p class="text-secondary"><%= t(".no_candidates") %></p>
</div>
<% end %>
<% end %>
</section>
<% if @potential_duplicates.any? %>
<%= f.submit t(".submit_button"), class: "w-full" %>
<% end %>
<% end %>
</div>
<% end %>
<% end %>

View File

@@ -241,6 +241,22 @@
frame: :modal
) %>
</div>
<!-- Pending Duplicate Merger -->
<% if @entry.entryable.is_a?(Transaction) && @entry.entryable.pending? %>
<div class="flex items-center justify-between gap-4 p-3">
<div class="text-sm space-y-1">
<h4 class="text-primary"><%= t("transactions.show.pending_duplicate_merger_title") %></h4>
<p class="text-secondary"><%= t("transactions.show.pending_duplicate_merger_description") %></p>
</div>
<%= render DS::Link.new(
text: t("transactions.show.pending_duplicate_merger_button"),
icon: "merge",
variant: "outline",
href: new_transaction_pending_duplicate_merges_path(@entry),
frame: :modal
) %>
</div>
<% end %>
<!-- Convert to Trade (Investment Accounts Only, not already converted) -->
<% if @entry.account.investment? && @entry.entryable.is_a?(Transaction) && !@entry.excluded? %>
<div class="flex items-center justify-between gap-2 p-3">

View File

@@ -0,0 +1,14 @@
---
en:
pending_duplicate_merges:
new:
title: Merge with Posted Transaction
warning_title: Manual Duplicate Merging
warning_description: Use this to manually merge a pending transaction with its posted version. This will delete the pending transaction and keep only the posted one.
pending_transaction: Pending Transaction
select_posted: Select Posted Transaction to Merge With
showing_range: "Showing %{start} - %{end}"
previous: "← Previous 10"
next: "Next 10 →"
no_candidates: No posted transactions found in this account.
submit_button: Merge Transactions

View File

@@ -40,6 +40,9 @@ en:
convert_to_trade_title: Convert to Security Trade
convert_to_trade_description: Convert this transaction into a Buy or Sell trade with security details for portfolio tracking.
convert_to_trade_button: Convert to Trade
pending_duplicate_merger_title: Duplicate of Posted Transaction?
pending_duplicate_merger_description: Manually merge this pending transaction with its posted version.
pending_duplicate_merger_button: Open merger
merchant_label: Merchant
name_label: Name
nature: Type
@@ -88,6 +91,13 @@ en:
dismiss_duplicate:
success: Kept as separate transactions
failure: Could not dismiss duplicate suggestion
pending_duplicate_merge:
possible_duplicate: Duplicate?
possible_duplicate_short: Dup?
review_recommended: Review
review_recommended_short: Rev
confirm_title: "Merge with posted transaction (%{posted_amount})"
reject_title: Keep as separate transactions
header:
edit_categories: Edit categories
edit_imports: Edit imports

View File

@@ -270,6 +270,7 @@ Rails.application.routes.draw do
resources :transactions, only: %i[index new create show update destroy] do
resource :transfer_match, only: %i[new create]
resource :pending_duplicate_merges, only: %i[new create]
resource :category, only: :update, controller: :transaction_categories
collection do

View File

@@ -0,0 +1,205 @@
require "test_helper"
class PendingDuplicateMergesControllerTest < ActionDispatch::IntegrationTest
include EntriesTestHelper
setup do
sign_in @user = users(:family_admin)
@account = accounts(:depository)
end
test "new displays pending transaction and candidate posted transactions" do
pending_transaction = create_pending_transaction(amount: -50, account: @account)
posted_transaction = create_transaction(amount: -50, account: @account)
get new_transaction_pending_duplicate_merges_path(pending_transaction)
assert_response :success
end
test "new redirects if transaction is not pending" do
posted_transaction = create_transaction(amount: -50, account: @account)
get new_transaction_pending_duplicate_merges_path(posted_transaction)
assert_redirected_to transactions_path
assert_equal "This feature is only available for pending transactions", flash[:alert]
end
test "create merges pending transaction with selected posted transaction" do
pending_transaction = create_pending_transaction(amount: -50, account: @account)
posted_transaction = create_transaction(amount: -50, account: @account)
assert_difference "Entry.count", -1 do
post transaction_pending_duplicate_merges_path(pending_transaction), params: {
pending_duplicate_merges: {
posted_entry_id: posted_transaction.id
}
}
end
assert_redirected_to transactions_path
assert_equal "Pending transaction merged with posted transaction", flash[:notice]
assert_nil Entry.find_by(id: pending_transaction.id), "Pending entry should be deleted after merge"
end
test "create redirects back to referer after successful merge" do
pending_transaction = create_pending_transaction(amount: -50, account: @account)
posted_transaction = create_transaction(amount: -50, account: @account)
assert_difference "Entry.count", -1 do
post transaction_pending_duplicate_merges_path(pending_transaction),
params: {
pending_duplicate_merges: {
posted_entry_id: posted_transaction.id
}
},
headers: { "HTTP_REFERER" => account_path(@account) }
end
assert_redirected_to account_path(@account)
assert_equal "Pending transaction merged with posted transaction", flash[:notice]
end
test "create redirects with error if no posted entry selected" do
pending_transaction = create_pending_transaction(amount: -50, account: @account)
post transaction_pending_duplicate_merges_path(pending_transaction), params: {
pending_duplicate_merges: {
posted_entry_id: ""
}
}
assert_redirected_to transactions_path
assert_equal "Please select a posted transaction to merge with", flash[:alert]
end
test "create stores potential_posted_match metadata before merging" do
pending_transaction = create_pending_transaction(amount: -50, account: @account)
posted_transaction = create_transaction(amount: -50, account: @account)
# Stub merge to prevent deletion so we can check metadata
Transaction.any_instance.stubs(:merge_with_duplicate!).returns(true)
post transaction_pending_duplicate_merges_path(pending_transaction), params: {
pending_duplicate_merges: {
posted_entry_id: posted_transaction.id
}
}
pending_transaction.reload
metadata = pending_transaction.entryable.extra["potential_posted_match"]
assert_not_nil metadata
assert_equal posted_transaction.id, metadata["entry_id"]
assert_equal "manual_match", metadata["reason"]
assert_equal posted_transaction.amount.to_s, metadata["posted_amount"]
assert_equal "high", metadata["confidence"]
assert_equal Date.current.to_s, metadata["detected_at"]
end
test "pending_duplicate_candidates excludes pending transactions" do
pending_transaction = create_pending_transaction(amount: -50, account: @account)
posted_transaction = create_transaction(amount: -50, account: @account)
another_pending = create_pending_transaction(amount: -40, account: @account)
candidates = pending_transaction.entryable.pending_duplicate_candidates
assert_includes candidates.map(&:id), posted_transaction.id
assert_not_includes candidates.map(&:id), another_pending.id
end
test "pending_duplicate_candidates only shows same account and currency" do
pending_transaction = create_pending_transaction(amount: -50, account: @account, currency: "USD")
same_account_transaction = create_transaction(amount: -50, account: @account, currency: "USD")
different_account_transaction = create_transaction(amount: -50, account: accounts(:investment), currency: "USD")
different_currency_transaction = create_transaction(amount: -50, account: @account, currency: "EUR")
candidates = pending_transaction.entryable.pending_duplicate_candidates
assert_includes candidates.map(&:id), same_account_transaction.id
assert_not_includes candidates.map(&:id), different_account_transaction.id
assert_not_includes candidates.map(&:id), different_currency_transaction.id
end
test "create rejects merge with pending transaction" do
pending_transaction = create_pending_transaction(amount: -50, account: @account)
another_pending = create_pending_transaction(amount: -50, account: @account)
assert_no_difference "Entry.count" do
post transaction_pending_duplicate_merges_path(pending_transaction), params: {
pending_duplicate_merges: {
posted_entry_id: another_pending.id
}
}
end
assert_redirected_to transactions_path
assert_equal "Invalid transaction selected for merge", flash[:alert]
end
test "create rejects merge with transaction from different account" do
pending_transaction = create_pending_transaction(amount: -50, account: @account)
different_account_transaction = create_transaction(amount: -50, account: accounts(:investment))
assert_no_difference "Entry.count" do
post transaction_pending_duplicate_merges_path(pending_transaction), params: {
pending_duplicate_merges: {
posted_entry_id: different_account_transaction.id
}
}
end
assert_redirected_to transactions_path
assert_equal "Invalid transaction selected for merge", flash[:alert]
end
test "create rejects merge with transaction in different currency" do
pending_transaction = create_pending_transaction(amount: -50, account: @account, currency: "USD")
different_currency_transaction = create_transaction(amount: -50, account: @account, currency: "EUR")
assert_no_difference "Entry.count" do
post transaction_pending_duplicate_merges_path(pending_transaction), params: {
pending_duplicate_merges: {
posted_entry_id: different_currency_transaction.id
}
}
end
assert_redirected_to transactions_path
assert_equal "Invalid transaction selected for merge", flash[:alert]
end
test "create rejects merge with invalid entry id" do
pending_transaction = create_pending_transaction(amount: -50, account: @account)
assert_no_difference "Entry.count" do
post transaction_pending_duplicate_merges_path(pending_transaction), params: {
pending_duplicate_merges: {
posted_entry_id: 999999
}
}
end
assert_redirected_to transactions_path
assert_equal "Invalid transaction selected for merge", flash[:alert]
end
private
def create_pending_transaction(attributes = {})
# Create a transaction with pending metadata
transaction = create_transaction(attributes)
# Mark it as pending by adding extra metadata
transaction.entryable.update!(
extra: {
"simplefin" => {
"pending" => true
}
}
)
transaction
end
end