From fc5996ff4d17b5a8849359a1783cf5fda984035f Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Mon, 4 Sep 2023 19:50:52 +0200 Subject: [PATCH 1/7] [#53] Making klantnummer optional and auto-generating klantnummer if not provided --- .../components/klanten/api/serializers.py | 7 +- .../klanten/api/tests/test_klant.py | 78 +++++++++++++++++++ .../components/klanten/models/klanten.py | 16 +++- 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/src/openklant/components/klanten/api/serializers.py b/src/openklant/components/klanten/api/serializers.py index 911aa04d..9ec443b6 100644 --- a/src/openklant/components/klanten/api/serializers.py +++ b/src/openklant/components/klanten/api/serializers.py @@ -297,6 +297,7 @@ class Meta: "url": {"lookup_field": "uuid"}, "subject": {"required": False, "validators": [URLValidator()]}, "subject_type": {"validators": [IsImmutableValidator()]}, + "klantnummer": {"required": False}, # Disabled for now, should return once logic is implemented # "geverifieerd": {"validators": [IsImmutableValidator()]}, # Disabled for now, see https://github.com/maykinmedia/open-klant/pull/11#pullrequestreview-805051480 @@ -315,6 +316,11 @@ def __init__(self, *args, **kwargs): self.fields[custom_field].help_text ) + def validate_klantnummer(self, value): + if not value: + value = str(Klant.objects.get_next_klantnummer()) + return value + def validate(self, attrs): validated_attrs = super().validate(attrs) subject = validated_attrs.get("subject", None) @@ -331,7 +337,6 @@ def validate(self, attrs): _("subject or subjectIdentificatie must be provided"), code="invalid-subject", ) - return validated_attrs def to_internal_value(self, data): diff --git a/src/openklant/components/klanten/api/tests/test_klant.py b/src/openklant/components/klanten/api/tests/test_klant.py index d1c5a3f7..2565c4a4 100644 --- a/src/openklant/components/klanten/api/tests/test_klant.py +++ b/src/openklant/components/klanten/api/tests/test_klant.py @@ -348,6 +348,84 @@ def test_create_klant_website_url_optional(self): self.assertEqual(klant.bronorganisatie, "950428139") self.assertEqual(klant.website_url, "") + def test_create_klant_website_url_optional(self): + list_url = reverse(Klant) + data = { + "bronorganisatie": "950428139", + "voornaam": "Xavier", + "achternaam": "Jackson", + "emailadres": "test@gmail.com", + "adres": { + "straatnaam": "Keizersgracht", + "huisnummer": "117", + "huisletter": "A", + "postcode": "1015CJ", + "woonplaatsnaam": "test", + "landcode": "1234", + }, + "subjectType": KlantType.natuurlijk_persoon, + "subject": SUBJECT, + } + + with requests_mock.Mocker() as m: + m.get(SUBJECT, json={}) + response = self.client.post(list_url, data) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + klant = Klant.objects.get() + + self.assertEqual(klant.bronorganisatie, "950428139") + self.assertEqual(klant.website_url, "") + self.assertTrue(klant.klantnummer != 0) + + def test_create_klant_website_url_duplicate_klantnummer(self): + list_url = reverse(Klant) + data = { + "bronorganisatie": "950428139", + "subjectType": KlantType.natuurlijk_persoon, + "klantnummer": "123", + "subject": SUBJECT, + } + + with requests_mock.Mocker() as m: + m.get(SUBJECT, json={}) + response = self.client.post(list_url, data) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + klant = Klant.objects.get() + + self.assertEqual(klant.bronorganisatie, "950428139") + self.assertEqual(klant.website_url, "") + self.assertEqual(klant.klantnummer, "123") + + with requests_mock.Mocker() as m: + m.get(SUBJECT, json={}) + response = self.client.post(list_url, data) + + self.assertEqual(response.status_code, 409) + klant = Klant.objects.get() + + def test_create_klant_website_url_invalid_klantnummer(self): + list_url = reverse(Klant) + data = { + "bronorganisatie": "950428139", + "subjectType": KlantType.natuurlijk_persoon, + "klantnummer": "123456789", + "subject": SUBJECT, + } + + with requests_mock.Mocker() as m: + m.get(SUBJECT, json={}) + response = self.client.post(list_url, data) + + self.assertEqual(response.status_code, 400) + + klanten = Klant.objects.all() + + self.assertFalse(klanten) + def test_create_klant_natuurlijkpersoon(self): list_url = reverse(Klant) data = { diff --git a/src/openklant/components/klanten/models/klanten.py b/src/openklant/components/klanten/models/klanten.py index 5b4a0ff4..8806dddd 100644 --- a/src/openklant/components/klanten/models/klanten.py +++ b/src/openklant/components/klanten/models/klanten.py @@ -6,12 +6,19 @@ from django.db import models from django.utils.translation import gettext_lazy as _ +from vng_api_common.exceptions import Conflict from vng_api_common.fields import BSNField, RSINField from vng_api_common.models import APIMixin from .constants import GeslachtsAanduiding, KlantType, SoortRechtsvorm +class KlantManager(models.Manager): + def get_next_klantnummer(self): + id_max = Klant.objects.all().aggregate(Max("klantnummer"))["id__max"] + return id_max + 1 if id_max else 1 + + class Klant(APIMixin, models.Model): uuid = models.UUIDField( unique=True, @@ -88,10 +95,17 @@ class Klant(APIMixin, models.Model): default=False, help_text=_("Geeft aan of de KLANT wel of niet geverifieerd is.") ) + objects = KlantManager() + class Meta: verbose_name = "klant" verbose_name_plural = "klanten" - unique_together = ("bronorganisatie", "klantnummer") + + def save(self, *args, **kwargs): + if not self.pk: + if Klant.objects.filter(klantnummer=self.klantnummer): + raise Conflict("Klantnummer bestaat al") + return super().save(*args, **kwargs) @property def subject_identificatie(self): From ccde650505a9513f55875d7c5c75eae85b3b5fd1 Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Mon, 4 Sep 2023 20:00:54 +0200 Subject: [PATCH 2/7] [#53] Ensuring auto-incrementing klantnummer works as expected --- .../components/klanten/api/tests/test_klant.py | 11 ++++++++++- src/openklant/components/klanten/models/klanten.py | 8 ++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/openklant/components/klanten/api/tests/test_klant.py b/src/openklant/components/klanten/api/tests/test_klant.py index 2565c4a4..7a5a0ed7 100644 --- a/src/openklant/components/klanten/api/tests/test_klant.py +++ b/src/openklant/components/klanten/api/tests/test_klant.py @@ -377,7 +377,16 @@ def test_create_klant_website_url_optional(self): self.assertEqual(klant.bronorganisatie, "950428139") self.assertEqual(klant.website_url, "") - self.assertTrue(klant.klantnummer != 0) + self.assertEqual(klant.klantnummer, "1") + + with requests_mock.Mocker() as m: + m.get(SUBJECT, json={}) + response = self.client.post(list_url, data) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + klant = Klant.objects.get(klantnummer__gt=1) + self.assertEqual(klant.klantnummer, "2") def test_create_klant_website_url_duplicate_klantnummer(self): list_url = reverse(Klant) diff --git a/src/openklant/components/klanten/models/klanten.py b/src/openklant/components/klanten/models/klanten.py index 8806dddd..0c16ed9d 100644 --- a/src/openklant/components/klanten/models/klanten.py +++ b/src/openklant/components/klanten/models/klanten.py @@ -15,8 +15,10 @@ class KlantManager(models.Manager): def get_next_klantnummer(self): - id_max = Klant.objects.all().aggregate(Max("klantnummer"))["id__max"] - return id_max + 1 if id_max else 1 + id_max = Klant.objects.all().aggregate(models.Max("klantnummer"))[ + "klantnummer__max" + ] + return int(id_max) + 1 if id_max else 1 class Klant(APIMixin, models.Model): @@ -102,6 +104,8 @@ class Meta: verbose_name_plural = "klanten" def save(self, *args, **kwargs): + if not self.klantnummer: + self.klantnummer = Klant.objects.get_next_klantnummer() if not self.pk: if Klant.objects.filter(klantnummer=self.klantnummer): raise Conflict("Klantnummer bestaat al") From 0dfc7eab111f5c20fc35aac6eb7f2ec103702ea9 Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Mon, 4 Sep 2023 20:03:36 +0200 Subject: [PATCH 3/7] [#53] Renaming test --- src/openklant/components/klanten/api/tests/test_klant.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openklant/components/klanten/api/tests/test_klant.py b/src/openklant/components/klanten/api/tests/test_klant.py index 7a5a0ed7..7d219eff 100644 --- a/src/openklant/components/klanten/api/tests/test_klant.py +++ b/src/openklant/components/klanten/api/tests/test_klant.py @@ -348,7 +348,7 @@ def test_create_klant_website_url_optional(self): self.assertEqual(klant.bronorganisatie, "950428139") self.assertEqual(klant.website_url, "") - def test_create_klant_website_url_optional(self): + def test_create_klant_website_url_optional_klantnummer(self): list_url = reverse(Klant) data = { "bronorganisatie": "950428139", From e79587d9e5d78bf738dea8f7beb166439837af30 Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Tue, 5 Sep 2023 14:11:34 +0200 Subject: [PATCH 4/7] [#53] Adding digit_validator to klantnummer --- .../components/klanten/api/tests/test_klant.py | 17 +++++++++++++++++ .../components/klanten/models/klanten.py | 2 ++ 2 files changed, 19 insertions(+) diff --git a/src/openklant/components/klanten/api/tests/test_klant.py b/src/openklant/components/klanten/api/tests/test_klant.py index 7d219eff..e1814d31 100644 --- a/src/openklant/components/klanten/api/tests/test_klant.py +++ b/src/openklant/components/klanten/api/tests/test_klant.py @@ -435,6 +435,23 @@ def test_create_klant_website_url_invalid_klantnummer(self): self.assertFalse(klanten) + data = { + "bronorganisatie": "950428139", + "subjectType": KlantType.natuurlijk_persoon, + "klantnummer": "KLANT1", + "subject": SUBJECT, + } + + with requests_mock.Mocker() as m: + m.get(SUBJECT, json={}) + response = self.client.post(list_url, data) + + self.assertEqual(response.status_code, 400) + + klanten = Klant.objects.all() + + self.assertFalse(klanten) + def test_create_klant_natuurlijkpersoon(self): list_url = reverse(Klant) data = { diff --git a/src/openklant/components/klanten/models/klanten.py b/src/openklant/components/klanten/models/klanten.py index 0c16ed9d..5f5c0a3b 100644 --- a/src/openklant/components/klanten/models/klanten.py +++ b/src/openklant/components/klanten/models/klanten.py @@ -9,6 +9,7 @@ from vng_api_common.exceptions import Conflict from vng_api_common.fields import BSNField, RSINField from vng_api_common.models import APIMixin +from vng_api_common.validators import validate_digits from .constants import GeslachtsAanduiding, KlantType, SoortRechtsvorm @@ -38,6 +39,7 @@ class Klant(APIMixin, models.Model): klantnummer = models.CharField( max_length=8, help_text=_("De unieke identificatie van de klant binnen de bronorganisatie."), + validators=[validate_digits], ) bedrijfsnaam = models.CharField( max_length=200, From ffca144d2789b9c99c09c59b34c7e34765828177 Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Tue, 5 Sep 2023 14:13:29 +0200 Subject: [PATCH 5/7] [#53] Removing unused call to get_next_klantnummer() --- src/openklant/components/klanten/api/serializers.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/openklant/components/klanten/api/serializers.py b/src/openklant/components/klanten/api/serializers.py index 9ec443b6..1738fa8b 100644 --- a/src/openklant/components/klanten/api/serializers.py +++ b/src/openklant/components/klanten/api/serializers.py @@ -316,11 +316,6 @@ def __init__(self, *args, **kwargs): self.fields[custom_field].help_text ) - def validate_klantnummer(self, value): - if not value: - value = str(Klant.objects.get_next_klantnummer()) - return value - def validate(self, attrs): validated_attrs = super().validate(attrs) subject = validated_attrs.get("subject", None) From 6a77891f6cd0bc80ab31b6aa94d402f5711dba4f Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Tue, 5 Sep 2023 14:15:27 +0200 Subject: [PATCH 6/7] [#53] Missing migration --- .../migrations/0005_auto_20230905_1215.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/openklant/components/klanten/migrations/0005_auto_20230905_1215.py diff --git a/src/openklant/components/klanten/migrations/0005_auto_20230905_1215.py b/src/openklant/components/klanten/migrations/0005_auto_20230905_1215.py new file mode 100644 index 00000000..c13b5ec9 --- /dev/null +++ b/src/openklant/components/klanten/migrations/0005_auto_20230905_1215.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.18 on 2023-09-05 12:15 + +import django.core.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('klanten', '0004_klant_geverifieerd'), + ] + + operations = [ + migrations.AlterField( + model_name='klant', + name='klantnummer', + field=models.CharField(help_text='De unieke identificatie van de klant binnen de bronorganisatie.', max_length=8, validators=[django.core.validators.RegexValidator(code='only-digits', message='Waarde moet numeriek zijn.', regex='^[0-9]+$')]), + ), + migrations.AlterUniqueTogether( + name='klant', + unique_together=set(), + ), + ] From 17854d1b7e7f22b0dc1ab4e4829339d8a98c81b3 Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Tue, 5 Sep 2023 15:19:27 +0200 Subject: [PATCH 7/7] [#53] Blacking migration --- .../migrations/0005_auto_20230905_1215.py | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/openklant/components/klanten/migrations/0005_auto_20230905_1215.py b/src/openklant/components/klanten/migrations/0005_auto_20230905_1215.py index c13b5ec9..cdf7e472 100644 --- a/src/openklant/components/klanten/migrations/0005_auto_20230905_1215.py +++ b/src/openklant/components/klanten/migrations/0005_auto_20230905_1215.py @@ -5,19 +5,28 @@ class Migration(migrations.Migration): - dependencies = [ - ('klanten', '0004_klant_geverifieerd'), + ("klanten", "0004_klant_geverifieerd"), ] operations = [ migrations.AlterField( - model_name='klant', - name='klantnummer', - field=models.CharField(help_text='De unieke identificatie van de klant binnen de bronorganisatie.', max_length=8, validators=[django.core.validators.RegexValidator(code='only-digits', message='Waarde moet numeriek zijn.', regex='^[0-9]+$')]), + model_name="klant", + name="klantnummer", + field=models.CharField( + help_text="De unieke identificatie van de klant binnen de bronorganisatie.", + max_length=8, + validators=[ + django.core.validators.RegexValidator( + code="only-digits", + message="Waarde moet numeriek zijn.", + regex="^[0-9]+$", + ) + ], + ), ), migrations.AlterUniqueTogether( - name='klant', + name="klant", unique_together=set(), ), ]