Merge pull request #336 from NHSDigital/PPHA-589-fix-missing-dynamic-type-names-in-smoking-journey

PPHA-589: Handle past and present in multipe smoking types

Author: Themitchell

features/smoked_total_years.feature

@@ -6,6 +6,7 @@ Feature: Smoked total years page
     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"
+    And I have answered questions showing I currently smoke "Cigarettes"
     When I go to "/cigarettes-smoked-total-years"
     Then there are no accessibility violations
 
@@ -14,6 +15,7 @@ Feature: Smoked total years page
     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"
+    And I have answered questions showing I currently smoke "Cigarettes"
     When I go to "/cigarettes-smoked-total-years"
     And I click "Continue"
     Then I am on "/cigarettes-smoked-total-years"
@@ -25,6 +27,7 @@ Feature: Smoked total years page
     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"
+    And I have answered questions showing I currently smoke "Cigarettes"
     When I go to "/cigarettes-smoked-total-years"
     Then I see a back link to "/cigarettes-smoking-current"
     When I fill in "Roughly how many years have you smoked cigarettes?" with "9"

features/smoking_frequency.feature

@@ -5,13 +5,15 @@ Feature: Smoking frequency page
     Given I am logged in
     And I have answered questions showing I am eligible
     And I have answered questions showing I have smoked "Cigarettes"
+    And I have answered questions showing I currently smoke "Cigarettes"
     When I go to "/cigarettes-smoking-frequency"
     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 "Cigarettes"
+    And I have answered questions showing I currently smoke "Cigarettes"
     When I go to "/cigarettes-smoking-frequency"
     And I click "Continue"
     Then I am on "/cigarettes-smoking-frequency"

features/steps/preflight_steps.py

@@ -147,6 +147,5 @@ def given_i_have_answered_questions_showing_i_have_changed_frequency_of_smoked_t
 def given_i_have_answered_questions_showing_i_have_changed_amount_of_smoked_tobacco_type(context, level, tobacco_type, amount):
     given_i_have_answered_questions_showing_i_have_smoked_tobacco_type(context, tobacco_type)
     context.page.goto(f"{context.live_server_url}/{tobacco_type.lower()}-smoked-{level}-amount")
-    screenshot(context)
     when_i_fill_in_label_with_value(context, f"roughly how many {tobacco_type.lower()} did you normally smoke a", amount, exact=False)
     when_i_submit_the_form(context)

lung_cancer_screening/questions/forms/mixins/__init__.py

@@ -0,0 +1,13 @@
+from functools import cached_property
+
+from lung_cancer_screening.questions.presenters.tobacco_smoking_history_presenter import TobaccoSmokingHistoryPresenter
+
+class SmokingFormPresenter:
+    @cached_property
+    def presenter(self):
+        return TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history)
+
+    @cached_property
+    def normal_presenter(self):
+        return TobaccoSmokingHistoryPresenter(self.normal_tobacco_smoking_history)
+

lung_cancer_screening/questions/forms/mixins/smoking_form_presenter.py

@@ -3,8 +3,10 @@
 from ...nhsuk_forms.integer_field import IntegerField
 from ..models.smoked_amount_response import SmokedAmountResponse
 
+from .mixins.smoking_form_presenter import SmokingFormPresenter
 
-class SmokedAmountForm(forms.ModelForm):
+
+class SmokedAmountForm(SmokingFormPresenter, forms.ModelForm):
 
     def __init__(self, tobacco_smoking_history, normal_tobacco_smoking_history = None, *args, **kwargs):
         self.tobacco_smoking_history = tobacco_smoking_history
