From 035353a3f5d229a7940d339d087e3f7e3af1e109 Mon Sep 17 00:00:00 2001 From: Chris Wesseling Date: Mon, 4 Sep 2023 17:38:23 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B[#3423]=20Fix=20500=20error=20on=20?= =?UTF-8?q?import?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ignores service fetch config on import, so existing exports can be imported. The cross instance case is not trivial: Services and config ids' may point to different things. We'd need to define equality for ServiceFetchConfiguration and Service. And user interaction on mismatches. Also, Services can (and probably will) contain credentials or tokens. These should probably not end up in some zipfile on someone's proverbial USB-stick. See #2683 Backport-of: #3458 --- src/openforms/forms/tests/test_copy.py | 22 +++++++++++++++++-- .../forms/tests/test_import_export.py | 22 +++++++++++++++---- src/openforms/forms/utils.py | 6 +++++ src/openforms/variables/tests/factories.py | 1 + 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/openforms/forms/tests/test_copy.py b/src/openforms/forms/tests/test_copy.py index b3eb448324..96d119990c 100644 --- a/src/openforms/forms/tests/test_copy.py +++ b/src/openforms/forms/tests/test_copy.py @@ -12,11 +12,13 @@ from openforms.accounts.tests.factories import SuperUserFactory, TokenFactory from openforms.tests.utils import NOOP_CACHES from openforms.variables.constants import FormVariableSources +from openforms.variables.tests.factories import ServiceFetchConfigurationFactory from ..models import Form, FormDefinition, FormStep, FormVariable from .factories import ( FormDefinitionFactory, FormFactory, + FormLogicFactory, FormStepFactory, FormVariableFactory, ) @@ -201,7 +203,7 @@ def setUpTestData(cls): cls.token = TokenFactory(user=user) def test_copy_form_with_variables(self): - form = FormFactory.create(slug="test-copying-with-vars") + form: Form = FormFactory.create(slug="test-copying-with-vars") FormStepFactory.create( form=form, form_definition__configuration={ @@ -218,11 +220,26 @@ def test_copy_form_with_variables(self): }, ) FormVariableFactory.create( - form=form, source=FormVariableSources.user_defined, key="bla" + form=form, + source=FormVariableSources.user_defined, + key="bla", + service_fetch_configuration=ServiceFetchConfigurationFactory.create(), + ) + FormLogicFactory.create( + form=form, + json_logic_trigger={"!": {"var": "bla"}}, + is_advanced=True, + actions=[ + { + "action": {"type": "fetch-from-service", "value": ""}, + "variable": "bla", + } + ], ) self.assertEqual(1, Form.objects.count()) self.assertEqual(3, FormVariable.objects.filter(form=form).count()) + self.assertEqual(form.formlogic_set.count(), 1) url = reverse("api:form-copy", args=(form.uuid,)) response = self.client.post( @@ -245,3 +262,4 @@ def test_copy_form_with_variables(self): self.assertEqual( 1, variables_copy.filter(source=FormVariableSources.user_defined).count() ) + self.assertEqual(form_copy.formlogic_set.count(), 1) diff --git a/src/openforms/forms/tests/test_import_export.py b/src/openforms/forms/tests/test_import_export.py index f18d64b83d..60a034eb31 100644 --- a/src/openforms/forms/tests/test_import_export.py +++ b/src/openforms/forms/tests/test_import_export.py @@ -145,13 +145,21 @@ def test_import(self): # fetch configurations are not exported (yet) # but shouldn't break export - import fetch_config = ServiceFetchConfigurationFactory.create() + far_fetched = FormVariableFactory.create( + form=form, + user_defined=True, + name="far_fetched", + key="farFetched", + service_fetch_configuration=fetch_config, + ) FormLogicFactory.create( form=form, - json_logic_trigger={"!": {"var": "test-user-defined"}}, + json_logic_trigger={"!": {"var": "farFetched"}}, + is_advanced=True, actions=[ { - "action": {"type": "fetch-from-service", "value": fetch_config.pk}, - "variable": "test-user-defined", + "action": {"type": "fetch-from-service", "value": ""}, + "variable": "farFetched", } ], ) @@ -177,6 +185,7 @@ def test_import(self): call_command("export", form.pk, self.filepath) # attempt to break ForeignKey constraint + far_fetched.delete() fetch_config.delete() old_form_definition_slug = form_definition.slug @@ -231,7 +240,12 @@ def test_import(self): user_defined_vars = FormVariable.objects.filter( source=FormVariableSources.user_defined ) - self.assertEqual(2, user_defined_vars.count()) + + # assert 3 user_defined_vars + # 1. original test-user-defined + # 2. imported test-user-defined + # 3. imported far_fetched (but without service_fetch_configuration) + self.assertEqual(user_defined_vars.count(), 3) form_logics = FormLogic.objects.all() self.assertEqual(4, form_logics.count()) diff --git a/src/openforms/forms/utils.py b/src/openforms/forms/utils.py index 14e3fc2fbc..e46bfca83e 100644 --- a/src/openforms/forms/utils.py +++ b/src/openforms/forms/utils.py @@ -218,6 +218,12 @@ def import_form_data( }, } ) + if "service_fetch_configuration" in entry: + # The transferring between systems case is very tricky + # better not import these, we don't know where this came from. + # services and ids may point to different things + # in different OF instances. + del entry["service_fetch_configuration"] if resource == "formLogic": # by now, the form resource has been created (or it was an existing one) diff --git a/src/openforms/variables/tests/factories.py b/src/openforms/variables/tests/factories.py index d9ee612813..81a49db54f 100644 --- a/src/openforms/variables/tests/factories.py +++ b/src/openforms/variables/tests/factories.py @@ -7,6 +7,7 @@ class ServiceFetchConfigurationFactory(factory.django.DjangoModelFactory): service = factory.SubFactory(ServiceFactory) + name = factory.Sequence(lambda n: f"Service Fetch Config #{n}") class Meta: model = ServiceFetchConfiguration