From 84ea173ced188dd749c4c25a0ee533caecf333b5 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Thu, 25 Aug 2016 10:12:39 +0100 Subject: [PATCH] Removed updates to templates statistics - on create notification we updated the templates stats to record the usage. - this is now based on notification history - this update and associated tests are now removed, --- app/dao/notifications_dao.py | 12 --- tests/app/dao/test_notification_dao.py | 138 +------------------------ 2 files changed, 2 insertions(+), 148 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 136f90bd6..b13c48870 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -16,7 +16,6 @@ from app.models import ( Service, Notification, NotificationHistory, - Job, NotificationStatistics, TemplateStatistics, SMS_TYPE, @@ -139,17 +138,6 @@ def dao_create_notification(notification, notification_type): ) db.session.add(stats) - update_count = db.session.query(TemplateStatistics).filter_by( - day=date.today(), - service_id=notification.service_id, - template_id=notification.template_id - ).update({'usage_count': TemplateStatistics.usage_count + 1, 'updated_at': datetime.utcnow()}) - - if update_count == 0: - template_stats = TemplateStatistics(template_id=notification.template_id, - service_id=notification.service_id) - db.session.add(template_stats) - if not notification.id: # need to populate defaulted fields before we create the notification history object notification.id = uuid.uuid4() diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 8384e8b5e..c5ec627e8 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -458,15 +458,8 @@ def test_create_notification_creates_notification_with_personalisation(notify_db assert stats.emails_requested == 0 assert stats.sms_requested == 1 - template_stats = TemplateStatistics.query.filter( - TemplateStatistics.service_id == sample_template_with_placeholders.service.id, - TemplateStatistics.template_id == sample_template_with_placeholders.id).first() - assert template_stats.service_id == sample_template_with_placeholders.service.id - assert template_stats.template_id == sample_template_with_placeholders.id - assert template_stats.usage_count == 1 - -def test_save_notification_creates_sms_and_template_stats(sample_template, sample_job, mmg_provider): +def test_save_notification_creates_sms(sample_template, sample_job, mmg_provider): assert Notification.query.count() == 0 assert NotificationStatistics.query.count() == 0 assert TemplateStatistics.query.count() == 0 @@ -491,14 +484,8 @@ def test_save_notification_creates_sms_and_template_stats(sample_template, sampl 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 - -def test_save_notification_and_create_email_and_template_stats(sample_email_template, sample_job, ses_provider): +def test_save_notification_and_create_email(sample_email_template, sample_job, ses_provider): assert Notification.query.count() == 0 assert NotificationStatistics.query.count() == 0 assert TemplateStatistics.query.count() == 0 @@ -525,13 +512,6 @@ def test_save_notification_and_create_email_and_template_stats(sample_email_temp assert stats.emails_requested == 1 assert stats.sms_requested == 0 - template_stats = TemplateStatistics.query.filter(TemplateStatistics.service_id == sample_email_template.service.id, - TemplateStatistics.template_id == sample_email_template.id).first() # noqa - - assert template_stats.service_id == sample_email_template.service.id - assert template_stats.template_id == sample_email_template.id - assert template_stats.usage_count == 1 - @freeze_time("2016-01-01 00:00:00.000000") def test_save_notification_handles_midnight_properly(sample_template, sample_job, mmg_provider): @@ -832,120 +812,6 @@ def test_should_not_delete_failed_notifications_before_seven_days(notify_db, not assert Notification.query.first().to == 'do_not_delete' -@freeze_time("2016-03-30") -def test_save_new_notification_creates_template_stats(sample_template, sample_job, mmg_provider): - assert Notification.query.count() == 0 - assert TemplateStatistics.query.count() == 0 - data = _notification_json(sample_template, job_id=sample_job.id) - - notification = Notification(**data) - dao_create_notification(notification, sample_template.template_type) - - assert TemplateStatistics.query.count() == 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 - assert template_stats.day == date(2016, 3, 30) - - -@freeze_time("2016-03-30") -def test_save_new_notification_creates_template_stats_per_day(sample_template, sample_job, mmg_provider): - assert Notification.query.count() == 0 - assert TemplateStatistics.query.count() == 0 - data = _notification_json(sample_template, job_id=sample_job.id) - - notification = Notification(**data) - dao_create_notification(notification, sample_template.template_type) - - assert TemplateStatistics.query.count() == 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 - assert template_stats.day == date(2016, 3, 30) - - # move on one day - with freeze_time('2016-03-31'): - assert TemplateStatistics.query.count() == 1 - new_notification = Notification(**data) - dao_create_notification(new_notification, sample_template.template_type) - - assert TemplateStatistics.query.count() == 2 - first_stats = TemplateStatistics.query.filter(TemplateStatistics.day == datetime(2016, 3, 30)).first() - second_stats = TemplateStatistics.query.filter(TemplateStatistics.day == datetime(2016, 3, 31)).first() - - assert first_stats.template_id == second_stats.template_id - assert first_stats.service_id == second_stats.service_id - - assert first_stats.day == date(2016, 3, 30) - assert first_stats.usage_count == 1 - - assert second_stats.day == date(2016, 3, 31) - assert second_stats.usage_count == 1 - - -def test_save_another_notification_increments_template_stats(sample_template, sample_job, mmg_provider): - assert Notification.query.count() == 0 - assert TemplateStatistics.query.count() == 0 - data = _notification_json(sample_template, job_id=sample_job.id) - - notification_1 = Notification(**data) - notification_2 = Notification(**data) - dao_create_notification(notification_1, sample_template.template_type) - - assert TemplateStatistics.query.count() == 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 - - dao_create_notification(notification_2, sample_template.template_type) - - assert TemplateStatistics.query.count() == 1 - template_stats = TemplateStatistics.query.filter(TemplateStatistics.service_id == sample_template.service.id, - TemplateStatistics.template_id == sample_template.id).first() - assert template_stats.usage_count == 2 - - -def test_successful_notification_inserts_followed_by_failure_does_not_increment_template_stats(sample_template, - sample_job, - mmg_provider): - assert Notification.query.count() == 0 - assert NotificationStatistics.query.count() == 0 - assert TemplateStatistics.query.count() == 0 - - data = _notification_json(sample_template, job_id=sample_job.id) - - notification_1 = Notification(**data) - notification_2 = Notification(**data) - 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) - - _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, - 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 == 3 - - failing_notification = Notification(**data) - try: - # Mess up db in really bad way - db.session.execute('DROP TABLE TEMPLATE_STATISTICS') - dao_create_notification(failing_notification, sample_template.template_type) - except Exception as e: - # There should be no additional notification stats or counts - _assert_notification_stats(sample_template.service.id, sms_requested=3) - - @freeze_time("2016-01-10") def test_should_limit_notifications_return_by_day_limit_plus_one(notify_db, notify_db_session, sample_service): assert len(Notification.query.all()) == 0