Merge pull request #568 from alphagov/template_history_view

Template history view

app/main/views/jobs.py

@@ -57,7 +57,9 @@ def view_jobs(service_id):
 @user_has_permissions('view_activity', admin_override=True)
 def view_job(service_id, job_id):
     job = job_api_client.get_job(service_id, job_id)['data']
-    template = service_api_client.get_service_template(service_id, job['template'])['data']
+    template = service_api_client.get_service_template(service_id=service_id,
+                                                       template_id=job['template'],
+                                                       version=job['template_version'])['data']
     notifications = notification_api_client.get_notifications_for_service(service_id, job_id)
     finished = job['status'] == 'finished'
     return render_template(

app/main/views/templates.py

@@ -21,7 +21,7 @@ page_headings = {
 }
 
 
-@main.route("/services/<service_id>/templates/<template_id>", methods=['GET'])
+@main.route("/services/<service_id>/templates/<template_id>")
 @login_required
 @user_has_permissions(
     'view_activity',
@@ -41,6 +41,27 @@ def view_template(service_id, template_id):
     )
 
 
+@main.route("/services/<service_id>/templates/<template_id>/version/<int:version>")
+@login_required
+@user_has_permissions(
+    'view_activity',
+    'send_texts',
+    'send_emails',
+    'manage_templates',
+    'manage_api_keys',
+    admin_override=True,
+    any_=True
+)
+def view_template_version(service_id, template_id, version):
+    return render_template(
+        'views/templates/template_history.html',
+        template=Template(
+            service_api_client.get_service_template(service_id, template_id, version)['data'],
+            prefix=current_service['name']
+        )
+    )
+
+
 @main.route("/services/<service_id>/templates/add-<template_type>", methods=['GET', 'POST'])
 @login_required
 @user_has_permissions('manage_templates', admin_override=True)
@@ -142,3 +163,30 @@ def delete_service_template(service_id, template_id):
         h1='Edit template',
         form=form,
         template_id=template_id)
+
+
+@main.route('/services/<service_id>/templates/<template_id>/versions')
+@login_required
+@user_has_permissions(
+    'view_activity',
+    'send_texts',
+    'send_emails',
+    'manage_templates',
+    'manage_api_keys',
+    admin_override=True,
+    any_=True
+)
+def view_template_versions(service_id, template_id):
+    return render_template(
+        'views/templates/choose_history.html',
+        template=Template(
+            service_api_client.get_service_template(service_id, template_id)['data'],
+            prefix=current_service['name']
+        ),
+        versions=[
+            Template(
+                template,
+                prefix=current_service['name']
+            ) for template in service_api_client.get_service_template_versions(service_id, template_id)['data']
+        ]
+    )

app/notify_client/api_client.py

@@ -128,13 +128,25 @@ class ServiceAPIClient(NotificationsAPIClient):
         endpoint = "/service/{0}/template/{1}".format(service_id, id_)
         return self.post(endpoint, data)
 
-    def get_service_template(self, service_id, template_id, *params):
+    def get_service_template(self, service_id, template_id, version=None, *params):
         """
         Retrieve a service template.
         """
         endpoint = '/service/{service_id}/template/{template_id}'.format(
             service_id=service_id,
             template_id=template_id)
+        if version:
+            endpoint = '{base}/version/{version}'.format(base=endpoint, version=version)
+        return self.get(endpoint, *params)
+
+    def get_service_template_versions(self, service_id, template_id, *params):
+        """
+        Retrieve a list of versions for a template
+        """
+        endpoint = '/service/{service_id}/template/{template_id}/versions'.format(
+            service_id=service_id,
+            template_id=template_id
+        )
         return self.get(endpoint, *params)
 
     def get_service_templates(self, service_id, *params):

app/templates/components/email-message.html

@@ -15,6 +15,7 @@
       {% else %}
         {{ name }}
       {% endif %}
+      {% if created_at %}{{ created_at }}{% endif %}
     </h3>
   {% endif %}
   <div class="email-message">

app/templates/components/sms-message.html

@@ -14,6 +14,7 @@
       {% else %}
         {{ name }}
       {% endif %}
+      {% if created_at %}{{ created_at }}{% endif %}
     </h3>
   {% endif %}
   {% if recipient is not none %}

app/templates/views/notifications.html

@@ -81,7 +81,7 @@
         {{ item.to }}
       </p>
       <p class="hint">
-        <a href="{{ url_for('.view_template', service_id=current_service.id, template_id=item.template.id) }}">{{ item.template.name }}</a>
+        <a href="{{ url_for('.view_template_version', service_id=current_service.id, template_id=item.template.id, version=item.template_version) }}">{{ item.template.name }}</a>
         sent from
         {% if item.job %}
           <a href="{{ url_for(".view_job", service_id=current_service.id, job_id=item.job.id) }}">{{ item.job.original_file_name }}</a>

app/templates/views/templates/_template.html

@@ -25,6 +25,7 @@
     {% if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) %}
        <a href="{{ url_for(".edit_service_template", service_id=current_service.id, template_id=template.id) }}">Edit template</a>
     {% endif %}
