PPHA-589: Ensure no change smoking redirects to next type

features/smoking_history.feature

@@ -108,6 +108,7 @@ Feature: Smoking history pages
     And I check "Rolling tobacco"
     And I check "Pipe"
     And I check "Cigarillos"
+    And I check "Small cigars"
     And I check "Medium cigars"
     And I submit the form
 
@@ -261,6 +262,29 @@ Feature: Smoking history pages
     When I fill in "Roughly how many years did you smoke 5 full pipe loads a month?" with "4"
     And I submit the form
 
+  # Small cigars with no change
+    Then I am on "/small-cigars-smoking-current"
+    And I see a page title "Do you currently smoke small cigars?"
+    When I check "Yes"
+    And I submit the form
+
+    Then I am on "/small-cigars-smoked-total-years"
+    When I fill in "Roughly how many years have you smoked small cigars?" with "8"
+    And I submit the form
+
+    Then I am on "/small-cigars-smoking-frequency"
+    And I see a page title "How often do you smoke small cigars?"
+    When I check "Monthly"
+    And I submit the form
+
+    Then I am on "/small-cigars-smoked-amount"
+    When I fill in "Roughly how many small cigars do you currently smoke in a normal month?" with "9"
+    And I submit the form
+
+    Then I am on "/small-cigars-smoking-change"
+    And I see a page title "Has the number of small cigars you normally smoke changed over time?"
+    When I check "No, it has not changed"
+    And I submit the form
 
   # Medium cigars with only decreased
     Then I am on "/medium-cigars-smoking-current"

lung_cancer_screening/questions/models/response_set.py

@@ -129,8 +129,12 @@ def types_tobacco_smoking_history(self):
         ))
 
 
+    def user_editable_tobacco_smoking_histories(self):
+        return self.tobacco_smoking_history.user_editable().in_form_order().all()
+
+
     def previous_normal_smoking_history(self, smoking_history_item):
-        histories = list(self.tobacco_smoking_history.in_form_order().all())
+        histories = list(self.user_editable_tobacco_smoking_histories())
         current_history_index = histories.index(smoking_history_item)
 
         if current_history_index == 0:
@@ -146,3 +150,13 @@ def previous_smoking_history(self, smoking_history_item):
         return self.tobacco_smoking_history.filter(
             type=self.previous_normal_smoking_history(smoking_history_item).type,
         ).in_form_order().last()
+
+
+    def next_smoking_history(self, smoking_history_item):
+        histories = list(self.user_editable_tobacco_smoking_histories())
+        current_history_index = histories.index(smoking_history_item)
+
+        if current_history_index == len(histories) - 1:
+            return None
+
+        return histories[current_history_index + 1]

lung_cancer_screening/questions/models/tobacco_smoking_history.py

@@ -28,14 +28,21 @@ def in_form_order(self):
             *[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_levels, order_types)
+        return self.order_by(order_types, order_levels)
+
+
+    def user_editable(self):
+        return self.exclude(level=Levels.NO_CHANGE)
+
 
     def increased(self):
         return self.filter(level='increased')
 
+
     def decreased(self):
         return self.filter(level='decreased')
 
+
     def normal(self):
         return self.filter(level='normal')
 
@@ -114,6 +121,7 @@ def clean(self):
         super().clean()
         self._validate_no_change_and_other_levels()
 
+
     def _validate_no_change_and_other_levels(self):
         others = self.response_set.tobacco_smoking_history.filter(type=self.type).exclude(pk=self.pk)
         if self.level == NO_CHANGE_VALUE:

lung_cancer_screening/questions/tests/unit/models/test_response_set.py

@@ -421,3 +421,48 @@ def test_returns_decreased_when_the_previous_normal_smoking_history_when_both_pr
             self.response_set.previous_smoking_history(current_smoking_history),
             decreased_smoking_history
         )
