Put organisations on the user model

As in other places, putting a model layer between the view and the API
client makes the code cleaner and clearer.

app/main/views/choose_account.py

@@ -1,7 +1,6 @@
 from flask import redirect, render_template, session, url_for
 from flask_login import current_user, login_required
 
-from app import user_api_client
 from app.main import main
 from app.utils import PermanentRedirect
 
@@ -19,11 +18,10 @@ def services_or_dashboard():
 @main.route("/accounts")
 @login_required
 def choose_account():
-    orgs_and_services = user_api_client.get_organisations_and_services_for_user(current_user)
+    orgs_and_services = current_user.orgs_and_services
 
     return render_template(
         'views/choose-account.html',
-        organisations=orgs_and_services['organisations'],
         services_without_organisations=orgs_and_services['services_without_organisations'],
         can_add_service=current_user.is_gov_user,
     )
@@ -40,13 +38,13 @@ def show_accounts_or_dashboard():
         return redirect(url_for('.service_dashboard', service_id=service_id))
 
     organisation_id = session.get('organisation_id')
-    if organisation_id and (organisation_id in current_user.organisations or current_user.platform_admin):
+    if organisation_id and (organisation_id in current_user.organisation_ids or current_user.platform_admin):
         return redirect(url_for('.organisation_dashboard', org_id=organisation_id))
 
-    if len(current_user.services) == 1 and not current_user.organisations:
+    if len(current_user.services) == 1 and not current_user.organisation_ids:
         return redirect(url_for('.service_dashboard', service_id=current_user.services[0]))
 
-    if len(current_user.organisations) == 1 and not current_user.services:
-        return redirect(url_for('.organisation_dashboard', org_id=current_user.organisations[0]))
+    if len(current_user.organisation_ids) == 1 and not current_user.services:
+        return redirect(url_for('.organisation_dashboard', org_id=current_user.organisation_ids[0]))
 
     return redirect(url_for('.choose_account'))

app/main/views/find_users.py

@@ -31,12 +31,9 @@ def find_users_by_email():
 @login_required
 @user_is_platform_admin
 def user_information(user_id):
-    user = User.from_id(user_id)
-    services = user_api_client.get_services_for_user(user)
     return render_template(
         'views/find-users/user-information.html',
-        user=user,
-        services=services,
+        user=User.from_id(user_id),
     )
 
 

app/main/views/templates.py

@@ -14,7 +14,6 @@ from app import (
     service_api_client,
     template_folder_api_client,
     template_statistics_client,
-    user_api_client,
 )
 from app.main import main
 from app.main.forms import (
@@ -370,7 +369,7 @@ def choose_template_to_copy(
             'views/templates/copy.html',
             services_templates_and_folders=TemplateLists([
                 Service(service) for service in
-                user_api_client.get_services_for_user(current_user)
+                current_user.all_services
             ], user=current_user),
             search_form=SearchByNameForm(),
         )

app/models/user.py

@@ -1,4 +1,5 @@
 from collections.abc import Sequence
+from itertools import chain
 
 from flask import abort, current_app, request, session
 from flask_login import AnonymousUserMixin, UserMixin, login_user
@@ -38,7 +39,6 @@ class User(JSONModel, UserMixin):
         'failed_login_count',
         'logged_in_at',
         'mobile_number',
-        'organisations',
         'password_changed_at',
         'permissions',
         'platform_admin',
@@ -193,7 +193,7 @@ class User(JSONModel, UserMixin):
             return True
 
         if org_id:
-            return org_id in self.organisations
+            return org_id in self.organisation_ids
         if not permissions:
             return service_id in self.services
         if service_id:
@@ -238,8 +238,15 @@ class User(JSONModel, UserMixin):
         return self.email_address.split('@')[-1]
 
     @cached_property
+    def orgs_and_services(self):
+        return user_api_client.get_organisations_and_services_for_user(self.id)
+
+    @property
     def all_services(self):
-        return user_api_client.get_services_for_user(self.id)
+        all_services = self.orgs_and_services['services_without_organisations'] + next(chain(
+            org['services'] for org in self.orgs_and_services['organisations']
+        ), [])
+        return sorted(all_services, key=lambda service: service['name'].lower())
 
     @property
     def all_service_ids(self):
@@ -261,6 +268,14 @@ class User(JSONModel, UserMixin):
             if not service['restricted']
         ]
 
+    @property
+    def organisations(self):
+        return self.orgs_and_services['organisations']
+
+    @property
+    def organisation_ids(self):
+        return self._dict['organisations']
+
     @cached_property
     def default_organisation(self):
         return Organisation(
@@ -291,7 +306,7 @@ class User(JSONModel, UserMixin):
             "state": self.state,
             "failed_login_count": self.failed_login_count,
             "permissions": [x for x in self._permissions],
-            "organisations": self.organisations,
+            "organisations": self.organisation_ids,
             "current_session_id": self.current_session_id
         }
         if hasattr(self, '_password'):

app/notify_client/user_api_client.py

@@ -1,5 +1,3 @@
-from itertools import chain
-
 from notifications_python_client.errors import HTTPError
 
 from app.models.roles_and_permissions import (
@@ -184,16 +182,9 @@ class UserApiClient(NotifyAdminAPIClient):
         data = {'email': new_email}
         self.post(endpoint, data)
 
-    def get_organisations_and_services_for_user(self, user):
-        endpoint = '/user/{}/organisations-and-services'.format(user.id)
+    def get_organisations_and_services_for_user(self, user_id):
+        endpoint = '/user/{}/organisations-and-services'.format(user_id)
         return self.get(endpoint)
 
-    def get_services_for_user(self, user):
-        orgs_and_services_for_user = self.get_organisations_and_services_for_user(user)
-        all_services = orgs_and_services_for_user['services_without_organisations'] + next(chain(
-            org['services'] for org in orgs_and_services_for_user['organisations']
-        ), [])
-        return sorted(all_services, key=lambda service: service['name'])
-
 
 user_api_client = UserApiClient()

app/templates/views/choose-account.html

@@ -17,7 +17,7 @@
         </li>
         <div class="keyline-block"></div>
       {% endif %}
-      {% for org in organisations %}
+      {% for org in current_user.organisations %}
         <li class="browse-list-item">
           <a href="{{ url_for('.organisation_dashboard', org_id=org.id) }}" class="browse-list-link">{{ org.name }}</a>
           {% if org.services %}
@@ -40,7 +40,7 @@
         {% endfor %}
         <div class="keyline-block"></div>
       {% endif %}
-      {% if not organisations %}
+      {% if not current_user.organisations %}
         {% if current_user.trial_mode_services %}
           </ul>
           <h2 class="heading-small">

app/templates/views/find-users/user-information.html

@@ -16,7 +16,7 @@
       <h2 class="heading-medium">Services</h2>
       <nav class="browse-list">
         <ul>
-        {% for service in services %}
+        {% for service in user.all_services %}
           <li class="browse-list-item">
             <a class="browse-list-hint" href={{url_for('.service_dashboard', service_id=service.id)}}>{{ service.name }}</a>
           </li>