Fix notification kanalen not being created

CHANGELOG.rst

@@ -2,6 +2,11 @@
 Changelog
 =========
 
+0.3.1 (2024-10-27)
+------------------
+
+* Fixed kanalen not being registered. This regression was introduced in `0.3.0`.
+
 0.3.0 (2024-10-24)
 ------------------
 

notifications_api_common/management/commands/register_kanalen.py

@@ -1,21 +1,54 @@
 import logging
+from typing import Optional
 
 from django.contrib.sites.models import Site
 from django.core.management.base import BaseCommand
 from django.urls import reverse
 
+from requests import Response
+from requests.exceptions import JSONDecodeError, RequestException
+from zgw_consumers.models import Service
+
 from ...kanalen import KANAAL_REGISTRY
 from ...models import NotificationsConfig
 from ...settings import get_setting
 
 logger = logging.getLogger(__name__)
 
 
-class KanaalExists(Exception):
-    pass
+class KanaalException(Exception):
+    kanaal: str
+    data: dict | list
+    service: Service
+
+    def __init__(
+        self, kanaal: str, service: Service, data: Optional[dict | list] = None
+    ):
+        super().__init__()
+
+        self.kanaal = kanaal
+        self.service = service
+        self.data = data or {}
+
+
+class KanaalRequestException(KanaalException):
+    def __str__(self) -> str:
+        return (
+            f"Unable to retrieve kanaal {self.kanaal} from {self.service}: {self.data}"
+        )
+
+
+class KanaalCreateException(KanaalException):
+    def __str__(self) -> str:
+        return f"Unable to create kanaal {self.kanaal} at {self.service}: {self.data}"
 
 
-def create_kanaal(kanaal: str) -> None:
+class KanaalExistsException(KanaalException):
+    def __str__(self) -> str:
+        return f"Kanaal '{self.kanaal}' already exists within {self.service}"
+
+
+def create_kanaal(kanaal: str, service: Service) -> None:
     """
     Create a kanaal, if it doesn't exist yet.
     """
@@ -26,9 +59,19 @@ 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})
+    response_data = []
+
+    try:
+        response: Response = client.get("kanaal", params={"naam": kanaal})
+        kanalen: list[dict] = response.json() or []
+        response.raise_for_status()
+    except (RequestException, JSONDecodeError) as exception:
+        raise KanaalRequestException(
+            kanaal=kanaal, service=service, data=response_data
+        ) from exception
+
     if kanalen:
-        raise KanaalExists()
+        raise KanaalExistsException(kanaal=kanaal, service=service, data=response_data)
 
     # build up own documentation URL
     domain = Site.objects.get_current().domain
@@ -37,14 +80,22 @@ def create_kanaal(kanaal: str) -> None:
         f"{protocol}://{domain}{reverse('notifications:kanalen')}#{kanaal}"
     )
 
-    client.post(
-        "kanaal",
-        json={
-            "naam": kanaal,
-            "documentatieLink": documentation_url,
-            "filters": list(_kanaal.kenmerken),
-        },
-    )
+    try:
+        response: Response = client.post(
+            "kanaal",
+            json={
+                "naam": kanaal,
+                "documentatieLink": documentation_url,
+                "filters": list(_kanaal.kenmerken),
+            },
+        )
+
+        response_data: dict = response.json() or {}
+        response.raise_for_status()
+    except (RequestException, JSONDecodeError) as exception:
+        raise KanaalCreateException(
+            kanaal=kanaal, service=service, data=response_data
+        ) from exception
 
 
 class Command(BaseCommand):
@@ -69,7 +120,7 @@ def handle(self, **options):
                 "`notifications_api_service` configured"
             )
 
-        api_root = config.notifications_api_service.api_root
+        service = config.notifications_api_service
 
         # use CLI arg or fall back to setting
         kanalen = options["kanalen"] or sorted(
@@ -78,7 +129,9 @@ def handle(self, **options):
 
         for kanaal in kanalen:
             try:
-                create_kanaal(kanaal)
-                self.stdout.write(f"Registered kanaal '{kanaal}' with {api_root}")
-            except KanaalExists:
-                self.stderr.write(f"Kanaal '{kanaal}' already exists within {api_root}")
+                create_kanaal(kanaal, service)
+                self.stdout.write(
+                    f"Registered kanaal '{kanaal}' with {service.api_root}"
+                )
+            except (KanaalException,) as exception:
+                self.stderr.write(f"{str(exception)} . Skipping..")

testapp/settings.py

@@ -25,6 +25,7 @@
     "django.contrib.contenttypes",
     "django.contrib.auth",
     "django.contrib.sessions",
+    "django.contrib.sites",
     "django.contrib.admin",
     "django.contrib.messages",
     "django.contrib.staticfiles",
@@ -64,3 +65,5 @@
 ]
 
 ROOT_URLCONF = "testapp.urls"
+
+SITE_ID = 1

tests/test_register_kanalen.py

