Merge pull request #1708 from alphagov/non-nullable-hidden-templates

Set Templates.hidden column as non-nullable and filter hidden templates

app/dao/templates_dao.py

@@ -75,9 +75,10 @@ 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(
             id=template_id,
+            hidden=False,
             service_id=service_id,
             version=version).one()
-    return Template.query.filter_by(id=template_id, service_id=service_id).one()
+    return Template.query.filter_by(id=template_id, hidden=False, service_id=service_id).one()
 
 
 def dao_get_template_by_id(template_id, version=None):
@@ -93,6 +94,7 @@ def dao_get_all_templates_for_service(service_id, template_type=None):
         return Template.query.filter_by(
             service_id=service_id,
             template_type=template_type,
+            hidden=False,
             archived=False
         ).order_by(
             asc(Template.name),
@@ -101,6 +103,7 @@ def dao_get_all_templates_for_service(service_id, template_type=None):
 
     return Template.query.filter_by(
         service_id=service_id,
+        hidden=False,
         archived=False
     ).order_by(
         asc(Template.name),
@@ -110,7 +113,8 @@ def dao_get_all_templates_for_service(service_id, template_type=None):
 
 def dao_get_template_versions(service_id, template_id):
     return TemplateHistory.query.filter_by(
-        service_id=service_id, id=template_id
+        service_id=service_id, id=template_id,
+        hidden=False,
     ).order_by(
         desc(TemplateHistory.version)
     ).all()

migrations/versions/0170_hidden_non_nullable.py

@@ -0,0 +1,25 @@
+"""
+
+Revision ID: 0170_hidden_non_nullable
+Revises: 0169_hidden_templates_nullable
+Create Date: 2018-02-21 14:05:04.448977
+
+"""
+from alembic import op
+
+
+revision = '0170_hidden_non_nullable'
+down_revision = '0169_hidden_templates_nullable'
+
+
+def upgrade():
+    op.execute('UPDATE templates SET hidden=false WHERE hidden is NULL')
+    op.execute('UPDATE templates_history SET hidden=false WHERE hidden is NULL')
+
+    op.alter_column('templates', 'hidden', nullable=False)
+    op.alter_column('templates_history', 'hidden', nullable=False)
+
+
+def downgrade():
+    op.alter_column('templates', 'hidden', nullable=True)
+    op.alter_column('templates_history', 'hidden', nullable=True)

tests/app/conftest.py

@@ -216,6 +216,7 @@ def sample_template(
     template_type="sms",
     content="This is a template:\nwith a newline",
     archived=False,
+    hidden=False,
     subject_line='Subject',
     user=None,
     service=None,
@@ -237,6 +238,7 @@ def sample_template(
         'service': service,
         'created_by': created_by,
         'archived': archived,
+        'hidden': hidden,
         'process_type': process_type
     }
     if template_type in ['email', 'letter']:

tests/app/dao/test_templates_dao.py

@@ -286,6 +286,29 @@ def test_get_all_templates_ignores_archived_templates(notify_db, notify_db_sessi
     assert templates[0] == normal_template
 
 
+def test_get_all_templates_ignores_hidden_templates(notify_db, notify_db_session, sample_service):
+    normal_template = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_name='Normal Template',
+        service=sample_service,
+        archived=False
+    )
+
+    create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_name='Hidden Template',
+        hidden=True,
+        service=sample_service
+    )
+
+    templates = dao_get_all_templates_for_service(sample_service.id)
+
+    assert len(templates) == 1
+    assert templates[0] == normal_template
+
+
 def test_get_template_by_id_and_service(notify_db, notify_db_session, sample_service):
     sample_template = create_sample_template(
         notify_db,
@@ -301,6 +324,39 @@ def test_get_template_by_id_and_service(notify_db, notify_db_session, sample_ser
     assert not template.redact_personalisation
 
 
+def test_get_template_by_id_and_service_returns_none_for_hidden_templates(notify_db, notify_db_session, sample_service):
+    sample_template = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_name='Test Template',
+        hidden=True,
+        service=sample_service
+    )
+
+    with pytest.raises(NoResultFound):
+        dao_get_template_by_id_and_service_id(
+            template_id=sample_template.id,
+            service_id=sample_service.id
+        )
+
+
+def test_get_template_version_returns_none_for_hidden_templates(notify_db, notify_db_session, sample_service):
+    sample_template = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_name='Test Template',
+        hidden=True,
+        service=sample_service
+    )
+
+    with pytest.raises(NoResultFound):
+        dao_get_template_by_id_and_service_id(
+            sample_template.id,
+            sample_service.id,
+            '1'
+        )
+
+
 def test_get_template_by_id_and_service_returns_none_if_no_template(sample_service, fake_uuid):
     with pytest.raises(NoResultFound) as e:
         dao_get_template_by_id_and_service_id(template_id=fake_uuid, service_id=sample_service.id)
@@ -408,6 +464,18 @@ def test_get_template_versions(sample_template):
     assert len(v) == 2
 
 
+def test_get_template_versions_is_empty_for_hidden_templates(notify_db, notify_db_session, sample_service):
+    sample_template = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_name='Test Template',
+        hidden=True,
+        service=sample_service
+    )
+    versions = dao_get_template_versions(service_id=sample_template.service_id, template_id=sample_template.id)
+    assert len(versions) == 0
+
+
 def test_get_templates_by_ids_successful(notify_db, notify_db_session):
     template_1 = create_sample_template(
         notify_db,