diff --git a/features/smoked_total_years.feature b/features/smoked_total_years.feature index d5e65938..d23afe30 100644 --- a/features/smoked_total_years.feature +++ b/features/smoked_total_years.feature @@ -5,7 +5,7 @@ Feature: Smoked total years page 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" + And I have answered questions showing I currently smoke "Cigarettes" When I go to "/cigarettes-smoked-total-years" Then there are no accessibility violations @@ -13,7 +13,7 @@ Feature: Smoked total years page 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" + 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" @@ -24,7 +24,7 @@ Feature: Smoked total years page 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" + 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" diff --git a/lung_cancer_screening/questions/forms/smoked_amount_form.py b/lung_cancer_screening/questions/forms/smoked_amount_form.py index 4d2da02a..c40edb6e 100644 --- a/lung_cancer_screening/questions/forms/smoked_amount_form.py +++ b/lung_cancer_screening/questions/forms/smoked_amount_form.py @@ -38,7 +38,12 @@ def more_or_fewer_text(self): def _normal_type_label(self): - return f"Roughly how many {self.type_string()} do you {self._currently_or_previously_text()} smoke in a normal {self.tobacco_smoking_history.frequency_singular()}?" + do_or_did = "do" if self.tobacco_smoking_history.is_current() else "did" + return ( + f"Roughly how many {self.type_string()} {do_or_did} you " + f"{self._currently_or_previously_text()} smoke in a normal " + f"{self.tobacco_smoking_history.frequency_singular()}?" + ) def _changed_type_label(self): @@ -53,11 +58,15 @@ def _type_label(self): def _currently_or_previously_text(self): - return "currently" if self.tobacco_smoking_history.smoking_current_response.value else "previously" + return "currently" if self.tobacco_smoking_history.is_current() else "previously" def _normal_type_required_error_message(self): - return f"Enter how many {self.type_string()} you {self._currently_or_previously_text()} smoke in a normal {self.tobacco_smoking_history.frequency_singular()}" + return ( + f"Enter how many {self.type_string()} you " + f"{self._currently_or_previously_text()} {self.smoke_or_smoked_text()} " + f"in a normal {self.tobacco_smoking_history.frequency_singular()}" + ) def _changed_type_required_error_message(self): @@ -70,9 +79,19 @@ def _type_required_error_message(self): else: return self._normal_type_required_error_message() + def smoke_or_smoked_text(self): + if self.tobacco_smoking_history.is_current(): + return "smoke" + else: + return "smoked" + def _type_min_value_error_message(self): - return f"The number of {self.type_string()} you smoke must be at least 1" + return ( + f"The number of {self.type_string()} you {self.smoke_or_smoked_text()} " + f"a {self.tobacco_smoking_history.frequency_singular()} " + "must be at least 1" + ) class Meta: model = SmokedAmountResponse diff --git a/lung_cancer_screening/questions/forms/smoked_total_years_form.py b/lung_cancer_screening/questions/forms/smoked_total_years_form.py index 7a98f1ec..5508571c 100644 --- a/lung_cancer_screening/questions/forms/smoked_total_years_form.py +++ b/lung_cancer_screening/questions/forms/smoked_total_years_form.py @@ -19,16 +19,66 @@ def __init__(self, tobacco_smoking_history,*args, **kwargs): required=True, suffix="years", error_messages={ - "required": "Enter the number of years you have smoked cigarettes", - "invalid": "Years must be in whole numbers" + "required": self.required_error_message(), + "invalid": "Years must be in whole numbers", + "min_value": self.min_value_error_message(), }, ) + + def normal_required_error_message(self): + return ( + "Enter the number of years you have smoked " + f"{self.tobacco_smoking_history.human_type().lower()}" + ) + + def changed_required_error_message(self): + return ( + "Enter the number of years you smoked " + f"{self.tobacco_smoking_history.to_sentence()}" + ) + + def required_error_message(self): + if self.tobacco_smoking_history.is_normal(): + return self.normal_required_error_message() + + return self.changed_required_error_message() + + + def normal_min_value_error_message(self): + return ( + "The number of years you smoked " + f"{self.tobacco_smoking_history.human_type().lower()} " + "must be at least 1" + ) + + + def changed_min_value_error_message(self): + return ( + "The number of years you smoked " + f"{self.tobacco_smoking_history.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() + + return self.changed_min_value_error_message() + + + def normal_label(self): + return f"Roughly how many years have you smoked {self.tobacco_smoking_history.human_type().lower()}?" + + def changed_label(self): + return f"Roughly how many years did you smoke {self.tobacco_smoking_history.to_sentence()}?" + def label(self): if self.tobacco_smoking_history.is_normal(): - return f"Roughly how many years have you smoked {self.tobacco_smoking_history.human_type().lower()}?" - else: - return f"Roughly how many years did you smoke {self.tobacco_smoking_history.amount()} {self.tobacco_smoking_history.human_type().lower()} a {self.tobacco_smoking_history.frequency_singular()}?" + return self.normal_label() + + return self.changed_label() class Meta: model = SmokedTotalYearsResponse diff --git a/lung_cancer_screening/questions/models/smoked_total_years_response.py b/lung_cancer_screening/questions/models/smoked_total_years_response.py index 2d160de5..43a153db 100644 --- a/lung_cancer_screening/questions/models/smoked_total_years_response.py +++ b/lung_cancer_screening/questions/models/smoked_total_years_response.py @@ -35,6 +35,25 @@ def _validate_age_when_started_smoking_response_exists(self): ) }) + def normal_max_value_error_message(self): + return ( + "The number of years you smoked cigarettes must be fewer " + "than the total number of years you have been smoking" + ) + + def changed_max_value_error_message(self): + return ( + "The number of years you smoked " + f"{self.tobacco_smoking_history.to_sentence()} " + "must be fewer than the total number of years you have been smoking" + ) + + def max_value_error_message(self): + if self.tobacco_smoking_history.is_normal(): + return self.normal_max_value_error_message() + + return self.changed_max_value_error_message() + def _validate_value_fewer_than_total_number_of_years_smoked(self): if not self.value: return None @@ -42,7 +61,7 @@ def _validate_value_fewer_than_total_number_of_years_smoked(self): 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" + self.max_value_error_message(), + code="max" ) }) diff --git a/lung_cancer_screening/questions/models/tobacco_smoking_history.py b/lung_cancer_screening/questions/models/tobacco_smoking_history.py index ea9bfd9a..ffee0dbf 100644 --- a/lung_cancer_screening/questions/models/tobacco_smoking_history.py +++ b/lung_cancer_screening/questions/models/tobacco_smoking_history.py @@ -129,6 +129,9 @@ def duration_years(self): else: return None + def to_sentence(self): + return f"{self.amount()} {self.human_type().lower()} a {self.frequency_singular()}" + def is_increased(self): return self.level == self.Levels.INCREASED @@ -139,8 +142,8 @@ def is_normal(self): return self.level == self.Levels.NORMAL def is_current(self): - if hasattr(self, "smoking_current_response"): - return self.smoking_current_response.value - else: + if not hasattr(self, "smoking_current_response"): return None + return self.is_normal() and self.smoking_current_response.value + diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py index d639fe03..feb31f3e 100644 --- a/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py +++ b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py @@ -17,7 +17,7 @@ def setUp(self): ) self.smoking_current_response = SmokingCurrentResponseFactory.create( tobacco_smoking_history=self.smoking_history, - value=False + value=True ) self.frequency_response = SmokingFrequencyResponseFactory.create( tobacco_smoking_history=self.smoking_history, @@ -55,24 +55,53 @@ def test_is_invalid_with_a_non_numeric_value(self): self.assertIn("value", form.errors) - def test_min_value_validation_has_the_correct_message(self): - self.smoking_history.type = TobaccoSmokingHistoryTypes.CIGARS.value - self.smoking_history.save() + def test_min_value_validation_with_current_smoking_history(self): + form = SmokedAmountForm( + instance=self.response, + tobacco_smoking_history=self.smoking_history, + data={"value": 0} + ) + form.full_clean() + self.assertIn("value", form.errors) + self.assertIn( + "The number of cigarettes you smoke a day must be at least 1", + form.errors["value"], + ) + + + def test_min_value_validation_has_with_previous_smoking_history(self): + self.smoking_current_response.value = False + self.smoking_current_response.save() form = SmokedAmountForm( instance=self.response, tobacco_smoking_history=self.smoking_history, data={"value": 0} ) + form.full_clean() self.assertIn("value", form.errors) self.assertIn( - "The number of cigars you smoke must be at least 1", + "The number of cigarettes you smoked a day must be at least 1", form.errors["value"], ) + def test_has_a_label_for_the_normal_type_current(self): + form = SmokedAmountForm( + instance=self.response, + data={"value": 20}, + tobacco_smoking_history=self.smoking_history + ) + + self.assertEqual( + form.fields["value"].label, + "Roughly how many cigarettes do you currently smoke in a normal day?" + ) + + def test_has_a_label_for_the_current_type_previously(self): + self.smoking_current_response.value = False + self.smoking_current_response.save() - def test_has_a_label_for_the_normal_type(self): form = SmokedAmountForm( instance=self.response, data={"value": 20}, @@ -81,7 +110,7 @@ def test_has_a_label_for_the_normal_type(self): self.assertEqual( form.fields["value"].label, - "Roughly how many cigarettes do you previously smoke in a normal day?" + "Roughly how many cigarettes did you previously smoke in a normal day?" ) @@ -141,10 +170,28 @@ def test_has_a_required_error_message_for_the_normal_type(self): form.full_clean() self.assertIn("value", form.errors) self.assertIn( - "Enter how many cigarettes you previously smoke in a normal day", + "Enter how many cigarettes you currently smoke in a normal day", + form.errors["value"], + ) + + def test_has_a_required_error_message_for_the_normal_type_previously(self): + self.smoking_current_response.value = False + self.smoking_current_response.save() + + form = SmokedAmountForm( + instance=self.response, + data={"value": None}, + tobacco_smoking_history=self.smoking_history + ) + + form.full_clean() + self.assertIn("value", form.errors) + self.assertIn( + "Enter how many cigarettes you previously smoked in a normal day", form.errors["value"], ) + def test_has_a_required_error_message_for_the_increased_type(self): increased_smoking_history = TobaccoSmokingHistoryFactory.create( type=TobaccoSmokingHistoryTypes.CIGARETTES.value, diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_smoked_total_years_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_total_years_form.py index aa76c8ce..b2e14b1c 100644 --- a/lung_cancer_screening/questions/tests/unit/forms/test_smoked_total_years_form.py +++ b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_total_years_form.py @@ -3,6 +3,8 @@ 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 ...factories.smoking_frequency_response_factory import SmokingFrequencyResponseFactory +from ...factories.smoked_amount_response_factory import SmokedAmountResponseFactory from ....models.tobacco_smoking_history import TobaccoSmokingHistoryTypes, TobaccoSmokingHistory from ....forms.smoked_total_years_form import SmokedTotalYearsForm @@ -37,6 +39,7 @@ def test_is_valid_with_a_valid_value(self): self.age_started_smoking_response.years_smoked_including_stopped() - 1 ) + def test_is_invalid_with_a_none_value(self): form = SmokedTotalYearsForm( instance=self.response, @@ -52,6 +55,31 @@ def test_is_invalid_with_a_none_value(self): ) + def test_has_a_required_error_message_when_increased_or_decreased(self): + self.smoking_history.level = TobaccoSmokingHistory.Levels.INCREASED + self.smoking_history.save() + + SmokingFrequencyResponseFactory.create( + tobacco_smoking_history=self.smoking_history, + weekly=True + ) + SmokedAmountResponseFactory.create( + tobacco_smoking_history=self.smoking_history, + value=200 + ) + + form = SmokedTotalYearsForm( + instance=self.response, + tobacco_smoking_history=self.smoking_history, + data={} + ) + + self.assertFalse(form.is_valid()) + self.assertIn( + "Enter the number of years you smoked 200 cigarettes a week", + form.errors["value"] + ) + def test_is_invalid_with_a_non_numeric_value(self): form = SmokedTotalYearsForm( instance=self.response, @@ -66,6 +94,51 @@ def test_is_invalid_with_a_non_numeric_value(self): form.errors["value"] ) + + def test_has_a_min_value_error_by_default(self): + form = SmokedTotalYearsForm( + instance=self.response, + tobacco_smoking_history=self.smoking_history, + data={ + "value": 0 + } + ) + + self.assertFalse(form.is_valid()) + self.assertIn( + "The number of years you smoked cigarettes must be at least 1", + form.errors["value"] + ) + + + def test_has_a_min_value_error_when_increased(self): + self.smoking_history.level = TobaccoSmokingHistory.Levels.INCREASED + self.smoking_history.save() + + SmokingFrequencyResponseFactory.create( + tobacco_smoking_history=self.smoking_history, + weekly=True + ) + SmokedAmountResponseFactory.create( + tobacco_smoking_history=self.smoking_history, + value=200 + ) + + form = SmokedTotalYearsForm( + instance=self.response, + tobacco_smoking_history=self.smoking_history, + data={ + "value": 0 + } + ) + + self.assertFalse(form.is_valid()) + self.assertIn( + "The number of years you smoked 200 cigarettes a week must be at least 1", + form.errors["value"] + ) + + def test_has_a_default_label_for_normal_type(self): form = SmokedTotalYearsForm( instance=self.response, @@ -90,3 +163,52 @@ def has_a_customised_label_for_a_non_normal_type(self): form.label(), "Roughly how many years did you smoke 200 cigarettes a week?" ) + + + def test_has_an_error_message_for_more_than_years_smoked(self): + form = SmokedTotalYearsForm( + instance=self.response, + tobacco_smoking_history=self.smoking_history, + data={ + "value": 100 + } + ) + self.assertFalse(form.is_valid()) + message = ( + "The number of years you smoked cigarettes must be fewer " + "than the total number of years you have been smoking" + ) + self.assertIn( + message, + form.errors["value"] + ) + + def test_has_an_error_message_for_more_than_years_smoked_when_increased(self): + self.smoking_history.level = TobaccoSmokingHistory.Levels.INCREASED + self.smoking_history.save() + + SmokingFrequencyResponseFactory.create( + tobacco_smoking_history=self.smoking_history, + weekly=True + ) + SmokedAmountResponseFactory.create( + tobacco_smoking_history=self.smoking_history, + value=200 + ) + + form = SmokedTotalYearsForm( + instance=self.response, + tobacco_smoking_history=self.smoking_history, + data={ + "value": 100 + } + ) + self.assertFalse(form.is_valid()) + message = ( + "The number of years you smoked 200 cigarettes a week must be fewer " + "than the total number of years you have been smoking" + ) + self.assertIn( + message, + form.errors["value"] + ) diff --git a/lung_cancer_screening/questions/tests/unit/views/test_smoked_total_years.py b/lung_cancer_screening/questions/tests/unit/views/test_smoked_total_years.py index fbf328cd..7d11564f 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_smoked_total_years.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_smoked_total_years.py @@ -7,6 +7,7 @@ from ...factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory from ...factories.age_when_started_smoking_response_factory import AgeWhenStartedSmokingResponseFactory from ...factories.smoking_frequency_response_factory import SmokingFrequencyResponseFactory +from ...factories.smoking_current_response_factory import SmokingCurrentResponseFactory @tag("SmokedTotalYears") @@ -21,6 +22,9 @@ def setUp(self): response_set=self.response_set, cigarettes=True, ) + self.smoking_current_response = SmokingCurrentResponseFactory.create( + tobacco_smoking_history=self.tobacco_smoking_history, + ) def test_redirects_if_the_user_is_not_logged_in(self): @@ -74,6 +78,19 @@ def test_404_when_a_smoking_history_item_does_not_exist_for_the_given_type(self) self.assertEqual(response.status_code, 404) + def test_redirects_to_smoking_current_if_the_user_has_not_answered_smoking_current_and_the_level_is_normal(self): + self.smoking_current_response.delete() + + response = self.client.get(reverse("questions:smoked_total_years", kwargs = { + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower(), + "level": TobaccoSmokingHistory.Levels.NORMAL.value, + })) + + self.assertRedirects(response, reverse("questions:smoking_current", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower(), + }), fetch_redirect_response=False) + + def test_redirects_to_increased_frequency_if_the_user_has_not_answered_frequency_and_the_level_is_changed(self): increased = TobaccoSmokingHistoryFactory.create( response_set=self.response_set, @@ -169,6 +186,9 @@ def setUp(self): response_set=self.response_set, cigarettes=True, ) + self.smoking_current_response = SmokingCurrentResponseFactory.create( + tobacco_smoking_history=self.tobacco_smoking_history, + ) self.valid_params = {"value": 1} @@ -243,6 +263,18 @@ def test_404_when_a_smoking_history_item_does_not_exist_for_the_given_type(self) self.assertEqual(response.status_code, 404) + def test_redirects_to_smoking_current_if_the_user_has_not_answered_smoking_current_and_the_level_is_normal(self): + self.smoking_current_response.delete() + + response = self.client.post(reverse("questions:smoked_total_years", kwargs = { + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower(), + "level": TobaccoSmokingHistory.Levels.NORMAL.value, + }), self.valid_params) + + self.assertRedirects(response, reverse("questions:smoking_current", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower(), + }), fetch_redirect_response=False) + def test_redirects_to_increased_frequency_if_the_user_has_not_answered_frequency_and_the_level_is_changed(self): increased = TobaccoSmokingHistoryFactory.create( response_set=self.response_set, diff --git a/lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py b/lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py index b8d38e69..9ee42247 100644 --- a/lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py +++ b/lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py @@ -18,10 +18,18 @@ def dispatch(self, request, *args, **kwargs): return super().dispatch(request, *args, **kwargs) + + def get_kwargs(self, prerequisite_response): + kwargs = self.kwargs.copy() + if prerequisite_response == "smoking_current_response": + kwargs.pop("level", None) + return kwargs + + def get_redirect_url(self, prerequisite_response): return reverse( self.RESPONSE_URL_MAPPING[prerequisite_response], - kwargs=self.kwargs, + kwargs=self.get_kwargs(prerequisite_response), query=self.change_query_params() ) diff --git a/lung_cancer_screening/questions/views/smoked_amount.py b/lung_cancer_screening/questions/views/smoked_amount.py index 8ff972ef..66cb5c50 100644 --- a/lung_cancer_screening/questions/views/smoked_amount.py +++ b/lung_cancer_screening/questions/views/smoked_amount.py @@ -54,10 +54,6 @@ def get_form_kwargs(self): return kwargs - def get_object_parent(self): - return self.tobacco_smoking_history_item() - - def prerequisite_responses(self): result = [ "smoking_frequency_response", diff --git a/lung_cancer_screening/questions/views/smoked_total_years.py b/lung_cancer_screening/questions/views/smoked_total_years.py index 532f017c..585d53d2 100644 --- a/lung_cancer_screening/questions/views/smoked_total_years.py +++ b/lung_cancer_screening/questions/views/smoked_total_years.py @@ -81,12 +81,10 @@ def get_back_link_url(self): def has_decreased_level(self): return self.request.response_set.tobacco_smoking_history.cigarettes().decreased().exists() - def get_object_parent(self): - return self.tobacco_smoking_history_item() def prerequisite_responses(self): if self.tobacco_smoking_history_item().is_normal(): - return [] + return ["smoking_current_response"] return [ "smoking_frequency_response",