From 2a502753a4a00f0e029af61a7d5072b42dbda40f Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 25 Jan 2017 15:59:06 +0000
Subject: [PATCH] Filter and navigate usage by financial year
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Right now we tell people that the usage page is for the current
financial year. This is a lie – it’s for all time.

So this commit calls through to the API to get the stats for (by
default) the current financial year.

We already do this for the monthly breakdown, this just does the same
thing for the yearly totals.

It also adds navigation to show the data for other financial years:
- previous so you can go back and see your usage and verify that the
  bill you’re about to pay is correct
- next so that you can check what your SMS allowance is going to be
  before you actually get into it
---
 app/main/views/dashboard.py             | 26 +++++--
 app/notify_client/service_api_client.py |  7 +-
 app/templates/components/pill.html      |  7 +-
 app/templates/views/usage.html          | 92 +++++++++++++------------
 app/utils.py                            |  8 +++
 tests/app/main/views/test_dashboard.py  | 41 ++++++-----
 tests/app/test_utils.py                 |  4 ++
 tests/conftest.py                       |  2 +-
 8 files changed, 112 insertions(+), 75 deletions(-)

diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py
index d91cbbef9..f171faf26 100644
--- a/app/main/views/dashboard.py
+++ b/app/main/views/dashboard.py
@@ -19,7 +19,7 @@ from app import (
     template_statistics_client
 )
 from app.statistics_utils import get_formatted_percentage, add_rate_to_job
-from app.utils import user_has_permissions
+from app.utils import user_has_permissions, get_current_financial_year
 
 
 # This is a placeholder view method to be replaced
@@ -79,17 +79,26 @@ def template_history(service_id):
 @login_required
 @user_has_permissions('manage_settings', admin_override=True)
 def usage(service_id):
+    current_financial_year = get_current_financial_year()
     try:
-        year = int(request.args.get('year', 2016))
+        year = int(request.args.get('year', current_financial_year))
     except ValueError:
         abort(404)
     return render_template(
         'views/usage.html',
-        months=get_free_paid_breakdown_for_billable_units(
+        months=list(get_free_paid_breakdown_for_billable_units(
             year, service_api_client.get_billable_units(service_id, year)
-        ),
-        year=year,
-        **calculate_usage(service_api_client.get_service_usage(service_id)['data'])
+        )),
+        selected_year=year,
+        years=[
+            (
+                'financial year',
+                year,
+                url_for('.usage', service_id=service_id, year=year),
+                '{} to {}'.format(year, year + 1),
+            ) for year in range(current_financial_year - 1, current_financial_year + 2)
+        ],
+        **calculate_usage(service_api_client.get_service_usage(service_id, year)['data'])
     )
 
 
@@ -156,7 +165,10 @@ def get_dashboard_partials(service_id):
         'has_jobs': bool(immediate_jobs),
         'usage': render_template(
             'views/dashboard/_usage.html',
-            **calculate_usage(service_api_client.get_service_usage(service_id)['data'])
+            **calculate_usage(service_api_client.get_service_usage(
+                service_id,
+                get_current_financial_year(),
+            )['data'])
         ),
     }
 
diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py
index 440301428..211eb46a1 100644
--- a/app/notify_client/service_api_client.py
+++ b/app/notify_client/service_api_client.py
@@ -213,8 +213,11 @@ class ServiceAPIClient(NotifyAdminAPIClient):
     def get_service_history(self, service_id):
         return self.get('/service/{0}/history'.format(service_id))
 
-    def get_service_usage(self, service_id):
-        return self.get('/service/{0}/fragment/aggregate_statistics'.format(service_id))
+    def get_service_usage(self, service_id, year=None):
+        return self.get(
+            '/service/{0}/fragment/aggregate_statistics'.format(service_id),
+            params=dict(year=year)
+        )
 
     def get_weekly_notification_stats(self, service_id):
         return self.get(url='/service/{}/notifications/weekly'.format(service_id))
diff --git a/app/templates/components/pill.html b/app/templates/components/pill.html
index 7f283d204..c77810e5c 100644
--- a/app/templates/components/pill.html
+++ b/app/templates/components/pill.html
@@ -3,7 +3,8 @@
 {% macro pill(
   title,
   items=[],
-  current_value=None
+  current_value=None,
+  big_number_args={'smaller': True}
 ) %}
   <nav role='navigation' class='pill' aria-labelledby="pill_{{title}}">
     <h2 id="pill_{{title}}" class="visuallyhidden">{{title}}</h2>
@@ -13,8 +14,8 @@
       {% else %}
         <a href="{{ link }}">
       {% endif %}
