Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sergei-maertens opened this issue Feb 4, 2022 · 6 comments
Open
Assignees

Comments

@sergei-maertens
Copy link
Member

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 sergei-maertens self-assigned this Feb 4, 2022
@sergei-maertens
Copy link
Member Author

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

@sergei-maertens
Copy link
Member Author

@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
Copy link
Contributor

damm89 commented Oct 11, 2023

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
Copy link
Member Author

sergei-maertens commented Oct 11, 2023

@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
Copy link
Contributor

damm89 commented Oct 13, 2023

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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants