Merge pull request #3840 from alphagov/accounts-page-fix

Fix /accounts page to only show trial services once

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 [

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(

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'

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 [