Replace wicked_pdf with ferrum_pdf for PDF generation#14221
Conversation
6a69b6e to
28a6cb6
Compare
ce32f3e to
f48d3df
Compare
rioug
left a comment
There was a problem hiding this comment.
It looks mostly good, there are a couple of things to address.
|
|
||
| class PdfRenderer | ||
| def render(html, display_url: default_display_url) | ||
| display_url ||= default_display_url |
There was a problem hiding this comment.
That seems redundant, specifying the default in the method definition should be enough ? or maybe I am missing something ?
There was a problem hiding this comment.
Yes, it seems so; however, if by any chance display_url is passed as nil, then the default value would be overridden with nil, which will break the flow.
There was a problem hiding this comment.
Now I think about it more, we could avoid sending display_url from here and just use the default value in the pdf renderer. I'll update that
| config.process_timeout = 60 | ||
| config.window_size = [1280, 800] | ||
|
|
||
| config.pdf_options.format = Rails.env.test? ? :A3 : :A4 |
There was a problem hiding this comment.
The original comment says :
# Conversion from PDF to text struggles with multi-line text.
# We avoid that by printing on bigger pages.
# https://github.com/openfoodfoundation/openfoodnetwork/pull/9674Should we check if this still an issue ?
Comment from the issue : #9674 (review)
It's due to known bug, which have not been fixed. So I think we can keep it like this, but could you add the original comment ?
There was a problem hiding this comment.
Good catch! Yes, I observed this as well while writing specs, and missed adding that comment. It's still an issue and a very annoying one 😅
It's addressed here: 4c27242
|
Thanks for the feedback, @rioug. It's addressed and ready for review. Thanks |
|
Hi @openfoodfoundation/reviewers , I have tested it and all seems ok, see the PDF attached, only there is a point of Attention below that I detected on Staging_AU (not related to this issue I think, but strange maybe to fix to be sure our tests are correct with the logos of enterprises) ATTENTION |
|
@ValVerCH FYI there are 2 invoices templates in OFN. Super admins can change the template here: https://staging.openfoodnetwork.org.au/admin/invoice_settings/edit Only the alternative invoice template has the logo. So what you've seen is the expected behavior of the classic OFN template. All good :) |
|
super Merci beaucoup Rachel !
[logo]
OPEN FOOD NETWORK Schweiz, Suisse, Svizzera
Valérie Veron (basée à Attalens-Fribourg)
PROJECT MANAGER
Circuits courts et Alimentation durable
T : M : F : [T :][M :]+41 79 256 42 42
***@***.***
Wasserwerkgasse 2
3011 Bern - SWITZERLAND
[https://]
[avatar]
Le 2026-05-20T17:34:49.000+02:00, Rachel Arnould
***@***.***> a écrit :
… RACHL left a comment (openfoodfoundation/openfoodnetwork#14221)
[#14221 (comment)]
***@***.*** [https://github.com/ValVerCH] FYI there are 2 invoices
templates in OFN. Super admins can change the template here:
https://staging.openfoodnetwork.org.au/admin/invoice_settings/edit
Only the alternative invoice template has the logo. So what you've
seen is the expected behavior of the classic OFN template. All good
:)
—
Reply to this email directly, view it on GitHub
[#14221 (comment)],
or unsubscribe
[https://github.com/notifications/unsubscribe-auth/B4YQQUYNLCEM3J26ZWHQKLL43XGBPAVCNFSM6AAAAACYISF3KCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DIOJZHE3DGNJYGM].
Triage notifications on the go with GitHub Mobile for iOS
[https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675]
or Android
[https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub].
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I've installed Chromium on all production servers. Merging. |
What? Why?
Replace
wicked_pdfandwkhtmltopdfwithferrum_pdffor PDF generation across invoices and reporting.wkhtmltopdf, the underlying tool currently used for PDF generation, has been archived and is no longer maintained. Whilewicked_pdfis still actively maintained, there are currently no plans to move away from supportingwkhtmltopdf, which makes our current PDF stack a growing piece of technical debt.Key changes:
wicked_pdfandwkhtmltopdf-binarydependencies.PdfRendererservice usingFerrumPdf.render_pdf.render_with_wicked_pdftosend_data.wicked_pdf-specific helpers and initializers.wicked_pdf_image_tagwith standard Railsimage_tag.display_url.0.85) to better match previouswicked_pdfoutput and reduce layout regressions.This migration also removes several workarounds and monkey patches that previously existed to support
wkhtmltopdflimitations.What should we test?
wicked_pdfoutput for layout consistency and scaling.Release notes
Changelog Category (reviewers may add a label for the release notes):
Dependencies