Merge pull request #519 from alphagov/week-activity

Change activity to view to request a week of data

app/main/views/jobs.py

@@ -1,14 +1,14 @@
 # -*- coding: utf-8 -*-
 
 import time
-import itertools
 
 from flask import (
     render_template,
     abort,
     jsonify,
     request,
-    url_for
+    url_for,
+    current_app
 )
 from flask_login import login_required
 from werkzeug.datastructures import MultiDict
@@ -28,7 +28,6 @@ from app.utils import (
 
 
 def _parse_filter_args(filter_dict):
-
     if not isinstance(filter_dict, MultiDict):
         filter_dict = MultiDict(filter_dict)
 
@@ -114,12 +113,15 @@ def view_notifications(service_id):
     page = get_page_from_request()
     if page is None:
         abort(404, "Invalid page argument ({}) reverting to page 1.".format(request.args['page'], None))
+
     filter_args = _parse_filter_args(request.args)
+
     notifications = notification_api_client.get_notifications_for_service(
         service_id=service_id,
         page=page,
-        template_type=filter_args.getlist('template_type') if 'template_type' in filter_args else None,
+        template_type=filter_args.get('template_type') if 'template_type' in filter_args else ['email', 'sms'],
-        status=filter_args.getlist('status') if 'status' in filter_args else ['delivered', 'failed'])
+        status=filter_args.get('status') if 'status' in filter_args else ['delivered', 'failed'],
+        limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'])
     view_dict = MultiDict(request.args)
     prev_page = None
     if notifications['links'].get('prev', None):
@@ -147,7 +149,8 @@ def view_notifications(service_id):
                 page_size=notifications['total'],
                 template_type=filter_args.getlist('template_type') if 'template_type' in filter_args else None,
                 status=filter_args.getlist('status')
-                if 'status' in filter_args else ['delivered', 'failed'])['notifications'])
+                if 'status' in filter_args else ['delivered', 'failed'],
+                limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'])['notifications'])
         return csv_content, 200, {
             'Content-Type': 'text/csv; charset=utf-8',
             'Content-Disposition': 'inline; filename="notifications.csv"'
@@ -168,14 +171,14 @@ def view_notifications(service_id):
             )] for item in [
                 ['Emails', 'email'],
                 ['Text messages', 'sms'],
-                ['Both', '']
+                ['Both', 'email,sms']
             ]
         ],
         status_filters=[
             [item[0], item[1], url_for(
                 '.view_notifications',
                 service_id=current_service['id'],
-                template_type=request.args.get('template_type', ''),
+                template_type=request.args.get('template_type', 'email,sms'),
                 status=item[1]
             )] for item in [
                 ['Successful', 'delivered'],

app/notify_client/notification_api_client.py

@@ -27,7 +27,8 @@ class NotificationApiClient(BaseAPIClient):
                                       template_type=None,
                                       status=None,
                                       page=None,
-                                      page_size=None):
+                                      page_size=None,
+                                      limit_days=None):
         params = {}
         if page is not None:
             params['page'] = page
@@ -43,6 +44,9 @@ class NotificationApiClient(BaseAPIClient):
                 params=params
             )
         else:
