fix: 🗑️ TT-131 remove feature toggle for user role field

tests/time_tracker_api/users/users_namespace_test.py

@@ -2,99 +2,33 @@
 from flask import json
 from flask.testing import FlaskClient
 from flask_restplus._http import HTTPStatus
-from utils.azure_users import AzureConnection
 from pytest import mark
 
 
-@patch('msal.ConfidentialClientApplication', Mock())
+@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
 @patch('utils.azure_users.AzureConnection.get_token', Mock())
 @patch(
     'utils.azure_users.AzureConnection.is_test_user', Mock(return_value=True)
 )
-@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,
+def test_users_response_contains_expected_props(
+    users_mock, client: FlaskClient, valid_header: dict,
 ):
-
-    is_toggle_enabled_for_user_mock.return_value = True
-    users_v2_mock.return_value = [
+    users_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()
+    users_mock.assert_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']
 
 
-@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(
-    client: FlaskClient, valid_header: dict, user_id: str,
-):
-    valid_user_role_data = {'role': 'admin'}
-    AzureConnection.update_user_role = Mock(
-        return_value={'name': 'dummy', 'email': 'dummy', 'role': 'dummy'}
-    )
-
-    response = client.post(
-        f'/users/{user_id}/roles',
-        headers=valid_header,
-        json=valid_user_role_data,
-    )
-
-    assert HTTPStatus.OK == response.status_code
-    assert 'name' in json.loads(response.data)
-    assert 'email' in json.loads(response.data)
-    assert 'role' in json.loads(response.data)
-
-
+@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
+@patch('utils.azure_users.AzureConnection.get_token', Mock())
 @patch('utils.azure_users.AzureConnection.update_role')
 @mark.parametrize(
     'role_id,action', [('test', 'grant'), ('admin', 'revoke')],
@@ -121,43 +55,8 @@ def test_update_role_response_contains_expected_props(
     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',
-        headers=valid_header,
-        json=valid_user_role_data,
-    )
-
-    assert HTTPStatus.OK == response.status_code
-    update_user_role_mock.assert_called_once_with(
-        user_id, valid_user_role_data['role']
-    )
-
-
-@patch('utils.azure_users.AzureConnection.update_user_role', new_callable=Mock)
-def test_on_delete_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 = {}
-    response = client.delete(
-        f'/users/{user_id}/roles/time-tracker-admin', headers=valid_header,
-    )
-
-    assert HTTPStatus.OK == response.status_code
-    update_user_role_mock.assert_called_once_with(user_id, role=None)
-
-
+@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
+@patch('utils.azure_users.AzureConnection.get_token', Mock())
 @patch('utils.azure_users.AzureConnection.update_role', new_callable=Mock)
 @mark.parametrize(
     'role_id,action,is_grant',

tests/utils/azure_users_test.py

@@ -1,9 +1,9 @@
 from unittest.mock import Mock, patch
-from utils.azure_users import AzureConnection, ROLE_FIELD_VALUES, AzureUser_v2
+from utils.azure_users import AzureConnection, ROLE_FIELD_VALUES, AzureUser
 from pytest import mark
 
 
-@patch('msal.ConfidentialClientApplication', Mock())
+@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
 @patch('utils.azure_users.AzureConnection.get_token', Mock())
 @patch('requests.get')
 @mark.parametrize(
@@ -26,7 +26,7 @@ def test_azure_connection_is_test_user(
     assert az_conn.is_test_user(test_user_id) == is_test_user_expected_value
 
 
-@patch('msal.ConfidentialClientApplication', Mock())
+@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
 @patch('utils.azure_users.AzureConnection.get_token', Mock())
 @patch('requests.get')
 def test_azure_connection_get_test_user_ids(get_mock):
@@ -42,16 +42,16 @@ def test_azure_connection_get_test_user_ids(get_mock):
     assert az_conn.get_test_user_ids() == ids
 
 
-@patch('msal.ConfidentialClientApplication', Mock())
+@patch('utils.azure_users.AzureConnection.get_msal_client', 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')
+@patch('utils.azure_users.AzureConnection.users')
 def test_azure_connection_get_non_test_users(
-    users_v2_mock, get_test_user_ids_mock
+    users_mock, get_test_user_ids_mock
 ):
-    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]
+    test_user = AzureUser('ID1', None, None, [])
+    non_test_user = AzureUser('ID2', None, None, [])
+    users_mock.return_value = [test_user, non_test_user]
     get_test_user_ids_mock.return_value = ['ID1']
     non_test_users = [non_test_user]
     az_conn = AzureConnection()

time_tracker_api/users/users_namespace.py

@@ -1,12 +1,10 @@
 from faker import Faker
 from flask_restplus import fields, Resource
-from flask_restplus._http import HTTPStatus
 
-from time_tracker_api.api import common_fields, api, NullableString
+from time_tracker_api.api import common_fields, api
 from time_tracker_api.security import current_user_id
 
 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')
 
@@ -27,12 +25,6 @@
             description='Email of the user that belongs to the tenant',
             example=Faker().email(),
         ),
-        'role': NullableString(
-            title="User's Role",
-            max_length=50,
-            description='Role assigned to the user by the tenant',
-            example=Faker().word(['time-tracker-admin']),
-        ),
         'roles': fields.List(
             fields.String(
                 title='Roles',
@@ -47,67 +39,22 @@
 
 user_response_fields.update(common_fields)
 
-user_role_input_fields = ns.model(
-    'UserRoleInput',
-    {
-        'role': NullableString(
-            title="User's Role",
-            required=True,
-            max_length=50,
-            description='Role assigned to the user by the tenant',
-            example=Faker().word(['time-tracker-admin']),
-        ),
-    },
-)
-
 
 @ns.route('')
 class Users(Resource):
     @ns.doc('list_users')
     @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():
-            azure_connection = AzureConnection()
-            is_current_user_a_tester = azure_connection.is_test_user(
-                current_user_id()
-            )
-            return (
-                azure_connection.users_v2()
-                if is_current_user_a_tester
-                else azure_connection.get_non_test_users()
-            )
-        return AzureConnection().users()
-
-
-@ns.route('/<string:id>/roles')
-@ns.response(HTTPStatus.NOT_FOUND, 'User not found')
-@ns.response(HTTPStatus.UNPROCESSABLE_ENTITY, 'The id has an invalid format')
-@ns.param('id', 'The user identifier')
-class UserRoles(Resource):
-    @ns.doc('create_user_role')
-    @ns.expect(user_role_input_fields)
-    @ns.response(
-        HTTPStatus.BAD_REQUEST, 'Invalid format or structure of the user'
-    )
-    @ns.marshal_with(user_response_fields)
-    def post(self, id):
-        """Create user's role"""
-        return AzureConnection().update_user_role(id, ns.payload['role'])
-
-
-@ns.route('/<string:user_id>/roles/<string:role_id>')
-@ns.response(HTTPStatus.NOT_FOUND, 'User not found')
-@ns.response(HTTPStatus.UNPROCESSABLE_ENTITY, 'The id has an invalid format')
-@ns.param('user_id', 'The user identifier')
-@ns.param('role_id', 'The role name identifier')
-class UserRole(Resource):
-    @ns.doc('delete_user_role')
-    @ns.marshal_with(user_response_fields)
-    def delete(self, user_id, role_id):
-        """Delete user's role"""
-        return AzureConnection().update_user_role(user_id, role=None)
+        azure_connection = AzureConnection()
+        is_current_user_a_tester = azure_connection.is_test_user(
+            current_user_id()
+        )
+        return (
+            azure_connection.users()
+            if is_current_user_a_tester
+            else azure_connection.get_non_test_users()
+        )
 
 
 @ns.route('/<string:user_id>/roles/<string:role_id>/grant')