darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-08-19 05:59:44 -04:00
Code Issues Packages Projects Releases Wiki Activity

Make user API client return JSON, not a model

Browse Source 
The data flow of other bits of our application looks like this:
```
                         API (returns JSON)
                                  ⬇
          API client (returns a built in type, usually `dict`)
                                  ⬇
          Model (returns an instance, eg of type `Service`)
                                  ⬇
                         View (returns HTML)
```
The user API client was architected weirdly, in that it returned a model
directly, like this:

```
                         API (returns JSON)
                                  ⬇
    API client (returns a model, of type `User`, `InvitedUser`, etc)
                                  ⬇
                         View (returns HTML)
```

This mixing of different layers of the application is bad because it
makes it hard to write model code that doesn’t have circular
dependencies. As our application gets more complicated we will be
relying more on models to manage this complexity, so we should make it
easy, not hard to write them.

It also means that most of our mocking was of the User model, not just
the underlying JSON. So it would have been easy to introduce subtle bugs
to the user model, because it wasn’t being comprehensively tested. A lot
of the changed lines of code in this commit mean changing the tests to
mock only the JSON, which means that the model layer gets implicitly
tested.

For those reasons this commit changes the user API client to return
JSON, not an instance of `User` or other models.
This commit is contained in:
Chris Hill-Scott
2019-05-23 15:27:35 +01:00
parent d66cabf300
commit 628e344b36
56 changed files with 961 additions and 872 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
app/notify_client/__init__.py
View File

@@ -60,3 +60,7 @@ class NotifyAdminAPIClient(BaseAPIClient):
def delete(self, *args, **kwargs):
self.check_inactive_service()
return super().delete(*args, **kwargs)
class InviteTokenError(Exception):
pass

15
app/notify_client/invite_api_client.py
View File

@@ -1,5 +1,4 @@
from app.models.user import (
InvitedUser,
from app.models.roles_and_permissions import (
roles,
translate_permissions_from_admin_roles_to_db,
)
@@ -31,16 +30,9 @@ class InviteApiClient(NotifyAdminAPIClient):
}
data = _attach_current_user(data)
resp = self.post(url='/service/{}/invite'.format(service_id), data=data)
return InvitedUser(**resp['data'])
return resp['data']
def get_invites_for_service(self, service_id):
return [
InvitedUser(**invite)
for invite in self._get_invites_for_service(service_id)
if invite['status'] != 'accepted'
]
def _get_invites_for_service(self, service_id):
return self.get(
'/service/{}/invite'.format(service_id)
)['data']
@@ -54,8 +46,7 @@ class InviteApiClient(NotifyAdminAPIClient):
])
def check_token(self, token):
resp = self.get(url='/invite/service/{}'.format(token))
return InvitedUser(**resp['data'])
return self.get(url='/invite/service/{}'.format(token))['data']
def cancel_invited_user(self, service_id, invited_user_id):
data = {'status': 'cancelled'}

16
app/notify_client/org_invite_api_client.py
View File

@@ -1,4 +1,3 @@
from app.models.user import InvitedOrgUser
from app.notify_client import NotifyAdminAPIClient, _attach_current_user
@@ -17,18 +16,16 @@ class OrgInviteApiClient(NotifyAdminAPIClient):
}
data = _attach_current_user(data)
resp = self.post(url='/organisation/{}/invite'.format(org_id), data=data)
return InvitedOrgUser(**resp['data'])
return resp['data']
def get_invites_for_organisation(self, org_id):
endpoint = '/organisation/{}/invite'.format(org_id)
resp = self.get(endpoint)
invites = resp['data']
invited_users = self._get_invited_org_users(invites)
return invited_users
return resp['data']
def check_token(self, token):
resp = self.get(url='/invite/organisation/{}'.format(token))
return InvitedOrgUser(**resp['data'])
return resp['data']
def cancel_invited_user(self, org_id, invited_user_id):
data = {'status': 'cancelled'}
@@ -41,12 +38,5 @@ class OrgInviteApiClient(NotifyAdminAPIClient):
self.post(url='/organisation/{0}/invite/{1}'.format(org_id, invited_user_id),
data=data)
def _get_invited_org_users(self, invites):
invited_users = []
for invite in invites:
invited_user = InvitedOrgUser(**invite)
invited_users.append(invited_user)
return invited_users
org_invite_api_client = OrgInviteApiClient()

56
app/notify_client/user_api_client.py
View File

