Merge pull request #4916 from open-formulieren/cleanup/3283-make-fields-not-nullable

💥 Make service FKs in API group config models not nullable

Author: sergei-maertens

Dockerfile

@@ -93,6 +93,7 @@ COPY \
     ./bin/check_celery_worker_liveness.py \
     ./bin/report_component_problems.py \
     ./bin/check_temporary_uploads.py \
+    ./bin/check_api_groups_null.py \
     ./bin/fix_selectboxes_component_default_values.py \
     ./bin/
 

bin/check_api_groups_null.py

@@ -0,0 +1,80 @@
+#!/usr/bin/env python
+import sys
+from pathlib import Path
+
+import django
+
+from tabulate import tabulate
+
+SRC_DIR = Path(__file__).parent.parent / "src"
+sys.path.insert(0, str(SRC_DIR.resolve()))
+
+
+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,
+    ).values_list("id", "name", "objects_service_id", "objecttypes_service_id")
+
+    for pk, name, objects_service_id, objecttypes_service_id in objects_groups:
+        problem = ("Objects API", pk, name)
+        if objects_service_id is None:
+            problems.append((*problem, "No objects service configured"))
+        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
+
+    print(
+        "Can't upgrade yet - some API group services are not properly configured yet."
+    )
+    print(
+        "Go into the admin to fix their configuration, and then try to upgrade again."
+    )
+    print(
+        tabulate(
+            problems,
+            headers=("API group type", "ID", "Name", "Problem"),
+        )
+    )
+    return True
+
+
+def main(skip_setup=False) -> bool:
+    from openforms.setup import setup_env
+
+    if not skip_setup:
+        setup_env()
+        django.setup()
+
+    return check_for_null_services_in_api_groups()
+
+
+if __name__ == "__main__":
+    main()

src/openforms/contrib/objects_api/clients/__init__.py

