Merge pull request #824 from NHSDigital/DTOSS-11781-cleanup-3

refactor medical history views

Author: MatMoore

manage_breast_screening/core/tests/views/__init__.py

@@ -0,0 +1,140 @@
+from unittest.mock import MagicMock
+
+import pytest
+from django import forms
+from django.contrib.contenttypes.models import ContentType
+from django.contrib.messages.middleware import MessageMiddleware
+from django.contrib.sessions.middleware import SessionMiddleware
+from django.test import RequestFactory
+
+from manage_breast_screening.auth.tests.factories import UserFactory
+from manage_breast_screening.core.models import AuditLog
+from manage_breast_screening.core.views.generic import (
+    AddWithAuditView,
+    UpdateWithAuditView,
+)
+from manage_breast_screening.users.models import User
+
+
+def apply_middleware(request):
+    for middleware_class in [SessionMiddleware, MessageMiddleware]:
+        middleware = middleware_class(get_response=MagicMock())
+        middleware.process_request(request)
+    return request
+
+
+class DummyForm(forms.Form):
+    first_name = forms.CharField()
+    last_name = forms.CharField()
+
+    def __init__(self, *args, instance=None, **kwargs):
+        self.instance = instance
+        super().__init__(*args, **kwargs)
+
+    def create(self):
+        return UserFactory.create(
+            first_name=self.cleaned_data["first_name"],
+            last_name=self.cleaned_data["last_name"],
+            nhs_uid="create_uid",
+        )
+
+    def update(self):
+        return UserFactory.create(
+            first_name=self.cleaned_data["first_name"],
+            last_name=self.cleaned_data["last_name"],
+            nhs_uid="update_uid",
+        )
+
+
+class AddView(AddWithAuditView):
+    form_class = DummyForm
+    thing_name = "test"
+    success_url = "/success"
+
+
+class UpdateView(UpdateWithAuditView):
+    form_class = DummyForm
+    thing_name = "test"
+    success_url = "/success"
+
+
+class TestAddWithAuditView:
+    def test_success_message_content(self):
+        obj = UserFactory.build()
+
+        assert AddView().get_success_message_content(obj) == "Added test"
+
+    def test_get_context_data(self):
+        request = RequestFactory().get("/")
+        context = AddView(request=request).get_context_data()
+
+        assert context["heading"] == "Add test"
+        assert context["page_title"] == "Add test"
+
+    @pytest.mark.django_db
+    def test_audits_if_form_valid(self):
+        request = apply_middleware(RequestFactory().post("/"))
+        request.user = UserFactory()
+        form = DummyForm({"first_name": "abc", "last_name": "def"})
+
+        assert form.is_valid()
+        AddView(request=request).form_valid(form)
+
+        last_audit = AuditLog.objects.last()
+        assert last_audit is not None
+        assert last_audit.content_type == ContentType.objects.get_for_model(User)
+        assert last_audit.actor == request.user
+        assert last_audit.operation == AuditLog.Operations.CREATE
+        assert last_audit.snapshot == {
+            "email": "",
+            "first_name": "abc",
+            "last_name": "def",
+            "is_staff": False,
+            "is_active": True,
+            "last_login": None,
+            "nhs_uid": "create_uid",
+        }
+        assert last_audit.system_update_id is None
+
+
+class TestUpdateWithAuditView:
+    def test_success_message_content(self):
+        obj = UserFactory.build()
+
+        assert UpdateView().get_success_message_content(obj) == "Updated test"
+
+    def test_get_context_data(self):
+        obj = UserFactory.build()
+        request = RequestFactory().get("/")
+        view = UpdateView(request=request)
+        view.object = obj
+
+        context = view.get_context_data()
+
+        assert context["heading"] == "Edit test"
+        assert context["page_title"] == "Edit test"
+
+    @pytest.mark.django_db
+    def test_audits_if_form_valid(self):
+        request = apply_middleware(RequestFactory().post("/"))
+        request.user = UserFactory()
+        form = DummyForm({"first_name": "new", "last_name": "name"})
+
+        assert form.is_valid()
+        UpdateView(request=request).form_valid(form)
+
+        last_audit = AuditLog.objects.last()
+        assert last_audit is not None
+        assert last_audit.content_type == ContentType.objects.get_for_model(User)
+        assert last_audit.actor == request.user
+        assert last_audit.operation == AuditLog.Operations.UPDATE
+        assert last_audit.snapshot == {
+            "email": "",
+            "first_name": "new",
+            "last_name": "name",
+            "is_staff": False,
+            "is_active": True,
+            "last_login": None,
+            "nhs_uid": "update_uid",
+        }
+        assert last_audit.system_update_id is None

