Merge pull request #803 from alphagov/remove-stats-from-job-page

Remove stats from job page

app/main/views/jobs.py

@@ -1,5 +1,4 @@
 # -*- coding: utf-8 -*-
-
 import time
 import dateutil
 from datetime import datetime, timedelta, timezone
@@ -21,7 +20,6 @@ from app import (
     job_api_client,
     notification_api_client,
     service_api_client,
-    statistics_api_client,
     current_service,
     format_datetime_short)
 from app.main import main
@@ -30,7 +28,7 @@ from app.utils import (
     generate_previous_next_dict,
     user_has_permissions,
     generate_notifications_csv)
-from app.statistics_utils import sum_of_statistics, statistics_by_state, add_rate_to_jobs
+from app.statistics_utils import add_rate_to_jobs
 from app.utils import get_help_argument
 
 
@@ -171,9 +169,6 @@ def view_notifications(service_id, message_type):
         template_type=[message_type],
         status=filter_args.get('status'),
         limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'])
-    service_statistics_by_state = statistics_by_state(sum_of_statistics(
-        statistics_api_client.get_statistics_for_service(service_id, limit_days=7)['data']
-    ))
     view_dict = dict(
         message_type=message_type,
         status=request.args.get('status')
@@ -223,23 +218,11 @@ def view_notifications(service_id, message_type):
             message_type=message_type,
             status=request.args.get('status')
         ),
-        status_filters=[
+        status_filters=get_status_filters(
-            [
+            current_service,
-                item[0], item[1],
+            message_type,
-                url_for(
+            service_api_client.get_detailed_service(service_id)['data']['statistics']
-                    '.view_notifications',
+        )
-                    service_id=current_service['id'],
-                    message_type=message_type,
-                    status=item[1]
-                ),
-                service_statistics_by_state[message_type][item[0]]
-            ] for item in [
-                ['processed', 'sending,delivered,failed'],
-                ['sending', 'sending'],
-                ['delivered', 'delivered'],
-                ['failed', 'failed'],
-            ]
-        ]
     )
 
 
@@ -261,6 +244,34 @@ def view_notification(service_id, job_id, notification_id):
     )
 
 
