From eb279c88d44238d19bb0fe940a77388b68b1fec0 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 29 Mar 2016 10:39:01 +0100 Subject: [PATCH] =?UTF-8?q?Only=20show=20=E2=80=98Choose=20service?= =?UTF-8?q?=E2=80=99=20if=20multiple=20services?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user clicks ‘GOV.UK Notify’ in the header, they should, by default, be redirected to the dashboard for their service. They should only see the ‘Choose service’ page if they have multiple services. This also allows some logic to be factored out of the template, so one route now handles all this redirection. In the future we might want to keep the last-used service in the session, and always redirect to that. But for now, this should fix most of the confusion for first-time users. --- app/main/views/choose_service.py | 22 +++++++++-- app/templates/admin_template.html | 7 +--- tests/app/main/views/test_choose_services.py | 41 ++++++++++++++++++++ 3 files changed, 61 insertions(+), 9 deletions(-) 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/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/tests/app/main/views/test_choose_services.py b/tests/app/main/views/test_choose_services.py index 02b0845a5..7f99c2e26 100644 --- a/tests/app/main/views/test_choose_services.py +++ b/tests/app/main/views/test_choose_services.py @@ -23,6 +23,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,