Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of invalid values in form numerical inputs #1111

Merged
merged 24 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f0f1f93
Add field changes and tests
petechd May 23, 2023
74db61e
Add tests, fix validation and form handling
petechd May 24, 2023
34ff1ae
Change validation text
petechd May 24, 2023
2553d9e
Add replace and isNaN in correct places
petechd May 25, 2023
700b5f1
Fix imports
petechd May 25, 2023
c7f44db
Change error handling
petechd May 25, 2023
63b3cd2
Merge branch 'main' into fix-invalid-inputs-number-fields
petechd May 25, 2023
9cc7884
Refactor isnan call
petechd May 25, 2023
9b9ab62
Merge branch 'fix-invalid-inputs-number-fields' of github.com:ONSdigi…
petechd May 25, 2023
a58f094
Fix error messages
petechd May 26, 2023
ce22c99
Merge branch 'main' into fix-invalid-inputs-number-fields
petechd May 26, 2023
9aaa6ab
Add review changes
petechd May 26, 2023
1c17821
Merge branch 'fix-invalid-inputs-number-fields' of github.com:ONSdigi…
petechd May 26, 2023
0b254a3
Fix isnan call
petechd May 26, 2023
cac59ce
Add field data check
petechd May 26, 2023
774b446
Remove redundant lines
petechd May 26, 2023
55a3cf0
Merge branch 'main' into fix-invalid-inputs-number-fields
petechd May 26, 2023
766d7c9
Add helper method and test fixes
petechd May 26, 2023
5e44b72
Merge branch 'fix-invalid-inputs-number-fields' of github.com:ONSdigi…
petechd May 26, 2023
1a579ac
Add form helpers module
petechd May 30, 2023
3f23c38
Move all helper functions to form helpers module
petechd May 30, 2023
c6b8d5c
Fix validation and tests
petechd May 30, 2023
5ea912e
Remove unused imports
petechd May 30, 2023
851d353
Remove extra value assignement
petechd May 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/forms/fields/decimal_field_with_separator.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ def process_formdata(self, valuelist):
if valuelist:
try:
self.data = Decimal(
valuelist[0].replace(numbers.get_group_symbol(DEFAULT_LOCALE), "")
valuelist[0]
.replace(numbers.get_group_symbol(DEFAULT_LOCALE), "")
.replace("_", "")
)
except (ValueError, TypeError, InvalidOperation):
pass
4 changes: 3 additions & 1 deletion app/forms/fields/integer_field_with_separator.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ def process_formdata(self, valuelist):
if valuelist:
try:
self.data = int(
valuelist[0].replace(numbers.get_group_symbol(DEFAULT_LOCALE), "")
valuelist[0]
.replace(numbers.get_group_symbol(DEFAULT_LOCALE), "")
.replace("_", "")
)
except ValueError:
pass
22 changes: 19 additions & 3 deletions app/forms/validators.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import math
import re
from datetime import datetime, timezone
from decimal import Decimal, InvalidOperation
Expand Down Expand Up @@ -48,11 +49,17 @@ def __call__(
form: FlaskForm,
field: Union[DecimalFieldWithSeparator, IntegerFieldWithSeparator],
) -> None:
try:
if math.isnan(float(field.raw_data[0])):
raise validators.StopValidation(self.message)
except (TypeError, ValueError):
pass

try:
Decimal(
field.raw_data[0].replace(
numbers.get_group_symbol(flask_babel.get_locale()), ""
)
field.raw_data[0]
.replace(numbers.get_group_symbol(flask_babel.get_locale()), "")
.replace("_", "")
)
except (ValueError, TypeError, InvalidOperation, AttributeError) as exc:
raise validators.StopValidation(self.message) from exc
Expand Down Expand Up @@ -126,6 +133,14 @@ def __call__(
field: Union[DecimalFieldWithSeparator, IntegerFieldWithSeparator],
) -> None:
value: Union[int, Decimal] = field.data
try:
raw_value = float(field.raw_data[0])
except (TypeError, ValueError):
raw_value = False

if math.isnan(raw_value):
raise validators.ValidationError(self.messages["INVALID_NUMBER"])

