From a77c316d2f7a7afae744e5ad6d9194f93e53588a Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Fri, 22 Sep 2017 12:22:40 +0100 Subject: [PATCH 1/9] Added stats annotation to a few more methods to track which methods need to be improved to increase billing performance --- app/dao/monthly_billing_dao.py | 2 ++ app/dao/notification_usage_dao.py | 1 + 2 files changed, 3 insertions(+) diff --git a/app/dao/monthly_billing_dao.py b/app/dao/monthly_billing_dao.py index 606c02cc9..4528f414d 100644 --- a/app/dao/monthly_billing_dao.py +++ b/app/dao/monthly_billing_dao.py @@ -26,6 +26,7 @@ def get_service_ids_that_need_billing_populated(start_date, end_date): ).distinct().all() +@statsd(namespace="dao") def create_or_update_monthly_billing(service_id, billing_month): start_date, end_date = get_month_start_and_end_date_in_utc(billing_month) _update_monthly_billing(service_id, start_date, end_date, SMS_TYPE) @@ -47,6 +48,7 @@ def _monthly_billing_data_to_json(billing_data): return results +@statsd(namespace="dao") @transactional def _update_monthly_billing(service_id, start_date, end_date, notification_type): billing_data = get_billing_data_for_month( diff --git a/app/dao/notification_usage_dao.py b/app/dao/notification_usage_dao.py index 7542fb0df..afb547287 100644 --- a/app/dao/notification_usage_dao.py +++ b/app/dao/notification_usage_dao.py @@ -105,6 +105,7 @@ def is_between(date, start_date, end_date): return start_date <= date <= end_date +@statsd(namespace="dao") def billing_data_per_month_query(rate, service_id, start_date, end_date, notification_type): month = get_london_month_from_utc_column(NotificationHistory.created_at) if notification_type == SMS_TYPE: From 8973e42b8602fe2dd1153bda0cfc32abc8552f40 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Tue, 7 Nov 2017 14:50:05 +0000 Subject: [PATCH 2/9] =?UTF-8?q?Added=20a=20new=20table=20=E2=80=98stats=5F?= =?UTF-8?q?template=5Fusage=5Fby=5Fmonth=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the 'See templates used by month' report is timing out for some services due to the query time of the data from notification_history This is a stats table which will hold the data and will be updated by a scheduled celery task once a day. This data will be combined with the 'live' data from notifications tables (which will be considerably less) to form the data of the new report. --- app/models.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/app/models.py b/app/models.py index 74b0af79a..0c64a4581 100644 --- a/app/models.py +++ b/app/models.py @@ -1555,3 +1555,37 @@ class AuthType(db.Model): __tablename__ = 'auth_type' name = db.Column(db.String, primary_key=True) + + +class StatsTemplateUsageByMonth(db.Model): + __tablename__ = "stats_template_usage_by_month" + + template_id = db.Column( + UUID(as_uuid=True), + db.ForeignKey('templates.id'), + unique=False, + index=True, + nullable=False, + primary_key=True + ) + month = db.Column( + db.Integer, + nullable=False, + index=True, + unique=False, + primary_key=True, + default=datetime.datetime.month + ) + year = db.Column( + db.Integer, + nullable=False, + index=True, + unique=False, + primary_key=True, + default=datetime.datetime.year + ) + count = db.Column( + db.Integer, + nullable=False, + default=0 + ) From 26617a921e37d58d74f31d32339c1878297b19ba Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Tue, 7 Nov 2017 14:57:31 +0000 Subject: [PATCH 3/9] Added database migration file Currently the 'See templates used by month' report is timing out for some services due to the query time of the data from notification_history This is a stats table which will hold the data and will be updated by a scheduled celery task once a day. This data will be combined with the 'live' data from notifications tables (which will be considerably less) to form the data of the new report. --- .../versions/0134_stats_template_usage.py | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 migrations/versions/0134_stats_template_usage.py diff --git a/migrations/versions/0134_stats_template_usage.py b/migrations/versions/0134_stats_template_usage.py new file mode 100644 index 000000000..7e1b26837 --- /dev/null +++ b/migrations/versions/0134_stats_template_usage.py @@ -0,0 +1,65 @@ +""" + +Revision ID: 0134_stats_template_usage +Revises: 0133_set_services_sms_prefix +Create Date: 2017-11-07 14:35:04.798561 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0134_stats_template_usage' +down_revision = '0133_set_services_sms_prefix' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('stats_template_usage_by_month', + sa.Column('template_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('month', sa.Integer(), nullable=False), + sa.Column('year', sa.Integer(), nullable=False), + sa.Column('count', sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(['template_id'], ['templates.id'], ), + sa.PrimaryKeyConstraint('template_id', 'month', 'year') + ) + op.create_index(op.f('ix_stats_template_usage_by_month_month'), 'stats_template_usage_by_month', ['month'], unique=False) + op.create_index(op.f('ix_stats_template_usage_by_month_template_id'), 'stats_template_usage_by_month', ['template_id'], unique=False) + op.create_index(op.f('ix_stats_template_usage_by_month_year'), 'stats_template_usage_by_month', ['year'], unique=False) + op.drop_table('notification_statistics') + op.alter_column('notification_history', 'international', + existing_type=sa.BOOLEAN(), + nullable=False) + op.alter_column('notifications', 'international', + existing_type=sa.BOOLEAN(), + nullable=False) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column('notifications', 'international', + existing_type=sa.BOOLEAN(), + nullable=True) + op.alter_column('notification_history', 'international', + existing_type=sa.BOOLEAN(), + nullable=True) + op.create_table('notification_statistics', + sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('service_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('emails_requested', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('emails_delivered', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('emails_failed', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('sms_requested', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('sms_delivered', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('sms_failed', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('day', sa.DATE(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint(['service_id'], ['services.id'], name='notification_statistics_service_id_fkey'), + sa.PrimaryKeyConstraint('id', name='notification_statistics_pkey'), + sa.UniqueConstraint('service_id', 'day', name='uix_service_to_day') + ) + op.drop_index(op.f('ix_stats_template_usage_by_month_year'), table_name='stats_template_usage_by_month') + op.drop_index(op.f('ix_stats_template_usage_by_month_template_id'), table_name='stats_template_usage_by_month') + op.drop_index(op.f('ix_stats_template_usage_by_month_month'), table_name='stats_template_usage_by_month') + op.drop_table('stats_template_usage_by_month') + # ### end Alembic commands ### From ab43803453682eb734caf4b7c88f2b2858ad965f Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Tue, 7 Nov 2017 15:30:08 +0000 Subject: [PATCH 4/9] Removed unwanted migration commands There were some unwanted db migration commands which crept into the migration file. removed these as they are being handled in other PRs. --- migrations/versions/0134_stats_template_usage.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/migrations/versions/0134_stats_template_usage.py b/migrations/versions/0134_stats_template_usage.py index 7e1b26837..4a1fd1963 100644 --- a/migrations/versions/0134_stats_template_usage.py +++ b/migrations/versions/0134_stats_template_usage.py @@ -26,24 +26,11 @@ def upgrade(): op.create_index(op.f('ix_stats_template_usage_by_month_month'), 'stats_template_usage_by_month', ['month'], unique=False) op.create_index(op.f('ix_stats_template_usage_by_month_template_id'), 'stats_template_usage_by_month', ['template_id'], unique=False) op.create_index(op.f('ix_stats_template_usage_by_month_year'), 'stats_template_usage_by_month', ['year'], unique=False) - op.drop_table('notification_statistics') - op.alter_column('notification_history', 'international', - existing_type=sa.BOOLEAN(), - nullable=False) - op.alter_column('notifications', 'international', - existing_type=sa.BOOLEAN(), - nullable=False) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.alter_column('notifications', 'international', - existing_type=sa.BOOLEAN(), - nullable=True) - op.alter_column('notification_history', 'international', - existing_type=sa.BOOLEAN(), - nullable=True) op.create_table('notification_statistics', sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), sa.Column('service_id', postgresql.UUID(), autoincrement=False, nullable=False), From ff911c30d67f5db388f31f6305de3f0a75a42e9f Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 9 Nov 2017 10:32:39 +0000 Subject: [PATCH 5/9] Added Scheduled task to get stats for template usage Currently some pages are timing out due to the time it takes to perform database queries. This is an attempt to improve the performance by performing the query against the notification history table once a day and use the notification table for a delta between midnight and the when the page is run and combine the results. - Added Celery task for doing the work - Added a dao to handle the insert and update of the stats table - Updated tests to test the new functionality --- app/celery/scheduled_tasks.py | 18 +++ app/config.py | 5 + app/dao/services_dao.py | 28 +++- app/dao/stats_template_usage_by_month_dao.py | 24 ++++ app/utils.py | 16 +++ tests/app/celery/test_scheduled_tasks.py | 124 +++++++++++++++++- tests/app/dao/test_services_dao.py | 39 +++++- .../test_stats_template_usage_by_month_dao.py | 45 +++++++ 8 files changed, 288 insertions(+), 11 deletions(-) create mode 100644 app/dao/stats_template_usage_by_month_dao.py create mode 100644 tests/app/dao/test_stats_template_usage_by_month_dao.py diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 2e0413fe8..9b1a830d9 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -11,6 +11,10 @@ from notifications_utils.s3 import s3upload from app.aws import s3 from app import notify_celery +from app.dao.services_dao import ( + dao_fetch_monthly_historical_stats_by_template +) +from app.dao.stats_template_usage_by_month_dao import insert_or_update_stats_for_template from app.performance_platform import total_sent_notifications, processing_time from app import performance_platform_client from app.dao.date_util import get_month_start_and_end_date_in_utc @@ -402,3 +406,17 @@ def check_job_status(): queue=QueueNames.JOBS ) raise JobIncompleteError("Job(s) {} have not completed.".format(job_ids)) + + +@notify_celery.task(name='daily-stats-template_usage_by_month') +@statsd(namespace="tasks") +def daily_stats_template_usage_my_month(): + results = dao_fetch_monthly_historical_stats_by_template() + + for result in results: + insert_or_update_stats_for_template( + result.template_id, + result.month.month, + result.year.year, + result.count + ) diff --git a/app/config.py b/app/config.py index 59be8ad02..afd70dade 100644 --- a/app/config.py +++ b/app/config.py @@ -241,6 +241,11 @@ class Config(object): 'task': 'check-job-status', 'schedule': crontab(), 'options': {'queue': QueueNames.PERIODIC} + }, + 'daily-stats-template_usage_by_month': { + 'task': 'daily-stats-template_usage_by_month', + 'schedule': crontab(hour=00, minute=50), + 'options': {'queue': QueueNames.PERIODIC} } } CELERY_QUEUES = [] diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 93905e7e6..684696d83 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,7 +1,7 @@ import uuid -from datetime import date, datetime, timedelta +from datetime import date, datetime, timedelta, time -from sqlalchemy import asc, func +from sqlalchemy import asc, func, extract from sqlalchemy.orm import joinedload from flask import current_app @@ -40,7 +40,7 @@ from app.models import ( ) from app.service.statistics import format_monthly_template_notification_stats from app.statsd_decorators import statsd -from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc +from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc, get_london_year_from_utc_column from app.dao.annual_billing_dao import dao_insert_annual_billing DEFAULT_SERVICE_PERMISSIONS = [ @@ -520,3 +520,25 @@ def dao_fetch_active_users_for_service(service_id): ) return query.all() + + +@statsd(namespace="dao") +def dao_fetch_monthly_historical_stats_by_template(): + month = get_london_month_from_utc_column(NotificationHistory.created_at) + year = get_london_year_from_utc_column(NotificationHistory.created_at) + end_date = datetime.combine(date.today(), time.min) + + return db.session.query( + NotificationHistory.template_id, + month.label('month'), + year.label('year'), + func.count().label('count') + ).filter( + NotificationHistory.created_at < end_date + ).group_by( + NotificationHistory.template_id, + month, + year + ).order_by( + NotificationHistory.template_id + ).all() diff --git a/app/dao/stats_template_usage_by_month_dao.py b/app/dao/stats_template_usage_by_month_dao.py new file mode 100644 index 000000000..ca94dd83f --- /dev/null +++ b/app/dao/stats_template_usage_by_month_dao.py @@ -0,0 +1,24 @@ +from app import db +from app.models import StatsTemplateUsageByMonth + + +def insert_or_update_stats_for_template(template_id, month, year, count): + result = db.session.query( + StatsTemplateUsageByMonth + ).filter( + StatsTemplateUsageByMonth.template_id == template_id, + StatsTemplateUsageByMonth.month == month, + StatsTemplateUsageByMonth.year == year + ).update( + { + 'count': count + } + ) + if result == 0: + new_sms_sender = StatsTemplateUsageByMonth( + template_id=template_id, + month=month, + year=year, + count=count + ) + db.session.add(new_sms_sender) diff --git a/app/utils.py b/app/utils.py index 49f6d61dc..7fe4bdf89 100644 --- a/app/utils.py +++ b/app/utils.py @@ -75,6 +75,22 @@ def get_london_month_from_utc_column(column): ) +def get_london_year_from_utc_column(column): + """ + Where queries need to count notifications by month it needs to be + the month in BST (British Summer Time). + The database stores all timestamps as UTC without the timezone. + - First set the timezone on created_at to UTC + - then convert the timezone to BST (or Europe/London) + - lastly truncate the datetime to month with which we can group + queries + """ + return func.date_trunc( + "month", + func.timezone("Europe/London", func.timezone("UTC", column)) + ) + + def cache_key_for_service_template_counter(service_id, limit_days=7): return "{}-template-counter-limit-{}-days".format(service_id, limit_days) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index a61c70807..7f60318cc 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -2,11 +2,13 @@ from datetime import datetime, timedelta from functools import partial from unittest.mock import call, patch, PropertyMock +import functools from flask import current_app import pytest from freezegun import freeze_time +from app import db from app.celery import scheduled_tasks from app.celery.scheduled_tasks import ( check_job_status, @@ -30,8 +32,8 @@ from app.celery.scheduled_tasks import ( send_total_sent_notifications_to_performance_platform, switch_current_sms_provider_on_slow_delivery, timeout_job_statistics, - timeout_notifications -) + timeout_notifications, + daily_stats_template_usage_my_month) from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient from app.config import QueueNames, TaskNames from app.dao.jobs_dao import dao_get_job_by_id @@ -49,19 +51,24 @@ from app.models import ( NOTIFICATION_PENDING, NOTIFICATION_CREATED, KEY_TYPE_TEST, - MonthlyBilling -) + MonthlyBilling, + StatsTemplateUsageByMonth) from app.utils import get_london_midnight_in_utc from app.v2.errors import JobIncompleteError from tests.app.db import create_notification, create_service, create_template, create_job, create_rate from tests.app.conftest import ( sample_job as create_sample_job, - sample_notification_history as create_notification_history, create_custom_template, datetime_in_past ) from tests.app.aws.test_s3 import single_s3_object_stub -from tests.conftest import set_config_values +from tests.conftest import set_config_values, notify_db, notify_db_session + +from tests.app.conftest import ( + sample_notification as create_notification, + sample_notification_history as create_notification_history, + sample_template as create_sample_template +) def _create_slow_delivery_notification(provider='mmg'): @@ -834,3 +841,108 @@ def test_check_job_status_task_raises_job_incomplete_error_for_multiple_jobs(moc args=([str(job.id), str(job_2.id)],), queue=QueueNames.JOBS ) + + +def test_daily_stats_template_usage_my_month(notify_db, notify_db_session): + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + status='delivered' + ) + + template_one = create_sample_template(notify_db, notify_db_session) + template_two = create_sample_template(notify_db, notify_db_session) + + notification_history(created_at=datetime(2017, 10, 1), sample_template=template_one) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime.now(), sample_template=template_two) + + daily_stats_template_usage_my_month() + + results = db.session.query(StatsTemplateUsageByMonth).all() + + assert len(results) == 2 + + for result in results: + if result.template_id == template_one.id: + assert result.template_id == template_one.id + assert result.month == 10 + assert result.year == 2017 + assert result.count == 1 + elif result.template_id == template_two.id: + assert result.template_id == template_two.id + assert result.month == 4 + assert result.year == 2016 + assert result.count == 2 + else: + raise AssertionError() + + +def test_daily_stats_template_usage_my_month_no_data(): + daily_stats_template_usage_my_month() + + results = db.session.query(StatsTemplateUsageByMonth).all() + + assert len(results) == 0 + + +def test_daily_stats_template_usage_my_month_multiple_runs(notify_db, notify_db_session): + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + status='delivered' + ) + + template_one = create_sample_template(notify_db, notify_db_session) + template_two = create_sample_template(notify_db, notify_db_session) + + notification_history(created_at=datetime(2017, 10, 1), sample_template=template_one) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime.now(), sample_template=template_two) + + daily_stats_template_usage_my_month() + + template_three = create_sample_template(notify_db, notify_db_session) + + notification_history(created_at=datetime(2017, 10, 1), sample_template=template_three) + notification_history(created_at=datetime(2017, 9, 1), sample_template=template_three) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime.now(), sample_template=template_two) + + daily_stats_template_usage_my_month() + + results = db.session.query(StatsTemplateUsageByMonth).all() + + assert len(results) == 4 + + for result in results: + if result.template_id == template_one.id: + assert result.template_id == template_one.id + assert result.month == 10 + assert result.year == 2017 + assert result.count == 1 + elif result.template_id == template_two.id: + assert result.template_id == template_two.id + assert result.month == 4 + assert result.year == 2016 + assert result.count == 4 + elif result.template_id == template_three.id: + if result.month == 10: + assert result.template_id == template_three.id + assert result.month == 10 + assert result.year == 2017 + assert result.count == 1 + elif result.month == 9: + assert result.template_id == template_three.id + assert result.month == 9 + assert result.year == 2017 + assert result.count == 1 + else: + raise AssertionError() + else: + raise AssertionError() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index c45ce0272..bc5a8af82 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -30,8 +30,8 @@ from app.dao.services_dao import ( dao_suspend_service, dao_resume_service, dao_fetch_active_users_for_service, - dao_fetch_service_by_inbound_number -) + dao_fetch_service_by_inbound_number, + dao_fetch_monthly_historical_stats_by_template_for_service_without_status) from app.dao.service_permissions_dao import dao_add_service_permission, dao_remove_service_permission from app.dao.users_dao import save_model_user from app.models import ( @@ -1004,3 +1004,38 @@ def _assert_service_permissions(service_permissions, expected): assert len(service_permissions) == len(expected) assert set(expected) == set(p.permission for p in service_permissions) + + +def test_dao_fetch_monthly_historical_stats_by_template(notify_db, notify_db_session): + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + status='delivered' + ) + + template_one = create_sample_template(notify_db, notify_db_session) + template_two = create_sample_template(notify_db, notify_db_session) + + notification_history(created_at=datetime(2017, 10, 1), sample_template=template_one) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) + notification_history(created_at=datetime.now(), sample_template=template_two) + + results = dao_fetch_monthly_historical_stats_by_template_for_service_without_status() + + assert len(results) == 2 + + for result in results: + if result.template_id == template_one.id: + assert result.template_id == template_one.id + assert result.month.month == 10 + assert result.year.year == 2017 + assert result.count == 1 + elif result.template_id == template_two.id: + assert result.template_id == template_two.id + assert result.month.month == 4 + assert result.year.year == 2016 + assert result.count == 2 + else: + raise AssertionError() diff --git a/tests/app/dao/test_stats_template_usage_by_month_dao.py b/tests/app/dao/test_stats_template_usage_by_month_dao.py new file mode 100644 index 000000000..ae46c682b --- /dev/null +++ b/tests/app/dao/test_stats_template_usage_by_month_dao.py @@ -0,0 +1,45 @@ +import pytest + +from app.dao.stats_template_usage_by_month_dao import insert_or_update_stats_for_template +from app.models import StatsTemplateUsageByMonth + +from tests.app.conftest import sample_notification, sample_email_template, sample_template, sample_job, sample_service + + +def test_create_stats_for_template(notify_db_session, sample_template): + assert StatsTemplateUsageByMonth.query.count() == 0 + + insert_or_update_stats_for_template(sample_template.id, 1, 2017, 10) + stats_by_month = StatsTemplateUsageByMonth.query.filter( + StatsTemplateUsageByMonth.template_id == sample_template.id + ).all() + + assert len(stats_by_month) == 1 + assert stats_by_month[0].template_id == sample_template.id + assert stats_by_month[0].month == 1 + assert stats_by_month[0].year == 2017 + assert stats_by_month[0].count == 10 + + +def test_update_stats_for_template(notify_db_session, sample_template): + assert StatsTemplateUsageByMonth.query.count() == 0 + + insert_or_update_stats_for_template(sample_template.id, 1, 2017, 10) + insert_or_update_stats_for_template(sample_template.id, 1, 2017, 20) + insert_or_update_stats_for_template(sample_template.id, 2, 2017, 30) + + stats_by_month = StatsTemplateUsageByMonth.query.filter( + StatsTemplateUsageByMonth.template_id == sample_template.id + ).order_by(StatsTemplateUsageByMonth.template_id).all() + + assert len(stats_by_month) == 2 + + assert stats_by_month[0].template_id == sample_template.id + assert stats_by_month[0].month == 1 + assert stats_by_month[0].year == 2017 + assert stats_by_month[0].count == 20 + + assert stats_by_month[1].template_id == sample_template.id + assert stats_by_month[1].month == 2 + assert stats_by_month[1].year == 2017 + assert stats_by_month[1].count == 30 From 98b762869d25a6454b374c2e81e354b2e37d2d75 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 9 Nov 2017 10:41:43 +0000 Subject: [PATCH 6/9] Renamed database migration file after merge Updated the database migration file name after a rebase as there were two 0134 files and this would break the upgrade / downgrade process. --- ...ats_template_usage.py => 0135_stats_template_usage.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0134_stats_template_usage.py => 0135_stats_template_usage.py} (94%) diff --git a/migrations/versions/0134_stats_template_usage.py b/migrations/versions/0135_stats_template_usage.py similarity index 94% rename from migrations/versions/0134_stats_template_usage.py rename to migrations/versions/0135_stats_template_usage.py index 4a1fd1963..722fc69ce 100644 --- a/migrations/versions/0134_stats_template_usage.py +++ b/migrations/versions/0135_stats_template_usage.py @@ -1,7 +1,7 @@ """ -Revision ID: 0134_stats_template_usage -Revises: 0133_set_services_sms_prefix +Revision ID: 0135_stats_template_usage +Revises: 0134_add_email_2fa_template Create Date: 2017-11-07 14:35:04.798561 """ @@ -9,8 +9,8 @@ from alembic import op import sqlalchemy as sa from sqlalchemy.dialects import postgresql -revision = '0134_stats_template_usage' -down_revision = '0133_set_services_sms_prefix' +revision = '0135_stats_template_usage' +down_revision = '0134_add_email_2fa_template' def upgrade(): From 59df6bdbb6e72c23115c0ad507733d9fd34d1a48 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 9 Nov 2017 11:33:56 +0000 Subject: [PATCH 7/9] Fixed imports which were producing broken tests - Some tests for failing because of an import which was overriding the one above it in test_scheduled_tasks.py - Import error fixed in test_services_dao.py after a rename of a method --- tests/app/celery/test_csv_files/email.csv | 2 ++ tests/app/celery/test_csv_files/empty.csv | 1 + tests/app/celery/test_csv_files/multiple_email.csv | 11 +++++++++++ tests/app/celery/test_csv_files/multiple_letter.csv | 11 +++++++++++ tests/app/celery/test_csv_files/multiple_sms.csv | 11 +++++++++++ tests/app/celery/test_csv_files/sms.csv | 2 ++ tests/app/celery/test_scheduled_tasks.py | 11 ++++------- tests/app/dao/test_services_dao.py | 5 +++-- 8 files changed, 45 insertions(+), 9 deletions(-) create mode 100644 tests/app/celery/test_csv_files/email.csv create mode 100644 tests/app/celery/test_csv_files/empty.csv create mode 100644 tests/app/celery/test_csv_files/multiple_email.csv create mode 100644 tests/app/celery/test_csv_files/multiple_letter.csv create mode 100644 tests/app/celery/test_csv_files/multiple_sms.csv create mode 100644 tests/app/celery/test_csv_files/sms.csv diff --git a/tests/app/celery/test_csv_files/email.csv b/tests/app/celery/test_csv_files/email.csv new file mode 100644 index 000000000..f0cefee69 --- /dev/null +++ b/tests/app/celery/test_csv_files/email.csv @@ -0,0 +1,2 @@ +email_address +test@test.com diff --git a/tests/app/celery/test_csv_files/empty.csv b/tests/app/celery/test_csv_files/empty.csv new file mode 100644 index 000000000..64e5bd0a3 --- /dev/null +++ b/tests/app/celery/test_csv_files/empty.csv @@ -0,0 +1 @@ +phone number diff --git a/tests/app/celery/test_csv_files/multiple_email.csv b/tests/app/celery/test_csv_files/multiple_email.csv new file mode 100644 index 000000000..5da15797d --- /dev/null +++ b/tests/app/celery/test_csv_files/multiple_email.csv @@ -0,0 +1,11 @@ +EMAILADDRESS +test1@test.com +test2@test.com +test3@test.com +test4@test.com +test5@test.com +test6@test.com +test7@test.com +test8@test.com +test9@test.com +test0@test.com diff --git a/tests/app/celery/test_csv_files/multiple_letter.csv b/tests/app/celery/test_csv_files/multiple_letter.csv new file mode 100644 index 000000000..0e9d847b8 --- /dev/null +++ b/tests/app/celery/test_csv_files/multiple_letter.csv @@ -0,0 +1,11 @@ +address_line_1, address_line_2, address_line_3 +name1, street1, town1, postcode1 +name2, street2, town2, postcode2 +name3, street3, town3, postcode3 +name4, street4, town4, postcode4 +name5, street5, town5, postcode5 +name6, street6, town6, postcode6 +name7, street7, town7, postcode7 +name8, street8, town8, postcode8 +name9, street9, town9, postcode9 +name0, street0, town0, postcode0 \ No newline at end of file diff --git a/tests/app/celery/test_csv_files/multiple_sms.csv b/tests/app/celery/test_csv_files/multiple_sms.csv new file mode 100644 index 000000000..2ecad9140 --- /dev/null +++ b/tests/app/celery/test_csv_files/multiple_sms.csv @@ -0,0 +1,11 @@ +PhoneNumber,Name ++441234123121,chris ++441234123122,chris ++441234123123,chris ++441234123124,chris ++441234123125,chris ++441234123126,chris ++441234123127,chris ++441234123128,chris ++441234123129,chris ++441234123120,chris diff --git a/tests/app/celery/test_csv_files/sms.csv b/tests/app/celery/test_csv_files/sms.csv new file mode 100644 index 000000000..728639972 --- /dev/null +++ b/tests/app/celery/test_csv_files/sms.csv @@ -0,0 +1,2 @@ +PHONE NUMBER, IGNORE THIS COLUMN ++441234123123, nope diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 7f60318cc..6f63f7396 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -56,19 +56,16 @@ from app.models import ( from app.utils import get_london_midnight_in_utc from app.v2.errors import JobIncompleteError from tests.app.db import create_notification, create_service, create_template, create_job, create_rate + from tests.app.conftest import ( sample_job as create_sample_job, + sample_notification_history as create_notification_history, + sample_template as create_sample_template, create_custom_template, datetime_in_past ) from tests.app.aws.test_s3 import single_s3_object_stub -from tests.conftest import set_config_values, notify_db, notify_db_session - -from tests.app.conftest import ( - sample_notification as create_notification, - sample_notification_history as create_notification_history, - sample_template as create_sample_template -) +from tests.conftest import set_config_values def _create_slow_delivery_notification(provider='mmg'): diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index bc5a8af82..8a28e13dd 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -31,7 +31,8 @@ from app.dao.services_dao import ( dao_resume_service, dao_fetch_active_users_for_service, dao_fetch_service_by_inbound_number, - dao_fetch_monthly_historical_stats_by_template_for_service_without_status) + dao_fetch_monthly_historical_stats_by_template +) from app.dao.service_permissions_dao import dao_add_service_permission, dao_remove_service_permission from app.dao.users_dao import save_model_user from app.models import ( @@ -1022,7 +1023,7 @@ def test_dao_fetch_monthly_historical_stats_by_template(notify_db, notify_db_ses notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) notification_history(created_at=datetime.now(), sample_template=template_two) - results = dao_fetch_monthly_historical_stats_by_template_for_service_without_status() + results = dao_fetch_monthly_historical_stats_by_template() assert len(results) == 2 From b78d989d4eb144760d7e1deb8fb65f1a2111c1a4 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 9 Nov 2017 14:13:42 +0000 Subject: [PATCH 8/9] Updates after review - Modified the services_dao to return an int instead of a datetime to make usage easier and removed the BST function on year as it is not relevant for year - Improved tests do there is less logic by ordering the result so there is less reliance on the template id - Renamed variable in stats_template_usage_by_month_dao.py to make it consistent with the method --- app/celery/scheduled_tasks.py | 6 +- app/config.py | 2 +- app/dao/services_dao.py | 8 +- app/dao/stats_template_usage_by_month_dao.py | 4 +- app/utils.py | 18 +-- .../versions/0135_stats_template_usage.py | 14 --- tests/app/celery/test_csv_files/email.csv | 2 - tests/app/celery/test_csv_files/empty.csv | 1 - .../celery/test_csv_files/multiple_email.csv | 11 -- .../celery/test_csv_files/multiple_letter.csv | 11 -- .../celery/test_csv_files/multiple_sms.csv | 11 -- tests/app/celery/test_csv_files/sms.csv | 2 - tests/app/celery/test_scheduled_tasks.py | 104 +++++++++--------- tests/app/dao/test_services_dao.py | 30 +++-- 14 files changed, 76 insertions(+), 148 deletions(-) delete mode 100644 tests/app/celery/test_csv_files/email.csv delete mode 100644 tests/app/celery/test_csv_files/empty.csv delete mode 100644 tests/app/celery/test_csv_files/multiple_email.csv delete mode 100644 tests/app/celery/test_csv_files/multiple_letter.csv delete mode 100644 tests/app/celery/test_csv_files/multiple_sms.csv delete mode 100644 tests/app/celery/test_csv_files/sms.csv diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 9b1a830d9..e39876df0 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -410,13 +410,13 @@ def check_job_status(): @notify_celery.task(name='daily-stats-template_usage_by_month') @statsd(namespace="tasks") -def daily_stats_template_usage_my_month(): +def daily_stats_template_usage_by_month(): results = dao_fetch_monthly_historical_stats_by_template() for result in results: insert_or_update_stats_for_template( result.template_id, - result.month.month, - result.year.year, + result.month, + result.year, result.count ) diff --git a/app/config.py b/app/config.py index afd70dade..16892ac28 100644 --- a/app/config.py +++ b/app/config.py @@ -244,7 +244,7 @@ class Config(object): }, 'daily-stats-template_usage_by_month': { 'task': 'daily-stats-template_usage_by_month', - 'schedule': crontab(hour=00, minute=50), + 'schedule': crontab(hour=0, minute=50), 'options': {'queue': QueueNames.PERIODIC} } } diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 684696d83..d023fb297 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -40,7 +40,7 @@ from app.models import ( ) from app.service.statistics import format_monthly_template_notification_stats from app.statsd_decorators import statsd -from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc, get_london_year_from_utc_column +from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc from app.dao.annual_billing_dao import dao_insert_annual_billing DEFAULT_SERVICE_PERMISSIONS = [ @@ -525,13 +525,13 @@ def dao_fetch_active_users_for_service(service_id): @statsd(namespace="dao") def dao_fetch_monthly_historical_stats_by_template(): month = get_london_month_from_utc_column(NotificationHistory.created_at) - year = get_london_year_from_utc_column(NotificationHistory.created_at) + year = func.date_trunc("year", NotificationHistory.created_at) end_date = datetime.combine(date.today(), time.min) return db.session.query( NotificationHistory.template_id, - month.label('month'), - year.label('year'), + extract('month', month).label('month'), + extract('year', year).label('year'), func.count().label('count') ).filter( NotificationHistory.created_at < end_date diff --git a/app/dao/stats_template_usage_by_month_dao.py b/app/dao/stats_template_usage_by_month_dao.py index ca94dd83f..2884e2c53 100644 --- a/app/dao/stats_template_usage_by_month_dao.py +++ b/app/dao/stats_template_usage_by_month_dao.py @@ -15,10 +15,10 @@ def insert_or_update_stats_for_template(template_id, month, year, count): } ) if result == 0: - new_sms_sender = StatsTemplateUsageByMonth( + monthly_stats = StatsTemplateUsageByMonth( template_id=template_id, month=month, year=year, count=count ) - db.session.add(new_sms_sender) + db.session.add(monthly_stats) diff --git a/app/utils.py b/app/utils.py index 7fe4bdf89..ffb628284 100644 --- a/app/utils.py +++ b/app/utils.py @@ -71,23 +71,7 @@ def get_london_month_from_utc_column(column): """ return func.date_trunc( "month", - func.timezone("Europe/London", func.timezone("UTC", column)) - ) - - -def get_london_year_from_utc_column(column): - """ - Where queries need to count notifications by month it needs to be - the month in BST (British Summer Time). - The database stores all timestamps as UTC without the timezone. - - First set the timezone on created_at to UTC - - then convert the timezone to BST (or Europe/London) - - lastly truncate the datetime to month with which we can group - queries - """ - return func.date_trunc( - "month", - func.timezone("Europe/London", func.timezone("UTC", column)) + func.timezone("Europe/London", column) ) diff --git a/migrations/versions/0135_stats_template_usage.py b/migrations/versions/0135_stats_template_usage.py index 722fc69ce..5a8f5ef7a 100644 --- a/migrations/versions/0135_stats_template_usage.py +++ b/migrations/versions/0135_stats_template_usage.py @@ -31,20 +31,6 @@ def upgrade(): def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.create_table('notification_statistics', - sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), - sa.Column('service_id', postgresql.UUID(), autoincrement=False, nullable=False), - sa.Column('emails_requested', sa.BIGINT(), autoincrement=False, nullable=False), - sa.Column('emails_delivered', sa.BIGINT(), autoincrement=False, nullable=False), - sa.Column('emails_failed', sa.BIGINT(), autoincrement=False, nullable=False), - sa.Column('sms_requested', sa.BIGINT(), autoincrement=False, nullable=False), - sa.Column('sms_delivered', sa.BIGINT(), autoincrement=False, nullable=False), - sa.Column('sms_failed', sa.BIGINT(), autoincrement=False, nullable=False), - sa.Column('day', sa.DATE(), autoincrement=False, nullable=False), - sa.ForeignKeyConstraint(['service_id'], ['services.id'], name='notification_statistics_service_id_fkey'), - sa.PrimaryKeyConstraint('id', name='notification_statistics_pkey'), - sa.UniqueConstraint('service_id', 'day', name='uix_service_to_day') - ) op.drop_index(op.f('ix_stats_template_usage_by_month_year'), table_name='stats_template_usage_by_month') op.drop_index(op.f('ix_stats_template_usage_by_month_template_id'), table_name='stats_template_usage_by_month') op.drop_index(op.f('ix_stats_template_usage_by_month_month'), table_name='stats_template_usage_by_month') diff --git a/tests/app/celery/test_csv_files/email.csv b/tests/app/celery/test_csv_files/email.csv deleted file mode 100644 index f0cefee69..000000000 --- a/tests/app/celery/test_csv_files/email.csv +++ /dev/null @@ -1,2 +0,0 @@ -email_address -test@test.com diff --git a/tests/app/celery/test_csv_files/empty.csv b/tests/app/celery/test_csv_files/empty.csv deleted file mode 100644 index 64e5bd0a3..000000000 --- a/tests/app/celery/test_csv_files/empty.csv +++ /dev/null @@ -1 +0,0 @@ -phone number diff --git a/tests/app/celery/test_csv_files/multiple_email.csv b/tests/app/celery/test_csv_files/multiple_email.csv deleted file mode 100644 index 5da15797d..000000000 --- a/tests/app/celery/test_csv_files/multiple_email.csv +++ /dev/null @@ -1,11 +0,0 @@ -EMAILADDRESS -test1@test.com -test2@test.com -test3@test.com -test4@test.com -test5@test.com -test6@test.com -test7@test.com -test8@test.com -test9@test.com -test0@test.com diff --git a/tests/app/celery/test_csv_files/multiple_letter.csv b/tests/app/celery/test_csv_files/multiple_letter.csv deleted file mode 100644 index 0e9d847b8..000000000 --- a/tests/app/celery/test_csv_files/multiple_letter.csv +++ /dev/null @@ -1,11 +0,0 @@ -address_line_1, address_line_2, address_line_3 -name1, street1, town1, postcode1 -name2, street2, town2, postcode2 -name3, street3, town3, postcode3 -name4, street4, town4, postcode4 -name5, street5, town5, postcode5 -name6, street6, town6, postcode6 -name7, street7, town7, postcode7 -name8, street8, town8, postcode8 -name9, street9, town9, postcode9 -name0, street0, town0, postcode0 \ No newline at end of file diff --git a/tests/app/celery/test_csv_files/multiple_sms.csv b/tests/app/celery/test_csv_files/multiple_sms.csv deleted file mode 100644 index 2ecad9140..000000000 --- a/tests/app/celery/test_csv_files/multiple_sms.csv +++ /dev/null @@ -1,11 +0,0 @@ -PhoneNumber,Name -+441234123121,chris -+441234123122,chris -+441234123123,chris -+441234123124,chris -+441234123125,chris -+441234123126,chris -+441234123127,chris -+441234123128,chris -+441234123129,chris -+441234123120,chris diff --git a/tests/app/celery/test_csv_files/sms.csv b/tests/app/celery/test_csv_files/sms.csv deleted file mode 100644 index 728639972..000000000 --- a/tests/app/celery/test_csv_files/sms.csv +++ /dev/null @@ -1,2 +0,0 @@ -PHONE NUMBER, IGNORE THIS COLUMN -+441234123123, nope diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 6f63f7396..d31e2b5a4 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -33,7 +33,8 @@ from app.celery.scheduled_tasks import ( switch_current_sms_provider_on_slow_delivery, timeout_job_statistics, timeout_notifications, - daily_stats_template_usage_my_month) + daily_stats_template_usage_by_month +) from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient from app.config import QueueNames, TaskNames from app.dao.jobs_dao import dao_get_job_by_id @@ -840,7 +841,7 @@ def test_check_job_status_task_raises_job_incomplete_error_for_multiple_jobs(moc ) -def test_daily_stats_template_usage_my_month(notify_db, notify_db_session): +def test_daily_stats_template_usage_by_month(notify_db, notify_db_session): notification_history = functools.partial( create_notification_history, notify_db, @@ -856,36 +857,37 @@ def test_daily_stats_template_usage_my_month(notify_db, notify_db_session): notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) notification_history(created_at=datetime.now(), sample_template=template_two) - daily_stats_template_usage_my_month() + daily_stats_template_usage_by_month() - results = db.session.query(StatsTemplateUsageByMonth).all() + result = db.session.query( + StatsTemplateUsageByMonth + ).order_by( + StatsTemplateUsageByMonth.year, + StatsTemplateUsageByMonth.month + ).all() - assert len(results) == 2 + assert len(result) == 2 - for result in results: - if result.template_id == template_one.id: - assert result.template_id == template_one.id - assert result.month == 10 - assert result.year == 2017 - assert result.count == 1 - elif result.template_id == template_two.id: - assert result.template_id == template_two.id - assert result.month == 4 - assert result.year == 2016 - assert result.count == 2 - else: - raise AssertionError() + assert result[0].template_id == template_two.id + assert result[0].month == 4 + assert result[0].year == 2016 + assert result[0].count == 2 + + assert result[1].template_id == template_one.id + assert result[1].month == 10 + assert result[1].year == 2017 + assert result[1].count == 1 -def test_daily_stats_template_usage_my_month_no_data(): - daily_stats_template_usage_my_month() +def test_daily_stats_template_usage_by_month_no_data(): + daily_stats_template_usage_by_month() results = db.session.query(StatsTemplateUsageByMonth).all() assert len(results) == 0 -def test_daily_stats_template_usage_my_month_multiple_runs(notify_db, notify_db_session): +def test_daily_stats_template_usage_by_month_multiple_runs(notify_db, notify_db_session): notification_history = functools.partial( create_notification_history, notify_db, @@ -896,12 +898,12 @@ def test_daily_stats_template_usage_my_month_multiple_runs(notify_db, notify_db_ template_one = create_sample_template(notify_db, notify_db_session) template_two = create_sample_template(notify_db, notify_db_session) - notification_history(created_at=datetime(2017, 10, 1), sample_template=template_one) + notification_history(created_at=datetime(2017, 11, 1), sample_template=template_one) notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) notification_history(created_at=datetime.now(), sample_template=template_two) - daily_stats_template_usage_my_month() + daily_stats_template_usage_by_month() template_three = create_sample_template(notify_db, notify_db_session) @@ -911,35 +913,33 @@ def test_daily_stats_template_usage_my_month_multiple_runs(notify_db, notify_db_ notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) notification_history(created_at=datetime.now(), sample_template=template_two) - daily_stats_template_usage_my_month() + daily_stats_template_usage_by_month() - results = db.session.query(StatsTemplateUsageByMonth).all() + result = db.session.query( + StatsTemplateUsageByMonth + ).order_by( + StatsTemplateUsageByMonth.year, + StatsTemplateUsageByMonth.month + ).all() - assert len(results) == 4 + assert len(result) == 4 - for result in results: - if result.template_id == template_one.id: - assert result.template_id == template_one.id - assert result.month == 10 - assert result.year == 2017 - assert result.count == 1 - elif result.template_id == template_two.id: - assert result.template_id == template_two.id - assert result.month == 4 - assert result.year == 2016 - assert result.count == 4 - elif result.template_id == template_three.id: - if result.month == 10: - assert result.template_id == template_three.id - assert result.month == 10 - assert result.year == 2017 - assert result.count == 1 - elif result.month == 9: - assert result.template_id == template_three.id - assert result.month == 9 - assert result.year == 2017 - assert result.count == 1 - else: - raise AssertionError() - else: - raise AssertionError() + assert result[0].template_id == template_two.id + assert result[0].month == 4 + assert result[0].year == 2016 + assert result[0].count == 4 + + assert result[1].template_id == template_three.id + assert result[1].month == 9 + assert result[1].year == 2017 + assert result[1].count == 1 + + assert result[2].template_id == template_three.id + assert result[2].month == 10 + assert result[2].year == 2017 + assert result[2].count == 1 + + assert result[3].template_id == template_one.id + assert result[3].month == 11 + assert result[3].year == 2017 + assert result[3].count == 1 diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 8a28e13dd..997301531 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1015,28 +1015,24 @@ def test_dao_fetch_monthly_historical_stats_by_template(notify_db, notify_db_ses status='delivered' ) - template_one = create_sample_template(notify_db, notify_db_session) - template_two = create_sample_template(notify_db, notify_db_session) + template_one = create_sample_template(notify_db, notify_db_session, template_name='1') + template_two = create_sample_template(notify_db, notify_db_session, template_name='2') notification_history(created_at=datetime(2017, 10, 1), sample_template=template_one) notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) notification_history(created_at=datetime.now(), sample_template=template_two) - results = dao_fetch_monthly_historical_stats_by_template() + result = sorted(dao_fetch_monthly_historical_stats_by_template(), key=lambda x: (x.month, x.year)) - assert len(results) == 2 + assert len(result) == 2 - for result in results: - if result.template_id == template_one.id: - assert result.template_id == template_one.id - assert result.month.month == 10 - assert result.year.year == 2017 - assert result.count == 1 - elif result.template_id == template_two.id: - assert result.template_id == template_two.id - assert result.month.month == 4 - assert result.year.year == 2016 - assert result.count == 2 - else: - raise AssertionError() + assert result[0].template_id == template_two.id + assert result[0].month == 4 + assert result[0].year == 2016 + assert result[0].count == 2 + + assert result[1].template_id == template_one.id + assert result[1].month == 10 + assert result[1].year == 2017 + assert result[1].count == 1 From 15e86170d9eb94c2fdbeb7ea617cdabfc50a0d94 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 9 Nov 2017 14:55:45 +0000 Subject: [PATCH 9/9] Changed timezone back Reverted back the timezoe change that was made in error and was making two tests fail. --- app/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/utils.py b/app/utils.py index ffb628284..49f6d61dc 100644 --- a/app/utils.py +++ b/app/utils.py @@ -71,7 +71,7 @@ def get_london_month_from_utc_column(column): """ return func.date_trunc( "month", - func.timezone("Europe/London", column) + func.timezone("Europe/London", func.timezone("UTC", column)) )