Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ [open-zaak/open-notificaties#156] Support nested fields on kanaal kenmerken #14

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions notifications_api_common/kanalen.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,35 @@
"""

from collections import defaultdict
from typing import Dict, Tuple
from typing import Dict, Tuple, Union

from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured
from django.db.models import Model
from django.db.models.base import ModelBase
from django.db.models import Field, Model

from rest_framework.request import Request

KANAAL_REGISTRY = set()


class Kanaal:
def __init__(self, label: str, main_resource: ModelBase, kenmerken: Tuple = None):
def __init__(
self,
label: str,
main_resource: Model,
kenmerken: Union[Tuple, None] = None,
extra_kwargs: Union[Dict, None] = None,
):
self.label = label
self.main_resource = main_resource
self.extra_kwargs = extra_kwargs or {}

self.usage = defaultdict(list) # filled in by metaclass of notifications

# check that we're refering to existing fields
self.kenmerken = kenmerken or ()
for kenmerk in self.kenmerken:
try:
self.main_resource._meta.get_field(kenmerk)
self.get_field(self.main_resource, kenmerk)
except FieldDoesNotExist as exc:
raise ImproperlyConfigured(
f"Kenmerk '{kenmerk}' does not exist on the model {main_resource}"
Expand All @@ -39,7 +47,28 @@ def __repr__(self):
self.main_resource,
)

def get_kenmerken(self, obj: Model, data: Dict = None) -> Dict:
@staticmethod
def get_field(model: Model, field: str) -> Field:
"""
Function to retrieve a field from a Model
"""
return model._meta.get_field(field)

def get_help_text(self, field: Field, kenmerk: str) -> str:
"""
Retrieve the help_text for a kenmerk, pulled from the model field by default,
but can be overridden by setting extra_kwargs on `Kanaal.__init__`
"""
if help_text := self.extra_kwargs.get(kenmerk, {}).get("help_text"):
return help_text
return field.help_text

def get_kenmerken(
self,
obj: Model,
data: Union[Dict, None] = None,
request: Union[Request, None] = None, # noqa
) -> Dict:
data = data or {}
return {
kenmerk: data.get(kenmerk, getattr(obj, kenmerk))
Expand All @@ -55,7 +84,9 @@ def description(self):
kenmerken = [
kenmerk_template.format(
kenmerk=kenmerk,
help_text=self.main_resource._meta.get_field(kenmerk).help_text,
help_text=self.get_help_text(
self.get_field(self.main_resource, kenmerk), kenmerk
),
)
for kenmerk in self.kenmerken
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def create_kanaal(kanaal: str) -> None:
# look up the exchange in the registry
_kanaal = next(k for k in KANAAL_REGISTRY if k.label == kanaal)

kanalen = client.get("kanaal", params={"naam": kanaal})
kanalen = client.get("kanaal", params={"naam": kanaal}).json()
if kanalen:
raise KanaalExists()

Expand Down
4 changes: 3 additions & 1 deletion notifications_api_common/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,9 @@ def construct_message(self, data: dict, instance: models.Model = None) -> dict:
"actie": self.action,
"aanmaakdatum": timezone.now(),
# each channel knows which kenmerken it has, so delegate this
"kenmerken": kanaal.get_kenmerken(main_object, main_object_data),
"kenmerken": kanaal.get_kenmerken(
main_object, main_object_data, request=self.request
),
}

# let the serializer & render machinery shape the data the way it
Expand Down
3 changes: 3 additions & 0 deletions testapp/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

DEBUG = True

SITE_ID = 1

USE_TZ = False

BASE_DIR = os.path.abspath(os.path.dirname(__file__))
Expand All @@ -28,6 +30,7 @@
"django.contrib.admin",
"django.contrib.messages",
"django.contrib.staticfiles",
"django.contrib.sites",
"solo",
"simple_certmanager",
"zgw_consumers",
Expand Down
16 changes: 15 additions & 1 deletion testapp/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,19 @@
from django.urls import include, path

from .api import router
from .views import KanalenView

urlpatterns = [path("admin/", admin.site.urls), path("api/", include(router.urls))]
urlpatterns = [
path("admin/", admin.site.urls),
path("api/", include(router.urls)),
path(
"notificaties/",
include(
(
[path("kanalen/", KanalenView.as_view(), name="kanalen")],
"notifications",
),
namespace="notifications",
),
),
]
5 changes: 5 additions & 0 deletions testapp/views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from django.views import View


class KanalenView(View):
pass
2 changes: 2 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from notifications_api_common.models import NotificationsConfig
from testapp import urls # noqa

NOTIFICATIONS_API_ROOT = "http://some-api-root/api/v1/"


def dummy_get_response(request):
raise NotImplementedError()
Expand Down
50 changes: 50 additions & 0 deletions tests/test_register_kanalen.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
from django.core.management import call_command
from django.utils.translation import gettext as _

import pytest
import requests_mock

from .conftest import NOTIFICATIONS_API_ROOT


@pytest.mark.django_db
def test_register_kanalen(notifications_config):
with requests_mock.Mocker() as m:
m.get(f"{NOTIFICATIONS_API_ROOT}kanaal?naam=personen", json=[])
m.post(f"{NOTIFICATIONS_API_ROOT}kanaal")
call_command("register_kanalen")

assert len(m.request_history) == 2

get_request = m.request_history[0]
assert get_request.url == f"{NOTIFICATIONS_API_ROOT}kanaal?naam=personen"

post_request = m.request_history[1]
assert post_request.url == f"{NOTIFICATIONS_API_ROOT}kanaal"
assert post_request.json() == {
"naam": "personen",
"documentatieLink": "https://example.com/notificaties/kanalen/#personen",
"filters": ["address_street"],
}


@pytest.mark.django_db
def test_register_kanalen_already_exists(notifications_config):
with requests_mock.Mocker() as m:
m.get(
f"{NOTIFICATIONS_API_ROOT}kanaal?naam=personen",
json=[
{
"naam": "personen",
"documentatieLink": "https://example.com/notificaties/kanalen/#personen",
"filters": ["address_street"],
}
],
)
m.post(f"{NOTIFICATIONS_API_ROOT}kanaal")
call_command("register_kanalen")

assert len(m.request_history) == 1

get_request = m.request_history[0]
assert get_request.url == f"{NOTIFICATIONS_API_ROOT}kanaal?naam=personen"
29 changes: 11 additions & 18 deletions tests/test_register_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,30 @@
from django.utils.translation import gettext as _

import pytest
from requests import Response
import requests_mock
from requests.exceptions import HTTPError, RequestException

from notifications_api_common.admin import register_webhook
from notifications_api_common.models import Subscription

from .conftest import NOTIFICATIONS_API_ROOT

class MockResponse:
def __init__(self, data):
self.data = data

def json(self):
return self.data


@patch(
"ape_pie.APIClient.post",
return_value=MockResponse({"url": "https://example.com/api/v1/abonnementen/1"}),
)
@pytest.mark.django_db
def test_register_webhook_success(
request_with_middleware, notifications_config, *mocks
):
def test_register_webhook_success(request_with_middleware, notifications_config):
subscription = Subscription.objects.create(
callback_url="https://example.com/callback",
client_id="client_id",
secret="secret",
channels=["zaken"],
)

register_webhook(object, request_with_middleware, Subscription.objects.all())
with requests_mock.Mocker() as m:
m.post(
f"{NOTIFICATIONS_API_ROOT}abonnement",
json={"url": "https://example.com/api/v1/abonnementen/1"},
)
register_webhook(object, request_with_middleware, Subscription.objects.all())

messages = list(get_messages(request_with_middleware))

Expand All @@ -55,7 +48,7 @@ def test_register_webhook_request_exception(
channels=["zaken"],
)

with patch("ape_pie.APIClient.post", side_effect=RequestException("exception")):
with patch("requests.Session.post", side_effect=RequestException("exception")):
register_webhook(object, request_with_middleware, Subscription.objects.all())

messages = list(get_messages(request_with_middleware))
Expand All @@ -75,7 +68,7 @@ def test_register_webhook_http_error(request_with_middleware, notifications_conf
channels=["zaken"],
)

with patch("ape_pie.APIClient.post", side_effect=HTTPError("400")):
with patch("requests.Session.post", side_effect=HTTPError("400")):
register_webhook(object, request_with_middleware, Subscription.objects.all())

messages = list(get_messages(request_with_middleware))
Expand Down
3 changes: 2 additions & 1 deletion tests/test_send_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
from notifications_api_common.tasks import NotificationException, send_notification
from testapp.models import Person

NOTIFICATIONS_API_ROOT = "http://some-api-root/api/v1/"
from .conftest import NOTIFICATIONS_API_ROOT

TESTS_DIR = Path(__file__).parent


Expand Down
Loading