From 6e19dd7c9cb09a5d217daac4224f41ffee50800d Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Mon, 18 Jan 2016 16:01:04 +0000 Subject: [PATCH] Added choose_service using services_dao. --- app/main/dao/services_dao.py | 4 + app/main/forms.py | 4 +- app/main/views/service_settings.py | 150 +++++++---- app/templates/views/service-settings.html | 10 +- .../views/service-settings/name.html | 2 +- tests/__init__.py | 7 +- tests/app/main/views/test_service_settings.py | 239 +++++++++++------- tests/conftest.py | 24 +- 8 files changed, 296 insertions(+), 144 deletions(-) diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index a46f7d1b4..39d2c2ac6 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -59,6 +59,10 @@ def find_service_by_service_name(service_name): return retval +def delete_service(id_): + return notifications_api_client.delete_service(id_) + + def find_all_service_names(): resp = notifications_api_client.get_services() return [x['name'] for x in resp['data']] diff --git a/app/main/forms.py b/app/main/forms.py index 36eccc981..95e9b4279 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -166,7 +166,7 @@ class AddServiceForm(Form): self._names_func = names_func super(AddServiceForm, self).__init__(*args, **kwargs) - service_name = StringField(validators=[ + name = StringField('Service Name', validators=[ DataRequired(message='Service name can not be empty')]) def validate_service_name(self, a): @@ -175,7 +175,7 @@ class AddServiceForm(Form): class ServiceNameForm(Form): - service_name = StringField(u'New name') + name = StringField(u'New name') class ConfirmPasswordForm(Form): diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 8bf9f702f..63dd24fac 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -2,18 +2,21 @@ from flask import render_template, redirect, request, url_for, abort from flask_login import login_required from app.main import main +from app.main.dao.services_dao import (get_service_by_id, delete_service) from app.main.forms import ConfirmPasswordForm, ServiceNameForm - -service = { - 'name': 'Service name', - 'live': False, - 'active': True -} +from client.errors import HTTPError @main.route("/services//service-settings") @login_required def service_settings(service_id): + try: + service = get_service_by_id(service_id)['data'] + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e return render_template( 'views/service-settings.html', service=service, @@ -23,42 +26,60 @@ def service_settings(service_id): @main.route("/services//service-settings/name", methods=['GET', 'POST']) @login_required -def name(service_id): +def service_name_change(service_id): + try: + service = get_service_by_id(service_id) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e form = ServiceNameForm() - form.service_name.data = 'Service name' - if request.method == 'GET': - return render_template( + if form.validate_on_submit(): + return redirect(url_for('.service_name_change_confirm', service_id=service_id)) + + return render_template( 'views/service-settings/name.html', service=service, form=form, - service_id=service_id - ) - elif request.method == 'POST': - return redirect(url_for('.confirm_name_change', service_id=service_id)) + service_id=service_id) @main.route("/services//service-settings/name/confirm", methods=['GET', 'POST']) @login_required -def confirm_name_change(service_id): +def service_name_change_confirm(service_id): + try: + service = get_service_by_id(service_id) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e form = ConfirmPasswordForm() - if request.method == 'GET': - return render_template( + if form.validate_on_submit(): + # TODO send call to API + return redirect(url_for('.service_settings', service_id=service_id)) + return render_template( 'views/service-settings/confirm.html', heading='Change your service name', form=form, - service_id=service_id - ) - elif request.method == 'POST': - return redirect(url_for('.service_settings', service_id=service_id)) + service_id=service_id) @main.route("/services//service-settings/request-to-go-live", methods=['GET', 'POST']) @login_required -def request_to_go_live(service_id): +def service_request_to_go_live(service_id): + try: + service = get_service_by_id(service_id) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e if request.method == 'GET': return render_template( 'views/service-settings/request-to-go-live.html', @@ -66,12 +87,21 @@ def request_to_go_live(service_id): service_id=service_id ) elif request.method == 'POST': + # TODO send call to API return redirect(url_for('.service_settings', service_id=service_id)) @main.route("/services//service-settings/status", methods=['GET', 'POST']) @login_required -def status(service_id): +def service_status_change(service_id): + try: + service = get_service_by_id(service_id) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e + if request.method == 'GET': return render_template( 'views/service-settings/status.html', @@ -79,30 +109,46 @@ def status(service_id): service_id=service_id ) elif request.method == 'POST': - return redirect(url_for('.confirm_status_change', service_id=service_id)) + # TODO send call to API + return redirect(url_for('.service_status_change_confirm', service_id=service_id)) @main.route("/services//service-settings/status/confirm", methods=['GET', 'POST']) @login_required -def confirm_status_change(service_id): +def service_status_change_confirm(service_id): + try: + service = get_service_by_id(service_id) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e + # TODO validate password, will leave until + # user management has been moved to the api. form = ConfirmPasswordForm() - if request.method == 'GET': - return render_template( + if form.validate_on_submit(): + return redirect(url_for('.service_settings', service_id=service_id)) + return render_template( 'views/service-settings/confirm.html', heading='Turn off all outgoing notifications', destructive=True, form=form, - service_id=service_id - ) - elif request.method == 'POST': - return redirect(url_for('.service_settings', service_id=service_id)) + service_id=service_id) @main.route("/services//service-settings/delete", methods=['GET', 'POST']) @login_required -def delete(service_id): +def service_delete(service_id): + try: + service = get_service_by_id(service_id) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e + if request.method == 'GET': return render_template( 'views/service-settings/delete.html', @@ -110,22 +156,36 @@ def delete(service_id): service_id=service_id ) elif request.method == 'POST': - return redirect(url_for('.confirm_delete', service_id=service_id)) + return redirect(url_for('.service_delete_confirm', service_id=service_id)) @main.route("/services//service-settings/delete/confirm", methods=['GET', 'POST']) @login_required -def confirm_delete(service_id): - +def service_delete_confirm(service_id): + try: + service = get_service_by_id(service_id) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e + # TODO validate password, will leave until + # user management has been moved to the api. form = ConfirmPasswordForm() - if request.method == 'GET': - return render_template( - 'views/service-settings/confirm.html', - heading='Delete this service from Notify', - destructive=True, - form=form, - service_id=service_id - ) - elif request.method == 'POST': - return redirect(url_for('.service_dashboard', service_id=service_id)) + if form.validate_on_submit(): + try: + service = delete_service(service_id) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e + return redirect(url_for('.choose_service')) + + return render_template( + 'views/service-settings/confirm.html', + heading='Delete this service from Notify', + destructive=True, + form=form, + service_id=service_id) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 7a3c06f62..fa29cccee 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -12,26 +12,26 @@ {{ browse_list([ { 'title': 'Change your service name', - 'link': url_for('.name', service_id=service_id), + 'link': url_for('.service_name_change', service_id=service_id), 'hint': 'Your service name ({}) is included in every sent notification'.format(service.name) }, { 'title': 'Request to go live and turn off sending restrictions', - 'link': url_for('.request_to_go_live', service_id=service_id), + 'link': url_for('.service_request_to_go_live', service_id=service_id), 'hint': 'A live service can send notifications to any phone number or email address', } if not service.live else { }, { 'title': 'Turn off all outgoing notifications', - 'link': url_for('.status', service_id=service_id), + 'link': url_for('.service_status_change', service_id=service_id), 'destructive': True } if service.active else { 'title': 'Restart sending notifications', - 'link': url_for('.status', service_id=service_id) + 'link': url_for('.service_status_change', service_id=service_id) }, { 'title': 'Delete this service from Notify', - 'link': url_for('.delete', service_id=service_id), + 'link': url_for('.service_delete', service_id=service_id), 'destructive': True }, ]) }} diff --git a/app/templates/views/service-settings/name.html b/app/templates/views/service-settings/name.html index 86a7a1039..1f133d4c2 100644 --- a/app/templates/views/service-settings/name.html +++ b/app/templates/views/service-settings/name.html @@ -21,7 +21,7 @@ GOV.UK Notify | Service settings
- {{ textbox(form.service_name) }} + {{ textbox(form.name) }} {{ page_footer( 'Save', back_link=url_for('.service_settings', service_id=service_id) diff --git a/tests/__init__.py b/tests/__init__.py index 648047b46..b2806dd6f 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -28,11 +28,12 @@ def service_json(id_, name, users, limit=1000, active=False, restricted=True): 'restricted': restricted } +TEST_USER_EMAIL = 'test@user.gov.uk' def create_test_user(state): user = User(name='Test User', password='somepassword', - email_address='test@user.gov.uk', + email_address=TEST_USER_EMAIL, mobile_number='+441234123412', role_id=1, state=state) @@ -49,3 +50,7 @@ def create_another_test_user(state): state=state) users_dao.insert_user(user) return user + + +def get_test_user(): + return users_dao.get_user_by_email(TEST_USER_EMAIL) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index f22963885..a010fefb6 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1,165 +1,230 @@ -from tests import create_test_user +from flask import url_for -def test_should_show_overview(app_, db_, db_session): +def test_should_show_overview(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.get('/services/123/service-settings') + client.login(active_user) + service_id = 123 + response = client.get(url_for( + 'main.service_settings', service_id=service_id)) assert response.status_code == 200 - assert 'Service settings' in response.get_data(as_text=True) + resp_data = response.get_data(as_text=True) + assert 'Service settings' in resp_data + service = mock_get_service.side_effect(service_id)['data'] + assert service['name'] in resp_data + assert mock_get_service.called -def test_should_show_service_name(app_, db_, db_session): +def test_should_show_service_name(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.get('/services/123/service-settings/name') + client.login(active_user) + service_id = 123 + response = client.get(url_for( + 'main.service_name_change', service_id=service_id)) assert response.status_code == 200 - assert 'Change your service name' in response.get_data(as_text=True) + resp_data = response.get_data(as_text=True) + assert 'Change your service name' in resp_data + assert mock_get_service.called + service = mock_get_service.side_effect(service_id)['data'] + assert service['name'] in resp_data -def test_should_redirect_after_change_service_name(app_, db_, db_session): +def test_should_redirect_after_change_service_name(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.post('/services/123/service-settings/request-to-go-live') + client.login(active_user) + service_id = 123 + response = client.post(url_for( + 'main.service_name_change', service_id=service_id)) assert response.status_code == 302 - assert 'http://localhost/services/123/service-settings' == response.location + settings_url = url_for( + 'main.service_settings', service_id=service_id, _external=True) + assert settings_url == response.location + assert mock_get_service.called -def test_should_show_service_name_confirmation(app_, db_, db_session): +def test_should_show_service_name_confirmation(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.get('/services/123/service-settings/name/confirm') + client.login(active_user) + service_id = 123 + response = client.get(url_for( + 'main.service_name_change_confirm', service_id=service_id)) assert response.status_code == 200 - assert 'Change your service name' in response.get_data(as_text=True) + resp_data = response.get_data(as_text=True) + assert 'Change your service name' in resp_data + assert mock_get_service.called -def test_should_redirect_after_service_name_confirmation(app_, db_, - db_session): +def test_should_redirect_after_service_name_confirmation(app_, + db_, + db_session, + active_user, + mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.post('/services/123/service-settings/name/confirm') + client.login(active_user) + service_id = 123 + response = client.post(url_for( + 'main.service_name_change_confirm', service_id=service_id)) assert response.status_code == 302 - assert 'http://localhost/services/123/service-settings' == response.location + settings_url = url_for( + 'main.service_settings', service_id=service_id, _external=True) + assert settings_url == response.location + assert mock_get_service.called -def test_should_show_request_to_go_live(app_, db_, db_session): +def test_should_show_request_to_go_live(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.get('/services/123/service-settings/request-to-go-live') + client.login(active_user) + service_id = 123 + response = client.get( + url_for('main.service_request_to_go_live', service_id=service_id)) + service = mock_get_service.side_effect(service_id)['data'] + assert response.status_code == 200 + resp_data = response.get_data(as_text=True) + assert 'Request to go live' in resp_data + assert mock_get_service.called + assert service['name'] in resp_data + + +def test_should_redirect_after_request_to_go_live(app_, + db_, + db_session, + active_user, + mock_get_service): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(active_user) + service_id = 123 + response = client.post(url_for( + 'main.service_request_to_go_live', service_id=service_id)) + + assert response.status_code == 302 + settings_url = url_for( + 'main.service_settings', service_id=service_id, _external=True) + assert settings_url == response.location + assert mock_get_service.called + + +def test_should_show_status_page(app_, db_, db_session, active_user, mock_get_service): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(active_user) + service_id = 123 + response = client.get(url_for( + 'main.service_status_change', service_id=service_id)) assert response.status_code == 200 - assert 'Request to go live' in response.get_data(as_text=True) + resp_data = response.get_data(as_text=True) + assert 'Turn off all outgoing notifications' in resp_data + assert mock_get_service.called -def test_should_redirect_after_request_to_go_live(app_, db_, db_session): +def test_should_show_redirect_after_status_change(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.post('/services/123/service-settings/request-to-go-live') + client.login(active_user) + service_id = 123 + response = client.post(url_for( + 'main.service_status_change', service_id=service_id)) assert response.status_code == 302 - assert 'http://localhost/services/123/service-settings' == response.location + redirect_url = url_for( + 'main.service_status_change_confirm', service_id=service_id) + assert redirect_url == response.location -def test_should_show_status_page(app_, db_, db_session): +def test_should_show_status_confirmation(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.get('/services/123/service-settings/status') + client.login(active_user) + service_id = 123 + response = client.get(url_for( + 'main.service_status_change_confirm', service_id=service_id)) assert response.status_code == 200 - assert 'Turn off all outgoing notifications' in response.get_data(as_text=True) + resp_data = response.get_data(as_text=True) + assert 'Turn off all outgoing notifications' in resp_data + assert mock_get_service.called -def test_should_show_redirect_after_status_change(app_, db_, db_session): +def test_should_redirect_after_status_confirmation(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.post('/services/123/service-settings/status') + client.login(active_user) + service_id = 123 + response = client.post(url_for( + 'main.service_status_change_confirm', service_id=service_id)) assert response.status_code == 302 - assert 'http://localhost/services/123/service-settings/status/confirm' == response.location + settings_url = url_for( + 'main.service_settings', service_id=service_id, _external=True) + assert settings_url == response.location -def test_should_show_status_confirmation(app_, db_, db_session): +def test_should_show_delete_page(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.get('/services/123/service-settings/status/confirm') - - assert response.status_code == 200 - assert 'Turn off all outgoing notifications' in response.get_data(as_text=True) - - -def test_should_redirect_after_status_confirmation(app_, db_, db_session): - with app_.test_request_context(): - with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.post('/services/123/service-settings/status/confirm') - - assert response.status_code == 302 - assert 'http://localhost/services/123/service-settings' == response.location - - -def test_should_show_delete_page(app_, db_, db_session): - with app_.test_request_context(): - with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.get('/services/123/service-settings/delete') + client.login(active_user) + service_id = 123 + response = client.get(url_for( + 'main.service_delete', service_id=service_id)) assert response.status_code == 200 assert 'Delete this service from Notify' in response.get_data(as_text=True) + assert mock_get_service.called -def test_should_show_redirect_after_deleting_service(app_, db_, db_session): +def test_should_show_redirect_after_deleting_service(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.post('/services/123/service-settings/delete') + client.login(active_user) + service_id = 123 + response = client.post(url_for( + 'main.service_delete', service_id=service_id)) assert response.status_code == 302 - assert 'http://localhost/services/123/service-settings/delete/confirm' == response.location + delete_url = url_for( + 'main.service_delete_confirm', service_id=service_id) + assert delete_url == response.location -def test_should_show_delete_confirmation(app_, db_, db_session): +def test_should_show_delete_confirmation(app_, db_, db_session, active_user, mock_get_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.get('/services/123/service-settings/delete/confirm') + client.login(active_user) + service_id = 123 + response = client.get(url_for( + 'main.service_delete_confirm', service_id=service_id)) assert response.status_code == 200 assert 'Delete this service from Notify' in response.get_data(as_text=True) + assert mock_get_service.called -def test_should_redirect_delete_confirmation(app_, db_, db_session): +def test_should_redirect_delete_confirmation(app_, + db_, + db_session, + active_user, + mock_get_service, + mock_delete_service): with app_.test_request_context(): with app_.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.post('/services/123/service-settings/delete/confirm') + client.login(active_user) + service_id = 123 + response = client.post(url_for( + 'main.service_delete_confirm', service_id=service_id)) assert response.status_code == 302 - assert 'http://localhost/services/123/dashboard' == response.location + choose_url = url_for( + 'main.choose_service', _external=True) + assert choose_url == response.location + assert mock_get_service.called + assert mock_delete_service.called diff --git a/tests/conftest.py b/tests/conftest.py index 2c1ea47e1..1e71b6c1c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,7 +6,8 @@ from flask.ext.migrate import Migrate, MigrateCommand from flask.ext.script import Manager from sqlalchemy.schema import MetaData from . import ( - create_test_user, create_another_test_user, service_json, TestClient) + create_test_user, create_another_test_user, service_json, TestClient, + get_test_user) from app import create_app, db @@ -66,6 +67,9 @@ def service_one(request): @pytest.fixture(scope='function') def active_user(request, db_, db_session): + usr = get_test_user() + if usr: + return usr return create_test_user('active') @@ -80,8 +84,13 @@ def mock_send_email(request, mocker): @pytest.fixture(scope='function') -def mock_get_service(mocker): - return mocker.patch('app.notifications_api_client.get_service') +def mock_get_service(mocker, active_user): + def _create(service_id): + service = service_json( + service_id, "Test Service", [active_user.id], limit=1000, + active=False, restricted=True) + return {'data': service, 'token': 1} + return mocker.patch('app.notifications_api_client.get_service', side_effect=_create) @pytest.fixture(scope='function') @@ -125,3 +134,12 @@ def mock_get_services(mocker, active_user): mock_class = mocker.patch( 'app.notifications_api_client.get_services', side_effect=_create) return mock_class + + +@pytest.fixture(scope='function') +def mock_delete_service(mocker, mock_get_service): + def _delete(service_id): + return mock_get_service.side_effect(service_id) + mock_class = mocker.patch( + 'app.notifications_api_client.delete_service', side_effect=_delete) + return mock_class