diff --git a/src/backend/InvenTree/InvenTree/models.py b/src/backend/InvenTree/InvenTree/models.py index 1c5a4c786b..1043fb6db9 100644 --- a/src/backend/InvenTree/InvenTree/models.py +++ b/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. diff --git a/src/backend/InvenTree/report/models.py b/src/backend/InvenTree/report/models.py index 61c6c5cf69..2fbaf3e0bd 100644 --- a/src/backend/InvenTree/report/models.py +++ b/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 diff --git a/src/backend/InvenTree/report/templatetags/report.py b/src/backend/InvenTree/report/templatetags/report.py index ce8388e71c..be28eef151 100644 --- a/src/backend/InvenTree/report/templatetags/report.py +++ b/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: - raise ValueError('parameter tag requires a valid Model instance') + if instance is None or not isinstance(instance, Model): + raise ValidationError('parameter tag requires a valid Model instance') - if not isinstance(instance, Model) or not hasattr(instance, 'parameters'): - raise TypeError("parameter tag requires a Model with 'parameters' attribute") + if not hasattr(instance, 'parameters'): + raise ValidationError( + "parameter tag requires a Model with 'parameters' attribute" + ) + + parameters = instance.parameters_list.all().prefetch_related('template') # First try with exact match - if ( - parameter := instance.parameters - .prefetch_related('template') - .filter(template__name=parameter_name) - .first() - ): + if parameter := parameters.filter(template__name=parameter_name).first(): return parameter # Next, try with case-insensitive match - if ( - parameter := instance.parameters - .prefetch_related('template') - .filter(template__name__iexact=parameter_name) - .first() - ): + if parameter := parameters.filter(template__name__iexact=parameter_name).first(): return parameter return None diff --git a/src/backend/InvenTree/report/test_tags.py b/src/backend/InvenTree/report/test_tags.py index 80d09dc90b..cf7c20ec4d 100644 --- a/src/backend/InvenTree/report/test_tags.py +++ b/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): diff --git a/src/frontend/src/components/editors/TemplateEditor/PdfPreview/PdfPreview.tsx b/src/frontend/src/components/editors/TemplateEditor/PdfPreview/PdfPreview.tsx index 3f4bf9951f..5a0fea7cc2 100644 --- a/src/frontend/src/components/editors/TemplateEditor/PdfPreview/PdfPreview.tsx +++ b/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) { diff --git a/src/frontend/src/components/editors/TemplateEditor/TemplateEditor.tsx b/src/frontend/src/components/editors/TemplateEditor/TemplateEditor.tsx index 786b4803da..6aafc01f54 100644 --- a/src/frontend/src/components/editors/TemplateEditor/TemplateEditor.tsx +++ b/src/frontend/src/components/editors/TemplateEditor/TemplateEditor.tsx @@ -223,7 +223,7 @@ export function TemplateEditor(props: Readonly) { }); }) .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) { return ( - + { @@ -282,7 +282,7 @@ export function TemplateEditor(props: Readonly) { keepMounted={false} style={{ minWidth: '300px', - flex: '1', + width: '50%', display: 'flex', flexDirection: 'column' }} @@ -348,6 +348,7 @@ export function TemplateEditor(props: Readonly) { keepMounted={false} style={{ minWidth: '200px', + width: '50%', display: 'flex', flexDirection: 'column' }} diff --git a/src/frontend/tests/pui_printing.spec.ts b/src/frontend/tests/pui_printing.spec.ts index e7941fdde8..9657e2f7d8 100644 --- a/src/frontend/tests/pui_printing.spec.ts +++ b/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(); +});