diff --git a/app/__init__.py b/app/__init__.py index 8b92019d4..7ca6252b1 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -27,8 +27,8 @@ def create_app(config_name, config_overrides=None): application.config.from_object(configs[config_name]) init_app(application, config_overrides) db.init_app(application) - init_csrf(application) logging.init_app(application) + init_csrf(application) notifications_api_client.init_app(application) @@ -75,15 +75,16 @@ def init_csrf(application): def init_app(app, config_overrides): - for key, value in app.config.items(): - if key in os.environ: - app.config[key] = convert_to_boolean(os.environ[key]) if config_overrides: for key in app.config.keys(): if key in config_overrides: app.config[key] = config_overrides[key] + for key, value in app.config.items(): + if key in os.environ: + app.config[key] = convert_to_boolean(os.environ[key]) + @app.context_processor def inject_global_template_variables(): return {'asset_path': '/static/'} diff --git a/app/assets/javascripts/detailsPolyfill.js b/app/assets/javascripts/detailsPolyfill.js new file mode 100644 index 000000000..d1e5a3ae9 --- /dev/null +++ b/app/assets/javascripts/detailsPolyfill.js @@ -0,0 +1,199 @@ +// Copied from: +// https://github.com/alphagov/govuk_elements/blob/4926897dc7734db2fc5e5ebb6acdc97f86e22e50/public/javascripts/vendor/details.polyfill.js + +// When this is moved to GOV.UK Frontend Toolkit, we should import it from there +// instead + +//
polyfill +// http://caniuse.com/#feat=details + +// FF Support for HTML5's
and +// https://bugzilla.mozilla.org/show_bug.cgi?id=591737 + +// http://www.sitepoint.com/fixing-the-details-element/ + +(function () { + 'use strict'; + + var NATIVE_DETAILS = typeof document.createElement('details').open === 'boolean'; + + // Add event construct for modern browsers or IE + // which fires the callback with a pre-converted target reference + function addEvent(node, type, callback) { + if (node.addEventListener) { + node.addEventListener(type, function (e) { + callback(e, e.target); + }, false); + } else if (node.attachEvent) { + node.attachEvent('on' + type, function (e) { + callback(e, e.srcElement); + }); + } + } + + // Handle cross-modal click events + function addClickEvent(node, callback) { + // Prevent space(32) from scrolling the page + addEvent(node, 'keypress', function (e, target) { + if (target.nodeName === 'SUMMARY') { + if (e.keyCode === 32) { + if (e.preventDefault) { + e.preventDefault(); + } else { + e.returnValue = false; + } + } + } + }); + // When the key comes up - check if it is enter(13) or space(32) + addEvent(node, 'keyup', function (e, target) { + if (e.keyCode === 13 || e.keyCode === 32) { callback(e, target); } + }); + addEvent(node, 'mouseup', function (e, target) { + callback(e, target); + }); + } + + // Get the nearest ancestor element of a node that matches a given tag name + function getAncestor(node, match) { + do { + if (!node || node.nodeName.toLowerCase() === match) { + break; + } + } while (node = node.parentNode); + + return node; + } + + // Create a started flag so we can prevent the initialisation + // function firing from both DOMContentLoaded and window.onload + var started = false; + + // Initialisation function + function addDetailsPolyfill(list) { + + // If this has already happened, just return + // else set the flag so it doesn't happen again + if (started) { + return; + } + started = true; + + // Get the collection of details elements, but if that's empty + // then we don't need to bother with the rest of the scripting + if ((list = document.getElementsByTagName('details')).length === 0) { + return; + } + + // else iterate through them to apply their initial state + var n = list.length, i = 0; + for (i; i < n; i++) { + var details = list[i]; + + // Save shortcuts to the inner summary and content elements + details.__summary = details.getElementsByTagName('summary').item(0); + details.__content = details.getElementsByTagName('div').item(0); + + // If the content doesn't have an ID, assign it one now + // which we'll need for the summary's aria-controls assignment + if (!details.__content.id) { + details.__content.id = 'details-content-' + i; + } + + // Add ARIA role="group" to details + details.setAttribute('role', 'group'); + + // Add role=button to summary + details.__summary.setAttribute('role', 'button'); + + // Add aria-controls + details.__summary.setAttribute('aria-controls', details.__content.id); + + // Set tabIndex so the summary is keyboard accessible for non-native elements + // http://www.saliences.com/browserBugs/tabIndex.html + if (!NATIVE_DETAILS) { + details.__summary.tabIndex = 0; + } + + // Detect initial open state + var openAttr = details.getAttribute('open') !== null; + if (openAttr === true) { + details.__summary.setAttribute('aria-expanded', 'true'); + details.__content.setAttribute('aria-hidden', 'false'); + } else { + details.__summary.setAttribute('aria-expanded', 'false'); + details.__content.setAttribute('aria-hidden', 'true'); + if (!NATIVE_DETAILS) { + details.__content.style.display = 'none'; + } + } + + // Create a circular reference from the summary back to its + // parent details element, for convenience in the click handler + details.__summary.__details = details; + + // If this is not a native implementation, create an arrow + // inside the summary + if (!NATIVE_DETAILS) { + + var twisty = document.createElement('i'); + + if (openAttr === true) { + twisty.className = 'arrow arrow-open'; + twisty.appendChild(document.createTextNode('\u25bc')); + } else { + twisty.className = 'arrow arrow-closed'; + twisty.appendChild(document.createTextNode('\u25ba')); + } + + details.__summary.__twisty = details.__summary.insertBefore(twisty, details.__summary.firstChild); + details.__summary.__twisty.setAttribute('aria-hidden', 'true'); + + } + } + + // Define a statechange function that updates aria-expanded and style.display + // Also update the arrow position + function statechange(summary) { + + var expanded = summary.__details.__summary.getAttribute('aria-expanded') === 'true'; + var hidden = summary.__details.__content.getAttribute('aria-hidden') === 'true'; + + summary.__details.__summary.setAttribute('aria-expanded', (expanded ? 'false' : 'true')); + summary.__details.__content.setAttribute('aria-hidden', (hidden ? 'false' : 'true')); + + if (!NATIVE_DETAILS) { + summary.__details.__content.style.display = (expanded ? 'none' : ''); + + var hasOpenAttr = summary.__details.getAttribute('open') !== null; + if (!hasOpenAttr) { + summary.__details.setAttribute('open', 'open'); + } else { + summary.__details.removeAttribute('open'); + } + } + + if (summary.__twisty) { + summary.__twisty.firstChild.nodeValue = (expanded ? '\u25ba' : '\u25bc'); + summary.__twisty.setAttribute('class', (expanded ? 'arrow arrow-closed' : 'arrow arrow-open')); + } + + return true; + } + + // Bind a click event to handle summary elements + addClickEvent(document, function(e, summary) { + if (!(summary = getAncestor(summary, 'summary'))) { + return true; + } + return statechange(summary); + }); + } + + // Bind two load events for modern and older browsers + // If the first one fires it will set a flag to block the second one + // but if it's not supported then the second one will fire + addEvent(document, 'DOMContentLoaded', addDetailsPolyfill); + addEvent(window, 'load', addDetailsPolyfill); + +})(); diff --git a/app/assets/javascripts/dropdown.js b/app/assets/javascripts/dropdown.js deleted file mode 100644 index c5a2c8e02..000000000 --- a/app/assets/javascripts/dropdown.js +++ /dev/null @@ -1,18 +0,0 @@ -(function(Modules) { - "use strict"; - - Modules.Dropdown = function() { - - this.start = function(component) { - - $('.dropdown-toggle', component) - .on( - 'click', () => $(component).toggleClass('js-closed') - ) - .trigger('click'); - - }; - - }; - -})(window.GOVUK.Modules); diff --git a/app/assets/stylesheets/components/dropdown.scss b/app/assets/stylesheets/components/dropdown.scss index 8be3a0bd5..7f87f0c49 100644 --- a/app/assets/stylesheets/components/dropdown.scss +++ b/app/assets/stylesheets/components/dropdown.scss @@ -3,56 +3,28 @@ position: relative; - .js-enabled &-toggle { + &-toggle { color: $link-colour; position: relative; - padding-left: 20px; cursor: pointer; + outline-offset: 2px; &:hover { color: $link-hover-colour; text-decoration: underline; } - &::before { - content: '▼'; - font-size: 12px; - position: absolute; - top: 1px; - left: -1px; - } - - } - - .js-closed &-toggle::before { - content: '▶'; - left: -1px; - top: 2px; } a { display: block; - margin-top: 3px; + margin-top: 7px; + padding-left: 20px; - .js-enabled & { - - padding-left: 20px; - - &:hover { - text-decoration: underline; - } - - } - - } - - &.js-closed { - - a { - left: -9999em; - position: absolute; + &:hover { + text-decoration: underline; } } diff --git a/app/assets/stylesheets/components/navigation.scss b/app/assets/stylesheets/components/navigation.scss index ed3a62268..9ccb97e4a 100644 --- a/app/assets/stylesheets/components/navigation.scss +++ b/app/assets/stylesheets/components/navigation.scss @@ -9,9 +9,13 @@ ul { border-bottom: 1px solid $border-colour; - margin: 0 20px 0 0; + margin: 10px 20px 0 0; list-style-type: none; - padding: 0; + padding: 0 0 8px 0; + } + + li { + margin: 7px 0 0 0; } a:link, diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 37478ad6d..ed749b6e3 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -41,3 +41,10 @@ @extend .table-field; text-align: right; } + +.table-empty-message { + @include core-16; + color: $secondary-text-colour; + border-bottom: 1px solid $border-colour; + padding: 5px 0 8px 0; +} diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 1c8842d4a..419e2cca5 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -19,6 +19,6 @@ def service_dashboard(service_id): return render_template( 'views/service_dashboard.html', jobs=jobs, - free_text_messages_remaining=560, + free_text_messages_remaining='25,000', spent_this_month='0.00', service_id=service_id) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index c548cb44c..ae653d405 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -50,7 +50,7 @@ messages = [ def view_jobs(service_id): return render_template( 'views/jobs.html', - jobs=jobs, + jobs=[], # use `jobs` for placeholder data service_id=service_id ) diff --git a/app/main/views/register.py b/app/main/views/register.py index 2ca7d8faa..c2856c1e6 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -1,12 +1,21 @@ from datetime import datetime, timedelta -from flask import render_template, redirect, session +from flask import ( + render_template, + redirect, + session, + current_app, + abort +) + +from client.errors import HTTPError from app.main import main +from app.models import User from app.main.dao import users_dao from app.main.forms import RegisterUserForm -from app.models import User +from app.notify_client.user_api_client import UserApiClient # TODO how do we handle duplicate unverifed email addresses? # malicious or otherwise. @@ -18,6 +27,8 @@ def register(): form = RegisterUserForm(users_dao.get_user_by_email) if form.validate_on_submit(): + + # TODO remove once all api integrations done user = User(name=form.name.data, email_address=form.email_address.data, mobile_number=form.mobile_number.data, @@ -25,6 +36,21 @@ def register(): created_at=datetime.now(), role_id=1) users_dao.insert_user(user) + + user_api_client = UserApiClient(current_app.config['NOTIFY_API_URL'], + current_app.config['ADMIN_CLIENT_USER_NAME'], + current_app.config['ADMIN_CLIENT_SECRET']) + try: + user_api_client.register_user(form.name.data, + form.email_address.data, + form.mobile_number.data, + form.password.data) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e + # TODO possibly there should be some exception handling # for sending sms and email codes. # How do we report to the user there is a problem with diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py new file mode 100644 index 000000000..bc6a70096 --- /dev/null +++ b/app/notify_client/user_api_client.py @@ -0,0 +1,18 @@ +from client.notifications import BaseAPIClient + + +class UserApiClient(BaseAPIClient): + + def __init__(self, base_url, client_id, secret): + super(self.__class__, self).__init__(base_url=base_url, + client_id=client_id, + secret=secret) + + def register_user(self, name, email_address, mobile_number, password): + data = { + "name": name, + "email_address": email_address, + "mobile_number": mobile_number, + "password": password} + + return self.post("/user", data) diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index e009a9e60..d3ac7c6da 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -51,13 +51,13 @@ Switch to A N Other service Add a new service to GOV.UK Notify - +
{{ current_user.name }} diff --git a/app/templates/components/table.html b/app/templates/components/table.html index bfa53232d..fb41a6aa7 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -23,20 +23,21 @@ {%- endmacro %} {% macro list_table(items, caption='', empty_message='', field_headings=[], field_headings_visible=True, caption_visible=True) -%} - {% if items %} - {% set parent_caller = caller %} - {% call mapping_table(caption, field_headings, field_headings_visible, caption_visible) %} - {% for item in items %} - {% call row() %} - {{ parent_caller(item) }} - {% endcall %} - {% endfor %} - {%- endcall %} - {% else %} -

