diff --git a/partner_communication/__manifest__.py b/partner_communication/__manifest__.py index 44b156191..f927d00f2 100644 --- a/partner_communication/__manifest__.py +++ b/partner_communication/__manifest__.py @@ -30,7 +30,7 @@ # pylint: disable=C8101 { "name": "Partner Communication", - "version": "18.0.1.0.2", + "version": "18.0.1.0.3", "category": "Other", "author": "Compassion Switzerland", "license": "AGPL-3", @@ -56,6 +56,7 @@ "views/generate_communication_wizard_view.xml", "views/ir_attachment_view.xml", "views/ir_actions_view.xml", + "views/ir_actions_report_view.xml", "views/download_print_wizard_view.xml", "views/settings_view.xml", "views/communication_snippet_view.xml", diff --git a/partner_communication/migrations/18.0.1.0.3/end-migration.py b/partner_communication/migrations/18.0.1.0.3/end-migration.py new file mode 100644 index 000000000..8797919d9 --- /dev/null +++ b/partner_communication/migrations/18.0.1.0.3/end-migration.py @@ -0,0 +1,47 @@ +import logging + +from openupgradelib import openupgrade + +_logger = logging.getLogger(__name__) + + +@openupgrade.migrate() +def migrate(env, version): + """ + The reports of our communications have no javascript, so wkhtmltopdf has + nothing to wait for: a print delay of 0 makes each of them about 2 seconds + faster. This is an end-migration so that all the modules are loaded, and it + takes the reports from what the communications use, whatever module defines + them (they may also have been added directly in the database), so that it + works for any of our instances (Switzerland, Nordic, ...). + """ + # Our code prints the reports of the communications by name: a report that + # was duplicated from the UI (same report name) can be the one that is used, + # so all the reports sharing the name of a report of the communications are + # updated. Only the ones that still have the default value (-1) change, to + # keep a value that someone would have set by hand. + env.cr.execute( + """ + UPDATE ir_act_report_xml + SET print_delay = 0 + WHERE print_delay < 0 AND report_name IN ( + SELECT report_name FROM ir_act_report_xml WHERE id IN ( + SELECT report_id FROM partner_communication_config + WHERE report_id IS NOT NULL + UNION + SELECT report_id FROM partner_communication_attachment + WHERE report_id IS NOT NULL + ) OR model IN ( + 'partner.communication.job', + 'correspondence', + 'compassion.child', + 'recurring.contract', + 'crm.claim', + 'compassion.field.office' + ) + ) + """ + ) + _logger.info( + "Print delay set to 0 for %s reports of the communications", env.cr.rowcount + ) diff --git a/partner_communication/models/__init__.py b/partner_communication/models/__init__.py index 2ed436fb8..ba477cb3a 100644 --- a/partner_communication/models/__init__.py +++ b/partner_communication/models/__init__.py @@ -15,6 +15,7 @@ communication_job, communication_snippet, ir_actions, + ir_actions_report, ir_attachment, mail_mail, res_partner, diff --git a/partner_communication/models/ir_actions_report.py b/partner_communication/models/ir_actions_report.py new file mode 100644 index 000000000..5e4d13a50 --- /dev/null +++ b/partner_communication/models/ir_actions_report.py @@ -0,0 +1,42 @@ +############################################################################## +# +# Copyright (C) 2026 Compassion CH (http://www.compassion.ch) +# Releasing children from poverty in Jesus' name +# @author: Emanuel Cino +# +# The licence is in the file __manifest__.py +# +############################################################################## +from odoo import api, fields, models + + +class IrActionsReport(models.Model): + _inherit = "ir.actions.report" + + print_delay = fields.Integer( + "Print delay (ms)", + default=-1, + help="Time that wkhtmltopdf waits for the javascript of the page before " + "printing it. Reports without javascript (all our communications) have " + "nothing to wait for and are generated about 2 seconds faster with 0. " + "-1 uses the system parameter report.print_delay (1 second by default).", + ) + + @api.model + def _run_wkhtmltopdf(self, bodies, report_ref=False, *args, **kwargs): + report = self._get_report(report_ref) if report_ref else None + if report and report.print_delay >= 0: + # The delay is read when the arguments of wkhtmltopdf are built + report_model = self.with_context(report_print_delay=report.print_delay) + return super(IrActionsReport, report_model)._run_wkhtmltopdf( + bodies, report_ref, *args, **kwargs + ) + return super()._run_wkhtmltopdf(bodies, report_ref, *args, **kwargs) + + @api.model + def _build_wkhtmltopdf_args(self, *args, **kwargs): + command_args = super()._build_wkhtmltopdf_args(*args, **kwargs) + delay = self.env.context.get("report_print_delay") + if delay is not None and "--javascript-delay" in command_args: + command_args[command_args.index("--javascript-delay") + 1] = str(delay) + return command_args diff --git a/partner_communication/report/a4_communication.xml b/partner_communication/report/a4_communication.xml index cac322a17..a1a2d6ec3 100644 --- a/partner_communication/report/a4_communication.xml +++ b/partner_communication/report/a4_communication.xml @@ -5,6 +5,7 @@ partner.communication.job A4 communication qweb-pdf + 0 partner_communication.a4_communication diff --git a/partner_communication/tests/__init__.py b/partner_communication/tests/__init__.py index 843ef3cc8..2366e3925 100644 --- a/partner_communication/tests/__init__.py +++ b/partner_communication/tests/__init__.py @@ -6,3 +6,4 @@ # The licence is in the file __manifest__.py # ############################################################################## +from . import test_ir_actions_report diff --git a/partner_communication/tests/test_ir_actions_report.py b/partner_communication/tests/test_ir_actions_report.py new file mode 100644 index 000000000..b1a8eb436 --- /dev/null +++ b/partner_communication/tests/test_ir_actions_report.py @@ -0,0 +1,48 @@ +from unittest.mock import patch + +from odoo.tests import TransactionCase + +from odoo.addons.base.models.ir_actions_report import IrActionsReport + + +class TestReportPrintDelay(TransactionCase): + def setUp(self): + super().setUp() + self.report = self.env.ref("partner_communication.report_a4_communication") + self.paperformat = self.report.get_paperformat() + + def _delay(self, args): + return args[args.index("--javascript-delay") + 1] + + def test_communication_report_has_no_delay(self): + self.assertEqual(self.report.print_delay, 0) + + def test_system_parameter_is_the_default(self): + self.env["ir.config_parameter"].sudo().set_param("report.print_delay", "750") + args = self.env["ir.actions.report"]._build_wkhtmltopdf_args( + self.paperformat, False + ) + self.assertEqual(self._delay(args), "750") + + def test_run_wkhtmltopdf_uses_the_delay_of_the_report(self): + seen = {} + + def fake_run(self, bodies, report_ref=False, *args, **kwargs): + seen["delay"] = self.env.context.get("report_print_delay") + return b"" + + with patch.object(IrActionsReport, "_run_wkhtmltopdf", fake_run): + self.env["ir.actions.report"]._run_wkhtmltopdf([], self.report.id) + self.assertEqual(seen["delay"], 0) + # -1 leaves the system parameter in charge + self.report.print_delay = -1 + self.env["ir.actions.report"]._run_wkhtmltopdf([], self.report.id) + self.assertIsNone(seen["delay"]) + + def test_delay_of_the_context_is_in_the_arguments(self): + args = ( + self.env["ir.actions.report"] + .with_context(report_print_delay=0) + ._build_wkhtmltopdf_args(self.paperformat, False) + ) + self.assertEqual(self._delay(args), "0") diff --git a/partner_communication/views/ir_actions_report_view.xml b/partner_communication/views/ir_actions_report_view.xml new file mode 100644 index 000000000..4b8fefc3d --- /dev/null +++ b/partner_communication/views/ir_actions_report_view.xml @@ -0,0 +1,13 @@ + + + + ir.actions.report.form.print.delay + ir.actions.report + + + + + + + + diff --git a/partner_communication_compassion/__manifest__.py b/partner_communication_compassion/__manifest__.py index 0652335dd..c7a989924 100644 --- a/partner_communication_compassion/__manifest__.py +++ b/partner_communication_compassion/__manifest__.py @@ -51,6 +51,7 @@ "data/communication_config.xml", "data/utm_data.xml", "data/ir_cron_data.xml", + "data/report_print_delay.xml", "views/communication_job_view.xml", "views/disaster_alert_view.xml", "views/partner_compassion_view.xml", diff --git a/partner_communication_compassion/data/report_print_delay.xml b/partner_communication_compassion/data/report_print_delay.xml new file mode 100644 index 000000000..23768edb9 --- /dev/null +++ b/partner_communication_compassion/data/report_print_delay.xml @@ -0,0 +1,17 @@ + + + + + 0 + + + 0 + + + 0 + + + 0 + + diff --git a/sbc_compassion/models/correspondence_page.py b/sbc_compassion/models/correspondence_page.py index 0e8b2c7ec..ba85717ee 100644 --- a/sbc_compassion/models/correspondence_page.py +++ b/sbc_compassion/models/correspondence_page.py @@ -25,6 +25,10 @@ BOX_SEPARATOR = "#BOX#" PAGE_SEPARATOR = "#PAGE#" +# Detection of the pages in the stacked image of Cloudinary +EDGE_WIDTH_PX = 4 +SEAM_EDGE_JUMP = 60 +SEAM_MAX_THICKNESS_PX = 12 class CorrespondencePage(models.Model): @@ -81,16 +85,9 @@ def _compute_page_image(self, image_type="original"): for page in self: cloudinary_url = getattr(page, cloudinary_url_field) page_url = getattr(page, url_field) - image = False - # A cropped image slices a stack of pages in equal parts, which is - # wrong when the pages don't have the same height: the page of the - # document is used first. - connect_first = bool(cloudinary_url and "c_crop" in cloudinary_url) - if connect_first: - image = page._get_connect_image(page_url) - if not image and cloudinary_url: - image = page._get_cloundinary_image(cloudinary_url) - if not image and not connect_first: + # Cloudinary is much faster than Connect, which is the fallback + image = page._get_cloundinary_image(cloudinary_url) + if not image: image = page._get_connect_image(page_url) if ( not image @@ -240,17 +237,21 @@ def _get_cloundinary_image(self, cloudinary_url): "correspondence_id.cloudinary_final_letter_url", ) def _compute_cloudinary_url(self): + # The stacked image of a letter is analysed once for all its pages + stacked_images = {} for page in self: page.cloudinary_original_page_url = page._get_cloudinary_url( - page.correspondence_id.cloudinary_original_letter_url + page.correspondence_id.cloudinary_original_letter_url, stacked_images ) page.cloudinary_final_page_url = page._get_cloudinary_url( - page.correspondence_id.cloudinary_final_letter_url + page.correspondence_id.cloudinary_final_letter_url, stacked_images ) - def _get_cloudinary_url(self, cloudinary_url): + def _get_cloudinary_url(self, cloudinary_url, stacked_images=None): """ Returns the cloudinary url with the correct crop so that the page is displayed + :param stacked_images: optional dict to share the analysis of a stacked image + between the pages of a letter """ # Pages added to hold overflowing text are not part of the letter image letter_pages = self.correspondence_id.page_ids @@ -260,16 +261,75 @@ def _get_cloudinary_url(self, cloudinary_url): scanned_index = list(scanned_pages).index(self) + 1 if cloudinary_url.endswith("p1.jpg"): return cloudinary_url.replace("p1.jpg", f"p{scanned_index}.jpg") - response = requests.get(cloudinary_url, timeout=3) - if not response.ok: + stacked_images = stacked_images if stacked_images is not None else {} + key = (cloudinary_url, len(scanned_pages)) + if key not in stacked_images: + stacked_images[key] = self._analyze_stacked_image( + cloudinary_url, len(scanned_pages) + ) + width, bounds = stacked_images[key] + if not bounds: + # No reliable crop: the page is fetched from Connect return False - width, total_height = Image.open(BytesIO(response.content)).size - height = total_height // len(scanned_pages) - y_start = (scanned_index - 1) * height + y_start, height = bounds[scanned_index - 1] crop_operator = f"c_crop,h_{height},w_{width},x_0,y_{y_start}" url_parts = cloudinary_url.split("upload/") return f"{url_parts[0]}upload/{crop_operator}/{url_parts[1]}" + def _analyze_stacked_image(self, cloudinary_url, nb_pages): + """ + :return: tuple (width, bounds) of the stacked image, where bounds are the + (y_start, height) of the pages, empty if they can't be found. + """ + # A network error is not caught: the url is stored and would never be + # computed again. + response = requests.get(cloudinary_url, timeout=3) + if not response.ok: + return 0, [] + try: + image = Image.open(BytesIO(response.content)) + image.load() + except OSError: + _logger.warning("Not an image: %s", cloudinary_url, exc_info=True) + return 0, [] + return image.width, self._find_page_bounds(image, nb_pages) + + @api.model + def _find_page_bounds(self, image, nb_pages): + """ + Cloudinary gives the pages of a letter stacked in one image, and the pages + don't always have the same height. A border is drawn at the edge of the + pages, so the brightness of the left edge changes sharply from one page + to the next one. + :param image: PIL image holding the stacked pages + :param nb_pages: number of pages in the image + :return: list of (y_start, height) for each page, or an empty list if the + pages could not be found reliably. + """ + width, total_height = image.size + if nb_pages <= 1: + return [(0, total_height)] if nb_pages == 1 else [] + edge = image.convert("L").crop((0, 0, min(width, EDGE_WIDTH_PX), total_height)) + edge = edge.resize((1, total_height), Image.Resampling.BOX) + brightness = list(edge.getdata()) + seams = [] + # The border at the top and bottom of the image is not between two pages + for y in range(SEAM_MAX_THICKNESS_PX, total_height - SEAM_MAX_THICKNESS_PX): + if abs(brightness[y] - brightness[y - 1]) < SEAM_EDGE_JUMP: + continue + if seams and y - seams[-1][-1] <= SEAM_MAX_THICKNESS_PX: + seams[-1].append(y) + else: + seams.append([y]) + if len(seams) != nb_pages - 1: + return [] + cuts = [0] + [(seam[0] + seam[-1]) // 2 for seam in seams] + [total_height] + heights = [end - start for start, end in zip(cuts, cuts[1:], strict=False)] + average = total_height / nb_pages + if any(not 0.5 * average <= height <= 1.5 * average for height in heights): + return [] + return list(zip(cuts, heights, strict=False)) + def sync_text_from_paragraphs(self): _fields = ["original_text", "english_text", "translated_text"] for page in self: diff --git a/sbc_compassion/tests/test_correspondence_template.py b/sbc_compassion/tests/test_correspondence_template.py index 96ed12bfe..80f8f3e69 100644 --- a/sbc_compassion/tests/test_correspondence_template.py +++ b/sbc_compassion/tests/test_correspondence_template.py @@ -1,3 +1,5 @@ +from PIL import Image + from odoo.tests import TransactionCase from ..models.correspondence_text_box import PAGE_WIDTH_PX @@ -108,3 +110,40 @@ def test_paragraph_filling_the_box(self): def test_nothing_fits_keeps_text(self): self.text_box.height = 1 self.assertEqual(self.text_box.split_text("word"), ("word", "")) + + +class TestPageBounds(TransactionCase): + """Pages are stacked in one image by Cloudinary, with different heights.""" + + def _stack(self, heights, width=300): + image = Image.new("L", (width, sum(heights)), 255) + y = 0 + for height in heights[:-1]: + y += height + # Dark border at the edge of the page, a few rows thick + for row in range(y - 1, y + 2): + for col in range(3): + image.putpixel((col, row), 20) + return image + + def test_pages_of_different_heights(self): + page = self.env["correspondence.page"] + bounds = page._find_page_bounds(self._stack([400, 400, 330]), 3) + self.assertEqual(len(bounds), 3) + for (start, height), (real_start, real_height) in zip( + bounds, [(0, 400), (400, 400), (800, 330)], strict=False + ): + self.assertAlmostEqual(start, real_start, delta=2) + self.assertAlmostEqual(height, real_height, delta=3) + self.assertEqual(sum(height for _start, height in bounds), 1130) + + def test_single_page(self): + page = self.env["correspondence.page"] + self.assertEqual(page._find_page_bounds(self._stack([500]), 1), [(0, 500)]) + + def test_no_reliable_crop_gives_nothing(self): + page = self.env["correspondence.page"] + # No border between the pages + self.assertEqual(page._find_page_bounds(Image.new("L", (300, 900), 255), 3), []) + # More borders than expected + self.assertEqual(page._find_page_bounds(self._stack([300, 300, 300]), 2), [])