✨ [#147] Convenience endpoint for Klantcontact/Betrokkene/Onderwerpobject #260

stevenbal · 2024-10-03T13:37:55Z

Fixes #147

Changes

Convenience endpoint for Klantcontact/Betrokkene/Onderwerpobject
Fix incorrect viewname for onderwerpobject URL attribute
Add typehints to remove redoc warnings
Fix tests not running in CI due to changed-files action

TODO:

rename endpoint to maak-klantcontact

stevenbal · 2024-10-03T14:43:51Z

src/openklant/components/klantinteracties/api/urls.py

@@ -48,6 +49,11 @@
 router.register("betrokkenen", BetrokkeneViewSet)
 router.register("onderwerpobjecten", OnderwerpobjectViewSet)
 router.register("bijlagen", BijlageViewSet)
+router.register(
+    "klantcontact-convenience",


not sure how else I could name it without being way too long 😬

I think it is a good name 👍

stevenbal · 2024-10-03T14:44:38Z

I had several questions, mostly related to the datamodel itself, because I haven't looked at it before:

should it be possible to pass Onderwerpobject.wasKlantcontact via convenience endpoint? Why does this field exist in the first place? -> yes
should it be possible to create without Onderwerpobject? -> yes
why does Betrokkene have had_klantcontact/klantcontact and was_partij/partij? -> naming to avoid having to use "was_partij" everywhere in the code

joeribekker · 2024-10-10T15:24:39Z

@stevenbal nog even bespreken? Voordat je merged :)

stevenbal · 2024-10-14T07:12:14Z

@joeribekker ja dat lijkt me goed, was ook nog niet van plan het te mergen :p

joeribekker · 2024-10-15T08:50:57Z

@stevenbal please contact me on Slack to discuss when you pick this up again.

stevenbal · 2024-10-17T13:07:50Z

@bart-maykin I discussed with Joeri and I made both Betrokkene and OnderwerpObject optional on the convenience endpoint

SonnyBA · 2024-10-17T13:24:45Z

.github/workflows/ci.yml

You should be able to rebase onto master whenever #268 gets merged

SonnyBA · 2024-10-17T13:27:34Z

src/openklant/components/klantinteracties/api/serializers/klantcontacten.py

@@ -367,7 +367,7 @@ class Meta:
        extra_kwargs = {
            "uuid": {"read_only": True},
            "url": {
-                "view_name": "klantinteracties:klantcontact-detail",
+                "view_name": "klantinteracties:onderwerpobject-detail",


Not sure how this wasn't caught by tests 😬

src/openklant/components/klantinteracties/api/serializers/klantcontacten.py

SonnyBA · 2024-10-17T13:32:02Z

src/openklant/components/klantinteracties/api/serializers/klantcontacten.py

+        Create the objects and use the original serializers to ensure all the correct
+        fields show up in the response
+        """
+        klantcontact_data = validated_data["klantcontact"]


What happens here when no klantcontact was given in the request data? Will it raise a KeyError?

Because it is required on the serializer, you will get a validation error:

{ "type": "http://localhost:8000/ref/fouten/ValidationError/", "code": "invalid", "title": "Invalid input.", "status": 400, "detail": "", "instance": "urn:uuid:5f54a6d6-5327-4ad5-a831-27b7be3c1035", "invalidParams": [ { "name": "klantcontact", "code": "required", "reason": "Dit veld is vereist." } ] }

src/openklant/components/klantinteracties/api/viewsets/klantcontacten.py

SonnyBA · 2024-10-17T13:52:54Z

src/openklant/components/klantinteracties/api/urls.py

@@ -48,6 +49,11 @@
 router.register("betrokkenen", BetrokkeneViewSet)
 router.register("onderwerpobjecten", OnderwerpobjectViewSet)
 router.register("bijlagen", BijlageViewSet)
+router.register(
+    "klantcontact-convenience",


I think it is a good name 👍

src/openklant/components/klantinteracties/api/urls.py

src/openklant/components/klantinteracties/api/tests/test_klantcontacten.py

SonnyBA · 2024-10-17T15:16:10Z

src/openklant/components/klantinteracties/api/serializers/klantcontacten.py

+        betrokkene = None
+        if betrokkene_data := validated_data.pop("betrokkene", None):
+            betrokkene_data["had_klantcontact"] = {"uuid": str(klantcontact.uuid)}
+            # TODO for some reason `was_partij` is converted to `partij` by the serializer


It is caused by the source option given to the was_partij field

open-klant/src/openklant/components/klantinteracties/api/serializers/klantcontacten.py

Line 139 in 2963f4c

source="partij",

Hmm I see, I don't think there's a way around this, since it always gets deserialized to partij right?

Yeah, I never knew this was how the source option behaved 😮. The comment can be removed though right?

previously this used the Klantcontact view, causing incorrect URLs to appear in the detail response for the Onderwerpobject endpoint

…ject to allow creation of these three resources in a single API call

… endpoint because it should be possible to create e.g.: - a KlantContact with a Betrokkene but no OnderwerpObject - a KlantContact with an OnderwerpObject but no Betrokkene

SonnyBA · 2024-10-18T15:01:15Z

src/openklant/components/klantinteracties/api/tests/factories.py

Nice this saved the tests some code 👍

SonnyBA · 2024-10-18T15:03:17Z

src/openklant/components/klantinteracties/api/viewsets/klantcontacten.py

+    """
+
+    serializer_class = KlantContactConvenienceSerializer
+    authentication_classes = (TokenAuthentication,)


In think this should be the default for the API but is not part of the ticket 😀

SonnyBA · 2024-10-18T15:07:55Z

src/openklant/components/klantinteracties/api/serializers/klantcontacten.py

+        betrokkene = None
+        if betrokkene_data := validated_data.pop("betrokkene", None):
+            betrokkene_data["had_klantcontact"] = {"uuid": str(klantcontact.uuid)}
+            # TODO for some reason `was_partij` is converted to `partij` by the serializer


Yeah, I never knew this was how the source option behaved 😮. The comment can be removed though right?

stevenbal marked this pull request as draft October 3, 2024 13:38

stevenbal force-pushed the feature/147-convenience-endpoint branch 5 times, most recently from 4709654 to d03fd1b Compare October 3, 2024 14:38

stevenbal marked this pull request as ready for review October 3, 2024 14:42

stevenbal requested review from SonnyBA, annashamray and bart-maykin October 3, 2024 14:43

stevenbal commented Oct 3, 2024

View reviewed changes

stevenbal force-pushed the feature/147-convenience-endpoint branch from dc1e6ba to 61d3f1f Compare October 7, 2024 14:28

bart-maykin approved these changes Oct 8, 2024

View reviewed changes

stevenbal requested a review from bart-maykin October 17, 2024 13:06

SonnyBA reviewed Oct 17, 2024

View reviewed changes

stevenbal added 5 commits October 18, 2024 10:55

🐛 Fix incorrect viewname for onderwerpobject URL attribute

abec9a7

previously this used the Klantcontact view, causing incorrect URLs to appear in the detail response for the Onderwerpobject endpoint

✨ [#147] Convenience endpoint for Klantcontact/Betrokkene/Onderwerpob…

c4664f6

…ject to allow creation of these three resources in a single API call

✅ [#147] Add tests for convenience endpoint

2483b95

🏷️ Add typehints to remove redoc warnings

74b50b2

♻️ [#147] Make Betrokkene and OnderwerpObject optional in convenience…

13203d1

… endpoint because it should be possible to create e.g.: - a KlantContact with a Betrokkene but no OnderwerpObject - a KlantContact with an OnderwerpObject but no Betrokkene

stevenbal force-pushed the feature/147-convenience-endpoint branch from 8c883f0 to 424d9fb Compare October 18, 2024 08:57

stevenbal requested a review from SonnyBA October 18, 2024 08:57

👌 [#147] PR feedback

ba76269

stevenbal force-pushed the feature/147-convenience-endpoint branch from 424d9fb to ba76269 Compare October 18, 2024 09:11

SonnyBA approved these changes Oct 18, 2024

View reviewed changes

stevenbal marked this pull request as draft October 22, 2024 10:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ [#147] Convenience endpoint for Klantcontact/Betrokkene/Onderwerpobject #260

✨ [#147] Convenience endpoint for Klantcontact/Betrokkene/Onderwerpobject #260

stevenbal commented Oct 3, 2024 •

edited

Loading

stevenbal Oct 3, 2024

SonnyBA Oct 17, 2024

stevenbal commented Oct 3, 2024 •

edited

Loading

joeribekker commented Oct 10, 2024

stevenbal commented Oct 14, 2024

joeribekker commented Oct 15, 2024

stevenbal commented Oct 17, 2024

SonnyBA Oct 17, 2024

SonnyBA Oct 17, 2024

stevenbal Oct 18, 2024

SonnyBA Oct 17, 2024

stevenbal Oct 18, 2024

SonnyBA Oct 17, 2024

SonnyBA Oct 17, 2024

stevenbal Oct 18, 2024

SonnyBA Oct 18, 2024

SonnyBA Oct 18, 2024

SonnyBA Oct 18, 2024

SonnyBA Oct 18, 2024

✨ [#147] Convenience endpoint for Klantcontact/Betrokkene/Onderwerpobject #260

Are you sure you want to change the base?

✨ [#147] Convenience endpoint for Klantcontact/Betrokkene/Onderwerpobject #260

Conversation

stevenbal commented Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenbal commented Oct 3, 2024 • edited Loading

joeribekker commented Oct 10, 2024

stevenbal commented Oct 14, 2024

joeribekker commented Oct 15, 2024

stevenbal commented Oct 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenbal commented Oct 3, 2024 •

edited

Loading

stevenbal commented Oct 3, 2024 •

edited

Loading