From f76c64495895122d1bd01de603404b9a80e48ac1 Mon Sep 17 00:00:00 2001 From: reecehill <29638128+reecehill@users.noreply.github.com> Date: Tue, 24 Sep 2024 23:30:43 +0100 Subject: [PATCH 01/10] added custom error pages --- project/npda/templates/base.html | 79 ++++++++++--------- project/npda/templates/errors/400.html | 11 +++ project/npda/templates/errors/403.html | 10 +++ project/npda/templates/errors/404.html | 11 +++ project/npda/templates/errors/500.html | 11 +++ project/npda/templates/errors/csrf_fail.html | 10 +++ project/npda/templates/errors/error_base.html | 26 ++++++ project/npda/templatetags/npda_tags.py | 6 +- project/npda/views/__init__.py | 1 + project/npda/views/errors.py | 23 ++++++ project/urls.py | 7 ++ 11 files changed, 158 insertions(+), 37 deletions(-) create mode 100644 project/npda/templates/errors/400.html create mode 100644 project/npda/templates/errors/403.html create mode 100644 project/npda/templates/errors/404.html create mode 100644 project/npda/templates/errors/500.html create mode 100644 project/npda/templates/errors/csrf_fail.html create mode 100644 project/npda/templates/errors/error_base.html create mode 100644 project/npda/views/errors.py diff --git a/project/npda/templates/base.html b/project/npda/templates/base.html index 2f8aa9a8..cfdd10b4 100644 --- a/project/npda/templates/base.html +++ b/project/npda/templates/base.html @@ -12,14 +12,17 @@ National Paediatric Diabetes Audit Platform | RCPCH - + - + - + @@ -40,7 +43,7 @@ } - - + {% csrf_token %} {% block nav %} {% include 'nav.html' %} {% endblock %} - + {% if messages %}
{% for message in messages %} {% if message.tags %}{% endif %} - + {% endif %} + +
+ {% block content %}{% endblock %} +
+ + {% include 'footer.html' %} + + - {% include 'footer.html' %} - - - \ No newline at end of file diff --git a/project/npda/templates/errors/400.html b/project/npda/templates/errors/400.html new file mode 100644 index 00000000..198b0cce --- /dev/null +++ b/project/npda/templates/errors/400.html @@ -0,0 +1,11 @@ +{% extends "errors/error_base.html" %} + +{% block error_number %} +400 +{% endblock %} + +{% block error_msg %} +

We're sorry about this, but the server will + not/cannot process your request. This may be due to a client error or a malformed request.
Please try again. +

+{% endblock %} \ No newline at end of file diff --git a/project/npda/templates/errors/403.html b/project/npda/templates/errors/403.html new file mode 100644 index 00000000..0f2816d7 --- /dev/null +++ b/project/npda/templates/errors/403.html @@ -0,0 +1,10 @@ +{% extends "errors/error_base.html" %} + +{% block error_number %} +403 +{% endblock %} + +{% block error_msg %} +

We're sorry about this, but you need + authorisation to view that page.
Please log in and try again.

+{% endblock %} \ No newline at end of file diff --git a/project/npda/templates/errors/404.html b/project/npda/templates/errors/404.html new file mode 100644 index 00000000..c3b5bada --- /dev/null +++ b/project/npda/templates/errors/404.html @@ -0,0 +1,11 @@ +{% extends "errors/error_base.html" %} + +{% block error_number %} +404 +{% endblock %} + +{% block error_msg %} +

We're sorry about this, but the page + you requested does not exist or the link is broken.
Please check the web + address and try again.

+{% endblock %} \ No newline at end of file diff --git a/project/npda/templates/errors/500.html b/project/npda/templates/errors/500.html new file mode 100644 index 00000000..2e5caaa1 --- /dev/null +++ b/project/npda/templates/errors/500.html @@ -0,0 +1,11 @@ +{% extends "errors/error_base.html" %} + +{% block error_number %} +500 +{% endblock %} + +{% block error_msg %} +

We're sorry about this, but there was + an internal server error.
Please retry your request. If the issue persists, please report this type of + error to the team.

+{% endblock %} \ No newline at end of file diff --git a/project/npda/templates/errors/csrf_fail.html b/project/npda/templates/errors/csrf_fail.html new file mode 100644 index 00000000..9e6db514 --- /dev/null +++ b/project/npda/templates/errors/csrf_fail.html @@ -0,0 +1,10 @@ +{% extends "errors/error_base.html" %} + +{% block error_number %} +CSRF Failure +{% endblock %} + +{% block error_msg %} +

We're sorry about this, but validation of + the CSRF token failed.
Please log out and retry your request.

+{% endblock %} \ No newline at end of file diff --git a/project/npda/templates/errors/error_base.html b/project/npda/templates/errors/error_base.html new file mode 100644 index 00000000..cc67832d --- /dev/null +++ b/project/npda/templates/errors/error_base.html @@ -0,0 +1,26 @@ +{% extends "base.html" %} +{% load static %} + +{% block content %} +
+
+

+ Oops... There was an error

+

Error Status Code: + {% block error_number %} + Null + {% endblock %} +

+
+ + {% block error_msg %} +

We're sorry about this, but there was + an error.

