diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 5614c361f..9c4dbb5c5 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -39,7 +39,7 @@ from app.notifications.process_notifications import send_notification_to_queue from app.statsd_decorators import statsd from app.celery.tasks import process_job from app.config import QueueNames, TaskNames -from app.utils import convert_utc_to_bst +from app.utils import convert_utc_to_bst, get_unrestricted_letter_ids @notify_celery.task(name="remove_csv_files") @@ -308,9 +308,14 @@ def populate_monthly_billing(): @statsd(namespace="tasks") def run_letter_jobs(): job_ids = dao_get_letter_job_ids_by_status(JOB_STATUS_READY_TO_SEND) - notify_celery.send_task( - name=TaskNames.DVLA_FILES, - args=(job_ids,), - queue=QueueNames.PROCESS_FTP - ) - current_app.logger.info("Queued {} ready letter job ids onto {}".format(len(job_ids), QueueNames.PROCESS_FTP)) + + unrestricted_job_ids = get_unrestricted_letter_ids(job_ids) + + if unrestricted_job_ids: + notify_celery.send_task( + name=TaskNames.DVLA_FILES, + args=(unrestricted_job_ids,), + queue=QueueNames.PROCESS_FTP + ) + current_app.logger.info( + "Queued {} ready letter job ids onto {}".format(len(unrestricted_job_ids), QueueNames.PROCESS_FTP)) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index ed7811ddb..a5e9f98d6 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -75,7 +75,7 @@ def process_job(job_id): db_template = dao_get_template_by_id(job.template_id, job.template_version) - if db_template.template_type == LETTER_TYPE and service.restricted: + if db_template.template_type == LETTER_TYPE and job.service.restricted: job.job_status = JOB_STATUS_ERROR dao_update_job(job) current_app.logger.warn( @@ -97,9 +97,9 @@ def process_job(job_id): update_job_to_sent_to_dvla.apply_async([str(job.id)], queue=QueueNames.RESEARCH_MODE) else: build_dvla_file.apply_async([str(job.id)], queue=QueueNames.JOBS) - # temporary logging - current_app.logger.info("send job {} to build-dvla-file in the {} queue".format( - job_id, QueueNames.JOBS if not service.research_mode else QueueNames.RESEARCH_MODE)) + # temporary logging + current_app.logger.info("send job {} to build-dvla-file in the {} queue".format( + job_id, QueueNames.JOBS)) else: job.job_status = JOB_STATUS_FINISHED diff --git a/app/letters/rest.py b/app/letters/rest.py index b9ce7bf35..458611a80 100644 --- a/app/letters/rest.py +++ b/app/letters/rest.py @@ -8,6 +8,7 @@ from app.schemas import job_schema from app.v2.errors import register_errors from app.letters.letter_schemas import letter_job_ids from app.schema_validation import validate +from app.utils import get_unrestricted_letter_ids letter_job = Blueprint("letter-job", __name__) register_errors(letter_job) @@ -16,7 +17,11 @@ register_errors(letter_job) @letter_job.route('/send-letter-jobs', methods=['POST']) def send_letter_jobs(): job_ids = validate(request.get_json(), letter_job_ids) - notify_celery.send_task(name=TaskNames.DVLA_FILES, args=(job_ids['job_ids'],), queue=QueueNames.PROCESS_FTP) + + unrestricted_job_ids = get_unrestricted_letter_ids(job_ids['job_ids']) + + notify_celery.send_task( + name=TaskNames.DVLA_FILES, args=(unrestricted_job_ids,), queue=QueueNames.PROCESS_FTP) return jsonify(data={"response": "Task created to send files to DVLA"}), 201 diff --git a/app/utils.py b/app/utils.py index cf4c28f06..538673fb3 100644 --- a/app/utils.py +++ b/app/utils.py @@ -1,7 +1,7 @@ from datetime import datetime, timedelta import pytz -from flask import url_for +from flask import current_app, url_for from sqlalchemy import func from notifications_utils.template import SMSMessageTemplate, PlainTextEmailTemplate, LetterPreviewTemplate @@ -86,3 +86,31 @@ def get_public_notify_type_text(notify_type, plural=False): notify_type_text = 'text message' return '{}{}'.format(notify_type_text, 's' if plural else '') + + +def get_unrestricted_letter_ids(job_ids): + from app.dao.jobs_dao import ( + dao_get_job_by_id, + dao_update_job + ) + + from app.models import (LETTER_TYPE, JOB_STATUS_ERROR) + from app.dao.templates_dao import dao_get_template_by_id + + unrestricted_job_ids = [] + + for job_id in job_ids: + job = dao_get_job_by_id(job_id) + + template = dao_get_template_by_id(job.template_id, job.template_version) + + if template.template_type == LETTER_TYPE: + if job.service.restricted: + job.job_status = JOB_STATUS_ERROR + dao_update_job(job) + current_app.logger.warn( + "Job {} has been set to error, service {} is in trial mode".format(job.id, job.service.id)) + else: + unrestricted_job_ids.append(job_id) + + return unrestricted_job_ids diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 0929291b2..cfcd386dd 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -41,7 +41,7 @@ from app.dao.provider_details_dao import ( from app.models import ( Service, Template, SMS_TYPE, LETTER_TYPE, - JOB_STATUS_READY_TO_SEND, + JOB_STATUS_READY_TO_SEND, JOB_STATUS_ERROR, MonthlyBilling) from app.utils import get_london_midnight_in_utc, convert_utc_to_bst from tests.app.db import create_notification, create_service, create_template, create_job, create_rate @@ -680,7 +680,9 @@ def test_populate_monthly_billing_updates_correct_month_in_bst(sample_template): def test_run_letter_jobs(client, mocker, sample_letter_template): - job_ids = [str(uuid.uuid4()), str(uuid.uuid4())] + jobs = [create_job(template=sample_letter_template, job_status=JOB_STATUS_READY_TO_SEND), + create_job(template=sample_letter_template, job_status=JOB_STATUS_READY_TO_SEND)] + job_ids = [str(j.id) for j in jobs] mocker.patch( "app.celery.scheduled_tasks.dao_get_letter_job_ids_by_status", return_value=job_ids @@ -692,3 +694,42 @@ def test_run_letter_jobs(client, mocker, sample_letter_template): mock_celery.assert_called_once_with(name=TaskNames.DVLA_FILES, args=(job_ids,), queue=QueueNames.PROCESS_FTP) + + +def test_run_letter_jobs_in_trial_sets_job_to_error(client, mocker, sample_letter_template): + sample_letter_template.service.restricted = True + job = create_job(sample_letter_template) + job_ids = [str(job.id)] + mocker.patch( + "app.celery.scheduled_tasks.dao_get_letter_job_ids_by_status", + return_value=job_ids + ) + mock_celery = mocker.patch("app.celery.tasks.notify_celery.send_task") + + run_letter_jobs() + + assert not mock_celery.called + assert job.job_status == JOB_STATUS_ERROR + + +def test_run_letter_jobs_in_trial_sets_job_to_error_and_process_live_services( + client, mocker, sample_letter_template): + live_job = create_job(sample_letter_template) + + service = create_service(service_name="Sample service 2", restricted=True) + template = create_template(service, template_type=LETTER_TYPE) + trial_job = create_job(template) + + job_ids = [str(live_job.id), str(trial_job.id)] + mocker.patch( + "app.celery.scheduled_tasks.dao_get_letter_job_ids_by_status", + return_value=job_ids + ) + mock_celery = mocker.patch("app.celery.tasks.notify_celery.send_task") + + run_letter_jobs() + + mock_celery.assert_called_once_with(name=TaskNames.DVLA_FILES, + args=([str(live_job.id)],), + queue=QueueNames.PROCESS_FTP) + assert trial_job.job_status == JOB_STATUS_ERROR diff --git a/tests/app/letters/test_send_letter_jobs.py b/tests/app/letters/test_send_letter_jobs.py index d38e24c8f..d899387b6 100644 --- a/tests/app/letters/test_send_letter_jobs.py +++ b/tests/app/letters/test_send_letter_jobs.py @@ -5,11 +5,15 @@ from flask import json from app.variables import LETTER_TEST_API_FILENAME from tests import create_authorization_header +from tests.app.db import create_job -def test_send_letter_jobs(client, mocker): +def test_send_letter_jobs(client, mocker, sample_letter_template): mock_celery = mocker.patch("app.letters.rest.notify_celery.send_task") - job_ids = {"job_ids": [str(uuid.uuid4()), str(uuid.uuid4()), str(uuid.uuid4())]} + job_1 = create_job(sample_letter_template) + job_2 = create_job(sample_letter_template) + job_3 = create_job(sample_letter_template) + job_ids = {"job_ids": [str(job_1.id), str(job_2.id), str(job_3.id)]} auth_header = create_authorization_header()