From f5e8e17047ffcad1d25dcb0420f20098bacea41c Mon Sep 17 00:00:00 2001 From: Floris272 Date: Thu, 6 Mar 2025 13:02:09 +0100 Subject: [PATCH] [#46] refactor prijs serializer --- .../producten/serializers/product.py | 17 ------- .../producttypen/serializers/prijs.py | 15 +++--- .../producttypen/serializers/validators.py | 50 ------------------- .../producttypen/tests/api/test_prijs.py | 6 ++- .../producttypen/tests/factories.py | 2 +- 5 files changed, 13 insertions(+), 77 deletions(-) diff --git a/src/open_producten/producten/serializers/product.py b/src/open_producten/producten/serializers/product.py index df56934..abd7ab5 100644 --- a/src/open_producten/producten/serializers/product.py +++ b/src/open_producten/producten/serializers/product.py @@ -57,11 +57,6 @@ def update(self, instance, validated_data): else: existing_eigenaar = Eigenaar.objects.get(id=eigenaar_id) EigenaarSerializer().update(existing_eigenaar, eigenaar) - - # serializer = EigenaarSerializer(existing_eigenaar, data=eigenaar, partial=self.partial) - # serializer.is_valid(raise_exception=True) - # serializer.save() - seen_eigenaren_ids.add(eigenaar_id) product.eigenaren.filter( @@ -69,15 +64,3 @@ def update(self, instance, validated_data): ).delete() return product - - -# def update_nested_object(object_list: list[dict], child_serializer: Serializer): -# ids = [obj["id"] for obj in object_list] -# product.eigenaren.exclude(id__in=ids).delete() -# -# for obj in object_list: -# if obj_id := obj.pop("id", None): -# existing_eigenaar = Eigenaar.objects.get(id=obj_id) -# child_serializer().update(existing_eigenaar, obj) -# else: -# child_serializer().create(obj | {"product": product}) diff --git a/src/open_producten/producttypen/serializers/prijs.py b/src/open_producten/producttypen/serializers/prijs.py index f627473..8b22681 100644 --- a/src/open_producten/producttypen/serializers/prijs.py +++ b/src/open_producten/producttypen/serializers/prijs.py @@ -3,8 +3,8 @@ from rest_framework import serializers +from ...utils.drf_validators import NestedObjectsValidator from ..models import Prijs, PrijsOptie, ProductType -from .validators import PrijsOptieValidator class PrijsOptieSerializer(serializers.ModelSerializer): @@ -24,7 +24,7 @@ class PrijsSerializer(serializers.ModelSerializer): class Meta: model = Prijs fields = ("id", "product_type_id", "prijsopties", "actief_vanaf") - validators = [PrijsOptieValidator()] + validators = [NestedObjectsValidator("prijsopties", PrijsOptie)] def validate_prijsopties(self, opties: list[PrijsOptie]) -> list[PrijsOptie]: if len(opties) == 0: @@ -39,7 +39,7 @@ def create(self, validated_data): prijs = Prijs.objects.create(**validated_data, product_type=product_type) for optie in prijsopties: - PrijsOptie.objects.create(prijs=prijs, **optie) + PrijsOptieSerializer().create(optie | {"prijs": prijs}) return prijs @@ -57,13 +57,14 @@ def update(self, instance, validated_data): for optie in opties: optie_id = optie.pop("id", None) if optie_id is None: - PrijsOptie.objects.create(prijs=prijs, **optie) + PrijsOptieSerializer().create(optie | {"prijs": prijs}) else: existing_optie = PrijsOptie.objects.get(id=optie_id) - existing_optie.bedrag = optie["bedrag"] - existing_optie.beschrijving = optie["beschrijving"] - existing_optie.save() + PrijsOptieSerializer(partial=self.partial).update( + existing_optie, optie + ) + seen_optie_ids.add(optie_id) PrijsOptie.objects.filter( id__in=(current_optie_ids - seen_optie_ids) diff --git a/src/open_producten/producttypen/serializers/validators.py b/src/open_producten/producttypen/serializers/validators.py index c62f184..0e81f51 100644 --- a/src/open_producten/producttypen/serializers/validators.py +++ b/src/open_producten/producttypen/serializers/validators.py @@ -1,10 +1,8 @@ from django.core.exceptions import ValidationError -from django.utils.translation import gettext_lazy as _ from rest_framework import serializers from ...utils.serializers import get_from_serializer_data_or_instance -from ..models import PrijsOptie from ..models.thema import disallow_self_reference, validate_gepubliceerd_state @@ -39,51 +37,3 @@ def __call__(self, value, serializer): disallow_self_reference(thema, hoofd_thema) except ValidationError as e: raise serializers.ValidationError({"hoofd_thema": e.message}) - - -class PrijsOptieValidator: - requires_context = True - - def __call__(self, value, serializer): - prijs_instance = serializer.instance - if not prijs_instance or not value.get("prijsopties"): - return - - optie_errors = [] - - current_optie_ids = set( - prijs_instance.prijsopties.values_list("id", flat=True).distinct() - ) - seen_optie_ids = set() - - for idx, optie in enumerate(value["prijsopties"]): - optie_id = optie.pop("id", None) - - if not optie_id: - continue - - if optie_id in current_optie_ids: - - if optie_id in seen_optie_ids: - optie_errors.append( - _("Dubbel id: {} op index {}.").format(optie_id, idx) - ) - seen_optie_ids.add(optie_id) - - else: - try: - PrijsOptie.objects.get(id=optie_id) - optie_errors.append( - _( - "Prijs optie id {} op index {} is niet onderdeel van het prijs object." - ).format(optie_id, idx) - ) - except PrijsOptie.DoesNotExist: - optie_errors.append( - _("Prijs optie id {} op index {} bestaat niet.").format( - optie_id, idx - ) - ) - - if optie_errors: - raise serializers.ValidationError({"prijsopties": optie_errors}) diff --git a/src/open_producten/producttypen/tests/api/test_prijs.py b/src/open_producten/producttypen/tests/api/test_prijs.py index 61ab521..29d9f9f 100644 --- a/src/open_producten/producttypen/tests/api/test_prijs.py +++ b/src/open_producten/producttypen/tests/api/test_prijs.py @@ -156,6 +156,7 @@ def test_update_prijs_updating_and_removing_opties(self): self.assertEqual(Prijs.objects.count(), 1) self.assertEqual(PrijsOptie.objects.count(), 1) self.assertEqual(PrijsOptie.objects.first().bedrag, Decimal("20")) + self.assertEqual(PrijsOptie.objects.first().id, optie_to_be_updated.id) def test_update_prijs_creating_and_deleting_opties(self): @@ -195,7 +196,7 @@ def test_update_prijs_with_optie_not_part_of_prijs_returns_error(self): "prijsopties": [ ErrorDetail( string=_( - "Prijs optie id {} op index 0 is niet onderdeel van het prijs object." + "Prijs optie id {} op index 0 is niet onderdeel van het Prijs object." ).format(optie.id), code="invalid", ) @@ -298,6 +299,7 @@ def test_partial_update_prijs_updating_and_removing_opties(self): self.assertEqual(Prijs.objects.count(), 1) self.assertEqual(PrijsOptie.objects.count(), 1) self.assertEqual(PrijsOptie.objects.first().bedrag, Decimal("20")) + self.assertEqual(PrijsOptie.objects.first().id, optie_to_be_updated.id) def test_partial_update_prijs_creating_and_deleting_opties(self): @@ -351,7 +353,7 @@ def test_partial_update_with_multiple_errors(self): ), ErrorDetail( string=_( - "Prijs optie id {} op index 2 is niet onderdeel van het prijs object." + "Prijs optie id {} op index 2 is niet onderdeel van het Prijs object." ).format(optie_of_other_prijs.id), code="invalid", ), diff --git a/src/open_producten/producttypen/tests/factories.py b/src/open_producten/producttypen/tests/factories.py index 62f9118..8a7d92e 100644 --- a/src/open_producten/producttypen/tests/factories.py +++ b/src/open_producten/producttypen/tests/factories.py @@ -20,7 +20,7 @@ class UniformeProductNaamFactory(factory.django.DjangoModelFactory): naam = factory.Sequence(lambda n: f"upn {n}") - uri = factory.Faker("url") + uri = factory.Sequence(lambda n: f"{fake.url()}/{n}") class Meta: model = UniformeProductNaam