make has_delete_task_run non-optional

just to ensure people think about the value of it when using the function
This commit is contained in:
Leo Hemsted
2019-08-22 17:24:51 +01:00
parent d83827579e
commit d457db4164
4 changed files with 18 additions and 20 deletions

View File

@@ -311,7 +311,7 @@ def fetch_billing_data_for_day(process_day, service_id=None):
for service in services: for service in services:
for notification_type in (SMS_TYPE, EMAIL_TYPE, LETTER_TYPE): for notification_type in (SMS_TYPE, EMAIL_TYPE, LETTER_TYPE):
table = get_notification_table_to_use(service, notification_type, process_day) table = get_notification_table_to_use(service, notification_type, process_day, has_delete_task_run=False)
results = _query_for_billing_data( results = _query_for_billing_data(
table=table, table=table,

View File

@@ -49,7 +49,7 @@ def fetch_notification_status_for_day(process_day):
# if no rows try notificationHistory # if no rows try notificationHistory
for service in services: for service in services:
for notification_type in [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE]: for notification_type in [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE]:
table = get_notification_table_to_use(service, notification_type, process_day) table = get_notification_table_to_use(service, notification_type, process_day, has_delete_task_run=False)
data_for_service_and_type = query_for_fact_status_data( data_for_service_and_type = query_for_fact_status_data(
table=table, table=table,

View File

@@ -115,12 +115,13 @@ def email_address_is_nhs(email_address):
)) ))
def get_notification_table_to_use(service, notification_type, process_day, has_delete_task_run=False): def get_notification_table_to_use(service, notification_type, process_day, has_delete_task_run):
""" """
Work out what table will contain notification data for a service by looking up their data retention. Work out what table will contain notification data for a service by looking up their data retention.
Make sure that when you run this you think about whether the delete task has run for that day! If it's run, the Make sure that when you run this you think about whether the delete task has run for that day! If it's run, the
notifications from that day will have moved to NotificationHistory. By default we assume it hasn't run, since notifications from that day will have moved to NotificationHistory. The delete tasks run between 4 and 5am every
morning.
""" """
from app.models import Notification, NotificationHistory from app.models import Notification, NotificationHistory

View File

@@ -61,28 +61,25 @@ def test_midnight_n_days_ago(current_time, arg, expected_datetime):
def test_get_notification_table_to_use(sample_service): def test_get_notification_table_to_use(sample_service):
# it's currently early morning of Thurs 10th Jan. # it's currently early morning of Thurs 10th Jan.
# When the delete task runs a bit later, it'll delete data from last wednesday 2nd. # When the delete task runs a bit later, it'll delete data from last wednesday 2nd.
assert get_notification_table_to_use(sample_service, 'sms', date(2018, 12, 31)) == NotificationHistory assert get_notification_table_to_use(sample_service, 'sms', date(2018, 12, 31), False) == NotificationHistory
assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 1)) == NotificationHistory assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 1), False) == NotificationHistory
assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 2)) == Notification assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 2), False) == Notification
assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 3)) == Notification assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 3), False) == Notification
@freeze_time('2019-01-10 00:30') @freeze_time('2019-01-10 00:30')
@pytest.mark.parametrize('has_delete_task_run, table', [ def test_get_notification_table_to_use_knows_if_delete_task_has_run(sample_service):
(True, NotificationHistory),
(False, Notification),
])
def test_get_notification_table_to_use_knows_if_delete_task_has_run(sample_service, has_delete_task_run, table):
# it's currently early morning of Thurs 10th Jan. # it's currently early morning of Thurs 10th Jan.
# The delete task deletes/moves data from last wednesday 2nd. # The delete task deletes/moves data from last wednesday 2nd.
assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 2), has_delete_task_run) == table assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 2), False) == Notification
assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 2), True) == NotificationHistory
@freeze_time('2019-06-09 23:30') @freeze_time('2019-06-09 23:30')
def test_get_notification_table_to_use_respects_daylight_savings_time(sample_service): def test_get_notification_table_to_use_respects_daylight_savings_time(sample_service):
# current time is 12:30am on 10th july in BST # current time is 12:30am on 10th july in BST
assert get_notification_table_to_use(sample_service, 'sms', date(2019, 6, 1)) == NotificationHistory assert get_notification_table_to_use(sample_service, 'sms', date(2019, 6, 1), False) == NotificationHistory
assert get_notification_table_to_use(sample_service, 'sms', date(2019, 6, 2)) == Notification assert get_notification_table_to_use(sample_service, 'sms', date(2019, 6, 2), False) == Notification
@freeze_time('2019-01-10 00:30') @freeze_time('2019-01-10 00:30')
@@ -92,9 +89,9 @@ def test_get_notification_table_to_use_checks_service_data_retention(sample_serv
# it's currently early morning of Thurs 10th Jan. # it's currently early morning of Thurs 10th Jan.
# three days retention means we'll delete sunday 6th's data when the delete task runs (so there's still three full # three days retention means we'll delete sunday 6th's data when the delete task runs (so there's still three full
# days of monday, tuesday and wednesday left over) # days of monday, tuesday and wednesday left over)
assert get_notification_table_to_use(sample_service, 'email', date(2019, 1, 5)) == NotificationHistory assert get_notification_table_to_use(sample_service, 'email', date(2019, 1, 5), False) == NotificationHistory
assert get_notification_table_to_use(sample_service, 'email', date(2019, 1, 6)) == Notification assert get_notification_table_to_use(sample_service, 'email', date(2019, 1, 6), False) == Notification
# falls back to 7 days if not specified # falls back to 7 days if not specified
assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 1)) == NotificationHistory assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 1), False) == NotificationHistory
assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 2)) == Notification assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 2), False) == Notification