[#267] Improvements

Author: danielmursa-dev

src/openklant/components/klantinteracties/admin/partijen.py

@@ -30,7 +30,6 @@ class Meta:
 
     def clean(self):
         cleaned_data = super().clean()
-        sub_identificator_van = cleaned_data["sub_identificator_van"]
         partij_identificator = {
             "code_objecttype": cleaned_data["partij_identificator_code_objecttype"],
             "code_soort_object_id": cleaned_data[
@@ -47,16 +46,13 @@ def clean(self):
         queryset = PartijIdentificator.objects.exclude()
         if self.instance:
             queryset = queryset.exclude(pk=self.instance.pk)
+
         PartijIdentificatorUniquenessValidator(
-            queryset=(
-                PartijIdentificator.objects.exclude(pk=self.instance.pk)
-                if self.instance
-                else PartijIdentificator.objects.all()
-            ),
-            partij_identificator=partij_identificator,
-            sub_identificator_van=sub_identificator_van,
             instance=self.instance,
-        ).check()
+            partij_identificator=partij_identificator,
+            sub_identificator_van=cleaned_data["sub_identificator_van"],
+            partij=cleaned_data["partij"],
+        ).validate()
 
         return cleaned_data
 

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

@@ -46,7 +46,7 @@
     PartijIdentificatorTypesValidator,
     PartijIdentificatorUniquenessValidator,
 )
-from openklant.utils.serializers import get_field_value
+from openklant.utils.serializers import get_field_instance_by_uuid, get_field_value
 
 
 class PartijForeignkeyBaseSerializer(serializers.HyperlinkedModelSerializer):
@@ -422,48 +422,24 @@ class Meta:
     def validate(self, attrs):
         instance = getattr(self, "instance", None)
         partij_identificator = get_field_value(self, attrs, "partij_identificator")
-        sub_identificator_van = get_field_value(self, attrs, "sub_identificator_van")
-        partij = get_field_value(self, attrs, "partij")
-        if "sub_identificator_van" in attrs and sub_identificator_van:
-            sub_identificator_van = PartijIdentificator.objects.get(
-                uuid=sub_identificator_van["uuid"]
-            )
+
+        sub_identificator_van = get_field_instance_by_uuid(
+            self, attrs, "sub_identificator_van", PartijIdentificator
+        )
+        partij = get_field_instance_by_uuid(self, attrs, "partij", Partij)
 
         PartijIdentificatorUniquenessValidator(
-            queryset=(
-                PartijIdentificator.objects.exclude(pk=instance.pk)
-                if instance
-                else PartijIdentificator.objects.all()
-            ),
+            instance=instance,
             partij_identificator=partij_identificator,
             sub_identificator_van=sub_identificator_van,
-            instance=instance,
-        ).check()
-        return super().validate(attrs)
+            partij=partij,
+        ).validate()
 
-    @transaction.atomic
-    def update(self, instance, validated_data):
-        if partij := validated_data.get("partij", None):
-            validated_data["partij"] = Partij.objects.get(uuid=partij["uuid"])
-        # TODO FAI QUESTO test
-        breakpoint()
-        if sub_identificator_van := validated_data.get("sub_identificator_van", None):
-            validated_data["sub_identificator_van"] = PartijIdentificator.objects.get(
-                uuid=sub_identificator_van["uuid"]
-            )
-        return super().update(instance, validated_data)
-
-    @transaction.atomic
-    def create(self, validated_data):
-        if sub_identificator_van := validated_data.get("sub_identificator_van", None):
-            validated_data["sub_identificator_van"] = PartijIdentificator.objects.get(
-                uuid=sub_identificator_van["uuid"]
-            )
-
-        if partij := validated_data.get("partij", None):
-            validated_data["partij"] = Partij.objects.get(uuid=partij["uuid"])
-
-        return super().create(validated_data)
+        attrs["sub_identificator_van"] = get_field_instance_by_uuid(
+            self, attrs, "sub_identificator_van", PartijIdentificator
+        )
+        attrs["partij"] = get_field_instance_by_uuid(self, attrs, "partij", Partij)
+        return super().validate(attrs)
 
 
 class PartijSerializer(NestedGegevensGroepMixin, PolymorphicSerializer):

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

