diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index 9881d6689..da4104755 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -1,5 +1,7 @@ from datetime import datetime +from sqlalchemy.orm import load_only + from app import db from app.models import Service @@ -39,3 +41,7 @@ def add_service(service): def find_service_by_service_name(service_name): return Service.query.filter_by(name=service_name).first() + + +def find_all_service_names(): + return [x.name for x in Service.query.options(load_only("name")).all()] diff --git a/app/main/forms.py b/app/main/forms.py index f73ac0ef2..28238e74e 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2,10 +2,9 @@ from datetime import datetime from flask import session from flask_wtf import Form -from wtforms import StringField, PasswordField +from wtforms import StringField, PasswordField, ValidationError from wtforms.validators import DataRequired, Email, Length, Regexp - -from app.main.dao import verify_codes_dao, services_dao +from app.main.dao import verify_codes_dao from app.main.encryption import check_hash from app.main.validators import Blacklist @@ -83,14 +82,15 @@ class TextNotReceivedForm(Form): class AddServiceForm(Form): + def __init__(self, service_names, *args, **kwargs): + self.service_names = service_names + super(AddServiceForm, self).__init__(*args, **kwargs) + service_name = StringField(validators=[DataRequired(message='Please enter your service name')]) def validate_service_name(self, a): - if services_dao.find_service_by_service_name(self.service_name.data) is not None: - self.service_name.errors.append('Duplicate service name') - return False - else: - return True + if self.service_name.data in self.service_names: + raise ValidationError('Service name already exists') def validate_codes(field, code_type): diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 4d03be4f8..0669b1742 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -1,25 +1,18 @@ from flask import render_template, jsonify, redirect, session from flask_login import login_required - from app.main import main from app.main.dao import services_dao, users_dao from app.main.forms import AddServiceForm -@main.route("/add-service", methods=['GET']) +@main.route("/add-service", methods=['GET', 'POST']) @login_required def add_service(): - return render_template('views/add-service.html', form=AddServiceForm()) - - -@main.route("/add-service", methods=['POST']) -@login_required -def process_add_service(): - form = AddServiceForm() - + form = AddServiceForm(services_dao.find_all_service_names()) if form.validate_on_submit(): + user = users_dao.get_user_by_id(session['user_id']) services_dao.insert_new_service(form.service_name.data, user) return redirect('/dashboard') else: - return jsonify(form.errors), 400 + return render_template('views/add-service.html', form=form) diff --git a/app/templates/components/form-field.html b/app/templates/components/form-field.html new file mode 100644 index 000000000..643099498 --- /dev/null +++ b/app/templates/components/form-field.html @@ -0,0 +1,12 @@ +{% macro render_field(field) %} +
{{ field.label }} +
{{ field(**kwargs)|safe }} + {% if field.errors %} + + {% endif %} +
+{% endmacro %} diff --git a/app/templates/govuk_template.html b/app/templates/govuk_template.html index dc5747325..8057ff1d8 100644 --- a/app/templates/govuk_template.html +++ b/app/templates/govuk_template.html @@ -1,3 +1,4 @@ +{%- from "components/form-field.html" import render_field %} {% block top_of_page %}{% endblock %} diff --git a/app/templates/views/add-service.html b/app/templates/views/add-service.html index 3b50f3371..4c6577c10 100644 --- a/app/templates/views/add-service.html +++ b/app/templates/views/add-service.html @@ -18,9 +18,7 @@ GOV.UK Notify | Set up service
{{ form.hidden_tag() }} - - {{ form.service_name(class="form-control-2-3", autocomplete="off") }}
- For example, 'Vehicle tax' or 'Carer's allowance' + {{ render_field(form.service_name, class='form-control-2-3') }}

diff --git a/tests/app/main/dao/test_service_dao.py b/tests/app/main/dao/test_service_dao.py index 30762b6ba..e023104b8 100644 --- a/tests/app/main/dao/test_service_dao.py +++ b/tests/app/main/dao/test_service_dao.py @@ -60,3 +60,14 @@ def test_should_not_allow_two_services_of_the_same_name(notifications_admin, not with pytest.raises(sqlalchemy.exc.IntegrityError) as error: services_dao.insert_new_service('duplicate service', user) assert 'duplicate key value violates unique constraint "services_name_key' in error.value + + +def test_should_return_list_of_service_names(notifications_admin, notifications_admin_db, notify_db_session): + user = create_test_user('active') + services_dao.insert_new_service('first service', user) + services_dao.insert_new_service('second service', user) + services_dao.insert_new_service('third service', user) + expected = ['first service', 'second service', 'third service'] + + actual = services_dao.find_all_service_names() + assert actual == expected diff --git a/tests/app/main/test_add_service_form.py b/tests/app/main/test_add_service_form.py index 53478266d..24f67361e 100644 --- a/tests/app/main/test_add_service_form.py +++ b/tests/app/main/test_add_service_form.py @@ -1,18 +1,12 @@ -from app.main.dao import services_dao from app.main.forms import AddServiceForm -from tests.app.main import create_test_user +from werkzeug.datastructures import MultiDict def test_form_should_have_errors_when_duplicate_service_is_added(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_request_context(method='POST', - data={'service_name': 'some service'}) as req: - user = create_test_user('active') - services_dao.insert_new_service('some service', user) - req.session['user_id'] = user.id - form = AddServiceForm(req.request.form) - assert form.validate() is False - assert len(form.errors) == 1 - expected = {'service_name': ['Duplicate service name']} - assert form.errors == expected + with notifications_admin.test_request_context(): + form = AddServiceForm(['some service', 'more names'], + formdata=MultiDict([('service_name', 'some service')])) + form.validate() + assert {'service_name': ['Service name already exists']} == form.errors diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index f5f2e3288..6163950ce 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -38,5 +38,5 @@ def test_should_return_form_errors_when_service_name_is_empty(notifications_admi verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') client.post('/two-factor', data={'sms_code': '12345'}) response = client.post('/add-service', data={}) - assert response.status_code == 400 + assert response.status_code == 200 assert 'Please enter your service name' in response.get_data(as_text=True) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index a8b129858..6f0a4bfe3 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -51,7 +51,6 @@ def test_should_login_user_when_multiple_valid_codes_exist(notifications_admin, response = client.post('/two-factor', data={'sms_code': '23456'}) assert response.status_code == 302 - print(user.id) codes = verify_codes_dao.get_codes(user_id=user.id, code_type='sms') # query will only return codes where code_used == False assert len(codes) == 0