👌 [#458] PR Feedback

Author: SilviaAmAm

backend/requirements/base.in

@@ -36,7 +36,7 @@ elastic-apm  # Elastic APM integration
 celery
 
 # Additional libraries
-zgw-consumers[setup-configuration] @ git+https://github.com/maykinmedia/zgw-consumers@feature/django-setup-configuration#egg=zgw-consumers
+zgw-consumers[setup-configuration]
 furl
 python-slugify
 XlsxWriter

backend/src/openarchiefbeheer/config/setup_configuration/steps.py

@@ -1,21 +1,11 @@
 from django_setup_configuration import BaseConfigurationStep
+from django_setup_configuration.exceptions import ConfigurationRunFailed
 from zgw_consumers.models import Service
 
 from ..models import APIConfig
 from .models import APIConfigConfigurationModel
 
 
-def get_service(slug: str) -> Service:
-    """
-    Try to find a Service and re-raise DoesNotExist with the identifier to make debugging
-    easier
-    """
-    try:
-        return Service.objects.get(slug=slug)
-    except Service.DoesNotExist as e:
-        raise Service.DoesNotExist(f"{str(e)} (identifier = {slug})")
-
-
 class APIConfigConfigurationStep(BaseConfigurationStep[APIConfigConfigurationModel]):
     """Configure API settings"""
 
@@ -25,16 +15,16 @@ class APIConfigConfigurationStep(BaseConfigurationStep[APIConfigConfigurationMod
     verbose_name = "API Configuration"
 
     def execute(self, model: APIConfigConfigurationModel) -> None:
-        service = get_service(model.selectielijst_service_identifier)
-
         config = APIConfig.get_solo()
 
-        # Idempotent configuration
-        if (
-            config.selectielijst_api_service
-            and service.pk == config.selectielijst_api_service.pk
-        ):
-            return
-
-        config.selectielijst_api_service = service
-        config.save()
+        try:
+            config.selectielijst_api_service = Service.objects.get(
+                slug=model.selectielijst_service_identifier
+            )
+        except Service.DoesNotExist:
+            raise ConfigurationRunFailed(
+                f"Could not find an existing `selectielijst` service with identifier `{model.selectielijst_service_identifier}`."
+                " Make sure it is already configured, manually or by first running the configuration step of `zgw_consumers`."
+            )
+
+        config.save(update_fields=["selectielijst_api_service"])

backend/src/openarchiefbeheer/config/setup_configuration/tests/files/setup_config_api_different_service.yaml

@@ -1,20 +1,18 @@
 from pathlib import Path
-from unittest.mock import Mock, patch
 
 from django.core.cache import cache
 from django.test import TestCase
 
+from django_setup_configuration.exceptions import ConfigurationRunFailed
 from django_setup_configuration.test_utils import execute_single_step
 from zgw_consumers.constants import APITypes
-from zgw_consumers.models import Service
 from zgw_consumers.test.factories import ServiceFactory
 
 from ...models import APIConfig
 from ..steps import APIConfigConfigurationStep
 
 TEST_FILES = (Path(__file__).parent / "files").resolve()
-CONFIG_FILE_PATH_1 = str(TEST_FILES / "setup_config_api.yaml")
-CONFIG_FILE_PATH_2 = str(TEST_FILES / "setup_config_api_different_service.yaml")
+CONFIG_FILE_PATH = str(TEST_FILES / "setup_config_api.yaml")
 
 
 class APIConfigConfigurationStepTests(TestCase):
@@ -29,10 +27,13 @@ def test_configure_api_config_create_new(self):
             api_type=APITypes.orc,
             api_root="https://selectielijst.openzaak.nl/api/v1/",
         )
+        config = APIConfig.get_solo()
 
-        execute_single_step(APIConfigConfigurationStep, yaml_source=CONFIG_FILE_PATH_1)
+        self.assertIsNone(config.selectielijst_api_service)
 
-        config = APIConfig.get_solo()
+        execute_single_step(APIConfigConfigurationStep, yaml_source=CONFIG_FILE_PATH)
+
+        config.refresh_from_db()
 
         self.assertEqual(service.pk, config.selectielijst_api_service.pk)
 
@@ -48,36 +49,22 @@ def test_configure_api_config_update_existing(self):
         config.selectielijst_api_service = service1
         config.save()
 
-        execute_single_step(APIConfigConfigurationStep, yaml_source=CONFIG_FILE_PATH_2)
+        execute_single_step(
+            APIConfigConfigurationStep,
+            object_source={
+                "api_configuration_enabled": True,
+                "api_configuration": {
+                    "selectielijst_service_identifier": "selectielijst-new"
+                },
+            },
+        )
 
         config.refresh_from_db()
 
         self.assertEqual(service2.pk, config.selectielijst_api_service.pk)
 
     def test_configure_api_config_missing_service(self):
-        with self.assertRaises(Service.DoesNotExist):
+        with self.assertRaises(ConfigurationRunFailed):
             execute_single_step(
-                APIConfigConfigurationStep, yaml_source=CONFIG_FILE_PATH_1
+                APIConfigConfigurationStep, yaml_source=CONFIG_FILE_PATH
             )
-
-    def test_idempotency(self):
-        service = ServiceFactory(
-            slug="selectielijst",
-            api_type=APITypes.orc,
-            api_root="https://selectielijst.openzaak.nl/api/v1/",
-        )
-
-        execute_single_step(APIConfigConfigurationStep, yaml_source=CONFIG_FILE_PATH_1)
-
-        mock = Mock()
-        config = APIConfig(selectielijst_api_service=service)
-        with patch.object(config, "save", new=mock.method):
-            with patch(
-                "openarchiefbeheer.config.setup_configuration.steps.APIConfig.get_solo",
-                return_value=config,
-            ):
-                execute_single_step(
-                    APIConfigConfigurationStep, yaml_source=CONFIG_FILE_PATH_1
-                )
-
-        mock.method.assert_not_called()

backend/src/openarchiefbeheer/config/setup_configuration/tests/test_setup_configuration.py

@@ -67,7 +67,6 @@ services:
     depends_on:
       - db
       - redis
-      - web
     networks:
       - open-archiefbeheer-dev