From b1f2b1bc0ffc2013f7973f927fe76af7b0b86f79 Mon Sep 17 00:00:00 2001
From: Katie Smith <katie.smith@digital.cabinet-office.gov.uk>
Date: Wed, 3 Oct 2018 09:13:28 +0100
Subject: [PATCH 1/3] Update billing mocks and get_sum_billing_units function

Updated the 'get_sum_billing_units' function to no longer multiply the
billing units by the rate multiplier. The billing_units that come from
notifications-api already consist of the billable_units * rate_multiplier.
The rate_multiplier is also not returned from the api response anymore.

Also updated the billing mocks since these were not mocking the right fields in
the JSON responses from the API billing endpoints, and added the new
'postage' field which will get returned from the monthly-usage endpoint
in notifications-api.
---
 app/main/views/dashboard.py |  4 +-
 tests/conftest.py           | 83 +++++++++++++++----------------------
 2 files changed, 35 insertions(+), 52 deletions(-)

diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py
index 72f8f62b8..9e0dba73d 100644
--- a/app/main/views/dashboard.py
+++ b/app/main/views/dashboard.py
@@ -418,8 +418,8 @@ def get_months_for_year(start, end, year):
 
 def get_sum_billing_units(billing_units, month=None):
     if month:
-        return sum(b['billing_units'] * b.get('rate_multiplier', 1) for b in billing_units if b['month'] == month)
-    return sum(b['billing_units'] * b.get('rate_multiplier', 1) for b in billing_units)
+        return sum(b['billing_units'] for b in billing_units if b['month'] == month)
+    return sum(b['billing_units'] for b in billing_units)
 
 
 def get_free_paid_breakdown_for_billable_units(year, free_sms_fragment_limit, billing_units):