+    <a href="{{ url_for('.view_template_versions', service_id=current_service.id, template_id=template.id) }}">Template versions</a>
     <a href="{{ url_for(".send_from_api", service_id=current_service.id, template_id=template.id) }}">API info</a>
   </div>
 </div>

app/templates/views/templates/_template_history.html

@@ -0,0 +1,40 @@
+{% from "components/email-message.html" import email_message %}
+{% from "components/sms-message.html" import sms_message %}
+
+<div class="column-two-thirds">
+  {% if 'email' == template.template_type %}
+    {{ email_message(
+      template.formatted_subject_as_markup,
+      template.formatted_as_markup,
+      name=template.name if show_title else None,
+      edit_link=url_for(
+        '.view_template_version',
+        service_id=current_service.id,
+        template_id=template.id,
+        version=template.get_raw('version')
+      )
+    ) }}
+  {% elif 'sms' == template.template_type %}
+    {{ sms_message(
+      template.formatted_as_markup,
+      name=template.name if show_title else None,
+      edit_link=url_for(
+        '.view_template_version',
+        service_id=current_service.id,
+        template_id=template.id,
+        version=template.get_raw('version')
+      )
+    ) }}
+  {% endif %}
+</div>
+
+<div class="column-one-third">
+  <div class="sms-message-use-links{% if show_title %}-with-title{% endif %}">
+    <p>Edited by {{ template.get_raw('created_by').name }}</p>
+    <p>Created on {{ template.get_raw('created_at')|format_datetime }}</p>
+    <p>Edited on
+      {% if template.get_raw('updated_at') %}
+        {{ template.get_raw('updated_at')|format_datetime }}</p>
+      {% endif %}
+  </div>
+</div>

app/templates/views/templates/choose_history.html

@@ -0,0 +1,16 @@
+{% extends "withnav_template.html" %}
+
+{% block page_title %}
+  {{ template.name }} – GOV.UK Notify
+{% endblock %}
+
+{% block maincolumn_content %}
+    <h1 class="heading-large">{{ template.name }} version history</h1>
+    <div class="grid-row">
+      {% for template in versions %}
+        {% with show_title=True %}
+          {% include 'views/templates/_template_history.html' %}
+        {% endwith %}
+      {% endfor %}
+    </div>
+{% endblock %}

app/templates/views/templates/template_history.html

@@ -0,0 +1,22 @@
+{% extends "withnav_template.html" %}
+{% from "components/email-message.html" import email_message %}
+{% from "components/sms-message.html" import sms_message %}
+{% from "components/page-footer.html" import page_footer %}
+{% from "components/textbox.html" import textbox %}
+
+{% block page_title %}
+  {{ template.name }} – GOV.UK Notify
+{% endblock %}
+
+{% block maincolumn_content %}
+
+
+  <h1 class="heading-large">{{ template.name }}</h1>
+
+  <div class="grid-row">
+    {% with show_title=False %}
+      {% include 'views/templates/_template_history.html' %}
+    {% endwith %}
+  </div>
+
+{% endblock %}

tests/__init__.py

@@ -1,6 +1,6 @@
 import pytest
 import uuid
-import datetime
+from datetime import datetime, timedelta
 from flask.testing import FlaskClient
 from flask import url_for
 from flask_login import login_user
@@ -53,19 +53,40 @@ def template_json(service_id,
                   name="sample template",
                   type_="sms",
                   content="template content",
-                  subject=None):
+                  subject=None,
+                  versions='1'):
     template = {
         'id': id_,
         'name': name,
         'template_type': type_,
         'content': content,
-        'service': service_id
+        'service': service_id,
+        'version': versions
     }
     if subject is not None:
         template['subject'] = subject
     return template
 
 
