Overhaul override_receivable_type input validation

leasing/serializers/rent.py

@@ -1,17 +1,13 @@
+from typing import Any
+
 from django.utils.translation import gettext_lazy as _
 from enumfields.drf import EnumSupportSerializerMixin
 from rest_framework import serializers
 from rest_framework.exceptions import ValidationError
 from rest_framework.serializers import ListSerializer
 
 from field_permissions.serializers import FieldPermissionsSerializerMixin
-from leasing.enums import (
-    DueDatesType,
-    RentAdjustmentAmountType,
-    RentCycle,
-    RentType,
-    is_akv_or_kuva_service_unit_id,
-)
+from leasing.enums import DueDatesType, RentAdjustmentAmountType, RentCycle, RentType
 from leasing.models import (
     ContractRent,
     Decision,
@@ -578,61 +574,182 @@ def validate_seasonal_values(self, rent_data: dict) -> None:
         validate_seasonal_day_for_month(start_day, start_month)
         validate_seasonal_day_for_month(end_day, end_month)
 
-    def validate_override_receivable_type_value(self, rent_data: dict) -> None:
+    def validate_override_receivable_type_value(
+        self, rent_data: dict[str, Any]
+    ) -> None:
         """
-        Override receivabletype is mandatory for AKV and KuVa leases, and not
-        used by MaKe/Tontit.
+        Currently, the rent override receivabletype is mandatory for AKV and
+        KuVa leases, and not used by MaKe/Tontit.
 
         It is only used in index rents, fixed rents, and manual rents, because
         these rent types can generate automatic invoices.
 
+        TODO Blind spot: this implementation cannot reject missing override
+        receivable type when creating a new rent, because the validator doesn't
+        receive enough details from the frontend.
+        - Cannot read service unit from the receivable type if it's missing
+        - Cannot find out the service unit from rent's lease if rent
+        doesn't exist yet.
+
         Raises: serializers.ValidationError
         """
-        override_receivable_type = rent_data.get("override_receivable_type")
+        rent_type = rent_data.get("type", {})
+        rent_types_that_can_use_override_receivable_type = [
+            RentType.INDEX,
+            RentType.INDEX2022,
+            RentType.FIXED,
+            RentType.MANUAL,
+        ]
+        # These rent types can generate automatic invoices, so they can utilize
+        # the override receivabletype, if required by service unit.
+        rent_type_uses_override = (
+            rent_type in rent_types_that_can_use_override_receivable_type
+        )
 
-        if not override_receivable_type:
-            # Empty override receivabletype input should be allowed, because it
-            # is always empty for all MaKe rents, and those must be allowed.
+        rent_id = rent_data.get("id")
+        if rent_id is None:
+            # The ID is not present in creation flow, and is removed from
+            # rent_data before later validations also during the update flow.
+            # Try get ID from the rent instance, which is present if an existing
+            # rent is being updated.
+            rent_id = self.instance.id if self.instance else None
+
+        if rent_id is not None:
+            self.full_validate_override_receivable_type(
+                rent_data, rent_id, rent_type_uses_override
+            )
+        else:
+            self.minimal_validate_override_receivable_type(
+                rent_data, rent_type_uses_override
+            )
+
+    def full_validate_override_receivable_type(
+        self,
+        rent_data: dict[str, Any],
+        rent_id: int,
+        rent_type_uses_override: bool,
+    ) -> None:
+        """
+        Perform full validation based on the service unit, receivabletype,
+        and rent type.
+
+        We have all necessary details to check whether the lease's service unit
+        uses the override receivabletype feature or not.
+
+        Raises: serializers.ValidationError
+        """
+        rent = Rent.objects.select_related("lease__service_unit").get(pk=rent_id)
+        use_rent_override_receivable_type: bool = (
+            rent.lease.service_unit.use_rent_override_receivable_type
+        )
+        override_receivable_type: ReceivableType | None = rent_data.get(
+            "override_receivable_type"
+        )
+
+        if (not use_rent_override_receivable_type) and (not override_receivable_type):
+            # This service unit does not use the override receivabletype feature,
+            # and none was supplied -> all good.
+            # For example, service unit MaKe/Tontit.
             return
 
-        elif not is_akv_or_kuva_service_unit_id(
-            override_receivable_type.service_unit_id
-        ):
-            # All MaKe receivabletypes should be rejected regardless of rent
-            # type, because MaKe doesn't use the override feature, and if any
-            # other service unit would use MaKe's receivabletypes, it would be
-            # a mistake.
-            raise serializers.ValidationError(
-                _(
-                    f'Override receivabletype "{override_receivable_type.name}" was unexpected. '
-                    "Override receivabletype is not used by MaKe/Tontit."
+        if use_rent_override_receivable_type and override_receivable_type:
+            if rent_type_uses_override:
+                # Override receivabletype is required, was supplied, and rent
+                # type is correct -> all good.
+                # For example, service units AKV and KuVa.
+                return
+            else:
+                raise serializers.ValidationError(
+                    _(
+                        f'Override receivabletype "{override_receivable_type.name}" was unexpected. '
+                        f"This rent type does not generate automatic invoices. "
+                        "Please contact MVJ developers about this error."
+                    )
                 )
-            )
 
-        elif is_akv_or_kuva_service_unit_id(override_receivable_type.service_unit_id):
-            rent_type = rent_data.get("type", {})
-
-            if rent_type in [
-                RentType.INDEX,
-                RentType.INDEX2022,
-                RentType.FIXED,
-                RentType.MANUAL,
-            ]:
-                # These rent types can generate automatic invoices, so they can
-                # utilize the override receivabletype.
+        if use_rent_override_receivable_type and (not override_receivable_type):
+            if not rent_type_uses_override:
+                # This rent type does not utilize rent_override_receivable_type,
+                # even if the service unit generally uses it.
                 return
             else:
                 raise serializers.ValidationError(
                     _(
-                        f'Override receivabletype "{override_receivable_type.name}" was unexpected. '
-                        f'Rent type "{rent_type.name}" does not generate automatic invoices.'
+                        f'Override receivabletype is required for service unit "{rent.lease.service_unit.name}", '
+                        f"and for this rent type."
                     )
                 )
 
-        else:
+        if (not use_rent_override_receivable_type) and override_receivable_type:
             raise serializers.ValidationError(
                 _(
-                    "Unhandled case in override receivabletype validation. Rejecting just in case."
+                    f'Override receivabletype "{override_receivable_type.name}" was unexpected. '
+                    f'Service unit "{rent.lease.service_unit.name}" does not use this feature. '
+                    "Please contact MVJ developers about this error."
+                )
+            )
+
+        raise serializers.ValidationError(
+            _(
+                "Unhandled case in override receivabletype validation. Rejecting just in case. "
+                "Please contact MVJ developers about this error."
+            )
+        )
+
+    def minimal_validate_override_receivable_type(
+        self,
+        rent_data: dict[str, Any],
+        rent_type_uses_override: bool,
+    ) -> None:
+        """
+        Only perform minimal validation based on the receivabletype, and rent
+        type.
+
+        This rent might be a new rent being created, so we can't reference the
+        containing lease and its service unit with the rent ID because the ID
+        doesn't exist yet.
+
+        This method is also visited during rent updates, when the logic performs
+        additional validation before saving the updated details to database.
+        In this case the rent ID is also not available, because it is stripped
+        from the input before calling validation.
+
+        Raises: serializers.ValidationError
+        """
+        override_receivable_type: ReceivableType | None = rent_data.get(
+            "override_receivable_type"
+        )
+        if override_receivable_type is None:
+            # Empty override receivabletype input must be allowed, because it
+            # is always empty for all MaKe rents, and those must be allowed.
+            # Without it, we cannot make further validations about its properties.
+            return
+
+        service_unit = override_receivable_type.service_unit
+        receivabletypes_service_unit_uses_override = (
+            service_unit.use_rent_override_receivable_type
+        )
+
+        if receivabletypes_service_unit_uses_override and rent_type_uses_override:
+            # Service unit and rent types require an override receivabletype, and it was supplied
+            # --> all good.
+            return
+
+        if not receivabletypes_service_unit_uses_override:
+            raise serializers.ValidationError(
+                _(
+                    f'Override receivabletype "{override_receivable_type.name}" was unexpected. '
+                    f'Override receivabletype is not used by service unit "{service_unit.name}". '
+                    "Please contact MVJ developers about this error."
+                )
+            )
+
+        if not rent_type_uses_override:
+            raise serializers.ValidationError(
+                _(
+                    f'Override receivabletype "{override_receivable_type.name}" was unexpected. '
+                    f"This rent type does not generate automatic invoices. "
+                    "Please contact MVJ developers about this error."
                 )
             )
 

leasing/tests/models/test_calculate_invoices.py

@@ -493,6 +493,10 @@ def test_calculate_invoices_uses_correct_receivable_type(
     rent_factory,
     contract_rent_factory,
 ):
+    """
+    By default, invoice generation uses the service unit's default
+    receivable type for rents, if no other receivable types are specified.
+    """
     service_units = [
         service_unit_factory(name="First service unit"),
         service_unit_factory(name="Second service unit"),
@@ -593,7 +597,8 @@ def test_calculate_invoices_uses_override_receivable_type(
 ):
     """
     If an override_receivable_type is defined in a rent, that receivable type is
-    used in the invoice over the default value.
+    used in the invoice generation over the service unit's
+    default_receivable_type_rent.
     """
     # Mandatory set up
     service_unit: ServiceUnit = service_unit_factory(name="ServiceUnitName")