diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py
index 6483ac766..8fc2f15f6 100644
--- a/app/notifications/process_notifications.py
+++ b/app/notifications/process_notifications.py
@@ -23,7 +23,8 @@ from app.models import (
LETTER_TYPE,
NOTIFICATION_CREATED,
Notification,
- ScheduledNotification
+ ScheduledNotification,
+ CHOOSE_POSTAGE
)
from app.dao.notifications_dao import (
dao_create_notification,
@@ -31,6 +32,8 @@ from app.dao.notifications_dao import (
dao_created_scheduled_notification
)
+from app.dao.templates_dao import dao_get_template_by_id
+
from app.v2.errors import BadRequestError
from app.utils import (
cache_key_for_service_template_counter,
@@ -109,7 +112,11 @@ def persist_notification(
elif notification_type == EMAIL_TYPE:
notification.normalised_to = format_email_address(notification.to)
elif notification_type == LETTER_TYPE:
- notification.postage = service.postage
+ template = dao_get_template_by_id(template_id, template_version)
+ if service.has_permission(CHOOSE_POSTAGE) and template.postage:
+ notification.postage = template.postage
+ else:
+ notification.postage = service.postage
# if simulated create a Notification model to return but do not persist the Notification to the dB
if not simulated:
diff --git a/app/template/rest.py b/app/template/rest.py
index 8cbb4125f..79b1d7d8b 100644
--- a/app/template/rest.py
+++ b/app/template/rest.py
@@ -31,7 +31,7 @@ from app.errors import (
InvalidRequest
)
from app.letters.utils import get_letter_pdf
-from app.models import SMS_TYPE, Template
+from app.models import SMS_TYPE, Template, CHOOSE_POSTAGE
from app.notifications.validators import service_has_permission, check_reply_to
from app.schema_validation import validate
from app.schemas import (template_schema, template_history_schema)
@@ -78,6 +78,12 @@ def create_template(service_id):
errors = {'template_type': [message]}
raise InvalidRequest(errors, 403)
+ if new_template.postage:
+ if not service_has_permission(CHOOSE_POSTAGE, fetched_service.permissions):
+ message = "Setting postage on templates is not enabled for this service."
+ errors = {'template_postage': [message]}
+ raise InvalidRequest(errors, 403)
+
new_template.service = fetched_service
over_limit = _content_count_greater_than_limit(new_template.content, new_template.template_type)
@@ -110,6 +116,12 @@ def update_template(service_id, template_id):
if data.get('redact_personalisation') is True:
return redact_template(fetched_template, data)
+ if data.get('postage'):
+ if not service_has_permission(CHOOSE_POSTAGE, fetched_template.service.permissions):
+ message = "Setting postage on templates is not enabled for this service."
+ errors = {'template_postage': [message]}
+ raise InvalidRequest(errors, 403)
+
if "reply_to" in data:
check_reply_to(service_id, data.get("reply_to"), fetched_template.template_type)
updated = dao_update_template_reply_to(template_id=template_id, reply_to=data.get("reply_to"))
@@ -191,7 +203,7 @@ def get_template_versions(service_id, template_id):
def _template_has_not_changed(current_data, updated_template):
return all(
current_data[key] == updated_template[key]
- for key in ('name', 'content', 'subject', 'archived', 'process_type')
+ for key in ('name', 'content', 'subject', 'archived', 'process_type', 'postage')
)
diff --git a/app/template/template_schemas.py b/app/template/template_schemas.py
index f63ad4575..9f38262a5 100644
--- a/app/template/template_schemas.py
+++ b/app/template/template_schemas.py
@@ -17,7 +17,8 @@ post_create_template_schema = {
"content": {"type": "string"},
"subject": {"type": "string"},
"created_by": uuid,
- "parent_folder_id": uuid
+ "parent_folder_id": uuid,
+ "postage": {"type": "string"},
},
"if": {
"properties": {
diff --git a/app/v2/template/template_schemas.py b/app/v2/template/template_schemas.py
index b1b1f4820..ebd0b8342 100644
--- a/app/v2/template/template_schemas.py
+++ b/app/v2/template/template_schemas.py
@@ -37,6 +37,7 @@ get_template_by_id_response = {
"body": {"type": "string"},
"subject": {"type": ["string", "null"]},
"name": {"type": "string"},
+ "postage": {"type": "string"}
},
"required": ["id", "type", "created_at", "updated_at", "version", "created_by", "body", "name"],
}
@@ -63,7 +64,8 @@ post_template_preview_response = {
"type": {"enum": TEMPLATE_TYPES},
"version": {"type": "integer"},
"body": {"type": "string"},
- "subject": {"type": ["string", "null"]}
+ "subject": {"type": ["string", "null"]},
+ "postage": {"type": "string"}
},
"required": ["id", "type", "version", "body"]
}
@@ -77,5 +79,6 @@ def create_post_template_preview_response(template, template_object):
"type": template.template_type,
"version": template.version,
"body": str(template_object),
- "subject": subject
+ "subject": subject,
+ "postage": template.postage
}
diff --git a/tests/app/conftest.py b/tests/app/conftest.py
index 15f76846f..6b04e83fe 100644
--- a/tests/app/conftest.py
+++ b/tests/app/conftest.py
@@ -158,6 +158,7 @@ def sample_service(
email_from=None,
permissions=None,
research_mode=None,
+ postage="second",
):
if user is None:
user = create_user()
@@ -170,6 +171,7 @@ def sample_service(
'restricted': restricted,
'email_from': email_from,
'created_by': user,
+ "postage": postage,
}
service = Service.query.filter_by(name=service_name).first()
if not service:
diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py
index d86cec7a0..cbe6c6b72 100644
--- a/tests/app/dao/test_templates_dao.py
+++ b/tests/app/dao/test_templates_dao.py
@@ -153,7 +153,7 @@ def test_dao_update_template_reply_to_none_to_some(sample_service, sample_user):
assert template_history.updated_at == updated.updated_at
-def test_dao_update_tempalte_reply_to_some_to_some(sample_service, sample_user):
+def test_dao_update_template_reply_to_some_to_some(sample_service, sample_user):
letter_contact = create_letter_contact(sample_service, 'Edinburgh, ED1 1AA')
letter_contact_2 = create_letter_contact(sample_service, 'London, N1 1DE')
diff --git a/tests/app/db.py b/tests/app/db.py
index 94d6ac287..02a2df85c 100644
--- a/tests/app/db.py
+++ b/tests/app/db.py
@@ -81,6 +81,7 @@ def create_service(
prefix_sms=True,
message_limit=1000,
organisation_type='central',
+ postage='second'
):
service = Service(
name=service_name,
@@ -90,6 +91,7 @@ def create_service(
created_by=user or create_user(email='{}@digital.cabinet-office.gov.uk'.format(uuid.uuid4())),
prefix_sms=prefix_sms,
organisation_type=organisation_type,
+ postage=postage
)
dao_create_service(service, service.created_by, service_id, service_permissions=service_permissions)
@@ -140,6 +142,7 @@ def create_template(
hidden=False,
archived=False,
folder=None,
+ postage=None,
):
data = {
'name': template_name or '{} Template Name'.format(template_type),
@@ -150,6 +153,7 @@ def create_template(
'reply_to': reply_to,
'hidden': hidden,
'folder': folder,
+ 'postage': postage,
}
if template_type != SMS_TYPE:
data['subject'] = subject
diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index 609a863d8..192644bde 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -13,7 +13,9 @@ from app.models import (
Notification,
NotificationHistory,
ScheduledNotification,
- Template
+ Template,
+ LETTER_TYPE,
+ CHOOSE_POSTAGE
)
from app.notifications.process_notifications import (
create_content_for_notification,
@@ -27,6 +29,8 @@ from app.utils import cache_key_for_service_template_counter
from app.v2.errors import BadRequestError
from tests.app.conftest import sample_api_key as create_api_key
+from tests.app.db import create_service, create_template
+
def test_create_content_for_notification_passes(sample_email_template):
template = Template.query.get(sample_email_template.id)
@@ -477,6 +481,41 @@ def test_persist_email_notification_stores_normalised_email(
assert persisted_notification.normalised_to == expected_recipient_normalised
+@pytest.mark.parametrize(
+ "service_permissions, template_postage, expected_postage",
+ [
+ ([LETTER_TYPE], "first", "second"),
+ ([LETTER_TYPE, CHOOSE_POSTAGE], "first", "first"),
+ ([LETTER_TYPE, CHOOSE_POSTAGE], None, "second"),
+ ]
+)
+def test_persist_letter_notification_finds_correct_postage(
+ mocker,
+ notify_db,
+ notify_db_session,
+ service_permissions,
+ template_postage,
+ expected_postage
+):
+ service = create_service(service_permissions=service_permissions, postage="second")
+ api_key = create_api_key(notify_db, notify_db_session, service=service)
+ template = create_template(service, template_type=LETTER_TYPE, postage=template_postage)
+ mocker.patch('app.dao.templates_dao.dao_get_template_by_id', return_value=template)
+ persist_notification(
+ template_id=template.id,
+ template_version=template.version,
+ recipient="Jane Doe, 10 Downing Street, London",
+ service=service,
+ personalisation=None,
+ notification_type=LETTER_TYPE,
+ api_key_id=api_key.id,
+ key_type=api_key.key_type,
+ )
+ persisted_notification = Notification.query.all()[0]
+
+ assert persisted_notification.postage == expected_postage
+
+
@pytest.mark.parametrize('utc_time, day_in_key', [
('2016-01-01 23:00:00', '2016-01-01'),
('2016-06-01 22:59:00', '2016-06-01'),
diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py
index 7cf46aa80..64c51becc 100644
--- a/tests/app/template/test_rest.py
+++ b/tests/app/template/test_rest.py
@@ -18,7 +18,8 @@ from app.models import (
LETTER_TYPE,
SMS_TYPE,
Template,
- TemplateHistory
+ TemplateHistory,
+ CHOOSE_POSTAGE
)
from app.dao.templates_dao import dao_get_template_by_id, dao_redact_template
@@ -43,7 +44,7 @@ from tests.conftest import set_config_values
def test_should_create_a_new_template_for_a_service(
client, sample_user, template_type, subject
):
- service = create_service(service_permissions=[template_type])
+ service = create_service(service_permissions=[template_type, CHOOSE_POSTAGE])
data = {
'name': 'my template',
'template_type': template_type,
@@ -53,6 +54,8 @@ def test_should_create_a_new_template_for_a_service(
}
if subject:
data.update({'subject': subject})
+ if template_type == LETTER_TYPE:
+ data.update({'postage': 'first'})
data = json.dumps(data)
auth_header = create_authorization_header()
@@ -76,6 +79,11 @@ def test_should_create_a_new_template_for_a_service(
else:
assert not json_resp['data']['subject']
+ if template_type == LETTER_TYPE:
+ assert json_resp['data']['postage'] == 'first'
+ else:
+ assert not json_resp['data']['postage']
+
template = Template.query.get(json_resp['data']['id'])
from app.schemas import template_schema
assert sorted(json_resp['data']) == sorted(template_schema.dump(template).data)
@@ -210,6 +218,34 @@ def test_should_raise_error_on_create_if_no_permission(
assert json_resp['message'] == expected_error
+def test_should_raise_error_on_create_if_no_choose_postage_permission(client, sample_user):
+ service = create_service(service_permissions=[LETTER_TYPE])
+ data = {
+ 'name': 'my template',
+ 'template_type': LETTER_TYPE,
+ 'content': 'template content',
+ 'service': str(service.id),
+ 'created_by': str(sample_user.id),
+ 'subject': "Some letter",
+ 'postage': 'first',
+ }
+
+ data = json.dumps(data)
+ auth_header = create_authorization_header()
+
+ response = client.post(
+ '/service/{}/template'.format(service.id),
+ headers=[('Content-Type', 'application/json'), auth_header],
+ data=data
+ )
+ json_resp = json.loads(response.get_data(as_text=True))
+ assert response.status_code == 403
+ assert json_resp['result'] == 'error'
+ assert json_resp['message'] == {
+ "template_postage": ["Setting postage on templates is not enabled for this service."]
+ }
+
+
@pytest.mark.parametrize('template_factory, expected_error', [
(sample_template_without_sms_permission, {'template_type': ['Updating text message templates is not allowed']}),
(sample_template_without_email_permission, {'template_type': ['Updating email templates is not allowed']}),
@@ -239,6 +275,33 @@ def test_should_be_error_on_update_if_no_permission(
assert json_resp['message'] == expected_error
+def test_should_be_error_on_update_if_no_choose_postage_permission(client, sample_user):
+ service = create_service(service_name='some_service', service_permissions=[LETTER_TYPE])
+ template = create_template(service, template_type=LETTER_TYPE)
+ data = {
+ 'content': 'new template content',
+ 'created_by': str(sample_user.id),
+ 'postage': 'first'
+ }
+
+ data = json.dumps(data)
+ auth_header = create_authorization_header()
+
+ update_response = client.post(
+ '/service/{}/template/{}'.format(
+ template.service_id, template.id),
+ headers=[('Content-Type', 'application/json'), auth_header],
+ data=data
+ )
+
+ json_resp = json.loads(update_response.get_data(as_text=True))
+ assert update_response.status_code == 403
+ assert json_resp['result'] == 'error'
+ assert json_resp['message'] == {
+ "template_postage": ["Setting postage on templates is not enabled for this service."]
+ }
+
+
def test_should_error_if_created_by_missing(client, sample_user, sample_service):
service_id = str(sample_service.id)
data = {
@@ -301,16 +364,19 @@ def test_must_have_a_subject_on_an_email_or_letter_template(client, sample_user,
assert json_resp['errors'][0]["message"] == 'subject is a required property'
-def test_update_should_update_a_template(client, sample_user, sample_template):
+def test_update_should_update_a_template(client, sample_user):
+ service = create_service(service_permissions=[LETTER_TYPE, CHOOSE_POSTAGE])
+ template = create_template(service, template_type="letter", postage="second")
data = {
- 'content': 'my template has new content ',
- 'created_by': str(sample_user.id)
+ 'content': 'my template has new content, swell!',
+ 'created_by': str(sample_user.id),
+ 'postage': 'first'
}
data = json.dumps(data)
auth_header = create_authorization_header()
update_response = client.post(
- '/service/{}/template/{}'.format(sample_template.service_id, sample_template.id),
+ '/service/{}/template/{}'.format(service.id, template.id),
headers=[('Content-Type', 'application/json'), auth_header],
data=data
)
@@ -318,10 +384,11 @@ def test_update_should_update_a_template(client, sample_user, sample_template):
assert update_response.status_code == 200
update_json_resp = json.loads(update_response.get_data(as_text=True))
assert update_json_resp['data']['content'] == (
- 'my template has new content '
+ 'my template has new content, swell!'
)
- assert update_json_resp['data']['name'] == sample_template.name
- assert update_json_resp['data']['template_type'] == sample_template.template_type
+ assert update_json_resp['data']['postage'] == 'first'
+ assert update_json_resp['data']['name'] == template.name
+ assert update_json_resp['data']['template_type'] == template.template_type
assert update_json_resp['data']['version'] == 2
diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py
index 4b42af6dc..734928088 100644
--- a/tests/app/v2/notifications/test_post_letter_notifications.py
+++ b/tests/app/v2/notifications/test_post_letter_notifications.py
@@ -20,6 +20,7 @@ from app.models import (
NOTIFICATION_DELIVERED,
NOTIFICATION_PENDING_VIRUS_CHECK,
SMS_TYPE,
+ CHOOSE_POSTAGE
)
from app.schema_validation import validate
from app.v2.errors import RateLimitError
@@ -95,12 +96,23 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo
mock.assert_called_once_with([str(notification.id)], queue=QueueNames.CREATE_LETTERS_PDF)
-@pytest.mark.parametrize('postage', ['first', 'second'])
-def test_post_letter_notification_sets_postage(client, sample_letter_template, mocker, postage):
- sample_letter_template.service.postage = postage
+@pytest.mark.parametrize('service_permissions, service_postage, template_postage, expected_postage', [
+ ([LETTER_TYPE], "second", "first", "second"),
+ ([LETTER_TYPE], "first", "second", "first"),
+ ([LETTER_TYPE], "first", None, "first"),
+ ([LETTER_TYPE, CHOOSE_POSTAGE], "second", "first", "first"),
+ ([LETTER_TYPE, CHOOSE_POSTAGE], "second", None, "second"),
+ ([LETTER_TYPE, CHOOSE_POSTAGE], "second", "second", "second"),
+ ([LETTER_TYPE, CHOOSE_POSTAGE], "first", "second", "second"),
+])
+def test_post_letter_notification_sets_postage(
+ client, notify_db_session, mocker, service_permissions, service_postage, template_postage, expected_postage
+):
+ service = create_service(service_permissions=service_permissions, postage=service_postage)
+ template = create_template(service, template_type="letter", postage=template_postage)
mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async')
data = {
- 'template_id': str(sample_letter_template.id),
+ 'template_id': str(template.id),
'personalisation': {
'address_line_1': 'Her Royal Highness Queen Elizabeth II',
'address_line_2': 'Buckingham Palace',
@@ -110,11 +122,11 @@ def test_post_letter_notification_sets_postage(client, sample_letter_template, m
}
}
- resp_json = letter_request(client, data, service_id=sample_letter_template.service_id)
+ resp_json = letter_request(client, data, service_id=service.id)
assert validate(resp_json, post_letter_response) == resp_json
notification = Notification.query.one()
- assert notification.postage == postage
+ assert notification.postage == expected_postage
@pytest.mark.parametrize('env', [