From 939954cd64eb515115ab136500f37658f853c0ce Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 5 Feb 2016 14:25:48 +0000 Subject: [PATCH] =?UTF-8?q?Skip=20=E2=80=98choose=20service=E2=80=99=20pag?= =?UTF-8?q?e=20if=20user=20has=20one=20service?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We used to do this by redirecting on the choose service page. However when we lost the dropdown and this page also became the page for adding a new service (in 3617f2e936aadbdf71d582e58d88a16629ee5ca2) the redirect was removed. This commit re-adds the redirect on the two factor page, so that it only happens on first login. So the flows are: **Multiple services** ``` `Sign in` → `Enter two factor code` → `Choose service` → `Service dashboard` ``` **One service** ``` `Sign in` → `Enter two factor code` → `Service dashboard` ``` **No services (you’ve deleted all your services)** `Sign in` → `Enter two factor code` → `Choose service` → `Add new service` --- app/main/views/two_factor.py | 8 ++++-- tests/app/main/views/test_two_factor.py | 37 +++++++++++++++++++++---- tests/conftest.py | 19 +++++++++++-- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index 84f55c8df..5a7af0112 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -5,7 +5,7 @@ from flask import ( from flask_login import login_user from app.main import main -from app.main.dao import users_dao +from app.main.dao import users_dao, services_dao from app.main.forms import TwoFactorForm @@ -22,6 +22,7 @@ def two_factor(): if form.validate_on_submit(): try: user = users_dao.get_user_by_id(user_id) + services = services_dao.get_services(user_id).get('data', []) # Check if coming from new password page if 'password' in session['user_details']: user.set_password(session['user_details']['password']) @@ -29,6 +30,9 @@ def two_factor(): login_user(user) finally: del session['user_details'] - return redirect(url_for('main.choose_service')) + if (len(services) == 1): + return redirect(url_for('main.service_dashboard', service_id=services[0]['id'])) + else: + return redirect(url_for('main.choose_service')) return render_template('views/two-factor.html', form=form) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 274306923..964842482 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -19,11 +19,35 @@ def test_should_render_two_factor_page(app_, assert '''We've sent you a text message with a verification code.''' in response.get_data(as_text=True) -def test_should_login_user_and_redirect_to_dashboard(app_, - api_user_active, - mock_get_user, - mock_get_user_by_email, - mock_check_verify_code): +def test_should_login_user_and_redirect_to_service_dashboard(app_, + api_user_active, + mock_get_user, + mock_get_user_by_email, + mock_check_verify_code, + mock_get_services_with_one_service): + with app_.test_request_context(): + with app_.test_client() as client: + with client.session_transaction() as session: + session['user_details'] = { + 'id': api_user_active.id, + 'email': api_user_active.email_address} + response = client.post(url_for('main.two_factor'), + data={'sms_code': '12345'}) + + assert response.status_code == 302 + assert response.location == url_for( + 'main.service_dashboard', + service_id="596364a0-858e-42c8-9062-a8fe822260eb", + _external=True + ) + + +def test_should_login_user_and_redirect_to_choose_services(app_, + api_user_active, + mock_get_user, + mock_get_user_by_email, + mock_check_verify_code, + mock_get_services): with app_.test_request_context(): with app_.test_client() as client: with client.session_transaction() as session: @@ -57,7 +81,8 @@ def test_should_login_user_when_multiple_valid_codes_exist(app_, api_user_active, mock_get_user, mock_get_user_by_email, - mock_check_verify_code): + mock_check_verify_code, + mock_get_services_with_one_service): with app_.test_request_context(): with app_.test_client() as client: with client.session_transaction() as session: diff --git a/tests/conftest.py b/tests/conftest.py index b4829be78..9c3182036 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -92,15 +92,30 @@ def mock_get_services(mocker, user=None): def _create(user_id=None): import uuid service_one = service_json( - uuid.uuid4(), "service_one", [user.id], 1000, True, False) + "596364a0-858e-42c8-9062-a8fe822260eb", "service_one", [user.id], 1000, True, False) service_two = service_json( - uuid.uuid4(), "service_two", [user.id], 1000, True, False) + "147ad62a-2951-4fa1-9ca0-093cd1a52c52", "service_two", [user.id], 1000, True, False) return {'data': [service_one, service_two]} return mocker.patch( 'app.notifications_api_client.get_services', side_effect=_create) +@pytest.fixture(scope='function') +def mock_get_services_with_one_service(mocker, user=None): + if user is None: + user = api_user_active() + + def _create(user_id=None): + import uuid + return {'data': [service_json( + "596364a0-858e-42c8-9062-a8fe822260eb", "service_one", [user.id], 1000, True, False + )]} + + return mocker.patch( + 'app.notifications_api_client.get_services', side_effect=_create) + + @pytest.fixture(scope='function') def mock_delete_service(mocker, mock_get_service): def _delete(service_id):