-          {{ big_number(count, smaller=True) }}
-          <div class="pill-label">{{ label }}</div>
+        {{ big_number(count, **big_number_args) }}
+        <div class="pill-label">{{ label }}</div>
       {% if current_value == option %}
         </span>
       {% else %}
diff --git a/app/templates/views/usage.html b/app/templates/views/usage.html
index e5d5f64b1..0059ed939 100644
--- a/app/templates/views/usage.html
+++ b/app/templates/views/usage.html
@@ -1,5 +1,6 @@
 {% from "components/big-number.html" import big_number %}
 {% from "components/table.html" import list_table, field, hidden_field_heading, row_heading, text_field %}
+{% from "components/pill.html" import pill %}
 
 {% extends "withnav_template.html" %}
 
@@ -9,15 +10,10 @@
 
 {% block maincolumn_content %}
 
-    <div class='grid-row'>
-      <div class='column-half'>
-        <h2 class='heading-large'>Usage</h2>
-      </div>
-      <div class='column-half'>
-        <div class='align-with-heading-copy'>
-          Financial year {{ year }} to {{ year + 1 }}
-        </div>
-      </div>
+    <h2 class='heading-large'>Usage</h2>
+
+    <div class="bottom-gutter">
+      {{ pill('Year', years, selected_year, big_number_args={'smallest': True}) }}
     </div>
 
     <div class='grid-row'>
@@ -63,45 +59,51 @@
       </div>
     </div>
 
-    <div class="dashboard-table body-copy-table">
-      {% call(month, row_index) list_table(
-        months,
-        caption="Total spend",
-        caption_visible=False,
-        empty_message='',
-        field_headings=[
-          'By month',
-          hidden_field_heading('Cost'),
-        ],
-        field_headings_visible=True
-      ) %}
-        {% call row_heading() %}
-          {{ month.name }}
+    {% if months %}
+      <div class="dashboard-table body-copy-table">
+        {% call(month, row_index) list_table(
+          months,
+          caption="Total spend",
+          caption_visible=False,
+          empty_message='',
+          field_headings=[
+            'By month',
+            hidden_field_heading('Cost'),
+          ],
+          field_headings_visible=True
+        ) %}
+          {% call row_heading() %}
+            {{ month.name }}
+          {% endcall %}
+          {% call field(align='left') %}
+            {{ big_number(
+              sms_rate * month.paid,
+              currency="£",
+              smallest=True
+            ) }}
+            <ul>
+            {% if month.free %}
+              <li class="tabular-numbers">{{ "{:,}".format(month.free) }} free text messages</li>
+            {% endif %}
+            {% if month.paid %}
+              <li class="tabular-numbers">{{ "{:,}".format(month.paid) }} text messages at
+              {{- ' {:.2f}p'.format(sms_rate * 100) }}</li>
+            {% endif %}
+            {% if not (month.free or month.paid) %}
+              <li aria-hidden="true">–</li>
+            {% endif %}
+            </ul>
+          {% endcall %}
         {% endcall %}
-        {% call field(align='left') %}
-          {{ big_number(
-            sms_rate * month.paid,
-            currency="£",
-            smallest=True
-          ) }}
-          <ul>
-          {% if month.free %}
-            <li class="tabular-numbers">{{ "{:,}".format(month.free) }} free text messages</li>
-          {% endif %}
-          {% if month.paid %}
-            <li class="tabular-numbers">{{ "{:,}".format(month.paid) }} text messages at
-            {{- ' {:.2f}p'.format(sms_rate * 100) }}</li>
-          {% endif %}
-          {% if not (month.free or month.paid) %}
-            <li aria-hidden="true">–</li>
-          {% endif %}
-          </ul>
-        {% endcall %}
-      {% endcall %}
-    </div>
+      </div>
+    {% endif %}
 
     <div class="grid-row">
-      <div class="column-half">&nbsp;</div>
+      <div class="column-half">
+        <p class="align-with-heading-copy">
+          Financial year ends 31 March.
+        </p>
+      </div>
       <div class="column-half">
         <p class="align-with-heading-copy">
           What counts as 1 text message?<br />
diff --git a/app/utils.py b/app/utils.py
index 9c4b600ed..002a2dd01 100644
--- a/app/utils.py
+++ b/app/utils.py
@@ -4,6 +4,7 @@ from io import StringIO, BytesIO
 from os import path
 from functools import wraps
 import unicodedata
+from datetime import datetime
 
 from flask import (
     abort,
@@ -322,3 +323,10 @@ def png_from_pdf(pdf_endpoint):
         filename_or_fp=output,
         mimetype='image/png',
     )