@@ -13,83 +15,77 @@ def __init__(self, tobacco_smoking_history, normal_tobacco_smoking_history = Non
         super().__init__(*args, **kwargs)
 
         self.fields["value"] = IntegerField(
-            label=self._type_label(),
+            label=self.label(),
             label_classes="nhsuk-label--m" if self.tobacco_smoking_history.is_normal() else "nhsuk-label--l",
             label_is_page_heading=not self.tobacco_smoking_history.is_normal(),
             classes="nhsuk-input--width-4",
             hint="Give an estimate if you are not sure",
             suffix=self.suffix(),
             required=True,
             error_messages={
-                "required": self._type_required_error_message(),
-                "min_value": self._type_min_value_error_message()
+                "required": self.required_error_message(),
+                "min_value": self.min_value_error_message()
             },
         )
 
 
-    def type_string(self):
-        return self.tobacco_smoking_history.human_type().lower()
-
-    def more_or_fewer_text(self):
-        if self.tobacco_smoking_history.is_increased():
-            return "more"
-        elif self.tobacco_smoking_history.is_decreased():
-            return "fewer"
-
-    def amount_prefix(self):
-        return "grams of " if self.tobacco_smoking_history.is_rolling_tobacco() else ""
-
     def suffix(self):
-        return "grams" if self.tobacco_smoking_history.is_rolling_tobacco() else self.tobacco_smoking_history.unit()
+        return "grams" if self.tobacco_smoking_history.is_rolling_tobacco() else self.presenter.unit()
 
 
-    def _normal_type_label(self):
-        return f"Roughly how many {self.tobacco_smoking_history.unit()} do you {self._currently_or_previously_text()} smoke in a normal {self.tobacco_smoking_history.frequency_singular()}?"
+    def normal_label(self):
+        return (
+            f"Roughly how many {self.presenter.unit()} "
+            f"{self.presenter.do_or_did()} you "
+            f"{self.presenter.currently_or_previously()} smoke in a normal "
+            f"{self.presenter.frequency()}?"
+        )
 
 
-    def _changed_type_label(self):
+    def changed_label(self):
         return (
-            f"When you smoked {self.more_or_fewer_text()} than "
-            f"{self.normal_tobacco_smoking_history.amount()} {self.tobacco_smoking_history.unit()} "
-            f"a {self.normal_tobacco_smoking_history.frequency_singular()}, "
-            f"roughly how many {self.tobacco_smoking_history.unit()} "
-            f"did you normally smoke a {self.tobacco_smoking_history.frequency_singular()}?"
+            f"When you smoked {self.presenter.more_or_fewer()} than "
+            f"{self.normal_presenter.to_sentence()}, "
+            f"roughly how many {self.presenter.unit()} "
+            f"did you normally smoke a {self.presenter.frequency()}?"
         )
 
 
-    def _type_label(self):
-        if self.normal_tobacco_smoking_history:
-            return self._changed_type_label()
+    def label(self):
+        if self.tobacco_smoking_history.is_normal():
+            return self.normal_label()
         else:
-            return self._normal_type_label()
+            return self.changed_label()
 
 
-    def _currently_or_previously_text(self):
-        return "currently" if self.tobacco_smoking_history.smoking_current_response.value else "previously"
-
-
-    def _normal_type_required_error_message(self):
-        return f"Enter how many {self.amount_prefix()}{self.type_string()} you {self._currently_or_previously_text()} smoke in a normal {self.tobacco_smoking_history.frequency_singular()}"
-
+    def normal_required_error_message(self):
+        return (
+            f"Enter how many {self.presenter.unit()} you "
+            f"{self.presenter.currently_or_previously()} {self.presenter.smoke_or_smoked()} "
+            f"in a normal {self.presenter.frequency()}"
+        )
 
-    def _changed_type_required_error_message(self):
+    def changed_required_error_message(self):
         return (
-            f"Enter the number of {self.amount_prefix()}{self.type_string()} "
-            f"you smoked when you smoked {self.more_or_fewer_text()} than "
-            f"{self.normal_tobacco_smoking_history.amount()} {self.amount_prefix()}{self.type_string()} "
-            f"a {self.normal_tobacco_smoking_history.frequency_singular()}"
+            f"Enter the number of {self.presenter.unit()} "
+            f"you smoked when you smoked {self.presenter.more_or_fewer()} than "
+            f"{self.normal_presenter.to_sentence()}"
         )
 
 
-    def _type_required_error_message(self):
+    def required_error_message(self):
         if self.normal_tobacco_smoking_history:
-            return self._changed_type_required_error_message()
+            return self.changed_required_error_message()
         else:
-            return self._normal_type_required_error_message()
+            return self.normal_required_error_message()
 
 
-    def _type_min_value_error_message(self):
-        return f"The number of {self.type_string()} you smoke must be at least 1"
+    def min_value_error_message(self):
+        return (
+            f"The number of {self.presenter.unit()} you "
+            f"{self.presenter.smoke_or_smoked()} a {self.presenter.frequency()} "
+            "must be at least 1"
+        )
 
     class Meta:
         model = SmokedAmountResponse

lung_cancer_screening/questions/forms/smoked_amount_form.py

@@ -2,11 +2,11 @@
 
 from ...nhsuk_forms.integer_field import IntegerField
 from ..models.smoked_total_years_response import SmokedTotalYearsResponse
+from .mixins.smoking_form_presenter import SmokingFormPresenter
 
 
-class SmokedTotalYearsForm(forms.ModelForm):
-
-    def __init__(self, tobacco_smoking_history,*args, **kwargs):
+class SmokedTotalYearsForm(SmokingFormPresenter, forms.ModelForm):
+    def __init__(self, tobacco_smoking_history, *args, **kwargs):
         self.tobacco_smoking_history = tobacco_smoking_history
         super().__init__(*args, **kwargs)
 
@@ -20,35 +20,97 @@ def __init__(self, tobacco_smoking_history,*args, **kwargs):
             suffix="years",
             error_messages={
                 "required": self.required_error_message(),
-                "invalid": "Years must be in whole numbers"
+                "invalid": "Years must be in whole numbers",
+                "min_value": self.min_value_error_message(),
+                "max_value": self.greater_than_years_smoked_error_message()
             },
         )
 
