From c5d729b5f2c894a00005dde52e4d1f447f8ffa01 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 25 Apr 2022 11:11:45 +0100 Subject: [PATCH 1/4] Remove unused `letter_cumulative` field I have searched for this in the code and can't see it being used anywhere so have removed it! --- app/main/views/dashboard.py | 3 --- tests/app/main/views/test_dashboard.py | 24 ++++++++++++------------ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 90de7ae15..ac4ff69db 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -406,7 +406,6 @@ def get_sum_billing_units(billing_units, month=None): def get_free_paid_breakdown_for_billable_units(year, free_sms_fragment_limit, billing_units): cumulative = 0 - letter_cumulative = 0 sms_units = [x for x in billing_units if x['notification_type'] == 'sms'] letter_units = [x for x in billing_units if x['notification_type'] == 'letter'] for month in get_months_for_financial_year(year): @@ -424,11 +423,9 @@ def get_free_paid_breakdown_for_billable_units(year, free_sms_fragment_limit, bi letter_total = 0 for x in letter_billing: letter_total += x.cost - letter_cumulative += letter_total yield { 'name': month, 'letter_total': letter_total, - 'letter_cumulative': letter_cumulative, 'paid': breakdown['paid'], 'free': breakdown['free'], 'letters': letter_billing diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 8939ae9d3..d61101f96 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -1595,18 +1595,18 @@ def test_get_free_paid_breakdown_for_billable_units(now, expected_number_of_mont ] ) assert list(billing_units) == [ - {'free': 100000, 'name': 'April', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 100000, 'name': 'May', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 50000, 'name': 'June', 'paid': 50000, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 0, 'name': 'July', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 0, 'name': 'August', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 0, 'name': 'September', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 0, 'name': 'October', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 0, 'name': 'November', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 0, 'name': 'December', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 0, 'name': 'January', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 0, 'name': 'February', 'paid': 2000, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0}, - {'free': 0, 'name': 'March', 'paid': 0, 'letter_total': 0, 'letters': [], 'letter_cumulative': 0} + {'free': 100000, 'name': 'April', 'paid': 0, 'letter_total': 0, 'letters': []}, + {'free': 100000, 'name': 'May', 'paid': 0, 'letter_total': 0, 'letters': []}, + {'free': 50000, 'name': 'June', 'paid': 50000, 'letter_total': 0, 'letters': []}, + {'free': 0, 'name': 'July', 'paid': 0, 'letter_total': 0, 'letters': []}, + {'free': 0, 'name': 'August', 'paid': 0, 'letter_total': 0, 'letters': []}, + {'free': 0, 'name': 'September', 'paid': 0, 'letter_total': 0, 'letters': []}, + {'free': 0, 'name': 'October', 'paid': 0, 'letter_total': 0, 'letters': []}, + {'free': 0, 'name': 'November', 'paid': 0, 'letter_total': 0, 'letters': []}, + {'free': 0, 'name': 'December', 'paid': 0, 'letter_total': 0, 'letters': []}, + {'free': 0, 'name': 'January', 'paid': 0, 'letter_total': 0, 'letters': []}, + {'free': 0, 'name': 'February', 'paid': 2000, 'letter_total': 0, 'letters': []}, + {'free': 0, 'name': 'March', 'paid': 0, 'letter_total': 0, 'letters': []} ][:expected_number_of_months] From d18c787a0207f19101b9e174c2233566aa9af8c1 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 25 Apr 2022 11:27:03 +0100 Subject: [PATCH 2/4] Give better names to usage page variables `free` becomes `sms_free_count` `paid` becomes `sms_paid_count` This small change is just to help with readability for this complex area of code --- app/main/views/dashboard.py | 6 +++--- app/templates/views/usage.html | 12 ++++++------ tests/app/main/views/test_dashboard.py | 24 ++++++++++++------------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index ac4ff69db..6c8fe6af5 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -426,9 +426,9 @@ def get_free_paid_breakdown_for_billable_units(year, free_sms_fragment_limit, bi yield { 'name': month, 'letter_total': letter_total, - 'paid': breakdown['paid'], - 'free': breakdown['free'], - 'letters': letter_billing + 'letters': letter_billing, + 'sms_paid_count': breakdown['paid'], + 'sms_free_count': breakdown['free'], } diff --git a/app/templates/views/usage.html b/app/templates/views/usage.html index 43f7ce51a..26b63e559 100644 --- a/app/templates/views/usage.html +++ b/app/templates/views/usage.html @@ -97,16 +97,16 @@ {% endcall %} {% call field(align='left') %} {{ big_number( - (sms_rate * month.paid) + month.letter_total, + (sms_rate * month.sms_paid_count) + month.letter_total, currency="£", smallest=True ) }} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index d61101f96..dfafc6b29 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -1595,18 +1595,18 @@ def test_get_free_paid_breakdown_for_billable_units(now, expected_number_of_mont ] ) assert list(billing_units) == [ - {'free': 100000, 'name': 'April', 'paid': 0, 'letter_total': 0, 'letters': []}, - {'free': 100000, 'name': 'May', 'paid': 0, 'letter_total': 0, 'letters': []}, - {'free': 50000, 'name': 'June', 'paid': 50000, 'letter_total': 0, 'letters': []}, - {'free': 0, 'name': 'July', 'paid': 0, 'letter_total': 0, 'letters': []}, - {'free': 0, 'name': 'August', 'paid': 0, 'letter_total': 0, 'letters': []}, - {'free': 0, 'name': 'September', 'paid': 0, 'letter_total': 0, 'letters': []}, - {'free': 0, 'name': 'October', 'paid': 0, 'letter_total': 0, 'letters': []}, - {'free': 0, 'name': 'November', 'paid': 0, 'letter_total': 0, 'letters': []}, - {'free': 0, 'name': 'December', 'paid': 0, 'letter_total': 0, 'letters': []}, - {'free': 0, 'name': 'January', 'paid': 0, 'letter_total': 0, 'letters': []}, - {'free': 0, 'name': 'February', 'paid': 2000, 'letter_total': 0, 'letters': []}, - {'free': 0, 'name': 'March', 'paid': 0, 'letter_total': 0, 'letters': []} + {'sms_free_count': 100000, 'name': 'April', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 100000, 'name': 'May', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 50000, 'name': 'June', 'sms_paid_count': 50000, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 0, 'name': 'July', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 0, 'name': 'August', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 0, 'name': 'September', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 0, 'name': 'October', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 0, 'name': 'November', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 0, 'name': 'December', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 0, 'name': 'January', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 0, 'name': 'February', 'sms_paid_count': 2000, 'letter_total': 0, 'letters': []}, + {'sms_free_count': 0, 'name': 'March', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []} ][:expected_number_of_months] From 2706ec4c73ec22701b4f9f1dd4aa44cc020ba64a Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 25 Apr 2022 11:48:50 +0100 Subject: [PATCH 3/4] Take sms_rate from monthly usage data At the moment, we put the sms rate on the usage page for each months billing data by taking the single sms rate for the year. The assumption that there will be a single sms rate for the year is no longer going to be true. Therefore, instead we take the sms rate from the monthly data itself which tells us the rate for a months worth of sent SMS. --- app/main/views/dashboard.py | 9 ++ app/templates/views/usage.html | 4 +- tests/app/main/views/test_dashboard.py | 112 +++++++++++++++++++++---- 3 files changed, 109 insertions(+), 16 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 6c8fe6af5..1d143ec31 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -429,6 +429,7 @@ def get_free_paid_breakdown_for_billable_units(year, free_sms_fragment_limit, bi 'letters': letter_billing, 'sms_paid_count': breakdown['paid'], 'sms_free_count': breakdown['free'], + 'sms_rate': breakdown['sms_rate'], } @@ -476,23 +477,31 @@ def get_free_paid_breakdown_for_month( ): allowance = free_sms_fragment_limit + # makes the assumption that there is either no item in `monthly_usage` because they have not sent any SMS + # or that they have sent SMS and that there is only a single item in `monthly_usage` because they have only + # been sent at a single rate during the month + sms_rate = monthly_usage[0]['rate'] if len(monthly_usage) else 0 + total_monthly_billing_units = get_sum_billing_units(monthly_usage) if cumulative < allowance: return { 'paid': 0, 'free': total_monthly_billing_units, + 'sms_rate': sms_rate, } elif previous_cumulative < allowance: remaining_allowance = allowance - previous_cumulative return { 'paid': total_monthly_billing_units - remaining_allowance, 'free': remaining_allowance, + 'sms_rate': sms_rate, } else: return { 'paid': total_monthly_billing_units, 'free': 0, + 'sms_rate': sms_rate, } diff --git a/app/templates/views/usage.html b/app/templates/views/usage.html index 26b63e559..4479c97fa 100644 --- a/app/templates/views/usage.html +++ b/app/templates/views/usage.html @@ -97,7 +97,7 @@ {% endcall %} {% call field(align='left') %} {{ big_number( - (sms_rate * month.sms_paid_count) + month.letter_total, + (month.sms_rate * month.sms_paid_count) + month.letter_total, currency="£", smallest=True ) }} @@ -107,7 +107,7 @@ {% endif %} {% if month.sms_paid_count %}
  • {{ month.sms_paid_count|message_count('sms') }} at - {{- ' {:.2f}p'.format(sms_rate * 100) }}
    • + {{- ' {:.2f}p'.format(month.sms_rate * 100) }} {% endif %} {% for letter in month.letters%} {% if letter.billing_units %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index dfafc6b29..e69751cfc 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -1586,27 +1586,111 @@ def test_get_free_paid_breakdown_for_billable_units(now, expected_number_of_mont }, { 'month': 'June', 'international': False, 'rate_multiplier': 1, - 'notification_type': 'sms', 'rate': 1.65, 'billing_units': 100000 + 'notification_type': 'sms', 'rate': 1.71, 'billing_units': 100000 }, { 'month': 'February', 'international': False, 'rate_multiplier': 1, - 'notification_type': 'sms', 'rate': 1.65, 'billing_units': 2000 + 'notification_type': 'sms', 'rate': 1.71, 'billing_units': 2000 }, ] ) assert list(billing_units) == [ - {'sms_free_count': 100000, 'name': 'April', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 100000, 'name': 'May', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 50000, 'name': 'June', 'sms_paid_count': 50000, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 0, 'name': 'July', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 0, 'name': 'August', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 0, 'name': 'September', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 0, 'name': 'October', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 0, 'name': 'November', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 0, 'name': 'December', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 0, 'name': 'January', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 0, 'name': 'February', 'sms_paid_count': 2000, 'letter_total': 0, 'letters': []}, - {'sms_free_count': 0, 'name': 'March', 'sms_paid_count': 0, 'letter_total': 0, 'letters': []} + { + 'sms_free_count': 100000, + 'name': 'April', + 'sms_paid_count': 0, + 'sms_rate': 1.65, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 100000, + 'name': 'May', + 'sms_paid_count': 0, + 'sms_rate': 1.65, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 50000, + 'name': 'June', + 'sms_paid_count': 50000, + 'sms_rate': 1.71, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 0, + 'name': 'July', + 'sms_paid_count': 0, + 'sms_rate': 0, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 0, + 'name': 'August', + 'sms_paid_count': 0, + 'sms_rate': 0, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 0, + 'name': 'September', + 'sms_paid_count': 0, + 'sms_rate': 0, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 0, + 'name': 'October', + 'sms_paid_count': 0, + 'sms_rate': 0, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 0, + 'name': 'November', + 'sms_paid_count': 0, + 'sms_rate': 0, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 0, + 'name': 'December', + 'sms_paid_count': 0, + 'sms_rate': 0, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 0, + 'name': 'January', + 'sms_paid_count': 0, + 'sms_rate': 0, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 0, + 'name': 'February', + 'sms_paid_count': 2000, + 'sms_rate': 1.71, + 'letter_total': 0, + 'letters': [] + }, + { + 'sms_free_count': 0, + 'name': 'March', + 'sms_paid_count': 0, + 'sms_rate': 0, + 'letter_total': 0, + 'letters': [] + }, ][:expected_number_of_months] From a02e1adbc5846add9590b7db11e879e6a0c2167b Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Tue, 26 Apr 2022 14:02:54 +0100 Subject: [PATCH 4/4] Tweak usage dashboard test to cover rate changes --- tests/app/main/views/test_dashboard.py | 8 ++++---- tests/conftest.py | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index e69751cfc..de47ffde5 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -1022,8 +1022,8 @@ def test_usage_page( assert 'March' in table assert '£28.99' in table assert '140 free text messages' in table - assert '£20.30' in table - assert '1,230 text messages at 1.65p' in table + assert '£20.91' in table + assert '1,230 text messages at 1.70p' in table @freeze_time("2012-03-31 12:12:12") @@ -1064,8 +1064,8 @@ def test_usage_page_with_letters( assert 'March' in table assert '£28.99' in table assert '140 free text messages' in table - assert '£20.30' in table - assert '1,230 text messages at 1.65p' in table + assert '£20.91' in table + assert '1,230 text messages at 1.70p' in table assert '10 second class letters at 31p' in normalize_spaces(table) assert '5 first class letters at 33p' in normalize_spaces(table) assert '10 international letters at 84p' in normalize_spaces(table) diff --git a/tests/conftest.py b/tests/conftest.py index 84bebc261..f5e397870 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2334,56 +2334,56 @@ def mock_get_billable_units(mocker): { 'month': 'April', 'notification_type': 'sms', - 'rate': 0.0165, + 'rate': 0.017, 'billing_units': 249500, 'postage': 'none', }, { 'month': 'April', 'notification_type': 'sms', - 'rate': 0.0165, + 'rate': 0.017, 'billing_units': 100, 'postage': 'none', }, { 'month': 'April', 'notification_type': 'sms', - 'rate': 0.0165, + 'rate': 0.017, 'billing_units': 200, 'postage': 'none', }, { 'month': 'April', 'notification_type': 'sms', - 'rate': 0.0165, + 'rate': 0.017, 'billing_units': 60, 'postage': 'none', }, { 'month': 'March', 'notification_type': 'sms', - 'rate': 0.0165, + 'rate': 0.017, 'billing_units': 1000, 'postage': 'none', }, { 'month': 'March', 'notification_type': 'sms', - 'rate': 0.0165, + 'rate': 0.017, 'billing_units': 100, 'postage': 'none', }, { 'month': 'March', 'notification_type': 'sms', - 'rate': 0.0165, + 'rate': 0.017, 'billing_units': 100, 'postage': 'none', }, { 'month': 'March', 'notification_type': 'sms', - 'rate': 0.0165, + 'rate': 0.017, 'billing_units': 30, 'postage': 'none', },