PPHA-529: Smoked total years validation on age started

Ensure that the smoked total years for a type of tobacco cannot be
longer than the overall total years smoked based on their age when
started

Author: Themitchell

features/smoked_total_years.feature

@@ -3,13 +3,15 @@ Feature: Smoked total years page
   Scenario: The page is accessible
     Given I am logged in
     And I have answered questions showing I am eligible
+    And I have answered questions showing I have smoked for "10" years
     And I have answered questions showing I have smoked "Cigarettes"
     When I go to "/cigarettes-smoked-total-years"
     Then there are no accessibility violations
 
   Scenario: Form errors
     Given I am logged in
     And I have answered questions showing I am eligible
+    And I have answered questions showing I have smoked for "10" years
     And I have answered questions showing I have smoked "Cigarettes"
     When I go to "/cigarettes-smoked-total-years"
     And I click "Continue"
@@ -20,27 +22,29 @@ Feature: Smoked total years page
   Scenario: Navigating backwards and forwards
     Given I am logged in
     And I have answered questions showing I am eligible
+    And I have answered questions showing I have smoked for "10" years
     And I have answered questions showing I have smoked "Cigarettes"
     When I go to "/cigarettes-smoked-total-years"
     Then I see a back link to "/types-tobacco-smoking"
-    When I fill in "Roughly how many years have you smoked cigarettes?" with "10"
+    When I fill in "Roughly how many years have you smoked cigarettes?" with "9"
     And I submit the form
     Then I am on "/check-your-answers"
 
   Scenario: Checking responses and changing them
     Given I am logged in
     And I have answered questions showing I am eligible
+    And I have answered questions showing I have smoked for "10" years
     And I have answered questions showing I have smoked "Cigarettes"
     When I go to "/cigarettes-smoked-total-years"
-    When I fill in "Roughly how many years have you smoked cigarettes?" with "10"
+    When I fill in "Roughly how many years have you smoked cigarettes?" with "9"
     And I submit the form
     When I go to "/check-your-answers"
-    Then I see "10" as a response to "Total number of years you have smoked cigarettes" under "Smoking history"
+    Then I see "9" as a response to "Total number of years you have smoked cigarettes" under "Smoking history"
     And I see "/cigarettes-smoked-total-years?change=True" as a link to change "Total number of years you have smoked cigarettes" under "Smoking history"
     When I click the link to change "Total number of years you have smoked cigarettes" under "Smoking history"
     Then I am on "/cigarettes-smoked-total-years?change=True"
-    And I see "10" filled in for "Roughly how many years have you smoked cigarettes?"
-    When I fill in "Roughly how many years have you smoked cigarettes?" with "20"
+    And I see "9" filled in for "Roughly how many years have you smoked cigarettes?"
+    When I fill in "Roughly how many years have you smoked cigarettes?" with "8"
     And I click "Continue"
     Then I am on "/check-your-answers"
-    And I see "20" as a response to "Total number of years you have smoked cigarettes" under "Smoking history"
+    And I see "8" as a response to "Total number of years you have smoked cigarettes" under "Smoking history"

features/steps/preflight_steps.py

@@ -87,4 +87,5 @@ def given_i_have_answered_questions_showing_i_have_smoked_tobacco_type(
 
     for tobacco_type in tobacco_types.split(","):
         when_i_check_label(context, tobacco_type.strip())
+
     when_i_submit_the_form(context)

features/types_tobacco_smoking.feature

@@ -3,12 +3,14 @@ Feature: Types tobacco smoking page
   Scenario: The page is accessible
     Given I am logged in
     And I have answered questions showing I am eligible
+    And I have answered questions showing I have smoked for "10" years
     When I go to "/types-tobacco-smoking"
     Then there are no accessibility violations
 
   Scenario: Form errors
     Given I am logged in
     And I have answered questions showing I am eligible
+    And I have answered questions showing I have smoked for "10" years
     When I go to "/types-tobacco-smoking"
     And I click "Continue"
     Then I am on "/types-tobacco-smoking"
@@ -18,6 +20,7 @@ Feature: Types tobacco smoking page
   Scenario: Navigating backwards and forwards
     Given I am logged in
     And I have answered questions showing I am eligible
+    And I have answered questions showing I have smoked for "10" years
     When I go to "/types-tobacco-smoking"
     Then I see a back link to "/periods-when-you-stopped-smoking"
     When I check "Cigarettes"
@@ -27,6 +30,7 @@ Feature: Types tobacco smoking page
   Scenario: Checking responses and changing them
     Given I am logged in
     And I have answered questions showing I am eligible
+    And I have answered questions showing I have smoked for "10" years
     When I go to "/types-tobacco-smoking"
     And I check "Cigarettes"
     And I check "Cigars"

lung_cancer_screening/questions/jinja2/question_form.jinja

@@ -9,7 +9,7 @@
 {% block page_content %}
   <div class="nhsuk-grid-row">
     <div class="nhsuk-grid-column-two-thirds">
-      <form action="{{ request.path }}" method="POST">
+      <form action="{{ request.path }}" method="POST" novalidate>
         {{ csrf_input }}
 
         {% if request.GET.get("change") == "True" %}

lung_cancer_screening/questions/models/smoked_total_years_response.py

@@ -1,4 +1,6 @@
 from django.db import models
+from django.core.exceptions import ValidationError
+from django.core.validators import MinValueValidator
 
 from .base import BaseModel
 from .tobacco_smoking_history import TobaccoSmokingHistory
@@ -10,4 +12,37 @@ class SmokedTotalYearsResponse(BaseModel):
         on_delete=models.CASCADE,
         related_name='smoked_total_years_response'
     )
