From a8394a3339c1f65a3de5beedf25bfb41fc3eee30 Mon Sep 17 00:00:00 2001 From: Emanuel Cino Date: Thu, 2 Oct 2025 15:14:10 +0200 Subject: [PATCH 1/8] T2379 Letter writing attachments order - For MyCompassion2, the order of the attachments need to be reversed --- sbc_compassion/models/correspondence_s2b_generator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sbc_compassion/models/correspondence_s2b_generator.py b/sbc_compassion/models/correspondence_s2b_generator.py index 64e62c198..d9662ee85 100644 --- a/sbc_compassion/models/correspondence_s2b_generator.py +++ b/sbc_compassion/models/correspondence_s2b_generator.py @@ -217,7 +217,7 @@ def generate_letters_job(self): "res_model": letters._name, }, ) - for atchmt in self.image_ids + for atchmt in self.image_ids.sorted(reverse=True) ] letters += letters.create(vals) @@ -284,7 +284,7 @@ def _get_pdf(self, sponsorship): sponsorship.display_name, (header, ""), # Headers (front/back) {"Original": [text]}, # Text - self.mapped("image_ids.datas"), # Images + self.mapped("image_ids").sorted(reverse=True).mapped("datas"), # Images ), text, ) From bf2189c1530997826322ec59d66e900a2423fde1 Mon Sep 17 00:00:00 2001 From: Shayan Mouktar <119674649+Shayan105@users.noreply.github.com> Date: Wed, 15 Oct 2025 12:26:08 +0200 Subject: [PATCH 2/8] T2624 Allow status updates during letter generation process (#2050) * T2624 Add callbacks for pdf generation * T2288 Force-the-maximum-pages-of-a-letter-to-be-15 (#2051) * Reapply "Add page nb check" This reverts commit 8d4d27f0a644deae76cf6fbe30162b22e38d4bfb. * Run precommit * Add args safety to callbacks Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Remove redundant error raised --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Remove callbacks * Code improvements * Fix concurency cohenrence * Fix error propagation when more than 15 pages * Run precommit --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Emanuel Cino --- .../models/correspondence_s2b_generator.py | 96 ++++-- .../models/correspondence_template.py | 278 +++++++++++------- 2 files changed, 238 insertions(+), 136 deletions(-) diff --git a/sbc_compassion/models/correspondence_s2b_generator.py b/sbc_compassion/models/correspondence_s2b_generator.py index d9662ee85..8222f478e 100644 --- a/sbc_compassion/models/correspondence_s2b_generator.py +++ b/sbc_compassion/models/correspondence_s2b_generator.py @@ -80,6 +80,22 @@ class CorrespondenceS2bGenerator(models.Model): preview_pdf = fields.Binary(readonly=True) filename = fields.Char(compute="_compute_filename") month = fields.Selection("_get_months") + generation_status = fields.Selection( + [ + ("creating_task", "creating_task"), + ("apply_template", "apply_template"), + ("apply_text", "apply_text"), + ("apply_images", "apply_images"), + ("generate_pdf", "generate_pdf"), + ("done", "done"), + ("failed", "failed"), + ("finalizing", "finalizing"), + ], + default="creating_task", + string="Generation Status", + ) + generation_error_message = fields.Text(string="Generation Message") + MAX_PAGE_COUNT = 15 # Maximum number of pages allowed in a letter def _compute_nb_letters(self): for generator in self: @@ -131,47 +147,53 @@ def onchange_month(self): def preview(self): """Generate a picture for preview.""" pdf = self._get_pdf(self.sponsorship_ids[:1])[0] + if self.template_id.layout == "CH-A-3S01-1": - # Read page 2 in_pdf = PdfFileReader(BytesIO(pdf)) output_pdf = PdfFileWriter() - out_data = BytesIO() output_pdf.addPage(in_pdf.getPage(1)) + out_data = BytesIO() output_pdf.write(out_data) - out_data.seek(0) - pdf = out_data.read() + pdf = out_data.getvalue() + + n_pages = PdfFileReader(BytesIO(pdf)).getNumPages() + if n_pages > self.MAX_PAGE_COUNT: + msg = _("Oops your letter has %d pages. The limit is %d.") % ( + n_pages, + self.MAX_PAGE_COUNT, + ) + + raise UserError(msg) try: with Image(blob=pdf, resolution=96) as pdf_image: preview = base64.b64encode(pdf_image.make_blob(format="jpeg")) - except PolicyError as error: - _logger.error( - "ImageMagick policy error. Please add following line to " - "/etc/Image-Magick-/policy.xml: " - '', - ) - raise UserError( + except (PolicyError, TypeError) as error: + error_message = ( _( - "Please allow ImageMagick to write PDF files." - " Ask an IT admin for help." + "Please allow ImageMagick to write PDF files. " + "Ask an IT admin for help." ) - ) from error - except TypeError as error: - raise UserError( - _( + if isinstance(error, PolicyError) + else _( "There was an error while generating the PDF of the letter. " "Please check FPDF logs for more information." ) - ) from error - - pdf_image = base64.b64encode(pdf) + ) + return self.write( + { + "generation_status": "failed", + "generation_error_message": error_message, + } + ) return self.write( { "state": "preview", + "generation_status": "done", + "generation_error_message": False, "preview_image": preview, - "preview_pdf": pdf_image, + "preview_pdf": base64.b64encode(pdf), } ) @@ -184,6 +206,7 @@ def generate_letters(self): Launch S2B Creation job :return: True """ + self.generation_status = "finalizing" self.with_delay( identity_key="s2b_generator." + str(self.ids) ).generate_letters_job() @@ -227,11 +250,24 @@ def generate_letters_job(self): # If the operation succeeds, notify the user message = "Letters have been successfully generated." self.env.user.notify_success(message=message) - return self.write({"state": "done", "date": fields.Datetime.now()}) + return self.write( + { + "state": "done", + "date": fields.Datetime.now(), + "generation_status": "done", + "generation_error_message": False, + } + ) except Exception as error: # If the operation fails, notify the user with the error message error_message = str(error) + self.write( + { + "generation_status": "failed", + "generation_error_message": error_message, + } + ) self.env.user.notify_danger(message=error_message) return True @@ -285,6 +321,20 @@ def _get_pdf(self, sponsorship): (header, ""), # Headers (front/back) {"Original": [text]}, # Text self.mapped("image_ids").sorted(reverse=True).mapped("datas"), # Images + s2b_generator=self, ), text, ) + + def update_generation_status(self, status, generation_error_message=None): + """Use a separate transaction to update the status of the generation.""" + if len(self) != 1: + return False + with self.env.registry.cursor() as new_cr: + new_env = self.env(cr=new_cr) + new_s2b_generator = new_env[self._name].browse(self.id) + new_s2b_generator.generation_status = status + if generation_error_message: + new_s2b_generator.generation_error_message = generation_error_message + new_cr.commit() + return True diff --git a/sbc_compassion/models/correspondence_template.py b/sbc_compassion/models/correspondence_template.py index 7e09255cd..3cc7560da 100644 --- a/sbc_compassion/models/correspondence_template.py +++ b/sbc_compassion/models/correspondence_template.py @@ -112,13 +112,24 @@ def _compute_template_image(self): ########################################################################## # PUBLIC METHODS # ########################################################################## - def generate_pdf(self, pdf_name, header, text, image_data, background_list=None): + # ruff: noqa: C901 (Yes this function is complex... it will be removed in v17) + def generate_pdf( + self, + pdf_name, + header, + text, + image_data, + background_list=None, + s2b_generator=None, + ): """ Generate a pdf file This function is nearly as generic as it should be to be implemented directly to generate PDF for any template, text and image We save every text to a temp txt file to avoid having to escape all characters that could potentially be problematic + :param s2b_generator: a s2b.generator record to update + the generation status :param pdf_name: path and name of the pdf file to write on :param header: tuple of text for the headers to display (first value is for front pages, second for back pages) @@ -130,129 +141,170 @@ def generate_pdf(self, pdf_name, header, text, image_data, background_list=None) the template. """ self.ensure_one() + if s2b_generator is None: + s2b_generator = self.env["correspondence.s2b.generator"] - # Images stored on disk for FPDF processing. We keep them in these lists - # to make sure we properly remove the files at the end of the process. - temp_img = [] - - if background_list is None: - background_list = [] - overflow_template = False - - pages = self.mapped("page_ids") - self.additional_page_id - template_list, header_data, image_boxes = self._generate_template_list( - pages, header, background_list, temp_img - ) - image_list = [] - - if background_list: - # An original document is provided. We want - # to complete the PDF document with the remaining pages - # and provide an overflow template in case the text is longer - # and we should add pages for additional translation. - if len(background_list) > len(pages): - for i in range(len(pages), len(background_list)): + # Keep file objects alive to prevent auto-deletion + temp_files = [] + std_err_file = None + pdf_file = None + + try: + if background_list is None: + background_list = [] + overflow_template = False + + pages = self.mapped("page_ids") - self.additional_page_id + template_list, header_data, image_boxes = self._generate_template_list( + pages, header, background_list, temp_files + ) + image_list = [] + + s2b_generator.update_generation_status("apply_template") + + if background_list: + # An original document is provided. We want + # to complete the PDF document with the remaining pages + # and provide an overflow template in case the text is longer + # and we should add pages for additional translation. + if len(background_list) > len(pages): + for i in range(len(pages), len(background_list)): + bf = tempfile.NamedTemporaryFile(prefix="img_", suffix=".jpg") + bf.write(base64.b64decode(background_list[i])) + bf.flush() + temp_files.append(bf) + template_list.append([bf.name, [], [], []]) + additional_page = self.env.ref("sbc_compassion.b2s_additional_page") + bf_name = False + if additional_page.background: bf = tempfile.NamedTemporaryFile(prefix="img_", suffix=".jpg") - bf.write(base64.b64decode(background_list[i])) + bf.write(base64.b64decode(additional_page.background)) bf.flush() - temp_img.append(bf) - template_list.append([bf.name, [], [], []]) - additional_page = self.env.ref("sbc_compassion.b2s_additional_page") - bf_name = False - if additional_page.background: - bf = tempfile.NamedTemporaryFile(prefix="img_", suffix=".jpg") - bf.write(base64.b64decode(additional_page.background)) - bf.flush() - bf_name = bf.name - temp_img.append(bf) - text_list = [] - for text_box in additional_page.text_box_ids: - text_list.append(text_box.get_json_repr()) - overflow_template = [bf_name, [], text_list, image_boxes] - elif self.additional_page_id: - # We are generating a new PDF (S2B case). We provide - # an overflow template using the template - add_background = tempfile.NamedTemporaryFile(prefix="img_", suffix=".jpg") - add_background.write(base64.b64decode(self.additional_page_id.background)) - add_background.flush() - temp_img.append(add_background) + bf_name = bf.name + temp_files.append(bf) + text_list = [] + for text_box in additional_page.text_box_ids: + text_list.append(text_box.get_json_repr()) + overflow_template = [bf_name, [], text_list, image_boxes] + elif self.additional_page_id: + # We are generating a new PDF (S2B case). We provide + # an overflow template using the template + add_background = tempfile.NamedTemporaryFile( + prefix="img_", suffix=".jpg" + ) + add_background.write( + base64.b64decode(self.additional_page_id.background) + ) + add_background.flush() + temp_files.append(add_background) + text_list = [] + for text_box in self.additional_page_id.text_box_ids: + text_list.append(text_box.get_json_repr()) + overflow_template = [add_background.name, header_data, text_list, []] + + s2b_generator.update_generation_status("apply_text") + text_list = [] - for text_box in self.additional_page_id.text_box_ids: - text_list.append(text_box.get_json_repr()) - overflow_template = [add_background.name, header_data, text_list, []] + for t_type, t_boxes in list(text.items()): + for txt in t_boxes: + txt_file = tempfile.NamedTemporaryFile( + "w", prefix=t_type + "_", suffix=".txt", encoding="utf-8" + ) + txt_file.write(txt) + txt_file.flush() + temp_files.append(txt_file) + text_list.append([txt_file.name, t_type]) - text_list = [] - for t_type, t_boxes in list(text.items()): - for txt in t_boxes: - txt_file = tempfile.NamedTemporaryFile( - "w", prefix=t_type + "_", suffix=".txt", encoding="utf-8" - ) - txt_file.write(txt) - txt_file.flush() - temp_img.append(txt_file) - text_list.append([txt_file.name, t_type]) - - for image in image_data: - ifile = tempfile.NamedTemporaryFile(prefix="img_", suffix=".jpg") - ifile.write(base64.b64decode(image)) - ifile.flush() - image_list.append(ifile.name) - temp_img.append(ifile) - - generated_json = { - "images": image_list, - "templates": template_list, - "texts": text_list, - # The output should at least contain 2 pages - "original_size": max(2, len(background_list)), - "overflow_template": overflow_template, - "lang": self.env.lang, - "prevent_overflow": self.type == "b2s", - } - - json_val = json.dumps(generated_json).replace(" ", "") - - std_err_file_path = self.path_to("stderr.txt") - std_err_file = open(std_err_file_path, "w", encoding="utf-8") - - php_command_args = ["php", self.path_to("pdf.php"), pdf_name, json_val] - if config.get("php_debug"): - # Allow php debugging with Xend - os.environ["XDEBUG_CONFIG"] = "PHPSTORM" - php_command_args.extend( - [ - "-dxdebug.remote_enable=1", - "-dxdebug.remote_mode=req", - "-dxdebug.remote_port=9000", - "-dxdebug.remote_host=127.0.0.1", - ] - ) - proc = subprocess.Popen(php_command_args, stderr=std_err_file) - proc.communicate() - - if proc.returncode != 0: - with open(std_err_file_path, "r", encoding="utf-8") as stderr: - _logger.error( - "FPDF returned nonzero exit code %d. stderr:\n%s", - proc.returncode, - stderr.read(), + s2b_generator.update_generation_status("apply_images") + + for image in image_data: + ifile = tempfile.NamedTemporaryFile(prefix="img_", suffix=".jpg") + ifile.write(base64.b64decode(image)) + ifile.flush() + image_list.append(ifile.name) + temp_files.append(ifile) + + generated_json = { + "images": image_list, + "templates": template_list, + "texts": text_list, + # The output should at least contain 2 pages + "original_size": max(2, len(background_list)), + "overflow_template": overflow_template, + "lang": self.env.lang, + "prevent_overflow": self.type == "b2s", + } + + json_val = json.dumps(generated_json).replace(" ", "") + + std_err_file_path = self.path_to("stderr.txt") + std_err_file = open(std_err_file_path, "w", encoding="utf-8") + + s2b_generator.update_generation_status("generate_pdf") + + php_command_args = ["php", self.path_to("pdf.php"), pdf_name, json_val] + if config.get("php_debug"): + # Allow php debugging with Xend + os.environ["XDEBUG_CONFIG"] = "PHPSTORM" + php_command_args.extend( + [ + "-dxdebug.remote_enable=1", + "-dxdebug.remote_mode=req", + "-dxdebug.remote_port=9000", + "-dxdebug.remote_host=127.0.0.1", + ] ) + proc = subprocess.Popen(php_command_args, stderr=std_err_file) + proc.communicate() - # Clean temp files - for img in temp_img: - img.close() - std_err_file.close() + if proc.returncode != 0: + with open(std_err_file_path, "r", encoding="utf-8") as stderr: + _logger.error( + "FPDF returned nonzero exit code %d. stderr:\n%s", + proc.returncode, + stderr.read(), + ) - # Read and return output - try: + # Read and return output pdf_file = open(pdf_name, "rb") res = pdf_file.read() pdf_file.close() - os.remove(pdf_file.name) - except FileNotFoundError: - _logger.error("Cannot read PDF made by FPDF.") - res = False - return res + pdf_file = None + os.remove(pdf_name) + + return res + + except Exception: + _logger.error("Cannot read PDF made by FPDF.", exc_info=True) + return False + + finally: + # Clean up all temporary files (they will be auto-deleted when closed) + for temp_file in temp_files: + try: + temp_file.close() + except Exception as e: + _logger.warning("Failed to close temp file: %s", str(e)) + + # Clean up stderr file + if std_err_file: + try: + std_err_file.close() + except Exception: + pass + try: + std_err_file_path = self.path_to("stderr.txt") + if os.path.exists(std_err_file_path): + os.remove(std_err_file_path) + except Exception as e: + _logger.warning("Failed to clean up stderr file: %s", str(e)) + + # Clean up PDF file if still open + if pdf_file: + try: + pdf_file.close() + except Exception: + pass # path of the FPDF folder _absolute_path = os.path.join( From 4ca569eb9d70715cd8d659562ac2a12ea50db583 Mon Sep 17 00:00:00 2001 From: Emanuel Cino Date: Thu, 16 Oct 2025 10:37:23 +0200 Subject: [PATCH 3/8] T2624 Improve error handling --- .../models/correspondence_s2b_generator.py | 86 +++++++++---------- .../correspondence_s2b_generator_view.xml | 1 + 2 files changed, 42 insertions(+), 45 deletions(-) diff --git a/sbc_compassion/models/correspondence_s2b_generator.py b/sbc_compassion/models/correspondence_s2b_generator.py index 8222f478e..013e8ae03 100644 --- a/sbc_compassion/models/correspondence_s2b_generator.py +++ b/sbc_compassion/models/correspondence_s2b_generator.py @@ -146,56 +146,58 @@ def onchange_month(self): def preview(self): """Generate a picture for preview.""" - pdf = self._get_pdf(self.sponsorship_ids[:1])[0] - - if self.template_id.layout == "CH-A-3S01-1": - in_pdf = PdfFileReader(BytesIO(pdf)) - output_pdf = PdfFileWriter() - output_pdf.addPage(in_pdf.getPage(1)) - out_data = BytesIO() - output_pdf.write(out_data) - pdf = out_data.getvalue() - - n_pages = PdfFileReader(BytesIO(pdf)).getNumPages() - if n_pages > self.MAX_PAGE_COUNT: - msg = _("Oops your letter has %d pages. The limit is %d.") % ( - n_pages, - self.MAX_PAGE_COUNT, - ) + try: + pdf = self._get_pdf(self.sponsorship_ids[:1])[0] + + if self.template_id.layout == "CH-A-3S01-1": + in_pdf = PdfFileReader(BytesIO(pdf)) + output_pdf = PdfFileWriter() + output_pdf.addPage(in_pdf.getPage(1)) + out_data = BytesIO() + output_pdf.write(out_data) + pdf = out_data.getvalue() + + n_pages = PdfFileReader(BytesIO(pdf)).getNumPages() + if n_pages > self.MAX_PAGE_COUNT: + msg = _("Oops your letter has %d pages. The limit is %d.") % ( + n_pages, + self.MAX_PAGE_COUNT, + ) - raise UserError(msg) + raise UserError(msg) - try: with Image(blob=pdf, resolution=96) as pdf_image: preview = base64.b64encode(pdf_image.make_blob(format="jpeg")) - except (PolicyError, TypeError) as error: + return self.write( + { + "state": "preview", + "generation_status": "done", + "generation_error_message": False, + "preview_image": preview, + "preview_pdf": base64.b64encode(pdf), + } + ) + + except (PolicyError, TypeError, UserError, Exception) as error: error_message = ( _( - "Please allow ImageMagick to write PDF files. " - "Ask an IT admin for help." + "Unfortunately the server cannot generate PDF documents " + "at the moment. Our IT team is informed and will fix this issue " + "as soon as possible." ) if isinstance(error, PolicyError) + else error.message + if isinstance(error, UserError) else _( "There was an error while generating the PDF of the letter. " "Please check FPDF logs for more information." ) ) - return self.write( - { - "generation_status": "failed", - "generation_error_message": error_message, - } - ) - - return self.write( - { - "state": "preview", - "generation_status": "done", - "generation_error_message": False, - "preview_image": preview, - "preview_pdf": base64.b64encode(pdf), - } - ) + _logger.error("Unable to generate PDF", exc_info=True) + if self.env.context.get("raise_error"): + raise UserError(error_message) from error + self.env.cr.rollback() + self.update_generation_status("failed", error_message) def edit(self): """Generate a picture for preview.""" @@ -261,15 +263,9 @@ def generate_letters_job(self): except Exception as error: # If the operation fails, notify the user with the error message + self.env.cr.rollback() error_message = str(error) - self.write( - { - "generation_status": "failed", - "generation_error_message": error_message, - } - ) - self.env.user.notify_danger(message=error_message) - + self.update_generation_status("failed", error_message) return True def open_letters(self): diff --git a/sbc_compassion/views/correspondence_s2b_generator_view.xml b/sbc_compassion/views/correspondence_s2b_generator_view.xml index 57985cf03..649a0fb16 100644 --- a/sbc_compassion/views/correspondence_s2b_generator_view.xml +++ b/sbc_compassion/views/correspondence_s2b_generator_view.xml @@ -25,6 +25,7 @@ icon="fa-search-plus" class="oe_stat_button" states="draft" + context="{'raise_error': True}" />