Merge pull request #528 from alphagov/activity-improvements

Link up the last 7 days

app/assets/stylesheets/components/big-number.scss

@@ -16,13 +16,38 @@
   @extend %big-number;
   background: $text-colour;
   color: $white;
+  position: relative;
 
   .big-number {
     padding: 15px;
+    position: relative;
+  }
+
+  .big-number-overlay-link {
+
+    background: transparent;
+    position: absolute;
+    top: 0;
+    left: 0;
+    background: transparent;
+    width: 100%;
+    height: 100%;
+
+    &:hover {
+      background: rgba($text-colour, 0.2);
+    }
+
   }
 
   .big-number-label {
+
     padding-bottom: 0;
+
+    &:link,
+    &:visited {
+      color: $white;
+    }
+
   }
 
   %big-number-status,

app/main/views/dashboard.py

@@ -82,20 +82,24 @@ def template_history(service_id):
 
 def add_rates_to(delivery_statistics):
 
+    keys = [
+        'emails_delivered',
+        'emails_requested',
+        'emails_failed',
+        'sms_requested',
+        'sms_delivered',
+        'sms_failed'
+    ]
+
     if not delivery_statistics or not delivery_statistics[0]:
-        return {}
+        return {
+            key: 0 for key in keys
+        }
 
     sum_of_statistics = reduce(
         lambda x, y: {
             key: x.get(key, 0) + y.get(key, 0)
-            for key in [
+            for key in keys
-                'emails_delivered',
-                'emails_requested',
-                'emails_failed',
-                'sms_requested',
-                'sms_delivered',
-                'sms_failed'
-            ]
         },
         delivery_statistics
     )

app/main/views/jobs.py

@@ -116,6 +116,8 @@ def view_notifications(service_id):
 
     filter_args = _parse_filter_args(request.args)
 
+    print(filter_args)
+
     notifications = notification_api_client.get_notifications_for_service(
         service_id=service_id,
         page=page,
@@ -147,8 +149,8 @@ def view_notifications(service_id):
                 service_id=service_id,
                 page=page,
                 page_size=notifications['total'],
-                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')
+                status=filter_args.get('status')
                 if 'status' in filter_args else ['delivered', 'failed'],
                 limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'])['notifications'])
         return csv_content, 200, {

app/templates/components/big-number.html

@@ -1,18 +1,31 @@
-{% macro big_number(number, label) %}
+{% macro big_number(number, label, label_link=None) %}
   <div class="big-number{% if right_aligned %}-right-aligned{% endif %}">
     {% if number is number %}
       {{ "{:,}".format(number) }}
     {% else %}
       {{ number }}
     {% endif %}
-    <span class="big-number-label">{{ label }}</span>
+    {% if label_link %}
+      <a class="big-number-label" href="{{ label_link }}">{{ label }}</a>
+      <a class="big-number-overlay-link" href="{{ label_link }}" aria-hidden="true"></a>
+    {% else %}
+      <span class="big-number-label">{{ label }}</span>
+    {% endif %}
   </div>
 {% endmacro %}
 
 
-{% macro big_number_with_status(number, label, failures, failure_percentage, danger_zone=False, failure_link=None) %}
+{% macro big_number_with_status(
+  number,
+  label,
+  failures,
+  failure_percentage,
+  danger_zone=False,
+  failure_link=None,
+  label_link=None
+) %}
   <div class="big-number-with-status">
-    {{ big_number(number, label) }}
+    {{ big_number(number, label, label_link) }}
     <div class="big-number-status{% if danger_zone %}-failing{% endif %}">
       {% if failures %}
         {% if failure_link %}

app/templates/main_nav.html

@@ -3,9 +3,6 @@
     <a href="{{ url_for('.service_dashboard', service_id=current_service.id) }}">{{ current_service.name }}</a>
   </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', 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>
     <li><a href="{{ url_for('.choose_template', service_id=current_service.id, template_type='sms') }}">Text message templates</a></li>

app/templates/views/dashboard/today.html

@@ -18,7 +18,8 @@
         statistics.emails_failed,
         statistics.get('emails_failure_rate', 0.0),
         statistics.get('emails_failure_rate', 0)|float > 3,
-        failure_link=url_for(".view_notifications", service_id=current_service.id, template_type='email', status='failed')
+        failure_link=url_for(".view_notifications", service_id=current_service.id, template_type='email', status='failed'),
+        label_link=url_for(".view_notifications", service_id=current_service.id, template_type='email', status='delivered,failed')
       ) }}
     </div>
     <div class="column-half">
@@ -28,7 +29,8 @@
         statistics.sms_failed,
         statistics.get('sms_failure_rate', 0.0),
         statistics.get('sms_failure_rate', 0)|float > 3,
-        failure_link=url_for(".view_notifications", service_id=current_service.id, template_type='sms', status='failed')
+        failure_link=url_for(".view_notifications", service_id=current_service.id, template_type='sms', status='failed'),
+        label_link=url_for(".view_notifications", service_id=current_service.id, template_type='sms', status='delivered,failed')
       ) }}
     </div>
   </div>

app/templates/views/notifications.html

@@ -11,36 +11,32 @@
 {% block maincolumn_content %}
 
   <h1 class="heading-large">