if value is not None:
error_message = self.validate_minimum(value) or self.validate_maximum(value)
if error_message:
Expand Down Expand Up @@ -183,6 +198,7 @@ def __call__(
field.raw_data[0]
.replace(numbers.get_group_symbol(flask_babel.get_locale()), "")
.replace(" ", "")
.replace("_", "")
)
decimal_symbol = numbers.get_decimal_symbol(flask_babel.get_locale())
if data and decimal_symbol in data:
Expand Down
37 changes: 37 additions & 0 deletions tests/app/forms/test_custom_fields.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import Decimal

import pytest
from wtforms.fields import Field

Expand Down Expand Up @@ -31,6 +33,41 @@ def test_integer_field(mock_form):
pytest.fail("Exceptions should not thrown by CustomIntegerField")


@pytest.mark.parametrize(
"number_input, result",
[
("_110", 110),
("1_10", 110),
("1__10", 110),
("1_1,0", 110),
("_1_1,0,0", 1100),
("1.10", None),
],
)
def test_integer_field_inputs(mock_form, number_input, result):
integer_field = IntegerFieldWithSeparator(_form=mock_form, name="aName")
integer_field.process_formdata([number_input])

assert integer_field.data == result


@pytest.mark.parametrize(
"number_input, result",
[
("1_1,0", Decimal("110")),
("1.10", Decimal("1.1")),
("_1.1_0", Decimal("1.1")),
("_1.1_0,0", Decimal("1.1")),
("_1._1,0,0", Decimal("1.1")),
],
)
def test_decimal_field_inputs(mock_form, number_input, result):
decimal_field = DecimalFieldWithSeparator(_form=mock_form, name="aName")
decimal_field.process_formdata([number_input])

assert decimal_field.data == result


def test_decimal_field(mock_form):
decimal_field = DecimalFieldWithSeparator(_form=mock_form, name="aName")
assert isinstance(decimal_field, Field)
Expand Down
42 changes: 42 additions & 0 deletions tests/app/forms/validation/test_number_range_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@
"NUMBER_TOO_SMALL_EXCLUSIVE",
{"min": "10"},
),
(
10,
100,
"NaN",
False,
True,
"INVALID_NUMBER",
{"max": "100"},
),
(
None,
1000,
Expand Down Expand Up @@ -57,13 +66,45 @@ def test_number_range_validator_raises_ValidationError(
maximum_exclusive=max_excl,
)
mock_field.data = value
mock_field.raw_data = [value]

with pytest.raises(ValidationError) as exc:
validator(mock_form, mock_field)

assert error_messages[error_type] % error_dict == str(exc.value)


@pytest.mark.usefixtures("gb_locale")
def test_number_range_validator_nan_check_passes(mock_form, mock_field, value=None):
validator = NumberRange(
minimum=10,
maximum=100,
minimum_exclusive=False,
maximum_exclusive=False,
)
mock_field.data = value
mock_field.raw_data = [value]

validator(mock_form, mock_field)


@pytest.mark.usefixtures("gb_locale")
def test_number_range_validator_nan_check_raises_exception(
mock_form, mock_field, value="NaN"
):
validator = NumberRange(
minimum=10,
maximum=100,
minimum_exclusive=False,
maximum_exclusive=False,
)
mock_field.data = value
mock_field.raw_data = [value]

with pytest.raises(ValidationError):
validator(mock_form, mock_field)


@pytest.mark.parametrize(
"minimum, maximum, value",
(
Expand All @@ -76,5 +117,6 @@ def test_number_range_validator_raises_ValidationError(
def test_number_range_validator(minimum, maximum, value, mock_form, mock_field):
validator = NumberRange(minimum=minimum, maximum=maximum)
mock_field.data = value
mock_field.raw_data = [value]

validator(mock_form, mock_field)
7 changes: 1 addition & 6 deletions tests/app/forms/validation/test_number_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@

@pytest.mark.parametrize(
"value",
(
[None],
[""],
["a"],
["2E2"],
),
([None], [""], ["a"], ["2E2"], ["NaN"]),
)
@pytest.mark.usefixtures("gb_locale")
def test_number_validator_raises_StopValidation(
Expand Down