diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 6d09fa785..cb9a40c88 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -25,8 +25,8 @@ from app.models import ( EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, - KEY_TYPE_NORMAL -) + KEY_TYPE_NORMAL, + JOB_STATUS_CANCELLED, JOB_STATUS_PENDING, JOB_STATUS_IN_PROGRESS, JOB_STATUS_FINISHED) from app.notifications.process_notifications import persist_notification from app.service.utils import service_allowed_to_send_to from app.statsd_decorators import statsd @@ -38,15 +38,22 @@ def process_job(job_id): start = datetime.utcnow() job = dao_get_job_by_id(job_id) - if job.job_status != 'pending': + if job.job_status != JOB_STATUS_PENDING: return service = job.service + if not service.active: + job.job_status = JOB_STATUS_CANCELLED + dao_update_job(job) + current_app.logger.warn( + "Job {} has been cancelled, service {} is inactive".format(job_id, service.id)) + return + if __sending_limits_for_job_exceeded(service, job, job_id): return - job.job_status = 'in progress' + job.job_status = JOB_STATUS_IN_PROGRESS dao_update_job(job) db_template = dao_get_template_by_id(job.template_id, job.template_version) @@ -62,7 +69,7 @@ def process_job(job_id): process_row(row_number, recipient, personalisation, template, job, service) finished = datetime.utcnow() - job.job_status = 'finished' + job.job_status = JOB_STATUS_FINISHED job.processing_started = start job.processing_finished = finished dao_update_job(job) diff --git a/app/dao/permissions_dao.py b/app/dao/permissions_dao.py index adb484ff9..162d08a46 100644 --- a/app/dao/permissions_dao.py +++ b/app/dao/permissions_dao.py @@ -59,7 +59,8 @@ class PermissionDAO(DAOClass): db.session.commit() def get_permissions_by_user_id(self, user_id): - return self.Meta.model.query.filter_by(user_id=user_id).all() + return self.Meta.model.query.filter_by(user_id=user_id)\ + .join(Permission.service).filter_by(active=True).all() permission_dao = PermissionDAO() diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 9af7b3ff7..34242d180 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -923,6 +923,22 @@ def test_persist_letter_saves_letter_to_database(sample_letter_job, mocker): assert notification_db.personalisation == personalisation +def test_should_cancel_job_if_service_is_inactive(sample_service, + sample_job, + mocker): + sample_service.active = False + + mocker.patch('app.celery.tasks.s3.get_job_from_s3') + mocker.patch('app.celery.tasks.process_row') + + process_job(sample_job.id) + + job = jobs_dao.dao_get_job_by_id(sample_job.id) + assert job.job_status == 'cancelled' + s3.get_job_from_s3.assert_not_called() + tasks.process_row.assert_not_called() + + @pytest.mark.parametrize('template_type, expected_class', [ (SMS_TYPE, SMSMessageTemplate), (EMAIL_TYPE, WithSubjectTemplate), diff --git a/tests/app/conftest.py b/tests/app/conftest.py index c63085399..d58cfef70 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -121,7 +121,6 @@ def sample_service(notify_db, notify_db_session, service_name="Sample service", user=None, - active=True, restricted=False, limit=1000, email_from=None): @@ -132,7 +131,6 @@ def sample_service(notify_db, data = { 'name': service_name, 'message_limit': limit, - 'active': active, 'restricted': restricted, 'email_from': email_from, 'created_by': user diff --git a/tests/app/dao/test_permissionDAO.py b/tests/app/dao/test_permissionDAO.py new file mode 100644 index 000000000..33dbc2e5a --- /dev/null +++ b/tests/app/dao/test_permissionDAO.py @@ -0,0 +1,26 @@ +from app.dao.permissions_dao import permission_dao +from tests.app.conftest import sample_service as create_service + + +def test_get_permissions_by_user_id_returns_all_permissions(sample_service): + permissions = permission_dao.get_permissions_by_user_id(user_id=sample_service.users[0].id) + assert len(permissions) == 8 + assert sorted(["manage_users", + "manage_templates", + "manage_settings", + "send_texts", + "send_emails", + "send_letters", + "manage_api_keys", + "view_activity"]) == sorted([i.permission for i in permissions]) + + +def test_get_permissions_by_user_id_returns_only_active_service(notify_db, notify_db_session, sample_user): + active_service = create_service(notify_db, notify_db_session, service_name="Active service", user=sample_user) + inactive_service = create_service(notify_db, notify_db_session, service_name="Inactive service", + user=sample_user) + inactive_service.active = False + permissions = permission_dao.get_permissions_by_user_id(user_id=sample_user.id) + assert len(permissions) == 8 + assert active_service in [i.service for i in permissions] + assert inactive_service not in [i.service for i in permissions]