From 8fd61996b6bcc0dc10197476900436fb354d7943 Mon Sep 17 00:00:00 2001 From: Ben Millar Date: Mon, 24 Feb 2025 00:41:25 +0000 Subject: [PATCH 1/4] 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 feeff8d390f205de8462b74469c45dc6a2ae7ba6 Mon Sep 17 00:00:00 2001 From: Ben Millar Date: Mon, 24 Feb 2025 01:11:05 +0000 Subject: [PATCH 2/4] Add additional FALA routes to more problems page --- app/templates/categories/more-problems.html | 18 +++++----- .../categories/test_more_problems.py | 35 +++++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 tests/functional_tests/categories/test_more_problems.py diff --git a/app/templates/categories/more-problems.html b/app/templates/categories/more-problems.html index 8a2ca49f5..a42e604e5 100644 --- a/app/templates/categories/more-problems.html +++ b/app/templates/categories/more-problems.html @@ -18,11 +18,11 @@

{{ title }}

{{ list_item_small(_("Adopting a child from outside the UK"), _("Adoption processes in the courts."), - url_for("categories.index")) }} + url_for("find-a-legal-adviser.search", category="mat")) }} {{ list_item_small(_("Appeal a decision that you cannot work with children or vulnerable adults"), _("Including if you’re on a ‘barred list’ or disqualified from teaching."), - url_for("categories.index")) }} + url_for("find-a-legal-adviser.search")) }} {{ list_item_small(_("Anti-social behaviour and gangs"), _("If you’re accused or taken to court for anti-social behaviour, including being in a gang."), @@ -34,7 +34,7 @@

{{ title }}

{{ list_item_small(_("Compensation for abuse, assault or neglect"), _("Includes child abuse, sexual assault, abuse of a vulnerable adult. Claims can be against a person or an organisation."), - url_for("categories.index")) }} + url_for("find-a-legal-adviser.search", category="aap")) }} {{ list_item_small(_("Domestic abuse - if you have been accused"), _("Legal help if you’ve been accused of domestic abuse or forced marriage. Includes non-molestation orders and other court orders."), @@ -42,7 +42,7 @@

{{ title }}

{{ list_item_small(_("Environmental pollution"), _("Issues about air, water or land pollution that is harming you or the environment."), - url_for("categories.index")) }} + url_for("find-a-legal-adviser.search", category="pub")) }} {{ list_item_small(_("Female genital mutilation (FGM)"), _("If you or someone else is at risk of FGM."), @@ -54,23 +54,23 @@

{{ title }}

{{ list_item_small(_("Inquests for family members"), _("Advice to prepare for the inquest of a family member."), - url_for("categories.index")) }} + url_for("find-a-legal-adviser.search")) }} {{ list_item_small(_("Mental health detention"), _("Help if you’re held in hospital (‘sectioned’), mental health tribunals and community treatment orders."), - url_for("categories.index"))}} + url_for("find-a-legal-adviser.search", category="mhe"))}} {{ list_item_small(_("Proceeds of Crime Act"), _("If you’re facing legal action to take your money or other assets."), - url_for("categories.index")) }} + url_for("find-a-legal-adviser.search", category="crm")) }} {{ list_item_small(_("Terrorism"), _("If you’re accused of terrorism or financing terrorist groups."), - url_for("categories.index")) }} + url_for("find-a-legal-adviser.search", category="immas", secondary_category="pub")) }} {{ list_item_small(_("Trafficking, modern slavery"), _("Help if you’re a victim of human trafficking or modern slavery."), - url_for("categories.index")) }} + url_for("find-a-legal-adviser.search", category="immas")) }}
{{ cannot_find_your_problem(None, url_for("categories.results.cannot_find_problem"))}} diff --git a/tests/functional_tests/categories/test_more_problems.py b/tests/functional_tests/categories/test_more_problems.py new file mode 100644 index 000000000..155378b2f --- /dev/null +++ b/tests/functional_tests/categories/test_more_problems.py @@ -0,0 +1,35 @@ +import pytest +from playwright.sync_api import Page + +FALA_ROUTES = [ + # Link text, FALA Primary Category, FALA Secondary Category + ("Adopting a child from outside the UK", "mat", ""), + ( + "Appeal a decision that you cannot work with children or vulnerable adults", + "", + "", + ), + ("Clinical negligence in babies", "med", ""), + ("Compensation for abuse, assault or neglect", "aap", ""), + ("Environmental pollution", "pub", ""), + ("Inquests for family members", "", ""), + ("Mental health detention", "mhe", ""), + ("Proceeds of Crime Act", "crm", ""), + ("Terrorism", "immas", "pub"), + ("Trafficking, modern slavery", "immas", ""), +] + + +@pytest.mark.usefixtures("live_server") +class TestMoreProblems: + @pytest.mark.parametrize("route, primary_category, secondary_category", FALA_ROUTES) + def test_fala_routes( + self, page: Page, route: str, primary_category: str, secondary_category: str + ): + page.get_by_role("link", name="More problems covered by legal aid").click() + page.get_by_role("link", name=route).click() + page.get_by_role("heading", name="Find a legal adviser").is_visible() + if primary_category: + assert f"category={primary_category}" in page.url + if secondary_category: + assert f"secondary_category={secondary_category}" in page.url From 691c59297937e6854c3eb7c90bcee76dfe1515e9 Mon Sep 17 00:00:00 2001 From: Ben Millar Date: Mon, 24 Feb 2025 01:16:59 +0000 Subject: [PATCH 3/4] Add additional are you at risk of harm routes --- app/templates/categories/more-problems.html | 6 +++--- .../categories/test_more_problems.py | 19 +++++++++++++++++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/app/templates/categories/more-problems.html b/app/templates/categories/more-problems.html index a42e604e5..b7f69e4e7 100644 --- a/app/templates/categories/more-problems.html +++ b/app/templates/categories/more-problems.html @@ -38,7 +38,7 @@

{{ title }}

{{ list_item_small(_("Domestic abuse - if you have been accused"), _("Legal help if you’ve been accused of domestic abuse or forced marriage. Includes non-molestation orders and other court orders."), - url_for("categories.index")) }} + url_for("categories.domestic_abuse.are_you_at_risk_of_harm")) }} {{ list_item_small(_("Environmental pollution"), _("Issues about air, water or land pollution that is harming you or the environment."), @@ -46,11 +46,11 @@

{{ title }}

{{ list_item_small(_("Female genital mutilation (FGM)"), _("If you or someone else is at risk of FGM."), - url_for("categories.index")) }} + url_for("categories.domestic_abuse.are_you_at_risk_of_harm")) }} {{ list_item_small(_("Forced marriage"), _("Help with forced marriage and Forced Marriage Protection Orders."), - url_for("categories.index")) }} + url_for("categories.domestic_abuse.are_you_at_risk_of_harm")) }} {{ list_item_small(_("Inquests for family members"), _("Advice to prepare for the inquest of a family member."), diff --git a/tests/functional_tests/categories/test_more_problems.py b/tests/functional_tests/categories/test_more_problems.py index 155378b2f..aaf65f864 100644 --- a/tests/functional_tests/categories/test_more_problems.py +++ b/tests/functional_tests/categories/test_more_problems.py @@ -1,5 +1,5 @@ import pytest -from playwright.sync_api import Page +from playwright.sync_api import Page, expect FALA_ROUTES = [ # Link text, FALA Primary Category, FALA Secondary Category @@ -19,6 +19,12 @@ ("Trafficking, modern slavery", "immas", ""), ] +DOMESTIC_ABUSE_ROUTES = [ + "Domestic abuse - if you have been accused", + "Female genital mutilation (FGM)", + "Forced marriage", +] + @pytest.mark.usefixtures("live_server") class TestMoreProblems: @@ -28,8 +34,17 @@ def test_fala_routes( ): page.get_by_role("link", name="More problems covered by legal aid").click() page.get_by_role("link", name=route).click() - page.get_by_role("heading", name="Find a legal adviser").is_visible() + expect(page.get_by_role("heading", name="Find a legal adviser")).to_be_visible() if primary_category: assert f"category={primary_category}" in page.url if secondary_category: assert f"secondary_category={secondary_category}" in page.url + + @pytest.mark.parametrize("route", DOMESTIC_ABUSE_ROUTES) + def test_domestic_abuse_routes(self, page: Page, route: str): + """These pages route to the "Are you worried about someone's safety?" page in the domestic abuse journey.""" + page.get_by_role("link", name="More problems covered by legal aid").click() + page.get_by_role("link", name=route).click() + expect( + page.get_by_role("heading", name="Are you worried about someone's safety?") + ).to_be_visible() From 70d03b2b6f784c36adf94d103ea1732c81abf506 Mon Sep 17 00:00:00 2001 From: Ben Millar Date: Mon, 24 Feb 2025 01:17:50 +0000 Subject: [PATCH 4/4] Revert "Add AllowedExceptions validator" This reverts commit 8fd61996b6bcc0dc10197476900436fb354d7943. --- app/means_test/forms/savings.py | 3 +- app/means_test/validators.py | 16 -------- .../unit_tests/means_test/test_validators.py | 41 ------------------- 3 files changed, 1 insertion(+), 59 deletions(-) delete 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 9722a6930..7901d8694 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, AllowedExceptions +from app.means_test.validators import ValidateIfSession class SavingsForm(BaseMeansTestForm): @@ -44,7 +44,6 @@ 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 9fc90b59b..4631d11d7 100644 --- a/app/means_test/validators.py +++ b/app/means_test/validators.py @@ -126,19 +126,3 @@ 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 deleted file mode 100644 index 40d830f39..000000000 --- a/tests/unit_tests/means_test/test_validators.py +++ /dev/null @@ -1,41 +0,0 @@ -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()