From 609f5f0a8d6977c4da911fcdd8b165f5db218a39 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Mon, 18 Jan 2016 17:35:28 +0000 Subject: [PATCH] Working service integration. --- app/main/dao/services_dao.py | 10 ++ app/main/forms.py | 4 +- app/main/views/add_service.py | 2 +- app/main/views/service_settings.py | 56 ++++--- app/templates/views/add-service.html | 2 +- tests/__init__.py | 2 + tests/app/main/dao/test_service_dao.py | 13 +- tests/app/main/test_add_service_form.py | 4 +- tests/app/main/views/test_add_service.py | 9 +- tests/app/main/views/test_service_settings.py | 150 ++++++++++-------- tests/conftest.py | 5 +- 11 files changed, 147 insertions(+), 110 deletions(-) diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index 39d2c2ac6..aea05e075 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -16,6 +16,16 @@ def insert_new_service(service_name, user_id): return resp['data']['id'] +def update_service(service): + return notifications_api_client.update_service( + service['id'], + service['name'], + service['active'], + service['limit'], + service['restricted'], + service['users']) + + def get_service_by_id(id_): return notifications_api_client.get_service(id_) diff --git a/app/main/forms.py b/app/main/forms.py index 95e9b4279..2a5a62dcf 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -169,8 +169,8 @@ class AddServiceForm(Form): name = StringField('Service Name', validators=[ DataRequired(message='Service name can not be empty')]) - def validate_service_name(self, a): - if self.service_name.data in self._names_func(): + def validate_name(self, a): + if a.data in self._names_func(): raise ValidationError('Service name already exists') diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index f011a2ab8..4bf3797af 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -12,7 +12,7 @@ def add_service(): form = AddServiceForm(services_dao.find_all_service_names) if form.validate_on_submit(): user = users_dao.get_user_by_id(session['user_id']) - service_id = services_dao.insert_new_service(form.service_name.data, user) + service_id = services_dao.insert_new_service(form.name.data, user) return redirect(url_for('main.service_dashboard', service_id=service_id)) else: return render_template('views/add-service.html', form=form) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 63dd24fac..412e2844d 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -1,8 +1,10 @@ -from flask import render_template, redirect, request, url_for, abort +from flask import ( + render_template, redirect, request, url_for, abort, session) 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.dao.services_dao import ( + get_service_by_id, delete_service, update_service) from app.main.forms import ConfirmPasswordForm, ServiceNameForm from client.errors import HTTPError @@ -28,7 +30,7 @@ def service_settings(service_id): @login_required def service_name_change(service_id): try: - service = get_service_by_id(service_id) + service = get_service_by_id(service_id)['data'] except HTTPError as e: if e.status_code == 404: abort(404) @@ -38,20 +40,21 @@ def service_name_change(service_id): form = ServiceNameForm() if form.validate_on_submit(): + session['service_name_change'] = form.name.data 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) + 'views/service-settings/name.html', + service=service, + form=form, + service_id=service_id) @main.route("/services//service-settings/name/confirm", methods=['GET', 'POST']) @login_required def service_name_change_confirm(service_id): try: - service = get_service_by_id(service_id) + service = get_service_by_id(service_id)['data'] except HTTPError as e: if e.status_code == 404: abort(404) @@ -61,20 +64,21 @@ def service_name_change_confirm(service_id): form = ConfirmPasswordForm() if form.validate_on_submit(): - # TODO send call to API + service['name'] = session['service_name_change'] + update_service(service) 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) + 'views/service-settings/confirm.html', + heading='Change your service name', + form=form, + service_id=service_id) @main.route("/services//service-settings/request-to-go-live", methods=['GET', 'POST']) @login_required def service_request_to_go_live(service_id): try: - service = get_service_by_id(service_id) + service = get_service_by_id(service_id)['data'] except HTTPError as e: if e.status_code == 404: abort(404) @@ -87,7 +91,8 @@ def service_request_to_go_live(service_id): service_id=service_id ) elif request.method == 'POST': - # TODO send call to API + service['restricted'] + update_service(service) return redirect(url_for('.service_settings', service_id=service_id)) @@ -95,7 +100,7 @@ def service_request_to_go_live(service_id): @login_required def service_status_change(service_id): try: - service = get_service_by_id(service_id) + service = get_service_by_id(service_id)['data'] except HTTPError as e: if e.status_code == 404: abort(404) @@ -109,7 +114,6 @@ def service_status_change(service_id): service_id=service_id ) elif request.method == 'POST': - # TODO send call to API return redirect(url_for('.service_status_change_confirm', service_id=service_id)) @@ -117,7 +121,7 @@ def service_status_change(service_id): @login_required def service_status_change_confirm(service_id): try: - service = get_service_by_id(service_id) + service = get_service_by_id(service_id)['data'] except HTTPError as e: if e.status_code == 404: abort(404) @@ -129,20 +133,22 @@ def service_status_change_confirm(service_id): form = ConfirmPasswordForm() if form.validate_on_submit(): + service['active'] = True + update_service(service) 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) + 'views/service-settings/confirm.html', + heading='Turn off all outgoing notifications', + destructive=True, + form=form, + service_id=service_id) @main.route("/services//service-settings/delete", methods=['GET', 'POST']) @login_required def service_delete(service_id): try: - service = get_service_by_id(service_id) + service = get_service_by_id(service_id)['data'] except HTTPError as e: if e.status_code == 404: abort(404) @@ -163,7 +169,7 @@ def service_delete(service_id): @login_required def service_delete_confirm(service_id): try: - service = get_service_by_id(service_id) + service = get_service_by_id(service_id)['data'] except HTTPError as e: if e.status_code == 404: abort(404) diff --git a/app/templates/views/add-service.html b/app/templates/views/add-service.html index 1e5784b58..03baddfb4 100644 --- a/app/templates/views/add-service.html +++ b/app/templates/views/add-service.html @@ -18,7 +18,7 @@ GOV.UK Notify | Set up service
- {{ textbox(form.service_name) }} + {{ textbox(form.name) }}

