Skip to content

Commit

Permalink
Merge pull request #3517 from open-formulieren/backport/3279-suppress…
Browse files Browse the repository at this point in the history
…-requests-errors

Backport: suppress requests errors
  • Loading branch information
sergei-maertens authored Oct 2, 2023
2 parents 2d9252b + adf44b3 commit 9d82735
Show file tree
Hide file tree
Showing 12 changed files with 405 additions and 0 deletions.
7 changes: 7 additions & 0 deletions docs/developers/backend/core/utils.rst
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ Decorators
:members:


Admin decorators
----------------

.. automodule:: openforms.admin.decorators
:members:


URL handling and manipulation
-----------------------------

Expand Down
70 changes: 70 additions & 0 deletions src/openforms/admin/decorators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
from functools import wraps

from django.contrib import admin
from django.db import models
from django.utils.html import format_html
from django.utils.translation import gettext_lazy as _

import requests


def suppress_requests_errors(model: type[models.Model], fields: list[str]):
"""
Add robustness to admin fields which make outgoing requests that may fail.
If a :class:`requests.RequestException` is caught, return the base (non-enhanced)
form field that Django would generate by default and append an error message
for the end-user.
:arg fields: A list of model field names for which to suppress errors.
"""

def fallback(self, db_field, request, *args, **kwargs):
kwargs = {**self.formfield_overrides[db_field.__class__], **kwargs}

class CustomWidget(kwargs["widget"]):
def render(self, *args, **kwargs):
output = super().render(*args, **kwargs)

error_message = _(
"Could not load data - enable and check the request logs for more details."
)

html = format_html(
"""
<div class="openforms-error-widget">
<div class="openforms-error-widget__column">
{}
<small class="openforms-error-widget openforms-error-widget--error-text">{}</small>
</div>
</div>""",
output,
error_message,
)

return html

kwargs["widget"] = CustomWidget
return db_field.formfield(**kwargs)

def _decorator(admin_class: type[admin.ModelAdmin]):
original_func = admin_class.formfield_for_dbfield
for field in fields:
assert model._meta.get_field(field)

@wraps(original_func)
def _wrapped(self, db_field, request, *args, **kwargs):
field_name = db_field.name

try:
return original_func(self, db_field, request, *args, **kwargs)
except requests.RequestException as exc:
if field_name not in fields:
raise exc
return fallback(self, db_field, request, *args, **kwargs)

admin_class.formfield_for_dbfield = _wrapped

return admin_class

return _decorator
3 changes: 3 additions & 0 deletions src/openforms/appointments/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

from solo.admin import SingletonModelAdmin

from openforms.admin.decorators import suppress_requests_errors

from .base import BasePlugin
from .constants import AppointmentDetailsStatus
from .fields import AppointmentBackendChoiceField
Expand All @@ -26,6 +28,7 @@ def formfield_for_dbfield(self, db_field, request, **kwargs):
return super().formfield_for_dbfield(db_field, request, **kwargs) # type: ignore


@suppress_requests_errors(AppointmentsConfig, fields=["limit_to_location"])
@admin.register(AppointmentsConfig)
class AppointmentsConfigAdmin(PluginFieldMixin, SingletonModelAdmin):
def formfield_for_dbfield(self, db_field, request, **kwargs):
Expand Down
117 changes: 117 additions & 0 deletions src/openforms/appointments/contrib/jcc/tests/test_admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
from unittest.mock import patch

from django.urls import reverse
from django.utils.translation import gettext as _

import requests
import requests_mock
from django_webtest import WebTest

from openforms.accounts.tests.factories import SuperUserFactory
from openforms.appointments.contrib.jcc.models import JccConfig
from openforms.appointments.models import AppointmentsConfig
from openforms.utils.tests.cache import clear_caches
from openforms.utils.tests.logging import disable_logging
from soap.tests.factories import SoapServiceFactory

from .utils import WSDL


@disable_logging()
class ApointmentConfigAdminTests(WebTest):
def setUp(self):
super().setUp()

self.addCleanup(clear_caches)

@requests_mock.Mocker()
def test_admin_while_service_is_down(self, m):
m.register_uri(
requests_mock.ANY, requests_mock.ANY, exc=requests.RequestException
)

