From 3057641e408845940f23fdccc78df3a8a6bb3696 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Wed, 6 Apr 2016 14:30:13 +0100 Subject: [PATCH 1/2] Change sort order for templates from name to date using full timestamp so that it would be most recently used at top. --- app/dao/notifications_dao.py | 8 ++--- app/models.py | 6 ++++ .../0045_template_stats_update_timestamp.py | 32 +++++++++++++++++ tests/app/dao/test_notification_dao.py | 36 ------------------- 4 files changed, 41 insertions(+), 41 deletions(-) create mode 100644 migrations/versions/0045_template_stats_update_timestamp.py diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 6196a7842..26172f25b 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -67,11 +67,9 @@ def dao_get_template_statistics_for_service(service_id, limit_days=None): desc(TemplateStatistics.day)).limit(1).first() if latest_stat: last_date_to_fetch = latest_stat.day - timedelta(days=limit_days) - else: - last_date_to_fetch = date.today() - timedelta(days=limit_days) - filter.append(TemplateStatistics.day > last_date_to_fetch) + filter.append(TemplateStatistics.day > last_date_to_fetch) return TemplateStatistics.query.filter(*filter).order_by( - desc(TemplateStatistics.day)).join(Template).order_by(func.lower(Template.name)).all() + desc(TemplateStatistics.updated_at)).all() @transactional @@ -102,7 +100,7 @@ def dao_create_notification(notification, notification_type): day=date.today(), service_id=notification.service_id, template_id=notification.template_id - ).update({'usage_count': TemplateStatistics.usage_count + 1}) + ).update({'usage_count': TemplateStatistics.usage_count + 1, 'updated_at': datetime.utcnow()}) if update_count == 0: template_stats = TemplateStatistics(template_id=notification.template_id, diff --git a/app/models.py b/app/models.py index 165903a90..721bb4a08 100644 --- a/app/models.py +++ b/app/models.py @@ -362,3 +362,9 @@ class TemplateStatistics(db.Model): template = db.relationship('Template') usage_count = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=1) day = db.Column(db.Date, index=True, nullable=False, unique=False, default=datetime.date.today) + updated_at = db.Column( + db.DateTime, + index=False, + unique=False, + nullable=False, + default=datetime.datetime.utcnow) diff --git a/migrations/versions/0045_template_stats_update_timestamp.py b/migrations/versions/0045_template_stats_update_timestamp.py new file mode 100644 index 000000000..fb9d8d0b1 --- /dev/null +++ b/migrations/versions/0045_template_stats_update_timestamp.py @@ -0,0 +1,32 @@ +"""empty message + +Revision ID: 0045_template_stats_update_time +Revises: 0044_add_template_stats +Create Date: 2016-04-05 14:32:45.165755 + +""" + +# revision identifiers, used by Alembic. +revision = '0045_template_stats_update_time' +down_revision = '0044_add_template_stats' + +import datetime + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.sql import table, column + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.add_column('template_statistics', sa.Column('updated_at', sa.DateTime(), nullable=True)) + updated_at = table('template_statistics', column('updated_at')) + op.execute(updated_at.update().values(updated_at=datetime.datetime.utcnow())) + op.alter_column('template_statistics', 'updated_at', nullable=False) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_column('template_statistics', 'updated_at') + ### end Alembic commands ### diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 5f7d80cd0..1cc6936a3 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -1016,39 +1016,3 @@ def test_get_template_stats_for_service_returns_results_from_first_day_with_data def test_get_template_stats_for_service_with_limit_if_no_records_returns_empty_list(sample_template): template_stats = dao_get_template_statistics_for_service(sample_template.service.id, limit_days=7) assert len(template_stats) == 0 - - -def test_get_template_stats_for_service_for_day_returns_stats_in_template_name_order(sample_service, sample_job): - - from app.dao.templates_dao import dao_create_template - from app.models import Template - - template_stats = dao_get_template_statistics_for_service(sample_service.id) - assert len(template_stats) == 0 - - template_names = ['Aardvark', 'ant', 'zebra', 'walrus', 'Donkey', 'Komodo dragon'] - for name in template_names: - data = { - 'name': name, - 'template_type': 'sms', - 'content': 'blah', - 'service': sample_service - } - template = Template(**data) - dao_create_template(template) - - notification_data = { - 'to': '+44709123456', - 'job_id': sample_job.id, - 'service': template.service, - 'service_id': template.service.id, - 'template': template, - 'template_id': template.id, - 'created_at': datetime.utcnow() - } - notification = Notification(**notification_data) - dao_create_notification(notification, template.template_type) - template_stats = dao_get_template_statistics_for_service(template.service.id) - - for i, name in enumerate(sorted(template_names, key=str.lower)): - assert template_stats[i].template.name == name From 4ed2e7f8f2a35c9fc6f0937657c12283236e18dd Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 7 Apr 2016 09:30:02 +0100 Subject: [PATCH 2/2] Fix for misunderstanding about date range required for templates stats. It should always be last n days, whether or not there is data. --- app/dao/notifications_dao.py | 7 ++---- tests/app/dao/test_notification_dao.py | 28 +++++----------------- tests/app/template_statistics/test_rest.py | 16 +++++++++---- 3 files changed, 20 insertions(+), 31 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 26172f25b..b1c7fface 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -63,11 +63,8 @@ def dao_get_notification_statistics_for_service_and_day(service_id, day): def dao_get_template_statistics_for_service(service_id, limit_days=None): filter = [TemplateStatistics.service_id == service_id] if limit_days: - latest_stat = TemplateStatistics.query.filter_by(service_id=service_id).order_by( - desc(TemplateStatistics.day)).limit(1).first() - if latest_stat: - last_date_to_fetch = latest_stat.day - timedelta(days=limit_days) - filter.append(TemplateStatistics.day > last_date_to_fetch) + last_date_to_fetch = date.today() - timedelta(days=limit_days) + filter.append(TemplateStatistics.day > last_date_to_fetch) return TemplateStatistics.query.filter(*filter).order_by( desc(TemplateStatistics.updated_at)).all() diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 1cc6936a3..acbc633ae 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -974,7 +974,7 @@ def test_get_template_stats_for_service_returns_stats_returns_all_stats_if_no_li @freeze_time('2016-04-30') -def test_get_template_stats_for_service_returns_results_from_first_day_with_data(sample_template): +def test_get_template_stats_for_service_returns_no_result_if_no_usage_within_limit_days(sample_template): template_stats = dao_get_template_statistics_for_service(sample_template.service.id) assert len(template_stats) == 0 @@ -988,29 +988,13 @@ def test_get_template_stats_for_service_returns_results_from_first_day_with_data db.session.add(template_stats) db.session.commit() - # Retrieve one day of stats - read date is 2016-04-30 - template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=1) - assert len(template_stats) == 1 - assert template_stats[0].day == date(2016, 4, 9) + # Retrieve a week of stats - read date is 2016-04-30 + template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=7) + assert len(template_stats) == 0 - # Retrieve three days of stats - template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=3) - assert len(template_stats) == 3 - assert template_stats[0].day == date(2016, 4, 9) - assert template_stats[1].day == date(2016, 4, 8) - assert template_stats[2].day == date(2016, 4, 7) - - # Retrieve nine days of stats - template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=9) + # Retrieve a month of stats - read date is 2016-04-30 + template_stats = dao_get_template_statistics_for_service(sample_template.service_id, limit_days=30) assert len(template_stats) == 9 - assert template_stats[0].day == date(2016, 4, 9) - assert template_stats[8].day == date(2016, 4, 1) - - # Retrieve with no limit - template_stats = dao_get_template_statistics_for_service(sample_template.service_id) - assert len(template_stats) == 9 - assert template_stats[0].day == date(2016, 4, 9) - assert template_stats[8].day == date(2016, 4, 1) def test_get_template_stats_for_service_with_limit_if_no_records_returns_empty_list(sample_template): diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index 20c99ba67..a8d4302e9 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -41,7 +41,7 @@ def test_get_template_statistics_for_service_for_last_week(notify_api, sample_te @freeze_time('2016-04-30') -def test_get_template_statistics_for_service_for_last_actual_week_with_data(notify_api, sample_template): +def test_get_template_statistics_for_service_for_last_week_with_no_data(notify_api, sample_template): # make 9 stats records from 1st to 9th April for i in range(1, 10): @@ -69,9 +69,17 @@ def test_get_template_statistics_for_service_for_last_actual_week_with_data(noti assert response.status_code == 200 json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 7 - assert json_resp['data'][0]['day'] == '2016-04-09' - assert json_resp['data'][6]['day'] == '2016-04-03' + assert len(json_resp['data']) == 0 + + response = client.get( + '/service/{}/template-statistics'.format(sample_template.service_id), + headers=[('Content-Type', 'application/json'), auth_header], + query_string={'limit_days': 30} + ) + + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 9 def test_get_all_template_statistics_for_service(notify_api, sample_template):