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 TradeTax > ExemptionReasonCode #48

Closed
wants to merge 5 commits into from

Conversation

pableu
Copy link

@pableu pableu commented Apr 21, 2023

No description provided.

@PATROMO
Copy link
Contributor

PATROMO commented Apr 24, 2023

@pableu see code style test.

@pableu
Copy link
Author

pableu commented Apr 25, 2023

@PATROMO fixed, thanks :)

@PATROMO
Copy link
Contributor

PATROMO commented Apr 26, 2023

@pableu thanks. Can you extend the Test tests/zugferd211/Tests/BuilderTest.php:169 with your new property?

@pableu
Copy link
Author

pableu commented May 8, 2023

I can, but I'd have to set it to null or also modify tests/zugferd211/Tests/official_example_xml/zugferd_2p1_XRECHNUNG_Einfach.xml.

The former seems a bit silly, and the latter feels wrong because it's an "official example" and I'd feel bad about modifying it.

Also, anything else than null won't make sense in the context of the sample document because the sample invoice there is including vat, so adding a (Vat)ExemptionReason(Code) other than "null" doesn't make sense.

What do you think? Silly test with null? Or no test?

Or a whole new test? I don't feel confident about adding a completely new test and new test data.

@Lorenzschaef
Copy link
Contributor

@pableu, you can extend the test testBuildXRechnungExtendedExample, which already has more elements than the official example.

guillaume-sainthillier added a commit to silarhi/zugferd-factur-x that referenced this pull request Feb 8, 2024
@BolZer BolZer mentioned this pull request Mar 1, 2024
@BolZer
Copy link
Member

BolZer commented Mar 11, 2024

This issue is done with #60. Gonna close this PR.

@BolZer BolZer closed this Mar 11, 2024
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.

4 participants