From 8e1d621db9a4f1123e5d8e39bd8ea40c42a098c9 Mon Sep 17 00:00:00 2001 From: Oliver Date: Sat, 25 Oct 2025 13:17:10 +1100 Subject: [PATCH] Report tag fixes (#10668) * remove duplicate template tag * Add "multiplier" argument to render_currency * Improve render_currency - Enable conversion of non-money values to a Money instance * Improve maths tags - Convert values to Decimal - Ability to cast result to different type * Updated docs * Improved feedback from maths tags * Updated unit testing * Improved rendering of printing errors * Add extra test for render_currency tag * Enfoce multiplier type * Fix docstrings * Improved error handling * Remove defunct unit test * Fix unit tests --- docs/docs/report/helpers.md | 2 +- .../InvenTree/InvenTree/helpers_model.py | 22 ++- .../templatetags/inventree_extras.py | 6 - src/backend/InvenTree/part/test_part.py | 4 - src/backend/InvenTree/report/models.py | 6 + .../InvenTree/report/templatetags/report.py | 161 +++++++++++++++--- src/backend/InvenTree/report/test_tags.py | 46 +++-- src/frontend/src/components/forms/ApiForm.tsx | 8 +- 8 files changed, 201 insertions(+), 54 deletions(-) diff --git a/docs/docs/report/helpers.md b/docs/docs/report/helpers.md index 750c261182..f6034fe523 100644 --- a/docs/docs/report/helpers.md +++ b/docs/docs/report/helpers.md @@ -267,7 +267,7 @@ Simple mathematical operators are available, as demonstrated in the example temp ### Input Types -These mathematical functions accept inputs of various input types, and attempt to perform the operation accordingly. Note that any inputs which are provided as strings will be converted to floating point numbers before the operation is performed. +These mathematical functions accept inputs of various input types, and attempt to perform the operation accordingly. Note that any inputs which are provided as strings or numbers will be converted to `Decimal` class types before the operation is performed. ### add diff --git a/src/backend/InvenTree/InvenTree/helpers_model.py b/src/backend/InvenTree/InvenTree/helpers_model.py index 62710407d8..16338266d8 100644 --- a/src/backend/InvenTree/InvenTree/helpers_model.py +++ b/src/backend/InvenTree/InvenTree/helpers_model.py @@ -6,6 +6,7 @@ from typing import Optional, cast from urllib.parse import urljoin from django.conf import settings +from django.core.exceptions import ValidationError from django.core.validators import URLValidator from django.db.utils import OperationalError, ProgrammingError from django.utils.translation import gettext_lazy as _ @@ -184,6 +185,7 @@ def render_currency( money: Money, decimal_places: Optional[int] = None, currency: Optional[str] = None, + multiplier: Optional[Decimal] = None, min_decimal_places: Optional[int] = None, max_decimal_places: Optional[int] = None, include_symbol: bool = True, @@ -194,6 +196,7 @@ def render_currency( money: The Money instance to be rendered decimal_places: The number of decimal places to render to. If unspecified, uses the PRICING_DECIMAL_PLACES setting. currency: Optionally convert to the specified currency + multiplier: An optional multiplier to apply to the money amount before rendering min_decimal_places: The minimum number of decimal places to render to. If unspecified, uses the PRICING_DECIMAL_PLACES_MIN setting. max_decimal_places: The maximum number of decimal places to render to. If unspecified, uses the PRICING_DECIMAL_PLACES setting. include_symbol: If True, include the currency symbol in the output @@ -202,7 +205,16 @@ def render_currency( return '-' if type(money) is not Money: - return '-' + # Try to convert to a Money object + try: + money = Money( + Decimal(str(money)), + currency or get_global_setting('INVENTREE_DEFAULT_CURRENCY'), + ) + except Exception: + raise ValidationError( + f"render_currency: {_('Invalid money value')}: '{money}' ({type(money).__name__})" + ) if currency is not None: # Attempt to convert to the provided currency @@ -212,6 +224,14 @@ def render_currency( except Exception: pass + if multiplier is not None: + try: + money *= Decimal(str(multiplier).strip()) + except Exception: + raise ValidationError( + f"render_currency: {_('Invalid multiplier value')}: '{multiplier}' ({type(multiplier).__name__})" + ) + if min_decimal_places is None or not isinstance(min_decimal_places, (int, float)): min_decimal_places = get_global_setting('PRICING_DECIMAL_PLACES_MIN', 0) diff --git a/src/backend/InvenTree/InvenTree/templatetags/inventree_extras.py b/src/backend/InvenTree/InvenTree/templatetags/inventree_extras.py index 8d12601121..0a37a097d8 100644 --- a/src/backend/InvenTree/InvenTree/templatetags/inventree_extras.py +++ b/src/backend/InvenTree/InvenTree/templatetags/inventree_extras.py @@ -70,12 +70,6 @@ def str2bool(x, *args, **kwargs): return InvenTree.helpers.str2bool(x) -@register.simple_tag() -def add(x, y, *args, **kwargs): - """Add two numbers together.""" - return x + y - - @register.simple_tag() def to_list(*args): """Return the input arguments as list.""" diff --git a/src/backend/InvenTree/part/test_part.py b/src/backend/InvenTree/part/test_part.py index 6fff694875..5bb704a500 100644 --- a/src/backend/InvenTree/part/test_part.py +++ b/src/backend/InvenTree/part/test_part.py @@ -39,10 +39,6 @@ class TemplateTagTest(InvenTreeTestCase): self.assertEqual(int(inventree_extras.str2bool('none')), False) self.assertEqual(int(inventree_extras.str2bool('off')), False) - def test_add(self): - """Test that the 'add.""" - self.assertEqual(int(inventree_extras.add(3, 5)), 8) - def test_inventree_instance_name(self): """Test the 'instance name' setting.""" self.assertEqual(inventree_extras.inventree_instance_name(), 'InvenTree') diff --git a/src/backend/InvenTree/report/models.py b/src/backend/InvenTree/report/models.py index b445ae1226..f2bedd9e67 100644 --- a/src/backend/InvenTree/report/models.py +++ b/src/backend/InvenTree/report/models.py @@ -546,6 +546,9 @@ class ReportTemplate(TemplateUploadMixin, ReportTemplateBase): msg = _('Template syntax error') output.mark_failure(msg) raise ValidationError(f'{msg}: {e!s}') + except ValidationError as e: + output.mark_failure(str(e)) + raise e except Exception as e: msg = _('Error rendering report') output.mark_failure(msg) @@ -582,6 +585,9 @@ class ReportTemplate(TemplateUploadMixin, ReportTemplateBase): msg = _('Template syntax error') output.mark_failure(error=_('Template syntax error')) raise ValidationError(f'{msg}: {e!s}') + except ValidationError as e: + output.mark_failure(str(e)) + raise e except Exception as e: msg = _('Error rendering report') output.mark_failure(error=msg) diff --git a/src/backend/InvenTree/report/templatetags/report.py b/src/backend/InvenTree/report/templatetags/report.py index 837b3e7fef..7a94d54915 100644 --- a/src/backend/InvenTree/report/templatetags/report.py +++ b/src/backend/InvenTree/report/templatetags/report.py @@ -4,7 +4,7 @@ import base64 import logging import os from datetime import date, datetime -from decimal import Decimal +from decimal import Decimal, InvalidOperation from typing import Any, Optional, Union from django import template @@ -413,54 +413,161 @@ def internal_link(link, text) -> str: return mark_safe(f'{text}') -def destringify(value: Any) -> Any: - """Convert a string value into a float. +def make_decimal(value: Any) -> Any: + """Convert an input value into a Decimal. - - If the value is a string, attempt to convert it to a float. - - If conversion fails, return the original string. - - If the value is not a string, return it unchanged. + - Converts [string, int, float] types into Decimal + - If conversion fails, returns the original value The purpose of this function is to provide "seamless" math operations in templates, where numeric values may be provided as strings, or converted to strings during template rendering. """ - if isinstance(value, str): - value = value.strip() + if any(isinstance(value, t) for t in [int, float, str]): try: - return float(value) - except ValueError: - return value + value = Decimal(str(value).strip()) + except (InvalidOperation, TypeError, ValueError): + logger.warning( + 'make_decimal: Failed to convert value to Decimal: %s (%s)', + value, + type(value), + ) return value -@register.simple_tag() -def add(x: Any, y: Any) -> Any: - """Add two numbers (or number like values) together.""" - return destringify(x) + destringify(y) +def cast_to_type(value: Any, cast: type) -> Any: + """Attempt to cast a value to the provided type. + + If casting fails, the original value is returned. + """ + if cast is not None: + try: + value = cast(value) + except (ValueError, TypeError): + pass + + return value + + +def debug_vars(x: Any, y: Any) -> str: + """Return a debug string showing the types and values of two variables.""" + return f": x='{x}' ({type(x).__name__}), y='{y}' ({type(y).__name__})" @register.simple_tag() -def subtract(x: Any, y: Any) -> Any: - """Subtract one number (or number-like value) from another.""" - return destringify(x) - destringify(y) +def add(x: Any, y: Any, cast: Optional[type] = None) -> Any: + """Add two numbers (or number like values) together. + + Arguments: + x: The first value to add + y: The second value to add + cast: Optional type to cast the result to (e.g. int, float, str) + + Raises: + ValidationError: If the values cannot be added together + """ + try: + result = make_decimal(x) + make_decimal(y) + except (InvalidOperation, TypeError, ValueError): + raise ValidationError( + _('Cannot add values of incompatible types') + debug_vars(x, y) + ) + return cast_to_type(result, cast) @register.simple_tag() -def multiply(x: Any, y: Any) -> Any: - """Multiply two numbers (or number-like values) together.""" - return destringify(x) * destringify(y) +def subtract(x: Any, y: Any, cast: Optional[type] = None) -> Any: + """Subtract one number (or number-like value) from another. + + Arguments: + x: The value to be subtracted from + y: The value to be subtracted + cast: Optional type to cast the result to (e.g. int, float, str) + + Raises: + ValidationError: If the values cannot be subtracted + """ + try: + result = make_decimal(x) - make_decimal(y) + except (InvalidOperation, TypeError, ValueError): + raise ValidationError( + _('Cannot subtract values of incompatible types') + debug_vars(x, y) + ) + + return cast_to_type(result, cast) @register.simple_tag() -def divide(x: Any, y: Any) -> Any: - """Divide one number (or number-like value) by another.""" - return destringify(x) / destringify(y) +def multiply(x: Any, y: Any, cast: Optional[type] = None) -> Any: + """Multiply two numbers (or number-like values) together. + + Arguments: + x: The first value to multiply + y: The second value to multiply + cast: Optional type to cast the result to (e.g. int, float, str) + + Raises: + ValidationError: If the values cannot be multiplied together + """ + try: + result = make_decimal(x) * make_decimal(y) + except (InvalidOperation, TypeError, ValueError): + raise ValidationError( + _('Cannot multiply values of incompatible types') + debug_vars(x, y) + ) + + return cast_to_type(result, cast) @register.simple_tag() -def modulo(x: Any, y: Any) -> Any: - """Calculate the modulo of one number (or number-like value) by another.""" - return destringify(x) % destringify(y) +def divide(x: Any, y: Any, cast: Optional[type] = None) -> Any: + """Divide one number (or number-like value) by another. + + Arguments: + x: The value to be divided + y: The value to divide by + cast: Optional type to cast the result to (e.g. int, float, str) + + Raises: + ValidationError: If the values cannot be divided + """ + try: + result = make_decimal(x) / make_decimal(y) + except (InvalidOperation, TypeError, ValueError): + raise ValidationError( + _('Cannot divide values of incompatible types') + debug_vars(x, y) + ) + except ZeroDivisionError: + raise ValidationError(_('Cannot divide by zero') + debug_vars(x, y)) + + return cast_to_type(result, cast) + + +@register.simple_tag() +def modulo(x: Any, y: Any, cast: Optional[type] = None) -> Any: + """Calculate the modulo of one number (or number-like value) by another. + + Arguments: + x: The first value to be used in the modulo operation + y: The second value to be used in the modulo operation + cast: Optional type to cast the result to (e.g. int, float, str) + + Raises: + ValidationError: If the values cannot be used in a modulo operation + """ + try: + result = make_decimal(x) % make_decimal(y) + except (InvalidOperation, TypeError, ValueError): + raise ValidationError( + _('Cannot perform modulo operation with values of incompatible types') + + debug_vars(x, y) + ) + except ZeroDivisionError: + raise ValidationError( + _('Cannot perform modulo operation with divisor of zero') + debug_vars(x, y) + ) + + return cast_to_type(result, cast) @register.simple_tag diff --git a/src/backend/InvenTree/report/test_tags.py b/src/backend/InvenTree/report/test_tags.py index 357be9def3..1d06ecceba 100644 --- a/src/backend/InvenTree/report/test_tags.py +++ b/src/backend/InvenTree/report/test_tags.py @@ -1,5 +1,6 @@ """Test for custom report tags.""" +from decimal import Decimal from zoneinfo import ZoneInfo from django.conf import settings @@ -198,23 +199,27 @@ class ReportTagTest(PartImageTestMixin, InvenTreeTestCase): """Simple tests for mathematical operator tags.""" self.assertEqual(report_tags.add(1, 2), 3) self.assertEqual(report_tags.add('-33', '33'), 0) - self.assertEqual(report_tags.subtract(10, 4.2), 5.8) - self.assertEqual(report_tags.multiply(2.3, 4), 9.2) - self.assertEqual(report_tags.multiply('-2', 4), -8.0) + self.assertEqual(report_tags.add(4.5, Decimal(4.5), cast=float), 9.0) + self.assertEqual(report_tags.subtract(10, 4.2, cast=float), 5.8) + self.assertEqual(report_tags.multiply(2.3, 4, cast=str), '9.2') + self.assertEqual(report_tags.multiply('-2', 4), -8) self.assertEqual(report_tags.divide(100, 5), 20) self.assertEqual(report_tags.modulo(10, 3), 1) self.assertEqual(report_tags.modulo('10', '4'), 2) - with self.assertRaises(ZeroDivisionError): + with self.assertRaises(ValidationError): report_tags.divide(100, 0) def test_maths_tags_with_strings(self): """Tests for mathematical operator tags with string inputs.""" - self.assertEqual(report_tags.add('10', '20'), 30) - self.assertEqual(report_tags.subtract('50.5', '20.2'), 30.3) + self.assertEqual(report_tags.add(Decimal('10'), '20'), 30) + self.assertEqual(report_tags.subtract('50.5', '20.2'), Decimal('30.3')) self.assertEqual(report_tags.multiply(3.0000000000000, '7'), 21) - self.assertEqual(report_tags.divide('100.0', '4'), 25.0) + + result = report_tags.divide(100, Decimal('4'), cast=int) + self.assertEqual(result, 25) + self.assertIsInstance(result, int) def test_maths_tags_with_decimal(self): """Tests for mathematical operator tags with Decimal inputs.""" @@ -231,6 +236,8 @@ class ReportTagTest(PartImageTestMixin, InvenTreeTestCase): report_tags.divide(Decimal('10.0'), Decimal('2.000')), Decimal('5.0') ) + self.assertEqual(report_tags.multiply(3.3, Decimal('2.0')), Decimal('6.6')) + def test_maths_tags_with_money(self): """Tests for mathematical operator tags with Money inputs.""" m1 = Money(100, 'USD') @@ -239,20 +246,24 @@ class ReportTagTest(PartImageTestMixin, InvenTreeTestCase): self.assertEqual(report_tags.add(m1, m2), Money(150, 'USD')) self.assertEqual(report_tags.subtract(m1, m2), Money(50, 'USD')) self.assertEqual(report_tags.multiply(m2, 3), Money(150, 'USD')) + self.assertEqual(report_tags.multiply(-3, m2), Money(-150, 'USD')) self.assertEqual(report_tags.divide(m1, '4'), Money(25, 'USD')) + result = report_tags.divide(Money(1000, 'GBP'), 4) + self.assertIsInstance(result, Money) + def test_maths_tags_invalid(self): """Tests for mathematical operator tags with invalid inputs.""" - with self.assertRaises(TypeError): + with self.assertRaises(ValidationError): report_tags.add('abc', 10) - with self.assertRaises(TypeError): + with self.assertRaises(ValidationError): report_tags.subtract(50, 'xyz') - with self.assertRaises(TypeError): + with self.assertRaises(ValidationError): report_tags.multiply('foo', 'bar') - with self.assertRaises(TypeError): + with self.assertRaises(ValidationError): report_tags.divide('100', 'baz') def test_number_tags(self): @@ -410,8 +421,19 @@ class ReportTagTest(PartImageTestMixin, InvenTreeTestCase): '$1,234.0', ) + # Test with non-currency values + self.assertEqual( + report_tags.render_currency(1234.45, currency='USD', decimal_places=2), + '$1,234.45', + ) + # Test with an invalid amount - self.assertEqual(report_tags.render_currency('abc'), '-') + with self.assertRaises(ValidationError): + report_tags.render_currency('abc', currency='-') + + with self.assertRaises(ValidationError): + report_tags.render_currency(m, multiplier='quork') + self.assertEqual(report_tags.render_currency(m, decimal_places='a'), exp_m) self.assertEqual(report_tags.render_currency(m, min_decimal_places='a'), exp_m) self.assertEqual(report_tags.render_currency(m, max_decimal_places='a'), exp_m) diff --git a/src/frontend/src/components/forms/ApiForm.tsx b/src/frontend/src/components/forms/ApiForm.tsx index 3e1534544e..632677f691 100644 --- a/src/frontend/src/components/forms/ApiForm.tsx +++ b/src/frontend/src/components/forms/ApiForm.tsx @@ -587,9 +587,11 @@ export function ApiForm({ {nonFieldErrors.length > 0 ? ( - {nonFieldErrors.map((message) => ( - {message} - ))} + {nonFieldErrors + .filter((message) => !!message && message !== 'None') + .map((message) => ( + {message} + ))} ) : ( {t`Errors exist for one or more form fields`}