diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index ca9a016e7..eedcbc6b4 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -77,8 +77,6 @@ def dao_create_notification(notification): notification.status = NOTIFICATION_CREATED db.session.add(notification) - if _should_record_notification_in_history_table(notification): - db.session.add(NotificationHistory.from_original(notification)) def _should_record_notification_in_history_table(notification): @@ -423,13 +421,10 @@ def _delete_letters_from_s3( @statsd(namespace="dao") @transactional -def dao_delete_notifications_and_history_by_id(notification_id): +def dao_delete_notifications_by_id(notification_id): db.session.query(Notification).filter( Notification.id == notification_id ).delete(synchronize_session='fetch') - db.session.query(NotificationHistory).filter( - NotificationHistory.id == notification_id - ).delete(synchronize_session='fetch') def _timeout_notifications(current_statuses, new_status, timeout_start, updated_at): @@ -438,17 +433,14 @@ def _timeout_notifications(current_statuses, new_status, timeout_start, updated_ Notification.status.in_(current_statuses), Notification.notification_type != LETTER_TYPE ).all() - for table in [NotificationHistory, Notification]: - q = table.query.filter( - table.created_at < timeout_start, - table.status.in_(current_statuses), - table.notification_type != LETTER_TYPE - ) - q.update( - {'status': new_status, 'updated_at': updated_at}, - synchronize_session=False - ) - # return a list of q = notification_ids in Notification table for sending delivery receipts + Notification.query.filter( + Notification.created_at < timeout_start, + Notification.status.in_(current_statuses), + Notification.notification_type != LETTER_TYPE + ).update( + {'status': new_status, 'updated_at': updated_at}, + synchronize_session=False + ) return notifications diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 7acc19d27..2280896d0 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -27,7 +27,7 @@ from app.models import ( ) from app.dao.notifications_dao import ( dao_create_notification, - dao_delete_notifications_and_history_by_id, + dao_delete_notifications_by_id, dao_created_scheduled_notification ) @@ -142,7 +142,7 @@ def send_notification_to_queue(notification, research_mode, queue=None): try: deliver_task.apply_async([str(notification.id)], queue=queue) except Exception: - dao_delete_notifications_and_history_by_id(notification.id) + dao_delete_notifications_by_id(notification.id) raise current_app.logger.debug( diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index 39c25725a..1f4a10cfa 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -7,7 +7,6 @@ from flask import current_app from app.exceptions import DVLAException, NotificationTechnicalFailureException from app.models import ( - Notification, NotificationHistory, NOTIFICATION_CREATED, NOTIFICATION_DELIVERED, @@ -28,7 +27,7 @@ from app.celery.tasks import ( ) from app.dao.daily_sorted_letter_dao import dao_get_daily_sorted_letter_by_billing_day -from tests.app.db import create_notification, create_service_callback_api +from tests.app.db import create_notification, create_service_callback_api, create_notification_history from tests.conftest import set_config @@ -56,9 +55,8 @@ def test_update_letter_notification_statuses_when_notification_does_not_exist_up ): valid_file = 'ref-foo|Sent|1|Unsorted' mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) - notification = create_notification(sample_letter_template, reference='ref-foo', status=NOTIFICATION_SENDING, - billable_units=1) - Notification.query.filter_by(id=notification.id).delete() + notification = create_notification_history(sample_letter_template, reference='ref-foo', status=NOTIFICATION_SENDING, + billable_units=1) update_letter_notifications_statuses(filename="NOTIFY-20170823160812-RSP.TXT") diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 80dcb4e66..fdffffc25 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -14,7 +14,6 @@ from notifications_utils.columns import Row from app import ( DATETIME_FORMAT, - db, encryption ) from app.celery import provider_tasks @@ -60,6 +59,7 @@ from tests.app.db import ( create_user, create_reply_to_email, create_service_with_defined_sms_sender, + create_notification_history ) from tests.conftest import set_config_values @@ -1601,11 +1601,9 @@ def test_process_returned_letters_list(sample_letter_template): def test_process_returned_letters_list_updates_history_if_notification_is_already_purged( sample_letter_template ): - create_notification(sample_letter_template, reference='ref1') - create_notification(sample_letter_template, reference='ref2') + create_notification_history(sample_letter_template, reference='ref1') + create_notification_history(sample_letter_template, reference='ref2') - Notification.query.delete() - db.session.commit() process_returned_letters_list(['ref1', 'ref2', 'unknown-ref']) notifications = NotificationHistory.query.all() diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 59bbc561d..8bbbb98bd 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -11,7 +11,7 @@ from sqlalchemy.orm.exc import NoResultFound from app.dao.notifications_dao import ( dao_create_notification, dao_created_scheduled_notification, - dao_delete_notifications_and_history_by_id, + dao_delete_notifications_by_id, dao_get_last_notification_added_for_job_id, dao_get_last_template_usage, dao_get_notifications_by_to_field, @@ -55,7 +55,8 @@ from tests.app.db import ( create_job, create_notification, create_service, - create_template + create_template, + create_notification_history ) @@ -71,7 +72,7 @@ def test_should_have_decorated_notifications_dao_functions(): assert get_notifications_for_service.__wrapped__.__name__ == 'get_notifications_for_service' # noqa assert get_notification_by_id.__wrapped__.__name__ == 'get_notification_by_id' # noqa assert delete_notifications_older_than_retention_by_type.__wrapped__.__name__ == 'delete_notifications_older_than_retention_by_type' # noqa - assert dao_delete_notifications_and_history_by_id.__wrapped__.__name__ == 'dao_delete_notifications_and_history_by_id' # noqa + assert dao_delete_notifications_by_id.__wrapped__.__name__ == 'dao_delete_notifications_by_id' # noqa def test_should_by_able_to_update_status_by_reference(sample_email_template, ses_provider): @@ -213,10 +214,8 @@ def test_should_not_update_status_by_reference_if_not_sending(sample_template): 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, status='sending') - notification = Notification(**data) - dao_create_notification(notification) - assert Notification.query.get(notification.id).status == 'sending' + notification = create_notification(template=sample_template, job=sample_job, status='sending') + assert update_notification_status_by_id(notification_id=notification.id, status='pending') assert Notification.query.get(notification.id).status == 'pending' @@ -225,16 +224,13 @@ def test_should_by_able_to_update_status_by_id_from_pending_to_delivered(sample_ 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, status='sending') - notification = Notification(**data) - dao_create_notification(notification) - assert Notification.query.get(notification.id).status == 'sending' + notification = create_notification(template=sample_template, job=sample_job, status='sending') + assert update_notification_status_by_id(notification_id=notification.id, status='pending') assert Notification.query.get(notification.id).status == 'pending' - assert update_notification_status_by_id( - notification.id, - status='permanent-failure') + assert update_notification_status_by_id(notification.id, status='permanent-failure') + assert Notification.query.get(notification.id).status == 'temporary-failure' @@ -335,7 +331,7 @@ def test_save_notification_and_create_email(sample_email_template, sample_job): assert notification_from_db.status == 'created' -def test_save_notification(sample_email_template, sample_job, ses_provider): +def test_save_notification(sample_email_template, sample_job): assert Notification.query.count() == 0 data = _notification_json(sample_email_template, job_id=sample_job.id) @@ -350,7 +346,7 @@ def test_save_notification(sample_email_template, sample_job, ses_provider): assert Notification.query.count() == 2 -def test_save_notification_creates_history(sample_email_template, sample_job): +def test_save_notification_does_not_creates_history(sample_email_template, sample_job): assert Notification.query.count() == 0 data = _notification_json(sample_email_template, job_id=sample_job.id) @@ -358,57 +354,13 @@ def test_save_notification_creates_history(sample_email_template, sample_job): dao_create_notification(notification_1) assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 1 - - -def test_save_notification_with_test_api_key_does_not_create_history(sample_email_template, sample_api_key): - assert Notification.query.count() == 0 - data = _notification_json(sample_email_template) - data['key_type'] = KEY_TYPE_TEST - data['api_key_id'] = sample_api_key.id - - notification_1 = Notification(**data) - dao_create_notification(notification_1) - - assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 0 - - -def test_save_notification_with_research_mode_service_does_not_create_history( - sample_template): - sample_template.service.research_mode = True - - assert Notification.query.count() == 0 - data = _notification_json(sample_template) - notification = Notification(**data) - dao_create_notification(notification) - assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 0 - - -def test_update_notification_with_test_api_key_does_not_update_or_create_history(sample_email_template, sample_api_key): - assert Notification.query.count() == 0 - data = _notification_json(sample_email_template) - data['key_type'] = KEY_TYPE_TEST - data['api_key_id'] = sample_api_key.id - - notification = Notification(**data) - dao_create_notification(notification) - - notification.status = 'delivered' - dao_update_notification(notification) - - assert Notification.query.one().status == 'delivered' assert NotificationHistory.query.count() == 0 def test_update_notification_with_research_mode_service_does_not_create_or_update_history( sample_template): sample_template.service.research_mode = True - - data = _notification_json(sample_template) - notification = Notification(**data) - dao_create_notification(notification) + notification = create_notification(template=sample_template) assert Notification.query.count() == 1 assert NotificationHistory.query.count() == 0 @@ -630,113 +582,55 @@ def test_should_limit_notifications_return_by_day_limit_plus_one(sample_template assert len(all_notifications) == 2 -def test_creating_notification_adds_to_notification_history(sample_template): - data = _notification_json(sample_template) - notification = Notification(**data) +def test_creating_notification_does_not_add_notification_history(sample_template): + create_notification(template=sample_template) + assert Notification.query.count() == 1 + assert NotificationHistory.query.count() == 0 - dao_create_notification(notification) + +def test_should_delete_notification_for_id(sample_template): + notification = create_notification(template=sample_template) assert Notification.query.count() == 1 + assert NotificationHistory.query.count() == 0 - hist = NotificationHistory.query.one() - assert hist.id == notification.id - assert hist.created_at == notification.created_at - assert hist.status == notification.status - assert not hasattr(hist, 'to') - assert not hasattr(hist, '_personalisation') - - -def test_should_delete_notification_and_notification_history_for_id(notify_db, notify_db_session, sample_template): - data = _notification_json(sample_template) - notification = Notification(**data) - - dao_create_notification(notification) - - assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 1 - - dao_delete_notifications_and_history_by_id(notification.id) + dao_delete_notifications_by_id(notification.id) assert Notification.query.count() == 0 - assert NotificationHistory.query.count() == 0 - - -def test_should_delete_notification_and_ignore_history_for_test_api( - sample_email_template, - sample_api_key): - data = _notification_json(sample_email_template) - data['key_type'] = KEY_TYPE_TEST - data['api_key_id'] = sample_api_key.id - - notification = Notification(**data) - dao_create_notification(notification) - - assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 0 - - dao_delete_notifications_and_history_by_id(notification.id) - - assert Notification.query.count() == 0 - assert NotificationHistory.query.count() == 0 def test_should_delete_notification_and_ignore_history_for_research_mode(sample_template): sample_template.service.research_mode = True - data = _notification_json(sample_template) - notification = Notification(**data) - dao_create_notification(notification) + notification = create_notification(template=sample_template) assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 0 - dao_delete_notifications_and_history_by_id(notification.id) + dao_delete_notifications_by_id(notification.id) assert Notification.query.count() == 0 - assert NotificationHistory.query.count() == 0 -def test_should_delete_only_notification_and_notification_history_with_id(sample_template): - id_1 = uuid.uuid4() - id_2 = uuid.uuid4() - data_1 = _notification_json(sample_template, id=id_1) - data_2 = _notification_json(sample_template, id=id_2) - - notification_1 = Notification(**data_1) - notification_2 = Notification(**data_2) - - dao_create_notification(notification_1) - dao_create_notification(notification_2) - +def test_should_delete_only_notification_with_id(sample_template): + notification_1 = create_notification(template=sample_template) + notification_2 = create_notification(template=sample_template) assert Notification.query.count() == 2 - assert NotificationHistory.query.count() == 2 - dao_delete_notifications_and_history_by_id(notification_1.id) + dao_delete_notifications_by_id(notification_1.id) assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 1 assert Notification.query.first().id == notification_2.id - assert NotificationHistory.query.first().id == notification_2.id -def test_should_delete_no_notifications_or_notification_historys_if_no_matching_ids( +def test_should_delete_no_notifications_if_no_matching_ids( sample_template ): - id_1 = uuid.uuid4() - id_2 = uuid.uuid4() - data_1 = _notification_json(sample_template, id=id_1) + create_notification(template=sample_template) + assert Notification.query.count() == 1 - notification_1 = Notification(**data_1) - - dao_create_notification(notification_1) + dao_delete_notifications_by_id(uuid.uuid4()) assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 1 - - dao_delete_notifications_and_history_by_id(id_2) - - assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 1 def _notification_json(sample_template, job_id=None, id=None, status=None): @@ -776,10 +670,6 @@ def test_dao_timeout_notifications(sample_template): assert Notification.query.get(sending.id).status == 'temporary-failure' assert Notification.query.get(pending.id).status == 'temporary-failure' assert Notification.query.get(delivered.id).status == 'delivered' - assert NotificationHistory.query.get(created.id).status == 'technical-failure' - assert NotificationHistory.query.get(sending.id).status == 'temporary-failure' - assert NotificationHistory.query.get(pending.id).status == 'temporary-failure' - assert NotificationHistory.query.get(delivered.id).status == 'delivered' assert len(technical_failure_notifications + temporary_failure_notifications) == 3 @@ -795,10 +685,6 @@ def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_t assert Notification.query.get(pending.id).status == 'pending' assert Notification.query.get(delivered.id).status == 'delivered' technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1) - assert NotificationHistory.query.get(created.id).status == 'created' - assert NotificationHistory.query.get(sending.id).status == 'sending' - assert NotificationHistory.query.get(pending.id).status == 'pending' - assert NotificationHistory.query.get(delivered.id).status == 'delivered' assert len(technical_failure_notifications + temporary_failure_notifications) == 0 @@ -816,12 +702,6 @@ def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template) technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1) - assert NotificationHistory.query.get(created.id).status == 'created' - assert NotificationHistory.query.get(sending.id).status == 'sending' - assert NotificationHistory.query.get(pending.id).status == 'pending' - assert NotificationHistory.query.get(delivered.id).status == 'delivered' - assert len(technical_failure_notifications + temporary_failure_notifications) == 0 - def test_should_return_notifications_excluding_jobs_by_default(sample_template, sample_job, sample_api_key): create_notification(sample_template, job=sample_job) @@ -1496,8 +1376,8 @@ def test_dao_update_notifications_by_reference_updated_notifications(sample_temp def test_dao_update_notifications_by_reference_updates_history_some_notifications_exist(sample_template): create_notification(template=sample_template, reference='ref1') - create_notification(template=sample_template, reference='ref2') - Notification.query.filter_by(reference='ref2').delete() + create_notification_history(template=sample_template, reference='ref2') + updated_count, updated_history_count = dao_update_notifications_by_reference( references=['ref1', 'ref2'], update_dict={ @@ -1506,14 +1386,13 @@ def test_dao_update_notifications_by_reference_updates_history_some_notification } ) assert updated_count == 1 - assert updated_history_count == 2 + assert updated_history_count == 1 def test_dao_update_notifications_by_reference_updates_history_no_notifications_exist(sample_template): - create_notification(template=sample_template, reference='ref1') - create_notification(template=sample_template, reference='ref2') - Notification.query.filter_by(reference='ref1').delete() - Notification.query.filter_by(reference='ref2').delete() + create_notification_history(template=sample_template, reference='ref1') + create_notification_history(template=sample_template, reference='ref2') + updated_count, updated_history_count = dao_update_notifications_by_reference( references=['ref1', 'ref2'], update_dict={ @@ -1554,11 +1433,9 @@ def test_dao_update_notifications_by_reference_set_returned_letter_status(sample def test_dao_update_notifications_by_reference_updates_history_when_one_of_two_notifications_exists( sample_letter_template ): - notification1 = create_notification(template=sample_letter_template, reference='ref1') + notification1 = create_notification_history(template=sample_letter_template, reference='ref1') notification2 = create_notification(template=sample_letter_template, reference='ref2') - Notification.query.filter_by(id=notification1.id).delete() - NotificationHistory.query.filter_by(id=notification2.id).delete() updated_count, updated_history_count = dao_update_notifications_by_reference( references=['ref1', 'ref2'], update_dict={"status": "returned-letter"} diff --git a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py index 669080b7e..bd66faba3 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py +++ b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py @@ -7,7 +7,6 @@ import pytest from flask import current_app from freezegun import freeze_time -from app import db from app.dao.notifications_dao import ( delete_notifications_older_than_retention_by_type, insert_update_notification_history @@ -81,10 +80,10 @@ def test_should_not_delete_notification_history(sample_service, notification_typ create_notification(template=sms_template, status='permanent-failure') create_notification(template=letter_template, status='permanent-failure') assert Notification.query.count() == 3 - assert NotificationHistory.query.count() == 3 + assert NotificationHistory.query.count() == 0 delete_notifications_older_than_retention_by_type(notification_type) assert Notification.query.count() == 2 - assert NotificationHistory.query.count() == 3 + assert NotificationHistory.query.count() == 1 @pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) @@ -285,17 +284,14 @@ def test_insert_update_notification_history_only_insert_update_given_service(sam def test_insert_update_notification_history_updates_history_with_new_status(sample_template): notification_1 = create_notification(template=sample_template, created_at=datetime.utcnow() - timedelta(days=3)) - notification_2 = create_notification(template=sample_template, created_at=datetime.utcnow() - timedelta(days=8)) - notification_2.status = 'delivered' - db.session.add(notification_2) - db.session.commit() + notification_2 = create_notification(template=sample_template, created_at=datetime.utcnow() - timedelta(days=8), + status='delivered') insert_update_notification_history( 'sms', datetime.utcnow() - timedelta(days=7), sample_template.service_id) - history_1 = NotificationHistory.query.get(notification_1.id) - assert history_1.id == notification_1.id - history_2 = NotificationHistory.query.get(notification_2.id) - assert history_2.id == notification_2.id - assert history_2.status == 'delivered' + history = NotificationHistory.query.get(notification_2.id) + assert history.id == notification_2.id + assert history.status == 'delivered' + assert not NotificationHistory.query.get(notification_1.id) def _create_templates(sample_service): diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index 43a23dc42..e5bd45a7b 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -28,7 +28,8 @@ from tests.app.db import ( create_template, create_notification, create_rate, - create_letter_rate + create_letter_rate, + create_notification_history ) @@ -95,6 +96,7 @@ def test_fetch_billing_data_for_today_includes_data_with_the_right_key_type(noti assert results[0].notifications_sent == 2 +@freeze_time('2018-04-02 01:20:00') def test_fetch_billing_data_for_today_includes_data_with_the_right_date(notify_db_session): process_day = datetime(2018, 4, 1, 13, 30, 0) service = create_service() @@ -235,8 +237,10 @@ def test_fetch_billing_data_for_day_returns_empty_list(notify_db_session): def test_fetch_billing_data_for_day_uses_notification_history(notify_db_session): service = create_service() sms_template = create_template(service=service, template_type='sms') - create_notification(template=sms_template, status='delivered', created_at=datetime.utcnow() - timedelta(days=8)) - create_notification(template=sms_template, status='delivered', created_at=datetime.utcnow() - timedelta(days=8)) + create_notification_history(template=sms_template, status='delivered', + created_at=datetime.utcnow() - timedelta(days=8)) + create_notification_history(template=sms_template, status='delivered', + created_at=datetime.utcnow() - timedelta(days=8)) Notification.query.delete() db.session.commit() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 5aae5854e..eab886f3f 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -73,6 +73,7 @@ from tests.app.db import ( create_invited_user, create_email_branding, create_letter_branding, + create_notification_history ) @@ -713,16 +714,13 @@ def test_fetch_stats_filters_on_service(notify_db_session): assert len(stats) == 0 -def test_fetch_stats_ignores_historical_notification_data(notify_db_session): - notification = create_notification(template=create_template(service=create_service())) - service_id = notification.service.id - - db.session.delete(notification) +def test_fetch_stats_ignores_historical_notification_data(sample_template): + create_notification_history(template=sample_template) assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 1 - stats = dao_fetch_stats_for_service(service_id, 7) + stats = dao_fetch_stats_for_service(sample_template.service_id, 7) assert len(stats) == 0 diff --git a/tests/app/db.py b/tests/app/db.py index 13d872d83..749462eb2 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -55,6 +55,7 @@ from app.models import ( TemplateFolder, LetterBranding, Domain, + NotificationHistory ) @@ -283,6 +284,74 @@ def create_notification( return notification +def create_notification_history( + template=None, + job=None, + job_row_number=None, + status='created', + reference=None, + created_at=None, + sent_at=None, + updated_at=None, + billable_units=1, + api_key=None, + key_type=KEY_TYPE_NORMAL, + sent_by=None, + client_reference=None, + rate_multiplier=None, + international=False, + phone_prefix=None, + created_by_id=None, + postage=None +): + assert job or template + if job: + template = job.template + + if created_at is None: + created_at = datetime.utcnow() + + if status != 'created': + sent_at = sent_at or datetime.utcnow() + updated_at = updated_at or datetime.utcnow() + + if template.template_type == 'letter' and postage is None: + postage = 'second' + + data = { + 'id': uuid.uuid4(), + 'job_id': job and job.id, + 'job': job, + 'service_id': template.service.id, + 'service': template.service, + 'template_id': template.id, + 'template_version': template.version, + 'status': status, + 'reference': reference, + 'created_at': created_at, + 'sent_at': sent_at, + 'billable_units': billable_units, + 'notification_type': template.template_type, + 'api_key': api_key, + 'api_key_id': api_key and api_key.id, + 'key_type': api_key.key_type if api_key else key_type, + 'sent_by': sent_by, + 'updated_at': updated_at, + 'client_reference': client_reference, + 'job_row_number': job_row_number, + 'rate_multiplier': rate_multiplier, + 'international': international, + 'phone_prefix': phone_prefix, + 'created_by_id': created_by_id, + 'postage': postage + } + notification_history = NotificationHistory(**data) + db.session.add(notification_history) + db.session.commit() + + return notification_history + + def create_job( template, notification_count=1, diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 268c820f0..8bae50c9e 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -4,16 +4,16 @@ import pytest from flask import json from sqlalchemy.exc import SQLAlchemyError -from app import db from app.dao.notifications_dao import get_notification_by_id -from app.models import Complaint, NotificationHistory, Notification +from app.models import Complaint from app.notifications.notifications_ses_callback import handle_complaint from tests.app.conftest import sample_notification as create_sample_notification from tests.app.db import ( create_notification, ses_complaint_callback_malformed_message_id, ses_complaint_callback_with_missing_complaint_type, - ses_complaint_callback + ses_complaint_callback, + create_notification_history ) @@ -56,8 +56,6 @@ def test_handle_complaint_does_raise_exception_if_notification_not_found(notify_ def test_process_ses_results_in_complaint_if_notification_history_does_not_exist(sample_email_template): notification = create_notification(template=sample_email_template, reference='ref1') - NotificationHistory.query.filter_by(id=notification.id).delete() - db.session.commit() handle_complaint(json.loads(ses_complaint_callback()['Message'])) complaints = Complaint.query.all() assert len(complaints) == 1 @@ -65,9 +63,7 @@ def test_process_ses_results_in_complaint_if_notification_history_does_not_exist def test_process_ses_results_in_complaint_if_notification_does_not_exist(sample_email_template): - notification = create_notification(template=sample_email_template, reference='ref1') - Notification.query.filter_by(id=notification.id).delete() - db.session.commit() + notification = create_notification_history(template=sample_email_template, reference='ref1') handle_complaint(json.loads(ses_complaint_callback()['Message'])) complaints = Complaint.query.all() assert len(complaints) == 1 diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 349cb82a6..656a7da47 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -73,27 +73,24 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api reply_to_text=sample_template.service.get_default_sms_sender()) assert Notification.query.get(notification.id) is not None - assert NotificationHistory.query.get(notification.id) is not None notification_from_db = Notification.query.one() - notification_history_from_db = NotificationHistory.query.one() - assert notification_from_db.id == notification_history_from_db.id - assert notification_from_db.template_id == notification_history_from_db.template_id - assert notification_from_db.template_version == notification_history_from_db.template_version - assert notification_from_db.api_key_id == notification_history_from_db.api_key_id - assert notification_from_db.key_type == notification_history_from_db.key_type - assert notification_from_db.key_type == notification_history_from_db.key_type - assert notification_from_db.billable_units == notification_history_from_db.billable_units - assert notification_from_db.notification_type == notification_history_from_db.notification_type - assert notification_from_db.created_at == notification_history_from_db.created_at + assert notification_from_db.id == notification.id + assert notification_from_db.template_id == notification.template_id + assert notification_from_db.template_version == notification.template_version + assert notification_from_db.api_key_id == notification.api_key_id + assert notification_from_db.key_type == notification.key_type + assert notification_from_db.key_type == notification.key_type + assert notification_from_db.billable_units == notification.billable_units + assert notification_from_db.notification_type == notification.notification_type + assert notification_from_db.created_at == notification.created_at assert not notification_from_db.sent_at - assert not notification_history_from_db.sent_at - assert notification_from_db.updated_at == notification_history_from_db.updated_at - assert notification_from_db.status == notification_history_from_db.status - assert notification_from_db.reference == notification_history_from_db.reference - assert notification_from_db.client_reference == notification_history_from_db.client_reference - assert notification_from_db.created_by_id == notification_history_from_db.created_by_id + assert notification_from_db.updated_at == notification.updated_at + assert notification_from_db.status == notification.status + assert notification_from_db.reference == notification.reference + assert notification_from_db.client_reference == notification.client_reference + assert notification_from_db.created_by_id == notification.created_by_id assert notification_from_db.reply_to_text == sample_template.service.get_default_sms_sender() mocked_redis.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count") @@ -187,7 +184,7 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker) created_by_id=sample_job.created_by_id ) assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 1 + assert NotificationHistory.query.count() == 0 persisted_notification = Notification.query.all()[0] assert persisted_notification.id == n_id persisted_notification.job_id == sample_job.id diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 4074fd3d9..f950135af 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -9,7 +9,6 @@ from app.config import QueueNames from app.models import ( Job, Notification, - NotificationHistory, EMAIL_TYPE, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, @@ -475,9 +474,6 @@ def test_post_precompiled_letter_notification_returns_201( assert notification.status == NOTIFICATION_PENDING_VIRUS_CHECK assert notification.postage == expected_postage - notification_history = NotificationHistory.query.one() - assert notification_history.postage == expected_postage - resp_json = json.loads(response.get_data(as_text=True)) assert resp_json == {'id': str(notification.id), 'reference': 'letter-reference', 'postage': expected_postage}