From 44e126f09ceb97fd7c8c1f755d74403549cd08f1 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Mon, 14 Mar 2016 10:49:11 +0000 Subject: [PATCH 1/7] added message if not templates and doesn't have ability to add templates. --- app/templates/views/choose-template.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/templates/views/choose-template.html b/app/templates/views/choose-template.html index e3d11ed02..3742638be 100644 --- a/app/templates/views/choose-template.html +++ b/app/templates/views/choose-template.html @@ -16,6 +16,8 @@ {% if current_user.has_permissions(['manage_templates']) %} Add a new template + {% else %} +
You need to ask your service manager to add templates before you can send messages
{% endif %} {% else %} From 9827c838793d6df2686ff853211b3e17c4b86425 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 15 Mar 2016 08:33:51 +0000 Subject: [PATCH 2/7] Revert "Stub out the send letters page" Reverts alphagov/notifications-admin#201 --- app/main/views/send.py | 7 ------- app/templates/main_nav.html | 1 - app/templates/views/letters.html | 21 --------------------- 3 files changed, 29 deletions(-) delete mode 100644 app/templates/views/letters.html diff --git a/app/main/views/send.py b/app/main/views/send.py index 72ee60a8c..4fa3ec0e0 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -63,13 +63,6 @@ def get_page_headings(template_type): return manage_templates_page_headings[template_type] -@main.route("/services//send/letters", methods=['GET']) -def letters_stub(service_id): - return render_template( - 'views/letters.html', service_id=service_id - ) - - @main.route("/services//send/", methods=['GET']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', or_=True) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index bafcfae2e..f1be21634 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -12,7 +12,6 @@ {% endif %} {% if current_user.has_permissions(['manage_users', 'manage_settings']) %} diff --git a/app/templates/views/letters.html b/app/templates/views/letters.html deleted file mode 100644 index 5ef3e4d6f..000000000 --- a/app/templates/views/letters.html +++ /dev/null @@ -1,21 +0,0 @@ -{% extends "withnav_template.html" %} - -{% block page_title %} - Send letters – GOV.UK Notify -{% endblock %} - -{% block maincolumn_content %} - -

- {% if current_user.has_permissions(['send_letters']) %} - Send letters - {% else %} - Letter templates - {% endif %} -

- -

- This page is where you would go to send letters. -

- -{% endblock %} From a23b55c258fc424fb5b8f869c2dd02461cde4bc6 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 15 Mar 2016 09:06:22 +0000 Subject: [PATCH 3/7] Changed div container to p tag. --- app/templates/views/choose-template.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/choose-template.html b/app/templates/views/choose-template.html index 3742638be..2ffffb959 100644 --- a/app/templates/views/choose-template.html +++ b/app/templates/views/choose-template.html @@ -17,7 +17,7 @@ {% if current_user.has_permissions(['manage_templates']) %} Add a new template {% else %} -
You need to ask your service manager to add templates before you can send messages
+

You need to ask your service manager to add templates before you can send messages