-
+    {%- if (request_args.get('template_type', 'email,sms') == 'email,sms') and (request_args.get('status', 'delivered,failed') == 'delivered,failed') -%}
-    {%- if (request_args.get('template_type', '') == '') and (request_args.get('status', 'delivered,failed') == 'delivered,failed') -%}
 
       Activity
 
     {%- else -%}
 
-      {% if request_args.get('status') != 'delivered,failed' %}
+      {%- if request_args.get('status') != 'delivered,failed' -%}
-        {% for label, option, _ in status_filters %}
+        {%- for label, option, _ in status_filters -%}
-          {% if request_args.get('status') == option %}
+          {%- if request_args.get('status', 'delivered,failed') == option -%}{{label}} {% endif -%}
-            {{ label }}
+        {%- endfor -%}
-          {% endif %}
+      {%- endif -%}
-        {% endfor %}
-      {% endif %}
 
-      {% if request_args.get('template_type') == '' %}
+      {%- if request_args.get('template_type', 'email,sms') == 'email,sms' %} emails and text messages
-        emails and text messages
+      {%- else -%}
-      {% else %}
 
-        {% for template_label, template_option, _ in type_filters %}
+        {%- for template_label, template_option, _ in type_filters -%}
-          {% if request_args.get('template_type') == template_option %}
+          {%- if request_args.get('template_type') == template_option -%}
-            {% if request_args.get('status', 'delivered,failed') == 'delivered,failed' %}
+            {%- if request_args.get('status', 'delivered,failed') == 'delivered,failed' -%}
               {{ template_label }}
-            {% else %}
+            {%- else -%}
               {{ template_label | lower }}
-            {% endif %}
+            {%- endif -%}
-          {% endif %}
+          {%- endif -%}
-        {% endfor %}
+        {%- endfor -%}
 
-      {% endif %}
+      {%- endif -%}
 
     {%- endif -%}
 

tests/app/main/views/test_jobs.py

@@ -107,6 +107,8 @@ def test_should_show_notifications_for_a_service(app_,
         assert notification['status'] in content
         assert notification['template']['name'] in content
         assert '.csv' in content
+        page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+        assert page.h1.string.strip() == 'Activity'
 
         mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed'], template_type=['email', 'sms'])  # noqa
 
@@ -126,7 +128,8 @@ def test_can_view_only_sms_notifications_for_a_service(app_,
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
-                template_type=['sms']))
+                template_type='sms',
+                status='delivered,failed'))
         assert response.status_code == 200
         content = response.get_data(as_text=True)
 
@@ -136,6 +139,8 @@ def test_can_view_only_sms_notifications_for_a_service(app_,
         assert notification['status'] in content
         assert notification['template']['name'] in content
         assert '.csv' in content
+        page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+        assert page.h1.string.strip() == 'Text messages'
 
         mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed'], template_type=['sms'])  # noqa
 
@@ -155,8 +160,8 @@ def test_can_view_only_email_notifications_for_a_service(app_,
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
-                status=['delivered', 'failed'],
+                status='delivered,failed',
-                template_type=['email']))
+                template_type='email'))
         assert response.status_code == 200
         content = response.get_data(as_text=True)
 
@@ -166,6 +171,8 @@ def test_can_view_only_email_notifications_for_a_service(app_,
         assert notification['status'] in content
         assert notification['template']['name'] in content
         assert '.csv' in content
+        page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+        assert page.h1.string.strip() == 'Emails'
 
         mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered', 'failed'], template_type=['email'])  # noqa
 
@@ -185,7 +192,7 @@ def test_can_view_successful_notifications_for_a_service(app_,
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
-                status=['delivered']))
+                status='delivered'))
         assert response.status_code == 200
         content = response.get_data(as_text=True)
         notifications = notification_json(service_one['id'])
@@ -194,6 +201,8 @@ def test_can_view_successful_notifications_for_a_service(app_,
         assert notification['status'] in content
         assert notification['template']['name'] in content
         assert '.csv' in content
+        page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+        assert page.h1.string.strip() == 'Successful  emails and text messages'
 
         mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['delivered'], template_type=['email', 'sms'])  # noqa
 
@@ -213,7 +222,7 @@ def test_can_view_failed_notifications_for_a_service(app_,
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
-                status=['failed']))
+                status='failed'))
         assert response.status_code == 200
         content = response.get_data(as_text=True)
         notifications = notification_json(service_one['id'])
@@ -222,10 +231,38 @@ def test_can_view_failed_notifications_for_a_service(app_,
         assert notification['status'] in content
         assert notification['template']['name'] in content
         assert '.csv' in content
+        page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+        assert page.h1.string.strip() == 'Failed  emails and text messages'
 
         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_can_view_failed_combination_of_notification_type_and_status(
+    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',
+                template_type='sms'))
+        assert response.status_code == 200
+        page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+        assert page.h1.string.strip() == 'Failed text messages'
+
+        mock_get_notifications.assert_called_with(limit_days=7, page=1, service_id=service_one['id'], status=['failed'], template_type=['sms'])  # noqa
+
+
 def test_should_show_notifications_for_a_service_with_next_previous(app_,
                                                                     service_one,
                                                                     api_user_active,