Skip to content

Commit

Permalink
🐛[#3423] Fix 500 error on import
Browse files Browse the repository at this point in the history
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
  • Loading branch information
CharString authored and sergei-maertens committed Sep 6, 2023
1 parent 8a944ff commit 035353a
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 6 deletions.
22 changes: 20 additions & 2 deletions src/openforms/forms/tests/test_copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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={
Expand All @@ -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(
Expand All @@ -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)
22 changes: 18 additions & 4 deletions src/openforms/forms/tests/test_import_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
],
)
Expand All @@ -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
Expand Down Expand Up @@ -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())
Expand Down
6 changes: 6 additions & 0 deletions src/openforms/forms/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions src/openforms/variables/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 035353a

Please sign in to comment.