-    value = models.IntegerField()
+
+    value = models.IntegerField(
+        validators=[
+            MinValueValidator(1, message="The number of years you smoked cigarettes must be at least 1")
+        ]
+    )
+
+
+    def clean(self):
+        super().clean()
+        self._validate_age_when_started_smoking_response_exists()
+        self._validate_value_fewer_than_total_number_of_years_smoked()
+
+
+    def _validate_age_when_started_smoking_response_exists(self):
+        if not hasattr(self.tobacco_smoking_history.response_set, 'age_when_started_smoking_response'):
+            raise ValidationError({
+                "value": ValidationError(
+                    "You must answer age when started smoking before answering how many years you have smoked cigarettes",
+                    code="age_when_started_smoking_response_not_found"
+                )
+            })
+
+    def _validate_value_fewer_than_total_number_of_years_smoked(self):
+        if not self.value:
+            return None
+
+        if self.value > self.tobacco_smoking_history.response_set.age_when_started_smoking_response.years_smoked_including_stopped():
+            raise ValidationError({
+                "value": ValidationError(
+                    "The number of years you smoked cigarettes must be fewer than the total number of years you have been smoking",
+                    code="value_greater_than_total_number_of_years_smoked"
+                )
+            })

lung_cancer_screening/questions/tests/factories/age_when_started_smoking_response_factory.py

@@ -8,7 +8,7 @@
 def calculate_value(instance):
     if not hasattr(instance.response_set, 'date_of_birth_response'):
         return None
-    return random.randint(1, instance.response_set.date_of_birth_response.age_in_years())
+    return random.randint(10, instance.response_set.date_of_birth_response.age_in_years())
 
 
 class AgeWhenStartedSmokingResponseFactory(factory.django.DjangoModelFactory):

lung_cancer_screening/questions/tests/factories/smoked_total_years_response_factory.py

@@ -9,4 +9,4 @@ class Meta:
         model = SmokedTotalYearsResponse
 
     tobacco_smoking_history = factory.SubFactory(TobaccoSmokingHistoryFactory)
-    value = 10
+    value = 1

lung_cancer_screening/questions/tests/unit/forms/test_smoked_total_years_form.py

@@ -2,6 +2,7 @@
 
 from ...factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory
 from ...factories.smoked_total_years_response_factory import SmokedTotalYearsResponseFactory
+from ...factories.age_when_started_smoking_response_factory import AgeWhenStartedSmokingResponseFactory
 from ....models.tobacco_smoking_history import TobaccoSmokingHistoryTypes
 from ....forms.smoked_total_years_form import SmokedTotalYearsForm
 
@@ -12,6 +13,9 @@ def setUp(self):
         self.smoking_history = TobaccoSmokingHistoryFactory.create(
             type=TobaccoSmokingHistoryTypes.CIGARETTES.value
         )
+        self.age_started_smoking_response = AgeWhenStartedSmokingResponseFactory.create(
+            response_set=self.smoking_history.response_set
+        )
         self.response = SmokedTotalYearsResponseFactory.build(
             tobacco_smoking_history=self.smoking_history
         )
@@ -21,13 +25,13 @@ def test_is_valid_with_a_valid_value(self):
         form = SmokedTotalYearsForm(
             instance=self.response,
             data={
-                "value": 10
+                "value": self.age_started_smoking_response.years_smoked_including_stopped() - 1
             }
         )
         self.assertTrue(form.is_valid())
         self.assertEqual(
             form.cleaned_data["value"],
-            10
+            self.age_started_smoking_response.years_smoked_including_stopped() - 1
         )
 
     def test_is_invalid_with_a_none_value(self):
