Merge pull request #2433 from alphagov/billing-fix

fix create_nightly_billing date bug

app/dao/fact_billing_dao.py

@@ -21,6 +21,7 @@ from app.models import (
     EMAIL_TYPE,
     NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS
 )
+from app.utils import get_london_midnight_in_utc
 
 
 def fetch_billing_totals_for_year(service_id, year):
@@ -209,10 +210,8 @@ def fetch_billing_data_for_day(process_day, service_id=None):
 
 
 def get_rates_for_billing():
-    non_letter_rates = [(r.notification_type, r.valid_from, r.rate) for r in
+    non_letter_rates = Rate.query.order_by(desc(Rate.valid_from)).all()
-                        Rate.query.order_by(desc(Rate.valid_from)).all()]
+    letter_rates = LetterRate.query.order_by(desc(LetterRate.start_date)).all()
-    letter_rates = [(r.start_date, r.crown, r.sheet_count, r.rate, r.post_class) for r in
-                    LetterRate.query.order_by(desc(LetterRate.start_date)).all()]
     return non_letter_rates, letter_rates
 
 
@@ -230,15 +229,28 @@ def get_service_ids_that_need_billing_populated(start_date, end_date):
 def get_rate(
     non_letter_rates, letter_rates, notification_type, date, crown=None, letter_page_count=None, post_class='second'
 ):
+    start_of_day = get_london_midnight_in_utc(date)
+
     if notification_type == LETTER_TYPE:
         if letter_page_count == 0:
             return 0
         return next(
-            r[3] for r in letter_rates if date >= r[0] and crown == r[1]
+            r.rate
-            and letter_page_count == r[2] and post_class == r[4]
+            for r in letter_rates if (
+                start_of_day >= r.start_date and
+                crown == r.crown and
+                letter_page_count == r.sheet_count and
+                post_class == r.post_class
+            )
         )
     elif notification_type == SMS_TYPE:
-        return next(r[2] for r in non_letter_rates if notification_type == r[0] and date >= r[1])
+        return next(
+            r.rate
+            for r in non_letter_rates if (
+                notification_type == r.notification_type and
+                start_of_day >= r.valid_from
+            )
+        )
     else:
         return 0
 

tests/app/celery/test_ftp_update_tasks.py

@@ -276,9 +276,10 @@ def test_record_daily_sorted_counts_raises_dvla_exception_with_unknown_sorted_st
     filename = "failed.txt"
     with pytest.raises(DVLAException) as e:
         record_daily_sorted_counts(filename=filename)
