From 337eeb60d1f43ee07af234b29858a4053cf150e1 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 27 Apr 2016 12:52:25 +0100 Subject: [PATCH 1/3] Update requirements version. --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 6c761f771..dd73379b3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,4 +9,4 @@ py-gfm==0.1.2 git+https://github.com/alphagov/notifications-python-client.git@0.5.0#egg=notifications-python-client==0.5.0 -git+https://github.com/alphagov/notifications-utils.git@4.1.3#egg=notifications-utils==4.1.3 +git+https://github.com/alphagov/notifications-utils.git@4.3.0#egg=notifications-utils==4.3.0 From fbf30129b0695f79ac692efea3f4d1b415030ba5 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Thu, 28 Apr 2016 14:02:51 +0100 Subject: [PATCH 2/3] Update app to the latest notifications_utils. --- app/main/views/send.py | 1 - app/templates/views/check.html | 12 ++++++------ requirements.txt | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 28947bcbb..f18bc8c19 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -237,7 +237,6 @@ def check_messages(service_id, template_type, upload_id): session['upload_data']['notification_count'] = len(list(recipients.rows)) session['upload_data']['valid'] = not recipients.has_errors - return render_template( 'views/check.html', recipients=recipients, diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 3ceeb76c7..5cbcbc4ee 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -109,19 +109,19 @@ ) %} {{ index_field(item.index + 2) }} {% for column in recipients.column_headers %} - {% if item[column].error %} + {% if item['columns'][column].error %} {% call field() %} - {{ item[column].error }} - {{ item[column].data if item[column].data != None }} + {{ item['columns'][column].error }} + {{ item['columns'][column].data if item['columns'][column].data != None }} {% endcall %} - {% elif item[column].ignore %} + {% elif item['columns'][column].ignore %} {% call field(status='default') %} - {{ item[column].data if item[column].data != None }} + {{ item['columns'][column].data if item['columns'][column].data != None }} {% endcall %} {% else %} - {{ text_field(item[column].data) }} + {{ text_field(item['columns'][column].data) }} {% endif %} {% endfor %} {% endcall %} diff --git a/requirements.txt b/requirements.txt index dd73379b3..f05778464 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,4 +9,4 @@ py-gfm==0.1.2 git+https://github.com/alphagov/notifications-python-client.git@0.5.0#egg=notifications-python-client==0.5.0 -git+https://github.com/alphagov/notifications-utils.git@4.3.0#egg=notifications-utils==4.3.0 +git+https://github.com/alphagov/notifications-utils.git@5.1.0#egg=notifications-utils==5.1.0 From 471eac209e334d9963c3aa4adbb7eeede00b1ca8 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Fri, 29 Apr 2016 09:38:59 +0100 Subject: [PATCH 3/3] Handling template content size error. All tests passing. --- app/main/views/templates.py | 70 ++++++++++++++++---------- app/notify_client/user_api_client.py | 2 +- tests/app/main/views/test_templates.py | 69 +++++++++++++++++++++++++ tests/conftest.py | 38 +++++++++++++- 4 files changed, 151 insertions(+), 28 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index eda6f572b..3728e20e9 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -2,6 +2,7 @@ from flask import request, render_template, redirect, url_for, flash, abort from flask_login import login_required from notifications_utils.template import Template +from notifications_python_client.errors import HTTPError from app.main import main from app.utils import user_has_permissions @@ -44,23 +45,31 @@ def view_template(service_id, template_id): @login_required @user_has_permissions('manage_templates', admin_override=True) def add_service_template(service_id, template_type): - if template_type not in ['sms', 'email']: abort(404) form = form_objects[template_type]() - if form.validate_on_submit(): - service_api_client.create_service_template( - form.name.data, - template_type, - form.template_content.data, - service_id, - form.subject.data if hasattr(form, 'subject') else None - ) - return redirect( - url_for('.choose_template', service_id=service_id, template_type=template_type) - ) + try: + service_api_client.create_service_template( + form.name.data, + template_type, + form.template_content.data, + service_id, + form.subject.data if hasattr(form, 'subject') else None + ) + except HTTPError as e: + if e.status_code == 400: + if 'content' in e.message and any(['character count greater than' in x for x in e.message['content']]): + form.template_content.errors.extend(e.message['content']) + else: + raise e + else: + raise e + else: + return redirect( + url_for('.choose_template', service_id=service_id, template_type=template_type) + ) return render_template( 'views/edit-{}-template.html'.format(template_type), @@ -79,20 +88,29 @@ def edit_service_template(service_id, template_id): form = form_objects[template['template_type']](**template) if form.validate_on_submit(): - service_api_client.update_service_template( - template_id, - form.name.data, - template['template_type'], - form.template_content.data, - service_id, - form.subject.data if getattr(form, 'subject', None) else None - ) - return redirect(url_for( - '.choose_template', - service_id=service_id, - template_type=template['template_type'] - )) - + try: + service_api_client.update_service_template( + template_id, + form.name.data, + template['template_type'], + form.template_content.data, + service_id, + form.subject.data if getattr(form, 'subject', None) else None + ) + except HTTPError as e: + if e.status_code == 400: + if 'content' in e.message and any(['character count greater than' in x for x in e.message['content']]): + form.template_content.errors.extend(e.message['content']) + else: + raise e + else: + raise e + else: + return redirect(url_for( + '.choose_template', + service_id=service_id, + template_type=template['template_type'] + )) return render_template( 'views/edit-{}-template.html'.format(template['template_type']), form=form, diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index b4ad5150e..9293e18bd 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -39,7 +39,7 @@ class UserApiClient(BaseAPIClient): try: return self.get_user_by_email(email_address) except HTTPError as e: - if HTTPError.status_code == 404: + if e.status_code == 404: return None def get_users(self): diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 5f36d6b24..fb8166ab3 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -67,6 +67,75 @@ def test_should_redirect_when_saving_a_template(app_, template_id, name, 'sms', content, service_id, None) +def test_should_not_create_too_big_template(app_, + api_user_active, + mock_login, + mock_get_service_template, + mock_get_user, + mock_get_service, + mock_get_user_by_email, + mock_create_service_template_content_too_big, + mock_has_permissions, + fake_uuid): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + service_id = fake_uuid + template_type = 'sms' + data = { + 'name': "new name", + 'template_content': "template content", + 'template_type': template_type, + 'service': service_id + } + resp = client.post(url_for( + '.add_service_template', + service_id=service_id, + template_type=template_type + ), data=data) + + assert resp.status_code == 200 + assert ( + "Content has a character count greater" + " than the limit of 459" + ) in resp.get_data(as_text=True) + + +def test_should_not_update_too_big_template(app_, + api_user_active, + mock_login, + mock_get_service_template, + mock_get_user, + mock_get_service, + mock_get_user_by_email, + mock_update_service_template_400_content_too_big, + mock_has_permissions, + fake_uuid): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + service_id = fake_uuid + template_type = 'sms' + template_id = fake_uuid + data = { + 'id': fake_uuid, + 'name': "new name", + 'template_content': "template content", + 'service': service_id, + 'template_type': 'sms' + } + resp = client.post(url_for( + '.edit_service_template', + service_id=service_id, + template_id=template_id), data=data) + + assert resp.status_code == 200 + assert ( + "Content has a character count greater" + " than the limit of 459" + ) in resp.get_data(as_text=True) + + def test_should_redirect_when_saving_a_template_email(app_, api_user_active, mock_login, diff --git a/tests/conftest.py b/tests/conftest.py index 3a12d25e7..e0d896170 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -207,7 +207,7 @@ def mock_get_service_email_template(mocker): @pytest.fixture(scope='function') def mock_create_service_template(mocker, fake_uuid): - def _create(name, type_, content, service): + def _create(name, type_, content, service, subject=None): template = template_json( fake_uuid, name, type_, content, service) return {'data': template} @@ -229,6 +229,42 @@ def mock_update_service_template(mocker): side_effect=_update) +@pytest.fixture(scope='function') +def mock_create_service_template_content_too_big(mocker): + def _create(name, type_, content, service, subject=None): + json_mock = Mock(return_value={ + 'message': {'content': ["Content has a character count greater than the limit of 459"]}, + 'result': 'error' + }) + resp_mock = Mock(status_code=400, json=json_mock) + http_error = HTTPError( + response=resp_mock, + message={'content': ["Content has a character count greater than the limit of 459"]}) + raise http_error + + return mocker.patch( + 'app.service_api_client.create_service_template', + side_effect=_create) + + +@pytest.fixture(scope='function') +def mock_update_service_template_400_content_too_big(mocker): + def _update(id_, name, type_, content, service, subject=None): + json_mock = Mock(return_value={ + 'message': {'content': ["Content has a character count greater than the limit of 459"]}, + 'result': 'error' + }) + resp_mock = Mock(status_code=400, json=json_mock) + http_error = HTTPError( + response=resp_mock, + message={'content': ["Content has a character count greater than the limit of 459"]}) + raise http_error + + return mocker.patch( + 'app.service_api_client.update_service_template', + side_effect=_update) + + @pytest.fixture(scope='function') def mock_get_service_templates(mocker): uuid1 = str(generate_uuid())