diff --git a/tests/conftest.py b/tests/conftest.py
index 66cd2ccce..cb43e4387 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -2278,16 +2278,11 @@ def mock_get_template_statistics_for_template(mocker, service_one):
 def mock_get_usage(mocker, service_one, fake_uuid):
     def _get_usage(service_id, year=None):
         return [
-            {"international": False, "rate": 0.00, "notification_type": "email",
-             "rate_multiplier": None, "billing_units": 1000},
-            {"international": False, "rate": 0.0165, "rate_multiplier": 1,
-             "notification_type": "sms", "billing_units": 251500},
-            {"international": True, "rate": 0.0165, "rate_multiplier": 1,
-             "notification_type": "sms", "billing_units": 300},
-            {"international": True, "rate": 0.0165, "rate_multiplier": 2,
-             "notification_type": "sms", "billing_units": 150},
-            {"international": True, "rate": 0.0165, "rate_multiplier": 3,
-             "notification_type": "sms", "billing_units": 30},
+            {"notification_type": "email", "billing_units": 1000, "rate": 0.00, "letter_total": 0},
+            {"notification_type": "sms", "billing_units": 251500, "rate": 0.0165, "letter_total": 0},
+            {"notification_type": "sms", "billing_units": 300, "rate": 0.0165, "letter_total": 0},
+            {"notification_type": "sms", "billing_units": 300, "rate": 0.0165, "letter_total": 0},
+            {"notification_type": "sms", "billing_units": 90, "rate": 0.0165, "letter_total": 0}
         ]
 
     return mocker.patch(
@@ -2300,99 +2295,87 @@ def mock_get_billable_units(mocker):
         return [
             {
                 'month': 'April',
-                'international': False,
-                'rate_multiplier': 1,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 249500
+                'billing_units': 249500,
+                'postage': 'none',
             },
             {
                 'month': 'April',
-                'international': True,
-                'rate_multiplier': 1,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 100
+                'billing_units': 100,
+                'postage': 'none',
             },
             {
                 'month': 'April',
-                'international': True,
-                'rate_multiplier': 2,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 100
+                'billing_units': 200,
+                'postage': 'none',
             },
             {
                 'month': 'April',
-                'international': True,
-                'rate_multiplier': 3,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 20
+                'billing_units': 60,
+                'postage': 'none',
             },
             {
                 'month': 'March',
-                'international': False,
-                'rate_multiplier': 1,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 1000
+                'billing_units': 1000,
+                'postage': 'none',
             },
             {
                 'month': 'March',
-                'international': True,
-                'rate_multiplier': 1,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 100
+                'billing_units': 100,
+                'postage': 'none',
             },
             {
                 'month': 'March',
-                'international': True,
-                'rate_multiplier': 2,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 50
+                'billing_units': 100,
+                'postage': 'none',
             },
             {
                 'month': 'March',
-                'international': True,
-                'rate_multiplier': 3,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 10
+                'billing_units': 30,
+                'postage': 'none',
             },
             {
                 'month': 'February',
-                'international': False,
-                'rate_multiplier': 1,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 1000
+                'billing_units': 1000,
+                'postage': 'none',
             },
             {
                 'month': 'February',
-                'international': True,
-                'rate_multiplier': 1,
                 'notification_type': 'sms',
                 'rate': 0.0165,
-                'billing_units': 100
+                'billing_units': 100,
+                'postage': 'none',
             },
             {
                 'month': 'February',
-                'international': False,
-                'rate_multiplier': 1,
                 'notification_type': 'letter',
                 'rate': 0.31,
-                'billing_units': 10
+                'billing_units': 10,
+                'postage': 'second',
             },
             {
                 'month': 'February',
-                'international': False,
-                'rate_multiplier': 1,
                 'notification_type': 'letter',
                 'rate': 0.33,
-                'billing_units': 5
+                'billing_units': 5,
+                'postage': 'first',
             }
         ]
 
@@ -2405,12 +2388,12 @@ def mock_get_future_usage(mocker, service_one, fake_uuid):
     def _get_usage(service_id, year=None):
         return [
             {
-                'notification_type': 'sms', 'international': False,
-                'credits': 0, 'rate_multiplier': 1, 'rate': 0.0158, 'billing_units': 0
+                'notification_type': 'sms', 'billing_units': 0,
+                'rate': 0.0158, 'letter_total': 0
             },
             {
-                'notification_type': 'email', 'international': False,
-                'credits': 0, 'rate_multiplier': 1, 'rate': 0, 'billing_units': 0
+                'notification_type': 'email', 'billing_units': 0,
+                'rate': 0.0, 'letter_total': 0
             }
         ]
 

From 4188c0297d9983980e49df2ba5eb78d462e80e56 Mon Sep 17 00:00:00 2001
From: Katie Smith <katie.smith@digital.cabinet-office.gov.uk>
Date: Wed, 3 Oct 2018 11:18:46 +0100
Subject: [PATCH 2/3] Display letter postage on the usage page

The monthly letter cost was already broken down by price on the usage
page, but this change adds postage details to the usage page too.
---
 app/main/views/dashboard.py                 |  6 +++-
 app/templates/views/usage-with-letters.html |  2 +-
 tests/app/main/views/test_dashboard.py      | 31 +++++++++++++++++++--
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py
index 9e0dba73d..1aa5c1c37 100644
--- a/app/main/views/dashboard.py
+++ b/app/main/views/dashboard.py
@@ -435,8 +435,12 @@ def get_free_paid_breakdown_for_billable_units(year, free_sms_fragment_limit, bi
             free_sms_fragment_limit, cumulative, previous_cumulative,
             [billing_month for billing_month in sms_units if billing_month['month'] == month]
         )
-        letter_billing = [(x['billing_units'], x['rate'], (x['billing_units'] * x['rate']))
+        letter_billing = [(x['billing_units'], x['rate'], (x['billing_units'] * x['rate']), x['postage'])
                           for x in letter_units if x['month'] == month]
+
+        if letter_billing:
+            letter_billing.sort(key=lambda x: (x[3], x[1]))
+
         letter_total = 0
         for x in letter_billing:
             letter_total += x[2]
diff --git a/app/templates/views/usage-with-letters.html b/app/templates/views/usage-with-letters.html
index 7d26976d6..ebda8c463 100644
--- a/app/templates/views/usage-with-letters.html
+++ b/app/templates/views/usage-with-letters.html
@@ -113,7 +113,7 @@
              <ul>
               {% for letter in month.letters%}
                 {% if letter[0] %}
-                <li class="tabular-numbers">{{ "{:,}".format(letter[0])}} letters at {{ '{:.0f}p'.format(letter[1] * 100) }}
+                <li class="tabular-numbers">{{ "{:,} {}".format(letter[0], letter[3])}} class letters at {{ '{:.0f}p'.format(letter[1] * 100) }}
                 </li>
                 {% endif %}
               {% endfor %}
diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py
index 0cac648ea..dbdd4604d 100644
--- a/tests/app/main/views/test_dashboard.py
+++ b/tests/app/main/views/test_dashboard.py
@@ -808,8 +808,35 @@ def test_usage_page_with_letters(
     assert '140 free text messages' in table
     assert '£20.30' in table
     assert '1,230 text messages at 1.65p' in table
-    assert '10 letters at 31p' in table
-    assert '5 letters at 33p' in table
+    assert '10 second class letters at 31p' in table
+    assert '5 first class letters at 33p' in table
+
+
+@freeze_time("2012-04-30 12:12:12")
+def test_usage_page_displays_letters_ordered_by_postage(
+    mocker,
+    logged_in_client,
+    service_one,
+    mock_get_usage,
+    mock_get_free_sms_fragment_limit
+):
+    billable_units_resp = [
+        {'month': 'April', 'notification_type': 'letter', 'rate': 0.5, 'billing_units': 1, 'postage': 'second'},
+        {'month': 'April', 'notification_type': 'letter', 'rate': 0.3, 'billing_units': 3, 'postage': 'second'},
+        {'month': 'April', 'notification_type': 'letter', 'rate': 0.5, 'billing_units': 1, 'postage': 'first'},
+    ]
+    mocker.patch('app.billing_api_client.get_billable_units_ft', return_value=billable_units_resp)
+    service_one['permissions'].append('letter')
+    response = logged_in_client.get(url_for('main.usage', service_id=SERVICE_ONE_ID))
+
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    row_for_april = page.find('table').find('tr', class_='table-row')
+    postage_details = row_for_april.find_all('li', class_='tabular-numbers')
+
+    assert len(postage_details) == 3
+    assert normalize_spaces(postage_details[0].text) == '1 first class letters at 50p'
+    assert normalize_spaces(postage_details[1].text) == '3 second class letters at 30p'
+    assert normalize_spaces(postage_details[2].text) == '1 second class letters at 50p'
 
 
 def test_usage_page_with_year_argument(

From 0d842d3700bba64b85d4f66404940de979cd54cc Mon Sep 17 00:00:00 2001
From: Katie Smith <katie.smith@digital.cabinet-office.gov.uk>
Date: Wed, 3 Oct 2018 15:15:19 +0100
Subject: [PATCH 3/3] Pluralize notification type correctly on usage page

This change makes sure that we display '1 letter' instead of '1 letters',
and '1 text message' instead of '1 text messages' on the usage page.
---
 app/templates/views/usage-with-letters.html | 9 +++++----
 app/templates/views/usage.html              | 7 ++++---
 tests/app/main/views/test_dashboard.py      | 8 ++++----
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/app/templates/views/usage-with-letters.html b/app/templates/views/usage-with-letters.html
index ebda8c463..ea5e06678 100644
--- a/app/templates/views/usage-with-letters.html
+++ b/app/templates/views/usage-with-letters.html
@@ -1,4 +1,5 @@
 {% from "components/big-number.html" import big_number %}
+{% from "components/message-count-label.html" import message_count_label %}
 {% from "components/table.html" import list_table, field, hidden_field_heading, row_heading, text_field %}
 {% from "components/pill.html" import pill %}
 
@@ -100,10 +101,10 @@
               ) }}
           <ul>
               {% if month.free %}
-                <li class="tabular-numbers">{{ "{:,}".format(month.free) }} free text messages</li>
+                <li class="tabular-numbers">{{ "{:,}".format(month.free) }} free {{ message_count_label(month.free, 'sms', '') }}</li>
               {% endif %}
               {% if month.paid %}
-                <li class="tabular-numbers">{{ "{:,}".format(month.paid) }} text messages at
+                <li class="tabular-numbers">{{ "{:,}".format(month.paid) }} {{ message_count_label(month.paid, 'sms', '') }}at
                 {{- ' {:.2f}p'.format(sms_rate * 100) }}</li>
               {% endif %}
               {% if not (month.free or month.paid) %}
@@ -113,8 +114,8 @@
              <ul>
               {% for letter in month.letters%}
                 {% if letter[0] %}
-                <li class="tabular-numbers">{{ "{:,} {}".format(letter[0], letter[3])}} class letters at {{ '{:.0f}p'.format(letter[1] * 100) }}
-                </li>
+                  <li class="tabular-numbers">{{ "{:,} {}".format(letter[0], letter[3])}} class {{ message_count_label(letter[0], 'letter', '') }}at
+                  {{ '{:.0f}p'.format(letter[1] * 100) }}</li>
                 {% endif %}
               {% endfor %}
             </ul>
diff --git a/app/templates/views/usage.html b/app/templates/views/usage.html
index 85fbf32c3..f85bd07c4 100644
--- a/app/templates/views/usage.html
+++ b/app/templates/views/usage.html
@@ -1,4 +1,5 @@
 {% from "components/big-number.html" import big_number %}
+{% from "components/message-count-label.html" import message_count_label %}
 {% from "components/table.html" import list_table, field, hidden_field_heading, row_heading, text_field %}
 {% from "components/pill.html" import pill %}
 
@@ -83,10 +84,10 @@
               ) }}
               <ul>
               {% if month.free %}
