mirror of
https://github.com/we-promise/sure.git
synced 2026-09-03 05:41:18 +00:00
Addresses two review threads on this PR:
1. jjmata (PR review): technical_remittance_line?'s date-stamp check required
exactly 2-digit day/month (\d{2}[./]\d{2}), so an un-padded date ("1.07."
instead of "01.07.") wasn't recognized as technical and the line would
resurface as the transaction name -- reproducing the original #2935 bug for
that date shape. Now accepts 1-2 digits for both.
2. john-frandsen (issue #2935 comment): suggested cleaning up the merchant line
further (e.g. "BILLA DANKT 0007114 SIEGENDORF 7011" -> "Billa"). Checked
point 1 (structured remittance fields) against Enable Banking's own API
docs -- no such field exists there, not applicable. Points 2/3/5 already
match current behavior. Point 4 (loyalty-marker cleanup) implemented as two
layers:
- Primary: match the line against merchants the family already knows
(Family#known_merchant_names) -- self-maintaining, no pattern-guessing,
and now also assigns the transaction's merchant when matched (previously
out of scope for blank-counterparty EB transactions). Case-insensitive,
regex-escaped, longest-match-wins, with a minimum length guard against
spurious short-name matches.
- Fallback (no known merchant yet): remove only the "DANKT"/"DANKE"
thank-you marker word itself, not a directional truncation -- the marker
can precede or follow the merchant name depending on phrasing ("X DANKT"
vs. "DANKE ... bei X"), so truncating at it risked deleting the real
merchant name in one of the two phrasings.
Verified against the full test suite, RuboCop, and Brakeman on a test-stack
Rails instance (0 RuboCop offenses, 0 Brakeman warnings; full-suite failures
present on that instance are pre-existing/environmental and unrelated to
these files).
Disclosure: this fix (investigation, implementation, and tests) was written
by Claude Code.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
367 lines
16 KiB
Ruby
367 lines
16 KiB
Ruby
require "digest/md5"
|
|
|
|
class EnableBankingEntry::Processor
|
|
include CurrencyNormalizable
|
|
|
|
# Small-merchant card terminal providers that prefix the payee with "KEYWORD *"
|
|
PAYMENT_PROCESSOR_PREFIX = /\A(SUMUP|SQ|IZETTLE|ZETTLE|PAYPAL)\s*\*\s*/i
|
|
|
|
# Austrian/German retail POS terminals often include a "DANKT"/"DANKE" thank-you
|
|
# marker somewhere in the merchant line (e.g. "<CHAIN> DANKT ..." or
|
|
# "DANKE ... <CHAIN>"). It's never part of the retailer's actual name, so it's
|
|
# safe to remove -- but its position relative to the merchant name isn't fixed,
|
|
# so only the marker word itself is removed rather than truncating the line at
|
|
# it. Used as a last-resort fallback when no known merchant name matches (see
|
|
# matched_known_merchant_name) -- generalizes across retailers and phrasings
|
|
# without risking the real merchant name being cut off.
|
|
LOYALTY_MARKER_WORD = /\b(DANKT|DANKE)\b/i
|
|
|
|
# Guard against spurious matches from very short known merchant names (e.g. a
|
|
# 2-letter FamilyMerchant name matching inside unrelated text, like "IT" would
|
|
# inside "NAME IT" -- a real chain name observed in this issue's own data).
|
|
MIN_KNOWN_MERCHANT_MATCH_LENGTH = 3
|
|
|
|
# enable_banking_transaction is the raw hash fetched from Enable Banking API
|
|
# Transaction structure from Enable Banking:
|
|
# {
|
|
# transaction_id, entry_reference, booking_date, value_date, transaction_date,
|
|
# 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
|
|
return "enable_banking_#{id}" if id
|
|
|
|
# Some ASPSPs omit both transaction_id and entry_reference (both are optional
|
|
# in PSD2). Generate a deterministic content-based ID so these transactions
|
|
# can still be imported idempotently. Uses the same fields as the importer's
|
|
# dedup key so the two strategies stay in sync.
|
|
date = data[:booking_date].presence || data[:value_date].presence || data[:transaction_date]
|
|
amount = data.dig(:transaction_amount, :amount).presence || data[:amount]
|
|
currency = data.dig(:transaction_amount, :currency).presence || data[:currency]
|
|
direction = data[:credit_debit_indicator]
|
|
creditor = data.dig(:creditor, :name).presence || data[:creditor_name]
|
|
debtor = data.dig(:debtor, :name).presence || data[:debtor_name]
|
|
remittance = data[:remittance_information]
|
|
remittance_key = remittance.is_a?(Array) ? remittance.compact.map(&:to_s).sort.join("|") : remittance.to_s
|
|
|
|
content = [ date, amount, currency, direction, creditor, debtor, remittance_key ].map(&:to_s).join("\x1F")
|
|
return nil if content.gsub("\x1F", "").blank?
|
|
|
|
"enable_banking_content_#{Digest::MD5.hexdigest(content)}"
|
|
end
|
|
|
|
def initialize(enable_banking_transaction, enable_banking_account:, import_adapter: nil)
|
|
@enable_banking_transaction = enable_banking_transaction
|
|
@enable_banking_account = enable_banking_account
|
|
@import_adapter = import_adapter
|
|
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 #{safe_id}"
|
|
return nil
|
|
end
|
|
|
|
begin
|
|
import_adapter.import_transaction(
|
|
external_id: external_id,
|
|
amount: amount,
|
|
currency: currency,
|
|
date: date,
|
|
name: name,
|
|
source: "enable_banking",
|
|
merchant: merchant,
|
|
notes: notes,
|
|
extra: extra
|
|
)
|
|
rescue ArgumentError => e
|
|
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 #{safe_id}: #{e.message}"
|
|
raise StandardError.new("Failed to import transaction: #{e.message}")
|
|
rescue => e
|
|
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
|
|
end
|
|
|
|
private
|
|
|
|
attr_reader :enable_banking_transaction, :enable_banking_account
|
|
|
|
def import_adapter
|
|
@import_adapter ||= Account::ProviderImportAdapter.new(account)
|
|
end
|
|
|
|
def account
|
|
@account ||= enable_banking_account.current_account
|
|
end
|
|
|
|
def data
|
|
@data ||= enable_banking_transaction.with_indifferent_access
|
|
end
|
|
|
|
def external_id
|
|
id = self.class.compute_external_id(data)
|
|
raise ArgumentError, "Enable Banking transaction missing required identifier (transaction_id, entry_reference, or identifiable content)" unless id
|
|
id
|
|
end
|
|
|
|
def name
|
|
# Build name from available Enable Banking transaction fields
|
|
# Priority: counterparty name > bank_transaction_code description > remittance_information
|
|
|
|
counterparty = counterparty_name
|
|
return counterparty if counterparty.present? && !technical_card_counterparty?(counterparty)
|
|
|
|
# Some institutions (e.g. Wise) use technical CARD-* identifiers as counterparties
|
|
# Prefer remittance_information first in that case since it contains the real merchant label for Wise
|
|
if technical_card_counterparty?(counterparty)
|
|
remittance = primary_remittance_information
|
|
return remittance.truncate(100) if remittance.present?
|
|
end
|
|
|
|
# Fall back to bank_transaction_code description
|
|
bank_tx_description = data.dig(:bank_transaction_code, :description)
|
|
return bank_tx_description if bank_tx_description.present?
|
|
|
|
# Fall back to remittance_information
|
|
remittance = primary_remittance_information
|
|
return remittance.truncate(100) if remittance.present?
|
|
|
|
# Final fallback: use transaction type indicator
|
|
credit_debit_indicator == "CRDT" ? "Incoming Transfer" : "Outgoing Transfer"
|
|
end
|
|
|
|
def merchant
|
|
# Use the counterparty when it is human readable; otherwise fall back to remittance
|
|
# for CARD-* transactions where the remittance often contains the actual merchant
|
|
merchant_name = merchant_name_candidate
|
|
return nil if merchant_name.blank?
|
|
|
|
merchant_id = Digest::MD5.hexdigest(merchant_name.downcase)
|
|
|
|
@merchant ||= begin
|
|
import_adapter.find_or_create_merchant(
|
|
provider_merchant_id: "enable_banking_merchant_#{merchant_id}",
|
|
name: merchant_name,
|
|
source: "enable_banking"
|
|
)
|
|
rescue ActiveRecord::RecordInvalid => e
|
|
Rails.logger.error "EnableBankingEntry::Processor - Failed to create merchant '#{merchant_name}': #{e.message}"
|
|
nil
|
|
end
|
|
end
|
|
|
|
def notes
|
|
parts = []
|
|
|
|
remittance = data[:remittance_information]
|
|
if remittance.is_a?(Array) && remittance.any?
|
|
parts << remittance.join("\n")
|
|
elsif remittance.is_a?(String) && remittance.present?
|
|
parts << remittance
|
|
end
|
|
|
|
parts << data[:note] if data[:note].present?
|
|
|
|
parts.join("\n\n").presence
|
|
end
|
|
|
|
def extra
|
|
eb = {}
|
|
|
|
if data[:exchange_rate].present?
|
|
eb[:fx_rate] = data.dig(:exchange_rate, :exchange_rate)
|
|
eb[:fx_unit_currency] = data.dig(:exchange_rate, :unit_currency)
|
|
eb[:fx_instructed_amount] = data.dig(:exchange_rate, :instructed_amount, :amount)
|
|
end
|
|
|
|
eb[:merchant_category_code] = data[:merchant_category_code] if data[:merchant_category_code].present?
|
|
eb[:pending] = true if data[:_pending] == true
|
|
|
|
eb.compact!
|
|
eb.empty? ? nil : { enable_banking: eb }
|
|
end
|
|
|
|
def amount_value
|
|
@amount_value ||= begin
|
|
tx_amount = data[:transaction_amount] || {}
|
|
raw_amount = tx_amount[:amount] || data[:amount] || "0"
|
|
|
|
absolute_amount = case raw_amount
|
|
when String
|
|
BigDecimal(raw_amount).abs
|
|
when Numeric
|
|
BigDecimal(raw_amount.to_s).abs
|
|
else
|
|
BigDecimal("0")
|
|
end
|
|
|
|
# Sure convention: positive = outflow (expense/debit from account), negative = inflow (income/credit)
|
|
# Enable Banking: DBIT = debit from account (outflow), CRDT = credit to account (inflow)
|
|
# Therefore: DBIT → +absolute_amount, CRDT → -absolute_amount
|
|
credit_debit_indicator == "CRDT" ? -absolute_amount : absolute_amount
|
|
rescue ArgumentError => e
|
|
Rails.logger.error "Failed to parse Enable Banking transaction amount: #{raw_amount.inspect} - #{e.message}"
|
|
raise
|
|
end
|
|
end
|
|
|
|
def credit_debit_indicator
|
|
data[:credit_debit_indicator]
|
|
end
|
|
|
|
def counterparty_name
|
|
# Determine counterparty based on transaction direction
|
|
# For outgoing payments (DBIT), counterparty is the creditor (who we paid)
|
|
# For incoming payments (CRDT), counterparty is the debtor (who paid us)
|
|
if credit_debit_indicator == "CRDT"
|
|
data.dig(:debtor, :name).presence || data[:debtor_name].presence
|
|
else
|
|
data.dig(:creditor, :name).presence || data[:creditor_name].presence
|
|
end
|
|
end
|
|
|
|
def technical_card_counterparty?(value)
|
|
# Some providers expose card transactions with CARD-<digits> placeholders instead of a real counterparty name
|
|
value.to_s.strip.match?(/\ACARD-\d+\z/i)
|
|
end
|
|
|
|
def primary_remittance_information
|
|
lines = remittance_information_lines
|
|
descriptive = lines.find { |line| !technical_remittance_line?(line) } || lines.first
|
|
return descriptive if descriptive.blank?
|
|
|
|
matched_known_merchant_name(descriptive) || strip_loyalty_marker(strip_payment_processor_prefix(descriptive))
|
|
end
|
|
|
|
def remittance_information_lines
|
|
remittance = data[:remittance_information]
|
|
Array.wrap(remittance)
|
|
.map { |value| value.to_s.strip.presence }
|
|
.compact
|
|
end
|
|
|
|
def technical_remittance_line?(value)
|
|
line = value.to_s.strip
|
|
# Terminal booking line, e.g. "POS 45,13 AT D6 31.07. 10:27": require the
|
|
# keyword+amount prefix AND the trailing date+time stamp TOGETHER, not either
|
|
# alone. Either signal in isolation false-positives on legitimate descriptors:
|
|
# a line like "POS 45,13 BILLA DANKT ..." (merchant appended after the amount,
|
|
# no separate technical-only element) would wrongly match on the prefix alone,
|
|
# and a line like "Invoice paid 31.07. 10:27" would wrongly match on the date
|
|
# suffix alone. Requiring both matches every real technical line observed in
|
|
# production while leaving both of those legitimate shapes untouched. Day/month
|
|
# accept 1-2 digits (not just 2) so an un-padded ASPSP date ("1.07." instead of
|
|
# "01.07.") is still recognized as technical.
|
|
line.match?(/\A(POS|ATM)\s+\d+[.,]\d{2}\b.*\d{1,2}[.\/]\d{1,2}\.?\s+\d{2}:\d{2}\z/i)
|
|
end
|
|
|
|
def strip_payment_processor_prefix(value)
|
|
return value if value.blank?
|
|
value.sub(PAYMENT_PROCESSOR_PREFIX, "").strip.presence || value
|
|
end
|
|
|
|
def strip_loyalty_marker(value)
|
|
return value if value.blank?
|
|
# Only touch the string when the marker is actually present -- squeeze/strip
|
|
# would otherwise also collapse intentional multi-space formatting (e.g. the
|
|
# raw technical POS line) on lines that never had a marker to remove.
|
|
return value unless value.match?(LOYALTY_MARKER_WORD)
|
|
value.sub(LOYALTY_MARKER_WORD, "").squeeze(" ").strip.presence || value
|
|
end
|
|
|
|
# Prefer a merchant name the family already knows over any text heuristic: it's
|
|
# already clean/trusted, and sidesteps guessing which parts of a POS line are
|
|
# noise (store numbers, city, thank-you markers, ...) vs. part of the name.
|
|
# Case-insensitive, whole-word match; the *stored* name (and its casing) wins,
|
|
# so e.g. "BILLA DANKT 0007114" resolves to "Billa", not "BILLA". Longest match
|
|
# wins when multiple known names match (prefer the more specific one).
|
|
def matched_known_merchant_name(line)
|
|
candidates = known_merchant_names.select { |name| name.length >= MIN_KNOWN_MERCHANT_MATCH_LENGTH }
|
|
# Lookaround instead of \b at both ends: \b only fires on a word/non-word
|
|
# transition, so it silently fails to match right after a name that itself
|
|
# ends in punctuation (e.g. "A+B (Café)" ends in ")" -- a non-word char next
|
|
# to another non-word char has no \b between them). Asserting "the boundary
|
|
# character, if any, isn't alphanumeric" works regardless of how the known
|
|
# name itself starts/ends.
|
|
matches = candidates.select do |name|
|
|
line.match?(/(?<![[:alnum:]_])#{Regexp.escape(name)}(?![[:alnum:]_])/i)
|
|
end
|
|
matches.max_by(&:length)
|
|
end
|
|
|
|
def known_merchant_names
|
|
@known_merchant_names ||= account&.family&.known_merchant_names || []
|
|
end
|
|
|
|
def merchant_name_candidate
|
|
counterparty = counterparty_name.to_s.strip
|
|
return counterparty if counterparty.present? && !technical_card_counterparty?(counterparty)
|
|
|
|
remittance = primary_remittance_information
|
|
return nil if remittance.blank?
|
|
|
|
# Trust remittance-derived text as a merchant candidate when either the
|
|
# counterparty was a technical CARD-* placeholder (existing Wise case), or
|
|
# the text matched an ALREADY-KNOWN merchant for this family. Inventing a
|
|
# brand-new merchant from raw noisy POS text (blank counterparty, no CARD-*
|
|
# signal, no known-merchant match) stays out of scope, unchanged from #2935.
|
|
return remittance.truncate(100, omission: "") if technical_card_counterparty?(counterparty)
|
|
return remittance if known_merchant_names.include?(remittance)
|
|
|
|
nil
|
|
end
|
|
|
|
def amount
|
|
# Sure convention: positive = outflow (debit/expense), negative = inflow (credit/income)
|
|
# amount_value already applies this: DBIT → +absolute, CRDT → -absolute
|
|
amount_value
|
|
end
|
|
|
|
def currency
|
|
tx_amount = data[:transaction_amount] || {}
|
|
parse_currency(tx_amount[:currency]) || parse_currency(data[:currency]) || account&.currency || "EUR"
|
|
end
|
|
|
|
def log_invalid_currency(currency_value)
|
|
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
|
|
# Prefer booking_date, fall back to value_date, then transaction_date
|
|
date_value = data[:booking_date] || data[:value_date] || data[:transaction_date]
|
|
|
|
case date_value
|
|
when String
|
|
if date_value.include?("T") || date_value.include?(":")
|
|
Time.parse(date_value).in_time_zone(account&.family&.timezone).to_date
|
|
else
|
|
Date.parse(date_value)
|
|
end
|
|
when Integer, Float
|
|
Time.at(date_value).in_time_zone(account&.family&.timezone).to_date
|
|
when Time, DateTime
|
|
date_value.in_time_zone(account&.family&.timezone).to_date
|
|
when Date
|
|
date_value
|
|
else
|
|
Rails.logger.error("Enable Banking transaction has invalid date value: #{date_value.inspect}")
|
|
raise ArgumentError, "Invalid date format: #{date_value.inspect}"
|
|
end
|
|
rescue ArgumentError, TypeError => e
|
|
Rails.logger.error("Failed to parse Enable Banking transaction date '#{date_value}': #{e.message}")
|
|
raise ArgumentError, "Unable to parse transaction date: #{date_value.inspect}"
|
|
end
|
|
end
|