diff --git a/app/__init__.py b/app/__init__.py index e9f1bc698..2b98ab712 100644 --- a/app/__init__.py +++ b/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) diff --git a/app/main/views/history.py b/app/main/views/history.py index 428d3b966..217177ff5 100644 --- a/app/main/views/history.py +++ b/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//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') diff --git a/app/models/__init__.py b/app/models/__init__.py index 2574495e2..def264a98 100644 --- a/app/models/__init__.py +++ b/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]) diff --git a/app/models/event.py b/app/models/event.py new file mode 100644 index 000000000..43dfba627 --- /dev/null +++ b/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 + ] diff --git a/app/models/service.py b/app/models/service.py index 7b43c53fa..341ecf6a7 100644 --- a/app/models/service.py +++ b/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'), + ) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index f247597be..8c45fb1b3 100644 --- a/app/notify_client/service_api_client.py +++ b/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)) diff --git a/app/templates/views/temp-history.html b/app/templates/views/temp-history.html index d817e9cea..6ca2b7764 100644 --- a/app/templates/views/temp-history.html +++ b/app/templates/views/temp-history.html @@ -10,96 +10,12 @@ Service and API key history {{ page_header("Service and API key history") }} -
- {% 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 %} - -
- - -
- {% 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 %} - -
- - {% 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 %} - + {% endblock %} diff --git a/app/utils.py b/app/utils.py index 9f3d9483e..dffa23ebe 100644 --- a/app/utils.py +++ b/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 diff --git a/tests/app/main/views/test_history.py b/tests/app/main/views/test_history.py index 21461d960..17e8ba7a7 100644 --- a/tests/app/main/views/test_history.py +++ b/tests/app/main/views/test_history.py @@ -1,8 +1,22 @@ -from tests.conftest import SERVICE_ONE_ID +from tests.conftest import SERVICE_ONE_ID, normalize_spaces def test_history( client_request, mock_get_service_history, ): - client_request.get('main.history', service_id=SERVICE_ONE_ID) + page = client_request.get('main.history', service_id=SERVICE_ONE_ID) + + assert normalize_spaces( + page.select_one('main').text + ) == ( + 'Service and API key history ' + '11 November at 12:12pm 6ce466d0-fd6a-11e5-82f5-e0accb9d11a6' + ' Revoked the ‘Bad key’ API key ' + '11 November at 11:11am 6ce466d0-fd6a-11e5-82f5-e0accb9d11a6' + ' Created an API key called ‘Bad key’ ' + '10 October at 11:10am 6ce466d0-fd6a-11e5-82f5-e0accb9d11a6' + ' Created an API key called ‘Good key’ ' + '10 October at 11:10am 6ce466d0-fd6a-11e5-82f5-e0accb9d11a6' + ' Created this service and called it ‘Example service’' + ) diff --git a/tests/app/models/test_event.py b/tests/app/models/test_event.py new file mode 100644 index 000000000..3b48d88f1 --- /dev/null +++ b/tests/app/models/test_event.py @@ -0,0 +1,89 @@ +import pytest + +from app.models.event import ServiceEvent +from tests.conftest import sample_uuid + + +@pytest.mark.parametrize('key, value_from, value_to, expected', ( + ('restricted', True, False, ( + 'Made this service live' + )), + ('restricted', False, True, ( + 'Put this service back into trial mode' + )), + ('active', False, True, ( + 'Unsuspended this service' + )), + ('active', True, False, ( + 'Deleted this service' + )), + ('contact_link', 'x', 'y', ( + 'Set the contact details for this service to ‘y’' + )), + ('email_branding', 'foo', 'bar', ( + 'Updated this service’s email branding' + )), + ('inbound_api', 'foo', 'bar', ( + 'Updated the callback for received text messages' + )), + ('letter_branding', None, sample_uuid(), ( + 'Updated the logo on this service’s letters' + )), + ('letter_branding', sample_uuid(), None, ( + 'Removed the logo from this service’s letters' + )), + ('letter_contact_block', None, sample_uuid(), ( + 'Updated the default letter contact block for this service' + )), + ('message_limit', 1, 2, ( + 'Increased this service’s daily message limit from 1 to 2' + )), + ('message_limit', 2, 1, ( + 'Reduced this service’s daily message limit from 2 to 1' + )), + ('name', 'Old', 'New', ( + 'Renamed this service from ‘Old’ to ‘New’' + )), + ('permissions', ['a', 'b', 'c'], ['a', 'b', 'c', 'd'], ( + 'Added ‘d’ to this service’s permissions' + )), + ('permissions', ['a', 'b', 'c'], ['a', 'b'], ( + 'Removed ‘c’ from this service’s permissions' + )), + ('permissions', ['a', 'b', 'c'], ['c', 'd', 'e'], ( + 'Removed ‘a’ and ‘b’ from this service’s permissions, added ‘d’ and ‘e’' + )), + ('prefix_sms', True, False, ( + 'Set text messages to not start with the name of this service' + )), + ('prefix_sms', False, True, ( + 'Set text messages to start with the name of this service' + )), + ('research_mode', True, False, ( + 'Took this service out of research mode' + )), + ('research_mode', False, True, ( + 'Put this service into research mode' + )), + ('service_callback_api', 'foo', 'bar', ( + 'Updated the callback for delivery receipts' + )), +)) +def test_service_event( + key, + value_from, + value_to, + expected, +): + event = ServiceEvent( + { + 'created_at': 'foo', + 'updated_at': 'bar', + 'created_by_id': sample_uuid(), + }, + key, + value_from, + value_to, + ) + assert event.relevant is True + assert str(event) == expected diff --git a/tests/conftest.py b/tests/conftest.py index c37a4b8fb..ca097949f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3505,8 +3505,39 @@ def mock_get_service_and_organisation_counts(mocker): @pytest.fixture(scope='function') def mock_get_service_history(mocker): - return mocker.patch('app.service_api_client.get_service_history', return_value={'data': { - 'service_history': [], - 'api_key_history': [], + return mocker.patch('app.service_api_client.get_service_history', return_value={ + 'service_history': [ + { + 'name': 'Example service', + 'created_at': '2010-10-10T10:10:10.000000Z', + 'updated_at': None, + 'created_by_id': sample_uuid(), + }, + { + 'created_at': '2010-10-10T10:10:10.000000Z', + 'updated_at': '2012-12-12T12:12:12.000000Z', + 'created_by_id': uuid4(), + }, + ], + 'api_key_history': [ + { + 'name': 'Good key', + 'updated_at': None, + 'created_at': '2010-10-10T10:10:10.000000Z', + 'created_by_id': sample_uuid(), + }, + { + 'name': 'Bad key', + 'updated_at': '2012-11-11T12:12:12.000000Z', + 'created_at': '2011-11-11T11:11:11.000000Z', + 'created_by_id': sample_uuid(), + }, + { + 'name': 'Bad key', + 'updated_at': None, + 'created_at': '2011-11-11T11:11:11.000000Z', + 'created_by_id': sample_uuid(), + }, + ], 'events': [], - }}) + })