manage_breast_screening/core/tests/views/test_generic.py

@@ -1,11 +1,174 @@
+import logging
+
 from django.contrib import messages
+from django.http import Http404
 from django.shortcuts import redirect
-from django.views.generic import DeleteView
+from django.views.generic import DeleteView, FormView
+from django.views.generic.detail import SingleObjectMixin
 
 from ..services.auditor import Auditor
 
+logger = logging.getLogger(__name__)
+
+
+class NamedThingMixin:
+    """
+    A helper to generate consistent messages and titles.
+    If needed, individual messages can be overriden in subclasses.
+    """
+
+    thing_name = None
+
+    def add_title(self, thing_name):
+        return f"Add {thing_name}"
+
+    def added_message(self, thing_name):
+        return f"Added {thing_name}"
+
+    def update_title(self, thing_name):
+        return f"Edit {thing_name}"
+
+    def updated_message(self, thing_name):
+        return f"Updated {thing_name}"
+
+    def delete_button_text(self, thing_name):
+        return f"Delete {thing_name}"
+
+    def deleted_message_text(self, thing_name):
+        return f"Deleted {thing_name}"
+
+    def confirm_delete_link_text(self, thing_name):
+        return f"Delete this {thing_name}"
+
+    def get_thing_name(self, instance=None):
+        """
+        The name of the thing in lowercase, e.g. "appointment"
+        """
+        if self.thing_name:
+            return self.thing_name
+
+        raise ValueError("thing_name is unset")
+
+
+class AddWithAuditView(NamedThingMixin, FormView):
+    """
+    A generic view that adds an object, similar to django.views.generic.CreateView but not based on ModelForms.
+    An audit record is created and a success message is shown on redirect.
+
+    If valid, the form's create() method will be called.
+    """
+
+    template_name = "layout-form.jinja"
+
+    def get_success_message_content(self, object):
+        return self.added_message(thing_name=self.get_thing_name(object))
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data()
+
+        title = self.add_title(thing_name=self.get_thing_name())
+
+        context.update(
+            {
+                "heading": title,
+                "page_title": title,
+            },
+        )
+
+        return context
+
+    def get_create_kwargs(self):
+        return {}
+
+    def form_valid(self, form):
+        created_object = form.create(**self.get_create_kwargs())
+
+        auditor = Auditor.from_request(self.request)
+        auditor.audit_create(created_object)
+
+        messages.add_message(
+            self.request,
+            messages.SUCCESS,
+            self.get_success_message_content(created_object),
+        )
+
+        return super().form_valid(form)
+
+
+class UpdateWithAuditView(NamedThingMixin, SingleObjectMixin, FormView):
+    """
+    A generic view that updates an object, similar to django.views.generic.UpdateView but not based on ModelForms.
+    An audit record is created and a success message is shown on redirect.
+
+    The form's constructor is expected to have an `instance` parameter.
+    If valid, the form's update() method will be called.
+    """
+
+    template_name = "layout-form.jinja"
+
+    def get_success_message_content(self, object):
+        return self.updated_message(thing_name=self.get_thing_name(object))
+
+    def get_form_kwargs(self):
+        kwargs = super().get_form_kwargs()
+        kwargs["instance"] = self.object
+        return kwargs
+
+    def get(self, request, *args, **kwargs):
+        self.object = self.get_object()
+        if self.object is None:
+            logger.warning("Object does not exist, redirecting to success URL")
+            return redirect(self.get_success_url())
+        return super().get(request, *args, **kwargs)
+
+    def post(self, request, *args, **kwargs):
+        self.object = self.get_object()
+        if self.object is None:
+            raise Http404
+        return super().post(request, *args, **kwargs)
+
+    def get_delete_url(self):
+        return None
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data()
+
+        thing_name = self.get_thing_name(self.object)
+        title = self.update_title(thing_name=thing_name)
+
+        delete_href = self.get_delete_url()
+        if delete_href:
+            context["delete_link"] = {
+                "text": self.confirm_delete_link_text(thing_name=thing_name),
+                "class": "nhsuk-link app-link--warning",
+                "href": delete_href,
+            }
+
+        context.update(
+            {
+                "heading": title,
+                "page_title": title,
+            },
+        )
+
+        return context
+
+    def form_valid(self, form):
+        created_object = form.update()
 
