diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index fa0c34d45..bb42be1cb 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -1,16 +1,25 @@ +import uuid from app import db from app.models import (Template, Service) from sqlalchemy import asc +from app.dao.dao_utils import ( + transactional, + version_class +) + +@transactional +@version_class(Template) def dao_create_template(template): + template.id = uuid.uuid4() # must be set now so version history model can use same id db.session.add(template) - db.session.commit() +@transactional +@version_class(Template) def dao_update_template(template): db.session.add(template) - db.session.commit() def dao_get_template_by_id_and_service_id(template_id, service_id): diff --git a/app/errors.py b/app/errors.py index 7617e0add..0ef24a5f0 100644 --- a/app/errors.py +++ b/app/errors.py @@ -59,4 +59,11 @@ def register_errors(blueprint): @blueprint.app_errorhandler(SQLAlchemyError) def db_error(e): current_app.logger.exception(e) - return jsonify(result='error', message=str(e)), 500 + if e.orig.pgerror and \ + ('duplicate key value violates unique constraint "services_name_key"' in e.orig.pgerror or + 'duplicate key value violates unique constraint "services_email_from_key"' in e.orig.pgerror): + return jsonify( + result='error', + message={'name': ["Duplicate service name '{}'".format(e.params.get('name', ''))]} + ), 400 + return jsonify(result='error', message="Internal server error"), 500 diff --git a/app/history_meta.py b/app/history_meta.py index 972ec0329..a7ee0f3d9 100644 --- a/app/history_meta.py +++ b/app/history_meta.py @@ -180,7 +180,6 @@ def create_history(obj): obj_state = attributes.instance_state(obj) data = {} - for prop in obj_mapper.iterate_properties: # expired object attributes and also deferred cols might not diff --git a/app/models.py b/app/models.py index a380b0ccd..a240d8fae 100644 --- a/app/models.py +++ b/app/models.py @@ -152,7 +152,7 @@ TEMPLATE_TYPE_LETTER = 'letter' TEMPLATE_TYPES = [TEMPLATE_TYPE_SMS, TEMPLATE_TYPE_EMAIL, TEMPLATE_TYPE_LETTER] -class Template(db.Model): +class Template(db.Model, Versioned): __tablename__ = 'templates' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) @@ -174,6 +174,8 @@ class Template(db.Model): service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, unique=False, nullable=False) service = db.relationship('Service', backref=db.backref('templates', lazy='dynamic')) subject = db.Column(db.Text, index=False, unique=True, nullable=True) + created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) + created_by = db.relationship('User') MMG_PROVIDER = "mmg" diff --git a/app/schemas.py b/app/schemas.py index 428248e29..9818e6a97 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -7,8 +7,9 @@ from marshmallow import ( validates_schema, pre_load ) - +from sqlalchemy.dialects.postgresql import UUID from marshmallow_sqlalchemy import field_for +from marshmallow_sqlalchemy.convert import ModelConverter from notifications_utils.recipients import ( validate_email_address, @@ -35,17 +36,6 @@ class BaseSchema(ma.ModelSchema): self.load_json = load_json super(BaseSchema, self).__init__(*args, **kwargs) - __envelope__ = { - 'single': None, - 'many': None - } - - def get_envelope_key(self, many): - """Helper to get the envelope key.""" - key = self.__envelope__['many'] if many else self.__envelope__['single'] - assert key is not None, "Envelope key undefined" - return key - @post_load def make_instance(self, data): """Deserialize data to an instance of the model. Update an existing row @@ -58,6 +48,25 @@ class BaseSchema(ma.ModelSchema): return super(BaseSchema, self).make_instance(data) +class CreatedBySchema(ma.Schema): + + created_by = fields.Str(required=True, load_only=True) + + @validates_schema + def validates_created_by(self, data): + try: + if not isinstance(data.get('created_by'), models.User): + created_by = models.User.query.filter_by(id=data.get('created_by')).one() + except: + raise ValidationError('Invalid template created_by: {}'.format(data)) + + @post_load + def format_created_by(self, item): + if not isinstance(item.get('created_by'), models.User): + item['created_by'] = models.User.query.filter_by(id=item.get('created_by')).one() + return item + + class UserSchema(BaseSchema): permissions = fields.Method("user_permissions", dump_only=True) @@ -80,7 +89,7 @@ class UserSchema(BaseSchema): "_password", "verify_codes") -class ServiceSchema(BaseSchema): +class ServiceSchema(BaseSchema, CreatedBySchema): class Meta: model = models.Service exclude = ("updated_at", "created_at", "api_keys", "templates", "jobs", 'old_id') @@ -92,12 +101,13 @@ class NotificationModelSchema(BaseSchema): class BaseTemplateSchema(BaseSchema): + class Meta: model = models.Template exclude = ("updated_at", "created_at", "service_id", "jobs") -class TemplateSchema(BaseTemplateSchema): +class TemplateSchema(BaseTemplateSchema, CreatedBySchema): @validates_schema def validate_type(self, data): @@ -113,7 +123,7 @@ class NotificationsStatisticsSchema(BaseSchema): model = models.NotificationStatistics -class ApiKeySchema(BaseSchema): +class ApiKeySchema(BaseSchema, CreatedBySchema): class Meta: model = models.ApiKey exclude = ("service", "secret") @@ -303,6 +313,18 @@ class ApiKeyHistorySchema(ma.Schema): created_by_id = fields.UUID() +class TemplateHistorySchema(ma.Schema): + id = fields.UUID() + name = fields.String() + template_type = fields.String() + created_at = fields.DateTime() + updated_at = fields.DateTime() + content = fields.String() + service_id = fields.UUID() + subject = fields.String() + created_by_id = fields.UUID() + + user_schema = UserSchema() user_schema_load_json = UserSchema(load_json=True) service_schema = ServiceSchema() @@ -329,3 +351,4 @@ notifications_filter_schema = NotificationsFilterSchema() template_statistics_schema = TemplateStatisticsSchema() service_history_schema = ServiceHistorySchema() api_key_history_schema = ApiKeyHistorySchema() +template_history_schema = TemplateHistorySchema() diff --git a/app/service/rest.py b/app/service/rest.py index 7a93a938b..0d13d18b5 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -185,9 +185,13 @@ def _process_permissions(user, service, permission_groups): # goes into how we want to fetch and view various items in history # tables. This is so product owner can pass stories as done @service.route('//history', methods=['GET']) -def get_service_and_api_key_history(service_id): - from app.models import Service, ApiKey - from app.schemas import service_history_schema, api_key_history_schema +def get_service_history(service_id): + from app.models import (Service, ApiKey, Template) + from app.schemas import ( + service_history_schema, + api_key_history_schema, + template_history_schema + ) service_history = Service.get_history_model().query.filter_by(id=service_id).all() service_data, errors = service_history_schema.dump(service_history, many=True) @@ -200,6 +204,12 @@ def get_service_and_api_key_history(service_id): if errors: return jsonify(result="error", message=errors), 400 - data = {'service_history': service_data, 'api_key_history': api_keys_data} + template_history = Template.get_history_model().query.filter_by(service_id=service_id).all() + template_data, errors = template_history_schema.dump(template_history, many=True) + + data = { + 'service_history': service_data, + 'api_key_history': api_keys_data, + 'template_history': template_data} return jsonify(data=data) diff --git a/app/template/rest.py b/app/template/rest.py index 15a4bcd51..26fd09bcf 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -28,7 +28,6 @@ register_errors(template) @template.route('', methods=['POST']) def create_template(service_id): fetched_service = dao_fetch_service_by_id(service_id=service_id) - new_template, errors = template_schema.load(request.get_json()) if errors: return jsonify(result="error", message=errors), 400 diff --git a/migrations/versions/0007_template_history.py b/migrations/versions/0007_template_history.py new file mode 100644 index 000000000..73a096e73 --- /dev/null +++ b/migrations/versions/0007_template_history.py @@ -0,0 +1,70 @@ +"""empty message + +Revision ID: 0007_template_history +Revises: 0006_api_keys_history +Create Date: 2016-04-22 09:51:55.615891 + +""" + +# revision identifiers, used by Alembic. +revision = '0007_template_history' +down_revision = '0006_api_keys_history' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.create_table('templates_history', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('name', sa.String(length=255), nullable=False), + sa.Column('template_type', sa.String(), nullable=False), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=True), + sa.Column('content', sa.Text(), nullable=False), + sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('subject', sa.Text(), nullable=True), + sa.Column('created_by_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('version', sa.Integer(), autoincrement=False, nullable=False), + sa.PrimaryKeyConstraint('id', 'version') + ) + op.create_index(op.f('ix_templates_history_created_by_id'), 'templates_history', ['created_by_id'], unique=False) + op.create_index(op.f('ix_templates_history_service_id'), 'templates_history', ['service_id'], unique=False) + op.add_column('templates', sa.Column('created_by_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.add_column('templates', sa.Column('version', sa.Integer(), nullable=True)) + + + op.get_bind() + op.execute('UPDATE templates SET created_by_id = (SELECT user_id FROM user_to_service WHERE templates.service_id = user_to_service.service_id LIMIT 1)') + op.execute('UPDATE templates SET version = 1, created_at = now()') + op.execute(( + 'INSERT INTO templates_history (id, name, template_type, created_at, updated_at, content, service_id, subject, version)' + ' SELECT id, name, template_type, created_at, updated_at, content, service_id, subject, version FROM templates')) + + + op.alter_column('templates', 'created_at', nullable=False) + op.alter_column('templates', 'created_by_id', nullable=False) + op.alter_column('templates', 'version', nullable=False) + + op.create_index(op.f('ix_templates_created_by_id'), 'templates', ['created_by_id'], unique=False) + op.create_foreign_key("fk_templates_created_by_id", 'templates', 'users', ['created_by_id'], ['id']) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint("fk_templates_created_by_id", 'templates', type_='foreignkey') + op.drop_index(op.f('ix_templates_created_by_id'), table_name='templates') + op.drop_column('templates', 'version') + op.drop_column('templates', 'created_by_id') + op.alter_column('api_keys_history', 'created_by_id', + existing_type=postgresql.UUID(), + nullable=True) + op.alter_column('api_keys_history', 'created_at', + existing_type=postgresql.TIMESTAMP(), + nullable=True) + op.drop_index(op.f('ix_templates_history_service_id'), table_name='templates_history') + op.drop_index(op.f('ix_templates_history_created_by_id'), table_name='templates_history') + op.drop_table('templates_history') + ### end Alembic commands ### diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index 8307fc381..c32cfa14b 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -132,10 +132,11 @@ def test_should_allow_valid_token_with_post_body(notify_api, sample_api_key): data = { 'email_from': 'new name', 'name': 'new name', - 'users': [service.users[0].id], + 'users': [str(service.users[0].id)], 'message_limit': 1000, 'restricted': False, - 'active': False} + 'active': False, + 'created_by': str(service.users[0].id)} token = create_jwt_token( request_method="POST", diff --git a/tests/app/conftest.py b/tests/app/conftest.py index f14f8cdf1..de319e9c1 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -135,7 +135,10 @@ def sample_template(notify_db, template_type="sms", content="This is a template", subject_line='Subject', + user=None, service=None): + if user is None: + user = sample_user(notify_db, notify_db_session) if service is None: service = sample_service(notify_db, notify_db_session) sample_api_key(notify_db, notify_db_session, service=service) @@ -143,7 +146,8 @@ def sample_template(notify_db, 'name': template_name, 'template_type': template_type, 'content': content, - 'service': service + 'service': service, + 'created_by': user } if template_type == 'email': data.update({ @@ -165,9 +169,12 @@ def sample_email_template( notify_db_session, template_name="Email Template Name", template_type="email", + user=None, content="This is a template", subject_line='Email Subject', service=None): + if user is None: + user = sample_user(notify_db, notify_db_session) if service is None: service = sample_service(notify_db, notify_db_session) sample_api_key(notify_db, notify_db_session, service=service) @@ -175,7 +182,8 @@ def sample_email_template( 'name': template_name, 'template_type': template_type, 'content': content, - 'service': service + 'service': service, + 'created_by': user } if subject_line: data.update({ diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index 2c680fac3..dbd163820 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -11,12 +11,13 @@ from app.models import Template import pytest -def test_create_template(sample_service): +def test_create_template(sample_service, sample_user): data = { 'name': 'Sample Template', 'template_type': "sms", 'content': "Template content", - 'service': sample_service + 'service': sample_service, + 'created_by': sample_user } template = Template(**data) dao_create_template(template) @@ -26,13 +27,14 @@ def test_create_template(sample_service): assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template' -def test_create_email_template(sample_service): +def test_create_email_template(sample_service, sample_user): data = { 'name': 'Sample Template', 'template_type': "email", 'subject': "subject", 'content': "Template content", - 'service': sample_service + 'service': sample_service, + 'created_by': sample_user } template = Template(**data) dao_create_template(template) @@ -42,12 +44,13 @@ def test_create_email_template(sample_service): assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template' -def test_update_template(sample_service): +def test_update_template(sample_service, sample_user): data = { 'name': 'Sample Template', 'template_type': "sms", 'content': "Template content", - 'service': sample_service + 'service': sample_service, + 'created_by': sample_user } template = Template(**data) dao_create_template(template) @@ -59,7 +62,7 @@ def test_update_template(sample_service): assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'new name' -def test_get_all_templates_for_service(service_factory): +def test_get_all_templates_for_service(notify_db, notify_db_session, service_factory): service_1 = service_factory.get('service 1', email_from='service.1') service_2 = service_factory.get('service 2', email_from='service.2') @@ -67,55 +70,61 @@ def test_get_all_templates_for_service(service_factory): assert len(dao_get_all_templates_for_service(service_1.id)) == 1 assert len(dao_get_all_templates_for_service(service_2.id)) == 1 - template_1 = Template( - name='Sample Template 1', + template_1 = create_sample_template( + notify_db, + notify_db_session, + template_name='Sample Template 1', template_type="sms", content="Template content", service=service_1 ) - template_2 = Template( - name='Sample Template 2', + template_2 = create_sample_template( + notify_db, + notify_db_session, + template_name='Sample Template 2', template_type="sms", content="Template content", service=service_1 ) - template_3 = Template( - name='Sample Template 3', + template_3 = create_sample_template( + notify_db, + notify_db_session, + template_name='Sample Template 3', template_type="sms", content="Template content", service=service_2 ) - dao_create_template(template_1) - dao_create_template(template_2) - dao_create_template(template_3) assert Template.query.count() == 5 assert len(dao_get_all_templates_for_service(service_1.id)) == 3 assert len(dao_get_all_templates_for_service(service_2.id)) == 2 -def test_get_all_templates_for_service_in_created_order(sample_service): - template_1 = Template( - name='Sample Template 1', +def test_get_all_templates_for_service_in_created_order(notify_db, notify_db_session, sample_service): + template_1 = create_sample_template( + notify_db, + notify_db_session, + template_name='Sample Template 1', template_type="sms", content="Template content", service=sample_service ) - template_2 = Template( - name='Sample Template 2', + template_2 = create_sample_template( + notify_db, + notify_db_session, + template_name='Sample Template 2', template_type="sms", content="Template content", service=sample_service ) - template_3 = Template( - name='Sample Template 3', + template_3 = create_sample_template( + notify_db, + notify_db_session, + template_name='Sample Template 3', template_type="sms", content="Template content", service=sample_service ) - dao_create_template(template_1) - dao_create_template(template_2) - dao_create_template(template_3) assert Template.query.count() == 3 assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template 1' @@ -148,3 +157,64 @@ def test_get_template_by_id_and_service_returns_none_if_no_template(sample_servi with pytest.raises(NoResultFound) as e: dao_get_template_by_id_and_service_id(template_id=fake_uuid, service_id=sample_service.id) assert 'No row was found for one' in str(e.value) + + +def test_create_template_creates_a_history_record_with_current_data(sample_service, sample_user): + assert Template.query.count() == 0 + assert Template.get_history_model().query.count() == 0 + data = { + 'name': 'Sample Template', + 'template_type': "email", + 'subject': "subject", + 'content': "Template content", + 'service': sample_service, + 'created_by': sample_user + } + template = Template(**data) + dao_create_template(template) + + assert Template.query.count() == 1 + + template_from_db = Template.query.first() + template_history = Template.get_history_model().query.first() + + assert template_from_db.id == template_history.id + assert template_from_db.name == template_history.name + assert template_from_db.version == 1 + assert template_from_db.version == template_history.version + assert sample_user.id == template_history.created_by_id + assert template_from_db.created_by.id == template_history.created_by_id + + +def test_update_template_creates_a_history_record_with_current_data(sample_service, sample_user): + assert Template.query.count() == 0 + assert Template.get_history_model().query.count() == 0 + data = { + 'name': 'Sample Template', + 'template_type': "email", + 'subject': "subject", + 'content': "Template content", + 'service': sample_service, + 'created_by': sample_user + } + template = Template(**data) + dao_create_template(template) + + created = dao_get_all_templates_for_service(sample_service.id)[0] + assert created.name == 'Sample Template' + assert Template.query.count() == 1 + assert Template.query.first().version == 1 + assert Template.get_history_model().query.count() == 1 + + created.name = 'new name' + dao_update_template(created) + + assert Template.query.count() == 1 + assert Template.get_history_model().query.count() == 2 + + template_from_db = Template.query.first() + + assert template_from_db.version == 2 + + assert Template.get_history_model().query.filter_by(name='Sample Template').one().version == 1 + assert Template.get_history_model().query.filter_by(name='new name').one().version == 2 diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index fad658901..cfb88c911 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -486,10 +486,10 @@ def test_filter_by_status_and_template_type(notify_api, headers=[auth_header]) notifications = json.loads(response.get_data(as_text=True)) + assert response.status_code == 200 assert len(notifications['notifications']) == 1 assert notifications['notifications'][0]['template']['template_type'] == 'email' assert notifications['notifications'][0]['status'] == 'delivered' - assert response.status_code == 200 def test_get_notification_statistics( diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index efb34c28b..8440afe4d 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -240,6 +240,33 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_u assert 'Missing data for required field.' in json_resp['message']['user_id'] +def test_should_error_if_created_by_missing(notify_api, sample_user): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = { + 'email_from': 'service', + 'name': 'created service', + 'message_limit': 1000, + 'restricted': False, + 'active': False, + 'user_id': str(sample_user.id) + } + auth_header = create_authorization_header( + path='/service', + method='POST', + request_body=json.dumps(data) + ) + headers = [('Content-Type', 'application/json'), auth_header] + resp = client.post( + '/service', + data=json.dumps(data), + headers=headers) + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 400 + assert json_resp['result'] == 'error' + assert 'Missing data for required field.' in json_resp['message']['created_by'] + + def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(notify_api, notify_db, notify_db_session, @@ -321,8 +348,9 @@ def test_should_not_create_service_with_duplicate_name(notify_api, '/service', data=json.dumps(data), headers=headers) - assert resp.status_code == 500 - assert 'Key (name)=(Sample service) already exists.' in resp.get_data(as_text=True) + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['result'] == 'error' + assert "Duplicate service name '{}'".format(sample_service.name) in json_resp['message']['name'] def test_create_service_should_throw_duplicate_key_constraint_for_existing_email_from(notify_api, @@ -333,8 +361,9 @@ def test_create_service_should_throw_duplicate_key_constraint_for_existing_email first_service = service_factory.get('First service', email_from='first.service') with notify_api.test_request_context(): with notify_api.test_client() as client: + service_name = 'First SERVICE' data = { - 'name': 'First SERVICE', + 'name': service_name, 'user_id': str(first_service.users[0].id), 'message_limit': 1000, 'restricted': False, @@ -351,8 +380,9 @@ def test_create_service_should_throw_duplicate_key_constraint_for_existing_email '/service', data=json.dumps(data), headers=headers) - assert resp.status_code == 500 - assert 'Key (email_from)=(first.service) already exists.' in resp.get_data(as_text=True) + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['result'] == 'error' + assert "Duplicate service name '{}'".format(service_name) in json_resp['message']['name'] def test_update_service(notify_api, sample_service): @@ -372,7 +402,8 @@ def test_update_service(notify_api, sample_service): data = { 'name': 'updated service name', - 'email_from': 'updated.service.name' + 'email_from': 'updated.service.name', + 'created_by': str(sample_service.created_by.id) } auth_header = create_authorization_header( @@ -400,14 +431,15 @@ def test_should_not_update_service_with_duplicate_name(notify_api, with notify_api.test_request_context(): with notify_api.test_client() as client: service_name = "another name" - create_sample_service( + service = create_sample_service( notify_db, notify_db_session, service_name=service_name, user=sample_user, email_from='another.name') data = { - 'name': service_name + 'name': service_name, + 'created_by': str(service.created_by.id) } auth_header = create_authorization_header( @@ -421,8 +453,10 @@ def test_should_not_update_service_with_duplicate_name(notify_api, data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header] ) - assert resp.status_code == 500 - assert 'Key (name)=(another name) already exists.' in resp.get_data(as_text=True) + assert resp.status_code == 400 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['result'] == 'error' + assert "Duplicate service name '{}'".format(service_name) in json_resp['message']['name'] def test_should_not_update_service_with_duplicate_email_from(notify_api, @@ -433,14 +467,17 @@ def test_should_not_update_service_with_duplicate_email_from(notify_api, with notify_api.test_request_context(): with notify_api.test_client() as client: email_from = "duplicate.name" - create_sample_service( + service_name = "duplicate name" + service = create_sample_service( notify_db, notify_db_session, - service_name="duplicate name", + service_name=service_name, user=sample_user, email_from=email_from) data = { - 'email_from': email_from + 'name': service_name, + 'email_from': email_from, + 'created_by': str(service.created_by.id) } auth_header = create_authorization_header( @@ -454,8 +491,10 @@ def test_should_not_update_service_with_duplicate_email_from(notify_api, data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header] ) - assert resp.status_code == 500 - assert 'Key (email_from)=(duplicate.name) already exists.' in resp.get_data(as_text=True) + assert resp.status_code == 400 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['result'] == 'error' + assert "Duplicate service name '{}'".format(service_name) in json_resp['message']['name'] def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db, notify_db_session): diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 13551afa9..40c6c5d44 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -4,14 +4,15 @@ import uuid from tests import create_authorization_header -def test_should_create_a_new_sms_template_for_a_service(notify_api, sample_service): +def test_should_create_a_new_sms_template_for_a_service(notify_api, sample_user, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { 'name': 'my template', 'template_type': 'sms', 'content': 'template content', - 'service': str(sample_service.id) + 'service': str(sample_service.id), + 'created_by': str(sample_user.id) } data = json.dumps(data) auth_header = create_authorization_header( @@ -35,7 +36,7 @@ def test_should_create_a_new_sms_template_for_a_service(notify_api, sample_servi assert not json_resp['data']['subject'] -def test_should_create_a_new_email_template_for_a_service(notify_api, sample_service): +def test_should_create_a_new_email_template_for_a_service(notify_api, sample_user, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { @@ -43,7 +44,8 @@ def test_should_create_a_new_email_template_for_a_service(notify_api, sample_ser 'template_type': 'email', 'subject': 'subject', 'content': 'template content', - 'service': str(sample_service.id) + 'service': str(sample_service.id), + 'created_by': str(sample_user.id) } data = json.dumps(data) auth_header = create_authorization_header( @@ -67,14 +69,15 @@ def test_should_create_a_new_email_template_for_a_service(notify_api, sample_ser assert json_resp['data']['id'] -def test_should_be_error_if_service_does_not_exist_on_create(notify_api, fake_uuid): +def test_should_be_error_if_service_does_not_exist_on_create(notify_api, sample_user, fake_uuid): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { 'name': 'my template', 'template_type': 'sms', 'content': 'template content', - 'service': fake_uuid + 'service': fake_uuid, + 'created_by': str(sample_user.id) } data = json.dumps(data) auth_header = create_authorization_header( @@ -94,6 +97,33 @@ def test_should_be_error_if_service_does_not_exist_on_create(notify_api, fake_uu assert json_resp['message'] == 'No result found' +def test_should_error_if_created_by_missing(notify_api, sample_user, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + service_id = str(sample_service.id) + data = { + 'name': 'my template', + 'template_type': 'sms', + 'content': 'template content', + 'service': service_id + } + data = json.dumps(data) + auth_header = create_authorization_header( + path='/service/{}/template'.format(service_id), + method='POST', + request_body=data + ) + + 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 == 400 + assert json_resp['result'] == 'error' + + def test_should_be_error_if_service_does_not_exist_on_update(notify_api, fake_uuid): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -118,14 +148,15 @@ def test_should_be_error_if_service_does_not_exist_on_update(notify_api, fake_uu assert json_resp['message'] == 'No result found' -def test_must_have_a_subject_on_an_email_template(notify_api, sample_service): +def test_must_have_a_subject_on_an_email_template(notify_api, sample_user, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { 'name': 'my template', 'template_type': 'email', 'content': 'template content', - 'service': str(sample_service.id) + 'service': str(sample_service.id), + 'created_by': str(sample_user.id) } data = json.dumps(data) auth_header = create_authorization_header( @@ -145,7 +176,7 @@ def test_must_have_a_subject_on_an_email_template(notify_api, sample_service): assert json_resp['message'] == {'subject': ['Invalid template subject']} -def test_must_have_a_uniqe_subject_on_an_email_template(notify_api, sample_service): +def test_must_have_a_uniqe_subject_on_an_email_template(notify_api, sample_user, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { @@ -153,7 +184,8 @@ def test_must_have_a_uniqe_subject_on_an_email_template(notify_api, sample_servi 'template_type': 'email', 'subject': 'subject', 'content': 'template content', - 'service': str(sample_service.id) + 'service': str(sample_service.id), + 'created_by': str(sample_user.id) } data = json.dumps(data) auth_header = create_authorization_header( @@ -174,7 +206,8 @@ def test_must_have_a_uniqe_subject_on_an_email_template(notify_api, sample_servi 'template_type': 'email', 'subject': 'subject', 'content': 'template content', - 'service': str(sample_service.id) + 'service': str(sample_service.id), + 'created_by': str(sample_user.id) } data = json.dumps(data) auth_header = create_authorization_header( @@ -194,7 +227,7 @@ def test_must_have_a_uniqe_subject_on_an_email_template(notify_api, sample_servi assert json_resp['message'][0]['subject'] == 'Duplicate template subject' -def test_should_be_able_to_update_a_template(notify_api, sample_service): +def test_should_be_able_to_update_a_template(notify_api, sample_user, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { @@ -202,7 +235,8 @@ def test_should_be_able_to_update_a_template(notify_api, sample_service): 'template_type': 'email', 'subject': 'subject', 'content': 'template content', - 'service': str(sample_service.id) + 'service': str(sample_service.id), + 'created_by': str(sample_user.id) } data = json.dumps(data) auth_header = create_authorization_header( @@ -220,7 +254,8 @@ def test_should_be_able_to_update_a_template(notify_api, sample_service): json_resp = json.loads(create_response.get_data(as_text=True)) assert json_resp['data']['name'] == 'my template' data = { - 'content': 'my template has new content ' + 'content': 'my template has new content ', + 'created_by': str(sample_user.id) } data = json.dumps(data) auth_header = create_authorization_header( @@ -240,7 +275,7 @@ def test_should_be_able_to_update_a_template(notify_api, sample_service): assert update_json_resp['data']['content'] == 'my template has new content alert("foo")' -def test_should_be_able_to_get_all_templates_for_a_service(notify_api, sample_service): +def test_should_be_able_to_get_all_templates_for_a_service(notify_api, sample_user, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { @@ -248,7 +283,8 @@ def test_should_be_able_to_get_all_templates_for_a_service(notify_api, sample_se 'template_type': 'email', 'subject': 'subject 1', 'content': 'template content', - 'service': str(sample_service.id) + 'service': str(sample_service.id), + 'created_by': str(sample_user.id) } data_1 = json.dumps(data) data = { @@ -256,7 +292,8 @@ def test_should_be_able_to_get_all_templates_for_a_service(notify_api, sample_se 'template_type': 'email', 'subject': 'subject 2', 'content': 'template content', - 'service': str(sample_service.id) + 'service': str(sample_service.id), + 'created_by': str(sample_user.id) } data_2 = json.dumps(data) auth_header = create_authorization_header( @@ -297,7 +334,7 @@ def test_should_be_able_to_get_all_templates_for_a_service(notify_api, sample_se assert update_json_resp['data'][1]['name'] == 'my template 2' -def test_should_get_only_templates_for_that_servcie(notify_api, service_factory): +def test_should_get_only_templates_for_that_service(notify_api, sample_user, service_factory): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -338,7 +375,8 @@ def test_should_get_only_templates_for_that_servcie(notify_api, service_factory) 'template_type': 'email', 'subject': 'subject 2', 'content': 'template content', - 'service': str(service_1.id) + 'service': str(service_1.id), + 'created_by': str(sample_user.id) } data = json.dumps(data) create_auth_header = create_authorization_header(