From 63ba3a6f30eccf0b42f9a517eb48b74f580f7dd8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 7 Jun 2019 09:26:11 +0100 Subject: [PATCH] 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 | 12 ++++------ app/main/views/find_users.py | 5 +--- app/main/views/templates.py | 3 +-- app/models/user.py | 23 +++++++++++++++---- app/notify_client/user_api_client.py | 13 ++--------- app/templates/views/choose-account.html | 4 ++-- .../views/find-users/user-information.html | 2 +- 7 files changed, 31 insertions(+), 31 deletions(-) diff --git a/app/main/views/choose_account.py b/app/main/views/choose_account.py index 7af74bc0b..b14dca2e4 100644 --- a/app/main/views/choose_account.py +++ b/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')) diff --git a/app/main/views/find_users.py b/app/main/views/find_users.py index 3d04eb42e..ed364b954 100644 --- a/app/main/views/find_users.py +++ b/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), ) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index e9a751190..1a3cd01ba 100644 --- a/app/main/views/templates.py +++ b/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(), ) diff --git a/app/models/user.py b/app/models/user.py index 8421c3878..d0c326273 100644 --- a/app/models/user.py +++ b/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'): diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 7d13935de..ababbe5b5 100644 --- a/app/notify_client/user_api_client.py +++ b/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() diff --git a/app/templates/views/choose-account.html b/app/templates/views/choose-account.html index bbe5ec629..b96856045 100644 --- a/app/templates/views/choose-account.html +++ b/app/templates/views/choose-account.html @@ -17,7 +17,7 @@
{% endif %} - {% for org in organisations %} + {% for org in current_user.organisations %}
  • {{ org.name }} {% if org.services %} @@ -40,7 +40,7 @@ {% endfor %}
    {% endif %} - {% if not organisations %} + {% if not current_user.organisations %} {% if current_user.trial_mode_services %}

    diff --git a/app/templates/views/find-users/user-information.html b/app/templates/views/find-users/user-information.html index 1f89d2f89..1e8d8ba9e 100644 --- a/app/templates/views/find-users/user-information.html +++ b/app/templates/views/find-users/user-information.html @@ -16,7 +16,7 @@

    Services