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

Issues/122 facturx #126

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

Conversation

jstaerk
Copy link

@jstaerk jstaerk commented Sep 9, 2024

This adds the functionality to create Factur-X invoices, create their XML, and add the XML with ghostscript.net into a PDF file to become a valid PDF/A-3 Factur-X/ZUGFeRD file. Some .net files have been converted from mustangproject.org with a Java to c# converter.

This is basically mustang.net https://www.mustangproject.org/net/
Legally, in germany as of 01.01.2028 all domestic sales will have to be invoiced using UBL-XML, CII-XML, or Factur-X (https://www.recht.bund.de/bgbl/1/2024/108/regelungstext.pdf?__blob=publicationFile&v=2 pg 23 art 23 in conjunction with https://web.archive.org/web/20240407134819/https://www.dstv.de/wp-content/uploads/2023/10/BMF_2023-0922192-R.pdf, plain PDF without XML or paper will no longer be allowed).

@stephanstapel
Copy link

Hi @jstaerk ,
thanks for this pull request. This is a huge step forward to create electronic invoices. I'd like to propose to remove the FacturX functionality from Postscript.net and keep it separately and only to add the PDF/A-3 generation functionality to Ghostscript.net (if the folks at Artifex are willing to accept this of course).

Two feedbacks concerning the actual implementation:

  • There are a lot of German comments in the code which imho should be English
  • Instead of throwing a Exception("file not found"), you should rather throw a FileNotFoundException

@jstaerk
Copy link
Author

jstaerk commented Sep 16, 2024

very cool, please proceed

@jamie-lemon
Copy link
Collaborator

Thanks for this PR - great to see support for electronic invoices! Hopefully will get someone to review soon. :)

@jstaerk
Copy link
Author

jstaerk commented Oct 3, 2024

any opinion from anybody on that topic? e.g. @jhabjan ?

@stephanstapel
Copy link

I'm happy to clean up the code asap, I just can't start refactoring because of #127 and have no idea how to find the solution. @jamie-lemon can you help?

@jamie-lemon
Copy link
Collaborator

@stephanstapel I'm not actually a .NET guy, but will see if I can get someone on the Artifex team who knows .NET to look into this. I also just asked for a bit more description about the bug on #127 as it wasn't 100% clear to me.

@jhabjan
Copy link
Contributor

jhabjan commented Oct 4, 2024

any opinion from anybody on that topic? e.g. @jhabjan ?

@jstaerk - Looks good. There are few minor bugs and fixes I mentioned here #127 (comment)

The only addition I would suggest is implementing a check to see if AdobeRGB1998.icc and pdfconvert.ps already exist, so they are not written to disk for every PDF conversion. This could help avoid issues in a multithreaded environment when running multiple instances in parallel. You might also want to append the Ghostscript.NET release version to the filenames, such as pdfconvert.gs.net.1.2.3.ps, to account for any changes in future releases.

I would agree with @stephanstapel, everything that's on German should be either removed or translated to English

@stephanstapel
Copy link

I ripped out LKVM dependency already, just have no idea where to push the changes :)
Next step is to rip out the postscript template from the code and make this a resource.
I also propose to remove the ZUGFeRD generation/ parsing as this will not have a chance zu survive there in the long run with invoicing code buried deep in a ps/pdf library.

Could we move the code into dedicated assembly, called e.g. Ghostscript.Net.PDFConverter? What do you think, @jhabjan ?

@jamie-lemon
Copy link
Collaborator

@stephanstapel Please let us know when you consider this PR "Ready for review". Thanks!

@jamie-lemon
Copy link
Collaborator

@stephanstapel If you put @green0317 and myself as reviewers that would be great.

@stephanstapel
Copy link

Hi @jamie-lemon , Hi @green0317,

I guess I am ready, see here:
https://github.com/stephanstapel/Ghostscript.NET

Before sending the PR, I'd like to ask if the structure is how you liked it to be.

@jamie-lemon jamie-lemon requested review from jamie-lemon and green0317 and removed request for jamie-lemon and green0317 October 4, 2024 15:59
@jamie-lemon
Copy link
Collaborator

@stephanstapel I guess we need another PR setup then , this one is about merging jstaerk:issues/122-facturx - I think we need another one which wants to merge in your https://github.com/stephanstapel/Ghostscript.NET

@stephanstapel
Copy link

@jamie-lemon , yes that is certainly right. I have quite restructued the code from @jstaerk . Before issuing the PR, I just wanted to ask for feedback if this looks alright for you. But I can also issue the PR, if you like

@jhabjan
Copy link
Contributor

jhabjan commented Oct 4, 2024

@stephanstapel - Just a recommendation, rather then naming it Ghostscript.NET.PDFConverter, I would name it Ghostscript.NET.PDFA3Converter because it's specific to PDF/A-3 (as well as main class from PDFConverter to PDFA3Converter).

@jamie-lemon
Copy link
Collaborator

@stephanstapel Please go ahead and issue the PR and then we can continue the discussion from there!

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.

5 participants