diff --git a/app/main/__init__.py b/app/main/__init__.py index ec9691d24..39f95b71d 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -3,7 +3,24 @@ from flask import Blueprint main = Blueprint('main', __name__) from app.main.views import ( - index, sign_in, sign_out, register, two_factor, verify, send, add_service, - code_not_received, jobs, dashboard, templates, service_settings, forgot_password, - new_password, styleguide, user_profile, choose_service, api_keys, manage_users + index, + sign_in, + sign_out, + register, + two_factor, + verify, + send, + add_service, + code_not_received, + jobs, dashboard, + templates, + service_settings, + forgot_password, + new_password, + styleguide, + user_profile, + choose_service, + api_keys, + manage_users, + invites ) diff --git a/app/main/views/invites.py b/app/main/views/invites.py new file mode 100644 index 000000000..c9052303d --- /dev/null +++ b/app/main/views/invites.py @@ -0,0 +1,34 @@ +from flask import ( + redirect, + url_for, + abort +) + +from notifications_python_client.errors import HTTPError + +from app.main import main +from app import ( + invite_api_client, + user_api_client +) + + +@main.route("/invitation/") +def accept_invite(token): + try: + invited_user = invite_api_client.accept_invite(token) + existing_user = user_api_client.get_user_by_email(invited_user.email_address) + + if existing_user: + user_api_client.add_user_to_service(invited_user.service, + existing_user.id) + + return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) + else: + # TODO implement registration flow for new users + abort(404) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 0b5f0e7f0..f5d83bfd6 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -58,8 +58,8 @@ def invite_user(service_id): email_address = form.email_address.data permissions = _get_permissions(request.form) try: - resp = invite_api_client.create_invite(current_user.id, service_id, email_address, permissions) - flash('Invite sent to {}'.format(resp['email_address']), 'default_with_tick') + invited_user = invite_api_client.create_invite(current_user.id, service_id, email_address, permissions) + flash('Invite sent to {}'.format(invited_user.email_address), 'default_with_tick') return redirect(url_for('.manage_users', service_id=service_id)) except HTTPError as e: diff --git a/app/main/views/send.py b/app/main/views/send.py index c22d1fb08..37415b54b 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -28,6 +28,7 @@ from app.main.dao import services_dao from app import job_api_client from app.utils import ( validate_recipient, InvalidPhoneError, InvalidEmailError, user_has_permissions) +from utils.process_csv import first_column_heading manage_service_page_headings = { @@ -115,11 +116,12 @@ def send_messages(service_id, template_id): templates_dao.get_service_template_or_404(service_id, template_id)['data'], prefix=service['name'] ) + recipient_column = first_column_heading[template.template_type] return render_template( 'views/send.html', template=template, - column_headers=['to'] + template.placeholders_as_markup, + recipient_column=first_column_heading[template.template_type], form=form, service=service, service_id=service_id @@ -130,17 +132,19 @@ def send_messages(service_id, template_id): @login_required @user_has_permissions('send_messages', 'manage_templates', or_=True) def get_example_csv(service_id, template_id): - template = templates_dao.get_service_template_or_404(service_id, template_id)['data'] - placeholders = list(Template(template).placeholders) + template = Template(templates_dao.get_service_template_or_404(service_id, template_id)['data']) output = io.StringIO() writer = csv.writer(output) - writer.writerow(['to'] + placeholders) + writer.writerow( + [first_column_heading[template.template_type]] + + list(template.placeholders) + ) writer.writerow([ { 'email': current_user.email_address, 'sms': current_user.mobile_number - }[template['template_type']] - ] + ["test {}".format(header) for header in placeholders]) + }[template.template_type] + ] + ["test {}".format(header) for header in template.placeholders]) return output.getvalue(), 200, {'Content-Type': 'text/csv; charset=utf-8'} @@ -148,12 +152,17 @@ def get_example_csv(service_id, template_id): @login_required @user_has_permissions('send_messages') def send_message_to_self(service_id, template_id): - template = templates_dao.get_service_template_or_404(service_id, template_id)['data'] - placeholders = list(Template(template).placeholders) + template = Template(templates_dao.get_service_template_or_404(service_id, template_id)['data']) output = io.StringIO() writer = csv.writer(output) - writer.writerow(['to'] + placeholders) - writer.writerow([current_user.mobile_number] + ["test {}".format(header) for header in placeholders]) + writer.writerow( + [first_column_heading[template.template_type]] + + list(template.placeholders) + ) + writer.writerow( + [current_user.mobile_number] + + ["test {}".format(header) for header in template.placeholders] + ) filedata = { 'file_name': 'Test run', 'data': output.getvalue().splitlines() @@ -187,7 +196,7 @@ def check_messages(service_id, upload_id): template = Template( raw_template, values=upload_result['rows'][0] if upload_result['valid'] else {}, - drop_values={'to'}, + drop_values={first_column_heading[raw_template['template_type']]}, prefix=service['name'] ) return render_template( @@ -195,7 +204,7 @@ def check_messages(service_id, upload_id): upload_result=upload_result, template=template, page_heading=get_page_headings(template.template_type), - column_headers=['to'] + list(template.placeholders_as_markup), + column_headers=[first_column_heading[template.template_type]] + list(template.placeholders_as_markup), original_file_name=upload_data.get('original_file_name'), service_id=service_id, service=service, @@ -257,10 +266,13 @@ def _get_rows(contents, raw_template): rows.append(row) try: validate_recipient( - row.get('to', ''), - template_type=raw_template['template_type'] + row, template_type=raw_template['template_type'] ) - Template(raw_template, values=row, drop_values={'to'}).replaced + Template( + raw_template, + values=row, + drop_values={first_column_heading[raw_template['template_type']]} + ).replaced except (InvalidEmailError, InvalidPhoneError, NeededByTemplateError, NoPlaceholderForDataError): valid = False return {"valid": valid, "rows": rows} diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index 1dffed425..d1acd5f6d 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -1,6 +1,5 @@ - from notifications_python_client.base import BaseAPIClient -from app.notify_client.models import User +from app.notify_client.models import InvitedUser class InviteApiClient(BaseAPIClient): @@ -22,9 +21,23 @@ class InviteApiClient(BaseAPIClient): 'permissions': permissions } resp = self.post(url='/service/{}/invite'.format(service_id), data=data) - return resp['data'] + return InvitedUser(**resp['data']) def get_invites_for_service(self, service_id): endpoint = '/service/{}/invite'.format(service_id) resp = self.get(endpoint) - return [User(data) for data in resp['data']] + invites = resp['data'] + invited_users = _get_invited_users(invites) + return invited_users + + def accept_invite(self, token): + resp = self.get(url='/invite/{}'.format(token)) + return InvitedUser(**resp['data']) + + +def _get_invited_users(invites): + invited_users = [] + for invite in invites: + invited_user = InvitedUser(**invite) + invited_users.append(invited_user) + return invited_users diff --git a/app/notify_client/models.py b/app/notify_client/models.py index 483ead66f..3cf6497b6 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -114,3 +114,18 @@ class User(UserMixin): def set_password(self, pwd): self._password = pwd + + +class InvitedUser(object): + + def __init__(self, id, service, from_user, email_address, permissions, status, created_at): + self.id = id + self.service = str(service) + self.from_user = from_user + self.email_address = email_address + self.permissions = permissions.split(',') + self.status = status + self.created_at = created_at + + def has_permissions(self, permission): + return permission in self.permissions diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 179a7d4f8..d6a4ad574 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -1,3 +1,5 @@ +import json + from notifications_python_client.notifications import BaseAPIClient from notifications_python_client.errors import HTTPError @@ -85,3 +87,8 @@ class UserApiClient(BaseAPIClient): endpoint = '/service/{}/users'.format(service_id) resp = self.get(endpoint) return [User(data) for data in resp['data']] + + def add_user_to_service(self, service_id, user_id): + endpoint = '/service/{}/users/{}'.format(service_id, user_id) + resp = self.post(endpoint, data={}) + return User(resp['data'], max_failed_login_count=self.max_failed_login_count) diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 315bc32eb..d60bef88f 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -61,13 +61,17 @@ caption=original_file_name, field_headings=column_headers ) %} - {% if item.to or ''|valid_phone_number %} + {% if item.get('phone number', '')|valid_phone_number %} {% call field() %} - {{ item.to }} + {{ item['phone number'] }} + {% endcall %} + {% elif item.get('email address') %} + {% call field() %} + {{ item['email address'] }} {% endcall %} {% else %} {% call field(status='missing') %} - {{ item.to }} + {{ item['phone number'] }} {% endcall %} {% endif %} {% for column in template.placeholders %} diff --git a/app/templates/views/job.html b/app/templates/views/job.html index ae60fed13..416da439e 100644 --- a/app/templates/views/job.html +++ b/app/templates/views/job.html @@ -3,6 +3,7 @@ {% from "components/big-number.html" import big_number %} {% from "components/banner.html" import banner %} {% from "components/sms-message.html" import sms_message %} +{% from "components/email-message.html" import email_message %} {% block page_title %} {{ uploaded_file_name }} – GOV.UK Notify diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index 02d3d817e..44a0b39e5 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -43,9 +43,9 @@ Manage users – GOV.UK Notify {% call field() %} {{ item.email_address }} {% endcall %} - {{ boolean_field(item.has_permissions(service_id, 'send_messages')) }} - {{ boolean_field(item.has_permissions(service_id, 'manage_service')) }} - {{ boolean_field(item.has_permissions(service_id, 'api_keys')) }} + {{ boolean_field(item.has_permissions('send_messages')) }} + {{ boolean_field(item.has_permissions('manage_service')) }} + {{ boolean_field(item.has_permissions('manage_api_keys')) }} {% call field(align='right') %} Change {% endcall %} diff --git a/app/templates/views/send.html b/app/templates/views/send.html index 7b0dcf027..2d62b9f2a 100644 --- a/app/templates/views/send.html +++ b/app/templates/views/send.html @@ -40,7 +40,7 @@ {% endif %}

