Let the API error when the service name is a duplicate.

- Remove the name_func from the AddServiceForm
- Catch the HTTPError and set the error on the form

app/main/forms.py

@@ -4,7 +4,6 @@ import pytz
 from flask_wtf import FlaskForm as Form
 from datetime import datetime, timedelta
 
-from notifications_python_client.errors import HTTPError
 from notifications_utils.recipients import (
     validate_phone_number,
     InvalidPhoneError
@@ -223,28 +222,6 @@ class AddServiceForm(Form):
         ]
     )
 
-    service_id = HiddenField('service_id')
-
-    def _create_service(self, service_name, email_from):
-        from app import service_api_client
-        from flask import current_app, session
-        service_id = service_api_client.create_service(service_name=service_name,
-                                                       message_limit=current_app.config['DEFAULT_SERVICE_LIMIT'],
-                                                       restricted=True,
-                                                       user_id=session['user_id'],
-                                                       email_from=email_from)
-        session['service_id'] = service_id
-        return service_id
-
-    def validate_name(self, a):
-        from app.utils import email_safe
-        email_from = email_safe(a.data)
-        try:
-            self.service_id = self._create_service(a.data, email_from)
-        except HTTPError as e:
-            if e.status_code == 400 and e.message['name']:
-                raise ValidationError(message=e.message['name'][0])
-
 
 class ServiceNameForm(Form):
     def __init__(self, names_func, *args, **kwargs):

app/main/views/add_service.py

@@ -1,4 +1,3 @@
-
 from flask import (
     render_template,
     redirect,
@@ -11,6 +10,7 @@ from flask_login import (
     current_user,
     login_required
 )
+from notifications_python_client.errors import HTTPError
 from werkzeug.exceptions import abort
 
 from app.main import main
@@ -39,14 +39,30 @@ def _add_invited_user_to_service(invited_user):
     return service_id
 
 
-def _create_service(service_name, email_from):
-    service_id = service_api_client.create_service(service_name=service_name,
-                                                   message_limit=current_app.config['DEFAULT_SERVICE_LIMIT'],
-                                                   restricted=True,
-                                                   user_id=session['user_id'],
-                                                   email_from=email_from)
-    session['service_id'] = service_id
-    return service_id
+def _create_service(service_name, email_from, form):
+    try:
+        service_id = service_api_client.create_service(service_name=service_name,
+                                                       message_limit=current_app.config['DEFAULT_SERVICE_LIMIT'],
+                                                       restricted=True,
+                                                       user_id=session['user_id'],
+                                                       email_from=email_from)
+        session['service_id'] = service_id
+        return service_id, None
+    except HTTPError as e:
+        if e.status_code == 400 and e.message['name']:
+            form.name.errors.append("This service name is already in use")
+            return None, e
+
+
+def _create_example_template(service_id):
+    example_sms_template = service_api_client.create_service_template(
+        'Example text message template',
+        'sms',
+        'Hey ((name)), I’m trying out Notify. Today is ((day of week)) and my favourite colour is ((colour)).',
+        service_id,
+        process_type='priority',
+    )
+    return example_sms_template
 
 
 @main.route("/add-service", methods=['GET', 'POST'])
@@ -64,20 +80,16 @@ def add_service():
     heading = 'Which service do you want to set up notifications for?'
 
     if form.validate_on_submit():
-        # email_from = email_safe(form.name.data)
-        # service_name = form.name.data
-        # service_id = _create_service(service_name, email_from)
-        service_id = form.service_id
+        email_from = email_safe(form.name.data)
+        service_name = form.name.data
+
+        service_id, error = _create_service(service_name, email_from, form)
+        if error:
+            return render_template('views/add-service.html', form=form, heading=heading)
         if len(service_api_client.get_active_services({'user_id': session['user_id']}).get('data', [])) > 1:
             return redirect(url_for('main.service_dashboard', service_id=service_id))
 
-        example_sms_template = service_api_client.create_service_template(
-            'Example text message template',
-            'sms',
-            'Hey ((name)), I’m trying out Notify. Today is ((day of week)) and my favourite colour is ((colour)).',
-            service_id,
-            process_type='priority',
-        )
+        example_sms_template = _create_example_template(service_id)
 
         return redirect(url_for(
             'main.start_tour',

tests/app/main/test_add_service_form.py

@@ -1,11 +0,0 @@
-from app.main.forms import AddServiceForm
-from werkzeug.datastructures import MultiDict
-
-
-def test_form_should_have_errors_when_duplicate_service_is_added(client):
-    def _get_form_names():
-        return ['some.service', 'more.names']
-    form = AddServiceForm(_get_form_names,
-                          formdata=MultiDict([('name', 'some service')]))
-    form.validate()
-    assert {'name': ['This service name is already in use']} == form.errors

tests/app/main/views/test_add_service.py

@@ -1,6 +1,5 @@
 from flask import url_for, session
 
-import app
 from app.utils import is_gov_user
 
 
@@ -17,9 +16,7 @@ def test_non_gov_user_cannot_see_add_service_button(
 
 
 def test_get_should_render_add_service_template(
-    logged_in_client,
-    api_user_active,
-    mocker,
+    logged_in_client
 ):
     response = logged_in_client.get(url_for('main.add_service'))
     assert response.status_code == 200
@@ -29,7 +26,6 @@ def test_get_should_render_add_service_template(
 def test_should_add_service_and_redirect_to_tour_when_no_services(
     app_,
     logged_in_client,
-    mocker,
     mock_create_service,
     mock_create_service_template,
     mock_get_services_with_no_services,
@@ -69,7 +65,6 @@ def test_should_add_service_and_redirect_to_tour_when_no_services(
 def test_should_add_service_and_redirect_to_dashboard_when_existing_service(
     app_,
     logged_in_client,
-    mocker,
     mock_create_service,
     mock_create_service_template,
     mock_get_services,
@@ -93,9 +88,7 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service(
 
 
 def test_should_return_form_errors_when_service_name_is_empty(
-    logged_in_client,
-    mocker,
-    api_user_active,
+    logged_in_client
 ):
     response = logged_in_client.post(url_for('main.add_service'), data={})
     assert response.status_code == 200
@@ -104,22 +97,16 @@ def test_should_return_form_errors_when_service_name_is_empty(
 
 def test_should_return_form_errors_with_duplicate_service_name_regardless_of_case(
     logged_in_client,
-    mocker,
-    service_one,
-    api_user_active,
     mock_create_duplicate_service,
 ):
-    response = logged_in_client.post(url_for('main.add_service'), data={'name': 'SERVICE TWO'})
-    print(response.status_code)
-    assert response.status_code == 400
-    assert response.message == "Duplicate service name 'SERVICE_TWO'"
+    response = logged_in_client.post(url_for('main.add_service'), data={'name': 'SERVICE ONE'})
+
+    assert response.status_code == 200
     assert 'This service name is already in use' in response.get_data(as_text=True)
-    assert mock_create_duplicate_service.called
 
 
 def test_non_whitelist_user_cannot_access_create_service_page(
     logged_in_client,
-    mock_login,
     mock_get_non_govuser,
     api_nongov_user_active,
 ):
@@ -130,7 +117,6 @@ def test_non_whitelist_user_cannot_access_create_service_page(
 
 def test_non_whitelist_user_cannot_create_service(
     logged_in_client,
-    mock_login,
     mock_get_non_govuser,
     api_nongov_user_active,
 ):