Fix registering kanalen

CHANGELOG.rst

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

notifications_api_common/management/commands/register_kanalen.py

@@ -4,6 +4,9 @@
 from django.core.management.base import BaseCommand
 from django.urls import reverse
 
+from requests import Response
+from requests.exceptions import JSONDecodeError, RequestException
+
 from ...kanalen import KANAAL_REGISTRY
 from ...models import NotificationsConfig
 from ...settings import get_setting
@@ -15,6 +18,27 @@ class KanaalExists(Exception):
     pass
 
 
+class KanaalException(Exception):
+    kanaal: str
+    data: dict | list
+
+    def __init__(self, kanaal: str, data: dict | list = {}):
+        super().__init__()
+
+        self.kanaal = kanaal
+        self.data = data
+
+
+class KanaalRequestException(KanaalException):
+    def __str__(self) -> str:
+        return f"Unable to retrieve kanaal {self.kanaal}: {self.data}"
+
+
+class KanaalCreateException(KanaalException):
+    def __str__(self) -> str:
+        return f"Unable to create kanaal {self.kanaal}: {self.data}"
+
+
 def create_kanaal(kanaal: str) -> None:
     """
     Create a kanaal, if it doesn't exist yet.
@@ -26,7 +50,15 @@ 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, data=response_data) from exception
+
     if kanalen:
         raise KanaalExists()
 
@@ -37,14 +69,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),
-        },
-    )
+    response_data = {}
+
+    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, data=response_data) from exception
 
 
 class Command(BaseCommand):
@@ -82,3 +122,12 @@ def handle(self, **options):
                 self.stdout.write(f"Registered kanaal '{kanaal}' with {api_root}")
             except KanaalExists:
                 self.stderr.write(f"Kanaal '{kanaal}' already exists within {api_root}")
+            except KanaalRequestException:
+                self.stderr.write(
+                    f"Request to retrieve existing kanalen for {kanaal} failed. "
+                    "Skipping.."
+                )
+            except KanaalCreateException:
+                self.stderr.write(
+                    f"Request to create kanaal for {kanaal} failed. 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,193 @@
+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)
+
+    failure_message = (
+        "Request to retrieve existing kanalen for foobar failed. Skipping.."
+    )
+
+    assert 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)
+
+    failure_message = "Request to create kanaal for foobar failed. Skipping.."
+
+    assert 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