Delete functions which call the job statistics tasks

The JobStatistics table is going to be deleted. There are currently
3 tasks which use the JobStatistics model via the Statistics DAO, so we
need to make sure that these tasks aren't being used before they are
deleted in a separate PR.

This commit deletes:
* The `create_initial_notification_statistic_tasks` function which gets
used to call the `record_initial_job_statistics` task.
* The `create_outcome_notification_statistic_tasks` function which gets
used to call the `record_outcome_job_statistics` task.
* And the scheduling of the `timeout-job-statistics` scheduled task.
This commit is contained in:
Katie Smith
2018-03-01 16:52:52 +00:00
parent 0c1d5c12a3
commit 7f2e9f507e
9 changed files with 4 additions and 276 deletions

View File

@@ -10,20 +10,9 @@ from app.dao.statistics_dao import (
update_job_stats_outcome_count update_job_stats_outcome_count
) )
from app.dao.notifications_dao import get_notification_by_id from app.dao.notifications_dao import get_notification_by_id
from app.models import NOTIFICATION_STATUS_TYPES_COMPLETED
from app.config import QueueNames from app.config import QueueNames
def create_initial_notification_statistic_tasks(notification):
if notification.job_id and notification.status:
record_initial_job_statistics.apply_async((str(notification.id),), queue=QueueNames.STATISTICS)
def create_outcome_notification_statistic_tasks(notification):
if notification.job_id and notification.status in NOTIFICATION_STATUS_TYPES_COMPLETED:
record_outcome_job_statistics.apply_async((str(notification.id),), queue=QueueNames.STATISTICS)
@worker_process_shutdown.connect @worker_process_shutdown.connect
def worker_process_shutdown(sender, signal, pid, exitcode): def worker_process_shutdown(sender, signal, pid, exitcode):
current_app.logger.info('Statistics worker shutdown: PID: {} Exitcode: {}'.format(pid, exitcode)) current_app.logger.info('Statistics worker shutdown: PID: {} Exitcode: {}'.format(pid, exitcode))

View File

@@ -231,11 +231,6 @@ class Config(object):
'schedule': crontab(hour=4, minute=40), 'schedule': crontab(hour=4, minute=40),
'options': {'queue': QueueNames.PERIODIC} 'options': {'queue': QueueNames.PERIODIC}
}, },
'timeout-job-statistics': {
'task': 'timeout-job-statistics',
'schedule': crontab(hour=5, minute=0),
'options': {'queue': QueueNames.PERIODIC}
},
'populate_monthly_billing': { 'populate_monthly_billing': {
'task': 'populate_monthly_billing', 'task': 'populate_monthly_billing',
'schedule': crontab(hour=5, minute=10), 'schedule': crontab(hour=5, minute=10),

View File

@@ -31,7 +31,6 @@ from app.models import (
NOTIFICATION_SENT, NOTIFICATION_SENT,
NOTIFICATION_SENDING NOTIFICATION_SENDING
) )
from app.celery.statistics_tasks import create_initial_notification_statistic_tasks
def send_sms_to_provider(notification): def send_sms_to_provider(notification):
@@ -83,8 +82,6 @@ def send_sms_to_provider(notification):
notification.billable_units = template.fragment_count notification.billable_units = template.fragment_count
update_notification(notification, provider, notification.international) update_notification(notification, provider, notification.international)
create_initial_notification_statistic_tasks(notification)
current_app.logger.debug( current_app.logger.debug(
"SMS {} sent to provider {} at {}".format(notification.id, provider.get_name(), notification.sent_at) "SMS {} sent to provider {} at {}".format(notification.id, provider.get_name(), notification.sent_at)
) )
@@ -138,8 +135,6 @@ def send_email_to_provider(notification):
notification.reference = reference notification.reference = reference
update_notification(notification, provider) update_notification(notification, provider)
create_initial_notification_statistic_tasks(notification)
current_app.logger.debug( current_app.logger.debug(
"Email {} sent to provider at {}".format(notification.id, notification.sent_at) "Email {} sent to provider at {}".format(notification.id, notification.sent_at)
) )

View File

@@ -11,7 +11,6 @@ from app.dao import (
notifications_dao notifications_dao
) )
from app.dao.service_callback_api_dao import get_service_callback_api_for_service 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.notifications.process_client_response import validate_callback_data
from app.celery.service_callback_tasks import send_delivery_status_to_service from app.celery.service_callback_tasks import send_delivery_status_to_service
from app.config import QueueNames from app.config import QueueNames
@@ -77,7 +76,6 @@ def process_ses_response(ses_request):
notification.sent_at notification.sent_at
) )
create_outcome_notification_statistic_tasks(notification)
_check_and_queue_callback_task(notification.id, notification.service_id) _check_and_queue_callback_task(notification.id, notification.service_id)
return return

