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

Extend invoice creation with json deserialization #588

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

phermsdorf
Copy link
Contributor

This pull request add JSON serialization and deserialization support for the following members of the Item class:

  • BuyerOrderReferencedDocumentLineID
  • Allowances
  • Charges

I extended the DeSerializationTest to verify the changes and show the usage.

@jstaerk
Copy link
Collaborator

jstaerk commented Dec 3, 2024

not sure about BuyerOrderReferencedDocumentLineID but I already did charges/allowances (sorry), could you please check and resolve? thanks and apologies.

@phermsdorf
Copy link
Contributor Author

Hi @jstaerk ,

i have seen you did charges and allowances for the invoice, my change/extension is for the item.

I could revert the name change of addReferencedLineID to setBuyerOrderReferencedDocumentLineID and just add the set method if that would be better.

Should i update my pull request to the current code with that changes in place?

Thansk, Bye Peter

@jstaerk
Copy link
Collaborator

jstaerk commented Jan 6, 2025

No matter what you do we need to at least stay compatible to the old public methods (and properties) or this has to wait for the next major version (maybe in a couple of years). e.g. public Item addReferencedLineID vs
setBuyerOrderReferencedDocumentLineID(String s) but also regards charges vs ItemCharges: actually I never tested that you have a chance that it never worked so far in which case you could of course decide for a name

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