Conversation
…gating twice The :62F: closing balance was keyed by its own date, so a bank or proxy that dates it with the next booking day (seen with the FinTS Connect proxy for PayPal) lost it entirely and Statement::getEndBalance() returned null. Attach it to the statement opened by the preceding :60F:/:60M: instead, and also accept a :62M: intermediate balance. The parser already negates a debit closing balance and StatementOfAccount negated it again, so an overdrawn account reported a positive end balance. Derive the sign from credit_debit alone, which also keeps the (unsigned) CAMT input correct.
|
@nemiah PR ist ready für's Review |
|
It probably won't break something and doesn't look too verbose for me. I'll merge it if your human takes responsibility |
Philipp91
left a comment
Philipp91
left a comment
There was a problem hiding this comment.
How was the problem fixed here originally identified?
| } | ||
| if (isset($statement['end_balance'])) { | ||
| $statementModel->setEndBalance((float) $statement['end_balance']['amount'] * ($statement['end_balance']['credit_debit'] == MT940::CD_CREDIT ? 1 : -1)); | ||
| // The MT940 parser already negates a debit closing balance, the CAMT parser does not, so derive |
There was a problem hiding this comment.
Makes me wonder whether we should instead adjust the parsers to be consistent.
There was a problem hiding this comment.
Done in 3749e89: the parser keeps the closing balance unsigned like the start balance and the CAMT balances, and this line is back to applying the sign once.
…he parsed array The start balance and both CAMT balances are stored unsigned with a separate credit_debit flag; only the MT940 closing balance was negated in the parser (turning the string into a float on the way), which is why the model had to guard against negating it twice. Drop the negation in the parser and let StatementOfAccount::fromMT940Array() apply the direction once, as it always did. getParsedMT940() now reports debit closing balances unsigned as well.
|
Fair point – the inconsistency sits in the MT940 parser itself: its own On how it was found: both in production, July 2026. The |
|
Thanks! You or Nena will probably want to squash those commits, so that StatementOfAccount.php isn't touched at all. |
| $result[$soaDate]['end_balance']['credit_debit'] = static::CD_CREDIT; | ||
| } elseif ($cdMark == 'D') { | ||
| $result[$soaDate]['end_balance']['credit_debit'] = static::CD_DEBIT; | ||
| $amount *= -1; |
There was a problem hiding this comment.
Just for the record, as you've said, removing this amount inversion makes things consistent with line 55 above, which is nice.
3 participants
Problem
Two independent defects around the MT940 closing balance (both also listed in #588):
:62F:was keyed by its own date ($soaDate = getDate(substr(...))) and only storedif (isset($result[$soaDate])). A bank or proxy that dates the closing balance with the next booking day — observed with the FinTS Connect proxy that fronts PayPal (BLZ 92020000) — loses it entirely, andStatement::getEndBalance()returnsnull.:62M:was not parsed at all.$amount *= -1, turning the string into a float on the way) andStatementOfAccount::fromMT940Array()negated it again (* -1forCD_DEBIT, added in eebafc7), so an overdrawn account reported a positive end balance. The start balance and both CAMT balances are stored unsigned with a separatecredit_debitflag; the MT940 closing balance was the only exception.Change
:62F:/:62M:to the statement opened by the preceding:60F:/:60M:regardless of the date it carries; a later:62F:overwrites a:62M:.-that sticks to the last field is stripped explicitly instead of blindly dropping the last character.end_balancefollows the same['amount' => unsigned, 'credit_debit' => C|D]shape asstart_balanceand the CAMT output.fromMT940Array()applies the direction once, as it always did. ConsequentlygetParsedMT940()now reports debit closing balances unsigned (and as a string, like the credit case).How it was found
Both in production, July 2026: the
:62F:date issue at the PayPal account behind the FinTS Connect proxy (getEndBalance()wasnull, the application fell back to the opening balance and showed −2.27 € instead of 12,944.64 €), the double negation in a second application when deriving balances from statements (overdrawn accounts came back positive). Both were worked around in-app first; this PR upstreams the fixes.Tests
Tests/Unit/MT940/MT940ClosingBalanceTest.php: different-date:62F:stays with its statement,:62M:is used, debit closing balance is unsigned in the parsed array and negative through the model, no:62F:→ no end balance.Note on authorship
Developed with AI assistance (Cursor) and reviewed by the submitting human; the vouching statement per #586 will be added by the author once the review is complete.