+ + {% set parent_caller = caller %} + {% call mapping_table(caption, field_headings, field_headings_visible, caption_visible) %} + {% for item in items %} + {% call row() %} + {{ parent_caller(item) }} + {% endcall %} + {% endfor %} + {%- endcall %} + {% if not items %} +

{{ empty_message }}

{% endif %} + {%- endmacro %} {% macro row() -%} diff --git a/app/templates/views/jobs.html b/app/templates/views/jobs.html index 0879fee7a..6d321c433 100644 --- a/app/templates/views/jobs.html +++ b/app/templates/views/jobs.html @@ -13,6 +13,7 @@ GOV.UK Notify | Notifications activity jobs, caption="Recent activity", caption_visible=False, + empty_message='You haven’t sent any notifications yet', field_headings=['Job', 'File', 'Time', 'Status'] ) %} {% call field() %} diff --git a/app/templates/views/service_dashboard.html b/app/templates/views/service_dashboard.html index 9e5510795..9ed330140 100644 --- a/app/templates/views/service_dashboard.html +++ b/app/templates/views/service_dashboard.html @@ -24,9 +24,9 @@ {% call(item) list_table( - jobs[:3], + [], caption="Recent text messages", - empty_message="No recent text messages", + empty_message='You haven’t sent any text messages yet', field_headings=['Job', 'File', 'Time', 'Status'] ) %} {% call field() %} @@ -42,9 +42,5 @@ {{ item.status }} {% endcall %} {% endcall %} -

