From f34a252e7287ecb2aea192c1d4a774c2f1dcf485 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 5 Jun 2019 12:14:50 +0100 Subject: [PATCH] Remove defaults from `User` model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the api always returns exactly: ``` id name email_address auth_type current_session_id failed_login_count logged_in_at mobile_number organisations password_changed_at permissions platform_admin services state ``` it does this through `models.py::User.serialize` – there is an old Marshmallow `user_schema` in `schemas.py` but this isn’t used for dumping return data, only parsing the json in the create user rest endpoint. This means we can rely on these keys always being in the dictionary. --- app/models/user.py | 24 ++++++++--------------- tests/__init__.py | 2 ++ tests/app/main/test_permissions.py | 3 ++- tests/app/main/views/test_manage_users.py | 9 +++++---- tests/conftest.py | 5 +++++ 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/app/models/user.py b/app/models/user.py index dec7c9692..d649c6ab2 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -33,24 +33,17 @@ class User(JSONModel, UserMixin): 'id', 'name', 'email_address', - 'mobile_number', - 'password_changed_at', 'auth_type', - 'failed_login_count', - 'state', - 'logged_in_at', - 'platform_admin', 'current_session_id', - 'services', + 'failed_login_count', + 'logged_in_at', + 'mobile_number', 'organisations', - } - - DEFAULTS = { - 'current_session_id': None, - 'services': [], - 'organisations': [], - 'logged_in_at': None, - 'platform_admin': False, + 'password_changed_at', + 'permissions', + 'platform_admin', + 'services', + 'state', } def __init__(self, _dict): @@ -320,7 +313,6 @@ class InvitedUser(JSONModel): ALLOWED_PROPERTIES = { 'id', 'service', - 'from_user', 'email_address', 'permissions', 'status', diff --git a/tests/__init__.py b/tests/__init__.py index cc60347eb..0fe944c57 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -61,6 +61,7 @@ def user_json( }, auth_type='sms_auth', failed_login_count=0, + logged_in_at=None, state='active', max_failed_login_count=3, platform_admin=False, @@ -78,6 +79,7 @@ def user_json( 'permissions': permissions, 'auth_type': auth_type, 'failed_login_count': failed_login_count, + 'logged_in_at': logged_in_at or datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S.%f'), 'state': state, 'max_failed_login_count': max_failed_login_count, 'platform_admin': platform_admin, diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py index 7306e7422..b44baf833 100644 --- a/tests/app/main/test_permissions.py +++ b/tests/app/main/test_permissions.py @@ -238,7 +238,8 @@ def _user_with_permissions(): 'permissions': {'foo': ['manage_users', 'manage_templates', 'manage_settings']}, 'platform_admin': False, 'organisations': ['org_1', 'org_2'], - 'services': ['foo', 'bar'] + 'services': ['foo', 'bar'], + 'current_session_id': None, } return user_data diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 1af53f8b7..5f3468953 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -162,8 +162,8 @@ def test_should_show_caseworker_on_overview_page( service_one, ): service_one['permissions'].append('caseworking') - current_user = active_user_view_permissions(active_user_view_permissions) - other_user = active_caseworking_user(fake_uuid) + current_user = active_user_view_permissions(fake_uuid) + other_user = active_caseworking_user(uuid.uuid4()) other_user['email_address'] = 'zzzzzzz@example.gov.uk' mocker.patch('app.user_api_client.get_user', return_value=current_user) @@ -312,10 +312,11 @@ def test_user_with_no_mobile_number_cant_be_set_to_sms_auth( sms_option_disabled, expected_label, service_one, - mocker + mocker, + fake_uuid, ): service_one['permissions'].append('email_auth') - mocker.patch('app.user_api_client.get_user', return_value=user(mocker)) + mocker.patch('app.user_api_client.get_user', return_value=user(fake_uuid)) page = client_request.get( 'main.edit_user_permissions', diff --git a/tests/conftest.py b/tests/conftest.py index 0f6bcffa1..35fb3ebf1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1087,6 +1087,7 @@ def api_user_pending(fake_uuid): 'mobile_number': '07700 900762', 'state': 'pending', 'failed_login_count': 0, + 'platform_admin': False, 'permissions': {}, 'organisations': [], 'current_session_id': None, @@ -1113,6 +1114,7 @@ def platform_admin_user(fake_uuid): 'view_activity']}, 'platform_admin': True, 'auth_type': 'sms_auth', + 'services': [], 'organisations': [], 'current_session_id': None, } @@ -1132,6 +1134,7 @@ def api_user_active(fake_uuid, email_address='test@user.gov.uk'): 'platform_admin': False, 'auth_type': 'sms_auth', 'password_changed_at': str(datetime.utcnow()), + 'services': [], 'organisations': [], 'current_session_id': None, } @@ -1151,6 +1154,7 @@ def api_user_active_email_auth(fake_uuid, email_address='test@user.gov.uk'): 'platform_admin': False, 'auth_type': 'email_auth', 'password_changed_at': str(datetime.utcnow()), + 'services': [], 'organisations': [], 'current_session_id': None, } @@ -1180,6 +1184,7 @@ def api_nongov_user_active(fake_uuid): 'platform_admin': False, 'auth_type': 'sms_auth', 'password_changed_at': str(datetime.utcnow()), + 'services': [], 'organisations': [], 'current_session_id': None, }