diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 57232b632..4a8d279cb 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -1,4 +1,4 @@ -from sqlalchemy import (desc, func, Integer, and_, or_, asc) +from sqlalchemy import (desc, func, Integer, or_, asc) from sqlalchemy.sql.expression import cast from datetime import ( @@ -35,11 +35,11 @@ from app.dao.dao_utils import transactional def dao_get_notification_statistics_for_service(service_id, limit_days=None): - filter = [NotificationStatistics.service_id == service_id] + query_filter = [NotificationStatistics.service_id == service_id] if limit_days is not None: - filter.append(NotificationStatistics.day >= days_ago(limit_days)) + query_filter.append(NotificationStatistics.day >= days_ago(limit_days)) return NotificationStatistics.query.filter( - *filter + *query_filter ).order_by( desc(NotificationStatistics.day) ).all() @@ -53,9 +53,7 @@ def dao_get_notification_statistics_for_service_and_day(service_id, day): def dao_get_notification_statistics_for_day(day): - return NotificationStatistics.query.filter_by( - day=day - ).all() + return NotificationStatistics.query.filter_by(day=day).all() def dao_get_potential_notification_statistics_for_day(day): @@ -131,10 +129,10 @@ def dao_get_7_day_agg_notification_statistics_for_service(service_id, def dao_get_template_statistics_for_service(service_id, limit_days=None): - filter = [TemplateStatistics.service_id == service_id] + query_filter = [TemplateStatistics.service_id == service_id] if limit_days is not None: - filter.append(TemplateStatistics.day >= days_ago(limit_days)) - return TemplateStatistics.query.filter(*filter).order_by( + query_filter.append(TemplateStatistics.day >= days_ago(limit_days)) + return TemplateStatistics.query.filter(*query_filter).order_by( desc(TemplateStatistics.updated_at)).all() @@ -153,7 +151,7 @@ def dao_create_notification(notification, notification_type, provider_identifier update_count = db.session.query(NotificationStatistics).filter_by( day=notification.created_at.date(), service_id=notification.service_id - ).update(update_query(notification_type, 'requested')) + ).update(update_notification_stats_query(notification_type, 'requested')) if update_count == 0: stats = NotificationStatistics( @@ -194,7 +192,7 @@ def dao_create_notification(notification, notification_type, provider_identifier db.session.add(notification) -def update_query(notification_type, status): +def update_notification_stats_query(notification_type, status): mapping = { TEMPLATE_TYPE_SMS: { STATISTICS_REQUESTED: NotificationStatistics.sms_requested, @@ -212,6 +210,26 @@ def update_query(notification_type, status): } +def _update_statistics(notification, notification_statistics_status): + db.session.query(NotificationStatistics).filter_by( + day=notification.created_at.date(), + service_id=notification.service_id + ).update( + update_notification_stats_query(notification.template.template_type, notification_statistics_status) + ) + if notification.job_id: + db.session.query(Job).filter_by(id=notification.job_id + ).update(update_job_stats_query(notification_statistics_status)) + + +def update_job_stats_query(status): + mapping = { + STATISTICS_FAILURE: Job.notifications_failed, + STATISTICS_DELIVERED: Job.notifications_delivered + } + return {mapping[status]: mapping[status] + 1} + + def dao_update_notification(notification): notification.updated_at = datetime.utcnow() db.session.add(notification) @@ -229,12 +247,7 @@ def update_notification_status_by_id(notification_id, status, notification_stati if count == 1 and notification_statistics_status: notification = Notification.query.get(notification_id) - db.session.query(NotificationStatistics).filter_by( - day=notification.created_at.date(), - service_id=notification.service_id - ).update( - update_query(notification.template.template_type, notification_statistics_status) - ) + _update_statistics(notification, notification_statistics_status) db.session.commit() return count @@ -246,16 +259,8 @@ def update_notification_status_by_reference(reference, status, notification_stat {Notification.status: status}) if count == 1: - notification = Notification.query.filter_by( - reference=reference - ).first() - - db.session.query(NotificationStatistics).filter_by( - day=notification.created_at.date(), - service_id=notification.service_id - ).update( - update_query(notification.template.template_type, notification_statistics_status) - ) + notification = Notification.query.filter_by(reference=reference).first() + _update_statistics(notification, notification_statistics_status) db.session.commit() return count @@ -279,7 +284,7 @@ def get_notifications_for_job(service_id, job_id, filter_dict=None, page=1, page if page_size is None: page_size = current_app.config['PAGE_SIZE'] query = Notification.query.filter_by(service_id=service_id, job_id=job_id) - query = filter_query(query, filter_dict) + query = _filter_query(query, filter_dict) return query.order_by(asc(Notification.job_row_number)).paginate( page=page, per_page=page_size @@ -308,14 +313,14 @@ def get_notifications_for_service(service_id, filters.append(func.date(Notification.created_at) >= days_ago) query = Notification.query.filter(*filters) - query = filter_query(query, filter_dict) + query = _filter_query(query, filter_dict) return query.order_by(desc(Notification.created_at)).paginate( page=page, per_page=page_size ) -def filter_query(query, filter_dict=None): +def _filter_query(query, filter_dict=None): if filter_dict is None: filter_dict = MultiDict() else: diff --git a/tests/app/conftest.py b/tests/app/conftest.py index b9bcf91a3..cdb4cb0ec 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -281,7 +281,7 @@ def sample_email_job(notify_db, service=service) job_id = uuid.uuid4() data = { - 'id': uuid.uuid4(), + 'id': job_id, 'service_id': service.id, 'service': service, 'template_id': template.id, diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 674f018a4..83e9559eb 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -33,8 +33,7 @@ from app.dao.notifications_dao import ( from notifications_utils.template import get_sms_fragment_count -from tests.app.conftest import sample_job -from tests.app.conftest import sample_notification +from tests.app.conftest import (sample_notification) def test_should_by_able_to_update_reference_by_id(sample_notification): @@ -57,38 +56,40 @@ def test_should_by_able_to_update_status_by_reference(sample_email_template, ses update_notification_reference_by_id(notification.id, 'reference') update_notification_status_by_reference('reference', 'delivered', 'delivered') assert Notification.query.get(notification.id).status == 'delivered' - assert NotificationStatistics.query.filter_by( - service_id=sample_email_template.service.id - ).one().emails_delivered == 1 - assert NotificationStatistics.query.filter_by( - service_id=sample_email_template.service.id - ).one().emails_requested == 1 - assert NotificationStatistics.query.filter_by( - service_id=sample_email_template.service.id - ).one().emails_failed == 0 + stats = NotificationStatistics.query.filter_by(service_id=sample_email_template.service.id).one() + assert stats.emails_delivered == 1 + assert stats.emails_requested == 1 + assert stats.emails_failed == 0 -def test_should_by_able_to_update_status_by_id(sample_notification): - assert Notification.query.get(sample_notification.id).status == 'sending' - count = update_notification_status_by_id(sample_notification.id, 'delivered', 'delivered') +def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_provider): + data = _notification_json(sample_template, job_id=sample_job.id) + notification = Notification(**data) + dao_create_notification(notification, sample_template.template_type, 'mmg') + assert Notification.query.get(notification.id).status == 'sending' + count = update_notification_status_by_id(notification.id, 'delivered', 'delivered') assert count == 1 - assert Notification.query.get(sample_notification.id).status == 'delivered' - stats = NotificationStatistics.query.filter_by(service_id=sample_notification.service.id).one() + assert Notification.query.get(notification.id).status == 'delivered' + stats = NotificationStatistics.query.filter_by(service_id=notification.service.id).one() assert stats.sms_delivered == 1 assert stats.sms_requested == 1 assert stats.sms_failed == 0 + job = Job.query.get(sample_job.id) + assert job.notification_count == 1 -def test_should_not_update_status_by_id_if_not_sending(notify_db, notify_db_session): +def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(notify_db, notify_db_session): notification = sample_notification(notify_db, notify_db_session, status='delivered') + job = Job.query.get(notification.job_id) assert Notification.query.get(notification.id).status == 'delivered' count = update_notification_status_by_id(notification.id, 'failed', 'failure') assert count == 0 assert Notification.query.get(notification.id).status == 'delivered' + assert job == Job.query.get(notification.job_id) -def test_should_not_update_status_one_notification_status_is_delivered(sample_email_template, ses_provider): - data = _notification_json(sample_email_template) +def test_should_not_update_status_one_notification_status_is_delivered(sample_email_template, sample_job, ses_provider): + data = _notification_json(sample_email_template, job_id=sample_job.id) notification = Notification(**data) dao_create_notification( @@ -101,28 +102,37 @@ def test_should_not_update_status_one_notification_status_is_delivered(sample_em update_notification_status_by_reference('reference', 'delivered', 'delivered') assert Notification.query.get(notification.id).status == 'delivered' - update_notification_status_by_reference('reference', 'delivered', 'temporary-failure') + update_notification_status_by_reference('reference', 'failed', 'temporary-failure') assert Notification.query.get(notification.id).status == 'delivered' stats = NotificationStatistics.query.filter_by(service_id=sample_email_template.service.id).one() assert stats.emails_delivered == 1 assert stats.emails_requested == 1 assert stats.emails_failed == 0 + job = Job.query.get(notification.job_id) + assert job.notification_count == 1 + assert job.notifications_delivered == 1 + assert job.notifications_failed == 0 + assert job.notifications_sent == 1 def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification): assert Notification.query.get(sample_notification.id).status == 'sending' - count = update_notification_status_by_id(sample_notification.id, 'delivered', 'failure') + count = update_notification_status_by_id(sample_notification.id, 'permanent-failure', 'failure') assert count == 1 - assert Notification.query.get(sample_notification.id).status == 'delivered' + assert Notification.query.get(sample_notification.id).status == 'permanent-failure' stats = NotificationStatistics.query.filter_by(service_id=sample_notification.service.id).one() assert stats.sms_delivered == 0 assert stats.sms_requested == 1 assert stats.sms_failed == 1 + job_stats = Job.query.filter_by(id=sample_notification.job_id).first() + assert job_stats.notification_count == 1 + assert job_stats.notifications_sent == 1 + assert job_stats.notifications_failed == 1 + assert job_stats.notifications_delivered == 0 -def test_should_be_able_to_record_statistics_failure_for_email(sample_email_template, ses_provider): - data = _notification_json(sample_email_template) - +def test_should_be_able_to_record_statistics_failure_for_email(sample_email_template, sample_job, ses_provider): + data = _notification_json(sample_email_template, job_id=sample_job.id) notification = Notification(**data) dao_create_notification(notification, sample_email_template.template_type, ses_provider.identifier) @@ -134,6 +144,11 @@ def test_should_be_able_to_record_statistics_failure_for_email(sample_email_temp assert stats.emails_delivered == 0 assert stats.emails_requested == 1 assert stats.emails_failed == 1 + job_stats = Job.query.filter_by(id=notification.job_id).first() + assert job_stats.notification_count == 1 + assert job_stats.notifications_sent == 1 + assert job_stats.notifications_failed == 1 + assert job_stats.notifications_delivered == 0 def test_should_return_zero_count_if_no_notification_with_id(): @@ -301,7 +316,11 @@ def test_save_notification_creates_sms_and_template_stats(sample_template, sampl assert data['template_version'] == notification_from_db.template_version assert data['created_at'] == notification_from_db.created_at assert 'sending' == notification_from_db.status - assert Job.query.get(sample_job.id).notifications_sent == 1 + job = Job.query.get(sample_job.id) + assert job.notifications_sent == 1 + assert job.notification_count == 1 + assert job.notifications_failed == 0 + assert job.notifications_delivered == 0 stats = NotificationStatistics.query.filter( NotificationStatistics.service_id == sample_template.service.id @@ -339,7 +358,11 @@ def test_save_notification_and_create_email_and_template_stats(sample_email_temp assert data['template_version'] == notification_from_db.template_version assert data['created_at'] == notification_from_db.created_at assert 'sending' == notification_from_db.status - assert Job.query.get(sample_job.id).notifications_sent == 1 + job = Job.query.get(sample_job.id) + assert job.notifications_sent == 1 + assert job.notification_count == 1 + assert job.notifications_delivered == 0 + assert job.notifications_failed == 0 stats = NotificationStatistics.query.filter( NotificationStatistics.service_id == sample_email_template.service.id @@ -511,6 +534,7 @@ def test_should_not_increment_job_if_notification_fails_to_persist(sample_templa def test_save_notification_and_increment_correct_job(notify_db, notify_db_session, sample_template, mmg_provider): + from tests.app.conftest import sample_job job_1 = sample_job(notify_db, notify_db_session, sample_template.service) job_2 = sample_job(notify_db, notify_db_session, sample_template.service) @@ -585,7 +609,6 @@ def test_get_notification_for_job(sample_notification): def test_get_all_notifications_for_job(notify_db, notify_db_session, sample_job): - from tests.app.conftest import sample_notification for i in range(0, 5): try: sample_notification(notify_db, @@ -910,7 +933,7 @@ def test_sms_fragment_count(char_count, expected_sms_fragment_count): assert get_sms_fragment_count(char_count) == expected_sms_fragment_count -def _notification_json(sample_template, job_id=None, id=None): +def _notification_json(sample_template, job_id=None, id=None, reference=None, status=None): data = { 'to': '+44709123456', 'service': sample_template.service, @@ -925,4 +948,8 @@ def _notification_json(sample_template, job_id=None, id=None): data.update({'job_id': job_id}) if id: data.update({'id': id}) + if reference: + data.update({'reference': reference}) + if status: + data.update({'status': status}) return data