diff --git a/app/main/views/choose_service.py b/app/main/views/choose_service.py index 39b0b9f8e..5ddaf61b0 100644 --- a/app/main/views/choose_service.py +++ b/app/main/views/choose_service.py @@ -1,13 +1,29 @@ from flask import (render_template, redirect, url_for, session) from flask_login import login_required, current_user -from app.main.dao import services_dao +from app.main.dao.services_dao import ServicesBrowsableItem +from app import service_api_client from app.main import main @main.route("/services") @login_required def choose_service(): - services = services_dao.get_services(current_user.id) return render_template( 'views/choose-service.html', - services=[services_dao.ServicesBrowsableItem(x) for x in services['data']]) + services=[ServicesBrowsableItem(x) for x in service_api_client.get_services()['data']] + ) + + +@main.route("/services-or-dashboard") +def show_all_services_or_dashboard(): + + if current_user.is_authenticated(): + + services = service_api_client.get_services()['data'] + + if 1 == len(services): + return redirect(url_for('.service_dashboard', service_id=services[0]['id'])) + else: + return redirect(url_for('.choose_service')) + + return redirect(url_for('main.index')) diff --git a/app/main/views/send.py b/app/main/views/send.py index a48d6c6f3..936be82d8 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -26,8 +26,10 @@ from app.main.uploader import ( s3download ) from app.main.dao import templates_dao -from app.main.dao import services_dao -from app import job_api_client +from app import ( + job_api_client, + service_api_client +) from app.utils import user_has_permissions, get_errors_for_csv @@ -74,7 +76,7 @@ def get_page_headings(template_type): admin_override=True, or_=True) def choose_template(service_id, template_type): - service = services_dao.get_service_by_id_or_404(service_id) + service = service_api_client.get_service(service_id)['data'] if template_type not in ['email', 'sms']: abort(404) @@ -102,6 +104,12 @@ def choose_template(service_id, template_type): @user_has_permissions('send_texts', 'send_emails', 'send_letters') def send_messages(service_id, template_id): + service = service_api_client.get_service(service_id)['data'] + template = Template( + templates_dao.get_service_template_or_404(service_id, template_id)['data'], + prefix=service['name'] + ) + form = CsvUploadForm() if form.validate_on_submit(): try: @@ -121,18 +129,13 @@ def send_messages(service_id, template_id): } return redirect(url_for('.check_messages', service_id=service_id, - upload_id=upload_id)) + upload_id=upload_id, + template_type=template.template_type)) except ValueError as e: flash('There was a problem uploading: {}'.format(form.file.data.filename)) flash(str(e)) return redirect(url_for('.send_messages', service_id=service_id, template_id=template_id)) - service = services_dao.get_service_by_id_or_404(service_id) - template = Template( - templates_dao.get_service_template_or_404(service_id, template_id)['data'], - prefix=service['name'] - ) - return render_template( 'views/send.html', template=template, @@ -188,12 +191,14 @@ def send_message_to_self(service_id, template_id): 'data': output.getvalue() } upload_id = str(uuid.uuid4()) + s3upload(upload_id, service_id, filedata, current_app.config['AWS_REGION']) session['upload_data'] = {"template_id": template_id, "original_file_name": filedata['file_name']} return redirect(url_for('.check_messages', service_id=service_id, - upload_id=upload_id)) + upload_id=upload_id, + template_type=template.template_type)) @main.route("/services//send//from-api", methods=['GET']) @@ -218,12 +223,15 @@ def send_from_api(service_id, template_id): ) -@main.route("/services//check/", methods=['GET']) +@main.route("/services///check/", methods=['GET']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') -def check_messages(service_id, upload_id): +def check_messages(service_id, template_type, upload_id): - service = services_dao.get_service_by_id_or_404(service_id) + if not session.get('upload_data'): + return redirect(url_for('main.choose_template', service_id=service_id, template_type=template_type)) + + service = service_api_client.get_service(service_id)['data'] contents = s3download(service_id, upload_id) if not contents: @@ -270,17 +278,18 @@ def check_messages(service_id, upload_id): send_button_text=get_send_button_text(template.template_type, session['upload_data']['notification_count']), service_id=service_id, service=service, + upload_id=upload_id, form=CsvUploadForm() ) -@main.route("/services//check/", methods=['POST']) +@main.route("/services//start-job/", methods=['POST']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') def start_job(service_id, upload_id): upload_data = session['upload_data'] - services_dao.get_service_by_id_or_404(service_id) + service = service_api_client.get_service(service_id)['data'] if request.files or not upload_data.get('valid'): # The csv was invalid, validate the csv again diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index f678afc40..c9aa103a8 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -31,6 +31,9 @@ def sign_in(): if form.validate_on_submit(): user = user_api_client.get_user_by_email_or_none(form.email_address.data) user = _get_and_verify_user(user, form.password.data) + if user and user.state == 'pending': + flash("You haven't verified your email or mobile number yet.") + return redirect(url_for('main.sign_in')) if user: # Remember me login if not login_fresh() and \ @@ -45,9 +48,7 @@ def sign_in(): return redirect(url_for('main.choose_service')) session['user_details'] = {"email": user.email_address, "id": user.id} - if user.state == 'pending': - return redirect(url_for('.verify')) - elif user.is_active(): + if user.is_active(): user_api_client.send_verify_code(user.id, 'sms', user.mobile_number) if request.args.get('next'): return redirect(url_for('.two_factor', next=request.args.get('next'))) diff --git a/app/main/views/verify.py b/app/main/views/verify.py index 9c701fdc0..b076bb5c6 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -6,7 +6,8 @@ from flask import ( session, url_for, current_app, - flash + flash, + abort ) from itsdangerous import SignatureExpired @@ -55,10 +56,17 @@ def verify_email(token): token_data = json.loads(token_data) verified = user_api_client.check_verify_code(token_data['user_id'], token_data['secret_code'], 'email') + user = user_api_client.get_user(token_data['user_id']) + if not user: + abort(404) + + if user.is_active(): + flash("That verification link has expired.") + return redirect(url_for('main.sign_in')) + + session['user_details'] = {"email": user.email_address, "id": user.id} if verified[0]: - user = user_api_client.get_user(token_data['user_id']) user_api_client.send_verify_code(user.id, 'sms', user.mobile_number) - session['user_details'] = {"email": user.email_address, "id": user.id} return redirect('verify') else: if verified[1] == 'Code has expired': diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 573b3f1b2..a371476db 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -59,12 +59,7 @@ {% set global_header_text = "GOV.UK Notify" %} - -{% if not current_user.is_authenticated() %} - {% set homepage_url = url_for('main.index') %} -{% else %} - {% set homepage_url = url_for('main.choose_service') %} -{% endif %} +{% set homepage_url = url_for('main.show_all_services_or_dashboard') %} {% block content %}
diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 018f0f491..0bd132ea9 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -64,7 +64,7 @@ {% if errors %} {{file_upload(form.file, button_text='Re-upload your file')}} {% else %} -
+ Back diff --git a/requirements.txt b/requirements.txt index b1c315f8d..d37ae37df 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@3.1.1#egg=notifications-utils==3.1.1 +git+https://github.com/alphagov/notifications-utils.git@3.1.3#egg=notifications-utils==3.1.3 diff --git a/tests/app/main/views/test_choose_services.py b/tests/app/main/views/test_choose_services.py index 0bf475fcf..cae274d07 100644 --- a/tests/app/main/views/test_choose_services.py +++ b/tests/app/main/views/test_choose_services.py @@ -21,6 +21,47 @@ def test_should_show_choose_services_page(app_, assert 'List all services' not in resp_data +def test_redirect_if_only_one_service( + app_, + mock_login, + mock_get_user, + api_user_active, + mock_get_services_with_one_service +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.get(url_for('main.show_all_services_or_dashboard')) + + service = mock_get_services_with_one_service.side_effect()['data'][0] + assert response.status_code == 302 + assert response.location == url_for('main.service_dashboard', service_id=service['id'], _external=True) + + +def test_redirect_if_multiple_services( + app_, + mock_login, + mock_get_user, + api_user_active, + mock_get_services +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.get(url_for('main.show_all_services_or_dashboard')) + + assert response.status_code == 302 + assert response.location == url_for('main.choose_service', _external=True) + + +def test_should_redirect_if_not_logged_in(app_): + with app_.test_request_context(): + with app_.test_client() as client: + response = client.get(url_for('main.show_all_services_or_dashboard')) + assert response.status_code == 302 + assert response.location == url_for('main.index', _external=True) + + def test_should_show_all_services_for_platform_admin_user(app_, platform_admin_user, mock_get_services, diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index d7b96c4aa..520ccc5be 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -40,7 +40,7 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors( assert 'Re-upload your file' in content -def test_send_test_message_to_self( +def test_send_test_sms_message_to_self( app_, mocker, api_user_active, @@ -51,7 +51,7 @@ def test_send_test_message_to_self( mock_has_permissions ): - expected_data = {'data': ['phone number', '+4412341234'], 'file_name': 'Test run'} + expected_data = {'data': 'phone number\r\n+4412341234\r\n', 'file_name': 'Test run'} mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234') with app_.test_request_context(): @@ -65,7 +65,7 @@ def test_send_test_message_to_self( mock_s3_upload.assert_called_with(ANY, '12345', expected_data, 'eu-west-1') -def test_send_test_message_to_self( +def test_send_test_email_message_to_self( app_, mocker, api_user_active, @@ -247,11 +247,8 @@ def test_check_messages_should_revalidate_file_when_uploading_file( 'app.main.views.send.s3download', return_value=""" phone number,name,,, - ++44 7700 900981,test1,,, - +44 7700 900981,test2,,, - ,,, - ,,, \t \t - + 123,test1,,, + 123,test2,,, """ ) with app_.test_request_context(): @@ -263,7 +260,7 @@ def test_check_messages_should_revalidate_file_when_uploading_file( 'notification_count': job_data['notification_count'], 'valid': True} response = client.post( - url_for('main.check_messages', service_id=service_id, upload_id=job_data['id']), + url_for('main.start_job', service_id=service_id, upload_id=job_data['id']), data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')}, content_type='multipart/form-data', follow_redirects=True diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index c3cc669d1..6d2a150c4 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -1,9 +1,5 @@ - -from datetime import datetime - -from app.main.dao import users_dao - from flask import url_for +from bs4 import BeautifulSoup def test_render_sign_in_returns_sign_in_template(app_): @@ -75,9 +71,11 @@ def test_should_return_redirect_when_user_is_pending(app_, response = app_.test_client().post( url_for('main.sign_in'), data={ 'email_address': 'pending_user@example.gov.uk', - 'password': 'val1dPassw0rd!'}) - assert response.status_code == 302 - assert response.location == url_for('main.verify', _external=True) + 'password': 'val1dPassw0rd!'}, follow_redirects=True) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string == 'Sign in' + flash_banner = page.find('div', class_='banner-dangerous').string.strip() + assert flash_banner == "You haven't verified your email or mobile number yet." def test_not_fresh_session_login(app_, diff --git a/tests/app/main/views/test_verify.py b/tests/app/main/views/test_verify.py index 4f756ced2..58b4f6301 100644 --- a/tests/app/main/views/test_verify.py +++ b/tests/app/main/views/test_verify.py @@ -70,18 +70,18 @@ def test_should_return_200_when_sms_code_is_wrong(app_, def test_verify_email_redirects_to_verify_if_token_valid(app_, mocker, - api_user_active, - mock_get_user, + api_user_pending, + mock_get_user_pending, mock_send_verify_code, mock_check_verify_code): import json - token_data = {"user_id": api_user_active.id, "secret_code": 12345} + token_data = {"user_id": api_user_pending.id, "secret_code": 12345} mocker.patch('utils.url_safe_token.check_token', return_value=json.dumps(token_data)) with app_.test_request_context(): with app_.test_client() as client: with client.session_transaction() as session: - session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id} + session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id} response = client.get(url_for('main.verify_email', token='notreal')) @@ -91,7 +91,7 @@ def test_verify_email_redirects_to_verify_if_token_valid(app_, def test_verify_email_redirects_to_email_sent_if_token_expired(app_, mocker, - api_user_active, + api_user_pending, mock_check_verify_code): from itsdangerous import SignatureExpired mocker.patch('utils.url_safe_token.check_token', side_effect=SignatureExpired('expired')) @@ -99,7 +99,7 @@ def test_verify_email_redirects_to_email_sent_if_token_expired(app_, with app_.test_request_context(): with app_.test_client() as client: with client.session_transaction() as session: - session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id} + session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id} response = client.get(url_for('main.verify_email', token='notreal')) @@ -109,8 +109,8 @@ def test_verify_email_redirects_to_email_sent_if_token_expired(app_, def test_verify_email_redirects_to_email_sent_if_token_used(app_, mocker, - api_user_active, - mock_get_user, + api_user_pending, + mock_get_user_pending, mock_send_verify_code, mock_check_verify_code_code_expired): from itsdangerous import SignatureExpired @@ -119,9 +119,31 @@ def test_verify_email_redirects_to_email_sent_if_token_used(app_, with app_.test_request_context(): with app_.test_client() as client: with client.session_transaction() as session: - session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id} + session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id} response = client.get(url_for('main.verify_email', token='notreal')) assert response.status_code == 302 assert response.location == url_for('main.resend_email_verification', _external=True) + + +def test_verify_email_redirects_to_sign_in_if_user_active(app_, + mocker, + api_user_active, + mock_get_user, + mock_send_verify_code, + mock_check_verify_code): + import json + token_data = {"user_id": api_user_active.id, "secret_code": 12345} + mocker.patch('utils.url_safe_token.check_token', return_value=json.dumps(token_data)) + + with app_.test_request_context(): + with app_.test_client() as client: + with client.session_transaction() as session: + session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id} + + response = client.get(url_for('main.verify_email', token='notreal'), follow_redirects=True) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.text == 'Sign in' + flash_banner = page.find('div', class_='banner-dangerous').string.strip() + assert flash_banner == "That verification link has expired."