Skip to content

Commit

Permalink
remove clean() calls from serializers
Browse files Browse the repository at this point in the history
  • Loading branch information
Floris272 committed Dec 24, 2024
1 parent ed01b58 commit 92e4d16
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 51 deletions.
39 changes: 21 additions & 18 deletions src/open_producten/producttypen/models/onderwerp.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,26 @@ def __str__(self):
return self.naam

def clean(self):
if (
self.gepubliceerd
and self.hoofd_onderwerp
and not self.hoofd_onderwerp.gepubliceerd
):
raise ValidationError(
_(
"Onderwerpen moeten gepubliceerd zijn voordat sub-onderwerpen kunnen worden gepubliceerd."
)
)
validate_gepubliceerd_state(
self.hoofd_onderwerp, self.gepubliceerd, self.sub_onderwerpen
)


if (
not self.gepubliceerd
and self.sub_onderwerpen.filter(gepubliceerd=True).exists()
):
raise ValidationError(
_(
"Onderwerpen kunnen niet ongepubliceerd worden als ze gepubliceerde sub-onderwerpen hebben."
)
def validate_gepubliceerd_state(hoofd_onderwerp, gepubliceerd, sub_onderwerpen=None):
if gepubliceerd and hoofd_onderwerp and not hoofd_onderwerp.gepubliceerd:
raise ValidationError(
_(
"Onderwerpen moeten gepubliceerd zijn voordat sub-onderwerpen kunnen worden gepubliceerd."
)
)

if (
not gepubliceerd
and sub_onderwerpen
and sub_onderwerpen.filter(gepubliceerd=True).exists()
):
raise ValidationError(
_(
"Onderwerpen kunnen niet ongepubliceerd worden als ze gepubliceerde sub-onderwerpen hebben."
)
)
17 changes: 6 additions & 11 deletions src/open_producten/producttypen/serializers/onderwerp.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from django.core.exceptions import ValidationError
from django.db import transaction

from rest_framework import serializers
Expand All @@ -10,6 +9,7 @@
)

from ...utils.drf_validators import DuplicateIdValidator
from .validators import OnderwerpGepubliceerdStateValidator
from .vraag import NestedVraagSerializer


Expand Down Expand Up @@ -62,21 +62,16 @@ class Meta:
"product_typen",
"product_type_ids",
)
validators = [DuplicateIdValidator(["product_type_ids"])]

def _validate_onderwerp(self, onderwerp):
try:
onderwerp.clean()
except ValidationError as err:
raise serializers.ValidationError({"hoofd_onderwerp": err.message})
validators = [
DuplicateIdValidator(["product_type_ids"]),
OnderwerpGepubliceerdStateValidator(),
]

@transaction.atomic()
def create(self, validated_data):
product_typen = validated_data.pop("product_typen")

onderwerp = Onderwerp.objects.create(**validated_data)

self._validate_onderwerp(onderwerp)
onderwerp.product_typen.set(product_typen)
onderwerp.save()

Expand All @@ -93,7 +88,7 @@ def update(self, instance, validated_data):
instance.hoofd_onderwerp = hoofd_onderwerp

instance = super().update(instance, validated_data)
self._validate_onderwerp(instance)

if product_typen:
instance.product_typen.set(product_typen)
instance.save()
Expand Down
2 changes: 0 additions & 2 deletions src/open_producten/producttypen/serializers/producttype.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ def create(self, validated_data):
product_type = ProductType.objects.create(**validated_data)
product_type.onderwerpen.set(onderwerpen)

product_type.clean()
product_type.save()

return product_type
Expand All @@ -113,7 +112,6 @@ def update(self, instance, validated_data):
instance = super().update(instance, validated_data)
if onderwerpen:
instance.onderwerpen.set(onderwerpen)
instance.clean()
instance.save()

return instance
Expand Down
21 changes: 21 additions & 0 deletions src/open_producten/producttypen/serializers/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from ...utils.serializers import get_from_serializer_data_or_instance
from ..models import PrijsOptie
from ..models.onderwerp import validate_gepubliceerd_state
from ..models.vraag import validate_onderwerp_or_product_type


