PPHA-262: Add imperial height form

Themitchell · Themitchell · commit f205d7569245 · 2025-10-20T09:34:13.000+01:00
diff --git a/lung_cancer_screening/core/form_fields.py b/lung_cancer_screening/core/form_fields.py
@@ -369,3 +369,61 @@ def __init__(
         self.label_classes = label_classes
 
         super().__init__(*args, **kwargs)
+
+
+class ImperialHeightWidget(widgets.MultiWidget):
+    """
+    A widget that splits height into feet and inches inputs.
+    """
+
+    def __init__(self, attrs=None):
+        height_widgets = (
+            widgets.NumberInput(attrs=attrs),
+            widgets.NumberInput(attrs=attrs),
+        )
+        super().__init__(height_widgets, attrs)
+
+    def decompress(self, value):
+        """
+        Convert total inches back to feet and inches for display.
+        """
+        if value:
+            feet = int(value // 12)
+            inches = value % 12
+            return [feet, inches]
+        return [None, None]
+
+    def subwidgets(self, name, value, attrs=None):
+        """
+        Expose data for each subwidget, so that we can render them separately in the template.
+        """
+        context = self.get_context(name, value, attrs)
+        for subwidget in context["widget"]["subwidgets"]:
+            yield subwidget
+
+
+class ImperialHeightField(forms.MultiValueField):
+    """
+    A field that combines feet and inches into a single height value in cm.
+    """
+
+    widget = ImperialHeightWidget
+
+    def __init__(self, *args, **kwargs):
+        fields = (
+            IntegerField(label="Feet"),
+            IntegerField(label="Inches"),
+        )
+        kwargs["template_name"] = "forms/imperial-height-input.jinja"
+
+        super().__init__(fields, *args, **kwargs)
+
+    def compress(self, data_list):
+        """
+        Convert feet and inches to total inches.
+        """
+        if data_list and all(data_list):
+            feet, inches = data_list
+            total_inches = feet * 12 + inches
+            return int(total_inches)
+        return None
diff --git a/lung_cancer_screening/core/jinja2/forms/imperial-height-input.jinja b/lung_cancer_screening/core/jinja2/forms/imperial-height-input.jinja
@@ -0,0 +1,38 @@
+{% from 'nhsuk/components/input/macro.jinja' import input %}
+
+{% if field.errors %}
+  {% set error_message = {"text": field.errors | first} %}
+{% endif %}
+{% set unbound_field = field.field %}
+{% set hint = unbound_field.hint %}
+
+{{ input({
+  "label": {
+    "text": "Feet",
+    "classes": "nhsuk-fieldset__legend--m",
+    "isPageHeading": false
+  },
+  "hint": {
+    "text": unbound_field.hint
+  } if unbound_field.hint,
+  "errorMessage": error_message,
+  "id": field.auto_id,
+  "name": field.html_name + "_0",
+  "value": field.subwidgets.0.data.value,
+  "classes": "nhsuk-input--width-2",
+  "type": "number"
+}) }}
+
+{{ input({
+  "label": {
+    "text": "Inches",
+    "classes": "nhsuk-fieldset__legend--m",
+    "isPageHeading": false
+  },
+  "errorMessage": error_message,
+  "id": field.auto_id + "_1",
+  "name": field.html_name + "_1",
+  "value": field.subwidgets.1.data.value,
+  "classes": "nhsuk-input--width-2",
+  "type": "number"
+}) }}
diff --git a/lung_cancer_screening/core/tests/acceptance/test_cannot_change_answers_after_submission.py b/lung_cancer_screening/core/tests/acceptance/test_cannot_change_answers_after_submission.py
@@ -6,7 +6,6 @@
 
 from .helpers.user_interaction_helpers import (
     fill_in_and_submit_height_metric,
-    fill_in_and_submit_height_imperial,
     fill_in_and_submit_participant_id,
     fill_in_and_submit_smoking_eligibility,
     fill_in_and_submit_date_of_birth
diff --git a/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py b/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py
@@ -70,7 +70,7 @@ def test_full_questionnaire_user_journey(self):
         expect(responses).to_contain_text("Have you ever smoked? Yes, I used to smoke regularly")
         expect(responses).to_contain_text(
             age.strftime("What is your date of birth? %Y-%m-%d"))
-        expect(responses).to_contain_text(f"What is your height? {height}")
+        expect(responses).to_contain_text(f"What is your height? {feet} feet {inches} inches")
 
         page.click("text=Submit")
 
diff --git a/lung_cancer_screening/core/tests/acceptance/test_questionnaire_validation_errors.py b/lung_cancer_screening/core/tests/acceptance/test_questionnaire_validation_errors.py
@@ -37,3 +37,26 @@ def test_date_of_birth_validation_errors(self):
         expect(page.locator(".nhsuk-error-message")).to_contain_text(
             "Enter your date of birth."
         )
+
+    def test_height_validation_errors(self):
+        participant_id = '123'
+
+        page = self.browser.new_page()
+        page.goto(f"{self.live_server_url}/start")
+        fill_in_and_submit_participant_id(page, participant_id)
+        page.goto(f"{self.live_server_url}/height")
+
+        page.click("text=Continue")
+
+        expect(page.locator(".nhsuk-error-message")).to_contain_text(
+            "Enter your height."
+        )
+
+        page.click("text=Switch to imperial")
+
+        page.click("text=Continue")
+
+        for error in page.locator(".nhsuk-error-message"):
+            expect(error).to_contain_text(
+                "Enter your height."
+            )
diff --git a/lung_cancer_screening/questions/forms/imperial_height_form.py b/lung_cancer_screening/questions/forms/imperial_height_form.py
@@ -1,30 +1,23 @@
-import decimal
 from django import forms
 
-from lung_cancer_screening.core.form_fields import DecimalField
+from lung_cancer_screening.core.form_fields import ImperialHeightField
 from ..models.response_set import ResponseSet
 
-class MetricHeightForm(forms.ModelForm):
+class ImperialHeightForm(forms.ModelForm):
 
     def __init__(self, *args, **kwargs):
         self.participant = kwargs.pop('participant')
         super().__init__(*args, **kwargs)
         self.instance.participant = self.participant
 
-        self.fields["height_feet"] = DecimalField(
-            label="Feet",
-            classes="nhsuk-input--width-4",
-        )
-
-        self.fields["height_inches"] = DecimalField(
-            label="Inches",
-            classes="nhsuk-input--width-4",
+        self.fields["height_imperial"] = ImperialHeightField(
+            label="Height",
+            required=True
         )
 
     def clean_height(self):
-      data = self.cleaned_data['height_feet']*12 + self.cleaned_data['height_inches']
-      return data*2.54
+        return None
 
     class Meta:
         model = ResponseSet
-        fields = ['height']
+        fields = ['height_imperial', 'height']
diff --git a/lung_cancer_screening/questions/forms/metric_height_form.py b/lung_cancer_screening/questions/forms/metric_height_form.py
@@ -1,4 +1,3 @@
-import decimal
 from django import forms
 
 from lung_cancer_screening.core.form_fields import DecimalField
@@ -14,11 +13,17 @@ def __init__(self, *args, **kwargs):
         self.fields["height"] = DecimalField(
             label="Centimetres",
             classes="nhsuk-input--width-4",
+            error_messages={
+                'required': 'Enter your height.',
+            }
         )
 
     def clean_height(self):
         return self.cleaned_data['height'] * 10
 
+    def clean_height_imperial(self):
+        return None
+
     class Meta:
         model = ResponseSet
-        fields = ['height']
+        fields = ['height', 'height_imperial']
diff --git a/lung_cancer_screening/questions/jinja2/height.jinja b/lung_cancer_screening/questions/jinja2/height.jinja
@@ -15,7 +15,12 @@
 {% block page_content %}
   <div class="nhsuk-grid-row">
     <div class="nhsuk-grid-column-two-thirds">
-      <form action="{{ request.path }}" method="POST" novalidate>
+      {% if unit %}
+        {% set action_url = request.path + '?unit=' + unit %}
+      {% else %}
+        {% set action_url = request.path %}
+      {% endif %}
+      <form action="{{ action_url }}" method="POST" novalidate>
         {{ csrf_input }}
         <h1 class="nhsuk-heading-l">What is your height?</h1>
         <p>An accurate measurement is important.
@@ -28,9 +33,13 @@
           }
         }) %}
 
