Merge pull request #1221 from alphagov/ken-no-letters-when-trial

Stop letters when service is in trial

app/celery/tasks.py

@@ -86,9 +86,11 @@ def process_job(job_id):
         process_row(row_number, recipient, personalisation, template, job, service)
 
     if template.template_type == LETTER_TYPE:
-        build_dvla_file.apply_async([str(job.id)], queue=QueueNames.JOBS)
+        if service.research_mode:
-        # temporary logging
+            update_job_to_sent_to_dvla.apply_async([str(job.id)], queue=QueueNames.RESEARCH_MODE)
-        current_app.logger.info("send job {} to build-dvla-file in the process-job queue".format(job_id))
+        else:
+            build_dvla_file.apply_async([str(job.id)], queue=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/job/rest.py

@@ -32,7 +32,7 @@ from app.schemas import (
 
 from app.celery.tasks import process_job
 
-from app.models import JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, JOB_STATUS_CANCELLED
+from app.models import JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, JOB_STATUS_CANCELLED, LETTER_TYPE
 
 from app.utils import pagination_links
 
@@ -190,6 +190,9 @@ def create_job(service_id):
     })
     template = dao_get_template_by_id(data['template'])
 
+    if template.template_type == LETTER_TYPE and service.restricted:
+        raise InvalidRequest("Create letter job is not allowed for service in trial mode ", 403)
+
     errors = unarchived_template_schema.validate({'archived': template.archived})
 
     if errors:

app/utils.py

@@ -3,7 +3,7 @@ from datetime import datetime, timedelta
 import pytz
 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")
 

app/v2/notifications/post_notifications.py

@@ -150,15 +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 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
@@ -44,7 +42,7 @@ from app.models import (
     SMS_TYPE, LETTER_TYPE,
     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,
@@ -683,7 +681,7 @@ def test_populate_monthly_billing_updates_correct_month_in_bst(sample_template):
 def test_run_letter_jobs(client, mocker, sample_letter_template):
     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(job.id) for job in jobs]
+    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

tests/app/celery/test_tasks.py

@@ -29,6 +29,7 @@ from app.celery.tasks import (
     update_letter_notifications_statuses,
     process_updates_from_file,
     send_inbound_sms_to_service)
+from app.config import QueueNames
 from app.dao import jobs_dao, services_dao
 from app.models import (
     Notification,
@@ -38,7 +39,8 @@ from app.models import (
     SMS_TYPE,
     EMAIL_TYPE,
     LETTER_TYPE,
-    Job)
+    Job,
+    JOB_STATUS_ERROR)
 
 from tests.app import load_example_csv
 from tests.conftest import set_config
@@ -608,6 +610,62 @@ def test_should_put_send_email_task_in_research_mode_queue_if_research_mode_serv
     )
 
 
