Refactored the DAO to be clearer, and wrote tests for the bug whereby different types of inserts/updates caused others to reset.

This commit is contained in:
Martyn Inglis
2017-05-11 12:09:57 +01:00
parent 19c14982a2
commit 22a47106af
2 changed files with 312 additions and 34 deletions

View File

@@ -11,8 +11,8 @@ from app.models import (
SMS_TYPE,
LETTER_TYPE,
NOTIFICATION_STATUS_TYPES_FAILED,
NOTIFICATION_DELIVERED
)
NOTIFICATION_DELIVERED,
NOTIFICATION_SENT)
from app.statsd_decorators import statsd
@@ -29,22 +29,17 @@ def create_or_update_job_sending_statistics(notification):
@transactional
def __update_job_stats_sent_count(notification):
update = {
JobStatistics.emails_sent:
JobStatistics.emails_sent + 1 if notification.notification_type == EMAIL_TYPE else 0,
JobStatistics.sms_sent:
JobStatistics.sms_sent + 1 if notification.notification_type == SMS_TYPE else 0,
JobStatistics.letters_sent:
JobStatistics.letters_sent + 1 if notification.notification_type == LETTER_TYPE else 0
}
column = columns(notification.notification_type, 'sent')
return db.session.query(JobStatistics).filter_by(
job_id=notification.job_id,
).update(update)
).update({
column: column + 1
})
@transactional
def __insert_job_stats(notification):
stats = JobStatistics(
job_id=notification.job_id,
emails_sent=1 if notification.notification_type == EMAIL_TYPE else 0,
@@ -55,31 +50,43 @@ def __insert_job_stats(notification):
db.session.add(stats)
def columns(notification_type, status):
keys = {
EMAIL_TYPE: {
'failed': JobStatistics.emails_failed,
'delivered': JobStatistics.emails_delivered,
'sent': JobStatistics.emails_sent
},
SMS_TYPE: {
'failed': JobStatistics.sms_failed,
'delivered': JobStatistics.sms_delivered,
'sent': JobStatistics.sms_sent
},
LETTER_TYPE: {
'failed': JobStatistics.letters_failed,
'sent': JobStatistics.letters_sent
}
}
return keys.get(notification_type).get(status)
@transactional
def update_job_stats_outcome_count(notification):
update = None
if notification.status in NOTIFICATION_STATUS_TYPES_FAILED:
update = {
JobStatistics.emails_failed:
JobStatistics.emails_failed + 1 if notification.notification_type == EMAIL_TYPE else 0,
JobStatistics.sms_failed:
JobStatistics.sms_failed + 1 if notification.notification_type == SMS_TYPE else 0,
JobStatistics.letters_failed:
JobStatistics.letters_failed + 1 if notification.notification_type == LETTER_TYPE else 0
}
column = columns(notification.notification_type, 'failed')
elif notification.status == NOTIFICATION_DELIVERED and notification.notification_type != LETTER_TYPE:
update = {
JobStatistics.emails_delivered:
JobStatistics.emails_delivered + 1 if notification.notification_type == EMAIL_TYPE else 0,
JobStatistics.sms_delivered:
JobStatistics.sms_delivered + 1 if notification.notification_type == SMS_TYPE else 0
}
elif notification.status in [NOTIFICATION_DELIVERED,
NOTIFICATION_SENT] and notification.notification_type != LETTER_TYPE:
column = columns(notification.notification_type, 'delivered')
if update:
else:
column = None
if column:
return db.session.query(JobStatistics).filter_by(
job_id=notification.job_id,
).update(update)
).update({
column: column + 1
})
else:
return 0

View File

