diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 7a9bef890..1961fb981 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -9,7 +9,6 @@ from flask import ( redirect, render_template, request, - session, url_for, ) from flask_login import current_user @@ -28,7 +27,6 @@ from app import ( notification_api_client, organisations_client, service_api_client, - user_api_client, ) from app.event_handlers import ( create_archive_service_event, @@ -42,7 +40,6 @@ from app.main import main from app.main.forms import ( BillingDetailsForm, BrandingOptions, - ConfirmPasswordForm, EditNotesForm, EstimateUsageForm, FreeSMSAllowance, @@ -95,24 +92,28 @@ def service_settings(service_id): @main.route("/services//service-settings/name", methods=['GET', 'POST']) @user_has_permissions('manage_service') def service_name_change(service_id): - form = RenameServiceForm() - - if request.method == 'GET': - form.name.data = current_service.name + form = RenameServiceForm(name=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)) + try: + current_service.update( + name=form.name.data, + email_from=email_safe(form.name.data), + ) + except HTTPError as http_error: + if http_error.status_code == 400 and any( + name_error_message.startswith('Duplicate service name') + for name_error_message in http_error.message['name'] + ): + form.name.errors.append('This service name is already in use') + else: + raise http_error + else: + return redirect(url_for('.service_settings', service_id=service_id)) if current_service.organisation_type == 'local': return render_template( @@ -126,42 +127,6 @@ def service_name_change(service_id): ) -@main.route("/services//service-settings/name/confirm", methods=['GET', 'POST']) -@user_has_permissions('manage_service') -def service_name_change_confirm(service_id): - if 'service_name_change' not in session: - flash("The change you made was not saved. Please try again.", 'error') - return redirect(url_for('main.service_name_change', service_id=service_id)) - - # Validate password for form - def _check_password(pwd): - return user_api_client.verify_password(current_user.id, pwd) - - form = ConfirmPasswordForm(_check_password) - - if form.validate_on_submit(): - try: - current_service.update( - name=session['service_name_change'], - email_from=email_safe(session['service_name_change']) - ) - except HTTPError as e: - error_msg = "Duplicate service name '{}'".format(session['service_name_change']) - if e.status_code == 400 and error_msg in e.message['name']: - # Redirect the user back to the change service name screen - flash('This service name is already in use', 'error') - return redirect(url_for('main.service_name_change', service_id=service_id)) - else: - raise e - else: - session.pop('service_name_change') - 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) - - @main.route("/services//service-settings/request-to-go-live/estimate-usage", methods=['GET', 'POST']) @user_has_permissions('manage_service') def estimate_usage(service_id): diff --git a/app/navigation.py b/app/navigation.py index 15939c553..8fb983ea0 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -239,7 +239,6 @@ class MainNavigation(Navigation): 'service_letter_contact_details', 'service_make_blank_default_letter_contact', 'service_name_change', - 'service_name_change_confirm', 'service_preview_email_branding', 'service_preview_letter_branding', 'service_set_auth_type', diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 6f90ff1eb..589beced8 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -15,7 +15,6 @@ from notifications_utils.clients.zendesk.zendesk_client import ( ) import app -from app.formatters import email_safe from tests import ( find_element_by_tag_and_partial_text, invite_json, @@ -542,26 +541,6 @@ def test_should_show_service_name_with_no_prefixing( ).text == 'Your service name should tell users what the message is about as well as who it’s from.' -def test_should_redirect_after_change_service_name( - client_request, - mock_update_service, - mock_service_name_is_unique, -): - client_request.post( - 'main.service_name_change', - service_id=SERVICE_ONE_ID, - _data={'name': "new name"}, - _expected_status=302, - _expected_redirect=url_for( - 'main.service_name_change_confirm', - service_id=SERVICE_ONE_ID, - _external=True, - ) - ) - - assert mock_service_name_is_unique.called is True - - def test_should_not_hit_api_if_service_name_hasnt_changed( client_request, mock_update_service, @@ -578,7 +557,6 @@ def test_should_not_hit_api_if_service_name_hasnt_changed( _external=True, ), ) - assert not mock_service_name_is_unique.called assert not mock_update_service.called @@ -590,7 +568,6 @@ def test_should_not_hit_api_if_service_name_hasnt_changed( def test_service_name_change_fails_if_new_name_fails_validation( client_request, mock_update_service, - mock_service_name_is_unique, name, error_message, ): @@ -600,7 +577,6 @@ def test_service_name_change_fails_if_new_name_fails_validation( _data={'name': name}, _expected_status=200, ) - assert not mock_service_name_is_unique.called assert not mock_update_service.called assert error_message in page.find("span", {"class": "govuk-error-message"}).text @@ -796,9 +772,9 @@ def test_switch_service_to_count_as_live( ) -def test_should_not_allow_duplicate_names( +def test_should_not_allow_duplicate_service_names( client_request, - mock_service_name_is_not_unique, + mock_update_service_raise_httperror_duplicate_name, service_one, ): page = client_request.post( @@ -809,39 +785,18 @@ def test_should_not_allow_duplicate_names( ) assert 'This service name is already in use' in page.text - app.service_api_client.is_service_name_unique.assert_called_once_with( - SERVICE_ONE_ID, - 'SErvICE TWO', - 'service.two', - ) - - -def test_should_show_service_name_confirmation( - client_request, -): - service_new_name = 'New Name' - with client_request.session_transaction() as session: - session['service_name_change'] = service_new_name - page = client_request.get( - 'main.service_name_change_confirm', - service_id=SERVICE_ONE_ID, - ) - assert 'Change your service name' in page.text - app.service_api_client.get_service.assert_called_with(SERVICE_ONE_ID) def test_should_redirect_after_service_name_confirmation( client_request, mock_update_service, - mock_verify_password, - mock_get_inbound_number_for_service, ): - service_new_name = 'New Name' - with client_request.session_transaction() as session: - session['service_name_change'] = service_new_name client_request.post( - 'main.service_name_change_confirm', + 'main.service_name_change', service_id=SERVICE_ONE_ID, + _data={ + 'name': 'New Name' + }, _expected_status=302, _expected_redirect=url_for( 'main.service_settings', @@ -852,47 +807,9 @@ def test_should_redirect_after_service_name_confirmation( mock_update_service.assert_called_once_with( SERVICE_ONE_ID, - name=service_new_name, - email_from=email_safe(service_new_name) + name='New Name', + email_from='new.name', ) - assert mock_verify_password.called is True - - -def test_should_raise_duplicate_name_handled( - client_request, - mock_update_service_raise_httperror_duplicate_name, - mock_verify_password, -): - with client_request.session_transaction() as session: - session['service_name_change'] = 'New Name' - - client_request.post( - 'main.service_name_change_confirm', - service_id=SERVICE_ONE_ID, - _expected_status=302, - _expected_redirect=url_for( - 'main.service_name_change', - service_id=SERVICE_ONE_ID, - _external=True, - ), - ) - - assert mock_update_service_raise_httperror_duplicate_name.called - assert mock_verify_password.called - - -def test_service_name_change_confirm_handles_expired_session( - client_request, mock_verify_password, mock_update_service -): - page = client_request.post( - 'main.service_name_change_confirm', - service_id=SERVICE_ONE_ID, - _follow_redirects=True - ) - assert mock_verify_password.called is False - assert mock_update_service.called is False - - assert page.find('div', 'banner-dangerous').text.strip() == "The change you made was not saved. Please try again." @pytest.mark.parametrize('volumes, consent_to_research, expected_estimated_volumes_item', [ @@ -2092,7 +2009,6 @@ def test_ready_to_go_live( @pytest.mark.parametrize('route', [ 'main.service_settings', 'main.service_name_change', - 'main.service_name_change_confirm', 'main.request_to_go_live', 'main.submit_request_to_go_live', 'main.archive_service' @@ -2127,7 +2043,6 @@ def test_route_permissions( @pytest.mark.parametrize('route', [ 'main.service_settings', 'main.service_name_change', - 'main.service_name_change_confirm', 'main.request_to_go_live', 'main.submit_request_to_go_live', 'main.service_switch_live', @@ -2157,7 +2072,6 @@ def test_route_invalid_permissions( @pytest.mark.parametrize('route', [ 'main.service_settings', 'main.service_name_change', - 'main.service_name_change_confirm', 'main.request_to_go_live', 'main.submit_request_to_go_live', ]) diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index fad19a5db..9a3262c2f 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -244,7 +244,6 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, { 'service_letter_contact_details', 'service_make_blank_default_letter_contact', 'service_name_change', - 'service_name_change_confirm', 'service_preview_email_branding', 'service_preview_letter_branding', 'service_set_auth_type',