From 09a7ab16722049725f8c6cd27c7ba142d5cc2d79 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Thu, 29 Jan 2026 10:06:23 +0000 Subject: [PATCH 1/6] PPHA-534: Add smoked total years for cigarette type --- .../tests/unit/presenters/test_response_set_presenter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ec33937d..17959332 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 @@ -462,7 +462,7 @@ def test_smoking_history_types_responses_items_sets_the_correct_value(self): response_set=self.response_set, type=TobaccoSmokingHistoryTypes.CIGARETTES ) smoked_total_years_response = SmokedTotalYearsResponseFactory.create( - tobacco_smoking_history=cigarettes + tobacco_smoking_history=cigarettes, ) presenter = ResponseSetPresenter(self.response_set) From 89a62268665e452812c41e828eededb39dfeeb56 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Thu, 29 Jan 2026 13:48:50 +0000 Subject: [PATCH 2/6] PPHA-534: Add normal amount smoked per duration --- features/age_when_started_smoking.feature | 1 + .../periods_when_you_stopped_smoking.feature | 1 + features/questionnaire.feature | 10 ++ features/smoked_amount.feature | 49 ++++++ features/smoked_total_years.feature | 8 +- features/smoking_current.feature | 6 +- features/smoking_frequency.feature | 16 +- features/steps/preflight_steps.py | 14 ++ features/types_tobacco_smoking.feature | 1 + lung_cancer_screening/jinja2_env.py | 7 + .../questions/forms/smoked_amount_form.py | 39 +++++ .../questions/jinja2/smoked_amount.jinja | 11 ++ .../migrations/0053_smokedamountresponse.py | 30 ++++ .../models/smoked_amount_response.py | 13 ++ .../presenters/response_set_presenter.py | 34 ++-- .../smoked_amount_response_factory.py | 12 ++ .../unit/forms/test_smoked_amount_form.py | 41 +++++ .../models/test_smoked_amount_response.py | 32 ++++ .../presenters/test_response_set_presenter.py | 7 +- .../tests/unit/views/test_smoked_amount.py | 158 ++++++++++++++++++ .../unit/views/test_smoked_total_years.py | 28 +++- .../tests/unit/views/test_smoking_current.py | 15 +- .../unit/views/test_smoking_frequency.py | 33 ++-- .../unit/views/test_types_tobacco_smoking.py | 5 +- lung_cancer_screening/questions/urls.py | 2 + .../mixins/ensure_smoking_history_for_type.py | 5 +- .../questions/views/question_base_view.py | 6 + .../questions/views/smoked_amount.py | 19 +++ .../questions/views/smoked_total_years.py | 21 ++- .../questions/views/smoking_current.py | 13 +- .../questions/views/smoking_frequency.py | 21 ++- .../smoking_history_question_base_view.py | 5 +- .../questions/views/types_tobacco_smoking.py | 7 +- poetry.lock | 26 +-- pyproject.toml | 2 +- 35 files changed, 605 insertions(+), 93 deletions(-) create mode 100644 features/smoked_amount.feature create mode 100644 lung_cancer_screening/questions/forms/smoked_amount_form.py create mode 100644 lung_cancer_screening/questions/jinja2/smoked_amount.jinja create mode 100644 lung_cancer_screening/questions/migrations/0053_smokedamountresponse.py create mode 100644 lung_cancer_screening/questions/models/smoked_amount_response.py create mode 100644 lung_cancer_screening/questions/tests/factories/smoked_amount_response_factory.py create mode 100644 lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py create mode 100644 lung_cancer_screening/questions/tests/unit/models/test_smoked_amount_response.py create mode 100644 lung_cancer_screening/questions/tests/unit/views/test_smoked_amount.py create mode 100644 lung_cancer_screening/questions/views/smoked_amount.py diff --git a/features/age_when_started_smoking.feature b/features/age_when_started_smoking.feature index 59dddddd..b3473365 100644 --- a/features/age_when_started_smoking.feature +++ b/features/age_when_started_smoking.feature @@ -1,3 +1,4 @@ +@SmokingHistory @AgeWhenStartedSmoking Feature: Age when started smoking Scenario: The page is accessible diff --git a/features/periods_when_you_stopped_smoking.feature b/features/periods_when_you_stopped_smoking.feature index 3d49652d..eeba1ce5 100644 --- a/features/periods_when_you_stopped_smoking.feature +++ b/features/periods_when_you_stopped_smoking.feature @@ -1,3 +1,4 @@ +@SmokingHistory @PeriodsWhenYouStoppedSmoking Feature: Periods when you stopped smoking page Scenario: The page is accessible diff --git a/features/questionnaire.feature b/features/questionnaire.feature index 20ed5edc..b42b4792 100644 --- a/features/questionnaire.feature +++ b/features/questionnaire.feature @@ -1,4 +1,9 @@ @Basic +@AgeWhenStartedSmoking +@SmokedTotalYears +@SmokedAmount +@SmokedChanged +@Questionnaire Feature: Questionnaire Scenario: Cannot change responses once submitted Given I am logged in @@ -79,6 +84,10 @@ Feature: Questionnaire Then I am on "/cigarettes-smoking-frequency" When I check "Daily" and submit + Then I am on "/cigarettes-smoked-amount" + When I fill in "Roughly how many cigarettes do you smoke in a normal day?" with "15" + 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" @@ -100,6 +109,7 @@ Feature: Questionnaire 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" + And I see "15 cigarettes a day" as a response to "Current cigarette smoking" under "Smoking history" When I click "Submit" Then I am on "/confirmation" diff --git a/features/smoked_amount.feature b/features/smoked_amount.feature new file mode 100644 index 00000000..d5530522 --- /dev/null +++ b/features/smoked_amount.feature @@ -0,0 +1,49 @@ +@SmokingHistory +@SmokedAmount +Feature: Smoked amount 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 "Cigarettes" + When I go to "/cigarettes-smoked-amount" + Then there are no accessibility violations + + Scenario: Form errors + Given I am logged in + And I have answered questions showing I am eligible + And I have answered questions showing I have smoked "Cigarettes" + When I go to "/cigarettes-smoked-amount" + And I click "Continue" + Then I am on "/cigarettes-smoked-amount" + And I see a form error "Enter how many cigarettes you currently smoke in a normal day" + 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 "Cigarettes" + When I go to "/cigarettes-smoked-amount" + Then I see a back link to "/cigarettes-smoked-total-years" + When I fill in "Roughly how many cigarettes do you smoke in a normal day?" with "20" + 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 "Cigarettes" + And I have answered questions showing I have smoked "Cigarettes" daily + When I go to "/cigarettes-smoked-amount" + When I fill in "Roughly how many cigarettes do you smoke in a normal day?" with "20" + And I submit the form + When I go to "/check-your-answers" + Then I see "20 cigarettes a day" as a response to "Current cigarette smoking" under "Smoking history" + And I see "/cigarettes-smoking-frequency?change=True" as a link to change "Current cigarette smoking" under "Smoking history" + When I click the link to change "Current cigarette smoking" under "Smoking history" + Then I am on "/cigarettes-smoking-frequency?change=True" + When I submit the form + Then I see "20" filled in for "Roughly how many cigarettes do you smoke in a normal day?" + When I fill in "Roughly how many cigarettes do you smoke in a normal day?" with "15" + And I click "Continue" + Then I am on "/check-your-answers" + And I see "15 cigarettes a day" as a response to "Current cigarette smoking" under "Smoking history" diff --git a/features/smoked_total_years.feature b/features/smoked_total_years.feature index a5f2acb6..ded8d98e 100644 --- a/features/smoked_total_years.feature +++ b/features/smoked_total_years.feature @@ -1,3 +1,4 @@ +@SmokingHistory @SmokedTotalYears Feature: Smoked total years page Scenario: The page is accessible @@ -25,7 +26,7 @@ Feature: Smoked total years page 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" + Then I see a back link to "/cigarettes-smoking-current" When I fill in "Roughly how many years have you smoked cigarettes?" with "9" And I submit the form Then I am on "/cigarettes-smoking-frequency" @@ -46,5 +47,6 @@ Feature: Smoked total years page 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" + Then I am on "/cigarettes-smoking-frequency?change=True" + When I go to "/check-your-answers" + Then I see "8" as a response to "Total number of years you have smoked cigarettes" under "Smoking history" diff --git a/features/smoking_current.feature b/features/smoking_current.feature index ae4c6c92..e78ef17f 100644 --- a/features/smoking_current.feature +++ b/features/smoking_current.feature @@ -1,3 +1,4 @@ +@SmokingHistory @SmokingCurrent Feature: Smoking current page Scenario: The page is accessible @@ -39,5 +40,6 @@ Feature: Smoking current page Then I am on "/cigarettes-smoking-current?change=True" And I see "Yes" selected When I check "No" and submit - Then I am on "/check-your-answers" - And I see "No" as a response to "Do you currently smoke cigarettes?" under "Smoking history" + Then I am on "/cigarettes-smoked-total-years?change=True" + When I go to "/check-your-answers" + Then I see "No" as a response to "Do you currently smoke cigarettes?" under "Smoking history" diff --git a/features/smoking_frequency.feature b/features/smoking_frequency.feature index c86c2966..f8e0cad2 100644 --- a/features/smoking_frequency.feature +++ b/features/smoking_frequency.feature @@ -1,3 +1,4 @@ +@SmokingHistory @SmokingFrequency Feature: Smoking frequency page Scenario: The page is accessible @@ -25,21 +26,24 @@ Feature: Smoking frequency page When I go to "/cigarettes-smoking-frequency" Then I see a back link to "/cigarettes-smoked-total-years" When I check "Daily" and submit - Then I am on "/check-your-answers" + Then I am on "/cigarettes-smoked-amount" + @wip 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" + And I have answered questions showing I have smoked 10 "Cigarettes" as the amount When I go to "/cigarettes-smoking-frequency" When I check "Daily" and submit When I go to "/check-your-answers" - Then I see "Cigarettes a day" as a response to "Current cigarettes smoking" under "Smoking history" - And I see "/cigarettes-smoking-frequency?change=True" as a link to change "Current cigarettes smoking" under "Smoking history" - When I click the link to change "Current cigarettes smoking" under "Smoking history" + Then I see "10 cigarettes a day" as a response to "Current cigarette smoking" under "Smoking history" + And I see "/cigarettes-smoking-frequency?change=True" as a link to change "Current cigarette smoking" under "Smoking history" + When I click the link to change "Current cigarette smoking" under "Smoking history" Then I am on "/cigarettes-smoking-frequency?change=True" And I see "Daily" selected When I check "Weekly" and submit - Then I am on "/check-your-answers" - And I see "Cigarettes a week" as a response to "Current cigarettes smoking" under "Smoking history" + Then I am on "/cigarettes-smoked-amount?change=True" + When I go to "/check-your-answers" + Then I see "10 cigarettes a week" as a response to "Current cigarette smoking" under "Smoking history" diff --git a/features/steps/preflight_steps.py b/features/steps/preflight_steps.py index f09fe4a2..cd990f82 100644 --- a/features/steps/preflight_steps.py +++ b/features/steps/preflight_steps.py @@ -1,4 +1,5 @@ from behave import given +from inflection import humanize from features.steps.form_steps import ( when_i_fill_in_and_submit_my_date_of_birth_as_x_years_ago, @@ -89,3 +90,16 @@ def given_i_have_answered_questions_showing_i_have_smoked_tobacco_type( when_i_check_label(context, tobacco_type.strip()) when_i_submit_the_form(context) + + +@given('I have answered questions showing I have smoked "{tobacco_type}" {frequency}') +def given_i_have_answered_questions_showing_i_have_smoked_tobacco_type_frequency(context, tobacco_type, frequency): + context.page.goto(f"{context.live_server_url}/{tobacco_type.lower()}-smoking-frequency") + when_i_check_label(context, humanize(frequency)) + when_i_submit_the_form(context) + +@given('I have answered questions showing I have smoked {amount} "{tobacco_type}" as the amount') +def given_i_have_answered_questions_showing_i_have_smoked_amount_tobacco_type(context, amount, tobacco_type): + context.page.goto(f"{context.live_server_url}/{tobacco_type.lower()}-smoked-amount") + when_i_fill_in_label_with_value(context, f"Roughly how many {tobacco_type.lower()} do you smoke in a normal day?", amount) + when_i_submit_the_form(context) diff --git a/features/types_tobacco_smoking.feature b/features/types_tobacco_smoking.feature index a4ef7540..56a612e2 100644 --- a/features/types_tobacco_smoking.feature +++ b/features/types_tobacco_smoking.feature @@ -1,3 +1,4 @@ +@SmokingHistory @TypesTobaccoSmoking Feature: Types tobacco smoking page Scenario: The page is accessible diff --git a/lung_cancer_screening/jinja2_env.py b/lung_cancer_screening/jinja2_env.py index 20714c7e..e81b6414 100644 --- a/lung_cancer_screening/jinja2_env.py +++ b/lung_cancer_screening/jinja2_env.py @@ -1,3 +1,5 @@ +import inflection + from django.conf import settings from django.templatetags.static import static from django.urls import reverse @@ -7,6 +9,7 @@ def environment(**options): env = Environment(**options, extensions=["jinja2.ext.do"]) + if env.loader: env.loader = ChoiceLoader( [ @@ -27,4 +30,8 @@ def environment(**options): {"static": static, "url": reverse, "STATIC_URL": settings.STATIC_URL} ) + env.filters.update( + {"singularize": inflection.singularize} + ) + return env diff --git a/lung_cancer_screening/questions/forms/smoked_amount_form.py b/lung_cancer_screening/questions/forms/smoked_amount_form.py new file mode 100644 index 00000000..2f876630 --- /dev/null +++ b/lung_cancer_screening/questions/forms/smoked_amount_form.py @@ -0,0 +1,39 @@ +from django import forms + +from ...nhsuk_forms.integer_field import IntegerField +from ..models.smoked_amount_response import SmokedAmountResponse + + +class SmokedAmountForm(forms.ModelForm): + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self.fields["value"] = IntegerField( + label=self._type_label(), + label_classes="nhsuk-label--m", + classes="nhsuk-input--width-4", + hint="Give an estimate if you are not sure", + suffix=self._type_suffix(), + required=True, + error_messages={ + "required": self._type_required_error_message() + }, + ) + + + def type_string(self): + return self.instance.tobacco_smoking_history.human_type().lower() + + def _type_label(self): + return f"Roughly how many {self.type_string()} do you smoke in a normal day?" + + def _type_required_error_message(self): + return f"Enter how many {self.type_string()} you currently smoke in a normal day" + + def _type_suffix(self): + return self.type_string() + + class Meta: + model = SmokedAmountResponse + fields = ['value'] diff --git a/lung_cancer_screening/questions/jinja2/smoked_amount.jinja b/lung_cancer_screening/questions/jinja2/smoked_amount.jinja new file mode 100644 index 00000000..840714e7 --- /dev/null +++ b/lung_cancer_screening/questions/jinja2/smoked_amount.jinja @@ -0,0 +1,11 @@ +{% extends 'question_form.jinja' %} + +{% block prelude %} + +

