Added choose_service using services_dao.

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']]

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):

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/<int:service_id>/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/<int:service_id>/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/<int:service_id>/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/<int:service_id>/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/<int:service_id>/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/<int:service_id>/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/<int:service_id>/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/<int:service_id>/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)

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
       },
     ]) }}

app/templates/views/service-settings/name.html

@@ -21,7 +21,7 @@ GOV.UK Notify | Service settings
       </ul>
 
       <form method="post">
-        {{ textbox(form.service_name) }}
+        {{ textbox(form.name) }}
         {{ page_footer(
           'Save',
           back_link=url_for('.service_settings', service_id=service_id)

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)

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

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