View File

@@ -8,7 +8,6 @@ from app.clients import ClientException
from app.dao import notifications_dao from app.dao import notifications_dao
from app.clients.sms.firetext import get_firetext_responses from app.clients.sms.firetext import get_firetext_responses
from app.clients.sms.mmg import get_mmg_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.celery.service_callback_tasks import send_delivery_status_to_service
from app.config import QueueNames from app.config import QueueNames
from app.dao.service_callback_api_dao import get_service_callback_api_for_service from app.dao.service_callback_api_dao import get_service_callback_api_for_service
@@ -80,7 +79,6 @@ def _process_for_status(notification_status, client_name, reference):
notification.sent_at notification.sent_at
) )
create_outcome_notification_statistic_tasks(notification)
# queue callback task only if the service_callback_api exists # queue callback task only if the service_callback_api exists
service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id)

View File

@@ -1,169 +0,0 @@
import pytest
from app.celery.statistics_tasks import (
record_initial_job_statistics,
record_outcome_job_statistics,
create_initial_notification_statistic_tasks,
create_outcome_notification_statistic_tasks)
from sqlalchemy.exc import SQLAlchemyError
from app import create_uuid
from tests.app.conftest import sample_notification
from app.models import (
NOTIFICATION_STATUS_TYPES_COMPLETED,
NOTIFICATION_SENDING,
NOTIFICATION_PENDING,
NOTIFICATION_CREATED,
NOTIFICATION_DELIVERED,
)
def test_should_create_initial_job_task_if_notification_is_related_to_a_job(
notify_db, notify_db_session, sample_job, mocker
):
mock = mocker.patch("app.celery.statistics_tasks.record_initial_job_statistics.apply_async")
notification = sample_notification(notify_db, notify_db_session, job=sample_job)
create_initial_notification_statistic_tasks(notification)
mock.assert_called_once_with((str(notification.id), ), queue="statistics-tasks")
@pytest.mark.parametrize('status', [
NOTIFICATION_SENDING, NOTIFICATION_CREATED, NOTIFICATION_PENDING
])
def test_should_create_intial_job_task_if_notification_is_not_in_completed_state(
notify_db, notify_db_session, sample_job, mocker, status
):
mock = mocker.patch("app.celery.statistics_tasks.record_initial_job_statistics.apply_async")
notification = sample_notification(notify_db, notify_db_session, job=sample_job, status=status)
create_initial_notification_statistic_tasks(notification)
mock.assert_called_once_with((str(notification.id), ), queue="statistics-tasks")
def test_should_not_create_initial_job_task_if_notification_is_not_related_to_a_job(
notify_db, notify_db_session, mocker
):
notification = sample_notification(notify_db, notify_db_session, status=NOTIFICATION_CREATED)
mock = mocker.patch("app.celery.statistics_tasks.record_initial_job_statistics.apply_async")
create_initial_notification_statistic_tasks(notification)
mock.assert_not_called()
def test_should_create_outcome_job_task_if_notification_is_related_to_a_job(
notify_db, notify_db_session, sample_job, mocker
):
mock = mocker.patch("app.celery.statistics_tasks.record_outcome_job_statistics.apply_async")
notification = sample_notification(notify_db, notify_db_session, job=sample_job, status=NOTIFICATION_DELIVERED)
create_outcome_notification_statistic_tasks(notification)
mock.assert_called_once_with((str(notification.id), ), queue="statistics-tasks")
@pytest.mark.parametrize('status', NOTIFICATION_STATUS_TYPES_COMPLETED)
def test_should_create_outcome_job_task_if_notification_is_in_completed_state(
notify_db, notify_db_session, sample_job, mocker, status
):
mock = mocker.patch("app.celery.statistics_tasks.record_outcome_job_statistics.apply_async")
notification = sample_notification(notify_db, notify_db_session, job=sample_job, status=status)
create_outcome_notification_statistic_tasks(notification)
mock.assert_called_once_with((str(notification.id), ), queue="statistics-tasks")
@pytest.mark.parametrize('status', [
NOTIFICATION_SENDING, NOTIFICATION_CREATED, NOTIFICATION_PENDING
])
def test_should_not_create_outcome_job_task_if_notification_is_not_in_completed_state_already(
notify_db, notify_db_session, sample_job, mocker, status
):
mock = mocker.patch("app.celery.statistics_tasks.record_initial_job_statistics.apply_async")
notification = sample_notification(notify_db, notify_db_session, job=sample_job, status=status)
create_outcome_notification_statistic_tasks(notification)
mock.assert_not_called()
def test_should_not_create_outcome_job_task_if_notification_is_not_related_to_a_job(
notify_db, notify_db_session, sample_notification, mocker
):
mock = mocker.patch("app.celery.statistics_tasks.record_outcome_job_statistics.apply_async")
create_outcome_notification_statistic_tasks(sample_notification)
mock.assert_not_called()
def test_should_call_create_job_stats_dao_methods(notify_db, notify_db_session, sample_notification, mocker):
dao_mock = mocker.patch("app.celery.statistics_tasks.create_or_update_job_sending_statistics")
record_initial_job_statistics(str(sample_notification.id))
dao_mock.assert_called_once_with(sample_notification)
def test_should_retry_if_persisting_the_job_stats_has_a_sql_alchemy_exception(
notify_db,
notify_db_session,
sample_notification,
mocker):
dao_mock = mocker.patch(
"app.celery.statistics_tasks.create_or_update_job_sending_statistics",
side_effect=SQLAlchemyError()
)
retry_mock = mocker.patch('app.celery.statistics_tasks.record_initial_job_statistics.retry')
record_initial_job_statistics(str(sample_notification.id))
dao_mock.assert_called_once_with(sample_notification)
retry_mock.assert_called_with(queue="retry-tasks")
def test_should_call_update_job_stats_dao_outcome_methods(notify_db, notify_db_session, sample_notification, mocker):
dao_mock = mocker.patch("app.celery.statistics_tasks.update_job_stats_outcome_count")
record_outcome_job_statistics(str(sample_notification.id))
dao_mock.assert_called_once_with(sample_notification)
def test_should_retry_if_persisting_the_job_outcome_stats_has_a_sql_alchemy_exception(
notify_db,
notify_db_session,
sample_notification,
mocker):
dao_mock = mocker.patch(
"app.celery.statistics_tasks.update_job_stats_outcome_count",
side_effect=SQLAlchemyError()
)
retry_mock = mocker.patch('app.celery.statistics_tasks.record_outcome_job_statistics.retry')
record_outcome_job_statistics(str(sample_notification.id))
dao_mock.assert_called_once_with(sample_notification)
retry_mock.assert_called_with(queue="retry-tasks")
def test_should_retry_if_persisting_the_job_outcome_stats_updates_zero_rows(
notify_db,
notify_db_session,
sample_notification,
mocker):
dao_mock = mocker.patch("app.celery.statistics_tasks.update_job_stats_outcome_count", return_value=0)
retry_mock = mocker.patch('app.celery.statistics_tasks.record_outcome_job_statistics.retry')
record_outcome_job_statistics(str(sample_notification.id))
dao_mock.assert_called_once_with(sample_notification)
retry_mock.assert_called_with(queue="retry-tasks")
def test_should_retry_if_persisting_the_job_stats_creation_cant_find_notification_by_id(
notify_db,
notify_db_session,
mocker):
dao_mock = mocker.patch("app.celery.statistics_tasks.create_or_update_job_sending_statistics")
retry_mock = mocker.patch('app.celery.statistics_tasks.record_initial_job_statistics.retry')
record_initial_job_statistics(str(create_uuid()))
dao_mock.assert_not_called()
retry_mock.assert_called_with(queue="retry-tasks")
def test_should_retry_if_persisting_the_job_stats_outcome_cant_find_notification_by_id(
notify_db,
notify_db_session,
mocker):
dao_mock = mocker.patch("app.celery.statistics_tasks.update_job_stats_outcome_count")
retry_mock = mocker.patch('app.celery.statistics_tasks.record_outcome_job_statistics.retry')
record_outcome_job_statistics(str(create_uuid()))
dao_mock.assert_not_called()
retry_mock.assert_called_with(queue="retry-tasks")