+def get_status_filters(service, message_type, statistics):
+    stats = statistics[message_type]
+    stats['sending'] = stats['requested'] - stats['delivered'] - stats['failed']
+
+    filters = [
+        # key, label, option
+        ('requested', 'processed', 'sending,delivered,failed'),
+        ('sending', 'sending', 'sending'),
+        ('delivered', 'delivered', 'delivered'),
+        ('failed', 'failed', 'failed'),
+    ]
+    return [
+        # return list containing label, option, link, count
+        (
+            label,
+            option,
+            url_for(
+                '.view_notifications',
+                service_id=service['id'],
+                message_type=message_type,
+                status=option
+            ),
+            stats[key]
+        )
+        for key, label, option in filters
+    ]
+
+
 def _get_job_counts(job, help_argument):
     return [
         (

app/notify_client/statistics_api_client.py

@@ -13,15 +13,6 @@ class StatisticsApiClient(BaseAPIClient):
         self.client_id = app.config['ADMIN_CLIENT_USER_NAME']
         self.secret = app.config['ADMIN_CLIENT_SECRET']
 
-    def get_statistics_for_service(self, service_id, limit_days=None):
-        params = {}
-        if limit_days is not None:
-            params['limit_days'] = limit_days
-        return self.get(
-            url='/service/{}/notifications-statistics'.format(service_id),
-            params=params
-        )
-
     def get_statistics_for_service_for_day(self, service_id, day):
         url = '/service/{}/notifications-statistics/day/{}'.format(service_id, day)
         try:

tests/app/main/views/test_accept_invite.py

@@ -311,7 +311,6 @@ def test_new_invited_user_verifies_and_added_to_service(app_,
                                                         mock_accept_invite,
                                                         mock_get_service,
                                                         mock_get_service_templates,
-                                                        mock_get_service_statistics,
                                                         mock_get_template_statistics,
                                                         mock_get_jobs,
                                                         mock_has_permissions,

tests/app/main/views/test_dashboard.py

@@ -65,7 +65,6 @@ def test_get_started(
     api_user_active,
     mock_get_service,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_service_statistics,
     mock_get_aggregate_service_statistics,
     mock_get_user,
     mock_get_user_by_email,
@@ -95,7 +94,6 @@ def test_get_started_is_hidden_once_templates_exist(
     api_user_active,
     mock_get_service,
     mock_get_service_templates,
-    mock_get_service_statistics,
     mock_get_aggregate_service_statistics,
     mock_get_user,
     mock_get_user_by_email,
@@ -166,7 +164,6 @@ def test_should_show_all_templates_on_template_statistics_page(
     api_user_active,
     mock_get_service,
     mock_get_service_templates,
-    mock_get_service_statistics,
     mock_get_user,
     mock_get_user_by_email,
     mock_login,
@@ -207,7 +204,6 @@ def test_should_show_recent_jobs_on_dashboard(
     api_user_active,
     mock_get_service,
     mock_get_service_templates,
-    mock_get_service_statistics,
     mock_get_aggregate_service_statistics,
     mock_get_user,
     mock_get_user_by_email,
@@ -253,7 +249,6 @@ def _test_dashboard_menu(mocker, app_, usr, service, permissions):
             mocker.patch('app.user_api_client.get_user', return_value=usr)
             mocker.patch('app.user_api_client.get_user_by_email', return_value=usr)
             mocker.patch('app.service_api_client.get_service', return_value={'data': service})
-            mocker.patch('app.statistics_api_client.get_statistics_for_service', return_value={'data': [{}]})
             client.login(usr)
             return client.get(url_for('main.service_dashboard', service_id=service['id']))
 
@@ -392,7 +387,6 @@ def test_route_for_service_permissions(mocker,
                                        mock_get_user,
                                        mock_get_service_templates,
                                        mock_get_jobs,
-                                       mock_get_service_statistics,
                                        mock_get_template_statistics,
                                        mock_get_detailed_service,
                                        mock_get_usage):
@@ -435,7 +429,6 @@ def test_service_dashboard_updates_gets_dashboard_totals(mocker,
                                                          mock_get_template_statistics,
                                                          mock_get_detailed_service,
                                                          mock_get_jobs,
-                                                         mock_get_service_statistics,
                                                          mock_get_usage):
     dashboard_totals = mocker.patch('app.main.views.dashboard.get_dashboard_totals', return_value={
         'email': {'requested': 123, 'delivered': 0, 'failed': 0},

tests/app/main/views/test_jobs.py

@@ -1,12 +1,13 @@
+import json
+from urllib.parse import quote
+
 import pytest
 from flask import url_for
 from bs4 import BeautifulSoup
-import json
+
 from app.utils import generate_notifications_csv
-from app.main.views.jobs import get_time_left
+from app.main.views.jobs import get_time_left, get_status_filters
-from tests import (notification_json, job_json)
+from tests import notification_json
-from tests.conftest import fake_uuid
-from tests.conftest import mock_get_job as mock_get_job1
 from freezegun import freeze_time
 
 
@@ -57,7 +58,6 @@ def test_should_show_page_for_one_job(
     service_one,
     active_user_with_permissions,
     mock_get_service_template,
-    mock_get_service_statistics,
     mock_get_job,
     mocker,
     mock_get_notifications,
@@ -111,7 +111,6 @@ def test_should_show_not_show_csv_download_in_tour(
     service_one,
     active_user_with_permissions,
     mock_get_service_template,
-    mock_get_service_statistics,
     mock_get_job,
     mocker,
     mock_get_notifications,
@@ -147,7 +146,6 @@ def test_should_show_updates_for_one_job_as_json(
     service_one,
     active_user_with_permissions,
     mock_get_notifications,
-    mock_get_service_statistics,
     mock_get_job,
     mocker,
     fake_uuid
@@ -203,7 +201,7 @@ def test_can_show_notifications(
     service_one,
     active_user_with_permissions,
     mock_get_notifications,
-    mock_get_service_statistics,
+    mock_get_detailed_service,
     mocker,
     message_type,
     page_title,
@@ -263,7 +261,7 @@ def test_should_show_notifications_for_a_service_with_next_previous(
     service_one,
     active_user_with_permissions,
     mock_get_notifications_with_previous_next,
-    mock_get_service_statistics,
+    mock_get_detailed_service,
     mocker
 ):
     with app_.test_request_context():
@@ -321,3 +319,41 @@ def test_should_download_notifications_for_a_job(app_,
 @freeze_time("2016-01-10 12:00:00.000000")
 def test_time_left(job_created_at, expected_message):
     assert get_time_left(job_created_at) == expected_message
+
+
+STATISTICS = {
+    'sms': {
+        'requested': 6,
+        'failed': 2,
+        'delivered': 1
+    }
+}
+
+
+def test_get_status_filters_calculates_stats(app_):
+    with app_.test_request_context():
+        ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS)
+
+    assert {label: count for label, _option, _link, count in ret} == {
+        'processed': 6,
+        'sending': 3,
+        'failed': 2,
+        'delivered': 1
+    }
+
+
+def test_get_status_filters_in_right_order(app_):
+    with app_.test_request_context():
+        ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS)
+
+    assert [label for label, _option, _link, _count in ret] == [
+        'processed', 'sending', 'delivered', 'failed'
+    ]
+
+
+def test_get_status_filters_constructs_links(app_):
+    with app_.test_request_context():
+        ret = get_status_filters({'id': 'foo'}, 'sms', STATISTICS)
+
+    link = ret[0][2]
+    assert link == '/services/foo/notifications/sms?status={}'.format(quote('sending,delivered,failed'))

tests/app/main/views/test_sign_out.py

@@ -17,7 +17,6 @@ def test_sign_out_user(app_,
                        mock_get_user_by_email,
                        mock_login,
                        mock_get_service_templates,
-                       mock_get_service_statistics,
                        mock_get_jobs,
                        mock_has_permissions,
                        mock_get_template_statistics,

tests/app/notify_client/test_statistics_client.py

@@ -4,34 +4,6 @@ from datetime import datetime
 from app.notify_client.statistics_api_client import StatisticsApiClient
 
 
-def test_notifications_statistics_client_calls_correct_api_endpoint(mocker, api_user_active):
-
-    some_service_id = uuid.uuid4()
-    expected_url = '/service/{}/notifications-statistics'.format(some_service_id)
-
-    client = StatisticsApiClient()
-
-    mock_get = mocker.patch('app.notify_client.statistics_api_client.StatisticsApiClient.get')
-
-    client.get_statistics_for_service(some_service_id)
-
-    mock_get.assert_called_once_with(url=expected_url, params={})
-
-
-def test_notifications_statistics_client_calls_correct_api_endpoint_with_params(mocker, api_user_active):
-
-    some_service_id = uuid.uuid4()
-    expected_url = '/service/{}/notifications-statistics'.format(some_service_id)
-
-    client = StatisticsApiClient()
-
-    mock_get = mocker.patch('app.notify_client.statistics_api_client.StatisticsApiClient.get')
-
-    client.get_statistics_for_service(some_service_id, limit_days=99)
-
-    mock_get.assert_called_once_with(url=expected_url, params={'limit_days': 99})
-
-
 def test_notifications_statistics_client_for_stats_by_day_calls_correct_api_endpoint(mocker, api_user_active):
 
     some_service_id = uuid.uuid4()

tests/conftest.py

@@ -200,15 +200,6 @@ def mock_delete_service(mocker, mock_get_service):
         'app.service_api_client.delete_service', side_effect=_delete)
 
 
-@pytest.fixture(scope='function')
-def mock_get_service_statistics(mocker):
-    def _create(service_id, limit_days=None):
-        return {'data': [{}]}
-
-    return mocker.patch(
-        'app.statistics_api_client.get_statistics_for_service', side_effect=_create)
-
-
 @pytest.fixture(scope='function')
 def mock_get_service_statistics_for_day(mocker):