-        {{ form.height.as_field_group() }}
-        
-        <a href="?unit={{ switch_to_unit }}">Switch to {{ switch_to_unit }}</a>
+          {% if unit == "imperial" %}
+            {{ form.height_imperial.as_field_group() }}
+          {% else %}
+            {{ form.height.as_field_group() }}
+          {% endif %}
+
+          <a href="?unit={{ switch_to_unit }}">Switch to {{ switch_to_unit }}</a>
 
         {% endcall %}
 
@@ -40,4 +49,4 @@
       </form>
     </div>
   </div>
-{% endblock %}
+{% endblock %}
diff --git a/lung_cancer_screening/questions/jinja2/responses.jinja b/lung_cancer_screening/questions/jinja2/responses.jinja
@@ -18,7 +18,7 @@
       <ul class="responses">
         <li>Have you ever smoked? {{ response_set.get_have_you_ever_smoked_display() }}</li>
         <li>What is your date of birth? {{ response_set.date_of_birth }}</li>
-        <li>What is your height? {{ response_set.height // 10 if response_set.height else ""}}</li>
+        <li>What is your height? {{ response_set.formatted_height }}</li>
       </ul>
 
       <form action="{{ request.path }}" method="post">
diff --git a/lung_cancer_screening/questions/migrations/0010_responseset_height_imperial_alter_responseset_height_and_more.py b/lung_cancer_screening/questions/migrations/0010_responseset_height_imperial_alter_responseset_height_and_more.py
@@ -0,0 +1,28 @@
+# Generated by Django 5.2.7 on 2025-10-17 13:52
+
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('questions', '0009_responseset_height_and_more'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='responseset',
+            name='height_imperial',
+            field=models.PositiveIntegerField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='responseset',
+            name='height',
+            field=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')]),
+        ),
+        migrations.AddConstraint(
+            model_name='responseset',
+            constraint=models.UniqueConstraint(condition=models.Q(('submitted_at__isnull', True)), fields=('participant',), name='unique_unsubmitted_response_per_participant', violation_error_message='An unsubmitted response set already exists for this participant'),
+        ),
+    ]
diff --git a/lung_cancer_screening/questions/models/response_set.py b/lung_cancer_screening/questions/models/response_set.py
@@ -3,6 +3,7 @@
 from django.core.validators import MaxValueValidator, MinValueValidator
 from dateutil.relativedelta import relativedelta
 from django.utils import timezone
+from decimal import Decimal
 
 from .base import BaseModel
 from .participant import Participant
@@ -23,12 +24,20 @@ class ResponseSet(BaseModel):
     )
     date_of_birth = models.DateField(null=True, blank=True)
 
