Skip to content
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

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

Merged
merged 2 commits into from
Jul 22, 2024
Merged

Conversation

fellmann
Copy link
Contributor

@fellmann fellmann commented Jul 5, 2024

Closes #66 and #67

  • BREAKING: remove support for some outdated formats
  • Add support for pain.001.001.08 and pain.001.001.09 formats for transfer transactions
  • Add tests for schema validation of the resulting output
  • Fix non-conformant serialization of debitor id
  • Fix broken validation of mandate ID for transfers

@kewisch
Copy link
Owner

kewisch commented Jul 5, 2024

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
Copy link
Contributor Author

fellmann commented Jul 5, 2024

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
Copy link
Contributor Author

fellmann commented Jul 5, 2024

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.

@fellmann fellmann changed the title add support for pain.001.001.08/09 add support for pain.001.001.09/pain.008.001.08 Jul 5, 2024
@leMaik
Copy link

leMaik commented Jul 5, 2024

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

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. 💸

@fellmann
Copy link
Contributor Author

fellmann commented Jul 6, 2024

@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
Copy link

leMaik commented Jul 6, 2024

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

@kewisch
Copy link
Owner

kewisch commented Jul 6, 2024

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
Copy link
Contributor Author

fellmann commented Jul 6, 2024

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
Copy link
Contributor Author

fellmann commented Jul 6, 2024

seems I forgot to remove the outdated tests

@leMaik
Copy link

leMaik commented Jul 8, 2024

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
Copy link
Owner

kewisch commented Jul 22, 2024

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?

@kewisch kewisch merged commit 3bf4792 into kewisch:main Jul 22, 2024
2 checks passed
@leMaik
Copy link

leMaik commented Jul 22, 2024

@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
Copy link
Owner

kewisch commented Jul 22, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

why do you need to provide a mandat for a money transfer? What is a debtorId?
3 participants