@@ -271,21 +271,82 @@ def test_should_update_a_stats_entry_with_its_error_outcomes_for_a_job(
assert stat.sms_delivered == 0
@pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count, status', [
(SMS_TYPE, 1, 0, 0, NOTIFICATION_DELIVERED),
(SMS_TYPE, 1, 0, 0, NOTIFICATION_SENT),
(EMAIL_TYPE, 0, 1, 0, NOTIFICATION_DELIVERED),
(EMAIL_TYPE, 0, 1, 0, NOTIFICATION_SENT),
(LETTER_TYPE, 0, 0, 1, NOTIFICATION_SENT),
(LETTER_TYPE, 0, 0, 1, NOTIFICATION_DELIVERED),
])
def test_should_update_a_stats_entry_with_its_success_outcomes_for_a_job(
notify_db,
notify_db_session,
sample_job,
sample_letter_template,
notification_type,
sms_count,
email_count,
letter_count,
status
):
assert not len(JobStatistics.query.all())
template = None
if notification_type == SMS_TYPE:
template = sample_template(notify_db, notify_db_session, service=sample_job.service)
if notification_type == EMAIL_TYPE:
template = sample_email_template(notify_db, notify_db_session, service=sample_job.service)
if notification_type == LETTER_TYPE:
template = sample_letter_template
notification = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=template,
job=sample_job,
status=status
)
create_or_update_job_sending_statistics(notification)
stats = JobStatistics.query.all()
assert len(stats) == 1
update_job_stats_outcome_count(notification)
stat = stats[0]
assert stat.job_id == sample_job.id
assert stat.emails_sent == email_count
assert stat.sms_sent == sms_count
assert stat.letters_sent == letter_count
assert stat.emails_failed == 0
assert stat.letters_failed == 0
assert stat.sms_failed == 0
assert stat.emails_delivered == email_count
assert stat.sms_delivered == sms_count
@pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count, status', [
(SMS_TYPE, 1, 0, 0, NOTIFICATION_PENDING),
(SMS_TYPE, 1, 0, 0, NOTIFICATION_CREATED),
(SMS_TYPE, 1, 0, 0, NOTIFICATION_FAILED),
(SMS_TYPE, 1, 0, 0, NOTIFICATION_SENT),
(SMS_TYPE, 1, 0, 0, NOTIFICATION_SENDING),
(EMAIL_TYPE, 0, 1, 0, NOTIFICATION_PENDING),
(EMAIL_TYPE, 0, 1, 0, NOTIFICATION_CREATED),
(EMAIL_TYPE, 0, 1, 0, NOTIFICATION_FAILED),
(EMAIL_TYPE, 0, 1, 0, NOTIFICATION_SENT),
(EMAIL_TYPE, 0, 1, 0, NOTIFICATION_SENDING),
(LETTER_TYPE, 0, 0, 1, NOTIFICATION_PENDING),
(LETTER_TYPE, 0, 0, 1, NOTIFICATION_CREATED),
(LETTER_TYPE, 0, 0, 1, NOTIFICATION_FAILED),
(LETTER_TYPE, 0, 0, 1, NOTIFICATION_SENT),
(LETTER_TYPE, 0, 0, 1, NOTIFICATION_SENDING)
])
def test_should_not_update_job_stats_if_irrelevant_status(
@@ -342,3 +403,213 @@ def test_should_not_update_job_stats_if_irrelevant_status(
assert stat.emails_delivered == 0
assert stat.sms_delivered == 0
@pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count', [
(SMS_TYPE, 2, 1, 1),
(EMAIL_TYPE, 1, 2, 1),
(LETTER_TYPE, 1, 1, 2)
])
def test_inserting_one_type_of_notification_maintains_other_counts(
notify_db,
notify_db_session,
sample_job,
sample_letter_template,
notification_type,
sms_count,
email_count,
letter_count
):
assert not len(JobStatistics.query.all())
sms_template = sample_template(notify_db, notify_db_session, service=sample_job.service)
email_template = sample_email_template(notify_db, notify_db_session, service=sample_job.service)
letter_template = sample_letter_template
template = None
if notification_type == SMS_TYPE:
template = sms_template
if notification_type == EMAIL_TYPE:
template = email_template
if notification_type == LETTER_TYPE:
template = letter_template
notification = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=template,
job=sample_job,
status=NOTIFICATION_CREATED
)
letter = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=letter_template,
job=sample_job,
status=NOTIFICATION_CREATED
)
email = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=email_template,
job=sample_job,
status=NOTIFICATION_CREATED
)
sms = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=sms_template,
job=sample_job,
status=NOTIFICATION_CREATED
)
create_or_update_job_sending_statistics(email)
create_or_update_job_sending_statistics(sms)
create_or_update_job_sending_statistics(letter)
intitial_stats = JobStatistics.query.all()
assert len(intitial_stats) == 1
assert intitial_stats[0].emails_sent == 1
assert intitial_stats[0].sms_sent == 1
assert intitial_stats[0].letters_sent == 1
create_or_update_job_sending_statistics(notification)
updated_stats = JobStatistics.query.all()
assert updated_stats[0].job_id == sample_job.id
assert updated_stats[0].emails_sent == email_count
assert updated_stats[0].sms_sent == sms_count
assert updated_stats[0].letters_sent == letter_count
def test_updating_one_type_of_notification_to_success_maintains_other_counts(
notify_db,
notify_db_session,
sample_job,
sample_letter_template
):
assert not len(JobStatistics.query.all())
sms_template = sample_template(notify_db, notify_db_session, service=sample_job.service)
email_template = sample_email_template(notify_db, notify_db_session, service=sample_job.service)
letter_template = sample_letter_template
letter = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=letter_template,
job=sample_job,
status=NOTIFICATION_CREATED
)
email = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=email_template,
job=sample_job,
status=NOTIFICATION_CREATED
)
sms = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=sms_template,
job=sample_job,
status=NOTIFICATION_CREATED
)
create_or_update_job_sending_statistics(email)
create_or_update_job_sending_statistics(sms)
create_or_update_job_sending_statistics(letter)
sms.status = NOTIFICATION_DELIVERED
email.status = NOTIFICATION_DELIVERED
letter.status = NOTIFICATION_DELIVERED
update_job_stats_outcome_count(letter)
update_job_stats_outcome_count(email)
update_job_stats_outcome_count(sms)
stats = JobStatistics.query.all()
assert len(stats) == 1
assert stats[0].emails_sent == 1
assert stats[0].sms_sent == 1
assert stats[0].letters_sent == 1
assert stats[0].emails_delivered == 1
assert stats[0].sms_delivered == 1
def test_updating_one_type_of_notification_to_error_maintains_other_counts(
notify_db,
notify_db_session,
sample_job,
sample_letter_template
):
assert not len(JobStatistics.query.all())
sms_template = sample_template(notify_db, notify_db_session, service=sample_job.service)
email_template = sample_email_template(notify_db, notify_db_session, service=sample_job.service)
letter_template = sample_letter_template
letter = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=letter_template,
job=sample_job,
status=NOTIFICATION_CREATED
)
email = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=email_template,
job=sample_job,
status=NOTIFICATION_CREATED
)
sms = sample_notification(
notify_db,
notify_db_session,
service=sample_job.service,
template=sms_template,
job=sample_job,
status=NOTIFICATION_CREATED
)
create_or_update_job_sending_statistics(email)
create_or_update_job_sending_statistics(sms)
create_or_update_job_sending_statistics(letter)
sms.status = NOTIFICATION_TECHNICAL_FAILURE
email.status = NOTIFICATION_TECHNICAL_FAILURE
letter.status = NOTIFICATION_TECHNICAL_FAILURE
update_job_stats_outcome_count(letter)
update_job_stats_outcome_count(email)
update_job_stats_outcome_count(sms)
stats = JobStatistics.query.all()
assert len(stats) == 1
assert stats[0].emails_sent == 1
assert stats[0].sms_sent == 1
assert stats[0].letters_sent == 1
assert stats[0].emails_delivered == 0
assert stats[0].sms_delivered == 0
assert stats[0].sms_failed == 1
assert stats[0].emails_failed == 1