From 913095fe607ada523d7ce031d376493462777fcd Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 12 Jun 2019 13:40:57 +0100
Subject: [PATCH] Wrap a model around list of all organisations

---
 app/main/views/organisations.py               |  5 ++--
 app/models/__init__.py                        | 28 +++++++++++++++++++
 app/models/organisation.py                    |  7 ++++-
 app/models/user.py                            | 19 +++----------
 app/templates/views/organisations/index.html  |  4 +--
 .../views/organisations/test_organisation.py  |  2 +-
 6 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py
index dcf81dd62..44d4c068d 100644
--- a/app/main/views/organisations.py
+++ b/app/main/views/organisations.py
@@ -31,6 +31,7 @@ from app.main.forms import (
     SetLetterBranding,
 )
 from app.main.views.service_settings import get_branding_as_value_and_label
+from app.models.organisation import Organisations
 from app.models.user import InvitedOrgUser, User
 from app.utils import user_has_permissions, user_is_platform_admin
 
@@ -39,11 +40,9 @@ from app.utils import user_has_permissions, user_is_platform_admin
 @login_required
 @user_is_platform_admin
 def organisations():
-    orgs = organisations_client.get_organisations()
-
     return render_template(
         'views/organisations/index.html',
-        organisations=orgs,
+        organisations=Organisations(),
         search_form=SearchByNameForm(),
     )
 
diff --git a/app/models/__init__.py b/app/models/__init__.py
index 520918223..7675d4a81 100644
--- a/app/models/__init__.py
+++ b/app/models/__init__.py
@@ -1,3 +1,6 @@
+from abc import ABC, abstractmethod
+from collections.abc import Sequence
+
 from flask import abort
 
 
@@ -33,5 +36,30 @@ class JSONModel():
             abort(404)
 
 
+class ModelList(ABC, Sequence):
+
+    @property
+    @abstractmethod
+    def client():
+        pass
+
+    @property
+    @abstractmethod
+    def model():
+        pass
+
+    def __init__(self):
+        self.items = self.client()
+
+    def __getitem__(self, index):
+        return self.model(self.items[index])
+
+    def __len__(self):
+        return len(self.items)
+
+    def __add__(self, other):
+        return list(self) + list(other)
+
+
 class InviteTokenError(Exception):
     pass
diff --git a/app/models/organisation.py b/app/models/organisation.py
index 1fee293b2..b0c10f863 100644
--- a/app/models/organisation.py
+++ b/app/models/organisation.py
@@ -1,7 +1,7 @@
 from flask import Markup, abort
 from werkzeug.utils import cached_property
 
-from app.models import JSONModel
+from app.models import JSONModel, ModelList
 from app.notify_client.organisations_api_client import organisations_client
 
 
@@ -155,3 +155,8 @@ class Organisation(JSONModel):
             self.invited_users + self.active_users,
             key=lambda user: user.email_address.lower(),
         )
+
+
+class Organisations(ModelList):
+    client = organisations_client.get_organisations
+    model = Organisation
diff --git a/app/models/user.py b/app/models/user.py
index 6119f5391..9b1be251a 100644
--- a/app/models/user.py
+++ b/app/models/user.py
@@ -1,11 +1,9 @@
-from collections.abc import Sequence
-
 from flask import abort, current_app, request, session
 from flask_login import AnonymousUserMixin, UserMixin, login_user
 from notifications_python_client.errors import HTTPError
 from werkzeug.utils import cached_property
 
-from app.models import JSONModel
+from app.models import JSONModel, ModelList
 from app.models.organisation import Organisation
 from app.models.roles_and_permissions import (
     all_permissions,
@@ -590,22 +588,13 @@ class AnonymousUser(AnonymousUserMixin):
         return Organisation(None)
 
 
-class Users(Sequence):
+class Users(ModelList):
 
     client = user_api_client.get_users_for_service
     model = User
 
     def __init__(self, service_id):
-        self.users = self.client(service_id)
-
-    def __getitem__(self, index):
-        return self.model(self.users[index])
-
-    def __len__(self):
-        return len(self.users)
-
-    def __add__(self, other):
-        return list(self) + list(other)
+        self.items = self.client(service_id)
 
 
 class OrganisationUsers(Users):
@@ -618,7 +607,7 @@ class InvitedUsers(Users):
     model = InvitedUser
 
     def __init__(self, service_id):
-        self.users = [
+        self.items = [
             user for user in self.client(service_id)
             if user['status'] != 'accepted'
         ]
diff --git a/app/templates/views/organisations/index.html b/app/templates/views/organisations/index.html
index e52677366..00a228459 100644
--- a/app/templates/views/organisations/index.html
+++ b/app/templates/views/organisations/index.html
@@ -32,8 +32,8 @@
             <ul>
               {% for org in organisations %}
                 <li class="browse-list-item">
-                  <a href="{{ url_for('main.organisation_dashboard', org_id=org['id']) }}" class="browse-list-link">{{ org['name'] }}</a>
-                  {% if not org['active'] %}
+                  <a href="{{ url_for('main.organisation_dashboard', org_id=org.id) }}" class="browse-list-link">{{ org.name }}</a>
+                  {% if not org.active %}
                     <span class="table-field-status-default heading-medium">- archived</span>
                   {% endif %}
                 </li>
diff --git a/tests/app/main/views/organisations/test_organisation.py b/tests/app/main/views/organisations/test_organisation.py
index 831487e0a..c72c34154 100644
--- a/tests/app/main/views/organisations/test_organisation.py
+++ b/tests/app/main/views/organisations/test_organisation.py
@@ -26,7 +26,7 @@ def test_organisation_page_shows_all_organisations(
     ]
 
     mocker.patch(
-        'app.organisations_client.get_organisations', return_value=orgs
+        'app.models.organisation.Organisations.client', return_value=orgs
     )
     response = logged_in_platform_admin_client.get(
         url_for('.organisations')