diff --git a/app/celery/callback_tasks.py b/app/celery/process_ses_receipts_tasks.py similarity index 100% rename from app/celery/callback_tasks.py rename to app/celery/process_ses_receipts_tasks.py diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index b37062826..509ee1681 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -5,7 +5,7 @@ from requests import request, RequestException, HTTPError from app.models import SMS_TYPE from app.config import QueueNames -from app.celery.callback_tasks import process_ses_results +from app.celery.process_ses_receipts_tasks import process_ses_results temp_fail = "7700900003" perm_fail = "7700900002" diff --git a/app/celery/service_callback_tasks.py b/app/celery/service_callback_tasks.py new file mode 100644 index 000000000..3bf033a80 --- /dev/null +++ b/app/celery/service_callback_tasks.py @@ -0,0 +1,71 @@ +import json +from app import ( + DATETIME_FORMAT, + notify_celery, +) +from app.dao.notifications_dao import ( + get_notification_by_id, +) + +from app.statsd_decorators import statsd +from app.dao.service_callback_api_dao import get_service_callback_api_for_service +from requests import ( + HTTPError, + request, + RequestException +) +from flask import current_app +from app.config import QueueNames + + +@notify_celery.task(bind=True, name="send-delivery-status", max_retries=5, default_retry_delay=300) +@statsd(namespace="tasks") +def send_delivery_status_to_service(self, notification_id): + # TODO: do we need to do rate limit this? + notification = get_notification_by_id(notification_id) + service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) + if not service_callback_api: + # No delivery receipt API info set + return + + data = { + "id": str(notification_id), + "reference": str(notification.client_reference), + "to": notification.to, + "status": notification.status, + "created_at": notification.created_at.strftime(DATETIME_FORMAT), # the time GOV.UK email sent the request + "updated_at": notification.updated_at.strftime(DATETIME_FORMAT), # the last time the status was updated + "sent_at": notification.sent_at.strftime(DATETIME_FORMAT), # the time the email was sent + "notification_type": notification.notification_type + } + + try: + response = request( + method="POST", + url=service_callback_api.url, + data=json.dumps(data), + headers={ + 'Content-Type': 'application/json', + 'Authorization': 'Bearer {}'.format(service_callback_api.bearer_token) + }, + timeout=60 + ) + current_app.logger.info('send_delivery_status_to_service sending {} to {}, response {}'.format( + notification_id, + service_callback_api.url, + response.status_code + )) + response.raise_for_status() + except RequestException as e: + current_app.logger.warning( + "send_delivery_status_to_service request failed for service_id: {} and url: {}. exc: {}".format( + notification_id, + service_callback_api.url, + e + ) + ) + if not isinstance(e, HTTPError) or e.response.status_code >= 500: + try: + self.retry(queue=QueueNames.RETRY) + except self.MaxRetriesExceededError: + current_app.logger.exception('Retry: send_delivery_status_to_service has retried the max num of times') diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 250f786eb..53d714213 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -29,6 +29,7 @@ from app import ( ) from app.aws import s3 from app.celery import provider_tasks +from app.celery.service_callback_tasks import send_delivery_status_to_service from app.config import QueueNames from app.dao.inbound_sms_dao import dao_get_inbound_sms_by_id from app.dao.jobs_dao import ( @@ -42,12 +43,14 @@ from app.dao.notifications_dao import ( get_notification_by_id, dao_update_notifications_for_job_to_sent_to_dvla, dao_update_notifications_by_reference, - dao_get_last_notification_added_for_job_id + dao_get_last_notification_added_for_job_id, + dao_get_notifications_by_references ) from app.dao.provider_details_dao import get_current_provider from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count from app.dao.templates_dao import dao_get_template_by_id +from app.dao.service_callback_api_dao import get_service_callback_api_for_service from app.models import ( DVLA_RESPONSE_STATUS_SENT, EMAIL_TYPE, @@ -391,6 +394,12 @@ def update_letter_notifications_to_error(self, notification_references): ) current_app.logger.info("Updated {} letter notifications to technical-failure".format(updated_count)) + notifications = dao_get_notifications_by_references(references=notification_references) + # queue callback task only if the service_callback_api exists + service_callback_api = get_service_callback_api_for_service(service_id=notifications[0].service_id) + if service_callback_api: + for notification in notifications: + send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.NOTIFY) def create_dvla_file_contents_for_job(job_id): @@ -455,7 +464,7 @@ def update_letter_notifications_statuses(self, filename): for update in notification_updates: status = NOTIFICATION_DELIVERED if update.status == DVLA_RESPONSE_STATUS_SENT \ else NOTIFICATION_TECHNICAL_FAILURE - notification = dao_update_notifications_by_reference( + updated_count = dao_update_notifications_by_reference( references=[update.reference], update_dict={"status": status, "billable_units": update.page_count, @@ -463,7 +472,7 @@ def update_letter_notifications_statuses(self, filename): } ) - if not notification: + if not updated_count: 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) @@ -472,6 +481,12 @@ def update_letter_notifications_statuses(self, filename): current_app.logger.info( 'DVLA file: {filename}, notification updated to {status}: {reference}'.format( filename=filename, status=status, reference=str(update.reference))) + notifications = dao_get_notifications_by_references(references=[update.reference]) + # queue callback task only if the service_callback_api exists + service_callback_api = get_service_callback_api_for_service(service_id=notifications[0].service_id) + if service_callback_api: + for notification in notifications: + send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.NOTIFY) def process_updates_from_file(response_file): diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 2eb713699..1f7b33167 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -459,6 +459,13 @@ def dao_get_notifications_by_to_field(service_id, search_term, statuses=None): return results +@statsd(namespace="dao") +def dao_get_notifications_by_references(references): + return Notification.query.filter( + Notification.reference.in_(references) + ).all() + + @statsd(namespace="dao") def dao_created_scheduled_notification(scheduled_notification): db.session.add(scheduled_notification) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 8753b2dc4..0b5f3e245 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -172,6 +172,7 @@ def dao_create_service(service, user, service_id=None, service_permissions=None) service.id = service_id or uuid.uuid4() # must be set now so version history model can use same id service.active = True service.research_mode = False + service.crown = service.organisation_type == 'central' for permission in service_permissions: service_permission = ServicePermission(service_id=service.id, permission=permission) diff --git a/app/models.py b/app/models.py index 9d81073c7..fdd00fa9a 100644 --- a/app/models.py +++ b/app/models.py @@ -249,6 +249,7 @@ class Service(db.Model, Versioned): db.String(255), nullable=True, ) + crown = db.Column(db.Boolean, index=False, nullable=False, default=True) association_proxy('permissions', 'service_permission_types') diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index b35704304..28fa96f2e 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -10,8 +10,11 @@ from app.clients.email.aws_ses import get_aws_responses from app.dao import ( notifications_dao ) +from app.dao.service_callback_api_dao import get_service_callback_api_for_service from app.celery.statistics_tasks import create_outcome_notification_statistic_tasks from app.notifications.process_client_response import validate_callback_data +from app.celery.service_callback_tasks import send_delivery_status_to_service +from app.config import QueueNames def process_ses_response(ses_request): @@ -75,7 +78,7 @@ def process_ses_response(ses_request): ) create_outcome_notification_statistic_tasks(notification) - + _check_and_queue_callback_task(notification.id, notification.service_id) return except KeyError: @@ -90,3 +93,10 @@ def process_ses_response(ses_request): def remove_emails_from_bounce(bounce_dict): for recip in bounce_dict['bouncedRecipients']: recip.pop('emailAddress') + + +def _check_and_queue_callback_task(notification_id, service_id): + # queue callback task only if the service_callback_api exists + service_callback_api = get_service_callback_api_for_service(service_id=service_id) + if service_callback_api: + send_delivery_status_to_service.apply_async([str(notification_id)], queue=QueueNames.NOTIFY) diff --git a/app/notifications/process_client_response.py b/app/notifications/process_client_response.py index 534408ce6..ed825766a 100644 --- a/app/notifications/process_client_response.py +++ b/app/notifications/process_client_response.py @@ -8,6 +8,9 @@ from app.dao import notifications_dao from app.clients.sms.firetext import get_firetext_responses from app.clients.sms.mmg import get_mmg_responses from app.celery.statistics_tasks import create_outcome_notification_statistic_tasks +from app.celery.service_callback_tasks import send_delivery_status_to_service +from app.config import QueueNames +from app.dao.service_callback_api_dao import get_service_callback_api_for_service sms_response_mapper = { @@ -82,6 +85,11 @@ def process_sms_client_response(status, reference, client_name): ) create_outcome_notification_statistic_tasks(notification) + # queue callback task only if the service_callback_api exists + service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) + + if service_callback_api: + send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.NOTIFY) success = "{} callback succeeded. reference {} updated".format(client_name, reference) return success, errors diff --git a/app/service/rest.py b/app/service/rest.py index efb7044a3..c4d0e2bb9 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -178,7 +178,11 @@ def update_service(service_id): service_going_live = fetched_service.restricted and not req_json.get('restricted', True) current_data = dict(service_schema.dump(fetched_service).data.items()) current_data.update(request.get_json()) + update_dict = service_schema.load(current_data).data + org_type = req_json.get('organisation_type', None) + if org_type: + update_dict.crown = org_type == 'central' dao_update_service(update_dict) # bridging code between frontend is deployed and data has not been migrated yet. Can only update current year diff --git a/migrations/versions/0149_add_crown_to_services.py b/migrations/versions/0149_add_crown_to_services.py new file mode 100644 index 000000000..bfbbf976c --- /dev/null +++ b/migrations/versions/0149_add_crown_to_services.py @@ -0,0 +1,50 @@ +""" + +Revision ID: 0149_add_crown_column_to_services +Revises: 0148_add_letters_as_pdf_svc_perm +Create Date: 2017-12-04 12:13:35.268712 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0149_add_crown_to_services' +down_revision = '0148_add_letters_as_pdf_svc_perm' + + +def upgrade(): + op.add_column('services', sa.Column('crown', sa.Boolean(), nullable=True)) + op.execute(""" + update services set crown = True + where organisation_type = 'central' + """) + op.execute(""" + update services set crown = True + where organisation_type is null + """) + op.execute(""" + update services set crown = False + where crown is null + """) + op.alter_column('services', 'crown', nullable=False) + + op.add_column('services_history', sa.Column('crown', sa.Boolean(), nullable=True)) + op.execute(""" + update services_history set crown = True + where organisation_type = 'central' + """) + op.execute(""" + update services_history set crown = True + where organisation_type is null + """) + op.execute(""" + update services_history set crown = False + where crown is null + """) + op.alter_column('services_history', 'crown', nullable=False) + + +def downgrade(): + op.drop_column('services', 'crown') + op.drop_column('services_history', 'crown') diff --git a/scripts/run_app_paas.sh b/scripts/run_app_paas.sh index 2813f851f..78610c1bd 100755 --- a/scripts/run_app_paas.sh +++ b/scripts/run_app_paas.sh @@ -16,6 +16,10 @@ function check_params { } function configure_aws_logs { + # create files so that aws logs agent doesn't complain + touch /home/vcap/logs/gunicorn_error.log + touch /home/vcap/logs/app.log.json + aws configure set plugins.cwlogs cwlogs export AWS_ACCESS_KEY_ID=$(echo ${VCAP_SERVICES} | jq -r '.["user-provided"][]|select(.name=="notify-aws")|.credentials.aws_access_key_id') diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index d9b1b22b9..2c6bc4393 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -21,8 +21,9 @@ from app.celery.tasks import ( update_letter_notifications_to_sent_to_dvla ) -from tests.app.db import create_notification +from tests.app.db import create_notification, create_service_callback_api from tests.conftest import set_config +from unittest.mock import call def test_update_job_to_sent_to_dvla(sample_letter_template, sample_letter_job): @@ -97,11 +98,15 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp billable_units=0) failed_letter = create_notification(sample_letter_template, reference='ref-bar', status=NOTIFICATION_SENDING, billable_units=0) - + create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com") 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) + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + update_letter_notifications_statuses(filename='foo.txt') assert sent_letter.status == NOTIFICATION_DELIVERED @@ -111,6 +116,25 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp assert failed_letter.billable_units == 2 assert failed_letter.updated_at + calls = [call([str(failed_letter.id)], queue="notify-internal-tasks"), + call([str(sent_letter.id)], queue="notify-internal-tasks")] + send_mock.assert_has_calls(calls, any_order=True) + + +def test_update_letter_notifications_does_not_call_send_callback_if_no_db_entry(notify_api, mocker, + sample_letter_template): + sent_letter = create_notification(sample_letter_template, reference='ref-foo', status=NOTIFICATION_SENDING, + billable_units=0) + valid_file = '{}|Sent|1|Unsorted\n'.format(sent_letter.reference) + mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) + + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + + update_letter_notifications_statuses(filename='foo.txt') + send_mock.assert_not_called() + def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notification_references( client, @@ -132,11 +156,15 @@ def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notificati def test_update_letter_notifications_to_error_updates_based_on_notification_references( client, - sample_letter_template + sample_letter_template, + mocker ): + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) first = create_notification(sample_letter_template, reference='first ref') second = create_notification(sample_letter_template, reference='second ref') - + create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com") dt = datetime.utcnow() with freeze_time(dt): update_letter_notifications_to_error([first.reference]) @@ -146,3 +174,4 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe assert first.sent_at is None assert first.updated_at == dt assert second.status == NOTIFICATION_CREATED + assert send_mock.called diff --git a/tests/app/celery/test_callback_tasks.py b/tests/app/celery/test_process_ses_receipts_tasks.py similarity index 91% rename from tests/app/celery/test_callback_tasks.py rename to tests/app/celery/test_process_ses_receipts_tasks.py index 4b5ce4547..ddca32e0e 100644 --- a/tests/app/celery/test_callback_tasks.py +++ b/tests/app/celery/test_process_ses_receipts_tasks.py @@ -1,7 +1,7 @@ import json from datetime import datetime -from app.celery.callback_tasks import process_ses_results +from app.celery.process_ses_receipts_tasks import process_ses_results from tests.app.db import create_notification @@ -18,7 +18,7 @@ def test_process_ses_results(sample_email_template): def test_process_ses_results_does_not_retry_if_errors(notify_db, mocker): - mocked = mocker.patch('app.celery.callback_tasks.process_ses_results.retry') + mocked = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry') response = json.loads(ses_notification_callback()) process_ses_results(response=response) assert mocked.call_count == 0 @@ -26,7 +26,7 @@ def test_process_ses_results_does_not_retry_if_errors(notify_db, mocker): def test_process_ses_results_retry_called(notify_db, mocker): mocker.patch("app.dao.notifications_dao.update_notification_status_by_reference", side_effect=Exception("EXPECTED")) - mocked = mocker.patch('app.celery.callback_tasks.process_ses_results.retry') + mocked = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry') response = json.loads(ses_notification_callback()) process_ses_results(response=response) assert mocked.call_count != 0 diff --git a/tests/app/celery/test_service_callback_tasks.py b/tests/app/celery/test_service_callback_tasks.py new file mode 100644 index 000000000..f09333d2d --- /dev/null +++ b/tests/app/celery/test_service_callback_tasks.py @@ -0,0 +1,185 @@ +import json +from datetime import datetime + +import pytest +import requests_mock + +from requests import RequestException + +from app import (DATETIME_FORMAT) + +from tests.app.conftest import ( + sample_service as create_sample_service, + sample_template as create_sample_template, +) +from tests.app.db import ( + create_notification, + create_user, + create_service_callback_api +) +from app.celery.service_callback_tasks import send_delivery_status_to_service + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_post_https_request_to_service(notify_db, + notify_db_session, + notification_type): + user = create_user() + service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True) + + callback_api = create_service_callback_api(service=service, url="https://some.service.gov.uk/", + bearer_token="something_unique") + template = create_sample_template( + notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello' + ) + + datestr = datetime(2017, 6, 20) + + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + + with requests_mock.Mocker() as request_mock: + request_mock.post(callback_api.url, + json={}, + status_code=200) + send_delivery_status_to_service(notification.id) + + mock_data = { + "id": str(notification.id), + "reference": str(notification.client_reference), + "to": notification.to, + "status": notification.status, + "created_at": datestr.strftime(DATETIME_FORMAT), # the time GOV.UK email sent the request + "updated_at": datestr.strftime(DATETIME_FORMAT), # the last time the status was updated + "sent_at": datestr.strftime(DATETIME_FORMAT), # the time the email was sent + "notification_type": notification_type + } + + assert request_mock.call_count == 1 + assert request_mock.request_history[0].url == callback_api.url + assert request_mock.request_history[0].method == 'POST' + assert request_mock.request_history[0].text == json.dumps(mock_data) + assert request_mock.request_history[0].headers["Content-type"] == "application/json" + assert request_mock.request_history[0].headers["Authorization"] == "Bearer {}".format(callback_api.bearer_token) + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_does_not_sent_request_when_service_callback_api_does_not_exist( + notify_db, notify_db_session, mocker, notification_type): + service = create_sample_service(notify_db, notify_db_session, restricted=True) + + template = create_sample_template( + notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello' + ) + datestr = datetime(2017, 6, 20) + + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + mocked = mocker.patch("requests.request") + send_delivery_status_to_service(notification.id) + + mocked.call_count == 0 + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_retries_if_request_returns_500(notify_db, + notify_db_session, + mocker, + notification_type): + 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=notification_type, subject_line='Hello' + ) + callback_api = create_service_callback_api(service=service, url="https://some.service.gov.uk/", + bearer_token="something_unique") + datestr = datetime(2017, 6, 20) + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') + with requests_mock.Mocker() as request_mock: + request_mock.post(callback_api.url, + json={}, + status_code=500) + send_delivery_status_to_service(notification.id) + + assert mocked.call_count == 1 + assert mocked.call_args[1]['queue'] == 'retry-tasks' + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_retries_if_request_throws_unknown(notify_db, + notify_db_session, + mocker, + notification_type): + 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=notification_type, subject_line='Hello' + ) + create_service_callback_api(service=service, url="https://some.service.gov.uk/", + bearer_token="something_unique") + datestr = datetime(2017, 6, 20) + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + + mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') + mocker.patch("app.celery.tasks.request", side_effect=RequestException()) + + send_delivery_status_to_service(notification.id) + + assert mocked.call_count == 1 + assert mocked.call_args[1]['queue'] == 'retry-tasks' + + +@pytest.mark.parametrize("notification_type", + ["email", "letter", "sms"]) +def test_send_delivery_status_to_service_does_not_retries_if_request_returns_404(notify_db, + notify_db_session, + mocker, + notification_type): + 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=notification_type, subject_line='Hello' + ) + callback_api = create_service_callback_api(service=service, url="https://some.service.gov.uk/", + bearer_token="something_unique") + datestr = datetime(2017, 6, 20) + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status='sent' + ) + mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') + with requests_mock.Mocker() as request_mock: + request_mock.post(callback_api.url, + json={}, + status_code=404) + send_delivery_status_to_service(notification.id) + + mocked.call_count == 0 diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 6d48acf56..e33956a56 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -29,7 +29,8 @@ from app.celery.tasks import ( process_incomplete_jobs, get_template_class, s3, - send_inbound_sms_to_service) + send_inbound_sms_to_service, +) from app.config import QueueNames from app.dao import jobs_dao, services_dao from app.models import ( @@ -64,7 +65,7 @@ from tests.app.db import ( create_template, create_user, create_reply_to_email, - create_service_with_defined_sms_sender + create_service_with_defined_sms_sender, ) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index c11d03268..0bb9b5322 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -29,7 +29,8 @@ from app.dao.notifications_dao import ( is_delivery_slow_for_provider, set_scheduled_notification_to_processed, update_notification_status_by_id, - update_notification_status_by_reference + update_notification_status_by_reference, + dao_get_notifications_by_references ) from app.dao.services_dao import dao_update_service from app.models import ( @@ -1989,3 +1990,14 @@ def test_dao_update_notifications_by_reference_returns_zero_when_no_notification "billable_units": 2} ) assert updated_count == 0 + + +def test_dao_get_notifications_by_reference(sample_template): + 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') + + notifications = dao_get_notifications_by_references(['ref']) + assert len(notifications) == 2 + assert notifications[0].id in [notification_1.id, notification_2.id] + assert notifications[1].id in [notification_1.id, notification_2.id] diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index d601bd59d..2caf092fe 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -87,6 +87,7 @@ def test_create_service(sample_user): email_from="email_from", message_limit=1000, restricted=False, + organisation_type='central', created_by=sample_user) dao_create_service(service, sample_user) assert Service.query.count() == 1 @@ -96,10 +97,14 @@ def test_create_service(sample_user): assert service_db.id == service.id assert service_db.branding == BRANDING_GOVUK assert service_db.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT + assert service_db.email_from == 'email_from' assert service_db.research_mode is False assert service_db.prefix_sms is True assert service.active is True assert sample_user in service_db.users + assert service_db.free_sms_fragment_limit == 250000 + assert service_db.organisation_type == 'central' + assert service_db.crown is True def test_cannot_create_two_services_with_same_name(sample_user): diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index 73d33c7a8..0e224871b 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -10,6 +10,7 @@ from app.dao.notifications_dao import ( get_notification_by_id ) from tests.app.conftest import sample_notification as create_sample_notification +from tests.app.db import create_service_callback_api def firetext_post(client, data): @@ -18,7 +19,7 @@ def firetext_post(client, data): data=data, headers=[ ('Content-Type', 'application/x-www-form-urlencoded'), - ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') + ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') # fake IPs ]) @@ -28,7 +29,7 @@ def mmg_post(client, data): data=data, headers=[ ('Content-Type', 'application/json'), - ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') + ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') # fake IPs ]) @@ -178,7 +179,9 @@ def test_firetext_callback_should_update_notification_status( notify_db, notify_db_session, client, sample_email_template, mocker ): mocker.patch('app.statsd_client.incr') - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -202,13 +205,16 @@ def test_firetext_callback_should_update_notification_status( updated = get_notification_by_id(notification.id) assert updated.status == 'delivered' assert get_notification_by_id(notification.id).status == 'delivered' + assert send_mock.called_once_with([notification.id], queue="notify-internal-tasks") def test_firetext_callback_should_update_notification_status_failed( notify_db, notify_db_session, client, sample_template, mocker ): mocker.patch('app.statsd_client.incr') - + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -235,6 +241,9 @@ def test_firetext_callback_should_update_notification_status_failed( def test_firetext_callback_should_update_notification_status_pending(client, notify_db, notify_db_session, mocker): mocker.patch('app.statsd_client.incr') + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -265,8 +274,11 @@ def test_process_mmg_response_return_200_when_cid_is_send_sms_code(client): def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -286,8 +298,11 @@ def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id( def test_process_mmg_response_status_5_updates_notification_with_permanently_failed( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -306,8 +321,11 @@ def test_process_mmg_response_status_5_updates_notification_with_permanently_fai def test_process_mmg_response_status_2_updates_notification_with_permanently_failed( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -325,8 +343,11 @@ def test_process_mmg_response_status_2_updates_notification_with_permanently_fai def test_process_mmg_response_status_4_updates_notification_with_temporary_failed( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -345,8 +366,11 @@ def test_process_mmg_response_status_4_updates_notification_with_temporary_faile def test_process_mmg_response_unknown_status_updates_notification_with_failed( - notify_db, notify_db_session, client + notify_db, notify_db_session, client, mocker ): + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -354,13 +378,14 @@ def test_process_mmg_response_unknown_status_updates_notification_with_failed( "CID": str(notification.id), "MSISDN": "447777349060", "status": 10}) - + create_service_callback_api(service=notification.service, url="https://original_url.com") response = mmg_post(client, data) assert response.status_code == 200 json_data = json.loads(response.data) assert json_data['result'] == 'success' assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(notification.id) assert get_notification_by_id(notification.id).status == 'failed' + assert send_mock.called def test_process_mmg_response_returns_400_for_malformed_data(client): @@ -392,6 +417,9 @@ def test_process_mmg_response_records_statsd(notify_db, notify_db_session, clien mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing_with_dates') + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) @@ -415,6 +443,9 @@ def test_firetext_callback_should_record_statsd(client, notify_db, notify_db_ses mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing_with_dates') + mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow() ) diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 330466234..49ce90829 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -10,6 +10,7 @@ from app.notifications.notifications_ses_callback import process_ses_response, r from app.celery.research_mode_tasks import ses_hard_bounce_callback, ses_soft_bounce_callback, ses_notification_callback from tests.app.conftest import sample_notification as create_sample_notification +from tests.app.db import create_service_callback_api def test_ses_callback_should_update_notification_status( @@ -24,7 +25,42 @@ def test_ses_callback_should_update_notification_status( stats_mock = mocker.patch( 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' ) + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + notification = create_sample_notification( + notify_db, + notify_db_session, + template=sample_email_template, + reference='ref', + status='sending', + sent_at=datetime.utcnow() + ) + create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") + assert get_notification_by_id(notification.id).status == 'sending' + errors = process_ses_response(ses_notification_callback(reference='ref')) + assert errors is None + assert get_notification_by_id(notification.id).status == 'delivered' + statsd_client.timing_with_dates.assert_any_call( + "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at + ) + statsd_client.incr.assert_any_call("callback.ses.delivered") + stats_mock.assert_called_once_with(notification) + send_mock.assert_called_once_with([str(notification.id)], queue="notify-internal-tasks") + + +def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry( + client, + notify_db, + notify_db_session, + sample_email_template, + mocker): + with freeze_time('2001-01-01T12:00:00'): + + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -39,11 +75,8 @@ def test_ses_callback_should_update_notification_status( errors = process_ses_response(ses_notification_callback(reference='ref')) assert errors is None assert get_notification_by_id(notification.id).status == 'delivered' - statsd_client.timing_with_dates.assert_any_call( - "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at - ) - statsd_client.incr.assert_any_call("callback.ses.delivered") - stats_mock.assert_called_once_with(notification) + + send_mock.assert_not_called() def test_ses_callback_should_update_multiple_notification_status_sent( @@ -56,7 +89,9 @@ def test_ses_callback_should_update_multiple_notification_status_sent( stats_mock = mocker.patch( 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' ) - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification1 = create_sample_notification( notify_db, notify_db_session, @@ -80,7 +115,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent( reference='ref3', sent_at=datetime.utcnow(), status='sending') - + create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") assert process_ses_response(ses_notification_callback(reference='ref1')) is None assert process_ses_response(ses_notification_callback(reference='ref2')) is None assert process_ses_response(ses_notification_callback(reference='ref3')) is None @@ -90,6 +125,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent( call(notification2), call(notification3) ]) + assert send_mock.called def test_ses_callback_should_set_status_to_temporary_failure(client, @@ -101,7 +137,9 @@ def test_ses_callback_should_set_status_to_temporary_failure(client, stats_mock = mocker.patch( 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' ) - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -110,9 +148,11 @@ def test_ses_callback_should_set_status_to_temporary_failure(client, status='sending', sent_at=datetime.utcnow() ) + create_service_callback_api(service=notification.service, url="https://original_url.com") assert get_notification_by_id(notification.id).status == 'sending' assert process_ses_response(ses_soft_bounce_callback(reference='ref')) is None assert get_notification_by_id(notification.id).status == 'temporary-failure' + assert send_mock.called stats_mock.assert_called_once_with(notification) @@ -146,7 +186,9 @@ def test_ses_callback_should_set_status_to_permanent_failure(client, stats_mock = mocker.patch( 'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks' ) - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) notification = create_sample_notification( notify_db, notify_db_session, @@ -155,10 +197,12 @@ def test_ses_callback_should_set_status_to_permanent_failure(client, status='sending', sent_at=datetime.utcnow() ) + create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") assert get_notification_by_id(notification.id).status == 'sending' assert process_ses_response(ses_hard_bounce_callback(reference='ref')) is None assert get_notification_by_id(notification.id).status == 'permanent-failure' + assert send_mock.called stats_mock.assert_called_once_with(notification) diff --git a/tests/app/notifications/test_process_client_response.py b/tests/app/notifications/test_process_client_response.py index a4e3d2413..40e2a608c 100644 --- a/tests/app/notifications/test_process_client_response.py +++ b/tests/app/notifications/test_process_client_response.py @@ -4,6 +4,7 @@ from app.notifications.process_client_response import ( validate_callback_data, process_sms_client_response ) +from tests.app.db import create_service_callback_api def test_validate_callback_data_returns_none_when_valid(): @@ -51,15 +52,32 @@ def test_outcome_statistics_called_for_successful_callback(sample_notification, 'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id', return_value=sample_notification ) - + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + create_service_callback_api(service=sample_notification.service, url="https://original_url.com") reference = str(uuid.uuid4()) success, error = process_sms_client_response(status='3', reference=reference, client_name='MMG') assert success == "MMG callback succeeded. reference {} updated".format(str(reference)) assert error is None + send_mock.assert_called_once_with([str(sample_notification.id)], queue="notify-internal-tasks") stats_mock.assert_called_once_with(sample_notification) +def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notification, mocker): + mocker.patch( + 'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id', + return_value=sample_notification + ) + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + reference = str(uuid.uuid4()) + process_sms_client_response(status='3', reference=reference, client_name='MMG') + send_mock.assert_not_called() + + def test_process_sms_response_return_success_for_send_sms_code_reference(mocker): stats_mock = mocker.patch('app.notifications.process_client_response.create_outcome_notification_statistic_tasks') diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 2c09939b0..cc8f13eaa 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -536,6 +536,26 @@ def test_update_service_flags(client, sample_service): assert set(result['data']['permissions']) == set([LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) +@pytest.mark.parametrize("org_type, expected", + [("central", True), + ('local', False), + ("nhs", False)]) +def test_update_service_sets_crown(client, sample_service, org_type, expected): + data = { + 'organisation_type': org_type, + } + auth_header = create_authorization_header() + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 200 + assert result['data']['crown'] is expected + + @pytest.fixture(scope='function') def service_with_no_permissions(notify_db, notify_db_session): return create_service(service_permissions=[])