From 3b8a163ff24d3821153c08bc643e7a146abdb6b2 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Tue, 19 Jul 2016 16:46:17 +0100 Subject: [PATCH 1/9] Db migration to retrospectively create notification history from jobs table. The migration selects only jobs from after the go live date and before the earliest record in notication history table. --- .../0043_jobs_to_notification_hist.py | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 migrations/versions/0043_jobs_to_notification_hist.py diff --git a/migrations/versions/0043_jobs_to_notification_hist.py b/migrations/versions/0043_jobs_to_notification_hist.py new file mode 100644 index 000000000..2cc2dab79 --- /dev/null +++ b/migrations/versions/0043_jobs_to_notification_hist.py @@ -0,0 +1,79 @@ +"""empty message + +Revision ID: 0043_jobs_to_notification_hist +Revises: 0042_notification_history +Create Date: 2016-07-15 13:28:41.441009 + +""" + +# revision identifiers, used by Alembic. +revision = '0043_jobs_to_notification_hist' +down_revision = '0042_notification_history' + +from alembic import op + +from sqlalchemy.orm.session import Session + +import uuid +import datetime +from app.models import Job, Template, NotificationHistory + + +def upgrade(): + + session = Session(bind=op.get_bind()) + + go_live = datetime.datetime.strptime('2016-05-18', '%Y-%m-%d') + notifications_history_start_date = datetime.datetime.strptime('2016-06-26 23:21:55', '%Y-%m-%d %H:%M:%S') + jobs = session.query(Job).join(Template).filter(Job.created_at >= go_live, + Job.created_at < notifications_history_start_date).all() + + for job in jobs: + for i in range(0, job.notifications_delivered): + notification = NotificationHistory(id=uuid.uuid4(), + job_id=job.id, + service_id=job.service_id, + template_id=job.template.id, + template_version=job.template_version, + key_type='normal', + content_char_count=len(job.template.content), + notification_type=job.template.template_type, + created_at=job.created_at, + sent_at=job.processing_finished, + sent_by='ses' if job.template.template_type == 'email' else 'mmg', + status='delivered') + + session.add(notification) + session.commit() + + for i in range(0, job.notifications_failed): + notification = NotificationHistory(id=uuid.uuid4(), + job_id=job.id, + service_id=job.service_id, + template_id=job.template.id, + template_version=job.template_version, + key_type='normal', + content_char_count=len(job.template.content), + notification_type=job.template.template_type, + created_at=job.created_at, + sent_at=job.processing_finished, + sent_by='ses' if job.template.template_type == 'email' else 'mmg', + status='permanent-failure') + + session.add(notification) + session.commit() + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + session = Session(bind=op.get_bind()) + + go_live = datetime.datetime.strptime('2016-05-18', '%Y-%m-%d') + notifications_history_start_date = datetime.datetime.strptime('2016-06-26 23:21:55', '%Y-%m-%d %H:%M:%S') + + session.query(NotificationHistory).filter(NotificationHistory.created_at >= go_live, + NotificationHistory.created_at < + notifications_history_start_date).delete() + + session.commit() + ### end Alembic commands ### From fd96c854e1bd6c0fff9e6524235f38fcf4c504c3 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 22 Jul 2016 15:16:24 +0100 Subject: [PATCH 2/9] add dao function to get notification stats for a server for toady only --- app/dao/services_dao.py | 13 ++++++++++++- tests/app/conftest.py | 4 +++- tests/app/dao/test_services_dao.py | 23 ++++++++++++++++++++++- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 7815a0656..f37e2f94d 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,4 +1,5 @@ import uuid +from datetime import date from sqlalchemy import asc, func from sqlalchemy.orm import joinedload @@ -136,6 +137,16 @@ def delete_service_and_all_associated_db_objects(service): def dao_fetch_stats_for_service(service_id): + return _stats_for_service_query(service_id).all() + + +def dao_fetch_todays_stats_for_service(service_id): + return _stats_for_service_query(service_id).filter( + func.date(Notification.created_at) == date.today() + ).all() + + +def _stats_for_service_query(service_id): return db.session.query( Notification.notification_type, Notification.status, @@ -145,4 +156,4 @@ def dao_fetch_stats_for_service(service_id): ).group_by( Notification.notification_type, Notification.status, - ).all() + ) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index f79088c9b..e6295860d 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -319,12 +319,14 @@ def sample_notification(notify_db, to_field=None, status='created', reference=None, - created_at=datetime.utcnow(), + created_at=None, content_char_count=160, create=True, personalisation=None, api_key_id=None, key_type=KEY_TYPE_NORMAL): + if created_at is None: + created_at = datetime.utcnow() if service is None: service = sample_service(notify_db, notify_db_session) if template is None: diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 86bfa7a82..dcf538a43 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -3,6 +3,7 @@ import pytest from sqlalchemy.orm.exc import FlushError, NoResultFound from sqlalchemy.exc import IntegrityError +from freezegun import freeze_time from app import db from app.dao.services_dao import ( @@ -14,7 +15,8 @@ from app.dao.services_dao import ( dao_fetch_all_services_by_user, dao_update_service, delete_service_and_all_associated_db_objects, - dao_fetch_stats_for_service + dao_fetch_stats_for_service, + dao_fetch_todays_stats_for_service ) from app.dao.users_dao import save_model_user from app.models import ( @@ -420,3 +422,22 @@ def test_fetch_stats_counts_correctly(notify_db, notify_db_session, sample_templ assert stats[2].notification_type == 'sms' assert stats[2].status == 'created' assert stats[2].count == 1 + + +def test_fetch_stats_for_today_only_includes_today(notify_db, notify_db_session, sample_template): + # two created email, one failed email, and one created sms + with freeze_time('2001-01-01T23:59:00'): + just_before_midnight_yesterday = create_notification(notify_db, None, to_field='1', status='delivered') + + with freeze_time('2001-01-02T00:01:00'): + just_after_midnight_today = create_notification(notify_db, None, to_field='2', status='failed') + + with freeze_time('2001-01-02T12:00:00'): + right_now = create_notification(notify_db, None, to_field='3', status='created') + + stats = dao_fetch_todays_stats_for_service(sample_template.service.id) + + stats = {row.status: row.count for row in stats} + assert 'delivered' not in stats + assert stats['failed'] == 1 + assert stats['created'] == 1 From 48eff9a2eebedef82cfac306a850013fbfc4ae45 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 25 Jul 2016 14:27:06 +0100 Subject: [PATCH 3/9] add today_only flag to GET /service/:id if both detailed=True and today_only=True are passed in, the stats returned will only be for today. if detailed is false or not specified, today_only has no effect --- app/service/rest.py | 12 +++++++----- tests/app/service/test_rest.py | 34 ++++++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 617ccb84d..c462f24be 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -22,7 +22,8 @@ from app.dao.services_dao import ( dao_fetch_all_services_by_user, dao_add_user_to_service, dao_remove_user_from_service, - dao_fetch_stats_for_service + dao_fetch_stats_for_service, + dao_fetch_todays_stats_for_service ) from app.dao import notifications_dao from app.dao.provider_statistics_dao import get_fragment_count @@ -60,8 +61,8 @@ def get_services(): @service.route('/', methods=['GET']) def get_service_by_id(service_id): - if 'detailed' in request.args: - return get_detailed_service(service_id) + if request.args.get('detailed') == 'True': + return get_detailed_service(service_id, today_only=request.args.get('today_only') == 'True') else: fetched = dao_fetch_service_by_id(service_id) @@ -235,9 +236,10 @@ def get_all_notifications_for_service(service_id): ), 200 -def get_detailed_service(service_id): +def get_detailed_service(service_id, today_only=False): service = dao_fetch_service_by_id(service_id) - statistics = dao_fetch_stats_for_service(service_id) + stats_fn = dao_fetch_todays_stats_for_service if today_only else dao_fetch_stats_for_service + statistics = stats_fn(service_id) service.statistics = format_statistics(statistics) data = detailed_service_schema.dump(service).data return jsonify(data=data) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 72f99f1ca..06784b4e0 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -4,6 +4,7 @@ import uuid import pytest from flask import url_for +from freezegun import freeze_time from app.dao.users_dao import save_model_user from app.dao.services_dao import dao_remove_user_from_service @@ -1094,23 +1095,36 @@ def test_set_sms_sender_for_service_rejects_invalid_characters(notify_api, sampl assert result['message'] == {'sms_sender': ['Only alphanumeric characters allowed']} -def test_get_detailed_service(notify_api, sample_service, sample_notification): +@pytest.mark.parametrize('today_only,stats', [ + ('False', { + 'requested': 2, + 'delivered': 1, + 'failed': 0 + }), + ('True', { + 'requested': 1, + 'delivered': 0, + 'failed': 0 + }) + ], ids=['seven_days', 'today'] +) +def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_service, today_only, stats): with notify_api.test_request_context(), notify_api.test_client() as client: - resp = client.get( - '/service/{}?detailed=False'.format(sample_service.id), - headers=[create_authorization_header()] - ) + with freeze_time('2000-01-01T12:00:00'): + create_sample_notification(notify_db, notify_db_session, status='delivered') + with freeze_time('2000-01-02T12:00:00'): + create_sample_notification(notify_db, notify_db_session, status='created') + resp = client.get( + '/service/{}?detailed=True&today_only={}'.format(sample_service.id, today_only), + headers=[create_authorization_header()] + ) assert resp.status_code == 200 service = json.loads(resp.get_data(as_text=True))['data'] assert service['id'] == str(sample_service.id) assert 'statistics' in service.keys() assert set(service['statistics'].keys()) == set(['sms', 'email']) - assert service['statistics']['sms'] == { - 'requested': 1, - 'delivered': 0, - 'failed': 0 - } + assert service['statistics']['sms'] == stats # email_counts and sms_counts are 3-tuple of requested, delivered, failed From 183fc7d639ba89a1d26305b1ca9081eadd71bd3d Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Tue, 26 Jul 2016 16:23:36 +0100 Subject: [PATCH 4/9] After doing some tests locally. To get this to run in a reasonable amount of time I've made the following changes. Only migrate jobs for the Digital Marketplace service. Batch up commits to run per job. --- migrations/versions/0043_jobs_to_notification_hist.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/migrations/versions/0043_jobs_to_notification_hist.py b/migrations/versions/0043_jobs_to_notification_hist.py index 2cc2dab79..0cad158b4 100644 --- a/migrations/versions/0043_jobs_to_notification_hist.py +++ b/migrations/versions/0043_jobs_to_notification_hist.py @@ -25,7 +25,8 @@ def upgrade(): go_live = datetime.datetime.strptime('2016-05-18', '%Y-%m-%d') notifications_history_start_date = datetime.datetime.strptime('2016-06-26 23:21:55', '%Y-%m-%d %H:%M:%S') - jobs = session.query(Job).join(Template).filter(Job.created_at >= go_live, + jobs = session.query(Job).join(Template).filter(Job.service_id == '95316ff0-e555-462d-a6e7-95d26fbfd091', + Job.created_at >= go_live, Job.created_at < notifications_history_start_date).all() for job in jobs: @@ -44,7 +45,6 @@ def upgrade(): status='delivered') session.add(notification) - session.commit() for i in range(0, job.notifications_failed): notification = NotificationHistory(id=uuid.uuid4(), @@ -59,9 +59,8 @@ def upgrade(): sent_at=job.processing_finished, sent_by='ses' if job.template.template_type == 'email' else 'mmg', status='permanent-failure') - session.add(notification) - session.commit() + session.commit() def downgrade(): From d8ab82b1396f3ffa1418b7538470240133c70bb6 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 29 Jul 2016 08:38:18 +0100 Subject: [PATCH 5/9] Fix Markdown/HTML email oddities Depends on: - [ ] https://github.com/alphagov/notifications-utils/pull/59 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 4a560ab81..660110ada 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,4 +23,4 @@ statsd==3.2.1 git+https://github.com/alphagov/notifications-python-client.git@1.0.0#egg=notifications-python-client==1.0.0 -git+https://github.com/alphagov/notifications-utils.git@8.6.0#egg=notifications-utils==8.6.0 +git+https://github.com/alphagov/notifications-utils.git@8.7.1#egg=notifications-utils==8.7.1 From 6edb9324da339487ea5fb68f4c93c570f6f9a5ad Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 1 Aug 2016 11:34:20 +0100 Subject: [PATCH 6/9] add indexes to notifications and notification_history --- .../versions/0043_notification_indexes.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 migrations/versions/0043_notification_indexes.py diff --git a/migrations/versions/0043_notification_indexes.py b/migrations/versions/0043_notification_indexes.py new file mode 100644 index 000000000..93fb3a19c --- /dev/null +++ b/migrations/versions/0043_notification_indexes.py @@ -0,0 +1,44 @@ +"""empty message + +Revision ID: 0043_notification_indexes +Revises: 0042_notification_history +Create Date: 2016-08-01 10:37:41.198070 + +""" + +# revision identifiers, used by Alembic. +revision = '0043_notification_indexes' +down_revision = '0042_notification_history' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.create_index(op.f('ix_notifications_created_at'), 'notifications', ['created_at']) + op.create_index(op.f('ix_notification_history_created_at'), 'notification_history', ['created_at']) + + op.create_index(op.f('ix_notifications_status'), 'notifications', ['status']) + op.create_index(op.f('ix_notification_history_status'), 'notification_history', ['status']) + + op.create_index(op.f('ix_notifications_notification_type'), 'notifications', ['notification_type']) + op.create_index(op.f('ix_notification_history_notification_type'), 'notification_history', ['notification_type']) + + op.create_index( + 'ix_notification_history_week_created', + 'notification_history', + [sa.text("date_trunc('week', created_at)")] + ) + + +def downgrade(): + op.drop_index(op.f('ix_notifications_created_at'), table_name='notifications') + op.drop_index(op.f('ix_notification_history_created_at'), table_name='notification_history') + + op.drop_index(op.f('ix_notifications_status'), table_name='notifications') + op.drop_index(op.f('ix_notification_history_status'), table_name='notification_history') + + op.drop_index(op.f('ix_notifications_notification_type'), table_name='notifications') + op.drop_index(op.f('ix_notification_history_notification_type'), table_name='notification_history') + + op.drop_index(op.f('ix_notification_history_week_created'), table_name='notification_history') From cfb29d440417e5bb3cc43f170ae03a25136c2f98 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Mon, 1 Aug 2016 12:20:06 +0100 Subject: [PATCH 7/9] Ensure the downgrade script targets only DMP services --- migrations/versions/0043_jobs_to_notification_hist.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/migrations/versions/0043_jobs_to_notification_hist.py b/migrations/versions/0043_jobs_to_notification_hist.py index 0cad158b4..64069d178 100644 --- a/migrations/versions/0043_jobs_to_notification_hist.py +++ b/migrations/versions/0043_jobs_to_notification_hist.py @@ -20,7 +20,6 @@ from app.models import Job, Template, NotificationHistory def upgrade(): - session = Session(bind=op.get_bind()) go_live = datetime.datetime.strptime('2016-05-18', '%Y-%m-%d') @@ -70,9 +69,10 @@ def downgrade(): go_live = datetime.datetime.strptime('2016-05-18', '%Y-%m-%d') notifications_history_start_date = datetime.datetime.strptime('2016-06-26 23:21:55', '%Y-%m-%d %H:%M:%S') - session.query(NotificationHistory).filter(NotificationHistory.created_at >= go_live, - NotificationHistory.created_at < - notifications_history_start_date).delete() + session.query(NotificationHistory).filter( + NotificationHistory.created_at >= go_live, + NotificationHistory.service_id == '95316ff0-e555-462d-a6e7-95d26fbfd091', + NotificationHistory.created_at < notifications_history_start_date).delete() session.commit() ### end Alembic commands ### From 482a435e66bda1ab7d22df5f3614d78fd86fb938 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 1 Aug 2016 14:23:10 +0100 Subject: [PATCH 8/9] Fix newlines in HTML emails Implements: - [ ] https://github.com/alphagov/notifications-utils/pull/60 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 660110ada..751797f28 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,4 +23,4 @@ statsd==3.2.1 git+https://github.com/alphagov/notifications-python-client.git@1.0.0#egg=notifications-python-client==1.0.0 -git+https://github.com/alphagov/notifications-utils.git@8.7.1#egg=notifications-utils==8.7.1 +git+https://github.com/alphagov/notifications-utils.git@8.7.2#egg=notifications-utils==8.7.2 From daf85e7787d64ecfb0f7d412f48a0b5c7a8c1c58 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Mon, 1 Aug 2016 15:19:16 +0100 Subject: [PATCH 9/9] Reorder the SQL scripts as clash --- ...fication_hist.py => 0044_jobs_to_notification_hist.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0043_jobs_to_notification_hist.py => 0044_jobs_to_notification_hist.py} (95%) diff --git a/migrations/versions/0043_jobs_to_notification_hist.py b/migrations/versions/0044_jobs_to_notification_hist.py similarity index 95% rename from migrations/versions/0043_jobs_to_notification_hist.py rename to migrations/versions/0044_jobs_to_notification_hist.py index 64069d178..dc4df6770 100644 --- a/migrations/versions/0043_jobs_to_notification_hist.py +++ b/migrations/versions/0044_jobs_to_notification_hist.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0043_jobs_to_notification_hist -Revises: 0042_notification_history +Revision ID: 0044_jobs_to_notification_hist +Revises: 0043_notification_indexes Create Date: 2016-07-15 13:28:41.441009 """ # revision identifiers, used by Alembic. -revision = '0043_jobs_to_notification_hist' -down_revision = '0042_notification_history' +revision = '0044_jobs_to_notification_hist' +down_revision = '0043_notification_indexes' from alembic import op