diff --git a/app/template/rest.py b/app/template/rest.py index 752417e0f..168d483ea 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -5,7 +5,6 @@ from flask import ( current_app ) import bleach -from sqlalchemy.exc import IntegrityError from app.dao.templates_dao import ( dao_update_template, @@ -59,14 +58,18 @@ def update_template(service_id, template_id): fetched_template = dao_get_template_by_id_and_service_id(template_id=template_id, service_id=service_id) current_data = dict(template_schema.dump(fetched_template).data.items()) - current_data.update(request.get_json()) - current_data['content'] = _strip_html(current_data['content']) + updated_template = dict(template_schema.dump(fetched_template).data.items()) + updated_template.update(request.get_json()) + updated_template['content'] = _strip_html(updated_template['content']) + # Check if there is a change to make. + if _template_has_not_changed(current_data, updated_template): + return jsonify(data=updated_template), 200 - update_dict, errors = template_schema.load(current_data) + update_dict, errors = template_schema.load(updated_template) if errors: return jsonify(result="error", message=errors), 400 over_limit, json_resp = _content_count_greater_than_limit( - current_data['content'], + updated_template['content'], fetched_template.template_type) if over_limit: return json_resp, 400 @@ -115,3 +118,10 @@ def get_template_versions(service_id, template_id): def _strip_html(content): return bleach.clean(content, tags=[], strip=True) + + +def _template_has_not_changed(current_data, updated_template): + return all( + current_data[key] == updated_template[key] + for key in ('name', 'content', 'subject', 'archived') + ) diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index b7ea2d47e..45e7300e6 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -3,6 +3,7 @@ import random import string from app.models import Template from tests import create_authorization_header +from app.dao.templates_dao import dao_get_template_by_id def test_should_create_a_new_sms_template_for_a_service(notify_api, sample_user, sample_service): @@ -155,29 +156,9 @@ def test_must_have_a_subject_on_an_email_template(notify_api, sample_user, sampl assert json_resp['message'] == {'subject': ['Invalid template subject']} -def test_should_be_able_to_update_a_template(notify_api, sample_user, sample_service): +def test_update_should_update_a_template(notify_api, sample_user, sample_template): with notify_api.test_request_context(): with notify_api.test_client() as client: - data = { - 'name': 'my template', - 'template_type': 'email', - 'subject': 'subject', - 'content': 'template content', - 'service': str(sample_service.id), - 'created_by': str(sample_user.id) - } - data = json.dumps(data) - auth_header = create_authorization_header() - - create_response = client.post( - '/service/{}/template'.format(sample_service.id), - headers=[('Content-Type', 'application/json'), auth_header], - data=data - ) - assert create_response.status_code == 201 - json_resp = json.loads(create_response.get_data(as_text=True)) - assert json_resp['data']['name'] == 'my template' - assert json_resp['data']['version'] == 1 data = { 'content': 'my template has new content ', 'created_by': str(sample_user.id) @@ -186,7 +167,7 @@ def test_should_be_able_to_update_a_template(notify_api, sample_user, sample_ser auth_header = create_authorization_header() update_response = client.post( - '/service/{}/template/{}'.format(sample_service.id, json_resp['data']['id']), + '/service/{}/template/{}'.format(sample_template.service_id, sample_template.id), headers=[('Content-Type', 'application/json'), auth_header], data=data ) @@ -194,10 +175,12 @@ def test_should_be_able_to_update_a_template(notify_api, sample_user, sample_ser 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 alert("foo")' + 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']['version'] == 2 -def test_should_be_able_to_archive_template(notify_api, sample_user, sample_service, sample_template): +def test_should_be_able_to_archive_template(notify_api, sample_template): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { @@ -444,3 +427,19 @@ def test_should_return_all_template_versions_for_service_and_template_id(notify_ assert x['content'] == original_content + '1' else: assert x['content'] == original_content + '2' + + +def test_update_does_not_create_new_version_when_there_is_no_change(notify_api, sample_template): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth_header = create_authorization_header() + data = { + 'template_type': sample_template.template_type, + 'content': sample_template.content, + } + resp = client.post('/service/{}/template/{}'.format(sample_template.service_id, sample_template.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 200 + template = dao_get_template_by_id(sample_template.id) + assert template.version == 1 diff --git a/tests/app/template/test_rest_history.py b/tests/app/template/test_rest_history.py index 5b0c919b6..482b04aad 100644 --- a/tests/app/template/test_rest_history.py +++ b/tests/app/template/test_rest_history.py @@ -1,8 +1,6 @@ import json from datetime import (datetime, date) from flask import url_for -from app.models import Template -from freezegun import freeze_time from app.dao.templates_dao import dao_update_template from tests import create_authorization_header