- See all notifications activity -

- {% endblock %} diff --git a/app/templates/views/styleguide.html b/app/templates/views/styleguide.html index 2231b1173..40669cbbf 100644 --- a/app/templates/views/styleguide.html +++ b/app/templates/views/styleguide.html @@ -148,6 +148,21 @@ {% endcall %} {% endcall %} + {% call(item) list_table( + [], + caption='Jobs', + field_headings=['Job', 'Time'], + caption_visible=True, + empty_message='You haven’t scheduled any jobs yet' + ) %} + {% call field() %} + {{ item.job }} + {% endcall %} + {% call field() %} + {{ item.time }} + {% endcall %} + {% endcall %} +

Textbox

{{ textbox(form.username) }} {{ textbox(form.password) }} diff --git a/config.py b/config.py index 9f98b37aa..6d3fcd2d9 100644 --- a/config.py +++ b/config.py @@ -22,10 +22,13 @@ class Config(object): SESSION_COOKIE_HTTPONLY = True SESSION_COOKIE_SECURE = True - NOTIFY_API_URL = os.getenv('NOTIFY_API_URL', "http://localhost:6001") + NOTIFY_API_URL = os.getenv('NOTIFY_API_URL') NOTIFY_API_SECRET = os.getenv('NOTIFY_API_SECRET', "dev-secret") NOTIFY_API_CLIENT = os.getenv('NOTIFY_API_CLIENT', "admin") + ADMIN_CLIENT_USER_NAME = os.getenv('ADMIN_CLIENT_USER_NAME') + ADMIN_CLIENT_SECRET = os.getenv('ADMIN_CLIENT_SECRET') + WTF_CSRF_ENABLED = True SECRET_KEY = 'secret-key' HTTP_PROTOCOL = 'http' @@ -38,10 +41,12 @@ class Config(object): class Development(Config): DEBUG = True + NOTIFY_API_URL = 'http://localhost:6011' + ADMIN_CLIENT_USER_NAME = 'dev-notify-admin' + ADMIN_CLIENT_SECRET = 'dev-notify-secret-key' -class Test(Config): - DEBUG = True +class Test(Development): SQLALCHEMY_DATABASE_URI = 'postgresql://localhost/test_notifications_admin' WTF_CSRF_ENABLED = False diff --git a/gulpfile.babel.js b/gulpfile.babel.js index 5207d6138..c7427d856 100644 --- a/gulpfile.babel.js +++ b/gulpfile.babel.js @@ -34,8 +34,8 @@ gulp.task('javascripts', () => gulp .src([ paths.npm + 'govuk_frontend_toolkit/javascripts/govuk/modules.js', paths.npm + 'govuk_frontend_toolkit/javascripts/govuk/selection-buttons.js', + paths.src + 'javascripts/detailsPolyfill.js', paths.src + 'javascripts/apiKey.js', - paths.src + 'javascripts/dropdown.js', paths.src + 'javascripts/highlightTags.js', paths.src + 'javascripts/main.js' ]) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index e0d163f99..4af4b63ac 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -13,5 +13,4 @@ def test_should_show_recent_jobs_on_dashboard(app_, response = client.get(url_for('main.service_dashboard', service_id=123)) assert response.status_code == 200 - assert 'Test message 1' in response.get_data(as_text=True) - assert 'Asdfgg' in response.get_data(as_text=True) + assert 'You haven’t sent any text messages yet' in response.get_data(as_text=True) diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 039d47dbf..aa900dd8b 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -11,8 +11,7 @@ def test_should_return_list_of_all_jobs(app_, db_, db_session, service_one): response = client.get(url_for('main.view_jobs', service_id=101)) assert response.status_code == 200 - assert 'Test message 1' in response.get_data(as_text=True) - assert 'Final reminder' in response.get_data(as_text=True) + assert 'You haven’t sent any notifications yet' in response.get_data(as_text=True) def test_should_show_page_for_one_job(app_, db_, db_session, service_one): diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index e23362fcf..2a3c78536 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -1,5 +1,7 @@ from flask import url_for +from tests.conftest import mock_register_user + def test_render_register_returns_template_with_form(app_, db_, db_session): response = app_.test_client().get('/register') @@ -12,13 +14,21 @@ def test_process_register_creates_new_user(app_, db_, db_session, mock_send_sms, - mock_send_email): + mock_send_email, + mocker): + + user_data = { + 'name': 'Some One Valid', + 'email_address': 'someone@example.gov.uk', + 'mobile_number': '+4407700900460', + 'password': 'validPassword!' + } + + mock_register_user(mocker, user_data) + with app_.test_request_context(): response = app_.test_client().post('/register', - data={'name': 'Some One Valid', - 'email_address': 'someone@example.gov.uk', - 'mobile_number': '+4407700900460', - 'password': 'validPassword!'}) + data=user_data) assert response.status_code == 302 assert response.location == url_for('main.verify', _external=True) @@ -57,13 +67,19 @@ def test_should_add_verify_codes_on_session(app_, db_, db_session, mock_send_sms, - mock_send_email): + mock_send_email, + mocker): + user_data = { + 'name': 'Test Codes', + 'email_address': 'test@example.gov.uk', + 'mobile_number': '+4407700900460', + 'password': 'validPassword!' + } + + mock_register_user(mocker, user_data) with app_.test_client() as client: response = client.post('/register', - data={'name': 'Test Codes', - 'email_address': 'test_codes@example.gov.uk', - 'mobile_number': '+4407700900460', - 'password': 'validPassword!'}) + data=user_data) assert response.status_code == 302 assert 'notify_admin_session' in response.headers.get('Set-Cookie') diff --git a/tests/conftest.py b/tests/conftest.py index 48df70174..599b5670b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -199,6 +199,18 @@ def mock_delete_service_template(mocker): template_id, "Template to delete", "sms", "content to be deleted", service_id) return {'data': template} - mock_class = mocker.patch( + return mocker.patch( 'app.notifications_api_client.delete_service_template', side_effect=_delete) + + +def mock_register_user(mocker, user_data): + data = { + "email_address": user_data['email_address'], + "failed_login_count": 0, + "mobile_number": user_data['mobile_number'], + "name": user_data['name'], + "state": "pending" + } + mock_class = mocker.patch('app.main.views.register.UserApiClient') + mock_class.register_user.return_value = data return mock_class