Report Generation Updates (#12187)

* Fix for TemplateEditor

- Allow dragging of split section

* Cleaner report template code

* Pass correct error message through

* Prevent multiple retries if running in worker thread

* Handle report merge error

* Add playwright tests for broken report printing

* Reduce scope for exception messages

* Reduce comment deltas

* Adjust unit test

* Raise ValidaitonError

* Handle message parsing

* Additional comment

* Fix unit tests

src/backend/InvenTree/InvenTree/models.py

@@ -568,7 +568,7 @@ class InvenTreeParameterMixin(InvenTreePermissionCheckMixin, models.Model):
             if 'parameters_list' in cache:
                 return cache['parameters_list']
 
-        return self.parameters_list.all()
+        return self.parameters_list.all().prefetch_related('template')
 
     def delete(self, *args, **kwargs):
         """Handle the deletion of a model instance.

src/backend/InvenTree/report/models.py

@@ -25,6 +25,7 @@ from pypdf import PdfWriter
 import InvenTree.exceptions
 import InvenTree.helpers
 import InvenTree.models
+import InvenTree.ready
 import report.helpers
 import report.validators
 from common.models import DataOutput, RenderChoices, UpdatedUserMixin
@@ -578,10 +579,10 @@ class ReportTemplate(TemplateUploadMixin, ReportTemplateBase):
                     raise ValidationError(msg)
                 except TemplateSyntaxError as e:
                     msg = _('Template syntax error')
-                    output.mark_failure(msg)
+                    output.mark_failure(str(e) or msg)
                     raise ValidationError(f'{msg}: {e!s}')
                 except ValidationError as e:
-                    output.mark_failure(str(e))
+                    output.mark_failure(','.join(e.messages))
                     raise e
                 except Exception as e:
                     msg = _('Error rendering report')
@@ -617,10 +618,10 @@ class ReportTemplate(TemplateUploadMixin, ReportTemplateBase):
                         raise ValidationError(msg)
                     except TemplateSyntaxError as e:
                         msg = _('Template syntax error')
-                        output.mark_failure(error=_('Template syntax error'))
+                        output.mark_failure(error=str(e) or msg)
                         raise ValidationError(f'{msg}: {e!s}')
                     except ValidationError as e:
-                        output.mark_failure(str(e))
+                        output.mark_failure(', '.join(e.messages))
                         raise e
                     except Exception as e:
                         msg = _('Error rendering report')
@@ -643,6 +644,13 @@ class ReportTemplate(TemplateUploadMixin, ReportTemplateBase):
             # Something went wrong during the report generation process
             log_report_error('ReportTemplate.print')
 
+            # If the error occurred in a worker thread, we do not want to raise an error,
+            # as this would cause the worker to retry the task indefinitely
+            if InvenTree.ready.isInWorkerThread():
+                return
+
+            # Raise a ValidationError with the error message
+            # This will be caught by the caller and displayed to the user
             raise ValidationError({
                 'error': _('Error generating report'),
                 'detail': str(exc),
@@ -677,6 +685,12 @@ class ReportTemplate(TemplateUploadMixin, ReportTemplateBase):
                 log_report_error('ReportTemplate.print')
                 msg = _('Error merging report outputs')
                 output.mark_failure(error=msg)
+
+                # If the error occurred in a worker thread, we do not want to raise an error,
+                # as this would cause the worker to retry the task indefinitely
+                if InvenTree.ready.isInWorkerThread():
+                    return
+
                 raise ValidationError(msg)
 
         # Save the generated report to the database

src/backend/InvenTree/report/templatetags/report.py

@@ -461,7 +461,7 @@ def part_image(part: Part, preview: bool = False, thumbnail: bool = False, **kwa
         TypeError: If provided part is not a Part instance
     """
     if not part or not isinstance(part, Part):
-        raise TypeError(_('part_image tag requires a Part instance'))
+        raise ValidationError(_('part_image tag requires a Part instance'))
 
     image_filename = InvenTree.helpers.image2name(part.image, preview, thumbnail)
 
@@ -487,28 +487,22 @@ def parameter(
     Returns:
         A Parameter object, or the provided default value if not found
     """
-    if instance is None:
+    if instance is None or not isinstance(instance, Model):
-        raise ValueError('parameter tag requires a valid Model instance')
+        raise ValidationError('parameter tag requires a valid Model instance')
 
-    if not isinstance(instance, Model) or not hasattr(instance, 'parameters'):
+    if not hasattr(instance, 'parameters'):
-        raise TypeError("parameter tag requires a Model with 'parameters' attribute")
+        raise ValidationError(
+            "parameter tag requires a Model with 'parameters' attribute"
+        )
+
+    parameters = instance.parameters_list.all().prefetch_related('template')
 
     # First try with exact match
-    if (
+    if parameter := parameters.filter(template__name=parameter_name).first():
-        parameter := instance.parameters
-        .prefetch_related('template')
-        .filter(template__name=parameter_name)
-        .first()
-    ):
         return parameter
 
     # Next, try with case-insensitive match
-    if (
+    if parameter := parameters.filter(template__name__iexact=parameter_name).first():
-        parameter := instance.parameters
-        .prefetch_related('template')
-        .filter(template__name__iexact=parameter_name)
-        .first()
-    ):
         return parameter
 
     return None

src/backend/InvenTree/report/test_tags.py

@@ -15,7 +15,7 @@ from PIL import Image
 
 from common.models import InvenTreeSetting, Parameter, ParameterTemplate
 from InvenTree.unit_test import InvenTreeTestCase
-from part.models import Part  # TODO fix import: PartParameter, PartParameterTemplate
+from part.models import Part
 from part.test_api import PartImageTestMixin
 from report.templatetags import barcode as barcode_tags
 from report.templatetags import report as report_tags
@@ -184,7 +184,7 @@ class ReportTagTest(PartImageTestMixin, InvenTreeTestCase):
 
     def test_part_image(self):
         """Unit tests for the 'part_image' tag."""
-        with self.assertRaises(TypeError):
+        with self.assertRaises(ValidationError):
             report_tags.part_image(None)
 
         obj = Part.objects.create(name='test', description='test')
@@ -502,11 +502,11 @@ class ReportTagTest(PartImageTestMixin, InvenTreeTestCase):
         self.assertEqual(report_tags.parameter(part, 'Template 1'), parameter)
 
         # Test with a null part
-        with self.assertRaises(ValueError):
+        with self.assertRaises(ValidationError):
             report_tags.parameter(None, 'name')
 
         # Test with an invalid model type
-        with self.assertRaises(TypeError):
+        with self.assertRaises(ValidationError):
             report_tags.parameter(parameter, 'name')
 
     def test_render_currency(self):

src/frontend/src/components/editors/TemplateEditor/PdfPreview/PdfPreview.tsx

@@ -68,9 +68,13 @@ export const PdfPreviewComponent: PreviewAreaComponent = forwardRef(
               api
                 .get(apiUrl(ApiEndpoints.data_output, preview.data.pk))
                 .then((response) => {
-                  if (response.data.error) {
+                  if (response.data.errors || response.data.error) {
                     clearInterval(interval);
-                    rej(response.data.error);
+                    rej(
+                      response.data.error ??
+                        response.data.errors?.error ??
+                        t`Process failed`
+                    );
                   }
 
                   if (response.data.complete) {

src/frontend/src/components/editors/TemplateEditor/TemplateEditor.tsx

@@ -223,7 +223,7 @@ export function TemplateEditor(props: Readonly<TemplateEditorProps>) {
           });
         })
         .catch((error) => {
-          const msg = error?.message;
+          const msg = error?.message || error?.toString();
 
           if (msg) {
             if (Array.isArray(msg)) {
@@ -272,7 +272,7 @@ export function TemplateEditor(props: Readonly<TemplateEditorProps>) {
   return (
     <Boundary label='TemplateEditor'>
       <Stack style={{ height: '100%', flex: '1' }}>
-        <Split style={{ gap: '10px' }}>
+        <Split visible style={{ flex: 1 }}>
           <Tabs
             value={editorValue}
             onChange={async (v) => {
@@ -282,7 +282,7 @@ export function TemplateEditor(props: Readonly<TemplateEditorProps>) {
             keepMounted={false}
             style={{
               minWidth: '300px',
-              flex: '1',
+              width: '50%',
               display: 'flex',
               flexDirection: 'column'
             }}
@@ -348,6 +348,7 @@ export function TemplateEditor(props: Readonly<TemplateEditorProps>) {
             keepMounted={false}
             style={{
               minWidth: '200px',
+              width: '50%',
               display: 'flex',
               flexDirection: 'column'
             }}

src/frontend/tests/pui_printing.spec.ts

@@ -1,7 +1,7 @@
 import type { Locator } from '@playwright/test';
 import { expect, test } from './baseFixtures.js';
 import { adminuser } from './defaults.js';
-import { activateTableView, loadTab } from './helpers.js';
+import { activateTableView, loadTab, navigate } from './helpers.js';
 import { doCachedLogin } from './login.js';
 import { setPluginState } from './settings.js';
 
@@ -207,3 +207,54 @@ test('Printing - Report Editing', async ({ browser }) => {
     state: false
   });
 });
+
+// Test report printing with an intentionally broken template, to verify that errors are handled gracefully
+test('Printing - Broken Template', async ({ browser }) => {
+  const page = await doCachedLogin(browser, {
+    user: adminuser,
+    url: 'sales/sales-order/14/detail'
+  });
+
+  // Print report from the "sales order" detail page
+  await page
+    .getByRole('button', { name: 'action-menu-printing-actions' })
+    .click();
+  await page
+    .getByRole('menuitem', {
+      name: 'action-menu-printing-actions-print-reports'
+    })
+    .click();
+  await page
+    .getByRole('combobox', { name: 'related-field-template' })
+    .fill('broken');
+  await page.getByText('Broken Sales Order Report').click();
+  await page.getByRole('button', { name: 'Print', exact: true }).click();
+
+  // Expected error message
+  await page
+    .getByText('parameter tag requires a valid Model instance')
+    .waitFor();
+
+  // Next, check error message from the template editor preview
+  await navigate(page, 'settings/admin/reports');
+  await page
+    .getByRole('textbox', { name: 'table-search-input' })
+    .fill('broken');
+  await page.getByRole('cell', { name: 'Broken Sales Order Report' }).click();
+
+  await page.getByLabel('split-button-preview-options-action').click();
+
+  await page
+    .getByLabel('split-button-preview-options-item-preview-save', {
+      exact: true
+    })
+    .click();
+
+  await page.getByRole('button', { name: 'Save & Reload' }).click();
+
+  // Expected error messages
+  await page.getByText('Error rendering template').waitFor();
+  await page
+    .getByText('parameter tag requires a valid Model instance')
+    .waitFor();
+});