diff --git a/app/celery/statistics_tasks.py b/app/celery/statistics_tasks.py index 141f829b0..df6774f71 100644 --- a/app/celery/statistics_tasks.py +++ b/app/celery/statistics_tasks.py @@ -10,20 +10,9 @@ from app.dao.statistics_dao import ( update_job_stats_outcome_count ) from app.dao.notifications_dao import get_notification_by_id -from app.models import NOTIFICATION_STATUS_TYPES_COMPLETED 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 def worker_process_shutdown(sender, signal, pid, exitcode): current_app.logger.info('Statistics worker shutdown: PID: {} Exitcode: {}'.format(pid, exitcode)) diff --git a/app/config.py b/app/config.py index 951f5ece3..d27e4fdf3 100644 --- a/app/config.py +++ b/app/config.py @@ -231,11 +231,6 @@ class Config(object): 'schedule': crontab(hour=4, minute=40), 'options': {'queue': QueueNames.PERIODIC} }, - 'timeout-job-statistics': { - 'task': 'timeout-job-statistics', - 'schedule': crontab(hour=5, minute=0), - 'options': {'queue': QueueNames.PERIODIC} - }, 'populate_monthly_billing': { 'task': 'populate_monthly_billing', 'schedule': crontab(hour=5, minute=10), diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index b272f116b..4c1a3600b 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -31,7 +31,6 @@ from app.models import ( NOTIFICATION_SENT, NOTIFICATION_SENDING ) -from app.celery.statistics_tasks import create_initial_notification_statistic_tasks def send_sms_to_provider(notification): @@ -83,8 +82,6 @@ def send_sms_to_provider(notification): notification.billable_units = template.fragment_count update_notification(notification, provider, notification.international) - create_initial_notification_statistic_tasks(notification) - current_app.logger.debug( "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 update_notification(notification, provider) - create_initial_notification_statistic_tasks(notification) - current_app.logger.debug( "Email {} sent to provider at {}".format(notification.id, notification.sent_at) ) diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index cf3da7508..99909272a 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -11,7 +11,6 @@ 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 @@ -77,7 +76,6 @@ def process_ses_response(ses_request): notification.sent_at ) - create_outcome_notification_statistic_tasks(notification) _check_and_queue_callback_task(notification.id, notification.service_id) return diff --git a/app/notifications/process_client_response.py b/app/notifications/process_client_response.py index 19aa2d290..55380b0cc 100644 --- a/app/notifications/process_client_response.py +++ b/app/notifications/process_client_response.py @@ -8,7 +8,6 @@ from app.clients import ClientException 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 @@ -80,7 +79,6 @@ def _process_for_status(notification_status, client_name, reference): notification.sent_at ) - 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) diff --git a/tests/app/celery/test_statistics_tasks.py b/tests/app/celery/test_statistics_tasks.py deleted file mode 100644 index bb6c4c5b6..000000000 --- a/tests/app/celery/test_statistics_tasks.py +++ /dev/null @@ -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") diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 4b973975e..627768220 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -1,7 +1,7 @@ import uuid from collections import namedtuple from datetime import datetime -from unittest.mock import ANY, call +from unittest.mock import ANY import pytest 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()) 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( db_notification @@ -88,8 +87,6 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( sender=current_app.config['FROM_NUMBER'] ) - stats_mock.assert_called_once_with(db_notification) - notification = Notification.query.filter_by(id=db_notification.id).one() 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') - stats_mock = mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks') send_to_providers.send_email_to_provider( db_notification @@ -124,7 +120,6 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist html_body=ANY, reply_to_address=None ) - stats_mock.assert_called_once_with(db_notification) assert '