diff --git a/app/main/forms.py b/app/main/forms.py index fafe580da..5f6560716 100644 --- a/app/main/forms.py +++ b/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): diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index fde726af0..49886ed4e 100644 --- a/app/main/views/add_service.py +++ b/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', diff --git a/tests/app/main/test_add_service_form.py b/tests/app/main/test_add_service_form.py deleted file mode 100644 index e8f5a5f74..000000000 --- a/tests/app/main/test_add_service_form.py +++ /dev/null @@ -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 diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index ca5fcaa10..703e8ab7a 100644 --- a/tests/app/main/views/test_add_service.py +++ b/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, ):