TP2000-1404: Reorganise measures and common code (#1266)

* Split up common views

* Split up measures views and forms code

* Add imports to module __init__ files

* Fix test

* Remove empty files

* Tidy up common views imports

* Revert imports

* Revert import

* Revert comment

* Fix import

.pre-commit-config.yaml

@@ -11,6 +11,7 @@ repos:
             "--in-place",
             "--remove-all-unused-imports",
             "--remove-unused-variable",
+            "--ignore-init-module-imports",
           ]
   - repo: https://github.com/pycqa/isort
     rev: 5.13.2

common/forms.py

@@ -1,6 +1,8 @@
 import re
 from collections import defaultdict
 from datetime import date
+from typing import Dict
+from typing import List
 from typing import Type
 
 from crispy_forms_gds.fields import DateInputField
@@ -792,3 +794,101 @@ def __init__(self, *args, **kwargs):
                 css_id="homepage-search-form",
             ),
         )
+
+
+class SerializableFormMixin:
+    """Provides a default implementation of `serializable_data()` that can be
+    used to obtain form data that can be serialized, or more specifically,
+    stored to a `JSONField` field."""
+
+    ignored_data_key_regexs = [
+        "^csrfmiddlewaretoken$",
+        "^measure_create_wizard-current_step$",
+        "^submit$",
+        "-ADD$",
+        "-DELETE$",
+        "_autocomplete$",
+        "INITIAL_FORMS$",
+        "MAX_NUM_FORMS$",
+        "MIN_NUM_FORMS$",
+        "TOTAL_FORMS$",
+    ]
+    """
+    Regexs of keys that may appear in a Form's `data` dictionary attribute and
+    which should be ignored when creating a serializable version of `data`.
+
+    Override this on a per form basis if there are other, redundant keys that
+    should be ignored. See the default implementation of
+    `SerializableFormMixin.get_serializable_data_keys()` to see how this class
+    attribute is used.
+    """
+
+    def get_serializable_data_keys(self) -> List[str]:
+        """
+        Default implementation returning a list of the `Form.data` attribute's
+        keys used when serializing `data`.
+
+        Override this function if neither `ignored_data_key_regexs` or this
+        default implementation is sufficient for identifying which of
+        `Form.data`'s keys should be used during a call to this mixin's
+        `serializable_data()` method.
+        """
+        combined_regexs = "(" + ")|(".join(self.ignored_data_key_regexs) + ")"
+        return [k for k in self.data.keys() if not re.search(combined_regexs, k)]
+
+    def serializable_data(self, remove_key_prefix: str = "") -> Dict:
+        """
+        Return serializable form data that can be serialized / stored as, say,
+        `django.db.models.JSONField` which can be used to recreate a valid form.
+
+        If `remove_key_prefix` is a non-empty string, then the keys in the
+        returned dictionary will be stripped of that string where it appears as
+        a key prefix in the origin `data` dictionary.
+
+        Note that this method should only be used immediately after a successful
+        call to the Form's is_valid() if the data that it returns is to be used
+        to recreate a valid form.
+        """
+        serialized_data = {}
+        data_keys = self.get_serializable_data_keys()
+
+        for data_key in data_keys:
+            serialized_key = data_key
+
+            if (
+                remove_key_prefix
+                and len(remove_key_prefix) < len(data_key)
+                and data_key.startswith(remove_key_prefix)
+            ):
+                prefix = f"{remove_key_prefix}-"
+                serialized_key = data_key.replace(prefix, "")
+
+            serialized_data[serialized_key] = self.data[data_key]
+
+        return serialized_data
+
+    @classmethod
+    def serializable_init_kwargs(cls, kwargs: Dict) -> Dict:
+        """
+        Get a serializable dictionary of arguments that can be used to
+        initialise the form. The `kwargs` parameter is the Python version of
+        kwargs that are used to initialise the form and is normally provided by
+        the same caller as would init the form (i.e. the view).
+
+        For instance, a SelectableObjectsForm subclass
+        requires a valid `objects` parameter to correctly construct and
+        validate the form, so we'd expect `kwargs` dictionary containing
+        an `objects` element.
+        """
+        return {}
+
+    @classmethod
+    def deserialize_init_kwargs(cls, form_kwargs: Dict) -> Dict:
+        """
+        Get a dictionary of arguments for use in initialising the form.
+
+        The 'form_kwargs` parameter is the serialized (actually, serializable)
+        version of the form's kwargs that require deserializing to their Python
+        representation.
+        """
+        return {}

common/tests/test_views.py

