From 1fce30aaa74eb308f3f939d5efa0b16b0a2a952b Mon Sep 17 00:00:00 2001
From: Paul Craig <paul_craig_16@hotmail.com>
Date: Thu, 24 Nov 2016 10:33:38 +0000
Subject: [PATCH] Cost savings

The "cost" value was flawed for a couple of reasons.

1. Lots of messages are free, so in those instances the "cost"
   doesn't tell you anything
2. The query to get the rate was expensive and we don't have
   an obvious way to get it back very efficiently for large numbers
   of notifications.

So we scrapped it.
---
 app/models.py                                 | 18 +--------
 app/v2/notifications/notification_schemas.py  |  4 +-
 tests/app/test_model.py                       | 37 -------------------
 .../notifications/test_get_notifications.py   |  1 -
 4 files changed, 2 insertions(+), 58 deletions(-)

diff --git a/app/models.py b/app/models.py
index b880ccf40..119341265 100644
--- a/app/models.py
+++ b/app/models.py
@@ -6,7 +6,7 @@ from sqlalchemy.dialects.postgresql import (
     UUID,
     JSON
 )
-from sqlalchemy import UniqueConstraint, and_, desc
+from sqlalchemy import UniqueConstraint, and_
 from sqlalchemy.orm import foreign, remote
 from notifications_utils.recipients import (
     validate_email_address,
@@ -544,21 +544,6 @@ class Notification(db.Model):
         if personalisation:
             self._personalisation = encryption.encrypt(personalisation)
 
-    def cost(self):
-        if not self.sent_by or self.billable_units == 0:
-            return 0
-
-        provider_rate = db.session.query(
-            ProviderRates
-        ).join(ProviderDetails).filter(
-            ProviderDetails.identifier == self.sent_by,
-            ProviderRates.provider_id == ProviderDetails.id
-        ).order_by(
-            desc(ProviderRates.valid_from)
-        ).limit(1).one()
-
-        return float(provider_rate.rate * self.billable_units)
-
     def completed_at(self):
         if self.status in [
             NOTIFICATION_DELIVERED,
@@ -591,7 +576,6 @@ class Notification(db.Model):
             "line_5": None,
             "line_6": None,
             "postcode": None,
-            "cost": self.cost(),
             "type": self.notification_type,
             "status": self.status,
             "template": template_dict,
diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py
index 4dfaee7b1..272800d5c 100644
--- a/app/v2/notifications/notification_schemas.py
+++ b/app/v2/notifications/notification_schemas.py
@@ -57,7 +57,6 @@ get_notification_response = {
         "line_5": {"type": ["string", "null"]},
         "line_6": {"type": ["string", "null"]},
         "postcode": {"type": ["string", "null"]},
-        "cost": {"type": "number"},
         "type": {"enum": ["sms", "letter", "email"]},
         "status": {"type": "string"},
         "template": template,
@@ -69,8 +68,7 @@ get_notification_response = {
         # technically, all keys are required since we always have all of them
         "id", "reference", "email_address", "phone_number",
         "line_1", "line_2", "line_3", "line_4", "line_5", "line_6", "postcode",
-        "cost", "type", "status", "template",
-        "created_at", "sent_at", "completed_at"
+        "type", "status", "template", "created_at", "sent_at", "completed_at"
     ]
 }
 
diff --git a/tests/app/test_model.py b/tests/app/test_model.py
index f60be4f08..ca6d11f9a 100644
--- a/tests/app/test_model.py
+++ b/tests/app/test_model.py
@@ -1,8 +1,5 @@
 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 (
     ServiceWhitelist,
     MOBILE_TYPE, EMAIL_TYPE)
@@ -35,37 +32,3 @@ def test_should_build_service_whitelist_from_email_address(email_address):
 def test_should_not_build_service_whitelist_from_invalid_contact(recipient_type, contact):
     with pytest.raises(ValueError):
         ServiceWhitelist.from_string('service_id', recipient_type, contact)
-
-
-@pytest.mark.parametrize('provider, billable_units, expected_cost', [
-    ('mmg', 1, 3.5),
-    ('firetext', 2, 0.025),
-    ('ses', 0, 0)
-])
-def test_calculate_cost_from_notification_billable_units(
-        notify_db, notify_db_session, provider, billable_units, expected_cost
-):
-    provider_rates = [
-        ('mmg', datetime(2016, 7, 1), 1.5),
-        ('firetext', datetime(2016, 7, 1), 0.0125),
-        ('mmg', datetime.utcnow(), 3.5),
-    ]
-    for provider_identifier, valid_from, rate in provider_rates:
-        sample_provider_rate(
-            notify_db,
-            notify_db_session,
-            provider_identifier=provider_identifier,
-            valid_from=valid_from,
-            rate=rate
-        )
-
-    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()
diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py
index 555490638..e042c3e33 100644
--- a/tests/app/v2/notifications/test_get_notifications.py
+++ b/tests/app/v2/notifications/test_get_notifications.py
@@ -46,7 +46,6 @@ def test_get_notification_by_id_returns_200(
         'line_5': None,
         'line_6': None,
         'postcode': None,
-        'cost': sample_notification.cost(),
         'type': '{}'.format(sample_notification.notification_type),
         'status': '{}'.format(sample_notification.status),
         'template': expected_template_response,