+
     def normal_label(self):
         return (
-            f"Roughly how many years have you smoked "
-            f"{self.tobacco_smoking_history.human_type().lower()}?"
+            f"Roughly how many years {self.presenter.have_you_smoked_or_did_you_smoke()} "
+            f"{self.presenter.human_type().lower()}?"
         )
 
+
     def changed_label(self):
         return (
-            f"Roughly how many years did you smoke {self.tobacco_smoking_history.amount()} "
-            f"{self.tobacco_smoking_history.unit()} "
-            f"a {self.tobacco_smoking_history.frequency_singular()}?"
+            f"Roughly how many years did you smoke {self.presenter.to_sentence()}?"
         )
 
+
     def label(self):
         if self.tobacco_smoking_history.is_normal():
             return self.normal_label()
         else:
             return self.changed_label()
 
+
+    def normal_required_error_message(self):
+        have_text = "have " if self.presenter.is_present_tense() else ""
+        return (
+            f"Enter the number of years you {have_text}smoked "
+            f"{self.presenter.human_type().lower()}"
+        )
+
+
+    def changed_required_error_message(self):
+        return (
+            "Enter the number of years you smoked "
+            f"{self.presenter.to_sentence()}"
+        )
+
+
     def required_error_message(self):
+        if self.tobacco_smoking_history.is_normal():
+            return self.normal_required_error_message()
+        else:
+            return self.changed_required_error_message()
+
+
+    def normal_min_value_error_message(self):
+        return (
+            "The number of years you smoked "
+            f"{self.presenter.human_type().lower()} must be at least 1"
+        )
+
+
+    def changed_min_value_error_message(self):
+        return (
+            "The number of years you smoked "
+            f"{self.presenter.to_sentence()} must be at least 1"
+        )
+
+
+    def min_value_error_message(self):
+        if self.tobacco_smoking_history.is_normal():
+            return self.normal_min_value_error_message()
+        else:
+            return self.changed_min_value_error_message()
+
+
+    def normal_greater_than_years_smoked_error_message(self):
         return (
-            "Enter the number of years you have smoked "
-            f"{self.tobacco_smoking_history.unit()}"
+            f"The number of years you smoked {self.presenter.human_type().lower()} "
+            "must equal to, or fewer than, the total number of years you smoked"
         )
 
+
+    def changed_greater_than_years_smoked_error_message(self):
+        return (
+            f"The number of years you smoked {self.presenter.to_sentence()} "
+            "must equal to, or fewer than, the total number of years you have been smoking"
+        )
+
+
+    def greater_than_years_smoked_error_message(self):
+        if self.tobacco_smoking_history.is_normal():
+            return self.normal_greater_than_years_smoked_error_message()
+        else:
+            return self.changed_greater_than_years_smoked_error_message()
+
+
     class Meta:
         model = SmokedTotalYearsResponse
         fields = ['value']