Munge stuff into a consistent event data type

We store our audit history in two ways:

  1. A list of versions of a service
  2. A list of events to do with API keys

In the future there could be auditing data which we want to display that
is stored in other formats (for example the event table).

This commit adds some objects which wrap around the different types of
auditing data, and expose a consistent interface to them. This
architecture will let us:
- write clean code in the presentation layer to display these events on
  a page
- add more types of events in the future by subclassing the `Event` data
  type, without having to rewrite anything in the presentation layer

app/__init__.py

@@ -3,7 +3,6 @@ import os
 import urllib
 from datetime import datetime, timedelta, timezone
 from functools import partial
-from numbers import Number
 from time import monotonic
 
 import ago
@@ -84,7 +83,7 @@ from app.notify_client.template_statistics_api_client import (
     template_statistics_client,
 )
 from app.notify_client.user_api_client import user_api_client
-from app.utils import get_logo_cdn_domain, id_safe
+from app.utils import format_thousands, get_logo_cdn_domain, id_safe
 
 login_manager = LoginManager()
 csrf = CSRFProtect()
@@ -351,14 +350,6 @@ def format_delta(date):
     )
 
 
-def format_thousands(value):
-    if isinstance(value, Number):
-        return '{:,.0f}'.format(value)
-    if value is None:
-        return ''
-    return value
-
-
 def valid_phone_number(phone_number):
     try:
         validate_phone_number(phone_number)

app/main/views/history.py

@@ -1,6 +1,5 @@
 from flask import render_template
 
-from app import current_service
 from app.main import main
 from app.utils import user_has_permissions
 
@@ -8,10 +7,4 @@ from app.utils import user_has_permissions
 @main.route("/services/<service_id>/history")
 @user_has_permissions('manage_service')
 def history(service_id):
-
-    return render_template(
-        'views/temp-history.html',
-        services=current_service.history['service_history'],
-        api_keys=current_service.history['api_key_history'],
-        events=current_service.history['events']
-    )
+    return render_template('views/temp-history.html')

app/models/__init__.py

@@ -63,8 +63,8 @@ class ModelList(ABC, Sequence):
     def model():
         pass
 
-    def __init__(self):
-        self.items = self.client()
+    def __init__(self, *args):
+        self.items = self.client(*args)
 
     def __getitem__(self, index):
         return self.model(self.items[index])

app/models/event.py