appointments_config = AppointmentsConfig.get_solo()
appointments_config.plugin = "jcc"
appointments_config.limit_to_location = "test"
appointments_config.save()

config = JccConfig.get_solo()
config.service = SoapServiceFactory.create(url=str(WSDL))
config.save()

superuser = SuperUserFactory.create()
response = self.app.get(
reverse(
"admin:appointments_appointmentsconfig_change",
args=(AppointmentsConfig.singleton_instance_id,),
),
user=superuser,
)
self.assertEqual(response.status_code, 200)

form = response.forms["appointmentsconfig_form"]

self.assertEqual(form["plugin"].value, "jcc")
self.assertEqual(form["limit_to_location"].value, "test")

error_node = response.pyquery(
".field-limit_to_location .openforms-error-widget .openforms-error-widget__column small"
)
self.assertNotIn(
_(
"Could not load data - enable and check the request logs for more details."
),
error_node.text(),
)

form.submit()

self.assertEqual(form["limit_to_location"].value, "test")

@requests_mock.Mocker()
@patch(
"openforms.appointments.contrib.jcc.plugin.get_client",
side_effect=requests.RequestException,
)
def test_admin_while_wsdl_is_down(self, m, mock_client):
m.register_uri(
requests_mock.ANY, requests_mock.ANY, exc=requests.RequestException
)

appointments_config = AppointmentsConfig.get_solo()
appointments_config.plugin = "jcc"
appointments_config.limit_to_location = "test"
appointments_config.save()

config = JccConfig.get_solo()
config.service = SoapServiceFactory.create(url=str(WSDL))
config.save()

superuser = SuperUserFactory.create()
response = self.app.get(
reverse(
"admin:appointments_appointmentsconfig_change",
args=(AppointmentsConfig.singleton_instance_id,),
),
user=superuser,
)
self.assertEqual(response.status_code, 200)

form = response.forms["appointmentsconfig_form"]

self.assertEqual(form["plugin"].value, "jcc")
self.assertEqual(form["limit_to_location"].value, "test")

error_node = response.pyquery(
".field-limit_to_location .openforms-error-widget .openforms-error-widget__column small"
)
self.assertIn(
_(
"Could not load data - enable and check the request logs for more details."
),
error_node.text(),
)

form.submit()

self.assertEqual(form["limit_to_location"].value, "test")
57 changes: 57 additions & 0 deletions src/openforms/appointments/contrib/qmatic/tests/test_admin.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
from django.urls import reverse
from django.utils.translation import gettext as _

import requests
import requests_mock
from django_webtest import WebTest

from openforms.accounts.tests.factories import SuperUserFactory
from openforms.appointments.contrib.qmatic.models import QmaticConfig
from openforms.appointments.models import AppointmentsConfig
from openforms.utils.tests.cache import clear_caches
from openforms.utils.tests.logging import disable_logging

from ..constants import CustomerFields
from .factories import ServiceFactory


class QmaticConfigAdminTests(WebTest):
Expand All @@ -26,3 +33,53 @@ def test_customer_fields_are_listed(self):
for value in checked_field_values:
with self.subTest(f"{value=}"):
self.assertIn(value, CustomerFields)


@disable_logging()
class ApointmentConfigAdminTests(WebTest):
def setUp(self):
super().setUp()

self.addCleanup(clear_caches)

@requests_mock.Mocker()
def test_admin_while_service_is_down(self, m):
appointments_config = AppointmentsConfig.get_solo()
appointments_config.plugin = "qmatic"
appointments_config.limit_to_location = "test"
appointments_config.save()

config = QmaticConfig.get_solo()
config.service = ServiceFactory.create()
config.save()

m.get(requests_mock.ANY, exc=requests.RequestException)

superuser = SuperUserFactory.create()
response = self.app.get(
reverse(
"admin:appointments_appointmentsconfig_change",
args=(AppointmentsConfig.singleton_instance_id,),
),
user=superuser,
)
self.assertEqual(response.status_code, 200)

form = response.forms["appointmentsconfig_form"]

self.assertEqual(form["plugin"].value, "qmatic")
self.assertEqual(form["limit_to_location"].value, "test")