+def template_version_json(service_id,
+                          id_,
+                          created_by,
+                          version=1,
+                          created_at=None,
+                          **kwargs):
+    template = template_json(service_id, id_, **kwargs)
+    template['created_by'] = {
+        'id': created_by.id,
+        'name': created_by.name,
+        'email_address': created_by.email_address
+    }
+    if created_at is None:
+        created_at = datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S.%f')
+    template['created_at'] = created_at
+    template['version'] = version
+    return template
+
+
 def api_key_json(id_, name, expiry_date=None):
     return {'id': id_,
             'name': name,
@@ -103,11 +124,12 @@ def create_test_api_user(state, permissions={}):
 
 def job_json():
     job_id = str(generate_uuid())
-    created_at = str(datetime.datetime.utcnow().time())
+    created_at = str(datetime.utcnow().time())
     data = {
         'id': job_id,
         'service': 1,
         'template': 1,
+        'template_version': 1,
         'original_file_name': 'thisisatest.csv',
         'created_at': created_at,
         'notification_count': 1,
@@ -122,8 +144,9 @@ def job_json_with_created_by(service_id=None, job_id=None):
         'id': job_id if job_id else str(generate_uuid()),
         'service': service_id if service_id else str(generate_uuid()),
         'template': 1,
+        'template_version': 1,
         'original_file_name': 'thisisatest.csv',
-        'created_at': str(datetime.datetime.now().time()),
+        'created_at': str(datetime.now().time()),
         'notification_count': 1,
         'notifications_sent': 1,
         'status': '',
@@ -146,11 +169,11 @@ def notification_json(service_id,
     if template is None:
         template = template_json(service_id, str(generate_uuid()))
     if sent_at is None:
-        sent_at = str(datetime.datetime.utcnow().time())
+        sent_at = str(datetime.utcnow().time())
     if created_at is None:
-        created_at = str(datetime.datetime.utcnow().time())
+        created_at = str(datetime.utcnow().time())
     if updated_at is None:
-        updated_at = str((datetime.datetime.utcnow() + datetime.timedelta(minutes=1)).time())
+        updated_at = str((datetime.utcnow() + timedelta(minutes=1)).time())
     links = {}
     if with_links:
         links = {
@@ -169,7 +192,8 @@ def notification_json(service_id,
             'sent_at': sent_at,
             'status': status,
             'created_at': created_at,
-            'updated_at': updated_at
+            'updated_at': updated_at,
+            'template_version': template['version']
         } for i in range(5)],
         'total': 5,
         'page_size': 50,

tests/app/main/views/test_template_history.py

@@ -0,0 +1,68 @@
+import json
+from flask import url_for
+
+
+def test_view_template_version(app_,
+                               api_user_active,
+                               mock_login,
+                               mock_get_service,
+                               mock_get_template_version,
+                               mock_get_user,
+                               mock_get_user_by_email,
+                               mock_has_permissions,
+                               fake_uuid):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            client.login(api_user_active)
+            service_id = fake_uuid
+            template_id = fake_uuid
+            version = 1
+            resp = client.get(url_for(
+                '.view_template_version',
+                service_id=service_id,
+                template_id=template_id,
+                version=version))
+
+    assert resp.status_code == 200
+    resp_data = resp.get_data(as_text=True)
+    template = mock_get_template_version(service_id, template_id, version)
+    assert api_user_active.name in resp_data
+    assert template['data']['content'] in resp_data
+    mock_get_template_version.assert_called_with(
+        service_id,
+        template_id,
+        version
+    )
+
+
+def test_view_template_versions(app_,
+                                api_user_active,
+                                mock_login,
+                                mock_get_service,
+                                mock_get_template_versions,
+                                mock_get_service_template,
+                                mock_get_user,
+                                mock_get_user_by_email,
+                                mock_has_permissions,
+                                fake_uuid):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            client.login(api_user_active)
+            service_id = fake_uuid
+            template_id = fake_uuid
+            version = 1
+            resp = client.get(url_for(
+                '.view_template_versions',
+                service_id=service_id,
+                template_id=template_id
+            ))
+
+    assert resp.status_code == 200
+    resp_data = resp.get_data(as_text=True)
+    versions = mock_get_template_versions(service_id, template_id)
+    assert api_user_active.name in resp_data
+    assert versions['data'][0]['content'] in resp_data
+    mock_get_template_versions.assert_called_with(
+        service_id,
+        template_id
+    )

tests/conftest.py

@@ -9,6 +9,7 @@ from . import (
     service_json,
     TestClient,
     template_json,
+    template_version_json,
     api_key_json,
     job_json,
     notification_json,
@@ -203,13 +204,55 @@ def mock_get_aggregate_service_statistics(mocker):
 
 @pytest.fixture(scope='function')
 def mock_get_service_template(mocker):
-    def _get(service_id, template_id):
+    def _get(service_id, template_id, version=None):
         template = template_json(
             service_id, template_id, "Two week reminder", "sms", "Your vehicle tax is about to expire")
+        if version:
+            template.update({'version': version})
         return {'data': template}
 
     return mocker.patch(
-        'app.service_api_client.get_service_template', side_effect=_get)
+        'app.service_api_client.get_service_template',
+        side_effect=_get
+    )
+
+
+@pytest.fixture(scope='function')
+def mock_get_template_version(mocker, fake_uuid, user=None):
+    if user is None:
+        user = api_user_active(fake_uuid)
+
+    def _get(service_id, template_id, version):
+        template_version = template_version_json(
+            service_id,
+            template_id,
+            user,
+            version=version
+        )
+        return {'data': template_version}
+    return mocker.patch(
+        'app.service_api_client.get_service_template',
+        side_effect=_get
+    )
+
+
+@pytest.fixture(scope='function')
+def mock_get_template_versions(mocker, fake_uuid, user=None):
+    if user is None:
+        user = api_user_active(fake_uuid)
+
+    def _get(service_id, template_id):
+        template_version = template_version_json(
+            service_id,
+            template_id,
+            user,
+            version=1
+        )
+        return {'data': [template_version]}
+    return mocker.patch(
+        'app.service_api_client.get_service_template_versions',
+        side_effect=_get
+    )
 
 
 @pytest.fixture(scope='function')
@@ -221,7 +264,9 @@ def mock_get_service_template_with_placeholders(mocker):
         return {'data': template}
 
     return mocker.patch(
-        'app.service_api_client.get_service_template', side_effect=_get)
+        'app.service_api_client.get_service_template',
+        side_effect=_get
+    )
 
 
 @pytest.fixture(scope='function')