Expand All @@ -22,6 +23,26 @@ def __call__(self, value, serializer):
raise serializers.ValidationError({"product_type_onderwerp": e.message})


class OnderwerpGepubliceerdStateValidator:
requires_context = True

def __call__(self, value, serializer):
hoofd_onderwerp = get_from_serializer_data_or_instance(
"hoofd_onderwerp", value, serializer
)
gepubliceerd = get_from_serializer_data_or_instance(
"gepubliceerd", value, serializer
)
sub_onderwerpen = (
serializer.instance.sub_onderwerpen if serializer.instance else None
)

try:
validate_gepubliceerd_state(hoofd_onderwerp, gepubliceerd, sub_onderwerpen)
except ValidationError as e:
raise serializers.ValidationError({"hoofd_onderwerp": e.message})


class PrijsOptieValidator:
requires_context = True

Expand Down
50 changes: 30 additions & 20 deletions src/open_producten/producttypen/tests/api/test_onderwerp.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,12 @@ def test_create_published_sub_onderwerp_with_unpublished_hoofd_onderwerp_returns
self.assertEqual(
response.data,
{
"hoofd_onderwerp": ErrorDetail(
string="Onderwerpen moeten gepubliceerd zijn voordat sub-onderwerpen kunnen worden gepubliceerd.",
code="invalid",
)
"hoofd_onderwerp": [
ErrorDetail(
string="Onderwerpen moeten gepubliceerd zijn voordat sub-onderwerpen kunnen worden gepubliceerd.",
code="invalid",
)
]
},
)

Expand Down Expand Up @@ -211,10 +213,12 @@ def test_update_unpublished_sub_onderwerp_to_published_with_unpublished_hoofd_on
self.assertEqual(
response.data,
{
"hoofd_onderwerp": ErrorDetail(
string="Onderwerpen moeten gepubliceerd zijn voordat sub-onderwerpen kunnen worden gepubliceerd.",
code="invalid",
)
"hoofd_onderwerp": [
ErrorDetail(
string="Onderwerpen moeten gepubliceerd zijn voordat sub-onderwerpen kunnen worden gepubliceerd.",
code="invalid",
)
]
},
)

Expand All @@ -234,10 +238,12 @@ def test_update_published_hoofd_onderwerp_to_unpublished_with_published_sub_onde
self.assertEqual(
response.data,
{
"hoofd_onderwerp": ErrorDetail(
string="Onderwerpen kunnen niet ongepubliceerd worden als ze gepubliceerde sub-onderwerpen hebben.",
code="invalid",
)
"hoofd_onderwerp": [
ErrorDetail(
string="Onderwerpen kunnen niet ongepubliceerd worden als ze gepubliceerde sub-onderwerpen hebben.",
code="invalid",
)
]
},
)

Expand Down Expand Up @@ -326,10 +332,12 @@ def test_partial_update_unpublished_sub_onderwerp_to_published_with_unpublished_
self.assertEqual(
response.data,
{
"hoofd_onderwerp": ErrorDetail(
string="Onderwerpen moeten gepubliceerd zijn voordat sub-onderwerpen kunnen worden gepubliceerd.",
code="invalid",
)
"hoofd_onderwerp": [
ErrorDetail(
string="Onderwerpen moeten gepubliceerd zijn voordat sub-onderwerpen kunnen worden gepubliceerd.",
code="invalid",
)
]
},
)

Expand All @@ -349,10 +357,12 @@ def test_partial_update_published_hoofd_onderwerp_to_unpublished_with_published_
self.assertEqual(
response.data,
{
"hoofd_onderwerp": ErrorDetail(
string="Onderwerpen kunnen niet ongepubliceerd worden als ze gepubliceerde sub-onderwerpen hebben.",
code="invalid",
)
"hoofd_onderwerp": [
ErrorDetail(
string="Onderwerpen kunnen niet ongepubliceerd worden als ze gepubliceerde sub-onderwerpen hebben.",
code="invalid",
)
]
},
)

Expand Down

0 comments on commit 92e4d16

Please sign in to comment.