Merge pull request #39 from alphagov/change_add_service_form

Refactor add-service form such that the dao is not exposed in the forms.
This commit is contained in:
NIcholas Staples
2016-01-04 17:24:07 +00:00
10 changed files with 50 additions and 36 deletions

View File

@@ -1,5 +1,7 @@
from datetime import datetime from datetime import datetime
from sqlalchemy.orm import load_only
from app import db from app import db
from app.models import Service from app.models import Service
@@ -39,3 +41,7 @@ def add_service(service):
def find_service_by_service_name(service_name): def find_service_by_service_name(service_name):
return Service.query.filter_by(name=service_name).first() 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()]

View File

@@ -2,10 +2,9 @@ from datetime import datetime
from flask import session from flask import session
from flask_wtf import Form 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 wtforms.validators import DataRequired, Email, Length, Regexp
from app.main.dao import verify_codes_dao
from app.main.dao import verify_codes_dao, services_dao
from app.main.encryption import check_hash from app.main.encryption import check_hash
from app.main.validators import Blacklist from app.main.validators import Blacklist
@@ -83,14 +82,15 @@ class TextNotReceivedForm(Form):
class AddServiceForm(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')]) service_name = StringField(validators=[DataRequired(message='Please enter your service name')])
def validate_service_name(self, a): def validate_service_name(self, a):
if services_dao.find_service_by_service_name(self.service_name.data) is not None: if self.service_name.data in self.service_names:
self.service_name.errors.append('Duplicate service name') raise ValidationError('Service name already exists')
return False
else:
return True
def validate_codes(field, code_type): def validate_codes(field, code_type):

View File

@@ -1,25 +1,18 @@
from flask import render_template, jsonify, redirect, session from flask import render_template, jsonify, redirect, session
from flask_login import login_required from flask_login import login_required
from app.main import main from app.main import main
from app.main.dao import services_dao, users_dao from app.main.dao import services_dao, users_dao
from app.main.forms import AddServiceForm from app.main.forms import AddServiceForm
@main.route("/add-service", methods=['GET']) @main.route("/add-service", methods=['GET', 'POST'])
@login_required @login_required
def add_service(): def add_service():
return render_template('views/add-service.html', form=AddServiceForm()) form = AddServiceForm(services_dao.find_all_service_names())
@main.route("/add-service", methods=['POST'])
@login_required
def process_add_service():
form = AddServiceForm()
if form.validate_on_submit(): if form.validate_on_submit():
user = users_dao.get_user_by_id(session['user_id']) user = users_dao.get_user_by_id(session['user_id'])
services_dao.insert_new_service(form.service_name.data, user) services_dao.insert_new_service(form.service_name.data, user)
return redirect('/dashboard') return redirect('/dashboard')
else: else:
return jsonify(form.errors), 400 return render_template('views/add-service.html', form=form)

View File

@@ -0,0 +1,12 @@
{% macro render_field(field) %}
<dt>{{ field.label }}
<dd>{{ field(**kwargs)|safe }}
{% if field.errors %}
<ul class=error-summary>
{% for error in field.errors %}
<li>{{ error }}</li>
{% endfor %}
</ul>
{% endif %}
</dd>
{% endmacro %}

View File

@@ -1,3 +1,4 @@
{%- from "components/form-field.html" import render_field %}
{% block top_of_page %}{% endblock %} {% block top_of_page %}{% endblock %}
<!DOCTYPE html> <!DOCTYPE html>
<!--[if lt IE 9]><html class="lte-ie8" lang="{{ html_lang|default('en') }}"><![endif]--> <!--[if lt IE 9]><html class="lte-ie8" lang="{{ html_lang|default('en') }}"><![endif]-->

View File

@@ -18,9 +18,7 @@ GOV.UK Notify | Set up service
<form autocomplete="off" action="" method="post"> <form autocomplete="off" action="" method="post">
{{ form.hidden_tag() }} {{ form.hidden_tag() }}
<label class="form-label">Service name</label> {{ render_field(form.service_name, class='form-control-2-3') }}
{{ form.service_name(class="form-control-2-3", autocomplete="off") }} <br>
<span class="font-xsmall">For example, 'Vehicle tax' or 'Carer's allowance'</span>
<p> <p>
<button class="button" href="dashboard" role="button">Continue</button> <button class="button" href="dashboard" role="button">Continue</button>

View File

@@ -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: with pytest.raises(sqlalchemy.exc.IntegrityError) as error:
services_dao.insert_new_service('duplicate service', user) services_dao.insert_new_service('duplicate service', user)
assert 'duplicate key value violates unique constraint "services_name_key' in error.value 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

View File

@@ -1,18 +1,12 @@
from app.main.dao import services_dao
from app.main.forms import AddServiceForm 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, def test_form_should_have_errors_when_duplicate_service_is_added(notifications_admin,
notifications_admin_db, notifications_admin_db,
notify_db_session): notify_db_session):
with notifications_admin.test_request_context(method='POST', with notifications_admin.test_request_context():
data={'service_name': 'some service'}) as req: form = AddServiceForm(['some service', 'more names'],
user = create_test_user('active') formdata=MultiDict([('service_name', 'some service')]))
services_dao.insert_new_service('some service', user) form.validate()
req.session['user_id'] = user.id assert {'service_name': ['Service name already exists']} == form.errors
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

View File

@@ -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') verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms')
client.post('/two-factor', data={'sms_code': '12345'}) client.post('/two-factor', data={'sms_code': '12345'})
response = client.post('/add-service', data={}) 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) assert 'Please enter your service name' in response.get_data(as_text=True)

View File

@@ -51,7 +51,6 @@ def test_should_login_user_when_multiple_valid_codes_exist(notifications_admin,
response = client.post('/two-factor', response = client.post('/two-factor',
data={'sms_code': '23456'}) data={'sms_code': '23456'})
assert response.status_code == 302 assert response.status_code == 302
print(user.id)
codes = verify_codes_dao.get_codes(user_id=user.id, code_type='sms') codes = verify_codes_dao.get_codes(user_id=user.id, code_type='sms')
# query will only return codes where code_used == False # query will only return codes where code_used == False
assert len(codes) == 0 assert len(codes) == 0