From 25a1b7f31cfd16e509d8f5b24662b34c7f375a6a Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 26 May 2016 16:46:00 +0100 Subject: [PATCH] Firetext does not have a status code for temporary-failure. In order to set a message as temporary-failure, we check if it is in pending status first. Otherwise a delivery receipt for failure is set to permanent failure. --- app/clients/sms/firetext.py | 4 +- app/dao/notifications_dao.py | 55 ++-- app/models.py | 2 +- .../versions/0022_add_pending_status.py | 51 ++++ tests/app/clients/test_firetext.py | 4 +- tests/app/dao/test_notification_dao.py | 237 +++++++++--------- tests/app/notifications/test_rest.py | 8 +- tests/conftest.py | 1 + 8 files changed, 208 insertions(+), 154 deletions(-) create mode 100644 migrations/versions/0022_add_pending_status.py diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index a8f42fdcf..430bc2ec4 100644 --- a/app/clients/sms/firetext.py +++ b/app/clients/sms/firetext.py @@ -26,8 +26,8 @@ firetext_responses = { '2': { "message": 'Undelivered (Pending with Network)', "success": True, - "notification_statistics_status": STATISTICS_DELIVERED, - "notification_status": 'delivered' + "notification_statistics_status": None, + "notification_status": 'pending' } } diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 72fc3a2f7..52dc18720 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -152,7 +152,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_notification_stats_query(notification_type, 'requested')) + ).update(_update_notification_stats_query(notification_type, 'requested')) if update_count == 0: stats = NotificationStatistics( @@ -193,7 +193,7 @@ def dao_create_notification(notification, notification_type, provider_identifier db.session.add(notification) -def update_notification_stats_query(notification_type, status): +def _update_notification_stats_query(notification_type, status): mapping = { TEMPLATE_TYPE_SMS: { STATISTICS_REQUESTED: NotificationStatistics.sms_requested, @@ -216,14 +216,14 @@ def _update_statistics(notification, notification_statistics_status): day=notification.created_at.date(), service_id=notification.service_id ).update( - update_notification_stats_query(notification.template.template_type, notification_statistics_status) + _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)) + ).update(_update_job_stats_query(notification_statistics_status)) -def update_job_stats_query(status): +def _update_job_stats_query(status): mapping = { STATISTICS_FAILURE: Job.notifications_failed, STATISTICS_DELIVERED: Job.notifications_delivered @@ -231,23 +231,32 @@ def update_job_stats_query(status): return {mapping[status]: mapping[status] + 1} -def dao_update_notification(notification): - notification.updated_at = datetime.utcnow() - db.session.add(notification) - db.session.commit() +def _set_firetext_status(notification, status): + # Firetext will send pending, then send either succes or fail. + # If we go from pending to delivered we need to set failure type as temporary-failure + if notification.status == 'pending': + if status == 'permanent-failure': + status = 'temporary-failure' + return status -def update_notification_status_by_id(notification_id, status, notification_statistics_status): +def _update_notification_status(notification, status): + if not notification: + return 0 + status = _set_firetext_status(notification=notification, status=status) + count = db.session.query(Notification).filter( - Notification.id == notification_id, - Notification.status == 'sending' - ).update({ - Notification.status: status - }) + Notification.id == notification.id, + or_(Notification.status == 'sending', + Notification.status == 'pending')).update({Notification.status: status}) + return count + + +def update_notification_status_by_id(notification_id, status, notification_statistics_status=None): + notification = Notification.query.get(notification_id) + count = _update_notification_status(notification=notification, status=status) if count == 1 and notification_statistics_status: - notification = Notification.query.get(notification_id) - _update_statistics(notification, notification_statistics_status) db.session.commit() @@ -255,18 +264,22 @@ def update_notification_status_by_id(notification_id, status, notification_stati def update_notification_status_by_reference(reference, status, notification_statistics_status): - count = db.session.query(Notification).filter(Notification.reference == reference, - Notification.status == 'sending').update( - {Notification.status: status}) + notification = Notification.query.filter_by(reference=reference).first() + count = _update_notification_status(notification=notification, status=status) if count == 1: - notification = Notification.query.filter_by(reference=reference).first() _update_statistics(notification, notification_statistics_status) db.session.commit() return count +def dao_update_notification(notification): + notification.updated_at = datetime.utcnow() + db.session.add(notification) + db.session.commit() + + def update_notification_reference_by_id(id, reference): count = db.session.query(Notification).filter_by( id=id diff --git a/app/models.py b/app/models.py index 894398ea8..42a8f30b1 100644 --- a/app/models.py +++ b/app/models.py @@ -308,7 +308,7 @@ class VerifyCode(db.Model): return check_hash(cde, self._code) -NOTIFICATION_STATUS_TYPES = ['sending', 'delivered', 'failed', +NOTIFICATION_STATUS_TYPES = ['sending', 'delivered', 'pending', 'failed', 'technical-failure', 'temporary-failure', 'permanent-failure'] diff --git a/migrations/versions/0022_add_pending_status.py b/migrations/versions/0022_add_pending_status.py new file mode 100644 index 000000000..3c748a0b7 --- /dev/null +++ b/migrations/versions/0022_add_pending_status.py @@ -0,0 +1,51 @@ +"""empty message + +Revision ID: a726fff4ed59 +Revises: 0022_add_pending_status +Create Date: 2016-05-25 15:47:32.568097 + +""" + +# revision identifiers, used by Alembic. +revision = '0022_add_pending_status' +down_revision = '0021_add_delivered_failed_counts' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + status_type = sa.Enum('sending', 'delivered', 'pending', 'failed', + 'technical-failure', 'temporary-failure', 'permanent-failure', + name='notify_status_types') + status_type.create(op.get_bind()) + op.add_column('notifications', sa.Column('new_status', status_type, nullable=True)) + op.execute('update notifications set new_status = CAST(CAST(status as text) as notify_status_types)') + op.alter_column('notifications', 'status', new_column_name='old_status') + op.alter_column('notifications', 'new_status', new_column_name='status') + op.drop_column('notifications', 'old_status') + op.get_bind() + op.execute('DROP TYPE notification_status_type') + op.alter_column('notifications', 'status', nullable=False) + + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + status_type = sa.Enum('sending', 'delivered', 'failed', + 'technical-failure', 'temporary-failure', 'permanent-failure', + name='notification_status_type') + status_type.create(op.get_bind()) + op.add_column('notifications', sa.Column('old_status', status_type, nullable=True)) + + op.execute("update notifications set status = 'sending' where status = 'pending'") + op.execute('update notifications set old_status = CAST(CAST(status as text) as notification_status_type)') + op.alter_column('notifications', 'status', new_column_name='new_status') + op.alter_column('notifications', 'old_status', new_column_name='status') + op.drop_column('notifications', 'new_status') + op.get_bind() + op.execute('DROP TYPE notify_status_types') + op.alter_column('notifications', 'status', nullable=False) + ### end Alembic commands ### diff --git a/tests/app/clients/test_firetext.py b/tests/app/clients/test_firetext.py index 271e9ff99..96627068f 100644 --- a/tests/app/clients/test_firetext.py +++ b/tests/app/clients/test_firetext.py @@ -22,8 +22,8 @@ def test_should_return_correct_details_for_bounced(): def test_should_return_correct_details_for_complaint(): response_dict = get_firetext_responses('2') assert response_dict['message'] == 'Undelivered (Pending with Network)' - assert response_dict['notification_status'] == 'delivered' - assert response_dict['notification_statistics_status'] == 'delivered' + assert response_dict['notification_status'] == 'pending' + assert response_dict['notification_statistics_status'] is None assert response_dict['success'] diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index d1ef6c6ab..a69e62f71 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -56,10 +56,7 @@ 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' - 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 + _assert_notification_stats(notification.service_id, emails_delivered=1, emails_requested=1, emails_failed=0) def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_provider): @@ -70,15 +67,8 @@ def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_ count = update_notification_status_by_id(notification.id, 'delivered', 'delivered') assert count == 1 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 - assert job.notifications_sent == 1 - assert job.notifications_delivered == 1 - assert job.notifications_failed == 0 + _assert_notification_stats(notification.service_id, sms_delivered=1, sms_requested=1, sms_failed=0) + _assert_job_stats(notification.job_id, sent=1, count=1, delivered=1, failed=0) def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(notify_db, notify_db_session): @@ -91,6 +81,57 @@ def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(n assert job == Job.query.get(notification.job_id) +def test_should_by_able_to_update_status_by_id_from_pending_to_delivered(sample_template, sample_job): + 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=notification.id, status='pending') + assert count == 1 + assert Notification.query.get(notification.id).status == 'pending' + _assert_notification_stats(notification.service_id, sms_requested=1, sms_delivered=0, sms_failed=0) + _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=0) + + count = update_notification_status_by_id(notification.id, 'delivered', 'delivered') + assert count == 1 + assert Notification.query.get(notification.id).status == 'delivered' + _assert_notification_stats(notification.service_id, sms_requested=1, sms_delivered=1, sms_failed=0) + _assert_job_stats(notification.job_id, sent=1, count=1, delivered=1, failed=0) + + +def test_should_by_able_to_update_status_by_id_from_pending_to_temporary_failure(sample_template, sample_job): + 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=notification.id, status='pending') + assert count == 1 + assert Notification.query.get(notification.id).status == 'pending' + _assert_notification_stats(notification.service_id, sms_requested=1, sms_delivered=0, sms_failed=0) + _assert_job_stats(notification.job_id, sent=1, count=1, delivered=0, failed=0) + + count = update_notification_status_by_id(notification.id, status='permanent-failure', + notification_statistics_status='failure') + assert count == 1 + assert Notification.query.get(notification.id).status == 'temporary-failure' + _assert_notification_stats(notification.service_id, sms_delivered=0, sms_requested=1, sms_failed=1) + _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=1) + + +def test_should_by_able_to_update_status_by_id_from_sending_to_permanent_failure(sample_template, sample_job): + data = _notification_json(sample_template, job_id=sample_job.id) + notification = Notification(**data) + dao_create_notification(notification, sample_template.template_type, 'firetext') + assert Notification.query.get(notification.id).status == 'sending' + + count = update_notification_status_by_id(notification.id, status='permanent-failure', + notification_statistics_status='failure') + assert count == 1 + assert Notification.query.get(notification.id).status == 'permanent-failure' + _assert_notification_stats(notification.service_id, sms_requested=1, sms_delivered=0, sms_failed=1) + _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=1) + + 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) @@ -107,15 +148,8 @@ def test_should_not_update_status_one_notification_status_is_delivered(sample_em 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 + _assert_notification_stats(notification.service_id, emails_requested=1, emails_delivered=1, emails_failed=0) + _assert_job_stats(notification.job_id, sent=1, count=1, delivered=1, failed=0) def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification): @@ -123,15 +157,8 @@ def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification count = update_notification_status_by_id(sample_notification.id, 'permanent-failure', 'failure') assert count == 1 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 + _assert_notification_stats(sample_notification.service_id, sms_requested=1, sms_delivered=0, sms_failed=1) + _assert_job_stats(sample_notification.job_id, sent=1, count=1, delivered=0, failed=1) def test_should_be_able_to_record_statistics_failure_for_email(sample_email_template, sample_job, ses_provider): @@ -143,15 +170,8 @@ def test_should_be_able_to_record_statistics_failure_for_email(sample_email_temp count = update_notification_status_by_reference('reference', 'failed', 'failure') assert count == 1 assert Notification.query.get(notification.id).status == 'failed' - stats = NotificationStatistics.query.filter_by(service_id=notification.service.id).one() - 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 + _assert_notification_stats(notification.service_id, emails_requested=1, emails_delivered=0, emails_failed=1) + _assert_job_stats(notification.job_id, sent=1, count=1, delivered=0, failed=1) def test_should_return_zero_count_if_no_notification_with_id(): @@ -168,17 +188,8 @@ def test_should_be_able_to_get_statistics_for_a_service(sample_template, mmg_pro notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier) - stats = dao_get_notification_statistics_for_service(sample_template.service.id) - assert len(stats) == 1 - assert stats[0].emails_requested == 0 - assert stats[0].sms_requested == 1 - assert stats[0].sms_delivered == 0 - assert stats[0].sms_failed == 0 - assert stats[0].day == notification.created_at.date() - assert stats[0].service_id == notification.service_id - assert stats[0].emails_requested == 0 - assert stats[0].emails_delivered == 0 - assert stats[0].emails_failed == 0 + _assert_notification_stats(notification.service_id, sms_requested=1, + notification_created_at=notification.created_at.date()) def test_should_be_able_to_get_statistics_for_a_service_for_a_day(sample_template, mmg_provider): @@ -205,8 +216,7 @@ def test_should_return_none_if_no_statistics_for_a_service_for_a_day(sample_temp notification = Notification(**data) dao_create_notification(notification, sample_template.template_type, mmg_provider.identifier) assert not dao_get_notification_statistics_for_service_and_day( - sample_template.service.id, (datetime.utcnow() - timedelta(days=1)).date() - ) + sample_template.service.id, (datetime.utcnow() - timedelta(days=1)).date()) def test_should_be_able_to_get_all_statistics_for_a_service(sample_template, mmg_provider): @@ -219,10 +229,7 @@ def test_should_be_able_to_get_all_statistics_for_a_service(sample_template, mmg dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier) dao_create_notification(notification_3, sample_template.template_type, mmg_provider.identifier) - stats = dao_get_notification_statistics_for_service(sample_template.service.id) - assert len(stats) == 1 - assert stats[0].emails_requested == 0 - assert stats[0].sms_requested == 3 + _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): @@ -231,18 +238,13 @@ def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days(sam today = datetime.utcnow() yesterday = datetime.utcnow() - timedelta(days=1) two_days_ago = datetime.utcnow() - timedelta(days=2) - data.update({ - 'created_at': today - }) + data.update({'created_at': today}) notification_1 = Notification(**data) - data.update({ - 'created_at': yesterday - }) + data.update({'created_at': yesterday}) notification_2 = Notification(**data) - data.update({ - 'created_at': two_days_ago - }) + data.update({'created_at': two_days_ago}) notification_3 = Notification(**data) + dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier) dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier) dao_create_notification(notification_3, sample_template.template_type, mmg_provider.identifier) @@ -271,25 +273,17 @@ def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days_pre today = datetime.utcnow() seven_days_ago = datetime.utcnow() - timedelta(days=7) eight_days_ago = datetime.utcnow() - timedelta(days=8) - data.update({ - 'created_at': today - }) + data.update({'created_at': today}) notification_1 = Notification(**data) - data.update({ - 'created_at': seven_days_ago - }) + data.update({'created_at': seven_days_ago}) notification_2 = Notification(**data) - data.update({ - 'created_at': eight_days_ago - }) + data.update({'created_at': eight_days_ago}) notification_3 = Notification(**data) dao_create_notification(notification_1, sample_template.template_type, mmg_provider.identifier) dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier) dao_create_notification(notification_3, sample_template.template_type, mmg_provider.identifier) - stats = dao_get_notification_statistics_for_service( - sample_template.service.id, 7 - ) + 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 @@ -319,22 +313,14 @@ 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 - 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 - ).first() + _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=0) + stats = NotificationStatistics.query.filter(NotificationStatistics.service_id == sample_template.service.id).first() assert stats.emails_requested == 0 assert stats.sms_requested == 1 template_stats = TemplateStatistics.query.filter(TemplateStatistics.service_id == sample_template.service.id, TemplateStatistics.template_id == sample_template.id).first() - assert template_stats.service_id == sample_template.service.id assert template_stats.template_id == sample_template.id assert template_stats.usage_count == 1 @@ -361,15 +347,10 @@ 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 - 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 + _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=0) stats = NotificationStatistics.query.filter( - NotificationStatistics.service_id == sample_email_template.service.id - ).first() + NotificationStatistics.service_id == sample_email_template.service.id).first() assert stats.emails_requested == 1 assert stats.sms_requested == 0 @@ -393,8 +374,7 @@ def test_save_notification_handles_midnight_properly(sample_template, sample_job assert Notification.query.count() == 1 stats = NotificationStatistics.query.filter( - NotificationStatistics.service_id == sample_template.service.id - ).first() + NotificationStatistics.service_id == sample_template.service.id).first() assert stats.day == date(2016, 1, 1) @@ -410,8 +390,7 @@ def test_save_notification_handles_just_before_midnight_properly(sample_template assert Notification.query.count() == 1 stats = NotificationStatistics.query.filter( - NotificationStatistics.service_id == sample_template.service.id - ).first() + NotificationStatistics.service_id == sample_template.service.id).first() assert stats.day == date(2016, 1, 1) @@ -427,8 +406,7 @@ def test_save_notification_and_increment_email_stats(sample_email_template, samp assert Notification.query.count() == 1 stats1 = NotificationStatistics.query.filter( - NotificationStatistics.service_id == sample_email_template.service.id - ).first() + NotificationStatistics.service_id == sample_email_template.service.id).first() assert stats1.emails_requested == 1 assert stats1.sms_requested == 0 @@ -438,8 +416,7 @@ def test_save_notification_and_increment_email_stats(sample_email_template, samp assert Notification.query.count() == 2 stats2 = NotificationStatistics.query.filter( - NotificationStatistics.service_id == sample_email_template.service.id - ).first() + NotificationStatistics.service_id == sample_email_template.service.id).first() assert stats2.emails_requested == 2 assert stats2.sms_requested == 0 @@ -512,7 +489,8 @@ def test_save_notification_and_increment_job(sample_template, sample_job, mmg_pr notification_2 = Notification(**data) dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier) assert Notification.query.count() == 2 - assert Job.query.get(sample_job.id).notifications_sent == 2 + # count is off because the count is defaulted to 1 in the sample_job + _assert_job_stats(sample_job.id, sent=2, count=1) def test_should_not_increment_job_if_notification_fails_to_persist(sample_template, sample_job, mmg_provider): @@ -557,8 +535,9 @@ def test_save_notification_and_increment_correct_job(notify_db, notify_db_sessio 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(job_1.id).notifications_sent == 1 - assert Job.query.get(job_2.id).notifications_sent == 0 + assert job_1.id != job_2.id + _assert_job_stats(job_id=job_1.id, sent=1, count=1) + _assert_job_stats(job_id=job_2.id, sent=0, count=1) def test_save_notification_with_no_job(sample_template, mmg_provider): @@ -622,14 +601,9 @@ def test_get_all_notifications_for_job(notify_db, notify_db_session, sample_job) except IntegrityError: pass - notifcations_from_db = get_notifications_for_job(sample_job.service.id, sample_job.id).items - assert len(notifcations_from_db) == 5 - stats = NotificationStatistics.query.filter( - NotificationStatistics.service_id == sample_job.service.id - ).first() - - assert stats.emails_requested == 0 - assert stats.sms_requested == 5 + notifications_from_db = get_notifications_for_job(sample_job.service.id, sample_job.id).items + assert len(notifications_from_db) == 5 + _assert_notification_stats(sample_job.service.id, sms_requested=5) def test_update_notification(sample_notification, sample_template): @@ -769,11 +743,7 @@ def test_successful_notification_inserts_followed_by_failure_does_not_increment_ dao_create_notification(notification_2, sample_template.template_type, mmg_provider.identifier) dao_create_notification(notification_3, sample_template.template_type, mmg_provider.identifier) - assert NotificationStatistics.query.count() == 1 - notication_stats = NotificationStatistics.query.filter( - NotificationStatistics.service_id == sample_template.service.id - ).first() - assert notication_stats.sms_requested == 3 + _assert_notification_stats(sample_template.service.id, sms_requested=3) assert TemplateStatistics.query.count() == 1 template_stats = TemplateStatistics.query.filter(TemplateStatistics.service_id == sample_template.service.id, @@ -789,11 +759,7 @@ def test_successful_notification_inserts_followed_by_failure_does_not_increment_ dao_create_notification(failing_notification, sample_template.template_type, mmg_provider.identifier) except Exception as e: # There should be no additional notification stats or counts - assert NotificationStatistics.query.count() == 1 - notication_stats = NotificationStatistics.query.filter( - NotificationStatistics.service_id == sample_template.service.id - ).first() - assert notication_stats.sms_requested == 3 + _assert_notification_stats(sample_template.service.id, sms_requested=3) @freeze_time("2016-03-30") @@ -952,3 +918,26 @@ def _notification_json(sample_template, job_id=None, id=None): if id: data.update({'id': id}) return data + + +def _assert_notification_stats(service_id, + emails_delivered=0, emails_requested=0, emails_failed=0, + sms_delivered=0, sms_requested=0, sms_failed=0, + notification_created_at=None): + stats = NotificationStatistics.query.filter_by(service_id=service_id).all() + assert len(stats) == 1 + assert stats[0].emails_delivered == emails_delivered + assert stats[0].emails_requested == emails_requested + assert stats[0].emails_failed == emails_failed + assert stats[0].sms_delivered == sms_delivered + assert stats[0].sms_requested == sms_requested + assert stats[0].sms_failed == sms_failed + assert stats[0].day == notification_created_at if notification_created_at else True + + +def _assert_job_stats(job_id, sent=0, count=0, delivered=0, failed=0): + job = Job.query.get(job_id) + assert job.notifications_sent == sent + assert job.notification_count == count + assert job.notifications_delivered == delivered + assert job.notifications_failed == failed diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 6526ee6d7..c9c64a5f6 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -1241,7 +1241,7 @@ def test_firetext_callback_should_update_notification_status_failed(notify_api, assert stats.sms_failed == 1 -def test_firetext_callback_should_update_notification_status_sent(notify_api, notify_db, notify_db_session): +def test_firetext_callback_should_update_notification_status_pending(notify_api, notify_db, notify_db_session): with notify_api.test_request_context(): with notify_api.test_client() as client: notification = create_sample_notification(notify_db, notify_db_session, status='sending') @@ -1261,9 +1261,9 @@ def test_firetext_callback_should_update_notification_status_sent(notify_api, no assert json_resp['message'] == 'Firetext callback succeeded. reference {} updated'.format( notification.id ) - assert get_notification_by_id(notification.id).status == 'delivered' + assert get_notification_by_id(notification.id).status == 'pending' stats = dao_get_notification_statistics_for_service(notification.service_id)[0] - assert stats.sms_delivered == 1 + assert stats.sms_delivered == 0 assert stats.sms_requested == 1 assert stats.sms_failed == 0 @@ -1797,7 +1797,7 @@ def test_firetext_callback_should_record_statsd(notify_api, notify_db, notify_db client.post( path='/notifications/sms/firetext', - data='mobile=441234123123&status=2&time=2016-03-10 14:17:00&reference={}'.format( + data='mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference={}'.format( notification.id ), headers=[('Content-Type', 'application/x-www-form-urlencoded')]) diff --git a/tests/conftest.py b/tests/conftest.py index 24f3702ba..c323c672f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -40,6 +40,7 @@ def notify_db(notify_api, request): def teardown(): db.session.remove() db.drop_all() + db.engine.execute("drop type notify_status_types") db.engine.execute("drop table alembic_version") db.get_engine(notify_api).dispose()