+
+
+def get_current_financial_year():
+    now = datetime.utcnow()
+    current_month = int(now.strftime('%-m'))
+    current_year = int(now.strftime('%Y'))
+    return current_year if current_month > 3 else current_year - 1
diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py
index 6a323d1b4..e3bb754c0 100644
--- a/tests/app/main/views/test_dashboard.py
+++ b/tests/app/main/views/test_dashboard.py
@@ -14,6 +14,7 @@ from app.main.views.dashboard import (
 
 from tests import validate_route_permission
 from tests.conftest import SERVICE_ONE_ID
+from tests.app.test_utils import normalize_spaces
 
 stub_template_stats = [
     {
@@ -233,33 +234,34 @@ def test_should_show_recent_jobs_on_dashboard(
             assert table_rows[index].find_all('td')[column_index].text.strip() == str(count)
 
 
-@freeze_time("2016-12-31 11:09:00.061258")
+@freeze_time("2012-03-31 12:12:12")
 def test_usage_page(
     logged_in_client,
-    api_user_active,
-    mock_get_service,
-    mock_get_user,
-    mock_has_permissions,
     mock_get_usage,
     mock_get_billable_units,
 ):
-    response = logged_in_client.get(url_for('main.usage', service_id=SERVICE_ONE_ID, year=2000))
+    response = logged_in_client.get(url_for('main.usage', service_id=SERVICE_ONE_ID))
 
     assert response.status_code == 200
 
-    mock_get_billable_units.assert_called_once_with(SERVICE_ONE_ID, 2000)
+    mock_get_billable_units.assert_called_once_with(SERVICE_ONE_ID, 2011)
+    mock_get_usage.assert_called_once_with(SERVICE_ONE_ID, 2011)
 
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
 
     cols = page.find_all('div', {'class': 'column-half'})
+    nav = page.find('nav', {'class': 'pill'})
+    nav_links = nav.find_all('a')
 
-    assert cols[1].text.strip() == 'Financial year 2000 to 2001'
+    assert normalize_spaces(nav_links[0].text) == '2010 to 2011 financial year'
+    assert normalize_spaces(nav.find('span').text) == '2011 to 2012 financial year'
+    assert normalize_spaces(nav_links[1].text) == '2012 to 2013 financial year'
 
-    assert '123' in cols[2].text
-    assert 'Emails' in cols[2].text
+    assert '123' in cols[0].text
+    assert 'Emails' in cols[0].text
 
-    assert '456,123' in cols[3].text
-    assert 'Text messages' in cols[3].text
+    assert '456,123' in cols[1].text
+    assert 'Text messages' in cols[1].text
 
     table = page.find('table').text.strip()
 
@@ -271,13 +273,18 @@ def test_usage_page(
     assert '206,246 text messages at 1.65p' in table
 
 
-@freeze_time("2016-12-31 11:09:00.061258")
+def test_usage_page_with_year_argument(
+    logged_in_client,
+    mock_get_usage,
+    mock_get_billable_units
+):
+    assert logged_in_client.get(url_for('main.usage', service_id=SERVICE_ONE_ID, year=2000)).status_code == 200
+    mock_get_billable_units.assert_called_once_with(SERVICE_ONE_ID, 2000)
+    mock_get_usage.assert_called_once_with(SERVICE_ONE_ID, 2000)
+
+
 def test_usage_page_for_invalid_year(
     logged_in_client,
-    api_user_active,
-    mock_get_service,
-    mock_get_user,
-    mock_has_permissions,
 ):
     assert logged_in_client.get(url_for('main.usage', service_id=SERVICE_ONE_ID, year='abcd')).status_code == 404
 
diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py
index 2c8ad1d83..224e2a244 100644
--- a/tests/app/test_utils.py
+++ b/tests/app/test_utils.py
@@ -148,3 +148,7 @@ def test_generate_notifications_csv_formats_row_number_correctly(mocker, row_num
 
     assert len(csv_rows) == 1
     assert csv_rows[0].get('Row number') == expected_result
+
+
+def normalize_spaces(string):
+    return ' '.join(string.split())
diff --git a/tests/conftest.py b/tests/conftest.py
index 84f810ea2..0e11976e4 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1256,7 +1256,7 @@ def mock_get_template_statistics_for_template(mocker, service_one):
 
 @pytest.fixture(scope='function')
 def mock_get_usage(mocker, service_one, fake_uuid):
-    def _get_usage(service_id):
+    def _get_usage(service_id, year=None):
         return {'data': {
             "sms_count": 456123,
             "email_count": 123