Skip to content

PPHA-529: Add smoked total years for cigarettes smoking type #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Themitchell merged 2 commits into from
Feb 3, 2026
Merged

PPHA-529: Add smoked total years for cigarettes smoking type #250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0fe6dcc
PPHA-529: Add smoked total years for cigarette type
Themitchell Jan 29, 2026
72739e1
PPHA-529: Smoked total years validation on age started
Themitchell Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions features/questionnaire.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ Feature: Questionnaire
And I check "Pipe"
And I submit the form

Then I am on "/cigarettes-smoked-total-years"
When I fill in "Roughly how many years have you smoked cigarettes?" with "10"
And I submit the form

Then I am on "/check-your-answers"
And I see "Yes, I used to smoke" as a response to "Have you ever smoked tobacco?" under "Eligibility"
And I see a date 55 years ago as a response to "Date of birth" under "Eligibility"
Expand All @@ -90,5 +94,7 @@ Feature: Questionnaire
And I see "18" as a response to "Age you started smoking" under "Smoking history"
And I see "Yes (10 years)" as a response to "Have you ever stopped smoking for periods of 1 year or longer?" under "Smoking history"

And I see "10" as a response to "Total number of years you have smoked cigarettes" under "Smoking history"

When I click "Submit"
Then I am on "/confirmation"
50 changes: 50 additions & 0 deletions features/smoked_total_years.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
@SmokedTotalYears
Feature: Smoked total years page
Scenario: The page is accessible
Given I am logged in
And I have answered questions showing I am eligible
And I have answered questions showing I have smoked for "10" years
And I have answered questions showing I have smoked "Cigarettes"
When I go to "/cigarettes-smoked-total-years"
Then there are no accessibility violations

Scenario: Form errors
Given I am logged in
And I have answered questions showing I am eligible
And I have answered questions showing I have smoked for "10" years
And I have answered questions showing I have smoked "Cigarettes"
When I go to "/cigarettes-smoked-total-years"
And I click "Continue"
Then I am on "/cigarettes-smoked-total-years"
And I see a form error "Enter the number of years you have smoked cigarettes"
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
And I have answered questions showing I have smoked for "10" years
And I have answered questions showing I have smoked "Cigarettes"
When I go to "/cigarettes-smoked-total-years"
Then I see a back link to "/types-tobacco-smoking"
When I fill in "Roughly how many years have you smoked cigarettes?" with "9"
And I submit the form
Then I am on "/check-your-answers"

