From 29fc81090e3608a044c64a331093e70a248d913f Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Wed, 28 Jun 2017 10:26:25 +0100
Subject: [PATCH 1/8] add template personalisation redaction

If passing in `redact_personalisation` to the template update endpoint,
we should mark that template permanently as redacted - this means that
we won't ever return the personalisation for any notifications for it.

This is to be used with templates containing one time passwords, 2FA
codes or other sensitive information that you may not want service
workers to be able to see.

This is implemented via a separate table, `template_redacted`, which
just contains when the template was redacted.
---
 app/dao/templates_dao.py                      | 18 ++++++++-
 app/models.py                                 | 27 ++++++++++---
 app/template/rest.py                          | 11 +++++-
 migrations/versions/0102_template_redacted.py | 31 +++++++++++++++
 tests/app/dao/test_templates_dao.py           | 38 +++++++++++++++++--
 5 files changed, 114 insertions(+), 11 deletions(-)
 create mode 100644 migrations/versions/0102_template_redacted.py

diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py
index 493c7c072..6cae74c0e 100644
--- a/app/dao/templates_dao.py
+++ b/app/dao/templates_dao.py
@@ -1,10 +1,11 @@
+from datetime import datetime
 import uuid
 
 from sqlalchemy import desc
 from sqlalchemy.sql.expression import bindparam
 
 from app import db
-from app.models import (Template, TemplateHistory)
+from app.models import (Template, TemplateHistory, TemplateRedacted)
 from app.dao.dao_utils import (
     transactional,
     version_class
@@ -16,6 +17,13 @@ from app.dao.dao_utils import (
 def dao_create_template(template):
     template.id = uuid.uuid4()  # must be set now so version history model can use same id
     template.archived = False
+
+    template.template_redacted = TemplateRedacted(
+        template=template,
+        redact_personalisation=False,
+        updated_by=template.created_by
+    )
+
     db.session.add(template)
 
 
@@ -25,6 +33,14 @@ def dao_update_template(template):
     db.session.add(template)
 
 
+@transactional
+def dao_redact_template(template, user_id):
+    template.template_redacted.redact_personalisation = True
+    template.template_redacted.updated_at = datetime.utcnow()
+    template.template_redacted.updated_by_id = user_id
+    db.session.add(template.template_redacted)
+
+
 def dao_get_template_by_id_and_service_id(template_id, service_id, version=None):
     if version is not None:
         return TemplateHistory.query.filter_by(
diff --git a/app/models.py b/app/models.py
index 7a4b9ef3b..e4af1f5e9 100644
--- a/app/models.py
+++ b/app/models.py
@@ -434,12 +434,14 @@ class Template(db.Model):
     subject = db.Column(db.Text, index=False, unique=False, nullable=True)
     created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False)
     created_by = db.relationship('User')
-    version = db.Column(db.Integer, default=1, nullable=False)
-    process_type = db.Column(db.String(255),
-                             db.ForeignKey('template_process_type.name'),
-                             index=True,
-                             nullable=False,
-                             default=NORMAL)
+    version = db.Column(db.Integer, default=0, nullable=False)
+    process_type = db.Column(
+        db.String(255),
+        db.ForeignKey('template_process_type.name'),
+        index=True,
+        nullable=False,
+        default=NORMAL
+    )
 
     def get_link(self):
         # TODO: use "/v2/" route once available
@@ -465,6 +467,19 @@ class Template(db.Model):
         return serialized
 
 
+class TemplateRedacted(db.Model):
+    __tablename__ = 'template_redacted'
+
+    template_id = db.Column(UUID(as_uuid=True), db.ForeignKey('templates.id'), primary_key=True, nullable=False)
+    redact_personalisation = db.Column(db.Boolean, nullable=False, default=False)
+    updated_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow)
+    updated_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), nullable=False)
+    updated_by = db.relationship('User')
+
+    # uselist=False as this is a one-to-one relationship
+    template = db.relationship('Template', uselist=False, backref=db.backref('template_redacted', uselist=False))
+
+
 class TemplateHistory(db.Model):
     __tablename__ = 'templates_history'
 
