Repository navigation
MT940: keep :62F:/:62M: with its statement and stop negating debit closing balances twice #593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| <?php | ||
|
|
||
| namespace Fhp\Tests\Unit\MT940; | ||
|
|
||
| use Fhp\Model\StatementOfAccount\StatementOfAccount; | ||
| use Fhp\MT940\MT940; | ||
| use PHPUnit\Framework\TestCase; | ||
|
|
||
| class MT940ClosingBalanceTest extends TestCase | ||
| { | ||
| /** | ||
| * The closing balance of the first statement carries the next day's date, as e.g. the FinTS Connect proxy that | ||
| * fronts PayPal sends it. It used to be dropped because the parser keyed it by its own date. | ||
| */ | ||
| public function testClosingBalanceWithDifferentDateStaysWithItsStatement(): void | ||
| { | ||
| $parsed = (new MT940())->parse(self::statement(':60F:D260717EUR2,27', ':62F:C260718EUR1234,56')); | ||
|
|
||
| $this->assertSame('1234.56', $parsed['2026-07-17']['end_balance']['amount']); | ||
| $this->assertSame(MT940::CD_CREDIT, $parsed['2026-07-17']['end_balance']['credit_debit']); | ||
| $this->assertSame(1234.56, StatementOfAccount::fromMT940Array($parsed)->getStatements()[0]->getEndBalance()); | ||
| } | ||
|
|
||
| public function testIntermediateClosingBalanceIsUsed(): void | ||
| { | ||
| $parsed = (new MT940())->parse(self::statement(':60F:C260717EUR1234,56', ':62M:C260717EUR1554,64')); | ||
|
|
||
| $this->assertSame('1554.64', $parsed['2026-07-17']['end_balance']['amount']); | ||
| } | ||
|
|
||
| /** | ||
| * The parser negates a debit closing balance and the model used to negate it again, so an overdrawn account | ||
| * reported a positive end balance. | ||
| */ | ||
| public function testDebitClosingBalanceIsNegative(): void | ||
| { | ||
| $parsed = (new MT940())->parse(self::statement(':60F:C260717EUR10,00', ':62F:D260717EUR5000,00')); | ||
|
|
||
| $this->assertSame(MT940::CD_DEBIT, $parsed['2026-07-17']['end_balance']['credit_debit']); | ||
| $this->assertSame(-5000.0, StatementOfAccount::fromMT940Array($parsed)->getStatements()[0]->getEndBalance()); | ||
| } | ||
|
|
||
| public function testStatementWithoutClosingBalanceHasNoEndBalance(): void | ||
| { | ||
| $parsed = (new MT940())->parse(self::statement(':60F:D260717EUR2,27', '')); | ||
|
|
||
| $this->assertArrayNotHasKey('end_balance', $parsed['2026-07-17']); | ||
| $this->assertNull(StatementOfAccount::fromMT940Array($parsed)->getStatements()[0]->getEndBalance()); | ||
| } | ||
|
|
||
| private static function statement(string $openingBalance, string $closingBalance): string | ||
| { | ||
| $crlf = "\r\n"; | ||
|
|
||
| return ':20:STARTUMS' . $crlf | ||
| . ':25:92020000/7210891793' . $crlf | ||
| . ':28C:0' . $crlf | ||
| . $openingBalance . $crlf | ||
| . ':61:2607170717CR320,08N062NONREF' . $crlf | ||
| . ':86:654801?00654801?20PAYMENT?32Max Mustermann' . $crlf | ||
| . ($closingBalance !== '' ? $closingBalance . $crlf : '') | ||
| . '-' . $crlf; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,7 +75,10 @@ public static function fromMT940Array(array $array): StatementOfAccount | |
| $statementModel->setStartBalance((float) $statement['start_balance']['amount']); | ||
| } | ||
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes me wonder whether we should instead adjust the parsers to be consistent.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| // the sign from credit_debit alone instead of negating a possibly negative amount a second time. | ||
| $endBalance = abs((float) $statement['end_balance']['amount']); | ||
| $statementModel->setEndBalance($statement['end_balance']['credit_debit'] == MT940::CD_CREDIT ? $endBalance : -$endBalance); | ||
| } | ||
| if (isset($statement['start_balance']['credit_debit'])) { | ||
| $statementModel->setCreditDebit($statement['start_balance']['credit_debit']); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for the record, as you've said, removing this amount inversion makes things consistent with line 55 above, which is nice.