Scenario: Checking responses and changing them
Given I am logged in
And I have answered questions showing I am eligible
And I have answered questions showing I have smoked for "10" years
And I have answered questions showing I have smoked "Cigarettes"
When I go to "/cigarettes-smoked-total-years"
When I fill in "Roughly how many years have you smoked cigarettes?" with "9"
And I submit the form
When I go to "/check-your-answers"
Then I see "9" as a response to "Total number of years you have smoked cigarettes" under "Smoking history"
And I see "/cigarettes-smoked-total-years?change=True" as a link to change "Total number of years you have smoked cigarettes" under "Smoking history"
When I click the link to change "Total number of years you have smoked cigarettes" under "Smoking history"
Then I am on "/cigarettes-smoked-total-years?change=True"
And I see "9" filled in for "Roughly how many years have you smoked cigarettes?"
When I fill in "Roughly how many years have you smoked cigarettes?" with "8"
And I click "Continue"
Then I am on "/check-your-answers"
And I see "8" as a response to "Total number of years you have smoked cigarettes" under "Smoking history"
12 changes: 12 additions & 0 deletions features/steps/preflight_steps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,15 @@ def given_i_have_answered_questions_showing_i_stopped_smoking_for_years_years(co
when_i_check_label(context, "Yes")
when_i_fill_in_label_with_value(context, "Enter the total number of years you stopped smoking for", years)
when_i_submit_the_form(context)


@given('I have answered questions showing I have smoked "{tobacco_types}"')
def given_i_have_answered_questions_showing_i_have_smoked_tobacco_type(
context, tobacco_types
):
context.page.goto(f"{context.live_server_url}/types-tobacco-smoking")

for tobacco_type in tobacco_types.split(","):
when_i_check_label(context, tobacco_type.strip())

when_i_submit_the_form(context)
11 changes: 8 additions & 3 deletions features/types_tobacco_smoking.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ Feature: Types tobacco smoking page
Scenario: The page is accessible
Given I am logged in
And I have answered questions showing I am eligible
And I have answered questions showing I have smoked for "10" years
When I go to "/types-tobacco-smoking"
Then there are no accessibility violations

Scenario: Form errors
Given I am logged in
And I have answered questions showing I am eligible
And I have answered questions showing I have smoked for "10" years
When I go to "/types-tobacco-smoking"
And I click "Continue"
Then I am on "/types-tobacco-smoking"
Expand All @@ -18,15 +20,17 @@ Feature: Types tobacco smoking page
Scenario: Navigating backwards and forwards
Given I am logged in
And I have answered questions showing I am eligible
And I have answered questions showing I have smoked for "10" years
When I go to "/types-tobacco-smoking"
Then I see a back link to "/periods-when-you-stopped-smoking"
When I check "Cigarettes"
And I submit the form
Then I am on "/check-your-answers"
Then I am on "/cigarettes-smoked-total-years"

Scenario: Checking responses and changing them
Given I am logged in
And I have answered questions showing I am eligible
And I have answered questions showing I have smoked for "10" years
When I go to "/types-tobacco-smoking"
And I check "Cigarettes"
And I check "Cigars"
Expand All @@ -41,5 +45,6 @@ Feature: Types tobacco smoking page
And I see "Cigars" selected
When I check "Pipe"
And I click "Continue"
Then I am on "/check-your-answers"
And I see "Cigarettes, Pipe, and Cigars" as a response to "Types of tobacco smoked" under "Smoking history"
Then I am on "/cigarettes-smoked-total-years"
When I go to "/check-your-answers"
Then I see "Cigarettes, Pipe, and Cigars" as a response to "Types of tobacco smoked" under "Smoking history"
28 changes: 28 additions & 0 deletions lung_cancer_screening/questions/forms/smoked_total_years_form.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from django import forms

from ...nhsuk_forms.integer_field import IntegerField
from ..models.smoked_total_years_response import SmokedTotalYearsResponse


class SmokedTotalYearsForm(forms.ModelForm):

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

self.fields["value"] = IntegerField(
label="Roughly how many years have you smoked cigarettes?",
label_is_page_heading=True,
label_classes="nhsuk-label--l",
classes="nhsuk-input--width-4",
hint="Give an estimate if you are not sure",
required=True,
suffix="years",
error_messages={
"required": "Enter the number of years you have smoked cigarettes",
"invalid": "Years must be in whole numbers"
},
)

class Meta:
model = SmokedTotalYearsResponse
fields = ['value']
2 changes: 1 addition & 1 deletion lung_cancer_screening/questions/jinja2/question_form.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
{% block page_content %}
<div class="nhsuk-grid-row">
<div class="nhsuk-grid-column-two-thirds">
<form action="{{ request.path }}" method="POST">
<form action="{{ request.path }}" method="POST" novalidate>
{{ csrf_input }}

{% if request.GET.get("change") == "True" %}
Expand Down
40 changes: 40 additions & 0 deletions lung_cancer_screening/questions/migrations/0049_smokedtotalyearsresponse_and_more.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Generated by Django 5.2.10 on 2026-01-28 15:38

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('questions', '0048_tobaccosmokinghistory_and_more'),
]

operations = [
migrations.CreateModel(
name='SmokedTotalYearsResponse',
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()),
],
options={
'abstract': False,
},
),
migrations.AlterField(
model_name='tobaccosmokinghistory',
name='response_set',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='tobacco_smoking_history', to='questions.responseset'),
),
migrations.AddConstraint(
model_name='tobaccosmokinghistory',
constraint=models.UniqueConstraint(fields=('response_set', 'type'), name='unique_tobacco_smoking_history_per_response_set', violation_error_message='A tobacco smoking history already exists for this response set and type'),
),
migrations.AddField(
model_name='smokedtotalyearsresponse',
name='tobacco_smoking_history',
field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='smoked_total_years_response', to='questions.tobaccosmokinghistory'),
),
]
48 changes: 48 additions & 0 deletions lung_cancer_screening/questions/models/smoked_total_years_response.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
from django.db import models
from django.core.exceptions import ValidationError
from django.core.validators import MinValueValidator

from .base import BaseModel
from .tobacco_smoking_history import TobaccoSmokingHistory


class SmokedTotalYearsResponse(BaseModel):
tobacco_smoking_history = models.OneToOneField(
TobaccoSmokingHistory,
on_delete=models.CASCADE,
related_name='smoked_total_years_response'
)

value = models.IntegerField(
validators=[
MinValueValidator(1, message="The number of years you smoked cigarettes must be at least 1")
]
)


def clean(self):
super().clean()
self._validate_age_when_started_smoking_response_exists()
self._validate_value_fewer_than_total_number_of_years_smoked()


def _validate_age_when_started_smoking_response_exists(self):
if not hasattr(self.tobacco_smoking_history.response_set, 'age_when_started_smoking_response'):
raise ValidationError({
"value": ValidationError(
"You must answer age when started smoking before answering how many years you have smoked cigarettes",
code="age_when_started_smoking_response_not_found"
)
})

def _validate_value_fewer_than_total_number_of_years_smoked(self):
if not self.value:
return None

if self.value > self.tobacco_smoking_history.response_set.age_when_started_smoking_response.years_smoked_including_stopped():
raise ValidationError({
"value": ValidationError(
"The number of years you smoked cigarettes must be fewer than the total number of years you have been smoking",
code="value_greater_than_total_number_of_years_smoked"
)
})
18 changes: 17 additions & 1 deletion lung_cancer_screening/questions/models/tobacco_smoking_history.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.db import models
from django.db.models import Case, Value, When