-class DeleteWithAuditView(DeleteView):
+        auditor = Auditor.from_request(self.request)
+        auditor.audit_update(created_object)
+
+        messages.add_message(
+            self.request,
+            messages.SUCCESS,
+            self.get_success_message_content(created_object),
+        )
+
+        return super().form_valid(form)
+
+
+class DeleteWithAuditView(NamedThingMixin, DeleteView):
     """
     A generic delete view with a confirmation page, success message,
     and some default context variables.
@@ -14,25 +177,24 @@ class DeleteWithAuditView(DeleteView):
     """
 
     template_name = "layout-confirmation.jinja"
-
-    def get_thing_name(self, object):
-        return object._meta.verbose_name
+    title_pattern = "Are you sure you want to delete this {thing_name}"
 
     def get_success_message_content(self, object):
-        return f"Deleted {self.get_thing_name(object)}"
+        return self.deleted_message_text(thing_name=self.get_thing_name(object))
 
     def get_cancel_url(self):
         return self.get_success_url()
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
         thing_name = self.get_thing_name(self.object)
+        title = self.title_pattern.format(thing_name=thing_name)
         context.update(
             {
-                "page_title": f"Are you sure you want to delete this {thing_name}?",
-                "heading": f"Are you sure you want to delete this {thing_name}?",
+                "page_title": title,
+                "heading": title,
                 "confirm_action": {
-                    "text": f"Delete {thing_name}",
+                    "text": self.delete_button_text(thing_name=thing_name),
                     "href": self.request.get_full_path(),
                 },
                 "cancel_action": {"href": self.get_cancel_url()},

manage_breast_screening/core/views/generic.py

@@ -1,6 +1,5 @@
 from django.forms import Textarea
 
-from manage_breast_screening.core.services.auditor import Auditor
 from manage_breast_screening.nhsuk_forms.fields.char_field import CharField
 from manage_breast_screening.nhsuk_forms.fields.choice_fields import (
     ChoiceField,
@@ -132,9 +131,7 @@ def initial_values(self, instance):
 
         return initial
 
-    def create(self, appointment, request):
-        auditor = Auditor.from_request(request)
-
+    def create(self, appointment):
         benign_lump_history_item = BenignLumpHistoryItem.objects.create(
             appointment=appointment,
             left_breast_procedures=self.cleaned_data.get("left_breast_procedures", []),
@@ -147,16 +144,12 @@ def create(self, appointment, request):
             additional_details=self.cleaned_data.get("additional_details", ""),
         )
 
-        auditor.audit_create(benign_lump_history_item)
-
         return benign_lump_history_item
 
-    def update(self, request):
+    def update(self):
         if self.instance is None:
             raise ValueError("Form has no instance")
 
-        auditor = Auditor.from_request(request)
-
         self.instance.left_breast_procedures = self.cleaned_data.get(
             "left_breast_procedures", []
         )
@@ -171,7 +164,6 @@ def update(self, request):
         )
 
         self.instance.save()
-        auditor.audit_update(self.instance)
 
         return self.instance