@@ -62,7 +62,10 @@ def test_index_displays_login_buttons_correctly_SSO_on(valid_user_client):
         ("common.views.HealthCheckView.check_s3", ("Not OK", 503)),
     ],
 )
-@patch("common.views.HealthCheckView.check_celery_broker", return_value=("OK", 200))
+@patch(
+    "common.views.HealthCheckView.check_celery_broker",
+    return_value=("OK", 200),
+)
 @patch("common.views.HealthCheckView.check_s3", return_value=("OK", 200))
 def test_health_check_view_response(
     check_celery_broker_mock,
@@ -116,7 +119,7 @@ def test_app_info_superuser(superuser_client, new_workbasket):
         },
     ]
 
-    with patch("common.views.sqlite_dumps", return_value=sqlite_dumps):
+    with patch("common.views.pages.sqlite_dumps", return_value=sqlite_dumps):
         response = superuser_client.get(reverse("app-info"))
 
     assert response.status_code == 200

common/views/__init__.py

@@ -0,0 +1,3 @@
+from .base import *
+from .mixins import *
+from .pages import *

common/views/base.py

@@ -0,0 +1,86 @@
+from typing import Optional
+
+from django.contrib.auth.mixins import PermissionRequiredMixin
+from django.db import transaction
+from django.template.response import TemplateResponse
+from django.views.generic import DetailView
+from django.views.generic.edit import FormMixin
+from django_filters.views import FilterView
+
+from common.validators import UpdateType
+from workbaskets.views.mixins import WithCurrentWorkBasket
+
+from .mixins import BusinessRulesMixin
+from .mixins import TrackedModelDetailMixin
+from .mixins import WithPaginationListMixin
+
+
+def handler403(request, *args, **kwargs):
+    return TemplateResponse(request=request, template="common/403.jinja", status=403)
+
+
+def handler500(request, *args, **kwargs):
+    return TemplateResponse(request=request, template="common/500.jinja", status=500)
+
+
+class WithPaginationListView(WithPaginationListMixin, FilterView):
+    """Generic filtered list view enabling pagination."""
+
+
+class TamatoListView(WithCurrentWorkBasket, WithPaginationListView):
+    """Base view class for listing tariff components including those in the
+    current workbasket, with pagination."""
+
+
+class TrackedModelDetailView(
+    WithCurrentWorkBasket,
+    TrackedModelDetailMixin,
+    DetailView,
+):
+    """Base view class for displaying a single TrackedModel."""
+
+
+class TrackedModelChangeView(
+    WithCurrentWorkBasket,
+    PermissionRequiredMixin,
+    BusinessRulesMixin,
+):
+    update_type: UpdateType
+    success_path: Optional[str] = None
+
+    @property
+    def success_url(self):
+        return self.object.get_url(self.success_path)
+
+    def get_result_object(self, form):
+        """
+        Overridable used to get a saved result.
+
+        In the default case (this implementation) a new version of a
+        TrackedModel instance is created. However, this function may be
+        overridden to provide alternative behaviour, such as simply updating the
+        TrackedModel instance.
+        """
+        # compares changed data against model fields to prevent unexpected kwarg TypeError
+        # e.g. `geographical_area_group` is a field on `MeasureUpdateForm` and included in cleaned data,
+        # but isn't a field on `Measure` and would cause a TypeError on model save()
+        model_fields = [f.name for f in self.model._meta.get_fields()]
+        form_changed_data = [f for f in form.changed_data if f in model_fields]
+        changed_data = {name: form.cleaned_data[name] for name in form_changed_data}
+
+        return form.instance.new_version(
+            workbasket=self.workbasket,
+            update_type=self.update_type,
+            **changed_data,
+        )
+
+    @transaction.atomic
+    def form_valid(self, form):
+        self.object = self.get_result_object(form)
+        violations = self.form_violates(form)
+
+        if violations:
+            transaction.set_rollback(True)
+            return self.form_invalid(form)
+
+        return FormMixin.form_valid(self, form)

common/views/mixins.py

