diff --git a/app/celery/tasks.py b/app/celery/tasks.py index bece59f7c..a042d7f0a 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -36,6 +36,7 @@ from app.dao.jobs_dao import ( dao_update_job_status) from app.dao.notifications_dao import ( get_notification_by_id, + update_notification_status_by_reference, dao_update_notifications_for_job_to_sent_to_dvla, dao_update_notifications_by_reference, dao_get_last_notification_added_for_job_id) @@ -46,6 +47,7 @@ from app.dao.templates_dao import dao_get_template_by_id from app.models import ( Job, Notification, + DVLA_RESPONSE_STATUS_SENT, EMAIL_TYPE, JOB_STATUS_CANCELLED, JOB_STATUS_FINISHED, @@ -55,6 +57,7 @@ from app.models import ( JOB_STATUS_SENT_TO_DVLA, JOB_STATUS_ERROR, KEY_TYPE_NORMAL, LETTER_TYPE, + NOTIFICATION_DELIVERED, NOTIFICATION_SENDING, NOTIFICATION_TECHNICAL_FAILURE, SMS_TYPE, @@ -443,8 +446,22 @@ def update_letter_notifications_statuses(self, filename): raise else: for update in notification_updates: - current_app.logger.info('DVLA update: {}'.format(str(update))) - # TODO: Update notifications with desired status + status = NOTIFICATION_DELIVERED if update.status == DVLA_RESPONSE_STATUS_SENT \ + else NOTIFICATION_TECHNICAL_FAILURE + notification = update_notification_status_by_reference( + update.reference, + status + ) + + if not notification: + msg = "Update letter notification file {filename} failed: notification either not found " \ + "or already updated from delivered. Status {status} for notification reference {reference}".format( + filename=filename, status=status, reference=update.reference) + current_app.logger.error(msg) + else: + current_app.logger.info( + 'DVLA file: {filename}, notification updated to {status}: {reference}'.format( + filename=filename, status=status, reference=str(update.reference))) def process_updates_from_file(response_file): diff --git a/app/dao/service_sms_sender_dao.py b/app/dao/service_sms_sender_dao.py index 69397f441..4b89b7e8d 100644 --- a/app/dao/service_sms_sender_dao.py +++ b/app/dao/service_sms_sender_dao.py @@ -83,6 +83,14 @@ def dao_update_service_sms_sender(service_id, service_sms_sender_id, is_default, return sms_sender_to_update +@transactional +def update_existing_sms_sender_with_inbound_number(service_sms_sender, sms_sender, inbound_number_id): + service_sms_sender.sms_sender = sms_sender + service_sms_sender.inbound_number_id = inbound_number_id + db.session.add(service_sms_sender) + return service_sms_sender + + def _get_existing_default(service_id): sms_senders = dao_get_sms_senders_by_service_id(service_id=service_id) if sms_senders: diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 808cd2cdb..c253748ad 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -73,7 +73,7 @@ def send_sms_to_provider(notification): to=validate_and_format_phone_number(notification.to, international=notification.international), content=str(template), reference=str(notification.id), - sender=service.get_inbound_number() + sender=service.get_default_sms_sender() ) except Exception as e: dao_toggle_sms_provider(provider.name) diff --git a/app/models.py b/app/models.py index a757485bc..d59164896 100644 --- a/app/models.py +++ b/app/models.py @@ -253,8 +253,6 @@ class Service(db.Model, Versioned): def get_inbound_number(self): if self.inbound_number and self.inbound_number.active: return self.inbound_number.number - else: - return self.get_default_sms_sender() def get_default_sms_sender(self): default_sms_sender = [x for x in self.service_sms_senders if x.is_default] @@ -871,7 +869,9 @@ NOTIFICATION_STATUS_TYPES_NON_BILLABLE = list(set(NOTIFICATION_STATUS_TYPES) - s NOTIFICATION_STATUS_TYPES_ENUM = db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_type') NOTIFICATION_STATUS_LETTER_ACCEPTED = 'accepted' -NOTIFICATION_STATUS_LETTER_ACCEPTED_PRETTY = 'Accepted' +NOTIFICATION_STATUS_LETTER_RECEIVED = 'received' + +DVLA_RESPONSE_STATUS_SENT = 'Sent' class NotificationStatusTypes(db.Model): @@ -982,6 +982,14 @@ class Notification(db.Model): ['technical-failure', 'temporary-failure', 'permanent-failure', 'created', 'sending'] + - + + > IN + 'delivered' + + < OUT + ['received'] + :param status_or_statuses: a single status or list of statuses :return: a single status or list with the current failure statuses substituted for 'failure' """ @@ -990,6 +998,7 @@ class Notification(db.Model): return ( NOTIFICATION_STATUS_TYPES_FAILED if _status == NOTIFICATION_FAILED else [NOTIFICATION_CREATED, NOTIFICATION_SENDING] if _status == NOTIFICATION_STATUS_LETTER_ACCEPTED else + NOTIFICATION_DELIVERED if _status == NOTIFICATION_STATUS_LETTER_RECEIVED else [_status] ) @@ -1038,8 +1047,9 @@ class Notification(db.Model): }, 'letter': { 'technical-failure': 'Technical failure', - 'sending': NOTIFICATION_STATUS_LETTER_ACCEPTED_PRETTY, - 'created': NOTIFICATION_STATUS_LETTER_ACCEPTED_PRETTY, + 'sending': 'Accepted', + 'created': 'Accepted', + 'delivered': 'Received' } }[self.template.template_type].get(self.status, self.status) @@ -1054,8 +1064,10 @@ class Notification(db.Model): # get the two code flows mixed up at all assert self.notification_type == LETTER_TYPE - if self.status == NOTIFICATION_CREATED or NOTIFICATION_SENDING: + if self.status in [NOTIFICATION_CREATED, NOTIFICATION_SENDING]: return NOTIFICATION_STATUS_LETTER_ACCEPTED + elif self.status == NOTIFICATION_DELIVERED: + return NOTIFICATION_STATUS_LETTER_RECEIVED else: # Currently can only be technical-failure return self.status diff --git a/app/service/rest.py b/app/service/rest.py index 3c19fb4ed..833bdb5de 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -28,8 +28,8 @@ from app.dao.service_sms_sender_dao import ( dao_add_sms_sender_for_service, dao_update_service_sms_sender, dao_get_service_sms_senders_by_id, - dao_get_sms_senders_by_service_id -) + dao_get_sms_senders_by_service_id, + update_existing_sms_sender_with_inbound_number) from app.dao.services_dao import ( dao_fetch_service_by_id, dao_fetch_all_services, @@ -653,10 +653,22 @@ def add_service_sms_sender(service_id): form = validate(request.get_json(), add_service_sms_sender_request) inbound_number_id = form.get('inbound_number_id', None) sms_sender = form.get('sms_sender') + if inbound_number_id: updated_number = dao_allocate_number_for_service(service_id=service_id, inbound_number_id=inbound_number_id) - # the sms_sender in the form is the inbound_number_id from client, use number from table. + # the sms_sender in the form is not set, use the inbound number sms_sender = updated_number.number + existing_sms_sender = dao_get_sms_senders_by_service_id(service_id) + # we don't want to create a new sms sender for the service if we are allocating an inbound number. + if len(existing_sms_sender) == 1: + update_existing_sms_sender = existing_sms_sender[0] + new_sms_sender = update_existing_sms_sender_with_inbound_number( + service_sms_sender=update_existing_sms_sender, + sms_sender=sms_sender, + inbound_number_id=inbound_number_id) + + return jsonify(new_sms_sender.serialize()), 201 + new_sms_sender = dao_add_sms_sender_for_service(service_id=service_id, sms_sender=sms_sender, is_default=form['is_default'], diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 2eeddc532..1689a7319 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -1,4 +1,9 @@ -from app.models import NOTIFICATION_STATUS_TYPES, NOTIFICATION_STATUS_LETTER_ACCEPTED, TEMPLATE_TYPES +from app.models import ( + NOTIFICATION_STATUS_TYPES, + NOTIFICATION_STATUS_LETTER_ACCEPTED, + NOTIFICATION_STATUS_LETTER_RECEIVED, + TEMPLATE_TYPES +) from app.schema_validation.definitions import (uuid, personalisation, letter_personalisation) @@ -59,7 +64,8 @@ get_notifications_request = { "status": { "type": "array", "items": { - "enum": NOTIFICATION_STATUS_TYPES + [NOTIFICATION_STATUS_LETTER_ACCEPTED] + "enum": NOTIFICATION_STATUS_TYPES + + [NOTIFICATION_STATUS_LETTER_ACCEPTED + ', ' + NOTIFICATION_STATUS_LETTER_RECEIVED] } }, "template_type": { diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 32b96cf8f..b4c837306 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -95,7 +95,7 @@ def post_notification(notification_type): if notification_type == SMS_TYPE: create_resp_partial = functools.partial( create_post_sms_response_from_notification, - from_number=authenticated_service.get_inbound_number() + from_number=authenticated_service.get_default_sms_sender() ) elif notification_type == EMAIL_TYPE: create_resp_partial = functools.partial( diff --git a/requirements.txt b/requirements.txt index 386828209..9d576ab16 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,16 +11,16 @@ Flask==0.12.2 gunicorn==19.7.1 iso8601==0.1.12 jsonschema==2.6.0 -marshmallow-sqlalchemy==0.13.1 +marshmallow-sqlalchemy==0.13.2 marshmallow==2.13.6 monotonic==1.3 -psycopg2==2.7.3.1 +psycopg2==2.7.3.2 PyJWT==1.5.3 SQLAlchemy-Utils==0.32.19 SQLAlchemy==1.1.14 statsd==3.2.1 -notifications-python-client==4.4.0 +notifications-python-client==4.5.0 # PaaS awscli>=1.11,<1.12 diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index dc96ac91d..eef0c8c36 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -7,17 +7,21 @@ from flask import current_app from app.models import ( Job, Notification, - NOTIFICATION_SENDING, NOTIFICATION_CREATED, + NOTIFICATION_DELIVERED, + NOTIFICATION_TECHNICAL_FAILURE, + NOTIFICATION_SENDING, + NOTIFICATION_STATUS_LETTER_RECEIVED, NOTIFICATION_TECHNICAL_FAILURE ) +from app.dao.notifications_dao import dao_update_notifications_by_reference from app.celery.tasks import ( - update_job_to_sent_to_dvla, + process_updates_from_file, update_dvla_job_to_error, + update_job_to_sent_to_dvla, update_letter_notifications_statuses, - update_letter_notifications_to_sent_to_dvla, update_letter_notifications_to_error, - process_updates_from_file + update_letter_notifications_to_sent_to_dvla ) from tests.app.db import create_notification @@ -91,6 +95,20 @@ def test_update_letter_notifications_statuses_builds_updates_list(notify_api, mo assert updates[1].cost_threshold == 'Sorted' +def test_update_letter_notifications_statuses_persisted(notify_api, mocker, sample_letter_template): + sent_letter = create_notification(sample_letter_template, reference='ref-foo', status=NOTIFICATION_SENDING) + failed_letter = create_notification(sample_letter_template, reference='ref-bar', status=NOTIFICATION_SENDING) + + valid_file = '{}|Sent|1|Unsorted\n{}|Failed|2|Sorted'.format( + sent_letter.reference, failed_letter.reference) + mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) + + update_letter_notifications_statuses(filename='foo.txt') + + assert sent_letter.status == NOTIFICATION_DELIVERED + assert failed_letter.status == NOTIFICATION_TECHNICAL_FAILURE + + def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notification_references( client, sample_letter_template diff --git a/tests/app/dao/test_service_sms_sender_dao.py b/tests/app/dao/test_service_sms_sender_dao.py index 3dcec1469..c38ebe2bc 100644 --- a/tests/app/dao/test_service_sms_sender_dao.py +++ b/tests/app/dao/test_service_sms_sender_dao.py @@ -6,9 +6,12 @@ from sqlalchemy.exc import SQLAlchemyError from app.dao.service_sms_sender_dao import ( insert_or_update_service_sms_sender, dao_add_sms_sender_for_service, - dao_update_service_sms_sender, dao_get_service_sms_senders_by_id, dao_get_sms_senders_by_service_id) + dao_update_service_sms_sender, + dao_get_service_sms_senders_by_id, + dao_get_sms_senders_by_service_id, + update_existing_sms_sender_with_inbound_number) from app.models import ServiceSmsSender -from tests.app.db import create_service +from tests.app.db import create_service, create_inbound_number def test_update_service_sms_sender_updates_existing_row(notify_db_session): @@ -145,3 +148,27 @@ def test_dao_update_service_sms_sender_raises_exception_when_no_default_after_up service_sms_sender_id=sms_sender.id, is_default=False, sms_sender="updated") + + +def test_update_existing_sms_sender_with_inbound_number(notify_db_session): + service = create_service(sms_sender='testing') + inbound_number = create_inbound_number(number='12345', service_id=service.id) + + existing_sms_sender = ServiceSmsSender.query.filter_by(service_id=service.id).one() + sms_sender = update_existing_sms_sender_with_inbound_number( + service_sms_sender=existing_sms_sender, sms_sender=inbound_number.number, inbound_number_id=inbound_number.id) + + assert sms_sender.inbound_number_id == inbound_number.id + assert sms_sender.sms_sender == inbound_number.number + assert sms_sender.is_default + + +def test_update_existing_sms_sender_with_inbound_number_raises_exception_if_inbound_number_does_not_exist( + notify_db_session +): + service = create_service(sms_sender='testing') + existing_sms_sender = ServiceSmsSender.query.filter_by(service_id=service.id).one() + with pytest.raises(expected_exception=SQLAlchemyError): + update_existing_sms_sender_with_inbound_number(service_sms_sender=existing_sms_sender, + sms_sender='blah', + inbound_number_id=uuid.uuid4()) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 37b5f2ffb..6733c5846 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -12,6 +12,7 @@ import app from app import mmg_client, firetext_client from app.dao import (provider_details_dao, notifications_dao) from app.dao.provider_details_dao import dao_switch_sms_provider_to_provider_with_identifier +from app.dao.service_sms_sender_dao import dao_add_sms_sender_for_service from app.delivery import send_to_providers from app.models import ( Notification, @@ -699,14 +700,18 @@ def test_should_handle_sms_sender_and_prefix_message( ) -def test_should_use_inbound_number_as_sender_if_set( - sample_service, - mocker +def test_should_use_inbound_number_as_sender_if_default_sms_sender( + notify_db_session, + mocker ): - sample_service.sms_sender = 'test sender' - template = create_template(sample_service, content='bar') + service = create_service(sms_sender='test sender') + inbound_number = create_inbound_number('1') + dao_add_sms_sender_for_service(service_id=service.id, + sms_sender=inbound_number.number, + is_default=True, + inbound_number_id=inbound_number.id) + template = create_template(service, content='bar') notification = create_notification(template) - inbound_number = create_inbound_number('1', service_id=sample_service.id) mocker.patch('app.mmg_client.send_sms') mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') @@ -721,6 +726,32 @@ def test_should_use_inbound_number_as_sender_if_set( ) +def test_should_use_default_sms_sender( + notify_db_session, + mocker +): + service = create_service(sms_sender='test sender') + inbound_number = create_inbound_number('1') + dao_add_sms_sender_for_service(service_id=service.id, + sms_sender=inbound_number.number, + is_default=False, + inbound_number_id=inbound_number.id) + template = create_template(service, content='bar') + notification = create_notification(template) + + 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, sample_email_template, diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 1a5ec4e99..e317f7a24 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2622,12 +2622,13 @@ def test_add_service_sms_sender_can_add_multiple_senders(client, notify_db_sessi assert len(senders) == 2 -def test_add_service_sms_sender_when_it_is_an_inbound_number(client, notify_db_session): - service = create_service() +def test_add_service_sms_sender_when_it_is_an_inbound_number_updates_the_only_existing_sms_sender( + client, notify_db_session): + service = create_service(sms_sender='GOVUK') inbound_number = create_inbound_number(number='12345') data = { "sms_sender": str(inbound_number.id), - "is_default": False, + "is_default": True, "inbound_number_id": str(inbound_number.id) } response = client.post('/service/{}/sms-sender'.format(service.id), @@ -2640,7 +2641,36 @@ def test_add_service_sms_sender_when_it_is_an_inbound_number(client, notify_db_s resp_json = json.loads(response.get_data(as_text=True)) assert resp_json['sms_sender'] == inbound_number.number assert resp_json['inbound_number_id'] == str(inbound_number.id) - assert not resp_json['is_default'] + assert resp_json['is_default'] + + senders = ServiceSmsSender.query.all() + assert len(senders) == 1 + + +def test_add_service_sms_sender_when_it_is_an_inbound_number_inserts_new_sms_sender_when_more_than_one( + client, notify_db_session): + service = create_service(sms_sender='GOVUK') + create_service_sms_sender(service=service, sms_sender="second", is_default=False) + inbound_number = create_inbound_number(number='12345') + data = { + "sms_sender": str(inbound_number.id), + "is_default": True, + "inbound_number_id": str(inbound_number.id) + } + response = client.post('/service/{}/sms-sender'.format(service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 201 + updated_number = InboundNumber.query.get(inbound_number.id) + assert updated_number.service_id == service.id + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['sms_sender'] == inbound_number.number + assert resp_json['inbound_number_id'] == str(inbound_number.id) + assert resp_json['is_default'] + + senders = ServiceSmsSender.query.filter_by(service_id=service.id).all() + assert len(senders) == 3 def test_add_service_sms_sender_switches_default(client, notify_db_session): diff --git a/tests/app/test_model.py b/tests/app/test_model.py index a1bc1e343..8809a51bb 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -10,12 +10,14 @@ from app.models import ( MOBILE_TYPE, EMAIL_TYPE, NOTIFICATION_CREATED, + NOTIFICATION_DELIVERED, NOTIFICATION_SENDING, NOTIFICATION_PENDING, NOTIFICATION_FAILED, - NOTIFICATION_TECHNICAL_FAILURE, + NOTIFICATION_STATUS_LETTER_ACCEPTED, + NOTIFICATION_STATUS_LETTER_RECEIVED, NOTIFICATION_STATUS_TYPES_FAILED, - NOTIFICATION_STATUS_LETTER_ACCEPTED + NOTIFICATION_TECHNICAL_FAILURE ) from tests.app.conftest import ( sample_template as create_sample_template, @@ -71,6 +73,7 @@ def test_should_not_build_service_whitelist_from_invalid_contact(recipient_type, ([NOTIFICATION_FAILED], NOTIFICATION_STATUS_TYPES_FAILED), ([NOTIFICATION_CREATED], [NOTIFICATION_CREATED]), ([NOTIFICATION_TECHNICAL_FAILURE], [NOTIFICATION_TECHNICAL_FAILURE]), + (NOTIFICATION_STATUS_LETTER_RECEIVED, NOTIFICATION_DELIVERED), # passing in lists containing multiple statuses ([NOTIFICATION_FAILED, NOTIFICATION_CREATED], NOTIFICATION_STATUS_TYPES_FAILED + [NOTIFICATION_CREATED]), ([NOTIFICATION_CREATED, NOTIFICATION_PENDING], [NOTIFICATION_CREATED, NOTIFICATION_PENDING]), @@ -132,7 +135,8 @@ def test_notification_for_csv_returns_correct_job_row_number(notify_db, notify_d ('sms', 'sent', 'Sent internationally'), ('letter', 'created', 'Accepted'), ('letter', 'sending', 'Accepted'), - ('letter', 'technical-failure', 'Technical failure') + ('letter', 'technical-failure', 'Technical failure'), + ('letter', 'delivered', 'Received') ]) def test_notification_for_csv_returns_formatted_status( notify_db, @@ -252,17 +256,11 @@ def test_inbound_number_returns_inbound_number(client, notify_db_session): assert service.get_inbound_number() == inbound_number.number -def test_inbound_number_returns_sms_sender(client, notify_db_session): - service = create_service(sms_sender='testing') - - assert service.get_inbound_number() == service.sms_sender - - -def test_inbound_number_returns_from_number_config(client, notify_db_session): +def test_inbound_number_returns_none_when_no_inbound_number(client, notify_db_session): with set_config(client.application, 'FROM_NUMBER', 'test'): service = create_service(sms_sender=None) - assert service.get_inbound_number() == 'test' + assert not service.get_inbound_number() def test_service_get_default_reply_to_email_address(sample_service): diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 926c8c1d8..d4306ba93 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -393,7 +393,7 @@ def test_get_all_notifications_filter_by_status_invalid_status(client, sample_no assert json_response['status_code'] == 400 assert len(json_response['errors']) == 1 assert json_response['errors'][0]['message'] == "status elephant is not one of [created, sending, sent, " \ - "delivered, pending, failed, technical-failure, temporary-failure, permanent-failure, accepted]" + "delivered, pending, failed, technical-failure, temporary-failure, permanent-failure, accepted, received]" def test_get_all_notifications_filter_by_multiple_statuses(client, sample_template): @@ -583,14 +583,25 @@ def test_get_all_notifications_renames_letter_statuses( pytest.fail() -def test_get_notifications_renames_letter_statuses(client, sample_letter_notification): - auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) +@pytest.mark.parametrize('db_status,expected_status', [ + ('created', 'accepted'), + ('sending', 'accepted'), + ('delivered', 'received'), + ('pending', 'pending'), + ('technical-failure', 'technical-failure') +]) +def test_get_notifications_renames_letter_statuses(client, sample_letter_template, db_status, expected_status): + letter_noti = create_notification( + sample_letter_template, + status=db_status, + personalisation={'address_line_1': 'Mr Foo', 'address_line_2': '1 Bar Street', 'postcode': 'N1'} + ) + auth_header = create_authorization_header(service_id=letter_noti.service_id) response = client.get( - path=url_for('v2_notifications.get_notification_by_id', id=sample_letter_notification.id), + path=url_for('v2_notifications.get_notification_by_id', id=letter_noti.id), headers=[('Content-Type', 'application/json'), auth_header] ) json_response = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 - - assert json_response['status'] == 'accepted' + assert json_response['status'] == expected_status diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index c667def11..6b51b195d 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -26,7 +26,7 @@ def test_get_notifications_request_invalid_statuses( ): partial_error_status = "is not one of " \ "[created, sending, sent, delivered, pending, failed, " \ - "technical-failure, temporary-failure, permanent-failure, accepted]" + "technical-failure, temporary-failure, permanent-failure, accepted, received]" with pytest.raises(ValidationError) as e: validate({'status': invalid_statuses + valid_statuses}, get_notifications_request) @@ -73,7 +73,7 @@ def test_get_notifications_request_invalid_statuses_and_template_types(): error_messages = [error['message'] for error in errors] for invalid_status in ["elephant", "giraffe"]: assert "status {} is not one of [created, sending, sent, delivered, " \ - "pending, failed, technical-failure, temporary-failure, permanent-failure, accepted]".format( + "pending, failed, technical-failure, temporary-failure, permanent-failure, accepted, received]".format( invalid_status ) in error_messages diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 94fea27e5..9d63c6bfb 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -8,7 +8,6 @@ from app.models import ( ScheduledNotification, SCHEDULE_NOTIFICATIONS, EMAIL_TYPE, - INTERNATIONAL_SMS_TYPE, SMS_TYPE ) from flask import json, current_app @@ -25,7 +24,11 @@ from tests.app.conftest import ( sample_template_without_sms_permission ) -from tests.app.db import create_inbound_number, create_service, create_template, create_reply_to_email +from tests.app.db import ( + create_service, + create_template, + create_reply_to_email +) @pytest.mark.parametrize("reference", [None, "reference_from_client"]) @@ -64,15 +67,16 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol assert mocked.called -def test_post_sms_notification_uses_inbound_number_as_sender(client, sample_template_with_placeholders, mocker): +def test_post_sms_notification_uses_inbound_number_as_sender(client, notify_db_session, mocker): + service = create_service(sms_sender='1', do_create_inbound_number=True) + template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon") mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') data = { 'phone_number': '+447700900855', - 'template_id': str(sample_template_with_placeholders.id), + 'template_id': str(template.id), 'personalisation': {' Name': 'Jo'} } - inbound_number = create_inbound_number('1', service_id=sample_template_with_placeholders.service_id) - auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) + auth_header = create_authorization_header(service_id=service.id) response = client.post( path='/v2/notifications/sms', @@ -85,7 +89,8 @@ def test_post_sms_notification_uses_inbound_number_as_sender(client, sample_temp assert len(notifications) == 1 notification_id = notifications[0].id assert resp_json['id'] == str(notification_id) - assert resp_json['content']['from_number'] == inbound_number.number + assert resp_json['content']['from_number'] == '1' + mocked.assert_called_once_with([str(notification_id)], queue='send-sms-tasks') @pytest.mark.parametrize("notification_type, key_send_to, send_to",