diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 9c4dbb5c5..5614c361f 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, get_unrestricted_letter_ids +from app.utils import convert_utc_to_bst @notify_celery.task(name="remove_csv_files") @@ -308,14 +308,9 @@ 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) - - 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)) + 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)) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index a5e9f98d6..e621350ce 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -75,13 +75,6 @@ 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 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, service.id)) - return - TemplateClass = get_template_class(db_template.template_type) template = TemplateClass(db_template.__dict__) @@ -97,9 +90,7 @@ 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)) + 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 458611a80..b9ce7bf35 100644 --- a/app/letters/rest.py +++ b/app/letters/rest.py @@ -8,7 +8,6 @@ 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) @@ -17,11 +16,7 @@ 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) - - 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) + notify_celery.send_task(name=TaskNames.DVLA_FILES, args=(job_ids['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 538673fb3..49f6d61dc 100644 --- a/app/utils.py +++ b/app/utils.py @@ -1,9 +1,9 @@ from datetime import datetime, timedelta import pytz -from flask import current_app, url_for +from flask import url_for from sqlalchemy import func -from notifications_utils.template import SMSMessageTemplate, PlainTextEmailTemplate, LetterPreviewTemplate +from notifications_utils.template import SMSMessageTemplate, PlainTextEmailTemplate local_timezone = pytz.timezone("Europe/London") @@ -86,31 +86,3 @@ 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/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index c1bcd168e..db69a5758 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -150,18 +150,16 @@ def process_letter_notification(*, letter_data, api_key, template): if api_key.key_type == KEY_TYPE_TEAM: raise BadRequestError(message='Cannot send letters with a team api key', status_code=403) - if api_key.service.restricted: - raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403) + if api_key.service.restricted and api_key.key_type != KEY_TYPE_TEST: + raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403) job = create_letter_api_job(template) notification = create_letter_notification(letter_data, job, api_key) if api_key.service.research_mode or api_key.key_type == KEY_TYPE_TEST: - # distinguish real API jobs from test jobs by giving the test jobs a different filename job.original_file_name = LETTER_TEST_API_FILENAME dao_update_job(job) - 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) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index cfcd386dd..2308e7247 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -1,5 +1,3 @@ -import uuid - from datetime import datetime, timedelta from functools import partial from unittest.mock import call, patch, PropertyMock @@ -41,9 +39,9 @@ from app.dao.provider_details_dao import ( from app.models import ( Service, Template, SMS_TYPE, LETTER_TYPE, - JOB_STATUS_READY_TO_SEND, JOB_STATUS_ERROR, + JOB_STATUS_READY_TO_SEND, MonthlyBilling) -from app.utils import get_london_midnight_in_utc, convert_utc_to_bst +from app.utils import get_london_midnight_in_utc from tests.app.db import create_notification, create_service, create_template, create_job, create_rate from tests.app.conftest import ( sample_job as create_sample_job, @@ -694,42 +692,3 @@ 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/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 344ac00ab..feca346e1 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1035,26 +1035,6 @@ def test_should_cancel_job_if_service_is_inactive(sample_service, mock_dvla_file_task.assert_not_called() -def test_should_error_job_if_service_is_restricted_and_letter_template_type( - sample_service, - sample_letter_job, - mocker -): - sample_service.restricted = True - - mocker.patch('app.celery.tasks.s3.get_job_from_s3') - mocker.patch('app.celery.tasks.process_row') - mock_dvla_file_task = mocker.patch('app.celery.tasks.build_dvla_file') - - process_job(sample_letter_job.id) - - job = jobs_dao.dao_get_job_by_id(sample_letter_job.id) - assert job.job_status == JOB_STATUS_ERROR - s3.get_job_from_s3.assert_not_called() - tasks.process_row.assert_not_called() - mock_dvla_file_task.assert_not_called() - - @pytest.mark.parametrize('template_type, expected_class', [ (SMS_TYPE, SMSMessageTemplate), (EMAIL_TYPE, WithSubjectTemplate), diff --git a/tests/app/service/test_service_whitelist.py b/tests/app/service/test_service_whitelist.py index 5edbe8e28..18c0595f8 100644 --- a/tests/app/service/test_service_whitelist.py +++ b/tests/app/service/test_service_whitelist.py @@ -30,10 +30,9 @@ def test_get_whitelist_separates_emails_and_phones(client, sample_service): response = client.get('service/{}/whitelist'.format(sample_service.id), headers=[create_authorization_header()]) assert response.status_code == 200 - assert json.loads(response.get_data(as_text=True)) == { - 'email_addresses': ['service@example.com'], - 'phone_numbers': ['+1800-555-555', '07123456789'] - } + json_resp = json.loads(response.get_data(as_text=True)) + assert json_resp['email_addresses'] == ['service@example.com'] + assert sorted(json_resp['phone_numbers']) == sorted(['+1800-555-555', '07123456789']) def test_get_whitelist_404s_with_unknown_service_id(client): diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index cb9d4bb29..aa4433a59 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -249,3 +249,19 @@ def test_post_letter_notification_doesnt_send_in_trial(client, sample_trial_lett assert error_json['status_code'] == 403 assert error_json['errors'] == [ {'error': 'BadRequestError', 'message': 'Cannot send letters when service is in trial mode'}] + + +def test_post_letter_notification_calls_update_job_sent_to_dvla_when_service_is_in_trial_mode_but_using_test_key( + client, sample_trial_letter_template, mocker): + build_dvla_task = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') + update_job_task = mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async') + + data = { + "template_id": sample_trial_letter_template.id, + "personalisation": {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'Baz'} + } + letter_request(client, data=data, service_id=sample_trial_letter_template.service_id, + key_type=KEY_TYPE_TEST) + job = Job.query.one() + update_job_task.assert_called_once_with([str(job.id)], queue='research-mode-tasks') + assert not build_dvla_task.called