@@ -2144,31 +2144,28 @@ def test_invalid_validation_partij_identificator_object_id(self):
             "Deze waarde is ongeldig, reden: Waarde moet 9 tekens lang zijn",
         )
 
-
-class PartijIdentificatorUniquenessTests(APITestCase):
-    def setUp(self):
-        self.list_url = reverse("klantinteracties:partijidentificator-list")
-        self.partij = PartijFactory.create()
-        super().setUp()
-
     def test_invalid_create_empty(self):
         # all partij_identificator fields required
+        partij = PartijFactory.create()
+        list_url = reverse("klantinteracties:partijidentificator-list")
         data = {
-            "identificeerdePartij": {"uuid": str(self.partij.uuid)},
+            "identificeerdePartij": {"uuid": str(partij.uuid)},
             "anderePartijIdentificator": "anderePartijIdentificator",
             "partijIdentificator": {},
         }
 
-        response = self.client.post(self.list_url, data)
+        response = self.client.post(list_url, data)
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(response.data["code"], "invalid")
         self.assertEqual(response.data["title"], "Invalid input.")
         self.assertEqual(len(response.data["invalid_params"]), 4)
 
     def test_invalid_create_partial(self):
         # all partij_identificator fields required
+        partij = PartijFactory.create()
+        list_url = reverse("klantinteracties:partijidentificator-list")
         data = {
-            "identificeerdePartij": {"uuid": str(self.partij.uuid)},
+            "identificeerdePartij": {"uuid": str(partij.uuid)},
             "anderePartijIdentificator": "anderePartijIdentificator",
             "partijIdentificator": {
                 "codeObjecttype": "natuurlijk_persoon",
@@ -2177,7 +2174,7 @@ def test_invalid_create_partial(self):
             },
         }
 
-        response = self.client.post(self.list_url, data)
+        response = self.client.post(list_url, data)
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(response.data["code"], "invalid")
         self.assertEqual(response.data["title"], "Invalid input.")
@@ -2192,8 +2189,10 @@ def test_invalid_create_partial(self):
 
     def test_invalid_update_partial(self):
         # all partij_identificator fields required
+        partij = PartijFactory.create()
+        list_url = reverse("klantinteracties:partijidentificator-list")
         partij_identificator = PartijIdentificatorFactory.create(
-            partij=self.partij,
+            partij=partij,
             andere_partij_identificator="anderePartijIdentificator",
             partij_identificator_code_objecttype="natuurlijk_persoon",
             partij_identificator_code_soort_object_id="bsn",
@@ -2216,7 +2215,7 @@ def test_invalid_update_partial(self):
 
         response = self.client.patch(detail_url, data)
 
-        response = self.client.post(self.list_url, data)
+        response = self.client.post(list_url, data)
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(response.data["code"], "invalid")
         self.assertEqual(response.data["title"], "Invalid input.")
@@ -2229,6 +2228,13 @@ def test_invalid_update_partial(self):
         )
         self.assertEqual(PartijIdentificator.objects.all().count(), 1)
 
