mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-02-05 02:42:26 -05:00
Service name uniqueness handled in all cases and tests passing.
This commit is contained in:
@@ -205,7 +205,24 @@ class AddServiceForm(Form):
|
||||
|
||||
|
||||
class ServiceNameForm(Form):
|
||||
name = StringField(u'New name')
|
||||
def __init__(self, names_func, *args, **kwargs):
|
||||
"""
|
||||
Keyword arguments:
|
||||
names_func -- Returns a list of unique service_names already registered
|
||||
on the system.
|
||||
"""
|
||||
self._names_func = names_func
|
||||
super(ServiceNameForm, self).__init__(*args, **kwargs)
|
||||
|
||||
name = StringField(
|
||||
u'New name',
|
||||
validators=[
|
||||
DataRequired(message='Service name can’t be empty')
|
||||
])
|
||||
|
||||
def validate_name(self, a):
|
||||
if a.data in self._names_func():
|
||||
raise ValidationError('This service name is already in use')
|
||||
|
||||
|
||||
class ConfirmPasswordForm(Form):
|
||||
|
||||
@@ -18,7 +18,8 @@ from notifications_python_client.errors import HTTPError
|
||||
from app.main.dao.services_dao import (
|
||||
get_service_by_id,
|
||||
delete_service,
|
||||
update_service
|
||||
update_service,
|
||||
find_all_service_names
|
||||
)
|
||||
|
||||
from app.main import main
|
||||
@@ -57,7 +58,7 @@ def service_name_change(service_id):
|
||||
else:
|
||||
raise e
|
||||
|
||||
form = ServiceNameForm()
|
||||
form = ServiceNameForm(find_all_service_names)
|
||||
|
||||
if form.validate_on_submit():
|
||||
session['service_name_change'] = form.name.data
|
||||
@@ -89,10 +90,20 @@ def service_name_change_confirm(service_id):
|
||||
|
||||
if form.validate_on_submit():
|
||||
service['name'] = session['service_name_change']
|
||||
update_service(service)
|
||||
session['service_name'] = service['name']
|
||||
session.pop('service_name_change')
|
||||
return redirect(url_for('.service_settings', service_id=service_id))
|
||||
try:
|
||||
update_service(service)
|
||||
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['service_name'] = service['name']
|
||||
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',
|
||||
|
||||
@@ -54,8 +54,9 @@ def test_should_redirect_after_change_service_name(app_,
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
response = client.post(url_for(
|
||||
'main.service_name_change', service_id=service_id))
|
||||
response = client.post(
|
||||
url_for('main.service_name_change', service_id=service_id),
|
||||
data={'name': "new name"})
|
||||
|
||||
assert response.status_code == 302
|
||||
settings_url = url_for(
|
||||
@@ -64,6 +65,27 @@ def test_should_redirect_after_change_service_name(app_,
|
||||
assert mock_get_service.called
|
||||
|
||||
|
||||
def test_should_not_allow_duplicate_names(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
mock_get_user,
|
||||
mock_get_user_by_email,
|
||||
mock_login,
|
||||
mock_has_permissions,
|
||||
mock_get_services):
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
response = client.post(
|
||||
url_for('main.service_name_change', service_id=service_id),
|
||||
data={'name': "service_one"})
|
||||
|
||||
assert response.status_code == 200
|
||||
resp_data = response.get_data(as_text=True)
|
||||
assert 'This service name is already in use' in resp_data
|
||||
|
||||
|
||||
def test_should_show_service_name_confirmation(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
@@ -112,6 +134,32 @@ def test_should_redirect_after_service_name_confirmation(app_,
|
||||
assert mock_update_service.called
|
||||
|
||||
|
||||
def test_should_raise_duplicate_name_handled(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
mock_update_service_raise_httperror_duplicate_name,
|
||||
mock_get_user,
|
||||
mock_get_user_by_email,
|
||||
mock_login,
|
||||
mock_verify_password,
|
||||
mock_has_permissions):
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
service_new_name = 'New Name'
|
||||
with client.session_transaction() as session:
|
||||
session['service_name_change'] = service_new_name
|
||||
response = client.post(url_for(
|
||||
'main.service_name_change_confirm', service_id=service_id))
|
||||
|
||||
assert response.status_code == 302
|
||||
name_change_url = url_for(
|
||||
'main.service_name_change', service_id=service_id, _external=True)
|
||||
resp_data = response.get_data(as_text=True)
|
||||
assert name_change_url == response.location
|
||||
|
||||
|
||||
def test_should_show_request_to_go_live(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import uuid
|
||||
from datetime import date, datetime, timedelta
|
||||
from unittest.mock import Mock
|
||||
import pytest
|
||||
|
||||
from app import create_app
|
||||
@@ -18,6 +19,8 @@ from app.notify_client.models import (
|
||||
InvitedUser
|
||||
)
|
||||
|
||||
from notifications_python_client.errors import HTTPError
|
||||
|
||||
|
||||
@pytest.fixture(scope='session')
|
||||
def app_(request):
|
||||
@@ -90,6 +93,24 @@ def mock_update_service(mocker):
|
||||
'app.notifications_api_client.update_service', side_effect=_update)
|
||||
|
||||
|
||||
@pytest.fixture(scope='function')
|
||||
def mock_update_service_raise_httperror_duplicate_name(mocker):
|
||||
|
||||
def _update(service_id,
|
||||
service_name,
|
||||
active,
|
||||
limit,
|
||||
restricted,
|
||||
users):
|
||||
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")
|
||||
raise http_error
|
||||
|
||||
return mocker.patch(
|
||||
'app.notifications_api_client.update_service', side_effect=_update)
|
||||
|
||||
|
||||
SERVICE_ONE_ID = "596364a0-858e-42c8-9062-a8fe822260eb"
|
||||
SERVICE_TWO_ID = "147ad62a-2951-4fa1-9ca0-093cd1a52c52"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user