From 49d1168a68489f2338061fccd01d96cbc9032111 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Tue, 31 Mar 2026 16:56:43 +0100 Subject: [PATCH 1/9] PPHA-593: First pass WhenYouQuitSmoking page --- features/smoking_history.feature | 97 +--------- features/steps/preflight_steps.py | 2 +- features/when_you_quit_smoking.feature | 41 ++++ .../forms/when_you_quit_smoking_form.py | 27 +++ .../0010_whenyouquitsmokingresponse.py | 27 +++ ...whenyouquitsmokingresponse_response_set.py | 19 ++ .../models/when_you_quit_smoking_response.py | 43 +++++ .../when_you_quit_smoking_response_factory.py | 12 ++ .../forms/test_when_you_quit_smoking_form.py | 126 +++++++++++++ .../test_when_you_quit_smoking_response.py | 72 +++++++ .../unit/views/test_when_you_quit_smoking.py | 175 ++++++++++++++++++ lung_cancer_screening/questions/urls.py | 2 + .../questions/views/when_you_quit_smoking.py | 31 ++++ 13 files changed, 578 insertions(+), 96 deletions(-) create mode 100644 features/when_you_quit_smoking.feature create mode 100644 lung_cancer_screening/questions/forms/when_you_quit_smoking_form.py create mode 100644 lung_cancer_screening/questions/migrations/0010_whenyouquitsmokingresponse.py create mode 100644 lung_cancer_screening/questions/migrations/0011_alter_whenyouquitsmokingresponse_response_set.py create mode 100644 lung_cancer_screening/questions/models/when_you_quit_smoking_response.py create mode 100644 lung_cancer_screening/questions/tests/factories/when_you_quit_smoking_response_factory.py create mode 100644 lung_cancer_screening/questions/tests/unit/forms/test_when_you_quit_smoking_form.py create mode 100644 lung_cancer_screening/questions/tests/unit/models/test_when_you_quit_smoking_response.py create mode 100644 lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py create mode 100644 lung_cancer_screening/questions/views/when_you_quit_smoking.py diff --git a/features/smoking_history.feature b/features/smoking_history.feature index 711a93d8..f7168f25 100644 --- a/features/smoking_history.feature +++ b/features/smoking_history.feature @@ -386,7 +386,7 @@ Feature: Smoking history pages And I see "2 cigarillos a month" as a response to "Current cigarillo smoking" under "Cigarillo smoking history" And I see "4 cigarillos a week for 3 years" as a response to "When you smoked more than 2 cigarillos a month" under "Cigarillo smoking history" - # Change cigarette smoking history with increased and decreased + # Change cigarette smoking history When I click the link to change "Cigarette" smoking history Then I am on "/cigarettes-smoking-current?change=True" @@ -419,102 +419,9 @@ Feature: Smoking history pages And I submit the form Then I am on "/cigarettes-smoked-increased-years?change=True" - When I submit the form - Then I am on "/cigarettes-smoking-decreased-frequency?change=True" - When I submit the form - - Then I am on "/cigarettes-smoked-decreased-amount?change=True" - When I submit the form - - Then I am on "/cigarettes-smoked-decreased-years?change=True" - When I submit the form - - Then I am on "/check-your-answers" + When I go to "/check-your-answers" Then I see "17 years" as a response to "Total number of years you smoked cigarettes" under "Cigarette smoking history" And I see "25 cigarettes a month" as a response to "Current cigarette smoking" under "Cigarette smoking history" And I see "40 cigarettes a day for 5 years" as a response to "When you smoked more than 25 cigarettes a month" under "Cigarette smoking history" - - # Change medium cigars with decreased - When I click the link to change "Medium cigar" smoking history - - Then I am on "/medium-cigars-smoking-current?change=True" - When I submit the form - - Then I am on "/medium-cigars-smoked-total-years?change=True" - When I submit the form - - Then I am on "/medium-cigars-smoking-frequency?change=True" - When I submit the form - - Then I am on "/medium-cigars-smoked-amount?change=True" - When I submit the form - - Then I am on "/medium-cigars-smoking-change?change=True" - When I submit the form - - Then I am on "/medium-cigars-smoking-decreased-frequency?change=True" - When I submit the form - - Then I am on "/medium-cigars-smoked-decreased-amount?change=True" - When I submit the form - - Then I am on "/medium-cigars-smoked-decreased-years?change=True" - When I submit the form - - Then I am on "/check-your-answers" - - # Change cigarillos with increased - When I click the link to change "Cigarillo" smoking history - - Then I am on "/cigarillos-smoking-current?change=True" - When I submit the form - - Then I am on "/cigarillos-smoked-total-years?change=True" - When I submit the form - - Then I am on "/cigarillos-smoking-frequency?change=True" - When I submit the form - - Then I am on "/cigarillos-smoked-amount?change=True" - When I submit the form - - Then I am on "/cigarillos-smoking-change?change=True" - When I submit the form - - Then I am on "/cigarillos-smoking-increased-frequency?change=True" - When I submit the form - - Then I am on "/cigarillos-smoked-increased-amount?change=True" - When I submit the form - - Then I am on "/cigarillos-smoked-increased-years?change=True" - When I submit the form - - Then I am on "/check-your-answers" - - # Change cigarettes change to no change - When I click the link to change "Cigarette" smoking history - - Then I am on "/cigarettes-smoking-current?change=True" - When I submit the form - - Then I am on "/cigarettes-smoked-total-years?change=True" - When I submit the form - - Then I am on "/cigarettes-smoking-frequency?change=True" - When I submit the form - - Then I am on "/cigarettes-smoked-amount?change=True" - When I submit the form - - Then I am on "/cigarettes-smoking-change?change=True" - When I uncheck "Yes, I used to smoke more than 25 cigarettes a month" - And I uncheck "Yes, I used to smoke fewer than 25 cigarettes a month" - And I check "No, it has not changed" - When I submit the form - - Then I am on "/check-your-answers" - - diff --git a/features/steps/preflight_steps.py b/features/steps/preflight_steps.py index d5d2dd0a..d84b11b9 100644 --- a/features/steps/preflight_steps.py +++ b/features/steps/preflight_steps.py @@ -75,7 +75,7 @@ def given_i_have_answered_questions_showing_i_am_a_current_smoker(context): when_i_check_label(context, "Yes, I currently smoke") when_i_submit_the_form(context) - +@given('I have answered questions showing I started smoking "{years}" years ago') @given('I have answered questions showing I have smoked for "{years}" years') def given_i_have_answered_questions_showing_i_have_smoked_for_years_years(context, years): response_set = get_or_create_response_set(context) diff --git a/features/when_you_quit_smoking.feature b/features/when_you_quit_smoking.feature new file mode 100644 index 00000000..800e2dd6 --- /dev/null +++ b/features/when_you_quit_smoking.feature @@ -0,0 +1,41 @@ +@SmokingHistory +@WhenYouQuitSmoking +Feature: Age quit smoking + # Scenario: The page is accessible + # Given I am logged in + # And I have answered questions showing I am eligible + # When I go to "/age-when-started-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 am 60 years old + And I have answered questions showing I started smoking "30" years ago + When I go to "/when-you-quit-smoking" + And I click "Continue" + Then I am on "/when-you-quit-smoking" + And I see a form error "Enter your age when you quit smoking" + And there are no accessibility violations + + # Scenario: Navigating backwards and forwards + # Given I am logged in + # And I have answered questions showing I am eligible + # When I go to "/age-when-started-smoking" + # Then I see a back link to "/relatives-age-when-diagnosed" + # When I fill in "How old were you when you started smoking?" as "18" and submit + # Then I am on "/periods-when-you-stopped-smoking" + + # Scenario: Checking responses and changing them + # Given I am logged in + # And I have answered questions showing I am eligible + # When I go to "/age-when-started-smoking" + # And I fill in "How old were you when you started smoking?" as "18" and submit + # When I go to "/check-your-answers" + # Then I see "18" as a response to "Age you started smoking" under "Smoking history" + # And I see "/age-when-started-smoking?change=True" as a link to change "Age you started smoking" under "Smoking history" + # When I click the link to change "Age you started smoking" under "Smoking History" + # Then I am on "/age-when-started-smoking?change=True" + # And I see "18" filled in for "How old were you when you started smoking?" + # When I fill in "How old were you when you started smoking?" as "22" and submit + # Then I am on "/periods-when-you-stopped-smoking?change=True" diff --git a/lung_cancer_screening/questions/forms/when_you_quit_smoking_form.py b/lung_cancer_screening/questions/forms/when_you_quit_smoking_form.py new file mode 100644 index 00000000..2638f8ce --- /dev/null +++ b/lung_cancer_screening/questions/forms/when_you_quit_smoking_form.py @@ -0,0 +1,27 @@ +from django import forms + +from lung_cancer_screening.nhsuk_forms.integer_field import IntegerField +from ..models.when_you_quit_smoking_response import WhenYouQuitSmokingResponse + +class WhenYouQuitSmokingForm(forms.ModelForm): + class Meta: + model = WhenYouQuitSmokingResponse + fields = ["value"] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self.fields["value"] = IntegerField( + label="How old were you when you quit smoking?", + label_classes="nhsuk-label--m", + classes="nhsuk-input--width-2", + hint="Give an estimate if you are not sure", + prefix="Age", + error_messages={ + "required": "Enter your age when you quit smoking", + "invalid": "Enter your age when you quit smoking", + "min_value":"The age you quit smoking must be between 1 and your current age", + # "age_started_smoking_greater_than_current_age":"The age you started smoking must be the same as, or less than your current age", + # "no_date_of_birth" : format_html("Provide your date of birth before answering this question", reverse_lazy("questions:date_of_birth")) + } + ) diff --git a/lung_cancer_screening/questions/migrations/0010_whenyouquitsmokingresponse.py b/lung_cancer_screening/questions/migrations/0010_whenyouquitsmokingresponse.py new file mode 100644 index 00000000..99004fa8 --- /dev/null +++ b/lung_cancer_screening/questions/migrations/0010_whenyouquitsmokingresponse.py @@ -0,0 +1,27 @@ +# Generated by Django 6.0.3 on 2026-03-31 14:46 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('questions', '0009_alter_sexatbirthresponse_value_alter_user_nhs_number'), + ] + + operations = [ + migrations.CreateModel( + name='WhenYouQuitSmokingResponse', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('value', models.IntegerField()), + ('response_set', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='questions.responseset')), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/lung_cancer_screening/questions/migrations/0011_alter_whenyouquitsmokingresponse_response_set.py b/lung_cancer_screening/questions/migrations/0011_alter_whenyouquitsmokingresponse_response_set.py new file mode 100644 index 00000000..f3d3c7f9 --- /dev/null +++ b/lung_cancer_screening/questions/migrations/0011_alter_whenyouquitsmokingresponse_response_set.py @@ -0,0 +1,19 @@ +# Generated by Django 6.0.3 on 2026-03-31 15:05 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('questions', '0010_whenyouquitsmokingresponse'), + ] + + operations = [ + migrations.AlterField( + model_name='whenyouquitsmokingresponse', + name='response_set', + field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='when_you_quit_smoking_response', to='questions.responseset'), + ), + ] diff --git a/lung_cancer_screening/questions/models/when_you_quit_smoking_response.py b/lung_cancer_screening/questions/models/when_you_quit_smoking_response.py new file mode 100644 index 00000000..5974fe19 --- /dev/null +++ b/lung_cancer_screening/questions/models/when_you_quit_smoking_response.py @@ -0,0 +1,43 @@ +from django.db import models +from django.core.validators import MinValueValidator +from django.core.exceptions import ValidationError + +from .base import BaseModel +from .response_set import ResponseSet + +class WhenYouQuitSmokingResponse(BaseModel): + response_set = models.OneToOneField( + ResponseSet, + on_delete=models.CASCADE, + related_name="when_you_quit_smoking_response", + ) + value = models.IntegerField( + validators=[ + MinValueValidator(1), + ] + ) + + def clean(self): + super().clean() + self.validates_date_of_birth_response() + self.validates_age_started_smoking_response() + + + def validates_date_of_birth_response(self): + if not hasattr(self.response_set, "date_of_birth_response"): + raise ValidationError({ + "value": ValidationError( + "date of birth not set", + code="no_date_of_birth" + ) + }) + + + def validates_age_started_smoking_response(self): + if not hasattr(self.response_set, "age_when_started_smoking_response"): + raise ValidationError({ + "value": ValidationError( + "age started smoking not set", + code="no_age_started_smoking" + ) + }) diff --git a/lung_cancer_screening/questions/tests/factories/when_you_quit_smoking_response_factory.py b/lung_cancer_screening/questions/tests/factories/when_you_quit_smoking_response_factory.py new file mode 100644 index 00000000..87bdc03b --- /dev/null +++ b/lung_cancer_screening/questions/tests/factories/when_you_quit_smoking_response_factory.py @@ -0,0 +1,12 @@ +import factory + +from .response_set_factory import ResponseSetFactory +from ...models.when_you_quit_smoking_response import WhenYouQuitSmokingResponse + + +class WhenYouQuitSmokingResponseFactory(factory.django.DjangoModelFactory): + class Meta: + model = WhenYouQuitSmokingResponse + + response_set = factory.SubFactory(ResponseSetFactory) + value = factory.Faker("pyint", min_value=18, max_value=100) diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_when_you_quit_smoking_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_when_you_quit_smoking_form.py new file mode 100644 index 00000000..e8be9fac --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/forms/test_when_you_quit_smoking_form.py @@ -0,0 +1,126 @@ +from django.test import TestCase, tag + +from django.utils import timezone +from dateutil.relativedelta import relativedelta + +from ....models.age_when_started_smoking_response import AgeWhenStartedSmokingResponse + +from ...factories.response_set_factory import ResponseSetFactory +from ...factories.date_of_birth_response_factory import DateOfBirthResponseFactory +from ...factories.age_when_started_smoking_response_factory import AgeWhenStartedSmokingResponseFactory +from ...factories.when_you_quit_smoking_response_factory import WhenYouQuitSmokingResponseFactory + +from ....forms.when_you_quit_smoking_form import WhenYouQuitSmokingForm + + +@tag("WhenYouQuitSmoking") +class TestWhenYouQuitSmokingForm(TestCase): + def setUp(self): + self.response_set = ResponseSetFactory() + self.date_of_birth_response = DateOfBirthResponseFactory( + response_set=self.response_set, + eligible=True, + ) + self.age_when_started_smoking_response = AgeWhenStartedSmokingResponseFactory( + response_set=self.response_set, + value=self.date_of_birth_response.age_in_years() - 20, + ) + self.response = WhenYouQuitSmokingResponseFactory(response_set=self.response_set) + + def test_is_valid_with_valid_input(self): + form = WhenYouQuitSmokingForm(instance=self.response, data={"value": 18}) + self.assertTrue(form.is_valid()) + self.assertEqual(form.cleaned_data["value"], 18) + + + def test_has_a_required_error_message(self): + form = WhenYouQuitSmokingForm(instance=self.response, data={"value": None}) + + self.assertFalse(form.is_valid()) + self.assertEqual( + form.errors["value"], ["Enter your age when you quit smoking"] + ) + + + def test_is_invalid_with_an_invalid_value(self): + form = WhenYouQuitSmokingForm( + instance=self.response, data={"value": "invalid"} + ) + self.assertFalse(form.is_valid()) + self.assertEqual( + form.errors["value"], ["Enter your age when you quit smoking"] + ) + + + def test_is_invalid_when_zero_is_entered(self): + form = WhenYouQuitSmokingForm(instance=self.response, data={"value": 0}) + self.assertFalse(form.is_valid()) + self.assertEqual( + form.errors["value"], + ["The age you quit smoking must be between 1 and your current age"], + ) + + # def test_is_invalid_if_no_date_of_birth(self): + # response_set = ResponseSetFactory() + # response = AgeWhenStartedSmokingResponse(response_set=response_set) + # form = AgeWhenStartedSmokingForm(instance=response, data={"value": 70}) + + # self.assertFalse(form.is_valid()) + # self.assertEqual( + # form.errors["value"], + # [ + # 'Provide your date of birth before answering this question' + # ], + # ) + + # def test_is_invalid_when_age_entered_greater_than_current_age(self): + # form = AgeWhenStartedSmokingForm(instance=self.response, data={"value": 70}) + + # self.assertFalse(form.is_valid()) + + # self.assertEqual( + # form.errors["value"], + # [ + # "The age you started smoking must be the same as, or less than your current age" + # ], + # ) + + # def test_deletes_periods_when_stopped_smoking_response_if_age_started_smoking_is_changed( + # self, + # ): + # self.response.value = 17 + # self.response.save() + + # PeriodsWhenYouStoppedSmokingResponseFactory.create( + # response_set=self.response_set, + # value=True, + # duration_years=self.response.years_smoked_including_stopped() - 1, + # ) + # form = AgeWhenStartedSmokingForm(instance=self.response, data={"value": 18}) + # form.save() + + # self.assertFalse( + # PeriodsWhenYouStoppedSmokingResponse.objects.filter( + # response_set=self.response_set + # ).exists() + # ) + + # def test_does_not_delete_periods_when_stopped_smoking_response_if_age_started_smoking_is_not_changed( + # self, + # ): + # self.response.value = 18 + # self.response.save() + + # PeriodsWhenYouStoppedSmokingResponseFactory.create( + # response_set=self.response_set, + # value=True, + # duration_years=self.response.years_smoked_including_stopped() - 1, + # ) + # form = AgeWhenStartedSmokingForm(instance=self.response, data={"value": 18}) + # form.save() + + # self.assertTrue( + # PeriodsWhenYouStoppedSmokingResponse.objects.filter( + # response_set=self.response_set + # ).exists() + # ) diff --git a/lung_cancer_screening/questions/tests/unit/models/test_when_you_quit_smoking_response.py b/lung_cancer_screening/questions/tests/unit/models/test_when_you_quit_smoking_response.py new file mode 100644 index 00000000..7305f4b2 --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/models/test_when_you_quit_smoking_response.py @@ -0,0 +1,72 @@ +from django.test import TestCase, tag +from django.core.exceptions import ValidationError + +from ...factories.response_set_factory import ResponseSetFactory +from ...factories.date_of_birth_response_factory import DateOfBirthResponseFactory +from ...factories.age_when_started_smoking_response_factory import AgeWhenStartedSmokingResponseFactory +from ...factories.when_you_quit_smoking_response_factory import WhenYouQuitSmokingResponseFactory + +from ....models.when_you_quit_smoking_response import WhenYouQuitSmokingResponse + + +@tag("WhenYouQuitSmoking") +class TestWhenYouQuitSmokingResponse(TestCase): + def setUp(self): + self.response_set = ResponseSetFactory() + self.date_of_birth_response = DateOfBirthResponseFactory( + response_set=self.response_set, + eligible=True + ) + self.age_when_started_smoking_response = AgeWhenStartedSmokingResponseFactory( + response_set=self.response_set, + value=self.date_of_birth_response.age_in_years() - 20, + ) + + + def test_has_a_valid_factory(self): + model = WhenYouQuitSmokingResponseFactory.build(response_set=self.response_set) + model.full_clean() + + + def test_has_response_set_as_foreign_key(self): + response = WhenYouQuitSmokingResponse.objects.create( + response_set=self.response_set, + value=18 + ) + + self.assertEqual(response.response_set, self.response_set) + + + def test_is_invalid_date_of_birth_is_unanswered(self): + self.response_set.date_of_birth_response.delete() + self.response_set.refresh_from_db() + response = WhenYouQuitSmokingResponseFactory.build( + response_set=self.response_set, + value=18 + ) + + with self.assertRaises(ValidationError) as context: + response.full_clean() + + self.assertEqual( + context.exception.message_dict["value"], + ["date of birth not set"] + ) + + + def test_is_invalid_if_age_started_smoking_is_unanswered(self): + self.age_when_started_smoking_response.delete() + self.response_set.refresh_from_db() + + response = WhenYouQuitSmokingResponseFactory.build( + response_set=self.response_set, + value=18 + ) + + with self.assertRaises(ValidationError) as context: + response.full_clean() + + self.assertEqual( + context.exception.message_dict["value"], + ["age started smoking not set"] + ) diff --git a/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py b/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py new file mode 100644 index 00000000..3f48894a --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py @@ -0,0 +1,175 @@ +from django.test import TestCase, tag +from django.urls import reverse + +from .helpers.authentication import login_user +from ...factories.response_set_factory import ResponseSetFactory +from ...factories.age_when_started_smoking_response_factory import AgeWhenStartedSmokingResponseFactory + + +@tag("WhenYouQuitSmoking") +class TestGetWhenYouQuitSmoking(TestCase): + def setUp(self): + self.user = login_user(self.client) + + def test_redirects_if_the_user_is_not_logged_in(self): + self.client.logout() + + response = self.client.get(reverse("questions:when_you_quit_smoking")) + + self.assertRedirects( + response, + "/oidc/authenticate/?next=/when-you-quit-smoking", + fetch_redirect_response=False, + ) + + + def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(self): + ResponseSetFactory.create(user=self.user, recently_submitted=True) + + response = self.client.get(reverse("questions:when_you_quit_smoking")) + + self.assertRedirects(response, reverse("questions:confirmation")) + + + def test_redirects_when_the_user_is_not_eligible(self): + ResponseSetFactory.create(user=self.user, eligible=False) + + response = self.client.get(reverse("questions:when_you_quit_smoking")) + + self.assertRedirects(response, reverse("questions:agree_terms_of_use")) + + + def test_responds_successfully(self): + ResponseSetFactory.create(user=self.user, eligible=True) + + response = self.client.get(reverse("questions:when_you_quit_smoking")) + self.assertEqual(response.status_code, 200) + + + def test_has_a_back_link_to_age_started_smoking(self): + ResponseSetFactory.create(user=self.user, eligible=True) + + response = self.client.get(reverse("questions:when_you_quit_smoking")) + + self.assertEqual( + response.context_data["back_link_url"], + reverse("questions:age_when_started_smoking"), + ) + + def test_has_a_back_link_when_change_is_true(self): + ResponseSetFactory.create(user=self.user, eligible=True) + + response = self.client.get( + reverse("questions:when_you_quit_smoking"), + {"change": "True"} + ) + + self.assertEqual( + response.context_data["back_link_url"], + reverse("questions:responses") + ) + + +@tag("WhenYouQuitSmoking") +class TestPostWhenYouQuitSmoking(TestCase): + def setUp(self): + self.user = login_user(self.client) + + self.valid_params = {"value": 18} + + def test_redirects_if_the_user_is_not_logged_in(self): + self.client.logout() + + response = self.client.post( + reverse("questions:when_you_quit_smoking"), self.valid_params + ) + + self.assertRedirects( + response, + "/oidc/authenticate/?next=/when-you-quit-smoking", + fetch_redirect_response=False, + ) + + + def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(self): + ResponseSetFactory.create(user=self.user, recently_submitted=True) + + response = self.client.post( + reverse("questions:when_you_quit_smoking"), self.valid_params + ) + + self.assertRedirects(response, reverse("questions:confirmation")) + + + def test_redirects_when_the_user_is_not_eligible(self): + ResponseSetFactory.create(user=self.user) + + response = self.client.post( + reverse("questions:when_you_quit_smoking"), self.valid_params + ) + + self.assertRedirects(response, reverse("questions:agree_terms_of_use")) + + + def test_creates_an_when_you_quit_smoking_response(self): + response_set = ResponseSetFactory.create(user=self.user, eligible=True) + AgeWhenStartedSmokingResponseFactory( + response_set=response_set, + value=response_set.date_of_birth_response.age_in_years() - 20, + ) + + self.client.post( + reverse("questions:when_you_quit_smoking"), self.valid_params + ) + + response_set.refresh_from_db() + self.assertEqual( + response_set.when_you_quit_smoking_response.value, + self.valid_params["value"], + ) + + def test_redirects_to_periods_when_you_stopped_smoking(self): + response_set = ResponseSetFactory.create(user=self.user, eligible=True) + AgeWhenStartedSmokingResponseFactory( + response_set=response_set, + value=response_set.date_of_birth_response.age_in_years() - 20, + ) + + response = self.client.post( + reverse("questions:when_you_quit_smoking"), self.valid_params + ) + + self.assertRedirects( + response, reverse("questions:periods_when_you_stopped_smoking") + ) + + + def test_redirects_to_periods_when_you_stopped_smoking_if_change_query_param_is_true( + self, + ): + response_set = ResponseSetFactory.create(user=self.user, eligible=True) + AgeWhenStartedSmokingResponseFactory( + response_set=response_set, + value=response_set.date_of_birth_response.age_in_years() - 20, + ) + + response = self.client.post( + reverse("questions:when_you_quit_smoking"), + {**self.valid_params, "change": "True"}, + ) + + self.assertRedirects( + response, + reverse( + "questions:periods_when_you_stopped_smoking", query={"change": "True"} + ), + ) + + def test_responds_with_422_if_the_response_fails_to_create(self): + ResponseSetFactory.create(user=self.user, eligible=True) + + response = self.client.post( + reverse("questions:when_you_quit_smoking"), {"value": "not a valid age"} + ) + + self.assertEqual(response.status_code, 422) diff --git a/lung_cancer_screening/questions/urls.py b/lung_cancer_screening/questions/urls.py index d6caf4c1..e8b7903e 100644 --- a/lung_cancer_screening/questions/urls.py +++ b/lung_cancer_screening/questions/urls.py @@ -46,6 +46,7 @@ from .views.weight import WeightView from .views.confirmation import ConfirmationView from .views.agree_terms_of_use import AgreeTermsOfUseView +from .views.when_you_quit_smoking import WhenYouQuitSmokingView urlpatterns = [ path('', RedirectView.as_view(url='/start'), name='root'), @@ -87,4 +88,5 @@ path('confirmation', ConfirmationView.as_view(), name='confirmation'), path('privacy-policy', TemplateView.as_view(template_name='privacy_policy.jinja'), name='privacy_policy'), path('cookies', TemplateView.as_view(template_name='cookies.jinja'), name='cookies'), + path('when-you-quit-smoking', WhenYouQuitSmokingView.as_view(), name='when_you_quit_smoking'), ] diff --git a/lung_cancer_screening/questions/views/when_you_quit_smoking.py b/lung_cancer_screening/questions/views/when_you_quit_smoking.py new file mode 100644 index 00000000..1db453bf --- /dev/null +++ b/lung_cancer_screening/questions/views/when_you_quit_smoking.py @@ -0,0 +1,31 @@ +from django.urls import reverse, reverse_lazy +from django.contrib.auth.mixins import LoginRequiredMixin + +from .mixins.ensure_response_set import EnsureResponseSet +from .mixins.ensure_eligible import EnsureEligibleMixin +from .question_base_view import QuestionBaseView +from ..forms.when_you_quit_smoking_form import WhenYouQuitSmokingForm +from ..models.when_you_quit_smoking_response import WhenYouQuitSmokingResponse + + +class WhenYouQuitSmokingView( + LoginRequiredMixin, + EnsureResponseSet, + EnsureEligibleMixin, + QuestionBaseView +): + template_name = "question_form.jinja" + form_class = WhenYouQuitSmokingForm + model = WhenYouQuitSmokingResponse + success_url = reverse_lazy("questions:periods_when_you_stopped_smoking") + back_link_url = reverse_lazy("questions:age_when_started_smoking") + # page_title = "How old were you when you started smoking? – NHS" + + def get_success_url(self): + if self.is_changing_responses(): + return reverse( + "questions:periods_when_you_stopped_smoking", + query={"change": "True"} + ) + else: + return super().get_success_url() From 8b253a7818cdd15a3019dc604b65515adf70496a Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Wed, 1 Apr 2026 11:00:05 +0100 Subject: [PATCH 2/9] PPHA-593: Add quit smoking prelude text to match figma --- features/steps/accessibility_steps.py | 3 +-- features/when_you_quit_smoking.feature | 10 +++++----- .../questions/jinja2/when_you_quit_smoking.jinja | 11 +++++++++++ .../questions/views/when_you_quit_smoking.py | 2 +- 4 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 lung_cancer_screening/questions/jinja2/when_you_quit_smoking.jinja diff --git a/features/steps/accessibility_steps.py b/features/steps/accessibility_steps.py index 3bf729cf..69450d61 100644 --- a/features/steps/accessibility_steps.py +++ b/features/steps/accessibility_steps.py @@ -7,7 +7,6 @@ def then_there_are_no_accessibility_violations(context): axe_results = axe.run(context.page) violations_msg = ( f"Found the following accessibility violations: \n" - # Use generate_report for more indepth information about the violations - f"{axe_results.generate_snapshot()}" + f"{axe_results.generate_report()}" ) assert axe_results.violations_count == 0, violations_msg diff --git a/features/when_you_quit_smoking.feature b/features/when_you_quit_smoking.feature index 800e2dd6..4348c61a 100644 --- a/features/when_you_quit_smoking.feature +++ b/features/when_you_quit_smoking.feature @@ -1,11 +1,11 @@ @SmokingHistory @WhenYouQuitSmoking Feature: Age quit smoking - # Scenario: The page is accessible - # Given I am logged in - # And I have answered questions showing I am eligible - # When I go to "/age-when-started-smoking" - # Then there are no accessibility violations + Scenario: The page is accessible + Given I am logged in + And I have answered questions showing I am eligible + When I go to "/age-when-started-smoking" + Then there are no accessibility violations Scenario: Form errors Given I am logged in diff --git a/lung_cancer_screening/questions/jinja2/when_you_quit_smoking.jinja b/lung_cancer_screening/questions/jinja2/when_you_quit_smoking.jinja new file mode 100644 index 00000000..3c7f9ebe --- /dev/null +++ b/lung_cancer_screening/questions/jinja2/when_you_quit_smoking.jinja @@ -0,0 +1,11 @@ +{% extends 'question_form.jinja' %} + +{% block pageTitle %}When did you quit smoking? – Check if you need a lung scan – NHS{% endblock %} + +{% block prelude %} +

