mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-03 09:51:11 -05:00
- wrap apply_async parameter notification_id in a str() argument
- check if service_callback_api exist before putting tasks on queue - create_service_callback_api in tests before asserting if send_delivery_status_to_service has been called.
This commit is contained in:
@@ -58,7 +58,7 @@ def send_delivery_status_to_service(self, notification_id):
|
||||
response.raise_for_status()
|
||||
except RequestException as e:
|
||||
current_app.logger.warning(
|
||||
"send_inbound_sms_to_service request failed for service_id: {} and url: {}. exc: {}".format(
|
||||
"send_delivery_status_to_service request failed for service_id: {} and url: {}. exc: {}".format(
|
||||
notification_id,
|
||||
service_callback_api.url,
|
||||
e
|
||||
@@ -68,4 +68,4 @@ def send_delivery_status_to_service(self, notification_id):
|
||||
try:
|
||||
self.retry(queue=QueueNames.RETRY)
|
||||
except self.MaxRetriesExceededError:
|
||||
current_app.logger.exception('Retry: send_inbound_sms_to_service has retried the max number of times')
|
||||
current_app.logger.exception('Retry: send_delivery_status_to_service has retried the max number of times')
|
||||
|
||||
@@ -44,12 +44,13 @@ from app.dao.notifications_dao import (
|
||||
dao_update_notifications_for_job_to_sent_to_dvla,
|
||||
dao_update_notifications_by_reference,
|
||||
dao_get_last_notification_added_for_job_id,
|
||||
dao_get_notifications_by_reference
|
||||
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,
|
||||
@@ -393,9 +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_reference(references=notification_references)
|
||||
for notification in notifications:
|
||||
send_delivery_status_to_service.apply_async([notification.id], queue=QueueNames.NOTIFY)
|
||||
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):
|
||||
@@ -477,9 +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_reference(references=[update.reference])
|
||||
for notification in notifications:
|
||||
send_delivery_status_to_service.apply_async([notification.id], queue=QueueNames.NOTIFY)
|
||||
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):
|
||||
|
||||
@@ -460,7 +460,7 @@ def dao_get_notifications_by_to_field(service_id, search_term, statuses=None):
|
||||
|
||||
|
||||
@statsd(namespace="dao")
|
||||
def dao_get_notifications_by_reference(references):
|
||||
def dao_get_notifications_by_references(references):
|
||||
return Notification.query.filter(
|
||||
Notification.reference.in_(references)
|
||||
).all()
|
||||
|
||||
@@ -10,6 +10,7 @@ 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
|
||||
@@ -77,7 +78,7 @@ def process_ses_response(ses_request):
|
||||
)
|
||||
|
||||
create_outcome_notification_statistic_tasks(notification)
|
||||
send_delivery_status_to_service.apply_async([notification.id], queue=QueueNames.NOTIFY)
|
||||
_check_and_queue_callback_task(notification.id, notification.service_id)
|
||||
return
|
||||
|
||||
except KeyError:
|
||||
@@ -92,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)
|
||||
|
||||
@@ -10,6 +10,8 @@ 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 = {
|
||||
'MMG': get_mmg_responses,
|
||||
@@ -83,8 +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)
|
||||
|
||||
send_delivery_status_to_service.apply_async([notification.id], queue=QueueNames.NOTIFY)
|
||||
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
|
||||
|
||||
@@ -21,7 +21,7 @@ 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
|
||||
|
||||
|
||||
@@ -97,10 +97,11 @@ 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'
|
||||
)
|
||||
@@ -144,7 +145,7 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe
|
||||
)
|
||||
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])
|
||||
|
||||
@@ -30,7 +30,7 @@ from app.dao.notifications_dao import (
|
||||
set_scheduled_notification_to_processed,
|
||||
update_notification_status_by_id,
|
||||
update_notification_status_by_reference,
|
||||
dao_get_notifications_by_reference
|
||||
dao_get_notifications_by_references
|
||||
)
|
||||
from app.dao.services_dao import dao_update_service
|
||||
from app.models import (
|
||||
@@ -1997,7 +1997,7 @@ def test_dao_get_notifications_by_reference(sample_template):
|
||||
notification_1 = create_notification(template=sample_template, reference='ref')
|
||||
notification_2 = create_notification(template=sample_template, reference='ref')
|
||||
|
||||
notifications = dao_get_notifications_by_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]
|
||||
|
||||
@@ -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):
|
||||
@@ -377,7 +378,7 @@ 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)
|
||||
|
||||
@@ -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(
|
||||
@@ -35,7 +36,7 @@ def test_ses_callback_should_update_notification_status(
|
||||
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'))
|
||||
@@ -46,7 +47,7 @@ def test_ses_callback_should_update_notification_status(
|
||||
)
|
||||
statsd_client.incr.assert_any_call("callback.ses.delivered")
|
||||
stats_mock.assert_called_once_with(notification)
|
||||
send_mock.assert_called_once_with([notification.id], queue="notify-internal-tasks")
|
||||
send_mock.assert_called_once_with([str(notification.id)], queue="notify-internal-tasks")
|
||||
|
||||
|
||||
def test_ses_callback_should_update_multiple_notification_status_sent(
|
||||
@@ -85,7 +86,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
|
||||
@@ -118,6 +119,7 @@ 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'
|
||||
@@ -166,6 +168,7 @@ 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
|
||||
|
||||
@@ -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():
|
||||
@@ -54,6 +55,7 @@ def test_outcome_statistics_called_for_successful_callback(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')
|
||||
|
||||
Reference in New Issue
Block a user