+
+
+    def test_next_smoking_history_returns_none_when_it_is_the_only_smoking_history_item(self):
+        smoking_history_item = TobaccoSmokingHistoryFactory.create(
+            response_set=self.response_set
+        )
+        self.assertIsNone(
+            self.response_set.next_smoking_history(smoking_history_item)
+        )
+
+
+    def test_next_smoking_history_returns_the_next_smoking_history_when_it_is_not_the_only_smoking_history_item_respecting_form_order(self):
+        TobaccoSmokingHistoryFactory.create(
+            response_set=self.response_set,
+            cigarillos=True,
+            increased=True,
+        )
+        next_smoking_history = TobaccoSmokingHistoryFactory.create(
+            response_set=self.response_set,
+            cigarillos=True,
+        )
+        current_smoking_history = TobaccoSmokingHistoryFactory.create(
+            response_set=self.response_set,
+            cigarettes=True,
+        )
+        self.assertEqual(
+            self.response_set.next_smoking_history(current_smoking_history),
+            next_smoking_history
+        )
+
+
+    def test_next_smoking_history_does_not_include_no_change(self):
+        current_smoking_history = TobaccoSmokingHistoryFactory.create(
+            response_set=self.response_set,
+            cigarillos=True,
+            normal=True,
+        )
+        TobaccoSmokingHistoryFactory.create(
+            response_set=self.response_set,
+            cigarillos=True,
+            no_change=True,
+        )
+        self.assertIsNone(
+            self.response_set.next_smoking_history(current_smoking_history)
+        )

lung_cancer_screening/questions/tests/unit/models/test_tobacco_smoking_history.py

@@ -652,6 +652,11 @@ def test_is_complete_returns_true_for_a_no_change_level_when_no_responses_are_pr
 
 
     def test_in_form_order_returns_the_tobacco_smoking_history_in_form_order(self):
