Always format ChangeEvent descriptions the same way

changelog.d/+809.changed.md

@@ -0,0 +1,2 @@
+Change how the description of a change event is formatted so that it is always
+consistent (not to mention DRY).

src/argus/incident/models.py

@@ -623,17 +623,28 @@ def __str__(self):
 
 
 class ChangeEvent(Event):
+    DESCRIPTION_FORMAT = 'Change: "{attribute}": {old} → {new}'
+
     class Meta:
         proxy = True
 
     def save(self, *args, **kwargs):
         self.type = self.Type.INCIDENT_CHANGE
         super().save(*args, **kwargs)
 
+    @classmethod
+    def format_description(cls, attribute, old, new):
+        context = {
+            "attribute": attribute,
+            "old": old if old is not None else "",
+            "new": new if new is not None else "",
+        }
+        return cls.DESCRIPTION_FORMAT.format(**context)
+
     @classmethod
     def change_level(cls, incident, actor, new_level, timestamp=None):
         timestamp = timestamp if timestamp else timezone.now()
-        description = f"Change: level {incident.level} → {new_level}"
+        description = cls.format_description("level", incident.level, new_level)
         event = cls(incident=incident, actor=actor, timestamp=timestamp, description=description)
         event.save()
         return event

src/argus/incident/serializers.py

@@ -204,7 +204,8 @@ def post_change_events(self, instance: Incident, user: User, validated_data: dic
             if attr in validated_data and validated_data[attr] != getattr(instance, attr):
                 old_value = getattr(instance, attr)
                 new_value = validated_data[attr]
-                description = f"Change: {attr} {old_value} → {new_value}"
+                description = ChangeEvent.format_description(attr, old_value, new_value)
+
                 ChangeEvent.objects.create(
                     incident=instance, actor=user, timestamp=timezone.now(), description=description
                 )
@@ -230,7 +231,11 @@ def add_and_remove_tags(instance: Incident, user: User, tags_data: List[dict]):
 
         # Post change events
         if remove_tag_relations or add_tags:
-            description = f"Change: tags {[str(tag) for tag in existing_tags]} → {[str(tag) for tag in posted_tags]}"
+            description = ChangeEvent.format_description(
+                "tags",
+                [str(tag) for tag in existing_tags],
+                [str(tag) for tag in posted_tags],
+            )
             ChangeEvent.objects.create(incident=instance, actor=user, timestamp=timezone.now(), description=description)
 
         for tag_relation in remove_tag_relations:

src/argus/incident/views.py

@@ -334,7 +334,7 @@ def ticket_url(self, request, pk=None):
             old_url = incident.ticket_url
             new_url = serializer.data["ticket_url"]
             if old_url != new_url:
-                description = f"Change: ticket_url {old_url} → {new_url}"
+                description = ChangeEvent.format_description("ticket_url", old_url, new_url)
                 ChangeEvent.objects.create(
                     incident=incident, actor=request.user, timestamp=timezone.now(), description=description
                 )
@@ -419,7 +419,7 @@ def update(self, request, incident_pk=None):
             )
 
         if url:
-            description = f"Change: ticket_url → {url}"
+            description = ChangeEvent.format_description("ticket_url", "", url)
             ChangeEvent.objects.create(
                 incident=incident, actor=request.user, timestamp=timezone.now(), description=description
             )

tests/incident/test_change_event.py

@@ -3,7 +3,7 @@
 from rest_framework import status
 from rest_framework.test import APITestCase
 from argus.incident.factories import StatefulIncidentFactory
-from argus.incident.models import Event
+from argus.incident.models import Event, ChangeEvent
 from argus.util.testing import disconnect_signals, connect_signals
 
 from . import IncidentBasedAPITestCaseHelper
@@ -25,7 +25,7 @@ def test_change_event_is_created_on_level_changes(self):
         data = {
             "level": 2,
         }
-        description = "Change: level 1 → 2"
+        description = ChangeEvent.format_description("level", 1, 2)
         response = self.user1_rest_client.patch(
             path=f"/api/v2/incidents/{self.incident.pk}/",
             data=data,
@@ -39,7 +39,7 @@ def test_change_event_is_created_on_level_changes(self):
 
     def test_change_event_is_created_on_ticket_url_changes(self):
         new_ticket_url = "http://www.example.com/repository/issues/other-issue"
-        description = f"Change: ticket_url {self.url} → {new_ticket_url}"
+        description = ChangeEvent.format_description("ticket_url", self.url, new_ticket_url)
 
         response = self.user1_rest_client.patch(
             path=f"/api/v2/incidents/{self.incident.pk}/",
@@ -56,7 +56,7 @@ def test_change_event_is_created_on_ticket_url_changes(self):
 
     def test_change_event_is_created_on_ticket_url_changes_for_ticket_url_endpoint(self):
         new_ticket_url = "http://www.example.com/repository/issues/other-issue"
-        description = f"Change: ticket_url {self.url} → {new_ticket_url}"
+        description = ChangeEvent.format_description("ticket_url", self.url, new_ticket_url)
 
         response = self.user1_rest_client.put(
             path=f"/api/v2/incidents/{self.incident.pk}/ticket_url/",
@@ -88,13 +88,13 @@ def test_change_event_is_created_on_ticket_url_changes_for_automatic_ticket_url_
         self.assertTrue(change_events)
 
         new_ticket_url = response.data["ticket_url"]
-        description = f"Change: ticket_url → {new_ticket_url}"
+        description = ChangeEvent.format_description("ticket_url", "", new_ticket_url)
         self.assertIn(description, change_events_descriptions)
         self.assertEqual(change_events.get(description=description).actor, self.user1)
 
     def test_change_event_is_created_on_details_url_changes(self):
         new_details_url = "http://www.example.com/repository/issues/other-issue"
-        description = f"Change: details_url {self.url} → {new_details_url}"
+        description = ChangeEvent.format_description("details_url", self.url, new_details_url)
 
         response = self.user1_rest_client.patch(
             path=f"/api/v2/incidents/{self.incident.pk}/",
@@ -111,7 +111,7 @@ def test_change_event_is_created_on_details_url_changes(self):
 
     def test_change_event_is_created_on_tag_changes(self):
         new_tag = "a=b"
-        description = f"Change: tags [] → ['{new_tag}']"
+        description = ChangeEvent.format_description("tags", "[]", [new_tag])
 
         response = self.user1_rest_client.patch(
             path=f"/api/v2/incidents/{self.incident.pk}/",