rename days_ago to midnight_n_days_ago

also add some more timezone boundary tests and minor code cleanup

app/dao/jobs_dao.py

@@ -10,7 +10,7 @@ from sqlalchemy import (
 )
 
 from app import db
-from app.utils import days_ago
+from app.utils import midnight_n_days_ago
 from app.models import (
     Job,
     JOB_STATUS_PENDING,
@@ -51,7 +51,7 @@ def dao_get_jobs_by_service_id(service_id, limit_days=None, page=1, page_size=50
         Job.original_file_name != current_app.config['ONE_OFF_MESSAGE_FILENAME'],
     ]
     if limit_days is not None:
-        query_filter.append(Job.created_at >= days_ago(limit_days))
+        query_filter.append(Job.created_at >= midnight_n_days_ago(limit_days))
     if statuses is not None and statuses != ['']:
         query_filter.append(
             Job.job_status.in_(statuses)

app/dao/notifications_dao.py

@@ -21,7 +21,7 @@ from sqlalchemy.sql import functions
 from notifications_utils.international_billing_rates import INTERNATIONAL_BILLING_RATES
 
 from app import db, create_uuid
-from app.utils import days_ago
+from app.utils import midnight_n_days_ago
 from app.errors import InvalidRequest
 from app.models import (
     Notification,
@@ -248,7 +248,7 @@ def get_notifications_for_service(
     filters = [Notification.service_id == service_id]
 
     if limit_days is not None:
-        filters.append(Notification.created_at >= days_ago(limit_days))
+        filters.append(Notification.created_at >= midnight_n_days_ago(limit_days))
 
     if older_than is not None:
         older_than_created_at = db.session.query(

app/template_statistics/rest.py

@@ -39,7 +39,7 @@ def get_template_statistics_for_service_by_day(service_id):
     if limit_days < 1 or limit_days > 7:
         raise InvalidRequest({'limit_days': ['limit_days must be between 1 and 7']}, status_code=400)
 
-    return jsonify(data=get_template_statistics_for_last_n_days(service_id, limit_days))
+    return jsonify(data=_get_template_statistics_for_last_n_days(service_id, limit_days))
 
 
 @template_statistics.route('/<template_id>')
@@ -58,7 +58,7 @@ def get_template_statistics_for_template_id(service_id, template_id):
     return jsonify(data=data)
 
 
-def get_template_statistics_for_last_n_days(service_id, limit_days):
+def _get_template_statistics_for_last_n_days(service_id, limit_days):
     template_stats_by_id = Counter()
 
     for day in last_n_days(limit_days):
@@ -77,6 +77,7 @@ def get_template_statistics_for_last_n_days(service_id, limit_days):
             # if there is data in db, but not in redis - lets put it in redis so we don't have to do
             # this calc again next time. If there isn't any data, we can't put it in redis.
             # Zero length hashes aren't a thing in redis. (There'll only be no data if the service has no templates)
+            # Nothing is stored if redis is down.
             if stats:
                 redis_store.set_hash_and_expire(
                     key,

app/utils.py

@@ -81,7 +81,7 @@ def cache_key_for_service_template_counter(service_id, limit_days=7):
 
 def cache_key_for_service_template_usage_per_day(service_id, datetime):
     """
-    You should probably pass a BST datetime into this function
+    You should pass a BST datetime into this function
     """
     return "service-{}-template-usage-{}".format(service_id, datetime.date().isoformat())
 
@@ -95,7 +95,7 @@ def get_public_notify_type_text(notify_type, plural=False):
     return '{}{}'.format(notify_type_text, 's' if plural else '')
 
 
-def days_ago(number_of_days):
+def midnight_n_days_ago(number_of_days):
     """
     Returns midnight a number of days ago. Takes care of daylight savings etc.
     """

tests/app/dao/notification_dao/test_notification_dao_template_usage.py

@@ -72,6 +72,7 @@ def test_last_template_usage_should_be_able_to_get_no_template_usage_history_if_
     assert not results
 
 
+@freeze_time('2018-01-01')
 def test_should_by_able_to_get_template_count(sample_template, sample_email_template):
     create_notification(sample_template)
     create_notification(sample_template)
@@ -80,15 +81,16 @@ def test_should_by_able_to_get_template_count(sample_template, sample_email_temp
     create_notification(sample_email_template)
 
     results = dao_get_template_usage(sample_template.service_id, date.today())
-    assert results[0].name == 'Email Template Name'
-    assert results[0].template_type == 'email'
+    assert results[0].name == sample_email_template.name
+    assert results[0].template_type == sample_email_template.template_type
     assert results[0].count == 2
 
-    assert results[1].name == 'Template Name'
-    assert results[1].template_type == 'sms'
+    assert results[1].name == sample_template.name
+    assert results[1].template_type == sample_template.template_type
     assert results[1].count == 3
 
 
+@freeze_time('2018-01-01')
 def test_template_usage_should_ignore_test_keys(
     sample_team_api_key,
     sample_test_api_key,
@@ -102,8 +104,8 @@ def test_template_usage_should_ignore_test_keys(
     create_notification(sample_template)
 
     results = dao_get_template_usage(sample_template.service_id, date.today())
-    assert results[0].name == 'Template Name'
-    assert results[0].template_type == 'sms'
+    assert results[0].name == sample_template.name
+    assert results[0].template_type == sample_template.template_type
     assert results[0].count == 3
 
 
@@ -148,7 +150,6 @@ def test_template_usage_should_by_able_to_get_zero_count_from_notifications_hist
     assert len(results) == 0
 
 
-@freeze_time('2017-06-10T12:00:00')
 def test_template_usage_should_by_able_to_get_template_count_for_specific_day(sample_template):
     # too early
     create_notification(sample_template, created_at=datetime(2017, 6, 7, 22, 59, 0))
@@ -165,3 +166,21 @@ def test_template_usage_should_by_able_to_get_template_count_for_specific_day(sa
 
     assert len(results) == 1
     assert results[0].count == 5
+
+
+def test_template_usage_should_by_able_to_get_template_count_for_specific_timezone_boundary(sample_template):
+    # too early
+    create_notification(sample_template, created_at=datetime(2018, 3, 24, 23, 59, 0))
+    # just right
+    create_notification(sample_template, created_at=datetime(2018, 3, 25, 0, 0, 0))
+    create_notification(sample_template, created_at=datetime(2018, 3, 25, 0, 0, 0))
+    create_notification(sample_template, created_at=datetime(2018, 3, 25, 22, 59, 0))
+    create_notification(sample_template, created_at=datetime(2018, 3, 25, 22, 59, 0))
+    create_notification(sample_template, created_at=datetime(2018, 3, 25, 22, 59, 0))
+    # too late
+    create_notification(sample_template, created_at=datetime(2018, 3, 25, 23, 0, 0))
+
+    results = dao_get_template_usage(sample_template.service_id, day=date(2018, 3, 25))
+
+    assert len(results) == 1
+    assert results[0].count == 5

tests/app/test_utils.py

@@ -8,7 +8,7 @@ from app.utils import (
     get_midnight_for_day_before,
     convert_utc_to_bst,
     convert_bst_to_utc,
-    days_ago,
+    midnight_n_days_ago,
     last_n_days
 )
 
@@ -51,22 +51,26 @@ def test_convert_bst_to_utc():
     assert utc == datetime(2017, 5, 12, 12, 15)
 
 
-@pytest.mark.parametrize('current_time, expected_datetime', [
+@pytest.mark.parametrize('current_time, arg, expected_datetime', [
     # winter
-    ('2018-01-10 23:59', datetime(2018, 1, 9, 0, 0)),
-    ('2018-01-11 00:00', datetime(2018, 1, 10, 0, 0)),
+    ('2018-01-10 23:59', 1, datetime(2018, 1, 9, 0, 0)),
+    ('2018-01-11 00:00', 1, datetime(2018, 1, 10, 0, 0)),
 
     # bst switchover at 1am 25th
-    ('2018-03-25 10:00', datetime(2018, 3, 24, 0, 0)),
-    ('2018-03-26 10:00', datetime(2018, 3, 25, 0, 0)),
-    ('2018-03-27 10:00', datetime(2018, 3, 25, 23, 0)),
+    ('2018-03-25 10:00', 1, datetime(2018, 3, 24, 0, 0)),
+    ('2018-03-26 10:00', 1, datetime(2018, 3, 25, 0, 0)),
+    ('2018-03-27 10:00', 1, datetime(2018, 3, 25, 23, 0)),
 
     # summer
-    ('2018-06-05 10:00', datetime(2018, 6, 3, 23, 0))
+    ('2018-06-05 10:00', 1, datetime(2018, 6, 3, 23, 0)),
+
+    # zero days ago
+    ('2018-01-11 00:00', 0, datetime(2018, 1, 11, 0, 0)),
+    ('2018-06-05 10:00', 0, datetime(2018, 6, 4, 23, 0)),
 ])
-def test_days_ago(current_time, expected_datetime):
+def test_midnight_n_days_ago(current_time, arg, expected_datetime):
     with freeze_time(current_time):
-        assert days_ago(1) == expected_datetime
+        assert midnight_n_days_ago(arg) == expected_datetime
 
 
 def test_last_n_days():
@@ -80,3 +84,8 @@ def test_last_n_days():
         datetime(2018, 3, 26, 0, 0),
         datetime(2018, 3, 27, 0, 0)
     ]
+
+
+@pytest.mark.parametrize('arg', [0, -1])
+def test_last_n_days_invalid_arg(arg):
+    assert last_n_days(arg) == []