From 151a61f7d3d5dbcf297c4b4f4b18421875b519e2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 26 Oct 2021 15:06:43 +0100 Subject: [PATCH] Fix headings on choose account page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit You’re supposed to see the two column layout on this page if you have multiple categories of things to show. We weren’t counting the ‘platform admin’ section as one of these categories so platform admin users with only live services or only trial mode services were inadvertenly seeing a mixture of the one column and two column layout. Also this logic around the headings wasn’t tested before – now it is. --- app/templates/views/choose-account.html | 10 ++-- .../views/accounts/test_choose_accounts.py | 50 +++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/app/templates/views/choose-account.html b/app/templates/views/choose-account.html index c5f3f78df..4ff59bb63 100644 --- a/app/templates/views/choose-account.html +++ b/app/templates/views/choose-account.html @@ -7,7 +7,7 @@ organisations=[], services=[] ) %} - {% if show_heading %} + {% if show_heading and (services or organisations) %}

@@ -33,7 +33,7 @@ {{ service.name }} {% endfor %} - {% if show_heading %} + {% if show_heading and (services or organisations) %}

@@ -77,14 +77,14 @@ {% if current_user.organisations %} {{ service_list( heading='Live services', - show_heading=current_user.trial_mode_services, + show_heading=current_user.trial_mode_services or current_user.platform_admin, organisations=current_user.organisations, services=current_user.live_services ) }} {% else %} {{ service_list( heading='Live services', - show_heading=(current_user.trial_mode_services and current_user.live_services), + show_heading=(current_user.trial_mode_services and current_user.live_services) or current_user.platform_admin, services=current_user.live_services ) }} {% endif %} @@ -92,7 +92,7 @@ {% if current_user.trial_mode_services %} {{ service_list( heading='Trial mode services', - show_heading=(current_user.organisations or current_user.live_services), + show_heading=(current_user.organisations or current_user.live_services or current_user.platform_admin), services=current_user.trial_mode_services ) }} {% endif %} diff --git a/tests/app/main/views/accounts/test_choose_accounts.py b/tests/app/main/views/accounts/test_choose_accounts.py index b39e25ac3..42938a5c2 100644 --- a/tests/app/main/views/accounts/test_choose_accounts.py +++ b/tests/app/main/views/accounts/test_choose_accounts.py @@ -86,9 +86,12 @@ def test_choose_account_should_show_choose_accounts_page( assert normalize_spaces(page.h1.text) == 'Choose service' outer_list_items = page.select('nav ul')[0].select('li') + headings = page.select('main h2') assert len(outer_list_items) == 8 + assert normalize_spaces(headings[0].text) == 'Live services' + # first org assert outer_list_items[0].a.text == 'Org 1' assert outer_list_items[0].a['href'] == url_for('.organisation_dashboard', org_id='o1') @@ -122,6 +125,8 @@ def test_choose_account_should_show_choose_accounts_page( assert outer_list_items[7].a.text == 'service two (org 2)' assert outer_list_items[7].a['href'] == url_for('.service_dashboard', service_id='67890') + assert normalize_spaces(headings[1].text) == 'Trial mode services' + # trial services trial_services_list_items = page.select('nav ul')[1].select('li') assert len(trial_services_list_items) == 3 @@ -151,9 +156,47 @@ def test_choose_account_should_show_choose_accounts_page_if_no_services( add_service_link = links[0] assert normalize_spaces(page.h1.text) == 'Choose service' assert normalize_spaces(add_service_link.text) == 'Add a new service' + assert not page.select('main h2') assert add_service_link['href'] == url_for('main.add_service') +@pytest.mark.parametrize('orgs_and_services, expected_headings', ( + ({ + 'organisations': [], + 'services': [] + }, [ + 'Platform admin', + ]), + (SAMPLE_DATA, [ + 'Platform admin', + 'Live services', + 'Trial mode services', + ]), + ({ + 'organisations': [], + 'services': [{ + 'name': 'Live service', + 'id': OS2, + 'restricted': False, + 'organisation': None, + }], + }, [ + 'Platform admin', + 'Live services', + ]), + ({ + 'organisations': [], + 'services': [{ + 'name': 'Trial service', + 'id': OS2, + 'restricted': True, + 'organisation': None, + }], + }, [ + 'Platform admin', + 'Trial mode services', + ]), +)) def test_choose_account_should_should_organisations_link_for_platform_admin( client_request, platform_admin_user, @@ -161,7 +204,10 @@ def test_choose_account_should_should_organisations_link_for_platform_admin( mock_get_orgs_and_services, mock_get_organisation_services, mock_get_service_and_organisation_counts, + orgs_and_services, + expected_headings, ): + mock_get_orgs_and_services.return_value = orgs_and_services client_request.login(platform_admin_user) page = client_request.get('main.choose_account') @@ -173,6 +219,10 @@ def test_choose_account_should_should_organisations_link_for_platform_admin( assert first_link['href'] == url_for('main.organisations') assert normalize_spaces(first_hint.text) == '3 organisations, 9,999 live services' + assert [ + normalize_spaces(h2.text) for h2 in page.select('main h2') + ] == expected_headings + def test_choose_account_should_show_back_to_service_link( client_request,