diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 90e42c98b..367905e7f 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -164,14 +164,10 @@ def update_notification_status_by_reference(reference, status): @statsd(namespace="dao") +@transactional def dao_update_notification(notification): notification.updated_at = datetime.utcnow() db.session.add(notification) - if _should_record_notification_in_history_table(notification): - notification_history = NotificationHistory.query.get(notification.id) - notification_history.update_from_original(notification) - db.session.add(notification_history) - db.session.commit() @statsd(namespace="dao") @@ -529,12 +525,14 @@ def dao_update_notifications_by_reference(references, update_dict): synchronize_session=False ) - updated_history_count = NotificationHistory.query.filter( - NotificationHistory.reference.in_(references) - ).update( - update_dict, - synchronize_session=False - ) + updated_history_count = 0 + if updated_count == 0: + updated_history_count = NotificationHistory.query.filter( + NotificationHistory.reference.in_(references) + ).update( + update_dict, + synchronize_session=False + ) return updated_count, updated_history_count diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index c4e029528..80dcb4e66 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -12,7 +12,11 @@ from celery.exceptions import Retry from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate from notifications_utils.columns import Row -from app import (encryption, DATETIME_FORMAT) +from app import ( + DATETIME_FORMAT, + db, + encryption +) from app.celery import provider_tasks from app.celery import tasks from app.celery.tasks import ( @@ -29,7 +33,7 @@ from app.celery.tasks import ( process_returned_letters_list, ) from app.config import QueueNames -from app.dao import jobs_dao, services_dao, service_email_reply_to_dao, service_sms_sender_dao +from app.dao import jobs_dao, service_email_reply_to_dao, service_sms_sender_dao from app.models import ( Job, Notification, @@ -44,15 +48,7 @@ from app.models import ( ) from tests.app import load_example_csv -from tests.app.conftest import ( - sample_service as create_sample_service, - sample_template as create_sample_template, - sample_job as create_sample_job, - sample_email_template as create_sample_email_template, - sample_notification as create_sample_notification, - sample_letter_template, - sample_letter_job -) + from tests.app.db import ( create_inbound_sms, create_job, @@ -97,7 +93,7 @@ def test_should_have_decorated_tasks_functions(): @pytest.fixture def email_job_with_placeholders(notify_db, notify_db_session, sample_email_template_with_placeholders): - return create_sample_job(notify_db, notify_db_session, template=sample_email_template_with_placeholders) + return create_job(template=sample_email_template_with_placeholders) # -------------- process_job tests -------------- # @@ -148,12 +144,12 @@ def test_should_process_sms_job_with_sender_id(sample_job, mocker, fake_uuid): @freeze_time("2016-01-01 11:09:00.061258") -def test_should_not_process_sms_job_if_would_exceed_send_limits(notify_db, - notify_db_session, - mocker): - service = create_sample_service(notify_db, notify_db_session, limit=9) - job = create_sample_job(notify_db, notify_db_session, service=service, notification_count=10) - +def test_should_not_process_sms_job_if_would_exceed_send_limits( + notify_db_session, mocker +): + service = create_service(message_limit=9) + template = create_template(service=service) + job = create_job(template=template, notification_count=10, original_file_name='multiple_sms.csv') mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) mocker.patch('app.celery.tasks.process_row') @@ -165,13 +161,14 @@ def test_should_not_process_sms_job_if_would_exceed_send_limits(notify_db, assert tasks.process_row.called is False -def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today(notify_db, - notify_db_session, - mocker): - service = create_sample_service(notify_db, notify_db_session, limit=1) - job = create_sample_job(notify_db, notify_db_session, service=service) +def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today( + notify_db_session, mocker +): + service = create_service(message_limit=1) + template = create_template(service=service) + job = create_job(template=template) - create_sample_notification(notify_db, notify_db_session, service=service, job=job) + create_notification(template=template, job=job) mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('sms')) mocker.patch('app.celery.tasks.process_row') @@ -184,12 +181,13 @@ def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today(notify assert tasks.process_row.called is False -def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(notify_db, notify_db_session, mocker): - service = create_sample_service(notify_db, notify_db_session, limit=1) - template = create_sample_email_template(notify_db, notify_db_session, service=service) - job = create_sample_job(notify_db, notify_db_session, service=service, template=template) +@pytest.mark.parametrize('template_type', ['sms', 'email']) +def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(notify_db_session, template_type, mocker): + service = create_service(message_limit=1) + template = create_template(service=service, template_type=template_type) + job = create_job(template=template) - create_sample_notification(notify_db, notify_db_session, service=service, job=job) + create_notification(template=template, job=job) mocker.patch('app.celery.tasks.s3.get_job_from_s3') mocker.patch('app.celery.tasks.process_row') @@ -202,25 +200,8 @@ def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(noti assert tasks.process_row.called is False -@freeze_time("2016-01-01 11:09:00.061258") -def test_should_not_process_email_job_if_would_exceed_send_limits(notify_db, notify_db_session, mocker): - service = create_sample_service(notify_db, notify_db_session, limit=0) - template = create_sample_email_template(notify_db, notify_db_session, service=service) - job = create_sample_job(notify_db, notify_db_session, service=service, template=template) - - mocker.patch('app.celery.tasks.s3.get_job_from_s3') - mocker.patch('app.celery.tasks.process_row') - - process_job(job.id) - - job = jobs_dao.dao_get_job_by_id(job.id) - assert job.job_status == 'sending limits exceeded' - assert s3.get_job_from_s3.called is False - assert tasks.process_row.called is False - - -def test_should_not_process_job_if_already_pending(notify_db, notify_db_session, mocker): - job = create_sample_job(notify_db, notify_db_session, job_status='scheduled') +def test_should_not_process_job_if_already_pending(sample_template, mocker): + job = create_job(template=sample_template, job_status='scheduled') mocker.patch('app.celery.tasks.s3.get_job_from_s3') mocker.patch('app.celery.tasks.process_row') @@ -231,12 +212,11 @@ def test_should_not_process_job_if_already_pending(notify_db, notify_db_session, assert tasks.process_row.called is False -def test_should_process_email_job_if_exactly_on_send_limits(notify_db, - notify_db_session, +def test_should_process_email_job_if_exactly_on_send_limits(notify_db_session, mocker): - service = create_sample_service(notify_db, notify_db_session, limit=10) - template = create_sample_email_template(notify_db, notify_db_session, service=service) - job = create_sample_job(notify_db, notify_db_session, service=service, template=template, notification_count=10) + service = create_service(message_limit=10) + template = create_template(service=service, template_type='email') + job = create_job(template=template, notification_count=10) mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_email')) mocker.patch('app.celery.tasks.save_email.apply_async') @@ -506,11 +486,9 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi def test_should_put_save_sms_task_in_research_mode_queue_if_research_mode_service(notify_db, notify_db_session, mocker): - service = create_sample_service(notify_db, notify_db_session) - service.research_mode = True - services_dao.dao_update_service(service) + service = create_service(research_mode=True, ) - template = create_sample_template(notify_db, notify_db_session, service=service) + template = create_template(service=service) notification = _notification_json(template, to="+447234123123") @@ -531,10 +509,10 @@ def test_should_put_save_sms_task_in_research_mode_queue_if_research_mode_servic assert mocked_deliver_sms.called -def test_should_save_sms_if_restricted_service_and_valid_number(notify_db, notify_db_session, mocker): +def test_should_save_sms_if_restricted_service_and_valid_number(notify_db_session, mocker): user = create_user(mobile_number="07700 900890") - service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) - template = create_sample_template(notify_db, notify_db_session, service=service) + service = create_service(user=user, restricted=True) + template = create_template(service=service) notification = _notification_json(template, "+447700900890") # The user’s own number, but in a different format mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') @@ -601,10 +579,10 @@ def test_save_sms_should_save_default_smm_sender_notification_reply_to_text_on(n assert persisted_notification.reply_to_text == '12345' -def test_should_not_save_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker): +def test_should_not_save_sms_if_restricted_service_and_invalid_number(notify_db_session, mocker): user = create_user(mobile_number="07700 900205") - service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) - template = create_sample_template(notify_db, notify_db_session, service=service) + service = create_service(user=user, restricted=True) + template = create_template(service=service) notification = _notification_json(template, "07700 900849") mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') @@ -619,12 +597,10 @@ def test_should_not_save_sms_if_restricted_service_and_invalid_number(notify_db, assert Notification.query.count() == 0 -def test_should_not_save_email_if_restricted_service_and_invalid_email_address(notify_db, notify_db_session, mocker): +def test_should_not_save_email_if_restricted_service_and_invalid_email_address(notify_db_session, mocker): user = create_user() - service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) - template = create_sample_template( - notify_db, notify_db_session, service=service, template_type='email', subject_line='Hello' - ) + service = create_service(user=user, restricted=True) + template = create_template(service=service, template_type='email', subject='Hello') notification = _notification_json(template, to="test@example.com") notification_id = uuid.uuid4() @@ -638,13 +614,11 @@ def test_should_not_save_email_if_restricted_service_and_invalid_email_address(n def test_should_put_save_email_task_in_research_mode_queue_if_research_mode_service( - notify_db, notify_db_session, mocker + notify_db_session, mocker ): - service = create_sample_service(notify_db, notify_db_session) - service.research_mode = True - services_dao.dao_update_service(service) + service = create_service(research_mode=True) - template = create_sample_email_template(notify_db, notify_db_session, service=service) + template = create_template(service=service, template_type='email') notification = _notification_json(template, to="test@test.com") @@ -699,11 +673,11 @@ def test_should_save_sms_template_to_and_persist_with_job_id(sample_job, mocker) ) -def test_should_not_save_sms_if_team_key_and_recipient_not_in_team(notify_db, notify_db_session, mocker): +def test_should_not_save_sms_if_team_key_and_recipient_not_in_team(notify_db_session, mocker): assert Notification.query.count() == 0 user = create_user(mobile_number="07700 900205") - service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) - template = create_sample_template(notify_db, notify_db_session, service=service) + service = create_service(user=user, restricted=True) + template = create_template(service=service) team_members = [user.mobile_number for user in service.users] assert "07890 300000" not in team_members @@ -1015,8 +989,8 @@ def test_save_letter_saves_letter_to_database(mocker, notify_db_session): @pytest.mark.parametrize('postage', ['first', 'second']) def test_save_letter_saves_letter_to_database_with_correct_postage(mocker, notify_db_session, postage): service = create_service(service_permissions=[LETTER_TYPE]) - template = sample_letter_template(service, postage=postage) - letter_job = sample_letter_job(template) + template = create_template(service=service, template_type=LETTER_TYPE, postage=postage) + letter_job = create_job(template=template) mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') notification_json = _notification_json( @@ -1612,17 +1586,29 @@ def test_process_incomplete_jobs_sets_status_to_in_progress_and_resets_processin assert mock_process_incomplete_job.mock_calls == [call(str(job1.id)), call(str(job2.id))] -def test_process_returned_letters_list(mocker, sample_letter_template): +def test_process_returned_letters_list(sample_letter_template): create_notification(sample_letter_template, reference='ref1') create_notification(sample_letter_template, reference='ref2') process_returned_letters_list(['ref1', 'ref2', 'unknown-ref']) notifications = Notification.query.all() - history = NotificationHistory.query.all() assert [n.status for n in notifications] == ['returned-letter', 'returned-letter'] assert all(n.updated_at for n in notifications) - assert [n.status for n in history] == ['returned-letter', 'returned-letter'] - assert all(n.updated_at for n in history) + +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') + + Notification.query.delete() + db.session.commit() + process_returned_letters_list(['ref1', 'ref2', 'unknown-ref']) + + notifications = NotificationHistory.query.all() + + assert [n.status for n in notifications] == ['returned-letter', 'returned-letter'] + assert all(n.updated_at for n in notifications) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index e831c5d3e..8a2b33ed3 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -34,7 +34,6 @@ from app.dao.notifications_dao import ( dao_get_notification_history_by_reference, notifications_not_yet_sent, ) -from app.dao.services_dao import dao_update_service from app.models import ( Job, Notification, @@ -52,12 +51,6 @@ from app.models import ( KEY_TYPE_TEST, JOB_STATUS_IN_PROGRESS ) -from tests.app.conftest import ( - sample_notification, - sample_template as create_sample_template, - sample_service, - sample_job, -) from tests.app.db import ( create_job, create_notification, @@ -115,42 +108,41 @@ def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_ assert notification.status == 'delivered' -def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(notify_db, notify_db_session): - job = sample_job(notify_db, notify_db_session) - notification = sample_notification(notify_db, notify_db_session, status='delivered', job=job) +def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(sample_job): + notification = create_notification(template=sample_job.template, status='delivered', job=sample_job) assert Notification.query.get(notification.id).status == 'delivered' assert not update_notification_status_by_id(notification.id, 'failed') assert Notification.query.get(notification.id).status == 'delivered' - assert job == Job.query.get(notification.job_id) + assert sample_job == Job.query.get(notification.job_id) -def test_should_not_update_status_by_reference_if_not_sending_and_does_not_update_job(notify_db, notify_db_session): - job = sample_job(notify_db, notify_db_session) - notification = sample_notification(notify_db, notify_db_session, status='delivered', reference='reference', job=job) +def test_should_not_update_status_by_reference_if_not_sending_and_does_not_update_job(sample_job): + notification = create_notification( + template=sample_job.template, status='delivered', reference='reference', job=sample_job + ) assert Notification.query.get(notification.id).status == 'delivered' assert not update_notification_status_by_reference('reference', 'failed') assert Notification.query.get(notification.id).status == 'delivered' - assert job == Job.query.get(notification.job_id) + assert sample_job == Job.query.get(notification.job_id) -def test_should_update_status_by_id_if_created(notify_db, notify_db_session): - notification = sample_notification(notify_db, notify_db_session, status='created') - assert Notification.query.get(notification.id).status == 'created' - updated = update_notification_status_by_id(notification.id, 'failed') - assert Notification.query.get(notification.id).status == 'failed' +def test_should_update_status_by_id_if_created(sample_template, sample_notification): + assert Notification.query.get(sample_notification.id).status == 'created' + updated = update_notification_status_by_id(sample_notification.id, 'failed') + assert Notification.query.get(sample_notification.id).status == 'failed' assert updated.status == 'failed' -def test_should_update_status_by_id_if_pending_virus_check(notify_db, notify_db_session): - notification = sample_notification(notify_db, notify_db_session, status='pending-virus-check') +def test_should_update_status_by_id_if_pending_virus_check(sample_letter_template): + notification = create_notification(template=sample_letter_template, status='pending-virus-check') assert Notification.query.get(notification.id).status == 'pending-virus-check' updated = update_notification_status_by_id(notification.id, 'cancelled') assert Notification.query.get(notification.id).status == 'cancelled' assert updated.status == 'cancelled' -def test_should_update_status_by_id_and_set_sent_by(notify_db, notify_db_session): - notification = sample_notification(notify_db, notify_db_session, status='sending') +def test_should_update_status_by_id_and_set_sent_by(sample_template): + notification = create_notification(template=sample_template, status='sending') updated = update_notification_status_by_id(notification.id, 'delivered', sent_by='mmg') assert updated.status == 'delivered' @@ -212,8 +204,8 @@ def test_should_not_update_status_by_id_if_sent_to_country_with_delivery_receipt assert notification.status == NOTIFICATION_DELIVERED -def test_should_not_update_status_by_reference_if_not_sending(notify_db, notify_db_session): - notification = sample_notification(notify_db, notify_db_session, status='created', reference='reference') +def test_should_not_update_status_by_reference_if_not_sending(sample_template): + notification = create_notification(template=sample_template, status='created', reference='reference') assert Notification.query.get(notification.id).status == 'created' updated = update_notification_status_by_reference('reference', 'failed') assert Notification.query.get(notification.id).status == 'created' @@ -259,12 +251,9 @@ def test_should_by_able_to_update_status_by_id_from_sending_to_permanent_failure assert Notification.query.get(notification.id).status == 'permanent-failure' -def test_should_not_update_status_one_notification_status_is_delivered(notify_db, notify_db_session, - sample_email_template, - ses_provider): - notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_email_template, - status='sending') +def test_should_not_update_status_once_notification_status_is_delivered( + sample_email_template): + notification = create_notification(template=sample_email_template, status='sending') assert Notification.query.get(notification.id).status == "sending" notification.reference = 'reference' @@ -284,13 +273,11 @@ def test_should_return_zero_count_if_no_notification_with_reference(): assert not update_notification_status_by_reference('something', 'delivered') -def test_create_notification_creates_notification_with_personalisation(notify_db, notify_db_session, - sample_template_with_placeholders, - sample_job, mmg_provider): +def test_create_notification_creates_notification_with_personalisation(sample_template_with_placeholders, + sample_job): assert Notification.query.count() == 0 - data = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_template_with_placeholders, + data = create_notification(template=sample_template_with_placeholders, job=sample_job, personalisation={'name': 'Jo'}, status='created') @@ -308,7 +295,7 @@ def test_create_notification_creates_notification_with_personalisation(notify_db assert {'name': 'Jo'} == notification_from_db.personalisation -def test_save_notification_creates_sms(sample_template, sample_job, mmg_provider): +def test_save_notification_creates_sms(sample_template, sample_job): assert Notification.query.count() == 0 data = _notification_json(sample_template, job_id=sample_job.id) @@ -388,15 +375,11 @@ def test_save_notification_with_test_api_key_does_not_create_history(sample_emai def test_save_notification_with_research_mode_service_does_not_create_history( - notify_db, - notify_db_session): - service = sample_service(notify_db, notify_db_session) - service.research_mode = True - dao_update_service(service) - template = create_sample_template(notify_db, notify_db_session, service=service) + sample_template): + sample_template.service.research_mode = True assert Notification.query.count() == 0 - data = _notification_json(template) + data = _notification_json(sample_template) notification = Notification(**data) dao_create_notification(notification) assert Notification.query.count() == 1 @@ -420,14 +403,10 @@ def test_update_notification_with_test_api_key_does_not_update_or_create_history def test_update_notification_with_research_mode_service_does_not_create_or_update_history( - notify_db, - notify_db_session): - service = sample_service(notify_db, notify_db_session) - service.research_mode = True - dao_update_service(service) - template = create_sample_template(notify_db, notify_db_session, service=service) + sample_template): + sample_template.service.research_mode = True - data = _notification_json(template) + data = _notification_json(sample_template) notification = Notification(**data) dao_create_notification(notification) @@ -520,9 +499,8 @@ def test_save_notification_with_no_job(sample_template, mmg_provider): assert notification_from_db.status == 'created' -def test_get_notification_with_personalisation_by_id(notify_db, notify_db_session, sample_template): - notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_template, +def test_get_notification_with_personalisation_by_id(sample_template): + notification = create_notification(template=sample_template, scheduled_for='2017-05-05 14:15', status='created') notification_from_db = get_notification_with_personalisation( @@ -540,7 +518,7 @@ def test_get_notification_by_id_when_notification_exists(sample_notification): assert sample_notification == notification_from_db -def test_get_notification_by_id_when_notification_does_not_exist(notify_db_session, fake_uuid): +def test_get_notification_by_id_when_notification_does_not_exist(notify_db, fake_uuid): notification_from_db = get_notification_by_id(fake_uuid) assert notification_from_db is None @@ -566,7 +544,7 @@ def test_get_notifications_by_reference(sample_template): assert len(all_notifications) == 2 -def test_save_notification_no_job_id(sample_template, mmg_provider): +def test_save_notification_no_job_id(sample_template): assert Notification.query.count() == 0 data = _notification_json(sample_template) @@ -592,14 +570,10 @@ def test_get_notification_for_job(sample_notification): assert sample_notification == notification_from_db -def test_get_all_notifications_for_job(notify_db, notify_db_session, sample_job): +def test_get_all_notifications_for_job(sample_job): for i in range(0, 5): try: - sample_notification(notify_db, - notify_db_session, - service=sample_job.service, - template=sample_job.template, - job=sample_job) + create_notification(template=sample_job.template, job=sample_job) except IntegrityError: pass @@ -607,14 +581,11 @@ def test_get_all_notifications_for_job(notify_db, notify_db_session, sample_job) assert len(notifications_from_db) == 5 -def test_get_all_notifications_for_job_by_status(notify_db, notify_db_session, sample_job): +def test_get_all_notifications_for_job_by_status(sample_job): notifications = partial(get_notifications_for_job, sample_job.service.id, sample_job.id) for status in NOTIFICATION_STATUS_TYPES: - sample_notification( - notify_db, - notify_db_session, - service=sample_job.service, + create_notification( template=sample_job.template, job=sample_job, status=status @@ -675,21 +646,6 @@ def test_creating_notification_adds_to_notification_history(sample_template): assert not hasattr(hist, '_personalisation') -def test_updating_notification_updates_notification_history(sample_notification): - hist = NotificationHistory.query.one() - assert hist.id == sample_notification.id - assert hist.status == 'created' - - sample_notification.status = 'sending' - dao_update_notification(sample_notification) - notification = Notification.query.one() - hist1 = NotificationHistory.query.one() - assert notification.id == sample_notification.id - assert notification.status == "sending" - assert hist1.id == sample_notification.id - assert hist1.status == 'sending' - - 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) @@ -706,8 +662,6 @@ def test_should_delete_notification_and_notification_history_for_id(notify_db, n def test_should_delete_notification_and_ignore_history_for_test_api( - notify_db, - notify_db_session, sample_email_template, sample_api_key): data = _notification_json(sample_email_template) @@ -726,13 +680,10 @@ def test_should_delete_notification_and_ignore_history_for_test_api( assert NotificationHistory.query.count() == 0 -def test_should_delete_notification_and_ignore_history_for_research_mode(notify_db, notify_db_session): - service = sample_service(notify_db, notify_db_session) - service.research_mode = True - dao_update_service(service) - template = create_sample_template(notify_db, notify_db_session, service=service) +def test_should_delete_notification_and_ignore_history_for_research_mode(sample_template): + sample_template.service.research_mode = True - data = _notification_json(template) + data = _notification_json(sample_template) notification = Notification(**data) dao_create_notification(notification) @@ -745,8 +696,7 @@ def test_should_delete_notification_and_ignore_history_for_research_mode(notify_ assert NotificationHistory.query.count() == 0 -def test_should_delete_only_notification_and_notification_history_with_id(notify_db, notify_db_session, - sample_template): +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) @@ -770,8 +720,6 @@ def test_should_delete_only_notification_and_notification_history_with_id(notify def test_should_delete_no_notifications_or_notification_historys_if_no_matching_ids( - notify_db, - notify_db_session, sample_template ): id_1 = uuid.uuid4() @@ -925,19 +873,19 @@ def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excludin sample_team_api_key, sample_test_api_key ): - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), job=sample_job ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) @@ -958,27 +906,24 @@ def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excludin def test_get_notifications_with_a_live_api_key_type( - notify_db, - notify_db_session, - sample_service, sample_job, sample_api_key, sample_team_api_key, sample_test_api_key ): - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), job=sample_job ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) @@ -986,121 +931,115 @@ def test_get_notifications_with_a_live_api_key_type( assert len(all_notifications) == 4 # only those created with normal API key, no jobs - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, key_type=KEY_TYPE_NORMAL).items + all_notifications = get_notifications_for_service( + sample_job.service.id, limit_days=1, key_type=KEY_TYPE_NORMAL + ).items assert len(all_notifications) == 1 # only those created with normal API key, with jobs - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True, - key_type=KEY_TYPE_NORMAL).items + all_notifications = get_notifications_for_service( + sample_job.service.id, limit_days=1, include_jobs=True, key_type=KEY_TYPE_NORMAL + ).items assert len(all_notifications) == 2 def test_get_notifications_with_a_test_api_key_type( - notify_db, - notify_db_session, - sample_service, sample_job, sample_api_key, sample_team_api_key, sample_test_api_key ): - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), job=sample_job ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) # only those created with test API key, no jobs - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, key_type=KEY_TYPE_TEST).items + all_notifications = get_notifications_for_service(sample_job.service_id, limit_days=1, key_type=KEY_TYPE_TEST).items assert len(all_notifications) == 1 # only those created with test API key, no jobs, even when requested - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True, + all_notifications = get_notifications_for_service(sample_job.service_id, limit_days=1, include_jobs=True, key_type=KEY_TYPE_TEST).items assert len(all_notifications) == 1 def test_get_notifications_with_a_team_api_key_type( - notify_db, - notify_db_session, - sample_service, sample_job, sample_api_key, sample_team_api_key, sample_test_api_key ): - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), job=sample_job ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, + create_notification( + sample_job.template, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) # only those created with team API key, no jobs - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, key_type=KEY_TYPE_TEAM).items + all_notifications = get_notifications_for_service(sample_job.service_id, limit_days=1, key_type=KEY_TYPE_TEAM).items assert len(all_notifications) == 1 # only those created with team API key, no jobs, even when requested - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True, + all_notifications = get_notifications_for_service(sample_job.service_id, limit_days=1, include_jobs=True, key_type=KEY_TYPE_TEAM).items assert len(all_notifications) == 1 def test_should_exclude_test_key_notifications_by_default( - notify_db, - notify_db_session, - sample_service, sample_job, sample_api_key, sample_team_api_key, sample_test_api_key ): - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), job=sample_job ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_api_key, key_type=sample_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_team_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_team_api_key, key_type=sample_team_api_key.key_type ) - sample_notification( - notify_db, notify_db_session, created_at=datetime.utcnow(), api_key=sample_test_api_key, + create_notification( + template=sample_job.template, created_at=datetime.utcnow(), api_key=sample_test_api_key, key_type=sample_test_api_key.key_type ) all_notifications = Notification.query.all() assert len(all_notifications) == 4 - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1).items + all_notifications = get_notifications_for_service(sample_job.service_id, limit_days=1).items assert len(all_notifications) == 2 - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True).items + all_notifications = get_notifications_for_service(sample_job.service_id, limit_days=1, include_jobs=True).items assert len(all_notifications) == 3 - all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, key_type=KEY_TYPE_TEST).items + all_notifications = get_notifications_for_service(sample_job.service_id, limit_days=1, key_type=KEY_TYPE_TEST).items assert len(all_notifications) == 1 @@ -1402,23 +1341,19 @@ def test_dao_created_scheduled_notification(sample_notification): assert saved_notification[0].scheduled_for == datetime(2017, 1, 5, 14, 15) -def test_dao_get_scheduled_notifications(notify_db, notify_db_session, sample_template): - notification_1 = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_template, scheduled_for='2017-05-05 14:15', +def test_dao_get_scheduled_notifications(sample_template): + notification_1 = create_notification(template=sample_template, scheduled_for='2017-05-05 14:15', status='created') - sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_template, scheduled_for='2017-05-04 14:15', status='delivered') - sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_template, status='created') + create_notification(template=sample_template, scheduled_for='2017-05-04 14:15', status='delivered') + create_notification(template=sample_template, status='created') scheduled_notifications = dao_get_scheduled_notifications() assert len(scheduled_notifications) == 1 assert scheduled_notifications[0].id == notification_1.id assert scheduled_notifications[0].scheduled_notification.pending -def test_set_scheduled_notification_to_processed(notify_db, notify_db_session, sample_template): - notification_1 = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_template, scheduled_for='2017-05-05 14:15', +def test_set_scheduled_notification_to_processed(sample_template): + notification_1 = create_notification(template=sample_template, scheduled_for='2017-05-05 14:15', status='created') scheduled_notifications = dao_get_scheduled_notifications() assert len(scheduled_notifications) == 1 @@ -1537,7 +1472,7 @@ def test_dao_get_last_notification_added_for_job_id_no_job(sample_template, fake assert dao_get_last_notification_added_for_job_id(fake_uuid) is None -def test_dao_update_notifications_by_reference_updated_notificaitons_and_history(sample_template): +def test_dao_update_notifications_by_reference_updated_notifications(sample_template): notification_0 = create_notification(template=sample_template, reference='noref') notification_1 = create_notification(template=sample_template, reference='ref') notification_2 = create_notification(template=sample_template, reference='ref') @@ -1557,13 +1492,7 @@ def test_dao_update_notifications_by_reference_updated_notificaitons_and_history assert updated_2.billable_units == 2 assert updated_2.status == 'delivered' - assert updated_history_count == 2 - updated_history_1 = NotificationHistory.query.get(notification_1.id) - assert updated_history_1.billable_units == 2 - assert updated_history_1.status == 'delivered' - updated_history_2 = Notification.query.get(notification_2.id) - assert updated_history_2.billable_units == 2 - assert updated_history_2.status == 'delivered' + assert updated_history_count == 0 assert notification_0 == Notification.query.get(notification_0.id) @@ -1590,7 +1519,7 @@ def test_dao_update_notifications_by_reference_set_returned_letter_status(sample ) assert updated_count == 1 - assert updated_history_count == 1 + assert updated_history_count == 0 assert Notification.query.get(notification.id).status == 'returned-letter' diff --git a/tests/app/db.py b/tests/app/db.py index e0863f006..13d872d83 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -211,7 +211,6 @@ def create_notification( scheduled_for=None, normalised_to=None, one_off=False, - sms_sender_id=None, reply_to_text=None, created_by_id=None, postage=None diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index ae8b06782..d04d05206 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -313,8 +313,7 @@ def test_send_sms_should_use_service_sms_sender( mocker.patch('app.mmg_client.send_sms') sms_sender = create_service_sms_sender(service=sample_service, sms_sender='123456', is_default=False) - db_notification = create_notification(template=sample_template, sms_sender_id=sms_sender.id, - reply_to_text=sms_sender.sms_sender) + db_notification = create_notification(template=sample_template, reply_to_text=sms_sender.sms_sender) send_to_providers.send_sms_to_provider( db_notification,