+    MAX_HEIGHT_METRIC = 2438
+    MIN_HEIGHT_METRIC = 1397
+    MAX_HEIGHT_IMPERIAL = 96
+    MIN_HEIGHT_IMPERIAL = 55
     height = models.PositiveIntegerField(null=True, blank=True, validators=[
-        MinValueValidator(1397, message="Height must be between 139.7cm and 243.8 cm"),
-        MaxValueValidator(2438, message="Height must be between 139.7cm and 243.8 cm"),
+        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"),
+    ])
+    height_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"),
     ])
-
-    #height_type
 
     submitted_at = models.DateTimeField(null=True, blank=True)
 
@@ -52,3 +61,12 @@ def clean(self):
             raise ValidationError(
                 "Responses have already been submitted for this participant"
             )
+
+    @property
+    def formatted_height(self):
+        if self.height:
+            return f"{Decimal(self.height) / 10}cm"
+        elif self.height_imperial:
+            value = Decimal(self.height_imperial)
+            return f"{value // 12} feet {value % 12} inches"
+
diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_imperial_height_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_imperial_height_form.py
@@ -0,0 +1,60 @@
+from django.test import TestCase
+
+from ....models.participant import Participant
+from ....forms.imperial_height_form import ImperialHeightForm
+
+
+class TestImperialHeightForm(TestCase):
+    def setUp(self):
+        self.participant = Participant.objects.create(unique_id="1234567890")
+        self.response_set = self.participant.responseset_set.create(
+            height=1704
+        )
+
+    def test_is_valid_with_valid_input(self):
+        form = ImperialHeightForm(
+            participant=self.participant,
+            instance=self.response_set,
+            data={
+                "height_imperial_0": "5",  # feet
+                "height_imperial_1": "9"   # inches
+            }
+        )
+
+        self.assertTrue(form.is_valid())
+
+    def test_converts_feet_and_inches_to_an_inches_integer(self):
+        form = ImperialHeightForm(
+            participant=self.participant,
+            instance=self.response_set,
+            data={
+                "height_imperial_0": "5",  # feet
+                "height_imperial_1": "9"   # inches
+            }
+        )
+
+        self.assertTrue(form.is_valid(), f"Form errors: {form.errors}")
+        self.assertEqual(form.cleaned_data['height_imperial'], 69)
+
+    def test_setting_imperial_height_clears_height(self):
+        form = ImperialHeightForm(
+            instance=self.response_set,
+            participant=self.participant,
+            data={
+                "height_imperial_0": "5",  # feet
+                "height_imperial_1": "9"   # inches
+            }
+        )
+        form.save()
+        self.assertEqual(self.response_set.height, None)
+
+    def test_is_invalid_with_missing_data(self):
+        form = ImperialHeightForm(
+            participant=self.participant,
+            instance=self.response_set,
+            data={
+                "height_imperial_0": "5",
+                # missing inches
+            }
+        )
+        self.assertFalse(form.is_valid())
diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_metric_height_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_metric_height_form.py
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
diff --git a/lung_cancer_screening/questions/tests/unit/views/test_height.py b/lung_cancer_screening/questions/tests/unit/views/test_height.py
diff --git a/lung_cancer_screening/questions/views/height.py b/lung_cancer_screening/questions/views/height.py
diff --git a/scripts/tests/unit.sh b/scripts/tests/unit.sh
