Skip to content

Commit

Permalink
💥 [#3283] Make ZGW API group service FKs non-nullable
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sergei-maertens committed Dec 12, 2024
1 parent 39daf50 commit 3f27cb1
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 69 deletions.
32 changes: 28 additions & 4 deletions bin/check_api_groups_null.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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

Expand All @@ -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

Expand Down
13 changes: 1 addition & 12 deletions src/openforms/registrations/contrib/zgw_apis/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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(
_(
Expand Down
16 changes: 6 additions & 10 deletions src/openforms/registrations/contrib/zgw_apis/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
@@ -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",
),
),
]
6 changes: 3 additions & 3 deletions src/openforms/registrations/contrib/zgw_apis/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,23 @@ 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",
verbose_name=_("Documenten API"),
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",
verbose_name=_("Catalogi API"),
on_delete=models.PROTECT,
limit_choices_to={"api_type": APITypes.ztc},
related_name="zgwset_ztc_config",
null=True,
null=False,
)

#
Expand Down
10 changes: 0 additions & 10 deletions src/openforms/registrations/contrib/zgw_apis/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 3f27cb1

Please sign in to comment.