Move HeightResponse to its own model

Author: Themitchell

lung_cancer_screening/questions/forms/imperial_height_form.py

@@ -1,15 +1,15 @@
 from django import forms
 
 from ...nhsuk_forms.imperial_height_field import ImperialHeightField
-from ..models.response_set import ResponseSet
+from ..models.height_response import HeightResponse
 
 
 class ImperialHeightForm(forms.ModelForm):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
-        self.fields["height_imperial"] = ImperialHeightField(
+        self.fields["imperial"] = ImperialHeightField(
             label="Height",
             required=True,
             require_all_fields=False,
@@ -18,9 +18,9 @@ def __init__(self, *args, **kwargs):
             }
         )
 
-    def clean_height(self):
+    def clean_metric(self):
         return None
 
     class Meta:
-        model = ResponseSet
-        fields = ['height_imperial', 'height_metric']
+        model = HeightResponse
+        fields = ['imperial', 'metric']

lung_cancer_screening/questions/forms/metric_height_form.py

@@ -1,7 +1,7 @@
 from django import forms
 
 from ...nhsuk_forms.decimal_field import DecimalField
-from ..models.response_set import ResponseSet
+from ..models.height_response import HeightResponse
 
 
 class MetricHeightForm(forms.ModelForm):
@@ -10,10 +10,10 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
         # Convert mm to cm for display
-        if self.instance and self.instance.height_metric is not None:
-            self.initial['height_metric'] = self.instance.height_metric / 10
+        if self.instance and self.instance.metric is not None:
+            self.initial['metric'] = self.instance.metric / 10
 
-        self.fields["height_metric"] = DecimalField(
+        self.fields["metric"] = DecimalField(
             decimal_places=1,
             label="Centimetres",
             classes="nhsuk-input--width-4",
@@ -27,12 +27,12 @@ def __init__(self, *args, **kwargs):
             suffix="cm"
         )
 
-    def clean_height_metric(self):
-        return self.cleaned_data['height_metric'] * 10
+    def clean_metric(self):
+        return int(self.cleaned_data['metric'] * 10)
 
-    def clean_height_imperial(self):
+    def clean_imperial(self):
         return None
 
     class Meta:
-        model = ResponseSet
-        fields = ['height_metric', 'height_imperial']
+        model = HeightResponse
+        fields = ['metric', 'imperial']

lung_cancer_screening/questions/jinja2/height.jinja

@@ -38,9 +38,9 @@
         %}
 
         {% if unit == "imperial" %}
-          {{ form.height_imperial.as_field_group() }}
+          {{ form.imperial.as_field_group() }}
         {% else %}
-          {{ form.height_metric.as_field_group() }}
+          {{ form.metric.as_field_group() }}
         {% endif %}
 
 

lung_cancer_screening/questions/migrations/0030_heightresponse.py

@@ -0,0 +1,60 @@
+# Generated manually to create HeightResponse and copy data
+
+import django.core.validators
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+def copy_height_data(apps, schema_editor):
+    with schema_editor.connection.cursor() as cursor:
+        cursor.execute("""
+            INSERT INTO questions_heightresponse (created_at, updated_at, metric, imperial, response_set_id)
+            SELECT NOW(), NOW(), height_metric, height_imperial, id
+            FROM questions_responseset
+            WHERE height_metric IS NOT NULL OR height_imperial IS NOT NULL
+            ON CONFLICT (response_set_id) DO NOTHING
+        """)
+
+
+def reverse_copy_height_data(apps, schema_editor):
+    with schema_editor.connection.cursor() as cursor:
+        cursor.execute("""
+            UPDATE questions_responseset rs
+            SET height_metric = h.metric,
+                height_imperial = h.imperial
+            FROM questions_heightresponse h
+            WHERE rs.id = h.response_set_id
+        """)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('questions', '0029_genderresponse'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='HeightResponse',
+            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)),
+                ('metric', models.PositiveIntegerField(blank=True, null=True, validators=[django.core.validators.MinValueValidator(1397, message='Height must be between 139.7cm and 243.8 cm'), django.core.validators.MaxValueValidator(2438, message='Height must be between 139.7cm and 243.8 cm')])),
+                ('imperial', models.PositiveIntegerField(blank=True, null=True, validators=[django.core.validators.MinValueValidator(55, message='Height must be between 4 feet 7 inches and 8 feet'), django.core.validators.MaxValueValidator(96, message='Height must be between 4 feet 7 inches and 8 feet')])),
+                ('response_set', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='height_response', to='questions.responseset')),
+            ],
+            options={
+                'abstract': False,
+            },
+        ),
+        migrations.RunPython(copy_height_data, reverse_copy_height_data),
+        migrations.RemoveField(
+            model_name='responseset',
+            name='height_metric',
+        ),
+        migrations.RemoveField(
+            model_name='responseset',
+            name='height_imperial',
+        ),
+    ]

lung_cancer_screening/questions/models/__init__.py

@@ -5,3 +5,4 @@
 from .date_of_birth_response import DateOfBirthResponse # noqa: F401
 from .ethnicity_response import EthnicityResponse # noqa: F401
 from .gender_response import GenderResponse # noqa: F401
+from .height_response import HeightResponse # noqa: F401

lung_cancer_screening/questions/models/height_response.py

