mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-05-28 18:10:11 -04:00
Update email_from when the service name is changed.
Update unit tests Service name is uniqueness is not based on case.
This commit is contained in:
@@ -180,7 +180,7 @@ class ServiceNameForm(Form):
|
||||
])
|
||||
|
||||
def validate_name(self, a):
|
||||
if a.data in self._names_func():
|
||||
if a.data.lower() in self._names_func():
|
||||
raise ValidationError('This service name is already in use')
|
||||
|
||||
|
||||
|
||||
@@ -15,7 +15,7 @@ from notifications_python_client import HTTPError
|
||||
|
||||
from app import service_api_client
|
||||
from app.main import main
|
||||
from app.utils import user_has_permissions
|
||||
from app.utils import user_has_permissions, email_safe
|
||||
from app.main.forms import ConfirmPasswordForm, ServiceNameForm
|
||||
from app import user_api_client
|
||||
|
||||
@@ -65,6 +65,7 @@ def service_name_change_confirm(service_id):
|
||||
|
||||
if form.validate_on_submit():
|
||||
service['name'] = session['service_name_change']
|
||||
service['email_from'] = email_safe(session['service_name_change'])
|
||||
try:
|
||||
service_api_client.update_service(
|
||||
service['id'],
|
||||
@@ -72,7 +73,8 @@ def service_name_change_confirm(service_id):
|
||||
service['active'],
|
||||
service['limit'],
|
||||
service['restricted'],
|
||||
service['users'])
|
||||
service['users'],
|
||||
service['email_from'])
|
||||
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']:
|
||||
@@ -144,7 +146,8 @@ def service_status_change_confirm(service_id):
|
||||
service['active'],
|
||||
service['limit'],
|
||||
service['restricted'],
|
||||
service['users'])
|
||||
service['users'],
|
||||
service['email_from'])
|
||||
return redirect(url_for('.service_settings', service_id=service_id))
|
||||
return render_template(
|
||||
'views/service-settings/confirm.html',
|
||||
|
||||
@@ -57,7 +57,8 @@ class ServiceAPIClient(NotificationsAPIClient):
|
||||
active,
|
||||
limit,
|
||||
restricted,
|
||||
users):
|
||||
users,
|
||||
email_from):
|
||||
"""
|
||||
Update a service.
|
||||
"""
|
||||
@@ -67,7 +68,8 @@ class ServiceAPIClient(NotificationsAPIClient):
|
||||
"active": active,
|
||||
"limit": limit,
|
||||
"restricted": restricted,
|
||||
"users": users
|
||||
"users": users,
|
||||
"email_from": email_from
|
||||
}
|
||||
endpoint = "/service/{0}".format(service_id)
|
||||
return self.post(endpoint, data)
|
||||
@@ -142,7 +144,7 @@ class ServiceAPIClient(NotificationsAPIClient):
|
||||
|
||||
def find_all_service_names(self, user_id=None):
|
||||
resp = self.get_services(user_id)
|
||||
return [x['name'] for x in resp['data']]
|
||||
return [x['name'].lower() for x in resp['data']]
|
||||
|
||||
|
||||
class ServicesBrowsableItem(BrowsableItem):
|
||||
|
||||
@@ -101,3 +101,9 @@ def generate_previous_next_dict(view, view_dict, page, title, label):
|
||||
'title': title,
|
||||
'label': label
|
||||
}
|
||||
|
||||
|
||||
def email_safe(string):
|
||||
return "".join([
|
||||
character.lower() if character.isalnum() or character == "." else "" for character in re.sub("\s+", ".", string.strip()) # noqa
|
||||
])
|
||||
|
||||
@@ -22,14 +22,15 @@ class TestClient(FlaskClient):
|
||||
self.get(url_for("main.logout"))
|
||||
|
||||
|
||||
def service_json(id_, name, users, limit=1000, active=False, restricted=True):
|
||||
def service_json(id_, name, users, limit=1000, active=False, restricted=True, email_from=None):
|
||||
return {
|
||||
'id': id_,
|
||||
'name': name,
|
||||
'users': users,
|
||||
'limit': limit,
|
||||
'active': active,
|
||||
'restricted': restricted
|
||||
'restricted': restricted,
|
||||
'email_from': email_from
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -1,82 +1,69 @@
|
||||
from flask import url_for
|
||||
|
||||
import app
|
||||
from app.utils import email_safe
|
||||
from tests import validate_route_permission
|
||||
from bs4 import BeautifulSoup
|
||||
|
||||
|
||||
def test_should_show_overview(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
mock_get_user,
|
||||
mock_get_user_by_email,
|
||||
mock_login,
|
||||
mock_has_permissions):
|
||||
active_user_with_permissions,
|
||||
mocker,
|
||||
service_one):
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
client.login(active_user_with_permissions, mocker, service_one)
|
||||
response = client.get(url_for(
|
||||
'main.service_settings', service_id=service_id))
|
||||
'main.service_settings', service_id=service_one['id']))
|
||||
assert response.status_code == 200
|
||||
resp_data = response.get_data(as_text=True)
|
||||
assert 'Service settings' in resp_data
|
||||
service = mock_get_service.side_effect(service_id)['data']
|
||||
assert mock_get_service.called
|
||||
app.service_api_client.get_service.assert_called_with(service_one['id'])
|
||||
|
||||
|
||||
def test_should_show_service_name(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
mock_get_user,
|
||||
mock_get_user_by_email,
|
||||
mock_login,
|
||||
mock_has_permissions):
|
||||
active_user_with_permissions,
|
||||
mocker,
|
||||
service_one):
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
client.login(active_user_with_permissions, mocker, service_one)
|
||||
response = client.get(url_for(
|
||||
'main.service_name_change', service_id=service_id))
|
||||
'main.service_name_change', service_id=service_one['id']))
|
||||
assert response.status_code == 200
|
||||
resp_data = response.get_data(as_text=True)
|
||||
assert 'Change your service name' in resp_data
|
||||
assert mock_get_service.called
|
||||
service = mock_get_service.side_effect(service_id)['data']
|
||||
app.service_api_client.get_service.assert_called_with(service_one['id'])
|
||||
|
||||
|
||||
def test_should_redirect_after_change_service_name(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
mock_get_user,
|
||||
mock_get_user_by_email,
|
||||
mock_login,
|
||||
mock_has_permissions):
|
||||
active_user_with_permissions,
|
||||
service_one,
|
||||
mocker,
|
||||
mock_get_services):
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
client.login(active_user_with_permissions, mocker, service_one)
|
||||
response = client.post(
|
||||
url_for('main.service_name_change', service_id=service_id),
|
||||
url_for('main.service_name_change', service_id=service_one['id']),
|
||||
data={'name': "new name"})
|
||||
|
||||
assert response.status_code == 302
|
||||
settings_url = url_for(
|
||||
'main.service_name_change_confirm', service_id=service_id, _external=True)
|
||||
'main.service_name_change_confirm', service_id=service_one['id'], _external=True)
|
||||
assert settings_url == response.location
|
||||
assert mock_get_service.called
|
||||
assert mock_get_services.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,
|
||||
active_user_with_permissions,
|
||||
mocker,
|
||||
service_one,
|
||||
mock_get_services):
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
client.login(active_user_with_permissions, mocker, service_one)
|
||||
service_id = service_one['id']
|
||||
response = client.post(
|
||||
url_for('main.service_name_change', service_id=service_id),
|
||||
data={'name': "service_one"})
|
||||
@@ -87,38 +74,32 @@ def test_should_not_allow_duplicate_names(app_,
|
||||
|
||||
|
||||
def test_should_show_service_name_confirmation(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
mock_get_user,
|
||||
mock_get_user_by_email,
|
||||
mock_login,
|
||||
mock_has_permissions):
|
||||
active_user_with_permissions,
|
||||
mocker,
|
||||
service_one):
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
client.login(active_user_with_permissions, mocker, service_one)
|
||||
|
||||
response = client.get(url_for(
|
||||
'main.service_name_change_confirm', service_id=service_id))
|
||||
'main.service_name_change_confirm', service_id=service_one['id']))
|
||||
|
||||
assert response.status_code == 200
|
||||
resp_data = response.get_data(as_text=True)
|
||||
assert 'Change your service name' in resp_data
|
||||
assert mock_get_service.called
|
||||
app.service_api_client.get_service.assert_called_with(service_one['id'])
|
||||
|
||||
|
||||
def test_should_redirect_after_service_name_confirmation(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
active_user_with_permissions,
|
||||
service_one,
|
||||
mocker,
|
||||
mock_update_service,
|
||||
mock_get_user,
|
||||
mock_get_user_by_email,
|
||||
mock_login,
|
||||
mock_verify_password,
|
||||
mock_has_permissions):
|
||||
mock_verify_password):
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
client.login(active_user_with_permissions, mocker, service_one)
|
||||
service_id = service_one['id']
|
||||
service_new_name = 'New Name'
|
||||
with client.session_transaction() as session:
|
||||
session['service_name_change'] = service_new_name
|
||||
@@ -126,38 +107,40 @@ def test_should_redirect_after_service_name_confirmation(app_,
|
||||
'main.service_name_change_confirm', service_id=service_id))
|
||||
|
||||
assert response.status_code == 302
|
||||
settings_url = url_for(
|
||||
'main.service_settings', service_id=service_id, _external=True)
|
||||
resp_data = response.get_data(as_text=True)
|
||||
settings_url = url_for('main.service_settings', service_id=service_id, _external=True)
|
||||
assert settings_url == response.location
|
||||
assert mock_get_service.called
|
||||
assert mock_update_service.called
|
||||
mock_update_service.assert_called_once_with(service_id,
|
||||
service_new_name,
|
||||
service_one['active'],
|
||||
service_one['limit'],
|
||||
service_one['restricted'],
|
||||
service_one['users'],
|
||||
email_safe(service_new_name))
|
||||
assert mock_verify_password.called
|
||||
|
||||
|
||||
def test_should_raise_duplicate_name_handled(app_,
|
||||
api_user_active,
|
||||
mock_get_service,
|
||||
active_user_with_permissions,
|
||||
service_one,
|
||||
mocker,
|
||||
mock_get_services,
|
||||
mock_update_service_raise_httperror_duplicate_name,
|
||||
mock_get_user,
|
||||
mock_get_user_by_email,
|
||||
mock_login,
|
||||
mock_verify_password,
|
||||
mock_has_permissions):
|
||||
mock_verify_password):
|
||||
with app_.test_request_context():
|
||||
with app_.test_client() as client:
|
||||
client.login(api_user_active)
|
||||
service_id = 123
|
||||
client.login(active_user_with_permissions, mocker, service_one)
|
||||
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))
|
||||
'main.service_name_change_confirm', service_id=service_one['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)
|
||||
'main.service_name_change', service_id=service_one['id'], _external=True)
|
||||
assert name_change_url == response.location
|
||||
assert mock_update_service_raise_httperror_duplicate_name.called
|
||||
assert mock_verify_password.called
|
||||
|
||||
|
||||
def test_should_show_request_to_go_live(app_,
|
||||
|
||||
@@ -77,14 +77,15 @@ def mock_update_service(mocker):
|
||||
active,
|
||||
limit,
|
||||
restricted,
|
||||
users):
|
||||
users,
|
||||
email_from):
|
||||
service = service_json(
|
||||
service_id, service_name, users, limit=limit,
|
||||
active=active, restricted=restricted)
|
||||
active=active, restricted=restricted, email_from=email_from)
|
||||
return {'data': service}
|
||||
|
||||
return mocker.patch(
|
||||
'app.service_api_client.update_service', side_effect=_update)
|
||||
'app.service_api_client.update_service', side_effect=_update, autospec=True)
|
||||
|
||||
|
||||
@pytest.fixture(scope='function')
|
||||
@@ -94,7 +95,8 @@ def mock_update_service_raise_httperror_duplicate_name(mocker):
|
||||
active,
|
||||
limit,
|
||||
restricted,
|
||||
users):
|
||||
users,
|
||||
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")
|
||||
|
||||
Reference in New Issue
Block a user