Reorder updates to the notification / stats tables to match create.

This commit is contained in:
Martyn Inglis
2016-05-31 10:17:15 +01:00
parent a7a669a7c2
commit 2108bb52d5
4 changed files with 45 additions and 42 deletions

View File

@@ -214,15 +214,16 @@ def _update_notification_stats_query(notification_type, status):
def _update_statistics(notification, notification_statistics_status): def _update_statistics(notification, 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))
db.session.query(NotificationStatistics).filter_by( db.session.query(NotificationStatistics).filter_by(
day=notification.created_at.date(), day=notification.created_at.date(),
service_id=notification.service_id service_id=notification.service_id
).update( ).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))
def _update_job_stats_query(status): def _update_job_stats_query(status):
@@ -243,16 +244,14 @@ def _decide_permanent_temporary_failure(current_status, status):
def _update_notification_status(notification, status, notification_statistics_status): def _update_notification_status(notification, status, notification_statistics_status):
if not notification:
return 0
status = _decide_permanent_temporary_failure(current_status=notification.status, status=status) status = _decide_permanent_temporary_failure(current_status=notification.status, status=status)
count = db.session.query(Notification).filter(Notification.id == notification.id if notification_statistics_status:
).update({Notification.status: status})
if count == 1 and notification_statistics_status:
_update_statistics(notification, notification_statistics_status) _update_statistics(notification, notification_statistics_status)
return count db.session.query(Notification).filter(Notification.id == notification.id
).update({Notification.status: status})
return True
@transactional @transactional
@@ -262,10 +261,14 @@ def update_notification_status_by_id(notification_id, status, notification_stati
Notification.status == 'pending') Notification.status == 'pending')
).first() ).first()
count = _update_notification_status(notification=notification, if not notification:
status=status, return False
notification_statistics_status=notification_statistics_status)
return count return _update_notification_status(
notification=notification,
status=status,
notification_statistics_status=notification_statistics_status
)
@transactional @transactional
@@ -274,10 +277,14 @@ def update_notification_status_by_reference(reference, status, notification_stat
or_(Notification.status == 'sending', or_(Notification.status == 'sending',
Notification.status == 'pending') Notification.status == 'pending')
).first() ).first()
if not notification:
return False
return _update_notification_status(notification=notification, return _update_notification_status(
status=status, notification=notification,
notification_statistics_status=notification_statistics_status) status=status,
notification_statistics_status=notification_statistics_status
)
def dao_update_notification(notification): def dao_update_notification(notification):

View File

