maykinmedia / zgw-consumers

Django app to store ZGW API configuration
MIT License
1 stars 2 forks source link

Replace zgw_consumers.drf.serializers with drf-dataclasses #31

Open sergei-maertens opened 2 years ago

sergei-maertens commented 2 years ago

See: https://pypi.org/project/djangorestframework-dataclasses/

The implementation appears to be the same - inspired on ModelSerializer.

To avoid breaking backwards compatibility, we can keep the custom classes around for a couple of releases and alias the drf-dataclasses (+ add a warning of course).

sergei-maertens commented 11 months ago

maybe it would be better to deprecate this and remove it in 1.0, in favour of the mentioned library.

sergei-maertens commented 11 months ago

@damm89 I know this thing is used in Utrecht - can you provide some insight of what the impact would be to switch to djangorestframework-dataclasses from zgw-consumers' APIModelSerializer?

damm89 commented 11 months ago

Hi @sergei-maertens,

The impact depends on the implementation. Deprecation would mean deprecating the ZGWModel? Or adapting it to drf-dataclasses but keeping ZGWModel?

sergei-maertens commented 11 months ago

@damm89 no - we're not touching ZGWModel (yet), I'm talking about https://github.com/maykinmedia/zgw-consumers/blob/main/zgw_consumers/drf/serializers.py#L20 - the proposed library looks like it would (almost?) be a drop-in replacement.

ZGWModel is a base class for dataclasses, so the mechanism should be the same

damm89 commented 11 months ago

I see, I misread.

I tried to replace the APIModelSerializer with the DataclassSerializer in the ZAC and where it seems to "break" with current functionality is the to_internal_value method.

When validating the data it tries to actually instantiate the dataclass in to_internal_value but it's obviously missing data. I don't really have a lot of time right now to check why this is happening and whether there's a quick option or fix somewhere.

Test it out for yourself with the example below.

Example:

from zgw_consumers.api_models.catalogi import StatusType
from zgw_consumers.api_models.zaken import Status
from rest_framework_dataclasses.serializers import DataclassSerializer

class StatusTypeSerializer(DataclassSerializer):
    class Meta:
        dataclass = StatusType
        fields = (
            "url",
            "omschrijving",
            "omschrijving_generiek",
            "statustekst",
            "volgnummer",
            "is_eindstatus",
        )
        read_only_fields = ["omschrijving", "omschrijving_generiek", "statustekst", "volgnummer", "is_eindstatus"]

class StatusSerializer(DataclassSerializer):
    statustype = StatusTypeSerializer()
    class Meta:
        dataclass = Status
        read_only_fields = ["url", "datum_status_gezet"]

data={
    "statustype": {'url': "https://some-url.com/"},
    "statustoelichting": "Some-toelichting"
}

serializer = StatusSerializer(data=data)
serializer.is_valid(raise_exception=True)
sergei-maertens commented 11 months ago

Thanks for getting back - we'll wait a little longer with deprecating this then!