From 9b6584c8a1c0e5c51ae25d4a4c36bc0554037f48 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Tue, 22 Nov 2016 16:34:53 +0000 Subject: [PATCH 1/3] Make result of `notification.cost()` into a float Cost was returning a `Decimal`, which jsonify doesn't like. Making it a float fixes the problem. [Relevant issue on github](https://github.com/pallets/flask/issues/835). --- app/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models.py b/app/models.py index eaebdfc03..b880ccf40 100644 --- a/app/models.py +++ b/app/models.py @@ -557,7 +557,7 @@ class Notification(db.Model): desc(ProviderRates.valid_from) ).limit(1).one() - return provider_rate.rate * self.billable_units + return float(provider_rate.rate * self.billable_units) def completed_at(self): if self.status in [ From a17adb470709ec7afdb3d679c98fdcb77f273ca2 Mon Sep 17 00:00:00 2001 From: Paul Craig Date: Tue, 22 Nov 2016 17:47:56 +0000 Subject: [PATCH 2/3] New test for `notification.cost()` method Added a test that asking for a nonexistent provider_rate blows everything up. Also updated existing test to use a weirder number. --- tests/app/test_model.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 1625d7119..f60be4f08 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -1,9 +1,9 @@ import pytest from datetime import datetime +from sqlalchemy.orm.exc import NoResultFound from tests.app.conftest import sample_notification, sample_provider_rate from app.models import ( - Notification, ServiceWhitelist, MOBILE_TYPE, EMAIL_TYPE) @@ -39,7 +39,7 @@ def test_should_not_build_service_whitelist_from_invalid_contact(recipient_type, @pytest.mark.parametrize('provider, billable_units, expected_cost', [ ('mmg', 1, 3.5), - ('firetext', 2, 5), + ('firetext', 2, 0.025), ('ses', 0, 0) ]) def test_calculate_cost_from_notification_billable_units( @@ -47,7 +47,7 @@ def test_calculate_cost_from_notification_billable_units( ): provider_rates = [ ('mmg', datetime(2016, 7, 1), 1.5), - ('firetext', datetime(2016, 7, 1), 2.5), + ('firetext', datetime(2016, 7, 1), 0.0125), ('mmg', datetime.utcnow(), 3.5), ] for provider_identifier, valid_from, rate in provider_rates: @@ -61,3 +61,11 @@ def test_calculate_cost_from_notification_billable_units( notification = sample_notification(notify_db, notify_db_session, billable_units=billable_units, sent_by=provider) assert notification.cost() == expected_cost + + +def test_billable_units_without_provider_rates_entry_raises_exception( + notify_db, notify_db_session, sample_provider_rate +): + notification = sample_notification(notify_db, notify_db_session, sent_by='not_a_provider') + with pytest.raises(NoResultFound): + notification.cost() From 92897e20a82bb469efe6832a5f4437ed4d54d847 Mon Sep 17 00:00:00 2001 From: Paul Craig Date: Tue, 22 Nov 2016 17:50:29 +0000 Subject: [PATCH 3/3] Test returning a notification with a non-zero cost Our previous test ws returning a notification without a `sent_by` attribute, which meant that cost was always 0. Unfortunately, this meant that returning a real value for cost was untested and (whaddya know) it broke immediately. Old test scenario: - billable_units=1, sent_by=None, cost=0 New scenarios - billable_units=0, sent_by='mmg', cost=0 - billable_units=1, sent_by='mmg', cost=1 --- .../v2/notifications/test_get_notifications.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index a9c583b5b..555490638 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -1,12 +1,24 @@ import json +import pytest from app import DATETIME_FORMAT from tests import create_authorization_header +from tests.app.conftest import sample_notification as create_sample_notification -def test_get_notification_by_id_returns_200(client, sample_notification): +@pytest.mark.parametrize('billable_units, provider', [ + (1, 'mmg'), + (0, 'mmg'), + (1, None) +]) +def test_get_notification_by_id_returns_200( + client, notify_db, notify_db_session, sample_provider_rate, billable_units, provider +): + sample_notification = create_sample_notification( + notify_db, notify_db_session, billable_units=billable_units, sent_by=provider + ) + auth_header = create_authorization_header(service_id=sample_notification.service_id) - response = client.get( path='/v2/notifications/{}'.format(sample_notification.id), headers=[('Content-Type', 'application/json'), auth_header])