From d86291cb7a72be32c1f43784a0b54fd825b2fed6 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 2 Jul 2018 11:00:02 +0100 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20error=20if=20a=20user=20keeps?= =?UTF-8?q?=20same=20service=20name?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sometimes a user will hit the green button without actually changing the service name (we saw this in a usability testing session). Currently this gives an error saying ‘service name already in use’, which is technically true (it’s in use by the current service) but practically just annoying. This commit adds some logic to silently ignore such requests. --- app/main/views/service_settings.py | 11 ++++++++-- tests/app/main/views/test_service_settings.py | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index d4a5855d1..5030e9ab2 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -113,19 +113,26 @@ def service_name_change(service_id): form = RenameServiceForm() if request.method == 'GET': - form.name.data = current_service.get('name') + form.name.data = current_service['name'] if form.validate_on_submit(): + + if form.name.data == current_service['name']: + return redirect(url_for('.service_settings', service_id=service_id)) + unique_name = service_api_client.is_service_name_unique(service_id, form.name.data, email_safe(form.name.data)) + if not unique_name: form.name.errors.append("This service name is already in use") return render_template('views/service-settings/name.html', form=form) + 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', - form=form) + form=form, + ) @main.route("/services//service-settings/name/confirm", methods=['GET', 'POST']) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 0609f625a..a9cf5e1d7 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -291,6 +291,26 @@ def test_should_redirect_after_change_service_name( assert mock_service_name_is_unique.called +def test_should_not_hit_api_if_service_name_hasnt_changed( + client_request, + mock_update_service, + mock_service_name_is_unique, +): + client_request.post( + 'main.service_name_change', + service_id=SERVICE_ONE_ID, + _data={'name': 'service one'}, + _expected_status=302, + _expected_redirect=url_for( + 'main.service_settings', + service_id=SERVICE_ONE_ID, + _external=True, + ), + ) + assert not mock_service_name_is_unique.called + assert not mock_update_service.called + + @pytest.mark.parametrize('user, expected_text, expected_link', [ ( active_user_with_permissions,