@@ -2,9 +2,7 @@ from itertools import chain
from notifications_python_client.errors import HTTPError
from app.models.user import (
User,
roles,
from app.models.roles_and_permissions import (
translate_permissions_from_admin_roles_to_db,
)
from app.notify_client import NotifyAdminAPIClient, cache
@@ -22,8 +20,6 @@ class UserApiClient(NotifyAdminAPIClient):
def init_app(self, app):
super().init_app(app)
self.max_failed_login_count = app.config["MAX_FAILED_LOGIN_COUNT"]
self.admin_url = app.config['ADMIN_BASE_URL']
def register_user(self, name, email_address, mobile_number, password, auth_type):
@@ -35,10 +31,10 @@ class UserApiClient(NotifyAdminAPIClient):
"auth_type": auth_type
}
user_data = self.post("/user", data)
return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
return user_data['data']
def get_user(self, user_id):
return User(self._get_user(user_id)['data'], max_failed_login_count=self.max_failed_login_count)
return self._get_user(user_id)['data']
@cache.set('user-{user_id}')
def _get_user(self, user_id):
@@ -46,7 +42,7 @@ class UserApiClient(NotifyAdminAPIClient):
def get_user_by_email(self, email_address):
user_data = self.get('/user/email', params={'email': email_address})
return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
return user_data['data']
def get_user_by_email_or_none(self, email_address):
try:
@@ -54,13 +50,7 @@ class UserApiClient(NotifyAdminAPIClient):
except HTTPError as e:
if e.status_code == 404:
return None
def get_users(self):
users_data = self.get("/user")['data']
users = []
for user in users_data:
users.append(User(user, max_failed_login_count=self.max_failed_login_count))
return users
raise e
@cache.delete('user-{user_id}')
def update_user_attribute(self, user_id, **kwargs):
@@ -73,20 +63,20 @@ class UserApiClient(NotifyAdminAPIClient):
url = "/user/{}".format(user_id)
user_data = self.post(url, data=data)
return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
return user_data['data']
@cache.delete('user-{user_id}')
def reset_failed_login_count(self, user_id):
url = "/user/{}/reset-failed-login-count".format(user_id)
user_data = self.post(url, data={})
return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
return user_data['data']
@cache.delete('user-{user_id}')
def update_password(self, user_id, password):
data = {"_password": password}
url = "/user/{}/update-password".format(user_id)
user_data = self.post(url, data=data)
return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
return user_data['data']
@cache.delete('user-{user_id}')
def verify_password(self, user_id, password):
@@ -132,21 +122,11 @@ class UserApiClient(NotifyAdminAPIClient):
def get_users_for_service(self, service_id):
endpoint = '/service/{}/users'.format(service_id)
resp = self.get(endpoint)
return [User(data) for data in resp['data']]
def get_count_of_users_with_permission(self, service_id, permission):
if permission not in roles.keys():
raise TypeError('{} is not a valid permission'.format(permission))
return len([
user for user in self.get_users_for_service(service_id)
if user.has_permission_for_service(service_id, permission)
])
return self.get(endpoint)['data']
def get_users_for_organisation(self, org_id):
endpoint = '/organisations/{}/users'.format(org_id)
resp = self.get(endpoint)
return [User(data) for data in resp['data']]
return self.get(endpoint)['data']
@cache.delete('service-{service_id}')
@cache.delete('service-{service_id}-template-folders')
@@ -164,7 +144,7 @@ class UserApiClient(NotifyAdminAPIClient):
@cache.delete('user-{user_id}')
def add_user_to_organisation(self, org_id, user_id):
resp = self.post('/organisations/{}/users/{}'.format(org_id, user_id), data={})
return User(resp['data'], max_failed_login_count=self.max_failed_login_count)
return resp['data']
@cache.delete('service-{service_id}-template-folders')
@cache.delete('user-{user_id}')
@@ -191,20 +171,8 @@ class UserApiClient(NotifyAdminAPIClient):
users = self.post(endpoint, data=data)
return users
def is_email_already_in_use(self, email_address):
if self.get_user_by_email_or_none(email_address):
return True
return False
def activate_user(self, user):
if user.state == 'pending':
user_data = self._activate_user(user.id)
return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
else:
return user
@cache.delete('user-{user_id}')
def _activate_user(self, user_id):
def activate_user(self, user_id):
return self.post("/user/{}/activate".format(user_id), data=None)
def send_change_email_verification(self, user_id, new_email):