search

AbcAeffchen / Sephpa

PHP class to create SEPA xml files for credit transfer and direct debit
GNU Lesser General Public License v3.0
71 stars 31 forks source link

$bicRequired is used to set an option 'allowEmptyBic' but shouldn't the value be flipped then? #36

Closed degitpatrick closed 3 years ago

degitpatrick commented 3 years ago

Looking at SepaCreditTransfer00100103::addPayment, a check is done to determine if a BIC code is required. This is done by checking SepaUtilities::isEEATransaction and then flipping the result. So if the 2 IBANs are both EEA accounts, SepaUtilities::isEEATransaction will return true, which will be flipped so $bicRequired becomes false.

Next, we see this call: SepaUtilities::checkAndSanitizeAll( $paymentInfo, $this->sanitizeFlags, ['allowEmptyBic' => $bicRequired] );

This would imply that if $bicRequired is false, allowEmptyBic will also be false, so even though no BIC code is required, no empty BIC code is allowed.

At the moment the only way to work around this would be to remove the 'bic' element from the payment array.

degitpatrick commented 3 years ago

Any news on this?

AbcAeffchen commented 3 years ago

Sorry, I was very busy.

It took me a while to check why this is obviously a bug but the tests are were passing none the less. Turns out that 'empty' was not tested. Just 'not set at all'.

However, I merged #37 and will add a followup commit to extend the tests.