Overhaul override_receivable_type input validation

leasing/migrations/0081_serviceunit_use_rent_override_receivable_type.py

@@ -1,4 +1,4 @@
-# Generated by Django 4.2.14 on 2024-11-20 11:01
+# Generated by Django 4.2.14 on 2024-11-20 12:46
 
 from django.db import migrations, models
 
@@ -15,14 +15,7 @@ class Migration(migrations.Migration):
             name="use_rent_override_receivable_type",
             field=models.BooleanField(
                 default=False,
-                help_text="Use the override receivable type from rent in "
-                "automatic invoices, if it is present. When creating a rent, some "
-                "service units (such as AKV and KuVa) want to select a receivable "
-                "type to be used in future automatic invoices. This helps avoid "
-                "some technical difficulties in invoice XML generation. "
-                "Generation logic would otherwise be unaware of the desired "
-                "receivable type, if it is different from the service unit's "
-                "default receivable type, or the leasetype's receivable type.",
+                help_text="Use the override receivable type from rent in automatic invoices, if it is present. When creating a rent, some service units (such as AKV and KuVa) want to select a receivable type to be used in future automatic invoices. This helps avoid some technical difficulties in invoice XML generation. Generation logic would otherwise be unaware of the desired receivable type, if it is different from the service unit's default receivable type, or the leasetype's receivable type.",
                 verbose_name="Use the override receivable type from rent in automatic invoices?",
             ),
         ),

leasing/models/service_unit.py

@@ -72,16 +72,17 @@ class ServiceUnit(TimeStampedSafeDeleteModel):
     )
     use_rent_override_receivable_type = models.BooleanField(
         verbose_name=_(
-            "Use the override receivabletype from rent in automatic invoices?"
+            "Use the override receivable type from rent in automatic invoices?"
         ),
         help_text=_(
-            "Use the override receivable type from rent in automatic invoices, if "
-            "it is present. When creating a rent, some service units (such as AKV "
-            "and KuVa) want to select a receivable type to be used in future "
-            "automatic invoices. This helps avoid some technical difficulties in "
-            "invoice XML generation. Generation logic would otherwise be unaware "
-            "of the desired receivable type, if it is different from the service "
-            "unit's default receivable type, or the leasetype's receivable type."
+            "Use the override receivable type from rent in "
+            "automatic invoices, if it is present. When creating a rent, some "
+            "service units (such as AKV and KuVa) want to select a receivable "
+            "type to be used in future automatic invoices. This helps avoid "
+            "some technical difficulties in invoice XML generation. "
+            "Generation logic would otherwise be unaware of the desired "
+            "receivable type, if it is different from the service unit's "
+            "default receivable type, or the leasetype's receivable type."
         ),
         default=False,
     )

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,
@@ -25,6 +21,7 @@
     RentAdjustment,
     RentDueDate,
     RentIntendedUse,
+    ServiceUnit,
 )
 from leasing.models.rent import (
     EqualizedRent,
@@ -578,61 +575,149 @@ 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.
-            return
+        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 = getattr(self.instance, "id", 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.
 
-        elif not is_akv_or_kuva_service_unit_id(
-            override_receivable_type.service_unit_id
+        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)
+        rents_service_unit_uses_override: bool = (
+            rent.lease.service_unit.use_rent_override_receivable_type
+        )
+        override_receivable_type: ReceivableType | None = rent_data.get(
+            "override_receivable_type"
+        )
+        if override_receivable_type and (not rents_service_unit_uses_override):
+            raise serializers.ValidationError(
+                _(
+                    f'Override receivable type "{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."
+                )
+            )
+        if override_receivable_type and (not rent_type_uses_override):
+            raise serializers.ValidationError(
+                _(
+                    f'Override receivable type "{override_receivable_type.name}" was unexpected. '
+                    f"This rent type does not generate automatic invoices. "
+                    "Please contact MVJ developers about this error."
+                )
+            )
+        if (
+            rents_service_unit_uses_override
+            and rent_type_uses_override
+            and (not override_receivable_type)
         ):
-            # 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."
+                    "Override receivable type is required for this rent type in service unit "
+                    f'"{rent.lease.service_unit.name}".'
                 )
             )
 
-        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.
-                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.'
-                    )
-                )
+    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.
 
-        else:
+        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: ServiceUnit = override_receivable_type.service_unit
+        if not service_unit.use_rent_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'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")