From 0a21f1f3e8ceebc9d56e5b64d807143273c0c4ca Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 11 Sep 2017 11:10:45 +0100 Subject: [PATCH] Small refactor to how notification_schemas are tested. My local build was not always getting the optional requirement for the jsonschema uri format checker (rfc3987). The requirement has been added to the requirements_for_test file. I changed the tests to validate the response schema from the post_notifications tests, this way we can tell if we are breaking the contract. This showed that the email_from was not returning the entire email address but just the username, that has been corrected here. Removed the response schema validation tests since they are not being testing in the post notification tests. --- app/v2/notifications/notification_schemas.py | 2 +- app/v2/notifications/post_notifications.py | 2 +- requirements_for_test.txt | 2 + .../test_notification_schemas.py | 145 +----------------- .../test_post_letter_notifications.py | 3 + .../notifications/test_post_notifications.py | 8 +- 6 files changed, 16 insertions(+), 146 deletions(-) diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index fefbf5634..a22ad863b 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -177,7 +177,7 @@ email_content = { post_email_response = { "$schema": "http://json-schema.org/draft-04/schema#", - "description": "POST sms notification response schema", + "description": "POST email notification response schema", "type": "object", "title": "response v2/notifications/email", "properties": { diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index db69a5758..c2544ffd4 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -88,7 +88,7 @@ def post_notification(notification_type): create_resp_partial = functools.partial( create_post_email_response_from_notification, subject=template_with_content.subject, - email_from=authenticated_service.email_from + email_from='{}@{}'.format(authenticated_service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']) ) elif notification_type == LETTER_TYPE: create_resp_partial = functools.partial( diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 8bfd763e2..53f76b059 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -8,4 +8,6 @@ coveralls==1.2.0 moto==1.1.2 freezegun==0.3.9 requests-mock==1.3.0 +# optional requirements for jsonschema strict-rfc3339==0.7 +rfc3987==1.3.7 \ No newline at end of file diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index 83ad8c5f7..9e3040194 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -5,15 +5,12 @@ from flask import json from freezegun import freeze_time from jsonschema import ValidationError +from app.schema_validation import validate from app.v2.notifications.notification_schemas import ( get_notifications_request, - get_notification_response, post_sms_request as post_sms_request_schema, - post_sms_response as post_sms_response_schema, - post_email_request as post_email_request_schema, - post_email_response as post_email_response_schema + post_email_request as post_email_request_schema ) -from app.schema_validation import validate @pytest.mark.parametrize('invalid_statuses, valid_statuses', [ @@ -163,40 +160,6 @@ def test_post_sms_request_schema_invalid_phone_number_and_missing_template(): assert {"error": "ValidationError", "message": "template_id is a required property"} in errors -def valid_sms_response(): - return { - "id": str(uuid.uuid4()), - "content": {"body": "contents of message", - "from_number": "46045"}, - "uri": "http://notify.api/v2/notifications/id", - "template": { - "id": str(uuid.uuid4()), - "version": 1, - "uri": "http://notify.api/v2/template/id" - } - } - - -def valid_sms_response_with_optionals(): - return { - "id": str(uuid.uuid4()), - "reference": "reference_from_service", - "content": {"body": "contents of message", - "from_number": "46045"}, - "uri": "http://notify.api/v2/notifications/id", - "template": { - "id": str(uuid.uuid4()), - "version": 1, - "uri": "http://notify.api/v2/template/id" - } - } - - -@pytest.mark.parametrize('input', [valid_sms_response()]) -def test_post_sms_response_schema_schema_is_valid(input): - assert validate(input, post_sms_response_schema) == input - - valid_post_email_json = {"email_address": "test@example.gov.uk", "template_id": str(uuid.uuid4()) } @@ -252,110 +215,6 @@ def valid_email_response(): } -def valid_email_response_with_optionals(): - return { - "id": str(uuid.uuid4()), - "reference": "some reference", - "content": {"body": "the body of the message", - "subject": "subject of the message", - "from_email": "service@dig.gov.uk"}, - "uri": "http://notify.api/v2/notifications/id", - "template": { - "id": str(uuid.uuid4()), - "version": 1, - "uri": "http://notify.api/v2/template/id" - }, - "schedule_for": "2017-05-12 13:00:00" - } - - -@pytest.mark.parametrize("input", [valid_email_response(), valid_email_response_with_optionals()]) -def test_post_email_response_schema(input): - assert validate(input, post_email_response_schema) == input - - -@pytest.mark.parametrize('response, schema', [ - (valid_email_response(), post_email_response_schema), - (valid_sms_response(), post_sms_response_schema) -]) -def test_post_sms_response_schema_missing_uri_raises_validation_error(response, schema): - del response['uri'] - with pytest.raises(ValidationError) as e: - validate(response, schema) - error = json.loads(str(e.value)) - assert error['status_code'] == 400 - assert error['errors'] == [{'error': 'ValidationError', - 'message': "uri is a required property"}] - - -@pytest.mark.parametrize('response, schema', [ - (valid_email_response(), post_email_response_schema), - (valid_sms_response(), post_sms_response_schema) -]) -def test_post_sms_response_schema_invalid_uri_raises_validation_error(response, schema): - response['uri'] = 'invalid-uri' - with pytest.raises(ValidationError) as e: - validate(response, schema) - error = json.loads(str(e.value)) - assert error['status_code'] == 400 - assert error['errors'] == [{'error': 'ValidationError', - 'message': "uri invalid-uri is not a valid URI."}] - - -@pytest.mark.parametrize('response, schema', [ - (valid_email_response(), post_email_response_schema), - (valid_sms_response(), post_sms_response_schema) -]) -def test_post_sms_response_schema_missing_template_uri_raises_validation_error(response, schema): - del response['template']['uri'] - with pytest.raises(ValidationError) as e: - validate(response, schema) - error = json.loads(str(e.value)) - assert error['status_code'] == 400 - assert error['errors'] == [{'error': 'ValidationError', - 'message': "template uri is a required property"}] - - -@pytest.mark.parametrize('response, schema', [ - (valid_email_response(), post_email_response_schema), - (valid_sms_response(), post_sms_response_schema) -]) -def test_post_sms_response_schema_invalid_template_uri_raises_validation_error(response, schema): - response['template']['uri'] = 'invalid-uri' - with pytest.raises(ValidationError) as e: - validate(response, schema) - error = json.loads(str(e.value)) - assert error['status_code'] == 400 - assert error['errors'] == [{'error': 'ValidationError', - 'message': "template invalid-uri is not a valid URI."}] - - -def test_get_notifications_response_with_email_and_phone_number(): - response = {"id": str(uuid.uuid4()), - "reference": "something", - "email_address": None, - "phone_number": "+447115411111", - "line_1": None, - "line_2": None, - "line_3": None, - "line_4": None, - "line_5": None, - "line_6": None, - "postcode": None, - "type": "email", - "status": "delivered", - "template": {"id": str(uuid.uuid4()), "version": 1, "uri": "http://template/id"}, - "body": "some body", - "subject": "some subject", - "created_at": "2016-01-01", - "sent_at": "2016-01-01", - "completed_at": "2016-01-01", - "schedule_for": "" - } - - assert validate(response, get_notification_response) == response - - @pytest.mark.parametrize("schema", [post_email_request_schema, post_sms_request_schema]) @freeze_time("2017-05-12 13:00:00") diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index aa4433a59..2f883d1be 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -12,7 +12,9 @@ from app.models import KEY_TYPE_TEST from app.models import LETTER_TYPE from app.models import Notification from app.models import SMS_TYPE +from app.schema_validation import validate from app.v2.errors import RateLimitError +from app.v2.notifications.notification_schemas import post_letter_response from app.variables import LETTER_TEST_API_FILENAME from app.variables import LETTER_API_FILENAME @@ -53,6 +55,7 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo resp_json = letter_request(client, data, service_id=sample_letter_template.service_id) + assert validate(resp_json, post_letter_response) == resp_json job = Job.query.one() assert job.original_file_name == LETTER_API_FILENAME notification = Notification.query.one() diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index bd5b4f711..4e72389c3 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -10,7 +10,9 @@ from app.models import ( from flask import json, current_app from app.models import Notification +from app.schema_validation import validate from app.v2.errors import RateLimitError +from app.v2.notifications.notification_schemas import post_sms_response, post_email_response from tests import create_authorization_header from tests.app.conftest import ( sample_template as create_sample_template, sample_service, @@ -38,6 +40,7 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 resp_json = json.loads(response.get_data(as_text=True)) + assert validate(resp_json, post_sms_response) == resp_json notifications = Notification.query.all() assert len(notifications) == 1 notification_id = notifications[0].id @@ -71,6 +74,7 @@ def test_post_sms_notification_uses_inbound_number_as_sender(client, sample_temp headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 resp_json = json.loads(response.get_data(as_text=True)) + assert validate(resp_json, post_sms_response) == resp_json notifications = Notification.query.all() assert len(notifications) == 1 notification_id = notifications[0].id @@ -170,6 +174,7 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 201 resp_json = json.loads(response.get_data(as_text=True)) + assert validate(resp_json, post_email_response) == resp_json notification = Notification.query.first() assert resp_json['id'] == str(notification.id) assert resp_json['reference'] == reference @@ -178,7 +183,8 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ .replace('((name))', 'Bob').replace('GOV.UK', u'GOV.\u200bUK') assert resp_json['content']['subject'] == sample_email_template_with_placeholders.subject \ .replace('((name))', 'Bob') - assert resp_json['content']['from_email'] == sample_email_template_with_placeholders.service.email_from + assert resp_json['content']['from_email'] == "{}@{}".format( + sample_email_template_with_placeholders.service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']) assert 'v2/notifications/{}'.format(notification.id) in resp_json['uri'] assert resp_json['template']['id'] == str(sample_email_template_with_placeholders.id) assert resp_json['template']['version'] == sample_email_template_with_placeholders.version