From 75bafd7a7d144e4dc1b0144870d72e40b3e47c39 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 4 Jan 2016 14:01:50 +0000 Subject: [PATCH 1/3] Add the Gemfile.lock back --- Gemfile.lock | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 Gemfile.lock diff --git a/Gemfile.lock b/Gemfile.lock new file mode 100644 index 000000000..3c5e06876 --- /dev/null +++ b/Gemfile.lock @@ -0,0 +1,13 @@ +GEM + remote: https://rubygems.org/ + specs: + sass (3.4.19) + +PLATFORMS + ruby + +DEPENDENCIES + sass + +BUNDLED WITH + 1.10.6 \ No newline at end of file From ac05f6931ec5da1f85136dbc250e636e877d74c3 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 4 Jan 2016 15:31:50 +0000 Subject: [PATCH 2/3] Refactor add-service form such that the dao is not exposed in the forms. --- app/main/dao/services_dao.py | 6 ++++++ app/main/forms.py | 20 ++++++++++++-------- app/main/views/add_service.py | 17 ++++++----------- app/templates/govuk_template.html | 1 + app/templates/macros.html | 12 ++++++++++++ app/templates/views/add-service.html | 4 +--- tests/app/main/dao/test_service_dao.py | 12 ++++++++++++ tests/app/main/test_add_service_form.py | 18 ++++++------------ tests/app/main/views/test_add_service.py | 2 +- 9 files changed, 57 insertions(+), 35 deletions(-) create mode 100644 app/templates/macros.html 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..98d82df59 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,19 @@ 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 + print("a {}".format(a)) + print("self.service_name {}".format(self.service_name.data)) + print("service_names {}".format(self.service_names)) + if self.service_name.data in self.service_names: + print('here') + 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..f6d74e7ac 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -1,25 +1,20 @@ 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()) + print("serivce_names {}".format(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) +§ \ No newline at end of file diff --git a/app/templates/govuk_template.html b/app/templates/govuk_template.html index dc5747325..c3a4fa78f 100644 --- a/app/templates/govuk_template.html +++ b/app/templates/govuk_template.html @@ -1,3 +1,4 @@ +{%- from "macros.html" import render_field %} {% block top_of_page %}{% endblock %} diff --git a/app/templates/macros.html b/app/templates/macros.html new file mode 100644 index 000000000..643099498 --- /dev/null +++ b/app/templates/macros.html @@ -0,0 +1,12 @@ +{% macro render_field(field) %} +
{{ field.label }} +
{{ field(**kwargs)|safe }} + {% if field.errors %} +
    + {% for error in field.errors %} +
  • {{ error }}
    • + {% endfor %} +
+ {% endif %} +
+{% endmacro %} 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..191e7da56 100644 --- a/tests/app/main/dao/test_service_dao.py +++ b/tests/app/main/dao/test_service_dao.py @@ -60,3 +60,15 @@ 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) From 785c413cde0c2b04d28ade0a018a759e06b6c9bf Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 4 Jan 2016 15:50:26 +0000 Subject: [PATCH 3/3] Move and rename macro.html Remove print statements Fix code style --- app/main/forms.py | 4 ---- app/main/views/add_service.py | 2 -- app/templates/{macros.html => components/form-field.html} | 0 app/templates/govuk_template.html | 2 +- tests/app/main/dao/test_service_dao.py | 1 - tests/app/main/views/test_two_factor.py | 1 - 6 files changed, 1 insertion(+), 9 deletions(-) rename app/templates/{macros.html => components/form-field.html} (100%) diff --git a/app/main/forms.py b/app/main/forms.py index 98d82df59..28238e74e 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -89,11 +89,7 @@ class AddServiceForm(Form): service_name = StringField(validators=[DataRequired(message='Please enter your service name')]) def validate_service_name(self, a): - print("a {}".format(a)) - print("self.service_name {}".format(self.service_name.data)) - print("service_names {}".format(self.service_names)) if self.service_name.data in self.service_names: - print('here') raise ValidationError('Service name already exists') diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index f6d74e7ac..0669b1742 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -9,7 +9,6 @@ from app.main.forms import AddServiceForm @login_required def add_service(): form = AddServiceForm(services_dao.find_all_service_names()) - print("serivce_names {}".format(services_dao.find_all_service_names())) if form.validate_on_submit(): user = users_dao.get_user_by_id(session['user_id']) @@ -17,4 +16,3 @@ def add_service(): return redirect('/dashboard') else: return render_template('views/add-service.html', form=form) -§ \ No newline at end of file diff --git a/app/templates/macros.html b/app/templates/components/form-field.html similarity index 100% rename from app/templates/macros.html rename to app/templates/components/form-field.html diff --git a/app/templates/govuk_template.html b/app/templates/govuk_template.html index c3a4fa78f..8057ff1d8 100644 --- a/app/templates/govuk_template.html +++ b/app/templates/govuk_template.html @@ -1,4 +1,4 @@ -{%- from "macros.html" import render_field %} +{%- from "components/form-field.html" import render_field %} {% block top_of_page %}{% endblock %} diff --git a/tests/app/main/dao/test_service_dao.py b/tests/app/main/dao/test_service_dao.py index 191e7da56..e023104b8 100644 --- a/tests/app/main/dao/test_service_dao.py +++ b/tests/app/main/dao/test_service_dao.py @@ -71,4 +71,3 @@ def test_should_return_list_of_service_names(notifications_admin, notifications_ actual = services_dao.find_all_service_names() assert actual == expected - 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