diff --git a/.flake8 b/.flake8 index bfe5a2603..fd1d86b57 100644 --- a/.flake8 +++ b/.flake8 @@ -1,7 +1,7 @@ [flake8] # Rule definitions: http://flake8.pycqa.org/en/latest/user/error-codes.html # W503: line break before binary operator -exclude = venv*,__pycache__,node_modules,cache,migrations +exclude = venv*,__pycache__,node_modules,cache,migrations,build ignore = W503 max-complexity = 14 max-line-length = 120 diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 2ea240f5b..2eb713699 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -24,14 +24,10 @@ from app import db, create_uuid from app.dao import days_ago from app.models import ( Notification, - NotificationEmailReplyTo, NotificationHistory, ScheduledNotification, - ServiceEmailReplyTo, Template, TemplateHistory, - EMAIL_TYPE, - SMS_TYPE, KEY_TYPE_NORMAL, KEY_TYPE_TEST, LETTER_TYPE, @@ -42,9 +38,7 @@ from app.models import ( NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE, - NOTIFICATION_SENT, - NotificationSmsSender, - ServiceSmsSender + NOTIFICATION_SENT ) from app.dao.dao_utils import transactional @@ -315,23 +309,6 @@ def _filter_query(query, filter_dict=None): @transactional def delete_notifications_created_more_than_a_week_ago_by_type(notification_type): seven_days_ago = date.today() - timedelta(days=7) - - # Following could be refactored when NotificationSmsReplyTo and NotificationLetterContact in models.py - if notification_type in [EMAIL_TYPE, SMS_TYPE]: - subq = db.session.query(Notification.id).filter( - func.date(Notification.created_at) < seven_days_ago, - Notification.notification_type == notification_type - ).subquery() - if notification_type == EMAIL_TYPE: - notification_sender_mapping_table = NotificationEmailReplyTo - if notification_type == SMS_TYPE: - notification_sender_mapping_table = NotificationSmsSender - db.session.query( - notification_sender_mapping_table - ).filter( - notification_sender_mapping_table.notification_id.in_(subq) - ).delete(synchronize_session='fetch') - deleted = db.session.query(Notification).filter( func.date(Notification.created_at) < seven_days_ago, Notification.notification_type == notification_type, @@ -570,26 +547,6 @@ def dao_set_created_live_letter_api_notifications_to_pending(): return notifications -@transactional -def dao_create_notification_email_reply_to_mapping(notification_id, email_reply_to_id): - notification_email_reply_to = NotificationEmailReplyTo( - notification_id=notification_id, - service_email_reply_to_id=email_reply_to_id - ) - db.session.add(notification_email_reply_to) - - -def dao_get_notification_email_reply_for_notification(notification_id): - email_reply_to = ServiceEmailReplyTo.query.join( - NotificationEmailReplyTo - ).filter( - NotificationEmailReplyTo.notification_id == notification_id - ).first() - - if email_reply_to: - return email_reply_to.email_address - - @statsd(namespace="dao") def dao_get_last_notification_added_for_job_id(job_id): last_notification_added = Notification.query.filter( @@ -599,23 +556,3 @@ def dao_get_last_notification_added_for_job_id(job_id): ).first() return last_notification_added - - -@transactional -def dao_create_notification_sms_sender_mapping(notification_id, sms_sender_id): - notification_to_sms_sender = NotificationSmsSender( - notification_id=notification_id, - service_sms_sender_id=sms_sender_id - ) - db.session.add(notification_to_sms_sender) - - -def dao_get_notification_sms_sender_mapping(notification_id): - sms_sender = ServiceSmsSender.query.join( - NotificationSmsSender - ).filter( - NotificationSmsSender.notification_id == notification_id - ).first() - - if sms_sender: - return sms_sender.sms_sender diff --git a/app/models.py b/app/models.py index f78466b85..fb3a2610d 100644 --- a/app/models.py +++ b/app/models.py @@ -1572,48 +1572,6 @@ class ServiceLetterContact(db.Model): } -class NotificationEmailReplyTo(db.Model): - __tablename__ = "notification_to_email_reply_to" - - notification_id = db.Column( - UUID(as_uuid=True), - db.ForeignKey('notifications.id'), - unique=True, - index=True, - nullable=False, - primary_key=True - ) - service_email_reply_to_id = db.Column( - UUID(as_uuid=True), - db.ForeignKey('service_email_reply_to.id'), - unique=False, - index=True, - nullable=False, - primary_key=True - ) - - -class NotificationSmsSender(db.Model): - __tablename__ = "notification_to_sms_sender" - - notification_id = db.Column( - UUID(as_uuid=True), - db.ForeignKey('notifications.id'), - unique=True, - index=True, - nullable=False, - primary_key=True - ) - service_sms_sender_id = db.Column( - UUID(as_uuid=True), - db.ForeignKey('service_sms_senders.id'), - unique=False, - index=True, - nullable=False, - primary_key=True - ) - - class AuthType(db.Model): __tablename__ = 'auth_type' diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index c2d2f1a52..df46b0f02 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -13,12 +13,19 @@ from notifications_utils.recipients import ( from app import redis_store from app.celery import provider_tasks from app.config import QueueNames -from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE, NOTIFICATION_CREATED, ScheduledNotification -from app.dao.notifications_dao import (dao_create_notification, - dao_delete_notifications_and_history_by_id, - dao_created_scheduled_notification, - dao_create_notification_email_reply_to_mapping, - dao_create_notification_sms_sender_mapping) +from app.models import ( + EMAIL_TYPE, + KEY_TYPE_TEST, + SMS_TYPE, + NOTIFICATION_CREATED, + Notification, + ScheduledNotification +) +from app.dao.notifications_dao import ( + dao_create_notification, + dao_delete_notifications_and_history_by_id, + dao_created_scheduled_notification +) from app.v2.errors import BadRequestError from app.utils import get_template_instance, cache_key_for_service_template_counter, convert_bst_to_utc @@ -145,11 +152,3 @@ def persist_scheduled_notification(notification_id, scheduled_for): scheduled_notification = ScheduledNotification(notification_id=notification_id, scheduled_for=scheduled_datetime) dao_created_scheduled_notification(scheduled_notification) - - -def persist_email_reply_to_id_for_notification(notification_id, email_reply_to_id): - dao_create_notification_email_reply_to_mapping(notification_id, email_reply_to_id) - - -def persist_sms_sender_id_for_notification(notification_id, sms_sender_id): - dao_create_notification_sms_sender_mapping(notification_id, sms_sender_id) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 5274d5e2b..634edc5ff 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -8,9 +8,7 @@ from app.notifications.validators import ( ) from app.notifications.process_notifications import ( persist_notification, - send_notification_to_queue, - persist_email_reply_to_id_for_notification, - persist_sms_sender_id_for_notification + send_notification_to_queue ) from app.models import ( KEY_TYPE_NORMAL, @@ -71,11 +69,6 @@ def send_one_off_notification(service_id, post_data): created_by_id=post_data['created_by'], reply_to_text=reply_to ) - if sender_id: - if template.template_type == EMAIL_TYPE: - persist_email_reply_to_id_for_notification(notification.id, sender_id) - if template.template_type == SMS_TYPE: - persist_sms_sender_id_for_notification(notification.id, sender_id) queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None send_notification_to_queue( diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index f4f08c9c8..be35f30b4 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -16,12 +16,11 @@ from app.models import ( ) from app.celery.tasks import update_letter_notifications_to_sent_to_dvla from app.notifications.process_notifications import ( - persist_email_reply_to_id_for_notification, persist_notification, persist_scheduled_notification, send_notification_to_queue, - simulated_recipient, - persist_sms_sender_id_for_notification) + simulated_recipient +) from app.notifications.process_letter_notifications import ( create_letter_notification ) @@ -144,8 +143,6 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ reply_to_text=reply_to_text ) - persist_sender_to_notification_mapping(form, notification) - scheduled_for = form.get("scheduled_for", None) if scheduled_for: persist_scheduled_notification(notification.id, form["scheduled_for"]) @@ -163,15 +160,6 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ return notification -def persist_sender_to_notification_mapping(form, notification): - email_reply_to_id = form.get("email_reply_to_id", None) - if email_reply_to_id: - persist_email_reply_to_id_for_notification(notification.id, email_reply_to_id) - sms_sender_id = form.get("sms_sender_id", None) - if sms_sender_id: - persist_sms_sender_id_for_notification(notification.id, sms_sender_id) - - def process_letter_notification(*, letter_data, api_key, template): if api_key.key_type == KEY_TYPE_TEAM: raise BadRequestError(message='Cannot send letters with a team api key', status_code=403) diff --git a/manifest-api-staging.yml b/manifest-api-staging.yml index d19455470..05b1d420d 100644 --- a/manifest-api-staging.yml +++ b/manifest-api-staging.yml @@ -1,7 +1,7 @@ --- inherit: manifest-api-base.yml -command: scripts/run_app_paas.sh gunicorn -c /home/vcap/app/gunicorn_config.py --error-logfile /home/vcap/logs/gunicorn_error.log -w 10 -b 0.0.0.0:$PORT application +command: scripts/run_app_paas.sh gunicorn -c /home/vcap/app/gunicorn_config.py --error-logfile /home/vcap/logs/gunicorn_error.log -w 6 -b 0.0.0.0:$PORT application services: - notify-aws - notify-config diff --git a/migrations/versions/0147_drop_mapping_tables.py b/migrations/versions/0147_drop_mapping_tables.py new file mode 100644 index 000000000..82d337f3a --- /dev/null +++ b/migrations/versions/0147_drop_mapping_tables.py @@ -0,0 +1,41 @@ +""" + +Revision ID: 0147_drop_mapping_tables +Revises: 0146_add_service_callback_api +Create Date: 2017-11-30 15:48:44.588438 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0147_drop_mapping_tables' +down_revision = '0146_add_service_callback_api' + + +def upgrade(): + op.drop_table('notification_to_sms_sender') + op.drop_table('notification_to_email_reply_to') + + +def downgrade(): + op.create_table('notification_to_email_reply_to', + sa.Column('notification_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('service_email_reply_to_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint(['notification_id'], ['notifications.id'], + name='notification_to_email_reply_to_notification_id_fkey'), + sa.ForeignKeyConstraint(['service_email_reply_to_id'], ['service_email_reply_to.id'], + name='notification_to_email_reply_to_service_email_reply_to_id_fkey'), + sa.PrimaryKeyConstraint('notification_id', 'service_email_reply_to_id', + name='notification_to_email_reply_to_pkey') + ) + op.create_table('notification_to_sms_sender', + sa.Column('notification_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('service_sms_sender_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint(['notification_id'], ['notifications.id'], + name='notification_to_sms_sender_notification_id_fkey'), + sa.ForeignKeyConstraint(['service_sms_sender_id'], ['service_sms_senders.id'], + name='notification_to_sms_sender_service_sms_sender_id_fkey'), + sa.PrimaryKeyConstraint('notification_id', 'service_sms_sender_id', + name='notification_to_sms_sender_pkey') + ) diff --git a/requirements.txt b/requirements.txt index 68937f75d..4cdb13bdb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,9 +23,12 @@ statsd==3.2.1 notifications-python-client==4.6.0 # PaaS -awscli>=1.11,<1.12 +awscli==1.14.1 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@23.1.0#egg=notifications-utils==23.1.0 +git+https://github.com/alphagov/notifications-utils.git@23.2.1#egg=notifications-utils==23.2.1 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 + +# required by utils +git+https://github.com/leohemsted/smartypants.py.git@regex-speedup#egg=smartypants==2.1.0 diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 8d88ef2f9..c11d03268 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -1,42 +1,18 @@ -from datetime import datetime, timedelta, date import uuid +from datetime import datetime, timedelta, date from functools import partial import pytest from freezegun import freeze_time from sqlalchemy.exc import SQLAlchemyError, IntegrityError -from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_service_id -from app.models import ( - Job, - Notification, - NotificationEmailReplyTo, - NotificationHistory, - NotificationSmsSender, - ScheduledNotification, - EMAIL_TYPE, - SMS_TYPE, - NOTIFICATION_STATUS_TYPES, - NOTIFICATION_STATUS_TYPES_FAILED, - NOTIFICATION_SENT, - NOTIFICATION_DELIVERED, - KEY_TYPE_NORMAL, - KEY_TYPE_TEAM, - KEY_TYPE_TEST, - JOB_STATUS_IN_PROGRESS -) - from app.dao.notifications_dao import ( dao_create_notification, - dao_create_notification_email_reply_to_mapping, - dao_create_notification_sms_sender_mapping, dao_created_scheduled_notification, dao_delete_notifications_and_history_by_id, dao_get_last_notification_added_for_job_id, dao_get_last_template_usage, - dao_get_notification_email_reply_for_notification, dao_get_notifications_by_to_field, - dao_get_notification_sms_sender_mapping, dao_get_scheduled_notifications, dao_get_template_usage, dao_timeout_notifications, @@ -55,14 +31,21 @@ from app.dao.notifications_dao import ( update_notification_status_by_id, update_notification_status_by_reference ) - from app.dao.services_dao import dao_update_service -from tests.app.db import ( - create_api_key, - create_job, - create_notification, - create_reply_to_email, - create_service_sms_sender) +from app.models import ( + Job, + Notification, + NotificationHistory, + ScheduledNotification, + NOTIFICATION_STATUS_TYPES, + NOTIFICATION_STATUS_TYPES_FAILED, + NOTIFICATION_SENT, + NOTIFICATION_DELIVERED, + KEY_TYPE_NORMAL, + KEY_TYPE_TEAM, + KEY_TYPE_TEST, + JOB_STATUS_IN_PROGRESS +) from tests.app.conftest import ( sample_notification, sample_template, @@ -70,7 +53,13 @@ from tests.app.conftest import ( sample_service, sample_job, sample_notification_history as create_notification_history, - sample_letter_template) + sample_letter_template +) +from tests.app.db import ( + create_api_key, + create_job, + create_notification +) def test_should_have_decorated_notifications_dao_functions(): @@ -985,73 +974,6 @@ def test_should_delete_notifications_by_type_after_seven_days( assert notification.created_at.date() >= date(2016, 1, 3) -@freeze_time("2016-01-10 12:00:00.000000") -def test_should_delete_notification_to_email_reply_to_after_seven_days( - notify_db, notify_db_session, sample_service, -): - assert len(Notification.query.all()) == 0 - - reply_to = create_reply_to_email(sample_service, 'test@example.com') - - email_template = sample_email_template(notify_db, notify_db_session, service=sample_service) - - # create one notification a day between 1st and 10th from 11:00 to 19:00 of each type - for i in range(1, 11): - past_date = '2016-01-{0:02d} {0:02d}:00:00.000000'.format(i) - with freeze_time(past_date): - notification = create_notification(email_template) - dao_create_notification_email_reply_to_mapping(notification.id, reply_to.id) - - all_notifications = Notification.query.all() - assert len(all_notifications) == 10 - - all_notification_email_reply_to = NotificationEmailReplyTo.query.all() - assert len(all_notification_email_reply_to) == 10 - - # Records before 3rd should be deleted - delete_notifications_created_more_than_a_week_ago_by_type(EMAIL_TYPE) - remaining_email_notifications = Notification.query.filter_by(notification_type=EMAIL_TYPE).all() - remaining_notification_to_email_reply_to = NotificationEmailReplyTo.query.filter_by().all() - - assert len(remaining_email_notifications) == 8 - assert len(remaining_notification_to_email_reply_to) == 8 - - for notification in remaining_email_notifications: - assert notification.created_at.date() >= date(2016, 1, 3) - - -@freeze_time("2016-01-10 12:00:00.000000") -def test_should_delete_notification_to_sms_sender_after_seven_days( - sample_template -): - assert len(Notification.query.all()) == 0 - - sms_sender = create_service_sms_sender(service=sample_template.service, sms_sender='123456', is_default=False) - - # create one notification a day between 1st and 10th from 11:00 to 19:00 of each type - for i in range(1, 11): - past_date = '2016-01-{0:02d} {0:02d}:00:00.000000'.format(i) - with freeze_time(past_date): - notification = create_notification(template=sample_template, sms_sender_id=sms_sender.id) - - all_notifications = Notification.query.all() - assert len(all_notifications) == 10 - - all_notification_sms_senders = NotificationSmsSender.query.all() - assert len(all_notification_sms_senders) == 10 - - # Records before 3rd should be deleted - delete_notifications_created_more_than_a_week_ago_by_type(SMS_TYPE) - remaining_notifications = Notification.query.filter_by(notification_type=SMS_TYPE).all() - remaining_notification_to_sms_sender = NotificationSmsSender.query.filter_by().all() - - assert len(remaining_notifications) == 8 - assert len(remaining_notification_to_sms_sender) == 8 - - for notification in remaining_notifications: - assert notification.created_at.date() >= date(2016, 1, 3) - - @pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) @freeze_time("2016-01-10 12:00:00.000000") def test_should_not_delete_notification_history(notify_db, notify_db_session, sample_service, notification_type): @@ -2006,48 +1928,6 @@ def test_dao_get_notifications_by_to_field_orders_by_created_at_desc(sample_temp assert notifications[1].id == notification_a_minute_ago.id -def test_dao_create_notification_email_reply_to_mapping(sample_service, sample_notification): - - create_reply_to_email(sample_service, "test@test.com") - - reply_to_address = dao_get_reply_to_by_service_id(sample_service.id) - - dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address[0].id) - - email_reply_to = NotificationEmailReplyTo.query.all() - - assert len(email_reply_to) == 1 - assert email_reply_to[0].notification_id == sample_notification.id - assert email_reply_to[0].service_email_reply_to_id == reply_to_address[0].id - - -def test_dao_create_multiple_notification_email_reply_to_mapping(sample_service, sample_notification): - reply_to_address = create_reply_to_email(sample_service, "test@test.com") - - dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address.id) - - with pytest.raises(IntegrityError) as e: - dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address.id) - - assert 'duplicate key value' in str(e.value) - - email_reply_to = NotificationEmailReplyTo.query.all() - - assert len(email_reply_to) == 1 - assert email_reply_to[0].notification_id == sample_notification.id - assert email_reply_to[0].service_email_reply_to_id == reply_to_address.id - - -def test_dao_get_notification_email_reply_for_notification(sample_service, sample_notification): - reply_to_address = create_reply_to_email(sample_service, "test@test.com") - dao_create_notification_email_reply_to_mapping(sample_notification.id, reply_to_address.id) - assert dao_get_notification_email_reply_for_notification(sample_notification.id) == "test@test.com" - - -def test_dao_get_notification_email_reply_for_notification_where_no_mapping(notify_db_session, fake_uuid): - assert dao_get_notification_email_reply_for_notification(fake_uuid) is None - - def test_dao_get_last_notification_added_for_job_id_valid_job_id(sample_template): job = create_job(template=sample_template, notification_count=10, created_at=datetime.utcnow() - timedelta(hours=2), @@ -2109,26 +1989,3 @@ def test_dao_update_notifications_by_reference_returns_zero_when_no_notification "billable_units": 2} ) assert updated_count == 0 - - -def test_dao_create_notification_sms_sender_mapping(sample_notification): - sms_sender = create_service_sms_sender(service=sample_notification.service, sms_sender='123456') - dao_create_notification_sms_sender_mapping(notification_id=sample_notification.id, - sms_sender_id=sms_sender.id) - notification_to_senders = NotificationSmsSender.query.all() - assert len(notification_to_senders) == 1 - assert notification_to_senders[0].notification_id == sample_notification.id - assert notification_to_senders[0].service_sms_sender_id == sms_sender.id - - -def test_dao_get_notification_sms_sender_mapping(sample_notification): - sms_sender = create_service_sms_sender(service=sample_notification.service, sms_sender='123456') - dao_create_notification_sms_sender_mapping(notification_id=sample_notification.id, - sms_sender_id=sms_sender.id) - notification_to_sender = dao_get_notification_sms_sender_mapping(sample_notification.id) - assert notification_to_sender == '123456' - - -def test_dao_get_notification_sms_sender_mapping_returns_none(sample_notification): - notification_to_sender = dao_get_notification_sms_sender_mapping(sample_notification.id) - assert not notification_to_sender diff --git a/tests/app/db.py b/tests/app/db.py index 879d9ee39..717b1aa2b 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -13,7 +13,6 @@ from app.models import ( Job, MonthlyBilling, Notification, - NotificationEmailReplyTo, Organisation, Rate, Service, @@ -34,8 +33,7 @@ from app.models import ( from app.dao.users_dao import save_model_user from app.dao.notifications_dao import ( dao_create_notification, - dao_created_scheduled_notification, - dao_create_notification_sms_sender_mapping + dao_created_scheduled_notification ) from app.dao.templates_dao import dao_create_template from app.dao.services_dao import dao_create_service @@ -221,9 +219,7 @@ def create_notification( if status != 'created': scheduled_notification.pending = False dao_created_scheduled_notification(scheduled_notification) - if sms_sender_id: - dao_create_notification_sms_sender_mapping(notification_id=notification.id, - sms_sender_id=sms_sender_id) + return notification @@ -445,24 +441,6 @@ def create_letter_contact( return letter_content -def create_reply_to_email_for_notification( - notification_id, - service, - email_address, - is_default=True -): - reply_to = create_reply_to_email(service, email_address, is_default) - - notification_email_reply_to = NotificationEmailReplyTo( - notification_id=str(notification_id), - service_email_reply_to_id=str(reply_to.id) - ) - db.session.add(notification_email_reply_to) - db.session.commit() - - return reply_to - - def create_annual_billing( service_id, free_sms_fragment_limit, financial_year_start ): diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 1b7081ee6..69a00d306 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -29,9 +29,7 @@ from tests.app.db import ( create_template, create_notification, create_reply_to_email, - create_reply_to_email_for_notification, create_service_sms_sender, - create_service_with_inbound_number, create_service_with_defined_sms_sender ) @@ -708,65 +706,13 @@ def test_should_handle_sms_sender_and_prefix_message( ) -def test_should_use_inbound_number_as_sender_if_default_sms_sender( - notify_db_session, - mocker -): - service = create_service_with_inbound_number(inbound_number='inbound') - create_service_sms_sender(service=service, sms_sender="sms_sender", is_default=False) - template = create_template(service, content='bar') - notification = create_notification(template, reply_to_text='inbound') - - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') - - send_to_providers.send_sms_to_provider(notification) - - mmg_client.send_sms.assert_called_once_with( - to=ANY, - content=ANY, - reference=str(notification.id), - sender='inbound' - ) - - -def test_should_use_default_sms_sender( - notify_db_session, - mocker -): - service = create_service_with_defined_sms_sender(sms_sender_value="test sender") - template = create_template(service, content='bar') - notification = create_notification(template, reply_to_text=service.get_default_sms_sender()) - - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') - - send_to_providers.send_sms_to_provider(notification) - - mmg_client.send_sms.assert_called_once_with( - to=ANY, - content=ANY, - reference=str(notification.id), - sender='test sender' - ) - - -def test_send_email_to_provider_get_linked_email_reply_to_default_is_false( - sample_service, +def test_send_email_to_provider_uses_reply_to_from_notification( sample_email_template, mocker): mocker.patch('app.aws_ses_client.send_email', return_value='reference') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com") - create_reply_to_email(service=sample_service, email_address="test@test.com") - - reply_to = create_reply_to_email_for_notification( - db_notification.id, - sample_service, - "test@test.com", - is_default=False - ) send_to_providers.send_email_to_provider( db_notification, @@ -778,45 +724,11 @@ def test_send_email_to_provider_get_linked_email_reply_to_default_is_false( ANY, body=ANY, html_body=ANY, - reply_to_address=reply_to.email_address - ) - - -def test_send_email_to_provider_get_linked_email_reply_to_create_service_email_after_notification_mapping( - sample_service, - sample_email_template, - mocker): - mocker.patch('app.aws_ses_client.send_email', return_value='reference') - mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') - - db_notification = create_notification(template=sample_email_template, - reply_to_text="test@test.com") - - # notification_to_email_reply_to is being deprecated. - reply_to = create_reply_to_email_for_notification( - db_notification.id, - sample_service, - "test@test.com" - ) - - create_reply_to_email(service=sample_service, email_address='foo@bar.com', is_default=False) - - send_to_providers.send_email_to_provider( - db_notification, - ) - - app.aws_ses_client.send_email.assert_called_once_with( - ANY, - ANY, - ANY, - body=ANY, - html_body=ANY, - reply_to_address=reply_to.email_address + reply_to_address="test@test.com" ) def test_send_email_to_provider_should_format_reply_to_email_address( - sample_service, sample_email_template, mocker): mocker.patch('app.aws_ses_client.send_email', return_value='reference') @@ -824,12 +736,6 @@ def test_send_email_to_provider_should_format_reply_to_email_address( db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com\t") - create_reply_to_email_for_notification( - db_notification.id, - sample_service, - "test@test.com" - ) - send_to_providers.send_email_to_provider( db_notification, ) diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 18a5dbd03..342a17987 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -7,21 +7,16 @@ from sqlalchemy.exc import SQLAlchemyError from freezegun import freeze_time from collections import namedtuple -from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_service_id from app.models import ( Notification, NotificationHistory, - NotificationEmailReplyTo, - NotificationSmsSender, ScheduledNotification, Template ) from app.notifications.process_notifications import ( create_content_for_notification, persist_notification, - persist_email_reply_to_id_for_notification, persist_scheduled_notification, - persist_sms_sender_id_for_notification, send_notification_to_queue, simulated_recipient ) @@ -29,7 +24,6 @@ from notifications_utils.recipients import validate_and_format_phone_number, val from app.utils import cache_key_for_service_template_counter from app.v2.errors import BadRequestError from tests.app.conftest import sample_api_key as create_api_key -from tests.app.db import create_reply_to_email, create_service_sms_sender def test_create_content_for_notification_passes(sample_email_template): @@ -452,27 +446,3 @@ def test_persist_email_notification_stores_normalised_email( assert persisted_notification.to == recipient assert persisted_notification.normalised_to == expected_recipient_normalised - - -def test_persist_email_reply_to_id_for_notification(sample_service, sample_notification): - create_reply_to_email(sample_service, "test@test.com") - reply_to_address = dao_get_reply_to_by_service_id(sample_service.id) - persist_email_reply_to_id_for_notification(sample_notification.id, reply_to_address[0].id) - - email_reply_to = NotificationEmailReplyTo.query.all() - - assert len(email_reply_to) == 1 - assert email_reply_to[0].notification_id == sample_notification.id - assert email_reply_to[0].service_email_reply_to_id == reply_to_address[0].id - - -def test_persist_sms_sender_id_for_notification(sample_service, sample_notification): - sms_sender = create_service_sms_sender(service=sample_service, sms_sender="123456") - persist_sms_sender_id_for_notification(notification_id=sample_notification.id, - sms_sender_id=sms_sender.id) - - notification_to_sms_sender = NotificationSmsSender.query.all() - - assert len(notification_to_sms_sender) == 1 - assert notification_to_sms_sender[0].notification_id == sample_notification.id - assert notification_to_sms_sender[0].service_sms_sender_id == sms_sender.id diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index 67500bf8f..14c49aa56 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -12,15 +12,12 @@ from app.models import ( KEY_TYPE_NORMAL, PRIORITY, SMS_TYPE, - NotificationEmailReplyTo, - Notification, - NotificationSmsSender + Notification ) from tests.app.db import ( create_user, create_reply_to_email, - create_service_sms_sender, create_service, create_template ) @@ -200,7 +197,7 @@ def test_send_one_off_notification_fails_if_created_by_other_service(sample_temp assert e.value.message == 'Can’t create notification - Test User is not part of the "Sample service" service' -def test_send_one_off_notification_should_add_email_reply_to_id_for_email(sample_email_template, celery_mock): +def test_send_one_off_notification_should_add_email_reply_to_text_for_notification(sample_email_template, celery_mock): reply_to_email = create_reply_to_email(sample_email_template.service, 'test@test.com') data = { 'to': 'ok@ok.com', @@ -216,8 +213,7 @@ def test_send_one_off_notification_should_add_email_reply_to_id_for_email(sample research_mode=False, queue=None ) - mapping_row = NotificationEmailReplyTo.query.filter_by(notification_id=notification_id['id']).first() - assert mapping_row.service_email_reply_to_id == reply_to_email.id + notification.reply_to_text == reply_to_email.email_address def test_send_one_off_notification_should_throw_exception_if_reply_to_id_doesnot_exist( @@ -234,26 +230,6 @@ def test_send_one_off_notification_should_throw_exception_if_reply_to_id_doesnot send_one_off_notification(service_id=sample_email_template.service.id, post_data=data) -def test_send_one_off_notification_should_add_sms_sender_mapping_for_sms(sample_template, celery_mock): - sms_sender = create_service_sms_sender(service=sample_template.service, sms_sender='123456') - data = { - 'to': '07700 900 001', - 'template_id': str(sample_template.id), - 'sender_id': sms_sender.id, - 'created_by': str(sample_template.service.created_by_id) - } - - notification_id = send_one_off_notification(service_id=sample_template.service.id, post_data=data) - notification = Notification.query.get(notification_id['id']) - celery_mock.assert_called_once_with( - notification=notification, - research_mode=False, - queue=None - ) - mapping_row = NotificationSmsSender.query.filter_by(notification_id=notification_id['id']).first() - assert mapping_row.service_sms_sender_id == sms_sender.id - - def test_send_one_off_notification_should_throw_exception_if_sms_sender_id_doesnot_exist( sample_template ): diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 361f729ef..aa1bb70f6 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -5,12 +5,10 @@ from freezegun import freeze_time from app.dao.service_sms_sender_dao import dao_update_service_sms_sender from app.models import ( - NotificationEmailReplyTo, ScheduledNotification, SCHEDULE_NOTIFICATIONS, EMAIL_TYPE, - SMS_TYPE, - NotificationSmsSender + SMS_TYPE ) from flask import json, current_app @@ -18,7 +16,6 @@ from app.models import Notification from app.schema_validation import validate from app.v2.errors import RateLimitError from app.v2.notifications.notification_schemas import post_sms_response, post_email_response -from app.v2.notifications.post_notifications import persist_sender_to_notification_mapping from tests import create_authorization_header from tests.app.conftest import ( sample_template as create_sample_template, @@ -31,7 +28,7 @@ from tests.app.db import ( create_service, create_template, create_reply_to_email, - create_service_sms_sender, create_notification, + create_service_sms_sender, create_service_with_inbound_number ) @@ -120,11 +117,7 @@ def test_post_sms_notification_returns_201_with_sms_sender_id( assert response.status_code == 201 resp_json = json.loads(response.get_data(as_text=True)) assert validate(resp_json, post_sms_response) == resp_json - notification_to_sms_sender = NotificationSmsSender.query.all() - assert len(notification_to_sms_sender) == 1 - assert str(notification_to_sms_sender[0].notification_id) == resp_json['id'] assert resp_json['content']['from_number'] == sms_sender.sms_sender - assert notification_to_sms_sender[0].service_sms_sender_id == sms_sender.id notifications = Notification.query.all() assert len(notifications) == 1 assert notifications[0].reply_to_text == sms_sender.sms_sender @@ -626,10 +619,6 @@ def test_post_email_notification_with_valid_reply_to_id_returns_201(client, samp assert resp_json['id'] == str(notification.id) assert mocked.called - email_reply_to = NotificationEmailReplyTo.query.one() - - assert email_reply_to.notification_id == notification.id - assert email_reply_to.service_email_reply_to_id == reply_to_email.id assert notification.reply_to_text == reply_to_email.email_address @@ -650,35 +639,3 @@ def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sa assert 'email_reply_to_id {} does not exist in database for service id {}'. \ format(fake_uuid, sample_email_template.service_id) in resp_json['errors'][0]['message'] assert 'BadRequestError' in resp_json['errors'][0]['error'] - - -def test_persist_sender_to_notification_mapping_for_email(sample_service): - template = create_template(service=sample_service, template_type=EMAIL_TYPE) - sender = create_reply_to_email(service=sample_service, email_address='reply@test.com', is_default=False) - form = { - "email_address": "recipient@test.com", - "template_id": str(template.id), - 'email_reply_to_id': str(sender.id) - } - notification = create_notification(template=template) - persist_sender_to_notification_mapping(form=form, notification=notification) - notification_to_email_reply_to = NotificationEmailReplyTo.query.all() - assert len(notification_to_email_reply_to) == 1 - assert notification_to_email_reply_to[0].notification_id == notification.id - assert notification_to_email_reply_to[0].service_email_reply_to_id == sender.id - - -def test_persist_sender_to_notification_mapping_for_sms(sample_service): - template = create_template(service=sample_service, template_type=SMS_TYPE) - sender = create_service_sms_sender(service=sample_service, sms_sender='12345', is_default=False) - form = { - 'phone_number': '+447700900855', - 'template_id': str(template.id), - 'sms_sender_id': str(sender.id) - } - notification = create_notification(template=template) - persist_sender_to_notification_mapping(form=form, notification=notification) - notification_to_sms_sender = NotificationSmsSender.query.all() - assert len(notification_to_sms_sender) == 1 - assert notification_to_sms_sender[0].notification_id == notification.id - assert notification_to_sms_sender[0].service_sms_sender_id == sender.id