Working tests and provider stats table.

Fix for tests and import error.

Added tests and updated for code review comments.

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):

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

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

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"

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()

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('/<uuid:service_id>/history', methods=['GET'])
-def get_service_and_api_key_history(service_id):
+def get_service_history(service_id):
-    from app.models import Service, ApiKey
+    from app.models import (Service, ApiKey, Template)
-    from app.schemas import service_history_schema, api_key_history_schema
+    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)

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

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 ###

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",

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({

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(
+    template_1 = create_sample_template(
-        name='Sample Template 1',
+        notify_db,
+        notify_db_session,
+        template_name='Sample Template 1',
         template_type="sms",
         content="Template content",
         service=service_1
     )
-    template_2 = Template(
+    template_2 = create_sample_template(
-        name='Sample Template 2',
+        notify_db,
+        notify_db_session,
+        template_name='Sample Template 2',
         template_type="sms",
         content="Template content",
         service=service_1
     )
-    template_3 = Template(
+    template_3 = create_sample_template(
-        name='Sample Template 3',
+        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):
+def test_get_all_templates_for_service_in_created_order(notify_db, notify_db_session, sample_service):
-    template_1 = Template(
+    template_1 = create_sample_template(
-        name='Sample Template 1',
+        notify_db,
+        notify_db_session,
+        template_name='Sample Template 1',
         template_type="sms",
         content="Template content",
         service=sample_service
     )
-    template_2 = Template(
+    template_2 = create_sample_template(
-        name='Sample Template 2',
+        notify_db,
+        notify_db_session,
+        template_name='Sample Template 2',
         template_type="sms",
         content="Template content",
         service=sample_service
     )
-    template_3 = Template(
+    template_3 = create_sample_template(
-        name='Sample Template 3',
+        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

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(

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
+            json_resp = json.loads(resp.get_data(as_text=True))
-            assert 'Key (name)=(Sample service) already exists.' in 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
+            json_resp = json.loads(resp.get_data(as_text=True))
-            assert 'Key (email_from)=(first.service) already exists.' in 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 resp.status_code == 400
-            assert 'Key (name)=(another name) 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_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 resp.status_code == 400
-            assert 'Key (email_from)=(duplicate.name) 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_should_404_if_id_is_invalid(notify_api, notify_db, notify_db_session):

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 <b>content</b>',
-                '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 <b>content</b>',
-                '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 <script type="text/javascript">alert("foo")</script>'
+                'content': 'my template has new content <script type="text/javascript">alert("foo")</script>',
+                '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(