Let the API return a 400 error message if the service name is a duplicate.
This commit is contained in:
Rebecca Law
2017-08-07 11:30:25 +01:00
parent 8b313ed8d1
commit 186202cc9d
5 changed files with 47 additions and 37 deletions
+22 -10
View File
@@ -3,6 +3,8 @@ import pytz
from flask_wtf import FlaskForm as Form from flask_wtf import FlaskForm as Form
from datetime import datetime, timedelta from datetime import datetime, timedelta
from notifications_python_client.errors import HTTPError
from notifications_utils.recipients import ( from notifications_utils.recipients import (
validate_phone_number, validate_phone_number,
InvalidPhoneError InvalidPhoneError
@@ -211,13 +213,7 @@ class TextNotReceivedForm(Form):
class AddServiceForm(Form): class AddServiceForm(Form):
def __init__(self, names_func, *args, **kwargs): def __init__(self, *args, **kwargs):
"""
Keyword arguments:
names_func -- Returns a list of unique service_names already registered
on the system.
"""
self._names_func = names_func
super(AddServiceForm, self).__init__(*args, **kwargs) super(AddServiceForm, self).__init__(*args, **kwargs)
name = StringField( name = StringField(
@@ -227,11 +223,27 @@ class AddServiceForm(Form):
] ]
) )
service_id = HiddenField('service_id')
def _create_service(self, service_name, email_from):
from app import service_api_client
from flask import current_app, session
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 validate_name(self, a): def validate_name(self, a):
from app.utils import email_safe from app.utils import email_safe
# make sure the email_from will be unique to all services email_from = email_safe(a.data)
if email_safe(a.data) in self._names_func(): try:
raise ValidationError('This service name is already in use') self.service_id = self._create_service(a.data, email_from)
except HTTPError as e:
if e.status_code == 400 and e.message['name']:
raise ValidationError(message=e.message['name'][0])
class ServiceNameForm(Form): class ServiceNameForm(Form):
+7 -7
View File
@@ -1,3 +1,4 @@
from flask import ( from flask import (
render_template, render_template,
redirect, redirect,
@@ -10,7 +11,6 @@ from flask_login import (
current_user, current_user,
login_required login_required
) )
from werkzeug.exceptions import abort from werkzeug.exceptions import abort
from app.main import main from app.main import main
@@ -60,15 +60,15 @@ def add_service():
if not is_gov_user(current_user.email_address): if not is_gov_user(current_user.email_address):
abort(403) 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?' heading = 'Which service do you want to set up notifications for?'
if form.validate_on_submit(): if form.validate_on_submit():
email_from = email_safe(form.name.data) # email_from = email_safe(form.name.data)
service_name = form.name.data # service_name = form.name.data
service_id = _create_service(service_name, email_from) # service_id = _create_service(service_name, email_from)
service_id = form.service_id
if (len(service_api_client.get_active_services({'user_id': session['user_id']}).get('data', [])) > 1): 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)) return redirect(url_for('main.service_dashboard', service_id=service_id))
example_sms_template = service_api_client.create_service_template( example_sms_template = service_api_client.create_service_template(
-12
View File
@@ -97,12 +97,6 @@
} }
} }
}, },
"argparse": {
"version": "1.0.9",
"resolved": "https://registry.npmjs.org/argparse/-/argparse-1.0.9.tgz",
"integrity": "sha1-c9g7wmP4bpf4zE9rrhsOkKfSLIY=",
"dev": true
},
"arr-diff": { "arr-diff": {
"version": "2.0.0", "version": "2.0.0",
"resolved": "https://registry.npmjs.org/arr-diff/-/arr-diff-2.0.0.tgz", "resolved": "https://registry.npmjs.org/arr-diff/-/arr-diff-2.0.0.tgz",
@@ -3766,12 +3760,6 @@
"resolved": "https://registry.npmjs.org/split/-/split-0.2.10.tgz", "resolved": "https://registry.npmjs.org/split/-/split-0.2.10.tgz",
"integrity": "sha1-Zwl8YB1pfOE2j0GPBs0gHPBSGlc=" "integrity": "sha1-Zwl8YB1pfOE2j0GPBs0gHPBSGlc="
}, },
"sprintf-js": {
"version": "1.0.3",
"resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz",
"integrity": "sha1-BOaSb2YolTVPPdAVIDYzuFcpfiw=",
"dev": true
},
"sshpk": { "sshpk": {
"version": "1.13.1", "version": "1.13.1",
"resolved": "https://registry.npmjs.org/sshpk/-/sshpk-1.13.1.tgz", "resolved": "https://registry.npmjs.org/sshpk/-/sshpk-1.13.1.tgz",
+6 -8
View File
@@ -1,5 +1,5 @@
from flask import url_for, session from flask import url_for, session
from unittest.mock import ANY
import app import app
from app.utils import is_gov_user from app.utils import is_gov_user
@@ -107,16 +107,14 @@ def test_should_return_form_errors_with_duplicate_service_name_regardless_of_cas
mocker, mocker,
service_one, service_one,
api_user_active, 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 TWO'})
print(response.status_code)
assert response.status_code == 200 assert response.status_code == 400
assert response.message == "Duplicate service name 'SERVICE_TWO'"
assert 'This service name is already in use' in response.get_data(as_text=True) 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 mock_create_duplicate_service.called
assert not mock_create_service.called
def test_non_whitelist_user_cannot_access_create_service_page( def test_non_whitelist_user_cannot_access_create_service_page(
+12
View File
@@ -173,6 +173,18 @@ def mock_create_service(mocker):
'app.service_api_client.create_service', side_effect=_create) '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') @pytest.fixture(scope='function')
def mock_update_service(mocker): def mock_update_service(mocker):
def _update(service_id, **kwargs): def _update(service_id, **kwargs):