+            if limit_days is not None:
+                params['limit_days'] = limit_days
+
             return self.get(
                 url='/service/{}/notifications'.format(service_id),
                 params=params

app/templates/main_nav.html

@@ -4,7 +4,7 @@
   </h2>
   <ul>
   {% if current_user.has_permissions(['view_activity'], admin_override=True) %}
-    <li><a href="{{ url_for('.view_notifications', service_id=current_service.id, status='delivered,failed') }}">Activity</a></li>
+    <li><a href="{{ url_for('.view_notifications', service_id=current_service.id, status='delivered,failed', template_type='email,sms') }}">Activity</a></li>
   {% endif %}
   {% if current_user.has_permissions(['view_activity', 'manage_templates', 'manage_api_keys'], admin_override=True, any_=True) %}
     <li><a href="{{ url_for('.choose_template', service_id=current_service.id, template_type='email') }}">Email templates</a></li>

config.py

@@ -40,6 +40,7 @@ class Config(object):
     DESKPRO_API_KEY = os.environ['DESKPRO_API_KEY']
     DESKPRO_PERSON_EMAIL = os.environ['DESKPRO_PERSON_EMAIL']
     DESKPRO_TEAM_ID = os.environ['DESKPRO_TEAM_ID']
+    ACTIVITY_STATS_LIMIT_DAYS = 7
 
     EMAIL_DOMAIN_REGEXES = [
         "gov\.uk",

tests/app/main/views/test_jobs.py

@@ -2,6 +2,7 @@ from flask import url_for
 from bs4 import BeautifulSoup
 import json
 from app.utils import generate_notifications_csv
+from tests import notification_json
 
 
 def test_should_return_list_of_all_jobs(app_,
@@ -70,7 +71,6 @@ def test_should_show_updates_for_one_job_as_json(
 ):
     service_id = job_data['service']
     job_id = job_data['id']
-    file_name = job_data['original_file_name']
 
     with app_.test_request_context():
         with app_.test_client() as client:
@@ -101,13 +101,15 @@ def test_should_show_notifications_for_a_service(app_,
             response = client.get(url_for('main.view_notifications', service_id=service_one['id']))
         assert response.status_code == 200
         content = response.get_data(as_text=True)
-        notifications = mock_get_notifications(service_one['id'])
+        notifications = notification_json(service_one['id'])
         notification = notifications['notifications'][0]
         assert notification['to'] in content
         assert notification['status'] in content
         assert notification['template']['name'] in content
         assert '.csv' in content
 
+        mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed'], template_type=['email', 'sms'])  # noqa
+
 
 def test_can_view_only_sms_notifications_for_a_service(app_,
                                                        service_one,
@@ -124,16 +126,49 @@ def test_can_view_only_sms_notifications_for_a_service(app_,
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
-                type='sms'))
+                template_type=['sms']))
         assert response.status_code == 200
         content = response.get_data(as_text=True)
-        notifications = mock_get_notifications(service_one['id'])
+
+        notifications = notification_json(service_one['id'])
         notification = notifications['notifications'][0]
         assert notification['to'] in content
         assert notification['status'] in content
         assert notification['template']['name'] in content
         assert '.csv' in content
 
+        mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed'], template_type=['sms'])  # noqa
+
+
+def test_can_view_only_email_notifications_for_a_service(app_,
+                                                         service_one,
+                                                         api_user_active,
+                                                         mock_login,
+                                                         mock_get_user,
+                                                         mock_get_user_by_email,
+                                                         mock_get_service,
+                                                         mock_get_notifications,
+                                                         mock_has_permissions):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            client.login(api_user_active)
+            response = client.get(url_for(
+                'main.view_notifications',
+                service_id=service_one['id'],
+                status=['delivered', 'failed'],
+                template_type=['email']))
+        assert response.status_code == 200
+        content = response.get_data(as_text=True)
+
+        notifications = notification_json(service_one['id'])
+        notification = notifications['notifications'][0]
+        assert notification['to'] in content
+        assert notification['status'] in content
+        assert notification['template']['name'] in content
+        assert '.csv' in content
+
+        mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed'], template_type=['email'])  # noqa
+
 
 def test_can_view_successful_notifications_for_a_service(app_,
                                                          service_one,
@@ -150,16 +185,46 @@ def test_can_view_successful_notifications_for_a_service(app_,
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
-                status=['sent', 'delivered']))
+                status=['delivered']))
         assert response.status_code == 200
         content = response.get_data(as_text=True)
-        notifications = mock_get_notifications(service_one['id'])
+        notifications = notification_json(service_one['id'])
         notification = notifications['notifications'][0]
         assert notification['to'] in content
         assert notification['status'] in content
         assert notification['template']['name'] in content
         assert '.csv' in content
 
+        mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered'], template_type=['email', 'sms'])  # noqa
+
+
+def test_can_view_failed_notifications_for_a_service(app_,
+                                                     service_one,
+                                                     api_user_active,
+                                                     mock_login,
+                                                     mock_get_user,
+                                                     mock_get_user_by_email,
+                                                     mock_get_service,
+                                                     mock_get_notifications,
+                                                     mock_has_permissions):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            client.login(api_user_active)
+            response = client.get(url_for(
+                'main.view_notifications',
+                service_id=service_one['id'],
+                status=['failed']))
+        assert response.status_code == 200
+        content = response.get_data(as_text=True)
+        notifications = notification_json(service_one['id'])
+        notification = notifications['notifications'][0]
+        assert notification['to'] in content
+        assert notification['status'] in content
+        assert notification['template']['name'] in content
+        assert '.csv' in content
+
+        mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['failed'], template_type=['email', 'sms'])  # noqa
+
 
 def test_should_show_notifications_for_a_service_with_next_previous(app_,
                                                                     service_one,

tests/conftest.py

@@ -691,7 +691,8 @@ def mock_get_notifications(mocker):
                            page=1,
                            page_size=50,
                            template_type=None,
-                           status=None):
+                           status=None,
+                           limit_days=None):
         return notification_json(service_id)
     return mocker.patch(
         'app.notification_api_client.get_notifications_for_service',
@@ -705,7 +706,8 @@ def mock_get_notifications_with_previous_next(mocker):
                            job_id=None,
                            page=1,
                            template_type=None,
-                           status=None):
+                           status=None,
+                           limit_days=None):
         return notification_json(service_id, with_links=True)
     return mocker.patch(
         'app.notification_api_client.get_notifications_for_service',