mirror of
https://github.com/we-promise/sure.git
synced 2026-05-12 23:25:00 +00:00
Merge branch 'main' into feat/surface-provider-status
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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}",
|
||||
|
||||
@@ -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
|
||||
@@ -180,7 +185,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
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -48,27 +51,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 +144,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 +164,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 +207,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 +245,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
|
||||
}
|
||||
)
|
||||
)
|
||||
@@ -370,7 +364,7 @@ class Entry < ApplicationRecord
|
||||
|
||||
# Splits this entry into child entries. Marks parent as excluded.
|
||||
#
|
||||
# @param splits [Array<Hash>] array of { name:, amount:, category_id: } hashes
|
||||
# @param splits [Array<Hash>] array of { name:, amount:, category_id:, excluded: } hashes
|
||||
# @return [Array<Entry>] the created child entries
|
||||
def split!(splits)
|
||||
total = splits.sum { |s| s[:amount].to_d }
|
||||
@@ -392,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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<%# locals: (transaction:) %>
|
||||
<%# locals: (transaction:, in_split_group: false) %>
|
||||
|
||||
<%= render DS::Menu.new(variant: "button") do |menu| %>
|
||||
<% menu.with_button(class: "block w-full overflow-hidden") do %>
|
||||
@@ -11,7 +11,7 @@
|
||||
<% end %>
|
||||
|
||||
<% menu.with_custom_content do %>
|
||||
<%= turbo_frame_tag "category_dropdown", src: category_dropdown_path(category_id: transaction.category_id, transaction_id: transaction.id), loading: :lazy do %>
|
||||
<%= turbo_frame_tag "category_dropdown", src: category_dropdown_path(category_id: transaction.category_id, transaction_id: transaction.id, grouped: in_split_group), loading: :lazy do %>
|
||||
<div class="p-6 flex items-center justify-center">
|
||||
<p class="text-sm text-secondary animate-pulse"><%= t(".loading") %></p>
|
||||
</div>
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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 %>
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -19,7 +19,7 @@
|
||||
} %>
|
||||
|
||||
<div class="flex md:hidden items-center gap-1 col-span-2 relative shrink-0">
|
||||
<%= render "transactions/transaction_category", transaction: transaction, variant: "mobile" %>
|
||||
<%= render "transactions/transaction_category", transaction: transaction, variant: "mobile", in_split_group: in_split_group %>
|
||||
<% if transaction.merchant&.logo_url.present? %>
|
||||
<%= image_tag Setting.transform_brand_fetch_url(transaction.merchant.logo_url),
|
||||
class: "w-5 h-5 rounded-full absolute -bottom-1 -right-1 border border-secondary pointer-events-none",
|
||||
@@ -70,7 +70,7 @@
|
||||
<% else %>
|
||||
<%= link_to(
|
||||
entry.name,
|
||||
entry_path(entry),
|
||||
in_split_group ? entry_path(entry, grouped: true) : entry_path(entry),
|
||||
data: {
|
||||
turbo_frame: "drawer",
|
||||
turbo_prefetch: false
|
||||
@@ -163,7 +163,7 @@
|
||||
<%# For investment accounts, show activity label instead of category %>
|
||||
<%= render "investment_activity/quick_edit_badge", entry: entry, entryable: transaction %>
|
||||
<% else %>
|
||||
<%= render "transactions/transaction_category", transaction: transaction, variant: "desktop" %>
|
||||
<%= render "transactions/transaction_category", transaction: transaction, variant: "desktop", in_split_group: in_split_group %>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
<%# locals: (transaction:, variant:) %>
|
||||
<%# locals: (transaction:, variant:, in_split_group: false) %>
|
||||
|
||||
<div id="<%= dom_id(transaction, "category_menu_#{variant}") %>" class="min-w-0 overflow-hidden">
|
||||
<% if transaction.transfer&.categorizable? || transaction.transfer.nil? %>
|
||||
<%= render "categories/menu", transaction: transaction %>
|
||||
<%= render "categories/menu", transaction: transaction, in_split_group: in_split_group %>
|
||||
<% else %>
|
||||
<div class="hidden lg:flex">
|
||||
<%= render "categories/badge", category: transaction.transfer&.payment? ? payment_category : transfer_category %>
|
||||
|
||||
@@ -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 %>
|
||||
|
||||
@@ -54,6 +54,7 @@
|
||||
url: transaction_path(@entry),
|
||||
class: "space-y-2",
|
||||
data: { controller: "auto-submit-form" } do |f| %>
|
||||
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
|
||||
<%= f.text_field :name,
|
||||
label: t(".name_label"),
|
||||
disabled: @entry.split_child? || edit_locked,
|
||||
@@ -95,6 +96,7 @@
|
||||
url: transaction_path(@entry),
|
||||
class: "space-y-2",
|
||||
data: { controller: "auto-submit-form" } do |f| %>
|
||||
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
|
||||
<% unless @entry.transaction.transfer? %>
|
||||
<%= f.select :account,
|
||||
options_for_select(
|
||||
@@ -262,12 +264,13 @@
|
||||
|
||||
<% if can_edit_entry? %>
|
||||
<% dialog.with_section(title: t(".settings")) do %>
|
||||
<% unless @entry.split_parent? || @entry.split_child? %>
|
||||
<% unless @entry.split_parent? %>
|
||||
<div class="pb-4">
|
||||
<%= styled_form_with model: @entry,
|
||||
url: transaction_path(@entry),
|
||||
class: "p-3",
|
||||
data: { controller: "auto-submit-form" } do |f| %>
|
||||
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
|
||||
<div class="flex cursor-pointer items-center gap-4 justify-between">
|
||||
<div class="text-sm space-y-1">
|
||||
<h4 class="text-primary"><%= t(".exclude") %></h4>
|
||||
@@ -284,6 +287,7 @@
|
||||
url: transaction_path(@entry),
|
||||
class: "p-3",
|
||||
data: { controller: "auto-submit-form" } do |f| %>
|
||||
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
|
||||
<%= f.fields_for :entryable do |ef| %>
|
||||
<div class="flex cursor-pointer items-center gap-4 justify-between">
|
||||
<div class="text-sm space-y-1">
|
||||
@@ -309,6 +313,7 @@
|
||||
url: transaction_path(@entry),
|
||||
class: "p-3",
|
||||
data: { controller: "auto-submit-form" } do |f| %>
|
||||
<%= hidden_field_tag :grouped, "true" if params[:grouped] == "true" %>
|
||||
<%= f.fields_for :entryable do |ef| %>
|
||||
<div class="flex cursor-pointer items-center gap-4 justify-between">
|
||||
<div class="text-sm space-y-1">
|
||||
|
||||
@@ -120,6 +120,26 @@ class SplitsControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_response :success
|
||||
end
|
||||
|
||||
test "create with excluded parameter sets child as excluded" do
|
||||
assert_difference "Entry.count", 2 do
|
||||
post transaction_split_path(@entry), params: {
|
||||
split: {
|
||||
splits: [
|
||||
{ name: "Groceries", amount: "-70", category_id: categories(:food_and_drink).id, excluded: "true" },
|
||||
{ name: "Household", amount: "-30", category_id: "", excluded: "false" }
|
||||
]
|
||||
}
|
||||
}
|
||||
end
|
||||
|
||||
assert_redirected_to transactions_url
|
||||
children = @entry.child_entries.order(:amount)
|
||||
# Household has amount 30 (smaller), Groceries has amount 70 (larger)
|
||||
# Household is NOT excluded, Groceries IS excluded
|
||||
refute children.first.excluded?
|
||||
assert children.last.excluded?
|
||||
end
|
||||
|
||||
# Edit action tests
|
||||
test "edit renders with existing children pre-filled" do
|
||||
@entry.split!([
|
||||
@@ -193,6 +213,27 @@ class SplitsControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_equal 2, @entry.reload.child_entries.count
|
||||
end
|
||||
|
||||
test "update with excluded parameter sets child as excluded" do
|
||||
@entry.split!([
|
||||
{ name: "Groceries", amount: 70, category_id: nil },
|
||||
{ name: "Household", amount: 30, category_id: nil }
|
||||
])
|
||||
|
||||
patch transaction_split_path(@entry), params: {
|
||||
split: {
|
||||
splits: [
|
||||
{ name: "Groceries", amount: "-70", category_id: "", excluded: "true" },
|
||||
{ name: "Household", amount: "-30", category_id: "", excluded: "false" }
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
assert_redirected_to transactions_url
|
||||
children = @entry.child_entries.order(:amount)
|
||||
refute children.first.excluded?
|
||||
assert children.last.excluded?
|
||||
end
|
||||
|
||||
# Destroy from child tests
|
||||
test "destroy from child resolves to parent and unsplits" do
|
||||
@entry.split!([
|
||||
|
||||
@@ -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
|
||||
@@ -174,4 +174,51 @@ class EntrySplitTest < ActiveSupport::TestCase
|
||||
assert children.first.split_child?
|
||||
refute @entry.split_child?
|
||||
end
|
||||
|
||||
test "split! creates child entries with excluded: true when specified" do
|
||||
splits = [
|
||||
{ name: "Part 1", amount: 50, category_id: nil, excluded: true },
|
||||
{ name: "Part 2", amount: 50, category_id: nil, excluded: false }
|
||||
]
|
||||
|
||||
children = @entry.split!(splits)
|
||||
|
||||
assert_equal 2, children.size
|
||||
assert children.first.excluded?
|
||||
refute children.last.excluded?
|
||||
end
|
||||
|
||||
test "split! properly casts excluded from string values" do
|
||||
splits = [
|
||||
{ name: "Part 1", amount: 50, category_id: nil, excluded: "true" },
|
||||
{ name: "Part 2", amount: 50, category_id: nil, excluded: "false" }
|
||||
]
|
||||
|
||||
children = @entry.split!(splits)
|
||||
|
||||
assert children.first.excluded?
|
||||
refute children.last.excluded?
|
||||
end
|
||||
|
||||
test "excluded split children are excluded from balance calculations" do
|
||||
@entry.split!([
|
||||
{ name: "Part 1", amount: 50, category_id: nil, excluded: true },
|
||||
{ name: "Part 2", amount: 50, category_id: nil, excluded: false }
|
||||
])
|
||||
|
||||
# Parent is always excluded for splits
|
||||
assert @entry.reload.excluded?
|
||||
|
||||
# Excluded child should be filtered out by where(excluded: false)
|
||||
excluded_child = @entry.child_entries.find { |c| c.name == "Part 1" }
|
||||
non_excluded_child = @entry.child_entries.find { |c| c.name == "Part 2" }
|
||||
|
||||
assert excluded_child.excluded?
|
||||
refute non_excluded_child.excluded?
|
||||
|
||||
# where(excluded: false) should only include the non-excluded child
|
||||
visible_entries = Entry.where(id: @entry.child_entries.map(&:id)).where(excluded: false)
|
||||
assert_includes visible_entries.pluck(:id), non_excluded_child.id
|
||||
refute_includes visible_entries.pluck(:id), excluded_child.id
|
||||
end
|
||||
end
|
||||
|
||||
308
test/models/transaction/merge_with_duplicate_test.rb
Normal file
308
test/models/transaction/merge_with_duplicate_test.rb
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user