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/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/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/app/main/views/send.py b/app/main/views/send.py index e95de47a2..aab52a6cf 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -64,13 +64,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..96c1c372b 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -6,13 +6,11 @@ {% elif current_user.has_permissions(['manage_templates']) %} {% endif %} {% if current_user.has_permissions(['manage_users', 'manage_settings']) %} diff --git a/app/templates/views/choose-template.html b/app/templates/views/choose-template.html index cc6781caa..a5d28fd88 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 %} 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 %} diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index b74142682..e38f150ec 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(): @@ -185,10 +212,12 @@ 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, mock_send_verify_code, + mock_get_users_by_service, mock_add_user_to_service, mock_accept_invite): @@ -222,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'], 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'], 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)