Added a check that the call is not using a test api key.

Removed the tests for trial mode service for the scheduled tasks and the process job.
Having the validation in the POST notification and create job endpoint is enough.
Updated the test_service_whitelist test because the order of the array is not gaurenteed.

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))

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
 

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
 

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

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)

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

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),

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):

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