Cleanup missing test coverage

lung_cancer_screening/core/jinja2/layout.jinja

@@ -25,7 +25,7 @@
         },
         {
           "text": "Log out",
-          "href": url("questions:logout")
+          "href": url("oidc_logout")
         }
       ] if request.user.is_authenticated else []
     }

lung_cancer_screening/questions/auth.py

@@ -92,8 +92,6 @@ def _create_client_assertion(self):
             headers=headers
         )
 
-        if isinstance(assertion, bytes):
-            return assertion.decode('utf-8')
         return assertion
 
     def get_token(self, token_payload):

lung_cancer_screening/questions/tests/unit/forms/test_types_tobacco_smoking_form.py

@@ -51,6 +51,37 @@ def test_is_invalid_when_no_option_is_selected(self):
             ["Select the type of tobacco you smoke or have smoked"],
         )
 
+    def test_does_not_save_smoking_history_if_invalid(self):
+        form = TypesTobaccoSmokingForm(
+            response_set=self.response_set,
+            data={
+                "value": ["invalid"]
+            }
+        )
+
+        form.save()
+
+        self.assertEqual(self.response_set.tobacco_smoking_history.count(), 0)
+
+
+    def test_does_not_delete_smoking_history_if_invalid(self):
+        history = TobaccoSmokingHistoryFactory(
+            response_set=self.response_set,
+            type=TobaccoSmokingHistoryTypes.CIGARETTES
+        )
+
+        form = TypesTobaccoSmokingForm(
+            response_set=self.response_set,
+            data={
+                "value": ["invalid"]
+            }
+        )
+
+        form.save()
+
+        self.assertEqual(self.response_set.tobacco_smoking_history.first(), history)
+
+
     def test_saves_an_tobacco_smoking_type_for_each_value_selected(self):
         form = TypesTobaccoSmokingForm(
             response_set=self.response_set,

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

@@ -1,4 +1,5 @@
 from django.test import TestCase, tag
+from django.core.exceptions import ValidationError
 
 from ...factories.response_set_factory import ResponseSetFactory
 from ...factories.height_response_factory import HeightResponseFactory
@@ -17,28 +18,118 @@ def test_has_a_valid_factory(self):
 
 
     def test_has_response_set_as_foreign_key(self):
-        response_set = ResponseSetFactory()
-        response = HeightResponse.objects.create(
-            response_set=response_set,
+        response = HeightResponseFactory.build(
+            response_set=self.response_set,
             metric=1700
         )
 
-        self.assertEqual(response.response_set, response_set)
+        self.assertEqual(response.response_set, self.response_set)
+
 
     def test_has_metric_as_int(self):
-        response_set = ResponseSetFactory()
-        response = HeightResponse.objects.create(
-            response_set=response_set,
+        response = HeightResponseFactory.build(
+            response_set=self.response_set,
             metric=1700
         )
 
         self.assertIsInstance(response.metric, int)
 
     def test_has_imperial_as_int(self):
-        response_set = ResponseSetFactory()
-        response = HeightResponse.objects.create(
-            response_set=response_set,
+        response = HeightResponseFactory.build(
+            response_set=self.response_set,
             imperial=68
         )
 
         self.assertIsInstance(response.imperial, int)
+
+
+    def test_is_invalid_if_neither_metric_nor_imperial_are_set(self):
+        response = HeightResponseFactory.build(
+            response_set=self.response_set,
+            metric=None,
+            imperial=None
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Either metric or imperial height must be provided.",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_both_metric_and_imperial_are_set(self):
+        response = HeightResponseFactory.build(
+            response_set=self.response_set,
+            metric=1700,
+            imperial=68
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Cannot provide both metric and imperial height.",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_metric_is_less_than_minimum_height(self):
+        response = HeightResponseFactory.build(
+            response_set=self.response_set,
+            metric=HeightResponse.MIN_HEIGHT_METRIC - 1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Height must be between 139.7cm and 243.8 cm",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_metric_is_greater_than_maximum_height(self):
+        response = HeightResponseFactory.build(
+            response_set=self.response_set,
+            metric=HeightResponse.MAX_HEIGHT_METRIC + 1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Height must be between 139.7cm and 243.8 cm",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_imperial_is_less_than_minimum_height(self):
+        response = HeightResponseFactory.build(
+            response_set=self.response_set,
+            imperial=HeightResponse.MIN_HEIGHT_IMPERIAL - 1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Height must be between 4 feet 7 inches and 8 feet",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_imperial_is_greater_than_maximum_height(self):
+        response = HeightResponseFactory.build(
+            response_set=self.response_set,
+            imperial=HeightResponse.MAX_HEIGHT_IMPERIAL + 1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Height must be between 4 feet 7 inches and 8 feet",
+            context.exception.messages,
+        )

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

@@ -1,4 +1,5 @@
 from django.test import TestCase, tag
+from django.core.exceptions import ValidationError
 
 from ...factories.response_set_factory import ResponseSetFactory
 from ...factories.weight_response_factory import WeightResponseFactory
@@ -18,28 +19,119 @@ def test_has_a_valid_factory(self):
 
 
     def test_has_response_set_as_foreign_key(self):
-        response_set = ResponseSetFactory()
         response = WeightResponse.objects.create(
-            response_set=response_set,
+            response_set=self.response_set,
             metric=680
         )
 
-        self.assertEqual(response.response_set, response_set)
+        self.assertEqual(response.response_set, self.response_set)
+
 
     def test_has_metric_as_int(self):
-        response_set = ResponseSetFactory()
         response = WeightResponse.objects.create(
-            response_set=response_set,
+            response_set=self.response_set,
             metric=680
         )
 
         self.assertIsInstance(response.metric, int)
 
+
     def test_has_imperial_as_int(self):
-        response_set = ResponseSetFactory()
         response = WeightResponse.objects.create(
-            response_set=response_set,
+            response_set=self.response_set,
             imperial=140
         )
 
         self.assertIsInstance(response.imperial, int)
+
+
+    def test_is_invalid_if_neither_metric_nor_imperial_are_set(self):
+        response = WeightResponseFactory.build(
+            response_set=self.response_set,
+            metric=None,
+            imperial=None
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Either metric or imperial weight must be provided.",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_both_metric_and_imperial_are_set(self):
+        response = WeightResponseFactory.build(
+            response_set=self.response_set,
+            metric=1700,
+            imperial=68
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Cannot provide both metric and imperial weight.",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_metric_is_less_than_minimum_weight(self):
+        response = WeightResponseFactory.build(
+            response_set=self.response_set,
+            metric=WeightResponse.MIN_WEIGHT_METRIC - 1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Weight must be between 25.4kg and 317.5kg",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_metric_is_greater_than_maximum_weight(self):
+        response = WeightResponseFactory.build(
+            response_set=self.response_set,
+            metric=WeightResponse.MAX_WEIGHT_METRIC + 1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Weight must be between 25.4kg and 317.5kg",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_imperial_is_less_than_minimum_weight(self):
+        response = WeightResponseFactory.build(
+            response_set=self.response_set,
+            imperial=WeightResponse.MIN_WEIGHT_IMPERIAL - 1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Weight must be between 4 stone and 50 stone",
+            context.exception.messages,
+        )
+
+
+    def test_is_invalid_if_imperial_is_greater_than_maximum_weight(self):
+        response = WeightResponseFactory.build(
+            response_set=self.response_set,
+            imperial=WeightResponse.MAX_WEIGHT_IMPERIAL + 1
+        )
+
+        with self.assertRaises(ValidationError) as context:
+            response.full_clean()
+
+        self.assertIn(
+            "Weight must be between 4 stone and 50 stone",
+            context.exception.messages,
+        )

lung_cancer_screening/questions/tests/unit/test_auth.py

@@ -99,6 +99,16 @@ def test_update_user_updates_the_user(self):
         self.assertEqual(user.nhs_number, '1234567890')
 
 
+
+    def test_get_token_raisees_if_private_key_not_parsable(self):
+        settings.OIDC_RP_CLIENT_PRIVATE_KEY = "invalid"
+
+        with self.assertRaises(ValueError) as context:
+            self.backend.get_token({'code': 'auth-code-123'})
+
+        self.assertIn("Failed to load private key", str(context.exception))
+
+
     @patch('lung_cancer_screening.questions.auth.requests.post')
     def test_get_token_success(self, mock_post):
         settings.OIDC_RP_CLIENT_PRIVATE_KEY = self.test_private_key_pem

lung_cancer_screening/questions/urls.py

@@ -45,7 +45,6 @@
 from .views.start import StartView
 from .views.weight import WeightView
 from .views.confirmation import ConfirmationView
-from .views.logout import LogoutView
 
 urlpatterns = [
     path('', RedirectView.as_view(url='/start'), name='root'),
@@ -78,5 +77,4 @@
     path('start', StartView.as_view(), name='start'),
     path('weight', WeightView.as_view(), name='weight'),
     path('confirmation', ConfirmationView.as_view(), name='confirmation'),
-    path('logout', LogoutView.as_view(), name='logout'),
 ]

lung_cancer_screening/settings.py

@@ -252,9 +252,8 @@ def list_env(key):
 LOGIN_URL = '/oidc/authenticate/'
 LOGIN_REDIRECT_URL = '/'
 LOGOUT_REDIRECT_URL = '/'
-
-
 LOGIN_REDIRECT_URL_FAILURE = "/agree-to-share-information"
+ALLOW_LOGOUT_GET_METHOD = True
 
 # Additional security settings for production
 if not DEBUG: