mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-04 10:21:14 -05:00
Merge pull request #271 from alphagov/limit_msg_size
Hard sms limits added to the api for send_notification and create_tem…
This commit is contained in:
@@ -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,
|
||||
get_character_count_of_content
|
||||
update_notification_reference_by_id
|
||||
)
|
||||
|
||||
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())
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 (
|
||||
dao_fetch_service_by_id
|
||||
)
|
||||
from notifications_utils.template import Template
|
||||
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
|
||||
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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']
|
||||
|
||||
Reference in New Issue
Block a user