From ec020a42f40d0b483249e14b8214e1dcaf495a5a Mon Sep 17 00:00:00 2001 From: Chris Heathcote Date: Mon, 14 Dec 2015 16:37:15 +0000 Subject: [PATCH 01/13] Initial nav & implementation on dashboard --- .gitignore | 2 ++ app/assets/stylesheets/app.scss | 32 +++++++++++++++++++ .../stylesheets/components/navigation.scss | 23 +++++++++++++ app/assets/stylesheets/main.scss | 1 + app/templates/main_nav.html | 19 +++++++++++ app/templates/views/dashboard.html | 26 +++++++-------- app/templates/withnav_template.html | 15 +++++++++ 7 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 app/assets/stylesheets/components/navigation.scss create mode 100644 app/templates/main_nav.html create mode 100644 app/templates/withnav_template.html diff --git a/.gitignore b/.gitignore index aca6ef03d..0a21cad9f 100644 --- a/.gitignore +++ b/.gitignore @@ -63,3 +63,5 @@ app/assets/stylesheets/govuk_template/.sass-cache/ .sass-cache/ cache/ app/static/* +app/static/stylesheets/*.css + diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index 50c9ec702..d7689cbd6 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -44,4 +44,36 @@ #global-header #logo { white-space: nowrap; +} + + + +#navigation { + padding: 50px 0 0 0; +} + +#navigation li { + @include core-16(); + margin: 3px 0; +} + +#navigation ul { + border-bottom: 1px solid #777; + margin: 0 20px 0 0; + list-style-type: none; + padding: 0; +} + +#navigation a:link,a:visited { + text-decoration: none; +} + +#navigation a:hover { + color: #2e8aca; +} + + + +a:visited { + color: #005ea5; } \ No newline at end of file diff --git a/app/assets/stylesheets/components/navigation.scss b/app/assets/stylesheets/components/navigation.scss new file mode 100644 index 000000000..f11e2c541 --- /dev/null +++ b/app/assets/stylesheets/components/navigation.scss @@ -0,0 +1,23 @@ +#navigation { + padding: 50px 0 0 0; +} + +#navigation li { + @include core-16(); + margin: 3px 0; +} + +#navigation ul { + border-bottom: 1px solid #777; + margin: 0 20px 0 0; + list-style-type: none; + padding: 0; +} + +#navigation a:link,a:visited { + text-decoration: none; +} + +#navigation a:hover { + color: #2e8aca; +} \ No newline at end of file diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index e0591db04..e2b93d0c5 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -33,6 +33,7 @@ @import "components/sms-message"; @import "components/submit-form"; @import "components/table"; +@import "components/navigation"; // TODO: break this up @import "app"; diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html new file mode 100644 index 000000000..8ee24ff6a --- /dev/null +++ b/app/templates/main_nav.html @@ -0,0 +1,19 @@ +{% macro main_nav() %} + + +{% endmacro %} \ No newline at end of file diff --git a/app/templates/views/dashboard.html b/app/templates/views/dashboard.html index a8d5e6e31..147a069c6 100644 --- a/app/templates/views/dashboard.html +++ b/app/templates/views/dashboard.html @@ -1,26 +1,24 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} + {% block page_title %} GOV.UK Notify | Dashboard {% endblock %} -{% block content %} +{% block maincolumn_content %} -
-

Dashboard

-
-
+ {% endblock %} diff --git a/app/templates/withnav_template.html b/app/templates/withnav_template.html new file mode 100644 index 000000000..b0bd19960 --- /dev/null +++ b/app/templates/withnav_template.html @@ -0,0 +1,15 @@ +{% extends "admin_template.html" %} +{% from "main_nav.html" import main_nav with context %} + +{% block content %} + +
+
+ {{ main_nav() }} +
+
+ {% block maincolumn_content %}{% endblock %} +
+
+ +{% endblock %} \ No newline at end of file From 15c9269a80220cda500390b30e3bc860e3a12de1 Mon Sep 17 00:00:00 2001 From: Chris Heathcote Date: Mon, 14 Dec 2015 16:53:07 +0000 Subject: [PATCH 02/13] Added main nav to logged in pages --- app/templates/views/api-keys.html | 9 +++------ app/templates/views/check-email.html | 9 +++------ app/templates/views/check-sms.html | 10 ++++------ app/templates/views/dashboard.html | 11 +---------- app/templates/views/edit-template.html | 10 ++++------ app/templates/views/job.html | 9 +++------ app/templates/views/jobs.html | 9 +++------ app/templates/views/manage-templates.html | 10 ++++------ app/templates/views/manage-users.html | 9 +++------ app/templates/views/notification.html | 10 ++++------ app/templates/views/send-email.html | 10 ++++------ app/templates/views/send-sms.html | 9 +++------ app/templates/views/service-settings.html | 9 +++------ app/templates/views/user-profile.html | 8 ++------ 14 files changed, 44 insertions(+), 88 deletions(-) diff --git a/app/templates/views/api-keys.html b/app/templates/views/api-keys.html index 557a0c0f0..dd1c7678d 100644 --- a/app/templates/views/api-keys.html +++ b/app/templates/views/api-keys.html @@ -1,20 +1,17 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | API keys and documentation {% endblock %} -{% block content %} +{% block maincolumn_content %} -
-

API keys and documentation

Here's where developers can access information about the API and access keys

Back to dashboard

-
-
+ {% endblock %} diff --git a/app/templates/views/check-email.html b/app/templates/views/check-email.html index 6f6bd51af..487f5de73 100644 --- a/app/templates/views/check-email.html +++ b/app/templates/views/check-email.html @@ -1,13 +1,11 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | Send email {% endblock %} -{% block content %} +{% block maincolumn_content %} -
-

Send email

This page will be where we check the email messages we're about to send

@@ -15,7 +13,6 @@ GOV.UK Notify | Send email

Send email messages

-
-
+ {% endblock %} diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html index 13084123a..104d1d075 100644 --- a/app/templates/views/check-sms.html +++ b/app/templates/views/check-sms.html @@ -1,4 +1,4 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% from "components/sms-message.html" import sms_message %} {% from "components/table.html" import table, field %} {% from "components/placeholder.html" import placeholder %} @@ -8,10 +8,9 @@ GOV.UK Notify | Send text messages {% endblock %} -{% block content %} +{% block maincolumn_content %} + -
-

Send text messages

Check and confirm

@@ -61,7 +60,6 @@ url_for(".sendsms") ) }} -
-
+ {% endblock %} diff --git a/app/templates/views/dashboard.html b/app/templates/views/dashboard.html index 147a069c6..3e294bb47 100644 --- a/app/templates/views/dashboard.html +++ b/app/templates/views/dashboard.html @@ -9,16 +9,7 @@ GOV.UK Notify | Dashboard

Dashboard

- +

Dashboard goes here.

{% endblock %} diff --git a/app/templates/views/edit-template.html b/app/templates/views/edit-template.html index a6da98b25..ed29b8074 100644 --- a/app/templates/views/edit-template.html +++ b/app/templates/views/edit-template.html @@ -1,20 +1,18 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | Edit template {% endblock %} -{% block content %} +{% block maincolumn_content %} + -
-

Edit template

Here's where you can edit an exiting template (including delete) or add a new one

Back to manage templates

-
-
+ {% endblock %} diff --git a/app/templates/views/job.html b/app/templates/views/job.html index ccaee802c..7457debbc 100644 --- a/app/templates/views/job.html +++ b/app/templates/views/job.html @@ -1,13 +1,11 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | Notifications activity {% endblock %} -{% block content %} +{% block maincolumn_content %} -
-

Notifications for a specific job

This page will be where we list the notifications for a specific job.

@@ -16,7 +14,6 @@ GOV.UK Notify | Notifications activity
  • view a specific notification
  • view all the activity for this service
    • -
    -
    + {% endblock %} diff --git a/app/templates/views/jobs.html b/app/templates/views/jobs.html index b38f5da9f..79a9604d8 100644 --- a/app/templates/views/jobs.html +++ b/app/templates/views/jobs.html @@ -1,13 +1,11 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | Notifications activity {% endblock %} -{% block content %} +{% block maincolumn_content %} -
    -

    Notifications activity

    This page will be where we show the list of jobs that this service has processed

    @@ -15,7 +13,6 @@ GOV.UK Notify | Notifications activity

    view a particular notification job

    -
    -
    + {% endblock %} diff --git a/app/templates/views/manage-templates.html b/app/templates/views/manage-templates.html index 08ed55c22..1554f3b09 100644 --- a/app/templates/views/manage-templates.html +++ b/app/templates/views/manage-templates.html @@ -1,13 +1,12 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | Manage templates {% endblock %} -{% block content %} +{% block maincolumn_content %} + -
    -

    Manage templates

    Here's where you can view templates, choose to add one, or edit/delete one.

    @@ -17,7 +16,6 @@ GOV.UK Notify | Manage templates

    Add a new message template

    -
    -
    + {% endblock %} diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index 8a93b2072..f60a6c53b 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -1,20 +1,17 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | Manage users {% endblock %} -{% block content %} +{% block maincolumn_content %} -
    -

    Manage users

    Here's where you can add or remove users of a service.

    Back to dashboard

    -
    -
    + {% endblock %} diff --git a/app/templates/views/notification.html b/app/templates/views/notification.html index bf7faa9e6..99a6902db 100644 --- a/app/templates/views/notification.html +++ b/app/templates/views/notification.html @@ -1,13 +1,12 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | Notifications activity {% endblock %} -{% block content %} +{% block maincolumn_content %} + -
    -

    A specific notification

    This page will be where we show what happened for a specific notification.

    @@ -15,7 +14,6 @@ GOV.UK Notify | Notifications activity

    View other notifications in this job

    -
    -
    + {% endblock %} diff --git a/app/templates/views/send-email.html b/app/templates/views/send-email.html index 9161af232..e5911ec4c 100644 --- a/app/templates/views/send-email.html +++ b/app/templates/views/send-email.html @@ -1,13 +1,12 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | Send email {% endblock %} -{% block content %} +{% block maincolumn_content %} + -
    -

    Send email

    This page will be where we construct email messages

    @@ -15,7 +14,6 @@ GOV.UK Notify | Send email

    Continue

    -
    -
    + {% endblock %} diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html index 0915d7ab2..ce07dc69e 100644 --- a/app/templates/views/send-sms.html +++ b/app/templates/views/send-sms.html @@ -1,14 +1,12 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% from "components/sms-message.html" import sms_message %} {% block page_title %} GOV.UK Notify | Send text messages {% endblock %} -{% block content %} +{% block maincolumn_content %}
    -
    -

    Send text messages

    @@ -46,8 +44,7 @@

    -
    -
    +
    {% endblock %} diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 3e6c7bcb4..e53fb9efe 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -1,20 +1,17 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | Service settings {% endblock %} -{% block content %} +{% block maincolumn_content %} -
    -

    Service settings

    Here's where users can update their service profile.

    Back to dashboard

    -
    -
    + {% endblock %} diff --git a/app/templates/views/user-profile.html b/app/templates/views/user-profile.html index a394b8bd8..12f322314 100644 --- a/app/templates/views/user-profile.html +++ b/app/templates/views/user-profile.html @@ -1,20 +1,16 @@ -{% extends "admin_template.html" %} +{% extends "withnav_template.html" %} {% block page_title %} GOV.UK Notify | User settings {% endblock %} -{% block content %} +{% block maincolumn_content %} -
    -

    User profile

    Here's where users can update their profile, password etc.

    Back to dashboard

    -
    -
    {% endblock %} From 89189f2e35de9335e27270fa970412fd1fa38b1d Mon Sep 17 00:00:00 2001 From: Chris Heathcote Date: Mon, 14 Dec 2015 16:58:43 +0000 Subject: [PATCH 03/13] Add URLs to main nav; add user profile to nav --- app/templates/main_nav.html | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 8ee24ff6a..0e2d190f3 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -1,19 +1,22 @@ {% macro main_nav() %} {% endmacro %} \ No newline at end of file From 8939f710a9c4eb683ea9f566a0dede8e64c82938 Mon Sep 17 00:00:00 2001 From: Chris Heathcote Date: Mon, 14 Dec 2015 17:05:39 +0000 Subject: [PATCH 04/13] Move navigation into its own css file --- app/assets/stylesheets/app.scss | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index d7689cbd6..e049ec7b4 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -48,29 +48,7 @@ -#navigation { - padding: 50px 0 0 0; -} -#navigation li { - @include core-16(); - margin: 3px 0; -} - -#navigation ul { - border-bottom: 1px solid #777; - margin: 0 20px 0 0; - list-style-type: none; - padding: 0; -} - -#navigation a:link,a:visited { - text-decoration: none; -} - -#navigation a:hover { - color: #2e8aca; -} From 4b01335703a0f301bd3e8069e0295fa941b0d84e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 14 Dec 2015 15:59:29 +0000 Subject: [PATCH 05/13] 110067722: Create the model and dao for services. This commit creates the data model for services and user_to_service. The dao is also created to insert, get, activate, and unrestrict the service. --- app/main/dao/services_dao.py | 38 ++++++++++++++++++++++ app/models.py | 35 ++++++++++++++++++++ migrations/versions/60_add_service.py | 44 ++++++++++++++++++++++++++ tests/app/main/dao/test_service_dao.py | 39 +++++++++++++++++++++++ 4 files changed, 156 insertions(+) create mode 100644 app/main/dao/services_dao.py create mode 100644 migrations/versions/60_add_service.py create mode 100644 tests/app/main/dao/test_service_dao.py diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py new file mode 100644 index 000000000..53a9f64f6 --- /dev/null +++ b/app/main/dao/services_dao.py @@ -0,0 +1,38 @@ +from datetime import datetime + +from app import db +from app.models import Service + + +def insert_new_service(service_name, user): + service = Service(name=service_name, + created_at=datetime.now(), + limit=1000, + active=False, + restricted=True) + + add_service(service) + service.users.append(user) + db.session.commit() + return service.id + + +def get_service_by_id(id): + return Service.query.get(id) + + +def unrestrict_service(service_id): + service = get_service_by_id(service_id) + service.restricted = False + add_service(service) + + +def activate_service(service_id): + service = get_service_by_id(service_id) + service.active = True + add_service(service) + + +def add_service(service): + db.session.add(service) + db.session.commit() diff --git a/app/models.py b/app/models.py index cb99c26a0..ba4492874 100644 --- a/app/models.py +++ b/app/models.py @@ -78,6 +78,41 @@ class User(db.Model): return True +user_to_service = db.Table( + 'user_to_service', + db.Model.metadata, + db.Column('user_id', db.Integer, db.ForeignKey('users.id')), + db.Column('service_id', db.Integer, db.ForeignKey('services.id')) +) + + +class Service(db.Model): + __tablename__ = 'services' + + id = db.Column(db.Integer, primary_key=True) + name = db.Column(db.String(255), nullable=False) + + token_id = db.Column(db.BigInteger, index=True, unique=True) + created_at = db.Column(db.DateTime, index=False, unique=False, nullable=False) + active = db.Column(db.Boolean, index=False, unique=False, nullable=False) + limit = db.Column(db.BigInteger, index=False, unique=False, nullable=False) + users = db.relationship('User', secondary=user_to_service, backref=db.backref('user_to_service', lazy='dynamic')) + restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) + + def serialize(self): + serialized = { + 'id': self.id, + 'name': self.name, + 'createdAt': self.created_at.strftime(DATETIME_FORMAT), + 'active': self.active, + 'restricted': self.restricted, + 'limit': self.limit, + 'user': self.users.serialize() + } + + return filter_null_value_fields(serialized) + + def filter_null_value_fields(obj): return dict( filter(lambda x: x[1] is not None, obj.items()) diff --git a/migrations/versions/60_add_service.py b/migrations/versions/60_add_service.py new file mode 100644 index 000000000..8e366e504 --- /dev/null +++ b/migrations/versions/60_add_service.py @@ -0,0 +1,44 @@ +"""empty message + +Revision ID: 60_add_service +Revises: 50_alter_verify_code_type +Create Date: 2015-12-14 15:22:55.938819 + +""" + +# revision identifiers, used by Alembic. +revision = '60_add_service' +down_revision = '50_alter_verify_code_type' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.create_table('services', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('name', sa.String(length=255), nullable=False), + sa.Column('token_id', sa.BigInteger(), nullable=True), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.Column('active', sa.Boolean(), nullable=False), + sa.Column('limit', sa.BigInteger(), nullable=False), + sa.Column('restricted', sa.Boolean(), nullable=False), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_services_token_id'), 'services', ['token_id'], unique=True) + op.create_table('user_to_service', + sa.Column('user_id', sa.Integer(), nullable=True), + sa.Column('service_id', sa.Integer(), nullable=True), + sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['users.id'], ) + ) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_table('user_to_service') + op.drop_index(op.f('ix_services_token_id'), table_name='services') + op.drop_table('services') + ### end Alembic commands ### diff --git a/tests/app/main/dao/test_service_dao.py b/tests/app/main/dao/test_service_dao.py new file mode 100644 index 000000000..661cc0cad --- /dev/null +++ b/tests/app/main/dao/test_service_dao.py @@ -0,0 +1,39 @@ +from app.main.dao import services_dao +from tests.app.main import create_test_user + + +def test_can_insert_and_retrieve_new_service(notifications_admin, notifications_admin_db, notify_db_session): + user = create_test_user() + + id = services_dao.insert_new_service('testing service', user) + saved_service = services_dao.get_service_by_id(id) + assert id == saved_service.id + assert saved_service.users == [user] + assert saved_service.name == 'testing service' + + +def test_unrestrict_service_updates_the_service(notifications_admin, notifications_admin_db, notify_db_session): + user = create_test_user() + id = services_dao.insert_new_service('unrestricted service', user) + saved_service = services_dao.get_service_by_id(id) + assert saved_service.restricted is True + services_dao.unrestrict_service(id) + unrestricted_service = services_dao.get_service_by_id(id) + assert unrestricted_service.restricted is False + + +def test_activate_service_update_service(notifications_admin, notifications_admin_db, notify_db_session): + user = create_test_user() + id = services_dao.insert_new_service('activated service', user) + service = services_dao.get_service_by_id(id) + assert service.active is False + services_dao.activate_service(id) + activated_service = services_dao.get_service_by_id(id) + assert activated_service.active is True + + +def test_get_service_returns_none_if_service_does_not_exist(notifications_admin, + notifications_admin_db, + notify_db_session): + service = services_dao.get_service_by_id(1) + assert service is None From 350ccda20833161b99bcfb875c706fed664c8e7d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 14 Dec 2015 17:12:28 +0000 Subject: [PATCH 06/13] 110067722: Added endpoints for add-service Post is not complete as of yet. --- app/main/__init__.py | 2 +- app/main/dao/services_dao.py | 4 ++++ app/main/forms.py | 4 ++++ app/main/views/add_service.py | 23 +++++++++++++++++++++ app/main/views/index.py | 6 ------ app/models.py | 2 +- migrations/versions/60_add_service.py | 5 +++-- tests/app/main/dao/test_service_dao.py | 25 ++++++++++++++++++++++- tests/app/main/views/test_add_service.py | 26 ++++++++++++++++++++++++ 9 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 app/main/views/add_service.py create mode 100644 tests/app/main/views/test_add_service.py diff --git a/app/main/__init__.py b/app/main/__init__.py index 2f617456e..4025b6dd1 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -3,4 +3,4 @@ from flask import Blueprint main = Blueprint('main', __name__) -from app.main.views import index, sign_in, register, two_factor, verify, sms +from app.main.views import index, sign_in, register, two_factor, verify, sms, add_service diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index 53a9f64f6..6d1f11aeb 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -36,3 +36,7 @@ def activate_service(service_id): def add_service(service): db.session.add(service) db.session.commit() + + +def find_service_by_service_name(service_name): + return Service.query.filter_by(name=service_name).first() diff --git a/app/main/forms.py b/app/main/forms.py index 8d94fdaf7..21352a7f4 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -82,3 +82,7 @@ def validate_code(field, code): return True else: return False + + +class AddServiceForm(Form): + service_name = StringField(validators=[DataRequired(message='Name can not be empty')]) diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py new file mode 100644 index 000000000..4d91f97f0 --- /dev/null +++ b/app/main/views/add_service.py @@ -0,0 +1,23 @@ +from flask import render_template, jsonify, redirect +from flask_login import login_required + +from app.main import main +from app.main.forms import AddServiceForm + + +@main.route("/add-service", methods=['GET']) +@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() + + if form.validate_on_submit(): + + return redirect('/dashboard') + else: + return jsonify(form.errors), 400 diff --git a/app/main/views/index.py b/app/main/views/index.py index f1525d4cd..485b4d2d6 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -35,12 +35,6 @@ def dashboard(): return render_template('views/dashboard.html') -@main.route("/add-service") -@login_required -def addservice(): - return render_template('views/add-service.html') - - @main.route("/email-not-received") def emailnotreceived(): return render_template('views/email-not-received.html') diff --git a/app/models.py b/app/models.py index ba4492874..f7b21e81a 100644 --- a/app/models.py +++ b/app/models.py @@ -90,7 +90,7 @@ class Service(db.Model): __tablename__ = 'services' id = db.Column(db.Integer, primary_key=True) - name = db.Column(db.String(255), nullable=False) + name = db.Column(db.String(255), nullable=False, unique=True) token_id = db.Column(db.BigInteger, index=True, unique=True) created_at = db.Column(db.DateTime, index=False, unique=False, nullable=False) diff --git a/migrations/versions/60_add_service.py b/migrations/versions/60_add_service.py index 8e366e504..eca5d0b72 100644 --- a/migrations/versions/60_add_service.py +++ b/migrations/versions/60_add_service.py @@ -2,7 +2,7 @@ Revision ID: 60_add_service Revises: 50_alter_verify_code_type -Create Date: 2015-12-14 15:22:55.938819 +Create Date: 2015-12-14 16:55:56.612005 """ @@ -24,7 +24,8 @@ def upgrade(): sa.Column('active', sa.Boolean(), nullable=False), sa.Column('limit', sa.BigInteger(), nullable=False), sa.Column('restricted', sa.Boolean(), nullable=False), - sa.PrimaryKeyConstraint('id') + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('name') ) op.create_index(op.f('ix_services_token_id'), 'services', ['token_id'], unique=True) op.create_table('user_to_service', diff --git a/tests/app/main/dao/test_service_dao.py b/tests/app/main/dao/test_service_dao.py index 661cc0cad..e8918eb1f 100644 --- a/tests/app/main/dao/test_service_dao.py +++ b/tests/app/main/dao/test_service_dao.py @@ -1,10 +1,12 @@ +import pytest +import sqlalchemy + from app.main.dao import services_dao from tests.app.main import create_test_user def test_can_insert_and_retrieve_new_service(notifications_admin, notifications_admin_db, notify_db_session): user = create_test_user() - id = services_dao.insert_new_service('testing service', user) saved_service = services_dao.get_service_by_id(id) assert id == saved_service.id @@ -37,3 +39,24 @@ def test_get_service_returns_none_if_service_does_not_exist(notifications_admin, notify_db_session): service = services_dao.get_service_by_id(1) assert service is None + + +def test_find_by_service_name_returns_right_service(notifications_admin, + notifications_admin_db, + notify_db_session): + user = create_test_user() + id = services_dao.insert_new_service('testing service', user) + another = services_dao.insert_new_service('Testing the Service', user) + found = services_dao.find_service_by_service_name('testing service') + assert found.id == id + assert found.name == 'testing service' + found_another = services_dao.find_service_by_service_name('Testing the Service') + assert found_another == services_dao.get_service_by_id(another) + + +def test_should_not_allow_two_services_of_the_same_name(notifications_admin, notifications_admin_db, notify_db_session): + user = create_test_user() + services_dao.insert_new_service('duplicate service', user) + 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 diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py new file mode 100644 index 000000000..3083e5180 --- /dev/null +++ b/tests/app/main/views/test_add_service.py @@ -0,0 +1,26 @@ +from app.main.dao import verify_codes_dao +from tests.app.main import create_test_user + + +def test_get_should_render_add_service_template(notifications_admin, notifications_admin_db, notify_db_session): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user() + session['user_id'] = user.id + verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms') + client.post('/two-factor', data={'sms_code': '12345'}) + response = client.get('/add-service') + assert response.status_code == 200 + assert 'Set up notifications for your service' in response.get_data(as_text=True) + + +def test_should_add_service_and_redirect_to_next_page(notifications_admin, notifications_admin_db, notify_db_session): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user() + session['user_id'] = user.id + 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={'service_name': 'testing the post'}) + assert response.status_code == 302 + assert response.location == 'http://localhost/dashboard' From 43f2605ac459f4184b870ba2c520656de551cc03 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 Dec 2015 09:28:15 +0000 Subject: [PATCH 07/13] 110067722: Post add-service endpoint saves the service and maps it to the user. --- app/main/dao/services_dao.py | 13 ++++++++----- app/main/views/add_service.py | 6 ++++-- app/models.py | 2 -- migrations/versions/60_add_service.py | 5 +---- tests/app/main/views/test_add_service.py | 4 +++- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index 6d1f11aeb..dff0e3a68 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -10,11 +10,14 @@ def insert_new_service(service_name, user): limit=1000, active=False, restricted=True) - - add_service(service) - service.users.append(user) - db.session.commit() - return service.id + try: + add_service(service) + service.users.append(user) + db.session.commit() + return service.id + except Exception as e: + print(e) + raise e def get_service_by_id(id): diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 4d91f97f0..4d03be4f8 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -1,7 +1,8 @@ -from flask import render_template, jsonify, redirect +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 @@ -17,7 +18,8 @@ def process_add_service(): form = AddServiceForm() 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 diff --git a/app/models.py b/app/models.py index f7b21e81a..d420c9070 100644 --- a/app/models.py +++ b/app/models.py @@ -91,8 +91,6 @@ class Service(db.Model): id = db.Column(db.Integer, primary_key=True) name = db.Column(db.String(255), nullable=False, unique=True) - - token_id = db.Column(db.BigInteger, index=True, unique=True) created_at = db.Column(db.DateTime, index=False, unique=False, nullable=False) active = db.Column(db.Boolean, index=False, unique=False, nullable=False) limit = db.Column(db.BigInteger, index=False, unique=False, nullable=False) diff --git a/migrations/versions/60_add_service.py b/migrations/versions/60_add_service.py index eca5d0b72..5dd28284d 100644 --- a/migrations/versions/60_add_service.py +++ b/migrations/versions/60_add_service.py @@ -2,7 +2,7 @@ Revision ID: 60_add_service Revises: 50_alter_verify_code_type -Create Date: 2015-12-14 16:55:56.612005 +Create Date: 2015-12-15 09:25:09.000431 """ @@ -19,7 +19,6 @@ def upgrade(): op.create_table('services', sa.Column('id', sa.Integer(), nullable=False), sa.Column('name', sa.String(length=255), nullable=False), - sa.Column('token_id', sa.BigInteger(), nullable=True), sa.Column('created_at', sa.DateTime(), nullable=False), sa.Column('active', sa.Boolean(), nullable=False), sa.Column('limit', sa.BigInteger(), nullable=False), @@ -27,7 +26,6 @@ def upgrade(): sa.PrimaryKeyConstraint('id'), sa.UniqueConstraint('name') ) - op.create_index(op.f('ix_services_token_id'), 'services', ['token_id'], unique=True) op.create_table('user_to_service', sa.Column('user_id', sa.Integer(), nullable=True), sa.Column('service_id', sa.Integer(), nullable=True), @@ -40,6 +38,5 @@ def upgrade(): def downgrade(): ### commands auto generated by Alembic - please adjust! ### op.drop_table('user_to_service') - op.drop_index(op.f('ix_services_token_id'), table_name='services') op.drop_table('services') ### end Alembic commands ### diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 3083e5180..8264a0f23 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -1,4 +1,4 @@ -from app.main.dao import verify_codes_dao +from app.main.dao import verify_codes_dao, services_dao from tests.app.main import create_test_user @@ -24,3 +24,5 @@ def test_should_add_service_and_redirect_to_next_page(notifications_admin, notif response = client.post('/add-service', data={'service_name': 'testing the post'}) assert response.status_code == 302 assert response.location == 'http://localhost/dashboard' + saved_service = services_dao.find_service_by_service_name('testing the post') + assert saved_service is not None From 23b5cffbe8ec26ec2004e8d611887b6fe6637926 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 Dec 2015 09:37:54 +0000 Subject: [PATCH 08/13] 110067722: Update template with the form. --- app/main/dao/services_dao.py | 12 ++++-------- app/templates/views/add-service.html | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index dff0e3a68..9881d6689 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -10,14 +10,10 @@ def insert_new_service(service_name, user): limit=1000, active=False, restricted=True) - try: - add_service(service) - service.users.append(user) - db.session.commit() - return service.id - except Exception as e: - print(e) - raise e + add_service(service) + service.users.append(user) + db.session.commit() + return service.id def get_service_by_id(id): diff --git a/app/templates/views/add-service.html b/app/templates/views/add-service.html index 130247a0d..3b50f3371 100644 --- a/app/templates/views/add-service.html +++ b/app/templates/views/add-service.html @@ -16,15 +16,17 @@ GOV.UK Notify | Set up service
  • as your email sender name
    • -

    - -
    - For example, 'Vehicle tax' or 'Carer's allowance' -

    +
    + {{ form.hidden_tag() }} + + {{ form.service_name(class="form-control-2-3", autocomplete="off") }}
    + For example, 'Vehicle tax' or 'Carer's allowance' -

    - Continue -

    +

    + +

    +
    + From eb0cff18c574547c6dff5e1483fb6ff8cde5bce0 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 Dec 2015 10:17:43 +0000 Subject: [PATCH 09/13] 110067722: Add form validation for duplicate service name. --- app/main/forms.py | 11 +++++++++-- tests/app/main/test_add_service_form.py | 18 ++++++++++++++++++ tests/app/main/views/test_add_service.py | 12 ++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 tests/app/main/test_add_service_form.py diff --git a/app/main/forms.py b/app/main/forms.py index 21352a7f4..300b2600f 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -5,7 +5,7 @@ from flask_wtf import Form from wtforms import StringField, PasswordField 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.validators import Blacklist @@ -85,4 +85,11 @@ def validate_code(field, code): class AddServiceForm(Form): - service_name = StringField(validators=[DataRequired(message='Name can not be empty')]) + 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 diff --git a/tests/app/main/test_add_service_form.py b/tests/app/main/test_add_service_form.py new file mode 100644 index 000000000..87f460c22 --- /dev/null +++ b/tests/app/main/test_add_service_form.py @@ -0,0 +1,18 @@ +from app.main.dao import services_dao +from app.main.forms import AddServiceForm +from tests.app.main import create_test_user + + +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() + 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 diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 8264a0f23..b341b6622 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -26,3 +26,15 @@ def test_should_add_service_and_redirect_to_next_page(notifications_admin, notif assert response.location == 'http://localhost/dashboard' saved_service = services_dao.find_service_by_service_name('testing the post') assert saved_service is not None + + +def test_should_return_form_errors_when_service_name_is_empty(notifications_admin, notifications_admin_db, notify_db_session): + with notifications_admin.test_client() as client: + with client.session_transaction() as session: + user = create_test_user() + session['user_id'] = user.id + 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 'Please enter your service name' in response.get_data(as_text=True) From 258ae3674b56ea91a9fc0ab9d83a23ff834fea0d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 Dec 2015 10:30:08 +0000 Subject: [PATCH 10/13] 110067722: Fix code style --- tests/app/main/views/test_add_service.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index b341b6622..8ac5755d4 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -28,7 +28,9 @@ def test_should_add_service_and_redirect_to_next_page(notifications_admin, notif assert saved_service is not None -def test_should_return_form_errors_when_service_name_is_empty(notifications_admin, notifications_admin_db, notify_db_session): +def test_should_return_form_errors_when_service_name_is_empty(notifications_admin, + notifications_admin_db, + notify_db_session): with notifications_admin.test_client() as client: with client.session_transaction() as session: user = create_test_user() From 17850dc17043191af1a1962dba23726309c4ccf0 Mon Sep 17 00:00:00 2001 From: Chris Heathcote Date: Tue, 15 Dec 2015 10:52:24 +0000 Subject: [PATCH 11/13] Changed nav urls to be url_fors --- app/templates/main_nav.html | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 0e2d190f3..105ada791 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -1,19 +1,18 @@ {% macro main_nav() %}