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

Support credit note type code #36

Merged
merged 5 commits into from
Jan 14, 2023

Conversation

ChristianVermeulen
Copy link
Contributor

When importing a customers administration we noticed all credit notes were imported as invoices. As it turnes out this library does not support credit notes.

This PR makes sure it does.

To be discussed: Can we live with the fact that everything is still called "invoice"? The CreditNoteTypeCode is really the only and single different node from the invoice spec. (personally I wonder why it wasn't just called TypeCode but yeah.

@ChristianVermeulen
Copy link
Contributor Author

I'm not too sure what to do about those CI failures.. The constant is referenced from tests, but I guess that folder is excluded. Also I wonder if the rules are maybe too tight? Maybe a warning should be sufficient for a non-referenced constant? Let me know what you want me to do about it :)

@josemmo
Copy link
Owner

josemmo commented Jan 12, 2023

Thanks for the PR, @ChristianVermeulen!

I've been browsing the specification for a bit and apparently Credit Notes are more complicated that I've thought. Some additional changes will be required, but it's still feasible.

EN 16931 defines BT-3 (Invoice type code) as "A code specifying the functional type of the Invoice". It also specifies that "a Credit Note is similar to an Invoice except that the Invoice Type is set to Credit Note". So far so good.

However, when importing/exporting UBL documents, PEPPOL has two separate profiles (<Invoice /> and <CreditNote />), each with their own XML element for the BT-3 field:

There's also rule PEPPOL-EN16931-P0101, that sets which profile should be used.

With all that in mind, I'll push some changes this week and hopefully have it merged before February.

@ChristianVermeulen
Copy link
Contributor Author

Ah, I didn't notice there was an actual difference in the profile node as well. I guess i then boils down to 2 possibilities:

Invoice:

  • <Invoice /> + <InvoiceTypeCode />

Credit Note

  • <CreditNote /> + <CreditNoteTypeCode />

The test you mentioned seems to just make sure all type codes not equal to 380 (Invoice) adhere to the <CreditNote/> type / profile.

I think the biggest question is if you want to fully support the credit note as profile/type and have a separate type / reader / writer for it, or just have an "$invoice" in general which can be recognised by the typecode to know which of the different types it is.

My personal opinion would be the latter. I don't really see why the spec even has different nodes. A credit note actually is a type of invoice (a negative / correcting one). So I think I would've had only <Invoice /> with <TypeCode /> as node to specify which type of document it is. That's pretty much all there is to a Credit Note anyway :-)

That being said, let me know if there is anything I can do to help!

- Added constants from UNCL1001 subset to Invoice class
- Updated UblWriter class
- Updated unit tests
- Updated UblReader class
- Updated unit tests
- Updated tests
- Created peppol-credit-note.xml example
@josemmo
Copy link
Owner

josemmo commented Jan 14, 2023

I've made some changes to fully support credit notes. Now, developers can set the invoice type code and the UBL profile will be changed seamlessly to use <Invoice /> or <CreditNote /> depending on the type. Same for importing invoices.

Merging to develop! 🎉

@josemmo josemmo merged commit a2fcfb4 into josemmo:develop Jan 14, 2023
@josemmo josemmo mentioned this pull request Jan 21, 2023
@ChristianVermeulen ChristianVermeulen deleted the credit-note-type-code branch January 23, 2023 12:29
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.

2 participants