diff --git a/.gitignore b/.gitignore index f388cc4a..e88632d2 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ __pycache__ lung_cancer_screening/assets/compiled/* !lung_cancer_screening/assets/compiled/.gitkeep .DS_Store +.venv diff --git a/.tool-versions b/.tool-versions index a3d8f27c..f73cf3b5 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,5 +1,6 @@ # This file is for you! Please, updated to the versions agreed by your team. +python 3.13.8 terraform 1.7.0 pre-commit 4.3.0 vale 3.6.0 diff --git a/.vscode/settings.json b/.vscode/settings.json index 423458c1..246a6ee8 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,5 +3,23 @@ "MD013": false, "MD024": { "siblings_only": true }, "MD033": false - } + }, + "cSpell.words": [ + "dateutil", + "endcall", + "endset", + "fieldset", + "gunicorn", + "jinja", + "makemigrations", + "nhsuk", + "novalidate", + "psycopg", + "relativedelta", + "responseset", + "stylesheet", + "toplevel", + "unsubmitted", + "whitenoise" + ] } diff --git a/Dockerfile b/Dockerfile index a66f5c21..6accfb0e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ FROM node:24.10.0-alpine3.21 AS asset_builder WORKDIR /app COPY package.json package-lock.json rollup.config.js ./ -COPY lung_cancer_screening ./lung_cancer_screening +COPY . . RUN npm ci RUN npm run compile diff --git a/docker-compose.yml b/docker-compose.yml index c3b15b37..2764ce11 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -13,6 +13,7 @@ services: - .env depends_on: - db + - asset_builder volumes: - ./:/app - venv:/app/.venv @@ -26,7 +27,8 @@ services: target: asset_builder command: npm run watch volumes: - - ./lung_cancer_screening/assets/compiled:/app/lung_cancer_screening/assets/compiled + - ./:/app + - node_modules:/app/node_modules db: image: postgres:15-alpine @@ -48,3 +50,4 @@ volumes: redis_data: venv: playwright_browsers: + node_modules: diff --git a/lung_cancer_screening/assets/sass/components/_multi_field_input.scss b/lung_cancer_screening/assets/sass/components/_multi_field_input.scss new file mode 100644 index 00000000..7705da0a --- /dev/null +++ b/lung_cancer_screening/assets/sass/components/_multi_field_input.scss @@ -0,0 +1,5 @@ +.multi-field-input__item { + display: inline-block; + margin-bottom: 0; + margin-right: 24px; +} diff --git a/lung_cancer_screening/assets/sass/main.scss b/lung_cancer_screening/assets/sass/main.scss index 26ef4fee..5efa3de7 100644 --- a/lung_cancer_screening/assets/sass/main.scss +++ b/lung_cancer_screening/assets/sass/main.scss @@ -2,4 +2,4 @@ @forward "nhsuk-frontend/dist/nhsuk"; // Components that are not in the NHS.UK frontend library -// @import "components/button"; +@forward "components/multi_field_input"; diff --git a/lung_cancer_screening/core/form_fields.py b/lung_cancer_screening/core/form_fields.py index 2b87c930..221a9294 100644 --- a/lung_cancer_screening/core/form_fields.py +++ b/lung_cancer_screening/core/form_fields.py @@ -207,6 +207,33 @@ def widget_attrs(self, widget): return attrs +class DecimalField(forms.DecimalField): + def __init__( + self, + *args, + hint=None, + label_classes=None, + classes=None, + **kwargs, + ): + kwargs["template_name"] = "forms/input.jinja" + + self.hint = hint + self.classes = classes + self.label_classes = label_classes + + super().__init__(*args, **kwargs) + + def widget_attrs(self, widget): + attrs = super().widget_attrs(widget) + + # Don't use min/max/step attributes. + attrs.pop("min", None) + attrs.pop("max", None) + attrs.pop("step", None) + + return attrs + class BoundChoiceField(forms.BoundField): """ @@ -225,7 +252,7 @@ def __init__(self, form: forms.Form, field: "ChoiceField", name: str): def add_conditional_html(self, value, html): if isinstance(self.field.widget, widgets.Select): raise ValueError( - "select comonent does not support conditional fields") + "select component does not support conditional fields") self._conditional_html[value] = html @@ -342,3 +369,75 @@ 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): + error_messages = kwargs.get("error_messages", {}) + + feet_kwargs = { + "error_messages": { + 'invalid': 'Feet must be in whole numbers.', + **error_messages, + }, + } + inches_kwargs = { + "error_messages": { + 'invalid': 'Inches must be in whole numbers.', + **error_messages, + }, + } + fields = ( + IntegerField(**feet_kwargs), + IntegerField(**inches_kwargs), + ) + 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 new file mode 100644 index 00000000..76b983dd --- /dev/null +++ b/lung_cancer_screening/core/jinja2/forms/imperial-height-input.jinja @@ -0,0 +1,52 @@ +{% from 'nhsuk/components/input/macro.jinja' import input %} +{% from 'nhsuk/components/error-message/macro.jinja' import errorMessage %} + +{% if field.errors | length > 0 %} + {% set error_message = field.errors | first %} +{% endif %} +{% set unbound_field = field.field %} +{% set hint = unbound_field.hint %} +{% set form_group_error_classes = ' nhsuk-form-group--error' if error_message else '' %} +{% set field_error_classes = ' nhsuk-input--error' if error_message else '' %} + +
+ {% if error_message %} + {{ errorMessage({ + "id": field.auto_id, + "text": error_message + }) }} + {% endif %} + +
+ {{ input({ + "label": { + "text": "Feet", + "classes": "nhsuk-fieldset__legend--m", + "isPageHeading": false + }, + "hint": { + "text": unbound_field.hint + } if unbound_field.hint, + "id": field.auto_id + "_0", + "name": field.html_name + "_0", + "value": field.subwidgets.0.data.value, + "classes": "nhsuk-input--width-2" + field_error_classes, + "type": "number" + }) }} +
+ +
+ {{ input({ + "label": { + "text": "Inches", + "classes": "nhsuk-fieldset__legend--m", + "isPageHeading": false + }, + "id": field.auto_id + "_1", + "name": field.html_name + "_1", + "value": field.subwidgets.1.data.value, + "classes": "nhsuk-input--width-2" + field_error_classes, + "type": "number" + }) }} +
+
diff --git a/lung_cancer_screening/core/jinja2/forms/input.jinja b/lung_cancer_screening/core/jinja2/forms/input.jinja new file mode 100644 index 00000000..b1f8e9fc --- /dev/null +++ b/lung_cancer_screening/core/jinja2/forms/input.jinja @@ -0,0 +1,34 @@ +{% from "nhsuk/components/input/macro.jinja" import input %} +{% set unbound_field = field.field %} +{% set widget = unbound_field.widget %} +{% if unbound_field.visually_hidden_label_suffix %} + {% set label_html %} + {{ field.label }}: {{ unbound_field.visually_hidden_label_suffix }} + {% endset %} +{% endif %} +{% set input_params = { + "label": { + "text": field.label, + "html": label_html if label_html, + "classes": unbound_field.label_classes if unbound_field.label_classes + }, + "hint": { + "text": unbound_field.hint + } if unbound_field.hint, + "id": field.auto_id, + "name": field.html_name, + "value": field.value() or "", + "classes": unbound_field.classes if unbound_field.classes, + "attributes": widget.attrs, + "type": widget.input_type + } %} + {% if field.errors %} + {% set error_params = { + "errorMessage": { + "text": field.errors | first + } + } %} + {% set input_params = dict(input_params, **error_params) %} + {% endif %} + +{{ input(input_params) }} diff --git a/lung_cancer_screening/core/tests/acceptance/helpers/user_interaction_helpers.py b/lung_cancer_screening/core/tests/acceptance/helpers/user_interaction_helpers.py index a50a633e..10322a40 100644 --- a/lung_cancer_screening/core/tests/acceptance/helpers/user_interaction_helpers.py +++ b/lung_cancer_screening/core/tests/acceptance/helpers/user_interaction_helpers.py @@ -5,7 +5,7 @@ def fill_in_and_submit_participant_id(page, participant_id): page.click('text=Start now') -def fill_in_and_submit_smoking_elligibility(page, smoking_status): +def fill_in_and_submit_smoking_eligibility(page, smoking_status): expect(page.locator("legend")).to_have_text( "Have you ever smoked?") @@ -22,3 +22,18 @@ def fill_in_and_submit_date_of_birth(page, age): page.get_by_label("Year").fill(str(age.year)) page.click("text=Continue") + +def fill_in_and_submit_height_metric(page, height): + expect(page.locator("h1")).to_have_text("What is your height?") + + page.get_by_label("Centimetre").fill(str(height)) + + page.click("text=Continue") + +def fill_in_and_submit_height_imperial(page, feet, inches): + expect(page.locator("h1")).to_have_text("What is your height?") + + page.get_by_label("Feet").fill(str(feet)) + page.get_by_label("Inches").fill(str(inches)) + + page.click("text=Continue") 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 index e1db0355..48e605c2 100644 --- 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 @@ -5,8 +5,9 @@ from dateutil.relativedelta import relativedelta from .helpers.user_interaction_helpers import ( + fill_in_and_submit_height_metric, fill_in_and_submit_participant_id, - fill_in_and_submit_smoking_elligibility, + fill_in_and_submit_smoking_eligibility, fill_in_and_submit_date_of_birth ) @@ -35,8 +36,9 @@ def test_cannot_change_responses_once_checked_and_submitted(self): page.goto(f"{self.live_server_url}/start") fill_in_and_submit_participant_id(page, participant_id) - fill_in_and_submit_smoking_elligibility(page, smoking_status) + fill_in_and_submit_smoking_eligibility(page, smoking_status) fill_in_and_submit_date_of_birth(page, age) + fill_in_and_submit_height_metric(page, "170") page.click("text=Submit") diff --git a/lung_cancer_screening/core/tests/acceptance/test_participant_not_smoker.py b/lung_cancer_screening/core/tests/acceptance/test_participant_not_smoker.py index 8b733c2b..a352d49e 100644 --- a/lung_cancer_screening/core/tests/acceptance/test_participant_not_smoker.py +++ b/lung_cancer_screening/core/tests/acceptance/test_participant_not_smoker.py @@ -5,7 +5,7 @@ from .helpers.user_interaction_helpers import ( fill_in_and_submit_participant_id, - fill_in_and_submit_smoking_elligibility + fill_in_and_submit_smoking_eligibility ) @@ -31,7 +31,7 @@ def test_participant_not_smoker(self): page.goto(f"{self.live_server_url}/start") fill_in_and_submit_participant_id(page, participant_id) - fill_in_and_submit_smoking_elligibility(page, 'No, I have never smoked') + fill_in_and_submit_smoking_eligibility(page, 'No, I have never smoked') expect(page).to_have_url(f"{self.live_server_url}/non-smoker-exit") diff --git a/lung_cancer_screening/core/tests/acceptance/test_participant_out_of_age_range.py b/lung_cancer_screening/core/tests/acceptance/test_participant_out_of_age_range.py index 2c9b2014..e146d60f 100644 --- a/lung_cancer_screening/core/tests/acceptance/test_participant_out_of_age_range.py +++ b/lung_cancer_screening/core/tests/acceptance/test_participant_out_of_age_range.py @@ -6,7 +6,7 @@ from .helpers.user_interaction_helpers import ( fill_in_and_submit_participant_id, - fill_in_and_submit_smoking_elligibility, + fill_in_and_submit_smoking_eligibility, fill_in_and_submit_date_of_birth ) @@ -33,7 +33,7 @@ def test_participant_out_of_age_range(self): page.goto(f"{self.live_server_url}/start") fill_in_and_submit_participant_id(page, participant_id) - fill_in_and_submit_smoking_elligibility(page, 'Yes, I used to smoke regularly') + fill_in_and_submit_smoking_eligibility(page, 'Yes, I used to smoke regularly') age = datetime.now() - relativedelta(years=20) fill_in_and_submit_date_of_birth(page, age) diff --git a/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py b/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py index 5bb9f140..88a7c426 100644 --- a/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py +++ b/lung_cancer_screening/core/tests/acceptance/test_questionnaire.py @@ -5,8 +5,10 @@ from dateutil.relativedelta import relativedelta from .helpers.user_interaction_helpers import ( + fill_in_and_submit_height_imperial, + fill_in_and_submit_height_metric, fill_in_and_submit_participant_id, - fill_in_and_submit_smoking_elligibility, + fill_in_and_submit_smoking_eligibility, fill_in_and_submit_date_of_birth ) @@ -27,10 +29,13 @@ def tearDownClass(cls): cls.browser.close() cls.playwright.stop() - def test_full_questionaire_user_journey(self): + def test_full_questionnaire_user_journey(self): participant_id = '123' smoking_status = 'Yes, I used to smoke regularly' age = datetime.now() - relativedelta(years=55) + height = "170" + feet = 5 + inches = 7 page = self.browser.new_page() page.goto(f"{self.live_server_url}/start") @@ -40,19 +45,35 @@ def test_full_questionaire_user_journey(self): expect(page).to_have_url( f"{self.live_server_url}/have-you-ever-smoked") - fill_in_and_submit_smoking_elligibility(page, smoking_status) + fill_in_and_submit_smoking_eligibility(page, smoking_status) expect(page).to_have_url(f"{self.live_server_url}/date-of-birth") expect_back_link_to_have_url(page, "/have-you-ever-smoked") fill_in_and_submit_date_of_birth(page, age) + expect(page).to_have_url(f"{self.live_server_url}/height") + + fill_in_and_submit_height_metric(page, height) + expect(page).to_have_url(f"{self.live_server_url}/responses") - expect(page.locator(".responses")).to_contain_text( - age.strftime("Have you ever smoked? Yes, I used to smoke regularly")) - expect(page.locator(".responses")).to_contain_text(age.strftime("What is your date of birth? %Y-%m-%d")) + page.click("text=Back") + + expect(page).to_have_url(f"{self.live_server_url}/height") + + page.click("text=Switch to imperial") + + fill_in_and_submit_height_imperial(page, feet, inches) + + responses = page.locator(".responses") + 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? {feet} feet {inches} inches") page.click("text=Submit") + + expect(page).to_have_url(f"{self.live_server_url}/your-results") 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 index 51917285..1139578b 100644 --- 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,49 @@ 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.get_by_label("Centimetre").fill('139.6') + page.click('text=Continue') + expect(page.locator(".nhsuk-error-message")).to_contain_text( + "Height must be between 139.7cm and 243.8 cm" + ) + + page.get_by_label("Centimetre").fill('243.9') + page.click('text=Continue') + expect(page.locator(".nhsuk-error-message")).to_contain_text( + "Height must be between 139.7cm and 243.8 cm" + ) + + page.click("text=Switch to imperial") + + page.click("text=Continue") + expect(page.locator(".nhsuk-error-message")).to_contain_text( + "Enter your height." + ) + + page.get_by_label("Feet").fill('5.2') + page.get_by_label("Inches").fill('2') + page.click('text=Continue') + expect(page.locator(".nhsuk-error-message")).to_contain_text( + "Feet must be in whole numbers" + ) + + page.get_by_label("Feet").fill('5') + page.get_by_label("Inches").fill('2.2') + page.click('text=Continue') + expect(page.locator(".nhsuk-error-message")).to_contain_text( + "Inches must be in whole numbers" + ) diff --git a/lung_cancer_screening/core/tests/unit/__init__.py b/lung_cancer_screening/core/tests/unit/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/lung_cancer_screening/core/tests/unit/test_form_fields.py b/lung_cancer_screening/core/tests/unit/not_test_form_fields.py similarity index 100% rename from lung_cancer_screening/core/tests/unit/test_form_fields.py rename to lung_cancer_screening/core/tests/unit/not_test_form_fields.py diff --git a/lung_cancer_screening/questions/forms/imperial_height_form.py b/lung_cancer_screening/questions/forms/imperial_height_form.py new file mode 100644 index 00000000..d75cd293 --- /dev/null +++ b/lung_cancer_screening/questions/forms/imperial_height_form.py @@ -0,0 +1,28 @@ +from django import forms + +from lung_cancer_screening.core.form_fields import ImperialHeightField +from ..models.response_set import ResponseSet + +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_imperial"] = ImperialHeightField( + label="Height", + required=True, + require_all_fields=False, + error_messages={ + 'required': 'Enter your height.', + 'incomplete': 'Enter your height.' + } + ) + + def clean_height(self): + return None + + class Meta: + model = ResponseSet + 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 new file mode 100644 index 00000000..0ebecd9e --- /dev/null +++ b/lung_cancer_screening/questions/forms/metric_height_form.py @@ -0,0 +1,29 @@ +from django import forms + +from lung_cancer_screening.core.form_fields import DecimalField +from ..models.response_set import ResponseSet + +class MetricHeightForm(forms.ModelForm): + + def __init__(self, *args, **kwargs): + self.participant = kwargs.pop('participant') + super().__init__(*args, **kwargs) + self.instance.participant = self.participant + + 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', 'height_imperial'] diff --git a/lung_cancer_screening/questions/jinja2/height.jinja b/lung_cancer_screening/questions/jinja2/height.jinja new file mode 100644 index 00000000..2d19600b --- /dev/null +++ b/lung_cancer_screening/questions/jinja2/height.jinja @@ -0,0 +1,53 @@ +{% extends 'layout.jinja' %} +{% from 'nhsuk/components/button/macro.jinja' import button %} +{% from 'nhsuk/components/back-link/macro.jinja' import backLink %} +{% from 'nhsuk/components/fieldset/macro.jinja' import fieldset %} + +{% block beforeContent %} + {{ + backLink({ + "href": url("questions:date_of_birth"), + "text": "Back" + }) + }} +{% endblock beforeContent %} + +{% block page_content %} +
+
+ {% if unit %} + {% set action_url = request.path + '?unit=' + unit %} + {% else %} + {% set action_url = request.path %} + {% endif %} +
+ {{ csrf_input }} +

What is your height?

+

An accurate measurement is important. + +

You can measure your height at home with a measuring tape. Some pharmacies and gyms have machines to measure your height. + {% call fieldset({ + "legend": { + "text": "Enter your height", + "classes": "nhsuk-label--m" + } + }) %} + + {% if unit == "imperial" %} + {{ form.height_imperial.as_field_group() }} + {% else %} + {{ form.height.as_field_group() }} + {% endif %} + + +

Switch to {{ switch_to_unit }}

+ + {% endcall %} + + {{ button({ + "text": "Continue" + }) }} +
+
+
+{% endblock %} diff --git a/lung_cancer_screening/questions/jinja2/responses.jinja b/lung_cancer_screening/questions/jinja2/responses.jinja index abaf1a09..043842be 100644 --- a/lung_cancer_screening/questions/jinja2/responses.jinja +++ b/lung_cancer_screening/questions/jinja2/responses.jinja @@ -1,13 +1,24 @@ {% extends 'layout.jinja' %} {% from 'nhsuk/components/button/macro.jinja' import button %} +{% from 'nhsuk/components/back-link/macro.jinja' import backLink %} -{% block content %} +{% block beforeContent %} + {{ + backLink({ + "href": url("questions:height"), + "text": "Back" + }) + }} +{% endblock beforeContent %} + +{% block page_content %}

Responses

diff --git a/lung_cancer_screening/questions/migrations/0009_responseset_height_and_more.py b/lung_cancer_screening/questions/migrations/0009_responseset_height_and_more.py new file mode 100644 index 00000000..e890a04e --- /dev/null +++ b/lung_cancer_screening/questions/migrations/0009_responseset_height_and_more.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.6 on 2025-10-06 15:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('questions', '0008_remove_responseset_submitted_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='responseset', + name='height', + field=models.PositiveIntegerField(blank=True, null=True), + ), + ] 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 new file mode 100644 index 00000000..af74cddb --- /dev/null +++ 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 index cb39e944..e448c711 100644 --- a/lung_cancer_screening/questions/models/response_set.py +++ b/lung_cancer_screening/questions/models/response_set.py @@ -1,7 +1,9 @@ from django.db import models from django.core.exceptions import ValidationError +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 @@ -22,6 +24,21 @@ 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(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"), + ]) + submitted_at = models.DateTimeField(null=True, blank=True) class Meta: @@ -44,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 new file mode 100644 index 00000000..c68e6c43 --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/forms/test_imperial_height_form.py @@ -0,0 +1,79 @@ +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()) + self.assertEqual( + form.errors["height_imperial"], + ["Enter your height."] + ) + + def test_is_invalid_when_given_a_decimal_feet_value(self): + form = ImperialHeightForm( + participant=self.participant, + instance=self.response_set, + data={ + "height_imperial_0": "5.2", + "height_imperial_1": "0" + } + ) + self.assertFalse(form.is_valid()) + self.assertEqual( + form.errors["height_imperial"], + ["Feet must be in whole numbers."] + ) 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 new file mode 100644 index 00000000..d7019399 --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/forms/test_metric_height_form.py @@ -0,0 +1,58 @@ +from django.test import TestCase + +from ....models.participant import Participant +from ....forms.metric_height_form import MetricHeightForm + + +class TestMetricHeightForm(TestCase): + def setUp(self): + self.participant = Participant.objects.create(unique_id="1234567890") + self.response_set = self.participant.responseset_set.create( + height_imperial=68 + ) + + def test_is_valid_with_valid_input(self): + height = "170.4" + form = MetricHeightForm( + participant=self.participant, + instance=self.response_set, + data={ + "height": height + } + ) + self.assertTrue(form.is_valid()) + + def test_converts_cm_to_mm(self): + height = "170.4" + form = MetricHeightForm( + participant=self.participant, + instance=self.response_set, + data={ + "height": height + } + ) + + form.is_valid() + self.assertEqual(form.cleaned_data["height"], 1704) + + def test_setting_height_clears_imperial_height(self): + height = "170.4" + form = MetricHeightForm( + instance=self.response_set, + participant=self.participant, + data={ + "height": height + } + ) + form.save() + self.assertEqual(self.response_set.height_imperial, None) + + def test_is_invalid(self): + form = MetricHeightForm( + participant=self.participant, + instance=self.response_set, + data={ + "height": "invalid" + } + ) + self.assertFalse(form.is_valid()) 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 index 019d0ac3..e771301c 100644 --- a/lung_cancer_screening/questions/tests/unit/models/test_response_set.py +++ b/lung_cancer_screening/questions/tests/unit/models/test_response_set.py @@ -5,9 +5,11 @@ from django.core.exceptions import ValidationError +from ....models.response_set import ResponseSet from ....models.participant import Participant from ....models.response_set import HaveYouEverSmokedValues + class TestResponseSet(TestCase): def setUp(self): participant = Participant.objects.create(unique_id="12345") @@ -31,6 +33,24 @@ def test_has_date_of_birth_as_a_date(self): date ) + def test_has_height_as_a_int(self): + self.response_set.height = 1700 + self.response_set.save() + + self.assertIsInstance( + self.response_set.height, + int + ) + + def test_has_an_imperial_height_as_a_int(self): + self.response_set.height_imperial = 68 + self.response_set.save() + + self.assertIsInstance( + self.response_set.height_imperial, + int + ) + def test_has_a_participant_as_a_foreign_key(self): self.assertIsInstance( self.response_set.participant, @@ -62,12 +82,12 @@ def test_has_submitted_at_as_a_datetime(self): datetime ) - def test_is_invalid_if_another_unsubmitted_response_set_exists(self): + def test_is_invalid_if_another_unsubmitted_response_set_exists(self): participant = Participant.objects.create(unique_id="56789") - participant.responseset_set.create(submitted_at = None) + participant.responseset_set.create(submitted_at=None) with self.assertRaises(ValidationError) as context: - participant.responseset_set.create(submitted_at = None) + participant.responseset_set.create(submitted_at=None) self.assertEqual( context.exception.messages[0], @@ -88,3 +108,64 @@ def test_is_invalid_if_another_response_set_was_submitted_within_the_last_year(s "Responses have already been submitted for this participant" ) + def test_is_invalid_if_height_is_below_lower_bound(self): + self.response_set.height = ResponseSet.MIN_HEIGHT_METRIC - 1 + + with self.assertRaises(ValidationError) as context: + self.response_set.full_clean() + + self.assertIn( + "Height must be between 139.7cm and 243.8 cm", + context.exception.messages + ) + + def test_is_invalid_if_height_is_above_upper_bound(self): + self.response_set.height = ResponseSet.MAX_HEIGHT_METRIC + 1 + + with self.assertRaises(ValidationError) as context: + self.response_set.full_clean() + + self.assertIn( + "Height must be between 139.7cm and 243.8 cm", + context.exception.messages + ) + + def test_is_invalid_if_imperial_height_is_below_lower_bound(self): + self.response_set.height_imperial = ResponseSet.MIN_HEIGHT_IMPERIAL - 1 + + with self.assertRaises(ValidationError) as context: + self.response_set.full_clean() + + self.assertIn( + "Height must be between 4 feet 7 inches and 8 feet", + context.exception.messages + ) + + def test_is_invalid_if_imperial_height_is_above_upper_bound(self): + self.response_set.height_imperial = ResponseSet.MAX_HEIGHT_IMPERIAL + 1 + + with self.assertRaises(ValidationError) as context: + self.response_set.full_clean() + + self.assertIn( + "Height must be between 4 feet 7 inches and 8 feet", + context.exception.messages + ) + + def test_formatted_height_returns_height_in_cm_if_set(self): + self.response_set.height = 1701 + self.response_set.save() + + self.assertEqual( + self.response_set.formatted_height, + "170.1cm" + ) + + def test_formatted_height_returns_imperial_height_if_set(self): + self.response_set.height_imperial = 68 + self.response_set.save() + + self.assertEqual( + self.response_set.formatted_height, + "5 feet 8 inches" + ) diff --git a/lung_cancer_screening/questions/tests/unit/views/test_date_of_birth.py b/lung_cancer_screening/questions/tests/unit/views/test_date_of_birth.py index ffa6cdf4..69e6456c 100644 --- a/lung_cancer_screening/questions/tests/unit/views/test_date_of_birth.py +++ b/lung_cancer_screening/questions/tests/unit/views/test_date_of_birth.py @@ -73,13 +73,13 @@ def test_post_sets_the_participant_id_in_session(self): self.assertEqual(self.client.session["participant_id"], "12345") - def test_post_redirects_to_responses_path(self): + def test_post_redirects_to_height_path(self): response = self.client.post( reverse("questions:date_of_birth"), self.valid_params ) - self.assertRedirects(response, reverse("questions:responses")) + self.assertRedirects(response, reverse("questions:height")) def test_post_responds_with_422_if_the_resource_is_invalid(self): response = self.client.post( diff --git a/lung_cancer_screening/questions/tests/unit/views/test_height.py b/lung_cancer_screening/questions/tests/unit/views/test_height.py new file mode 100644 index 00000000..a05d1522 --- /dev/null +++ b/lung_cancer_screening/questions/tests/unit/views/test_height.py @@ -0,0 +1,85 @@ +from django.test import TestCase +from django.urls import reverse + +from ....models.participant import Participant + + +class TestHeight(TestCase): + def setUp(self): + self.participant = Participant.objects.create(unique_id="12345") + self.participant.responseset_set.create() + + self.valid_height = 170 + self.valid_params = {"height": self.valid_height} + self.invalid_height = 80000 + + session = self.client.session + session['participant_id'] = self.participant.unique_id + + session.save() + + def test_get_redirects_if_the_participant_does_not_exist(self): + session = self.client.session + session['participant_id'] = "somebody none existent participant" + session.save() + + response = self.client.get( + reverse("questions:height") + ) + + self.assertRedirects(response, reverse("questions:start")) + + def test_get_responds_successfully(self): + response = self.client.get(reverse("questions:height")) + + self.assertEqual(response.status_code, 200) + + def test_get_renders_the_metric_form_by_default(self): + response = self.client.get(reverse("questions:height")) + + self.assertContains(response, "Centimetres") + + def test_get_renders_the_imperial_form_if_specified(self): + response = self.client.get(reverse("questions:height"), {"unit": "imperial"}) + + self.assertContains(response, "Feet") + self.assertContains(response, "Inches") + + def test_post_redirects_if_the_participant_does_not_exist(self): + session = self.client.session + session['participant_id'] = "somebody none existent participant" + session.save() + + response = self.client.post( + reverse("questions:height"), + self.valid_params + ) + + self.assertRedirects(response, reverse("questions:start")) + + def test_post_stores_a_valid_response_set_for_the_participant(self): + self.client.post( + reverse("questions:height"), + self.valid_params + ) + + response_set = self.participant.responseset_set.first() + + self.assertEqual(response_set.height, self.valid_height*10) + self.assertEqual(response_set.participant, self.participant) + + def test_post_redirects_to_responses_path(self): + response = self.client.post( + reverse("questions:height"), + self.valid_params + ) + + self.assertRedirects(response, reverse("questions:responses")) + + def test_post_responds_with_422_if_the_resource_is_invalid(self): + response = self.client.post( + reverse("questions:height"), + {"height": "a"} + ) + + self.assertEqual(response.status_code, 422) diff --git a/lung_cancer_screening/questions/urls.py b/lung_cancer_screening/questions/urls.py index 5948cec3..fdc1e9c8 100644 --- a/lung_cancer_screening/questions/urls.py +++ b/lung_cancer_screening/questions/urls.py @@ -22,11 +22,13 @@ from .views.age_range_exit import age_range_exit from .views.non_smoker_exit import non_smoker_exit from .views.your_results import your_results +from .views.height import height urlpatterns = [ path('start', start, name='start'), path('have-you-ever-smoked', have_you_ever_smoked, name='have_you_ever_smoked'), path('date-of-birth', date_of_birth, name='date_of_birth'), + path('height', height, name='height'), path('responses', responses, name='responses'), path('age-range-exit', age_range_exit, name='age_range_exit'), path('non-smoker-exit', non_smoker_exit, name='non_smoker_exit'), diff --git a/lung_cancer_screening/questions/views/date_of_birth.py b/lung_cancer_screening/questions/views/date_of_birth.py index ec769a75..c8c06da9 100644 --- a/lung_cancer_screening/questions/views/date_of_birth.py +++ b/lung_cancer_screening/questions/views/date_of_birth.py @@ -24,7 +24,7 @@ def date_of_birth(request): response_set.date_of_birth = date_of_birth response_set.save() - return redirect(reverse("questions:responses")) + return redirect(reverse("questions:height")) else: return redirect(reverse("questions:age_range_exit")) diff --git a/lung_cancer_screening/questions/views/height.py b/lung_cancer_screening/questions/views/height.py new file mode 100644 index 00000000..3bdf78a8 --- /dev/null +++ b/lung_cancer_screening/questions/views/height.py @@ -0,0 +1,44 @@ +from django.shortcuts import render, redirect + +from lung_cancer_screening.questions.forms.metric_height_form import MetricHeightForm +from lung_cancer_screening.questions.forms.imperial_height_form import ImperialHeightForm +from .decorators.participant_decorators import require_participant + +@require_participant +def height(request): + unit = request.GET.get('unit') + form_klass = ImperialHeightForm if unit == "imperial" else MetricHeightForm + + if request.method == "POST": + form = form_klass( + instance = request.participant.responseset_set.last(), + data=request.POST, + participant=request.participant + ) + + if form.is_valid(): + form.save() + + return redirect("questions:responses") + else: + return render( + request, + "height.jinja", + { + "form": form, + "unit": unit, + "switch_to_unit": "metric" if unit == "imperial" else "imperial" + }, + status=422 + ) + + return render( + request, + "height.jinja", + { + "form": form_klass(participant=request.participant), + "unit": unit, + "switch_to_unit": "metric" if unit == "imperial" else "imperial" + } + ) + diff --git a/makefiles/dev.mk b/makefiles/dev.mk index 4d55dc23..328ff395 100644 --- a/makefiles/dev.mk +++ b/makefiles/dev.mk @@ -7,7 +7,7 @@ dev-build: $(DOCKER_COMPOSE_CMD) build dev-run: - $(DOCKER_COMPOSE_CMD) up --build + $(DOCKER_COMPOSE_CMD) up dev-up: $(DOCKER_COMPOSE_CMD) up -d --build diff --git a/package.json b/package.json index 06f0daba..a7e915be 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ ], "scripts": { "compile": "concurrently npm:compile:js npm:compile:css --group --prefix none", - "compile:css": "sass --color --quiet-deps --silence-deprecation=import --load-path=. --load-path=node_modules lung_cancer_screening/assets/sass:lung_cancer_screening/assets/compiled/css", + "compile:css": "sass --color --quiet-deps --load-path=node_modules lung_cancer_screening/assets/sass:lung_cancer_screening/assets/compiled/css", "compile:css:watch": "npm run compile:css -- --watch", "compile:js": "rollup -c rollup.config.js --sourcemap", "compile:js:watch": "npm run compile:js -- --watch", diff --git a/scripts/tests/unit.sh b/scripts/tests/unit.sh index dfd0c621..3012b098 100755 --- a/scripts/tests/unit.sh +++ b/scripts/tests/unit.sh @@ -17,5 +17,8 @@ cd "$(git rev-parse --show-toplevel)" # tests from here. If you want to run other test suites, see the predefined # tasks in scripts/test.mk. -docker compose run --rm --remove-orphans web poetry run python manage.py test --settings=lung_cancer_screening.settings_test - +if [[ -n "${TEST_MODULE:-}" ]]; then + docker compose run --rm --remove-orphans web poetry run python manage.py test $TEST_MODULE --settings=lung_cancer_screening.settings_test +else + docker compose run --rm --remove-orphans web poetry run python manage.py test --settings=lung_cancer_screening.settings_test +fi