darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-02-05 10:42:41 -05:00
Code Issues Packages Projects Releases Wiki Activity

Only send to the provider if the notification has a created status.

Browse Source 
If the notification ends up in the retry queue and the delivery app is restarted the notification will get sent twice.
This is because when the app is restarted another message will be in the retry queue as message available which is a
duplicate of the one in the queue that is a  message in flight.

This should complete https://www.pivotaltracker.com/story/show/121844427
This commit is contained in:
Rebecca Law
2016-06-27 14:47:20 +01:00
parent 8f19ad19f8
commit 8a0211b3eb
6 changed files with 69 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

87
app/celery/provider_tasks.py
View File

@@ -54,52 +54,53 @@ def send_sms_to_provider(self, service_id, notification_id, encrypted_notificati
service = dao_fetch_service_by_id(service_id)
provider = provider_to_use('sms', notification_id)
notification = get_notification_by_id(notification_id)
if notification.status == 'created':
template = Template(
dao_get_template_by_id(notification.template_id, notification.template_version).__dict__,
values={} if not notification.personalisation else notification.personalisation,
prefix=service.name
)
template = Template(
dao_get_template_by_id(notification.template_id, notification.template_version).__dict__,
values={} if not notification.personalisation else notification.personalisation,
prefix=service.name
)
try:
if service.research_mode:
send_sms_response.apply_async(
(provider.get_name(), str(notification_id), notification.to), queue='research-mode'
)
else:
provider.send_sms(
to=validate_and_format_phone_number(notification.to),
content=template.replaced,
reference=str(notification_id)
)
update_provider_stats(
notification_id,
'sms',
provider.get_name(),
content_char_count=template.replaced_content_count
)
notification.sent_at = datetime.utcnow()
notification.sent_by = provider.get_name(),
notification.content_char_count = template.replaced_content_count
notification.status = 'sending'
dao_update_notification(notification)
except SmsClientException as e:
try:
current_app.logger.error(
"SMS notification {} failed".format(notification_id)
)
current_app.logger.exception(e)
self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries))
except self.MaxRetriesExceededError:
update_notification_status_by_id(notification.id, 'technical-failure', 'failure')
if service.research_mode:
send_sms_response.apply_async(
(provider.get_name(), str(notification_id), notification.to), queue='research-mode'
)
else:
provider.send_sms(
to=validate_and_format_phone_number(notification.to),
content=template.replaced,
reference=str(notification_id)
)
current_app.logger.info(
"SMS {} created at {} sent at {}".format(notification_id, notification.created_at, notification.sent_at)
)
statsd_client.incr("notifications.tasks.send-sms-to-provider")
statsd_client.timing("notifications.tasks.send-sms-to-provider.task-time", monotonic() - task_start)
statsd_client.timing("notifications.sms.total-time", datetime.utcnow() - notification.created_at)
update_provider_stats(
notification_id,
'sms',
provider.get_name(),
content_char_count=template.replaced_content_count
)
notification.sent_at = datetime.utcnow()
notification.sent_by = provider.get_name(),
notification.content_char_count = template.replaced_content_count
notification.status = 'sending'
dao_update_notification(notification)
except SmsClientException as e:
try:
current_app.logger.error(
"SMS notification {} failed".format(notification_id)
)
current_app.logger.exception(e)
self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries))
except self.MaxRetriesExceededError:
update_notification_status_by_id(notification.id, 'technical-failure', 'failure')
current_app.logger.info(
"SMS {} created at {} sent at {}".format(notification_id, notification.created_at, notification.sent_at)
)
statsd_client.incr("notifications.tasks.send-sms-to-provider")
statsd_client.timing("notifications.tasks.send-sms-to-provider.task-time", monotonic() - task_start)
statsd_client.timing("notifications.sms.total-time", datetime.utcnow() - notification.created_at)
def provider_to_use(notification_type, notification_id):

4
tests/app/celery/test_provider_tasks.py
View File