View File

@@ -1,7 +1,7 @@
import uuid import uuid
from collections import namedtuple from collections import namedtuple
from datetime import datetime from datetime import datetime
from unittest.mock import ANY, call from unittest.mock import ANY
import pytest import pytest
from flask import current_app from flask import current_app
@@ -75,7 +75,6 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender()) reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender())
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_sms_to_provider( send_to_providers.send_sms_to_provider(
db_notification db_notification
@@ -88,8 +87,6 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
sender=current_app.config['FROM_NUMBER'] sender=current_app.config['FROM_NUMBER']
) )
stats_mock.assert_called_once_with(db_notification)
notification = Notification.query.filter_by(id=db_notification.id).one() notification = Notification.query.filter_by(id=db_notification.id).one()
assert notification.status == 'sending' assert notification.status == 'sending'
@@ -110,7 +107,6 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist
) )
mocker.patch('app.aws_ses_client.send_email', return_value='reference') mocker.patch('app.aws_ses_client.send_email', return_value='reference')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_email_to_provider( send_to_providers.send_email_to_provider(
db_notification db_notification
@@ -124,7 +120,6 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist
html_body=ANY, html_body=ANY,
reply_to_address=None reply_to_address=None
) )
stats_mock.assert_called_once_with(db_notification)
assert '<!DOCTYPE html' in app.aws_ses_client.send_email.call_args[1]['html_body'] assert '<!DOCTYPE html' in app.aws_ses_client.send_email.call_args[1]['html_body']
assert '&lt;em&gt;some HTML&lt;/em&gt;' in app.aws_ses_client.send_email.call_args[1]['html_body'] assert '&lt;em&gt;some HTML&lt;/em&gt;' in app.aws_ses_client.send_email.call_args[1]['html_body']
@@ -141,11 +136,9 @@ def test_should_not_send_email_message_when_service_is_inactive_notifcation_is_i
): ):
sample_service.active = False sample_service.active = False
send_mock = mocker.patch("app.aws_ses_client.send_email", return_value='reference') send_mock = mocker.patch("app.aws_ses_client.send_email", return_value='reference')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_email_to_provider(sample_notification) send_to_providers.send_email_to_provider(sample_notification)
send_mock.assert_not_called() send_mock.assert_not_called()
stats_mock.assert_not_called()
assert Notification.query.get(sample_notification.id).status == 'technical-failure' assert Notification.query.get(sample_notification.id).status == 'technical-failure'
@@ -154,11 +147,9 @@ def test_should_not_send_sms_message_when_service_is_inactive_notifcation_is_in_
sample_service, sample_notification, mocker, client_send): sample_service, sample_notification, mocker, client_send):
sample_service.active = False sample_service.active = False
send_mock = mocker.patch(client_send, return_value='reference') send_mock = mocker.patch(client_send, return_value='reference')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_sms_to_provider(sample_notification) send_to_providers.send_sms_to_provider(sample_notification)
send_mock.assert_not_called() send_mock.assert_not_called()
stats_mock.assert_not_called()
assert Notification.query.get(sample_notification.id).status == 'technical-failure' assert Notification.query.get(sample_notification.id).status == 'technical-failure'
@@ -169,7 +160,6 @@ def test_send_sms_should_use_template_version_from_notification_not_latest(
reply_to_text=sample_template.service.get_default_sms_sender()) reply_to_text=sample_template.service.get_default_sms_sender())
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
version_on_notification = sample_template.version version_on_notification = sample_template.version
@@ -209,7 +199,6 @@ def test_should_call_send_sms_response_task_if_research_mode(
): ):
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.delivery.send_to_providers.send_sms_response') mocker.patch('app.delivery.send_to_providers.send_sms_response')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
if research_mode: if research_mode:
sample_service.research_mode = True sample_service.research_mode = True
@@ -222,7 +211,6 @@ def test_should_call_send_sms_response_task_if_research_mode(
sample_notification sample_notification
) )
assert not mmg_client.send_sms.called assert not mmg_client.send_sms.called
stats_mock.assert_called_once_with(sample_notification)
app.delivery.send_to_providers.send_sms_response.assert_called_once_with( app.delivery.send_to_providers.send_sms_response.assert_called_once_with(
'mmg', str(sample_notification.id), sample_notification.to 'mmg', str(sample_notification.id), sample_notification.to
@@ -260,7 +248,6 @@ def test_should_set_billable_units_to_zero_in_research_mode_or_test_key(
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.delivery.send_to_providers.send_sms_response') mocker.patch('app.delivery.send_to_providers.send_sms_response')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
if research_mode: if research_mode:
sample_service.research_mode = True sample_service.research_mode = True
@@ -271,7 +258,6 @@ def test_should_set_billable_units_to_zero_in_research_mode_or_test_key(
send_to_providers.send_sms_to_provider( send_to_providers.send_sms_to_provider(
sample_notification sample_notification
) )
stats_mock.assert_called_once_with(sample_notification)
assert notifications_dao.get_notification_by_id(sample_notification.id).billable_units == 0 assert notifications_dao.get_notification_by_id(sample_notification.id).billable_units == 0
@@ -282,7 +268,6 @@ def test_should_not_send_to_provider_when_status_is_not_created(
notification = create_notification(template=sample_template, status='sending') notification = create_notification(template=sample_template, status='sending')
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
response_mock = mocker.patch('app.delivery.send_to_providers.send_sms_response') response_mock = mocker.patch('app.delivery.send_to_providers.send_sms_response')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_sms_to_provider( send_to_providers.send_sms_to_provider(
notification notification
@@ -290,7 +275,6 @@ def test_should_not_send_to_provider_when_status_is_not_created(
app.mmg_client.send_sms.assert_not_called() app.mmg_client.send_sms.assert_not_called()
response_mock.assert_not_called() response_mock.assert_not_called()
stats_mock.assert_not_called()
def test_should_send_sms_with_downgraded_content(notify_db_session, mocker): def test_should_send_sms_with_downgraded_content(notify_db_session, mocker):
@@ -307,7 +291,6 @@ def test_should_send_sms_with_downgraded_content(notify_db_session, mocker):
) )
mocker.patch('app.mmg_client.send_sms') 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(db_notification) send_to_providers.send_sms_to_provider(db_notification)
@@ -324,7 +307,6 @@ def test_send_sms_should_use_service_sms_sender(
sample_template, sample_template,
mocker): mocker):
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
sms_sender = create_service_sms_sender(service=sample_service, sms_sender='123456', is_default=False) sms_sender = create_service_sms_sender(service=sample_service, sms_sender='123456', is_default=False)
db_notification = create_notification(template=sample_template, sms_sender_id=sms_sender.id, db_notification = create_notification(template=sample_template, sms_sender_id=sms_sender.id,
@@ -363,14 +345,12 @@ def test_send_email_to_provider_should_call_research_mode_task_response_task_if_
mocker.patch('app.uuid.uuid4', return_value=reference) mocker.patch('app.uuid.uuid4', return_value=reference)
mocker.patch('app.aws_ses_client.send_email') mocker.patch('app.aws_ses_client.send_email')
mocker.patch('app.delivery.send_to_providers.send_email_response') mocker.patch('app.delivery.send_to_providers.send_email_response')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_email_to_provider( send_to_providers.send_email_to_provider(
notification notification
) )
assert not app.aws_ses_client.send_email.called assert not app.aws_ses_client.send_email.called
stats_mock.assert_called_once_with(notification)
app.delivery.send_to_providers.send_email_response.assert_called_once_with(str(reference), 'john@smith.com') app.delivery.send_to_providers.send_email_response.assert_called_once_with(str(reference), 'john@smith.com')
persisted_notification = Notification.query.filter_by(id=notification.id).one() persisted_notification = Notification.query.filter_by(id=notification.id).one()
assert persisted_notification.to == 'john@smith.com' assert persisted_notification.to == 'john@smith.com'
@@ -390,12 +370,10 @@ def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_c
notification = create_notification(template=sample_email_template, status='sending') notification = create_notification(template=sample_email_template, status='sending')
mocker.patch('app.aws_ses_client.send_email') mocker.patch('app.aws_ses_client.send_email')
mocker.patch('app.delivery.send_to_providers.send_email_response') mocker.patch('app.delivery.send_to_providers.send_email_response')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_sms_to_provider( send_to_providers.send_sms_to_provider(
notification notification
) )
stats_mock.assert_not_called()
app.aws_ses_client.send_email.assert_not_called() app.aws_ses_client.send_email.assert_not_called()
app.delivery.send_to_providers.send_email_response.assert_not_called() app.delivery.send_to_providers.send_email_response.assert_not_called()
@@ -405,7 +383,6 @@ def test_send_email_should_use_service_reply_to_email(
sample_email_template, sample_email_template,
mocker): mocker):
mocker.patch('app.aws_ses_client.send_email', return_value='reference') mocker.patch('app.aws_ses_client.send_email', return_value='reference')
mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
db_notification = create_notification(template=sample_email_template, reply_to_text='foo@bar.com') db_notification = create_notification(template=sample_email_template, reply_to_text='foo@bar.com')
create_reply_to_email(service=sample_service, email_address='foo@bar.com') create_reply_to_email(service=sample_service, email_address='foo@bar.com')
@@ -512,7 +489,6 @@ def test_get_logo_url_works_for_different_environments(base_url, expected_url):
def test_should_not_set_billable_units_if_research_mode(notify_db, sample_service, sample_notification, mocker): def test_should_not_set_billable_units_if_research_mode(notify_db, sample_service, sample_notification, mocker):
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.delivery.send_to_providers.send_sms_response') mocker.patch('app.delivery.send_to_providers.send_sms_response')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
sample_service.research_mode = True sample_service.research_mode = True
notify_db.session.add(sample_service) notify_db.session.add(sample_service)
@@ -521,7 +497,6 @@ def test_should_not_set_billable_units_if_research_mode(notify_db, sample_servic
send_to_providers.send_sms_to_provider( send_to_providers.send_sms_to_provider(
sample_notification sample_notification
) )
stats_mock.assert_called_once_with(sample_notification)
persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id)
assert persisted_notification.billable_units == 0 assert persisted_notification.billable_units == 0
@@ -545,7 +520,6 @@ def test_should_update_billable_units_according_to_research_mode_and_key_type(
): ):
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.delivery.send_to_providers.send_sms_response') mocker.patch('app.delivery.send_to_providers.send_sms_response')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
if research_mode: if research_mode:
sample_service.research_mode = True sample_service.research_mode = True
@@ -557,7 +531,6 @@ def test_should_update_billable_units_according_to_research_mode_and_key_type(
send_to_providers.send_sms_to_provider( send_to_providers.send_sms_to_provider(
sample_notification sample_notification
) )
stats_mock.assert_called_once_with(sample_notification)
assert sample_notification.billable_units == billable_units assert sample_notification.billable_units == billable_units
@@ -591,7 +564,6 @@ def test_should_send_sms_to_international_providers(
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.firetext_client.send_sms') mocker.patch('app.firetext_client.send_sms')
stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_sms_to_provider( send_to_providers.send_sms_to_provider(
db_notification_uk db_notification_uk
@@ -618,11 +590,6 @@ def test_should_send_sms_to_international_providers(
notification_uk = Notification.query.filter_by(id=db_notification_uk.id).one() notification_uk = Notification.query.filter_by(id=db_notification_uk.id).one()
notification_int = Notification.query.filter_by(id=db_notification_international.id).one() notification_int = Notification.query.filter_by(id=db_notification_international.id).one()
stats_mock.assert_has_calls([
call(db_notification_uk),
call(db_notification_international)
])
assert notification_uk.status == 'sending' assert notification_uk.status == 'sending'
assert notification_uk.sent_by == 'firetext' assert notification_uk.sent_by == 'firetext'
assert notification_int.status == 'sent' assert notification_int.status == 'sent'
@@ -642,7 +609,6 @@ def test_should_send_international_sms_with_formatted_phone_number(
send_notification_mock = mocker.patch('app.mmg_client.send_sms') send_notification_mock = mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.delivery.send_to_providers.send_sms_response') mocker.patch('app.delivery.send_to_providers.send_sms_response')
mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_sms_to_provider( send_to_providers.send_sms_to_provider(
notification notification
@@ -664,7 +630,6 @@ def test_should_set_international_phone_number_to_sent_status(
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.delivery.send_to_providers.send_sms_response') mocker.patch('app.delivery.send_to_providers.send_sms_response')
mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_sms_to_provider( send_to_providers.send_sms_to_provider(
notification notification
@@ -691,7 +656,6 @@ def test_should_handle_sms_sender_and_prefix_message(
notify_db_session notify_db_session
): ):
mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.send_sms')
mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
service = create_service_with_defined_sms_sender(sms_sender_value=sms_sender, prefix_sms=prefix_sms) service = create_service_with_defined_sms_sender(sms_sender_value=sms_sender, prefix_sms=prefix_sms)
template = create_template(service, content='bar') template = create_template(service, content='bar')
notification = create_notification(template, reply_to_text=sms_sender) notification = create_notification(template, reply_to_text=sms_sender)
@@ -710,7 +674,6 @@ def test_send_email_to_provider_uses_reply_to_from_notification(
sample_email_template, sample_email_template,
mocker): mocker):
mocker.patch('app.aws_ses_client.send_email', return_value='reference') mocker.patch('app.aws_ses_client.send_email', return_value='reference')
mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com") db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com")
@@ -732,7 +695,6 @@ def test_send_email_to_provider_should_format_reply_to_email_address(
sample_email_template, sample_email_template,
mocker): mocker):
mocker.patch('app.aws_ses_client.send_email', return_value='reference') mocker.patch('app.aws_ses_client.send_email', return_value='reference')
mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com\t") db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com\t")
@@ -753,7 +715,6 @@ def test_send_email_to_provider_should_format_reply_to_email_address(
def test_send_sms_to_provider_should_format_phone_number(sample_notification, mocker): def test_send_sms_to_provider_should_format_phone_number(sample_notification, mocker):
sample_notification.to = '+44 (7123) 123-123' sample_notification.to = '+44 (7123) 123-123'
send_mock = mocker.patch('app.mmg_client.send_sms') send_mock = 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(sample_notification) send_to_providers.send_sms_to_provider(sample_notification)
@@ -763,7 +724,6 @@ def test_send_sms_to_provider_should_format_phone_number(sample_notification, mo
def test_send_email_to_provider_should_format_email_address(sample_email_notification, mocker): def test_send_email_to_provider_should_format_email_address(sample_email_notification, mocker):
sample_email_notification.to = 'test@example.com\t' sample_email_notification.to = 'test@example.com\t'
send_mock = mocker.patch('app.aws_ses_client.send_email', return_value='reference') send_mock = mocker.patch('app.aws_ses_client.send_email', return_value='reference')
mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
send_to_providers.send_email_to_provider(sample_email_notification) send_to_providers.send_email_to_provider(sample_email_notification)

View File

@@ -1,5 +1,4 @@
from datetime import datetime from datetime import datetime
from unittest.mock import call
from flask import json from flask import json
from freezegun import freeze_time from freezegun import freeze_time
@@ -22,9 +21,6 @@ def test_ses_callback_should_update_notification_status(
with freeze_time('2001-01-01T12:00:00'): with freeze_time('2001-01-01T12:00:00'):
mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.incr')
mocker.patch('app.statsd_client.timing_with_dates') mocker.patch('app.statsd_client.timing_with_dates')
stats_mock = mocker.patch(
'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
)
send_mock = mocker.patch( send_mock = mocker.patch(
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
) )
@@ -46,7 +42,6 @@ def test_ses_callback_should_update_notification_status(
"callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
) )
statsd_client.incr.assert_any_call("callback.ses.delivered") 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="service-callbacks") send_mock.assert_called_once_with([str(notification.id)], queue="service-callbacks")
@@ -86,13 +81,10 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
sample_email_template, sample_email_template,
mocker): mocker):
stats_mock = mocker.patch(
'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
)
send_mock = mocker.patch( send_mock = mocker.patch(
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
) )
notification1 = create_sample_notification( create_sample_notification(
notify_db, notify_db,
notify_db_session, notify_db_session,
template=sample_email_template, template=sample_email_template,
@@ -100,7 +92,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
sent_at=datetime.utcnow(), sent_at=datetime.utcnow(),
status='sending') status='sending')
notification2 = create_sample_notification( create_sample_notification(
notify_db, notify_db,
notify_db_session, notify_db_session,
template=sample_email_template, template=sample_email_template,
@@ -108,7 +100,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
sent_at=datetime.utcnow(), sent_at=datetime.utcnow(),
status='sending') status='sending')
notification3 = create_sample_notification( create_sample_notification(
notify_db, notify_db,
notify_db_session, notify_db_session,
template=sample_email_template, template=sample_email_template,
@@ -119,12 +111,6 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
assert process_ses_response(ses_notification_callback(reference='ref1')) is None 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='ref2')) is None
assert process_ses_response(ses_notification_callback(reference='ref3')) is None assert process_ses_response(ses_notification_callback(reference='ref3')) is None
stats_mock.assert_has_calls([
call(notification1),
call(notification2),
call(notification3)
])
assert send_mock.called assert send_mock.called
@@ -133,10 +119,6 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
notify_db_session, notify_db_session,
sample_email_template, sample_email_template,
mocker): mocker):
stats_mock = mocker.patch(
'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
)
send_mock = mocker.patch( send_mock = mocker.patch(
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
) )
@@ -153,7 +135,6 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
assert process_ses_response(ses_soft_bounce_callback(reference='ref')) is None assert process_ses_response(ses_soft_bounce_callback(reference='ref')) is None
assert get_notification_by_id(notification.id).status == 'temporary-failure' assert get_notification_by_id(notification.id).status == 'temporary-failure'
assert send_mock.called assert send_mock.called
stats_mock.assert_called_once_with(notification)
def test_ses_callback_should_not_set_status_once_status_is_delivered(client, def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
@@ -161,10 +142,6 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
notify_db_session, notify_db_session,
sample_email_template, sample_email_template,
mocker): mocker):
stats_mock = mocker.patch(
'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
)
notification = create_sample_notification( notification = create_sample_notification(
notify_db, notify_db,
notify_db_session, notify_db_session,
@@ -175,7 +152,6 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
) )
assert get_notification_by_id(notification.id).status == 'delivered' assert get_notification_by_id(notification.id).status == 'delivered'
stats_mock.assert_not_called()
def test_ses_callback_should_set_status_to_permanent_failure(client, def test_ses_callback_should_set_status_to_permanent_failure(client,
@@ -183,9 +159,6 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
notify_db_session, notify_db_session,
sample_email_template, sample_email_template,
mocker): mocker):
stats_mock = mocker.patch(
'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
)
send_mock = mocker.patch( send_mock = mocker.patch(
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
) )
@@ -203,7 +176,6 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
assert process_ses_response(ses_hard_bounce_callback(reference='ref')) is None assert process_ses_response(ses_hard_bounce_callback(reference='ref')) is None
assert get_notification_by_id(notification.id).status == 'permanent-failure' assert get_notification_by_id(notification.id).status == 'permanent-failure'
assert send_mock.called assert send_mock.called
stats_mock.assert_called_once_with(notification)
def test_remove_emails_from_bounce(): def test_remove_emails_from_bounce():

View File

@@ -50,7 +50,6 @@ def test_validate_callback_data_returns_error_for_empty_string():
def test_outcome_statistics_called_for_successful_callback(sample_notification, mocker): def test_outcome_statistics_called_for_successful_callback(sample_notification, mocker):
stats_mock = mocker.patch('app.notifications.process_client_response.create_outcome_notification_statistic_tasks')
mocker.patch( mocker.patch(
'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id', 'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id',
return_value=sample_notification return_value=sample_notification
@@ -65,7 +64,6 @@ def test_outcome_statistics_called_for_successful_callback(sample_notification,
assert success == "MMG callback succeeded. reference {} updated".format(str(reference)) assert success == "MMG callback succeeded. reference {} updated".format(str(reference))
assert error is None assert error is None
send_mock.assert_called_once_with([str(sample_notification.id)], queue="service-callbacks") send_mock.assert_called_once_with([str(sample_notification.id)], queue="service-callbacks")
stats_mock.assert_called_once_with(sample_notification)
def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notification, mocker): def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notification, mocker):
@@ -82,30 +80,22 @@ def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notificat
def test_process_sms_response_return_success_for_send_sms_code_reference(mocker): 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')
success, error = process_sms_client_response(status='000', reference='send-sms-code', client_name='sms-client') success, error = process_sms_client_response(status='000', reference='send-sms-code', client_name='sms-client')
assert success == "{} callback succeeded: send-sms-code".format('sms-client') assert success == "{} callback succeeded: send-sms-code".format('sms-client')
assert error is None assert error is None
stats_mock.assert_not_called()
def test_process_sms_response_returns_error_bad_reference(mocker): def test_process_sms_response_returns_error_bad_reference(mocker):
stats_mock = mocker.patch('app.notifications.process_client_response.create_outcome_notification_statistic_tasks')
success, error = process_sms_client_response(status='000', reference='something-bad', client_name='sms-client') success, error = process_sms_client_response(status='000', reference='something-bad', client_name='sms-client')
assert success is None assert success is None
assert error == "{} callback with invalid reference {}".format('sms-client', 'something-bad') assert error == "{} callback with invalid reference {}".format('sms-client', 'something-bad')
stats_mock.assert_not_called()
def test_process_sms_response_raises_client_exception_for_unknown_sms_client(mocker): def test_process_sms_response_raises_client_exception_for_unknown_sms_client(mocker):
stats_mock = mocker.patch('app.notifications.process_client_response.create_outcome_notification_statistic_tasks')
success, error = process_sms_client_response(status='000', reference=str(uuid.uuid4()), client_name='sms-client') success, error = process_sms_client_response(status='000', reference=str(uuid.uuid4()), client_name='sms-client')
assert success is None assert success is None
assert error == 'unknown sms client: {}'.format('sms-client') assert error == 'unknown sms client: {}'.format('sms-client')
stats_mock.assert_not_called()
def test_process_sms_response_raises_client_exception_for_unknown_status(mocker): def test_process_sms_response_raises_client_exception_for_unknown_status(mocker):