{{ form.type_string() | title | singularize }} smoking

+ +

This is a test of the online service. In the future, we will use this information to give you an accurate result.

+ +

The question on the next page will ask if the number of cigarettes you smoke has changed over time.

+ +{% endblock prelude %} diff --git a/lung_cancer_screening/questions/migrations/0053_smokedamountresponse.py b/lung_cancer_screening/questions/migrations/0053_smokedamountresponse.py new file mode 100644 index 00000000..7b4deb83 --- /dev/null +++ b/lung_cancer_screening/questions/migrations/0053_smokedamountresponse.py @@ -0,0 +1,30 @@ +# Generated by Django 5.2.10 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "questions", + "0052_smokingfrequencyresponse", + ), + ] + + operations = [ + migrations.CreateModel( + name='SmokedAmountResponse', + 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()), + ('tobacco_smoking_history', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='smoked_amount_response', to='questions.tobaccosmokinghistory')), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/lung_cancer_screening/questions/models/smoked_amount_response.py b/lung_cancer_screening/questions/models/smoked_amount_response.py new file mode 100644 index 00000000..fa708cd2 --- /dev/null +++ b/lung_cancer_screening/questions/models/smoked_amount_response.py @@ -0,0 +1,13 @@ +from django.db import models + +from .base import BaseModel +from .tobacco_smoking_history import TobaccoSmokingHistory + + +class SmokedAmountResponse(BaseModel): + tobacco_smoking_history = models.OneToOneField( + TobaccoSmokingHistory, + on_delete=models.CASCADE, + related_name='smoked_amount_response' + ) + value = models.IntegerField() diff --git a/lung_cancer_screening/questions/presenters/response_set_presenter.py b/lung_cancer_screening/questions/presenters/response_set_presenter.py index 7877fe0e..80bf8e3b 100644 --- a/lung_cancer_screening/questions/presenters/response_set_presenter.py +++ b/lung_cancer_screening/questions/presenters/response_set_presenter.py @@ -1,4 +1,4 @@ -import humps +from inflection import dasherize, singularize from decimal import Decimal from django.urls import reverse @@ -261,30 +261,36 @@ def smoking_history_types_responses_items(self): return results def smoking_history_summary_items_for_type(self, type_history): - return [self._check_your_answer_item( - f"Do you currently smoke {type_history.human_type().lower()}?", + type_label = type_history.human_type().lower() + tobacco_type_kwargs = {"tobacco_type": dasherize(type_history.type).lower()} + + return [ + self._check_your_answer_item( + f"Do you currently smoke {type_label}?", self._boolean_response_to_yes_no(type_history, "smoking_current_response"), "questions:smoking_current", - kwargs = { "tobacco_type": humps.kebabize(type_history.type) } + kwargs=tobacco_type_kwargs ), - (self._check_your_answer_item( - f"Total number of years you have smoked {type_history.human_type().lower()}", + self._check_your_answer_item( + f"Total number of years you have smoked {type_label}", type_history.smoked_total_years_response.value if hasattr(type_history, 'smoked_total_years_response') else self.NOT_ANSWERED_TEXT, "questions:smoked_total_years", - kwargs = { "tobacco_type": humps.kebabize(type_history.type) } - )), - (self._check_your_answer_item( - f"Current {type_history.human_type().lower()} smoking", + kwargs = tobacco_type_kwargs + ), + self._check_your_answer_item( + f"Current {singularize(type_label)} smoking", self._smoking_type_to_text(type_history), "questions:smoking_frequency", - kwargs = { "tobacco_type": humps.kebabize(type_history.type) } - )) + kwargs = tobacco_type_kwargs + ) ] + def _smoking_type_to_text(self, type_history): - if not hasattr(type_history, 'smoking_frequency_response'): + if not hasattr(type_history, 'smoking_frequency_response') or not hasattr(type_history, 'smoked_amount_response'): return self.NOT_ANSWERED_TEXT - return f"{type_history.human_type()} a {self._frequency_response_to_text(type_history.smoking_frequency_response)}" + + return f"{type_history.smoked_amount_response.value} {type_history.human_type().lower()} a {self._frequency_response_to_text(type_history.smoking_frequency_response)}" def _frequency_response_to_text(self, frequency_response): if frequency_response.value == SmokingFrequencyValues.DAILY: diff --git a/lung_cancer_screening/questions/tests/factories/smoked_amount_response_factory.py b/lung_cancer_screening/questions/tests/factories/smoked_amount_response_factory.py new file mode 100644 index 00000000..cfd2fe69 --- /dev/null +++ b/lung_cancer_screening/questions/tests/factories/smoked_amount_response_factory.py @@ -0,0 +1,12 @@ +import factory + +from .tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory +from ...models.smoked_amount_response import SmokedAmountResponse + + +class SmokedAmountResponseFactory(factory.django.DjangoModelFactory): + class Meta: + model = SmokedAmountResponse + + tobacco_smoking_history = factory.SubFactory(TobaccoSmokingHistoryFactory) + value = 20 diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py new file mode 100644 index 00000000..9868c17f --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py @@ -0,0 +1,41 @@ +from django.test import TestCase, tag + +from ...factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory +from ...factories.smoked_amount_response_factory import SmokedAmountResponseFactory +from ....models.tobacco_smoking_history import TobaccoSmokingHistoryTypes +from ....forms.smoked_amount_form import SmokedAmountForm + + +@tag("SmokedAmount") +class TestSmokedAmountForm(TestCase): + def setUp(self): + self.smoking_history = TobaccoSmokingHistoryFactory.create( + type=TobaccoSmokingHistoryTypes.CIGARETTES.value + ) + self.response = SmokedAmountResponseFactory.build( + tobacco_smoking_history=self.smoking_history + ) + + def test_is_valid_with_a_valid_value(self): + form = SmokedAmountForm( + instance=self.response, + data={"value": 20} + ) + self.assertTrue(form.is_valid()) + self.assertEqual(form.cleaned_data["value"], 20) + + def test_is_invalid_with_a_none_value(self): + form = SmokedAmountForm( + instance=self.response, + data={"value": None} + ) + self.assertFalse(form.is_valid()) + self.assertIn("value", form.errors) + + def test_is_invalid_with_a_non_numeric_value(self): + form = SmokedAmountForm( + instance=self.response, + data={"value": "not a number"} + ) + self.assertFalse(form.is_valid()) + self.assertIn("value", form.errors) diff --git a/lung_cancer_screening/questions/tests/unit/models/test_smoked_amount_response.py b/lung_cancer_screening/questions/tests/unit/models/test_smoked_amount_response.py new file mode 100644 index 00000000..21d108e4 --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/models/test_smoked_amount_response.py @@ -0,0 +1,32 @@ +from django.test import TestCase, tag + +from ...factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory +from ...factories.smoked_amount_response_factory import SmokedAmountResponseFactory +from ....models.tobacco_smoking_history import TobaccoSmokingHistoryTypes + + +@tag("SmokedAmount") +class TestSmokedAmountResponse(TestCase): + def setUp(self): + self.tobacco_smoking_history = TobaccoSmokingHistoryFactory.create( + type=TobaccoSmokingHistoryTypes.CIGARETTES.value + ) + + def test_has_a_valid_factory(self): + model = SmokedAmountResponseFactory.build( + tobacco_smoking_history=self.tobacco_smoking_history + ) + model.full_clean() + + def test_has_tobacco_smoking_history_as_foreign_key(self): + response = SmokedAmountResponseFactory.build( + tobacco_smoking_history=self.tobacco_smoking_history + ) + self.assertEqual(response.tobacco_smoking_history, self.tobacco_smoking_history) + + def test_has_value_as_integer(self): + response = SmokedAmountResponseFactory.build( + tobacco_smoking_history=self.tobacco_smoking_history, + value=15 + ) + self.assertEqual(response.value, 15) 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 17959332..b9fd1fcb 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 @@ -1,7 +1,6 @@ -import humps - from django.test import TestCase, tag from django.urls import reverse +from inflection import dasherize from ...factories.response_set_factory import ResponseSetFactory from ...factories.have_you_ever_smoked_response_factory import HaveYouEverSmokedResponseFactory @@ -491,9 +490,9 @@ def test_smoking_history_types_responses_items_sets_the_correct_change_link(self reverse( "questions:smoked_total_years", kwargs={ - "tobacco_type": humps.kebabize( + "tobacco_type": dasherize( TobaccoSmokingHistoryTypes.CIGARETTES - ) + ).lower() }, query={"change": "True"}, ), diff --git a/lung_cancer_screening/questions/tests/unit/views/test_smoked_amount.py b/lung_cancer_screening/questions/tests/unit/views/test_smoked_amount.py new file mode 100644 index 00000000..9d05efd7 --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/views/test_smoked_amount.py @@ -0,0 +1,158 @@ +from django.test import TestCase, tag +from django.urls import reverse + +from .helpers.authentication import login_user +from lung_cancer_screening.questions.models.tobacco_smoking_history import TobaccoSmokingHistoryTypes +from ...factories.response_set_factory import ResponseSetFactory +from ...factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory + + +@tag("SmokedAmount") +class TestGetSmokedAmount(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:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }) + ) + + self.assertRedirects( + response, + "/oidc/authenticate/?next=/cigarettes-smoked-amount", + fetch_redirect_response=False + ) + + def test_redirects_when_the_user_is_not_eligible(self): + ResponseSetFactory.create(user=self.user, eligible=False) + + response = self.client.get( + reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }) + ) + + self.assertRedirects(response, reverse("questions:have_you_ever_smoked")) + + def test_404_when_a_smoking_history_item_does_not_exist_for_the_given_type(self): + ResponseSetFactory.create(user=self.user, eligible=True) + + response = self.client.get(reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + })) + + self.assertEqual(response.status_code, 404) + + def test_responds_successfully(self): + response_set = ResponseSetFactory.create(user=self.user, eligible=True) + TobaccoSmokingHistoryFactory.create( + response_set=response_set, + type=TobaccoSmokingHistoryTypes.CIGARETTES.value + ) + + response = self.client.get(reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + })) + + self.assertEqual(response.status_code, 200) + + +@tag("SmokedAmount") +class TestPostSmokedAmount(TestCase): + def setUp(self): + self.user = login_user(self.client) + self.response_set = ResponseSetFactory.create(user=self.user, eligible=True) + self.valid_params = {"value": 20} + + def test_redirects_if_the_user_is_not_logged_in(self): + self.client.logout() + + response = self.client.post( + reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }), + self.valid_params + ) + + self.assertRedirects( + response, + "/oidc/authenticate/?next=/cigarettes-smoked-amount", + fetch_redirect_response=False + ) + + def test_redirects_when_the_user_is_not_eligible(self): + self.response_set.delete() + ResponseSetFactory.create(user=self.user) + + response = self.client.post( + reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }), + self.valid_params + ) + + self.assertRedirects(response, reverse("questions:have_you_ever_smoked")) + + def test_404_when_a_smoking_history_item_does_not_exist_for_the_given_type(self): + response = self.client.post(reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }), self.valid_params) + + self.assertEqual(response.status_code, 404) + + def test_creates_a_smoked_amount_response(self): + smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=TobaccoSmokingHistoryTypes.CIGARETTES.value + ) + + self.client.post(reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }), self.valid_params) + + smoking_history.refresh_from_db() + self.assertEqual( + smoking_history.smoked_amount_response.value, self.valid_params["value"] + ) + + def test_redirects_to_responses(self): + TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=TobaccoSmokingHistoryTypes.CIGARETTES.value + ) + + response = self.client.post( + reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }), + self.valid_params + ) + + self.assertRedirects(response, reverse("questions:responses")) + + + def test_redirects_to_next_question_forwarding_the_change_query_param(self): + TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=TobaccoSmokingHistoryTypes.CIGARETTES.value + ) + + response = self.client.post( + reverse( + "questions:smoked_amount", + kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }, + ), + {**self.valid_params, "change": "True"}, + ) + + self.assertRedirects( + response, + reverse("questions:responses"), + fetch_redirect_response=False, + ) diff --git a/lung_cancer_screening/questions/tests/unit/views/test_smoked_total_years.py b/lung_cancer_screening/questions/tests/unit/views/test_smoked_total_years.py index d86c36a9..2ff0eb71 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_smoked_total_years.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_smoked_total_years.py @@ -192,18 +192,28 @@ def test_redirects_to_next_question(self): })) - def test_redirects_to_responses_if_change_query_param_is_true(self): + def test_redirects_to_next_question_forwarding_the_change_query_param(self): response = self.client.post( - reverse("questions:smoked_total_years", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() - }), - { - **self.valid_params, - "change": "True" - } + reverse( + "questions:smoked_total_years", + kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }, + ), + {**self.valid_params, "change": "True"}, ) - self.assertRedirects(response, reverse("questions:responses")) + self.assertRedirects( + response, + reverse( + "questions:smoking_frequency", + kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }, + query={"change": "True"}, + ), + fetch_redirect_response=False, + ) def test_responds_with_422_if_the_response_fails_to_create(self): diff --git a/lung_cancer_screening/questions/tests/unit/views/test_smoking_current.py b/lung_cancer_screening/questions/tests/unit/views/test_smoking_current.py index 5ae6002e..04c87533 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_smoking_current.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_smoking_current.py @@ -1,7 +1,6 @@ -import humps - from django.test import TestCase, tag from django.urls import reverse +from inflection import dasherize from lung_cancer_screening.questions.models.tobacco_smoking_history import TobaccoSmokingHistoryTypes from lung_cancer_screening.questions.tests.factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory @@ -143,10 +142,11 @@ def test_redirects_to_next_question(self): ) self.assertRedirects(response, reverse("questions:smoked_total_years", kwargs={ - "tobacco_type": humps.kebabize(TobaccoSmokingHistoryTypes.CIGARETTES.value) + "tobacco_type": dasherize(TobaccoSmokingHistoryTypes.CIGARETTES.value).lower() }), fetch_redirect_response=False) - def test_redirects_to_responses_if_change_query_param_is_true(self): + + def test_redirects_to_next_question_forwarding_the_change_query_param(self): response = self.client.post( reverse("questions:smoking_current", kwargs = { "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() @@ -157,7 +157,12 @@ def test_redirects_to_responses_if_change_query_param_is_true(self): } ) - self.assertRedirects(response, reverse("questions:responses")) + self.assertRedirects(response, reverse("questions:smoked_total_years", + kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }, + query={"change": "True"} + ), fetch_redirect_response=False) def test_responds_with_422_if_the_response_fails_to_create(self): diff --git a/lung_cancer_screening/questions/tests/unit/views/test_smoking_frequency.py b/lung_cancer_screening/questions/tests/unit/views/test_smoking_frequency.py index 3c8a15e8..21e9b618 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_smoking_frequency.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_smoking_frequency.py @@ -142,17 +142,30 @@ def test_redirects_to_next_question(self): self.valid_params ) - self.assertRedirects(response, reverse("questions:responses")) + self.assertRedirects(response, reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + })) - def test_redirects_to_responses_if_change_query_param_is_true(self): + + def test_redirects_to_next_question_forwarding_the_change_query_param(self): response = self.client.post( - reverse("questions:smoking_frequency", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() - }), - { - **self.valid_params, - "change": "True" - } + reverse( + "questions:smoking_frequency", + kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }, + ), + {**self.valid_params, "change": "True"}, ) - self.assertRedirects(response, reverse("questions:responses")) + self.assertRedirects( + response, + reverse( + "questions:smoked_amount", + kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }, + query={"change": "True"}, + ), + fetch_redirect_response=False, + ) diff --git a/lung_cancer_screening/questions/tests/unit/views/test_types_tobacco_smoking.py b/lung_cancer_screening/questions/tests/unit/views/test_types_tobacco_smoking.py index 91f532cf..0f7b0d92 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_types_tobacco_smoking.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_types_tobacco_smoking.py @@ -1,7 +1,6 @@ -import humps - from django.test import TestCase, tag from django.urls import reverse +from inflection import dasherize from .helpers.authentication import login_user from ...factories.response_set_factory import ResponseSetFactory @@ -127,5 +126,5 @@ def test_post_redirects_to_the_first_type_of_tobacco_smoking_history_question(se ) self.assertRedirects(response, reverse("questions:smoking_current", kwargs={ - "tobacco_type": humps.kebabize(TobaccoSmokingHistoryTypes.CIGARETTES.value) + "tobacco_type": dasherize(TobaccoSmokingHistoryTypes.CIGARETTES.value).lower() }), fetch_redirect_response=False) diff --git a/lung_cancer_screening/questions/urls.py b/lung_cancer_screening/questions/urls.py index 95bf1f68..2b2e7489 100644 --- a/lung_cancer_screening/questions/urls.py +++ b/lung_cancer_screening/questions/urls.py @@ -40,6 +40,7 @@ from .views.smoking_current import SmokingCurrentView from .views.smoked_total_years import SmokedTotalYearsView from .views.smoking_frequency import SmokingFrequencyView +from .views.smoked_amount import SmokedAmountView from .views.start import StartView from .views.weight import WeightView from .views.confirmation import ConfirmationView @@ -68,6 +69,7 @@ path('types-tobacco-smoking', TypesTobaccoSmokingView.as_view(), name='types_tobacco_smoking'), path('-smoked-total-years', SmokedTotalYearsView.as_view(), name='smoked_total_years'), path('-smoking-frequency', SmokingFrequencyView.as_view(), name='smoking_frequency'), + path('-smoked-amount', SmokedAmountView.as_view(), name='smoked_amount'), path('check-your-answers', ResponsesView.as_view(), name='responses'), path('sex-at-birth', SexAtBirthView.as_view(), name='sex_at_birth'), path('start', StartView.as_view(), name='start'), diff --git a/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_for_type.py b/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_for_type.py index f3777851..564ce2f0 100644 --- a/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_for_type.py +++ b/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_for_type.py @@ -1,11 +1,10 @@ -import humps from django.http import Http404 +from inflection import camelize class EnsureSmokingHistoryForTypeMixin: def dispatch(self, request, *args, **kwargs): - url_tobacco_type = humps.pascalize(kwargs["tobacco_type"]) - + url_tobacco_type = camelize(kwargs["tobacco_type"]) if request.response_set.tobacco_smoking_history.filter(type=url_tobacco_type).exists(): return super().dispatch(request, *args, **kwargs) diff --git a/lung_cancer_screening/questions/views/question_base_view.py b/lung_cancer_screening/questions/views/question_base_view.py index f6984d71..7d802bed 100644 --- a/lung_cancer_screening/questions/views/question_base_view.py +++ b/lung_cancer_screening/questions/views/question_base_view.py @@ -8,6 +8,12 @@ class QuestionBaseView(UpdateView): def should_redirect_to_responses(self, request): return bool(request.POST.get("change")) + def get_change_query_params(self): + if not self.should_redirect_to_responses(self.request): + return {} + + return {"change": "True"} + def get_back_link_url(self): return self.back_link_url diff --git a/lung_cancer_screening/questions/views/smoked_amount.py b/lung_cancer_screening/questions/views/smoked_amount.py new file mode 100644 index 00000000..fd9b9763 --- /dev/null +++ b/lung_cancer_screening/questions/views/smoked_amount.py @@ -0,0 +1,19 @@ +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 .mixins.ensure_smoking_history_for_type import EnsureSmokingHistoryForTypeMixin +from .smoking_history_question_base_view import SmokingHistoryQuestionBaseView +from ..forms.smoked_amount_form import SmokedAmountForm +from ..models.smoked_amount_response import SmokedAmountResponse + + +class SmokedAmountView(LoginRequiredMixin, EnsureResponseSet, EnsureEligibleMixin, EnsureSmokingHistoryForTypeMixin, SmokingHistoryQuestionBaseView): + template_name = "smoked_amount.jinja" + form_class = SmokedAmountForm + model = SmokedAmountResponse + success_url = reverse_lazy("questions:responses") + + def get_back_link_url(self): + return reverse("questions:smoked_total_years", kwargs={"tobacco_type": self.kwargs["tobacco_type"]}) diff --git a/lung_cancer_screening/questions/views/smoked_total_years.py b/lung_cancer_screening/questions/views/smoked_total_years.py index 5245b00d..cd886ed7 100644 --- a/lung_cancer_screening/questions/views/smoked_total_years.py +++ b/lung_cancer_screening/questions/views/smoked_total_years.py @@ -1,5 +1,5 @@ -from django.urls import reverse_lazy, reverse from django.shortcuts import redirect +from django.urls import reverse from django.contrib.auth.mixins import LoginRequiredMixin from .mixins.ensure_response_set import EnsureResponseSet @@ -26,7 +26,18 @@ class SmokedTotalYearsView( template_name = "question_form.jinja" form_class = SmokedTotalYearsForm model = SmokedTotalYearsResponse - success_url = reverse_lazy("questions:smoking_frequency", kwargs={ - "tobacco_type": "cigarettes" - }) - back_link_url = reverse_lazy("questions:types_tobacco_smoking") + + def get_success_url(self): + return reverse( + "questions:smoking_frequency", + kwargs={"tobacco_type": self.kwargs["tobacco_type"]}, + query=self.get_change_query_params(), + ) + + def get_back_link_url(self): + + return reverse( + "questions:smoking_current", + kwargs={"tobacco_type": self.kwargs["tobacco_type"]}, + query=self.get_change_query_params(), + ) diff --git a/lung_cancer_screening/questions/views/smoking_current.py b/lung_cancer_screening/questions/views/smoking_current.py index 0ad7c031..23f74898 100644 --- a/lung_cancer_screening/questions/views/smoking_current.py +++ b/lung_cancer_screening/questions/views/smoking_current.py @@ -1,4 +1,4 @@ -from django.urls import reverse_lazy +from django.urls import reverse, reverse_lazy from django.contrib.auth.mixins import LoginRequiredMixin from lung_cancer_screening.questions.views.smoking_history_question_base_view import SmokingHistoryQuestionBaseView @@ -13,7 +13,12 @@ class SmokingCurrentView(LoginRequiredMixin, EnsureResponseSet, EnsureEligibleMi template_name = "question_form.jinja" form_class = SmokingCurrentForm model = SmokingCurrentResponse - success_url = reverse_lazy("questions:smoked_total_years", kwargs={ - "tobacco_type": "cigarettes" - }) back_link_url = reverse_lazy("questions:age_when_started_smoking") + + def get_success_url(self): + return reverse( + "questions:smoked_total_years", + kwargs={"tobacco_type": self.kwargs["tobacco_type"]}, + query=self.get_change_query_params(), + ) + diff --git a/lung_cancer_screening/questions/views/smoking_frequency.py b/lung_cancer_screening/questions/views/smoking_frequency.py index 16615122..39142692 100644 --- a/lung_cancer_screening/questions/views/smoking_frequency.py +++ b/lung_cancer_screening/questions/views/smoking_frequency.py @@ -1,4 +1,4 @@ -from django.urls import reverse_lazy +from django.urls import reverse from django.contrib.auth.mixins import LoginRequiredMixin from .mixins.ensure_response_set import EnsureResponseSet @@ -19,7 +19,18 @@ class SmokingFrequencyView( template_name = "question_form.jinja" form_class = SmokingFrequencyForm model = SmokingFrequencyResponse - success_url = reverse_lazy("questions:responses") - back_link_url = reverse_lazy("questions:smoked_total_years", kwargs={ - "tobacco_type": "cigarettes" - }) + + def get_success_url(self): + return reverse( + "questions:smoked_amount", + kwargs={"tobacco_type": self.kwargs["tobacco_type"]}, + query=self.get_change_query_params(), + ) + + + def get_back_link_url(self): + return reverse( + "questions:smoked_total_years", + kwargs={"tobacco_type": self.kwargs["tobacco_type"]}, + query=self.get_change_query_params(), + ) diff --git a/lung_cancer_screening/questions/views/smoking_history_question_base_view.py b/lung_cancer_screening/questions/views/smoking_history_question_base_view.py index 25d33c8c..9eb5b52a 100644 --- a/lung_cancer_screening/questions/views/smoking_history_question_base_view.py +++ b/lung_cancer_screening/questions/views/smoking_history_question_base_view.py @@ -1,4 +1,4 @@ -import humps +from inflection import camelize from .question_base_view import QuestionBaseView class SmokingHistoryQuestionBaseView(QuestionBaseView): @@ -7,7 +7,8 @@ def get_object(self): tobacco_smoking_history=self._get_history_item_for_type() )[0] + def _get_history_item_for_type(self): return self.request.response_set.tobacco_smoking_history.filter( - type=humps.pascalize(self.kwargs["tobacco_type"]) + type=camelize(self.kwargs["tobacco_type"]) ).first() diff --git a/lung_cancer_screening/questions/views/types_tobacco_smoking.py b/lung_cancer_screening/questions/views/types_tobacco_smoking.py index bcbecb22..48cf8cc3 100644 --- a/lung_cancer_screening/questions/views/types_tobacco_smoking.py +++ b/lung_cancer_screening/questions/views/types_tobacco_smoking.py @@ -1,8 +1,7 @@ -import humps - from django.urls import reverse from django.contrib.auth.mixins import LoginRequiredMixin from django.views.generic.edit import FormView +from inflection import dasherize from .mixins.ensure_response_set import EnsureResponseSet from .mixins.ensure_eligible import EnsureEligibleMixin @@ -35,8 +34,8 @@ def get_success_url(self): return reverse( "questions:smoking_current", kwargs={ - "tobacco_type": humps.kebabize( + "tobacco_type": dasherize( self.request.response_set.tobacco_smoking_history.first().type - ) + ).lower() }, ) diff --git a/poetry.lock b/poetry.lock index a81a22df..ebd5c532 100644 --- a/poetry.lock +++ b/poetry.lock @@ -760,6 +760,18 @@ files = [ [package.extras] all = ["flake8 (>=7.1.1)", "mypy (>=1.11.2)", "pytest (>=8.3.2)", "ruff (>=0.6.2)"] +[[package]] +name = "inflection" +version = "0.5.1" +description = "A port of Ruby on Rails inflector to Python" +optional = false +python-versions = ">=3.5" +groups = ["main"] +files = [ + {file = "inflection-0.5.1-py2.py3-none-any.whl", hash = "sha256:f38b2b640938a4f35ade69ac3d053042959b62a0f1076a5bbaa1b9526605a8a2"}, + {file = "inflection-0.5.1.tar.gz", hash = "sha256:1a29730d366e996aaacffb2f1f1cb9593dc38e2ddd30c91250c6dde09ea9b417"}, +] + [[package]] name = "jinja2" version = "3.1.6" @@ -1141,18 +1153,6 @@ typing-extensions = "*" [package.extras] dev = ["black", "build", "flake8", "flake8-black", "isort", "jupyter-console", "mkdocs", "mkdocs-include-markdown-plugin", "mkdocstrings[python]", "mypy", "pytest", "pytest-asyncio ; python_version >= \"3.4\"", "pytest-trio ; python_version >= \"3.7\"", "sphinx", "toml", "tox", "trio", "trio ; python_version > \"3.6\"", "trio-typing ; python_version > \"3.6\"", "twine", "twisted", "validate-pyproject[all]"] -[[package]] -name = "pyhumps" -version = "3.8.0" -description = "🐫 Convert strings (and dictionary keys) between snake case, camel case and pascal case in Python. Inspired by Humps for Node" -optional = false -python-versions = "*" -groups = ["main"] -files = [ - {file = "pyhumps-3.8.0-py3-none-any.whl", hash = "sha256:060e1954d9069f428232a1adda165db0b9d8dfdce1d265d36df7fbff540acfd6"}, - {file = "pyhumps-3.8.0.tar.gz", hash = "sha256:498026258f7ee1a8e447c2e28526c0bea9407f9a59c03260aee4bd6c04d681a3"}, -] - [[package]] name = "pyjwt" version = "2.11.0" @@ -1340,4 +1340,4 @@ brotli = ["brotli"] [metadata] lock-version = "2.1" python-versions = ">=3.13, <4.0" -content-hash = "7ed27a9e2c8bfd716f5b4d005dd63dc346501692001ef441fef99795ccc7f2a8" +content-hash = "fbacfd5bb699a05f71bdac39a08ac32671098ac72c5b6984d64b0cf11a29dd45" diff --git a/pyproject.toml b/pyproject.toml index 8f816e0c..9ee9917e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,8 +17,8 @@ dependencies = [ "mozilla-django-oidc (>=3.0.0,<4.0.0)", "pyjwt (>=2.8.0,<3.0.0)", "cryptography (>=46.0.3)", - "pyhumps (>=3.8.0,<4.0.0)", "coverage (>=7.13.3,<8.0.0)", + "inflection (>=0.5.1,<0.6.0)", ] [tool.poetry] From 5bf561f11e8eb167054fc89c7d4bc4d881ad42df Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Thu, 29 Jan 2026 14:02:34 +0000 Subject: [PATCH 3/6] PPHA-534: Use inflection over pyhumps --- lung_cancer_screening/jinja2_env.py | 5 ++--- .../questions/presenters/response_set_presenter.py | 5 ++--- .../views/mixins/ensure_smoking_history_for_type.py | 2 +- .../questions/views/smoking_history_question_base_view.py | 1 + 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lung_cancer_screening/jinja2_env.py b/lung_cancer_screening/jinja2_env.py index e81b6414..9df59df2 100644 --- a/lung_cancer_screening/jinja2_env.py +++ b/lung_cancer_screening/jinja2_env.py @@ -1,8 +1,7 @@ -import inflection - from django.conf import settings from django.templatetags.static import static from django.urls import reverse +from inflection import singularize from jinja2 import ChoiceLoader, Environment, PackageLoader @@ -31,7 +30,7 @@ def environment(**options): ) env.filters.update( - {"singularize": inflection.singularize} + {"singularize": singularize} ) return env diff --git a/lung_cancer_screening/questions/presenters/response_set_presenter.py b/lung_cancer_screening/questions/presenters/response_set_presenter.py index 80bf8e3b..d7cbe78c 100644 --- a/lung_cancer_screening/questions/presenters/response_set_presenter.py +++ b/lung_cancer_screening/questions/presenters/response_set_presenter.py @@ -1,7 +1,6 @@ -from inflection import dasherize, singularize - from decimal import Decimal from django.urls import reverse +from inflection import dasherize, singularize from ..models.respiratory_conditions_response import RespiratoryConditionValues @@ -253,11 +252,11 @@ def family_history_responses_items(self): return items - def smoking_history_types_responses_items(self): results = [] for type_history in self.response_set.tobacco_smoking_history.in_form_order(): results.extend(self.smoking_history_summary_items_for_type(type_history)) + return results def smoking_history_summary_items_for_type(self, type_history): diff --git a/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_for_type.py b/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_for_type.py index 564ce2f0..7035f28f 100644 --- a/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_for_type.py +++ b/lung_cancer_screening/questions/views/mixins/ensure_smoking_history_for_type.py @@ -1,5 +1,5 @@ -from django.http import Http404 from inflection import camelize +from django.http import Http404 class EnsureSmokingHistoryForTypeMixin: diff --git a/lung_cancer_screening/questions/views/smoking_history_question_base_view.py b/lung_cancer_screening/questions/views/smoking_history_question_base_view.py index 9eb5b52a..4fb808ec 100644 --- a/lung_cancer_screening/questions/views/smoking_history_question_base_view.py +++ b/lung_cancer_screening/questions/views/smoking_history_question_base_view.py @@ -1,4 +1,5 @@ from inflection import camelize + from .question_base_view import QuestionBaseView class SmokingHistoryQuestionBaseView(QuestionBaseView): From 417e5285988d8b80951dbee897fb705956c42729 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Tue, 10 Feb 2026 14:04:42 +0000 Subject: [PATCH 4/6] PPHA-534: Add frequency to amount smoked form --- features/smoked_amount.feature | 3 + features/smoking_frequency.feature | 6 +- features/steps/preflight_steps.py | 6 -- .../questions/forms/smoked_amount_form.py | 7 +- .../models/smoking_frequency_response.py | 10 ++- .../presenters/response_set_presenter.py | 13 +-- .../unit/forms/test_smoked_amount_form.py | 40 ++++++++++ .../models/test_smoking_frequency_response.py | 23 ++++++ .../tests/unit/views/test_smoked_amount.py | 80 +++++++++++++------ .../questions/views/smoked_amount.py | 33 +++++++- .../smoking_history_question_base_view.py | 4 +- 11 files changed, 171 insertions(+), 54 deletions(-) diff --git a/features/smoked_amount.feature b/features/smoked_amount.feature index d5530522..ebf283ca 100644 --- a/features/smoked_amount.feature +++ b/features/smoked_amount.feature @@ -5,6 +5,7 @@ Feature: Smoked amount page Given I am logged in And I have answered questions showing I am eligible And I have answered questions showing I have smoked "Cigarettes" + And I have answered questions showing I have smoked "Cigarettes" daily When I go to "/cigarettes-smoked-amount" Then there are no accessibility violations @@ -12,6 +13,7 @@ Feature: Smoked amount page Given I am logged in And I have answered questions showing I am eligible And I have answered questions showing I have smoked "Cigarettes" + And I have answered questions showing I have smoked "Cigarettes" daily When I go to "/cigarettes-smoked-amount" And I click "Continue" Then I am on "/cigarettes-smoked-amount" @@ -22,6 +24,7 @@ Feature: Smoked amount page Given I am logged in And I have answered questions showing I am eligible And I have answered questions showing I have smoked "Cigarettes" + And I have answered questions showing I have smoked "Cigarettes" daily When I go to "/cigarettes-smoked-amount" Then I see a back link to "/cigarettes-smoked-total-years" When I fill in "Roughly how many cigarettes do you smoke in a normal day?" with "20" diff --git a/features/smoking_frequency.feature b/features/smoking_frequency.feature index f8e0cad2..9435115d 100644 --- a/features/smoking_frequency.feature +++ b/features/smoking_frequency.feature @@ -28,15 +28,15 @@ Feature: Smoking frequency page When I check "Daily" and submit Then I am on "/cigarettes-smoked-amount" - @wip 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" - And I have answered questions showing I have smoked 10 "Cigarettes" as the amount When I go to "/cigarettes-smoking-frequency" - When I check "Daily" and submit + And I check "Daily" and submit + And I fill in "Roughly how many cigarettes do you smoke in a normal day?" with "10" + And I submit the form When I go to "/check-your-answers" Then I see "10 cigarettes a day" as a response to "Current cigarette smoking" under "Smoking history" And I see "/cigarettes-smoking-frequency?change=True" as a link to change "Current cigarette smoking" under "Smoking history" diff --git a/features/steps/preflight_steps.py b/features/steps/preflight_steps.py index cd990f82..2d3359f6 100644 --- a/features/steps/preflight_steps.py +++ b/features/steps/preflight_steps.py @@ -97,9 +97,3 @@ def given_i_have_answered_questions_showing_i_have_smoked_tobacco_type_frequency context.page.goto(f"{context.live_server_url}/{tobacco_type.lower()}-smoking-frequency") when_i_check_label(context, humanize(frequency)) when_i_submit_the_form(context) - -@given('I have answered questions showing I have smoked {amount} "{tobacco_type}" as the amount') -def given_i_have_answered_questions_showing_i_have_smoked_amount_tobacco_type(context, amount, tobacco_type): - context.page.goto(f"{context.live_server_url}/{tobacco_type.lower()}-smoked-amount") - when_i_fill_in_label_with_value(context, f"Roughly how many {tobacco_type.lower()} do you smoke in a normal day?", amount) - when_i_submit_the_form(context) diff --git a/lung_cancer_screening/questions/forms/smoked_amount_form.py b/lung_cancer_screening/questions/forms/smoked_amount_form.py index 2f876630..78304e3e 100644 --- a/lung_cancer_screening/questions/forms/smoked_amount_form.py +++ b/lung_cancer_screening/questions/forms/smoked_amount_form.py @@ -7,6 +7,8 @@ class SmokedAmountForm(forms.ModelForm): def __init__(self, *args, **kwargs): + self.frequency_text = kwargs.pop("frequency_text", "day") + super().__init__(*args, **kwargs) self.fields["value"] = IntegerField( @@ -21,15 +23,14 @@ def __init__(self, *args, **kwargs): }, ) - def type_string(self): return self.instance.tobacco_smoking_history.human_type().lower() def _type_label(self): - return f"Roughly how many {self.type_string()} do you smoke in a normal day?" + return f"Roughly how many {self.type_string()} do you smoke in a normal {self.frequency_text}?" def _type_required_error_message(self): - return f"Enter how many {self.type_string()} you currently smoke in a normal day" + return f"Enter how many {self.type_string()} you currently smoke in a normal {self.frequency_text}" def _type_suffix(self): return self.type_string() diff --git a/lung_cancer_screening/questions/models/smoking_frequency_response.py b/lung_cancer_screening/questions/models/smoking_frequency_response.py index 0019d2f0..e4f6fa35 100644 --- a/lung_cancer_screening/questions/models/smoking_frequency_response.py +++ b/lung_cancer_screening/questions/models/smoking_frequency_response.py @@ -10,13 +10,21 @@ class SmokingFrequencyValues(models.TextChoices): MONTHLY = "M", "Monthly" class SmokingFrequencyResponse(BaseModel): + SINGULAR_TEXT_MAP = { + SmokingFrequencyValues.DAILY: "day", + SmokingFrequencyValues.WEEKLY: "week", + SmokingFrequencyValues.MONTHLY: "month", + } + tobacco_smoking_history = models.OneToOneField( TobaccoSmokingHistory, on_delete=models.CASCADE, related_name='smoking_frequency_response' ) - value = models.CharField( max_length=1, choices=SmokingFrequencyValues.choices ) + + def get_value_display_as_singleton_text(self): + return self.SINGULAR_TEXT_MAP.get(self.value) diff --git a/lung_cancer_screening/questions/presenters/response_set_presenter.py b/lung_cancer_screening/questions/presenters/response_set_presenter.py index d7cbe78c..4356aa22 100644 --- a/lung_cancer_screening/questions/presenters/response_set_presenter.py +++ b/lung_cancer_screening/questions/presenters/response_set_presenter.py @@ -3,11 +3,7 @@ from inflection import dasherize, singularize from ..models.respiratory_conditions_response import RespiratoryConditionValues - from ..models.education_response import EducationValues - -from ..models.smoking_frequency_response import SmokingFrequencyValues - from ..models.family_history_lung_cancer_response import FamilyHistoryLungCancerValues class ResponseSetPresenter: @@ -289,15 +285,8 @@ def _smoking_type_to_text(self, type_history): if not hasattr(type_history, 'smoking_frequency_response') or not hasattr(type_history, 'smoked_amount_response'): return self.NOT_ANSWERED_TEXT - return f"{type_history.smoked_amount_response.value} {type_history.human_type().lower()} a {self._frequency_response_to_text(type_history.smoking_frequency_response)}" + return f"{type_history.smoked_amount_response.value} {type_history.human_type().lower()} a {type_history.smoking_frequency_response.get_value_display_as_singleton_text()}" - def _frequency_response_to_text(self, frequency_response): - if frequency_response.value == SmokingFrequencyValues.DAILY: - return "day" - elif frequency_response.value == SmokingFrequencyValues.WEEKLY: - return "week" - elif frequency_response.value == SmokingFrequencyValues.MONTHLY: - return "month" def smoking_history_responses_items(self): return [ diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py index 9868c17f..b2173619 100644 --- a/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py +++ b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py @@ -2,7 +2,9 @@ from ...factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory from ...factories.smoked_amount_response_factory import SmokedAmountResponseFactory +from ...factories.smoking_frequency_response_factory import SmokingFrequencyResponseFactory from ....models.tobacco_smoking_history import TobaccoSmokingHistoryTypes +from ....models.smoking_frequency_response import SmokingFrequencyValues from ....forms.smoked_amount_form import SmokedAmountForm @@ -12,6 +14,10 @@ def setUp(self): self.smoking_history = TobaccoSmokingHistoryFactory.create( type=TobaccoSmokingHistoryTypes.CIGARETTES.value ) + self.frequency_response = SmokingFrequencyResponseFactory.create( + tobacco_smoking_history=self.smoking_history, + value=SmokingFrequencyValues.DAILY + ) self.response = SmokedAmountResponseFactory.build( tobacco_smoking_history=self.smoking_history ) @@ -39,3 +45,37 @@ def test_is_invalid_with_a_non_numeric_value(self): ) self.assertFalse(form.is_valid()) self.assertIn("value", form.errors) + + def test_label_has_the_correct_type(self): + cigarettes_smoking_history = TobaccoSmokingHistoryFactory.create( + type=TobaccoSmokingHistoryTypes.CIGARETTES.value + ) + cigars_smoking_history = TobaccoSmokingHistoryFactory.create( + type=TobaccoSmokingHistoryTypes.CIGARS.value + ) + cigarettes_response = SmokedAmountResponseFactory.create( + tobacco_smoking_history=cigarettes_smoking_history + ) + cigars_response = SmokedAmountResponseFactory.create( + tobacco_smoking_history=cigars_smoking_history + ) + cigarettes_form = SmokedAmountForm( + instance=cigarettes_response, + data={"value": 20} + ) + cigars_form = SmokedAmountForm( + instance=cigars_response, + data={"value": 20} + ) + self.assertIn("cigarettes", cigarettes_form.fields["value"].label) + self.assertIn("cigars", cigars_form.fields["value"].label) + + + def test_label_has_the_correct_frequency(self): + form = SmokedAmountForm( + instance=self.response, + data={"value": 20}, + frequency_text="my frequency" + ) + + self.assertIn("my frequency", form.fields["value"].label) diff --git a/lung_cancer_screening/questions/tests/unit/models/test_smoking_frequency_response.py b/lung_cancer_screening/questions/tests/unit/models/test_smoking_frequency_response.py index 56a103ad..83e89acd 100644 --- a/lung_cancer_screening/questions/tests/unit/models/test_smoking_frequency_response.py +++ b/lung_cancer_screening/questions/tests/unit/models/test_smoking_frequency_response.py @@ -33,3 +33,26 @@ def test_has_value_as_enum(self): ) self.assertIsInstance(response.value, str) + + def test_get_value_display_as_singleton_text_returns_the_correct_text_daily(self): + response = SmokingFrequencyResponseFactory.build( + tobacco_smoking_history=self.tobacco_smoking_history, + value=SmokingFrequencyValues.DAILY.value + ) + self.assertEqual(response.get_value_display_as_singleton_text(), "day") + + + def test_get_value_display_as_singleton_text_returns_the_correct_text_weekly(self): + response = SmokingFrequencyResponseFactory.build( + tobacco_smoking_history=self.tobacco_smoking_history, + value=SmokingFrequencyValues.WEEKLY.value + ) + self.assertEqual(response.get_value_display_as_singleton_text(), "week") + + + def test_get_value_display_as_singleton_text_returns_the_correct_text_monthly(self): + response = SmokingFrequencyResponseFactory.build( + tobacco_smoking_history=self.tobacco_smoking_history, + value=SmokingFrequencyValues.MONTHLY.value + ) + self.assertEqual(response.get_value_display_as_singleton_text(), "month") diff --git a/lung_cancer_screening/questions/tests/unit/views/test_smoked_amount.py b/lung_cancer_screening/questions/tests/unit/views/test_smoked_amount.py index 9d05efd7..a3cdf270 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_smoked_amount.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_smoked_amount.py @@ -5,12 +5,22 @@ from lung_cancer_screening.questions.models.tobacco_smoking_history import TobaccoSmokingHistoryTypes from ...factories.response_set_factory import ResponseSetFactory from ...factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory - +from ...factories.smoking_frequency_response_factory import SmokingFrequencyResponseFactory +from ....models.smoking_frequency_response import SmokingFrequencyValues @tag("SmokedAmount") class TestGetSmokedAmount(TestCase): def setUp(self): self.user = login_user(self.client) + self.response_set = ResponseSetFactory.create(user=self.user, eligible=True) + self.smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=TobaccoSmokingHistoryTypes.CIGARETTES + ) + self.frequency_response = SmokingFrequencyResponseFactory.create( + tobacco_smoking_history=self.smoking_history, + value=SmokingFrequencyValues.DAILY + ) def test_redirects_if_the_user_is_not_logged_in(self): self.client.logout() @@ -28,7 +38,8 @@ def test_redirects_if_the_user_is_not_logged_in(self): ) def test_redirects_when_the_user_is_not_eligible(self): - ResponseSetFactory.create(user=self.user, eligible=False) + self.response_set.delete() + ResponseSetFactory.create(user=self.user) response = self.client.get( reverse("questions:smoked_amount", kwargs={ @@ -39,7 +50,7 @@ def test_redirects_when_the_user_is_not_eligible(self): self.assertRedirects(response, reverse("questions:have_you_ever_smoked")) def test_404_when_a_smoking_history_item_does_not_exist_for_the_given_type(self): - ResponseSetFactory.create(user=self.user, eligible=True) + self.smoking_history.delete() response = self.client.get(reverse("questions:smoked_amount", kwargs={ "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() @@ -47,13 +58,20 @@ def test_404_when_a_smoking_history_item_does_not_exist_for_the_given_type(self) self.assertEqual(response.status_code, 404) - def test_responds_successfully(self): - response_set = ResponseSetFactory.create(user=self.user, eligible=True) - TobaccoSmokingHistoryFactory.create( - response_set=response_set, - type=TobaccoSmokingHistoryTypes.CIGARETTES.value - ) + def test_redirects_when_the_smoking_history_item_does_not_have_a_smoking_frequency_response(self): + self.frequency_response.delete() + + response = self.client.get(reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + })) + + self.assertRedirects(response, reverse("questions:smoking_frequency", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + })) + + + def test_responds_successfully(self): response = self.client.get(reverse("questions:smoked_amount", kwargs={ "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() })) @@ -66,6 +84,15 @@ class TestPostSmokedAmount(TestCase): def setUp(self): self.user = login_user(self.client) self.response_set = ResponseSetFactory.create(user=self.user, eligible=True) + self.smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=TobaccoSmokingHistoryTypes.CIGARETTES + ) + self.frequency_response = SmokingFrequencyResponseFactory.create( + tobacco_smoking_history=self.smoking_history, + value=SmokingFrequencyValues.DAILY + ) + self.valid_params = {"value": 20} def test_redirects_if_the_user_is_not_logged_in(self): @@ -98,33 +125,39 @@ def test_redirects_when_the_user_is_not_eligible(self): self.assertRedirects(response, reverse("questions:have_you_ever_smoked")) def test_404_when_a_smoking_history_item_does_not_exist_for_the_given_type(self): + self.smoking_history.delete() + response = self.client.post(reverse("questions:smoked_amount", kwargs={ "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() }), self.valid_params) self.assertEqual(response.status_code, 404) - def test_creates_a_smoked_amount_response(self): - smoking_history = TobaccoSmokingHistoryFactory.create( - response_set=self.response_set, - type=TobaccoSmokingHistoryTypes.CIGARETTES.value - ) + + def test_redirects_when_the_smoking_history_item_does_not_have_a_smoking_frequency_response(self): + self.frequency_response.delete() + + response = self.client.post(reverse("questions:smoked_amount", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + }), self.valid_params) + + self.assertRedirects(response, reverse("questions:smoking_frequency", kwargs={ + "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + })) + + + def test_creates_a_smoked_amount_response(self): self.client.post(reverse("questions:smoked_amount", kwargs={ "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() }), self.valid_params) - smoking_history.refresh_from_db() + self.smoking_history.refresh_from_db() self.assertEqual( - smoking_history.smoked_amount_response.value, self.valid_params["value"] + self.smoking_history.smoked_amount_response.value, self.valid_params["value"] ) def test_redirects_to_responses(self): - TobaccoSmokingHistoryFactory.create( - response_set=self.response_set, - type=TobaccoSmokingHistoryTypes.CIGARETTES.value - ) - response = self.client.post( reverse("questions:smoked_amount", kwargs={ "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() @@ -136,11 +169,6 @@ def test_redirects_to_responses(self): def test_redirects_to_next_question_forwarding_the_change_query_param(self): - TobaccoSmokingHistoryFactory.create( - response_set=self.response_set, - type=TobaccoSmokingHistoryTypes.CIGARETTES.value - ) - response = self.client.post( reverse( "questions:smoked_amount", diff --git a/lung_cancer_screening/questions/views/smoked_amount.py b/lung_cancer_screening/questions/views/smoked_amount.py index fd9b9763..17c5e667 100644 --- a/lung_cancer_screening/questions/views/smoked_amount.py +++ b/lung_cancer_screening/questions/views/smoked_amount.py @@ -1,5 +1,6 @@ from django.urls import reverse, reverse_lazy from django.contrib.auth.mixins import LoginRequiredMixin +from django.shortcuts import redirect from .mixins.ensure_response_set import EnsureResponseSet from .mixins.ensure_eligible import EnsureEligibleMixin @@ -9,7 +10,26 @@ from ..models.smoked_amount_response import SmokedAmountResponse -class SmokedAmountView(LoginRequiredMixin, EnsureResponseSet, EnsureEligibleMixin, EnsureSmokingHistoryForTypeMixin, SmokingHistoryQuestionBaseView): +class EnsureSmokingFrequencyResponseMixin: + def dispatch(self, request, *args, **kwargs): + if not hasattr(self.get_object().tobacco_smoking_history, 'smoking_frequency_response'): + return redirect(reverse( + "questions:smoking_frequency", + kwargs={"tobacco_type": self.kwargs["tobacco_type"]}, + query=self.get_change_query_params() + )) + + return super().dispatch(request, *args, **kwargs) + + +class SmokedAmountView( + LoginRequiredMixin, + EnsureResponseSet, + EnsureEligibleMixin, + EnsureSmokingHistoryForTypeMixin, + EnsureSmokingFrequencyResponseMixin, + SmokingHistoryQuestionBaseView +): template_name = "smoked_amount.jinja" form_class = SmokedAmountForm model = SmokedAmountResponse @@ -17,3 +37,14 @@ class SmokedAmountView(LoginRequiredMixin, EnsureResponseSet, EnsureEligibleMixi def get_back_link_url(self): return reverse("questions:smoked_total_years", kwargs={"tobacco_type": self.kwargs["tobacco_type"]}) + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs["frequency_text"] = self.get_frequency_text() + return kwargs + + def get_frequency_text(self): + return self.get_frequency_response().get_value_display_as_singleton_text() + + def get_frequency_response(self): + return self.get_smoking_history_item().smoking_frequency_response diff --git a/lung_cancer_screening/questions/views/smoking_history_question_base_view.py b/lung_cancer_screening/questions/views/smoking_history_question_base_view.py index 4fb808ec..dc9fa8ad 100644 --- a/lung_cancer_screening/questions/views/smoking_history_question_base_view.py +++ b/lung_cancer_screening/questions/views/smoking_history_question_base_view.py @@ -5,11 +5,11 @@ class SmokingHistoryQuestionBaseView(QuestionBaseView): def get_object(self): return self.model.objects.get_or_build( - tobacco_smoking_history=self._get_history_item_for_type() + tobacco_smoking_history=self.get_smoking_history_item() )[0] - def _get_history_item_for_type(self): + def get_smoking_history_item(self): return self.request.response_set.tobacco_smoking_history.filter( type=camelize(self.kwargs["tobacco_type"]) ).first() From 1f6234cdc92aa3396d454252a5bba41484f78141 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Tue, 10 Feb 2026 16:44:09 +0000 Subject: [PATCH 5/6] PPHA-534: Add min valud validation to SmokedAmountResponse --- .../questions/forms/smoked_amount_form.py | 6 +++++- .../questions/models/smoked_amount_response.py | 7 ++++++- .../tests/unit/forms/test_smoked_amount_form.py | 16 ++++++++++++++++ .../unit/models/test_smoked_amount_response.py | 16 ++++++++++++++++ 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/lung_cancer_screening/questions/forms/smoked_amount_form.py b/lung_cancer_screening/questions/forms/smoked_amount_form.py index 78304e3e..fcfd1b58 100644 --- a/lung_cancer_screening/questions/forms/smoked_amount_form.py +++ b/lung_cancer_screening/questions/forms/smoked_amount_form.py @@ -19,7 +19,8 @@ def __init__(self, *args, **kwargs): suffix=self._type_suffix(), required=True, error_messages={ - "required": self._type_required_error_message() + "required": self._type_required_error_message(), + "min_value": self._type_min_value_error_message() }, ) @@ -32,6 +33,9 @@ def _type_label(self): def _type_required_error_message(self): return f"Enter how many {self.type_string()} you currently smoke in a normal {self.frequency_text}" + def _type_min_value_error_message(self): + return f"The number of {self.type_string()} you smoke must be at least 1" + def _type_suffix(self): return self.type_string() diff --git a/lung_cancer_screening/questions/models/smoked_amount_response.py b/lung_cancer_screening/questions/models/smoked_amount_response.py index fa708cd2..ecc46e86 100644 --- a/lung_cancer_screening/questions/models/smoked_amount_response.py +++ b/lung_cancer_screening/questions/models/smoked_amount_response.py @@ -1,4 +1,5 @@ from django.db import models +from django.core.validators import MinValueValidator from .base import BaseModel from .tobacco_smoking_history import TobaccoSmokingHistory @@ -10,4 +11,8 @@ class SmokedAmountResponse(BaseModel): on_delete=models.CASCADE, related_name='smoked_amount_response' ) - value = models.IntegerField() + value = models.IntegerField( + validators=[ + MinValueValidator(1) + ] + ) diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py index b2173619..fbe617bc 100644 --- a/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py +++ b/lung_cancer_screening/questions/tests/unit/forms/test_smoked_amount_form.py @@ -79,3 +79,19 @@ def test_label_has_the_correct_frequency(self): ) self.assertIn("my frequency", form.fields["value"].label) + + + def test_min_value_validation_has_the_correct_message(self): + self.smoking_history.type = TobaccoSmokingHistoryTypes.CIGARS.value + self.smoking_history.save() + + form = SmokedAmountForm( + instance=self.response, + data={"value": 0} + ) + form.full_clean() + self.assertIn("value", form.errors) + self.assertIn( + "The number of cigars you smoke must be at least 1", + form.errors["value"], + ) diff --git a/lung_cancer_screening/questions/tests/unit/models/test_smoked_amount_response.py b/lung_cancer_screening/questions/tests/unit/models/test_smoked_amount_response.py index 21d108e4..7ba8f79c 100644 --- a/lung_cancer_screening/questions/tests/unit/models/test_smoked_amount_response.py +++ b/lung_cancer_screening/questions/tests/unit/models/test_smoked_amount_response.py @@ -1,4 +1,5 @@ from django.test import TestCase, tag +from django.core.exceptions import ValidationError from ...factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory from ...factories.smoked_amount_response_factory import SmokedAmountResponseFactory @@ -30,3 +31,18 @@ def test_has_value_as_integer(self): value=15 ) self.assertEqual(response.value, 15) + + def test_is_invalid_if_value_is_less_than_1(self): + response = SmokedAmountResponseFactory.build( + tobacco_smoking_history=self.tobacco_smoking_history, + value=0 + ) + + with self.assertRaises(ValidationError) as context: + response.full_clean() + + self.assertIn("value", context.exception.message_dict) + self.assertIn( + "Ensure this value is greater than or equal to 1.", + context.exception.message_dict["value"], + ) From d203971b979573ca0fede95ba35b9830ab367a08 Mon Sep 17 00:00:00 2001 From: Jamie Falcus <50366804+jamiefalcus@users.noreply.github.com> Date: Wed, 11 Feb 2026 10:18:50 +0000 Subject: [PATCH 6/6] Combine type smoked with frequency in preflight steps --- features/smoked_amount.feature | 4 ---- features/steps/preflight_steps.py | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/features/smoked_amount.feature b/features/smoked_amount.feature index ebf283ca..a65bb1e6 100644 --- a/features/smoked_amount.feature +++ b/features/smoked_amount.feature @@ -4,7 +4,6 @@ Feature: Smoked amount 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 "Cigarettes" And I have answered questions showing I have smoked "Cigarettes" daily When I go to "/cigarettes-smoked-amount" Then there are no accessibility violations @@ -12,7 +11,6 @@ Feature: Smoked amount page Scenario: Form errors Given I am logged in And I have answered questions showing I am eligible - And I have answered questions showing I have smoked "Cigarettes" And I have answered questions showing I have smoked "Cigarettes" daily When I go to "/cigarettes-smoked-amount" And I click "Continue" @@ -23,7 +21,6 @@ Feature: Smoked amount 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 "Cigarettes" And I have answered questions showing I have smoked "Cigarettes" daily When I go to "/cigarettes-smoked-amount" Then I see a back link to "/cigarettes-smoked-total-years" @@ -34,7 +31,6 @@ Feature: Smoked amount page 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 "Cigarettes" And I have answered questions showing I have smoked "Cigarettes" daily When I go to "/cigarettes-smoked-amount" When I fill in "Roughly how many cigarettes do you smoke in a normal day?" with "20" diff --git a/features/steps/preflight_steps.py b/features/steps/preflight_steps.py index 2d3359f6..4411115c 100644 --- a/features/steps/preflight_steps.py +++ b/features/steps/preflight_steps.py @@ -94,6 +94,7 @@ def given_i_have_answered_questions_showing_i_have_smoked_tobacco_type( @given('I have answered questions showing I have smoked "{tobacco_type}" {frequency}') def given_i_have_answered_questions_showing_i_have_smoked_tobacco_type_frequency(context, tobacco_type, frequency): + given_i_have_answered_questions_showing_i_have_smoked_tobacco_type(context, tobacco_type) context.page.goto(f"{context.live_server_url}/{tobacco_type.lower()}-smoking-frequency") when_i_check_label(context, humanize(frequency)) when_i_submit_the_form(context)