kewisch / sepa.js

Create and validate SEPA XML transactions
http://kewisch.github.io/sepa.js
Other
88 stars 61 forks source link

add support for pain.001.001.09/pain.008.001.08 #68

Closed fellmann closed 3 months ago

fellmann commented 4 months ago

Closes #66 and #67

kewisch commented 4 months ago

Hey, thanks for the pull request and insights! I think if you add xsd-validator to devDependencies we should be good. What is the license on the xsd files, are they compatible with MPL and free to use?

fellmann commented 4 months ago

The xsd files seem to be free to use, see https://www.iso20022.org/terms-use But before merging, let me check if the direct debit files still work

fellmann commented 4 months ago

I have added support and schema validation tests for direct debit. To make it work, I had to remove support for some very old formats. This will make the change breaking.

leMaik commented 4 months ago

Nice to see this implemented, I had to manually patch our pain.008.001.08 xml file before :tada:

However, the generated direct debit file from this PR doesn't validate against the pain.008.001.08_GBIC_4.xsd found here in DK-TVS_SEPA_GBIC_4undISO_Originale.zip (checked using xmllint).
In particular, the Id element in Cdtr is unexpected and needs to be removed. It already appears in CdtrSchmeId, as expected. We successfully submitted the (patched and validating) file to a bank. :money_with_wings:

fellmann commented 4 months ago

@leMaik thanks for validating! I found the ebics specification is more strict than the ISO one. The Cdtr -> Id field is optional in the ISO specification, and disallowed in ebics. I now removed it, as for direct debit it is included in CdtrSchmeId, and for transfers, the debit id does not exist.

leMaik commented 4 months ago

@fellmann Nice! I can confirm that the file now validates against the EBICS schema when generated with this PR.

kewisch commented 4 months ago

Would it make sense to introduce different validation modes where the consumer can pick iso vs ebics and the code generates slightly different xml? Or is that a non-issue with the current state?

fellmann commented 4 months ago

I am no expert, but I believe it is ok as it is. The xml makes sense now, and validates against both schemas. Including an unused field is not desirable in any scenario.

fellmann commented 4 months ago

seems I forgot to remove the outdated tests

leMaik commented 4 months ago

At least our bank seems to validate against the ebics schema and declines files that are valid iso but not valid ebics. So imho this library should aim to support both schemas by omitting fields that are optional in one schema and disallowed in the other.

kewisch commented 3 months ago

I'm going to merge this for now, but wouldn't be opposed to different validation settings and then including the optional field if it is helpful for some scenarios. @leMaik is that maybe something you'd be willing to contribute?

leMaik commented 3 months ago

@kewisch Tbh I don't have a use case for the less strict (ie. invalid according to our bank) schema.

Would it be useful to add validation against the ebics specification, additionally to the ISO one? We could then add a toggle for which target specification the XML should match, if there are any differences in the future. That would also allow to support multiple SEPA versions at the same time (if that's a use-case).

kewisch commented 3 months ago

Yes, this is what I meant, a toggle for the target specification between iso and ebics. If there are use cases for the addtional fields in iso and someone's bank supports iso, having a toggle would be useful.