From 8fd61996b6bcc0dc10197476900436fb354d7943 Mon Sep 17 00:00:00 2001 From: Ben Millar Date: Mon, 24 Feb 2025 00:41:25 +0000 Subject: [PATCH 1/2] Add AllowedExceptions validator --- app/means_test/forms/savings.py | 3 +- app/means_test/validators.py | 16 ++++++++ .../unit_tests/means_test/test_validators.py | 41 +++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 tests/unit_tests/means_test/test_validators.py diff --git a/app/means_test/forms/savings.py b/app/means_test/forms/savings.py index 7901d8694..9722a6930 100644 --- a/app/means_test/forms/savings.py +++ b/app/means_test/forms/savings.py @@ -4,7 +4,7 @@ from app.means_test.forms import BaseMeansTestForm from app.means_test.widgets import MoneyInput from flask_babel import lazy_gettext as _ -from app.means_test.validators import ValidateIfSession +from app.means_test.validators import ValidateIfSession, AllowedExceptions class SavingsForm(BaseMeansTestForm): @@ -44,6 +44,7 @@ class SavingsForm(BaseMeansTestForm): validators=[ ValidateIfSession("has_valuables", True), InputRequired(message=_("Enter the total of all valuable items over £500")), + AllowedExceptions(0), NumberRange( min=50000, message=_("Enter 0 if you have no valuable items worth over £500 each"), diff --git a/app/means_test/validators.py b/app/means_test/validators.py index 4631d11d7..9fc90b59b 100644 --- a/app/means_test/validators.py +++ b/app/means_test/validators.py @@ -126,3 +126,19 @@ def validate_currency(value: str | None) -> Decimal | None: raise ValueError("Enter a valid amount (maximum 2 decimal places)") return decimal_value + + +class AllowedExceptions: + """Allows for additional allowed values that are accepted but would fail subsequent validation. + This is typically used for when we want to allow £0 or a value greater than £x. + """ + + def __init__(self, allowed_values: list[int | str] | int | str): + # Convert single value to list for consistency + self.allowed_values = ( + [allowed_values] if not isinstance(allowed_values, list) else allowed_values + ) + + def __call__(self, form, field): + if field.data in self.allowed_values: + raise StopValidation() diff --git a/tests/unit_tests/means_test/test_validators.py b/tests/unit_tests/means_test/test_validators.py new file mode 100644 index 000000000..40d830f39 --- /dev/null +++ b/tests/unit_tests/means_test/test_validators.py @@ -0,0 +1,41 @@ +from wtforms import Form, IntegerField +from wtforms.validators import NumberRange +from app.means_test.validators import AllowedExceptions + + +def test_single_allowed_value(): + class TestForm(Form): + amount = IntegerField( + "Amount", validators=[AllowedExceptions(0), NumberRange(min=501)] + ) + + form = TestForm(amount=0) + assert form.validate() + + +def test_list_of_allowed_values(): + class TestForm(Form): + amount = IntegerField( + "Amount", validators=[AllowedExceptions([0, 1, 2]), NumberRange(min=501)] + ) + + # Test each allowed value + for value in [0, 1, 2]: + form = TestForm(amount=value) + assert form.validate() + + +def test_non_allowed_value(): + class TestForm(Form): + amount = IntegerField( + "Amount", validators=[AllowedExceptions([0, 1]), NumberRange(min=500)] + ) + + # Should fail because 3 is neither in allowed values nor >= 500 + form = TestForm(amount=3) + assert form.validate() is False + assert "Number must be at least 500." in form.amount.errors + + # Should pass because 500 meets the NumberRange requirement + form = TestForm(amount=500) + assert form.validate() From 47724bb16460d1ce522afe44ba2c880e67ea26a9 Mon Sep 17 00:00:00 2001 From: Ben Millar Date: Mon, 24 Feb 2025 11:48:58 +0000 Subject: [PATCH 2/2] Refactor to use NumberRangeAllowZero validator --- app/means_test/forms/savings.py | 7 +-- app/means_test/validators.py | 20 +++---- .../unit_tests/means_test/test_validators.py | 59 ++++++++++++------- 3 files changed, 48 insertions(+), 38 deletions(-) diff --git a/app/means_test/forms/savings.py b/app/means_test/forms/savings.py index 9722a6930..bc09d0b62 100644 --- a/app/means_test/forms/savings.py +++ b/app/means_test/forms/savings.py @@ -1,10 +1,10 @@ from flask import session -from wtforms.validators import InputRequired, NumberRange +from wtforms.validators import InputRequired from app.means_test.fields import MoneyField from app.means_test.forms import BaseMeansTestForm from app.means_test.widgets import MoneyInput from flask_babel import lazy_gettext as _ -from app.means_test.validators import ValidateIfSession, AllowedExceptions +from app.means_test.validators import ValidateIfSession, NumberRangeAllowZero class SavingsForm(BaseMeansTestForm): @@ -44,8 +44,7 @@ class SavingsForm(BaseMeansTestForm): validators=[ ValidateIfSession("has_valuables", True), InputRequired(message=_("Enter the total of all valuable items over £500")), - AllowedExceptions(0), - NumberRange( + NumberRangeAllowZero( min=50000, message=_("Enter 0 if you have no valuable items worth over £500 each"), ), # This value is in pence diff --git a/app/means_test/validators.py b/app/means_test/validators.py index 9fc90b59b..649fe2932 100644 --- a/app/means_test/validators.py +++ b/app/means_test/validators.py @@ -3,7 +3,7 @@ from decimal import Decimal, InvalidOperation from enum import Enum from flask import session -from wtforms.validators import StopValidation +from wtforms.validators import StopValidation, NumberRange class ValidateIfType(Enum): @@ -128,17 +128,11 @@ def validate_currency(value: str | None) -> Decimal | None: return decimal_value -class AllowedExceptions: - """Allows for additional allowed values that are accepted but would fail subsequent validation. - This is typically used for when we want to allow £0 or a value greater than £x. - """ - - def __init__(self, allowed_values: list[int | str] | int | str): - # Convert single value to list for consistency - self.allowed_values = ( - [allowed_values] if not isinstance(allowed_values, list) else allowed_values - ) +class NumberRangeAllowZero(NumberRange): + """Number range validator that allows 0 as a valid input.""" def __call__(self, form, field): - if field.data in self.allowed_values: - raise StopValidation() + if field.data == 0: + return + + super().__call__(form, field) diff --git a/tests/unit_tests/means_test/test_validators.py b/tests/unit_tests/means_test/test_validators.py index 40d830f39..8d2bde46c 100644 --- a/tests/unit_tests/means_test/test_validators.py +++ b/tests/unit_tests/means_test/test_validators.py @@ -1,41 +1,58 @@ from wtforms import Form, IntegerField -from wtforms.validators import NumberRange -from app.means_test.validators import AllowedExceptions +from app.means_test.validators import NumberRangeAllowZero -def test_single_allowed_value(): +def test_allows_zero(): class TestForm(Form): - amount = IntegerField( - "Amount", validators=[AllowedExceptions(0), NumberRange(min=501)] - ) + amount = IntegerField("Amount", validators=[NumberRangeAllowZero(min=500)]) form = TestForm(amount=0) assert form.validate() -def test_list_of_allowed_values(): +def test_respects_minimum(): class TestForm(Form): - amount = IntegerField( - "Amount", validators=[AllowedExceptions([0, 1, 2]), NumberRange(min=501)] - ) + amount = IntegerField("Amount", validators=[NumberRangeAllowZero(min=500)]) - # Test each allowed value - for value in [0, 1, 2]: - form = TestForm(amount=value) - assert form.validate() + form = TestForm(amount=499) + assert form.validate() is False + assert "Number must be at least 500." in form.amount.errors + + form = TestForm(amount=501) + assert form.validate() -def test_non_allowed_value(): +def test_respects_maximum(): class TestForm(Form): amount = IntegerField( - "Amount", validators=[AllowedExceptions([0, 1]), NumberRange(min=500)] + "Amount", validators=[NumberRangeAllowZero(min=500, max=1000)] ) - # Should fail because 3 is neither in allowed values nor >= 500 - form = TestForm(amount=3) + # Test zero (should pass) + form = TestForm(amount=0) + assert form.validate() + + # Test value above maximum + form = TestForm(amount=1001) assert form.validate() is False - assert "Number must be at least 500." in form.amount.errors + assert "Number must be between 500 and 1000." in form.amount.errors - # Should pass because 500 meets the NumberRange requirement - form = TestForm(amount=500) + # Test value at maximum + form = TestForm(amount=1000) assert form.validate() + + +def test_non_numeric_value(): + class TestForm(Form): + amount = IntegerField("Amount", validators=[NumberRangeAllowZero(min=500)]) + + form = TestForm(amount="not_a_number") + assert form.validate() is False + + +def test_none_value(): + class TestForm(Form): + amount = IntegerField("Amount", validators=[NumberRangeAllowZero(min=500)]) + + form = TestForm(amount=None) + assert form.validate() is False