@@ -0,0 +1,191 @@
+from abc import ABC, abstractmethod
+
+from notifications_utils.formatters import formatted_list
+
+from app.models import ModelList
+from app.notify_client.service_api_client import service_api_client
+from app.utils import format_thousands
+
+
+class Event(ABC):
+
+    def __init__(
+        self,
+        item,
+        key=None,
+        value_from=None,
+        value_to=None,
+    ):
+        self.item = item
+        self.time = item['updated_at'] or item['created_at']
+        self.user_id = item['created_by_id']
+        self.key = key
+        self.value_from = value_from
+        self.value_to = value_to
+
+    @abstractmethod
+    def __str__(self):
+        pass
+
+    @property
+    @abstractmethod
+    def relevant(self):
+        pass
+
+
+class ServiceCreationEvent(Event):
+
+    relevant = True
+
+    def __str__(self):
+        return 'Created this service and called it ‘{}’'.format(
+            self.item['name']
+        )
+
+
+class ServiceEvent(Event):
+
+    @property
+    def relevant(self):
+        return self.value_from != self.value_to and bool(self._formatter)
+
+    def __str__(self):
+        return self._formatter()
+
+    @property
+    def _formatter(self):
+        return getattr(self, 'format_{}'.format(self.key), None)
+
+    def format_restricted(self):
+        if self.value_to is False:
+            return 'Made this service live'
+        if self.value_to is True:
+            return 'Put this service back into trial mode'
+
+    def format_active(self):
+        if self.value_to is False:
+            return 'Deleted this service'
+        if self.value_to is True:
+            return 'Unsuspended this service'
+
+    def format_contact_link(self):
+        return 'Set the contact details for this service to ‘{}’'.format(
+            self.value_to
+        )
+
+    def format_email_branding(self):
+        return 'Updated this service’s email branding'
+
+    def format_inbound_api(self):
+        return 'Updated the callback for received text messages'
+
+    def format_letter_branding(self):
+        if self.value_to is None:
+            return 'Removed the logo from this service’s letters'
+        return 'Updated the logo on this service’s letters'
+
+    def format_letter_contact_block(self):
+        return 'Updated the default letter contact block for this service'
+
+    def format_message_limit(self):
+        return (
+            '{} this service’s daily message limit from {} to {}'
+        ).format(
+            'Reduced' if self.value_from > self.value_to else 'Increased',
+            format_thousands(self.value_from),
+            format_thousands(self.value_to),
+        )
+
+    def format_name(self):
+        return (
+            'Renamed this service from ‘{}’ to ‘{}’'
+        ).format(
+            self.value_from, self.value_to
+        )
+
+    def format_permissions(self):
+        added = list(sorted(set(self.value_to) - set(self.value_from)))
+        removed = list(sorted(set(self.value_from) - set(self.value_to)))
+        if removed and added:
+            return 'Removed {} from this service’s permissions, added {}'.format(
+                formatted_list(removed),
+                formatted_list(added),
+            )
+        if added:
+            return 'Added {} to this service’s permissions'.format(
+                formatted_list(added)
+            )
+        if removed:
+            return 'Removed {} from this service’s permissions'.format(
+                formatted_list(removed)
+            )
+
+    def format_prefix_sms(self):
+        if self.value_to is True:
+            return 'Set text messages to start with the name of this service'
+        else:
+            return 'Set text messages to not start with the name of this service'
+
+    def format_research_mode(self):
+        if self.value_to is True:
+            return 'Put this service into research mode'
+        else:
+            return 'Took this service out of research mode'
+
+    def format_service_callback_api(self):
+        return 'Updated the callback for delivery receipts'
+
+    def format_go_live_user(self):
+        return 'Requested for this service to go live'
+
+
+class APIKeyEvent(Event):
+
+    relevant = True
+
+    def __str__(self):
+        if self.item['updated_at']:
+            return (
+                'Revoked the ‘{}’ API key'
+            ).format(self.item['name'])
+        else:
+            return (
+                'Created an API key called ‘{}’'
+            ).format(self.item['name'])
+
+
+class APIKeyEvents(ModelList):
+
+    model = APIKeyEvent
+    client = service_api_client.get_service_api_key_history
+
+
+class ServiceEvents(ModelList):
+
+    client = service_api_client.get_service_service_history
+
+    @property
+    def model(self):
+        return lambda x: x
+
+    @staticmethod
+    def splat(events):
+        for index, item in enumerate(sorted(
+            events,
+            key=lambda event: event['updated_at'] or event['created_at']
+        )):
+            if index == 0:
+                yield ServiceCreationEvent(item)
+            else:
+                for key in sorted(item.keys()):
+                    yield ServiceEvent(
+                        item,
+                        key,
+                        events[index - 1][key],
+                        events[index][key],
+                    )
+
+    def __init__(self, service_id):
+        self.items = [
+            event for event in self.splat(self.client(service_id)) if event.relevant
+        ]

app/models/service.py

@@ -1,3 +1,5 @@
+from operator import attrgetter
+
 from flask import Markup, abort, current_app
 from notifications_utils.field import Field
 from notifications_utils.formatters import nl2br
@@ -5,6 +7,7 @@ from notifications_utils.take import Take
 from werkzeug.utils import cached_property
 
 from app.models import JSONModel
+from app.models.event import APIKeyEvents, ServiceEvents
 from app.models.organisation import Organisation
 from app.models.user import InvitedUsers, User, Users
 from app.notify_client.api_key_api_client import api_key_api_client
@@ -632,6 +635,9 @@ class Service(JSONModel):
             if test:
                 yield BASE + '_incomplete' + tag
 
-    @cached_property
+    @property
     def history(self):
-        return service_api_client.get_service_history(self.id)['data']
+        return sorted(
+            ServiceEvents(self.id) + APIKeyEvents(self.id),
+            key=attrgetter('time'),
+        )

app/notify_client/service_api_client.py

