Move RespiratoryConditionsResponse to its own model

Themitchell · Themitchell · commit 516168eb4c51 · 2025-12-22T12:09:15.000Z
diff --git a/lung_cancer_screening/questions/forms/respiratory_conditions_form.py b/lung_cancer_screening/questions/forms/respiratory_conditions_form.py
@@ -1,15 +1,15 @@
 from django import forms
 
 from ...nhsuk_forms.choice_field import MultipleChoiceField
-from ..models.response_set import ResponseSet, RespiratoryConditionValues
+from ..models.respiratory_conditions_response import RespiratoryConditionsResponse, RespiratoryConditionValues
 
 
 class RespiratoryConditionsForm(forms.ModelForm):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
-        self.fields["respiratory_conditions"] = MultipleChoiceField(
+        self.fields["value"] = MultipleChoiceField(
             choices=RespiratoryConditionValues.choices,
             widget=forms.CheckboxSelectMultiple,
             label=(
@@ -31,7 +31,7 @@ def __init__(self, *args, **kwargs):
         )
 
         # Add hints for each choice
-        respiratory_conditions_field = self["respiratory_conditions"]
+        respiratory_conditions_field = self["value"]
         respiratory_conditions_field.add_hint_for_choice(
             RespiratoryConditionValues.PNEUMONIA,
             "An infection of the lungs, usually diagnosed by a chest x-ray"
@@ -66,5 +66,5 @@ def __init__(self, *args, **kwargs):
         )
 
     class Meta:
-        model = ResponseSet
-        fields = ['respiratory_conditions']
+        model = RespiratoryConditionsResponse
+        fields = ['value']
diff --git a/lung_cancer_screening/questions/migrations/0031_respiratoryconditionsresponse.py b/lung_cancer_screening/questions/migrations/0031_respiratoryconditionsresponse.py
@@ -0,0 +1,55 @@
+# Generated manually to create RespiratoryConditionsResponse and copy data
+
+import django.contrib.postgres.fields
+import django.db.models.deletion
+import lung_cancer_screening.questions.models.respiratory_conditions_response
+from django.db import migrations, models
+
+
+def copy_respiratory_conditions_data(apps, schema_editor):
+    with schema_editor.connection.cursor() as cursor:
+        cursor.execute("""
+            INSERT INTO questions_respiratoryconditionsresponse (created_at, updated_at, value, response_set_id)
+            SELECT NOW(), NOW(), respiratory_conditions, id
+            FROM questions_responseset
+            WHERE respiratory_conditions IS NOT NULL
+            ON CONFLICT (response_set_id) DO NOTHING
+        """)
+
+
+def reverse_copy_respiratory_conditions_data(apps, schema_editor):
+    with schema_editor.connection.cursor() as cursor:
+        cursor.execute("""
+            UPDATE questions_responseset rs
+            SET respiratory_conditions = r.value
+            FROM questions_respiratoryconditionsresponse r
+            WHERE rs.id = r.response_set_id
+        """)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('questions', '0030_heightresponse'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='RespiratoryConditionsResponse',
+            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', django.contrib.postgres.fields.ArrayField(base_field=models.CharField(choices=[('P', 'Pneumonia'), ('E', 'Emphysema'), ('B', 'Bronchitis'), ('T', 'Tuberculosis (TB)'), ('C', 'Chronic obstructive pulmonary disease (COPD)'), ('N', 'No, I have not had any of these respiratory conditions')], max_length=1), size=None, validators=[lung_cancer_screening.questions.models.respiratory_conditions_response.validate_singleton_option])),
+                ('response_set', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='respiratory_conditions_response', to='questions.responseset')),
+            ],
+            options={
+                'abstract': False,
+            },
+        ),
+        migrations.RunPython(copy_respiratory_conditions_data, reverse_copy_respiratory_conditions_data),
+        migrations.RemoveField(
+            model_name='responseset',
+            name='respiratory_conditions',
+        ),
+    ]
diff --git a/lung_cancer_screening/questions/models/__init__.py b/lung_cancer_screening/questions/models/__init__.py
@@ -6,3 +6,4 @@
 from .ethnicity_response import EthnicityResponse # noqa: F401
 from .gender_response import GenderResponse # noqa: F401
 from .height_response import HeightResponse # noqa: F401
