From 155e1733631d4ac1b24eb40571b05470d6f53680 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 11 Apr 2017 15:39:34 +0100 Subject: [PATCH 01/10] Update send letter jobs to return json --- app/letters/send_letter_jobs.py | 8 ++++++-- tests/app/letters/test_send_letter_jobs.py | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/letters/send_letter_jobs.py b/app/letters/send_letter_jobs.py index 806689567..9a1145b82 100644 --- a/app/letters/send_letter_jobs.py +++ b/app/letters/send_letter_jobs.py @@ -1,3 +1,5 @@ +import json + from flask import Blueprint, jsonify from flask import request @@ -14,10 +16,12 @@ 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) + req_json = request.get_json() + job_ids = validate(req_json, letter_job_ids) + notify_celery.send_task(name="send-files-to-dvla", args=(job_ids['job_ids'],), queue="process-ftp") - return "Task created to send files to DVLA" + return jsonify(data={"response": "Task created to send files to DVLA"}), 200 @letter_job.route('/letter-jobs', methods=['GET']) diff --git a/tests/app/letters/test_send_letter_jobs.py b/tests/app/letters/test_send_letter_jobs.py index f006a981d..5cc2d94b7 100644 --- a/tests/app/letters/test_send_letter_jobs.py +++ b/tests/app/letters/test_send_letter_jobs.py @@ -17,7 +17,7 @@ def test_send_letter_jobs(client, mocker): headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 - assert response.get_data(as_text=True) == "Task created to send files to DVLA" + assert json.loads(response.get_data())['data'] == {'response': "Task created to send files to DVLA"} mock_celery.assert_called_once_with(name="send-files-to-dvla", args=(job_ids['job_ids'],), From ad6bdbcc9743cbe4bb45ec5f7118e785bd3b8333 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 11 Apr 2017 15:49:44 +0100 Subject: [PATCH 02/10] Removed redundant json import --- app/letters/send_letter_jobs.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/letters/send_letter_jobs.py b/app/letters/send_letter_jobs.py index 9a1145b82..155015531 100644 --- a/app/letters/send_letter_jobs.py +++ b/app/letters/send_letter_jobs.py @@ -1,5 +1,3 @@ -import json - from flask import Blueprint, jsonify from flask import request @@ -16,9 +14,7 @@ register_errors(letter_job) @letter_job.route('/send-letter-jobs', methods=['POST']) def send_letter_jobs(): - req_json = request.get_json() - job_ids = validate(req_json, letter_job_ids) - + job_ids = validate(request.get_json(), letter_job_ids) notify_celery.send_task(name="send-files-to-dvla", args=(job_ids['job_ids'],), queue="process-ftp") return jsonify(data={"response": "Task created to send files to DVLA"}), 200 From eba88b08c522b8ef28be1121f58352b63213c495 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 4 Apr 2017 16:14:54 +0100 Subject: [PATCH 03/10] Removed URI from preview response --- app/v2/template/post_template.py | 3 +-- app/v2/template/template_schemas.py | 5 ++--- tests/app/v2/template/test_post_template.py | 1 - 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/app/v2/template/post_template.py b/app/v2/template/post_template.py index 531a9683c..ca94ee157 100644 --- a/app/v2/template/post_template.py +++ b/app/v2/template/post_template.py @@ -29,8 +29,7 @@ def post_template_preview(template_id): check_placeholders(template_object) resp = create_post_template_preview_response(template=template, - body=str(template_object), - url_root=request.url_root) + body=str(template_object)) return jsonify(resp), 200 diff --git a/app/v2/template/template_schemas.py b/app/v2/template/template_schemas.py index bc405129c..0ad632b9a 100644 --- a/app/v2/template/template_schemas.py +++ b/app/v2/template/template_schemas.py @@ -68,12 +68,11 @@ post_template_preview_response = { } -def create_post_template_preview_response(template, body, url_root): +def create_post_template_preview_response(template, body): return { "id": template.id, "type": template.template_type, "version": template.version, "body": body, - "subject": template.subject, - "uri": "{}v2/template/{}/preview".format(url_root, template.id) + "subject": template.subject } diff --git a/tests/app/v2/template/test_post_template.py b/tests/app/v2/template/test_post_template.py index f99b3ee86..a6c8d542d 100644 --- a/tests/app/v2/template/test_post_template.py +++ b/tests/app/v2/template/test_post_template.py @@ -31,7 +31,6 @@ def test_valid_post_template_returns_200(client, sample_service, tmp_type): resp_json = json.loads(response.get_data(as_text=True)) assert resp_json['id'] == str(template.id) - assert 'v2/template/{}/preview'.format(template.id) in resp_json['uri'] assert 'Dear {}'.format(valid_data['personalisation']['Name']) in resp_json['body'] From 06ec7a59a6f5cafb73c09a7537650d7620d219bb Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 6 Apr 2017 12:10:06 +0100 Subject: [PATCH 04/10] Refactored code to personalise subject --- app/v2/template/post_template.py | 7 +++ app/v2/template/template_schemas.py | 6 +-- tests/app/db.py | 3 +- tests/app/v2/template/test_post_template.py | 52 ++++++++++++++++--- .../app/v2/template/test_template_schemas.py | 1 - 5 files changed, 56 insertions(+), 13 deletions(-) diff --git a/app/v2/template/post_template.py b/app/v2/template/post_template.py index ca94ee157..c4ecbc6fe 100644 --- a/app/v2/template/post_template.py +++ b/app/v2/template/post_template.py @@ -6,6 +6,7 @@ from werkzeug.exceptions import abort from app import api_user from app.dao import templates_dao +from app.models import SMS_TYPE from app.schema_validation import validate from app.utils import get_template_instance from app.v2.errors import BadRequestError @@ -16,6 +17,9 @@ from app.v2.template.template_schemas import post_template_preview_request, crea @v2_template_blueprint.route("//preview", methods=['POST']) def post_template_preview(template_id): _data = request.get_json() + if _data is None: + _data = {} + _data['id'] = template_id data = validate(_data, post_template_preview_request) @@ -28,7 +32,10 @@ def post_template_preview(template_id): check_placeholders(template_object) + subject = template_object.subject if template.template_type != SMS_TYPE else None + resp = create_post_template_preview_response(template=template, + subject=subject, body=str(template_object)) return jsonify(resp), 200 diff --git a/app/v2/template/template_schemas.py b/app/v2/template/template_schemas.py index 0ad632b9a..dac2c4cde 100644 --- a/app/v2/template/template_schemas.py +++ b/app/v2/template/template_schemas.py @@ -49,7 +49,7 @@ post_template_preview_request = { "id": uuid, "personalisation": personalisation }, - "required": ["id", "personalisation"] + "required": ["id"] } post_template_preview_response = { @@ -68,11 +68,11 @@ post_template_preview_response = { } -def create_post_template_preview_response(template, body): +def create_post_template_preview_response(template, subject, body): return { "id": template.id, "type": template.template_type, "version": template.version, "body": body, - "subject": template.subject + "subject": subject } diff --git a/tests/app/db.py b/tests/app/db.py index bcd94c9c3..e2a24f1c3 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -39,6 +39,7 @@ def create_service(user=None, service_name="Sample service", service_id=None): def create_template( service, template_type=SMS_TYPE, + subject='Template subject', content='Dear Sir/Madam, Hello. Yours Truly, The Government.', template_id=None ): @@ -50,7 +51,7 @@ def create_template( 'created_by': service.created_by, } if template_type != SMS_TYPE: - data['subject'] = 'Template subject' + data['subject'] = subject template = Template(**data) dao_create_template(template) return template diff --git a/tests/app/v2/template/test_post_template.py b/tests/app/v2/template/test_post_template.py index a6c8d542d..4501852c2 100644 --- a/tests/app/v2/template/test_post_template.py +++ b/tests/app/v2/template/test_post_template.py @@ -3,27 +3,61 @@ import uuid from flask import json -from app.models import TEMPLATE_TYPES +from app.models import SMS_TYPE, TEMPLATE_TYPES from tests import create_authorization_header from tests.app.db import create_template -valid_data = { +valid_personalisation = { 'personalisation': {'Name': 'Jo'} } +valid_post = [ + ( + "Some subject", + "Some content", + None, + "Some subject", + "Some content" + ), + ( + "Some subject", + "Dear ((Name)), Hello. Yours Truly, The Government.", + valid_personalisation, + "Some subject", + "Dear Jo, Hello. Yours Truly, The Government." + ), + ( + "Message for ((Name))", + "Dear ((Name)), Hello. Yours Truly, The Government.", + valid_personalisation, + "Message for Jo", + "Dear Jo, Hello. Yours Truly, The Government." + ), + ( + "Message for ((Name))", + "Some content", + valid_personalisation, + "Message for Jo", + "Some content" + ), +] + @pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES) -def test_valid_post_template_returns_200(client, sample_service, tmp_type): +@pytest.mark.parametrize("subject,content,post_data,expected_subject,expected_content", valid_post) +def test_valid_post_template_returns_200( + client, sample_service, tmp_type, subject, content, post_data, expected_subject, expected_content): template = create_template( sample_service, template_type=tmp_type, - content='Dear ((Name)), Hello. Yours Truly, The Government.') + subject=subject, + content=content) auth_header = create_authorization_header(service_id=sample_service.id) response = client.post( path='/v2/template/{}/preview'.format(template.id), - data=json.dumps(valid_data), + data=json.dumps(post_data), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 200 @@ -31,7 +65,9 @@ def test_valid_post_template_returns_200(client, sample_service, tmp_type): resp_json = json.loads(response.get_data(as_text=True)) assert resp_json['id'] == str(template.id) - assert 'Dear {}'.format(valid_data['personalisation']['Name']) in resp_json['body'] + if tmp_type != SMS_TYPE: + assert expected_subject in resp_json['subject'] + assert expected_content in resp_json['body'] @pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES) @@ -45,7 +81,7 @@ def test_invalid_post_template_returns_400(client, sample_service, tmp_type): response = client.post( path='/v2/template/{}/preview'.format(template.id), - data=json.dumps(valid_data), + data=json.dumps(valid_personalisation), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 400 @@ -61,7 +97,7 @@ def test_post_template_with_non_existent_template_id_returns_404(client, fake_uu response = client.post( path='/v2/template/{}/preview'.format(fake_uuid), - data=json.dumps(valid_data), + data=json.dumps(valid_personalisation), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 404 diff --git a/tests/app/v2/template/test_template_schemas.py b/tests/app/v2/template/test_template_schemas.py index 97fca1ca4..c89802942 100644 --- a/tests/app/v2/template/test_template_schemas.py +++ b/tests/app/v2/template/test_template_schemas.py @@ -54,7 +54,6 @@ invalid_json_post_args = [ ({"id": "invalid_uuid", "personalisation": {"key": "value"}}, ["id is not a valid UUID"]), ({"id": str(uuid.uuid4()), "personalisation": "invalid_personalisation"}, ["personalisation should contain key value pairs"]), - ({"id": str(uuid.uuid4())}, ["personalisation is a required property"]), ({"personalisation": "invalid_personalisation"}, ["id is a required property", "personalisation is a required property", From 7c0aeca66eba15cfdf01d2662eedbb9bac467044 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 6 Apr 2017 12:13:32 +0100 Subject: [PATCH 05/10] Removed unused imports --- app/v2/template/post_template.py | 3 --- tests/app/v2/template/test_post_template.py | 1 - 2 files changed, 4 deletions(-) diff --git a/app/v2/template/post_template.py b/app/v2/template/post_template.py index c4ecbc6fe..ae9b16002 100644 --- a/app/v2/template/post_template.py +++ b/app/v2/template/post_template.py @@ -1,8 +1,5 @@ -import uuid - from flask import jsonify, request from jsonschema.exceptions import ValidationError -from werkzeug.exceptions import abort from app import api_user from app.dao import templates_dao diff --git a/tests/app/v2/template/test_post_template.py b/tests/app/v2/template/test_post_template.py index 4501852c2..158d9b258 100644 --- a/tests/app/v2/template/test_post_template.py +++ b/tests/app/v2/template/test_post_template.py @@ -1,5 +1,4 @@ import pytest -import uuid from flask import json From ccd25382eae448df8e11e912483030d98ddc5325 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 11 Apr 2017 16:37:30 +0100 Subject: [PATCH 06/10] Refactored post template --- app/v2/template/post_template.py | 5 +---- app/v2/template/template_schemas.py | 8 +++++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/v2/template/post_template.py b/app/v2/template/post_template.py index ae9b16002..3da768ef1 100644 --- a/app/v2/template/post_template.py +++ b/app/v2/template/post_template.py @@ -29,11 +29,8 @@ def post_template_preview(template_id): check_placeholders(template_object) - subject = template_object.subject if template.template_type != SMS_TYPE else None - resp = create_post_template_preview_response(template=template, - subject=subject, - body=str(template_object)) + template_object=template_object) return jsonify(resp), 200 diff --git a/app/v2/template/template_schemas.py b/app/v2/template/template_schemas.py index dac2c4cde..6f08d9cfc 100644 --- a/app/v2/template/template_schemas.py +++ b/app/v2/template/template_schemas.py @@ -1,4 +1,4 @@ -from app.models import TEMPLATE_TYPES +from app.models import SMS_TYPE, TEMPLATE_TYPES from app.schema_validation.definitions import uuid, personalisation @@ -68,11 +68,13 @@ post_template_preview_response = { } -def create_post_template_preview_response(template, subject, body): +def create_post_template_preview_response(template, template_object): + subject = template_object.subject if template.template_type != SMS_TYPE else None + return { "id": template.id, "type": template.template_type, "version": template.version, - "body": body, + "body": str(template_object), "subject": subject } From 879246199887ebe849c820f717ff4c39221c4fa5 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 11 Apr 2017 16:42:08 +0100 Subject: [PATCH 07/10] Changed response to 201 for send_letter_jobs --- app/letters/send_letter_jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/letters/send_letter_jobs.py b/app/letters/send_letter_jobs.py index 155015531..7030b9bc5 100644 --- a/app/letters/send_letter_jobs.py +++ b/app/letters/send_letter_jobs.py @@ -17,7 +17,7 @@ def send_letter_jobs(): job_ids = validate(request.get_json(), letter_job_ids) notify_celery.send_task(name="send-files-to-dvla", args=(job_ids['job_ids'],), queue="process-ftp") - return jsonify(data={"response": "Task created to send files to DVLA"}), 200 + return jsonify(data={"response": "Task created to send files to DVLA"}), 201 @letter_job.route('/letter-jobs', methods=['GET']) From c1ae3369a6376db9a8df10ade79bc79f2780d059 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 11 Apr 2017 16:53:30 +0100 Subject: [PATCH 08/10] Update test for 201 response from send_letter_jobs --- tests/app/letters/test_send_letter_jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/letters/test_send_letter_jobs.py b/tests/app/letters/test_send_letter_jobs.py index 5cc2d94b7..7f1d88c85 100644 --- a/tests/app/letters/test_send_letter_jobs.py +++ b/tests/app/letters/test_send_letter_jobs.py @@ -16,7 +16,7 @@ def test_send_letter_jobs(client, mocker): data=json.dumps(job_ids), headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 200 + assert response.status_code == 201 assert json.loads(response.get_data())['data'] == {'response': "Task created to send files to DVLA"} mock_celery.assert_called_once_with(name="send-files-to-dvla", From b5368d296f4912b61d7134275a445b049e30e47b Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 11 Apr 2017 17:28:35 +0100 Subject: [PATCH 09/10] Reduce boto logging level. Update notifications-utils to version 15.1.1 in the hopes that the new logging level for s3tranfer will stop the boto logging of the letters body. But I can only test this on preview. --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 0557ed9dc..47c013646 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,6 +29,6 @@ notifications-python-client>=3.1,<3.2 awscli>=1.11,<1.12 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@15.0.4#egg=notifications-utils==15.0.4 +git+https://github.com/alphagov/notifications-utils.git@15.1.1#egg=notifications-utils==15.1.1 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 From 78825db1602e2d078a610744470276a2e5f5cf23 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 12 Apr 2017 16:11:30 +0100 Subject: [PATCH 10/10] Bump utils to fix problems with our output to DVLA Depends on: - [ ] https://github.com/alphagov/notifications-utils/pull/148 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 47c013646..eb74c28d6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,6 +29,6 @@ notifications-python-client>=3.1,<3.2 awscli>=1.11,<1.12 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@15.1.1#egg=notifications-utils==15.1.1 +git+https://github.com/alphagov/notifications-utils.git@15.1.2#egg=notifications-utils==15.1.2 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3