+        cigarettes_decreased = TobaccoSmokingHistoryFactory(
+            response_set=self.response_set,
+            cigarettes=True,
+            decreased=True,
+        )
         medium_cigars_decreased = TobaccoSmokingHistoryFactory(
             response_set=self.response_set,
             medium_cigars=True,
@@ -676,6 +681,18 @@ def test_in_form_order_returns_the_tobacco_smoking_history_in_form_order(self):
         in_form_order = TobaccoSmokingHistory.objects.in_form_order()
         self.assertQuerySetEqual(
             in_form_order,
-            [cigarettes_normal, medium_cigars_normal, medium_cigars_increased, medium_cigars_decreased],
+            [cigarettes_normal, cigarettes_decreased, medium_cigars_normal, medium_cigars_increased, medium_cigars_decreased],
             ordered=True,
         )
+
+
+    def test_user_editable_does_not_include_no_change(self):
+        TobaccoSmokingHistoryFactory.create(
+            response_set=self.response_set,
+            cigarillos=True,
+            no_change=True,
+        )
+        self.assertQuerySetEqual(
+            TobaccoSmokingHistory.objects.user_editable().all(),
+            [],
+        )

lung_cancer_screening/questions/tests/unit/views/test_smoking_change.py

@@ -191,6 +191,7 @@ def test_redirects_when_the_user_is_not_eligible(self):
         self.assertRedirects(response, reverse("questions:have_you_ever_smoked"))
 
 
+
     def test_redirects_to_the_next_question_given_no_level(self):
         response = self.client.post(
             reverse("questions:smoking_change", kwargs = {
@@ -202,6 +203,35 @@ def test_redirects_to_the_next_question_given_no_level(self):
         self.assertRedirects(response, reverse("questions:responses"))
 
 
+    def test_redirects_to_the_next_smoking_type_given_no_change_level_and_other_smoking_histories_exist(self):
+        self.tobacco_smoking_history.delete()
+        current_smoking_history = TobaccoSmokingHistoryFactory.create(
+            response_set=self.response_set,
+            cigarettes=True,
+            complete=True,
+            normal=True,
+        )
+        next_smoking_history = TobaccoSmokingHistoryFactory.create(
+            response_set=self.response_set,
+            medium_cigars=True,
+            complete=True,
+            normal=True,
+        )
+
+        response = self.client.post(
+            reverse("questions:smoking_change", kwargs = {
+                "tobacco_type": current_smoking_history.url_type()
+            }),
+            {"value": [TobaccoSmokingHistory.Levels.NO_CHANGE]}
+        )
+
+        self.assertRedirects(response,
+            reverse("questions:smoking_current", kwargs={
+                "tobacco_type": next_smoking_history.url_type()
+            }),
+            fetch_redirect_response=False
+        )
+
     def test_redirects_to_the_next_question_given_level_increased(self):
         response = self.client.post(
             reverse("questions:smoking_change", kwargs = {
@@ -230,31 +260,6 @@ def test_redirects_to_the_next_question_given_level_decreased_only(self):
         }))
 
 
-    def test_does_not_redirect_to_increased_if_increased_exists_for_another_type_and_is_not_selected(self):
-        TobaccoSmokingHistoryFactory.create(
-            response_set=self.response_set,
-            type=self.tobacco_smoking_history.type,
-            complete=True,
-            increased=True,
-        )
-        medium_cigars = TobaccoSmokingHistoryFactory.create(
-            response_set=self.response_set,
-            medium_cigars=True,
-            complete=True
-        )
-
-        response = self.client.post(
-            reverse("questions:smoking_change", kwargs = {
-                "tobacco_type": medium_cigars.url_type()
-            }),
-            {"value": [TobaccoSmokingHistory.Levels.NO_CHANGE]}
-        )
-
-        self.assertRedirects(response, reverse("questions:responses"),
-            fetch_redirect_response=False
-        )
-
-
     def test_creates_a_smoking_change_response(self):
         self.client.post(
             reverse("questions:smoking_change", kwargs = {

lung_cancer_screening/questions/views/smoked_total_years.py

@@ -105,7 +105,6 @@ def prerequisite_responses(self):
             "smoked_amount_response"
         ]
 
-
     def remaining_unanswered_histories(self):
         tobacco_type = camelize(underscore(self.kwargs["tobacco_type"]))
         histories = self.request.response_set.types_tobacco_smoking_history()

lung_cancer_screening/questions/views/smoking_change.py

@@ -3,7 +3,6 @@
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.views.generic.edit import FormView
 
-from lung_cancer_screening.questions.models.tobacco_smoking_history import TobaccoSmokingHistory
 
 from .mixins.ensure_response_set import EnsureResponseSet
 from .mixins.ensure_eligible import EnsureEligibleMixin
@@ -57,25 +56,24 @@ def form_valid(self, form):
 
 
     def get_success_url(self):
-        tobacco_smoking_history = self.tobacco_smoking_history()
-        if tobacco_smoking_history.increased().exists():
-            return reverse(
-                "questions:smoking_frequency",
-                kwargs={
-                    "tobacco_type": self.kwargs["tobacco_type"],
-                    "level": TobaccoSmokingHistory.Levels.INCREASED,
-                },
-                query=self.get_change_query_params()
-            )
-        elif tobacco_smoking_history.decreased().exists():
-            return reverse(
-                "questions:smoking_frequency",
-                kwargs={
-                    "tobacco_type": self.kwargs["tobacco_type"],
-                    "level": TobaccoSmokingHistory.Levels.DECREASED,
-                },
-                query=self.get_change_query_params(),
-            )
+        if self.next_smoking_history():
+            if self.next_smoking_history().is_normal():
+                return reverse(
+                    "questions:smoking_current",
+                    kwargs={
+                        "tobacco_type": self.next_smoking_history().url_type(),
+                    },
+                    query=self.get_change_query_params(),
+                )
+            else:
+                return reverse(
+                    "questions:smoking_frequency",
+                    kwargs={
+                        "tobacco_type": self.next_smoking_history().url_type(),
+                        "level": self.next_smoking_history().level,
+                    },
+                    query=self.get_change_query_params(),
+                )
         else:
             return reverse("questions:responses")
 
@@ -95,3 +93,9 @@ def tobacco_smoking_history(self):
         return self.request.response_set.tobacco_smoking_history.by_url_type(
             self.kwargs["tobacco_type"]
         )
+
+
+    def next_smoking_history(self):
+        return self.request.response_set.next_smoking_history(
+            self.tobacco_smoking_history_item()
+        )