@@ -0,0 +1,191 @@
+from io import StringIO
+from unittest.mock import Mock, patch
+from urllib.parse import urlencode
+
+from django.test.testcases import call_command
+
+import pytest
+
+from notifications_api_common.kanalen import KANAAL_REGISTRY, Kanaal
+
+kanalen = set(
+    (
+        Kanaal(label="foobar", main_resource=Mock()),
+        Kanaal(label="boofar", main_resource=Mock()),
+    )
+)
+
+KANAAL_REGISTRY.clear()
+KANAAL_REGISTRY.update(kanalen)
+
+
+@pytest.mark.django_db
+def test_register_kanalen_success(notifications_config, requests_mock):
+    kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal"
+    params = urlencode(dict(naam="foobar"))
+
+    requests_mock.get(f"{kanaal_url}?{params}", json=[])
+
+    requests_mock.post(
+        kanaal_url,
+        json={
+            "url": "http://example.com",
+            "naam": "string",
+            "documentatieLink": "http://example.com",
+            "filters": ["string"],
+        },
+        status_code=201,
+    )
+
+    reverse_patch = (
+        "notifications_api_common.management.commands.register_kanalen.reverse"
+    )
+
+    with patch(reverse_patch) as mocked_reverse:
+        mocked_reverse.return_value = "/notifications/kanalen"
+
+        call_command("register_kanalen", kanalen=["foobar"])
+
+    assert len(requests_mock.request_history) == 2
+
+    get_request = requests_mock.request_history[0]
+
+    assert get_request._request.url == f"{kanaal_url}?{params}"
+
+    post_request = requests_mock.request_history[1]
+
+    assert post_request._request.url == kanaal_url
+
+
+@pytest.mark.django_db
+def test_register_kanalen_from_registry_success(notifications_config, requests_mock):
+    kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal"
+
+    url_mapping = {
+        kanaal.label: f"{kanaal_url}?{urlencode(dict(naam=kanaal.label))}"
+        for kanaal in kanalen
+    }
+
+    for kanaal in kanalen:
+        requests_mock.get(url_mapping[kanaal.label], json=[])
+
+        requests_mock.post(
+            kanaal_url,
+            json={
+                "url": "http://example.com",
+                "naam": kanaal.label,
+                "documentatieLink": "http://example.com",
+                "filters": ["string"],
+            },
+            status_code=201,
+        )
+
+    reverse_patch = (
+        "notifications_api_common.management.commands.register_kanalen.reverse"
+    )
+
+    with patch(reverse_patch) as mocked_reverse:
+        mocked_reverse.return_value = "/notifications/kanalen"
+
+        call_command("register_kanalen")
+
+    assert len(requests_mock.request_history) == 4
+
+    # kanalen are sorted by label
+    first_get_request = requests_mock.request_history[0]
+    assert first_get_request._request.url == url_mapping["boofar"]
+
+    first_post_request = requests_mock.request_history[1]
+    assert first_post_request._request.url == kanaal_url
+
+    second_get_request = requests_mock.request_history[2]
+    assert second_get_request._request.url == url_mapping["foobar"]
+
+    second_post_request = requests_mock.request_history[3]
+    assert second_post_request._request.url == kanaal_url
+
+
+@pytest.mark.django_db
+def test_register_kanalen_existing_kanalen(notifications_config, requests_mock):
+    """
+    Test that already registered kanalen does not cause issues
+    """
+    kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal"
+    params = urlencode(dict(naam="foobar"))
+
+    requests_mock.get(
+        f"{kanaal_url}?{params}",
+        json=[
+            {
+                "url": "http://example.com",
+                "naam": "foobar",
+                "documentatieLink": "http://example.com",
+                "filters": ["string"],
+            }
+        ],
+    )
+
+    call_command("register_kanalen", kanalen=["foobar"])
+
+    assert len(requests_mock.request_history) == 1
+
+    request = requests_mock.request_history[0]
+
+    assert request._request.url == f"{kanaal_url}?{params}"
+
+
+@pytest.mark.django_db
+def test_register_kanalen_unknown_url(notifications_config, requests_mock):
+    kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal"
+    params = urlencode(dict(naam="foobar"))
+
+    requests_mock.get(f"{kanaal_url}?{params}", status_code=404)
+
+    stderr = StringIO()
+
+    call_command("register_kanalen", kanalen=["foobar"], stderr=stderr)
+
+    partial_failure_message = "Unable to retrieve kanaal foobar"
+
+    assert partial_failure_message in stderr.getvalue()
+
+    assert len(requests_mock.request_history) == 1
+
+    request = requests_mock.request_history[0]
+
+    assert request._request.url == f"{kanaal_url}?{params}"
+
+
+@pytest.mark.django_db
+def test_register_kanalen_incorrect_post(notifications_config, requests_mock):
+    kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal"
+    params = urlencode(dict(naam="foobar"))
+
+    requests_mock.get(f"{kanaal_url}?{params}", json=[])
+
+    requests_mock.post(kanaal_url, json={"error": "foo"}, status_code=400)
+
+    stderr = StringIO()
+
+    reverse_patch = (
+        "notifications_api_common.management.commands.register_kanalen.reverse"
+    )
+
+    with patch(reverse_patch) as mocked_reverse:
+        mocked_reverse.return_value = "/notifications/kanalen"
+
+        call_command("register_kanalen", kanalen=["foobar"], stderr=stderr)
+
+    partial_failure_message = "Unable to create kanaal foobar"
+
+    assert partial_failure_message in stderr.getvalue()
+
+    assert len(requests_mock.request_history) == 2
+
+    get_request = requests_mock.request_history[0]
+
+    assert get_request._request.url == f"{kanaal_url}?{params}"
+
+    post_request = requests_mock.request_history[1]
+
+    assert post_request._request.url == kanaal_url

tests/test_register_webhook.py

@@ -4,7 +4,6 @@
 from django.utils.translation import gettext as _
 
 import pytest
-from requests import Response
 from requests.exceptions import HTTPError, RequestException
 
 from notifications_api_common.admin import register_webhook