feat: TT-122 solve PR comments

tests/time_tracker_api/users/users_namespace_test.py

@@ -188,17 +188,17 @@ def test_update_role_is_called_properly_on_each_action(
     )
 
 
-@patch('msal.ConfidentialClientApplication')
-@patch('utils.azure_users.AzureConnection.get_token')
+@patch('msal.ConfidentialClientApplication', Mock())
+@patch('utils.azure_users.AzureConnection.get_token', Mock())
 @patch('utils.azure_users.AzureConnection.get_test_user_ids')
 @patch('utils.azure_users.AzureConnection.users_v2')
 def test_azure_connection_get_non_test_users(
-    users_v2_mock, get_test_user_ids_mock, get_token_mock, msal_client_mock,
+    users_v2_mock, get_test_user_ids_mock
 ):
-    az1 = AzureUser_v2('ID1', None, None, [])
-    az2 = AzureUser_v2('ID2', None, None, [])
-    users_v2_mock.return_value = [az1, az2]
+    test_user = AzureUser_v2('ID1', None, None, [])
+    non_test_user = AzureUser_v2('ID2', None, None, [])
+    users_v2_mock.return_value = [test_user, non_test_user]
     get_test_user_ids_mock.return_value = ['ID1']
-    non_test_users = [az2]
+    non_test_users = [non_test_user]
     az_conn = AzureConnection()
     assert az_conn.get_non_test_users() == non_test_users

time_tracker_api/users/users_namespace.py

@@ -70,13 +70,14 @@ def get(self):
         user_role_field_toggle = FeatureToggleManager('bk-user-role-field')
         if user_role_field_toggle.is_toggle_enabled_for_user():
             azure_connection = AzureConnection()
-            current_user_is_tester = azure_connection.is_test_user(
+            is_current_user_a_tester = azure_connection.is_test_user(
                 current_user_id()
             )
-            if current_user_is_tester:
-                return azure_connection.users_v2()
-            else:
-                return azure_connection.get_non_test_users()
+            return (
+                azure_connection.users_v2()
+                if is_current_user_a_tester
+                else azure_connection.get_non_test_users()
+            )
         return AzureConnection().users()
 
 

utils/azure_users.py

@@ -178,20 +178,17 @@ def update_role(self, user_id, role_id, is_grant):
 
         return self.to_azure_user_v2(response.json())
 
-    def _get_user(self, user_id):
+    def is_test_user(self, user_id):
         endpoint = "{endpoint}/users/{user_id}?api-version=1.6".format(
             endpoint=self.config.ENDPOINT, user_id=user_id
         )
         response = requests.get(endpoint, auth=BearerAuth(self.access_token))
         assert 200 == response.status_code
-        return response.json()
-
-    def is_test_user(self, user_id):
-        response = self._get_user(user_id)
+        item = response.json()
         field_name, field_value = ROLE_FIELD_VALUES['test']
-        return field_name in response and field_value == response[field_name]
+        return field_name in item and field_value == item[field_name]
 
-    def _get_test_user_ids(self):
+    def get_test_user_ids(self):
         field_name, field_value = ROLE_FIELD_VALUES['test']
         endpoint = "{endpoint}/users?api-version=1.6&$select=objectId,{field_name}&$filter={field_name} eq '{field_value}'".format(
             endpoint=self.config.ENDPOINT,
@@ -201,11 +198,7 @@ def _get_test_user_ids(self):
         response = requests.get(endpoint, auth=BearerAuth(self.access_token))
         assert 200 == response.status_code
         assert 'value' in response.json()
-        return response.json()['value']
-
-    def get_test_user_ids(self):
-        response = self._get_test_user_ids()
-        return [item['objectId'] for item in response]
+        return [item['objectId'] for item in response.json()['value']]
 
     def get_non_test_users(self) -> List[AzureUser]:
         test_user_ids = self.get_test_user_ids()