Merge pull request #1409 from alphagov/let-api-check-unique-email

Let api check unique email
This commit is contained in:
Rebecca Law
2017-08-08 15:25:38 +01:00
committed by GitHub
5 changed files with 52 additions and 64 deletions

View File

@@ -3,6 +3,7 @@ import pytz
from flask_wtf import FlaskForm as Form
from datetime import datetime, timedelta
from notifications_utils.recipients import (
validate_phone_number,
InvalidPhoneError
@@ -211,13 +212,7 @@ class TextNotReceivedForm(Form):
class AddServiceForm(Form):
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
def __init__(self, *args, **kwargs):
super(AddServiceForm, self).__init__(*args, **kwargs)
name = StringField(
@@ -227,12 +222,6 @@ class AddServiceForm(Form):
]
)
def validate_name(self, a):
from app.utils import email_safe
# make sure the email_from will be unique to all services
if email_safe(a.data) in self._names_func():
raise ValidationError('This service name is already in use')
class ServiceNameForm(Form):
def __init__(self, names_func, *args, **kwargs):

View File

@@ -10,7 +10,7 @@ from flask_login import (
current_user,
login_required
)
from notifications_python_client.errors import HTTPError
from werkzeug.exceptions import abort
from app.main import main
@@ -39,14 +39,32 @@ def _add_invited_user_to_service(invited_user):
return service_id
def _create_service(service_name, email_from):
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)
session['service_id'] = service_id
return service_id
def _create_service(service_name, 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)
session['service_id'] = service_id
return service_id, None
except HTTPError as e:
if e.status_code == 400 and e.message['name']:
form.name.errors.append("This service name is already in use")
return None, e
else:
raise e
def _create_example_template(service_id):
example_sms_template = service_api_client.create_service_template(
'Example text message template',
'sms',
'Hey ((name)), Im trying out Notify. Today is ((day of week)) and my favourite colour is ((colour)).',
service_id,
process_type='priority',
)
return example_sms_template
@main.route("/add-service", methods=['GET', 'POST'])
@@ -60,24 +78,20 @@ def add_service():
if not is_gov_user(current_user.email_address):
abort(403)
form = AddServiceForm(service_api_client.find_all_service_email_from)
form = AddServiceForm()
heading = 'Which service do you want to set up notifications for?'
if form.validate_on_submit():
email_from = email_safe(form.name.data)
service_name = form.name.data
service_id = _create_service(service_name, email_from)
if (len(service_api_client.get_active_services({'user_id': session['user_id']}).get('data', [])) > 1):
service_id, error = _create_service(service_name, 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:
return redirect(url_for('main.service_dashboard', service_id=service_id))
example_sms_template = service_api_client.create_service_template(
'Example text message template',
'sms',
'Hey ((name)), Im trying out Notify. Today is ((day of week)) and my favourite colour is ((colour)).',
service_id,
process_type='priority',
)
example_sms_template = _create_example_template(service_id)
return redirect(url_for(
'main.start_tour',

View File

@@ -1,11 +0,0 @@
from app.main.forms import AddServiceForm
from werkzeug.datastructures import MultiDict
def test_form_should_have_errors_when_duplicate_service_is_added(client):
def _get_form_names():
return ['some.service', 'more.names']
form = AddServiceForm(_get_form_names,
formdata=MultiDict([('name', 'some service')]))
form.validate()
assert {'name': ['This service name is already in use']} == form.errors

View File

@@ -1,6 +1,5 @@
from flask import url_for, session
from unittest.mock import ANY
import app
from app.utils import is_gov_user
@@ -17,9 +16,7 @@ def test_non_gov_user_cannot_see_add_service_button(
def test_get_should_render_add_service_template(
logged_in_client,
api_user_active,
mocker,
logged_in_client
):
response = logged_in_client.get(url_for('main.add_service'))
assert response.status_code == 200
@@ -29,7 +26,6 @@ def test_get_should_render_add_service_template(
def test_should_add_service_and_redirect_to_tour_when_no_services(
app_,
logged_in_client,
mocker,
mock_create_service,
mock_create_service_template,
mock_get_services_with_no_services,
@@ -69,7 +65,6 @@ def test_should_add_service_and_redirect_to_tour_when_no_services(
def test_should_add_service_and_redirect_to_dashboard_when_existing_service(
app_,
logged_in_client,
mocker,
mock_create_service,
mock_create_service_template,
mock_get_services,
@@ -93,9 +88,7 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service(
def test_should_return_form_errors_when_service_name_is_empty(
logged_in_client,
mocker,
api_user_active,
logged_in_client
):
response = logged_in_client.post(url_for('main.add_service'), data={})
assert response.status_code == 200
@@ -104,24 +97,16 @@ def test_should_return_form_errors_when_service_name_is_empty(
def test_should_return_form_errors_with_duplicate_service_name_regardless_of_case(
logged_in_client,
mocker,
service_one,
api_user_active,
mock_create_service,
mock_create_duplicate_service,
):
mocker.patch('app.service_api_client.find_all_service_email_from',
return_value=['service_one', 'service.two'])
response = logged_in_client.post(url_for('main.add_service'), data={'name': 'SERVICE TWO'})
response = logged_in_client.post(url_for('main.add_service'), data={'name': 'SERVICE ONE'})
assert response.status_code == 200
assert 'This service name is already in use' in response.get_data(as_text=True)
app.service_api_client.find_all_service_email_from.assert_called_once_with()
assert not mock_create_service.called
def test_non_whitelist_user_cannot_access_create_service_page(
logged_in_client,
mock_login,
mock_get_non_govuser,
api_nongov_user_active,
):
@@ -132,7 +117,6 @@ def test_non_whitelist_user_cannot_access_create_service_page(
def test_non_whitelist_user_cannot_create_service(
logged_in_client,
mock_login,
mock_get_non_govuser,
api_nongov_user_active,
):

View File

@@ -173,6 +173,18 @@ def mock_create_service(mocker):
'app.service_api_client.create_service', side_effect=_create)
@pytest.fixture(scope='function')
def mock_create_duplicate_service(mocker):
def _create(service_name, 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")
raise http_error
return mocker.patch(
'app.service_api_client.create_service', side_effect=_create)
@pytest.fixture(scope='function')
def mock_update_service(mocker):
def _update(service_id, **kwargs):