@@ -38,11 +42,12 @@ def test_is_invalid_with_a_none_value(self):
             }
         )
         self.assertFalse(form.is_valid())
-        self.assertEqual(
-            form.errors["value"],
-            ["Enter the number of years you have smoked cigarettes"]
+        self.assertIn(
+            "Enter the number of years you have smoked cigarettes",
+            form.errors["value"]
         )
 
+
     def test_is_invalid_with_a_non_numeric_value(self):
         form = SmokedTotalYearsForm(
             instance=self.response,
@@ -51,7 +56,7 @@ def test_is_invalid_with_a_non_numeric_value(self):
             }
         )
         self.assertFalse(form.is_valid())
-        self.assertEqual(
-            form.errors["value"],
-            ["Years must be in whole numbers"]
+        self.assertIn(
+            "Years must be in whole numbers",
+            form.errors["value"]
         )

lung_cancer_screening/questions/tests/unit/models/test_smoked_total_years_response.py

@@ -1,16 +1,24 @@
 from django.test import TestCase, tag
+from django.core.exceptions import ValidationError
 
 from ...factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory
 from ...factories.smoked_total_years_response_factory import SmokedTotalYearsResponseFactory
+from ...factories.age_when_started_smoking_response_factory import AgeWhenStartedSmokingResponseFactory
 
 
 @tag("SmokedTotalYears")
 class TestSmokedTotalYearsResponse(TestCase):
     def setUp(self):
         self.tobacco_smoking_history = TobaccoSmokingHistoryFactory()
+        self.age_started_smoking_response = AgeWhenStartedSmokingResponseFactory.create(
+            response_set=self.tobacco_smoking_history.response_set
+        )
+
 
     def test_has_a_valid_factory(self):
-        model = SmokedTotalYearsResponseFactory.build(tobacco_smoking_history=self.tobacco_smoking_history)
+        model = SmokedTotalYearsResponseFactory.build(
+            tobacco_smoking_history=self.tobacco_smoking_history
+        )
         model.full_clean()
 
 
@@ -29,3 +37,50 @@ def test_has_value_as_int(self):
         )
 
         self.assertIsInstance(response.value, int)
+
+
+    def test_is_invalid_if_the_value_is_less_than_0(self):
+        response = SmokedTotalYearsResponseFactory.build(
+            tobacco_smoking_history=self.tobacco_smoking_history,
+            value=0
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn("value", context.exception.message_dict)
+        self.assertIn("The number of years you smoked cigarettes must be at least 1", context.exception.message_dict["value"])
+
+
+    def test_is_invalid_if_there_is_no_age_when_started_smoking_response(self):
+        self.age_started_smoking_response.delete()
+
+        response = SmokedTotalYearsResponseFactory.build(
+            tobacco_smoking_history=self.tobacco_smoking_history,
+            value=1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn("value", context.exception.message_dict)
+        self.assertIn(
+            "You must answer age when started smoking before answering how many years you have smoked cigarettes",
+            context.exception.message_dict["value"]
+        )
+
+
+    def test_is_invalid_if_the_value_is_greater_than_the_years_smoked_overall(self):
+        response = SmokedTotalYearsResponseFactory.build(
+            tobacco_smoking_history=self.tobacco_smoking_history,
+            value=self.age_started_smoking_response.years_smoked_including_stopped() + 1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn("value", context.exception.message_dict)
+        self.assertIn(
+            "The number of years you smoked cigarettes must be fewer than the total number of years you have been smoking",
+            context.exception.message_dict["value"]
+        )

lung_cancer_screening/questions/tests/unit/presenters/test_response_set_presenter.py

@@ -301,11 +301,15 @@ def test_smoking_history_types_responses_items_sets_the_correct_key(self):
 
 
     def test_smoking_history_types_responses_items_sets_the_correct_value(self):
+        AgeWhenStartedSmokingResponseFactory.create(
+            response_set=self.response_set,
+            value=10
+        )
         cigarettes = TobaccoSmokingHistoryFactory.create(
             response_set=self.response_set, type=TobaccoSmokingHistoryTypes.CIGARETTES
         )
         smoked_total_years_response = SmokedTotalYearsResponseFactory.create(
-            tobacco_smoking_history=cigarettes,
+            tobacco_smoking_history=cigarettes
         )
 
         presenter = ResponseSetPresenter(self.response_set)