From f065b08db241183fef4708e1ca3b072631333f75 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 11 Aug 2016 16:08:28 +0100 Subject: [PATCH] remove unused dao functions --- app/__init__.py | 2 +- app/dao/notifications_dao.py | 46 +------ tests/app/dao/test_notification_dao.py | 69 +--------- .../dao/test_notification_statistics_dao.py | 122 ------------------ .../app/notifications/rest/test_callbacks.py | 24 ++-- 5 files changed, 18 insertions(+), 245 deletions(-) delete mode 100644 tests/app/dao/test_notification_statistics_dao.py diff --git a/app/__init__.py b/app/__init__.py index 029bc72a5..e0928a4b9 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -80,7 +80,7 @@ def create_app(app_name=None): application.register_blueprint(job_blueprint) application.register_blueprint(invite_blueprint) application.register_blueprint(accept_invite, url_prefix='/invite') - + application.register_blueprint(template_statistics_blueprint) application.register_blueprint(events_blueprint) application.register_blueprint(provider_details_blueprint, url_prefix='/provider-details') diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index d25adac3a..09a101ccd 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -34,31 +34,15 @@ from app.dao.dao_utils import transactional from app.statsd_decorators import statsd -@statsd(namespace="dao") -def dao_get_notification_statistics_for_service(service_id, limit_days=None): - query_filter = [NotificationStatistics.service_id == service_id] - if limit_days is not None: - query_filter.append(NotificationStatistics.day >= days_ago(limit_days)) - return NotificationStatistics.query.filter( - *query_filter - ).order_by( - desc(NotificationStatistics.day) - ).all() - - @statsd(namespace="dao") def dao_get_notification_statistics_for_service_and_day(service_id, day): + # only used by stat-updating code in tasks.py return NotificationStatistics.query.filter_by( service_id=service_id, day=day ).order_by(desc(NotificationStatistics.day)).first() -@statsd(namespace="dao") -def dao_get_notification_statistics_for_day(day): - return NotificationStatistics.query.filter_by(day=day).all() - - @statsd(namespace="dao") def dao_get_potential_notification_statistics_for_day(day): all_services = db.session.query( @@ -107,34 +91,6 @@ def create_notification_statistics_dict(service_id, day): } -@statsd(namespace="dao") -def dao_get_7_day_agg_notification_statistics_for_service(service_id, - date_from, - week_count=52): - doy = date_from.timetuple().tm_yday - return db.session.query( - cast(func.floor((func.extract('doy', NotificationStatistics.day) - doy) / 7), Integer), - cast(func.sum(NotificationStatistics.emails_requested), Integer), - cast(func.sum(NotificationStatistics.emails_delivered), Integer), - cast(func.sum(NotificationStatistics.emails_failed), Integer), - cast(func.sum(NotificationStatistics.sms_requested), Integer), - cast(func.sum(NotificationStatistics.sms_delivered), Integer), - cast(func.sum(NotificationStatistics.sms_failed), Integer) - ).filter( - NotificationStatistics.service_id == service_id - ).filter( - NotificationStatistics.day >= date_from - ).filter( - NotificationStatistics.day < date_from + timedelta(days=7 * week_count) - ).group_by( - func.floor(((func.extract('doy', NotificationStatistics.day) - doy) / 7)) - ).order_by( - desc(func.floor(((func.extract('doy', NotificationStatistics.day) - doy) / 7))) - ).limit( - week_count - ) - - @statsd(namespace="dao") def dao_get_template_statistics_for_service(service_id, limit_days=None): query_filter = [TemplateStatistics.service_id == service_id] diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 40c5228a2..a5dc571af 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -26,15 +26,14 @@ from app.dao.notifications_dao import ( get_notification, get_notification_for_job, get_notifications_for_job, - dao_get_notification_statistics_for_service, delete_notifications_created_more_than_a_week_ago, dao_get_notification_statistics_for_service_and_day, update_notification_status_by_id, update_provider_stats, update_notification_status_by_reference, dao_get_template_statistics_for_service, - get_notifications_for_service, dao_get_7_day_agg_notification_statistics_for_service, - dao_get_potential_notification_statistics_for_day, dao_get_notification_statistics_for_day, + get_notifications_for_service, + dao_get_potential_notification_statistics_for_day, dao_get_template_statistics_for_template, get_notification_by_id) from notifications_utils.template import get_sms_fragment_count @@ -43,11 +42,8 @@ from tests.app.conftest import (sample_notification) def test_should_have_decorated_notifications_dao_functions(): - assert dao_get_notification_statistics_for_service.__wrapped__.__name__ == 'dao_get_notification_statistics_for_service' # noqa assert dao_get_notification_statistics_for_service_and_day.__wrapped__.__name__ == 'dao_get_notification_statistics_for_service_and_day' # noqa - assert dao_get_notification_statistics_for_day.__wrapped__.__name__ == 'dao_get_notification_statistics_for_day' # noqa assert dao_get_potential_notification_statistics_for_day.__wrapped__.__name__ == 'dao_get_potential_notification_statistics_for_day' # noqa - assert dao_get_7_day_agg_notification_statistics_for_service.__wrapped__.__name__ == 'dao_get_7_day_agg_notification_statistics_for_service' # noqa assert dao_get_template_statistics_for_service.__wrapped__.__name__ == 'dao_get_template_statistics_for_service' # noqa assert dao_get_template_statistics_for_template.__wrapped__.__name__ == 'dao_get_template_statistics_for_template' # noqa assert dao_create_notification.__wrapped__.__name__ == 'dao_create_notification' # noqa @@ -272,67 +268,6 @@ def test_should_be_able_to_get_all_statistics_for_a_service(sample_template, mmg _assert_notification_stats(sample_template.service.id, sms_requested=3) -def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days(sample_template, mmg_provider): - data = _notification_json(sample_template) - - today = datetime.utcnow() - yesterday = datetime.utcnow() - timedelta(days=1) - two_days_ago = datetime.utcnow() - timedelta(days=2) - data.update({'created_at': today}) - notification_1 = Notification(**data) - data.update({'created_at': yesterday}) - notification_2 = Notification(**data) - data.update({'created_at': two_days_ago}) - notification_3 = Notification(**data) - - dao_create_notification(notification_1, sample_template.template_type) - dao_create_notification(notification_2, sample_template.template_type) - dao_create_notification(notification_3, sample_template.template_type) - - stats = dao_get_notification_statistics_for_service(sample_template.service.id) - assert len(stats) == 3 - assert stats[0].emails_requested == 0 - assert stats[0].sms_requested == 1 - assert stats[0].day == today.date() - assert stats[1].emails_requested == 0 - assert stats[1].sms_requested == 1 - assert stats[1].day == yesterday.date() - assert stats[2].emails_requested == 0 - assert stats[2].sms_requested == 1 - assert stats[2].day == two_days_ago.date() - - -def test_should_be_empty_list_if_no_statistics_for_a_service(sample_service): - assert len(dao_get_notification_statistics_for_service(sample_service.id)) == 0 - - -def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days_previous(sample_template, - mmg_provider): - data = _notification_json(sample_template) - - today = datetime.utcnow() - seven_days_ago = datetime.utcnow() - timedelta(days=7) - eight_days_ago = datetime.utcnow() - timedelta(days=8) - data.update({'created_at': today}) - notification_1 = Notification(**data) - data.update({'created_at': seven_days_ago}) - notification_2 = Notification(**data) - data.update({'created_at': eight_days_ago}) - notification_3 = Notification(**data) - dao_create_notification(notification_1, sample_template.template_type) - dao_create_notification(notification_2, sample_template.template_type) - dao_create_notification(notification_3, sample_template.template_type) - - stats = dao_get_notification_statistics_for_service(sample_template.service.id, 7) - assert len(stats) == 2 - assert stats[0].emails_requested == 0 - assert stats[0].sms_requested == 1 - assert stats[0].day == today.date() - assert stats[1].emails_requested == 0 - assert stats[1].sms_requested == 1 - assert stats[1].day == seven_days_ago.date() - - def test_create_notification_creates_notification_with_personalisation(notify_db, notify_db_session, sample_template_with_placeholders, sample_job, mmg_provider): diff --git a/tests/app/dao/test_notification_statistics_dao.py b/tests/app/dao/test_notification_statistics_dao.py deleted file mode 100644 index b8ffe8445..000000000 --- a/tests/app/dao/test_notification_statistics_dao.py +++ /dev/null @@ -1,122 +0,0 @@ -from datetime import (date, timedelta) - -from app.models import NotificationStatistics -from tests.app.conftest import sample_notification_statistics as create_sample_notification_statistics -from app.dao.notifications_dao import dao_get_7_day_agg_notification_statistics_for_service - - -def test_display_weekly_notification_statistics_sum_over_week(notify_db, - notify_db_session, - sample_service): - fools = date(2016, 4, 1) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=fools - ) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=fools + timedelta(days=1) - ) - assert dao_get_7_day_agg_notification_statistics_for_service( - sample_service.id, - fools - ).all() == [(0, 4, 2, 2, 4, 2, 2)] - - -def test_display_weekly_notification_statistics_separate_over_weeks(notify_db, - notify_db_session, - sample_service): - fools = date(2016, 4, 1) - next_week = fools + timedelta(days=7) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=fools - ) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=next_week - ) - assert dao_get_7_day_agg_notification_statistics_for_service( - sample_service.id, - fools - ).all() == [(1, 2, 1, 1, 2, 1, 1), (0, 2, 1, 1, 2, 1, 1)] - - -def test_display_weekly_notification_statistics_7_days_from_date_from(notify_db, - notify_db_session, - sample_service): - fools = date(2016, 4, 1) - eow_fools = fools + timedelta(days=6) - next_week = fools + timedelta(days=7) - two_weeks_later = fools + timedelta(days=14) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=fools - ) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=eow_fools - ) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=next_week - ) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=two_weeks_later - ) - assert dao_get_7_day_agg_notification_statistics_for_service( - sample_service.id, - fools - ).all() == [(2, 2, 1, 1, 2, 1, 1), (1, 2, 1, 1, 2, 1, 1), (0, 4, 2, 2, 4, 2, 2)] - - -def test_display_weekly_notification_statistics_week_number_misses_week(notify_db, - notify_db_session, - sample_service): - fools = date(2016, 4, 1) - two_weeks_later = fools + timedelta(days=14) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=fools - ) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=two_weeks_later - ) - assert dao_get_7_day_agg_notification_statistics_for_service( - sample_service.id, - fools - ).all() == [(2, 2, 1, 1, 2, 1, 1), (0, 2, 1, 1, 2, 1, 1)] - - -def test_display_weekly_notification_statistics_week_limit(notify_db, - notify_db_session, - sample_service): - fools = date(2016, 4, 1) - two_weeks_later = fools + timedelta(days=14) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=fools - ) - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=two_weeks_later - ) - assert dao_get_7_day_agg_notification_statistics_for_service( - sample_service.id, - fools, - 1 - ).all() == [(0, 2, 1, 1, 2, 1, 1)] diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index 738e8ae95..b6fc84757 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -4,9 +4,9 @@ from flask import json import app.celery.tasks from app.dao.notifications_dao import ( - get_notification_by_id, - dao_get_notification_statistics_for_service + get_notification_by_id ) +from app.models import NotificationStatistics from tests.app.conftest import sample_notification as create_sample_notification @@ -162,7 +162,7 @@ def test_firetext_callback_should_update_notification_status(notify_api, sample_ updated = get_notification_by_id(sample_notification.id) assert updated.status == 'delivered' assert get_notification_by_id(sample_notification.id).status == 'delivered' - stats = dao_get_notification_statistics_for_service(sample_notification.service_id)[0] + stats = get_notification_stats(sample_notification.service_id) assert stats.sms_delivered == 1 assert stats.sms_requested == 1 assert stats.sms_failed == 0 @@ -189,7 +189,7 @@ def test_firetext_callback_should_update_notification_status_failed(notify_api, sample_notification.id ) assert get_notification_by_id(sample_notification.id).status == 'permanent-failure' - stats = dao_get_notification_statistics_for_service(sample_notification.service_id)[0] + stats = get_notification_stats(sample_notification.service_id) assert stats.sms_delivered == 0 assert stats.sms_requested == 1 assert stats.sms_failed == 1 @@ -217,7 +217,7 @@ def test_firetext_callback_should_update_notification_status_pending(notify_api, notification.id ) assert get_notification_by_id(notification.id).status == 'pending' - stats = dao_get_notification_statistics_for_service(notification.service_id)[0] + stats = get_notification_stats(notification.service_id) assert stats.sms_delivered == 0 assert stats.sms_requested == 1 assert stats.sms_failed == 0 @@ -257,7 +257,7 @@ def test_firetext_callback_should_update_multiple_notification_status_sent( ), headers=[('Content-Type', 'application/x-www-form-urlencoded')]) - stats = dao_get_notification_statistics_for_service(notification1.service_id)[0] + stats = get_notification_stats(notification1.service_id) assert stats.sms_delivered == 3 assert stats.sms_requested == 3 assert stats.sms_failed == 0 @@ -481,7 +481,7 @@ def test_ses_callback_should_update_notification_status( assert json_resp['result'] == 'success' assert json_resp['message'] == 'SES callback succeeded' assert get_notification_by_id(notification.id).status == 'delivered' - stats = dao_get_notification_statistics_for_service(notification.service_id)[0] + stats = get_notification_stats(notification.service_id) assert stats.emails_delivered == 1 assert stats.emails_requested == 1 assert stats.emails_failed == 0 @@ -536,7 +536,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent( assert resp2.status_code == 200 assert resp3.status_code == 200 - stats = dao_get_notification_statistics_for_service(notification1.service_id)[0] + stats = get_notification_stats(notification1.service_id) assert stats.emails_delivered == 3 assert stats.emails_requested == 3 assert stats.emails_failed == 0 @@ -568,7 +568,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(notify_api, assert json_resp['result'] == 'success' assert json_resp['message'] == 'SES callback succeeded' assert get_notification_by_id(notification.id).status == 'temporary-failure' - stats = dao_get_notification_statistics_for_service(notification.service_id)[0] + stats = get_notification_stats(notification.service_id) assert stats.emails_delivered == 0 assert stats.emails_requested == 1 assert stats.emails_failed == 1 @@ -628,7 +628,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(notify_api, assert json_resp['result'] == 'success' assert json_resp['message'] == 'SES callback succeeded' assert get_notification_by_id(notification.id).status == 'permanent-failure' - stats = dao_get_notification_statistics_for_service(notification.service_id)[0] + stats = get_notification_stats(notification.service_id) assert stats.emails_delivered == 0 assert stats.emails_requested == 1 assert stats.emails_failed == 1 @@ -730,3 +730,7 @@ def ses_hard_bounce_callback(): def ses_soft_bounce_callback(): return b'{\n "Type" : "Notification",\n "MessageId" : "ref",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Bounce\\",\\"bounce\\":{\\"bounceType\\":\\"Undetermined\\",\\"bounceSubType\\":\\"General\\"}, \\"mail\\":{\\"messageId\\":\\"ref\\",\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"test@test-domain.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n "Timestamp" : "2016-03-14T12:35:26.665Z",\n "SignatureVersion" : "1",\n "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa + + +def get_notification_stats(service_id): + return NotificationStatistics.query.filter_by(service_id=service_id).one()