Message limit added and all tests passing.

app/celery/tasks.py

@@ -38,8 +38,7 @@ from app.dao.notifications_dao import (
     dao_update_notification,
     delete_notifications_created_more_than_a_week_ago,
     dao_get_notification_statistics_for_service_and_day,
-    update_notification_reference_by_id,
+    update_notification_reference_by_id
-    get_character_count_of_content
 )
 
 from app.dao.jobs_dao import (
@@ -233,7 +232,7 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
             created_at=datetime.strptime(created_at, DATETIME_FORMAT),
             sent_at=sent_at,
             sent_by=client.get_name(),
-            content_char_count=get_character_count_of_content(template.replaced)
+            content_char_count=template.replaced_content_count
         )
 
         dao_create_notification(notification_db_object, TEMPLATE_TYPE_SMS, client.get_name())

app/dao/notifications_dao.py

@@ -22,6 +22,8 @@ from app.models import (
     ProviderStatistics
 )
 
+from notifications_utils.template import get_sms_fragment_count
+
 from app.clients import (
     STATISTICS_FAILURE,
     STATISTICS_DELIVERED,
@@ -31,14 +33,6 @@ from app.clients import (
 from app.dao.dao_utils import transactional
 
 
-def get_character_count_of_content(content, encoding='utf-8'):
-    return len(content.encode(encoding))
-
-
-def get_sms_message_count(char_count):
-    return 1 if char_count <= 160 else math.ceil(float(char_count) / 153)
-
-
 def dao_get_notification_statistics_for_service(service_id, limit_days=None):
     filter = [NotificationStatistics.service_id == service_id]
     if limit_days is not None:
@@ -105,14 +99,14 @@ def dao_create_notification(notification, notification_type, provider):
         service_id=notification.service_id,
         provider=provider
     ).update({'unit_count': ProviderStatistics.unit_count + (
-        1 if notification_type == TEMPLATE_TYPE_EMAIL else get_sms_message_count(notification.content_char_count))})
+        1 if notification_type == TEMPLATE_TYPE_EMAIL else get_sms_fragment_count(notification.content_char_count))})
 
     if update_count == 0:
         provider_stats = ProviderStatistics(
             day=notification.created_at.date(),
             service_id=notification.service_id,
             provider=provider,
-            unit_count=1 if notification_type == TEMPLATE_TYPE_EMAIL else get_sms_message_count(
+            unit_count=1 if notification_type == TEMPLATE_TYPE_EMAIL else get_sms_fragment_count(
                 notification.content_char_count))
         db.session.add(provider_stats)
 

app/notifications/rest.py

@@ -338,6 +338,12 @@ def send_notification(notification_type):
             }
         ), 400
 
