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 20 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
7 changes: 2 additions & 5 deletions app/forms/fields/decimal_field_with_separator.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
from decimal import Decimal, InvalidOperation

from babel import numbers
from wtforms import DecimalField

from app.settings import DEFAULT_LOCALE
from app.helpers.form_helpers import sanitise_number


class DecimalFieldWithSeparator(DecimalField):
Expand All @@ -23,8 +22,6 @@ def __init__(self, **kwargs):
def process_formdata(self, valuelist):
if valuelist:
try:
self.data = Decimal(
valuelist[0].replace(numbers.get_group_symbol(DEFAULT_LOCALE), "")
)
self.data = Decimal(sanitise_number(valuelist[0]))
except (ValueError, TypeError, InvalidOperation):
pass
7 changes: 2 additions & 5 deletions app/forms/fields/integer_field_with_separator.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from babel import numbers
from wtforms import IntegerField

from app.settings import DEFAULT_LOCALE
from app.helpers.form_helpers import sanitise_number


class IntegerFieldWithSeparator(IntegerField):
Expand All @@ -21,8 +20,6 @@ def __init__(self, **kwargs):
def process_formdata(self, valuelist):
if valuelist:
try:
self.data = int(
valuelist[0].replace(numbers.get_group_symbol(DEFAULT_LOCALE), "")
)
self.data = int(sanitise_number(valuelist[0]))
except ValueError:
pass
19 changes: 8 additions & 11 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 All @@ -19,6 +20,7 @@
DecimalFieldWithSeparator,
IntegerFieldWithSeparator,
)
from app.helpers.form_helpers import sanitise_number
from app.jinja_filters import format_number, get_formatted_currency
from app.questionnaire.questionnaire_store_updater import QuestionnaireStoreUpdater
from app.questionnaire.rules.utils import parse_datetime
Expand Down Expand Up @@ -49,15 +51,13 @@ def __call__(
field: Union[DecimalFieldWithSeparator, IntegerFieldWithSeparator],
) -> None:
try:
Decimal(
field.raw_data[0].replace(
numbers.get_group_symbol(flask_babel.get_locale()), ""
)
)
Decimal(sanitise_number(number=field.raw_data[0]))
except (ValueError, TypeError, InvalidOperation, AttributeError) as exc:
raise validators.StopValidation(self.message) from exc

if "e" in field.raw_data[0].lower():
if "e" in field.raw_data[0].lower() or (
field.data is not None and math.isnan(field.data)
):
raise validators.StopValidation(self.message)


Expand Down Expand Up @@ -126,6 +126,7 @@ def __call__(
field: Union[DecimalFieldWithSeparator, IntegerFieldWithSeparator],
) -> None:
value: Union[int, Decimal] = field.data

if value is not None:
error_message = self.validate_minimum(value) or self.validate_maximum(value)
if error_message:
Expand Down Expand Up @@ -179,11 +180,7 @@ def __init__(self, max_decimals: int = 0, messages: OptionalMessage = None):
def __call__(
self, form: "QuestionnaireForm", field: DecimalFieldWithSeparator
) -> None:
data = (
field.raw_data[0]
.replace(numbers.get_group_symbol(flask_babel.get_locale()), "")
.replace(" ", "")
)
data = sanitise_number(field.raw_data[0])
decimal_symbol = numbers.get_decimal_symbol(flask_babel.get_locale())
if data and decimal_symbol in data:
if self.max_decimals == 0:
Expand Down
10 changes: 10 additions & 0 deletions app/helpers/form_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import flask_babel
from babel import numbers


def sanitise_number(number: str) -> str:
return (
number.replace(numbers.get_group_symbol(flask_babel.get_locale()), "")
.replace("_", "")
.replace(" ", "")
)
41 changes: 41 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 All @@ -21,6 +23,7 @@ def test_text_area_supports_maxlength_property(mock_form):
assert text_area.maxlength == 20


@pytest.mark.usefixtures("gb_locale")
def test_integer_field(mock_form):
integer_field = IntegerFieldWithSeparator(_form=mock_form, name="aName")
assert isinstance(integer_field, Field)
Expand All @@ -31,6 +34,44 @@ def test_integer_field(mock_form):
pytest.fail("Exceptions should not thrown by CustomIntegerField")


@pytest.mark.usefixtures("gb_locale")
@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.usefixtures("gb_locale")
@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


@pytest.mark.usefixtures("gb_locale")
def test_decimal_field(mock_form):
decimal_field = DecimalFieldWithSeparator(_form=mock_form, name="aName")
assert isinstance(decimal_field, Field)
Expand Down
32 changes: 22 additions & 10 deletions tests/app/forms/validation/test_number_validator.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from decimal import Decimal

import pytest
from wtforms.validators import StopValidation, ValidationError

Expand All @@ -7,12 +9,7 @@

@pytest.mark.parametrize(
"value",
(
[None],
[""],
["a"],
["2E2"],
),
([None], [""], ["a"], ["2E2"]),
)
@pytest.mark.usefixtures("gb_locale")
def test_number_validator_raises_StopValidation(
Expand All @@ -26,6 +23,20 @@ def test_number_validator_raises_StopValidation(
assert error_messages["INVALID_NUMBER"] == str(exc.value)


@pytest.mark.usefixtures("gb_locale")
def test_number_validator_NaN_raises_StopValidation(
number_check, mock_form, mock_field
):
value = "NaN"
mock_field.raw_data = [value]
mock_field.data = Decimal(value)

with pytest.raises(StopValidation) as exc:
number_check(mock_form, mock_field)

assert error_messages["INVALID_NUMBER"] == str(exc.value)


@pytest.mark.parametrize(
"decimals, error",
(
Expand All @@ -49,12 +60,13 @@ def test_decimal_validator_raises_StopValidation(
@pytest.mark.parametrize(
"value",
(
["0"],
["10"],
["-10"],
"0",
"10",
"-10",
),
)
@pytest.mark.usefixtures("gb_locale")
def test_number_validator(number_check, value, mock_form, mock_field):
mock_field.raw_data = value
mock_field.raw_data = [value]
mock_field.data = Decimal(value)
number_check(mock_form, mock_field)