Merge pull request #522 from alphagov/sms_msg_limit

Sms msg limit

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,

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(
+        try:
-            form.name.data,
+            service_api_client.create_service_template(
-            template_type,
+                form.name.data,
-            form.template_content.data,
+                template_type,
-            service_id,
+                form.template_content.data,
-            form.subject.data if hasattr(form, 'subject') else None
+                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)
+        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(
+        try:
-            template_id,
+            service_api_client.update_service_template(
-            form.name.data,
+                template_id,
-            template['template_type'],
+                form.name.data,
-            form.template_content.data,
+                template['template_type'],
-            service_id,
+                form.template_content.data,
-            form.subject.data if getattr(form, 'subject', None) else None
+                service_id,
-        )
+                form.subject.data if getattr(form, 'subject', None) else None
-        return redirect(url_for(
+            )
-            '.choose_template',
+        except HTTPError as e:
-            service_id=service_id,
+            if e.status_code == 400:
-            template_type=template['template_type']
+                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,

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

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() %}
           <span class="table-field-error">
-            <span class="table-field-error-label">{{ item[column].error }}</span>
+            <span class="table-field-error-label">{{ item['columns'][column].error }}</span>
-            {{ item[column].data if item[column].data != None }}
+            {{ item['columns'][column].data if item['columns'][column].data != None }}
           </span>
         {% 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 %}

requirements.txt

@@ -10,4 +10,4 @@ blinker==1.4
 
 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@5.1.0#egg=notifications-utils==5.1.0

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,

tests/conftest.py

@@ -219,7 +219,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}
@@ -241,6 +241,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())