@@ -54,10 +54,9 @@ def process_sms_client_response(status, reference, client_name):
notification_success = response_dict['success'] notification_success = response_dict['success']
# record stats # record stats
update_success = notifications_dao.update_notification_status_by_id(reference, if not notifications_dao.update_notification_status_by_id(reference,
notification_status, notification_status,
notification_statistics_status) notification_statistics_status):
if update_success == 0:
status_error = "{} callback failed: notification {} either not found or already updated " \ status_error = "{} callback failed: notification {} either not found or already updated " \
"from sending. Status {}".format(client_name, "from sending. Status {}".format(client_name,
reference, reference,

View File

@@ -90,11 +90,11 @@ def process_ses_response():
), 200 ), 200
reference = ses_message['mail']['messageId'] reference = ses_message['mail']['messageId']
if notifications_dao.update_notification_status_by_reference( if not notifications_dao.update_notification_status_by_reference(
reference, reference,
notification_status, notification_status,
notification_statistics_status notification_statistics_status
) == 0: ):
message = "SES callback failed: notification either not found or already updated " \ message = "SES callback failed: notification either not found or already updated " \
"from sending. Status {}".format(notification_status) "from sending. Status {}".format(notification_status)
current_app.logger.info( current_app.logger.info(

View File

@@ -64,8 +64,7 @@ def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_
notification = Notification(**data) notification = Notification(**data)
dao_create_notification(notification, sample_template.template_type, 'mmg') dao_create_notification(notification, sample_template.template_type, 'mmg')
assert Notification.query.get(notification.id).status == 'sending' assert Notification.query.get(notification.id).status == 'sending'
count = update_notification_status_by_id(notification.id, 'delivered', 'delivered') assert update_notification_status_by_id(notification.id, 'delivered', 'delivered')
assert count == 1
assert Notification.query.get(notification.id).status == 'delivered' assert Notification.query.get(notification.id).status == 'delivered'
_assert_notification_stats(notification.service_id, sms_delivered=1, sms_requested=1, sms_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) _assert_job_stats(notification.job_id, sent=1, count=1, delivered=1, failed=0)
@@ -75,8 +74,7 @@ def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(n
notification = sample_notification(notify_db, notify_db_session, status='delivered') notification = sample_notification(notify_db, notify_db_session, status='delivered')
job = Job.query.get(notification.job_id) job = Job.query.get(notification.job_id)
assert Notification.query.get(notification.id).status == 'delivered' assert Notification.query.get(notification.id).status == 'delivered'
count = update_notification_status_by_id(notification.id, 'failed', 'failure') assert not update_notification_status_by_id(notification.id, 'failed', 'failure')
assert count == 0
assert Notification.query.get(notification.id).status == 'delivered' assert Notification.query.get(notification.id).status == 'delivered'
assert job == Job.query.get(notification.job_id) assert job == Job.query.get(notification.job_id)
@@ -86,14 +84,12 @@ def test_should_by_able_to_update_status_by_id_from_pending_to_delivered(sample_
notification = Notification(**data) notification = Notification(**data)
dao_create_notification(notification, sample_template.template_type, 'mmg') dao_create_notification(notification, sample_template.template_type, 'mmg')
assert Notification.query.get(notification.id).status == 'sending' assert Notification.query.get(notification.id).status == 'sending'
count = update_notification_status_by_id(notification_id=notification.id, status='pending') assert update_notification_status_by_id(notification_id=notification.id, status='pending')
assert count == 1
assert Notification.query.get(notification.id).status == 'pending' assert Notification.query.get(notification.id).status == 'pending'
_assert_notification_stats(notification.service_id, sms_requested=1, sms_delivered=0, sms_failed=0) _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) _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 update_notification_status_by_id(notification.id, 'delivered', 'delivered')
assert count == 1
assert Notification.query.get(notification.id).status == 'delivered' assert Notification.query.get(notification.id).status == 'delivered'
_assert_notification_stats(notification.service_id, sms_requested=1, sms_delivered=1, sms_failed=0) _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) _assert_job_stats(notification.job_id, sent=1, count=1, delivered=1, failed=0)
@@ -104,15 +100,15 @@ def test_should_by_able_to_update_status_by_id_from_pending_to_temporary_failure
notification = Notification(**data) notification = Notification(**data)
dao_create_notification(notification, sample_template.template_type, 'mmg') dao_create_notification(notification, sample_template.template_type, 'mmg')
assert Notification.query.get(notification.id).status == 'sending' assert Notification.query.get(notification.id).status == 'sending'
count = update_notification_status_by_id(notification_id=notification.id, status='pending') assert update_notification_status_by_id(notification_id=notification.id, status='pending')
assert count == 1
assert Notification.query.get(notification.id).status == 'pending' assert Notification.query.get(notification.id).status == 'pending'
_assert_notification_stats(notification.service_id, sms_requested=1, sms_delivered=0, sms_failed=0) _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) _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', assert update_notification_status_by_id(
notification_statistics_status='failure') notification.id,
assert count == 1 status='permanent-failure',
notification_statistics_status='failure')
assert Notification.query.get(notification.id).status == 'temporary-failure' 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_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) _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=1)
@@ -124,9 +120,11 @@ def test_should_by_able_to_update_status_by_id_from_sending_to_permanent_failure
dao_create_notification(notification, sample_template.template_type, 'firetext') dao_create_notification(notification, sample_template.template_type, 'firetext')
assert Notification.query.get(notification.id).status == 'sending' assert Notification.query.get(notification.id).status == 'sending'
count = update_notification_status_by_id(notification.id, status='permanent-failure', assert update_notification_status_by_id(
notification_statistics_status='failure') notification.id,
assert count == 1 status='permanent-failure',
notification_statistics_status='failure'
)
assert Notification.query.get(notification.id).status == 'permanent-failure' 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_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) _assert_job_stats(sample_job.id, sent=1, count=1, delivered=0, failed=1)
@@ -154,8 +152,7 @@ def test_should_not_update_status_one_notification_status_is_delivered(sample_em
def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification): def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification):
assert Notification.query.get(sample_notification.id).status == 'sending' assert Notification.query.get(sample_notification.id).status == 'sending'
count = update_notification_status_by_id(sample_notification.id, 'permanent-failure', 'failure') assert update_notification_status_by_id(sample_notification.id, 'permanent-failure', 'failure')
assert count == 1
assert Notification.query.get(sample_notification.id).status == 'permanent-failure' assert Notification.query.get(sample_notification.id).status == 'permanent-failure'
_assert_notification_stats(sample_notification.service_id, sms_requested=1, sms_delivered=0, sms_failed=1) _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) _assert_job_stats(sample_notification.job_id, sent=1, count=1, delivered=0, failed=1)
@@ -175,11 +172,11 @@ def test_should_be_able_to_record_statistics_failure_for_email(sample_email_temp
def test_should_return_zero_count_if_no_notification_with_id(): def test_should_return_zero_count_if_no_notification_with_id():
assert update_notification_status_by_id(str(uuid.uuid4()), 'delivered', 'delivered') == 0 assert not update_notification_status_by_id(str(uuid.uuid4()), 'delivered', 'delivered')
def test_should_return_zero_count_if_no_notification_with_reference(): def test_should_return_zero_count_if_no_notification_with_reference():
assert update_notification_status_by_reference('something', 'delivered', 'delivered') == 0 assert not update_notification_status_by_reference('something', 'delivered', 'delivered')
def test_should_be_able_to_get_statistics_for_a_service(sample_template, mmg_provider): def test_should_be_able_to_get_statistics_for_a_service(sample_template, mmg_provider):