From fff81b49102a464a4e8f32c65fe75bd5ae7b3b62 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 11 Aug 2016 11:53:08 +0100 Subject: [PATCH 01/11] remove unused notification-statistics endpoints --- app/__init__.py | 3 +- app/notifications_statistics/__init__.py | 0 app/notifications_statistics/rest.py | 105 --------- .../test_notification_statistics_rest.py | 207 ------------------ 4 files changed, 1 insertion(+), 314 deletions(-) delete mode 100644 app/notifications_statistics/__init__.py delete mode 100644 app/notifications_statistics/rest.py delete mode 100644 tests/app/notifications/test_notification_statistics_rest.py diff --git a/app/__init__.py b/app/__init__.py index af3f15c74..029bc72a5 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -66,7 +66,6 @@ def create_app(app_name=None): from app.notifications.rest import notifications as notifications_blueprint from app.invite.rest import invite as invite_blueprint from app.accept_invite.rest import accept_invite - from app.notifications_statistics.rest import notifications_statistics as notifications_statistics_blueprint from app.template_statistics.rest import template_statistics as template_statistics_blueprint from app.events.rest import events as events_blueprint from app.provider_details.rest import provider_details as provider_details_blueprint @@ -81,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(notifications_statistics_blueprint) + 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/notifications_statistics/__init__.py b/app/notifications_statistics/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/app/notifications_statistics/rest.py b/app/notifications_statistics/rest.py deleted file mode 100644 index 6613b10f8..000000000 --- a/app/notifications_statistics/rest.py +++ /dev/null @@ -1,105 +0,0 @@ -from datetime import ( - date, - timedelta, - datetime -) - -from flask import ( - Blueprint, - jsonify, - request, - current_app -) - -from app import DATE_FORMAT - -from app.dao.notifications_dao import ( - dao_get_notification_statistics_for_service, - dao_get_7_day_agg_notification_statistics_for_service, - dao_get_notification_statistics_for_service_and_day -) -from app.schemas import ( - notifications_statistics_schema, - week_aggregate_notification_statistics_schema -) - -notifications_statistics = Blueprint( - 'notifications-statistics', - __name__, url_prefix='/service//notifications-statistics' -) - -from app.errors import ( - register_errors, - InvalidRequest -) - -register_errors(notifications_statistics) - - -@notifications_statistics.route('', methods=['GET']) -def get_all_notification_statistics_for_service(service_id): - - if request.args.get('limit_days'): - try: - statistics = dao_get_notification_statistics_for_service( - service_id=service_id, - limit_days=int(request.args['limit_days']) - ) - except ValueError as e: - message = '{} is not an integer'.format(request.args['limit_days']) - errors = {'limit_days': [message]} - raise InvalidRequest(errors, status_code=400) - else: - statistics = dao_get_notification_statistics_for_service(service_id=service_id) - - data, errors = notifications_statistics_schema.dump(statistics, many=True) - return jsonify(data=data) - - -@notifications_statistics.route('/day/', methods=['GET']) -def get_notification_statistics_for_service_for_day(service_id, day): - - try: - datetime.strptime(day, DATE_FORMAT) - except ValueError: - raise InvalidRequest('Invalid date {}'.format(day), status_code=400) - - service_stats = dao_get_notification_statistics_for_service_and_day( - service_id, - day - ) - - if not service_stats: - message = 'No statistics found for service id: {} on day: {} '.format(service_id, day) - errors = {'not found': [message]} - raise InvalidRequest(errors, status_code=404) - - data = notifications_statistics_schema.dump(service_stats).data - return jsonify(data=data) - - -@notifications_statistics.route('/seven_day_aggregate') -def get_notification_statistics_for_service_seven_day_aggregate(service_id): - data = week_aggregate_notification_statistics_schema.load(request.args).data - date_from = data['date_from'] if 'date_from' in data else date(date.today().year, 4, 1) - week_count = data['week_count'] if 'week_count' in data else 52 - stats = dao_get_7_day_agg_notification_statistics_for_service( - service_id, - date_from, - week_count).all() - json_stats = [] - for x in range(week_count - 1, -1, -1): - week_stats = stats.pop(0) if len(stats) > 0 and stats[0][0] == x else [x, 0, 0, 0, 0, 0, 0] - week_start = (date_from + timedelta(days=week_stats[0] * 7)) - if week_start <= date.today(): - json_stats.append({ - 'week_start': week_start.strftime('%Y-%m-%d'), - 'week_end': (date_from + timedelta(days=(week_stats[0] * 7) + 6)).strftime('%Y-%m-%d'), - 'emails_requested': week_stats[1], - 'emails_delivered': week_stats[2], - 'emails_failed': week_stats[3], - 'sms_requested': week_stats[4], - 'sms_delivered': week_stats[5], - 'sms_failed': week_stats[6] - }) - return jsonify(data=json_stats) diff --git a/tests/app/notifications/test_notification_statistics_rest.py b/tests/app/notifications/test_notification_statistics_rest.py deleted file mode 100644 index d298aa7c5..000000000 --- a/tests/app/notifications/test_notification_statistics_rest.py +++ /dev/null @@ -1,207 +0,0 @@ -import json -from datetime import ( - date, - timedelta -) - -from flask import url_for -from tests import create_authorization_header -from tests.app.conftest import sample_notification_statistics as create_sample_notification_statistics - -from freezegun import freeze_time - - -def test_get_notification_statistics_returns_empty_list_if_no_stats(notify_api, - notify_db, - notify_db_session, - sample_template, - sample_email_template): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - - path = '/service/{}/notifications-statistics'.format(sample_email_template.service.id) - - auth_header = create_authorization_header( - service_id=sample_email_template.service_id) - - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - - stats = json.loads(response.get_data(as_text=True)) - assert stats['data'] == [] - - -def test_get_week_aggregate_statistics(notify_api, - notify_db, - notify_db_session, - sample_service): - with notify_api.test_request_context(): - sample_notification_statistics = create_sample_notification_statistics( - notify_db, - notify_db_session, - day=date(date.today().year, 4, 1)) - with notify_api.test_client() as client: - endpoint = url_for( - 'notifications-statistics.get_notification_statistics_for_service_seven_day_aggregate', - service_id=sample_service.id) - auth_header = create_authorization_header( - service_id=sample_service.id) - - resp = client.get(endpoint, headers=[auth_header]) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) - week_len_index = len(json_resp['data']) - 1 - assert json_resp['data'][week_len_index]['emails_requested'] == 2 - assert json_resp['data'][week_len_index]['sms_requested'] == 2 - assert json_resp['data'][week_len_index]['week_start'] == date(date.today().year, 4, 1).strftime('%Y-%m-%d') - assert json_resp['data'][week_len_index]['week_end'] == date(date.today().year, 4, 7).strftime('%Y-%m-%d') - - -def test_get_week_aggregate_statistics_date_from(notify_api, - notify_db, - notify_db_session, - sample_service): - with notify_api.test_request_context(): - sample_notification_statistics = create_sample_notification_statistics( - notify_db, - notify_db_session, - day=date(date.today().year, 4, 1)) - date_from_str = date(date.today().year, 4, 1).strftime('%Y-%m-%d') - with notify_api.test_client() as client: - endpoint = url_for( - 'notifications-statistics.get_notification_statistics_for_service_seven_day_aggregate', - service_id=sample_service.id, - date_from=date_from_str) - auth_header = create_authorization_header( - service_id=sample_service.id) - - resp = client.get(endpoint, headers=[auth_header]) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) - week_len_index = len(json_resp['data']) - 1 - assert json_resp['data'][week_len_index]['emails_requested'] == 2 - assert json_resp['data'][week_len_index]['sms_requested'] == 2 - assert json_resp['data'][week_len_index]['week_start'] == date_from_str - assert json_resp['data'][week_len_index]['week_end'] == date(date.today().year, 4, 7).strftime('%Y-%m-%d') - - -def test_get_week_aggregate_statistics_date_in_future(notify_api, - notify_db, - notify_db_session, - sample_service): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - endpoint = url_for( - 'notifications-statistics.get_notification_statistics_for_service_seven_day_aggregate', - service_id=sample_service.id, - date_from=(date.today() + timedelta(days=1)).strftime('%Y-%m-%d')) - auth_header = create_authorization_header( - service_id=sample_service.id) - - resp = client.get(endpoint, headers=[auth_header]) - assert resp.status_code == 400 - json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['result'] == 'error' - assert json_resp['message']['date_from'][0] == 'Date cannot be in the future' - - -def test_get_week_aggregate_statistics_invalid_week_count(notify_api, - notify_db, - notify_db_session, - sample_service): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - endpoint = url_for( - 'notifications-statistics.get_notification_statistics_for_service_seven_day_aggregate', - service_id=sample_service.id, - week_count=-1) - auth_header = create_authorization_header( - service_id=sample_service.id) - - resp = client.get(endpoint, headers=[auth_header]) - assert resp.status_code == 400 - json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['result'] == 'error' - assert json_resp['message']['week_count'][0] == 'Not a positive integer' - - -@freeze_time('2016-01-01') -def test_get_notification_statistics_for_specific_day(notify_api, - notify_db, - notify_db_session, - sample_template): - the_day = date.today() - - sample_notification_statistics = create_sample_notification_statistics( - notify_db, - notify_db_session, - day=the_day) - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - path = '/service/{}/notifications-statistics/day/{}'.format(sample_template.service_id, the_day) - auth_header = create_authorization_header(service_id=sample_template.service_id) - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - stats = json.loads(response.get_data(as_text=True)) - - assert stats['data']['id'] == str(sample_notification_statistics.id) - assert stats['data']['day'] == the_day.strftime('%Y-%m-%d') - - another_day = the_day - timedelta(days=1) - path = '/service/{}/notifications-statistics/day/{}'.format(sample_template.service_id, another_day) - - response = client.get(path, headers=[auth_header]) - assert response.status_code == 404 - - -@freeze_time('2016-01-01') -def test_get_notification_statistics_for_specific_day_returns_404_if_no_stats(notify_api, - notify_db, - notify_db_session, - sample_template): - the_day = date.today() - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - path = '/service/{}/notifications-statistics/day/{}'.format(sample_template.service_id, the_day) - auth_header = create_authorization_header(service_id=sample_template.service_id) - response = client.get(path, headers=[auth_header]) - assert response.status_code == 404 - - -@freeze_time('2016-01-01') -def test_get_notification_statistics_for_specific_day_returns_400_for_incorrect_date(notify_api, - notify_db, - notify_db_session, - sample_template): - the_day = date.today() - incorrect_date_format = the_day.strftime('%d-%m-%Y') - - create_sample_notification_statistics( - notify_db, - notify_db_session, - day=the_day) - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - path = '/service/{}/notifications-statistics/day/{}'.format( - sample_template.service_id, - incorrect_date_format) - auth_header = create_authorization_header(service_id=sample_template.service_id) - response = client.get(path, headers=[auth_header]) - assert response.status_code == 400 - resp_json = json.loads(response.get_data(as_text=True)) - assert resp_json['result'] == 'error' - assert resp_json['message'] == 'Invalid date 01-01-2016' - - another_dodgy_date = 'fish' - path = '/service/{}/notifications-statistics/day/{}'.format( - sample_template.service_id, - another_dodgy_date) - - response = client.get(path, headers=[auth_header]) - assert response.status_code == 400 - resp_json = json.loads(response.get_data(as_text=True)) - assert resp_json['result'] == 'error' - assert resp_json['message'] == 'Invalid date fish' From f065b08db241183fef4708e1ca3b072631333f75 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 11 Aug 2016 16:08:28 +0100 Subject: [PATCH 02/11] 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() From b22e3845aadba8dc0023d6e4b0c777e5f2bcbe6c Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 11 Aug 2016 16:34:36 +0100 Subject: [PATCH 03/11] kill unused schema --- app/schemas.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 93171dec7..78319cb43 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -501,23 +501,6 @@ class DaySchema(ma.Schema): _validate_not_in_future(value) -class WeekAggregateNotificationStatisticsSchema(ma.Schema): - - class Meta: - strict = True - - date_from = fields.Date() - week_count = fields.Int() - - @validates('date_from') - def validate_date_from(self, value): - _validate_not_in_future(value) - - @validates('week_count') - def validate_week_count(self, value): - _validate_positive_number(value) - - class UnarchivedTemplateSchema(BaseSchema): archived = fields.Boolean(required=True) @@ -560,6 +543,5 @@ event_schema = EventSchema() organisation_schema = OrganisationSchema() from_to_date_schema = FromToDateSchema() provider_details_schema = ProviderDetailsSchema() -week_aggregate_notification_statistics_schema = WeekAggregateNotificationStatisticsSchema() day_schema = DaySchema() unarchived_template_schema = UnarchivedTemplateSchema() From 8fb0ba56edbb67ca0aab8774715ef8cc9b5cc463 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 11 Aug 2016 17:30:50 +0100 Subject: [PATCH 04/11] unused schema --- app/schemas.py | 25 ------------------------- app/service/rest.py | 3 --- 2 files changed, 28 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 78319cb43..858983ad8 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -465,30 +465,6 @@ class OrganisationSchema(BaseSchema): strict = True -class FromToDateSchema(ma.Schema): - - class Meta: - strict = True - - date_from = fields.Date() - date_to = fields.Date() - - @validates('date_from') - def validate_date_from(self, value): - _validate_not_in_future(value) - - @validates('date_to') - def validate_date_to(self, value): - _validate_not_in_future(value) - - @validates_schema - def validate_dates(self, data): - df = data.get('date_from') - dt = data.get('date_to') - if (df and dt) and (df > dt): - raise ValidationError("date_from needs to be greater than date_to") - - class DaySchema(ma.Schema): class Meta: @@ -541,7 +517,6 @@ api_key_history_schema = ApiKeyHistorySchema() template_history_schema = TemplateHistorySchema() event_schema = EventSchema() organisation_schema = OrganisationSchema() -from_to_date_schema = FromToDateSchema() provider_details_schema = ProviderDetailsSchema() day_schema = DaySchema() unarchived_template_schema = UnarchivedTemplateSchema() diff --git a/app/service/rest.py b/app/service/rest.py index bf84fb859..35f22769d 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1,5 +1,3 @@ -from datetime import date, timedelta - from flask import ( jsonify, request, @@ -32,7 +30,6 @@ from app.schemas import ( service_schema, api_key_schema, user_schema, - from_to_date_schema, permission_schema, notification_with_template_schema, notifications_filter_schema, From 09cb94081bcbffb09ba28f1a2bff6cbd915ba37e Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 23 Aug 2016 16:46:58 +0100 Subject: [PATCH 05/11] Returns the outcome statistics for the job on the API call. --- app/dao/jobs_dao.py | 21 ++++++-- app/job/rest.py | 7 ++- app/schemas.py | 6 ++- tests/app/dao/test_jobs_dao.py | 89 ++++++++++++++++++++++++++++++- tests/app/job/test_rest.py | 96 ++++++++++++++++++++++++++++------ 5 files changed, 197 insertions(+), 22 deletions(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 13742418a..1ebba4893 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -1,9 +1,24 @@ -from datetime import date, timedelta - from sqlalchemy import desc, cast, Date as sql_date from app import db from app.dao import days_ago -from app.models import Job +from app.models import Job, NotificationHistory +from app.statsd_decorators import statsd +from sqlalchemy import func, asc + + +@statsd(namespace="dao") +def dao_get_notification_outcomes_for_job(service_id, job_id): + query = db.session.query( + func.count(NotificationHistory.status).label('count'), + NotificationHistory.status.label('status') + ) + + return query \ + .filter(NotificationHistory.service_id == service_id) \ + .filter(NotificationHistory.job_id == job_id)\ + .group_by(NotificationHistory.status) \ + .order_by(asc(NotificationHistory.status)) \ + .all() def dao_get_job_by_service_id_and_job_id(service_id, job_id): diff --git a/app/job/rest.py b/app/job/rest.py index c40172a85..a9efb3ebf 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -8,7 +8,8 @@ from flask import ( from app.dao.jobs_dao import ( dao_create_job, dao_get_job_by_service_id_and_job_id, - dao_get_jobs_by_service_id + dao_get_jobs_by_service_id, + dao_get_notification_outcomes_for_job ) from app.dao.services_dao import ( @@ -42,7 +43,11 @@ register_errors(job) @job.route('/', methods=['GET']) def get_job_by_service_and_job_id(service_id, job_id): job = dao_get_job_by_service_id_and_job_id(service_id, job_id) + statistics = dao_get_notification_outcomes_for_job(service_id, job_id) data = job_schema.dump(job).data + + data['statistics'] = [{'status': statistic[1], 'count': statistic[0]} for statistic in statistics] + return jsonify(data=data) diff --git a/app/schemas.py b/app/schemas.py index a28c4aa94..db654b0ed 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -210,7 +210,11 @@ class JobSchema(BaseSchema): class Meta: model = models.Job - exclude = ('notifications',) + exclude = ( + 'notifications', + 'notifications_sent', + 'notifications_delivered', + 'notifications_failed') strict = True diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 9f265ab0d..0a69ffbc3 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -5,10 +5,95 @@ from app.dao.jobs_dao import ( dao_get_job_by_service_id_and_job_id, dao_create_job, dao_update_job, - dao_get_jobs_by_service_id -) + dao_get_jobs_by_service_id, + dao_get_notification_outcomes_for_job) from app.models import Job +from tests.app.conftest import sample_notification, sample_job, sample_service + + +def test_should_have_decorated_notifications_dao_functions(): + assert dao_get_notification_outcomes_for_job.__wrapped__.__name__ == 'dao_get_notification_outcomes_for_job' # noqa + + +def test_should_get_all_statuses_for_notifications_associated_with_job( + notify_db, + notify_db_session, + sample_service, + sample_job): + + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='pending') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='failed') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='technical-failure') # noqa + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='temporary-failure') # noqa + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='permanent-failure') # noqa + + results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id) + assert [(row.count, row.status) for row in results] == [ + (1, 'created'), + (1, 'sending'), + (1, 'delivered'), + (1, 'pending'), + (1, 'failed'), + (1, 'technical-failure'), + (1, 'temporary-failure'), + (1, 'permanent-failure') + ] + + +def test_should_count_of_statuses_for_notifications_associated_with_job( + notify_db, + notify_db_session, + sample_service, + sample_job): + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered') + sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered') + + results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id) + assert [(row.count, row.status) for row in results] == [ + (2, 'created'), + (4, 'sending'), + (2, 'delivered') + ] + + +def test_should_return_zero_length_array_if_no_notifications_for_job(sample_service, sample_job): + assert len(dao_get_notification_outcomes_for_job(sample_job.id, sample_service.id)) == 0 + + +def test_should_return_notifications_only_for_this_job(notify_db, notify_db_session, sample_service): + job_1 = sample_job(notify_db, notify_db_session, service=sample_service) + job_2 = sample_job(notify_db, notify_db_session, service=sample_service) + + sample_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') + sample_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='created') + + results = dao_get_notification_outcomes_for_job(sample_service.id, job_1.id) + assert [(row.count, row.status) for row in results] == [ + (1, 'created') + ] + + +def test_should_return_notifications_only_for_this_service(notify_db, notify_db_session): + service_1 = sample_service(notify_db, notify_db_session, service_name="one", email_from="one") + service_2 = sample_service(notify_db, notify_db_session, service_name="two", email_from="two") + + job_1 = sample_job(notify_db, notify_db_session, service=service_1) + job_2 = sample_job(notify_db, notify_db_session, service=service_2) + + sample_notification(notify_db, notify_db_session, service=service_1, job=job_1, status='created') + sample_notification(notify_db, notify_db_session, service=service_2, job=job_2, status='created') + + assert len(dao_get_notification_outcomes_for_job(service_1.id, job_2.id)) == 0 def test_create_job(sample_template): diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index ea65bb447..bf4afeca0 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -9,7 +9,7 @@ import app.celery.tasks from tests import create_authorization_header from tests.app.conftest import ( sample_job as create_job, - sample_notification as create_sample_notification) + sample_notification as create_sample_notification, sample_notification) from app.dao.templates_dao import dao_update_template from app.models import NOTIFICATION_STATUS_TYPES @@ -94,20 +94,6 @@ def test_get_job_with_unknown_id_returns404(notify_api, sample_template, fake_uu } -def test_get_job_by_id(notify_api, sample_job): - job_id = str(sample_job.id) - service_id = sample_job.service.id - with notify_api.test_request_context(): - with notify_api.test_client() as client: - path = '/service/{}/job/{}'.format(service_id, job_id) - auth_header = create_authorization_header(service_id=sample_job.service.id) - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) - assert resp_json['data']['id'] == job_id - assert resp_json['data']['created_by']['name'] == 'Test User' - - def test_create_job(notify_api, sample_template, mocker, fake_uuid): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -350,3 +336,83 @@ def test_get_all_notifications_for_job_filtered_by_status( resp = json.loads(response.get_data(as_text=True)) assert len(resp['notifications']) == expected_notification_count assert response.status_code == 200 + + +def test_get_job_by_id(notify_api, sample_job): + job_id = str(sample_job.id) + service_id = sample_job.service.id + with notify_api.test_request_context(): + with notify_api.test_client() as client: + path = '/service/{}/job/{}'.format(service_id, job_id) + auth_header = create_authorization_header(service_id=sample_job.service.id) + response = client.get(path, headers=[auth_header]) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['data']['id'] == job_id + assert resp_json['data']['statistics'] == [] + assert resp_json['data']['created_by']['name'] == 'Test User' + + +def test_get_job_by_id_should_return_statistics(notify_db, notify_db_session, notify_api, sample_job): + job_id = str(sample_job.id) + service_id = sample_job.service.id + + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='delivered') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='pending') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure') # noqa + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure') # noqa + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='permanent-failure') # noqa + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + path = '/service/{}/job/{}'.format(service_id, job_id) + auth_header = create_authorization_header(service_id=sample_job.service.id) + response = client.get(path, headers=[auth_header]) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + print(resp_json) + assert resp_json['data']['id'] == job_id + assert {'status': 'created', 'count': 1} in resp_json['data']['statistics'] + assert {'status': 'sending', 'count': 1} in resp_json['data']['statistics'] + assert {'status': 'delivered', 'count': 1} in resp_json['data']['statistics'] + assert {'status': 'pending', 'count': 1} in resp_json['data']['statistics'] + assert {'status': 'failed', 'count': 1} in resp_json['data']['statistics'] + assert {'status': 'technical-failure', 'count': 1} in resp_json['data']['statistics'] + assert {'status': 'temporary-failure', 'count': 1} in resp_json['data']['statistics'] + assert {'status': 'permanent-failure', 'count': 1} in resp_json['data']['statistics'] + assert resp_json['data']['created_by']['name'] == 'Test User' + + +def test_get_job_by_id_should_return_summed_statistics(notify_db, notify_db_session, notify_api, sample_job): + job_id = str(sample_job.id) + service_id = sample_job.service.id + + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed') + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure') # noqa + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure') # noqa + sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure') # noqa + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + path = '/service/{}/job/{}'.format(service_id, job_id) + auth_header = create_authorization_header(service_id=sample_job.service.id) + response = client.get(path, headers=[auth_header]) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + print(resp_json) + assert resp_json['data']['id'] == job_id + assert {'status': 'created', 'count': 3} in resp_json['data']['statistics'] + assert {'status': 'sending', 'count': 1} in resp_json['data']['statistics'] + assert {'status': 'failed', 'count': 3} in resp_json['data']['statistics'] + assert {'status': 'technical-failure', 'count': 1} in resp_json['data']['statistics'] + assert {'status': 'temporary-failure', 'count': 2} in resp_json['data']['statistics'] + assert resp_json['data']['created_by']['name'] == 'Test User' From ebb13a12519d4f6ef185a72635f8b388fb4d5034 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 11 Aug 2016 17:24:44 +0100 Subject: [PATCH 06/11] create initial stats query get statistics for all services, for today only --- app/dao/services_dao.py | 20 ++++++++++++++++++++ app/service/rest.py | 15 +++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index eff4679dd..5015ba363 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -180,3 +180,23 @@ def dao_fetch_weekly_historical_stats_for_service(service_id): ).order_by( asc(monday_of_notification_week), NotificationHistory.status ).all() + + +@statsd(namespace='dao') +def dao_fetch_todays_stats_for_all_services(): + return db.session.query( + Notification.notification_type, + Notification.status, + func.count(Notification.id).label('count') + ).select_from( + Service + ).join( + # don't want to create a relationship in case we accidentally lazily load it, so manually define the join term + Notification + ).filter( + func.date(Notification.created_at) == date.today() + ).group_by( + Notification.notification_type, + Notification.status, + Notification.service_id + ) diff --git a/app/service/rest.py b/app/service/rest.py index 1c0096000..6445a8311 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -52,6 +52,8 @@ def get_services(): if user_id: services = dao_fetch_all_services_by_user(user_id) else: + if request.args.get('detailed') == 'True': + return get_detailed_services() services = dao_fetch_all_services() data = service_schema.dump(services, many=True).data return jsonify(data=data) @@ -245,3 +247,16 @@ def get_detailed_service(service_id, today_only=False): data = detailed_service_schema.dump(service).data return jsonify(data=data) + + +def get_detailed_services(): + services = {service.id: service for service in dao_fetch_all_services()} + stats = dao_fetch_todays_stats_for_all_services(service_id) + + for row in stats: + services[row.service_id].statistics + # todo: how do we separate rows of statistics by service? + service.statistics = statistics.format_statistics(stats) + + data = detailed_service_schema.dump(service).data + return jsonify(data=data) From 00d19f63f05e53419d9c9099c3eacb5de0759bf3 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 19 Aug 2016 16:36:20 +0100 Subject: [PATCH 07/11] group results by service using itertools allows us to nicely reuse the existing format_statistics function --- app/dao/services_dao.py | 3 +++ app/service/rest.py | 19 ++++++++++--------- tests/app/notifications/test_rest.py | 2 +- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 5015ba363..c1d2c4672 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -187,6 +187,7 @@ def dao_fetch_todays_stats_for_all_services(): return db.session.query( Notification.notification_type, Notification.status, + Notification.service_id, func.count(Notification.id).label('count') ).select_from( Service @@ -199,4 +200,6 @@ def dao_fetch_todays_stats_for_all_services(): Notification.notification_type, Notification.status, Notification.service_id + ).order_by( + Notification.service_id ) diff --git a/app/service/rest.py b/app/service/rest.py index 6445a8311..149e2ca14 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1,3 +1,5 @@ +import itertools + from flask import ( jsonify, request, @@ -21,7 +23,8 @@ from app.dao.services_dao import ( dao_remove_user_from_service, dao_fetch_stats_for_service, dao_fetch_todays_stats_for_service, - dao_fetch_weekly_historical_stats_for_service + dao_fetch_weekly_historical_stats_for_service, + dao_fetch_todays_stats_for_all_services ) from app.dao import notifications_dao from app.dao.provider_statistics_dao import get_fragment_count @@ -51,9 +54,9 @@ def get_services(): user_id = request.args.get('user_id', None) if user_id: services = dao_fetch_all_services_by_user(user_id) + elif request.args.get('detailed') == 'True': + return get_detailed_services() else: - if request.args.get('detailed') == 'True': - return get_detailed_services() services = dao_fetch_all_services() data = service_schema.dump(services, many=True).data return jsonify(data=data) @@ -251,12 +254,10 @@ def get_detailed_service(service_id, today_only=False): def get_detailed_services(): services = {service.id: service for service in dao_fetch_all_services()} - stats = dao_fetch_todays_stats_for_all_services(service_id) + stats = dao_fetch_todays_stats_for_all_services() - for row in stats: - services[row.service_id].statistics - # todo: how do we separate rows of statistics by service? - service.statistics = statistics.format_statistics(stats) + for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): + services[service_id].statistics = statistics.format_statistics(rows) - data = detailed_service_schema.dump(service).data + data = detailed_service_schema.dump(service, many=True).data return jsonify(data=data) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index df611adca..778bbe66e 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -207,7 +207,7 @@ def test_normal_api_key_returns_notifications_created_from_jobs_and_from_api( notifications = json.loads(response.get_data(as_text=True))['notifications'] assert len(notifications) == 2 - assert set(x['id'] for x in notifications) == set([str(sample_notification.id), str(api_notification.id)]) + assert set(x['id'] for x in notifications) == {str(sample_notification.id), str(api_notification.id)} @pytest.mark.parametrize('key_type', [KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST]) From 2053ebd933cd5fe7cb3d92ca6ada04f71f908384 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 19 Aug 2016 17:30:25 +0100 Subject: [PATCH 08/11] rename service to service_blueprint it was causing a bug where a local variable service was not being instantiated and we were trying to operate on the blueprint instead it's being used in so few places it makes sense to rename it --- app/__init__.py | 2 +- app/service/rest.py | 36 ++++++++++++++++++------------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index e0928a4b9..464157f60 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -58,7 +58,7 @@ def create_app(app_name=None): encryption.init_app(application) clients.init_app(sms_clients=[firetext_client, mmg_client, loadtest_client], email_clients=[aws_ses_client]) - from app.service.rest import service as service_blueprint + from app.service.rest import service_blueprint from app.user.rest import user as user_blueprint from app.template.rest import template as template_blueprint from app.status.healthcheck import status as status_blueprint diff --git a/app/service/rest.py b/app/service/rest.py index 149e2ca14..14ee99616 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -45,11 +45,11 @@ from app.errors import ( ) from app.service import statistics -service = Blueprint('service', __name__) -register_errors(service) +service_blueprint = Blueprint('service', __name__) +register_errors(service_blueprint) -@service.route('', methods=['GET']) +@service_blueprint.route('', methods=['GET']) def get_services(): user_id = request.args.get('user_id', None) if user_id: @@ -62,7 +62,7 @@ def get_services(): return jsonify(data=data) -@service.route('/', methods=['GET']) +@service_blueprint.route('/', methods=['GET']) def get_service_by_id(service_id): if request.args.get('detailed') == 'True': return get_detailed_service(service_id, today_only=request.args.get('today_only') == 'True') @@ -73,7 +73,7 @@ def get_service_by_id(service_id): return jsonify(data=data) -@service.route('', methods=['POST']) +@service_blueprint.route('', methods=['POST']) def create_service(): data = request.get_json() if not data.get('user_id', None): @@ -87,7 +87,7 @@ def create_service(): return jsonify(data=service_schema.dump(valid_service).data), 201 -@service.route('/', methods=['POST']) +@service_blueprint.route('/', methods=['POST']) def update_service(service_id): fetched_service = dao_fetch_service_by_id(service_id) current_data = dict(service_schema.dump(fetched_service).data.items()) @@ -97,7 +97,7 @@ def update_service(service_id): return jsonify(data=service_schema.dump(fetched_service).data), 200 -@service.route('//api-key', methods=['POST']) +@service_blueprint.route('//api-key', methods=['POST']) def create_api_key(service_id=None): fetched_service = dao_fetch_service_by_id(service_id=service_id) valid_api_key = api_key_schema.load(request.get_json()).data @@ -107,14 +107,14 @@ def create_api_key(service_id=None): return jsonify(data=unsigned_api_key), 201 -@service.route('//api-key/revoke/', methods=['POST']) +@service_blueprint.route('//api-key/revoke/', methods=['POST']) def revoke_api_key(service_id, api_key_id): expire_api_key(service_id=service_id, api_key_id=api_key_id) return jsonify(), 202 -@service.route('//api-keys', methods=['GET']) -@service.route('//api-keys/', methods=['GET']) +@service_blueprint.route('//api-keys', methods=['GET']) +@service_blueprint.route('//api-keys/', methods=['GET']) def get_api_keys(service_id, key_id=None): dao_fetch_service_by_id(service_id=service_id) @@ -130,14 +130,14 @@ def get_api_keys(service_id, key_id=None): return jsonify(apiKeys=api_key_schema.dump(api_keys, many=True).data), 200 -@service.route('//users', methods=['GET']) +@service_blueprint.route('//users', methods=['GET']) def get_users_for_service(service_id): fetched = dao_fetch_service_by_id(service_id) result = user_schema.dump(fetched.users, many=True) return jsonify(data=result.data) -@service.route('//users/', methods=['POST']) +@service_blueprint.route('//users/', methods=['POST']) def add_user_to_service(service_id, user_id): service = dao_fetch_service_by_id(service_id) user = get_model_users(user_id=user_id) @@ -152,7 +152,7 @@ def add_user_to_service(service_id, user_id): return jsonify(data=data), 201 -@service.route('//users/', methods=['DELETE']) +@service_blueprint.route('//users/', methods=['DELETE']) def remove_user_from_service(service_id, user_id): service = dao_fetch_service_by_id(service_id) user = get_model_users(user_id=user_id) @@ -168,7 +168,7 @@ def remove_user_from_service(service_id, user_id): return jsonify({}), 204 -@service.route('//fragment/aggregate_statistics') +@service_blueprint.route('//fragment/aggregate_statistics') def get_service_provider_aggregate_statistics(service_id): return jsonify(data=get_fragment_count(service_id)) @@ -176,7 +176,7 @@ def get_service_provider_aggregate_statistics(service_id): # This is placeholder get method until more thought # goes into how we want to fetch and view various items in history # tables. This is so product owner can pass stories as done -@service.route('//history', methods=['GET']) +@service_blueprint.route('//history', methods=['GET']) def get_service_history(service_id): from app.models import (Service, ApiKey, Template, TemplateHistory, Event) from app.schemas import ( @@ -206,7 +206,7 @@ def get_service_history(service_id): return jsonify(data=data) -@service.route('//notifications', methods=['GET']) +@service_blueprint.route('//notifications', methods=['GET']) def get_all_notifications_for_service(service_id): data = notifications_filter_schema.load(request.args).data page = data['page'] if 'page' in data else 1 @@ -233,7 +233,7 @@ def get_all_notifications_for_service(service_id): ), 200 -@service.route('//notifications/weekly', methods=['GET']) +@service_blueprint.route('//notifications/weekly', methods=['GET']) def get_weekly_notification_stats(service_id): service = dao_fetch_service_by_id(service_id) stats = dao_fetch_weekly_historical_stats_for_service(service_id) @@ -259,5 +259,5 @@ def get_detailed_services(): for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): services[service_id].statistics = statistics.format_statistics(rows) - data = detailed_service_schema.dump(service, many=True).data + data = detailed_service_schema.dump(services, many=True).data return jsonify(data=data) From 29df7edaf90ccc1ba6fb6aa52214ff931dd2cce1 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 19 Aug 2016 17:36:31 +0100 Subject: [PATCH 09/11] tests for detailed services --- app/dao/services_dao.py | 1 - app/service/rest.py | 11 ++- requirements_for_test.txt | 2 +- tests/app/service/test_rest.py | 125 +++++++++++++++++++++++++++------ 4 files changed, 111 insertions(+), 28 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index c1d2c4672..031c10939 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -192,7 +192,6 @@ def dao_fetch_todays_stats_for_all_services(): ).select_from( Service ).join( - # don't want to create a relationship in case we accidentally lazily load it, so manually define the join term Notification ).filter( func.date(Notification.created_at) == date.today() diff --git a/app/service/rest.py b/app/service/rest.py index 14ee99616..1ba277957 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -55,7 +55,7 @@ def get_services(): if user_id: services = dao_fetch_all_services_by_user(user_id) elif request.args.get('detailed') == 'True': - return get_detailed_services() + return jsonify(data=get_detailed_services()) else: services = dao_fetch_all_services() data = service_schema.dump(services, many=True).data @@ -65,7 +65,8 @@ def get_services(): @service_blueprint.route('/', methods=['GET']) def get_service_by_id(service_id): if request.args.get('detailed') == 'True': - return get_detailed_service(service_id, today_only=request.args.get('today_only') == 'True') + data = get_detailed_service(service_id, today_only=request.args.get('today_only') == 'True') + return jsonify(data=data) else: fetched = dao_fetch_service_by_id(service_id) @@ -248,8 +249,7 @@ def get_detailed_service(service_id, today_only=False): service.statistics = statistics.format_statistics(stats) - data = detailed_service_schema.dump(service).data - return jsonify(data=data) + return detailed_service_schema.dump(service).data def get_detailed_services(): @@ -259,5 +259,4 @@ def get_detailed_services(): for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): services[service_id].statistics = statistics.format_statistics(rows) - data = detailed_service_schema.dump(services, many=True).data - return jsonify(data=data) + return detailed_service_schema.dump(services.values(), many=True).data diff --git a/requirements_for_test.txt b/requirements_for_test.txt index f5db40330..1ea6f7a8d 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -6,6 +6,6 @@ pytest-cov==2.2.0 coveralls==1.1 mock==1.0.1 moto==0.4.19 -flex==5.6.0 +flex==5.7.0 freezegun==0.3.6 requests-mock==0.7.0 diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 1d3612b01..4a64e86ac 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1,3 +1,4 @@ +from datetime import datetime import json import uuid @@ -15,6 +16,7 @@ from tests.app.conftest import ( sample_user as create_sample_user, sample_notification as create_sample_notification ) +from app.service.rest import get_detailed_services def test_get_service_list(notify_api, service_factory): @@ -263,7 +265,7 @@ def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(no json_resp = json.loads(resp.get_data(as_text=True)) assert resp.status_code == 404 assert json_resp['result'] == 'error' - assert 'No result found' == json_resp['message'] + assert json_resp['message'] == 'No result found' def test_should_not_create_service_if_missing_data(notify_api, sample_user): @@ -288,8 +290,6 @@ def test_should_not_create_service_if_missing_data(notify_api, sample_user): def test_should_not_create_service_with_duplicate_name(notify_api, - notify_db, - notify_db_session, sample_user, sample_service): with notify_api.test_request_context(): @@ -314,8 +314,6 @@ def test_should_not_create_service_with_duplicate_name(notify_api, def test_create_service_should_throw_duplicate_key_constraint_for_existing_email_from(notify_api, - notify_db, - notify_db_session, service_factory, sample_user): first_service = service_factory.get('First service', email_from='first.service') @@ -432,7 +430,7 @@ def test_update_service_research_mode_throws_validation_error(notify_api, sample headers=[('Content-Type', 'application/json'), auth_header] ) result = json.loads(resp.get_data(as_text=True)) - result['message']['research_mode'][0] == "Not a valid boolean." + assert result['message']['research_mode'][0] == "Not a valid boolean." assert resp.status_code == 400 @@ -505,7 +503,7 @@ def test_should_not_update_service_with_duplicate_email_from(notify_api, ) -def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db, notify_db_session): +def test_update_service_should_404_if_id_is_invalid(notify_api): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { @@ -524,7 +522,7 @@ def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db, notif assert resp.status_code == 404 -def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_service): +def test_get_users_by_service(notify_api, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: user_on_service = sample_service.users[0] @@ -544,8 +542,6 @@ def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_s def test_get_users_for_service_returns_empty_list_if_no_users_associated_with_service(notify_api, - notify_db, - notify_db_session, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -1100,16 +1096,16 @@ def test_set_sms_sender_for_service_rejects_invalid_characters(notify_api, sampl @pytest.mark.parametrize('today_only,stats', [ - ('False', { - 'requested': 2, - 'delivered': 1, - 'failed': 0 - }), - ('True', { - 'requested': 1, - 'delivered': 0, - 'failed': 0 - }) + ('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): @@ -1156,3 +1152,92 @@ def test_get_weekly_notification_stats(notify_api, notify_db, notify_db_session) } } } + + +def test_get_services_with_detailed_flag(notify_api, notify_db, notify_db_session): + notifications = [ + create_sample_notification(notify_db, notify_db_session), + create_sample_notification(notify_db, notify_db_session) + ] + with notify_api.test_request_context(), notify_api.test_client() as client: + resp = client.get( + '/service?detailed=True', + headers=[create_authorization_header()] + ) + + assert resp.status_code == 200 + data = json.loads(resp.get_data(as_text=True))['data'] + assert len(data) == 1 + assert data[0]['name'] == 'Sample service' + assert data[0]['id'] == str(notifications[0].service_id) + assert data[0]['statistics'] == { + 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'sms': {'delivered': 0, 'failed': 0, 'requested': 2} + } + + +def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): + service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1') + service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2') + + create_sample_notification(notify_db, notify_db_session, service=service_1, status='created') + create_sample_notification(notify_db, notify_db_session, service=service_2, status='created') + create_sample_notification(notify_db, notify_db_session, service=service_1, status='delivered') + create_sample_notification(notify_db, notify_db_session, service=service_1, status='created') + + data = get_detailed_services() + data = sorted(data, key=lambda x: x['id']) + + assert len(data) == 2 + assert data[0]['id'] == str(service_1.id) + assert data[0]['statistics'] == { + 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'sms': {'delivered': 0, 'failed': 0, 'requested': 1} + } + assert data[1]['id'] == str(service_2.id) + assert data[1]['statistics'] == { + 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'sms': {'delivered': 1, 'failed': 0, 'requested': 2} + } + + +def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): + service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1') + service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2') + + create_sample_notification(notify_db, notify_db_session, service=service_1, status='created') + create_sample_notification(notify_db, notify_db_session, service=service_2, status='created') + create_sample_notification(notify_db, notify_db_session, service=service_1, status='delivered') + create_sample_notification(notify_db, notify_db_session, service=service_1, status='created') + + data = get_detailed_services() + data = sorted(data, key=lambda x: x['name']) + + assert len(data) == 2 + assert data[0]['id'] == str(service_1.id) + assert data[0]['statistics'] == { + 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'sms': {'delivered': 1, 'failed': 0, 'requested': 3} + } + assert data[1]['id'] == str(service_2.id) + assert data[1]['statistics'] == { + 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'sms': {'delivered': 0, 'failed': 0, 'requested': 1} + } + + + +def test_get_detailed_services_only_includes_todays_notifications(notify_db, notify_db_session): + create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 9, 23, 59)) + create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 10, 0, 0)) + create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 10, 12, 0)) + + with freeze_time('2015-10-10T12:00:00'): + data = get_detailed_services() + data = sorted(data, key=lambda x: x['id']) + + assert len(data) == 1 + assert data[0]['statistics'] == { + 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'sms': {'delivered': 0, 'failed': 0, 'requested': 2} + } From 556b69a487536c123abbaa26dc15218d1b2129ed Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 23 Aug 2016 17:08:53 +0100 Subject: [PATCH 10/11] still return service if they have never sent any notifications --- app/service/rest.py | 5 ++ app/service/statistics.py | 6 +-- tests/app/service/test_rest.py | 74 ++++++++++++++-------------- tests/app/service/test_statistics.py | 4 +- 4 files changed, 47 insertions(+), 42 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 1ba277957..5a80cdc76 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -259,4 +259,9 @@ def get_detailed_services(): for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): services[service_id].statistics = statistics.format_statistics(rows) + # if service has not sent anything, query will not have set statistics correctly + for service in services.values(): + if not hasattr(service, 'statistics'): + service.statistics = statistics.create_zeroed_stats_dicts() + return detailed_service_schema.dump(services.values(), many=True).data diff --git a/app/service/statistics.py b/app/service/statistics.py index abcb25740..3f06fcacb 100644 --- a/app/service/statistics.py +++ b/app/service/statistics.py @@ -8,7 +8,7 @@ def format_statistics(statistics): # statistics come in a named tuple with uniqueness from 'notification_type', 'status' - however missing # statuses/notification types won't be represented and the status types need to be simplified/summed up # so we can return emails/sms * created, sent, and failed - counts = _create_zeroed_stats_dicts() + counts = create_zeroed_stats_dicts() for row in statistics: _update_statuses_from_row(counts[row.notification_type], row) @@ -20,7 +20,7 @@ def format_weekly_notification_stats(statistics, service_created_at): # turn a datetime into midnight that day http://stackoverflow.com/a/1937636 preceeding_monday_midnight = datetime.combine(preceeding_monday.date(), datetime.min.time()) week_dict = { - week: _create_zeroed_stats_dicts() + week: create_zeroed_stats_dicts() for week in _weeks_for_range(preceeding_monday_midnight, datetime.utcnow()) } for row in statistics: @@ -29,7 +29,7 @@ def format_weekly_notification_stats(statistics, service_created_at): return week_dict -def _create_zeroed_stats_dicts(): +def create_zeroed_stats_dicts(): return { template_type: { status: 0 for status in ('requested', 'delivered', 'failed') diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 4a64e86ac..ba6587a61 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -16,7 +16,6 @@ from tests.app.conftest import ( sample_user as create_sample_user, sample_notification as create_sample_notification ) -from app.service.rest import get_detailed_services def test_get_service_list(notify_api, service_factory): @@ -1096,18 +1095,17 @@ def test_set_sms_sender_for_service_rejects_invalid_characters(notify_api, sampl @pytest.mark.parametrize('today_only,stats', [ - ('False', { - 'requested': 2, - 'delivered': 1, - 'failed': 0 - }), - ('True', { - 'requested': 1, - 'delivered': 0, - 'failed': 0 - }) - ], ids=['seven_days', 'today'] -) + ('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: with freeze_time('2000-01-01T12:00:00'): @@ -1177,31 +1175,8 @@ def test_get_services_with_detailed_flag(notify_api, notify_db, notify_db_sessio def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): - service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1') - service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2') + from app.service.rest import get_detailed_services - create_sample_notification(notify_db, notify_db_session, service=service_1, status='created') - create_sample_notification(notify_db, notify_db_session, service=service_2, status='created') - create_sample_notification(notify_db, notify_db_session, service=service_1, status='delivered') - create_sample_notification(notify_db, notify_db_session, service=service_1, status='created') - - data = get_detailed_services() - data = sorted(data, key=lambda x: x['id']) - - assert len(data) == 2 - assert data[0]['id'] == str(service_1.id) - assert data[0]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 0, 'failed': 0, 'requested': 1} - } - assert data[1]['id'] == str(service_2.id) - assert data[1]['statistics'] == { - 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, - 'sms': {'delivered': 1, 'failed': 0, 'requested': 2} - } - - -def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1') service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2') @@ -1226,8 +1201,33 @@ def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): } +def test_get_detailed_services_includes_services_with_no_notifications(notify_db, notify_db_session): + from app.service.rest import get_detailed_services + + service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1') + service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2') + + create_sample_notification(notify_db, notify_db_session, service=service_1) + + data = get_detailed_services() + data = sorted(data, key=lambda x: x['name']) + + assert len(data) == 2 + assert data[0]['id'] == str(service_1.id) + assert data[0]['statistics'] == { + 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'sms': {'delivered': 0, 'failed': 0, 'requested': 1} + } + assert data[1]['id'] == str(service_2.id) + assert data[1]['statistics'] == { + 'email': {'delivered': 0, 'failed': 0, 'requested': 0}, + 'sms': {'delivered': 0, 'failed': 0, 'requested': 0} + } + def test_get_detailed_services_only_includes_todays_notifications(notify_db, notify_db_session): + from app.service.rest import get_detailed_services + create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 9, 23, 59)) create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 10, 0, 0)) create_sample_notification(notify_db, notify_db_session, created_at=datetime(2015, 10, 10, 12, 0)) diff --git a/tests/app/service/test_statistics.py b/tests/app/service/test_statistics.py index a4d8648e0..fcf854da8 100644 --- a/tests/app/service/test_statistics.py +++ b/tests/app/service/test_statistics.py @@ -7,7 +7,7 @@ from freezegun import freeze_time from app.service.statistics import ( format_statistics, _weeks_for_range, - _create_zeroed_stats_dicts, + create_zeroed_stats_dicts, format_weekly_notification_stats ) @@ -63,7 +63,7 @@ def test_weeks_for_range(start, end, dates): def test_create_zeroed_stats_dicts(): - assert _create_zeroed_stats_dicts() == { + assert create_zeroed_stats_dicts() == { 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, } From 135ac6f23ea57df952edc1ed296893c05273d9e2 Mon Sep 17 00:00:00 2001 From: bandesz Date: Wed, 24 Aug 2016 15:55:29 +0100 Subject: [PATCH 11/11] Rename development env to preview --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index c3228ac88..3f7783596 100644 --- a/Makefile +++ b/Makefile @@ -35,9 +35,9 @@ check-env-vars: ## Check mandatory environment variables $(if ${AWS_ACCESS_KEY_ID},,$(error Must specify AWS_ACCESS_KEY_ID)) $(if ${AWS_SECRET_ACCESS_KEY},,$(error Must specify AWS_SECRET_ACCESS_KEY)) -.PHONY: development -development: ## Set environment to development - $(eval export DEPLOY_ENV=development) +.PHONY: preview +preview: ## Set environment to preview + $(eval export DEPLOY_ENV=preview) $(eval export DNS_NAME="notify.works") @true