Add Channel.opsgenie property

It replaces:

* Channel.opsgenie_key
* Channel.opsgenie_region

Also, add data migration to normalize Opsgenie channel values
to always be JSON maps.

hc/api/migrations/0100_opsgenie_values.py

@@ -0,0 +1,27 @@
+# Generated by Django 4.2.5 on 2023-09-11 13:06
+
+from __future__ import annotations
+
+import json
+
+from django.db import migrations
+
+
+def normalize_opsgenie_values(apps, schema_editor):
+    Channel = apps.get_model("api", "Channel")
+    for ch in Channel.objects.filter(kind="opsgenie").only("value"):
+        if ch.value.startswith("{"):
+            continue
+
+        ch.value = json.dumps({"key": ch.value, "region": "us"})
+        ch.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("api", "0099_alter_channel_disabled"),
+    ]
+
+    operations = [
+        migrations.RunPython(normalize_opsgenie_values, migrations.RunPython.noop)
+    ]

hc/api/models.py

@@ -741,6 +741,11 @@ class EmailConf(BaseModel):
         return super().model_validate_json(data)
 
 
+class OpsgenieConf(BaseModel):
+    key: str
+    region: str
+
+
 class Channel(models.Model):
     name = models.CharField(max_length=100, blank=True)
     code = models.UUIDField(default=uuid.uuid4, editable=False, unique=True)
@@ -995,20 +1000,8 @@ class Channel(models.Model):
         return EmailConf.load(self.value)
 
     @property
-    def opsgenie_key(self) -> str:
-        assert self.kind == "opsgenie"
-        if not self.value.startswith("{"):
-            return self.value
-
-        return self.json["key"]
-
-    @property
-    def opsgenie_region(self) -> str:
-        assert self.kind == "opsgenie"
-        if not self.value.startswith("{"):
-            return "us"
-
-        return self.json["region"]
+    def opsgenie(self) -> OpsgenieConf:
+        return OpsgenieConf.model_validate_json(self.value)
 
     zulip_bot_email = json_property("zulip", "bot_email")
     zulip_api_key = json_property("zulip", "api_key")

hc/api/tests/test_channel_model.py

@@ -42,13 +42,8 @@ class ChannelModelTestCase(BaseTestCase):
             ),
         )
 
-    def test_it_handles_legacy_opsgenie_value(self) -> None:
-        c = Channel(kind="opsgenie", value="foo123")
-        self.assertEqual(c.opsgenie_key, "foo123")
-        self.assertEqual(c.opsgenie_region, "us")
-
     def test_it_handles_json_opsgenie_value(self) -> None:
         c = Channel(kind="opsgenie")
         c.value = json.dumps({"key": "abc", "region": "eu"})
-        self.assertEqual(c.opsgenie_key, "abc")
-        self.assertEqual(c.opsgenie_region, "eu")
+        self.assertEqual(c.opsgenie.key, "abc")
+        self.assertEqual(c.opsgenie.region, "eu")

hc/api/tests/test_notify_opsgenie.py

@@ -31,7 +31,7 @@ class NotifyOpsGenieTestCase(BaseTestCase):
 
     @patch("hc.api.transports.curl.request")
     def test_opsgenie_with_legacy_value(self, mock_post: Mock) -> None:
-        self._setup_data("123")
+        self._setup_data(json.dumps({"key": "123", "region": "us"}))
         mock_post.return_value.status_code = 202
 
         self.channel.notify(self.check)
@@ -46,7 +46,7 @@ class NotifyOpsGenieTestCase(BaseTestCase):
 
     @patch("hc.api.transports.curl.request")
     def test_opsgenie_up(self, mock_post: Mock) -> None:
-        self._setup_data("123", status="up")
+        self._setup_data(json.dumps({"key": "123", "region": "us"}), status="up")
         mock_post.return_value.status_code = 202
 
         self.channel.notify(self.check)
@@ -58,7 +58,7 @@ class NotifyOpsGenieTestCase(BaseTestCase):
         self.assertTrue(str(self.check.code) in url)
 
     @patch("hc.api.transports.curl.request")
-    def test_opsgenie_with_json_value(self, mock_post: Mock) -> None:
+    def test_opsgenie_with_eu_region(self, mock_post: Mock) -> None:
         self._setup_data(json.dumps({"key": "456", "region": "eu"}))
         mock_post.return_value.status_code = 202
 
@@ -72,7 +72,7 @@ class NotifyOpsGenieTestCase(BaseTestCase):
 
     @patch("hc.api.transports.curl.request")
     def test_opsgenie_returns_error(self, mock_post: Mock) -> None:
-        self._setup_data("123")
+        self._setup_data(json.dumps({"key": "123", "region": "us"}))
         mock_post.return_value.status_code = 403
         mock_post.return_value.content = b"""{"message": "Nice try"}"""
 
@@ -82,7 +82,7 @@ class NotifyOpsGenieTestCase(BaseTestCase):
 
     @patch("hc.api.transports.curl.request")
     def test_it_handles_non_json_error_response(self, mock_post: Mock) -> None:
-        self._setup_data("123")
+        self._setup_data(json.dumps({"key": "123", "region": "us"}))
         mock_post.return_value.status_code = 403
         mock_post.return_value.json = Mock(side_effect=ValueError)
 
@@ -92,7 +92,7 @@ class NotifyOpsGenieTestCase(BaseTestCase):
 
     @override_settings(OPSGENIE_ENABLED=False)
     def test_it_requires_opsgenie_enabled(self) -> None:
-        self._setup_data("123")
+        self._setup_data(json.dumps({"key": "123", "region": "us"}))
         self.channel.notify(self.check)
 
         n = Notification.objects.get()

hc/api/transports.py

@@ -466,7 +466,7 @@ class Opsgenie(HttpTransport):
 
         headers = {
             "Content-Type": "application/json",
-            "Authorization": "GenieKey %s" % self.channel.opsgenie_key,
+            "Authorization": "GenieKey %s" % self.channel.opsgenie.key,
         }
 
         payload: JSONDict = {"alias": str(check.code), "source": settings.SITE_NAME}
@@ -478,7 +478,7 @@ class Opsgenie(HttpTransport):
             payload["description"] = tmpl("opsgenie_description.html", check=check)
 
         url = "https://api.opsgenie.com/v2/alerts"
-        if self.channel.opsgenie_region == "eu":
+        if self.channel.opsgenie.region == "eu":
             url = "https://api.eu.opsgenie.com/v2/alerts"
 
         if check.status == "up":