From cfb3f96b01d656850fbadbb933951193dcf83354 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 19 Jan 2016 15:54:12 +0000 Subject: [PATCH] Mapped template actions to the api and mocked tests. --- app/main/dao/services_dao.py | 6 +- app/main/dao/templates_dao.py | 49 +++++++++ app/main/forms.py | 12 ++- app/main/views/templates.py | 123 +++++++++++++--------- app/notify_client/api_client.py | 14 +++ app/templates/main_nav.html | 2 +- app/templates/views/edit-template.html | 8 +- app/templates/views/job.html | 2 +- app/templates/views/manage-templates.html | 12 +-- app/templates/views/send-sms.html | 2 +- tests/__init__.py | 10 ++ tests/app/main/views/test_templates.py | 103 +++++++++++++++--- tests/conftest.py | 66 +++++++++++- 13 files changed, 321 insertions(+), 88 deletions(-) create mode 100644 app/main/dao/templates_dao.py diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index aea05e075..cf4c096be 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -1,9 +1,5 @@ from flask import url_for -from datetime import datetime -from client.errors import HTTPError, InvalidResponse -from sqlalchemy.orm import load_only -from flask.ext.login import current_user -from app import (db, notifications_api_client) +from app import notifications_api_client from app.main.utils import BrowsableItem diff --git a/app/main/dao/templates_dao.py b/app/main/dao/templates_dao.py new file mode 100644 index 000000000..461af615c --- /dev/null +++ b/app/main/dao/templates_dao.py @@ -0,0 +1,49 @@ +from flask import url_for +from app import notifications_api_client +from app.main.utils import BrowsableItem + + +def insert_service_template(name, type_, content, service_id): + return notifications_api_client.create_service_template( + name, type_, content, service_id) + + +def update_service_template(id_, name, type_, content, service_id): + return notifications_api_client.update_service_template( + id_, name, type_, content, service_id) + + +def get_service_templates(service_id): + return notifications_api_client.get_service_templates(service_id) + + +def get_service_template(service_id, template_id): + return notifications_api_client.get_service_template( + service_id, template_id) + + +def delete_service_template(service_id, template_id): + return notifications_api_client.delete_service_template( + service_id, template_id) + + +class TemplatesBrowsableItem(BrowsableItem): + + @property + def title(self): + return self._item['name'] + + @property + def link(self): + return url_for( + 'main.edit_service_template', + service_id=self._item['service'], + template_id=self._item['id']) + + @property + def destructive(self): + return False + + @property + def hint(self): + return "Some service template hint here" diff --git a/app/main/forms.py b/app/main/forms.py index 11b201402..df9515a5c 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -6,7 +6,8 @@ from wtforms import ( PasswordField, ValidationError, TextAreaField, - FileField + FileField, + SelectField ) from wtforms.validators import DataRequired, Email, Length, Regexp @@ -187,8 +188,13 @@ class ConfirmPasswordForm(Form): class TemplateForm(Form): - template_name = StringField(u'Template name') - template_body = TextAreaField(u'Message') + name = StringField( + u'Template name', + validators=[DataRequired(message="Template name cannot be empty")]) + template_type = SelectField(u'Template type', choices=[('sms', 'SMS')]) + content = TextAreaField( + u'Message', + validators=[DataRequired(message="Template content cannot be empty")]) class ForgotPasswordForm(Form): diff --git a/app/main/views/templates.py b/app/main/views/templates.py index fdba437fe..f075601e9 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -3,80 +3,99 @@ from flask_login import login_required from app.main import main from app.main.forms import TemplateForm - -from ._templates import templates +from app.main.dao.services_dao import get_service_by_id +from app.main.dao import templates_dao as dao +from client.errors import HTTPError @main.route("/services//templates") @login_required -def manage_templates(service_id): +def manage_service_templates(service_id): + try: + templates = dao.get_service_templates(service_id)['data'] + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e return render_template( 'views/manage-templates.html', service_id=service_id, - templates=templates, - ) + templates=templates) @main.route("/services//templates/add", methods=['GET', 'POST']) @login_required -def add_template(service_id): +def add_service_template(service_id): + try: + service = dao.get_service_by_id(service_id)['data'] + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e form = TemplateForm() - if request.method == 'GET': - return render_template( - 'views/edit-template.html', - h1='Add template', - form=form, - service_id=service_id - ) - elif request.method == 'POST': - return redirect(url_for('.manage_templates', service_id=service_id)) + if form.validate_on_submit(): + dao.insert_service_template( + form.name.data, form.template_type.data, form.content.data, service_id) + return redirect(url_for( + '.manage_service_templates', service_id=service_id)) + return render_template( + 'views/edit-template.html', + h1='Add template', + form=form, + service_id=service_id) @main.route("/services//templates/", methods=['GET', 'POST']) @login_required -def edit_template(service_id, template_id): +def edit_service_template(service_id, template_id): + try: + template = dao.get_service_template(service_id, template_id)['data'] + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e + form = TemplateForm(form_data=template) - form = TemplateForm() + if form.validate_on_submit(): + dao.update_service_template( + template_id, form.name.data, form.template_type.data, + form.content.data, service_id) + return redirect(url_for('.manage_service_templates', service_id=service_id)) - form.template_name.data = templates[template_id - 1]['name'] - form.template_body.data = templates[template_id - 1]['body'] - - if request.method == 'GET': - return render_template( - 'views/edit-template.html', - h1='Edit template', - form=form, - service_id=service_id, - template_id=template_id - ) - elif request.method == 'POST': - return redirect(url_for('.manage_templates', service_id=service_id)) + return render_template( + 'views/edit-template.html', + h1='Edit template', + form=form, + service_id=service_id, + template_id=template_id) @main.route("/services//templates//delete", methods=['GET', 'POST']) @login_required -def delete_template(service_id, template_id): - - form = TemplateForm() - - form.template_name.data = templates[template_id - 1]['name'] - form.template_body.data = templates[template_id - 1]['body'] - - if request.method == 'GET': - - flash('Are you sure you want to delete ‘{}’?'.format(form.template_name.data), 'delete') - - return render_template( - 'views/edit-template.html', - h1='Edit template', - form=form, - service_id=service_id, - template_id=template_id - ) - elif request.method == 'POST': - if request.form.get('delete'): - return redirect(url_for('.manage_templates', service_id=service_id)) +def delete_service_template(service_id, template_id): + try: + template = dao.get_service_template(service_id, template_id)['data'] + except HTTPError as e: + if e.status_code == 404: + abort(404) else: - return redirect(url_for('.manage_templates', service_id=service_id)) + raise e + + form = TemplateForm(form_data=template) + + if form.validate_on_submit(): + dao.delete_service_template(service_id, template_id) + return redirect(url_for('.manage_service_templates', service_id=service_id)) + + flash('Are you sure you want to delete ‘{}’?'.format(form.name.data), 'delete') + return render_template( + 'views/edit-template.html', + h1='Edit template', + form=form, + service_id=service_id, + template_id=template_id) diff --git a/app/notify_client/api_client.py b/app/notify_client/api_client.py index 50217721b..8da2174db 100644 --- a/app/notify_client/api_client.py +++ b/app/notify_client/api_client.py @@ -83,6 +83,20 @@ class NotificationsAdminAPIClient(NotificationsAPIClient): endpoint = "/service/{0}/template".format(service_id) return self.post(endpoint, data) + def update_service_template(self, id_, name, type_, content, service_id): + """ + Update a service template. + """ + data = { + 'id': id_, + 'name': name, + 'template_type': type_, + 'content': content, + 'service': service_id + } + endpoint = "/service/{0}/template/{1}".format(service_id, id_) + return self.put(endpoint, data) + def get_service_template(self, service_id, template_id, *params): """ Retrieve a service template. diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index d4000984b..65cc28c57 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -6,7 +6,7 @@
  • Send text messages
  • Send emails
  • Activity
    • -
  • Templates
    • +
  • Templates
    • API keys and documentation
      • diff --git a/app/templates/views/edit-template.html b/app/templates/views/edit-template.html index 20cc74482..07155a454 100644 --- a/app/templates/views/edit-template.html +++ b/app/templates/views/edit-template.html @@ -11,12 +11,12 @@ GOV.UK Notify | Edit template

      {{ h1 }}

      - {{ textbox(form.template_name) }} - {{ textbox(form.template_body, highlight_tags=True) }} + {{ textbox(form.name) }} + {{ textbox(form.content, highlight_tags=True) }} {{ page_footer( 'Save', - delete_link=url_for('.delete_template', service_id=service_id, template_id=template_id) if template_id or None, - back_link=url_for('.manage_templates', service_id=service_id), + delete_link=url_for('.delete_service_template', service_id=service_id, template_id=template_id) if template_id or None, + back_link=url_for('.manage_service_templates', service_id=service_id), back_link_text='Back to templates' ) }}
      diff --git a/app/templates/views/job.html b/app/templates/views/job.html index b5970e947..01916ea24 100644 --- a/app/templates/views/job.html +++ b/app/templates/views/job.html @@ -38,7 +38,7 @@ GOV.UK Notify | Notifications activity

    - Sent with template {{ template_used }} at {{ uploaded_file_time }} + Sent with template {{ template_used }} at {{ uploaded_file_time }}

    {% call(item) list_table( diff --git a/app/templates/views/manage-templates.html b/app/templates/views/manage-templates.html index dbdbed51f..50e541959 100644 --- a/app/templates/views/manage-templates.html +++ b/app/templates/views/manage-templates.html @@ -15,21 +15,21 @@ GOV.UK Notify | Manage templates

    Templates

    - Create new template + Create new template {% for template in templates %} {% if template.type == 'sms' %} {{ sms_message( - template.body, + template.content, name=template.name, - edit_link=url_for('.edit_template', service_id=service_id, template_id=loop.index) + edit_link=url_for('.edit_service_template', service_id=template.service, template_id=template.id) ) }} {% elif template.type == 'email' %} {{ email_message( - template.subject, - template.body, + "template subject", + template.content, name=template.name, - edit_link=url_for('.edit_template', service_id=service_id, template_id=loop.index) + edit_link=url_for('.edit_service_template', service_id=template.service, template_id=template.id) ) }} {% endif %} {% endfor %} diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html index ebfa317cc..338497756 100644 --- a/app/templates/views/send-sms.html +++ b/app/templates/views/send-sms.html @@ -23,7 +23,7 @@

    - or create a new template + or create a new template

    2. Add recipients

    diff --git a/tests/__init__.py b/tests/__init__.py index 4bd8718e9..339886b10 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -29,6 +29,16 @@ def service_json(id_, name, users, limit=1000, active=False, restricted=True): } +def template_json(id_, name, type_, content, service_id): + return { + 'id': id_, + 'name': name, + 'template_type': type_, + 'content': content, + 'service': service_id + } + + TEST_USER_EMAIL = 'test@user.gov.uk' diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index da0489c7d..d010be260 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1,49 +1,128 @@ +import json from flask import url_for -def test_should_return_list_of_all_templates(app_, db_, db_session, active_user): +def test_should_return_list_of_all_templates(app_, + db_, + db_session, + active_user, + mock_get_service_templates): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user) - response = client.get(url_for('.manage_templates', service_id=123)) + service_id = 123 + response = client.get(url_for( + '.manage_service_templates', service_id=service_id)) assert response.status_code == 200 + mock_get_service_templates.assert_called_with(service_id) -def test_should_show_page_for_one_templates(app_, db_, db_session, active_user): +def test_should_show_page_for_one_templates(app_, + db_, + db_session, + active_user, + mock_get_service_template): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user) - response = client.get(url_for('.edit_template', service_id=123, template_id=1)) + service_id = 123 + template_id = 456 + response = client.get(url_for( + '.edit_service_template', + service_id=service_id, + template_id=template_id)) assert response.status_code == 200 + mock_get_service_template.assert_called_with( + service_id, template_id) -def test_should_redirect_when_saving_a_template(app_, db_, db_session, active_user): +def test_should_redirect_when_saving_a_template(app_, + db_, + db_session, + active_user, + mock_get_service_template, + mock_update_service_template): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user) - response = client.post(url_for('.edit_template', service_id=123, template_id=1)) + service_id = 123 + template_id = 456 + name = "new name" + type_ = "sms" + content = "template content" + data = { + 'id': template_id, + 'name': name, + 'template_type': type_, + "content": content, + "service": service_id + } + response = client.post(url_for( + '.edit_service_template', + service_id=service_id, + template_id=template_id), data=data) assert response.status_code == 302 - assert response.location == url_for('.manage_templates', service_id=123, _external=True) + assert response.location == url_for( + '.manage_service_templates', service_id=service_id, _external=True) + mock_update_service_template.assert_called_with( + template_id, name, type_, content, service_id) -def test_should_show_delete_template_page(app_, db_, db_session, active_user): +def test_should_show_delete_template_page(app_, + db_, + db_session, + active_user, + mock_get_service_template): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user) - response = client.get(url_for('.delete_template', service_id=123, template_id=1)) + service_id = 123 + template_id = 456 + response = client.get(url_for( + '.delete_service_template', + service_id=service_id, + template_id=template_id)) assert response.status_code == 200 assert 'Are you sure' in response.get_data(as_text=True) + mock_get_service_template.assert_called_with( + service_id, template_id) -def test_should_redirect_when_deleting_a_template(app_, db_, db_session, active_user): +def test_should_redirect_when_deleting_a_template(app_, + db_, + db_session, + active_user, + mock_get_service_template, + mock_delete_service_template): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user) - response = client.post(url_for('.delete_template', service_id=123, template_id=1)) + service_id = 123 + template_id = 456 + name = "new name" + type_ = "sms" + content = "template content" + data = { + 'id': template_id, + 'name': name, + 'template_type': type_, + 'content': content, + 'service': service_id + } + response = client.post(url_for( + '.delete_service_template', + service_id=service_id, + template_id=template_id), data=data) assert response.status_code == 302 - assert response.location == url_for('.manage_templates', service_id=123, _external=True) + assert response.location == url_for( + '.manage_service_templates', + service_id=service_id, _external=True) + mock_get_service_template.assert_called_with( + service_id, template_id) + mock_delete_service_template.assert_called_with( + service_id, template_id) diff --git a/tests/conftest.py b/tests/conftest.py index 7776cd767..48df70174 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,7 +7,7 @@ from flask.ext.script import Manager from sqlalchemy.schema import MetaData from . import ( create_test_user, create_another_test_user, service_json, TestClient, - get_test_user) + get_test_user, template_json) from app import create_app, db @@ -94,12 +94,11 @@ def mock_get_service(mocker, active_user): @pytest.fixture(scope='function') def mock_create_service(mocker): - # TODO fix token generation def _create(service_name, active, limit, restricted, user_id): service = service_json( 101, service_name, [user_id], limit=limit, active=active, restricted=restricted) - return {'data': service, 'token': 1} + return {'data': service} mock_class = mocker.patch( 'app.notifications_api_client.create_service', side_effect=_create) return mock_class @@ -142,3 +141,64 @@ def mock_delete_service(mocker, mock_get_service): mock_class = mocker.patch( 'app.notifications_api_client.delete_service', side_effect=_delete) return mock_class + + +@pytest.fixture(scope='function') +def mock_get_service_template(mocker): + def _create(service_id, template_id): + template = template_json( + template_id, "Template Name", "sms", "template content", service_id) + return {'data': template} + return mocker.patch( + 'app.notifications_api_client.get_service_template', + side_effect=_create) + + +@pytest.fixture(scope='function') +def mock_create_service_template(mocker): + def _create(name, type_, content, service): + template = template_json( + 101, name, type_, content, service) + return {'data': template} + mock_class = mocker.patch( + 'app.notifications_api_client.create_service_template', + side_effect=_create) + return mock_class + + +@pytest.fixture(scope='function') +def mock_update_service_template(mocker): + def _update(id_, name, type_, content, service): + template = template_json( + id_, name, type_, content, service) + return {'data': template} + mock_class = mocker.patch( + 'app.notifications_api_client.update_service_template', + side_effect=_update) + return mock_class + + +@pytest.fixture(scope='function') +def mock_get_service_templates(mocker): + def _create(service_id): + template_one = template_json( + 1, "template_one", "sms", "template one content", service_id) + template_two = template_json( + 2, "template_two", "sms", "template two content", service_id) + return {'data': [template_one, template_two]} + mock_class = mocker.patch( + 'app.notifications_api_client.get_service_templates', + side_effect=_create) + return mock_class + + +@pytest.fixture(scope='function') +def mock_delete_service_template(mocker): + def _delete(service_id, template_id): + template = template_json( + template_id, "Template to delete", + "sms", "content to be deleted", service_id) + return {'data': template} + mock_class = mocker.patch( + 'app.notifications_api_client.delete_service_template', side_effect=_delete) + return mock_class