diff --git a/tests/__init__.py b/tests/__init__.py index b2806dd6f..4bd8718e9 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -28,8 +28,10 @@ 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', diff --git a/tests/app/main/dao/test_service_dao.py b/tests/app/main/dao/test_service_dao.py index 11601322e..f1c0f6c9a 100644 --- a/tests/app/main/dao/test_service_dao.py +++ b/tests/app/main/dao/test_service_dao.py @@ -16,9 +16,8 @@ def test_can_insert_new_service(db_, def test_unrestrict_service_updates_the_service(db_, db_session, mock_get_service, - mock_update_service, - service_one): - mock_get_service.return_value = {'data': service_one} + mock_update_service): + service_one = mock_get_service.side_effect(123)['data'] services_dao.unrestrict_service(service_one['id']) mock_update_service.assert_called_once_with(service_one['id'], service_one['name'], @@ -30,10 +29,10 @@ def test_unrestrict_service_updates_the_service(db_, def test_activate_service_update_service(db_, db_session, + active_user, mock_get_service, - mock_update_service, - service_one): - mock_get_service.return_value = {'data': service_one} + mock_update_service): + service_one = mock_get_service.side_effect(123)['data'] services_dao.activate_service(service_one['id']) mock_update_service.assert_called_once_with(service_one['id'], service_one['name'], @@ -44,7 +43,7 @@ def test_activate_service_update_service(db_, def test_get_service_returns_none_if_service_does_not_exist(db_, db_session, mock_get_service): - mock_get_service.return_value = None + mock_get_service.side_effect = lambda x: None service = services_dao.get_service_by_id(1) assert service is None diff --git a/tests/app/main/test_add_service_form.py b/tests/app/main/test_add_service_form.py index 5f97b7c23..4bc866501 100644 --- a/tests/app/main/test_add_service_form.py +++ b/tests/app/main/test_add_service_form.py @@ -9,6 +9,6 @@ def test_form_should_have_errors_when_duplicate_service_is_added(app_, return ['some service', 'more names'] with app_.test_request_context(): form = AddServiceForm(_get_form_names, - formdata=MultiDict([('service_name', 'some service')])) + formdata=MultiDict([('name', 'some service')])) form.validate() - assert {'service_name': ['Service name already exists']} == form.errors + assert {'name': ['Service name already exists']} == form.errors diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 1f12bee39..c7b203e6f 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -4,11 +4,10 @@ from tests import create_test_user from app.models import User -def test_get_should_render_add_service_template(app_, db_, db_session, mock_get_service): +def test_get_should_render_add_service_template(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) + client.login(active_user) response = client.get(url_for('main.add_service')) assert response.status_code == 200 assert 'Set up notifications for your service' in response.get_data(as_text=True) @@ -25,7 +24,7 @@ def test_should_add_service_and_redirect_to_next_page(app_, client.login(user) response = client.post( url_for('main.add_service'), - data={'service_name': 'testing the post'}) + data={'name': 'testing the post'}) assert response.status_code == 302 assert response.location == url_for('main.service_dashboard', service_id=101, _external=True) assert mock_create_service.called @@ -54,7 +53,7 @@ def test_should_return_form_errors_with_duplicate_service_name(app_, user = User.query.first() client.login(user) response = client.post( - url_for('main.add_service'), data={'service_name': 'service_one'}) + url_for('main.add_service'), data={'name': 'service_one'}) assert response.status_code == 200 assert 'Service name already exists' in response.get_data(as_text=True) assert mock_get_services.called diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index a010fefb6..4d0647b01 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1,4 +1,4 @@ -from flask import url_for +from flask import (url_for, session) def test_should_show_overview(app_, db_, db_session, active_user, mock_get_service): @@ -28,7 +28,6 @@ def test_should_show_service_name(app_, db_, db_session, active_user, mock_get_s 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, active_user, mock_get_service): @@ -39,14 +38,18 @@ def test_should_redirect_after_change_service_name(app_, db_, db_session, active response = client.post(url_for( 'main.service_name_change', 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 + assert response.status_code == 302 + settings_url = url_for( + 'main.service_name_change_confirm', 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, active_user, mock_get_service): +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: client.login(active_user) @@ -54,29 +57,35 @@ def test_should_show_service_name_confirmation(app_, db_, db_session, active_use response = client.get(url_for( 'main.service_name_change_confirm', service_id=service_id)) - assert response.status_code == 200 - resp_data = response.get_data(as_text=True) - assert 'Change your service name' in resp_data - assert mock_get_service.called + assert response.status_code == 200 + 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, active_user, - mock_get_service): + mock_get_service, + mock_update_service): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user) service_id = 123 + service_new_name = 'New Name' + with client.session_transaction() as session: + session['service_name_change'] = service_new_name response = client.post(url_for( 'main.service_name_change_confirm', 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 + assert response.status_code == 302 + settings_url = url_for( + 'main.service_settings', service_id=service_id, _external=True) + resp_data = response.get_data(as_text=True) + assert settings_url == response.location + assert mock_get_service.called + assert mock_update_service.called def test_should_show_request_to_go_live(app_, db_, db_session, active_user, mock_get_service): @@ -86,19 +95,19 @@ def test_should_show_request_to_go_live(app_, db_, db_session, active_user, mock 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 + 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 def test_should_redirect_after_request_to_go_live(app_, db_, db_session, active_user, - mock_get_service): + mock_get_service, + mock_update_service): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user) @@ -106,11 +115,12 @@ def test_should_redirect_after_request_to_go_live(app_, 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 + 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 + assert mock_update_service.called def test_should_show_status_page(app_, db_, db_session, active_user, mock_get_service): @@ -121,13 +131,17 @@ def test_should_show_status_page(app_, db_, db_session, active_user, mock_get_se response = client.get(url_for( 'main.service_status_change', service_id=service_id)) - assert response.status_code == 200 - resp_data = response.get_data(as_text=True) - assert 'Turn off all outgoing notifications' in resp_data - assert mock_get_service.called + assert response.status_code == 200 + 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, active_user, mock_get_service): +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: client.login(active_user) @@ -135,10 +149,11 @@ def test_should_show_redirect_after_status_change(app_, db_, db_session, active_ response = client.post(url_for( 'main.service_status_change', service_id=service_id)) - assert response.status_code == 302 - redirect_url = url_for( - 'main.service_status_change_confirm', service_id=service_id) - assert redirect_url == response.location + assert response.status_code == 302 + redirect_url = url_for( + 'main.service_status_change_confirm', service_id=service_id, _external=True) + assert redirect_url == response.location + assert mock_get_service.called def test_should_show_status_confirmation(app_, db_, db_session, active_user, mock_get_service): @@ -149,13 +164,18 @@ def test_should_show_status_confirmation(app_, db_, db_session, active_user, moc response = client.get(url_for( 'main.service_status_change_confirm', service_id=service_id)) - assert response.status_code == 200 - resp_data = response.get_data(as_text=True) - assert 'Turn off all outgoing notifications' in resp_data - assert mock_get_service.called + assert response.status_code == 200 + 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_status_confirmation(app_, db_, db_session, active_user, mock_get_service): +def test_should_redirect_after_status_confirmation(app_, + db_, + db_session, + active_user, + mock_get_service, + mock_update_service): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user) @@ -163,10 +183,12 @@ def test_should_redirect_after_status_confirmation(app_, db_, db_session, active response = client.post(url_for( 'main.service_status_change_confirm', 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 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 + assert mock_update_service.called def test_should_show_delete_page(app_, db_, db_session, active_user, mock_get_service): @@ -177,9 +199,9 @@ def test_should_show_delete_page(app_, db_, db_session, active_user, mock_get_se 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 + 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, active_user, mock_get_service): @@ -190,10 +212,10 @@ def test_should_show_redirect_after_deleting_service(app_, db_, db_session, acti response = client.post(url_for( 'main.service_delete', service_id=service_id)) - assert response.status_code == 302 - delete_url = url_for( - 'main.service_delete_confirm', service_id=service_id) - assert delete_url == response.location + assert response.status_code == 302 + delete_url = url_for( + 'main.service_delete_confirm', service_id=service_id, _external=True) + assert delete_url == response.location def test_should_show_delete_confirmation(app_, db_, db_session, active_user, mock_get_service): @@ -204,9 +226,9 @@ def test_should_show_delete_confirmation(app_, db_, db_session, active_user, moc 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 + 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_, @@ -222,9 +244,9 @@ def test_should_redirect_delete_confirmation(app_, response = client.post(url_for( 'main.service_delete_confirm', service_id=service_id)) - assert response.status_code == 302 - choose_url = url_for( - 'main.choose_service', _external=True) - assert choose_url == response.location - assert mock_get_service.called - assert mock_delete_service.called + assert response.status_code == 302 + 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 1e71b6c1c..7776cd767 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -60,9 +60,8 @@ def db_session(request): @pytest.fixture(scope='function') -def service_one(request): - usr = create_test_user('active') - return service_json(1, 'service one', [usr]) +def service_one(request, active_user): + return service_json(1, 'service one', [active_user.id]) @pytest.fixture(scope='function')