diff --git a/app/template/rest.py b/app/template/rest.py
index acb2043dd..747fb71cf 100644
--- a/app/template/rest.py
+++ b/app/template/rest.py
@@ -8,6 +8,7 @@ from flask import (
 from app.dao.templates_dao import (
     dao_update_template,
     dao_create_template,
+    dao_redact_template,
     dao_get_template_by_id_and_service_id,
     dao_get_all_templates_for_service,
     dao_get_template_versions
@@ -55,9 +56,17 @@ def create_template(service_id):
 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)
 
+    data = request.get_json()
+
+    # if redacting, don't update anything else
+    if data.get('redact_personalisation') is True and 'updated_by_id' in data:
+        # we also don't need to check what was passed in redact_personalisation - its presence in the dict is enough.
+        dao_redact_template(fetched_template, data['updated_by_id'])
+        return '', 200
+
     current_data = dict(template_schema.dump(fetched_template).data.items())
     updated_template = dict(template_schema.dump(fetched_template).data.items())
-    updated_template.update(request.get_json())
+    updated_template.update(data)
     # Check if there is a change to make.
     if _template_has_not_changed(current_data, updated_template):
         return jsonify(data=updated_template), 200
diff --git a/migrations/versions/0102_template_redacted.py b/migrations/versions/0102_template_redacted.py
new file mode 100644
index 000000000..16d670d39
--- /dev/null
+++ b/migrations/versions/0102_template_redacted.py
@@ -0,0 +1,31 @@
+"""empty message
+
+Revision ID: db6d9d9f06bc
+Revises: 0101_een_logo
+Create Date: 2017-06-27 15:37:28.878359
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'db6d9d9f06bc'
+down_revision = '0101_een_logo'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+def upgrade():
+    op.create_table('template_redacted',
+        sa.Column('template_id', postgresql.UUID(as_uuid=True), nullable=False),
+        sa.Column('redact_personalisation', sa.Boolean(), nullable=False),
+        sa.Column('updated_at', sa.DateTime(), nullable=False),
+        sa.Column('updated_by_id', postgresql.UUID(as_uuid=True), nullable=False),
+        sa.ForeignKeyConstraint(['template_id'], ['templates.id'], ),
+        sa.ForeignKeyConstraint(['updated_by_id'], ['users.id'], ),
+        sa.PrimaryKeyConstraint('template_id')
+    )
+    op.create_index(op.f('ix_template_redacted_updated_by_id'), 'template_redacted', ['updated_by_id'], unique=False)
+
+
+def downgrade():
+    op.drop_table('template_redacted')
diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py
index 182f91928..58ad14e15 100644
--- a/tests/app/dao/test_templates_dao.py
+++ b/tests/app/dao/test_templates_dao.py
@@ -1,14 +1,21 @@
+from datetime import datetime
+
+from freezegun import freeze_time
 from sqlalchemy.orm.exc import NoResultFound
+import pytest
+
 from app.dao.templates_dao import (
     dao_create_template,
     dao_get_template_by_id_and_service_id,
     dao_get_all_templates_for_service,
     dao_update_template,
     dao_get_template_versions,
-    dao_get_templates_for_cache)
+    dao_get_templates_for_cache,
+    dao_redact_template)
+from app.models import Template, TemplateHistory, TemplateRedacted
+
 from tests.app.conftest import sample_template as create_sample_template
-from app.models import Template, TemplateHistory
-import pytest
+from tests.app.db import create_template
 
 
 @pytest.mark.parametrize('template_type, subject', [
@@ -35,6 +42,17 @@ def test_create_template(sample_service, sample_user, template_type, subject):
     assert dao_get_all_templates_for_service(sample_service.id)[0].process_type == 'normal'
 
 
+def test_create_template_creates_redact_entry(sample_service):
+    assert TemplateRedacted.query.count() == 0
+
+    template = create_template(sample_service)
+
+    redacted = TemplateRedacted.query.one()
+    assert redacted.template_id == template.id
+    assert redacted.redact_personalisation is False
+    assert redacted.updated_by_id == sample_service.created_by_id
+
+
 def test_update_template(sample_service, sample_user):
     data = {
         'name': 'Sample Template',
@@ -53,6 +71,20 @@ def test_update_template(sample_service, sample_user):
     assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'new name'
 
 
+def test_redact_template(sample_template):
+    redacted = TemplateRedacted.query.one()
+    assert redacted.template_id == sample_template.id
+    assert redacted.redact_personalisation is False
+
+    time = datetime.now()
+    with freeze_time(time):
+        dao_redact_template(sample_template, sample_template.created_by_id)
+
+    assert redacted.redact_personalisation is True
+    assert redacted.updated_at == time
+    assert redacted.updated_by_id == sample_template.created_by_id
+
+
 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')

From bd71ee9d029a8aebcb219e499689d00cf0143ac5 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Wed, 28 Jun 2017 16:10:22 +0100
Subject: [PATCH 2/8] add redact to notification with template schema.

So that when the admin gets notifications, the template they return
also has a "redact_personalisation" boolean attached to it. Note, it
won't do the redacting on the api - that'll be part of the admin.

Under the hood, this uses an association_proxy, which is essentially
black magic. But it proxies the `redact_personalisation` property of
`TemplateRedacted` onto the `Template` object, so that Marshmallow
can pick it up.

Note: NOT currently added to NotificationWithTemplateHistory
---
 app/models.py                  |  2 ++
 app/schemas.py                 |  7 ++++++-
 tests/app/service/test_rest.py | 37 +++++++++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/app/models.py b/app/models.py
index e4af1f5e9..8d6a64464 100644
--- a/app/models.py
+++ b/app/models.py
@@ -443,6 +443,8 @@ class Template(db.Model):
         default=NORMAL
     )
 
+    redact_personalisation = association_proxy('template_redacted', 'redact_personalisation')
+
     def get_link(self):
         # TODO: use "/v2/" route once available
         return url_for(
diff --git a/app/schemas.py b/app/schemas.py
index 34f057524..48151dba2 100644
--- a/app/schemas.py
+++ b/app/schemas.py
@@ -297,6 +297,11 @@ class BaseTemplateSchema(BaseSchema):
         strict = True
 
 
+class TemplateRedactedSchema(BaseSchema):
+    class Meta:
+        model = models.TemplateRedacted
+
+
 class TemplateSchema(BaseTemplateSchema):
 
     created_by = field_for(models.Template, 'created_by', required=True)
@@ -440,7 +445,7 @@ class NotificationWithTemplateSchema(BaseSchema):
 
     template = fields.Nested(
         TemplateSchema,
-        only=['id', 'version', 'name', 'template_type', 'content', 'subject'],
+        only=['id', 'version', 'name', 'template_type', 'content', 'subject', 'redact_personalisation'],
         dump_only=True
     )
     job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True)
diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py
index 26fa9034e..de24830f0 100644
--- a/tests/app/service/test_rest.py
+++ b/tests/app/service/test_rest.py
@@ -11,6 +11,7 @@ from freezegun import freeze_time
 from app import encryption
 from app.dao.users_dao import save_model_user
 from app.dao.services_dao import dao_remove_user_from_service
+from app.dao.templates_dao import dao_redact_template
 from app.models import (
     User, Organisation, Rate, Service, ServicePermission, Notification,
     DVLA_ORG_LAND_REGISTRY,
@@ -19,7 +20,7 @@ from app.models import (
 )
 
 from tests import create_authorization_header
-from tests.app.db import create_template, create_service_inbound_api, create_user
+from tests.app.db import create_template, create_service_inbound_api, create_user, create_notification
 from tests.app.conftest import (
     sample_service as create_service,
     sample_user_service_permission as create_user_service_permission,
@@ -2263,3 +2264,37 @@ def test_send_one_off_notification(admin_request, sample_template, mocker):
 
     noti = Notification.query.one()
     assert response['id'] == str(noti.id)
+
+
+def test_get_notification_for_service_includes_template_redacted(admin_request, sample_notification):
+    resp = admin_request.get(
+        'service.get_notification_for_service',
+        service_id=sample_notification.service_id,
+        notification_id=sample_notification.id
+    )
+
+    assert resp['id'] == str(sample_notification.id)
+    assert resp['template']['redact_personalisation'] is False
+
+
+def test_get_all_notifications_for_service_includes_template_redacted(admin_request, sample_service):
+    normal_template = create_template(sample_service)
+
+    redacted_template = create_template(sample_service)
+    dao_redact_template(redacted_template, sample_service.created_by_id)
+
+    with freeze_time('2000-01-01'):
+        redacted_noti = create_notification(redacted_template)
+    with freeze_time('2000-01-02'):
+        normal_noti = create_notification(normal_template)
+
+    resp = admin_request.get(
+        'service.get_all_notifications_for_service',
+        service_id=sample_service.id
+    )
+
+    assert resp['notifications'][0]['id'] == str(normal_noti.id)
+    assert resp['notifications'][0]['template']['redact_personalisation'] is False
+
+    assert resp['notifications'][1]['id'] == str(redacted_noti.id)
+    assert resp['notifications'][1]['template']['redact_personalisation'] is True

From 8ad10261ec86519a66fe4ea898be7d6c32c3fb26 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Wed, 28 Jun 2017 16:49:43 +0100
Subject: [PATCH 3/8] add tests for redact_template rest

---
 app/dao/services_dao.py         |  2 +
 app/template/rest.py            |  6 ++-
 tests/app/template/test_rest.py | 92 ++++++++++++++++++++++++++++++++-
 3 files changed, 97 insertions(+), 3 deletions(-)

diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py
index 24c95567a..282e9e9f0 100644
--- a/app/dao/services_dao.py
+++ b/app/dao/services_dao.py
@@ -19,6 +19,7 @@ from app.models import (
     ApiKey,
     Template,
     TemplateHistory,
+    TemplateRedacted,
     Job,
     NotificationHistory,
     Notification,
@@ -218,6 +219,7 @@ def delete_service_and_all_associated_db_objects(service):
     _delete_commit(Job.query.filter_by(service=service))
     _delete_commit(NotificationHistory.query.filter_by(service=service))
     _delete_commit(Notification.query.filter_by(service=service))
+    _delete_commit(TemplateRedacted.query.filter_by(service=service))
     _delete_commit(Template.query.filter_by(service=service))
     _delete_commit(TemplateHistory.query.filter_by(service_id=service.id))
     _delete_commit(ServicePermission.query.filter_by(service_id=service.id))
diff --git a/app/template/rest.py b/app/template/rest.py
index 747fb71cf..867ecad8e 100644
--- a/app/template/rest.py
+++ b/app/template/rest.py
@@ -61,8 +61,10 @@ def update_template(service_id, template_id):
     # if redacting, don't update anything else
     if data.get('redact_personalisation') is True and 'updated_by_id' in data:
         # we also don't need to check what was passed in redact_personalisation - its presence in the dict is enough.
-        dao_redact_template(fetched_template, data['updated_by_id'])
-        return '', 200
+        # also, only
+        if not fetched_template.redact_personalisation:
+            dao_redact_template(fetched_template, data['updated_by_id'])
+        return 'null', 200
 
     current_data = dict(template_schema.dump(fetched_template).data.items())
     updated_template = dict(template_schema.dump(fetched_template).data.items())
diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py
index 13e21dcfd..a8646feb8 100644
--- a/tests/app/template/test_rest.py
+++ b/tests/app/template/test_rest.py
@@ -1,11 +1,17 @@
+import uuid
 import json
 import random
 import string
+from datetime import datetime, timedelta
+
 import pytest
+from freezegun import freeze_time
+
 from app.models import Template
+from app.dao.templates_dao import dao_get_template_by_id, dao_redact_template
+
 from tests import create_authorization_header
 from tests.app.conftest import sample_template as create_sample_template
-from app.dao.templates_dao import dao_get_template_by_id
 
 
 @pytest.mark.parametrize('template_type, subject', [
@@ -539,3 +545,87 @@ def test_update_set_process_type_on_template(client, sample_template):
 
     template = dao_get_template_by_id(sample_template.id)
     assert template.process_type == 'priority'
+
+
+def test_update_redact_template(admin_request, sample_template):
+    assert sample_template.redact_personalisation is False
+
+    data = {
+        'redact_personalisation': True,
+        'updated_by_id': str(sample_template.created_by_id)
+    }
+
+    dt = datetime.now()
+
+    with freeze_time(dt):
+        resp = admin_request.post(
+            'template.update_template',
+            service_id=sample_template.service_id,
+            template_id=sample_template.id,
+            _data=data
+        )
+
+    assert resp is None
+
+    assert sample_template.redact_personalisation is True
+    assert sample_template.template_redacted.updated_by_id == sample_template.created_by_id
+    assert sample_template.template_redacted.updated_at == dt
+
+    assert sample_template.version == 1
+
+
+def test_update_redact_template_ignores_other_properties(admin_request, sample_template):
+    data = {
+        'name': 'Foo',
+        'redact_personalisation': True,
+        'updated_by_id': str(sample_template.created_by_id)
+    }
+
+    admin_request.post(
+        'template.update_template',
+        service_id=sample_template.service_id,
+        template_id=sample_template.id,
+        _data=data
+    )
+
+    assert sample_template.redact_personalisation is True
+    assert sample_template.name != 'Foo'
+
+
+def test_update_redact_template_does_nothing_if_already_redacted(admin_request, sample_template):
+    dt = datetime.now()
+    with freeze_time(dt):
+        dao_redact_template(sample_template, sample_template.created_by_id)
+
+    data = {
+        'redact_personalisation': True,
+        'updated_by_id': str(sample_template.created_by_id)
+    }
+
+    with freeze_time(dt + timedelta(days=1)):
+        resp = admin_request.post(
+            'template.update_template',
+            service_id=sample_template.service_id,
+            template_id=sample_template.id,
+            _data=data
+        )
+
+    assert resp is None
+
+    assert sample_template.redact_personalisation is True
+    # make sure that it hasn't been updated
+    assert sample_template.template_redacted.updated_at == dt
+
+
+
+def test_update_redact_template_does_nothing_if_no_updated_by(admin_request, sample_template):
+    original_updated_time = sample_template.template_redacted.updated_at
+    admin_request.post(
+        'template.update_template',
+        service_id=sample_template.service_id,
+        template_id=sample_template.id,
+        _data={'redact_personalisation': True}
+    )
+
+    assert sample_template.redact_personalisation is False
+    assert sample_template.template_redacted.updated_at == original_updated_time

From 3f663daafe325e3d21b7206f3a5d1ae341b8afdb Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Wed, 28 Jun 2017 17:03:12 +0100
Subject: [PATCH 4/8] redacting a template now 400s if no updated_by_id
 supplied

---
 app/template/rest.py            | 21 +++++++++++++++------
 tests/app/template/test_rest.py | 13 +++++++++----
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/app/template/rest.py b/app/template/rest.py
index 867ecad8e..d4f7c4e64 100644
--- a/app/template/rest.py
+++ b/app/template/rest.py
@@ -59,12 +59,8 @@ def update_template(service_id, template_id):
     data = request.get_json()
 
     # if redacting, don't update anything else
-    if data.get('redact_personalisation') is True and 'updated_by_id' in data:
-        # we also don't need to check what was passed in redact_personalisation - its presence in the dict is enough.
-        # also, only
-        if not fetched_template.redact_personalisation:
-            dao_redact_template(fetched_template, data['updated_by_id'])
-        return 'null', 200
+    if data.get('redact_personalisation') is True:
+        return redact_template(fetched_template, data)
 
     current_data = dict(template_schema.dump(fetched_template).data.items())
     updated_template = dict(template_schema.dump(fetched_template).data.items())
@@ -143,3 +139,16 @@ def _template_has_not_changed(current_data, updated_template):
         current_data[key] == updated_template[key]
         for key in ('name', 'content', 'subject', 'archived', 'process_type')
     )
+
+
+def redact_template(template, data):
+    # we also don't need to check what was passed in redact_personalisation - its presence in the dict is enough.
+    if 'updated_by_id' not in data:
+        message = 'Field is required'
+        errors = {'updated_by_id': [message]}
+        raise InvalidRequest(errors, status_code=400)
+
+    # if it's already redacted, then just return 200 straight away.
+    if not template.redact_personalisation:
+        dao_redact_template(template, data['updated_by_id'])
+    return 'null', 200
diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py
index a8646feb8..07db2b020 100644
--- a/tests/app/template/test_rest.py
+++ b/tests/app/template/test_rest.py
@@ -617,15 +617,20 @@ def test_update_redact_template_does_nothing_if_already_redacted(admin_request,
     assert sample_template.template_redacted.updated_at == dt
 
 
-
-def test_update_redact_template_does_nothing_if_no_updated_by(admin_request, sample_template):
+def test_update_redact_template_400s_if_no_updated_by(admin_request, sample_template):
     original_updated_time = sample_template.template_redacted.updated_at
-    admin_request.post(
+    resp = admin_request.post(
         'template.update_template',
         service_id=sample_template.service_id,
         template_id=sample_template.id,
-        _data={'redact_personalisation': True}
+        _data={'redact_personalisation': True},
+        _expected_status=400
     )
 
+    assert resp == {
+        'result': 'error',
+        'message': {'updated_by_id': ['Field is required']}
+    }
+
     assert sample_template.redact_personalisation is False
     assert sample_template.template_redacted.updated_at == original_updated_time

From 3a0bc01a55017c808f2ff99f855d3c077a3edea0 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Wed, 28 Jun 2017 17:19:53 +0100
Subject: [PATCH 5/8] fix service_delete function to clean up template_redacted
 objects properly

---
 app/dao/services_dao.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py
index 282e9e9f0..e39778217 100644
--- a/app/dao/services_dao.py
+++ b/app/dao/services_dao.py
@@ -202,13 +202,16 @@ def dao_remove_user_from_service(service, user):
 def delete_service_and_all_associated_db_objects(service):
 
     def _delete_commit(query):
-        query.delete()
+        query.delete(synchronize_session=False)
         db.session.commit()
 
     job_stats = JobStatistics.query.join(Job).filter(Job.service_id == service.id)
     list(map(db.session.delete, job_stats))
     db.session.commit()
 
+    subq = db.session.query(Template.id).filter_by(service=service).subquery()
+    _delete_commit(TemplateRedacted.query.filter(TemplateRedacted.template_id.in_(subq)))
+
     _delete_commit(NotificationStatistics.query.filter_by(service=service))
     _delete_commit(TemplateStatistics.query.filter_by(service=service))
     _delete_commit(ProviderStatistics.query.filter_by(service=service))
@@ -219,7 +222,6 @@ def delete_service_and_all_associated_db_objects(service):
     _delete_commit(Job.query.filter_by(service=service))
     _delete_commit(NotificationHistory.query.filter_by(service=service))
     _delete_commit(Notification.query.filter_by(service=service))
-    _delete_commit(TemplateRedacted.query.filter_by(service=service))
     _delete_commit(Template.query.filter_by(service=service))
     _delete_commit(TemplateHistory.query.filter_by(service_id=service.id))
     _delete_commit(ServicePermission.query.filter_by(service_id=service.id))

From 52debfb412211d88dedb477613c50f4c09df7548 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Thu, 29 Jun 2017 10:34:38 +0100
Subject: [PATCH 6/8] Load all template model relationships when archiving a
 service

Since the version classes hinge on delicately preserving the session,
we need to take lots of care to ensure that we don't accidentally flush
half-way through. By joinedloading the template_redacted beforehand,
we prevent a flush which would inadvertantly remove the Service object
from the session, while it's still waiting in line to be versioned.
---
 app/dao/services_dao.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py
index e39778217..7429a1e10 100644
--- a/app/dao/services_dao.py
+++ b/app/dao/services_dao.py
@@ -111,6 +111,7 @@ def dao_archive_service(service_id):
     # to ensure that db.session still contains the models when it comes to creating history objects
     service = Service.query.options(
         joinedload('templates'),
+        joinedload('templates.template_redacted'),
         joinedload('api_keys'),
     ).filter(Service.id == service_id).one()
 

From 2f973b8af0709995aeabb40c95111107471b8188 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Thu, 29 Jun 2017 12:39:02 +0100
Subject: [PATCH 7/8] use created_by instead of updated_by

to behave in same way as other endpoints
---
 app/template/rest.py            |  6 +++---
 tests/app/template/test_rest.py | 10 +++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/app/template/rest.py b/app/template/rest.py
index d4f7c4e64..a9bd44f1f 100644
--- a/app/template/rest.py
+++ b/app/template/rest.py
@@ -143,12 +143,12 @@ def _template_has_not_changed(current_data, updated_template):
 
 def redact_template(template, data):
     # we also don't need to check what was passed in redact_personalisation - its presence in the dict is enough.
-    if 'updated_by_id' not in data:
+    if 'created_by' not in data:
         message = 'Field is required'
-        errors = {'updated_by_id': [message]}
+        errors = {'created_by': [message]}
         raise InvalidRequest(errors, status_code=400)
 
     # if it's already redacted, then just return 200 straight away.
     if not template.redact_personalisation:
-        dao_redact_template(template, data['updated_by_id'])
+        dao_redact_template(template, data['created_by'])
     return 'null', 200
diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py
index 07db2b020..dfb773453 100644
--- a/tests/app/template/test_rest.py
+++ b/tests/app/template/test_rest.py
@@ -552,7 +552,7 @@ def test_update_redact_template(admin_request, sample_template):
 
     data = {
         'redact_personalisation': True,
-        'updated_by_id': str(sample_template.created_by_id)
+        'created_by': str(sample_template.created_by_id)
     }
 
     dt = datetime.now()
@@ -578,7 +578,7 @@ def test_update_redact_template_ignores_other_properties(admin_request, sample_t
     data = {
         'name': 'Foo',
         'redact_personalisation': True,
-        'updated_by_id': str(sample_template.created_by_id)
+        'created_by': str(sample_template.created_by_id)
     }
 
     admin_request.post(
@@ -599,7 +599,7 @@ def test_update_redact_template_does_nothing_if_already_redacted(admin_request,
 
     data = {
         'redact_personalisation': True,
-        'updated_by_id': str(sample_template.created_by_id)
+        'created_by': str(sample_template.created_by_id)
     }
 
     with freeze_time(dt + timedelta(days=1)):
@@ -617,7 +617,7 @@ def test_update_redact_template_does_nothing_if_already_redacted(admin_request,
     assert sample_template.template_redacted.updated_at == dt
 
 
-def test_update_redact_template_400s_if_no_updated_by(admin_request, sample_template):
+def test_update_redact_template_400s_if_no_created_by(admin_request, sample_template):
     original_updated_time = sample_template.template_redacted.updated_at
     resp = admin_request.post(
         'template.update_template',
@@ -629,7 +629,7 @@ def test_update_redact_template_400s_if_no_updated_by(admin_request, sample_temp
 
     assert resp == {
         'result': 'error',
-        'message': {'updated_by_id': ['Field is required']}
+        'message': {'created_by': ['Field is required']}
     }
 
     assert sample_template.redact_personalisation is False

From c24edcf38825c694fdc14e5a8e3888f136650506 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Thu, 29 Jun 2017 12:54:48 +0100
Subject: [PATCH 8/8] add historical redaction data

every current template gets a row in the template_redacted table -
this inserts one for any template that doesn't already have a row,
with redact set to false, the user set to NOTIFY_USER since it was
just a script, and the updated_at set to the time the script is run
---
 .../versions/0103_add_historical_redact.py    | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 migrations/versions/0103_add_historical_redact.py

diff --git a/migrations/versions/0103_add_historical_redact.py b/migrations/versions/0103_add_historical_redact.py
new file mode 100644
index 000000000..8d073bbd3
--- /dev/null
+++ b/migrations/versions/0103_add_historical_redact.py
@@ -0,0 +1,43 @@
+"""empty message
+
+Revision ID: 0103_add_historical_redact
+Revises: db6d9d9f06bc
+Create Date: 2017-06-29 12:44:16.815039
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '0103_add_historical_redact'
+down_revision = 'db6d9d9f06bc'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+from flask import current_app
+
+def upgrade():
+    op.execute(
+        """
+        INSERT INTO template_redacted
+        (
+            template_id,
+            redact_personalisation,
+            updated_at,
+            updated_by_id
+        )
+        SELECT
+            templates.id,
+            false,
+            now(),
+            '{notify_user}'
+        FROM
+            templates
+        LEFT JOIN template_redacted on template_redacted.template_id = templates.id
+        WHERE template_redacted.template_id IS NULL
+        """.format(notify_user=current_app.config['NOTIFY_USER_ID'])
+    )
+
+
+def downgrade():
+    # data migration, no downloads
+    pass