+from .respiratory_conditions_response import RespiratoryConditionsResponse # noqa: F401
diff --git a/lung_cancer_screening/questions/models/respiratory_conditions_response.py b/lung_cancer_screening/questions/models/respiratory_conditions_response.py
@@ -0,0 +1,41 @@
+from django.db import models
+from django.contrib.postgres.fields import ArrayField
+from django.core.exceptions import ValidationError
+
+from .base import BaseModel
+from .response_set import ResponseSet
+
+
+class RespiratoryConditionValues(models.TextChoices):
+    PNEUMONIA = "P", "Pneumonia"
+    EMPHYSEMA = "E", "Emphysema"
+    BRONCHITIS = "B", "Bronchitis"
+    TUBERCULOSIS = "T", "Tuberculosis (TB)"
+    COPD = "C", "Chronic obstructive pulmonary disease (COPD)"
+    NONE = "N", "No, I have not had any of these respiratory conditions"
+
+
+def validate_singleton_option(value):
+    if value and "N" in value and len(value) > 1:
+        raise ValidationError(
+            "Cannot have singleton value and other values selected",
+            code="singleton_option",
+        )
+
+
+class RespiratoryConditionsResponse(BaseModel):
+    response_set = models.OneToOneField(ResponseSet, on_delete=models.CASCADE, related_name='respiratory_conditions_response')
+    value = ArrayField(
+        models.CharField(max_length=1, choices=RespiratoryConditionValues.choices),
+        validators=[validate_singleton_option]
+    )
+
+    @property
+    def formatted(self):
+        if not self.value:
+            return None
+        display_values = [
+            RespiratoryConditionValues(code).label
+            for code in self.value
+        ]
+        return ", ".join(display_values)
diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_respiratory_conditions_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_respiratory_conditions_form.py
@@ -1,92 +1,95 @@
 from django.test import TestCase
 
-from ...factories.user_factory import UserFactory
-from ....models.response_set import ResponseSet
+from ...factories.response_set_factory import ResponseSetFactory
+from ....models.respiratory_conditions_response import RespiratoryConditionsResponse
 from ....forms.respiratory_conditions_form import RespiratoryConditionsForm
 
 
 class TestRespiratoryConditionsForm(TestCase):
     def setUp(self):
-        self.user = UserFactory()
-        self.response_set = ResponseSet(user=self.user)
+        self.response_set = ResponseSetFactory()
+        self.response = RespiratoryConditionsResponse.objects.create(
+            response_set=self.response_set,
+            value=["P"]
+        )
 
     def test_is_valid_with_single_condition(self):
         form = RespiratoryConditionsForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "respiratory_conditions": ["P"]
+                "value": ["P"]
             }
         )
         self.assertTrue(form.is_valid())
         self.assertEqual(
-            form.cleaned_data["respiratory_conditions"],
+            form.cleaned_data["value"],
             ["P"]
         )
 
     def test_is_valid_with_multiple_conditions(self):
         form = RespiratoryConditionsForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "respiratory_conditions": ["P", "E", "C"]
+                "value": ["P", "E", "C"]
             }
         )
         self.assertTrue(form.is_valid())
         self.assertEqual(
-            form.cleaned_data["respiratory_conditions"],
+            form.cleaned_data["value"],
             ["P", "E", "C"]
         )
 
     def test_is_valid_with_none_of_the_above(self):
         form = RespiratoryConditionsForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "respiratory_conditions": ["N"]
+                "value": ["N"]
             }
         )
         self.assertTrue(form.is_valid())
         self.assertEqual(
-            form.cleaned_data["respiratory_conditions"],
+            form.cleaned_data["value"],
             ["N"]
         )
 
     def test_is_invalid_with_an_invalid_value(self):
         form = RespiratoryConditionsForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "respiratory_conditions": ["invalid"]
+                "value": ["invalid"]
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["respiratory_conditions"],
+            form.errors["value"],
             ["Select a valid choice. invalid is not one of the available choices."]
         )
 
     def test_is_invalid_when_no_option_is_selected(self):
         form = RespiratoryConditionsForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "respiratory_conditions": []
+                "value": []
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["respiratory_conditions"],
+            form.errors["value"],
             ["Select if you have had any respiratory conditions"]
         )
 
     def test_is_invalid_with_none_of_the_above_selected_and_other_options_selected(
         self
     ):
         form = RespiratoryConditionsForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "respiratory_conditions": ["N", "P", "E", "C"]