+    if template_object.replaced_content_count > current_app.config.get('SMS_CHAR_COUNT_LIMIT'):
+        return jsonify(
+            result="error",
+            message={'content': ['Content has a character count greater than the limit of {}'.format(
+                current_app.config.get('SMS_CHAR_COUNT_LIMIT'))]}), 400
+
     if service.restricted and not allowed_to_send_to(
         notification['to'],
         itertools.chain.from_iterable(

app/template/rest.py

@@ -13,9 +13,8 @@ from app.dao.templates_dao import (
     dao_get_template_by_id_and_service_id,
     dao_get_all_templates_for_service
 )
-from app.dao.services_dao import (
+from notifications_utils.template import Template
-    dao_fetch_service_by_id
+from app.dao.services_dao import dao_fetch_service_by_id
-)
 from app.schemas import template_schema
 
 template = Blueprint('template', __name__, url_prefix='/service/<uuid:service_id>/template')
@@ -25,6 +24,18 @@ from app.errors import register_errors
 register_errors(template)
 
 
+def _content_count_greater_than_limit(content, template_type):
+    template = Template({'content': content, 'template_type': template_type})
+    if template_type == 'sms' and \
+       template.content_count > current_app.config.get('SMS_CHAR_COUNT_LIMIT'):
+        return True, jsonify(
+            result="error",
+            message={'content': ['Content has a character count greater than the limit of {}'.format(
+                current_app.config.get('SMS_CHAR_COUNT_LIMIT'))]}
+        )
+    return False, ''
+
+
 @template.route('', methods=['POST'])
 def create_template(service_id):
     fetched_service = dao_fetch_service_by_id(service_id=service_id)
@@ -33,6 +44,11 @@ def create_template(service_id):
         return jsonify(result="error", message=errors), 400
     new_template.service = fetched_service
     new_template.content = _strip_html(new_template.content)
+    over_limit, json_resp = _content_count_greater_than_limit(
+        new_template.content,
+        new_template.template_type)
+    if over_limit:
+        return json_resp, 400
     try:
         dao_create_template(new_template)
     except IntegrityError as ex:
@@ -40,7 +56,7 @@ def create_template(service_id):
         message = "Failed to create template"
         if "templates_subject_key" in str(ex):
             message = 'Duplicate template subject'
-            return jsonify(result="error", message=[{'subject': message}]), 400
+            return jsonify(result="error", message={'subject': [message]}), 400
         return jsonify(result="error", message=message), 500
 
     return jsonify(data=template_schema.dump(new_template).data), 201
@@ -57,7 +73,11 @@ def update_template(service_id, template_id):
     update_dict, errors = template_schema.load(current_data)
     if errors:
         return jsonify(result="error", message=errors), 400
-
+    over_limit, json_resp = _content_count_greater_than_limit(
+        current_data['content'],
+        fetched_template.template_type)
+    if over_limit:
+        return json_resp, 400
     dao_update_template(update_dict)
     return jsonify(data=template_schema.dump(update_dict).data), 200
 

config.py

@@ -26,6 +26,7 @@ class Config(object):
     VERIFY_CODE_FROM_EMAIL_ADDRESS = os.environ['VERIFY_CODE_FROM_EMAIL_ADDRESS']
     NOTIFY_EMAIL_DOMAIN = os.environ['NOTIFY_EMAIL_DOMAIN']
     PAGE_SIZE = 50
+    SMS_CHAR_COUNT_LIMIT = 495
 
     BROKER_URL = 'sqs://'
     BROKER_TRANSPORT_OPTIONS = {

requirements.txt

@@ -22,4 +22,4 @@ monotonic==0.3
 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.2.0#egg=notifications-utils==4.2.0
+git+https://github.com/alphagov/notifications-utils.git@5.1.0#egg=notifications-utils==5.1.0

tests/app/dao/test_notification_dao.py

@@ -28,11 +28,11 @@ from app.dao.notifications_dao import (
     update_notification_reference_by_id,
     update_notification_status_by_reference,
     dao_get_template_statistics_for_service,
-    get_character_count_of_content,
-    get_sms_message_count,
     get_notifications_for_service
 )
 
+from notifications_utils.template import get_sms_fragment_count
+
 from tests.app.conftest import sample_job
 from tests.app.conftest import sample_notification
 
@@ -1089,18 +1089,6 @@ def test_should_limit_notifications_return_by_day_limit_plus_one(notify_db, noti
     assert len(all_notifications) == 2
 
 
-@pytest.mark.parametrize(
-    "content,encoding,expected_length",
-    [
-        ("The quick brown fox jumped over the lazy dog", "utf-8", 44),
-        ("深", "utf-8", 3),
-        ("'First line.\n", 'utf-8', 13),
-        ("\t\n\r", 'utf-8', 3)
-    ])
-def test_get_character_count_of_content(content, encoding, expected_length):
-    assert get_character_count_of_content(content, encoding=encoding) == expected_length
-
-
 @pytest.mark.parametrize(
     "char_count, expected_sms_fragment_count",
     [
@@ -1114,4 +1102,4 @@ def test_get_character_count_of_content(content, encoding, expected_length):
         (461, 4)
     ])
 def test_sms_fragment_count(char_count, expected_sms_fragment_count):
-    assert get_sms_message_count(char_count) == expected_sms_fragment_count
+    assert get_sms_fragment_count(char_count) == expected_sms_fragment_count

tests/app/notifications/test_rest.py

@@ -1,5 +1,7 @@
 from datetime import datetime
 import uuid
+import random
+import string
 import app.celery.tasks
 from tests import create_authorization_header
 from tests.app.conftest import sample_notification as create_sample_notification
@@ -7,7 +9,7 @@ from tests.app.conftest import sample_job as create_sample_job
 from tests.app.conftest import sample_service as create_sample_service
 from tests.app.conftest import sample_email_template as create_sample_email_template
 from tests.app.conftest import sample_template as create_sample_template
-from flask import json
+from flask import (json, current_app, url_for)
 from app.models import Service
 from app.dao.templates_dao import dao_get_all_templates_for_service
 from app.dao.services_dao import dao_update_service
@@ -764,6 +766,37 @@ def test_should_not_allow_template_from_another_service(notify_api, service_fact
             assert test_string in json_resp['message']
 
 
+def test_should_not_allow_template_content_too_large(notify_api, notify_db, notify_db_session, sample_user):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            template = create_sample_template(notify_db, notify_db_session, content="((long_text))")
+            limit = current_app.config.get('SMS_CHAR_COUNT_LIMIT')
+            json_data = json.dumps({
+                'to': sample_user.mobile_number,
+                'template': template.id,
+                'personalisation': {
+                    'long_text': ''.join(
+                        random.choice(string.ascii_uppercase + string.digits) for _ in range(limit + 1))
+                }
+            })
+            endpoint = url_for('notifications.send_notification', notification_type='sms')
+            auth_header = create_authorization_header(
+                service_id=template.service.id,
+                request_body=json_data,
+                path=endpoint,
+                method='POST')
+
+            resp = client.post(
+                path=endpoint,
+                data=json_data,
+                headers=[('Content-Type', 'application/json'), auth_header])
+            assert resp.status_code == 400
+            json_resp = json.loads(resp.get_data(as_text=True))
+            assert json_resp['message']['content'][0] == (
+                'Content has a character count greater'
+                ' than the limit of {}').format(limit)
+
+
 @freeze_time("2016-01-01 11:09:00.061258")
 def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker):
     with notify_api.test_request_context():

tests/app/template/test_rest.py

@@ -1,5 +1,7 @@
 import json
 import uuid
+import random
+import string
 from app.models import Template
 from tests import create_authorization_header
 
@@ -224,7 +226,7 @@ def test_must_have_a_uniqe_subject_on_an_email_template(notify_api, sample_user,
             assert response.status_code == 400
             json_resp = json.loads(response.get_data(as_text=True))
             assert json_resp['result'] == 'error'
-            assert json_resp['message'][0]['subject'] == 'Duplicate template subject'
+            assert json_resp['message']['subject'][0] == 'Duplicate template subject'
 
 
 def test_should_be_able_to_update_a_template(notify_api, sample_user, sample_service):
@@ -479,3 +481,59 @@ def test_should_return_404_if_no_templates_for_service_with_id(notify_api, sampl
             json_resp = json.loads(response.get_data(as_text=True))
             assert json_resp['result'] == 'error'
             assert json_resp['message'] == 'No result found'
+
+
+def test_create_400_for_over_limit_content(notify_api, sample_user, sample_service, fake_uuid):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            limit = notify_api.config.get('SMS_CHAR_COUNT_LIMIT')
+            content = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(limit + 1))
+            data = {
+                'name': 'too big template',
+                'template_type': 'sms',
+                'content': content,
+                'service': str(sample_service.id),
+                'created_by': str(sample_user.id)
+            }
+            data = json.dumps(data)
+            auth_header = create_authorization_header(
+                path='/service/{}/template'.format(sample_service.id),
+                method='POST',
+                request_body=data
+            )
+
+            response = client.post(
+                '/service/{}/template'.format(sample_service.id),
+                headers=[('Content-Type', 'application/json'), auth_header],
+                data=data
+            )
+            assert response.status_code == 400
+            json_resp = json.loads(response.get_data(as_text=True))
+            assert (
+                'Content has a character count greater than the limit of {}'
+            ).format(limit) in json_resp['message']['content']
+
+
+def test_update_400_for_over_limit_content(notify_api, sample_user, sample_template):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            limit = notify_api.config.get('SMS_CHAR_COUNT_LIMIT')
+            json_data = json.dumps({
+                'content': ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(limit + 1)),
+                'created_by': str(sample_user.id)
+            })
+            auth_header = create_authorization_header(
+                path='/service/{}/template/{}'.format(sample_template.service.id, sample_template.id),
+                method='POST',
+                request_body=json_data
+            )
+            resp = client.post(
+                '/service/{}/template/{}'.format(sample_template.service.id, sample_template.id),
+                headers=[('Content-Type', 'application/json'), auth_header],
+                data=json_data
+            )
+            assert resp.status_code == 400
+            json_resp = json.loads(resp.get_data(as_text=True))
+            assert (
+                'Content has a character count greater than the limit of {}'
+            ).format(limit) in json_resp['message']['content']