fix(enable-banking): use real merchant instead of POS terminal line for name (#2968)

* fix(enable-banking): use real merchant instead of POS terminal line for name Some ASPSPs (e.g. BankDirekt/Raiffeisen in Austria) return remittance_information as a multi-element array where the first line is a generic card terminal descriptor (\"POS 45,13 AT D6 31.07. 10:27\") and a later line holds the real merchant. EnableBankingEntry::Processor always used the first array element, so transaction names showed the terminal string instead of the merchant. primary_remittance_information now skips lines that look like a technical terminal booking (POS/ATM + amount, or a trailing date+time stamp) and prefers the first descriptive line, falling back to the original element when nothing better is available. It also strips known small-merchant payment-processor prefixes (SumUp, Square, iZettle, PayPal) from the selected line. Fixes #2935 Disclosure: this fix was written by Claude Code, verified against the reporter's real (decrypted) Enable Banking payload and against test-stack Rails test / RuboCop / Brakeman runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(enable-banking): require both technical-line signals together Address CodeRabbit/Codex review feedback on #2968: the POS/ATM+amount prefix and the trailing date+time suffix were OR'd, so either alone could misclassify a legitimate line as technical. A line embedding the merchant right after the amount (e.g. "POS 45,13 BILLA DANKT ...") or a legitimate descriptor that happens to end in a timestamp (e.g. "Invoice paid 31.07. 10:27") would have been wrongly skipped. Both signals are now required together in a single anchored pattern, matching every real technical line observed in production while no longer misclassifying either of the scenarios above. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(enable-banking): tighten date-stamp regex and clean up the merchant line 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> * fix(enable-banking): address CodeRabbit/Codex review on merchant-line cleanup Follow-up to 5e39ee6b, in response to automated review on that push: - CodeRabbit (functional correctness): remittance cleanup was order-dependent -- strip_payment_processor_prefix ran before strip_loyalty_marker, so a marker preceding the (start-anchored) processor-prefix pattern would leave the prefix unstripped. Swapped the order (marker removal first) so the result no longer depends on which came first in the input. Also switched strip_loyalty_marker from sub to gsub so it removes every marker occurrence, not just the first. - CodeRabbit + Codex (performance, independently flagged by both): Family# known_merchant_names was re-queried for every transaction in a sync batch, since EnableBankingAccount::Transactions::Processor creates a new EnableBankingEntry::Processor per row. Added an optional known_merchant_names: keyword to the processor's constructor (same pattern already used for the shared import_adapter) and compute it once per batch in the caller instead of once per row. - CodeRabbit (test quality, nitpick): the known_merchant_names test only used distinct names, so `assert_equal names.uniq, names` couldn't actually catch a deduplication bug, and never exercised the documented recently-unlinked-merchants exclusion. Rewrote it to create a genuine duplicate name across two different merchant records and to assign-then- unlink a merchant, asserting it's excluded from known_merchant_names while still present in available_merchants (the deliberate difference between the two methods). Re-verified test/models/enable_banking_entry/processor_test.rb + test/models/family_test.rb (67 runs, 170 assertions, 0 failures) and RuboCop on all changed files on a test-stack instance. Disclosure: this fix was written by Claude Code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(enable-banking): drop DANKT/DANKE loyalty-marker stripping Country-specific fallback heuristic flagged in review (only recognized German "thank you" markers). The core fix (skipping the technical POS/ ATM line and matching against the family's known merchant list) is already market-independent; this text heuristic only helped cosmetically for the first transaction from a not-yet-known German/Austrian merchant. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-08 16:14:23 +00:00 · 2026-08-22 23:52:35 +02:00
parent 0d0003b032
commit 0252787dc0
5 changed files with 354 additions and 6 deletions
@@ -3,6 +3,14 @@ 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
+
+  # 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:
  # {
@@ -34,10 +42,16 @@ class EnableBankingEntry::Processor
    "enable_banking_content_#{Digest::MD5.hexdigest(content)}"
  end

-  def initialize(enable_banking_transaction, enable_banking_account:, import_adapter: nil)
+  # known_merchant_names: optional pre-fetched Family#known_merchant_names, so a
+  # caller processing many transactions in one batch (see
+  # EnableBankingAccount::Transactions::Processor) can compute it once instead of
+  # once per row -- same pattern as the shared import_adapter. Falls back to
+  # fetching it lazily per-instance when not provided (e.g. in isolation/tests).
+  def initialize(enable_banking_transaction, enable_banking_account:, import_adapter: nil, known_merchant_names: nil)
    @enable_banking_transaction = enable_banking_transaction
    @enable_banking_account = enable_banking_account
    @import_adapter = import_adapter
+    @known_merchant_names = known_merchant_names
  end

  def process
@@ -219,19 +233,78 @@ class EnableBankingEntry::Processor
    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_payment_processor_prefix(descriptive)
+    end
+
+    def remittance_information_lines
      remittance = data[:remittance_information]
      Array.wrap(remittance)
        .map { |value| value.to_s.strip.presence }
        .compact
-        .first
+    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
+
+    # 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, loyalty 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)
-      # For technical CARD-* counterparties, reuse remittance as the best merchant candidate
+
      remittance = primary_remittance_information
-      return remittance.truncate(100, omission: "") if remittance.present? && technical_card_counterparty?(counterparty)
+      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