👌 [#4787] Process PR feedback

* add type: ignore to specific lines in steps.py instead of ignoring entire file with pyright
* several docs improvements
* remove unused SCRIPTPATH variable in wait_for_db.sh and setup_configuration.sh
* move tests to objects_api.tests directory

.gitignore

@@ -62,3 +62,7 @@ src/openforms/static/sdk
 # Custom extensions symlinks
 src/prefill_haalcentraalhr
 src/token_exchange
+
+# docker volumes
+setup_configuration/*
+!setup_configuration/.gitkeep

bin/docker_start.sh

@@ -11,7 +11,7 @@ uwsgi_threads=${UWSGI_THREADS:-1}
 mountpoint=${SUBPATH:-/}
 
 # wait for required services
-${SCRIPTPATH}/wait_for_db.sh
+/wait_for_db.sh
 
 # Apply database migrations
 >&2 echo "Apply database migrations"

bin/setup_configuration.sh

@@ -7,7 +7,7 @@ set -e
 
 if [[ "${RUN_SETUP_CONFIG,,}" =~ ^(true|1|yes)$ ]]; then
     # wait for required services
-    ${SCRIPTPATH}/wait_for_db.sh
+    /wait_for_db.sh
 
     src/manage.py migrate
     src/manage.py setup_configuration --yaml-file setup_configuration/data.yaml

bin/wait_for_db.sh

@@ -1,6 +1,6 @@
 #!/bin/sh
 
-set -e
+set -ex
 
 # Wait for the database container
 # See: https://docs.docker.com/compose/startup-order/

docs/configuration/index.rst

@@ -7,6 +7,12 @@ There are many configuration options in Open Forms. Some of these are included
 in the core of Open Forms, and some are included by plugins. We cover various
 configuration topics that come with Open Forms by default.
 
+Initial configuration
+---------------------
+
+Open Forms supports the ``setup_configuration`` management command, which allows loading configuration via
+YAML files. The shape of these files is described at :ref:`installation_configuration_cli`.
+
 .. toctree::
    :maxdepth: 2
 

docs/installation/config.rst

@@ -392,13 +392,6 @@ variables, linking to the description of their behaviour in their respective mod
      A formal and more complete authentication context data model is used - existing
      installations likely do not provide all this information yet.
 
-
-Initial configuration
----------------------
-
-Open Forms supports the ``setup_configuration`` management command, which allows loading configuration via
-YAML files. The shape of these files is described at :ref:`installation_configuration_cli`.
-
 Specifying the environment variables
 =====================================
 

docs/installation/index.rst

@@ -16,6 +16,8 @@ and expertise.
    If you don't want to install Open Forms yourself, you can ask an Open Forms
    supplier to host and manage it for you.
 
+After installation, follow the :ref:`configuration_index` instructions to enable all available features.
+
 .. toctree::
    :maxdepth: 1
    :caption: Further reading

docs/installation/setup_configuration.rst

@@ -6,7 +6,7 @@ Open Forms configuration (CLI)
 
 After deploying Open Forms, it needs to be configured to be fully functional. The
 command line tool ``setup_configuration`` assist with this configuration by loading a
-YAML file in which the configuration information is specified
+YAML file in which the configuration information is specified.
 
 You can get the full command documentation with:
 
@@ -43,13 +43,10 @@ Execution
 Open Forms configuration
 ------------------------
 
-With the full command invocation, everything is configured at once. Each configuration step
-is idempotent, so any manual changes made via the admin interface will be updated if the command
-is run afterwards.
+With the full command invocation, all defined configuration steps are applied. Each step is idempotent,
+so it's safe to run the command multiple times. The steps will overwrite any manual changes made in
+the admin if you run the command after making these changes.
 
 .. code-block:: bash
 
     src/manage.py setup_configuration
-
-.. note:: Due to a cache-bug in the underlying framework, you need to restart all
-   replicas for part of this change to take effect everywhere.

pyright.pyproject.toml

@@ -53,8 +53,6 @@ exclude = [
     "src/openforms/formio/formatters/tests/",
     "src/openforms/registrations/contrib/zgw_apis/tests/test_backend_partial_failure.py",
     "src/openforms/registrations/contrib/zgw_apis/tests/test_utils.py",
-    # setup_configuration typing doesn't work for `django_model_refs` yet (https://github.com/maykinmedia/django-setup-configuration/issues/25)
-    "src/openforms/contrib/objects_api/setup_configuration/steps.py",
 ]
 ignore = []
 

src/openforms/conf/base.py

@@ -1206,6 +1206,14 @@
     ],
 }
 
+#
+# DJANGO-SETUP-CONFIGURATION
+#
+SETUP_CONFIGURATION_STEPS = [
+    "zgw_consumers.contrib.setup_configuration.steps.ServiceConfigurationStep",
+    "openforms.contrib.objects_api.setup_configuration.steps.ObjectsAPIConfigurationStep",
+]
+
 #
 # Open Forms extensions
 #
@@ -1214,9 +1222,3 @@
 
 if OPEN_FORMS_EXTENSIONS:
     INSTALLED_APPS += OPEN_FORMS_EXTENSIONS
-
-
-SETUP_CONFIGURATION_STEPS = [
-    "zgw_consumers.contrib.setup_configuration.steps.ServiceConfigurationStep",
-    "openforms.contrib.objects_api.setup_configuration.steps.ObjectsAPIConfigurationStep",
-]

src/openforms/contrib/objects_api/setup_configuration/steps.py

@@ -30,15 +30,18 @@ class ObjectsAPIConfigurationStep(BaseConfigurationStep[ObjectsAPIGroupConfigMod
     def execute(self, model: ObjectsAPIGroupConfigModel):
         config: SingleObjectsAPIGroupConfigModel
         for config in model.groups:
+            # setup_configuration typing doesn't work for `django_model_refs` yet,
+            # hence the type: ignores
+            # (https://github.com/maykinmedia/django-setup-configuration/issues/25)
             defaults = {
-                "name": config.name,
+                "name": config.name,  # type: ignore
                 "objects_service": get_service(config.objects_service_identifier),
                 "objecttypes_service": get_service(
                     config.objecttypes_service_identifier
                 ),
-                "catalogue_domain": config.catalogue_domain,
-                "catalogue_rsin": config.catalogue_rsin,
-                "organisatie_rsin": config.organisatie_rsin,
+                "catalogue_domain": config.catalogue_domain,  # type: ignore
+                "catalogue_rsin": config.catalogue_rsin,  # type: ignore
+                "organisatie_rsin": config.organisatie_rsin,  # type: ignore
                 "iot_submission_report": config.document_type_submission_report,
                 "iot_submission_csv": config.document_type_submission_csv,
                 "iot_attachment": config.document_type_attachment,
@@ -53,6 +56,6 @@ def execute(self, model: ObjectsAPIGroupConfigModel):
                 )
 
             ObjectsAPIGroupConfig.objects.update_or_create(
-                identifier=config.identifier,
+                identifier=config.identifier,  # type: ignore
                 defaults=defaults,
             )

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

@@ -1,5 +1,3 @@
-from django.utils.text import slugify
-
 import factory
 from zgw_consumers.constants import APITypes, AuthTypes
 from zgw_consumers.test.factories import ServiceFactory
@@ -9,7 +7,7 @@
 
 class ObjectsAPIGroupConfigFactory(factory.django.DjangoModelFactory):
     name = factory.Sequence(lambda n: f"Objects API group {n:03}")
-    identifier = factory.LazyAttribute(lambda o: slugify(o.name))
+    identifier = factory.Sequence(lambda n: f"objects-api-group-{n}")
     objects_service = factory.SubFactory(
         "zgw_consumers.test.factories.ServiceFactory", api_type=APITypes.orc
     )

src/openforms/contrib/objects_api/setup_configuration/tests/test_objects_api_config.py → src/openforms/contrib/objects_api/tests/setup_configuration/test_objects_api_config.py

@@ -8,10 +8,11 @@
 from zgw_consumers.test.factories import ServiceFactory
 
 from openforms.contrib.objects_api.models import ObjectsAPIGroupConfig
+from openforms.contrib.objects_api.setup_configuration.steps import (
+    ObjectsAPIConfigurationStep,
+)
 from openforms.contrib.objects_api.tests.factories import ObjectsAPIGroupConfigFactory
 
-from ..steps import ObjectsAPIConfigurationStep
-
 TEST_FILES = (Path(__file__).parent / "files").resolve()
 CONFIG_FILE_PATH = str(TEST_FILES / "setup_config_objects_api.yaml")
 CONFIG_FILE_PATH_REQUIRED_FIELDS = str(
@@ -25,10 +26,11 @@
 class ObjectsAPIConfigurationStepTests(TestCase):
     maxDiff = None
 
-    def setUp(self):
-        super().setUp()
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
 
-        self.objecttypes_service = ServiceFactory.create(
+        cls.objecttypes_service = ServiceFactory.create(
             slug="objecttypen-test",
             label="Objecttypen API test",
             api_root="http://localhost:8001/api/v2/",
@@ -37,7 +39,7 @@ def setUp(self):
             header_key="Authorization",
             header_value="Token foo",
         )
-        self.objects_service = ServiceFactory.create(
+        cls.objects_service = ServiceFactory.create(
             slug="objecten-test",
             label="Objecten API test",
             api_root="http://localhost:8002/api/v2/",
@@ -46,7 +48,7 @@ def setUp(self):
             header_key="Authorization",
             header_value="Token bar",
         )
-        self.drc_service = ServiceFactory.create(
+        cls.drc_service = ServiceFactory.create(
             slug="documenten-test",
             label="Documenten API test",
             api_root="http://localhost:8003/documenten/api/v1/",
@@ -55,7 +57,7 @@ def setUp(self):
             client_id="test_client_id",
             secret="test_secret_key",
         )
-        self.catalogi_service = ServiceFactory.create(
+        cls.catalogi_service = ServiceFactory.create(
             slug="catalogi-test",
             label="Catalogi API test",
             api_root="http://localhost:8003/catalogi/api/v1/",
@@ -70,7 +72,7 @@ def test_execute_success(self):
 
         self.assertEqual(ObjectsAPIGroupConfig.objects.count(), 2)
 
-        config1, config2 = ObjectsAPIGroupConfig.objects.all()
+        config1, config2 = ObjectsAPIGroupConfig.objects.order_by("pk")
 
         self.assertEqual(config1.name, "Config 1")
         self.assertEqual(config1.identifier, "config-1")
@@ -105,33 +107,13 @@ def test_execute_update_existing_config(self):
 
         self.assertEqual(ObjectsAPIGroupConfig.objects.count(), 2)
 
-        config1, config2 = ObjectsAPIGroupConfig.objects.all()
+        config1, config2 = ObjectsAPIGroupConfig.objects.order_by("pk")
 
         self.assertEqual(config1.name, "Config 1")
         self.assertEqual(config1.identifier, "config-1")
-        self.assertEqual(config1.objects_service, self.objects_service)
-        self.assertEqual(config1.objecttypes_service, self.objecttypes_service)
-        self.assertEqual(config1.drc_service, self.drc_service)
-        self.assertEqual(config1.catalogi_service, self.catalogi_service)
-        self.assertEqual(config1.catalogue_domain, "TEST")
-        self.assertEqual(config1.catalogue_rsin, "000000000")
-        self.assertEqual(config1.organisatie_rsin, "000000000")
-        self.assertEqual(config1.iot_submission_report, "PDF Informatieobjecttype")
-        self.assertEqual(config1.iot_submission_csv, "CSV Informatieobjecttype")
-        self.assertEqual(config1.iot_attachment, "Attachment Informatieobjecttype")
 
         self.assertEqual(config2.name, "Config 2")
         self.assertEqual(config2.identifier, "config-2")
-        self.assertEqual(config2.objects_service, self.objects_service)
-        self.assertEqual(config2.objecttypes_service, self.objecttypes_service)
-        self.assertEqual(config2.drc_service, self.drc_service)
-        self.assertEqual(config2.catalogi_service, self.catalogi_service)
-        self.assertEqual(config2.catalogue_domain, "OTHER")
-        self.assertEqual(config2.catalogue_rsin, "000000000")
-        self.assertEqual(config2.organisatie_rsin, "000000000")
-        self.assertEqual(config2.iot_submission_report, "")
-        self.assertEqual(config2.iot_submission_csv, "")
-        self.assertEqual(config2.iot_attachment, "")
 
     def test_execute_with_required_fields(self):
         execute_single_step(
@@ -147,8 +129,8 @@ def test_execute_with_required_fields(self):
         self.assertEqual(config.objects_service, self.objects_service)
         self.assertEqual(config.objecttypes_service, self.objecttypes_service)
 
-        self.assertEqual(config.drc_service, None)
-        self.assertEqual(config.catalogi_service, None)
+        self.assertIsNone(config.drc_service)
+        self.assertIsNone(config.catalogi_service)
         self.assertEqual(config.catalogue_domain, "")
         self.assertEqual(config.catalogue_rsin, "")
         self.assertEqual(config.organisatie_rsin, "")
@@ -179,35 +161,22 @@ def test_execute_with_all_fields(self):
         self.assertEqual(config.iot_attachment, "Attachment Informatieobjecttype")
 
     def test_execute_is_idempotent(self):
-        def make_assertions():
-            self.assertEqual(ObjectsAPIGroupConfig.objects.count(), 1)
+        self.assertFalse(ObjectsAPIGroupConfig.objects.exists())
 
-            config = ObjectsAPIGroupConfig.objects.get()
-
-            self.assertEqual(config.name, "Config 1")
-            self.assertEqual(config.identifier, "config-1")
-            self.assertEqual(config.objects_service, self.objects_service)
-            self.assertEqual(config.objecttypes_service, self.objecttypes_service)
-            self.assertEqual(config.drc_service, self.drc_service)
-            self.assertEqual(config.catalogi_service, self.catalogi_service)
-            self.assertEqual(config.catalogue_domain, "TEST")
-            self.assertEqual(config.catalogue_rsin, "000000000")
-            self.assertEqual(config.organisatie_rsin, "000000000")
-            self.assertEqual(config.iot_submission_report, "PDF Informatieobjecttype")
-            self.assertEqual(config.iot_submission_csv, "CSV Informatieobjecttype")
-            self.assertEqual(config.iot_attachment, "Attachment Informatieobjecttype")
-
-        execute_single_step(
-            ObjectsAPIConfigurationStep, yaml_source=CONFIG_FILE_PATH_ALL_FIELDS
-        )
+        with self.subTest("run step first time"):
+            execute_single_step(
+                ObjectsAPIConfigurationStep, yaml_source=CONFIG_FILE_PATH_ALL_FIELDS
+            )
 
-        make_assertions()
+            self.assertEqual(ObjectsAPIGroupConfig.objects.count(), 1)
 
-        execute_single_step(
-            ObjectsAPIConfigurationStep, yaml_source=CONFIG_FILE_PATH_ALL_FIELDS
-        )
+        with self.subTest("run step second time"):
+            execute_single_step(
+                ObjectsAPIConfigurationStep, yaml_source=CONFIG_FILE_PATH_ALL_FIELDS
+            )
 
-        make_assertions()
+            # no additional configs created, but existing one updated
+            self.assertEqual(ObjectsAPIGroupConfig.objects.count(), 1)
 
     def test_execute_service_not_found_raises_error(self):
         self.objecttypes_service.delete()

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

@@ -18,7 +18,7 @@ def test_identifiers_generated(self):
         ObjectsAPIGroupConfig = self.apps.get_model(
             "objects_api", "ObjectsAPIGroupConfig"
         )
-        groups = ObjectsAPIGroupConfig.objects.all()
+        groups = ObjectsAPIGroupConfig.objects.order_by("pk")
 
         self.assertEqual(groups.count(), 3)