@@ -0,0 +1,164 @@
+from typing import Optional
+from typing import Tuple
+from typing import Type
+
+from django.conf import settings
+from django.contrib.auth.mixins import UserPassesTestMixin
+from django.core.paginator import Paginator
+from django.db.models import Model
+from django.db.models import QuerySet
+from django.http import Http404
+
+from common.business_rules import BusinessRule
+from common.business_rules import BusinessRuleViolation
+from common.models import TrackedModel
+from common.pagination import build_pagination_list
+
+
+class WithPaginationListMixin:
+    """Mixin that can be inherited by a ListView subclass to enable this
+    project's pagination capabilities."""
+
+    paginator_class = Paginator
+    paginate_by = settings.REST_FRAMEWORK["PAGE_SIZE"]
+
+    def get_context_data(self, *, object_list=None, **kwargs):
+        """Adds a page link list to the context."""
+        data = super().get_context_data(object_list=object_list, **kwargs)
+        page_obj = data["page_obj"]
+        page_number = page_obj.number
+        data["page_links"] = build_pagination_list(
+            page_number,
+            page_obj.paginator.num_pages,
+        )
+        return data
+
+
+class RequiresSuperuserMixin(UserPassesTestMixin):
+    """Only allow superusers to see this view."""
+
+    def test_func(self):
+        return self.request.user.is_superuser
+
+
+class TrackedModelDetailMixin:
+    """Allows detail URLs to use <Identifying-Fields> instead of <pk>"""
+
+    model: Type[TrackedModel]
+    required_url_kwargs = None
+
+    def get_object(self, queryset: Optional[QuerySet] = None) -> Model:
+        """
+        Fetch the model instance by primary key or by identifying_fields in the
+        URL.
+
+        :param queryset Optional[QuerySet]: Get the object from this queryset
+        :rtype: Model
+        """
+        if queryset is None:
+            queryset = self.get_queryset()
+
+        required_url_kwargs = self.required_url_kwargs or self.model.identifying_fields
+
+        if any(key not in self.kwargs for key in required_url_kwargs):
+            raise AttributeError(
+                f"{self.__class__.__name__} must be called with {', '.join(required_url_kwargs)} in the URLconf.",
+            )
+
+        queryset = queryset.filter(**self.kwargs)
+
+        try:
+            obj = queryset.get()
+        except queryset.model.DoesNotExist:
+            raise Http404(f"No {self.model.__name__} matching the query {self.kwargs}")
+
+        return obj
+
+
+class BusinessRulesMixin:
+    """Check business rules on form_submission."""
+
+    validate_business_rules: Tuple[Type[BusinessRule], ...] = tuple()
+
+    def form_violates(self, form, transaction=None) -> bool:
+        """
+        If any of the specified business rules are violated, reshow the form
+        with the violations as form errors.
+
+        :param form: The submitted form
+        :param transaction: The transaction containing the version of the object to be validated. Defaults to `self.object.transaction`
+        """
+        violations = False
+        transaction = transaction or self.object.transaction
+
+        for rule in self.validate_business_rules:
+            try:
+                rule(transaction).validate(self.object)
+            except BusinessRuleViolation as v:
+                form.add_error(None, v.args[0])
+                violations = True
+
+        return violations
+
+    def form_valid(self, form):
+        if self.form_violates(form):
+            return self.form_invalid(form)
+
+        return super().form_valid(form)
+
+
+class DescriptionDeleteMixin:
+    """Prevents the only description of the described object from being
+    deleted."""
+
+    def form_valid(self, form):
+        described_object = self.object.get_described_object()
+        if described_object.get_descriptions().count() == 1:
+            form.add_error(
+                None,
+                "This description cannot be deleted because at least one description record is mandatory.",
+            )
+            return self.form_invalid(form)
+        return super().form_valid(form)
+
+
+class SortingMixin:
+    """
+    Can be used to sort a queryset in a view using GET params. Checks the GET
+    param against sort_by_fields to pass a valid field to .order_by(). If the
+    GET param doesn't match the desired .order_by() field, a dictionary mapping
+    can be added as custom_sorting.
+
+    Example usage:
+
+    class YourModelListView(SortingMixin, ListView):
+        sort_by_fields = ["sid", "model", "valid_between"]
+        custom_sorting = {
+            "model": "model__polymorphic_ctype",
+        }
+
+        def get_queryset(self):
+            self.queryset = YourModel.objects.all()
+            return super().get_queryset()
+    """
+
+    def get_ordering(self):
+        sort_by = self.request.GET.get("sort_by")
+        ordered = self.request.GET.get("ordered")
+        assert hasattr(
+            self,
+            "sort_by_fields",
+        ), "SortingMixin requires class attribute sort_by_fields to be set"
+        assert isinstance(self.sort_by_fields, list), "sort_by_fields must be a list"
+
+        if sort_by and sort_by in self.sort_by_fields:
+            if hasattr(self, "custom_sorting") and self.custom_sorting.get(sort_by):
+                sort_by = self.custom_sorting.get(sort_by)
+
+            if ordered == "desc":
+                sort_by = f"-{sort_by}"
+
+            return sort_by
+
+        else:
+            return None