-    unknown_values = set({'invalid', 'mm'})
+
-    assert "DVLA response file: {} contains unknown Sorted status {}".format(
+    assert "DVLA response file: {} contains unknown Sorted status".format(filename) in e.value.message
-        filename, unknown_values.__repr__()) == e.value.message
+    assert "'mm'" in e.value.message
+    assert "'invalid'" in e.value.message
 
 
 def test_record_daily_sorted_counts_persists_daily_sorted_letter_count_with_no_sorted_values(

tests/app/celery/test_reporting_tasks.py

@@ -3,7 +3,6 @@ from decimal import Decimal
 
 import pytest
 from freezegun import freeze_time
-from sqlalchemy import desc
 
 from app.celery.reporting_tasks import create_nightly_billing, create_nightly_notification_status
 from app.dao.fact_billing_dao import get_rate
@@ -14,14 +13,12 @@ from app.models import (
     EMAIL_TYPE,
     SMS_TYPE, FactNotificationStatus
 )
-from app.models import LetterRate, Rate
-from app import db
 
-from tests.app.db import create_service, create_template, create_notification
+from tests.app.db import create_service, create_template, create_notification, create_rate, create_letter_rate
 
 
 def mocker_get_rate(
-    non_letter_rates, letter_rates, notification_type, date, crown=None, rate_multiplier=None, post_class="second"
+    non_letter_rates, letter_rates, notification_type, bst_date, crown=None, rate_multiplier=None, post_class="second"
 ):
     if notification_type == LETTER_TYPE:
         return Decimal(2.1)
@@ -322,38 +319,26 @@ def test_create_nightly_billing_consolidate_from_3_days_delta(
 
 
 def test_get_rate_for_letter_latest(notify_db_session):
-    non_letter_rates = [(r.notification_type, r.valid_from, r.rate) for r in
-                        Rate.query.order_by(desc(Rate.valid_from)).all()]
-
     # letter rates should be passed into the get_rate function as a tuple of start_date, crown, sheet_count,
     # rate and post_class
-    new_letter_rate = (datetime(2017, 12, 1), True, 1, Decimal(0.33), 'second')
+    new = create_letter_rate(datetime(2017, 12, 1), crown=True, sheet_count=1, rate=0.33, post_class='second')
-    old_letter_rate = (datetime(2016, 12, 1), True, 1, Decimal(0.30), 'second')
+    old = create_letter_rate(datetime(2016, 12, 1), crown=True, sheet_count=1, rate=0.30, post_class='second')
-    letter_rates = [new_letter_rate, old_letter_rate]
+    letter_rates = [new, old]
 
-    rate = get_rate(non_letter_rates, letter_rates, LETTER_TYPE, datetime(2018, 1, 1), True, 1)
+    rate = get_rate([], letter_rates, LETTER_TYPE, date(2018, 1, 1), True, 1)
-    assert rate == Decimal(0.33)
+    assert rate == Decimal('0.33')
 
 
 def test_get_rate_for_sms_and_email(notify_db_session):
-    sms_rate = Rate(valid_from=datetime(2017, 12, 1),
+    non_letter_rates = [
-                    rate=Decimal(0.15),
+        create_rate(datetime(2017, 12, 1), 0.15, SMS_TYPE),
-                    notification_type=SMS_TYPE)
+        create_rate(datetime(2017, 12, 1), 0, EMAIL_TYPE)
-    db.session.add(sms_rate)
+    ]
-    email_rate = Rate(valid_from=datetime(2017, 12, 1),
-                      rate=Decimal(0),
-                      notification_type=EMAIL_TYPE)
-    db.session.add(email_rate)
 
-    non_letter_rates = [(r.notification_type, r.valid_from, r.rate) for r in
+    rate = get_rate(non_letter_rates, [], SMS_TYPE, date(2018, 1, 1))
-                        Rate.query.order_by(desc(Rate.valid_from)).all()]
-    letter_rates = [(r.start_date, r.crown, r.sheet_count, r.rate) for r in
-                    LetterRate.query.order_by(desc(LetterRate.start_date)).all()]
-
-    rate = get_rate(non_letter_rates, letter_rates, SMS_TYPE, datetime(2018, 1, 1))
     assert rate == Decimal(0.15)
 
-    rate = get_rate(non_letter_rates, letter_rates, EMAIL_TYPE, datetime(2018, 1, 1))
+    rate = get_rate(non_letter_rates, [], EMAIL_TYPE, date(2018, 1, 1))
     assert rate == Decimal(0)
 
 

tests/app/dao/test_ft_billing_dao.py

@@ -1,7 +1,7 @@
 from calendar import monthrange
 from decimal import Decimal
 
-from datetime import datetime, timedelta
+from datetime import datetime, timedelta, date
 from freezegun import freeze_time
 
 import pytest
@@ -28,6 +28,7 @@ from tests.app.db import (
     create_template,
     create_notification,
     create_rate,
+    create_letter_rate
 )
 
 
@@ -95,7 +96,7 @@ def test_fetch_billing_data_for_today_includes_data_with_the_right_key_type(noti
 
 
 def test_fetch_billing_data_for_today_includes_data_with_the_right_date(notify_db_session):
-    process_day = datetime(2018, 4, 1, 13, 30, 00)
+    process_day = datetime(2018, 4, 1, 13, 30, 0)
     service = create_service()
     template = create_template(service=service, template_type="email")
     create_notification(template=template, status='delivered', created_at=process_day)
@@ -282,24 +283,30 @@ def test_get_rates_for_billing(notify_db_session):
     create_rate(start_date=datetime.utcnow(), value=12, notification_type='email')
     create_rate(start_date=datetime.utcnow(), value=22, notification_type='sms')
     create_rate(start_date=datetime.utcnow(), value=33, notification_type='email')
+    create_letter_rate(start_date=datetime.utcnow(), rate=0.66, post_class='first')
+    create_letter_rate(start_date=datetime.utcnow(), rate=0.33, post_class='second')
     non_letter_rates, letter_rates = get_rates_for_billing()
 
     assert len(non_letter_rates) == 3
-    assert len(letter_rates) == 29
+    assert len(letter_rates) == 2
 
 
+@freeze_time('2017-06-01 12:00')
 def test_get_rate(notify_db_session):
-    create_rate(start_date=datetime.utcnow(), value=1.2, notification_type='email')
+    create_rate(start_date=datetime(2017, 5, 30, 23, 0), value=1.2, notification_type='email')
-    create_rate(start_date=datetime.utcnow(), value=2.2, notification_type='sms')
+    create_rate(start_date=datetime(2017, 5, 30, 23, 0), value=2.2, notification_type='sms')
-    create_rate(start_date=datetime.utcnow(), value=3.3, notification_type='email')
+    create_rate(start_date=datetime(2017, 5, 30, 23, 0), value=3.3, notification_type='email')
+    create_letter_rate(start_date=datetime(2017, 5, 30, 23, 0), rate=0.66, post_class='first')
+    create_letter_rate(start_date=datetime(2017, 5, 30, 23, 0), rate=0.3, post_class='second')
+
     non_letter_rates, letter_rates = get_rates_for_billing()
     rate = get_rate(non_letter_rates=non_letter_rates, letter_rates=letter_rates, notification_type='sms',
-                    date=datetime.utcnow())
+                    date=date(2017, 6, 1))
     letter_rate = get_rate(non_letter_rates=non_letter_rates, letter_rates=letter_rates,
                            notification_type='letter',
                            crown=True,
                            letter_page_count=1,
-                           date=datetime.utcnow())
+                           date=date(2017, 6, 1))
 
     assert rate == 2.2
     assert letter_rate == Decimal('0.3')
@@ -307,6 +314,9 @@ def test_get_rate(notify_db_session):
 
 @pytest.mark.parametrize("letter_post_class,expected_rate", [("first", "0.61"), ("second", "0.35")])
 def test_get_rate_filters_letters_by_post_class(notify_db_session, letter_post_class, expected_rate):
+    create_letter_rate(start_date=datetime(2017, 5, 30, 23, 0), sheet_count=2, rate=0.61, post_class='first')
+    create_letter_rate(start_date=datetime(2017, 5, 30, 23, 0), sheet_count=2, rate=0.35, post_class='second')
+
     non_letter_rates, letter_rates = get_rates_for_billing()
     rate = get_rate(non_letter_rates, letter_rates, "letter", datetime(2018, 10, 1), True, 2, letter_post_class)
     assert rate == Decimal(expected_rate)
@@ -314,6 +324,9 @@ def test_get_rate_filters_letters_by_post_class(notify_db_session, letter_post_c
 
 @pytest.mark.parametrize("date,expected_rate", [(datetime(2018, 9, 30), '0.33'), (datetime(2018, 10, 1), '0.35')])
 def test_get_rate_chooses_right_rate_depending_on_date(notify_db_session, date, expected_rate):
+    create_letter_rate(start_date=datetime(2016, 1, 1, 0, 0), sheet_count=2, rate=0.33, post_class='second')
+    create_letter_rate(start_date=datetime(2018, 9, 30, 23, 0), sheet_count=2, rate=0.35, post_class='second')
+
     non_letter_rates, letter_rates = get_rates_for_billing()
     rate = get_rate(non_letter_rates, letter_rates, "letter", date, True, 2, "second")
     assert rate == Decimal(expected_rate)

tests/app/db.py

@@ -29,6 +29,7 @@ from app.models import (
     Job,
     Notification,
     EmailBranding,
+    LetterRate,
     Organisation,
     Rate,
     Service,
@@ -403,6 +404,23 @@ def create_rate(start_date, value, notification_type):
     return rate
 
 
+def create_letter_rate(start_date=None, end_date=None, crown=True, sheet_count=1, rate=0.33, post_class='second'):
+    if start_date is None:
+        start_date = datetime(2016, 1, 1)
+    rate = LetterRate(
+        id=uuid.uuid4(),
+        start_date=start_date,
+        end_date=end_date,
+        crown=crown,
+        sheet_count=sheet_count,
+        rate=rate,
+        post_class=post_class
+    )
+    db.session.add(rate)
+    db.session.commit()
+    return rate
+
+
 def create_api_key(service, key_type=KEY_TYPE_NORMAL):
     id_ = uuid.uuid4()
     api_key = ApiKey(

tests/conftest.py

@@ -105,7 +105,6 @@ def notify_db_session(notify_db):
                             "service_permission_types",
                             "auth_type",
                             "invite_status_type",
-                            "letter_rates",
                             "service_callback_type"]:
             notify_db.engine.execute(tbl.delete())
     notify_db.session.commit()