TT - 112 Update role field (#251)

* TT-112 feat: support role field as list

* TT-112 feat: add feature toggle

* TT-112 feat: add unit test, update role contains expected props

* TT-112 feat: add unit testfor users endpoint under feature toggle

* TT-112 feat: fix broken test

* TT-112 fix: update role field value

* TT-112 feat: apply comments of PR

* TT-112 feat: update var name of feature manager

Author: Angeluz-07

tests/time_tracker_api/users/users_namespace_test.py

@@ -6,20 +6,68 @@
 from pytest import mark
 
 
-def test_users_response_contains_expected_props(
-    client: FlaskClient, valid_header: dict,
+@patch(
+    'commons.feature_toggles.feature_toggle_manager.FeatureToggleManager.get_azure_app_configuration_client'
+)
+@patch(
+    'commons.feature_toggles.feature_toggle_manager.FeatureToggleManager.is_toggle_enabled_for_user'
+)
+@patch('utils.azure_users.AzureConnection.users')
+@patch('utils.azure_users.AzureConnection.users_v2')
+def test_feature_toggle_is_on_then_role_field_is_list(
+    users_v2_mock,
+    users_mock,
+    is_toggle_enabled_for_user_mock,
+    get_azure_app_configuration_client_mock,
+    client: FlaskClient,
+    valid_header: dict,
 ):
 
-    AzureConnection.users = Mock(
-        return_value=[{'name': 'dummy', 'email': 'dummy', 'role': 'dummy'}]
-    )
+    is_toggle_enabled_for_user_mock.return_value = True
+    users_v2_mock.return_value = [
+        {'name': 'dummy', 'email': 'dummy', 'roles': ['dummy-role']}
+    ]
+    response = client.get('/users', headers=valid_header)
+
+    users_v2_mock.assert_called()
+    users_mock.assert_not_called()
+    assert HTTPStatus.OK == response.status_code
+    assert 'name' in json.loads(response.data)[0]
+    assert 'email' in json.loads(response.data)[0]
+    assert 'roles' in json.loads(response.data)[0]
+    assert ['dummy-role'] == json.loads(response.data)[0]['roles']
 
-    response = client.get('/users', headers=valid_header,)
 
+@patch(
+    'commons.feature_toggles.feature_toggle_manager.FeatureToggleManager.get_azure_app_configuration_client'
+)
+@patch(
+    'commons.feature_toggles.feature_toggle_manager.FeatureToggleManager.is_toggle_enabled_for_user'
+)
+@patch('utils.azure_users.AzureConnection.users')
+@patch('utils.azure_users.AzureConnection.users_v2')
+def test_feature_toggle_is_off_then_role_field_is_string(
+    users_v2_mock,
+    users_mock,
+    is_toggle_enabled_for_user_mock,
+    get_azure_app_configuration_client_mock,
+    client: FlaskClient,
+    valid_header: dict,
+):
+    is_toggle_enabled_for_user_mock.return_value = False
+    users_mock.return_value = [
+        {'name': 'dummy', 'email': 'dummy', 'role': 'dummy-role'}
+    ]
+
+    response = client.get('/users', headers=valid_header)
+
+    users_mock.assert_called()
+    users_v2_mock.assert_not_called()
     assert HTTPStatus.OK == response.status_code
     assert 'name' in json.loads(response.data)[0]
     assert 'email' in json.loads(response.data)[0]
     assert 'role' in json.loads(response.data)[0]
+    assert 'dummy-role' == json.loads(response.data)[0]['role']
 
 
 def test_update_user_role_response_contains_expected_props(
@@ -42,14 +90,40 @@ def test_update_user_role_response_contains_expected_props(
     assert 'role' in json.loads(response.data)
 
 
+@patch('utils.azure_users.AzureConnection.update_role')
+@mark.parametrize(
+    'role_id,action', [('test', 'grant'), ('admin', 'revoke')],
+)
+def test_update_role_response_contains_expected_props(
+    update_role_mock,
+    client: FlaskClient,
+    valid_header: dict,
+    user_id: str,
+    role_id,
+    action,
+):
+    update_role_mock.return_value = {
+        'name': 'dummy',
+        'email': 'dummy',
+        'roles': [],
+    }
+    response = client.post(
+        f'/users/{user_id}/roles/{role_id}/{action}', headers=valid_header,
+    )
+    assert HTTPStatus.OK == response.status_code
+    assert 'name' in json.loads(response.data)
+    assert 'email' in json.loads(response.data)
+    assert 'roles' in json.loads(response.data)
+
+
 @patch('utils.azure_users.AzureConnection.update_user_role', new_callable=Mock)
 def test_on_post_update_user_role_is_being_called_with_valid_arguments(
     update_user_role_mock,
     client: FlaskClient,
     valid_header: dict,
     user_id: str,
 ):
-
+    update_user_role_mock.return_value = {}
     valid_user_role_data = {'role': 'admin'}
     response = client.post(
         f'/users/{user_id}/roles',
@@ -70,7 +144,7 @@ def test_on_delete_update_user_role_is_being_called_with_valid_arguments(
     valid_header: dict,
     user_id: str,
 ):
-
+    update_user_role_mock.return_value = {}
     response = client.delete(
         f'/users/{user_id}/roles/time-tracker-admin', headers=valid_header,
     )
@@ -98,6 +172,7 @@ def test_update_role_is_called_properly_on_each_action(
     action,
     is_grant,
 ):
+    update_role_mock.return_value = {}
     response = client.post(
         f'/users/{user_id}/roles/{role_id}/{action}', headers=valid_header,
     )

time_tracker_api/users/users_namespace.py

@@ -5,7 +5,7 @@
 from time_tracker_api.api import common_fields, api, NullableString
 
 from utils.azure_users import AzureConnection
-
+from commons.feature_toggles.feature_toggle_manager import FeatureToggleManager
 
 ns = api.namespace('users', description='Namespace of the API for users')
 
@@ -32,6 +32,15 @@
             description='Role assigned to the user by the tenant',
             example=Faker().word(['time-tracker-admin']),
         ),
+        'roles': fields.List(
+            fields.String(
+                title='Roles',
+                description='List of the roles assigned to the user by the tenant',
+            ),
+            example=Faker().words(
+                3, ['time-tracker-admin', 'test-user', 'guest',],
+            ),
+        ),
     },
 )
 
@@ -57,6 +66,9 @@ class Users(Resource):
     @ns.marshal_list_with(user_response_fields)
     def get(self):
         """List all users"""
+        user_role_field_toggle = FeatureToggleManager('bk-user-role-field')
+        if user_role_field_toggle.is_toggle_enabled_for_user():
+            return AzureConnection().users_v2()
         return AzureConnection().users()
 
 

utils/azure_users.py

@@ -45,6 +45,14 @@ def __init__(self, id, name, email, role):
         self.role = role
 
 
+class AzureUser_v2:
+    def __init__(self, id, name, email, roles):
+        self.id = id
+        self.name = name
+        self.email = email
+        self.roles = roles
+
+
 HTTP_PATCH_HEADERS = {
     'Content-type': 'application/json',
     'Accept': 'application/json',
@@ -53,9 +61,12 @@ def __init__(self, id, name, email, role):
 ROLE_FIELD_VALUES = {
     'admin': (
         'extension_1d76efa96f604499acc0c0ee116a1453_role',
-        'time_tracker_admin',
+        'time-tracker-admin',
+    ),
+    'test': (
+        'extension_1d76efa96f604499acc0c0ee116a1453_role_test',
+        'time-tracker-tester',
     ),
-    'test': ('waitforrealvalue', 'waitforrealvalue'),
 }
 
 
@@ -91,6 +102,22 @@ def users(self) -> List[AzureUser]:
         assert 'value' in response.json()
         return [self.to_azure_user(item) for item in response.json()['value']]
 
+    def users_v2(self) -> List[AzureUser]:
+        role_fields_params = ','.join(
+            [field_name for field_name, _ in ROLE_FIELD_VALUES.values()]
+        )
+        endpoint = "{endpoint}/users?api-version=1.6&$select=displayName,otherMails,objectId,{role_fields_params}".format(
+            endpoint=self.config.ENDPOINT,
+            role_fields_params=role_fields_params,
+        )
+        response = requests.get(endpoint, auth=BearerAuth(self.access_token))
+
+        assert 200 == response.status_code
+        assert 'value' in response.json()
+        return [
+            self.to_azure_user_v2(item) for item in response.json()['value']
+        ]
+
     def update_user_role(self, id, role):
         endpoint = "{endpoint}/users/{user_id}?api-version=1.6".format(
             endpoint=self.config.ENDPOINT, user_id=id
@@ -119,6 +146,19 @@ def to_azure_user(self, item) -> AzureUser:
         role = item[self.role_field] if there_is_role else None
         return AzureUser(id, name, email, role)
 
+    def to_azure_user_v2(self, item) -> AzureUser_v2:
+        there_is_email = len(item['otherMails']) > 0
+
+        id = item['objectId']
+        name = item['displayName']
+        email = item['otherMails'][0] if there_is_email else ''
+        roles = [
+            item[field_name]
+            for (field_name, field_value) in ROLE_FIELD_VALUES.values()
+            if field_name in item
+        ]
+        return AzureUser_v2(id, name, email, roles)
+
     def update_role(self, user_id, role_id, is_grant):
         endpoint = "{endpoint}/users/{user_id}?api-version=1.6".format(
             endpoint=self.config.ENDPOINT, user_id=user_id
@@ -136,7 +176,7 @@ def update_role(self, user_id, role_id, is_grant):
         response = requests.get(endpoint, auth=BearerAuth(self.access_token))
         assert 200 == response.status_code
 
-        return self.to_azure_user(response.json())
+        return self.to_azure_user_v2(response.json())
 
     def get_role_data(self, role_id, is_grant=True):
         assert role_id in ROLE_FIELD_VALUES.keys()