+                "value": ["N", "P", "E", "C"]
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["respiratory_conditions"][0],
+            form.errors["value"][0],
             (
                 "Select if you have had any respiratory conditions, "
                 "or select 'No, I have not had any of these "
diff --git a/lung_cancer_screening/questions/tests/unit/models/test_respiratory_conditions_response.py b/lung_cancer_screening/questions/tests/unit/models/test_respiratory_conditions_response.py
@@ -0,0 +1,24 @@
+from django.test import TestCase
+
+from ...factories.response_set_factory import ResponseSetFactory
+from ....models.respiratory_conditions_response import RespiratoryConditionsResponse, RespiratoryConditionValues
+
+
+class TestRespiratoryConditionsResponse(TestCase):
+    def test_has_response_set_as_foreign_key(self):
+        response_set = ResponseSetFactory()
+        response = RespiratoryConditionsResponse.objects.create(
+            response_set=response_set,
+            value=[RespiratoryConditionValues.PNEUMONIA]
+        )
+
+        self.assertEqual(response.response_set, response_set)
+
+    def test_has_value_as_list(self):
+        response_set = ResponseSetFactory()
+        response = RespiratoryConditionsResponse.objects.create(
+            response_set=response_set,
+            value=[RespiratoryConditionValues.PNEUMONIA, RespiratoryConditionValues.BRONCHITIS]
+        )
+
+        self.assertIsInstance(response.value, list)
diff --git a/lung_cancer_screening/questions/tests/unit/views/test_respiratory_conditions.py b/lung_cancer_screening/questions/tests/unit/views/test_respiratory_conditions.py
@@ -4,6 +4,7 @@
 from django.utils import timezone
 
 from .helpers.authentication import login_user
+from lung_cancer_screening.questions.models.respiratory_conditions_response import RespiratoryConditionsResponse
 
 
 class TestGetRespiratoryConditions(TestCase):
@@ -68,7 +69,7 @@ class TestPostRespiratoryConditions(TestCase):
     def setUp(self):
         self.user = login_user(self.client)
 
-        self.valid_params = {"respiratory_conditions": ["P", "E"]}
+        self.valid_params = {"value": ["P", "E"]}
 
     def test_post_redirects_if_the_user_is_not_logged_in(self):
         self.client.logout()
@@ -96,8 +97,8 @@ def test_post_creates_unsubmitted_response_set_when_no_response_set_exists(
         self.assertEqual(self.user.responseset_set.count(), 1)
         self.assertEqual(response_set.submitted_at, None)
         self.assertEqual(
-            response_set.respiratory_conditions,
-            self.valid_params["respiratory_conditions"]
+            RespiratoryConditionsResponse.objects.get(response_set=response_set).value,
+            self.valid_params["value"]
         )
         self.assertEqual(response_set.user, self.user)
 
@@ -113,8 +114,8 @@ def test_post_updates_unsubmitted_response_set_when_one_exists(self):
         self.assertEqual(self.user.responseset_set.count(), 1)
         self.assertEqual(response_set.submitted_at, None)
         self.assertEqual(
-            response_set.respiratory_conditions,
-            self.valid_params["respiratory_conditions"]
+            RespiratoryConditionsResponse.objects.get(response_set=response_set).value,
+            self.valid_params["value"]
         )
         self.assertEqual(response_set.user, self.user)
 
@@ -136,8 +137,8 @@ def test_post_creates_new_unsubmitted_response_set_when_submitted_exists_over_ye
         response_set = self.user.responseset_set.last()
         self.assertEqual(response_set.submitted_at, None)
         self.assertEqual(
-            response_set.respiratory_conditions,
-            self.valid_params["respiratory_conditions"]
+            RespiratoryConditionsResponse.objects.get(response_set=response_set).value,
+            self.valid_params["value"]
         )
         self.assertEqual(response_set.user, self.user)
 
@@ -163,20 +164,20 @@ def test_post_stores_a_valid_response_for_the_user(self):
 
         response_set = self.user.responseset_set.first()
         self.assertEqual(
-            response_set.respiratory_conditions,
-            self.valid_params["respiratory_conditions"]
+            RespiratoryConditionsResponse.objects.get(response_set=response_set).value,
+            self.valid_params["value"]
         )
         self.assertEqual(response_set.user, self.user)
 
     def test_post_stores_single_selection(self):
         self.client.post(
             reverse("questions:respiratory_conditions"),
-            {"respiratory_conditions": ["N"]}
+            {"value": ["N"]}
         )
 
         response_set = self.user.responseset_set.first()
         self.assertEqual(
-            response_set.respiratory_conditions,
+            RespiratoryConditionsResponse.objects.get(response_set=response_set).value,
             ["N"]
         )
 
@@ -201,7 +202,7 @@ def test_post_responds_with_422_if_the_response_fails_to_create(self):
     def test_post_responds_with_422_if_no_selection_is_made(self):
         response = self.client.post(
             reverse("questions:respiratory_conditions"),
-            {"respiratory_conditions": []}
+            {"value": []}
         )
 
         self.assertEqual(response.status_code, 422)
@@ -212,7 +213,6 @@ def test_post_does_not_update_response_set_on_invalid_data(self):
             {"respiratory_conditions": ["INVALID"]}
         )
 
-        self.assertEqual(
-            self.user.responseset_set.first().respiratory_conditions,
-            None
-        )
+        response_set = self.user.responseset_set.first()
+        if response_set:
+            self.assertFalse(RespiratoryConditionsResponse.objects.filter(response_set=response_set).exists())
diff --git a/lung_cancer_screening/questions/views/respiratory_conditions.py b/lung_cancer_screening/questions/views/respiratory_conditions.py
