Skip to content

Commit

Permalink
fix: escalate print failures
Browse files Browse the repository at this point in the history
Print failures shouldn't generate PDF with failure message but instead escalate the error.

This prevent all the PDFs that just contain "PermissionError" from being sent.

(cherry picked from commit dbc2e09)
  • Loading branch information
ankush authored and mergify[bot] committed Feb 28, 2024
1 parent 898c9b0 commit a987c2d
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 17 deletions.
33 changes: 18 additions & 15 deletions frappe/__init__.py
Expand Up @@ -2089,24 +2089,27 @@ def get_print(
:param as_pdf: Return as PDF. Default False.
:param password: Password to encrypt the pdf with. Default None"""
from frappe.utils.pdf import get_pdf
from frappe.website.serve import get_response_content
from frappe.website.serve import get_response_without_exception_handling

original_form_dict = copy.deepcopy(local.form_dict)
try:
local.form_dict.doctype = doctype
local.form_dict.name = name
local.form_dict.format = print_format
local.form_dict.style = style
local.form_dict.doc = doc
local.form_dict.no_letterhead = no_letterhead
local.form_dict.letterhead = letterhead

pdf_options = pdf_options or {}
if password:
pdf_options["password"] = password

response = get_response_without_exception_handling("printview", 200)
html = str(response.data, "utf-8")
finally:
local.form_dict = original_form_dict

local.form_dict.doctype = doctype
local.form_dict.name = name
local.form_dict.format = print_format
local.form_dict.style = style
local.form_dict.doc = doc
local.form_dict.no_letterhead = no_letterhead
local.form_dict.letterhead = letterhead

pdf_options = pdf_options or {}
if password:
pdf_options["password"] = password

html = get_response_content("printview")
local.form_dict = original_form_dict
return get_pdf(html, options=pdf_options, output=output) if as_pdf else html


Expand Down
13 changes: 13 additions & 0 deletions frappe/tests/test_printview.py
@@ -1,4 +1,5 @@
import frappe
from frappe.core.doctype.doctype.test_doctype import new_doctype
from frappe.tests.utils import FrappeTestCase
from frappe.www.printview import get_html_and_style

Expand All @@ -17,3 +18,15 @@ def test_print_view_without_errors(self):

# html should exist
self.assertTrue(bool(ret["html"]))

def test_print_error(self):
"""Print failures shouldn't generate PDF with failure message but instead escalate the error"""
doctype = new_doctype(is_submittable=1).insert()

doc = frappe.new_doc(doctype.name)
doc.insert()
doc.submit()
doc.cancel()

# cancelled doc can't be printed by default
self.assertRaises(frappe.PermissionError, frappe.attach_print, doc.doctype, doc.name)
18 changes: 16 additions & 2 deletions frappe/website/serve.py
@@ -1,3 +1,5 @@
from werkzeug.wrappers import Response

import frappe
from frappe.website.page_renderers.error_page import ErrorPage
from frappe.website.page_renderers.not_found_page import NotFoundPage
Expand All @@ -6,7 +8,7 @@
from frappe.website.path_resolver import PathResolver


def get_response(path=None, http_status_code=200):
def get_response(path=None, http_status_code=200) -> Response:
"""Resolves path and renders page"""
response = None
path = path or frappe.local.request.path
Expand All @@ -28,6 +30,18 @@ def get_response(path=None, http_status_code=200):
return response


def get_response_content(path=None, http_status_code=200):
def get_response_content(path=None, http_status_code=200) -> str:
response = get_response(path, http_status_code)
return str(response.data, "utf-8")


def get_response_without_exception_handling(path=None, http_status_code=200) -> Response:
"""Resolves path and renders page.
Note: This doesn't do any exception handling and assumes you'll implement the exception
handling that's required."""
path = path or frappe.local.request.path

path_resolver = PathResolver(path, http_status_code)
_endpoint, renderer_instance = path_resolver.resolve()
return renderer_instance.render()

0 comments on commit a987c2d

Please sign in to comment.