From 64e7a7b010734dc813a36fcbb2679d5c02db425f Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Wed, 11 May 2016 11:20:45 +0100 Subject: [PATCH 1/2] Update all dates to use utc, only in the template is it converted to british time. Template versions and template version pages added. Fix merge changes. --- app/main/views/templates.py | 50 +++++++++++++- app/notify_client/api_client.py | 14 +++- app/templates/components/email-message.html | 1 + app/templates/components/sms-message.html | 1 + app/templates/views/templates/_template.html | 1 + .../views/templates/_template_history.html | 36 ++++++++++ .../views/templates/choose_history.html | 16 +++++ .../views/templates/template_history.html | 22 ++++++ requirements.txt | 2 +- tests/__init__.py | 38 ++++++++--- tests/app/main/views/test_template_history.py | 68 +++++++++++++++++++ tests/conftest.py | 47 ++++++++++++- 12 files changed, 283 insertions(+), 13 deletions(-) create mode 100644 app/templates/views/templates/_template_history.html create mode 100644 app/templates/views/templates/choose_history.html create mode 100644 app/templates/views/templates/template_history.html create mode 100644 tests/app/main/views/test_template_history.py diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 3728e20e9..915206649 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -21,7 +21,7 @@ page_headings = { } -@main.route("/services//templates/", methods=['GET']) +@main.route("/services//templates/") @login_required @user_has_permissions( 'view_activity', @@ -41,6 +41,27 @@ def view_template(service_id, template_id): ) +@main.route("/services//templates//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//templates/add-", 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//templates//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'] + ] + ) diff --git a/app/notify_client/api_client.py b/app/notify_client/api_client.py index 550821799..52e106bde 100644 --- a/app/notify_client/api_client.py +++ b/app/notify_client/api_client.py @@ -126,13 +126,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}/version'.format( + service_id=service_id, + template_id=template_id + ) return self.get(endpoint, *params) def get_service_templates(self, service_id, *params): diff --git a/app/templates/components/email-message.html b/app/templates/components/email-message.html index 297a97a4d..31e62f05c 100644 --- a/app/templates/components/email-message.html +++ b/app/templates/components/email-message.html @@ -15,6 +15,7 @@ {% else %} {{ name }} {% endif %} + {% if created_at %}{{ created_at }}{% endif %} {% endif %} \ No newline at end of file diff --git a/app/templates/views/templates/_template_history.html b/app/templates/views/templates/_template_history.html new file mode 100644 index 000000000..7bbb71f26 --- /dev/null +++ b/app/templates/views/templates/_template_history.html @@ -0,0 +1,36 @@ +{% from "components/email-message.html" import email_message %} +{% from "components/sms-message.html" import sms_message %} + +
+ {% 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 %} +
+ +
+
+

Edited by {{ template.get_raw('created_by').name }}

+

Edited on {{ template.get_raw('created_at')|format_datetime }}

+
+
\ No newline at end of file diff --git a/app/templates/views/templates/choose_history.html b/app/templates/views/templates/choose_history.html new file mode 100644 index 000000000..b7d65b3b0 --- /dev/null +++ b/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 %} +

{{ template.name }} version history

+
+ {% for template in versions %} + {% with show_title=True %} + {% include 'views/templates/_template_history.html' %} + {% endwith %} + {% endfor %} +
+{% endblock %} diff --git a/app/templates/views/templates/template_history.html b/app/templates/views/templates/template_history.html new file mode 100644 index 000000000..847e4dd69 --- /dev/null +++ b/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 %} + + +

{{ template.name }}

+ +
+ {% with show_title=False %} + {% include 'views/templates/_template_history.html' %} + {% endwith %} +
+ +{% endblock %} \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index a30cd42fa..ebc4fbe15 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,4 +10,4 @@ blinker==1.4 git+https://github.com/alphagov/notifications-python-client.git@1.0.0#egg=notifications-python-client==1.0.0 -git+https://github.com/alphagov/notifications-utils.git@5.2.0#egg=notifications-utils==5.2.0 +git+https://github.com/alphagov/notifications-utils.git@5.2.1#egg=notifications-utils==5.2.1 diff --git a/tests/__init__.py b/tests/__init__.py index 28929e720..d12d9990c 100644 --- a/tests/__init__.py +++ b/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 @@ -52,19 +52,41 @@ 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, + 'versions': 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 + del template['versions'] + return template + + def api_key_json(id_, name, expiry_date=None): return {'id': id_, 'name': name, @@ -102,7 +124,7 @@ 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, @@ -122,7 +144,7 @@ def job_json_with_created_by(service_id=None, job_id=None): 'service': service_id if service_id else str(generate_uuid()), 'template': 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': '', @@ -145,11 +167,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 = { diff --git a/tests/app/main/views/test_template_history.py b/tests/app/main/views/test_template_history.py new file mode 100644 index 000000000..fe39532c4 --- /dev/null +++ b/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 + ) diff --git a/tests/conftest.py b/tests/conftest.py index 2a573f754..7a8104681 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,6 +9,7 @@ from . import ( service_json, TestClient, template_json, + template_version_json, api_key_json, job_json, notification_json, @@ -208,7 +209,47 @@ def mock_get_service_template(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') +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') @@ -220,7 +261,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') From d4225de45e56382530363457ea53a2f4485d767d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 16 May 2016 11:53:22 +0100 Subject: [PATCH 2/2] Update json structure for unit tests. --- app/main/views/jobs.py | 4 +++- tests/__init__.py | 10 ++++++---- tests/conftest.py | 4 +++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 05785a6b1..bb8275d2f 100644 --- a/app/main/views/jobs.py +++ b/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'], job['template_version'])['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( diff --git a/tests/__init__.py b/tests/__init__.py index d12d9990c..707aaa60e 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -53,14 +53,14 @@ def template_json(service_id, type_="sms", content="template content", subject=None, - versions=['1']): + versions='1'): template = { 'id': id_, 'name': name, 'template_type': type_, 'content': content, 'service': service_id, - 'versions': versions + 'version': versions } if subject is not None: template['subject'] = subject @@ -83,7 +83,6 @@ def template_version_json(service_id, created_at = datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S.%f') template['created_at'] = created_at template['version'] = version - del template['versions'] return template @@ -129,6 +128,7 @@ def job_json(): 'id': job_id, 'service': 1, 'template': 1, + 'template_version': 1, 'original_file_name': 'thisisatest.csv', 'created_at': created_at, 'notification_count': 1, @@ -143,6 +143,7 @@ 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.now().time()), 'notification_count': 1, @@ -190,7 +191,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, diff --git a/tests/conftest.py b/tests/conftest.py index 7a8104681..9bb87852f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -203,9 +203,11 @@ 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(