diff --git a/app/main/forms.py b/app/main/forms.py index e04b8d148..803698290 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -213,7 +213,7 @@ class TextNotReceivedForm(Form): mobile_number = international_phone_number() -class ServiceNameForm(Form): +class RenameServiceForm(Form): name = StringField( u'Service name', validators=[ @@ -221,6 +221,23 @@ class ServiceNameForm(Form): ]) +class CreateServiceForm(Form): + name = StringField( + u'What’s your service called?', + validators=[ + DataRequired(message='Can’t be empty') + ]) + organisation_type = RadioField( + 'Who runs this service?', + choices=[ + ('central', 'Central government'), + ('local', 'Local government'), + ('nhs', 'NHS'), + ], + validators=[DataRequired()], + ) + + class ConfirmPasswordForm(Form): def __init__(self, validate_password_func, *args, **kwargs): self.validate_password_func = validate_password_func diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 9b7d5b9fd..8019199e6 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -14,7 +14,7 @@ from notifications_python_client.errors import HTTPError from werkzeug.exceptions import abort from app.main import main -from app.main.forms import ServiceNameForm +from app.main.forms import CreateServiceForm from app.notify_client.models import InvitedUser from app import ( @@ -39,13 +39,16 @@ def _add_invited_user_to_service(invited_user): return service_id -def _create_service(service_name, email_from, form): +def _create_service(service_name, organisation_type, 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) + service_id = service_api_client.create_service( + service_name=service_name, + organisation_type=organisation_type, + 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: @@ -78,14 +81,14 @@ def add_service(): if not is_gov_user(current_user.email_address): abort(403) - form = ServiceNameForm() - heading = 'Which service do you want to set up notifications for?' + form = CreateServiceForm() + heading = 'About your service' if form.validate_on_submit(): email_from = email_safe(form.name.data) service_name = form.name.data - service_id, error = _create_service(service_name, email_from, form) + service_id, error = _create_service(service_name, form.organisation_type.data, 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: diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 7356f5c53..6278b5818 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -25,7 +25,7 @@ from app.main import main from app.utils import user_has_permissions, email_safe, get_cdn_domain from app.main.forms import ( ConfirmPasswordForm, - ServiceNameForm, + RenameServiceForm, RequestToGoLiveForm, ServiceReplyToEmailForm, ServiceSmsSender, @@ -100,7 +100,7 @@ def service_settings(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_name_change(service_id): - form = ServiceNameForm() + form = RenameServiceForm() if request.method == 'GET': form.name.data = current_service.get('name') diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index d8c034eaa..aa92b0e1b 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -16,12 +16,21 @@ class ServiceAPIClient(NotifyAdminAPIClient): self.service_id = application.config['ADMIN_CLIENT_USER_NAME'] self.api_key = application.config['ADMIN_CLIENT_SECRET'] - def create_service(self, service_name, message_limit, restricted, user_id, email_from): + def create_service( + self, + service_name, + organisation_type, + message_limit, + restricted, + user_id, + email_from, + ): """ Create a service and return the json. """ data = { "name": service_name, + "organisation_type": organisation_type, "active": True, "message_limit": message_limit, "user_id": user_id, diff --git a/app/templates/views/add-service.html b/app/templates/views/add-service.html index 75dcc3cae..a7c88d2d2 100644 --- a/app/templates/views/add-service.html +++ b/app/templates/views/add-service.html @@ -1,4 +1,5 @@ {% extends "withoutnav_template.html" %} +{% from "components/radios.html" import radios %} {% from "components/textbox.html" import textbox %} {% from "components/page-footer.html" import page_footer %} @@ -12,13 +13,15 @@

- When people receive notifications, who should they be from? + About your service

{{ textbox(form.name, hint="You can change this later") }} + {{ radios(form.organisation_type) }} + {{ page_footer('Add service') }}
diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 703e8ab7a..b40b7a85d 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -20,7 +20,7 @@ def test_get_should_render_add_service_template( ): response = logged_in_client.get(url_for('main.add_service')) assert response.status_code == 200 - assert 'Which service do you want to set up notifications for?' in response.get_data(as_text=True) + assert 'About your service' in response.get_data(as_text=True) def test_should_add_service_and_redirect_to_tour_when_no_services( @@ -33,10 +33,15 @@ def test_should_add_service_and_redirect_to_tour_when_no_services( ): response = logged_in_client.post( url_for('main.add_service'), - data={'name': 'testing the post'}) + data={ + 'name': 'testing the post', + 'organisation_type': 'local', + } + ) assert mock_get_services_with_no_services.called mock_create_service.assert_called_once_with( service_name='testing the post', + organisation_type='local', message_limit=app_.config['DEFAULT_SERVICE_LIMIT'], restricted=True, user_id=api_user_active.id, @@ -72,10 +77,15 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service( ): response = logged_in_client.post( url_for('main.add_service'), - data={'name': 'testing the post'}) + data={ + 'name': 'testing the post', + 'organisation_type': 'central', + } + ) assert mock_get_services.called mock_create_service.assert_called_once_with( service_name='testing the post', + organisation_type='central', message_limit=app_.config['DEFAULT_SERVICE_LIMIT'], restricted=True, user_id=api_user_active.id, @@ -99,7 +109,13 @@ def test_should_return_form_errors_with_duplicate_service_name_regardless_of_cas logged_in_client, mock_create_duplicate_service, ): - response = logged_in_client.post(url_for('main.add_service'), data={'name': 'SERVICE ONE'}) + response = logged_in_client.post( + url_for('main.add_service'), + data={ + 'name': 'SERVICE ONE', + 'organisation_type': 'central', + }, + ) assert response.status_code == 200 assert 'This service name is already in use' in response.get_data(as_text=True) diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 9fa6cd4d9..710d2260d 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -43,3 +43,38 @@ def test_client_only_updates_allowed_attributes(mocker): with pytest.raises(TypeError) as error: ServiceAPIClient().update_service('service_id', foo='bar') assert str(error.value) == 'Not allowed to update service attributes: foo' + + +def test_client_creates_service_with_correct_data( + mocker, + active_user_with_permissions, + fake_uuid, +): + client = ServiceAPIClient() + mock_post = mocker.patch.object(client, 'post') + mocker.patch('app.notify_client.current_user', id='123') + + client.create_service( + service_name='My first service', + organisation_type='central_government', + message_limit=1, + restricted=True, + user_id=fake_uuid, + email_from='test@example.com', + ) + mock_post.assert_called_once_with( + '/service', + dict( + # Autogenerated arguments + created_by='123', + active=True, + # ‘service_name’ argument is coerced to ‘name’ + name='My first service', + # The rest pass through with the same names + organisation_type='central_government', + message_limit=1, + restricted=True, + user_id=fake_uuid, + email_from='test@example.com', + ), + ) diff --git a/tests/conftest.py b/tests/conftest.py index c58dad210..2051247fd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -488,7 +488,7 @@ def mock_get_service_with_letters(mocker, api_user_active): @pytest.fixture(scope='function') def mock_create_service(mocker): - def _create(service_name, message_limit, restricted, user_id, email_from): + def _create(service_name, organisation_type, message_limit, restricted, user_id, email_from): service = service_json( 101, service_name, [user_id], message_limit=message_limit, restricted=restricted, email_from=email_from) return service['id'] @@ -499,7 +499,7 @@ def mock_create_service(mocker): @pytest.fixture(scope='function') def mock_create_duplicate_service(mocker): - def _create(service_name, message_limit, restricted, user_id, email_from): + def _create(service_name, organisation_type, message_limit, restricted, user_id, email_from): json_mock = Mock(return_value={'message': {'name': ["Duplicate service name '{}'".format(service_name)]}}) resp_mock = Mock(status_code=400, json=json_mock) http_error = HTTPError(response=resp_mock, message="Default message")