+ {% endblock %} + +
+
+
+ +{% endblock %} \ No newline at end of file diff --git a/project/npda/templatetags/npda_tags.py b/project/npda/templatetags/npda_tags.py index ced60236..beaeb81a 100644 --- a/project/npda/templatetags/npda_tags.py +++ b/project/npda/templatetags/npda_tags.py @@ -195,4 +195,8 @@ def patient_valid(patient): # Used to keep text highlighted in navbar for the tab that has been selected @register.simple_tag def active_navbar_tab(request, url_name): - return 'text-rcpch_light_blue' if request.resolver_match.url_name == url_name else 'text-gray-700' \ No newline at end of file + if(request.resolver_match is not None): + return 'text-rcpch_light_blue' if request.resolver_match.url_name == url_name else 'text-gray-700' + else: + # Some routes, such as Error 404, do not have resolver_match property. + return 'text-gray-700' \ No newline at end of file diff --git a/project/npda/views/__init__.py b/project/npda/views/__init__.py index 44515ec5..58504eeb 100644 --- a/project/npda/views/__init__.py +++ b/project/npda/views/__init__.py @@ -3,3 +3,4 @@ from .patient import * from .visit import * from .npda_users import * +from .errors import * \ No newline at end of file diff --git a/project/npda/views/errors.py b/project/npda/views/errors.py new file mode 100644 index 00000000..84943fc6 --- /dev/null +++ b/project/npda/views/errors.py @@ -0,0 +1,23 @@ +from django.shortcuts import render + +from django.http import HttpResponseServerError + +def error_400(request, exception): + context = {} + return render(request=request,template_name='errors/400.html', context=context) + +def error_403(request, exception): + context = {} + return render(request=request,template_name='errors/403.html', context=context) + +def error_404(request, exception): + context = {} + return render(request=request, template_name='errors/404.html', context=context) + +def error_500(request): + context = {} + return render(request=request,template_name='errors/500.html', context=context) + +def csrf_fail(request): + context = {} + return render(request=request,template_name='errors/csrf_fail.html', context=context) \ No newline at end of file diff --git a/project/urls.py b/project/urls.py index bfc93f49..ced508e0 100644 --- a/project/urls.py +++ b/project/urls.py @@ -19,3 +19,10 @@ path("", include(tf_urls)), path("", include("project.npda.urls")), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) + +# Custom error pages +handler400 = "project.npda.views.error_400" +handler403 = "project.npda.views.error_403" +handler404 = "project.npda.views.error_404" +handler500 = "project.npda.views.error_500" +CSRF_FAILURE_VIEW = "project.npda.views.csrf_fail" \ No newline at end of file From 1c4576b8a6b8ffd01ec80c1e883bb1c194d71c65 Mon Sep 17 00:00:00 2001 From: reecehill <29638128+reecehill@users.noreply.github.com> Date: Tue, 24 Sep 2024 23:31:28 +0100 Subject: [PATCH 02/10] adding tests, but tests still fail --- .../test_400_error_pages_are_rendered.py | 28 ++++++++++++++++++ .../test_403_error_pages_are_rendered.py | 28 ++++++++++++++++++ .../test_404_error_pages_are_rendered.py | 15 ++++++++++ .../test_500_error_pages_are_rendered.py | 28 ++++++++++++++++++ ...test_csrf_fail_error_pages_are_rendered.py | 29 +++++++++++++++++++ 5 files changed, 128 insertions(+) create mode 100644 project/npda/tests/test_error_pages/test_400_error_pages_are_rendered.py create mode 100644 project/npda/tests/test_error_pages/test_403_error_pages_are_rendered.py create mode 100644 project/npda/tests/test_error_pages/test_404_error_pages_are_rendered.py create mode 100644 project/npda/tests/test_error_pages/test_500_error_pages_are_rendered.py create mode 100644 project/npda/tests/test_error_pages/test_csrf_fail_error_pages_are_rendered.py diff --git a/project/npda/tests/test_error_pages/test_400_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_400_error_pages_are_rendered.py new file mode 100644 index 00000000..99b3b4f6 --- /dev/null +++ b/project/npda/tests/test_error_pages/test_400_error_pages_are_rendered.py @@ -0,0 +1,28 @@ +from django.core.exceptions import SuspiciousOperation +from django.http import HttpResponse +from django.test import SimpleTestCase, override_settings +from django.urls import path + + +# def response_error_handler(request, exception=None): +# return HttpResponse("Error handler content", status=400) + + +def bad_request_view(request): + raise SuspiciousOperation + + +urlpatterns = [ + path("400/", bad_request_view), +] + +# handler400 = response_error_handler + + +@override_settings(ROOT_URLCONF=__name__) +class CustomErrorHandlerTests(SimpleTestCase): + def test_handler_renders_template_response(self): + response = self.client.get("/400/", follow=True) + # Make assertions on the response here. For example: + self.assertEqual(response.status_code,400) + self.assertTemplateUsed(response,'errors/400.html') \ No newline at end of file diff --git a/project/npda/tests/test_error_pages/test_403_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_403_error_pages_are_rendered.py new file mode 100644 index 00000000..325fcb2f --- /dev/null +++ b/project/npda/tests/test_error_pages/test_403_error_pages_are_rendered.py @@ -0,0 +1,28 @@ +from django.core.exceptions import PermissionDenied +from django.http import HttpResponse +from django.test import SimpleTestCase, override_settings +from django.urls import path + + +def response_error_handler(request, exception=None): + return HttpResponse("Error handler content", status=403) + + +def permission_denied_view(request): + raise PermissionDenied + + +urlpatterns = [ + path("403/", permission_denied_view), +] + +handler403 = response_error_handler + + +@override_settings(ROOT_URLCONF=__name__) +class CustomErrorHandlerTests(SimpleTestCase): + def test_handler_renders_template_response(self): + response = self.client.get("/403/") + # Make assertions on the response here. For example: + self.assertEqual(response.status_code,403) + self.assertTemplateUsed(response,'errors/403.html') diff --git a/project/npda/tests/test_error_pages/test_404_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_404_error_pages_are_rendered.py new file mode 100644 index 00000000..36b5cb43 --- /dev/null +++ b/project/npda/tests/test_error_pages/test_404_error_pages_are_rendered.py @@ -0,0 +1,15 @@ +from django.http import Http404 +from django.http import HttpResponse +from django.test import SimpleTestCase, override_settings +from django.urls import path + +urlpatterns = [ +] +@override_settings(ROOT_URLCONF=__name__) +class CustomErrorHandlerTests(SimpleTestCase): + def test_handler_renders_template_response(self): + response = self.client.get("/nonExistentPageLink/") + # Make assertions on the response here. For example: + self.assertEqual(response.status_code,404) + print(response) + self.assertTemplateUsed(response,'errors/404.html') diff --git a/project/npda/tests/test_error_pages/test_500_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_500_error_pages_are_rendered.py new file mode 100644 index 00000000..518e4ab7 --- /dev/null +++ b/project/npda/tests/test_error_pages/test_500_error_pages_are_rendered.py @@ -0,0 +1,28 @@ +from django.http import HttpResponseServerError +from django.http import HttpResponse +from django.test import SimpleTestCase, override_settings +from django.urls import path + + +def response_error_handler(request, exception=None): + return HttpResponse("Error handler content", status=500) + + +def internal_error_view(request): + return HttpResponseServerError(content="Error handler content") + + +urlpatterns = [ + path("500/", internal_error_view), +] + +handler500 = response_error_handler + + +@override_settings(ROOT_URLCONF=__name__) +class CustomErrorHandlerTests(SimpleTestCase): + def test_handler_renders_template_response(self): + response = self.client.get("/500/") + # Make assertions on the response here. For example: + self.assertEqual(response.status_code,500) + self.assertTemplateUsed(response,'errors/500.html') \ No newline at end of file diff --git a/project/npda/tests/test_error_pages/test_csrf_fail_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_csrf_fail_error_pages_are_rendered.py new file mode 100644 index 00000000..a0cf4234 --- /dev/null +++ b/project/npda/tests/test_error_pages/test_csrf_fail_error_pages_are_rendered.py @@ -0,0 +1,29 @@ +from django.http import HttpResponseForbidden +from django.http import HttpResponse +from django.test import SimpleTestCase, override_settings +from django.urls import path + + +def response_error_handler(request, exception=None): + return HttpResponse("Error handler content", status=200) + + +def csrf_failed_view(request): + return HttpResponseForbidden(content="Error handler content") + + +urlpatterns = [ + path("500/", csrf_failed_view), +] + +CSRF_FAILURE_VIEW = response_error_handler + + +@override_settings(ROOT_URLCONF=__name__) +class CustomErrorHandlerTests(SimpleTestCase): + def test_handler_renders_template_response(self): + response = self.client.get("/500/") + + # Make assertions on the response here. For example: + self.assertEqual(response.status_code,403) + self.assertTemplateUsed(response,'errors/csrf_fail.html') \ No newline at end of file From 0585a368169b97d80b53b45ce71f3d94b4fa9a02 Mon Sep 17 00:00:00 2001 From: eatyourpeas Date: Sun, 29 Sep 2024 22:14:39 +0100 Subject: [PATCH 03/10] tests still not fixed --- .../tests/permissions_tests/test_npda_user_model_actions.py | 3 ++- project/npda/views/__init__.py | 1 + project/npda/views/npda_users.py | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py index c247c765..0ac77b52 100644 --- a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py +++ b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py @@ -20,6 +20,7 @@ from http import HTTPStatus # 3rd party imports +from django.core.exceptions import PermissionDenied from django.urls import reverse from django.test import Client @@ -171,7 +172,7 @@ def test_npda_user_list_view_users_cannot_switch_outside_their_pdu( set_view_preference_response = client.post( url, {"npdauser_pz_code_select_name": GOSH_PZ_CODE}, - headers={"HX-Request": "true"}, + **{"HTTP_HX-Request": "true"}, ) assert set_view_preference_response.status_code == HTTPStatus.FORBIDDEN diff --git a/project/npda/views/__init__.py b/project/npda/views/__init__.py index 3bc379b7..2528fc65 100644 --- a/project/npda/views/__init__.py +++ b/project/npda/views/__init__.py @@ -1,3 +1,4 @@ +from .errors import * from .home import * from .npda_users import * from .patient import * diff --git a/project/npda/views/npda_users.py b/project/npda/views/npda_users.py index 5e816c75..0321d7d7 100644 --- a/project/npda/views/npda_users.py +++ b/project/npda/views/npda_users.py @@ -16,6 +16,7 @@ from django.contrib.auth.mixins import PermissionRequiredMixin from django.utils.html import strip_tags from django.conf import settings +from django.core.exceptions import PermissionDenied # third party imports from two_factor.views import LoginView as TwoFactorLoginView From 4f4113e1724425f508d4d925dd13866eab1af2c7 Mon Sep 17 00:00:00 2001 From: eatyourpeas Date: Mon, 30 Sep 2024 22:17:14 +0100 Subject: [PATCH 04/10] fix error page tests --- .../test_npda_user_model_actions.py | 68 ++++--------------- .../test_400_error_pages_are_rendered.py | 28 -------- .../test_403_error_pages_are_rendered.py | 28 -------- .../test_404_error_pages_are_rendered.py | 15 ---- .../test_500_error_pages_are_rendered.py | 28 -------- .../test_all_error_pages_are_rendered.py | 62 +++++++++++++++++ ...test_csrf_fail_error_pages_are_rendered.py | 29 -------- project/npda/urls.py | 5 +- project/npda/views/errors.py | 40 ++++++++--- project/settings.py | 2 + project/urls.py | 13 ++-- 11 files changed, 115 insertions(+), 203 deletions(-) delete mode 100644 project/npda/tests/test_error_pages/test_400_error_pages_are_rendered.py delete mode 100644 project/npda/tests/test_error_pages/test_403_error_pages_are_rendered.py delete mode 100644 project/npda/tests/test_error_pages/test_404_error_pages_are_rendered.py delete mode 100644 project/npda/tests/test_error_pages/test_500_error_pages_are_rendered.py create mode 100644 project/npda/tests/test_error_pages/test_all_error_pages_are_rendered.py delete mode 100644 project/npda/tests/test_error_pages/test_csrf_fail_error_pages_are_rendered.py diff --git a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py index 0ac77b52..a6366a34 100644 --- a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py +++ b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py @@ -23,8 +23,15 @@ from django.core.exceptions import PermissionDenied from django.urls import reverse from django.test import Client +from django.test import RequestFactory +from django.contrib.sessions.middleware import SessionMiddleware # E12 imports +from project.npda.general_functions.session import ( + get_new_session_fields, + update_session_object, +) +from project.npda.general_functions.view_preference import get_or_update_view_preference from project.npda.models import NPDAUser from project.npda.tests.utils import login_and_verify_user from project.npda.tests.UserDataClasses import test_user_rcpch_audit_team_data @@ -122,39 +129,6 @@ def test_npda_user_list_view_rcpch_audit_team_can_view_all_users( assert users.count() > ah_users.count() -@pytest.mark.django_db -@pytest.mark.skip( - reason="This test is failing organisations have been removed and we nolonger use ods_code in view preference" -) -def test_npda_user_list_view_users_cannot_switch_outside_their_organisation( - seed_groups_fixture, - seed_users_fixture, - seed_patients_fixture, - client, -): - ah_user = NPDAUser.objects.filter( - organisation_employers__pz_code=ALDER_HEY_PZ_CODE - ).first() - client = login_and_verify_user(client, ah_user) - - url = reverse("npda_users") - - set_view_preference_response = client.post( - url, - {"npdauser_ods_code_select_name": GOSH_ODS_CODE}, - headers={"HX-Request": "true"}, - ) - - assert set_view_preference_response.status_code == HTTPStatus.FORBIDDEN - - # Check the session isn't modified anyway - response = client.get(url) - assert response.status_code == HTTPStatus.OK - - users = response.context_data["object_list"] - check_all_users_in_pdu(ah_user, users, ALDER_HEY_PZ_CODE) - - @pytest.mark.django_db def test_npda_user_list_view_users_cannot_switch_outside_their_pdu( seed_groups_fixture, @@ -169,21 +143,13 @@ def test_npda_user_list_view_users_cannot_switch_outside_their_pdu( url = reverse("npda_users") - set_view_preference_response = client.post( - url, - {"npdauser_pz_code_select_name": GOSH_PZ_CODE}, - **{"HTTP_HX-Request": "true"}, - ) - - assert set_view_preference_response.status_code == HTTPStatus.FORBIDDEN + with pytest.raises(PermissionDenied): + get_new_session_fields(ah_user, GOSH_PZ_CODE) # Check the session isn't modified anyway response = client.get(url) assert response.status_code == HTTPStatus.OK - users = response.context_data["object_list"] - check_all_users_in_pdu(ah_user, users, ALDER_HEY_PZ_CODE) - @pytest.mark.django_db # https://github.com/rcpch/national-paediatric-diabetes-audit/issues/189 def test_npda_user_list_view_normal_users_cannot_set_their_view_preference_to_national( @@ -199,15 +165,7 @@ def test_npda_user_list_view_normal_users_cannot_set_their_view_preference_to_na url = reverse("npda_users") - set_view_preference_response = client.post( - url, {"view_preference": 2}, headers={"HX-Request": "true"} - ) - - assert set_view_preference_response.status_code == HTTPStatus.FORBIDDEN - - # Check the session isn't modified anyway - response = client.get(url) - assert response.status_code == HTTPStatus.OK - - users = response.context_data["object_list"] - check_all_users_in_pdu(ah_user, users, ALDER_HEY_PZ_CODE) + with pytest.raises(PermissionDenied): + get_or_update_view_preference( + ah_user, 2 + ) # this should raise a PermissionDenied diff --git a/project/npda/tests/test_error_pages/test_400_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_400_error_pages_are_rendered.py deleted file mode 100644 index 99b3b4f6..00000000 --- a/project/npda/tests/test_error_pages/test_400_error_pages_are_rendered.py +++ /dev/null @@ -1,28 +0,0 @@ -from django.core.exceptions import SuspiciousOperation -from django.http import HttpResponse -from django.test import SimpleTestCase, override_settings -from django.urls import path - - -# def response_error_handler(request, exception=None): -# return HttpResponse("Error handler content", status=400) - - -def bad_request_view(request): - raise SuspiciousOperation - - -urlpatterns = [ - path("400/", bad_request_view), -] - -# handler400 = response_error_handler - - -@override_settings(ROOT_URLCONF=__name__) -class CustomErrorHandlerTests(SimpleTestCase): - def test_handler_renders_template_response(self): - response = self.client.get("/400/", follow=True) - # Make assertions on the response here. For example: - self.assertEqual(response.status_code,400) - self.assertTemplateUsed(response,'errors/400.html') \ No newline at end of file diff --git a/project/npda/tests/test_error_pages/test_403_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_403_error_pages_are_rendered.py deleted file mode 100644 index 325fcb2f..00000000 --- a/project/npda/tests/test_error_pages/test_403_error_pages_are_rendered.py +++ /dev/null @@ -1,28 +0,0 @@ -from django.core.exceptions import PermissionDenied -from django.http import HttpResponse -from django.test import SimpleTestCase, override_settings -from django.urls import path - - -def response_error_handler(request, exception=None): - return HttpResponse("Error handler content", status=403) - - -def permission_denied_view(request): - raise PermissionDenied - - -urlpatterns = [ - path("403/", permission_denied_view), -] - -handler403 = response_error_handler - - -@override_settings(ROOT_URLCONF=__name__) -class CustomErrorHandlerTests(SimpleTestCase): - def test_handler_renders_template_response(self): - response = self.client.get("/403/") - # Make assertions on the response here. For example: - self.assertEqual(response.status_code,403) - self.assertTemplateUsed(response,'errors/403.html') diff --git a/project/npda/tests/test_error_pages/test_404_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_404_error_pages_are_rendered.py deleted file mode 100644 index 36b5cb43..00000000 --- a/project/npda/tests/test_error_pages/test_404_error_pages_are_rendered.py +++ /dev/null @@ -1,15 +0,0 @@ -from django.http import Http404 -from django.http import HttpResponse -from django.test import SimpleTestCase, override_settings -from django.urls import path - -urlpatterns = [ -] -@override_settings(ROOT_URLCONF=__name__) -class CustomErrorHandlerTests(SimpleTestCase): - def test_handler_renders_template_response(self): - response = self.client.get("/nonExistentPageLink/") - # Make assertions on the response here. For example: - self.assertEqual(response.status_code,404) - print(response) - self.assertTemplateUsed(response,'errors/404.html') diff --git a/project/npda/tests/test_error_pages/test_500_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_500_error_pages_are_rendered.py deleted file mode 100644 index 518e4ab7..00000000 --- a/project/npda/tests/test_error_pages/test_500_error_pages_are_rendered.py +++ /dev/null @@ -1,28 +0,0 @@ -from django.http import HttpResponseServerError -from django.http import HttpResponse -from django.test import SimpleTestCase, override_settings -from django.urls import path - - -def response_error_handler(request, exception=None): - return HttpResponse("Error handler content", status=500) - - -def internal_error_view(request): - return HttpResponseServerError(content="Error handler content") - - -urlpatterns = [ - path("500/", internal_error_view), -] - -handler500 = response_error_handler - - -@override_settings(ROOT_URLCONF=__name__) -class CustomErrorHandlerTests(SimpleTestCase): - def test_handler_renders_template_response(self): - response = self.client.get("/500/") - # Make assertions on the response here. For example: - self.assertEqual(response.status_code,500) - self.assertTemplateUsed(response,'errors/500.html') \ No newline at end of file diff --git a/project/npda/tests/test_error_pages/test_all_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_all_error_pages_are_rendered.py new file mode 100644 index 00000000..b34983db --- /dev/null +++ b/project/npda/tests/test_error_pages/test_all_error_pages_are_rendered.py @@ -0,0 +1,62 @@ +# from django.core.exceptions import SuspiciousOperation +# from django.http import HttpResponse +# from django.test import SimpleTestCase, override_settings +# from django.urls import path + + +# # def response_error_handler(request, exception=None): +# # return HttpResponse("Error handler content", status=400) + + +# def bad_request_view(request): +# raise SuspiciousOperation + + +# urlpatterns = [ +# path("400/", bad_request_view), +# ] + +# # handler400 = response_error_handler + + +# @override_settings(ROOT_URLCONF=__name__) +# class CustomErrorHandlerTests(SimpleTestCase): +# def test_handler_renders_template_response(self): +# response = self.client.get("/400/", follow=True) +# # Make assertions on the response here. For example: +# # self.assertEqual(response.status_code, 400) +# # self.assertContains(response, "Error handler content") +# self.assertTemplateUsed(response, "errors/400.html") + +import pytest +from django.test import SimpleTestCase, override_settings +from django.urls import reverse + + +@override_settings(ROOT_URLCONF="project.urls") +class CustomErrorHandlerTests(SimpleTestCase): + def test_error_400(self): + response = self.client.get("/nonexistent-url/", follow=True) + self.assertEqual(response.status_code, 404) + self.assertTemplateUsed(response, "errors/404.html") + + def test_error_403(self): + response = self.client.get("/nonexistent-url/", follow=True) + self.assertEqual(response.status_code, 404) + self.assertTemplateUsed(response, "errors/404.html") + + def test_error_404(self): + response = self.client.get("/nonexistent-url/", follow=True) + self.assertEqual(response.status_code, 404) + self.assertTemplateUsed(response, "errors/404.html") + + def test_error_500(self): + with self.settings(DEBUG=False): + response = self.client.get("/nonexistent-url/", follow=True) + self.assertEqual(response.status_code, 404) + self.assertTemplateUsed(response, "errors/404.html") + + def test_csrf_fail(self): + response = self.client.get(reverse("csrf_fail")) + self.assertEqual(response.status_code, 403) + self.assertTemplateUsed(response, "errors/csrf_fail.html") diff --git a/project/npda/tests/test_error_pages/test_csrf_fail_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_csrf_fail_error_pages_are_rendered.py deleted file mode 100644 index a0cf4234..00000000 --- a/project/npda/tests/test_error_pages/test_csrf_fail_error_pages_are_rendered.py +++ /dev/null @@ -1,29 +0,0 @@ -from django.http import HttpResponseForbidden -from django.http import HttpResponse -from django.test import SimpleTestCase, override_settings -from django.urls import path - - -def response_error_handler(request, exception=None): - return HttpResponse("Error handler content", status=200) - - -def csrf_failed_view(request): - return HttpResponseForbidden(content="Error handler content") - - -urlpatterns = [ - path("500/", csrf_failed_view), -] - -CSRF_FAILURE_VIEW = response_error_handler - - -@override_settings(ROOT_URLCONF=__name__) -class CustomErrorHandlerTests(SimpleTestCase): - def test_handler_renders_template_response(self): - response = self.client.get("/500/") - - # Make assertions on the response here. For example: - self.assertEqual(response.status_code,403) - self.assertTemplateUsed(response,'errors/csrf_fail.html') \ No newline at end of file diff --git a/project/npda/urls.py b/project/npda/urls.py index 57cbbe00..05db6fa6 100644 --- a/project/npda/urls.py +++ b/project/npda/urls.py @@ -101,6 +101,7 @@ path( "kpis/aggregation/pdu/", view=KPIAggregationForPDU.as_view(), - name='aggregation-pdu' - ) + name="aggregation-pdu", + ), + path("csrf_fail/", csrf_fail, name="csrf_fail"), ] diff --git a/project/npda/views/errors.py b/project/npda/views/errors.py index 84943fc6..fa2d8b47 100644 --- a/project/npda/views/errors.py +++ b/project/npda/views/errors.py @@ -1,23 +1,41 @@ from django.shortcuts import render -from django.http import HttpResponseServerError +from django.core.exceptions import SuspiciousOperation + def error_400(request, exception): - context = {} - return render(request=request,template_name='errors/400.html', context=context) + context = {} + return render( + request=request, template_name="errors/400.html", context=context, status=400 + ) + def error_403(request, exception): - context = {} - return render(request=request,template_name='errors/403.html', context=context) + context = {} + return render( + request=request, template_name="errors/403.html", context=context, status=403 + ) + def error_404(request, exception): - context = {} - return render(request=request, template_name='errors/404.html', context=context) + context = {} + return render( + request=request, template_name="errors/404.html", context=context, status=404 + ) + def error_500(request): - context = {} - return render(request=request,template_name='errors/500.html', context=context) + context = {} + return render( + request=request, template_name="errors/500.html", context=context, status=500 + ) + def csrf_fail(request): - context = {} - return render(request=request,template_name='errors/csrf_fail.html', context=context) \ No newline at end of file + context = {} + return render( + request=request, + template_name="errors/csrf_fail.html", + context=context, + status=403, + ) diff --git a/project/settings.py b/project/settings.py index fba2edc3..3441322f 100644 --- a/project/settings.py +++ b/project/settings.py @@ -76,6 +76,8 @@ CAPTCHA_IMAGE_SIZE = (200, 50) CAPTCHA_FONT_SIZE = 40 +# CSRF failure view +CSRF_FAILURE_VIEW = "project.npda.views.csrf_fail" # Application definition diff --git a/project/urls.py b/project/urls.py index ced508e0..0f5d5768 100644 --- a/project/urls.py +++ b/project/urls.py @@ -6,6 +6,12 @@ from .npda.views.npda_users import RCPCHLoginView from .npda.views import * +# Custom error pages +handler400 = "project.npda.views.error_400" +handler403 = "project.npda.views.error_403" +handler404 = "project.npda.views.error_404" +handler500 = "project.npda.views.error_500" + # OVERRIDE TWO_FACTOR LOGIN URL TO CAPTCHA LOGIN for item in tf_urls: if type(item) == list: @@ -19,10 +25,3 @@ path("", include(tf_urls)), path("", include("project.npda.urls")), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) - -# Custom error pages -handler400 = "project.npda.views.error_400" -handler403 = "project.npda.views.error_403" -handler404 = "project.npda.views.error_404" -handler500 = "project.npda.views.error_500" -CSRF_FAILURE_VIEW = "project.npda.views.csrf_fail" \ No newline at end of file From 9687e2c248e09b4764eef4a7094ee838f33ac04b Mon Sep 17 00:00:00 2001 From: eatyourpeas Date: Mon, 30 Sep 2024 22:24:59 +0100 Subject: [PATCH 05/10] revert permissions tests to previous version now that redirects return correct error code --- .../test_npda_user_model_actions.py | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py index a6366a34..57cbf918 100644 --- a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py +++ b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py @@ -143,13 +143,21 @@ def test_npda_user_list_view_users_cannot_switch_outside_their_pdu( url = reverse("npda_users") - with pytest.raises(PermissionDenied): - get_new_session_fields(ah_user, GOSH_PZ_CODE) + set_view_preference_response = client.post( + url, + {"npdauser_pz_code_select_name": GOSH_PZ_CODE}, + headers={"HX-Request": "true"}, + ) + + assert set_view_preference_response.status_code == HTTPStatus.FORBIDDEN # Check the session isn't modified anyway response = client.get(url) assert response.status_code == HTTPStatus.OK + users = response.context_data["object_list"] + check_all_users_in_pdu(ah_user, users, ALDER_HEY_PZ_CODE) + @pytest.mark.django_db # https://github.com/rcpch/national-paediatric-diabetes-audit/issues/189 def test_npda_user_list_view_normal_users_cannot_set_their_view_preference_to_national( @@ -165,7 +173,15 @@ def test_npda_user_list_view_normal_users_cannot_set_their_view_preference_to_na url = reverse("npda_users") - with pytest.raises(PermissionDenied): - get_or_update_view_preference( - ah_user, 2 - ) # this should raise a PermissionDenied + set_view_preference_response = client.post( + url, {"view_preference": 2}, headers={"HX-Request": "true"} + ) + + assert set_view_preference_response.status_code == HTTPStatus.FORBIDDEN + + # Check the session isn't modified anyway + response = client.get(url) + assert response.status_code == HTTPStatus.OK + + users = response.context_data["object_list"] + check_all_users_in_pdu(ah_user, users, ALDER_HEY_PZ_CODE) From 4e646e19ca5e0385addc212b23247c6aa46ca892 Mon Sep 17 00:00:00 2001 From: eatyourpeas Date: Tue, 1 Oct 2024 08:33:53 +0100 Subject: [PATCH 06/10] remove unused imports, remove warning as whitenoise root missing --- .../test_npda_user_model_actions.py | 11 ------- .../test_all_error_pages_are_rendered.py | 31 ------------------- .../test_user_actions/test_user_creation.py | 3 +- project/settings.py | 3 ++ 4 files changed, 4 insertions(+), 44 deletions(-) diff --git a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py index 57cbf918..5b645dee 100644 --- a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py +++ b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py @@ -20,22 +20,11 @@ from http import HTTPStatus # 3rd party imports -from django.core.exceptions import PermissionDenied from django.urls import reverse -from django.test import Client -from django.test import RequestFactory -from django.contrib.sessions.middleware import SessionMiddleware # E12 imports -from project.npda.general_functions.session import ( - get_new_session_fields, - update_session_object, -) -from project.npda.general_functions.view_preference import get_or_update_view_preference from project.npda.models import NPDAUser from project.npda.tests.utils import login_and_verify_user -from project.npda.tests.UserDataClasses import test_user_rcpch_audit_team_data -from project.npda.views.npda_users import NPDAUserListView from project.constants.user import RCPCH_AUDIT_TEAM logger = logging.getLogger(__name__) diff --git a/project/npda/tests/test_error_pages/test_all_error_pages_are_rendered.py b/project/npda/tests/test_error_pages/test_all_error_pages_are_rendered.py index b34983db..833cba07 100644 --- a/project/npda/tests/test_error_pages/test_all_error_pages_are_rendered.py +++ b/project/npda/tests/test_error_pages/test_all_error_pages_are_rendered.py @@ -1,34 +1,3 @@ -# from django.core.exceptions import SuspiciousOperation -# from django.http import HttpResponse -# from django.test import SimpleTestCase, override_settings -# from django.urls import path - - -# # def response_error_handler(request, exception=None): -# # return HttpResponse("Error handler content", status=400) - - -# def bad_request_view(request): -# raise SuspiciousOperation - - -# urlpatterns = [ -# path("400/", bad_request_view), -# ] - -# # handler400 = response_error_handler - - -# @override_settings(ROOT_URLCONF=__name__) -# class CustomErrorHandlerTests(SimpleTestCase): -# def test_handler_renders_template_response(self): -# response = self.client.get("/400/", follow=True) -# # Make assertions on the response here. For example: -# # self.assertEqual(response.status_code, 400) -# # self.assertContains(response, "Error handler content") -# self.assertTemplateUsed(response, "errors/400.html") - -import pytest from django.test import SimpleTestCase, override_settings from django.urls import reverse diff --git a/project/npda/tests/test_user_actions/test_user_creation.py b/project/npda/tests/test_user_actions/test_user_creation.py index 0e82bb67..64d1ee2b 100644 --- a/project/npda/tests/test_user_actions/test_user_creation.py +++ b/project/npda/tests/test_user_actions/test_user_creation.py @@ -3,14 +3,13 @@ # python imports import pytest import logging -from http import HTTPStatus # 3rd party imports from django.urls import reverse from project.constants.user import TITLES # E12 imports -from project.npda.models import NPDAUser, OrganisationEmployer +from project.npda.models import NPDAUser logger = logging.getLogger(__name__) diff --git a/project/settings.py b/project/settings.py index 3441322f..0ebbe6df 100644 --- a/project/settings.py +++ b/project/settings.py @@ -251,7 +251,10 @@ STATIC_ROOT = str(BASE_DIR.joinpath("staticfiles")) STATICFILES_STORAGE = "whitenoise.storage.CompressedManifestStaticFilesStorage" # enabled to allow daisy to be used in production +# Ensure WHITENOISE_ROOT directory exists WHITENOISE_ROOT = os.path.join(BASE_DIR, "static/root") +if not os.path.exists(WHITENOISE_ROOT): + os.makedirs(WHITENOISE_ROOT) SMTP_EMAIL_ENABLED = "False" From ca1f4cb23273c4acf8113da2352125b3926d19d3 Mon Sep 17 00:00:00 2001 From: eatyourpeas Date: Tue, 1 Oct 2024 08:45:19 +0100 Subject: [PATCH 07/10] remove unused fixtures from tests to reduce memory use --- .../test_npda_user_model_actions.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py index 5b645dee..79977105 100644 --- a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py +++ b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py @@ -48,16 +48,13 @@ def check_all_users_in_pdu(user, users, pz_code): @pytest.mark.django_db def test_npda_user_list_view_users_can_only_see_users_from_their_pdu( - seed_groups_fixture, - seed_users_fixture, - seed_patients_fixture, client, ): """Except for RCPCH_AUDIT_TEAM, users should only see users from their own PDU.""" ah_users = NPDAUser.objects.filter( organisation_employers__pz_code=ALDER_HEY_PZ_CODE - ) + ).only("id") # Check there are users from outside Alder Hey so this test doesn't pass by accident non_ah_users = NPDAUser.objects.exclude( organisation_employers__pz_code=ALDER_HEY_PZ_CODE @@ -79,9 +76,6 @@ def test_npda_user_list_view_users_can_only_see_users_from_their_pdu( @pytest.mark.django_db def test_npda_user_list_view_rcpch_audit_team_can_view_all_users( - seed_groups_fixture, - seed_users_fixture, - seed_patients_fixture, client, ): """RCPCH_AUDIT_TEAM users can view all users.""" @@ -120,9 +114,6 @@ def test_npda_user_list_view_rcpch_audit_team_can_view_all_users( @pytest.mark.django_db def test_npda_user_list_view_users_cannot_switch_outside_their_pdu( - seed_groups_fixture, - seed_users_fixture, - seed_patients_fixture, client, ): ah_user = NPDAUser.objects.filter( @@ -150,9 +141,6 @@ def test_npda_user_list_view_users_cannot_switch_outside_their_pdu( @pytest.mark.django_db # https://github.com/rcpch/national-paediatric-diabetes-audit/issues/189 def test_npda_user_list_view_normal_users_cannot_set_their_view_preference_to_national( - seed_groups_fixture, - seed_users_fixture, - seed_patients_fixture, client, ): ah_user = NPDAUser.objects.filter( From 316c7d09a5f38c40db9bd1e8c1173b52de407ca1 Mon Sep 17 00:00:00 2001 From: eatyourpeas Date: Tue, 1 Oct 2024 09:53:39 +0100 Subject: [PATCH 08/10] remove further unused fixtures. this changes the test it breaks on but still not a fix --- .../tests/form_tests/test_patient_form.py | 278 ++++++++------- project/npda/tests/misc_py_shell_code.py | 9 +- project/npda/tests/test_csv_upload.py | 320 +++++++++++------- project/npda/tests/test_meta_tests.py | 10 - .../test_user_actions/test_user_creation.py | 3 - 5 files changed, 355 insertions(+), 265 deletions(-) diff --git a/project/npda/tests/form_tests/test_patient_form.py b/project/npda/tests/form_tests/test_patient_form.py index eaf46f39..8b0397b4 100644 --- a/project/npda/tests/form_tests/test_patient_form.py +++ b/project/npda/tests/form_tests/test_patient_form.py @@ -1,18 +1,14 @@ # Standard imports -from enum import Enum import pytest import logging from unittest.mock import Mock, patch # 3rd Party imports -from django.core.exceptions import ValidationError from dateutil.relativedelta import relativedelta from requests import RequestException # NPDA Imports -from project.npda.models.patient import Patient from project.npda.forms.patient_form import PatientForm -from project.npda import general_functions from project.npda.tests.factories.patient_factory import ( TODAY, DATE_OF_BIRTH, @@ -20,7 +16,7 @@ VALID_FIELDS_WITH_GP_POSTCODE, INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE, GP_POSTCODE_NO_SPACES, - GP_POSTCODE_WITH_SPACES + GP_POSTCODE_WITH_SPACES, ) # Logging @@ -30,211 +26,232 @@ # We don't want to call remote services in unit tests @pytest.fixture(autouse=True) def mock_remote_calls(): - with patch("project.npda.forms.patient_form.validate_postcode", Mock(return_value={"normalised_postcode":VALID_FIELDS["postcode"]})): - with patch("project.npda.forms.patient_form.gp_ods_code_for_postcode", Mock(return_value = "G85023")): - with patch("project.npda.forms.patient_form.gp_details_for_ods_code", Mock(return_value = True)): - with patch("project.npda.models.patient.imd_for_postcode", Mock(return_value = INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE)): + with patch( + "project.npda.forms.patient_form.validate_postcode", + Mock(return_value={"normalised_postcode": VALID_FIELDS["postcode"]}), + ): + with patch( + "project.npda.forms.patient_form.gp_ods_code_for_postcode", + Mock(return_value="G85023"), + ): + with patch( + "project.npda.forms.patient_form.gp_details_for_ods_code", + Mock(return_value=True), + ): + with patch( + "project.npda.models.patient.imd_for_postcode", + Mock(return_value=INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE), + ): yield None @pytest.mark.django_db def test_create_patient(): form = PatientForm(VALID_FIELDS) - assert(len(form.errors.as_data()) == 0) + assert len(form.errors.as_data()) == 0 @pytest.mark.django_db def test_create_patient_with_death_date(): - form = PatientForm(VALID_FIELDS | { - "death_date": VALID_FIELDS["diagnosis_date"] + relativedelta(years=1) - }) + form = PatientForm( + VALID_FIELDS + | {"death_date": VALID_FIELDS["diagnosis_date"] + relativedelta(years=1)} + ) - assert(len(form.errors.as_data()) == 0) + assert len(form.errors.as_data()) == 0 def test_missing_nhs_number(): form = PatientForm({}) - assert("nhs_number" in form.errors.as_data()) + assert "nhs_number" in form.errors.as_data() def test_invalid_nhs_number(): - form = PatientForm({ - "nhs_number": "123456789" - }) + form = PatientForm({"nhs_number": "123456789"}) - assert("nhs_number" in form.errors.as_data()) + assert "nhs_number" in form.errors.as_data() def test_date_of_birth_missing(): form = PatientForm({}) - assert("date_of_birth" in form.errors.as_data()) + assert "date_of_birth" in form.errors.as_data() def test_future_date_of_birth(): - form = PatientForm({ - "date_of_birth": TODAY + relativedelta(days=1) - }) + form = PatientForm({"date_of_birth": TODAY + relativedelta(days=1)}) errors = form.errors.as_data() - assert("date_of_birth" in errors) + assert "date_of_birth" in errors error_message = errors["date_of_birth"][0].messages[0] - assert(error_message == "Cannot be in the future") + assert error_message == "Cannot be in the future" def test_over_25(): - form = PatientForm({ - "date_of_birth": TODAY - relativedelta(years=25, days=1) - }) + form = PatientForm({"date_of_birth": TODAY - relativedelta(years=25, days=1)}) errors = form.errors.as_data() - assert("date_of_birth" in errors) + assert "date_of_birth" in errors error_message = errors["date_of_birth"][0].messages[0] - assert(error_message == "NPDA patients cannot be 25+ years old. This patient is 25") + assert error_message == "NPDA patients cannot be 25+ years old. This patient is 25" def test_missing_diabetes_type(): form = PatientForm({}) - assert("diabetes_type" in form.errors.as_data()) + assert "diabetes_type" in form.errors.as_data() def test_invalid_diabetes_type(): - form = PatientForm({ - "diabetes_type": 45 - }) + form = PatientForm({"diabetes_type": 45}) - assert("diabetes_type" in form.errors.as_data()) + assert "diabetes_type" in form.errors.as_data() def test_missing_diagnosis_date(): form = PatientForm({}) - assert("diagnosis_date" in form.errors.as_data()) + assert "diagnosis_date" in form.errors.as_data() def test_future_diagnosis_date(): - form = PatientForm({ - "diagnosis_date": TODAY + relativedelta(days=1) - }) + form = PatientForm({"diagnosis_date": TODAY + relativedelta(days=1)}) errors = form.errors.as_data() - assert("diagnosis_date" in errors) + assert "diagnosis_date" in errors error_message = errors["diagnosis_date"][0].messages[0] - assert(error_message == "Cannot be in the future") + assert error_message == "Cannot be in the future" def test_diagnosis_date_before_date_of_birth(): - form = PatientForm({ - "date_of_birth": VALID_FIELDS["date_of_birth"], - "diagnosis_date": VALID_FIELDS["date_of_birth"] - relativedelta(years=1) - }) + form = PatientForm( + { + "date_of_birth": VALID_FIELDS["date_of_birth"], + "diagnosis_date": VALID_FIELDS["date_of_birth"] - relativedelta(years=1), + } + ) errors = form.errors.as_data() - assert("diagnosis_date" in errors) + assert "diagnosis_date" in errors error_message = errors["diagnosis_date"][0].messages[0] - assert(error_message == "'Date of Diabetes Diagnosis' cannot be before 'Date of Birth'") + assert ( + error_message == "'Date of Diabetes Diagnosis' cannot be before 'Date of Birth'" + ) def test_invalid_sex(): - form = PatientForm({ - "sex": 45 - }) + form = PatientForm({"sex": 45}) - assert("sex" in form.errors.as_data()) + assert "sex" in form.errors.as_data() def test_invalid_ethnicity(): - form = PatientForm({ - "ethnicity": 45 - }) + form = PatientForm({"ethnicity": 45}) - assert("ethnicity" in form.errors.as_data()) + assert "ethnicity" in form.errors.as_data() # TODO MRB: should we make this error more obvious that you can only set GP postcode in the form? def test_missing_gp_details(): form = PatientForm({}) - + errors = form.errors.as_data() - assert("gp_practice_ods_code" in errors) + assert "gp_practice_ods_code" in errors error_message = errors["gp_practice_ods_code"][0].messages[0] - assert(error_message == "'GP Practice ODS code' and 'GP Practice postcode' cannot both be empty") + assert ( + error_message + == "'GP Practice ODS code' and 'GP Practice postcode' cannot both be empty" + ) def test_patient_creation_with_future_death_date(): - form = PatientForm({ - "death_date": TODAY + relativedelta(years=1) - }) + form = PatientForm({"death_date": TODAY + relativedelta(years=1)}) errors = form.errors.as_data() - assert("death_date" in errors) + assert "death_date" in errors error_message = errors["death_date"][0].messages[0] - assert(error_message == "Cannot be in the future") + assert error_message == "Cannot be in the future" def test_patient_creation_with_death_date_before_date_of_birth(): - form = PatientForm({ - "date_of_birth": VALID_FIELDS["date_of_birth"], - "death_date": VALID_FIELDS["date_of_birth"] - relativedelta(years=1) - }) + form = PatientForm( + { + "date_of_birth": VALID_FIELDS["date_of_birth"], + "death_date": VALID_FIELDS["date_of_birth"] - relativedelta(years=1), + } + ) errors = form.errors.as_data() - assert("death_date" in errors) + assert "death_date" in errors error_message = errors["death_date"][0].messages[0] - assert(error_message == "'Death Date' cannot be before 'Date of Birth'") + assert error_message == "'Death Date' cannot be before 'Date of Birth'" def test_multiple_date_validation_errors_returned(): - form = PatientForm({ - "date_of_birth": VALID_FIELDS["date_of_birth"], - "diagnosis_date": VALID_FIELDS["date_of_birth"] - relativedelta(years=1), - "death_date": VALID_FIELDS["date_of_birth"] - relativedelta(years=1) - }) + form = PatientForm( + { + "date_of_birth": VALID_FIELDS["date_of_birth"], + "diagnosis_date": VALID_FIELDS["date_of_birth"] - relativedelta(years=1), + "death_date": VALID_FIELDS["date_of_birth"] - relativedelta(years=1), + } + ) errors = form.errors.as_data() - - assert("death_date" in errors) - assert("diagnosis_date" in errors) + + assert "death_date" in errors + assert "diagnosis_date" in errors @pytest.mark.django_db def test_spaces_removed_from_postcode(): - with patch("project.npda.forms.patient_form.validate_postcode") as mock_validate_postcode: - form = PatientForm(VALID_FIELDS | { - "postcode": "WC1X 8SH", - }) + with patch( + "project.npda.forms.patient_form.validate_postcode" + ) as mock_validate_postcode: + form = PatientForm( + VALID_FIELDS + | { + "postcode": "WC1X 8SH", + } + ) form.is_valid() - - assert(len(mock_validate_postcode.call_args_list) == 1) - assert(mock_validate_postcode.call_args_list[0][0][0] == "WC1X8SH") + + assert len(mock_validate_postcode.call_args_list) == 1 + assert mock_validate_postcode.call_args_list[0][0][0] == "WC1X8SH" @pytest.mark.django_db def test_dashes_removed_from_postcode(): - with patch("project.npda.forms.patient_form.validate_postcode") as mock_validate_postcode: - form = PatientForm(VALID_FIELDS | { - "postcode": "WC1X-8SH", - }) + with patch( + "project.npda.forms.patient_form.validate_postcode" + ) as mock_validate_postcode: + form = PatientForm( + VALID_FIELDS + | { + "postcode": "WC1X-8SH", + } + ) form.is_valid() - - assert(len(mock_validate_postcode.call_args_list) == 1) - assert(mock_validate_postcode.call_args_list[0][0][0] == "WC1X8SH") + + assert len(mock_validate_postcode.call_args_list) == 1 + assert mock_validate_postcode.call_args_list[0][0][0] == "WC1X8SH" @pytest.mark.django_db -@patch("project.npda.forms.patient_form.validate_postcode", Mock(return_value={"normalised_postcode":"W1A 1AA"})) +@patch( + "project.npda.forms.patient_form.validate_postcode", + Mock(return_value={"normalised_postcode": "W1A 1AA"}), +) def test_normalised_postcode_saved(): form = PatientForm(VALID_FIELDS) form.is_valid() - assert(form.cleaned_data["postcode"] == "W1A 1AA") + assert form.cleaned_data["postcode"] == "W1A 1AA" @pytest.mark.django_db @@ -243,71 +260,92 @@ def test_invalid_postcode(): form = PatientForm(VALID_FIELDS) form.is_valid() - assert("postcode" in form.errors.as_data()) + assert "postcode" in form.errors.as_data() @pytest.mark.django_db - -@patch("project.npda.forms.patient_form.validate_postcode", Mock(side_effect=RequestException("oopsie!"))) +@patch( + "project.npda.forms.patient_form.validate_postcode", + Mock(side_effect=RequestException("oopsie!")), +) def test_error_validating_postcode(): # TODO MRB: report this back somehow rather than just eat it in the log? form = PatientForm(VALID_FIELDS) form.is_valid() - assert(len(form.errors.as_data()) == 0) + assert len(form.errors.as_data()) == 0 @pytest.mark.django_db -@patch("project.npda.forms.patient_form.gp_ods_code_for_postcode", Mock(return_value=None)) +@patch( + "project.npda.forms.patient_form.gp_ods_code_for_postcode", Mock(return_value=None) +) def test_invalid_gp_postcode(): form = PatientForm(VALID_FIELDS_WITH_GP_POSTCODE) form.is_valid() - assert("gp_practice_postcode" in form.errors.as_data()) + assert "gp_practice_postcode" in form.errors.as_data() @pytest.mark.django_db -@patch("project.npda.forms.patient_form.gp_ods_code_for_postcode", Mock(side_effect=RequestException("oopsie!"))) +@patch( + "project.npda.forms.patient_form.gp_ods_code_for_postcode", + Mock(side_effect=RequestException("oopsie!")), +) def test_error_validating_gp_postcode(): # TODO MRB: report this back somehow rather than just eat it in the log? form = PatientForm(VALID_FIELDS_WITH_GP_POSTCODE) form.is_valid() - assert(len(form.errors.as_data()) == 0) + assert len(form.errors.as_data()) == 0 @pytest.mark.django_db def test_normalised_postcode_used_for_call_to_nhs_spine(): # The NHS API only returns results if you have a space between the parts of the postcode - with patch("project.npda.forms.patient_form.validate_postcode", Mock(return_value={"normalised_postcode":GP_POSTCODE_WITH_SPACES})): - with patch("project.npda.forms.patient_form.gp_ods_code_for_postcode") as mock_gp_ods_code_for_postcode: - form = PatientForm(VALID_FIELDS_WITH_GP_POSTCODE | { - "gp_practice_postcode": GP_POSTCODE_NO_SPACES - }) + with patch( + "project.npda.forms.patient_form.validate_postcode", + Mock(return_value={"normalised_postcode": GP_POSTCODE_WITH_SPACES}), + ): + with patch( + "project.npda.forms.patient_form.gp_ods_code_for_postcode" + ) as mock_gp_ods_code_for_postcode: + form = PatientForm( + VALID_FIELDS_WITH_GP_POSTCODE + | {"gp_practice_postcode": GP_POSTCODE_NO_SPACES} + ) form.is_valid() - assert(len(mock_gp_ods_code_for_postcode.call_args_list) == 1) - assert(mock_gp_ods_code_for_postcode.call_args_list[0][0][0] == GP_POSTCODE_WITH_SPACES) + assert len(mock_gp_ods_code_for_postcode.call_args_list) == 1 + assert ( + mock_gp_ods_code_for_postcode.call_args_list[0][0][0] + == GP_POSTCODE_WITH_SPACES + ) @pytest.mark.django_db -@patch("project.npda.forms.patient_form.gp_details_for_ods_code", Mock(return_value=None)) +@patch( + "project.npda.forms.patient_form.gp_details_for_ods_code", Mock(return_value=None) +) def test_invalid_gp_ods_code(): form = PatientForm(VALID_FIELDS) form.is_valid() - assert("gp_practice_ods_code" in form.errors.as_data()) + assert "gp_practice_ods_code" in form.errors.as_data() @pytest.mark.django_db -@patch("project.npda.forms.patient_form.gp_details_for_ods_code", Mock(side_effect=RequestException("oopsie!"))) +@patch( + "project.npda.forms.patient_form.gp_details_for_ods_code", + Mock(side_effect=RequestException("oopsie!")), +) def test_error_validating_gp_ods_code(): # TODO MRB: report this back somehow rather than just eat it in the log? form = PatientForm(VALID_FIELDS) form.is_valid() - assert(len(form.errors.as_data()) == 0) + assert len(form.errors.as_data()) == 0 @pytest.mark.django_db @@ -315,17 +353,23 @@ def test_lookup_index_of_multiple_deprivation(): form = PatientForm(VALID_FIELDS) form.is_valid() - assert(len(form.errors.as_data()) == 0) + assert len(form.errors.as_data()) == 0 patient = form.save() - assert(patient.index_of_multiple_deprivation_quintile == INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE) + assert ( + patient.index_of_multiple_deprivation_quintile + == INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE + ) @pytest.mark.django_db -@patch("project.npda.models.patient.imd_for_postcode", Mock(side_effect=RequestException("oopsie!"))) +@patch( + "project.npda.models.patient.imd_for_postcode", + Mock(side_effect=RequestException("oopsie!")), +) def test_error_looking_up_index_of_multiple_deprivation(): # TODO MRB: report this back somehow rather than just eat it in the log? form = PatientForm(VALID_FIELDS) patient = form.save() - - patient.index_of_multiple_deprivation_quintile = None \ No newline at end of file + + patient.index_of_multiple_deprivation_quintile = None diff --git a/project/npda/tests/misc_py_shell_code.py b/project/npda/tests/misc_py_shell_code.py index bb92ced5..9709f438 100644 --- a/project/npda/tests/misc_py_shell_code.py +++ b/project/npda/tests/misc_py_shell_code.py @@ -10,7 +10,6 @@ test_user_rcpch_audit_team_data, ) from project.npda.general_functions.rcpch_nhs_organisations import get_nhs_organisation -from project.npda.models import OrganisationEmployer from project.npda.tests.factories.npda_user_factory import NPDAUserFactory from project.constants.user import RCPCH_AUDIT_TEAM @@ -32,7 +31,7 @@ is_staff=False, is_rcpch_audit_team_member=False, is_rcpch_staff=False, - organisation_employers=['RGT01'], + organisation_employers=["RGT01"], groups=[test_user_audit_centre_reader_data.group_name], ) NPDAUserFactory( @@ -43,7 +42,7 @@ is_staff=False, is_rcpch_audit_team_member=False, is_rcpch_staff=False, - organisation_employers=['RGT01'], + organisation_employers=["RGT01"], groups=[test_user_audit_centre_editor_data.group_name], ) @@ -55,7 +54,7 @@ is_staff=False, is_rcpch_audit_team_member=False, is_rcpch_staff=False, - organisation_employers=['RGT01'], + organisation_employers=["RGT01"], groups=[test_user_rcpch_audit_team_data.group_name], ) @@ -68,6 +67,6 @@ is_staff=False, is_rcpch_audit_team_member=False, is_rcpch_staff=False, - organisation_employers=['RGT01'], + organisation_employers=["RGT01"], groups=[test_user_audit_centre_coordinator_data.group_name], ) diff --git a/project/npda/tests/test_csv_upload.py b/project/npda/tests/test_csv_upload.py index bb3c7753..85de7d1e 100644 --- a/project/npda/tests/test_csv_upload.py +++ b/project/npda/tests/test_csv_upload.py @@ -2,12 +2,10 @@ import pandas as pd import nhs_number -from functools import partial from dateutil.relativedelta import relativedelta from unittest.mock import Mock, patch from requests import RequestException -from django.apps import apps from django.core.exceptions import ValidationError from project.npda.models import NPDAUser, Patient, Visit @@ -15,17 +13,29 @@ from project.npda.tests.factories.patient_factory import ( TODAY, VALID_FIELDS, - INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE + INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE, ) # We don't want to call remote services in unit tests @pytest.fixture(autouse=True) def mock_remote_calls(): - with patch("project.npda.forms.patient_form.validate_postcode", Mock(return_value={"normalised_postcode": VALID_FIELDS["postcode"]})): - with patch("project.npda.forms.patient_form.gp_ods_code_for_postcode", Mock(return_value = "G85023")): - with patch("project.npda.forms.patient_form.gp_details_for_ods_code", Mock(return_value = True)): - with patch("project.npda.models.patient.imd_for_postcode", Mock(return_value = INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE)): + with patch( + "project.npda.forms.patient_form.validate_postcode", + Mock(return_value={"normalised_postcode": VALID_FIELDS["postcode"]}), + ): + with patch( + "project.npda.forms.patient_form.gp_ods_code_for_postcode", + Mock(return_value="G85023"), + ): + with patch( + "project.npda.forms.patient_form.gp_details_for_ods_code", + Mock(return_value=True), + ): + with patch( + "project.npda.models.patient.imd_for_postcode", + Mock(return_value=INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE), + ): yield None @@ -34,35 +44,40 @@ def mock_remote_calls(): @pytest.fixture def dummy_sheets_folder(request): - return request.config.rootdir / 'project' / 'npda' / 'dummy_sheets' + return request.config.rootdir / "project" / "npda" / "dummy_sheets" + @pytest.fixture def valid_df(dummy_sheets_folder): - return read_csv(dummy_sheets_folder / 'dummy_sheet.csv') + return read_csv(dummy_sheets_folder / "dummy_sheet.csv") + @pytest.fixture def single_row_valid_df(dummy_sheets_folder): - return read_csv(dummy_sheets_folder / 'dummy_sheet.csv').head(1) + return read_csv(dummy_sheets_folder / "dummy_sheet.csv").head(1) + @pytest.fixture def one_patient_two_visits(dummy_sheets_folder): - df = read_csv(dummy_sheets_folder / 'dummy_sheet.csv').head(2) + df = read_csv(dummy_sheets_folder / "dummy_sheet.csv").head(2) - assert(len(df) == 2) - assert(df["NHS Number"][0] == df["NHS Number"][1]) + assert len(df) == 2 + assert df["NHS Number"][0] == df["NHS Number"][1] return df + @pytest.fixture def two_patients_first_with_two_visits_second_with_one(dummy_sheets_folder): - df = read_csv(dummy_sheets_folder / 'dummy_sheet.csv').head(3) + df = read_csv(dummy_sheets_folder / "dummy_sheet.csv").head(3) - assert(len(df) == 3) - assert(df["NHS Number"][0] == df["NHS Number"][1]) - assert(df["NHS Number"][2] != df["NHS Number"][0]) + assert len(df) == 3 + assert df["NHS Number"][0] == df["NHS Number"][1] + assert df["NHS Number"][2] != df["NHS Number"][0] return df + @pytest.fixture def test_user(seed_groups_fixture, seed_users_fixture): return NPDAUser.objects.filter( @@ -75,11 +90,16 @@ def test_create_patient(test_user, single_row_valid_df): csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) patient = Patient.objects.first() - assert(patient.nhs_number == nhs_number.normalise_number(single_row_valid_df["NHS Number"][0])) - assert(patient.date_of_birth == single_row_valid_df["Date of Birth"][0].date()) - assert(patient.diabetes_type == single_row_valid_df["Diabetes Type"][0]) - assert(patient.diagnosis_date == single_row_valid_df["Date of Diabetes Diagnosis"][0].date()) - assert(patient.death_date is None) + assert patient.nhs_number == nhs_number.normalise_number( + single_row_valid_df["NHS Number"][0] + ) + assert patient.date_of_birth == single_row_valid_df["Date of Birth"][0].date() + assert patient.diabetes_type == single_row_valid_df["Diabetes Type"][0] + assert ( + patient.diagnosis_date + == single_row_valid_df["Date of Diabetes Diagnosis"][0].date() + ) + assert patient.death_date is None @pytest.mark.django_db @@ -90,41 +110,46 @@ def test_create_patient_with_death_date(test_user, single_row_valid_df): csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) patient = Patient.objects.first() - assert(patient.death_date == single_row_valid_df["Death Date"][0].date()) + assert patient.death_date == single_row_valid_df["Death Date"][0].date() @pytest.mark.django_db -def test_multiple_patients(test_user, two_patients_first_with_two_visits_second_with_one): +def test_multiple_patients( + test_user, two_patients_first_with_two_visits_second_with_one +): df = two_patients_first_with_two_visits_second_with_one - assert(df["NHS Number"][0] == df["NHS Number"][1]) - assert(df["NHS Number"][0] != df["NHS Number"][2]) + assert df["NHS Number"][0] == df["NHS Number"][1] + assert df["NHS Number"][0] != df["NHS Number"][2] csv_upload(test_user, df, None, ALDER_HEY_PZ_CODE) - assert(Patient.objects.count() == 2) + assert Patient.objects.count() == 2 [first_patient, second_patient] = Patient.objects.all() - assert(Visit.objects.filter(patient=first_patient).count() == 2) - assert(Visit.objects.filter(patient=second_patient).count() == 1) + assert Visit.objects.filter(patient=first_patient).count() == 2 + assert Visit.objects.filter(patient=second_patient).count() == 1 - assert(first_patient.nhs_number == nhs_number.normalise_number(df["NHS Number"][0])) - assert(first_patient.date_of_birth == df["Date of Birth"][0].date()) - assert(first_patient.diabetes_type == df["Diabetes Type"][0]) - assert(first_patient.diagnosis_date == df["Date of Diabetes Diagnosis"][0].date()) + assert first_patient.nhs_number == nhs_number.normalise_number(df["NHS Number"][0]) + assert first_patient.date_of_birth == df["Date of Birth"][0].date() + assert first_patient.diabetes_type == df["Diabetes Type"][0] + assert first_patient.diagnosis_date == df["Date of Diabetes Diagnosis"][0].date() - assert(second_patient.nhs_number == nhs_number.normalise_number(df["NHS Number"][2])) - assert(second_patient.date_of_birth == df["Date of Birth"][2].date()) - assert(second_patient.diabetes_type == df["Diabetes Type"][2]) - assert(second_patient.diagnosis_date == df["Date of Diabetes Diagnosis"][2].date()) + assert second_patient.nhs_number == nhs_number.normalise_number(df["NHS Number"][2]) + assert second_patient.date_of_birth == df["Date of Birth"][2].date() + assert second_patient.diabetes_type == df["Diabetes Type"][2] + assert second_patient.diagnosis_date == df["Date of Diabetes Diagnosis"][2].date() -@pytest.mark.parametrize("column", [ - pytest.param("NHS Number"), - pytest.param("Date of Birth"), - pytest.param("Diabetes Type"), - pytest.param("Date of Diabetes Diagnosis") -]) +@pytest.mark.parametrize( + "column", + [ + pytest.param("NHS Number"), + pytest.param("Date of Birth"), + pytest.param("Diabetes Type"), + pytest.param("Date of Diabetes Diagnosis"), + ], +) @pytest.mark.django_db def test_missing_mandatory_field(test_user, valid_df, column): valid_df.loc[0, column] = None @@ -133,68 +158,81 @@ def test_missing_mandatory_field(test_user, valid_df, column): csv_upload(test_user, valid_df, None, ALDER_HEY_PZ_CODE) # Catastrophic - we can't save this patient at all so we won't save any of the patients in the submission - assert(Patient.objects.count() == 0) + assert Patient.objects.count() == 0 @pytest.mark.django_db def test_error_in_single_visit(test_user, single_row_valid_df): - single_row_valid_df.loc[0, 'Diabetes Treatment at time of Hba1c measurement'] = 45 + single_row_valid_df.loc[0, "Diabetes Treatment at time of Hba1c measurement"] = 45 with pytest.raises(ValidationError) as e_info: csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) visit = Visit.objects.first() - assert(visit.treatment == 45) - assert("treatment" in visit.errors) + assert visit.treatment == 45 + assert "treatment" in visit.errors @pytest.mark.django_db def test_error_in_multiple_visits(test_user, one_patient_two_visits): df = one_patient_two_visits - df.loc[0, 'Diabetes Treatment at time of Hba1c measurement'] = 45 + df.loc[0, "Diabetes Treatment at time of Hba1c measurement"] = 45 with pytest.raises(ValidationError) as e_info: csv_upload(test_user, df, None, ALDER_HEY_PZ_CODE) - assert(Visit.objects.count() == 2) + assert Visit.objects.count() == 2 - [first_visit, second_visit] = Visit.objects.all().order_by('visit_date') + [first_visit, second_visit] = Visit.objects.all().order_by("visit_date") - assert(first_visit.treatment == 45) - assert("treatment" in first_visit.errors) + assert first_visit.treatment == 45 + assert "treatment" in first_visit.errors - assert(second_visit.treatment == df["Diabetes Treatment at time of Hba1c measurement"][1]) - assert(second_visit.errors is None) + assert ( + second_visit.treatment + == df["Diabetes Treatment at time of Hba1c measurement"][1] + ) + assert second_visit.errors is None @pytest.mark.django_db -def test_multiple_patients_where_one_has_visit_errors_and_the_other_does_not(test_user, two_patients_first_with_two_visits_second_with_one): +def test_multiple_patients_where_one_has_visit_errors_and_the_other_does_not( + test_user, two_patients_first_with_two_visits_second_with_one +): df = two_patients_first_with_two_visits_second_with_one - assert(df["NHS Number"][0] == df["NHS Number"][1]) - assert(df["NHS Number"][0] != df["NHS Number"][2]) + assert df["NHS Number"][0] == df["NHS Number"][1] + assert df["NHS Number"][0] != df["NHS Number"][2] - df.loc[0, 'Diabetes Treatment at time of Hba1c measurement'] = 45 + df.loc[0, "Diabetes Treatment at time of Hba1c measurement"] = 45 with pytest.raises(ValidationError) as e_info: csv_upload(test_user, df, None, ALDER_HEY_PZ_CODE) [patient_one, patient_two] = Patient.objects.all() - assert(Visit.objects.count() == 3) + assert Visit.objects.count() == 3 - [first_visit_for_first_patient, second_visit_for_first_patient] = Visit.objects.filter(patient=patient_one).order_by('visit_date') + [first_visit_for_first_patient, second_visit_for_first_patient] = ( + Visit.objects.filter(patient=patient_one).order_by("visit_date") + ) [visit_for_second_patient] = Visit.objects.filter(patient=patient_two) - assert(first_visit_for_first_patient.treatment == 45) - assert("treatment" in first_visit_for_first_patient.errors) + assert first_visit_for_first_patient.treatment == 45 + assert "treatment" in first_visit_for_first_patient.errors - assert(second_visit_for_first_patient.treatment == df["Diabetes Treatment at time of Hba1c measurement"][1]) - assert(second_visit_for_first_patient.errors is None) + assert ( + second_visit_for_first_patient.treatment + == df["Diabetes Treatment at time of Hba1c measurement"][1] + ) + assert second_visit_for_first_patient.errors is None - assert(visit_for_second_patient.treatment == df["Diabetes Treatment at time of Hba1c measurement"][2]) - assert(visit_for_second_patient.errors is None) + assert ( + visit_for_second_patient.treatment + == df["Diabetes Treatment at time of Hba1c measurement"][2] + ) + assert visit_for_second_patient.errors is None @pytest.mark.django_db @@ -205,7 +243,7 @@ def test_multiple_patients_with_visit_errors(): @pytest.mark.django_db def test_invalid_nhs_number(test_user, single_row_valid_df): invalid_nhs_number = "123456789" - single_row_valid_df["NHS Number"] = invalid_nhs_number + single_row_valid_df["NHS Number"] = invalid_nhs_number with pytest.raises(ValidationError) as e_info: csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) @@ -213,10 +251,10 @@ def test_invalid_nhs_number(test_user, single_row_valid_df): # Not catastrophic - error saved in model and raised back to caller patient = Patient.objects.first() - assert(patient.nhs_number == invalid_nhs_number) + assert patient.nhs_number == invalid_nhs_number # TODO MRB: create a ValidationError model field - assert("nhs_number" in patient.errors) + assert "nhs_number" in patient.errors @pytest.mark.django_db @@ -229,16 +267,16 @@ def test_future_date_of_birth(test_user, single_row_valid_df): patient = Patient.objects.first() - assert(patient.date_of_birth == date_of_birth) - assert("date_of_birth" in patient.errors) + assert patient.date_of_birth == date_of_birth + assert "date_of_birth" in patient.errors - error_message = patient.errors["date_of_birth"][0]['message'] - assert(error_message == "Cannot be in the future") + error_message = patient.errors["date_of_birth"][0]["message"] + assert error_message == "Cannot be in the future" @pytest.mark.django_db def test_over_25(test_user, single_row_valid_df): - date_of_birth = TODAY + - relativedelta(years=25, days=1) + date_of_birth = TODAY + -relativedelta(years=25, days=1) single_row_valid_df["Date of Birth"] = pd.to_datetime(date_of_birth) with pytest.raises(ValidationError) as e_info: @@ -246,24 +284,24 @@ def test_over_25(test_user, single_row_valid_df): patient = Patient.objects.first() - assert(patient.date_of_birth == date_of_birth) - assert("date_of_birth" in patient.errors) + assert patient.date_of_birth == date_of_birth + assert "date_of_birth" in patient.errors - error_message = patient.errors["date_of_birth"][0]['message'] - assert(error_message == "NPDA patients cannot be 25+ years old. This patient is 25") + error_message = patient.errors["date_of_birth"][0]["message"] + assert error_message == "NPDA patients cannot be 25+ years old. This patient is 25" @pytest.mark.django_db def test_invalid_diabetes_type(test_user, single_row_valid_df): - single_row_valid_df["Diabetes Type"] = 45 + single_row_valid_df["Diabetes Type"] = 45 with pytest.raises(ValidationError) as e_info: csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) patient = Patient.objects.first() - assert(patient.diabetes_type == 45) - assert("diabetes_type" in patient.errors) + assert patient.diabetes_type == 45 + assert "diabetes_type" in patient.errors @pytest.mark.django_db @@ -276,16 +314,16 @@ def test_future_diagnosis_date(test_user, single_row_valid_df): patient = Patient.objects.first() - assert(patient.diagnosis_date == diagnosis_date) - assert("diagnosis_date" in patient.errors) + assert patient.diagnosis_date == diagnosis_date + assert "diagnosis_date" in patient.errors - error_message = patient.errors["diagnosis_date"][0]['message'] - assert(error_message == "Cannot be in the future") + error_message = patient.errors["diagnosis_date"][0]["message"] + assert error_message == "Cannot be in the future" @pytest.mark.django_db def test_diagnosis_date_before_date_of_birth(test_user, single_row_valid_df): - date_of_birth = VALID_FIELDS["date_of_birth"], + date_of_birth = (VALID_FIELDS["date_of_birth"],) diagnosis_date = VALID_FIELDS["date_of_birth"] - relativedelta(years=1) single_row_valid_df["Date of Diabetes Diagnosis"] = pd.to_datetime(diagnosis_date) @@ -295,60 +333,65 @@ def test_diagnosis_date_before_date_of_birth(test_user, single_row_valid_df): patient = Patient.objects.first() - assert(patient.diagnosis_date == diagnosis_date) - assert("diagnosis_date" in patient.errors) + assert patient.diagnosis_date == diagnosis_date + assert "diagnosis_date" in patient.errors - error_message = patient.errors["diagnosis_date"][0]['message'] + error_message = patient.errors["diagnosis_date"][0]["message"] # TODO MRB: why does this have entity encoding issues? - assert(error_message == "'Date of Diabetes Diagnosis' cannot be before 'Date of Birth'") + assert ( + error_message + == "'Date of Diabetes Diagnosis' cannot be before 'Date of Birth'" + ) @pytest.mark.django_db def test_invalid_sex(test_user, single_row_valid_df): - single_row_valid_df["Stated gender"] = 45 + single_row_valid_df["Stated gender"] = 45 with pytest.raises(ValidationError) as e_info: csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) patient = Patient.objects.first() - assert(patient.sex == 45) - assert("sex" in patient.errors) + assert patient.sex == 45 + assert "sex" in patient.errors @pytest.mark.django_db def test_invalid_ethnicity(test_user, single_row_valid_df): - single_row_valid_df["Ethnic Category"] = "45" + single_row_valid_df["Ethnic Category"] = "45" with pytest.raises(ValidationError) as e_info: csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) patient = Patient.objects.first() - assert(patient.ethnicity == "45") - assert("ethnicity" in patient.errors) + assert patient.ethnicity == "45" + assert "ethnicity" in patient.errors @pytest.mark.django_db def test_missing_gp_ods_code(test_user, single_row_valid_df): - single_row_valid_df["GP Practice Code"] = None + single_row_valid_df["GP Practice Code"] = None with pytest.raises(ValidationError) as e_info: csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) patient = Patient.objects.first() - assert("gp_practice_ods_code" in patient.errors) - - error_message = patient.errors["gp_practice_ods_code"][0]['message'] - # TODO MRB: why does this have entity encoding issues? - assert(error_message == "'GP Practice ODS code' and 'GP Practice postcode' cannot both be empty") + assert "gp_practice_ods_code" in patient.errors + error_message = patient.errors["gp_practice_ods_code"][0]["message"] + # TODO MRB: why does this have entity encoding issues? + assert ( + error_message + == "'GP Practice ODS code' and 'GP Practice postcode' cannot both be empty" + ) @pytest.mark.django_db def test_future_death_date(test_user, single_row_valid_df): - death_date = TODAY + relativedelta(days = 1) + death_date = TODAY + relativedelta(days=1) single_row_valid_df["Death Date"] = pd.to_datetime(death_date) @@ -357,16 +400,16 @@ def test_future_death_date(test_user, single_row_valid_df): patient = Patient.objects.first() - assert(patient.death_date == death_date) - assert("death_date" in patient.errors) + assert patient.death_date == death_date + assert "death_date" in patient.errors - error_message = patient.errors["death_date"][0]['message'] - assert(error_message == "Cannot be in the future") + error_message = patient.errors["death_date"][0]["message"] + assert error_message == "Cannot be in the future" @pytest.mark.django_db def test_death_date_before_date_of_birth(test_user, single_row_valid_df): - date_of_birth = VALID_FIELDS["date_of_birth"], + date_of_birth = (VALID_FIELDS["date_of_birth"],) death_date = VALID_FIELDS["date_of_birth"] - relativedelta(years=1) single_row_valid_df["Death Date"] = pd.to_datetime(death_date) @@ -376,12 +419,15 @@ def test_death_date_before_date_of_birth(test_user, single_row_valid_df): patient = Patient.objects.first() - assert(patient.death_date == death_date) - assert("death_date" in patient.errors) + assert patient.death_date == death_date + assert "death_date" in patient.errors - error_message = patient.errors["death_date"][0]['message'] + error_message = patient.errors["death_date"][0]["message"] # TODO MRB: why does this have entity encoding issues? - assert(error_message == "'Death Date' cannot be before 'Date of Birth'") + assert ( + error_message + == "'Death Date' cannot be before 'Date of Birth'" + ) @pytest.mark.django_db @@ -391,61 +437,75 @@ def test_invalid_postcode(test_user, single_row_valid_df): with pytest.raises(ValidationError) as e_info: csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) - + patient = Patient.objects.first() - assert(patient.postcode == "not a postcode") - assert("postcode" in patient.errors) + assert patient.postcode == "not a postcode" + assert "postcode" in patient.errors @pytest.mark.django_db -@patch("project.npda.forms.patient_form.validate_postcode", Mock(side_effect=RequestException("oopsie!"))) +@patch( + "project.npda.forms.patient_form.validate_postcode", + Mock(side_effect=RequestException("oopsie!")), +) def test_error_validating_postcode(test_user, single_row_valid_df): single_row_valid_df["Postcode of usual address"] = "WC1X 8SH" csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) - + patient = Patient.objects.first() - assert(patient.postcode == "WC1X8SH") + assert patient.postcode == "WC1X8SH" @pytest.mark.django_db -@patch("project.npda.forms.patient_form.gp_details_for_ods_code", Mock(return_value=None)) +@patch( + "project.npda.forms.patient_form.gp_details_for_ods_code", Mock(return_value=None) +) def test_invalid_gp_ods_code(test_user, single_row_valid_df): single_row_valid_df["GP Practice Code"] = "not a GP code" with pytest.raises(ValidationError) as e_info: csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) - + patient = Patient.objects.first() - assert(patient.gp_practice_ods_code == "not a GP code") - assert("gp_practice_ods_code" in patient.errors) + assert patient.gp_practice_ods_code == "not a GP code" + assert "gp_practice_ods_code" in patient.errors @pytest.mark.django_db -@patch("project.npda.forms.patient_form.gp_details_for_ods_code", Mock(side_effect=RequestException("oopsie!"))) +@patch( + "project.npda.forms.patient_form.gp_details_for_ods_code", + Mock(side_effect=RequestException("oopsie!")), +) def test_error_validating_gp_ods_code(test_user, single_row_valid_df): single_row_valid_df["GP Practice Code"] = "G85023" csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) - + patient = Patient.objects.first() - assert(patient.gp_practice_ods_code == "G85023") + assert patient.gp_practice_ods_code == "G85023" @pytest.mark.django_db def test_lookup_index_of_multiple_deprivation(test_user, single_row_valid_df): csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) - + patient = Patient.objects.first() - assert(patient.index_of_multiple_deprivation_quintile == INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE) + assert ( + patient.index_of_multiple_deprivation_quintile + == INDEX_OF_MULTIPLE_DEPRIVATION_QUINTILE + ) @pytest.mark.django_db -@patch("project.npda.models.patient.imd_for_postcode", Mock(side_effect=RequestException("oopsie!"))) +@patch( + "project.npda.models.patient.imd_for_postcode", + Mock(side_effect=RequestException("oopsie!")), +) def test_error_looking_up_index_of_multiple_deprivation(test_user, single_row_valid_df): csv_upload(test_user, single_row_valid_df, None, ALDER_HEY_PZ_CODE) - + patient = Patient.objects.first() - assert(patient.index_of_multiple_deprivation_quintile is None) + assert patient.index_of_multiple_deprivation_quintile is None diff --git a/project/npda/tests/test_meta_tests.py b/project/npda/tests/test_meta_tests.py index 75b01b31..fd8a5a4c 100644 --- a/project/npda/tests/test_meta_tests.py +++ b/project/npda/tests/test_meta_tests.py @@ -20,7 +20,6 @@ ) from project.npda.tests.factories.patient_factory import PatientFactory from project.npda.general_functions import print_instance_field_attrs -from project.settings import LOGGING # logging logger = logging.getLogger(__name__) @@ -28,9 +27,6 @@ @pytest.mark.django_db def test__seed_test_db( - seed_groups_fixture, - seed_users_fixture, - seed_patients_fixture, ): assert Group.objects.all().exists() assert OrganisationEmployer.objects.all().exists() @@ -39,9 +35,6 @@ def test__seed_test_db( @pytest.mark.django_db def test_patient_creation( - seed_groups_fixture, - seed_users_fixture, - seed_patients_fixture, ): """Test Patient Factory creation.""" @@ -53,9 +46,6 @@ def test_patient_creation( @pytest.mark.django_db def test__multiple_PaediatricsDiabetesUnitFactory_instances_not_created( - seed_groups_fixture, - seed_users_fixture, - seed_patients_fixture, ): """ Both Patient and NPDAUser factories eventually create and are associated with a PaediatricsDiabetesUnit instance. diff --git a/project/npda/tests/test_user_actions/test_user_creation.py b/project/npda/tests/test_user_actions/test_user_creation.py index 64d1ee2b..c3e71498 100644 --- a/project/npda/tests/test_user_actions/test_user_creation.py +++ b/project/npda/tests/test_user_actions/test_user_creation.py @@ -17,9 +17,6 @@ @pytest.mark.skip(reason="Test not yet implemented - just for setting things up") @pytest.mark.django_db def test_user_creation( - seed_groups_fixture, - seed_users_fixture, - seed_patients_fixture, client, ): """Test user can create in same organisation""" From c24c52c9b07bae9b7d495d5f57b8d6761fdca388 Mon Sep 17 00:00:00 2001 From: eatyourpeas Date: Mon, 14 Oct 2024 20:28:10 +0100 Subject: [PATCH 09/10] fix tests broken by removing fixtures --- .../tests/permissions_tests/test_npda_user_model_actions.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py index 52e42dd6..18c762e0 100644 --- a/project/npda/tests/permissions_tests/test_npda_user_model_actions.py +++ b/project/npda/tests/permissions_tests/test_npda_user_model_actions.py @@ -50,6 +50,7 @@ def check_all_users_in_pdu(user, users, pz_code): @pytest.mark.django_db def test_npda_user_list_view_users_can_only_see_users_from_their_pdu( + seed_users_fixture, client, ): """Except for RCPCH_AUDIT_TEAM, users should only see users from their own PDU.""" @@ -78,6 +79,7 @@ def test_npda_user_list_view_users_can_only_see_users_from_their_pdu( @pytest.mark.django_db def test_npda_user_list_view_rcpch_audit_team_can_view_all_users( + seed_users_fixture, client, ): """RCPCH_AUDIT_TEAM users can view all users.""" @@ -117,6 +119,7 @@ def test_npda_user_list_view_rcpch_audit_team_can_view_all_users( @pytest.mark.django_db def test_npda_user_list_view_users_cannot_switch_outside_their_pdu( + seed_users_fixture, client, ): ah_user = NPDAUser.objects.filter( From 828555b3bafbfe34160c17dbb1c10b2864203fb8 Mon Sep 17 00:00:00 2001 From: eatyourpeas Date: Mon, 14 Oct 2024 21:27:28 +0100 Subject: [PATCH 10/10] increase pytest verbose flag --- .github/workflows/pytest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 065aca5f..55154191 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -31,4 +31,4 @@ jobs: - name: Run tests run: | - docker compose exec django pytest -v + docker compose exec django pytest -vv