@@ -0,0 +1,40 @@
+from django.db import models
+from django.core.exceptions import ValidationError
+from django.core.validators import MaxValueValidator, MinValueValidator
+from decimal import Decimal
+
+from .base import BaseModel
+from .response_set import ResponseSet
+
+
+class HeightResponse(BaseModel):
+    MAX_HEIGHT_METRIC = 2438
+    MIN_HEIGHT_METRIC = 1397
+    MAX_HEIGHT_IMPERIAL = 96
+    MIN_HEIGHT_IMPERIAL = 55
+
+    response_set = models.OneToOneField(ResponseSet, on_delete=models.CASCADE, related_name='height_response')
+
+    metric = models.PositiveIntegerField(null=True, blank=True, validators=[
+        MinValueValidator(MIN_HEIGHT_METRIC, message="Height must be between 139.7cm and 243.8 cm"),
+        MaxValueValidator(MAX_HEIGHT_METRIC, message="Height must be between 139.7cm and 243.8 cm"),
+    ])
+    imperial = models.PositiveIntegerField(null=True, blank=True, validators=[
+        MinValueValidator(MIN_HEIGHT_IMPERIAL, message="Height must be between 4 feet 7 inches and 8 feet"),
+        MaxValueValidator(MAX_HEIGHT_IMPERIAL, message="Height must be between 4 feet 7 inches and 8 feet"),
+    ])
+
+    def clean(self):
+        if not self.metric and not self.imperial:
+            raise ValidationError("Either metric or imperial height must be provided.")
+        if self.metric and self.imperial:
+            raise ValidationError("Cannot provide both metric and imperial height.")
+
+    @property
+    def formatted(self):
+        if self.metric:
+            return f"{Decimal(self.metric) / 10}cm"
+        elif self.imperial:
+            value = Decimal(self.imperial)
+            return f"{value // 12} feet {value % 12} inches"
+        return None

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

@@ -1,158 +1,161 @@
 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.height_response import HeightResponse
 from ....forms.imperial_height_form import ImperialHeightForm
 
 
 class TestImperialHeightForm(TestCase):
     def setUp(self):
-        self.user = UserFactory()
-        self.response_set = ResponseSet(user=self.user)
-        self.response_set.height_metric = 1704
+        self.response_set = ResponseSetFactory()
+        self.response = HeightResponse.objects.create(
+            response_set=self.response_set,
+            metric=1704
+        )
 
     def test_is_valid_with_valid_input(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "height_imperial_0": "5",  # feet
-                "height_imperial_1": "9"   # inches
+                "imperial_0": "5",  # feet
+                "imperial_1": "9"   # inches
             }
         )
 
         self.assertTrue(form.is_valid())
 
     def test_converts_feet_and_inches_to_an_inches_integer(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "height_imperial_0": "5",  # feet
-                "height_imperial_1": "9"   # inches
+                "imperial_0": "5",  # feet
+                "imperial_1": "9"   # inches
             }
         )
 
         self.assertTrue(form.is_valid(), f"Form errors: {form.errors}")
-        self.assertEqual(form.cleaned_data['height_imperial'], 69)
+        self.assertEqual(form.cleaned_data['imperial'], 69)
 
     def test_setting_imperial_height_clears_height(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "height_imperial_0": "5",  # feet
-                "height_imperial_1": "9"   # inches
+                "imperial_0": "5",  # feet
+                "imperial_1": "9"   # inches
             }
         )
         form.save()
-        self.assertEqual(self.response_set.height_metric, None)
+        self.response.refresh_from_db()
+        self.assertEqual(self.response.metric, None)
 
     def test_is_invalid_with_missing_data(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
                 # missing feet
                 # missing inches
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["height_imperial"],
+            form.errors["imperial"],
             ["Enter your height"]
         )
 
     def test_is_invalid_with_missing_inches(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "height_imperial_0": "5",
+                "imperial_0": "5",
                 # missing inches
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["height_imperial"],
+            form.errors["imperial"],
             ["Inches must be between 0 and 11"]
         )
 
     def test_is_invalid_with_missing_feet(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                #"height_imperial_0": "5",
-                "height_imperial_1": "5"
+                #"imperial_0": "5",
+                "imperial_1": "5"
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["height_imperial"],
+            form.errors["imperial"],
             ["Feet must be between 4 and 8"]
         )
 
     def test_is_invalid_when_given_a_decimal_feet_value(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "height_imperial_0": "5.2",
-                "height_imperial_1": "0"
+                "imperial_0": "5.2",
+                "imperial_1": "0"
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["height_imperial"],
+            form.errors["imperial"],
             ["Feet must be in whole numbers"]
         )
 
-    def test_is_invalid_when_inches_under_11(self):
+    def test_is_invalid_when_inches_over_11(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "height_imperial_0": "5",
-                "height_imperial_1": "12"
+                "imperial_0": "5",
+                "imperial_1": "12"
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["height_imperial"],
+            form.errors["imperial"],
             ["Inches must be between 0 and 11"]
         )
 
-    def test_is_invalid_when_inches_over_0(self):
+    def test_is_invalid_when_inches_under_0(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "height_imperial_0": "5",
-                "height_imperial_1": "-1"
+                "imperial_0": "5",
+                "imperial_1": "-1"
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["height_imperial"],
+            form.errors["imperial"],
             ["Inches must be between 0 and 11"]
         )
 
     def test_is_invalid_when_feet_over_4(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "height_imperial_0": "3",
-                "height_imperial_1": "10"
+                "imperial_0": "3",
+                "imperial_1": "10"
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["height_imperial"],
+            form.errors["imperial"],
             ["Feet must be between 4 and 8"]
         )
 
     def test_is_invalid_when_feet_under_8(self):
         form = ImperialHeightForm(
-            instance=self.response_set,
+            instance=self.response,
             data={
-                "height_imperial_0": "9",
-                "height_imperial_1": "0"
+                "imperial_0": "9",
+                "imperial_1": "0"
             }
         )
         self.assertFalse(form.is_valid())
         self.assertEqual(
-            form.errors["height_imperial"],
+            form.errors["imperial"],
             ["Feet must be between 4 and 8"]
         )