When you quit smoking?

+ +

If you stopped smoking more than once, tell us your age when you last quit smoking.

+ +

The question on the next page will ask if you ever stopped smoking for periods of 1 year or longer before you quit.

+{% endblock prelude %} diff --git a/lung_cancer_screening/questions/views/when_you_quit_smoking.py b/lung_cancer_screening/questions/views/when_you_quit_smoking.py index 1db453bf..81b596cb 100644 --- a/lung_cancer_screening/questions/views/when_you_quit_smoking.py +++ b/lung_cancer_screening/questions/views/when_you_quit_smoking.py @@ -14,7 +14,7 @@ class WhenYouQuitSmokingView( EnsureEligibleMixin, QuestionBaseView ): - template_name = "question_form.jinja" + template_name = "when_you_quit_smoking.jinja" form_class = WhenYouQuitSmokingForm model = WhenYouQuitSmokingResponse success_url = reverse_lazy("questions:periods_when_you_stopped_smoking") From 3d691cd4578c75664b6e6b4aa6042a5c81d17524 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Wed, 1 Apr 2026 11:11:44 +0100 Subject: [PATCH 3/9] PPHA-593: Rename EnsurePrerequisiteResponses to EnsureSmokingHistoryPrerequisiteResponses --- ...ensure_smoking_history_prerequsite_response_mixin.py} | 9 ++++++--- ... => ensure_smoking_history_prerequisite_responses.py} | 2 +- lung_cancer_screening/questions/views/smoked_amount.py | 4 ++-- .../questions/views/smoked_total_years.py | 4 ++-- .../questions/views/smoking_frequency.py | 4 ++-- 5 files changed, 13 insertions(+), 10 deletions(-) rename lung_cancer_screening/questions/tests/unit/views/mixins/{test_ensure_prerequsite_response_mixin.py => test_ensure_smoking_history_prerequsite_response_mixin.py} (88%) rename lung_cancer_screening/questions/views/mixins/{ensure_prerequisite_responses.py => ensure_smoking_history_prerequisite_responses.py} (95%) diff --git a/lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_prerequsite_response_mixin.py b/lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_smoking_history_prerequsite_response_mixin.py similarity index 88% rename from lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_prerequsite_response_mixin.py rename to lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_smoking_history_prerequsite_response_mixin.py index c2670f3a..fe2ae124 100644 --- a/lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_prerequsite_response_mixin.py +++ b/lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_smoking_history_prerequsite_response_mixin.py @@ -8,16 +8,19 @@ from ....factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory from ....factories.smoking_current_response_factory import SmokingCurrentResponseFactory -from .....views.mixins.ensure_prerequisite_responses import EnsurePrerequisiteResponsesMixin +from .....views.mixins.ensure_smoking_history_prerequisite_responses import EnsureSmokingHistoryPrerequisiteResponsesMixin from .....views.mixins.ensure_smoking_history_for_type import EnsureSmokingHistoryForTypeMixin -class BaseFakeView(EnsureSmokingHistoryForTypeMixin, EnsurePrerequisiteResponsesMixin, View): +class BaseFakeView( + EnsureSmokingHistoryForTypeMixin, EnsureSmokingHistoryPrerequisiteResponsesMixin, + View +): def get(self, request, *args, **kwargs): return HttpResponse(status=200) @tag("mixins") -class EnsurePrerequisiteResponsesMixinTest(TestCase): +class EnsureSmokingHistoryPrerequisiteResponsesMixinTest(TestCase): def setUp(self): self.factory = RequestFactory() self.request = self.factory.get("/") diff --git a/lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py b/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_prerequisite_responses.py similarity index 95% rename from lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py rename to lung_cancer_screening/questions/views/mixins/ensure_smoking_history_prerequisite_responses.py index b8d38e69..738a55a1 100644 --- a/lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py +++ b/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_prerequisite_responses.py @@ -2,7 +2,7 @@ from django.urls import reverse -class EnsurePrerequisiteResponsesMixin: +class EnsureSmokingHistoryPrerequisiteResponsesMixin: RESPONSE_URL_MAPPING = { "smoking_current_response": "questions:smoking_current", diff --git a/lung_cancer_screening/questions/views/smoked_amount.py b/lung_cancer_screening/questions/views/smoked_amount.py index 479cd988..06987a39 100644 --- a/lung_cancer_screening/questions/views/smoked_amount.py +++ b/lung_cancer_screening/questions/views/smoked_amount.py @@ -4,7 +4,7 @@ from .mixins.ensure_response_set import EnsureResponseSet from .mixins.ensure_eligible import EnsureEligibleMixin from .mixins.ensure_smoking_history_for_type import EnsureSmokingHistoryForTypeMixin -from .mixins.ensure_prerequisite_responses import EnsurePrerequisiteResponsesMixin +from .mixins.ensure_smoking_history_prerequisite_responses import EnsureSmokingHistoryPrerequisiteResponsesMixin from .smoking_history_question_base_view import SmokingHistoryQuestionBaseView from ..forms.smoked_amount_form import SmokedAmountForm from ..models.smoked_amount_response import SmokedAmountResponse @@ -15,7 +15,7 @@ class SmokedAmountView( EnsureResponseSet, EnsureEligibleMixin, EnsureSmokingHistoryForTypeMixin, - EnsurePrerequisiteResponsesMixin, + EnsureSmokingHistoryPrerequisiteResponsesMixin, SmokingHistoryQuestionBaseView ): template_name = "smoked_amount.jinja" diff --git a/lung_cancer_screening/questions/views/smoked_total_years.py b/lung_cancer_screening/questions/views/smoked_total_years.py index 01dc891a..47c7ba55 100644 --- a/lung_cancer_screening/questions/views/smoked_total_years.py +++ b/lung_cancer_screening/questions/views/smoked_total_years.py @@ -6,7 +6,7 @@ from .mixins.ensure_response_set import EnsureResponseSet from .mixins.ensure_eligible import EnsureEligibleMixin from .mixins.ensure_smoking_history_for_type import EnsureSmokingHistoryForTypeMixin -from .mixins.ensure_prerequisite_responses import EnsurePrerequisiteResponsesMixin +from .mixins.ensure_smoking_history_prerequisite_responses import EnsureSmokingHistoryPrerequisiteResponsesMixin from .smoking_history_question_base_view import SmokingHistoryQuestionBaseView from ..forms.smoked_total_years_form import SmokedTotalYearsForm from ..models.smoked_total_years_response import SmokedTotalYearsResponse @@ -25,7 +25,7 @@ class SmokedTotalYearsView( EnsureResponseSet, EnsureEligibleMixin, EnsureSmokingHistoryForTypeMixin, - EnsurePrerequisiteResponsesMixin, + EnsureSmokingHistoryPrerequisiteResponsesMixin, EnsureAnsweredAgeWhenStartedSmokingMixin, SmokingHistoryQuestionBaseView, ): diff --git a/lung_cancer_screening/questions/views/smoking_frequency.py b/lung_cancer_screening/questions/views/smoking_frequency.py index bd20e456..16875b3a 100644 --- a/lung_cancer_screening/questions/views/smoking_frequency.py +++ b/lung_cancer_screening/questions/views/smoking_frequency.py @@ -6,7 +6,7 @@ from .mixins.ensure_response_set import EnsureResponseSet from .mixins.ensure_eligible import EnsureEligibleMixin from .mixins.ensure_smoking_history_for_type import EnsureSmokingHistoryForTypeMixin -from .mixins.ensure_prerequisite_responses import EnsurePrerequisiteResponsesMixin +from .mixins.ensure_smoking_history_prerequisite_responses import EnsureSmokingHistoryPrerequisiteResponsesMixin from .smoking_history_question_base_view import SmokingHistoryQuestionBaseView from ..forms.smoking_frequency_form import SmokingFrequencyForm from ..models.smoking_frequency_response import SmokingFrequencyResponse @@ -17,7 +17,7 @@ class SmokingFrequencyView( EnsureResponseSet, EnsureEligibleMixin, EnsureSmokingHistoryForTypeMixin, - EnsurePrerequisiteResponsesMixin, + EnsureSmokingHistoryPrerequisiteResponsesMixin, SmokingHistoryQuestionBaseView ): template_name = "question_form.jinja" From a891b33bf22d0ca0bdee2231adea5bf4a69a3f95 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Wed, 1 Apr 2026 12:07:40 +0100 Subject: [PATCH 4/9] PPHA-593: Redirect age quit to age started smoking when unanswered --- features/when_you_quit_smoking.feature | 18 +++-- .../test_ensure_prerequsite_response_mixin.py | 77 +++++++++++++++++++ .../unit/views/test_when_you_quit_smoking.py | 55 +++++++------ .../mixins/ensure_prerequisite_responses.py | 29 +++++++ .../questions/views/when_you_quit_smoking.py | 4 +- 5 files changed, 146 insertions(+), 37 deletions(-) create mode 100644 lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_prerequsite_response_mixin.py create mode 100644 lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py diff --git a/features/when_you_quit_smoking.feature b/features/when_you_quit_smoking.feature index 4348c61a..ebdb5da9 100644 --- a/features/when_you_quit_smoking.feature +++ b/features/when_you_quit_smoking.feature @@ -4,6 +4,8 @@ Feature: Age quit smoking Scenario: The page is accessible Given I am logged in And I have answered questions showing I am eligible + And I am 60 years old + And I have answered questions showing I started smoking "30" years ago When I go to "/age-when-started-smoking" Then there are no accessibility violations @@ -18,13 +20,15 @@ Feature: Age quit smoking And I see a form error "Enter your age when you quit smoking" And there are no accessibility violations - # Scenario: Navigating backwards and forwards - # Given I am logged in - # And I have answered questions showing I am eligible - # When I go to "/age-when-started-smoking" - # Then I see a back link to "/relatives-age-when-diagnosed" - # When I fill in "How old were you when you started smoking?" as "18" and submit - # Then I am on "/periods-when-you-stopped-smoking" + Scenario: Navigating backwards and forwards + Given I am logged in + And I have answered questions showing I am eligible + And I am 60 years old + And I have answered questions showing I started smoking "30" years ago + When I go to "/when-you-quit-smoking" + Then I see a back link to "/age-when-started-smoking" + When I fill in "How old were you when you quit smoking?" as "30" and submit + Then I am on "/periods-when-you-stopped-smoking" # Scenario: Checking responses and changing them # Given I am logged in diff --git a/lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_prerequsite_response_mixin.py b/lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_prerequsite_response_mixin.py new file mode 100644 index 00000000..206d0375 --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/views/mixins/test_ensure_prerequsite_response_mixin.py @@ -0,0 +1,77 @@ +from django.http import HttpResponse +from django.urls import reverse +from django.test import TestCase, tag +from django.test.client import RequestFactory +from django.views.generic import View + +from ....factories.response_set_factory import ResponseSetFactory +from ....factories.age_when_started_smoking_response_factory import AgeWhenStartedSmokingResponseFactory + +from .....views.mixins.ensure_prerequisite_responses import EnsurePrerequisiteResponsesMixin + + +class BaseFakeView( + EnsurePrerequisiteResponsesMixin, + View +): + def get(self, request, *args, **kwargs): + return HttpResponse(status=200) + +@tag("mixins") +class EnsureSmokingHistoryPrerequisiteResponsesMixinTest(TestCase): + def setUp(self): + self.factory = RequestFactory() + self.response_set = ResponseSetFactory() + self.request = self.factory.get("/") + self.request.response_set = self.response_set + + + def _dispatch(self, view, request, query={}): + view.request = request + view.args = () + return view, view.dispatch(request, **query) + + + def test_returns_200_by_default(self): + view, response = self._dispatch(BaseFakeView(), self.request) + + self.assertEqual(response.status_code, 200) + + + def test_returns_200_when_prerequisite_responses_are_present(self): + class FakeView(BaseFakeView): + prerequisite_responses = ["age_when_started_smoking_response"] + + AgeWhenStartedSmokingResponseFactory( + response_set=self.request.response_set + ) + + view, response = self._dispatch(FakeView(), self.request) + + self.assertEqual(response.status_code, 200) + + + def test_redirects_when_the_prerequisite_responses_do_not_exist(self): + class FakeView(BaseFakeView): + prerequisite_responses = ["age_when_started_smoking_response"] + + view, response = self._dispatch(FakeView(), self.request) + + self.assertRedirects(response, reverse( + "questions:age_when_started_smoking", + ), fetch_redirect_response=False) + + + def test_redirects_when_the_prerequisite_responses_do_not_exist_when_changing_responses(self): + class FakeView(BaseFakeView): + prerequisite_responses = ["age_when_started_smoking_response"] + + request = self.factory.get("/", {"change": "True"}) + request.response_set = self.response_set + view, response = self._dispatch(FakeView(), request) + + self.assertRedirects(response, reverse( + "questions:age_when_started_smoking", + query={"change": "True"}, + ), fetch_redirect_response=False) + diff --git a/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py b/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py index 3f48894a..33eca42d 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py @@ -10,6 +10,11 @@ class TestGetWhenYouQuitSmoking(TestCase): def setUp(self): self.user = login_user(self.client) + self.response_set = ResponseSetFactory.create(user=self.user, eligible=True) + self.age_when_started_smoking_response = AgeWhenStartedSmokingResponseFactory( + response_set=self.response_set, + value=self.response_set.date_of_birth_response.age_in_years() - 20, + ) def test_redirects_if_the_user_is_not_logged_in(self): self.client.logout() @@ -24,6 +29,7 @@ def test_redirects_if_the_user_is_not_logged_in(self): def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(self): + self.response_set.delete() ResponseSetFactory.create(user=self.user, recently_submitted=True) response = self.client.get(reverse("questions:when_you_quit_smoking")) @@ -32,6 +38,7 @@ def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(sel def test_redirects_when_the_user_is_not_eligible(self): + self.response_set.delete() ResponseSetFactory.create(user=self.user, eligible=False) response = self.client.get(reverse("questions:when_you_quit_smoking")) @@ -39,16 +46,21 @@ def test_redirects_when_the_user_is_not_eligible(self): self.assertRedirects(response, reverse("questions:agree_terms_of_use")) - def test_responds_successfully(self): - ResponseSetFactory.create(user=self.user, eligible=True) + def test_redirects_to_age_started_smoking_if_the_user_has_not_answered_age_started_smoking(self): + self.age_when_started_smoking_response.delete() + + response = self.client.get(reverse("questions:when_you_quit_smoking")) + + self.assertRedirects(response, reverse("questions:age_when_started_smoking")) + + def test_responds_successfully(self): response = self.client.get(reverse("questions:when_you_quit_smoking")) + self.assertEqual(response.status_code, 200) def test_has_a_back_link_to_age_started_smoking(self): - ResponseSetFactory.create(user=self.user, eligible=True) - response = self.client.get(reverse("questions:when_you_quit_smoking")) self.assertEqual( @@ -57,8 +69,6 @@ def test_has_a_back_link_to_age_started_smoking(self): ) def test_has_a_back_link_when_change_is_true(self): - ResponseSetFactory.create(user=self.user, eligible=True) - response = self.client.get( reverse("questions:when_you_quit_smoking"), {"change": "True"} @@ -74,6 +84,11 @@ def test_has_a_back_link_when_change_is_true(self): class TestPostWhenYouQuitSmoking(TestCase): def setUp(self): self.user = login_user(self.client) + self.response_set = ResponseSetFactory.create(user=self.user, eligible=True) + self.age_when_started_smoking_response = AgeWhenStartedSmokingResponseFactory( + response_set=self.response_set, + value=self.response_set.date_of_birth_response.age_in_years() - 20, + ) self.valid_params = {"value": 18} @@ -92,6 +107,7 @@ def test_redirects_if_the_user_is_not_logged_in(self): def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(self): + self.response_set.delete() ResponseSetFactory.create(user=self.user, recently_submitted=True) response = self.client.post( @@ -102,7 +118,8 @@ def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(sel def test_redirects_when_the_user_is_not_eligible(self): - ResponseSetFactory.create(user=self.user) + self.response_set.delete() + ResponseSetFactory.create(user=self.user, eligible=False) response = self.client.post( reverse("questions:when_you_quit_smoking"), self.valid_params @@ -112,29 +129,17 @@ def test_redirects_when_the_user_is_not_eligible(self): def test_creates_an_when_you_quit_smoking_response(self): - response_set = ResponseSetFactory.create(user=self.user, eligible=True) - AgeWhenStartedSmokingResponseFactory( - response_set=response_set, - value=response_set.date_of_birth_response.age_in_years() - 20, - ) - self.client.post( reverse("questions:when_you_quit_smoking"), self.valid_params ) - response_set.refresh_from_db() + self.response_set.refresh_from_db() self.assertEqual( - response_set.when_you_quit_smoking_response.value, + self.response_set.when_you_quit_smoking_response.value, self.valid_params["value"], ) def test_redirects_to_periods_when_you_stopped_smoking(self): - response_set = ResponseSetFactory.create(user=self.user, eligible=True) - AgeWhenStartedSmokingResponseFactory( - response_set=response_set, - value=response_set.date_of_birth_response.age_in_years() - 20, - ) - response = self.client.post( reverse("questions:when_you_quit_smoking"), self.valid_params ) @@ -147,12 +152,6 @@ def test_redirects_to_periods_when_you_stopped_smoking(self): def test_redirects_to_periods_when_you_stopped_smoking_if_change_query_param_is_true( self, ): - response_set = ResponseSetFactory.create(user=self.user, eligible=True) - AgeWhenStartedSmokingResponseFactory( - response_set=response_set, - value=response_set.date_of_birth_response.age_in_years() - 20, - ) - response = self.client.post( reverse("questions:when_you_quit_smoking"), {**self.valid_params, "change": "True"}, @@ -166,8 +165,6 @@ def test_redirects_to_periods_when_you_stopped_smoking_if_change_query_param_is_ ) def test_responds_with_422_if_the_response_fails_to_create(self): - ResponseSetFactory.create(user=self.user, eligible=True) - response = self.client.post( reverse("questions:when_you_quit_smoking"), {"value": "not a valid age"} ) diff --git a/lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py b/lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py new file mode 100644 index 00000000..acd288eb --- /dev/null +++ b/lung_cancer_screening/questions/views/mixins/ensure_prerequisite_responses.py @@ -0,0 +1,29 @@ +from django.shortcuts import redirect +from django.urls import reverse + + +class EnsurePrerequisiteResponsesMixin: + RESPONSE_URL_MAPPING = { + "age_when_started_smoking_response": "questions:age_when_started_smoking", + } + + prerequisite_responses = [] + + def dispatch(self, request, *args, **kwargs): + for prerequisite_response in self.prerequisite_responses: + if not hasattr(self.request.response_set, prerequisite_response): + return redirect(self.get_redirect_url(prerequisite_response)) + + return super().dispatch(request, *args, **kwargs) + + def get_redirect_url(self, prerequisite_response): + return reverse( + self.RESPONSE_URL_MAPPING[prerequisite_response], + query=self.change_query_params() + ) + + def change_query_params(self): + if not bool(self.request.GET.get("change")): + return {} + + return {"change": "True"} diff --git a/lung_cancer_screening/questions/views/when_you_quit_smoking.py b/lung_cancer_screening/questions/views/when_you_quit_smoking.py index 81b596cb..e95af83a 100644 --- a/lung_cancer_screening/questions/views/when_you_quit_smoking.py +++ b/lung_cancer_screening/questions/views/when_you_quit_smoking.py @@ -3,6 +3,7 @@ from .mixins.ensure_response_set import EnsureResponseSet from .mixins.ensure_eligible import EnsureEligibleMixin +from .mixins.ensure_prerequisite_responses import EnsurePrerequisiteResponsesMixin from .question_base_view import QuestionBaseView from ..forms.when_you_quit_smoking_form import WhenYouQuitSmokingForm from ..models.when_you_quit_smoking_response import WhenYouQuitSmokingResponse @@ -12,6 +13,7 @@ class WhenYouQuitSmokingView( LoginRequiredMixin, EnsureResponseSet, EnsureEligibleMixin, + EnsurePrerequisiteResponsesMixin, QuestionBaseView ): template_name = "when_you_quit_smoking.jinja" @@ -19,7 +21,7 @@ class WhenYouQuitSmokingView( model = WhenYouQuitSmokingResponse success_url = reverse_lazy("questions:periods_when_you_stopped_smoking") back_link_url = reverse_lazy("questions:age_when_started_smoking") - # page_title = "How old were you when you started smoking? – NHS" + prerequisite_responses = ["age_when_started_smoking_response"] def get_success_url(self): if self.is_changing_responses(): From b34929da2bcb1cd5c05a09d4553041cd795a5144 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Wed, 1 Apr 2026 12:13:56 +0100 Subject: [PATCH 5/9] PPHA-593: Ensure users can change answers to quit smoking --- features/when_you_quit_smoking.feature | 28 ++++++++-------- .../presenters/response_set_presenter.py | 33 +++++++++++++------ 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/features/when_you_quit_smoking.feature b/features/when_you_quit_smoking.feature index ebdb5da9..ec658408 100644 --- a/features/when_you_quit_smoking.feature +++ b/features/when_you_quit_smoking.feature @@ -30,16 +30,18 @@ Feature: Age quit smoking When I fill in "How old were you when you quit smoking?" as "30" and submit Then I am on "/periods-when-you-stopped-smoking" - # Scenario: Checking responses and changing them - # Given I am logged in - # And I have answered questions showing I am eligible - # When I go to "/age-when-started-smoking" - # And I fill in "How old were you when you started smoking?" as "18" and submit - # When I go to "/check-your-answers" - # Then I see "18" as a response to "Age you started smoking" under "Smoking history" - # And I see "/age-when-started-smoking?change=True" as a link to change "Age you started smoking" under "Smoking history" - # When I click the link to change "Age you started smoking" under "Smoking History" - # Then I am on "/age-when-started-smoking?change=True" - # And I see "18" filled in for "How old were you when you started smoking?" - # When I fill in "How old were you when you started smoking?" as "22" and submit - # Then I am on "/periods-when-you-stopped-smoking?change=True" + Scenario: Checking responses and changing them + Given I am logged in + And I have answered questions showing I am eligible + And I am 60 years old + And I have answered questions showing I started smoking "30" years ago + When I go to "/when-you-quit-smoking" + And I fill in "How old were you when you quit smoking?" as "40" and submit + When I go to "/check-your-answers" + Then I see "40" as a response to "Age you quit smoking" under "Smoking history" + And I see "/when-you-quit-smoking?change=True" as a link to change "Age you quit smoking" under "Smoking history" + When I click the link to change "Age you quit smoking" under "Smoking History" + Then I am on "/when-you-quit-smoking?change=True" + And I see "40" filled in for "How old were you when you quit smoking?" + When I fill in "How old were you when you quit smoking?" as "45" and submit + Then I am on "/periods-when-you-stopped-smoking?change=True" diff --git a/lung_cancer_screening/questions/presenters/response_set_presenter.py b/lung_cancer_screening/questions/presenters/response_set_presenter.py index 5b638c9f..fc411d03 100644 --- a/lung_cancer_screening/questions/presenters/response_set_presenter.py +++ b/lung_cancer_screening/questions/presenters/response_set_presenter.py @@ -21,15 +21,6 @@ def have_you_ever_smoked(self): return self.response_set.have_you_ever_smoked_response.get_value_display() - @property - def periods_when_you_stopped_smoking(self): - if not hasattr(self.response_set, 'periods_when_you_stopped_smoking_response'): - return self.NOT_ANSWERED_TEXT - - if self.response_set.periods_when_you_stopped_smoking_response.value: - return f"Yes ({self.response_set.periods_when_you_stopped_smoking_response.duration_years} years)" - else: - return "No" @property def date_of_birth(self): @@ -130,6 +121,23 @@ def age_when_started_smoking(self): return str(self.response_set.age_when_started_smoking_response.value) + @property + def age_when_quit_smoking(self): + if not hasattr(self.response_set, "when_you_quit_smoking_response"): + return self.NOT_ANSWERED_TEXT + + return str(self.response_set.when_you_quit_smoking_response.value) + + @property + def periods_when_you_stopped_smoking(self): + if not hasattr(self.response_set, "periods_when_you_stopped_smoking_response"): + return self.NOT_ANSWERED_TEXT + + if self.response_set.periods_when_you_stopped_smoking_response.value: + return f"Yes ({self.response_set.periods_when_you_stopped_smoking_response.duration_years} years)" + else: + return "No" + @property def respiratory_conditions(self): if not hasattr(self.response_set, 'respiratory_conditions_response'): @@ -263,6 +271,11 @@ def smoking_history_responses_items(self): self.age_when_started_smoking, "questions:age_when_started_smoking", ), + self._check_your_answer_item( + "Age you quit smoking", + self.age_when_quit_smoking, + "questions:when_you_quit_smoking", + ), self._check_your_answer_item( "Have you ever stopped smoking for periods of 1 year or longer?", self.periods_when_you_stopped_smoking, @@ -272,7 +285,7 @@ def smoking_history_responses_items(self): "Types of tobacco smoked", self.types_tobacco_smoking, "questions:types_tobacco_smoking", - ) + ), ] From 637aa6122793f89a2f68f300483cfe340fc832c2 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Wed, 1 Apr 2026 16:00:05 +0100 Subject: [PATCH 6/9] PPHA-593: Only former smoker is asked when they quit --- features/age_when_started_smoking.feature | 30 +++++++- features/steps/preflight_steps.py | 12 ++- .../forms/test_when_you_quit_smoking_form.py | 3 - .../views/test_age_when_started_smoking.py | 77 +++++++++++++++---- .../test_relatives_age_when_diagnosed.py | 2 +- .../views/age_when_started_smoking.py | 11 ++- .../questions/views/when_you_quit_smoking.py | 12 +-- 7 files changed, 109 insertions(+), 38 deletions(-) diff --git a/features/age_when_started_smoking.feature b/features/age_when_started_smoking.feature index b3473365..d43541af 100644 --- a/features/age_when_started_smoking.feature +++ b/features/age_when_started_smoking.feature @@ -21,17 +21,28 @@ Feature: Age when started smoking Then I see a form error "The age you started smoking must be the same as, or less than your current age" And there are no accessibility violations - Scenario: Navigating backwards and forwards + Scenario: Navigating backwards and forwards as a former smoker Given I am logged in And I have answered questions showing I am eligible + And I have answered questions showing I am a former smoker + When I go to "/age-when-started-smoking" + Then I see a back link to "/relatives-age-when-diagnosed" + When I fill in "How old were you when you started smoking?" as "18" and submit + Then I am on "/when-you-quit-smoking" + + Scenario: Navigating backwards and forwards as a current smoker + Given I am logged in + And I have answered questions showing I am eligible + And I have answered questions showing I am a current smoker When I go to "/age-when-started-smoking" Then I see a back link to "/relatives-age-when-diagnosed" When I fill in "How old were you when you started smoking?" as "18" and submit Then I am on "/periods-when-you-stopped-smoking" - Scenario: Checking responses and changing them + Scenario: Checking responses and changing them as a current smoker Given I am logged in And I have answered questions showing I am eligible + And I have answered questions showing I am a current smoker When I go to "/age-when-started-smoking" And I fill in "How old were you when you started smoking?" as "18" and submit When I go to "/check-your-answers" @@ -42,3 +53,18 @@ Feature: Age when started smoking And I see "18" filled in for "How old were you when you started smoking?" When I fill in "How old were you when you started smoking?" as "22" and submit Then I am on "/periods-when-you-stopped-smoking?change=True" + + Scenario: Checking responses and changing them as a former smoker + Given I am logged in + And I have answered questions showing I am eligible + And I have answered questions showing I am a former smoker + When I go to "/age-when-started-smoking" + And I fill in "How old were you when you started smoking?" as "18" and submit + When I go to "/check-your-answers" + Then I see "18" as a response to "Age you started smoking" under "Smoking history" + And I see "/age-when-started-smoking?change=True" as a link to change "Age you started smoking" under "Smoking history" + When I click the link to change "Age you started smoking" under "Smoking History" + Then I am on "/age-when-started-smoking?change=True" + And I see "18" filled in for "How old were you when you started smoking?" + When I fill in "How old were you when you started smoking?" as "22" and submit + Then I am on "/when-you-quit-smoking?change=True" diff --git a/features/steps/preflight_steps.py b/features/steps/preflight_steps.py index d84b11b9..39e42e7a 100644 --- a/features/steps/preflight_steps.py +++ b/features/steps/preflight_steps.py @@ -68,11 +68,17 @@ def given_i_have_answered_questions_showing_i_am_eligible(context): eligible=True, ) -@given("I have answered questions showing I am a current smoker") -def given_i_have_answered_questions_showing_i_am_a_current_smoker(context): +@given("I have answered questions showing I am a {current_or_former} smoker") +def given_i_have_answered_questions_showing_i_am_a_current_smoker(context, current_or_former): context.page.goto(f"{context.live_server_url}/have-you-ever-smoked") - when_i_check_label(context, "Yes, I currently smoke") + if current_or_former == "current": + when_i_check_label(context, "Yes, I currently smoke") + elif current_or_former == "former": + when_i_check_label(context, "Yes, I used to smoke") + else: + raise ValueError(f"Invalid current_or_former: {current_or_former}") + when_i_submit_the_form(context) @given('I have answered questions showing I started smoking "{years}" years ago') diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_when_you_quit_smoking_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_when_you_quit_smoking_form.py index e8be9fac..f47c7beb 100644 --- a/lung_cancer_screening/questions/tests/unit/forms/test_when_you_quit_smoking_form.py +++ b/lung_cancer_screening/questions/tests/unit/forms/test_when_you_quit_smoking_form.py @@ -1,9 +1,6 @@ from django.test import TestCase, tag -from django.utils import timezone -from dateutil.relativedelta import relativedelta -from ....models.age_when_started_smoking_response import AgeWhenStartedSmokingResponse from ...factories.response_set_factory import ResponseSetFactory from ...factories.date_of_birth_response_factory import DateOfBirthResponseFactory diff --git a/lung_cancer_screening/questions/tests/unit/views/test_age_when_started_smoking.py b/lung_cancer_screening/questions/tests/unit/views/test_age_when_started_smoking.py index 726c1202..781706ca 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_age_when_started_smoking.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_age_when_started_smoking.py @@ -3,6 +3,7 @@ from .helpers.authentication import login_user from ...factories.response_set_factory import ResponseSetFactory +from ...factories.have_you_ever_smoked_response_factory import HaveYouEverSmokedResponseFactory @tag("AgeWhenStartedSmoking") class TestGetAgeWhenStartedSmoking(TestCase): @@ -73,8 +74,11 @@ def test_responds_successfully(self): class TestPostAgeWhenStartedSmoking(TestCase): def setUp(self): self.user = login_user(self.client) + self.response_set = ResponseSetFactory.create(user=self.user, eligible=True) - self.valid_params = {"value": 18} + self.valid_params = { + "value": self.response_set.date_of_birth_response.age_in_years() - 20 + } def test_redirects_if_the_user_is_not_logged_in(self): self.client.logout() @@ -90,11 +94,10 @@ def test_redirects_if_the_user_is_not_logged_in(self): fetch_redirect_response=False ) + def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(self): - ResponseSetFactory.create( - user=self.user, - recently_submitted=True - ) + self.response_set.delete() + ResponseSetFactory.create(user=self.user, recently_submitted=True) response = self.client.post( reverse("questions:age_when_started_smoking"), @@ -103,7 +106,9 @@ def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(sel self.assertRedirects(response, reverse("questions:confirmation")) + def test_redirects_when_the_user_is_not_eligible(self): + self.response_set.delete() ResponseSetFactory.create(user=self.user) response = self.client.post( @@ -113,18 +118,22 @@ def test_redirects_when_the_user_is_not_eligible(self): self.assertRedirects(response, reverse("questions:agree_terms_of_use")) - def test_creates_an_age_when_started_smoking_response(self): - response_set = ResponseSetFactory.create(user=self.user, eligible=True) + def test_creates_an_age_when_started_smoking_response(self): self.client.post(reverse("questions:age_when_started_smoking"), self.valid_params) - response_set.refresh_from_db() + self.response_set.refresh_from_db() self.assertEqual( - response_set.age_when_started_smoking_response.value, self.valid_params["value"] + self.response_set.age_when_started_smoking_response.value, self.valid_params["value"] ) - def test_redirects_to_periods_when_you_stopped_smoking(self): - ResponseSetFactory.create(user=self.user, eligible=True) + + def test_redirects_to_periods_when_you_stopped_smoking_as_a_current_smoker(self): + self.response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, + current_smoker=True + ) response = self.client.post( reverse("questions:age_when_started_smoking"), @@ -133,8 +142,12 @@ def test_redirects_to_periods_when_you_stopped_smoking(self): self.assertRedirects(response, reverse("questions:periods_when_you_stopped_smoking")) - def test_redirects_to_periods_when_you_stopped_smoking_if_change_query_param_is_true(self): - ResponseSetFactory.create(user=self.user, eligible=True) + + def test_redirects_to_periods_when_you_stopped_smoking_as_a_current_smoker_if_change_query_param_is_true(self): + self.response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, current_smoker=True + ) response = self.client.post( reverse("questions:age_when_started_smoking"), @@ -150,9 +163,43 @@ def test_redirects_to_periods_when_you_stopped_smoking_if_change_query_param_is_ ) - def test_responds_with_422_if_the_response_fails_to_create(self): - ResponseSetFactory.create(user=self.user, eligible=True) + def test_redirects_to_when_you_quit_smoking_as_a_former_smoker(self): + self.response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, + former_smoker=True + ) + + response = self.client.post( + reverse("questions:age_when_started_smoking"), + self.valid_params + ) + + self.assertRedirects(response, reverse("questions:when_you_quit_smoking")) + + + def test_redirects_to_when_you_quit_smoking_as_a_former_smoker_if_change_query_param_is_true(self): + self.response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, + former_smoker=True + ) + + response = self.client.post( + reverse("questions:age_when_started_smoking"), + { + **self.valid_params, + "change": "True" + } + ) + + self.assertRedirects( + response, + reverse("questions:when_you_quit_smoking", query={"change": "True"}) + ) + + def test_responds_with_422_if_the_response_fails_to_create(self): response = self.client.post( reverse("questions:age_when_started_smoking"), {"value": "not a valid age"} diff --git a/lung_cancer_screening/questions/tests/unit/views/test_relatives_age_when_diagnosed.py b/lung_cancer_screening/questions/tests/unit/views/test_relatives_age_when_diagnosed.py index 15447683..f60cc3e7 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_relatives_age_when_diagnosed.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_relatives_age_when_diagnosed.py @@ -99,7 +99,7 @@ def test_back_link_url_points_to_family_history_lung_cancer_if_change_query_para self.assertEqual(response.context_data["back_link_url"], reverse("questions:family_history_lung_cancer")) - @tag("wip") + def test_back_link_url_points_to_family_history_lung_cancer_if_change_query_param_is_true_and_they_came_from_family_history(self): FamilyHistoryLungCancerResponseFactory( response_set=ResponseSetFactory.create(user=self.user, eligible=True), diff --git a/lung_cancer_screening/questions/views/age_when_started_smoking.py b/lung_cancer_screening/questions/views/age_when_started_smoking.py index 9edc6d8e..94e732ec 100644 --- a/lung_cancer_screening/questions/views/age_when_started_smoking.py +++ b/lung_cancer_screening/questions/views/age_when_started_smoking.py @@ -15,12 +15,11 @@ class AgeWhenStartedSmokingView(LoginRequiredMixin, EnsureResponseSet, EnsureEli back_link_url = reverse_lazy("questions:relatives_age_when_diagnosed") page_title = "How old were you when you started smoking? – NHS" + def get_success_url(self): - if self.is_changing_responses(): - return reverse( - "questions:periods_when_you_stopped_smoking", - query={"change": "True"} - ) + if self.request.response_set.current_smoker(): + url_lookup = "questions:periods_when_you_stopped_smoking" else: - return super().get_success_url() + url_lookup = "questions:when_you_quit_smoking" + return reverse(url_lookup, query=self.get_change_query_params()) diff --git a/lung_cancer_screening/questions/views/when_you_quit_smoking.py b/lung_cancer_screening/questions/views/when_you_quit_smoking.py index e95af83a..cd08c32f 100644 --- a/lung_cancer_screening/questions/views/when_you_quit_smoking.py +++ b/lung_cancer_screening/questions/views/when_you_quit_smoking.py @@ -19,15 +19,11 @@ class WhenYouQuitSmokingView( template_name = "when_you_quit_smoking.jinja" form_class = WhenYouQuitSmokingForm model = WhenYouQuitSmokingResponse - success_url = reverse_lazy("questions:periods_when_you_stopped_smoking") back_link_url = reverse_lazy("questions:age_when_started_smoking") prerequisite_responses = ["age_when_started_smoking_response"] def get_success_url(self): - if self.is_changing_responses(): - return reverse( - "questions:periods_when_you_stopped_smoking", - query={"change": "True"} - ) - else: - return super().get_success_url() + return reverse( + "questions:periods_when_you_stopped_smoking", + query=self.get_change_query_params() + ) From 7d9baed7a44f43d4652fdc1c3f8156cdcabfbb66 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Wed, 1 Apr 2026 16:30:37 +0100 Subject: [PATCH 7/9] PPHA-593: Ensure quit smoking must be answered for former smokers --- .../models/have_you_ever_smoked_response.py | 4 ++ .../questions/models/response_set.py | 10 ++++ .../have_you_ever_smoked_response_factory.py | 2 +- .../test_have_you_ever_smoked_response.py | 18 ++++++ .../tests/unit/models/test_response_set.py | 59 ++++++++++++++++--- 5 files changed, 85 insertions(+), 8 deletions(-) diff --git a/lung_cancer_screening/questions/models/have_you_ever_smoked_response.py b/lung_cancer_screening/questions/models/have_you_ever_smoked_response.py index a83b4a64..426f031e 100644 --- a/lung_cancer_screening/questions/models/have_you_ever_smoked_response.py +++ b/lung_cancer_screening/questions/models/have_you_ever_smoked_response.py @@ -33,3 +33,7 @@ def is_eligible(self): def is_current_smoker(self): return self.value == HaveYouEverSmokedValues.YES_I_CURRENTLY_SMOKE.value + + + def is_former_smoker(self): + return self.value == HaveYouEverSmokedValues.YES_I_USED_TO_SMOKE_REGULARLY.value diff --git a/lung_cancer_screening/questions/models/response_set.py b/lung_cancer_screening/questions/models/response_set.py index 187a9ba7..4648569d 100644 --- a/lung_cancer_screening/questions/models/response_set.py +++ b/lung_cancer_screening/questions/models/response_set.py @@ -85,6 +85,9 @@ def _response_attrs(self): if hasattr(self, 'family_history_lung_cancer') and self.family_history_lung_cancer.is_truthy(): response_attrs.append('relatives_age_when_diagnosed') + if self.former_smoker(): + response_attrs.append('when_you_quit_smoking_response') + return response_attrs @@ -170,3 +173,10 @@ def current_smoker(self): return None return self.have_you_ever_smoked_response.is_current_smoker() + + + def former_smoker(self): + if not hasattr(self, 'have_you_ever_smoked_response'): + return None + + return self.have_you_ever_smoked_response.is_former_smoker() diff --git a/lung_cancer_screening/questions/tests/factories/have_you_ever_smoked_response_factory.py b/lung_cancer_screening/questions/tests/factories/have_you_ever_smoked_response_factory.py index fa90db99..8fd9cdcf 100644 --- a/lung_cancer_screening/questions/tests/factories/have_you_ever_smoked_response_factory.py +++ b/lung_cancer_screening/questions/tests/factories/have_you_ever_smoked_response_factory.py @@ -13,7 +13,7 @@ class Meta: class Params: eligible = factory.Trait( - value=factory.Iterator(HaveYouEverSmokedResponse.ELIGIBLE_VALUES) + value=HaveYouEverSmokedResponse.Values.YES_I_CURRENTLY_SMOKE.value ) ineligible = factory.Trait( diff --git a/lung_cancer_screening/questions/tests/unit/models/test_have_you_ever_smoked_response.py b/lung_cancer_screening/questions/tests/unit/models/test_have_you_ever_smoked_response.py index 57fc1603..96993f71 100644 --- a/lung_cancer_screening/questions/tests/unit/models/test_have_you_ever_smoked_response.py +++ b/lung_cancer_screening/questions/tests/unit/models/test_have_you_ever_smoked_response.py @@ -85,3 +85,21 @@ def test_is_current_smoker_returns_false_when_they_used_to_smoke_regularly(self) ) self.assertFalse(response.is_current_smoker()) + + + def test_is_former_smoker_returns_true_when_they_are_a_former_smoker(self): + response = HaveYouEverSmokedResponse.objects.create( + response_set=self.response_set, + value=HaveYouEverSmokedValues.YES_I_USED_TO_SMOKE_REGULARLY + ) + + self.assertTrue(response.is_former_smoker()) + + + def test_is_former_smoker_returns_false_when_they_are_not_a_former_smoker(self): + response = HaveYouEverSmokedResponse.objects.create( + response_set=self.response_set, + value=HaveYouEverSmokedValues.YES_I_CURRENTLY_SMOKE + ) + + self.assertFalse(response.is_former_smoker()) diff --git a/lung_cancer_screening/questions/tests/unit/models/test_response_set.py b/lung_cancer_screening/questions/tests/unit/models/test_response_set.py index fa4a34e3..21e762b2 100644 --- a/lung_cancer_screening/questions/tests/unit/models/test_response_set.py +++ b/lung_cancer_screening/questions/tests/unit/models/test_response_set.py @@ -11,6 +11,8 @@ from ...factories.have_you_ever_smoked_response_factory import HaveYouEverSmokedResponseFactory from ...factories.date_of_birth_response_factory import DateOfBirthResponseFactory from ...factories.check_need_appointment_response_factory import CheckNeedAppointmentResponseFactory +from ...factories.when_you_quit_smoking_response_factory import WhenYouQuitSmokingResponseFactory + from ....models.user import User from ....models.family_history_lung_cancer_response import FamilyHistoryLungCancerValues from ....models.have_you_ever_smoked_response import HaveYouEverSmokedValues @@ -195,18 +197,40 @@ def test_is_complete_returns_true_if_family_history_cancer_no_and_none_age_diagn self.assertTrue(response_set.is_complete()) - def test_is_complete_returns_false_if_any_tobacco_smoking_history_is_not_complete(self): + def test_is_complete_returns_true_if_current_smoker_and_when_you_quit_smoking_is_incomplete(self): response_set = ResponseSetFactory.create(complete=True) - response_set.tobacco_smoking_history.first().smoking_current_response.delete() - response_set.refresh_from_db() + response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=response_set, + current_smoker=True + ) - self.assertFalse(response_set.is_complete()) + self.assertTrue(response_set.is_complete()) - def test_is_complete_returns_false_if_there_are_no_smoking_histories(self): + def test_is_complete_returns_true_if_former_smoker_and_when_you_quit_smoking_is_complete(self): response_set = ResponseSetFactory.create(complete=True) - response_set.tobacco_smoking_history.all().delete() - response_set.refresh_from_db() + response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=response_set, + former_smoker=True + ) + WhenYouQuitSmokingResponseFactory.create( + response_set=response_set, + value=response_set.age_when_started_smoking_response.value + 1, + ) + + self.assertTrue(response_set.is_complete()) + + + def test_is_complete_returns_false_if_former_smoker_and_when_you_quit_smoking_is_incomplete(self): + response_set = ResponseSetFactory.create(complete=True) + response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=response_set, + former_smoker=True + ) + self.assertFalse(response_set.is_complete()) @@ -490,5 +514,26 @@ def test_current_smoker_returns_false_when_the_user_is_not_currently_smoking(sel ) self.assertFalse(self.response_set.current_smoker()) + def test_current_smoker_returns_none_when_the_has_not_answered_the_question(self): self.assertIsNone(self.response_set.current_smoker()) + + + def test_former_smoker_returns_true_when_the_user_is_a_former_smoker(self): + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, + value=HaveYouEverSmokedValues.YES_I_USED_TO_SMOKE_REGULARLY.value, + ) + self.assertTrue(self.response_set.former_smoker()) + + + def test_former_smoker_returns_false_when_the_user_is_not_a_former_smoker(self): + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, + value=HaveYouEverSmokedValues.YES_I_CURRENTLY_SMOKE.value, + ) + self.assertFalse(self.response_set.former_smoker()) + + + def test_former_smoker_returns_none_when_the_has_not_answered_the_question(self): + self.assertIsNone(self.response_set.former_smoker()) From 9f68b88f8bc13a811c0a48ef09df0e5e80553669 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Thu, 2 Apr 2026 10:07:14 +0100 Subject: [PATCH 8/9] PPHA-593: Add correct back links for quit smoking --- features/when_you_quit_smoking.feature | 1 + .../presenters/response_set_presenter.py | 6 +- .../presenters/test_response_set_presenter.py | 43 ++++++++++++ .../test_periods_when_you_stopped_smoking.py | 66 +++++++++++++++++-- .../unit/views/test_when_you_quit_smoking.py | 12 +++- .../views/periods_when_you_stopped_smoking.py | 20 +++++- .../questions/views/question_base_view.py | 1 + 7 files changed, 138 insertions(+), 11 deletions(-) diff --git a/features/when_you_quit_smoking.feature b/features/when_you_quit_smoking.feature index ec658408..4288312b 100644 --- a/features/when_you_quit_smoking.feature +++ b/features/when_you_quit_smoking.feature @@ -33,6 +33,7 @@ Feature: Age quit smoking 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 am a former smoker And I am 60 years old And I have answered questions showing I started smoking "30" years ago When I go to "/when-you-quit-smoking" diff --git a/lung_cancer_screening/questions/presenters/response_set_presenter.py b/lung_cancer_screening/questions/presenters/response_set_presenter.py index fc411d03..a64b7d73 100644 --- a/lung_cancer_screening/questions/presenters/response_set_presenter.py +++ b/lung_cancer_screening/questions/presenters/response_set_presenter.py @@ -265,7 +265,7 @@ def family_history_responses_items(self): return items def smoking_history_responses_items(self): - return [ + return list(filter(None, [ self._check_your_answer_item( "Age you started smoking", self.age_when_started_smoking, @@ -275,7 +275,7 @@ def smoking_history_responses_items(self): "Age you quit smoking", self.age_when_quit_smoking, "questions:when_you_quit_smoking", - ), + ) if self.response_set.former_smoker() else None, self._check_your_answer_item( "Have you ever stopped smoking for periods of 1 year or longer?", self.periods_when_you_stopped_smoking, @@ -286,7 +286,7 @@ def smoking_history_responses_items(self): self.types_tobacco_smoking, "questions:types_tobacco_smoking", ), - ] + ])) def is_complete(self): diff --git a/lung_cancer_screening/questions/tests/unit/presenters/test_response_set_presenter.py b/lung_cancer_screening/questions/tests/unit/presenters/test_response_set_presenter.py index d5907765..4bb4345a 100644 --- a/lung_cancer_screening/questions/tests/unit/presenters/test_response_set_presenter.py +++ b/lung_cancer_screening/questions/tests/unit/presenters/test_response_set_presenter.py @@ -33,6 +33,7 @@ from ....presenters.response_set_presenter import ResponseSetPresenter +@tag("ResponseSetPresenter", "presenters") class TestResponseSetPresenter(TestCase): def setUp(self): self.response_set = ResponseSetFactory.create() @@ -470,3 +471,45 @@ def test_check_your_answer_item_has_visually_hidden_text(self): answers_item["actions"]["items"][0]["visuallyHiddenText"], "answer for have you ever smoked" ) + + + def test_smoking_history_responses_items_returns_the_correct_items_when_current_smoker(self): + HaveYouEverSmokedResponseFactory( + response_set=self.response_set, + current_smoker=True + ) + + presenter = ResponseSetPresenter(self.response_set) + + self.assertEqual( + [ + item.get("key").get("text") + for item in presenter.smoking_history_responses_items() + ], + [ + "Age you started smoking", + "Have you ever stopped smoking for periods of 1 year or longer?", + "Types of tobacco smoked", + ] + ) + + + def test_smoking_history_responses_items_returns_the_correct_items_when_former_smoker(self): + HaveYouEverSmokedResponseFactory( + response_set=self.response_set, + former_smoker=True + ) + presenter = ResponseSetPresenter(self.response_set) + + self.assertEqual( + [ + item.get("key").get("text") + for item in presenter.smoking_history_responses_items() + ], + [ + "Age you started smoking", + "Age you quit smoking", + "Have you ever stopped smoking for periods of 1 year or longer?", + "Types of tobacco smoked", + ], + ) diff --git a/lung_cancer_screening/questions/tests/unit/views/test_periods_when_you_stopped_smoking.py b/lung_cancer_screening/questions/tests/unit/views/test_periods_when_you_stopped_smoking.py index a995a8c4..23d653ea 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_periods_when_you_stopped_smoking.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_periods_when_you_stopped_smoking.py @@ -4,6 +4,7 @@ from .helpers.authentication import login_user from ...factories.response_set_factory import ResponseSetFactory from ...factories.age_when_started_smoking_response_factory import AgeWhenStartedSmokingResponseFactory +from ...factories.have_you_ever_smoked_response_factory import HaveYouEverSmokedResponseFactory @tag("PeriodsWhenYouStoppedSmoking") @@ -47,17 +48,72 @@ def test_redirects_when_the_user_is_not_eligible(self): self.assertRedirects(response, reverse("questions:agree_terms_of_use")) - def test_back_link_url_is_responses_if_change_query_param_is_true(self): + + def test_back_link_url_is_age_when_started_smoking_if_user_is_current_smoker(self): + self.response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, + current_smoker=True + ) response = self.client.get( - reverse("questions:periods_when_you_stopped_smoking", query={"change": "True"}) + reverse("questions:periods_when_you_stopped_smoking") + ) + self.assertEqual(response.context_data["back_link_url"], reverse("questions:age_when_started_smoking")) + + + def test_back_link_url_is_when_you_quit_smoking_if_user_is_former_smoker(self): + self.response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, + former_smoker=True + ) + response = self.client.post(reverse("questions:periods_when_you_stopped_smoking")) + self.assertEqual(response.context_data["back_link_url"], reverse("questions:when_you_quit_smoking")) + + + def test_back_link_url_is_responses_if_change_query_param_is_true_and_they_came_from_responses(self): + response = self.client.get( + reverse("questions:periods_when_you_stopped_smoking", query={"change": "True"}), + headers={"Referer": reverse("questions:responses")} ) self.assertEqual(response.context_data["back_link_url"], reverse("questions:responses")) - def test_back_link_url_is_age_when_started_smoking_if_change_query_param_is_not_true(self): + + def test_back_link_url_is_age_started_smoking_if_change_query_param_is_true_and_user_is_current_smoker_and_they_did_not_come_directly_from_responses(self): + self.response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, + current_smoker=True + ) response = self.client.get( - reverse("questions:periods_when_you_stopped_smoking") + reverse("questions:periods_when_you_stopped_smoking", query={"change": "True"}) ) - self.assertEqual(response.context_data["back_link_url"], reverse("questions:age_when_started_smoking")) + + self.assertEqual( + response.context_data["back_link_url"], + reverse("questions:age_when_started_smoking", query={"change": "True"}) + ) + + + def test_back_link_url_is_when_you_quit_if_change_query_param_is_true_and_user_is_former_smoker_and_they_did_not_come_directly_from_responses( + self, + ): + self.response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory.create( + response_set=self.response_set, former_smoker=True + ) + response = self.client.get( + reverse( + "questions:periods_when_you_stopped_smoking", query={"change": "True"} + ), + headers={"Referer": reverse("questions:age_when_started_smoking")}, + ) + + self.assertEqual( + response.context_data["back_link_url"], + reverse("questions:when_you_quit_smoking", query={"change": "True"}), + ) + def test_responds_successfully(self): response = self.client.get(reverse("questions:periods_when_you_stopped_smoking")) diff --git a/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py b/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py index 33eca42d..ad36681d 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py @@ -68,7 +68,17 @@ def test_has_a_back_link_to_age_started_smoking(self): reverse("questions:age_when_started_smoking"), ) - def test_has_a_back_link_when_change_is_true(self): + + def test_has_a_back_link_to_responses_if_change_is_true_and_they_came_from_responses(self): + response = self.client.get( + reverse("questions:when_you_quit_smoking"), + {"change": "True"}, + headers={"Referer": reverse("questions:responses")} + ) + self.assertEqual(response.context_data["back_link_url"], reverse("questions:responses")) + + + def test_has_a_back_link_to_age_started_smoking_if_change_is_true_and_they_did_not_come_directly_from_responses(self): response = self.client.get( reverse("questions:when_you_quit_smoking"), {"change": "True"} diff --git a/lung_cancer_screening/questions/views/periods_when_you_stopped_smoking.py b/lung_cancer_screening/questions/views/periods_when_you_stopped_smoking.py index b5b8606b..e62a95f0 100644 --- a/lung_cancer_screening/questions/views/periods_when_you_stopped_smoking.py +++ b/lung_cancer_screening/questions/views/periods_when_you_stopped_smoking.py @@ -1,4 +1,4 @@ -from django.urls import reverse_lazy +from django.urls import reverse_lazy, reverse from django.contrib.auth.mixins import LoginRequiredMixin from .mixins.ensure_response_set import EnsureResponseSet @@ -13,10 +13,26 @@ class PeriodsWhenYouStoppedSmokingView(LoginRequiredMixin, EnsureResponseSet, En form_class = PeriodsWhenYouStoppedSmokingForm model = PeriodsWhenYouStoppedSmokingResponse success_url = reverse_lazy("questions:types_tobacco_smoking") - back_link_url = reverse_lazy("questions:age_when_started_smoking") def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) form = context.get("form") or self.get_form() context["page_title"] = form.page_title() return context + + def came_from_responses(self): + return self.request.headers.get("Referer", "").endswith( + reverse("questions:responses") + ) + + + def get_back_link_url(self): + if self.is_changing_responses() and self.came_from_responses(): + return reverse("questions:responses") + + if self.request.response_set.former_smoker(): + url_lookup = "questions:when_you_quit_smoking" + else: + url_lookup = "questions:age_when_started_smoking" + + return reverse(url_lookup, query=self.get_change_query_params()) diff --git a/lung_cancer_screening/questions/views/question_base_view.py b/lung_cancer_screening/questions/views/question_base_view.py index eea1899a..1c995015 100644 --- a/lung_cancer_screening/questions/views/question_base_view.py +++ b/lung_cancer_screening/questions/views/question_base_view.py @@ -15,6 +15,7 @@ def get_change_query_params(self): return {"change": "True"} + def get_back_link_url(self): if self.is_changing_responses(): return reverse("questions:responses") From e84affa492b99f5511c7bd59c8ef870c73f2c2e3 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Thu, 2 Apr 2026 10:19:05 +0100 Subject: [PATCH 9/9] PPHA-593: Ensure quit page cannot be accessed for current smokers --- features/when_you_quit_smoking.feature | 3 +++ .../unit/views/test_when_you_quit_smoking.py | 15 +++++++++++++++ .../questions/views/when_you_quit_smoking.py | 10 ++++++++++ 3 files changed, 28 insertions(+) diff --git a/features/when_you_quit_smoking.feature b/features/when_you_quit_smoking.feature index 4288312b..885c908f 100644 --- a/features/when_you_quit_smoking.feature +++ b/features/when_you_quit_smoking.feature @@ -4,6 +4,7 @@ Feature: Age quit smoking 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 am a former smoker And I am 60 years old And I have answered questions showing I started smoking "30" years ago When I go to "/age-when-started-smoking" @@ -12,6 +13,7 @@ Feature: Age quit smoking Scenario: Form errors Given I am logged in And I have answered questions showing I am eligible + And I have answered questions showing I am a former smoker And I am 60 years old And I have answered questions showing I started smoking "30" years ago When I go to "/when-you-quit-smoking" @@ -23,6 +25,7 @@ Feature: Age quit smoking 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 am a former smoker And I am 60 years old And I have answered questions showing I started smoking "30" years ago When I go to "/when-you-quit-smoking" diff --git a/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py b/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py index ad36681d..6a9c2ed4 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_when_you_quit_smoking.py @@ -3,6 +3,7 @@ from .helpers.authentication import login_user from ...factories.response_set_factory import ResponseSetFactory +from ...factories.have_you_ever_smoked_response_factory import HaveYouEverSmokedResponseFactory from ...factories.age_when_started_smoking_response_factory import AgeWhenStartedSmokingResponseFactory @@ -11,6 +12,11 @@ class TestGetWhenYouQuitSmoking(TestCase): def setUp(self): self.user = login_user(self.client) self.response_set = ResponseSetFactory.create(user=self.user, eligible=True) + self.response_set.have_you_ever_smoked_response.delete() + self.have_you_ever_smoked_response = HaveYouEverSmokedResponseFactory( + response_set=self.response_set, + former_smoker=True + ) self.age_when_started_smoking_response = AgeWhenStartedSmokingResponseFactory( response_set=self.response_set, value=self.response_set.date_of_birth_response.age_in_years() - 20, @@ -54,6 +60,15 @@ def test_redirects_to_age_started_smoking_if_the_user_has_not_answered_age_start self.assertRedirects(response, reverse("questions:age_when_started_smoking")) + def test_redirects_to_periods_you_stopped_smoking_if_user_is_current_smoker(self): + self.response_set.have_you_ever_smoked_response.delete() + HaveYouEverSmokedResponseFactory( + response_set=self.response_set, + current_smoker=True + ) + response = self.client.get(reverse("questions:when_you_quit_smoking")) + self.assertRedirects(response, reverse("questions:periods_when_you_stopped_smoking")) + def test_responds_successfully(self): response = self.client.get(reverse("questions:when_you_quit_smoking")) diff --git a/lung_cancer_screening/questions/views/when_you_quit_smoking.py b/lung_cancer_screening/questions/views/when_you_quit_smoking.py index cd08c32f..500caabf 100644 --- a/lung_cancer_screening/questions/views/when_you_quit_smoking.py +++ b/lung_cancer_screening/questions/views/when_you_quit_smoking.py @@ -1,4 +1,5 @@ from django.urls import reverse, reverse_lazy +from django.shortcuts import redirect from django.contrib.auth.mixins import LoginRequiredMixin from .mixins.ensure_response_set import EnsureResponseSet @@ -9,11 +10,20 @@ from ..models.when_you_quit_smoking_response import WhenYouQuitSmokingResponse +class EnsureFormerSmokerMixin(): + def get(self, request, *args, **kwargs): + if not request.response_set.former_smoker(): + return redirect(reverse("questions:periods_when_you_stopped_smoking")) + + return super().get(request, *args, **kwargs) + + class WhenYouQuitSmokingView( LoginRequiredMixin, EnsureResponseSet, EnsureEligibleMixin, EnsurePrerequisiteResponsesMixin, + EnsureFormerSmokerMixin, QuestionBaseView ): template_name = "when_you_quit_smoking.jinja"