+
+class PartijIdentificatorUniquenessTests(APITestCase):
+    def setUp(self):
+        self.list_url = reverse("klantinteracties:partijidentificator-list")
+        self.partij = PartijFactory.create()
+        super().setUp()
+
     def test_bsn_valid_create(self):
         data = {
             "identificeerdePartij": {"uuid": str(self.partij.uuid)},
@@ -2245,49 +2251,30 @@ def test_bsn_valid_create(self):
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         self.assertEqual(PartijIdentificator.objects.all().count(), 1)
 
-    def test_bsn_invalid_create_for_same_partij_unique(self):
+    def test_bsn_valid_create_sub_identificator_van(self):
+        # sub_identificator_van not allowed for partij_identificator_code_soort_object_id = 'bsn'
+        partij_identificator = PartijIdentificatorFactory.create(
+            partij=self.partij,
+            andere_partij_identificator="anderePartijIdentificator",
+            partij_identificator_code_objecttype="natuurlijk_persoon",
+            partij_identificator_code_soort_object_id="bsn",
+            partij_identificator_object_id="123456782",
+            partij_identificator_code_register="brp",
+        )
+
         data = {
             "identificeerdePartij": {"uuid": str(self.partij.uuid)},
-            "anderePartijIdentificator": "anderePartijIdentificator",
             "partijIdentificator": {
                 "codeObjecttype": "natuurlijk_persoon",
                 "codeSoortObjectId": "bsn",
                 "objectId": "296648875",
                 "codeRegister": "brp",
             },
         }
-
+        data["sub_identificator_van"] = {"uuid": str(partij_identificator.uuid)}
         response = self.client.post(self.list_url, data)
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        self.assertEqual(PartijIdentificator.objects.all().count(), 1)
-
-        with self.subTest("failed_subtest_same_partij_1"):
-            # same partij, same data_values
-            response = self.client.post(self.list_url, data)
-            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-            self.assertEqual(response.data["code"], "invalid")
-            self.assertEqual(response.data["title"], "Invalid input.")
-            self.assertEqual(
-                response.data["invalid_params"][0]["reason"],
-                "`PartijIdentificator` moet uniek zijn, er bestaat er al een met deze gegevenscombinatie.",
-            )
-
-        with self.subTest("failed_subtest_same_partij_2"):
-            # same partij, new data_values
-            data = {
-                "identificeerdePartij": {"uuid": str(self.partij.uuid)},
-                "anderePartijIdentificator": "anderePartijIdentificator",
-                "partijIdentificator": {
-                    "codeObjecttype": "natuurlijk_persoon",
-                    "codeSoortObjectId": "bsn",
-                    "objectId": "123456782",
-                    "codeRegister": "brp",
-                },
-            }
-
-            response = self.client.post(self.list_url, data)
-            self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-            self.assertEqual(PartijIdentificator.objects.all().count(), 2)
+        self.assertEqual(PartijIdentificator.objects.all().count(), 2)
 
     def test_bsn_invalid_create_global_unique(self):
         PartijIdentificatorFactory.create(
@@ -2332,28 +2319,6 @@ def test_bsn_invalid_create_global_unique(self):
                 "`PartijIdentificator` moet uniek zijn, er bestaat er al een met deze gegevenscombinatie.",
             )
 
-    def test_bsn_invalid_create_sub_identificator_van(self):
-        # sub_identificator_van not allowed for partij_identificator_code_soort_object_id = 'bsn'
-        partij_identificator = PartijIdentificatorFactory.create()
-        data = {
-            "identificeerdePartij": {"uuid": str(self.partij.uuid)},
-            "partijIdentificator": {
-                "codeObjecttype": "natuurlijk_persoon",
-                "codeSoortObjectId": "bsn",
-                "objectId": "296648875",
-                "codeRegister": "brp",
-            },
-        }
-        data["sub_identificator_van"] = {"uuid": str(partij_identificator.uuid)}
-        response = self.client.post(self.list_url, data)
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(response.data["code"], "invalid")
-        self.assertEqual(response.data["title"], "Invalid input.")
-        self.assertEqual(
-            response.data["invalid_params"][0]["reason"],
-            "Alleen een identifier met code_soort_object_id kan een `sub_identificator_van' hebben.",
-        )
-
     def test_bsn_valid_partial_update_unique(self):
         partij_identificator = PartijIdentificatorFactory.create(
             partij=self.partij,
@@ -2571,54 +2536,6 @@ def test_vestigingsnummer_invalid_create_invalid_sub_identificator_van(self):
             "Het is alleen mogelijk om sub_identifier_vans te selecteren die CodeSoortObjectId = `kvk_nummer` hebben.",
         )
 
-    def test_vestigingsnummer_valid_create(self):
-        sub_identificator_van = PartijIdentificatorFactory.create(
-            partij=self.partij,
-            partij_identificator_code_objecttype="niet_natuurlijk_persoon",
-            partij_identificator_code_soort_object_id="kvk_nummer",
-            partij_identificator_object_id="12345678",
-            partij_identificator_code_register="hr",
-        )
-
-        data = {
-            "identificeerdePartij": {"uuid": str(self.partij.uuid)},
-            "sub_identificator_van": {"uuid": str(sub_identificator_van.uuid)},
-            "partijIdentificator": {
-                "codeObjecttype": "vestiging",
-                "codeSoortObjectId": "vestigingsnummer",
-                "objectId": "296648875154",
-                "codeRegister": "hr",
-            },
-        }
-
-        response = self.client.post(self.list_url, data)
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        partij_identificatoren = PartijIdentificator.objects.all()
-        self.assertEqual(partij_identificatoren.count(), 2)
-
-        with self.subTest("subtest_global_1"):
-            # create new with objectId changed
-            data["partijIdentificator"]["objectId"] = "123412341234"
-            response = self.client.post(self.list_url, data)
-            self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-            partij_identificatoren = PartijIdentificator.objects.all()
-            self.assertEqual(partij_identificatoren.count(), 3)
-
-        with self.subTest("subtest_global_2"):
-            # create new with sub_identificator_van changed
-            sub_identificator_van = PartijIdentificatorFactory.create(
-                partij=self.partij,
-                partij_identificator_code_objecttype="niet_natuurlijk_persoon",
-                partij_identificator_code_soort_object_id="kvk_nummer",
-                partij_identificator_object_id="87654321",
-                partij_identificator_code_register="hr",
-            )
-            data["sub_identificator_van"] = {"uuid": str(sub_identificator_van.uuid)}
-            response = self.client.post(self.list_url, data)
-            self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-            partij_identificatoren = PartijIdentificator.objects.all()
-            self.assertEqual(partij_identificatoren.count(), 5)
-
     def test_vestigingsnummer_invalid_create(self):
         sub_identificator_van = PartijIdentificatorFactory.create(
             partij=self.partij,
@@ -2733,6 +2650,43 @@ def test_vestigingsnummer_invalid_update_set_sub_identificator_van_null(self):
             in response.data["invalid_params"][0]["reason"]
         )
 
+    def test_invalid_vestigingsnummer_and_kvk_nummer_combination_unique(self):
+        sub_identificator_van = PartijIdentificatorFactory.create(
+            partij=self.partij,
+            partij_identificator_code_objecttype="niet_natuurlijk_persoon",
+            partij_identificator_code_soort_object_id="kvk_nummer",
+            partij_identificator_object_id="12345678",
+            partij_identificator_code_register="hr",
+        )
+        PartijIdentificatorFactory.create(
+            partij=self.partij,
+            sub_identificator_van=sub_identificator_van,
+            partij_identificator_code_objecttype="vestiging",
+            partij_identificator_code_soort_object_id="vestigingsnummer",
+            partij_identificator_object_id="296648875154",
+            partij_identificator_code_register="hr",
+        )
+        # Same sub_identificator_van and same data_values
+        data = {
+            "identificeerdePartij": {"uuid": str(self.partij.uuid)},
+            "sub_identificator_van": {"uuid": str(sub_identificator_van.uuid)},
+            "partijIdentificator": {
+                "codeObjecttype": "vestiging",
+                "codeSoortObjectId": "vestigingsnummer",
+                "objectId": "296648875154",
+                "codeRegister": "hr",
+            },
+        }
+
+        response = self.client.post(self.list_url, data)
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(response.data["code"], "invalid")
+        self.assertEqual(response.data["title"], "Invalid input.")
+        self.assertEqual(
+            response.data["invalid_params"][0]["reason"],
+            "`PartijIdentificator` moet uniek zijn, er bestaat er al een met deze gegevenscombinatie.",
+        )
+
     def test_vestigingsnummer_invalid_update_unique(self):
         sub_identificator_van_a = PartijIdentificatorFactory.create(
             partij=self.partij,
@@ -2756,7 +2710,7 @@ def test_vestigingsnummer_invalid_update_unique(self):
             partij_identificator_object_id="12345678",
             partij_identificator_code_register="hr",
         )
-        partij_identificator_b = PartijIdentificatorFactory.create(
+        PartijIdentificatorFactory.create(
             partij=self.partij,
             sub_identificator_van=sub_identificator_van_b,
             partij_identificator_code_objecttype="vestiging",