error_node = response.pyquery(
".field-limit_to_location .openforms-error-widget .openforms-error-widget__column small"
)
self.assertNotIn(
_(
"Could not load data - enable and check the request logs for more details."
),
error_node.text(),
)

form.submit()

self.assertEqual(form["limit_to_location"].value, "test")
3 changes: 3 additions & 0 deletions src/openforms/registrations/contrib/zgw_apis/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from solo.admin import SingletonModelAdmin
from zgw_consumers.admin import ListZaaktypenMixin

from openforms.admin.decorators import suppress_requests_errors

from .models import ZGWApiGroupConfig, ZgwConfig


Expand All @@ -11,6 +13,7 @@ class ZgwConfigAdmin(SingletonModelAdmin):
pass


@suppress_requests_errors(ZGWApiGroupConfig, fields=["zaaktype"])
@admin.register(ZGWApiGroupConfig)
class ZGWApiGroupConfigAdmin(ListZaaktypenMixin, admin.ModelAdmin):
zaaktype_fields = [
Expand Down
63 changes: 63 additions & 0 deletions src/openforms/registrations/contrib/zgw_apis/tests/test_admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
from django.urls import reverse
from django.utils.translation import gettext as _

import requests
import requests_mock
from django_webtest import WebTest
from zgw_consumers.api_models.constants import VertrouwelijkheidsAanduidingen

from openforms.accounts.tests.factories import SuperUserFactory
from openforms.tests.utils import disable_2fa

from .factories import ZGWApiGroupConfigFactory


@disable_2fa
class ZGWApiGroupConfigAdminTests(WebTest):
@requests_mock.Mocker()
def test_admin_while_services_are_down(self, m):
m.register_uri(
requests_mock.ANY, requests_mock.ANY, exc=requests.ConnectTimeout
)
zgw_group = ZGWApiGroupConfigFactory.create(
zrc_service__api_root="https://zaken-1.nl/api/v1/",
zrc_service__oas="https://zaken-1.nl/api/v1/schema/openapi.yaml",
drc_service__api_root="https://documenten-1.nl/api/v1/",
drc_service__oas="https://documenten-1.nl/api/v1/schema/openapi.yaml",
ztc_service__api_root="https://catalogus-1.nl/api/v1/",
ztc_service__oas="https://catalogus-1.nl/api/v1/schema/openapi.yaml",
zaaktype="https://catalogi-1.nl/api/v1/zaaktypen/1",
informatieobjecttype="https://catalogi-1.nl/api/v1/informatieobjecttypen/1",
organisatie_rsin="000000000",
zaak_vertrouwelijkheidaanduiding=VertrouwelijkheidsAanduidingen.openbaar,
)
superuser = SuperUserFactory.create()

response = self.app.get(
reverse("admin:zgw_apis_zgwapigroupconfig_change", args=(zgw_group.pk,)),
user=superuser,
)

self.assertEqual(response.status_code, 200)
form = response.forms["zgwapigroupconfig_form"]
self.assertEqual(
form["zaaktype"].value, "https://catalogi-1.nl/api/v1/zaaktypen/1"
)
error_node = response.pyquery(
".field-zaaktype .openforms-error-widget .openforms-error-widget__column small"
)
self.assertEqual(
error_node.text(),
_(
"Could not load data - enable and check the request "
"logs for more details."
),
)

with self.subTest("submitting form doesn't clear configuration"):
form.submit()

zgw_group.refresh_from_db()
self.assertEqual(
zgw_group.zaaktype, "https://catalogi-1.nl/api/v1/zaaktypen/1"
)
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
class ZGWRegistrationMultipleZGWAPIsTests(TestCase):
@classmethod
def setUpTestData(cls):
super().setUpTestData()
cls.zgw_group1 = ZGWApiGroupConfigFactory.create(
zrc_service__api_root="https://zaken-1.nl/api/v1/",
zrc_service__oas="https://zaken-1.nl/api/v1/schema/openapi.yaml",
Expand Down
1 change: 1 addition & 0 deletions src/openforms/scss/admin/admin_overrides.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@import 'admin_theme';
@import 'app_overrides';
@import 'hijack';
@import 'widgets';
Loading

0 comments on commit 9d82735

Please sign in to comment.