Extend user client to count users with permission

One of the things we need to know for a service to go live is whether
they have at least two users with the ‘manage service’ permission.

So this commit adds a method to the client to count how many users have
a given permission. We can do logic on this count later. But having the
counting done in the client feels like a cleaner separation of concerns.

Meant some refactoring of the way `service_id` is extracted from the
request, in order to make it easier to mock.

app/notify_client/models.py

@@ -1,6 +1,6 @@
 from itertools import chain
 from flask_login import UserMixin, AnonymousUserMixin
-from flask import session
+from flask import request, session
 
 
 roles = {
@@ -13,6 +13,10 @@ roles = {
 all_permissions = set(chain.from_iterable(roles.values())) | {'view_activity'}
 
 
+def _get_service_id_from_view_args():
+    return request.view_args.get('service_id', None)
+
+
 class User(UserMixin):
     def __init__(self, fields, max_failed_login_count=3):
         self._id = fields.get('id')
@@ -117,9 +121,8 @@ class User(UserMixin):
         if admin_override and not permissions:
             return False
 
-        from flask import request
         # Service id is always set on the request for service specific views.
-        service_id = request.view_args.get('service_id', None)
+        service_id = _get_service_id_from_view_args()
         if service_id in self._permissions:
             if any_:
                 return any([x in self._permissions[service_id] for x in permissions])

app/notify_client/user_api_client.py

@@ -131,6 +131,12 @@ class UserApiClient(NotifyAdminAPIClient):
         resp = self.get(endpoint)
         return [User(data) for data in resp['data']]
 
+    def get_count_of_users_with_permission(self, service_id, permission):
+        return len([
+            user for user in self.get_users_for_service(service_id)
+            if user.has_permissions(permission, any_=True)
+        ])
+
     def add_user_to_service(self, service_id, user_id, permissions):
         endpoint = '/service/{}/users/{}'.format(service_id, user_id)
         data = [{'permission': x} for x in permissions]

tests/app/notify_client/test_user_client.py

@@ -1,6 +1,7 @@
 import pytest
-
+from unittest.mock import call
 from app import user_api_client
+from app.notify_client.models import User
 from tests.conftest import SERVICE_ONE_ID
 
 
@@ -24,6 +25,47 @@ def test_client_gets_all_users_for_service(
     assert users[0].id == fake_uuid
 
 
+def test_client_returns_count_of_users_with_manage_service(
+    app_,
+    client,
+    mocker,
+    fake_uuid,
+):
+
+    def _service_one_user_with_permissions(*permissions):
+        return User({'permissions': {SERVICE_ONE_ID: list(permissions)}})
+
+    mock_get_users = mocker.patch(
+        'app.notify_client.user_api_client.UserApiClient.get_users_for_service',
+        return_value=[
+            _service_one_user_with_permissions('manage_settings', 'view_activity'),
+            _service_one_user_with_permissions('manage_settings'),
+            _service_one_user_with_permissions('view_activity'),
+            _service_one_user_with_permissions('manage_templates'),
+        ]
+    )
+
+    mocker.patch(
+        'app.notify_client.models._get_service_id_from_view_args',
+        return_value=SERVICE_ONE_ID,
+    )
+
+    assert user_api_client.get_count_of_users_with_permission(
+        SERVICE_ONE_ID,
+        'manage_settings'
+    ) == 2
+
+    assert user_api_client.get_count_of_users_with_permission(
+        SERVICE_ONE_ID,
+        'manage_templates'
+    ) == 1
+
+    assert mock_get_users.call_args_list == [
+        call(SERVICE_ONE_ID),
+        call(SERVICE_ONE_ID)
+    ]
+
+
 def test_client_uses_correct_find_by_email(mocker, api_user_active):
 
     expected_url = '/user/email'