From 3f27cb13015ed032eb3164105254cd1c6d6afebc Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Thu, 12 Dec 2024 23:50:41 +0100 Subject: [PATCH] :boom: [#3283] Make ZGW API group service FKs non-nullable Left-overs from a django-solo model too (see previous commit), these fields really cannot be empty to do anything useful. This properly sets up the required constraints at the database level and removes confusion in the application code. --- bin/check_api_groups_null.py | 32 +++++++++++-- .../registrations/contrib/zgw_apis/checks.py | 13 +---- .../registrations/contrib/zgw_apis/client.py | 16 +++---- ..._zgwapigroupconfig_drc_service_and_more.py | 48 +++++++++++++++++++ .../registrations/contrib/zgw_apis/models.py | 6 +-- .../contrib/zgw_apis/tests/test_models.py | 10 ---- .../zgw_apis/tests/test_zgw_api_config.py | 30 ------------ 7 files changed, 86 insertions(+), 69 deletions(-) create mode 100644 src/openforms/registrations/contrib/zgw_apis/migrations/0018_alter_zgwapigroupconfig_drc_service_and_more.py diff --git a/bin/check_api_groups_null.py b/bin/check_api_groups_null.py index 36df0ad59c..3b638e6f4c 100755 --- a/bin/check_api_groups_null.py +++ b/bin/check_api_groups_null.py @@ -12,11 +12,13 @@ def check_for_null_services_in_api_groups(): from openforms.contrib.objects_api.models import ObjectsAPIGroupConfig + from openforms.registrations.contrib.zgw_apis.models import ZGWApiGroupConfig problems: list[tuple[str, int, str, str]] = [] objects_groups = ObjectsAPIGroupConfig.objects.exclude( - objects_service__isnull=False, objecttypes_service__isnull=False + objects_service__isnull=False, + objecttypes_service__isnull=False, ).values_list("id", "name", "objects_service_id", "objecttypes_service_id") for pk, name, objects_service_id, objecttypes_service_id in objects_groups: @@ -26,6 +28,26 @@ def check_for_null_services_in_api_groups(): if objecttypes_service_id is None: problems.append((*problem, "No object types service configured")) + zgw_groups = ZGWApiGroupConfig.objects.exclude( + zrc_service__isnull=False, + drc_service__isnull=False, + ztc_service__isnull=False, + ).values_list( + "id", + "name", + "zrc_service_id", + "drc_service_id", + "ztc_service_id", + ) + for pk, name, zrc_service_id, drc_service_id, ztc_service_id in zgw_groups: + problem = ("ZGW APIs", pk, name) + if zrc_service_id is None: + problems.append((*problem, "No Zaken API service configured")) + if drc_service_id is None: + problems.append((*problem, "No Documenten API service configured")) + if ztc_service_id is None: + problems.append((*problem, "No Catalogi API service configured")) + if not problems: return False @@ -35,9 +57,11 @@ def check_for_null_services_in_api_groups(): print( "Go into the admin to fix their configuration, and then try to upgrade again." ) - tabulate( - problems, - headers=("API group type", "ID", "Name", "Problem"), + print( + tabulate( + problems, + headers=("API group type", "ID", "Name", "Problem"), + ) ) return True diff --git a/src/openforms/registrations/contrib/zgw_apis/checks.py b/src/openforms/registrations/contrib/zgw_apis/checks.py index ec6c9a048d..a34e30dc30 100644 --- a/src/openforms/registrations/contrib/zgw_apis/checks.py +++ b/src/openforms/registrations/contrib/zgw_apis/checks.py @@ -4,12 +4,7 @@ from openforms.plugins.exceptions import InvalidPluginConfiguration -from .client import ( - NoServiceConfigured, - get_catalogi_client, - get_documents_client, - get_zaken_client, -) +from .client import get_catalogi_client, get_documents_client, get_zaken_client from .models import ZGWApiGroupConfig @@ -43,12 +38,6 @@ def check_config(): try: check_function(config) - except NoServiceConfigured as exc: - raise InvalidPluginConfiguration( - _( - "{api_name} endpoint is not configured for ZGW API set {zgw_api_set}." - ).format(api_name=api_name, zgw_api_set=config.name) - ) from exc except requests.RequestException as exc: raise InvalidPluginConfiguration( _( diff --git a/src/openforms/registrations/contrib/zgw_apis/client.py b/src/openforms/registrations/contrib/zgw_apis/client.py index f486cfdaf8..644a5aca50 100644 --- a/src/openforms/registrations/contrib/zgw_apis/client.py +++ b/src/openforms/registrations/contrib/zgw_apis/client.py @@ -16,23 +16,19 @@ from .models import ZGWApiGroupConfig -class NoServiceConfigured(RuntimeError): - pass - - def get_zaken_client(config: ZGWApiGroupConfig) -> ZakenClient: - if not (service := config.zrc_service): - raise NoServiceConfigured("No Zaken API service configured!") + service = config.zrc_service + assert service is not None return build_client(service, client_factory=ZakenClient) def get_documents_client(config: ZGWApiGroupConfig) -> DocumentenClient: - if not (service := config.drc_service): - raise NoServiceConfigured("No Documents API service configured!") + service = config.drc_service + assert service is not None return build_client(service, client_factory=DocumentenClient) def get_catalogi_client(config: ZGWApiGroupConfig) -> CatalogiClient: - if not (service := config.ztc_service): - raise NoServiceConfigured("No Catalogi API service configured!") + service = config.ztc_service + assert service is not None return build_client(service, client_factory=CatalogiClient) diff --git a/src/openforms/registrations/contrib/zgw_apis/migrations/0018_alter_zgwapigroupconfig_drc_service_and_more.py b/src/openforms/registrations/contrib/zgw_apis/migrations/0018_alter_zgwapigroupconfig_drc_service_and_more.py new file mode 100644 index 0000000000..37768a9422 --- /dev/null +++ b/src/openforms/registrations/contrib/zgw_apis/migrations/0018_alter_zgwapigroupconfig_drc_service_and_more.py @@ -0,0 +1,48 @@ +# Generated by Django 4.2.17 on 2024-12-12 22:49 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("zgw_consumers", "0022_set_default_service_slug"), + ("zgw_apis", "0017_alter_zgwapigroupconfig_identifier"), + ] + + operations = [ + migrations.AlterField( + model_name="zgwapigroupconfig", + name="drc_service", + field=models.ForeignKey( + limit_choices_to={"api_type": "drc"}, + on_delete=django.db.models.deletion.PROTECT, + related_name="zgwset_drc_config", + to="zgw_consumers.service", + verbose_name="Documenten API", + ), + ), + migrations.AlterField( + model_name="zgwapigroupconfig", + name="zrc_service", + field=models.ForeignKey( + limit_choices_to={"api_type": "zrc"}, + on_delete=django.db.models.deletion.PROTECT, + related_name="zgwset_zrc_config", + to="zgw_consumers.service", + verbose_name="Zaken API", + ), + ), + migrations.AlterField( + model_name="zgwapigroupconfig", + name="ztc_service", + field=models.ForeignKey( + limit_choices_to={"api_type": "ztc"}, + on_delete=django.db.models.deletion.PROTECT, + related_name="zgwset_ztc_config", + to="zgw_consumers.service", + verbose_name="Catalogi API", + ), + ), + ] diff --git a/src/openforms/registrations/contrib/zgw_apis/models.py b/src/openforms/registrations/contrib/zgw_apis/models.py index c911a4978e..aa9f6f8f6d 100644 --- a/src/openforms/registrations/contrib/zgw_apis/models.py +++ b/src/openforms/registrations/contrib/zgw_apis/models.py @@ -67,7 +67,7 @@ class ZGWApiGroupConfig(models.Model): on_delete=models.PROTECT, limit_choices_to={"api_type": APITypes.zrc}, related_name="zgwset_zrc_config", - null=True, + null=False, ) drc_service = models.ForeignKey( "zgw_consumers.Service", @@ -75,7 +75,7 @@ class ZGWApiGroupConfig(models.Model): on_delete=models.PROTECT, limit_choices_to={"api_type": APITypes.drc}, related_name="zgwset_drc_config", - null=True, + null=False, ) ztc_service = models.ForeignKey( "zgw_consumers.Service", @@ -83,7 +83,7 @@ class ZGWApiGroupConfig(models.Model): on_delete=models.PROTECT, limit_choices_to={"api_type": APITypes.ztc}, related_name="zgwset_ztc_config", - null=True, + null=False, ) # diff --git a/src/openforms/registrations/contrib/zgw_apis/tests/test_models.py b/src/openforms/registrations/contrib/zgw_apis/tests/test_models.py index 28287f6154..6a87d688fe 100644 --- a/src/openforms/registrations/contrib/zgw_apis/tests/test_models.py +++ b/src/openforms/registrations/contrib/zgw_apis/tests/test_models.py @@ -69,16 +69,6 @@ def test_validate_no_catalogue_specified(self): except ValidationError as exc: raise self.failureException("Not specifying a catalogue is valid.") from exc - def test_catalogue_specified_but_ztc_service_is_missing(self): - config = ZGWApiGroupConfigFactory.create( - ztc_service=None, catalogue_domain="TEST", catalogue_rsin="000000000" - ) - - with self.assertRaises(ValidationError) as exc_context: - config.full_clean() - - self.assertIn("ztc_service", exc_context.exception.error_dict) - def test_validate_catalogue_exists(self): # validates against the fixtures in docker/open-zaak config = ZGWApiGroupConfigFactory.create( diff --git a/src/openforms/registrations/contrib/zgw_apis/tests/test_zgw_api_config.py b/src/openforms/registrations/contrib/zgw_apis/tests/test_zgw_api_config.py index f5826f26ae..d8e7774545 100644 --- a/src/openforms/registrations/contrib/zgw_apis/tests/test_zgw_api_config.py +++ b/src/openforms/registrations/contrib/zgw_apis/tests/test_zgw_api_config.py @@ -536,36 +536,6 @@ def test_check_config(self, m): self.assertEqual(list_documenten_2.hostname, "documenten-2.nl") self.assertEqual(list_zaaktypen_2.hostname, "catalogi-2.nl") - def test_check_config_no_zrc_service(self, m): - self.install_mocks(m) - self.zgw_group1.zrc_service = None - self.zgw_group1.save() - - plugin = ZGWRegistration("zgw") - - with self.assertRaises(InvalidPluginConfiguration): - plugin.check_config() - - def test_check_config_no_drc_service(self, m): - self.install_mocks(m) - self.zgw_group1.drc_service = None - self.zgw_group1.save() - - plugin = ZGWRegistration("zgw") - - with self.assertRaises(InvalidPluginConfiguration): - plugin.check_config() - - def test_check_config_no_ztc_service(self, m): - self.install_mocks(m) - self.zgw_group1.ztc_service = None - self.zgw_group1.save() - - plugin = ZGWRegistration("zgw") - - with self.assertRaises(InvalidPluginConfiguration): - plugin.check_config() - def test_check_config_http_error(self, m): self.install_mocks(m) m.get("https://zaken-1.nl/api/v1/zaken", exc=HTTPError)