from .base import BaseModel
from .base import BaseModel, BaseQuerySet
from .response_set import ResponseSet


Expand All @@ -13,6 +14,16 @@ class TobaccoSmokingHistoryTypes(models.TextChoices):
SHISHA = "Shisha", "Shisha"


class TobaccoSmokingHistoryQuerySet(BaseQuerySet):
def in_form_order(self):
form_order = [choice[0] for choice in TobaccoSmokingHistoryTypes.choices]
order = Case(
*[When(type=type_val, then=Value(i)) for i, type_val in enumerate(form_order)],
default=Value(len(form_order)),
)
return self.order_by(order)


class TobaccoSmokingHistory(BaseModel):
response_set = models.ForeignKey(
ResponseSet,
Expand All @@ -23,6 +34,8 @@ class TobaccoSmokingHistory(BaseModel):
choices=TobaccoSmokingHistoryTypes.choices
)

objects = TobaccoSmokingHistoryQuerySet.as_manager()

class Meta:
constraints = [
models.UniqueConstraint(
Expand All @@ -31,3 +44,6 @@ class Meta:
violation_error_message="A tobacco smoking history already exists for this response set and type"
)
]

def human_type(self):
return TobaccoSmokingHistoryTypes(self.type).label
29 changes: 19 additions & 10 deletions lung_cancer_screening/questions/presenters/response_set_presenter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import humps

from decimal import Decimal
from django.urls import reverse

from ..models.education_response import EducationValues
from ..models.respiratory_conditions_response import RespiratoryConditionValues
from ..models.family_history_lung_cancer_response import FamilyHistoryLungCancerValues
from ..models.tobacco_smoking_history import TobaccoSmokingHistoryTypes

class ResponseSetPresenter:
DATE_FORMAT = "%-d %B %Y" # eg 8 September 2000
Expand Down Expand Up @@ -150,13 +151,9 @@ def respiratory_conditions(self):

@property
def types_tobacco_smoking(self):
types_smoked = self.response_set.tobacco_smoking_history.values_list('type', flat=True)

types_smoked_ordered = sorted(types_smoked, key=lambda x: TobaccoSmokingHistoryTypes.values.index(x))

return self._list_to_sentence([
TobaccoSmokingHistoryTypes(code).label
for code in types_smoked_ordered
tobacco_smoking_history.human_type()
for tobacco_smoking_history in self.response_set.tobacco_smoking_history.in_form_order()
])


Expand Down Expand Up @@ -249,6 +246,17 @@ def family_history_responses_items(self):
return items


def smoking_history_types_responses_items(self):
return [*[
self._check_your_answer_item(
f"Total number of years you have smoked {type_history.human_type().lower()}",
type_history.smoked_total_years_response.value if hasattr(type_history, 'smoked_total_years_response') else None,
"questions:smoked_total_years",
kwargs = { "tobacco_type": humps.kebabize(type_history.type) },
)
for type_history in self.response_set.tobacco_smoking_history.in_form_order()
]]

def smoking_history_responses_items(self):
return [
self._check_your_answer_item(
Expand All @@ -265,7 +273,8 @@ def smoking_history_responses_items(self):
"Types of tobacco smoked",
self.types_tobacco_smoking,
"questions:types_tobacco_smoking",
)
),
*self.smoking_history_types_responses_items()
]


Expand Down Expand Up @@ -296,14 +305,14 @@ def _change_query_params(self):
return { "change": "True" }


def _check_your_answer_item(self, question, value, url_lookup_name):
def _check_your_answer_item(self, question, value, url_lookup_name, kwargs = {}):
return {
"key": { "text": question },
"value": { "text": value },
"actions": {
"items": [
{
"href": reverse(url_lookup_name, query = self._change_query_params()),
"href": reverse(url_lookup_name, kwargs = kwargs, query = self._change_query_params()),
"text": "Change"
}
]
Expand Down
2 changes: 1 addition & 1 deletion lung_cancer_screening/questions/tests/factories/age_when_started_smoking_response_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
def calculate_value(instance):
if not hasattr(instance.response_set, 'date_of_birth_response'):
return None
return random.randint(1, instance.response_set.date_of_birth_response.age_in_years())
return random.randint(10, instance.response_set.date_of_birth_response.age_in_years())


class AgeWhenStartedSmokingResponseFactory(factory.django.DjangoModelFactory):
Expand Down
12 changes: 12 additions & 0 deletions lung_cancer_screening/questions/tests/factories/smoked_total_years_response_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import factory

from .tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory
from ...models.smoked_total_years_response import SmokedTotalYearsResponse


class SmokedTotalYearsResponseFactory(factory.django.DjangoModelFactory):
class Meta:
model = SmokedTotalYearsResponse

tobacco_smoking_history = factory.SubFactory(TobaccoSmokingHistoryFactory)
value = 1
Loading