-
-
Notifications
You must be signed in to change notification settings - Fork 94
Proposal to generate an invoice with Gotenberg #382
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
base: 2.0
Are you sure you want to change the base?
Conversation
| <tr> | ||
| <td> | ||
| <img width="160" src="{{ logoPath }}"> | ||
| <img width="160" src="{{ generateWithGotenberg ? gotenberg_asset(logoPath) : logoPath }}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inject the config key $config['pdf_generator']['generate_with_gotenberg'] to wrap this logic in an if maybe ?
| "php": "^8.2", | ||
| "knplabs/knp-snappy-bundle": "^1.10", | ||
| "ramsey/uuid": "^4.7", | ||
| "sensiolabs/gotenberg-bundle": "^1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest / require-dev ?
|
|
||
| if ($config['pdf_generator']['generate_with_gotenberg'] === true) { | ||
| $definition = new Definition(TwigToGotenbergPdfGenerator::class); | ||
| $definition->addArgument(new Reference('sensiolabs_gotenberg')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check if the bundle is installed and throw an exception if necessary.
IMO the bundle should be optional.
| if ($config['pdf_generator']['generate_with_gotenberg'] === true) { | ||
| $definition = new Definition(TwigToGotenbergPdfGenerator::class); | ||
| $definition->addArgument(new Reference('sensiolabs_gotenberg')); | ||
| $definition->setDecoratedService('sylius_invoicing.generator.twig_to_pdf'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is decoration needed since we just replace the service?
| ->children() | ||
| ->booleanNode('enabled')->defaultTrue() | ||
| ->end() | ||
| ->booleanNode('generate_with_gotenberg')->defaultFalse() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about adding a service property to set the PDF generator to use? It could be Wkhtmltopdf by default to avoid BC break and Gotenberg if the bundle is installed.
Related to #376
A simple implementation for GotenbergBundle, potentially replacing wkhtmltopdf.
While integration with GotenbergBundle is possible and desirable, further testing and implementation will be required for a smooth transition of all current use cases.
Rendering by GotenbergBundle
