From a33b6e0a0de75a9e81973a368caedbc8c8a12094 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 16 Mar 2021 15:34:35 +0000 Subject: [PATCH 1/2] Fix /accounts page to only show trial services once The `/accounts` page was listing trial mode services twice if a user belonged to an org. They were shown under both the 'Live services' and 'Trial mode services' sections. After this change, 'Live services' will show all live services (whether or not they belong to an org) and 'Trial mode services' will show all trial mode services. If a user belongs to an org, they will also see the summary of how many services per org at the top of the page. A couple of services in tests were renamed for clarity. --- app/templates/views/choose-account.html | 2 +- .../main/views/accounts/test_choose_accounts.py | 16 +++++++++++----- tests/conftest.py | 4 ++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/templates/views/choose-account.html b/app/templates/views/choose-account.html index 87b593d99..c5f3f78df 100644 --- a/app/templates/views/choose-account.html +++ b/app/templates/views/choose-account.html @@ -79,7 +79,7 @@ heading='Live services', show_heading=current_user.trial_mode_services, organisations=current_user.organisations, - services=current_user.live_services_not_belonging_to_users_organisations + services=current_user.live_services ) }} {% else %} {{ service_list( diff --git a/tests/app/main/views/accounts/test_choose_accounts.py b/tests/app/main/views/accounts/test_choose_accounts.py index ec6150896..b39e25ac3 100644 --- a/tests/app/main/views/accounts/test_choose_accounts.py +++ b/tests/app/main/views/accounts/test_choose_accounts.py @@ -87,7 +87,7 @@ 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') - assert len(outer_list_items) == 7 + assert len(outer_list_items) == 8 # first org assert outer_list_items[0].a.text == 'Org 1' @@ -110,13 +110,19 @@ def test_choose_account_should_show_choose_accounts_page( '0 live services' ) - # orphaned live services + # live services assert outer_list_items[3].a.text == 'Service 1' assert outer_list_items[3].a['href'] == url_for('.service_dashboard', service_id=SERVICE_TWO_ID) - assert outer_list_items[4].a.text == 'service one' - assert outer_list_items[4].a['href'] == url_for('.service_dashboard', service_id='12345') + assert outer_list_items[4].a.text == 'Service 2' + assert outer_list_items[4].a['href'] == url_for('.service_dashboard', service_id=SERVICE_TWO_ID) + assert outer_list_items[5].a.text == 'service one' + assert outer_list_items[5].a['href'] == url_for('.service_dashboard', service_id='12345') + assert outer_list_items[6].a.text == 'service one (org 2)' + assert outer_list_items[6].a['href'] == url_for('.service_dashboard', service_id='12345') + 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') - # orphaned trial services + # trial services trial_services_list_items = page.select('nav ul')[1].select('li') assert len(trial_services_list_items) == 3 assert trial_services_list_items[0].a.text == 'service three' diff --git a/tests/conftest.py b/tests/conftest.py index 1f6ea450e..64d2163c6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3447,8 +3447,8 @@ def _get_organisation_services(organisation_id): ] if organisation_id == 'o2': return [ - service_json('12345', 'service one', restricted=False), - service_json('67890', 'service two', restricted=False), + service_json('12345', 'service one (org 2)', restricted=False), + service_json('67890', 'service two (org 2)', restricted=False), service_json('abcde', 'service three'), ] return [ From 82733d615fc51bb97d26950c5df3b052015bbbb9 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 16 Mar 2021 15:45:21 +0000 Subject: [PATCH 2/2] Delete User properties which are now no longer used --- app/models/user.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/app/models/user.py b/app/models/user.py index bce03d7a9..52f13ae7c 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -289,13 +289,6 @@ class User(JSONModel, UserMixin): if self.belongs_to_organisation(service.organisation_id) ] - @property - def services_without_organisations(self): - return [ - service for service in self.services - if not self.belongs_to_organisation(service.organisation_id) - ] - @property def service_ids(self): return self._dict['services'] @@ -312,12 +305,6 @@ class User(JSONModel, UserMixin): service for service in self.services if service.live ] - @property - def live_services_not_belonging_to_users_organisations(self): - return self.sort_services( - set(self.live_services).union(self.services_without_organisations) - ) - @property def organisations(self): return [