{% endif %} {% else %} From 5ae582f9be58e96b9e18f85dbd8a492f8dc0c093 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 15 Mar 2016 12:02:03 +0000 Subject: [PATCH 4/7] Remove reference to deleted endpoint --- app/templates/main_nav.html | 1 - tests/app/main/views/test_dashboard.py | 3 --- 2 files changed, 4 deletions(-) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index f1be21634..96c1c372b 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -6,7 +6,6 @@ {% elif current_user.has_permissions(['manage_templates']) %}
    diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 4fb92a2b5..0902f6293 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -47,7 +47,6 @@ def test_menu_send_messages(mocker, app_, api_user_active, service_one, mock_get service_one, ['send_texts', 'send_emails', 'send_letters']) page = resp.get_data(as_text=True) - assert url_for('main.letters_stub', service_id=service_one['id']) in page assert url_for( 'main.choose_template', service_id=service_one['id'], @@ -73,7 +72,6 @@ def test_menu_manage_service(mocker, app_, api_user_active, service_one, mock_ge service_one, ['manage_users', 'manage_templates', 'manage_settings']) page = resp.get_data(as_text=True) - assert url_for('main.letters_stub', service_id=service_one['id'])in page assert url_for( 'main.choose_template', service_id=service_one['id'], @@ -99,7 +97,6 @@ def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_g service_one, ['manage_api_keys', 'access_developer_docs']) page = resp.get_data(as_text=True) - assert url_for('main.letters_stub', service_id=service_one['id']) not in page assert url_for( 'main.choose_template', service_id=service_one['id'], From 7dca13407c3f4a34fb47ce9e7028549e49ff9837 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Tue, 15 Mar 2016 15:32:30 +0000 Subject: [PATCH 5/7] Additional check needed to see if user was already a user for the service that they were invited to. --- app/main/views/invites.py | 22 +++++++++++------ tests/app/main/views/test_accept_invite.py | 28 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 0bc7ad3c5..251416fd0 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -30,15 +30,23 @@ def accept_invite(token): flash('You have already accepted this invitation', 'default') return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) - existing_user = user_api_client.get_user_by_email(invited_user.email_address) session['invited_user'] = invited_user.serialize() - if existing_user: + existing_user = user_api_client.get_user_by_email(invited_user.email_address) - user_api_client.add_user_to_service(invited_user.service, - existing_user.id, - invited_user.permissions) - invite_api_client.accept_invite(invited_user.service, invited_user.id) - return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) + service_users = user_api_client.get_users_for_service(invited_user.service) + + if existing_user: + if existing_user in service_users: + session.pop('invited_user', None) + flash('You have already accepted an invitation to this service', 'default') + invite_api_client.accept_invite(invited_user.service, invited_user.id) + return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) + else: + user_api_client.add_user_to_service(invited_user.service, + existing_user.id, + invited_user.permissions) + invite_api_client.accept_invite(invited_user.service, invited_user.id) + return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) else: return redirect(url_for('main.register_from_invite')) diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index b74142682..f656e6bc1 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -15,6 +15,7 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(app_, sample_invite, mock_check_invite_token, mock_get_user_by_email, + mock_get_users_by_service, mock_add_user_to_service, mock_accept_invite): @@ -43,6 +44,7 @@ def test_existing_user_with_no_permissions_accept_invite(app_, sample_invite, mock_check_invite_token, mock_get_user_by_email, + mock_get_users_by_service, mock_add_user_to_service): expected_service = service_one['id'] @@ -78,12 +80,35 @@ def test_existing_user_cant_accept_twice(app_, assert flash_banners[0].text.strip() == 'You have already accepted this invitation' +def test_existing_of_service_get_message_that_they_are_already_part_of_service(app_, + mocker, + api_user_active, + sample_invite, + mock_get_user_by_email, + mock_accept_invite): + sample_invite['email_address'] = api_user_active.email_address + invite = InvitedUser(**sample_invite) + mocker.patch('app.invite_api_client.check_token', return_value=invite) + mocker.patch('app.user_api_client.get_users_for_service', return_value=[api_user_active]) + + with app_.test_request_context(): + with app_.test_client() as client: + response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Sign in' + flash_banners = page.find_all('div', class_='banner-default') + assert len(flash_banners) == 2 + assert flash_banners[0].text.strip() == 'You have already accepted an invitation to this service' + + def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(app_, service_one, api_user_active, sample_invite, mock_check_invite_token, mock_get_user_by_email, + mock_get_users_by_service, mock_add_user_to_service, mock_accept_invite): @@ -113,6 +138,7 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, mock_check_invite_token, mock_dont_get_user_by_email, mock_add_user_to_service, + mock_get_users_by_service, mock_accept_invite): expected_redirect_location = 'http://localhost/register-from-invite' @@ -134,6 +160,7 @@ def test_new_user_accept_invite_calls_api_and_views_registration_page(app_, mock_check_invite_token, mock_dont_get_user_by_email, mock_add_user_to_service, + mock_get_users_by_service, mock_accept_invite): with app_.test_request_context(): @@ -189,6 +216,7 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(a mock_dont_get_user_by_email, mock_register_user, mock_send_verify_code, + mock_get_users_by_service, mock_add_user_to_service, mock_accept_invite): From 4adbcebc6f38f45ef100b0d2dfb4ae3aae0f683c Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Tue, 15 Mar 2016 16:58:26 +0000 Subject: [PATCH 6/7] Do not send email in case of invite. The user does not have to validate the email token, but it was still being sent. --- app/main/views/register.py | 7 ++++--- tests/app/main/views/test_accept_invite.py | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/main/views/register.py b/app/main/views/register.py index f9d0fa47c..2893fe8b6 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -49,7 +49,7 @@ def register_from_invite(): if form.validate_on_submit(): if form.service.data != invited_user['service'] or form.email_address.data != invited_user['email_address']: abort(400) - registered = _do_registration(form) + registered = _do_registration(form, send_email=False) if registered: return redirect(url_for('main.verify')) else: @@ -61,7 +61,7 @@ def register_from_invite(): return render_template('views/register-from-invite.html', email_address=invited_user['email_address'], form=form) -def _do_registration(form, service=None): +def _do_registration(form, service=None, send_email=True): if users_dao.is_email_unique(form.email_address.data): user = user_api_client.register_user(form.name.data, form.email_address.data, @@ -74,7 +74,8 @@ def _do_registration(form, service=None): # sending codes apart from service unavailable? # at the moment i believe http 500 is fine. users_dao.send_verify_code(user.id, 'sms', user.mobile_number) - users_dao.send_verify_code(user.id, 'email', user.email_address) + if send_email: + users_dao.send_verify_code(user.id, 'email', user.email_address) session['expiry_date'] = str(datetime.now() + timedelta(hours=1)) session['user_details'] = {"email": user.email_address, "id": user.id} return True diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index f656e6bc1..e38f150ec 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -212,6 +212,7 @@ def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(app_, service_one, sample_invite, + api_user_active, mock_check_invite_token, mock_dont_get_user_by_email, mock_register_user, @@ -250,6 +251,9 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(a assert response.status_code == 302 assert response.location == expected_redirect_location + from unittest.mock import ANY + mock_send_verify_code.assert_called_once_with(ANY, 'sms', data['mobile_number']) + mock_register_user.assert_called_with(data['name'], data['email_address'], data['mobile_number'], From 271e194e1cd173c401ab639890dca953b8df0e29 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 16 Mar 2016 14:19:41 +0000 Subject: [PATCH 7/7] Should show the sent password reset link page when the user is not found. --- app/main/views/forgot_password.py | 10 ++++++++-- tests/app/main/views/test_forgot_password.py | 21 +++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index f63d8227f..88744b954 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -1,6 +1,7 @@ from flask import ( render_template, ) +from notifications_python_client.errors import HTTPError from app.main import main from app.main.forms import ForgotPasswordForm @@ -11,8 +12,13 @@ from app import user_api_client def forgot_password(): form = ForgotPasswordForm() if form.validate_on_submit(): - user_api_client.send_reset_password_url(form.email_address.data) - + try: + user_api_client.send_reset_password_url(form.email_address.data) + except HTTPError as e: + if e.status_code == 404: + return render_template('views/password-reset-sent.html') + else: + raise e return render_template('views/password-reset-sent.html') return render_template('views/forgot-password.html', form=form) diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index 721c2d53c..7f45ce6c2 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -1,4 +1,5 @@ -from flask import url_for +from flask import url_for, Response +from notifications_python_client.errors import HTTPError import app @@ -25,3 +26,21 @@ def test_should_redirect_to_password_reset_sent_for_valid_email( 'You have been sent an email containing a link' ' to reset your password.') in response.get_data(as_text=True) app.user_api_client.send_reset_password_url.assert_called_once_with(api_user_active.email_address) + + +def test_should_redirect_to_password_reset_sent_for_missing_email( + app_, + api_user_active, + mocker): + with app_.test_request_context(): + + mocker.patch('app.user_api_client.send_reset_password_url', side_effect=HTTPError(Response(status=404), + 'Not found')) + response = app_.test_client().post( + url_for('.forgot_password'), + data={'email_address': api_user_active.email_address}) + assert response.status_code == 200 + assert ( + 'You have been sent an email containing a link' + ' to reset your password.') in response.get_data(as_text=True) + app.user_api_client.send_reset_password_url.assert_called_once_with(api_user_active.email_address)