@@ -311,7 +311,13 @@ class ServiceAPIClient(NotifyAdminAPIClient):
 
     # Temp access of service history data. Includes service and api key history
     def get_service_history(self, service_id):
-        return self.get('/service/{0}/history'.format(service_id))
+        return self.get('/service/{0}/history'.format(service_id))['data']
+
+    def get_service_service_history(self, service_id):
+        return self.get_service_history(service_id)['service_history']
+
+    def get_service_api_key_history(self, service_id):
+        return self.get_service_history(service_id)['api_key_history']
 
     def get_monthly_notification_stats(self, service_id, year):
         return self.get(url='/service/{}/notifications/monthly?year={}'.format(service_id, year))

app/templates/views/temp-history.html

@@ -10,96 +10,12 @@ Service and API key history
 
   {{ page_header("Service and API key history") }}
 
-  <div class="grid-row">
-    {% call(item, row_number) list_table(
-        services,
-        caption="Service history",
-        field_headings=['ID','Name','Created at','Updated at','Active','Message limit','Restricted','Created by id']
-    )%}
-      {% call field() %}
-        {{item.id}}
-      {% endcall %}
-      {% call field() %}
-        {{item.name}}
-      {% endcall %}
-      {% call field() %}
-        {{item.created_at}}
-      {% endcall %}
-      {% call field() %}
-        {{item.updated_at}}
-      {% endcall %}
-      {% call field() %}
-        {{item.active}}
-      {% endcall %}
-      {% call field() %}
-        {{item.message_limit}}
-      {% endcall %}
-      {% call field() %}
-        {{item.restricted}}
-      {% endcall %}
-      {% call field() %}
-        {{item.created_by_id}}
-      {% endcall %}
-
-    {% endcall %}
-
-  </div>
-
-
-  <div class="grid-row">
-    {% call(item, row_number) list_table(
-        api_keys,
-        caption="API key history",
-        field_headings=['ID','Name','Service ID','Exiry date','Created at','Updated at','Created by id']
-    )%}
-      {% call field() %}
-        {{item.id}}
-      {% endcall %}
-      {% call field() %}
-        {{item.name}}
-      {% endcall %}
-      {% call field() %}
-        {{item.service_id}}
-      {% endcall %}
-      {% call field() %}
-        {{item.expiry_date}}
-      {% endcall %}
-      {% call field() %}
-        {{item.created_at}}
-      {% endcall %}
-      {% call field() %}
-        {{item.updated_at}}
-      {% endcall %}
-      {% call field() %}
-        {{item.created_by_id}}
-      {% endcall %}
-
-    {% endcall %}
-
-  </div
-
-  <div class="grid-row">
-
-     {% call(item, row_number) list_table(
-        events,
-        caption="Events",
-        field_headings=['ID','Event type','User ID','IP Address','Event data']
-    )%}
-      {% call field() %}
-        {{item.id}}
-      {% endcall %}
-      {% call field() %}
-        {{item.event_type}}
-      {% endcall %}
-      {% call field() %}
-        {{item.data.user_id}}
-      {% endcall %}
-      {% call field() %}
-        {{item.data.ip_address}}
-      {% endcall %}
-      {% call field() %}
-        {{item.data}}
-      {% endcall %}
-    {% endcall %}
-  </div>
+  <ul>
+    {% for event in current_service.history|reverse %}
+      <li>
+        {{ event.time|format_datetime_relative }} {{ event.user_id }}<br />
+        {{ event|string }}
+      </li>
+    {% endfor %}
+  </ul>
 {% endblock %}

app/utils.py

@@ -6,6 +6,7 @@ from datetime import datetime, time, timedelta, timezone
 from functools import wraps
 from io import BytesIO, StringIO
 from itertools import chain
+from numbers import Number
 from os import path
 from urllib.parse import urlparse
 
@@ -602,3 +603,11 @@ class PermanentRedirect(RequestRedirect):
     and Windows 8.1, so this class keeps the original status code of 301.
     """
     code = 301
+
+
+def format_thousands(value):
+    if isinstance(value, Number):
+        return '{:,.0f}'.format(value)
+    if value is None:
+        return ''
+    return value