@@ -116,7 +116,8 @@ def test_send_sms_should_use_template_version_from_notification_not_latest(
sample_template,
mocker):
db_notification = sample_notification(notify_db, notify_db_session,
template=sample_template, to_field='+447234123123')
template=sample_template, to_field='+447234123123',
status='created')
mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.mmg_client.get_name', return_value="mmg")
@@ -146,6 +147,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest(
assert persisted_notification.template_version == version_on_notification
assert persisted_notification.template_version != sample_template.version
assert persisted_notification.content_char_count == len("Sample service: This is a template")
assert persisted_notification.status == 'sending'
assert not persisted_notification.personalisation

2
tests/app/conftest.py
View File

@@ -316,7 +316,7 @@ def sample_notification(notify_db,
job=None,
job_row_number=None,
to_field=None,
status='sending',
status='created',
reference=None,
created_at=datetime.utcnow(),
content_char_count=160,

2
tests/app/dao/test_notification_dao.py
View File

@@ -678,7 +678,7 @@ def test_get_all_notifications_for_job_by_status(notify_db, notify_db_session, s
def test_update_notification(sample_notification, sample_template):
assert sample_notification.status == 'sending'
assert sample_notification.status == 'created'
sample_notification.status = 'failed'
dao_update_notification(sample_notification)
notification_from_db = Notification.query.get(sample_notification.id)

23
tests/app/notifications/test_rest.py
View File

@@ -23,7 +23,7 @@ def test_get_notification_by_id(notify_api, sample_notification):
headers=[auth_header])
notification = json.loads(response.get_data(as_text=True))['data']['notification']
assert notification['status'] == 'sending'
assert notification['status'] == 'created'
assert notification['template'] == {
'id': str(sample_notification.template.id),
'name': sample_notification.template.name,
@@ -63,7 +63,7 @@ def test_get_all_notifications(notify_api, sample_notification):
headers=[auth_header])
notifications = json.loads(response.get_data(as_text=True))
assert notifications['notifications'][0]['status'] == 'sending'
assert notifications['notifications'][0]['status'] == 'created'
assert notifications['notifications'][0]['template'] == {
'id': str(sample_notification.template.id),
'name': sample_notification.template.name,
@@ -459,7 +459,8 @@ def test_filter_by_multiple_statuss(notify_api,
notify_db,
notify_db_session,
service=sample_email_template.service,
template=sample_email_template)
template=sample_email_template,
status='sending')
auth_header = create_authorization_header(service_id=sample_email_template.service_id)
@@ -645,7 +646,7 @@ def test_firetext_callback_should_update_notification_status(notify_api, sample_
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr')
original = get_notification_by_id(sample_notification.id)
assert original.status == 'sending'
assert original.status == 'created'
response = client.post(
path='/notifications/sms/firetext',
@@ -674,7 +675,7 @@ def test_firetext_callback_should_update_notification_status_failed(notify_api,
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr')
original = get_notification_by_id(sample_notification.id)
assert original.status == 'sending'
assert original.status == 'created'
response = client.post(
path='/notifications/sms/firetext',
@@ -966,7 +967,8 @@ def test_ses_callback_should_update_notification_status(
notify_db,
notify_db_session,
template=sample_email_template,
reference='ref'
reference='ref',
status='sending'
)
assert get_notification_by_id(notification.id).status == 'sending'
@@ -1056,7 +1058,8 @@ def test_ses_callback_should_update_record_statsd(
notify_db,
notify_db_session,
template=sample_email_template,
reference='ref'
reference='ref',
status='sending'
)
assert get_notification_by_id(notification.id).status == 'sending'
@@ -1079,7 +1082,8 @@ def test_ses_callback_should_set_status_to_temporary_failure(notify_api,
notify_db,
notify_db_session,
template=sample_email_template,
reference='ref'
reference='ref',
status='sending'
)
assert get_notification_by_id(notification.id).status == 'sending'
@@ -1138,7 +1142,8 @@ def test_ses_callback_should_set_status_to_permanent_failure(notify_api,
notify_db,
notify_db_session,
template=sample_email_template,
reference='ref'
reference='ref',
status='sending'
)
assert get_notification_by_id(notification.id).status == 'sending'

9
tests/app/status/test_delivery_status.py
View File

@@ -5,6 +5,8 @@ from datetime import (
timedelta
)
from tests.app.conftest import sample_notification
def test_get_delivery_status_all_ok(notify_api, notify_db):
with notify_api.test_request_context():
@@ -17,11 +19,12 @@ def test_get_delivery_status_all_ok(notify_api, notify_db):
assert resp_json['message'] == '0 notifications in sending state over 5 minutes'
def test_get_delivery_status_with_undelivered_notification(notify_api, notify_db, sample_notification):
def test_get_delivery_status_with_undelivered_notification(notify_api, notify_db, notify_db_session):
notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, status='sending')
more_than_five_mins_ago = datetime.utcnow() - timedelta(minutes=10)
sample_notification.created_at = more_than_five_mins_ago
notify_db.session.add(sample_notification)
notification.created_at = more_than_five_mins_ago
notify_db.session.add(notification)
notify_db.session.commit()
with notify_api.test_request_context():