-                <li class="tabular-numbers">{{ "{:,}".format(month.free) }} free text messages</li>
+                <li class="tabular-numbers">{{ "{:,}".format(month.free) }} free {{ message_count_label(month.free, 'sms', '') }}</li>
               {% endif %}
               {% if month.paid %}
-                <li class="tabular-numbers">{{ "{:,}".format(month.paid) }} text messages at
+                <li class="tabular-numbers">{{ "{:,}".format(month.paid) }} {{ message_count_label(month.free, 'sms', '') }}at
                 {{- ' {:.2f}p'.format(sms_rate * 100) }}</li>
               {% endif %}
               {% if not (month.free or month.paid) %}
@@ -113,4 +114,4 @@
       </div>
     </div>
 
-{% endblock %}
\ No newline at end of file
+{% endblock %}
diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py
index dbdd4604d..862206efb 100644
--- a/tests/app/main/views/test_dashboard.py
+++ b/tests/app/main/views/test_dashboard.py
@@ -808,8 +808,8 @@ def test_usage_page_with_letters(
     assert '140 free text messages' in table
     assert '£20.30' in table
     assert '1,230 text messages at 1.65p' in table
-    assert '10 second class letters at 31p' in table
-    assert '5 first class letters at 33p' in table
+    assert '10 second class letters at 31p' in normalize_spaces(table)
+    assert '5 first class letters at 33p' in normalize_spaces(table)
 
 
 @freeze_time("2012-04-30 12:12:12")
@@ -834,9 +834,9 @@ def test_usage_page_displays_letters_ordered_by_postage(
     postage_details = row_for_april.find_all('li', class_='tabular-numbers')
 
     assert len(postage_details) == 3
-    assert normalize_spaces(postage_details[0].text) == '1 first class letters at 50p'
+    assert normalize_spaces(postage_details[0].text) == '1 first class letter at 50p'
     assert normalize_spaces(postage_details[1].text) == '3 second class letters at 30p'
-    assert normalize_spaces(postage_details[2].text) == '1 second class letters at 50p'
+    assert normalize_spaces(postage_details[2].text) == '1 second class letter at 50p'
 
 
 def test_usage_page_with_year_argument(