From edd2b7299dd457ed28c7b16fd2cfc47672639f7d Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Fri, 6 Dec 2024 14:56:47 +0100 Subject: [PATCH] :ok_hand: [#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 | 4 + bin/docker_start.sh | 2 +- bin/setup_configuration.sh | 2 +- bin/wait_for_db.sh | 2 +- docs/configuration/index.rst | 6 ++ docs/installation/config.rst | 7 -- docs/installation/index.rst | 2 + docs/installation/setup_configuration.rst | 11 +-- pyright.pyproject.toml | 2 - .../.gitkeep | 0 src/openforms/conf/base.py | 14 ++-- .../objects_api/setup_configuration/steps.py | 13 +-- .../contrib/objects_api/tests/factories.py | 4 +- .../tests/setup_configuration/__init__.py | 0 .../files/setup_config_objects_api.yaml | 0 .../setup_config_objects_api_all_fields.yaml | 0 ...up_config_objects_api_required_fields.yaml | 0 .../test_objects_api_config.py | 83 ++++++------------- .../objects_api/tests/test_migrations.py | 2 +- 19 files changed, 63 insertions(+), 91 deletions(-) rename src/openforms/contrib/objects_api/setup_configuration/tests/__init__.py => setup_configuration/.gitkeep (100%) create mode 100644 src/openforms/contrib/objects_api/tests/setup_configuration/__init__.py rename src/openforms/contrib/objects_api/{setup_configuration/tests => tests/setup_configuration}/files/setup_config_objects_api.yaml (100%) rename src/openforms/contrib/objects_api/{setup_configuration/tests => tests/setup_configuration}/files/setup_config_objects_api_all_fields.yaml (100%) rename src/openforms/contrib/objects_api/{setup_configuration/tests => tests/setup_configuration}/files/setup_config_objects_api_required_fields.yaml (100%) rename src/openforms/contrib/objects_api/{setup_configuration/tests => tests/setup_configuration}/test_objects_api_config.py (68%) diff --git a/.gitignore b/.gitignore index dcde1e6474..e57d71233d 100644 --- a/.gitignore +++ b/.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 diff --git a/bin/docker_start.sh b/bin/docker_start.sh index 577712ea0f..a88c40396f 100755 --- a/bin/docker_start.sh +++ b/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" diff --git a/bin/setup_configuration.sh b/bin/setup_configuration.sh index 4e56378e07..f3a6c89c71 100755 --- a/bin/setup_configuration.sh +++ b/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 diff --git a/bin/wait_for_db.sh b/bin/wait_for_db.sh index 89e15e6a1b..fdd9c24b61 100755 --- a/bin/wait_for_db.sh +++ b/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/ diff --git a/docs/configuration/index.rst b/docs/configuration/index.rst index 65d28c67f1..21c88b05a9 100644 --- a/docs/configuration/index.rst +++ b/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 diff --git a/docs/installation/config.rst b/docs/installation/config.rst index f9d8668b4c..cff983805f 100644 --- a/docs/installation/config.rst +++ b/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 ===================================== diff --git a/docs/installation/index.rst b/docs/installation/index.rst index 4141612d85..f66f149907 100644 --- a/docs/installation/index.rst +++ b/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 diff --git a/docs/installation/setup_configuration.rst b/docs/installation/setup_configuration.rst index f2d1cd1a62..249c942aa2 100644 --- a/docs/installation/setup_configuration.rst +++ b/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. diff --git a/pyright.pyproject.toml b/pyright.pyproject.toml index ffb3adea3f..55892ac49c 100644 --- a/pyright.pyproject.toml +++ b/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 = [] diff --git a/src/openforms/contrib/objects_api/setup_configuration/tests/__init__.py b/setup_configuration/.gitkeep similarity index 100% rename from src/openforms/contrib/objects_api/setup_configuration/tests/__init__.py rename to setup_configuration/.gitkeep diff --git a/src/openforms/conf/base.py b/src/openforms/conf/base.py index 8f16115542..3b35decec6 100644 --- a/src/openforms/conf/base.py +++ b/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", -] diff --git a/src/openforms/contrib/objects_api/setup_configuration/steps.py b/src/openforms/contrib/objects_api/setup_configuration/steps.py index 188554dab7..34e6baa667 100644 --- a/src/openforms/contrib/objects_api/setup_configuration/steps.py +++ b/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, ) diff --git a/src/openforms/contrib/objects_api/tests/factories.py b/src/openforms/contrib/objects_api/tests/factories.py index aeb3b5caf1..f762875233 100644 --- a/src/openforms/contrib/objects_api/tests/factories.py +++ b/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 ) diff --git a/src/openforms/contrib/objects_api/tests/setup_configuration/__init__.py b/src/openforms/contrib/objects_api/tests/setup_configuration/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/openforms/contrib/objects_api/setup_configuration/tests/files/setup_config_objects_api.yaml b/src/openforms/contrib/objects_api/tests/setup_configuration/files/setup_config_objects_api.yaml similarity index 100% rename from src/openforms/contrib/objects_api/setup_configuration/tests/files/setup_config_objects_api.yaml rename to src/openforms/contrib/objects_api/tests/setup_configuration/files/setup_config_objects_api.yaml diff --git a/src/openforms/contrib/objects_api/setup_configuration/tests/files/setup_config_objects_api_all_fields.yaml b/src/openforms/contrib/objects_api/tests/setup_configuration/files/setup_config_objects_api_all_fields.yaml similarity index 100% rename from src/openforms/contrib/objects_api/setup_configuration/tests/files/setup_config_objects_api_all_fields.yaml rename to src/openforms/contrib/objects_api/tests/setup_configuration/files/setup_config_objects_api_all_fields.yaml diff --git a/src/openforms/contrib/objects_api/setup_configuration/tests/files/setup_config_objects_api_required_fields.yaml b/src/openforms/contrib/objects_api/tests/setup_configuration/files/setup_config_objects_api_required_fields.yaml similarity index 100% rename from src/openforms/contrib/objects_api/setup_configuration/tests/files/setup_config_objects_api_required_fields.yaml rename to src/openforms/contrib/objects_api/tests/setup_configuration/files/setup_config_objects_api_required_fields.yaml diff --git a/src/openforms/contrib/objects_api/setup_configuration/tests/test_objects_api_config.py b/src/openforms/contrib/objects_api/tests/setup_configuration/test_objects_api_config.py similarity index 68% rename from src/openforms/contrib/objects_api/setup_configuration/tests/test_objects_api_config.py rename to src/openforms/contrib/objects_api/tests/setup_configuration/test_objects_api_config.py index f6c249e043..34a43c1ddb 100644 --- a/src/openforms/contrib/objects_api/setup_configuration/tests/test_objects_api_config.py +++ b/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() diff --git a/src/openforms/contrib/objects_api/tests/test_migrations.py b/src/openforms/contrib/objects_api/tests/test_migrations.py index e986b1b8fb..757ca14ba6 100644 --- a/src/openforms/contrib/objects_api/tests/test_migrations.py +++ b/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)