@@ -9,6 +9,8 @@
   in the form builder
 """
 
+from __future__ import annotations
+
 from typing import TYPE_CHECKING
 
 from zgw_consumers.client import build_client
@@ -26,25 +28,25 @@ class NoServiceConfigured(RuntimeError):
     pass
 
 
-def get_objects_client(config: "ObjectsAPIGroupConfig") -> ObjectsClient:
-    if not (service := config.objects_service):
-        raise NoServiceConfigured("No Objects API service configured!")
+def get_objects_client(config: ObjectsAPIGroupConfig) -> ObjectsClient:
+    service = config.objects_service
+    assert service is not None
     return build_client(service, client_factory=ObjectsClient)
 
 
-def get_objecttypes_client(config: "ObjectsAPIGroupConfig") -> ObjecttypesClient:
-    if not (service := config.objecttypes_service):
-        raise NoServiceConfigured("No Objecttypes API service configured!")
+def get_objecttypes_client(config: ObjectsAPIGroupConfig) -> ObjecttypesClient:
+    service = config.objecttypes_service
+    assert service is not None
     return build_client(service, client_factory=ObjecttypesClient)
 
 
-def get_documents_client(config: "ObjectsAPIGroupConfig") -> DocumentenClient:
+def get_documents_client(config: ObjectsAPIGroupConfig) -> DocumentenClient:
     if not (service := config.drc_service):
         raise NoServiceConfigured("No Documents API service configured!")
     return build_client(service, client_factory=DocumentenClient)
 
 
-def get_catalogi_client(config: "ObjectsAPIGroupConfig") -> CatalogiClient:
+def get_catalogi_client(config: ObjectsAPIGroupConfig) -> CatalogiClient:
     if not (service := config.catalogi_service):
         raise NoServiceConfigured("No Catalogi API service configured!")
     return build_client(service, client_factory=CatalogiClient)

src/openforms/contrib/objects_api/migrations/0004_alter_objectsapigroupconfig_objects_service_and_more.py

@@ -0,0 +1,37 @@
+# Generated by Django 4.2.17 on 2024-12-12 22:40
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("zgw_consumers", "0022_set_default_service_slug"),
+        ("objects_api", "0003_objectsapigroupconfig_identifier_unique"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="objectsapigroupconfig",
+            name="objects_service",
+            field=models.ForeignKey(
+                limit_choices_to={"api_type": "orc"},
+                on_delete=django.db.models.deletion.PROTECT,
+                related_name="+",
+                to="zgw_consumers.service",
+                verbose_name="Objects API",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="objectsapigroupconfig",
+            name="objecttypes_service",
+            field=models.ForeignKey(
+                limit_choices_to={"api_type": "orc"},
+                on_delete=django.db.models.deletion.PROTECT,
+                related_name="+",
+                to="zgw_consumers.service",
+                verbose_name="Objecttypes API",
+            ),
+        ),
+    ]

src/openforms/contrib/objects_api/models.py

@@ -12,7 +12,6 @@
 
 
 class ObjectsAPIGroupConfig(models.Model):
-    # TODO OF 3.0: remove `null=True` from the service fields
     name = models.CharField(
         _("name"),
         max_length=255,
@@ -30,15 +29,15 @@ class ObjectsAPIGroupConfig(models.Model):
         verbose_name=_("Objects API"),
         on_delete=models.PROTECT,
         limit_choices_to={"api_type": APITypes.orc},
-        null=True,
+        null=False,
         related_name="+",
     )
     objecttypes_service = models.ForeignKey(
         "zgw_consumers.Service",
         verbose_name=_("Objecttypes API"),
         on_delete=models.PROTECT,
         limit_choices_to={"api_type": APITypes.orc},
-        null=True,
+        null=False,
         related_name="+",
     )
     drc_service = models.ForeignKey(

src/openforms/contrib/objects_api/tests/test_admin.py

@@ -9,7 +9,6 @@
 from openforms.accounts.tests.factories import SuperUserFactory
 from openforms.utils.tests.vcr import OFVCRMixin
 
-from ..models import ObjectsAPIGroupConfig
 from .factories import ObjectsAPIGroupConfigFactory
 
 TEST_FILES = Path(__file__).parent / "files"
@@ -20,7 +19,7 @@ class ObjectsAPIGroupConfigAdminTest(OFVCRMixin, WebTest):
     VCR_TEST_FILES = TEST_FILES
 
     def test_name(self):
-        ObjectsAPIGroupConfig.objects.create(name="test group")
+        ObjectsAPIGroupConfigFactory.create(name="test group")
         user = SuperUserFactory.create()
 
         response = self.app.get(

src/openforms/prefill/contrib/objects_api/tests/test_config.py

@@ -1,9 +1,6 @@
 from pathlib import Path
 from unittest.mock import patch
 
-from django.test import override_settings
-from django.utils.translation import gettext as _
-
 from rest_framework.test import APITestCase
 from zgw_consumers.constants import APITypes, AuthTypes
 from zgw_consumers.test.factories import ServiceFactory
@@ -41,19 +38,6 @@ def setUp(self):
             for_test_docker_compose=True
         )
 
-    @override_settings(LANGUAGE_CODE="en")
-    def test_undefined_service_raises_exception(self):
-        self.objects_api_group.objects_service = None
-        self.objects_api_group.save()
-
-        with self.assertRaisesMessage(
-            InvalidPluginConfiguration,
-            _(
-                "Objects API endpoint is not configured for Objects API group {objects_api_group}."
-            ).format(objects_api_group=self.objects_api_group),
-        ):
-            plugin.check_config()
-
     def test_invalid_service_raises_exception(self):
         objects_service = ServiceFactory.create(
             api_root="http://localhost:8002/api/v2/invalid",

src/openforms/registrations/contrib/objects_api/config.py

@@ -1,6 +1,6 @@
 import warnings
 
-from django.db.models import IntegerChoices, Q
+from django.db.models import IntegerChoices
 from django.utils.translation import gettext_lazy as _
 
 from drf_spectacular.utils import OpenApiExample, extend_schema_serializer
@@ -67,11 +67,9 @@ class ObjecttypeVariableMappingSerializer(serializers.Serializer):
 
 class ObjectsAPIOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer):
     objects_api_group = PrimaryKeyRelatedAsChoicesField(
-        queryset=ObjectsAPIGroupConfig.objects.exclude(
-            Q(objects_service=None)
-            | Q(objecttypes_service=None)
-            | Q(drc_service=None)
-            | Q(catalogi_service=None)
+        queryset=ObjectsAPIGroupConfig.objects.filter(
+            drc_service__isnull=False,
+            catalogi_service__isnull=False,
         ),
         label=("Objects API group"),
         help_text=_("Which Objects API group to use."),

src/openforms/registrations/contrib/objects_api/tests/test_api_endpoints.py

@@ -4,34 +4,16 @@
 from rest_framework.reverse import reverse_lazy
 from rest_framework.test import APITestCase
 from zgw_consumers.constants import APITypes, AuthTypes
-from zgw_consumers.models import Service
 from zgw_consumers.test.factories import ServiceFactory
 
 from openforms.accounts.tests.factories import StaffUserFactory, UserFactory
 from openforms.contrib.objects_api.tests.factories import ObjectsAPIGroupConfigFactory
 from openforms.utils.tests.feature_flags import enable_feature_flag
 from openforms.utils.tests.vcr import OFVCRMixin
 
-from ..models import ObjectsAPIGroupConfig
-
 TEST_FILES = Path(__file__).parent / "files"
 
 
-def get_test_config() -> ObjectsAPIGroupConfig:
-    """Returns a preconfigured ``ObjectsAPIGroupConfig`` instance matching the docker compose configuration."""
-
-    return ObjectsAPIGroupConfig(
-        objecttypes_service=Service(
-            api_root="http://localhost:8001/api/v2/",
-            api_type=APITypes.orc,
-            oas="https://example.com/",
-            header_key="Authorization",
-            header_value="Token 171be5abaf41e7856b423ad513df1ef8f867ff48",
-            auth_type=AuthTypes.api_key,
-        )
-    )
-
-
 class ObjecttypesAPIEndpointTests(OFVCRMixin, APITestCase):
 
     VCR_TEST_FILES = TEST_FILES
@@ -40,9 +22,8 @@ class ObjecttypesAPIEndpointTests(OFVCRMixin, APITestCase):
     @classmethod
     def setUpTestData(cls) -> None:
         super().setUpTestData()
-        cls.config = get_test_config()
-        cls.config.objecttypes_service.save()
-        cls.config.save()
+
+        cls.config = ObjectsAPIGroupConfigFactory.create(for_test_docker_compose=True)
 
     def test_auth_required(self):
         response = self.client.get(self.endpoint)
@@ -101,9 +82,8 @@ class ObjecttypeVersionsAPIEndpointTests(OFVCRMixin, APITestCase):
     @classmethod
     def setUpTestData(cls) -> None:
         super().setUpTestData()
-        cls.config = get_test_config()
-        cls.config.objecttypes_service.save()
-        cls.config.save()
+
+        cls.config = ObjectsAPIGroupConfigFactory.create(for_test_docker_compose=True)
 
     def test_auth_required(self):
         response = self.client.get(self.endpoint)
@@ -156,9 +136,8 @@ class TargetPathsAPIEndpointTests(OFVCRMixin, APITestCase):
     @classmethod
     def setUpTestData(cls) -> None:
         super().setUpTestData()
-        cls.config = get_test_config()
-        cls.config.objecttypes_service.save()
-        cls.config.save()
+
+        cls.config = ObjectsAPIGroupConfigFactory.create(for_test_docker_compose=True)
 
     def test_auth_required(self):
         response = self.client.post(self.endpoint)

src/openforms/registrations/contrib/objects_api/tests/test_config_checks.py

@@ -40,28 +40,6 @@ def _mockForValidServiceConfiguration(self, m: requests_mock.Mocker) -> None:
             headers={"API-version": "1.0.0"},
         )
 
-    def test_no_objects_service_configured(self):
-        self.config.objects_service = None
-        self.config.save()
-        plugin = ObjectsAPIRegistration(PLUGIN_IDENTIFIER)
-
-        with self.assertRaises(InvalidPluginConfiguration):
-            plugin.check_config()
-
-    @requests_mock.Mocker()
-    def test_no_objecttypes_service_configured(self, m: requests_mock.Mocker):
-        # Objects API needs to be set up as services are checked in a certain order
-        m.get(
-            "https://objects.example.com/api/v1/objects?pageSize=1",
-            json={"results": []},
-        )
-        self.config.objecttypes_service = None
-        self.config.save()
-        plugin = ObjectsAPIRegistration(PLUGIN_IDENTIFIER)
-
-        with self.assertRaises(InvalidPluginConfiguration):
-            plugin.check_config()
-
     @requests_mock.Mocker()
     def test_objects_service_misconfigured_connection_error(self, m):
         m.get(