diff --git a/leasing/migrations/0081_serviceunit_use_rent_override_receivable_type.py b/leasing/migrations/0081_serviceunit_use_rent_override_receivable_type.py index ee8732cf..550050ea 100644 --- a/leasing/migrations/0081_serviceunit_use_rent_override_receivable_type.py +++ b/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?", ), ), diff --git a/leasing/models/service_unit.py b/leasing/models/service_unit.py index 5f6df1ef..00808ca6 100644 --- a/leasing/models/service_unit.py +++ b/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, ) diff --git a/leasing/serializers/rent.py b/leasing/serializers/rent.py index 3dffbc68..7235ce53 100644 --- a/leasing/serializers/rent.py +++ b/leasing/serializers/rent.py @@ -1,3 +1,5 @@ +from typing import Any + from django.utils.translation import gettext_lazy as _ from enumfields.drf import EnumSupportSerializerMixin from rest_framework import serializers @@ -5,13 +7,7 @@ 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." ) ) diff --git a/leasing/tests/models/test_calculate_invoices.py b/leasing/tests/models/test_calculate_invoices.py index a6c3d688..a6716047 100644 --- a/leasing/tests/models/test_calculate_invoices.py +++ b/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") diff --git a/leasing/tests/serializers/test_rent.py b/leasing/tests/serializers/test_rent.py index 7f518aaf..98256767 100644 --- a/leasing/tests/serializers/test_rent.py +++ b/leasing/tests/serializers/test_rent.py @@ -1,83 +1,222 @@ import pytest from rest_framework.exceptions import ValidationError -from leasing.enums import RentType, ServiceUnitId -from leasing.models import ServiceUnit +from leasing.enums import RentType +from leasing.models import Rent from leasing.serializers.rent import RentCreateUpdateSerializer -@pytest.mark.django_db -def test_is_valid_override_receivable_type(django_db_setup, receivable_type_factory): +@pytest.fixture +def rent_types_with_override() -> list[str]: + """These rent types can use the rent override receivabletype during invoicing.""" + return [ + RentType.FIXED, + RentType.INDEX, + RentType.INDEX2022, + RentType.MANUAL, + ] + + +@pytest.fixture +def rent_types_without_override() -> list[str]: + """These rent types do not use the rent override receivabletype during invoicing.""" + return [RentType.FREE, RentType.ONE_TIME] + + +@pytest.fixture +def rent_without_override_receivable_type( + service_unit_factory, + lease_factory, + rent_factory, +) -> Rent: """ - Test that the requirements described in the target function docstring hold. + Test data fixture for a rent with a lease and service unit that doesn't use + the override receivabletype feature. """ - make = ServiceUnit.objects.get(pk=ServiceUnitId.MAKE) - akv = ServiceUnit.objects.get(pk=ServiceUnitId.AKV) - kuva_lipa = ServiceUnit.objects.get(pk=ServiceUnitId.KUVA_LIPA) - kuva_upa = ServiceUnit.objects.get(pk=ServiceUnitId.KUVA_UPA) - kuva_nup = ServiceUnit.objects.get(pk=ServiceUnitId.KUVA_NUP) + service_unit_without_override = service_unit_factory( + name="Service unit without rent override receivabletype", + use_rent_override_receivable_type=False, + ) + lease_from_unit_without_override = lease_factory( + service_unit=service_unit_without_override, + ) + return rent_factory(lease=lease_from_unit_without_override) - serializer = RentCreateUpdateSerializer() - # Validator should reject all MaKe receivable types regardless of rent type. - rent_datas_make = [ +@pytest.fixture +def rent_with_override_receivable_type( + service_unit_factory, + lease_factory, + rent_factory, +) -> Rent: + """ + Test data fixture for a rent with a lease and service unit that uses the + override receivabletype feature. + """ + service_unit_with_override = service_unit_factory( + name="Service unit with rent override receivabletype", + use_rent_override_receivable_type=True, + ) + lease_from_unit_with_override = lease_factory( + service_unit=service_unit_with_override, + ) + return rent_factory(lease=lease_from_unit_with_override) + + +@pytest.mark.django_db +def test_validate_override_receivable_type_invalid_service_unit( + receivable_type_factory, + rent_without_override_receivable_type, +): + """ + RentCreateUpdateSerializer should reject receivable types from service units + that don't use override receivabletype, regardless of rent type. + """ + serializer = RentCreateUpdateSerializer() + rent_datas_invalid_service_unit = [ { "type": rent_type, - "override_receivable_type": receivable_type_factory(service_unit=make), + "override_receivable_type": receivable_type_factory( + service_unit=rent_without_override_receivable_type.lease.service_unit + ), } for rent_type in RentType ] - for data in rent_datas_make: + for data in rent_datas_invalid_service_unit: + # Input without a rent ID (e.g. during create) with pytest.raises(ValidationError): serializer.validate(data) - # Validator should allow empty override receivable type input. - rent_datas_empty = [ + # Input with a rent ID (e.g. during update) + data["id"] = rent_without_override_receivable_type.id + with pytest.raises(ValidationError): + serializer.validate(data) + + +@pytest.mark.django_db +def test_validate_override_receivable_type_valid_inputs( + receivable_type_factory, + rent_with_override_receivable_type, + rent_types_with_override, +): + """ + RentCreateUpdateSerializer should allow override receivabletype from service + units that use the feature, if rent type is valid. + """ + serializer = RentCreateUpdateSerializer() + rent_datas_valid_types_valid_unit = [ + { + "type": rent_type, + "override_receivable_type": receivable_type_factory( + service_unit=rent_with_override_receivable_type.lease.service_unit + ), + } + for rent_type in rent_types_with_override + ] + for data in rent_datas_valid_types_valid_unit: + # Input without a rent ID (e.g. during create) + assert serializer.validate(data) + + # Input with a rent ID (e.g. during update) + data["id"] = rent_with_override_receivable_type.id + assert serializer.validate(data) + + +@pytest.mark.django_db +def test_validate_override_receivable_type_lacking_inputs(): + """ + RentCreateUpdateSerializer should allow empty override receivable type + input, when no additional details are known about the service unit or + receivable type. + + This is the case for example when creating new rents with rent types that + don't utilize the rent override receivabletype feature. + """ + serializer = RentCreateUpdateSerializer() + rent_datas_without_override_or_rent_id = [ { "type": rent_type, "override_receivable_type": None, } for rent_type in RentType ] - for data in rent_datas_empty: + for data in rent_datas_without_override_or_rent_id: assert serializer.validate(data) - # Validator should allow AKV and KuVa receivable types, if rent type is valid. - rent_datas_akv_and_kuva = [ + +@pytest.mark.django_db +def test_validate_override_receivable_type_missing_receivabletype( + rent_with_override_receivable_type, + rent_types_with_override, +): + """ + RentCreateUpdateSerializer should reject the input when service unit and + rent types require override receivabletype, but it was not supplied, and + the rent ID is known. + + The expected case is during an update of existing rent. + During create, rent ID would not be known in validation. + """ + serializer = RentCreateUpdateSerializer() + rent_datas_valid_types_valid_unit_existing_rent_no_override = [ { - "type": RentType.INDEX2022, - "override_receivable_type": receivable_type_factory(service_unit=unit), + "id": rent_with_override_receivable_type.id, + "type": rent_type, + "override_receivable_type": None, } - for unit in [akv, kuva_lipa, kuva_upa, kuva_nup] + for rent_type in rent_types_with_override ] - for data in rent_datas_akv_and_kuva: - assert serializer.validate(data) + for data in rent_datas_valid_types_valid_unit_existing_rent_no_override: + with pytest.raises(ValidationError): + serializer.validate(data) + - # Validator should allow rent types that can generate automatic invoices. - rent_datas_valid_types = [ +@pytest.mark.django_db +def test_validate_override_receivable_type_invalid_rent_type( + receivable_type_factory, + rent_with_override_receivable_type, + rent_without_override_receivable_type, + rent_types_without_override, +): + """ + RentCreateUpdateSerializer should reject the input when rent type doesn't + use override receivabletype in invoicing, but it was supplied. + """ + serializer = RentCreateUpdateSerializer() + rent_datas_invalid_types_invalid_unit = [ { "type": rent_type, - "override_receivable_type": receivable_type_factory(service_unit=akv), + "override_receivable_type": receivable_type_factory( + service_unit=rent_without_override_receivable_type.lease.service_unit + ), } - for rent_type in [ - RentType.FIXED, - RentType.INDEX, - RentType.INDEX2022, - RentType.MANUAL, - ] + for rent_type in rent_types_without_override ] - for data in rent_datas_valid_types: - assert serializer.validate(data) + for data in rent_datas_invalid_types_invalid_unit: + # Flow without rent ID + with pytest.raises(ValidationError): + serializer.validate(data) + + # Flow with rent ID + data["id"] = rent_without_override_receivable_type.id + with pytest.raises(ValidationError): + serializer.validate(data) - # Validator should reject the receivable type input for rent types that - # don't generate automatic invoices. - rent_datas_invalid_types = [ + rent_datas_invalid_types_valid_unit = [ { "type": rent_type, - "override_receivable_type": receivable_type_factory(service_unit=akv), + "override_receivable_type": receivable_type_factory( + service_unit=rent_with_override_receivable_type.lease.service_unit + ), } - for rent_type in [RentType.FREE, RentType.ONE_TIME] + for rent_type in rent_types_without_override ] - for data in rent_datas_invalid_types: + for data in rent_datas_invalid_types_valid_unit: + # Flow without rent ID + with pytest.raises(ValidationError): + serializer.validate(data) + + # Flow with rent ID + data["id"] = rent_with_override_receivable_type.id with pytest.raises(ValidationError): serializer.validate(data)