- to + {{ recipient_column }} {{ template.placeholders_as_markup|join(" ") }}

diff --git a/app/utils.py b/app/utils.py index 280b052e1..965939028 100644 --- a/app/utils.py +++ b/app/utils.py @@ -3,6 +3,8 @@ import re from functools import wraps from flask import (abort, session) +from utils.process_csv import get_recipient_from_row + class BrowsableItem(object): """ @@ -87,11 +89,11 @@ def validate_email_address(email_address): raise InvalidEmailError('Not a valid email address') -def validate_recipient(recipient, template_type): +def validate_recipient(row, template_type): return { 'email': validate_email_address, 'sms': validate_phone_number - }[template_type](recipient) + }[template_type](get_recipient_from_row(row, template_type)) def user_has_permissions(*permissions, or_=False): diff --git a/requirements.txt b/requirements.txt index 7fbf848cc..7a0048d08 100644 --- a/requirements.txt +++ b/requirements.txt @@ -14,4 +14,4 @@ Pygments==2.0.2 git+https://github.com/alphagov/notifications-python-client.git@0.3.1#egg=notifications-python-client==0.3.1 -git+https://github.com/alphagov/notifications-utils.git@0.1.2#egg=notifications-utils==0.1.2 +git+https://github.com/alphagov/notifications-utils.git@1.0.0#egg=notifications-utils==1.0.0 diff --git a/tests/__init__.py b/tests/__init__.py index 4792e8c7e..57e3984fb 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -48,12 +48,15 @@ def api_key_json(id_, name, expiry_date=None): } -def invite_json(id, from_user, service_id, email_address): +def invite_json(id, from_user, service_id, email_address, permissions, created_at): return {'id': id, 'from_user': from_user, 'service': service_id, 'email_address': email_address, - 'status': 'pending'} + 'status': 'pending', + 'permissions': permissions, + 'created_at': created_at + } TEST_USER_EMAIL = 'test@user.gov.uk' diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py new file mode 100644 index 000000000..54b527d93 --- /dev/null +++ b/tests/app/main/views/test_accept_invite.py @@ -0,0 +1,30 @@ +from flask import url_for + +from app.notify_client.models import InvitedUser +from notifications_python_client.errors import HTTPError + +import pytest + + +def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(app_, + service_one, + api_user_active, + sample_invite, + mock_accept_invite, + mock_get_user_by_email, + mock_add_user_to_service): + + expected_service = service_one['id'] + expected_redirect_location = 'http://localhost/services/{}/dashboard'.format(expected_service) + + with app_.test_request_context(): + with app_.test_client() as client: + + response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + + mock_accept_invite.assert_called_with('thisisnotarealtoken') + mock_get_user_by_email.assert_called_with('invited_user@test.gov.uk') + mock_add_user_to_service.assert_called_with(expected_service, api_user_active.id) + + assert response.status_code == 302 + assert response.location == expected_redirect_location diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 4af60ee7c..438e3060e 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -3,22 +3,22 @@ from flask import url_for from bs4 import BeautifulSoup -# def test_should_show_overview_page( -# app_, -# api_user_active, -# mock_login, -# mock_get_service, -# mock_get_users_by_service, -# mock_get_invites_for_service -# ): -# with app_.test_request_context(): -# with app_.test_client() as client: -# client.login(api_user_active) -# response = client.get(url_for('main.manage_users', service_id=55555)) -# -# assert 'Manage team' in response.get_data(as_text=True) -# assert response.status_code == 200 -# mock_get_users_by_service.assert_called_once_with(service_id='55555') +def test_should_show_overview_page( + app_, + api_user_active, + mock_login, + mock_get_service, + mock_get_users_by_service, + mock_get_invites_for_service +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.get(url_for('main.manage_users', service_id=55555)) + + assert 'Manage team' in response.get_data(as_text=True) + assert response.status_code == 200 + mock_get_users_by_service.assert_called_once_with(service_id='55555') def test_should_show_page_for_one_user( @@ -70,37 +70,37 @@ def test_should_show_page_for_inviting_user( assert 'Add a new team member' in response.get_data(as_text=True) assert response.status_code == 200 -# -# def test_invite_user( -# app_, -# service_one, -# api_user_active, -# mock_login, -# mock_get_users_by_service, -# mock_create_invite, -# mock_get_invites_for_service -# ): -# from_user = api_user_active.id -# service_id = service_one['id'] -# email_address = 'test@example.gov.uk' -# permissions = 'send_messages,manage_service,manage_api_keys' -# -# with app_.test_request_context(): -# with app_.test_client() as client: -# client.login(api_user_active) -# response = client.post( -# url_for('main.invite_user', service_id=service_id), -# data={'email_address': email_address, -# 'send_messages': 'yes', -# 'manage_service': 'yes', -# 'manage_api_keys': 'yes'}, -# follow_redirects=True -# ) -# -# assert response.status_code == 200 -# mock_create_invite.assert_called_with(from_user, service_id, email_address, permissions) -# mock_get_invites_for_service.assert_called_with(service_id=service_id) -# page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') -# assert page.h1.string.strip() == 'Manage team' -# flash_banner = page.find('div', class_='banner-default-with-tick').string.strip() -# assert flash_banner == 'Invite sent to test@example.gov.uk' + +def test_invite_user( + app_, + service_one, + api_user_active, + mock_login, + mock_get_users_by_service, + mock_create_invite, + mock_get_invites_for_service +): + from_user = api_user_active.id + service_id = service_one['id'] + email_address = 'test@example.gov.uk' + permissions = 'send_messages,manage_service,manage_api_keys' + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.post( + url_for('main.invite_user', service_id=service_id), + data={'email_address': email_address, + 'send_messages': 'yes', + 'manage_service': 'yes', + 'manage_api_keys': 'yes'}, + follow_redirects=True + ) + + assert response.status_code == 200 + mock_create_invite.assert_called_with(from_user, service_id, email_address, permissions) + mock_get_invites_for_service.assert_called_with(service_id=service_id) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Manage team' + flash_banner = page.find('div', class_='banner-default-with-tick').string.strip() + assert flash_banner == 'Invite sent to test@example.gov.uk' diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 28116626c..1c132bf5c 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -70,7 +70,7 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors( mock_has_permissions ): - contents = 'to,name\n+44 123,test1\n+44 456,test2' + contents = 'phone number,name\n+44 123,test1\n+44 456,test2' file_data = (BytesIO(contents.encode('utf-8')), 'invalid.csv') mocker.patch('app.main.views.send.s3download', return_value=contents) @@ -102,14 +102,14 @@ def test_upload_csvfile_removes_empty_lines_and_trailing_commas( mock_has_permissions ): - contents = 'to,name,,,\n++44 7700 900981,test1,,,\n+44 7700 900981,test2,,,\n ,,, \n ,,, \t \t \n' + contents = 'phone number,name,,,\n++44 7700 900981,test1,,,\n+44 7700 900981,test2,,,\n ,,, \n ,,, \t \t \n' file_data = (BytesIO(contents.encode('utf-8')), 'invalid.csv') - expected_data = {'data': ['to,name', '++44 7700 900981,test1', '+44 7700 900981,test2'], + expected_data = {'data': ['phone number,name', '++44 7700 900981,test1', '+44 7700 900981,test2'], 'file_name': 'invalid.csv'} mocker.patch('app.main.views.send.s3download', - return_value='to,name\n++44 7700 900981,test1\n+44 7700 900981,test2') + return_value='phone number,name\n++44 7700 900981,test1\n+44 7700 900981,test2') with app_.test_request_context(): with app_.test_client() as client: @@ -135,8 +135,8 @@ def test_send_test_message_to_self( mock_has_permissions ): - expected_data = {'data': ['to', '+4412341234'], 'file_name': 'Test run'} - mocker.patch('app.main.views.send.s3download', return_value='to\r\n+4412341234') + expected_data = {'data': ['phone number', '+4412341234'], 'file_name': 'Test run'} + mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234') with app_.test_request_context(): with app_.test_client() as client: @@ -167,7 +167,7 @@ def test_download_example_csv( follow_redirects=True ) assert response.status_code == 200 - assert response.get_data(as_text=True) == 'to\r\n+4412341234\r\n' + assert response.get_data(as_text=True) == 'phone number\r\n+4412341234\r\n' assert 'text/csv' in response.headers['Content-Type'] @@ -182,7 +182,7 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( mock_has_permissions ): - contents = 'to\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986' # noqa + contents = 'phone number\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986' # noqa file_data = (BytesIO(contents.encode('utf-8')), 'valid.csv') mocker.patch('app.main.views.send.s3download', return_value=contents) diff --git a/tests/conftest.py b/tests/conftest.py index 9dfadcd13..81c159cf2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,7 +12,10 @@ from . import ( job_json, invite_json ) -from app.notify_client.models import User +from app.notify_client.models import ( + User, + InvitedUser +) @pytest.fixture(scope='session') @@ -551,27 +554,53 @@ def mock_s3_upload(mocker): @pytest.fixture(scope='function') -def mock_create_invite(mocker): +def sample_invite(mocker, service_one): + import datetime + id = str(uuid.uuid4()) + from_user = service_one['users'][0] + email_address = 'invited_user@test.gov.uk' + service_id = service_one['id'] + permissions = 'send_messages,manage_service,manage_api_keys' + created_at = datetime.datetime.now() + return invite_json(id, from_user, service_id, email_address, permissions, created_at) + + +@pytest.fixture(scope='function') +def mock_create_invite(mocker, sample_invite): + def _create_invite(from_user, service_id, email_address, permissions): - data = {'id': uuid.uuid4(), - 'from_user': from_user, - 'service': service_id, - 'email_address': email_address, - 'status': 'pending', - 'permissions': permissions} - return data + sample_invite['from_user'] = from_user + sample_invite['service'] = service_id + sample_invite['email_address'] = email_address + sample_invite['status'] = 'pending' + sample_invite['permissions'] = permissions + return InvitedUser(**sample_invite) return mocker.patch('app.invite_api_client.create_invite', side_effect=_create_invite) @pytest.fixture(scope='function') -def mock_get_invites_for_service(mocker, service_one): +def mock_get_invites_for_service(mocker, service_one, sample_invite): + import copy + def _get_invites(service_id): data = [] - from_user = service_one['users'][0] - service_id = service_one['id'] for i in range(0, 5): - email_address = 'user_{}@testnotify.gov.uk'.format(i) - invite = invite_json(uuid.uuid4(), from_user, service_id, email_address) - data.append(invite) + invite = copy.copy(sample_invite) + invite['email_address'] = 'user_{}@testnotify.gov.uk'.format(i) + data.append(InvitedUser(**invite)) return data return mocker.patch('app.invite_api_client.get_invites_for_service', side_effect=_get_invites) + + +@pytest.fixture(scope='function') +def mock_accept_invite(mocker, sample_invite): + def _accept_token(token): + return InvitedUser(**sample_invite) + return mocker.patch('app.invite_api_client.accept_invite', side_effect=_accept_token) + + +@pytest.fixture(scope='function') +def mock_add_user_to_service(mocker, service_one, api_user_active): + def _add_user(service_id, user_id): + return api_user_active + return mocker.patch('app.user_api_client.add_user_to_service', side_effect=_add_user)