+def test_should_not_build_dvla_file_in_research_mode_for_letter_job(
+        mocker, sample_service, sample_letter_job, fake_uuid
+):
+    test_encrypted_data = 'some encrypted data'
+    sample_service.research_mode = True
+
+    csv = """address_line_1,address_line_2,address_line_3,address_line_4,postcode,name
+    A1,A2,A3,A4,A_POST,Alice
+    """
+    mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=csv)
+    mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async')
+    mocker.patch('app.celery.tasks.persist_letter.apply_async')
+    mocker.patch('app.celery.tasks.create_uuid', return_value=fake_uuid)
+    mocker.patch('app.celery.tasks.encryption.encrypt', return_value=test_encrypted_data)
+    mock_dvla_file_task = mocker.patch('app.celery.tasks.build_dvla_file.apply_async')
+
+    process_job(sample_letter_job.id)
+
+    assert not mock_dvla_file_task.called
+
+
+@freeze_time("2017-08-29 17:30:00")
+def test_should_update_job_to_sent_to_dvla_in_research_mode_for_letter_job(
+        mocker, sample_service, sample_letter_job, fake_uuid
+):
+    test_encrypted_data = 'some encrypted data'
+    sample_service.research_mode = True
+
+    csv = """address_line_1,address_line_2,address_line_3,address_line_4,postcode,name
+    A1,A2,A3,A4,A_POST,Alice
+    """
+    mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=csv)
+    mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async')
+    mocker.patch('app.celery.tasks.persist_letter.apply_async')
+    mocker.patch('app.celery.tasks.create_uuid', return_value=fake_uuid)
+    mocker.patch('app.celery.tasks.encryption.encrypt', return_value=test_encrypted_data)
+    mock_dvla_file_task = mocker.patch('app.celery.tasks.build_dvla_file.apply_async')
+
+    process_job(sample_letter_job.id)
+
+    job = jobs_dao.dao_get_job_by_id(sample_letter_job.id)
+
+    persist_letter.apply_async.assert_called_once_with(
+        (
+            str(sample_service.id),
+            fake_uuid,
+            test_encrypted_data,
+            datetime.utcnow().strftime(DATETIME_FORMAT)
+        ),
+        queue=QueueNames.RESEARCH_MODE
+    )
+
+    update_job_to_sent_to_dvla.apply_async.assert_called_once_with(
+        [str(job.id)], queue=QueueNames.RESEARCH_MODE)
+
+
 def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_api_key, mocker):
     notification = _notification_json(
         sample_job.template,

tests/app/conftest.py

@@ -278,6 +278,12 @@ def sample_letter_template(sample_service_full_permissions):
     return create_template(sample_service_full_permissions, template_type=LETTER_TYPE)
 
 
+@pytest.fixture
+def sample_trial_letter_template(sample_service_full_permissions):
+    sample_service_full_permissions.restricted = True
+    return create_template(sample_service_full_permissions, template_type=LETTER_TYPE)
+
+
 @pytest.fixture(scope='function')
 def sample_email_template_with_placeholders(notify_db, notify_db_session):
     return sample_email_template(

tests/app/job/test_rest.py

@@ -186,6 +186,29 @@ def test_create_job_returns_403_if_service_is_not_active(client, fake_uuid, samp
     mock_job_dao.assert_not_called()
 
 
+def test_create_job_returns_403_if_letter_template_type_and_service_in_trial(
+        client, fake_uuid, sample_service, sample_trial_letter_template, mocker):
+    data = {
+        'id': fake_uuid,
+        'service': str(sample_trial_letter_template.service.id),
+        'template': str(sample_trial_letter_template.id),
+        'original_file_name': 'thisisatest.csv',
+        'notification_count': 1,
+        'created_by': str(sample_trial_letter_template.created_by.id)
+    }
+    mock_job_dao = mocker.patch("app.dao.jobs_dao.dao_create_job")
+    auth_header = create_authorization_header()
+    response = client.post('/service/{}/job'.format(sample_service.id),
+                           data=json.dumps(data),
+                           headers=[('Content-Type', 'application/json'), auth_header])
+
+    assert response.status_code == 403
+    resp_json = json.loads(response.get_data(as_text=True))
+    assert resp_json['result'] == 'error'
+    assert resp_json['message'] == "Create letter job is not allowed for service in trial mode "
+    mock_job_dao.assert_not_called()
+
+
 def test_should_not_create_scheduled_job_more_then_24_hours_hence(notify_api, sample_template, mocker, fake_uuid):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:

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

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)) == {
+    json_resp = json.loads(response.get_data(as_text=True))
-        'email_addresses': ['service@example.com'],
+    assert json_resp['email_addresses'] == ['service@example.com']
-        'phone_numbers': ['+1800-555-555', '07123456789']
+    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

@@ -17,13 +17,12 @@ from app.variables import LETTER_TEST_API_FILENAME
 from app.variables import LETTER_API_FILENAME
 
 from tests import create_authorization_header
-from tests.app.db import create_service
+from tests.app.db import create_service, create_template
-from tests.app.db import create_template
 
 
 def letter_request(client, data, service_id, key_type=KEY_TYPE_NORMAL, _expected_status=201):
     resp = client.post(
-        url_for('v2_notifications.post_notification', notification_type='letter'),
+        url_for('v2_notifications.post_notification', notification_type=LETTER_TYPE),
         data=json.dumps(data),
         headers=[
             ('Content-Type', 'application/json'),
@@ -232,3 +231,37 @@ def test_post_letter_notification_doesnt_accept_team_key(client, sample_letter_t
 
     assert error_json['status_code'] == 403
     assert error_json['errors'] == [{'error': 'BadRequestError', 'message': 'Cannot send letters with a team api key'}]
+
+
+def test_post_letter_notification_doesnt_send_in_trial(client, sample_trial_letter_template):
+    data = {
+        'template_id': str(sample_trial_letter_template.id),
+        'personalisation': {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'Baz'}
+    }
+
+    error_json = letter_request(
+        client,
+        data,
+        sample_trial_letter_template.service_id,
+        _expected_status=403
+    )
+
+    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