Skip to content

Commit

Permalink
Implement deactivate/activate staff user (#2038)
Browse files Browse the repository at this point in the history
* Add toggle user active status backend and front

* initialize user status and add loader

* fix lint issues

* implement todo

* fix if statement

* Add api tests

* Add front end tests
  • Loading branch information
jadmsaadaot authored Aug 18, 2023
1 parent 7b6ae28 commit ab5ef70
Show file tree
Hide file tree
Showing 21 changed files with 497 additions and 46 deletions.
28 changes: 28 additions & 0 deletions met-api/migrations/versions/f40da1b8f3e0_initialize_user_status.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
""" Initialize user status
P
Revision ID: f40da1b8f3e0
Revises: 31041fb90d53
Create Date: 2023-08-18 09:50:27.567044
"""
from alembic import op

# revision identifiers, used by Alembic.
revision = 'f40da1b8f3e0'
down_revision = '31041fb90d53'
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.execute("UPDATE staff_users SET status_id = 1")
op.alter_column('survey', 'is_template', nullable=False, server_default='1')
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.alter_column('survey', 'is_template', nullable=True, server_default=None)
op.execute("UPDATE staff_users SET status_id = None")
# ### end Alembic commands ###
2 changes: 1 addition & 1 deletion met-api/src/met_api/models/staff_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class StaffUser(BaseModel):
email_address = Column(db.String(100), nullable=True)
contact_number = Column(db.String(50), nullable=True)
external_id = Column(db.String(50), nullable=False, unique=True)
status_id = db.Column(db.Integer, ForeignKey('user_status.id'))
status_id = db.Column(db.Integer, ForeignKey('user_status.id'), nullable=False, server_default='1')
tenant_id = db.Column(db.Integer, db.ForeignKey('tenant.id'), nullable=True)

@classmethod
Expand Down
24 changes: 24 additions & 0 deletions met-api/src/met_api/resources/staff_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,30 @@ def get(user_id):
return user, HTTPStatus.OK


@cors_preflight('PATCH')
@API.route('/<user_id>/status')
class StaffUserStatus(Resource):
"""User controller class."""

@staticmethod
@cross_origin(origins=allowedorigins())
@_jwt.has_one_of_roles([Role.TOGGLE_USER_STATUS.value])
def patch(user_id):
"""Return a set of users(staff only)."""
try:
data = request.get_json()
if data.get('active', None) is None:
return {'message': 'active field is required'}, HTTPStatus.BAD_REQUEST

user = StaffUserService.toggle_user_active_status(
user_id,
active=data.get('active'),
)
return user, HTTPStatus.OK
except (KeyError, ValueError) as err:
return str(err), HTTPStatus.BAD_REQUEST


@cors_preflight('POST')
@API.route('/<user_id>/groups')
class UserGroup(Resource):
Expand Down
1 change: 1 addition & 0 deletions met-api/src/met_api/schemas/staff_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ class Meta: # pylint: disable=too-few-public-methods
updated_date = fields.Str(data_key='updated_date')
roles = fields.List(fields.Str(data_key='roles'))
tenant_id = fields.Str(data_key='tenant_id')
status_id = fields.Int(data_key='status_id')
21 changes: 21 additions & 0 deletions met-api/src/met_api/services/keycloak.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,3 +204,24 @@ def get_user_by_username(username, admin_token=None):
query_user_url = f'{base_url}/auth/admin/realms/{realm}/users?username={username}'
response = requests.get(query_user_url, headers=headers, timeout=timeout)
return response.json()[0]

@staticmethod
def toggle_user_enabled_status(user_id, enabled):
"""Toggle the enabled status of a user in Keycloak."""
base_url = current_app.config.get('KEYCLOAK_BASE_URL')
realm = current_app.config.get('KEYCLOAK_REALMNAME')
timeout = current_app.config.get('CONNECT_TIMEOUT', 60)
admin_token = KeycloakService._get_admin_token()
headers = {
'Content-Type': ContentType.JSON.value,
'Authorization': f'Bearer {admin_token}'
}

user_data = {
'enabled': enabled # Set the user's enabled status based on 'enable' parameter
}

# Update the user's enabled status
update_user_url = f'{base_url}/auth/admin/realms/{realm}/users/{user_id}'
response = requests.put(update_user_url, json=user_data, headers=headers, timeout=timeout)
response.raise_for_status()
14 changes: 13 additions & 1 deletion met-api/src/met_api/services/staff_user_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from met_api.services.keycloak import KeycloakService
from met_api.utils import notification
from met_api.utils.constants import GROUP_NAME_MAPPING, Groups
from met_api.utils.enums import KeycloakGroupName
from met_api.utils.enums import KeycloakGroupName, UserStatus
from met_api.utils.template import Template


Expand Down Expand Up @@ -175,3 +175,15 @@ def validate_user(db_user: StaffUserModel):
raise BusinessException(
error='This user is already a Superuser.',
status_code=HTTPStatus.CONFLICT.value)

@staticmethod
def toggle_user_active_status(user_external_id: str, active: bool):
"""Toggle user active status."""
db_user = StaffUserModel.get_user_by_external_id(user_external_id)
if db_user is None:
raise KeyError('User not found')

KEYCLOAK_SERVICE.toggle_user_enabled_status(user_id=user_external_id, enabled=active)
db_user.status_id = UserStatus.ACTIVE.value if active else UserStatus.INACTIVE.value
db_user.save()
return StaffUserSchema().dump(db_user)
7 changes: 7 additions & 0 deletions met-api/src/met_api/utils/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,10 @@ class SourceAction(Enum):

CREATED = 'created'
PUBLISHED = 'published'


class UserStatus(IntEnum):
"""User status."""

ACTIVE = 1
INACTIVE = 2
1 change: 1 addition & 0 deletions met-api/src/met_api/utils/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class Role(Enum):
CREATE_TENANT = 'create_tenant'
VIEW_TENANT = 'view_tenant'
VIEW_USERS = 'view_users'
TOGGLE_USER_STATUS = 'toggle_user_status'
CREATE_ADMIN_USER = 'create_admin_user'
CREATE_TEAM = 'create_team'
CREATE_ENGAGEMENT = 'create_engagement'
Expand Down
87 changes: 86 additions & 1 deletion met-api/tests/unit/api/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from flask import current_app

from met_api.models import Tenant as TenantModel
from met_api.utils.enums import ContentType, KeycloakGroupName
from met_api.utils.enums import ContentType, KeycloakGroupName, UserStatus
from tests.utilities.factory_scenarios import TestJwtClaims, TestUserInfo
from tests.utilities.factory_utils import factory_auth_header, factory_staff_user_model

Expand Down Expand Up @@ -135,3 +135,88 @@ def test_add_user_to_team_member_group(mocker, client, jwt, session):
assert rv.status_code == HTTPStatus.OK
mock_add_user_to_group_keycloak.assert_called()
mock_get_user_groups_keycloak.assert_called()


def mock_toggle_user_status(mocker):
"""Mock the KeycloakService.add_user_to_group method."""
mock_response = MagicMock()
mock_response.status_code = HTTPStatus.NO_CONTENT

mock_toggle_user_status = mocker.patch(
f'{KEYCLOAK_SERVICE_MODULE}.toggle_user_enabled_status',
return_value=mock_response
)

return mock_toggle_user_status


def test_toggle_user_active_status(mocker, client, jwt, session):
"""Assert that a user can be toggled."""
user = factory_staff_user_model()
mocked_toggle_user_status = mock_toggle_user_status(mocker)

assert user.status_id == UserStatus.ACTIVE.value
claims = TestJwtClaims.staff_admin_role
headers = factory_auth_header(jwt=jwt, claims=claims)
rv = client.patch(
f'/api/user/{user.external_id}/status',
headers=headers,
json={'active': False},
content_type=ContentType.JSON.value
)
assert rv.status_code == HTTPStatus.OK
assert rv.json.get('status_id') == UserStatus.INACTIVE.value
mocked_toggle_user_status.assert_called()


def test_team_member_cannot_toggle_user_active_status(mocker, client, jwt, session):
"""Assert that a team member cannot toggle user status."""
user = factory_staff_user_model()
mocked_toggle_user_status = mock_toggle_user_status(mocker)

assert user.status_id == UserStatus.ACTIVE.value
claims = TestJwtClaims.team_member_role
headers = factory_auth_header(jwt=jwt, claims=claims)
rv = client.patch(
f'/api/user/{user.external_id}/status',
headers=headers,
json={'active': False},
content_type=ContentType.JSON.value
)
assert rv.status_code == HTTPStatus.UNAUTHORIZED
mocked_toggle_user_status.assert_not_called()


def test_reviewer_cannot_toggle_user_active_status(mocker, client, jwt, session):
"""Assert that a reviewer cannot toggle user status."""
user = factory_staff_user_model()
mocked_toggle_user_status = mock_toggle_user_status(mocker)

assert user.status_id == UserStatus.ACTIVE.value
claims = TestJwtClaims.reviewer_role
headers = factory_auth_header(jwt=jwt, claims=claims)
rv = client.patch(
f'/api/user/{user.external_id}/status',
headers=headers,
json={'active': False},
content_type=ContentType.JSON.value
)
assert rv.status_code == HTTPStatus.UNAUTHORIZED
mocked_toggle_user_status.assert_not_called()


def test_toggle_user_active_status_empty_body(mocker, client, jwt, session):
"""Assert that returns bad request if bad request body."""
user = factory_staff_user_model()
mocked_toggle_user_status = mock_toggle_user_status(mocker)

assert user.status_id == UserStatus.ACTIVE.value
claims = TestJwtClaims.staff_admin_role
headers = factory_auth_header(jwt=jwt, claims=claims)
rv = client.patch(
f'/api/user/{user.external_id}/status',
headers=headers,
content_type=ContentType.JSON.value
)
assert rv.status_code == HTTPStatus.BAD_REQUEST
mocked_toggle_user_status.assert_not_called()
5 changes: 4 additions & 1 deletion met-api/tests/utilities/factory_scenarios.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

from met_api.constants.feedback import CommentType, FeedbackSourceType, RatingType
from met_api.constants.widget import WidgetType
from met_api.utils.enums import LoginSource
from met_api.utils.enums import LoginSource, UserStatus


fake = Faker()
Expand All @@ -42,13 +42,15 @@ class TestUserInfo(dict, Enum):
user = {
'id': 123,
'first_name': 'System',
'status_id': UserStatus.ACTIVE.value,
}

user_staff_1 = {
'first_name': fake.name(),
'middle_name': fake.name(),
'last_name': fake.name(),
'email_address': fake.email(),
'status_id': UserStatus.ACTIVE.value,
}


Expand Down Expand Up @@ -298,6 +300,7 @@ class TestJwtClaims(dict, Enum):
'review_comments',
'review_all_comments',
'view_all_engagements',
'toggle_user_status',
]
}
}
Expand Down
1 change: 1 addition & 0 deletions met-api/tests/utilities/factory_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ def factory_staff_user_model(external_id=None, user_info: dict = TestUserInfo.us
middle_name=user_info['middle_name'],
email_address=user_info['email_address'],
external_id=str(external_id),
status_id=user_info['status_id'],
)
user.save()
return user
Expand Down
1 change: 1 addition & 0 deletions met-web/src/apiManager/endpoints/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const Endpoints = {
GET_LIST: `${AppConfig.apiUrl}/user/`,
ADD_TO_GROUP: `${AppConfig.apiUrl}/user/user_id/groups`,
GET_USER_ENGAGEMENTS: `${AppConfig.apiUrl}/user/user_id/engagements`,
TOGGLE_USER_STATUS: `${AppConfig.apiUrl}/user/user_id/status`,
},
Document: {
OSS_HEADER: `${AppConfig.apiUrl}/document/`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import TextField from '@mui/material/TextField';
import Grid from '@mui/material/Grid';
import Stack from '@mui/material/Stack';
import SearchIcon from '@mui/icons-material/Search';
import { User } from 'models/user';
import { USER_STATUS, User } from 'models/user';
import { HeadCell, PaginationOptions } from 'components/common/Table/types';
import { MetPageGridContainer, PrimaryButton } from 'components/common';
import { Link } from 'react-router-dom';
Expand Down Expand Up @@ -49,14 +49,13 @@ const UserManagementListing = () => {
renderCell: (row: User) => formatDate(row.created_date),
},
{
key: 'status',
key: 'status_id',
numeric: false,
disablePadding: true,
label: 'Status',
allowSort: true,
/* TODO Hardcoded value since currently we have all users as active.
Need to change once we have different user status */
renderCell: () => 'Active',
renderCell: (row: User) =>
Object.values(USER_STATUS).find((status) => status.value === row.status_id)?.label ?? '',
},
{
key: 'id',
Expand Down
44 changes: 10 additions & 34 deletions met-web/src/components/userManagement/userDetails/UserDetails.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useContext, useState, ChangeEvent } from 'react';
import React, { useContext, useState } from 'react';
import { Grid, FormControlLabel, Switch } from '@mui/material';
import { MetLabel, MetParagraph, MetPageGridContainer, PrimaryButton } from 'components/common';
import { useAppSelector, useAppDispatch } from 'hooks';
Expand All @@ -7,15 +7,16 @@ import { openNotificationModal } from 'services/notificationModalService/notific
import { openNotification } from 'services/notificationService/notificationSlice';
import { formatDate } from 'components/common/dateHelper';
import AssignedEngagementsListing from './AssignedEngagementsListing';
import UserStatusToggle from './UserStatusToggle';
import UserDetailsSkeleton from './UserDetailsSkeleton';

export const UserDetails = () => {
const { roles } = useAppSelector((state) => state.user);
const { savedUser, setAddUserModalOpen } = useContext(UserDetailsContext);
const { savedUser, setAddUserModalOpen, isUserLoading } = useContext(UserDetailsContext);
const [superUserAssigned, setSuperUser] = useState(false);
const [deactivatedUser, setDeactivatedUser] = useState(false);
const dispatch = useAppDispatch();

const handleToggleChange = (event: ChangeEvent<HTMLInputElement>, checked: boolean) => {
const handleToggleChange = () => {
if (roles.includes('SuperUser')) {
dispatch(
openNotificationModal({
Expand All @@ -41,32 +42,10 @@ export const UserDetails = () => {
dispatch(openNotification({ severity: 'error', text: 'You do not have permissions to give user roles' }));
}
};
const handleUserDeactivated = (event: ChangeEvent<HTMLInputElement>, checked: boolean) => {
if (roles.includes('SuperUser')) {
dispatch(
openNotificationModal({
open: true,
data: {
header: `Remove SuperUser role from ${savedUser?.username}`,
subText: [
{
text: `You are attempting to remove the SuperUser role from ${savedUser?.username}`,
},
{
text: 'Are you sure?',
},
],
handleConfirm: () => {
setDeactivatedUser(!deactivatedUser);
},
},
type: 'confirm',
}),
);
} else {
dispatch(openNotification({ severity: 'error', text: 'You do not have permissions to remove user roles' }));
}
};

if (isUserLoading) {
return <UserDetailsSkeleton />;
}

return (
<MetPageGridContainer>
Expand Down Expand Up @@ -120,10 +99,7 @@ export const UserDetails = () => {

<Grid container direction="row" item xs={6} spacing={1}>
<Grid item xs={12}>
<FormControlLabel
control={<Switch checked={deactivatedUser} onChange={handleUserDeactivated} />}
label={<MetLabel>Deactivate User</MetLabel>}
/>
<UserStatusToggle />
</Grid>
</Grid>
</Grid>
Expand Down
Loading

0 comments on commit ab5ef70

Please sign in to comment.