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

chore: warn if wkhtmltopdf is invalid #26174

Merged

Conversation

maharshivpatel
Copy link
Collaborator

wkhtmltopdf ( with patched qt ) is required to generate pdfs. when user clicks on PDF, it will be generated and downloaded. however, on print preview page warning will be shown.

invalid wkhtmltopdf warning

wkhtmltopdf ( with patched qt ) is required to generate pdfs.
when user clicks on PDF, pdf will be generated and downloaded.
however, on print preview page warning will be shown.
@maharshivpatel maharshivpatel requested a review from a team as a code owner April 26, 2024 11:28
@maharshivpatel maharshivpatel requested review from ankush and removed request for a team April 26, 2024 11:28
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Apr 26, 2024
Copy link
Member

@akhilnarang akhilnarang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason to use a redis hash here since its a single value.

Also should probably set a low expiry? (or not cache it at all)

Otherwise if people update it they'll still see the same error message?

frappe/utils/pdf.py Outdated Show resolved Hide resolved
frappe/utils/pdf.py Outdated Show resolved Hide resolved
frappe/utils/pdf.py Outdated Show resolved Hide resolved
@maharshivpatel maharshivpatel added the backport version-15-hotfix Backport the PR to v15 label Apr 30, 2024
frappe/utils/pdf.py Outdated Show resolved Hide resolved
frappe/utils/pdf.py Outdated Show resolved Hide resolved
Co-authored-by: Ankush Menat <ankushmenat@gmail.com>
@maharshivpatel maharshivpatel enabled auto-merge (squash) April 30, 2024 10:29
@maharshivpatel maharshivpatel merged commit 6a6ded1 into frappe:develop Apr 30, 2024
24 checks passed
mergify bot pushed a commit that referenced this pull request Apr 30, 2024
* chore: warn if wkhtmltopdf is invalid

wkhtmltopdf ( with patched qt ) is required to generate pdfs properly.
when user clicks on PDF, pdf will be generated and downloaded.
however, on print preview page warning will be shown.

* chore: refactor based on review comments

* chore: return False incase of exception

* chore: refactor and better naming

Co-authored-by: Ankush Menat <ankushmenat@gmail.com>
(cherry picked from commit 6a6ded1)
maharshivpatel added a commit that referenced this pull request Apr 30, 2024
…-26174

chore: warn if wkhtmltopdf is invalid (backport #26174)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add-test-cases Add test case to validate fix or enhancement backport version-15-hotfix Backport the PR to v15
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants