From 231a9ce920e158712fb8cf84152d146734850918 Mon Sep 17 00:00:00 2001 From: Andy Mitchell <326561+Themitchell@users.noreply.github.com> Date: Wed, 11 Mar 2026 11:22:16 +0000 Subject: [PATCH] PPHA-589: Correct the current smoker back link for multiple smoking selection --- .../questions/models/response_set.py | 23 +++- .../models/tobacco_smoking_history.py | 31 +++-- .../tests/unit/models/test_response_set.py | 121 ++++++++++++++++++ .../models/test_tobacco_smoking_history.py | 30 +++++ .../tests/unit/views/test_smoking_current.py | 73 +++++++++-- .../questions/views/smoking_current.py | 25 +++- .../smoking_history_question_base_view.py | 6 + 7 files changed, 281 insertions(+), 28 deletions(-) diff --git a/lung_cancer_screening/questions/models/response_set.py b/lung_cancer_screening/questions/models/response_set.py index a2275f5b..e3b6ee30 100644 --- a/lung_cancer_screening/questions/models/response_set.py +++ b/lung_cancer_screening/questions/models/response_set.py @@ -124,6 +124,25 @@ def is_eligible(self): def types_tobacco_smoking_history(self): - return list(self.tobacco_smoking_history.in_form_order().values_list( + return list(self.tobacco_smoking_history.normal().in_form_order().values_list( "type", flat=True - ).distinct()) + )) + + + def previous_normal_smoking_history(self, smoking_history_item): + histories = list(self.tobacco_smoking_history.in_form_order().all()) + current_history_index = histories.index(smoking_history_item) + + if current_history_index == 0: + return None + + return histories[histories.index(smoking_history_item) - 1] + + + def previous_smoking_history(self, smoking_history_item): + if not self.previous_normal_smoking_history(smoking_history_item): + return None + + return self.tobacco_smoking_history.filter( + type=self.previous_normal_smoking_history(smoking_history_item).type, + ).in_form_order().last() diff --git a/lung_cancer_screening/questions/models/tobacco_smoking_history.py b/lung_cancer_screening/questions/models/tobacco_smoking_history.py index f7596f34..6b7b54b2 100644 --- a/lung_cancer_screening/questions/models/tobacco_smoking_history.py +++ b/lung_cancer_screening/questions/models/tobacco_smoking_history.py @@ -18,12 +18,17 @@ class TobaccoSmokingHistoryTypes(models.TextChoices): 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)), + types_order = [choice[0] for choice in TobaccoSmokingHistoryTypes.choices] + order_types = Case( + *[When(type=type_val, then=Value(i)) for i, type_val in enumerate(types_order)], + default=Value(len(types_order)), + ) + levels_order = [choice[0] for choice in Levels.choices] + order_levels = Case( + *[When(level=level_val, then=Value(i)) for i, level_val in enumerate(levels_order)], + default=Value(len(levels_order)), ) - return self.order_by(order) + return self.order_by(order_levels, order_types) def increased(self): return self.filter(level='increased') @@ -68,16 +73,18 @@ def by_url_type(self, url_type): NO_CHANGE_VALUE = "no_change" -class TobaccoSmokingHistory(BaseModel): - class Levels(models.TextChoices): - NORMAL = "normal", "Normal" - INCREASED = "increased", "Yes, I used to smoke more" - DECREASED = "decreased", "Yes, I used to smoke fewer" +class Levels(models.TextChoices): + NORMAL = "normal", "Normal" + INCREASED = "increased", "Yes, I used to smoke more" + DECREASED = "decreased", "Yes, I used to smoke fewer" - # Only used to populate values in the form - NO_CHANGE = NO_CHANGE_VALUE, "No, it has not changed" + # Only used to populate values in the form + NO_CHANGE = NO_CHANGE_VALUE, "No, it has not changed" +class TobaccoSmokingHistory(BaseModel): + Levels = Levels + response_set = models.ForeignKey( ResponseSet, on_delete=models.CASCADE, diff --git a/lung_cancer_screening/questions/tests/unit/models/test_response_set.py b/lung_cancer_screening/questions/tests/unit/models/test_response_set.py index 94c13658..35e544a7 100644 --- a/lung_cancer_screening/questions/tests/unit/models/test_response_set.py +++ b/lung_cancer_screening/questions/tests/unit/models/test_response_set.py @@ -300,3 +300,124 @@ def test_types_tobacco_smoking_history_returns_all_types_of_tobacco_smoking_hist list(self.response_set.types_tobacco_smoking_history()), [cigarettes.type.value, cigarillos.type.value] ) + + + def test_previous_normal_smoking_history_returns_none_when_it_is_the_only_smoking_history_item (self): + self.response_set.tobacco_smoking_history.all().delete() + smoking_history_item = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set + ) + self.assertIsNone( + self.response_set.previous_normal_smoking_history( + smoking_history_item + ) + ) + + + def test_previous_normal_smoking_history_returns_none_when_it_is_the_first_smoking_history_item_respecting_form_order(self): + self.response_set.tobacco_smoking_history.all().delete() + TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarillos=True, + ) + smoking_history_item = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarettes=True, + ) + self.assertIsNone( + self.response_set.previous_normal_smoking_history( + smoking_history_item + ) + ) + + + def test_previous_normal_smoking_history_returns_the_previous_normal_smoking_history_when_it_is_not_the_only_smoking_history_item(self): + self.response_set.tobacco_smoking_history.all().delete() + previous_normal_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarettes=True + ) + smoking_history_item = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarillos=True + ) + self.assertEqual( + self.response_set.previous_normal_smoking_history(smoking_history_item), + previous_normal_smoking_history + ) + + + def test_previous_smoking_history_returns_none_when_it_is_the_only_smoking_history_item(self): + self.response_set.tobacco_smoking_history.all().delete() + smoking_history_item = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set + ) + self.assertIsNone( + self.response_set.previous_smoking_history(smoking_history_item) + ) + + + def test_returns_the_increased_smoking_history_when_it_is_attached_to_the_previous_normal_history(self): + self.response_set.tobacco_smoking_history.all().delete() + previous_normal_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarettes=True + ) + increased_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=previous_normal_smoking_history.type, + increased=True + ) + current_normal_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarillos=True, + ) + self.assertEqual( + self.response_set.previous_smoking_history(current_normal_smoking_history), + increased_smoking_history + ) + + def test_returns_the_decreased_smoking_history_when_it_is_attached_to_the_previous_normal_history(self): + self.response_set.tobacco_smoking_history.all().delete() + previous_normal_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarettes=True + ) + decreased_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=previous_normal_smoking_history.type, + decreased=True + ) + current_normal_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarillos=True, + ) + self.assertEqual( + self.response_set.previous_smoking_history(current_normal_smoking_history), + decreased_smoking_history + ) + + def test_returns_decreased_when_the_previous_normal_smoking_history_when_both_previous_and_current_exist(self): + self.response_set.tobacco_smoking_history.all().delete() + previous_normal_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarettes=True + ) + decreased_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=previous_normal_smoking_history.type, + decreased=True + ) + TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=previous_normal_smoking_history.type, + increased=True + ) + current_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + cigarillos=True, + ) + self.assertEqual( + self.response_set.previous_smoking_history(current_smoking_history), + decreased_smoking_history + ) diff --git a/lung_cancer_screening/questions/tests/unit/models/test_tobacco_smoking_history.py b/lung_cancer_screening/questions/tests/unit/models/test_tobacco_smoking_history.py index 56c26c15..3e1c3039 100644 --- a/lung_cancer_screening/questions/tests/unit/models/test_tobacco_smoking_history.py +++ b/lung_cancer_screening/questions/tests/unit/models/test_tobacco_smoking_history.py @@ -649,3 +649,33 @@ def test_is_complete_returns_true_for_a_no_change_level_when_no_responses_are_pr ) self.assertTrue(tobacco_smoking_history.is_complete()) + + + def test_in_form_order_returns_the_tobacco_smoking_history_in_form_order(self): + medium_cigars_decreased = TobaccoSmokingHistoryFactory( + response_set=self.response_set, + medium_cigars=True, + decreased=True, + ) + cigarettes_normal = TobaccoSmokingHistoryFactory( + response_set=self.response_set, + cigarettes=True, + normal=True, + ) + medium_cigars_increased = TobaccoSmokingHistoryFactory( + response_set=self.response_set, + medium_cigars=True, + increased=True, + ) + medium_cigars_normal = TobaccoSmokingHistoryFactory( + response_set=self.response_set, + medium_cigars=True, + normal=True, + ) + + in_form_order = TobaccoSmokingHistory.objects.in_form_order() + self.assertQuerySetEqual( + in_form_order, + [cigarettes_normal, medium_cigars_normal, medium_cigars_increased, medium_cigars_decreased], + ordered=True, + ) 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 2815afa7..41112fec 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 @@ from django.test import TestCase, tag from django.urls import reverse -from lung_cancer_screening.questions.models.tobacco_smoking_history import TobaccoSmokingHistoryTypes from lung_cancer_screening.questions.tests.factories.tobacco_smoking_history_factory import TobaccoSmokingHistoryFactory from .helpers.authentication import login_user @@ -24,7 +23,7 @@ def test_redirects_if_the_user_is_not_logged_in(self): response = self.client.get( reverse("questions:smoking_current", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() }) ) @@ -40,7 +39,7 @@ def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(sel response = self.client.get( reverse("questions:smoking_current", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() }) ) @@ -53,7 +52,7 @@ def test_redirects_when_the_user_is_not_eligible(self): response = self.client.get( reverse("questions:smoking_current", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() }) ) @@ -62,12 +61,62 @@ def test_redirects_when_the_user_is_not_eligible(self): def test_responds_successfully(self): response = self.client.get(reverse("questions:smoking_current", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() })) self.assertEqual(response.status_code, 200) + def test_has_a_back_link_default(self): + response = self.client.get(reverse("questions:smoking_current", kwargs = { + "tobacco_type": self.tobacco_smoking_history.url_type() + })) + + self.assertEqual( + response.context_data["back_link_url"], + reverse("questions:types_tobacco_smoking") + ) + + + def test_has_a_back_link_to_the_previous_tobacco_type_when_on_a_later_type_with_no_changed_level(self): + later_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + small_cigars=True + ) + response = self.client.get(reverse("questions:smoking_current", kwargs = { + "tobacco_type": later_smoking_history.url_type() + })) + + self.assertEqual( + response.context_data["back_link_url"], + reverse("questions:smoked_amount", kwargs = { + "tobacco_type": self.tobacco_smoking_history.url_type() + }) + ) + + + def test_has_a_back_link_to_the_previous_tobacco_type_when_on_a_later_type_with_increased_level(self): + increased = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + type=self.tobacco_smoking_history.type, + increased=True + ) + later_smoking_history = TobaccoSmokingHistoryFactory.create( + response_set=self.response_set, + small_cigars=True + ) + response = self.client.get(reverse("questions:smoking_current", kwargs = { + "tobacco_type": later_smoking_history.url_type(), + })) + + self.assertEqual( + response.context_data["back_link_url"], + reverse("questions:smoked_total_years", kwargs = { + "tobacco_type": increased.url_type(), + "level": increased.level + }) + ) + @tag("SmokingCurrent") class TestPostSmokingCurrent(TestCase): def setUp(self): @@ -101,7 +150,7 @@ def test_redirects_when_a_submitted_response_set_exists_within_the_last_year(sel response = self.client.post( reverse("questions:smoking_current", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() }), self.valid_params ) @@ -115,7 +164,7 @@ def test_redirects_when_the_user_is_not_eligible(self): response = self.client.post( reverse("questions:smoking_current", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() }), self.valid_params ) @@ -124,7 +173,7 @@ def test_redirects_when_the_user_is_not_eligible(self): def test_creates_a_smoking_current_response(self): self.client.post(reverse("questions:smoking_current", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() }), self.valid_params) self.tobacco_smoking_history.refresh_from_db() @@ -135,7 +184,7 @@ def test_creates_a_smoking_current_response(self): def test_redirects_to_next_question(self): response = self.client.post( reverse("questions:smoking_current", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() }), self.valid_params ) @@ -148,7 +197,7 @@ def test_redirects_to_next_question(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() + "tobacco_type": self.tobacco_smoking_history.url_type() }), { **self.valid_params, @@ -158,7 +207,7 @@ def test_redirects_to_next_question_forwarding_the_change_query_param(self): self.assertRedirects(response, reverse("questions:smoked_total_years", kwargs={ - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() }, query={"change": "True"} ), fetch_redirect_response=False) @@ -167,7 +216,7 @@ def test_redirects_to_next_question_forwarding_the_change_query_param(self): def test_responds_with_422_if_the_response_fails_to_create(self): response = self.client.post( reverse("questions:smoking_current", kwargs = { - "tobacco_type": TobaccoSmokingHistoryTypes.CIGARETTES.value.lower() + "tobacco_type": self.tobacco_smoking_history.url_type() }), {"value": "something not in list"} ) diff --git a/lung_cancer_screening/questions/views/smoking_current.py b/lung_cancer_screening/questions/views/smoking_current.py index 89257bdf..f6970bd9 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, reverse_lazy +from django.urls import reverse from django.contrib.auth.mixins import LoginRequiredMixin from lung_cancer_screening.questions.views.smoking_history_question_base_view import SmokingHistoryQuestionBaseView @@ -20,7 +20,6 @@ class SmokingCurrentView( template_name = "question_form.jinja" form_class = SmokingCurrentForm model = SmokingCurrentResponse - back_link_url = reverse_lazy("questions:types_tobacco_smoking") def get_success_url(self): return reverse( @@ -30,6 +29,28 @@ def get_success_url(self): ) + def get_back_link_url(self): + if self.previous_smoking_history(): + if self.previous_smoking_history().is_normal(): + return reverse( + "questions:smoked_amount", + kwargs={"tobacco_type": self.previous_smoking_history().url_type()}, + query=self.get_change_query_params(), + ) + + return reverse( + "questions:smoked_total_years", + kwargs={ + "tobacco_type": self.previous_smoking_history().url_type(), + "level": self.previous_smoking_history().level, + }, + query=self.get_change_query_params(), + ) + + return reverse("questions:types_tobacco_smoking") + + + def get_form_kwargs(self): kwargs = super().get_form_kwargs() kwargs["tobacco_smoking_history"] = self.tobacco_smoking_history_item() 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 c42eb09f..addb46ae 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 @@ -13,3 +13,9 @@ def get_normal_smoking_history_item(self): return self.request.response_set.tobacco_smoking_history.by_url_type( self.kwargs["tobacco_type"] ).normal().first() + + + def previous_smoking_history(self): + return self.request.response_set.previous_smoking_history( + self.tobacco_smoking_history_item() + )