From b04c745d5c0b9820dbe50d5dde88f1b1b68add12 Mon Sep 17 00:00:00 2001 From: stephhou Date: Wed, 1 Apr 2026 13:09:43 +0100 Subject: [PATCH 1/4] fix: Changed fewer to less for past rolling tobacco smoking. --- features/smoking_history.feature | 4 ++-- .../questions/forms/smoked_amount_form.py | 4 ++-- .../questions/forms/smoking_change_form.py | 14 +++++++++----- .../questions/forms/smoking_frequency_form.py | 4 ++-- .../tobacco_smoking_history_presenter.py | 5 ++++- .../test_tobacco_smoking_history_presenter.py | 10 +++++----- 6 files changed, 24 insertions(+), 17 deletions(-) diff --git a/features/smoking_history.feature b/features/smoking_history.feature index 711a93d8..c5324f37 100644 --- a/features/smoking_history.feature +++ b/features/smoking_history.feature @@ -184,7 +184,7 @@ Feature: Smoking history pages Then I am on "/rolling-tobacco-smoking-change" When I check "Yes, I used to smoke more than 25 grams of rolling tobacco a week" - And I check "Yes, I used to smoke fewer than 25 grams of rolling tobacco a week" + And I check "Yes, I used to smoke less than 25 grams of rolling tobacco a week" And I submit the form Then I am on "/rolling-tobacco-smoking-increased-frequency" @@ -205,7 +205,7 @@ Feature: Smoking history pages And I submit the form Then I am on "/rolling-tobacco-smoked-decreased-amount" - When I fill in "When you smoked fewer than 25 grams of rolling tobacco a week, roughly how many grams of rolling tobacco did you normally smoke a month?" with "5" + When I fill in "When you smoked less than 25 grams of rolling tobacco a week, roughly how many grams of rolling tobacco did you normally smoke a month?" with "5" And I submit the form Then I am on "/rolling-tobacco-smoked-decreased-years" diff --git a/lung_cancer_screening/questions/forms/smoked_amount_form.py b/lung_cancer_screening/questions/forms/smoked_amount_form.py index 04ac2f6b..58f761e7 100644 --- a/lung_cancer_screening/questions/forms/smoked_amount_form.py +++ b/lung_cancer_screening/questions/forms/smoked_amount_form.py @@ -45,7 +45,7 @@ def normal_label(self): def changed_label(self): return ( - f"When you smoked {self.presenter.more_or_fewer()} than " + f"When you smoked {self.presenter.more_or_fewer_or_less()} than " f"{self.normal_presenter.to_sentence()}, " f"roughly how many {self.presenter.unit()} " f"did you normally smoke a {self.presenter.frequency()}?" @@ -69,7 +69,7 @@ def normal_required_error_message(self): def changed_required_error_message(self): return ( f"Enter the number of {self.presenter.unit()} " - f"you smoked when you smoked {self.presenter.more_or_fewer()} than " + f"you smoked when you smoked {self.presenter.more_or_fewer_or_less()} than " f"{self.normal_presenter.to_sentence()}" ) diff --git a/lung_cancer_screening/questions/forms/smoking_change_form.py b/lung_cancer_screening/questions/forms/smoking_change_form.py index 04e0d110..ae99df05 100644 --- a/lung_cancer_screening/questions/forms/smoking_change_form.py +++ b/lung_cancer_screening/questions/forms/smoking_change_form.py @@ -1,7 +1,7 @@ from django import forms from ...nhsuk_forms.choice_field import MultipleChoiceField -from ..models.tobacco_smoking_history import TobaccoSmokingHistory +from ..models.tobacco_smoking_history import TobaccoSmokingHistory, TobaccoSmokingHistoryTypes from .mixins.smoking_form_presenter import SmokingFormPresenter @@ -64,10 +64,14 @@ def generate_label(self, value, label): if value == TobaccoSmokingHistory.Levels.NO_CHANGE: return label - return ( - f"{TobaccoSmokingHistory.Levels(value).label} " - f" than {self.presenter.to_sentence()}" - ) + if (value == TobaccoSmokingHistory.Levels.DECREASED.value and + self.tobacco_type == TobaccoSmokingHistoryTypes.ROLLING_TOBACCO + ): + prefix = "Yes, I used to smoke less" + else: + prefix = TobaccoSmokingHistory.Levels(value).label + + return f"{prefix} than {self.presenter.to_sentence()}" def required_error_message(self): return ( diff --git a/lung_cancer_screening/questions/forms/smoking_frequency_form.py b/lung_cancer_screening/questions/forms/smoking_frequency_form.py index f0ef6c5a..b8d63eed 100644 --- a/lung_cancer_screening/questions/forms/smoking_frequency_form.py +++ b/lung_cancer_screening/questions/forms/smoking_frequency_form.py @@ -54,7 +54,7 @@ def normal_label(self): def changed_label(self): return ( - f"When you smoked {self.presenter.more_or_fewer()} " + f"When you smoked {self.presenter.more_or_fewer_or_less()} " f"than {self.normal_presenter.to_sentence()}, how often did " f"you smoke {self.presenter.human_type().lower()}?" ) @@ -77,7 +77,7 @@ def normal_required_error_message(self): def changed_required_error_message(self): return ( f"Select how often you smoked {self.presenter.human_type().lower()} " - f"when you smoked {self.presenter.more_or_fewer()} " + f"when you smoked {self.presenter.more_or_fewer_or_less()} " f"than {self.normal_presenter.to_sentence()}" ) diff --git a/lung_cancer_screening/questions/presenters/tobacco_smoking_history_presenter.py b/lung_cancer_screening/questions/presenters/tobacco_smoking_history_presenter.py index 24f4c0fd..a451e70d 100644 --- a/lung_cancer_screening/questions/presenters/tobacco_smoking_history_presenter.py +++ b/lung_cancer_screening/questions/presenters/tobacco_smoking_history_presenter.py @@ -64,12 +64,15 @@ def have_you_smoked_or_did_you_smoke(self): return "did you smoke" - def more_or_fewer(self): + def more_or_fewer_or_less(self): if self.tobacco_smoking_history.is_increased(): return "more" elif self.tobacco_smoking_history.is_decreased(): + if(self.tobacco_smoking_history.is_rolling_tobacco()): + return "less" return "fewer" + def increased_or_decreased(self): if self.tobacco_smoking_history.is_increased(): return "increased" diff --git a/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py b/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py index 2b9f1417..8c7e3eae 100644 --- a/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py +++ b/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py @@ -129,26 +129,26 @@ def test_frequency_returns_the_human_frequency_of_the_tobacco_smoking_history(se ) - def test_more_or_fewer_text_returns_more_if_increased_level(self): + def test_more_or_fewer_or_less_text_returns_more_if_increased_level(self): self.tobacco_smoking_history.level = TobaccoSmokingHistory.Levels.INCREASED self.tobacco_smoking_history.save() presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) self.assertEqual( - presenter.more_or_fewer(), + presenter.more_or_fewer_or_less(), "more" ) - def test_more_or_fewer_text_returns_fewer_if_decreased_level(self): + def test_more_or_fewer_or_less_text_returns_fewer_if_decreased_level(self): self.tobacco_smoking_history.level = TobaccoSmokingHistory.Levels.DECREASED self.tobacco_smoking_history.save() presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) self.assertEqual( - presenter.more_or_fewer(), - "fewer" + presenter.more_or_fewer_or_less(), + "less" ) def test_do_or_did_returns_do_if_current_normal_level(self): From 5b7f9a16f7d28b1f9c06311ddaeed495357b2b08 Mon Sep 17 00:00:00 2001 From: stephhou Date: Wed, 1 Apr 2026 13:20:29 +0100 Subject: [PATCH 2/4] fix: indentation bug --- lung_cancer_screening/questions/forms/smoking_change_form.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lung_cancer_screening/questions/forms/smoking_change_form.py b/lung_cancer_screening/questions/forms/smoking_change_form.py index ae99df05..487c6efb 100644 --- a/lung_cancer_screening/questions/forms/smoking_change_form.py +++ b/lung_cancer_screening/questions/forms/smoking_change_form.py @@ -65,8 +65,7 @@ def generate_label(self, value, label): return label if (value == TobaccoSmokingHistory.Levels.DECREASED.value and - self.tobacco_type == TobaccoSmokingHistoryTypes.ROLLING_TOBACCO - ): + self.tobacco_type == TobaccoSmokingHistoryTypes.ROLLING_TOBACCO): prefix = "Yes, I used to smoke less" else: prefix = TobaccoSmokingHistory.Levels(value).label From 3c944c8fd5f45f9a5b4aee8da10144c20df6313e Mon Sep 17 00:00:00 2001 From: stephhou Date: Wed, 1 Apr 2026 13:36:03 +0100 Subject: [PATCH 3/4] fix: test name and if parentheses --- .../questions/presenters/tobacco_smoking_history_presenter.py | 3 +-- .../unit/presenters/test_tobacco_smoking_history_presenter.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lung_cancer_screening/questions/presenters/tobacco_smoking_history_presenter.py b/lung_cancer_screening/questions/presenters/tobacco_smoking_history_presenter.py index a451e70d..7d36b324 100644 --- a/lung_cancer_screening/questions/presenters/tobacco_smoking_history_presenter.py +++ b/lung_cancer_screening/questions/presenters/tobacco_smoking_history_presenter.py @@ -68,11 +68,10 @@ def more_or_fewer_or_less(self): if self.tobacco_smoking_history.is_increased(): return "more" elif self.tobacco_smoking_history.is_decreased(): - if(self.tobacco_smoking_history.is_rolling_tobacco()): + if self.tobacco_smoking_history.is_rolling_tobacco(): return "less" return "fewer" - def increased_or_decreased(self): if self.tobacco_smoking_history.is_increased(): return "increased" diff --git a/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py b/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py index 8c7e3eae..8642bec2 100644 --- a/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py +++ b/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py @@ -140,7 +140,7 @@ def test_more_or_fewer_or_less_text_returns_more_if_increased_level(self): "more" ) - def test_more_or_fewer_or_less_text_returns_fewer_if_decreased_level(self): + def test_more_or_fewer_or_less_text_returns_less_if_decreased_level(self): self.tobacco_smoking_history.level = TobaccoSmokingHistory.Levels.DECREASED self.tobacco_smoking_history.save() From 81d6c272bb050eee9590260a75621d223c7f8e8d Mon Sep 17 00:00:00 2001 From: stephhou Date: Thu, 2 Apr 2026 12:12:03 +0100 Subject: [PATCH 4/4] fix: updated tests to run on fuzzy where possible, kept rolling tobacco specific tests --- .../test_tobacco_smoking_history_presenter.py | 141 +++++++++++------- 1 file changed, 85 insertions(+), 56 deletions(-) diff --git a/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py b/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py index 8642bec2..a1e5c8b6 100644 --- a/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py +++ b/lung_cancer_screening/questions/tests/unit/presenters/test_tobacco_smoking_history_presenter.py @@ -11,35 +11,17 @@ ) from lung_cancer_screening.questions.models.tobacco_smoking_history import TobaccoSmokingHistory - -class TestTobaccoSmokingHistoryPresenter(TestCase): +class TestTobaccoSmokingHistoryPresenterAllTobaccoTypes(TestCase): def setUp(self): self.response_set = ResponseSetFactory.create() self.age_when_started_smoking_response = AgeWhenStartedSmokingResponseFactory.create( response_set=self.response_set ) self.tobacco_smoking_history = TobaccoSmokingHistoryFactory.create( - rolling_tobacco=True, complete=True, response_set=self.response_set ) - def test_delegates_human_type_to_tobacco_smoking_history(self): - presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) - - self.assertEqual( - presenter.human_type(), - "Rolling tobacco" - ) - - def test_url_type_returns_the_url_type_of_the_tobacco_smoking_history(self): - presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) - - self.assertEqual( - presenter.url_type(), - "rolling-tobacco" - ) - def test_duration_years_returns_not_answered_text_if_duration_years_is_not_set(self): self.tobacco_smoking_history.smoked_total_years_response = None presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) @@ -67,26 +49,6 @@ def is_current_delegates_to_tobacco_smoking_history(self): ) - def test_to_sentence_returns_the_amount_of_type_per_frequency(self): - self.tobacco_smoking_history.smoked_amount_response.value = 7 - self.tobacco_smoking_history.smoking_frequency_response.value = SmokingFrequencyValues.WEEKLY - - presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) - - self.assertEqual( - presenter.to_sentence(), - "7 grams of rolling tobacco a week" - ) - - def test_unit_returns_the_unit_of_the_tobacco_smoking_history(self): - presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) - - self.assertEqual( - presenter.unit(), - "grams of rolling tobacco" - ) - - def test_smoke_or_smoked_returns_present_tense_if_current_normal_level(self): self.tobacco_smoking_history.smoking_current_response.value = True self.tobacco_smoking_history.smoking_current_response.save() @@ -140,17 +102,6 @@ def test_more_or_fewer_or_less_text_returns_more_if_increased_level(self): "more" ) - def test_more_or_fewer_or_less_text_returns_less_if_decreased_level(self): - self.tobacco_smoking_history.level = TobaccoSmokingHistory.Levels.DECREASED - self.tobacco_smoking_history.save() - - presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) - - self.assertEqual( - presenter.more_or_fewer_or_less(), - "less" - ) - def test_do_or_did_returns_do_if_current_normal_level(self): self.tobacco_smoking_history.smoking_current_response.value = True self.tobacco_smoking_history.smoking_current_response.save() @@ -207,7 +158,7 @@ def test_have_you_smoked_or_did_you_smoke_returns_did_if_not_current_normal_leve ) - def currently_or_previously_returns_currently_if_current_normal_level(self): + def test_currently_or_previously_returns_currently_if_current_normal_level(self): self.tobacco_smoking_history.smoking_current_response.value = True self.tobacco_smoking_history.smoking_current_response.save() @@ -218,7 +169,7 @@ def currently_or_previously_returns_currently_if_current_normal_level(self): "currently" ) - def currently_or_previously_returns_previously_if_not_current_normal_level(self): + def test_currently_or_previously_returns_previously_if_not_current_normal_level(self): self.tobacco_smoking_history.smoking_current_response.value = False self.tobacco_smoking_history.smoking_current_response.save() @@ -229,7 +180,7 @@ def currently_or_previously_returns_previously_if_not_current_normal_level(self) "previously" ) - def currently_or_previously_returns_previously_if_changed_level(self): + def test_currently_or_previously_returns_previously_if_changed_level(self): self.tobacco_smoking_history.level = TobaccoSmokingHistory.Levels.INCREASED self.tobacco_smoking_history.save() @@ -240,7 +191,38 @@ def currently_or_previously_returns_previously_if_changed_level(self): "previously" ) +class TestTobaccoSmokingHistoryPresenterRollingTobaccoAndPipe(TestCase): + def setUp(self): + self.response_set = ResponseSetFactory.create() + self.age_when_started_smoking_response = AgeWhenStartedSmokingResponseFactory.create( + response_set=self.response_set + ) + self.tobacco_smoking_history = TobaccoSmokingHistoryFactory.create( + complete=True, + response_set=self.response_set + ) + + # Tests for Rolling Tobacco + def test_delegates_human_type_to_tobacco_smoking_history_rolling_tobacco(self): + self.tobacco_smoking_history.type = TobaccoSmokingHistoryTypes.ROLLING_TOBACCO + presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) + + self.assertEqual( + presenter.human_type(), + "Rolling tobacco" + ) + + def test_url_type_returns_the_url_type_of_the_tobacco_smoking_history_rolling_tobacco(self): + self.tobacco_smoking_history.type = TobaccoSmokingHistoryTypes.ROLLING_TOBACCO + presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) + + self.assertEqual( + presenter.url_type(), + "rolling-tobacco" + ) + def test_amount_prefix_when_rolling_tobacco(self): + self.tobacco_smoking_history.type = TobaccoSmokingHistoryTypes.ROLLING_TOBACCO self.tobacco_smoking_history.rolling_tobacco = True self.tobacco_smoking_history.save() @@ -251,13 +233,60 @@ def test_amount_prefix_when_rolling_tobacco(self): "grams of " ) - def test_amount_prefix_defaults_to_empty_string(self): + def test_to_sentence_returns_the_amount_of_type_per_frequency_rolling_tobacco(self): + self.tobacco_smoking_history.type = TobaccoSmokingHistoryTypes.ROLLING_TOBACCO + self.tobacco_smoking_history.smoked_amount_response.value = 7 + self.tobacco_smoking_history.smoking_frequency_response.value = SmokingFrequencyValues.WEEKLY + + presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) + + self.assertEqual( + presenter.to_sentence(), + "7 grams of rolling tobacco a week" + ) + + def test_unit_returns_the_unit_of_the_tobacco_smoking_history_rolling_tobacco(self): + self.tobacco_smoking_history.type = TobaccoSmokingHistoryTypes.ROLLING_TOBACCO + presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) + + self.assertEqual( + presenter.unit(), + "grams of rolling tobacco" + ) + + def test_more_or_fewer_or_less_text_returns_less_if_decreased_level_rolling_tobacco(self): + self.tobacco_smoking_history.type = TobaccoSmokingHistoryTypes.ROLLING_TOBACCO + self.tobacco_smoking_history.level = TobaccoSmokingHistory.Levels.DECREASED + self.tobacco_smoking_history.save() + + presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) + + self.assertEqual( + presenter.more_or_fewer_or_less(), + "less" + ) + + # Tests for Pipe + def test_more_or_fewer_or_less_text_returns_fewer_if_decreased_level_cigarettes(self): self.tobacco_smoking_history.type = TobaccoSmokingHistoryTypes.PIPE + self.tobacco_smoking_history.level = TobaccoSmokingHistory.Levels.DECREASED self.tobacco_smoking_history.save() presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) self.assertEqual( - presenter.amount_prefix(), - "" + presenter.more_or_fewer_or_less(), + "fewer" + ) + + def test_to_sentence_returns_the_amount_of_type_per_frequency_pipe(self): + self.tobacco_smoking_history.type = TobaccoSmokingHistoryTypes.PIPE + self.tobacco_smoking_history.smoked_amount_response.value = 7 + self.tobacco_smoking_history.smoking_frequency_response.value = SmokingFrequencyValues.WEEKLY + + presenter = TobaccoSmokingHistoryPresenter(self.tobacco_smoking_history) + + self.assertEqual( + presenter.to_sentence(), + "7 full pipe loads a week" )