Fix #3683: Allow Admins to delete a user from Users page (#3743)

* Fix #3683: Allow Admins to delete a user from Users page

openmetadata-ui/src/main/resources/ui/src/axiosAPIs/userAPI.ts

@@ -106,3 +106,7 @@ export const getUserCounts = () => {
     `/search/query?q=*&from=0&size=0&index=${SearchIndex.USER}`
   );
 };
+
+export const deleteUser = (id: string) => {
+  return APIClient.delete(`/users/${id}`);
+};

openmetadata-ui/src/main/resources/ui/src/components/UserDataCard/UserDataCard.test.tsx

@@ -26,16 +26,43 @@ const mockItem = {
   teamCount: 'Cloud_Infra',
 };
 
-const mockRemove = jest.fn();
+const mockSelect = jest.fn();
+const mockDelete = jest.fn();
+
+jest.mock('../../auth-provider/AuthProvider', () => {
+  return {
+    useAuthContext: jest.fn(() => ({
+      isAuthDisabled: false,
+      isAuthenticated: true,
+      isProtectedRoute: jest.fn().mockReturnValue(true),
+      isTourRoute: jest.fn().mockReturnValue(false),
+      onLogoutHandler: jest.fn(),
+    })),
+  };
+});
 
 jest.mock('../../components/common/avatar/Avatar', () => {
   return jest.fn().mockReturnValue(<p data-testid="avatar">Avatar</p>);
 });
 
+jest.mock('../../utils/SvgUtils', () => {
+  return {
+    __esModule: true,
+    default: jest.fn().mockReturnValue(<p data-testid="svg-icon">SVGIcons</p>),
+    Icons: {
+      DELETE: 'delete',
+    },
+  };
+});
+
 describe('Test UserDataCard component', () => {
   it('Component should render', async () => {
     const { container } = render(
-      <UserDataCard item={mockItem} onClick={mockRemove} />,
+      <UserDataCard
+        item={mockItem}
+        onClick={mockSelect}
+        onDelete={mockDelete}
+      />,
       {
         wrapper: MemoryRouter,
       }
@@ -50,7 +77,11 @@ describe('Test UserDataCard component', () => {
 
   it('Data should render', async () => {
     const { container } = render(
-      <UserDataCard item={mockItem} onClick={mockRemove} />,
+      <UserDataCard
+        item={mockItem}
+        onClick={mockSelect}
+        onDelete={mockDelete}
+      />,
       {
         wrapper: MemoryRouter,
       }

openmetadata-ui/src/main/resources/ui/src/components/UserDataCard/UserDataCard.tsx

@@ -12,8 +12,11 @@
  */
 
 import classNames from 'classnames';
+import { isNil } from 'lodash';
 import React from 'react';
+import SVGIcons, { Icons } from '../../utils/SvgUtils';
 import Avatar from '../common/avatar/Avatar';
+import NonAdminAction from '../common/non-admin-action/NonAdminAction';
 
 type Item = {
   description: string;
@@ -28,47 +31,74 @@ type Item = {
 type Props = {
   item: Item;
   onClick: (value: string) => void;
+  onDelete?: (id: string, name: string) => void;
 };
 
-const UserDataCard = ({ item, onClick }: Props) => {
+const UserDataCard = ({ item, onClick, onDelete }: Props) => {
   return (
     <div
-      className="tw-card tw-flex tw-gap-1 tw-py-2 tw-px-3 tw-group"
+      className="tw-card tw-flex tw-justify-between tw-py-2 tw-px-3 tw-group"
       data-testid="user-card-container">
-      {item.profilePhoto ? (
-        <div className="tw-h-9 tw-w-9">
-          <img
-            alt="profile"
-            className="tw-rounded-full tw-w-full"
-            src={item.profilePhoto}
-          />
-        </div>
-      ) : (
-        <Avatar name={item.description} />
-      )}
+      <div className="tw-flex tw-gap-1">
+        {item.profilePhoto ? (
+          <div className="tw-h-9 tw-w-9">
+            <img
+              alt="profile"
+              className="tw-rounded-full tw-w-full"
+              src={item.profilePhoto}
+            />
+          </div>
+        ) : (
+          <Avatar name={item.description} />
+        )}
 
-      <div
-        className="tw-flex tw-flex-col tw-flex-1 tw-pl-2"
-        data-testid="data-container">
-        <div className="tw-flex tw-justify-between">
-          <p
-            className={classNames('tw-font-normal', {
-              'tw-cursor-pointer': Boolean(onClick),
-            })}
-            onClick={() => {
-              onClick(item.id as string);
-            }}>
-            {item.description}
-          </p>
-          {!item.isActiveUser && (
-            <span className="tw-text-xs tw-bg-badge tw-border tw-px-2 tw-py-0.5 tw-rounded">
-              Inactive
-            </span>
-          )}
+        <div
+          className="tw-flex tw-flex-col tw-flex-1 tw-pl-2"
+          data-testid="data-container">
+          <div className="tw-flex tw-justify-between">
+            <p
+              className={classNames('tw-font-normal', {
+                'tw-cursor-pointer': Boolean(onClick),
+              })}
+              onClick={() => {
+                onClick(item.id as string);
+              }}>
+              {item.description}
+            </p>
+            {!item.isActiveUser && (
+              <span className="tw-text-xs tw-bg-badge tw-border tw-px-2 tw-py-0.5 tw-rounded">
+                Inactive
+              </span>
+            )}
+          </div>
+          <p className="tw-truncate">{item.email}</p>
+          <p>Teams: {item.teamCount}</p>
         </div>
-        <p className="tw-truncate">{item.email}</p>
-        <p>Teams: {item.teamCount}</p>
       </div>
+      {!isNil(onDelete) && (
+        <div className="tw-flex-none">
+          <NonAdminAction
+            position="bottom"
+            title="You do not have permission to delete user.">
+            <span
+              className="tw-h-8 tw-rounded tw-mb-3"
+              data-testid="remove"
+              onClick={(e) => {
+                e.preventDefault();
+                e.stopPropagation();
+                onDelete(item.id as string, item.description);
+              }}>
+              <SVGIcons
+                alt="delete"
+                className="tw-cursor-pointer tw-opacity-0 group-hover:tw-opacity-100"
+                icon={Icons.DELETE}
+                title="Delete"
+                width="12px"
+              />
+            </span>
+          </NonAdminAction>
+        </div>
+      )}
     </div>
   );
 };

openmetadata-ui/src/main/resources/ui/src/components/UserList/UserList.tsx

@@ -27,6 +27,7 @@ import { Button } from '../buttons/Button/Button';
 import ErrorPlaceHolder from '../common/error-with-placeholder/ErrorPlaceHolder';
 import NonAdminAction from '../common/non-admin-action/NonAdminAction';
 import Searchbar from '../common/searchbar/Searchbar';
+import ConfirmationModal from '../Modals/ConfirmationModal/ConfirmationModal';
 import UserDetailsModal from '../Modals/UserDetailsModal/UserDetailsModal';
 import UserDataCard from '../UserDataCard/UserDataCard';
 
@@ -34,14 +35,21 @@ interface Props {
   teams: Array<Team>;
   roles: Array<Role>;
   allUsers: Array<User>;
+  deleteUser: (id: string) => void;
   updateUser: (id: string, data: Operation[], updatedUser: User) => void;
   handleAddUserClick: () => void;
   isLoading: boolean;
 }
 
+interface DeleteUserInfo {
+  name: string;
+  id: string;
+}
+
 const UserList: FunctionComponent<Props> = ({
   allUsers = [],
   isLoading,
+  deleteUser,
   updateUser,
   handleAddUserClick,
   teams = [],
@@ -55,6 +63,7 @@ const UserList: FunctionComponent<Props> = ({
   const [currentTab, setCurrentTab] = useState<number>(1);
   const [selectedUser, setSelectedUser] = useState<User>();
   const [searchText, setSearchText] = useState('');
+  const [deletingUser, setDeletingUser] = useState<DeleteUserInfo>();
 
   const handleSearchAction = (searchValue: string) => {
     setSearchText(searchValue);
@@ -160,6 +169,18 @@ const UserList: FunctionComponent<Props> = ({
     }
   };
 
+  const handleDeleteUser = (id: string, name: string) => {
+    setDeletingUser({
+      name,
+      id,
+    });
+  };
+
+  const onConfirmDeleteUser = (id: string) => {
+    deleteUser(id);
+    setDeletingUser(undefined);
+  };
+
   const handleTabChange = (tab: number) => {
     setSearchText('');
     setCurrentTab(tab);
@@ -343,7 +364,11 @@ const UserList: FunctionComponent<Props> = ({
                 className="tw-cursor-pointer"
                 key={index}
                 onClick={() => selectUser(User.id)}>
-                <UserDataCard item={User} onClick={selectUser} />
+                <UserDataCard
+                  item={User}
+                  onClick={selectUser}
+                  onDelete={handleDeleteUser}
+                />
               </div>
             );
           })}
@@ -376,6 +401,18 @@ const UserList: FunctionComponent<Props> = ({
                     onSave={handleSave}
                   />
                 )}
+                {!isUndefined(deletingUser) && (
+                  <ConfirmationModal
+                    bodyText={`Are you sure you want to delete ${deletingUser.name}?`}
+                    cancelText="Cancel"
+                    confirmText="Confirm"
+                    header="Delete user"
+                    onCancel={() => setDeletingUser(undefined)}
+                    onConfirm={() => {
+                      onConfirmDeleteUser(deletingUser.id);
+                    }}
+                  />
+                )}
               </>
             )}
           </>

openmetadata-ui/src/main/resources/ui/src/jsons/en.ts

@@ -27,11 +27,12 @@ const jsonData = {
 
     'delete-glossary-error': 'Error while deleting glossary!',
     'delete-glossary-term-error': 'Error while deleting glossary term!',
-    'delete-team-error': 'Error while deleting team!',
-    'delete-lineage-error': 'Error while deleting edge!',
-    'delete-test-error': 'Error while deleting test!',
-    'delete-message-error': 'Error while deleting message!',
     'delete-ingestion-error': 'Error while deleting ingestion workflow',
+    'delete-lineage-error': 'Error while deleting edge!',
+    'delete-message-error': 'Error while deleting message!',
+    'delete-team-error': 'Error while deleting team!',
+    'delete-test-error': 'Error while deleting test!',
+    'delete-user-error': 'Error while deleting user!',
 
     'elastic-search-error': 'Error while fetch data from Elasticsearch!',
 
@@ -46,6 +47,7 @@ const jsonData = {
     'fetch-glossary-error': 'Error while fetching glossary!',
     'fetch-glossary-list-error': 'Error while fetching glossaries!',
     'fetch-glossary-term-error': 'Error while fetching glossary term!',
+    'fetch-ingestion-error': 'Error while fetching ingestion workflow!',
     'fetch-lineage-error': 'Error while fetching lineage data!',
     'fetch-lineage-node-error': 'Error while fetching lineage node!',
     'fetch-pipeline-details-error': 'Error while fetching pipeline details!',
@@ -57,23 +59,24 @@ const jsonData = {
     'fetch-thread-error': 'Error while fetching threads!',
     'fetch-updated-conversation-error':
       'Error while fetching updated conversation!',
-    'fetch-ingestion-error': 'Error while fetching ingestion workflow!',
     'fetch-service-error': 'Error while fetching service details!',
+    'fetch-teams-error': 'Error while fetching teams!',
 
     'unexpected-server-response': 'Unexpected response from server!',
 
     'update-chart-error': 'Error while updating charts!',
-    'update-owner-error': 'Error while updating owner',
-    'update-glossary-term-error': 'Error while updating glossary term!',
     'update-description-error': 'Error while updating description!',
     'update-entity-error': 'Error while updating entity!',
-    'update-team-error': 'Error while updating team',
-    'update-tags-error': 'Error while updating tags!',
-    'update-task-error': 'Error while updating tasks!',
     'update-entity-follow-error': 'Error while following entity!',
     'update-entity-unfollow-error': 'Error while unfollowing entity!',
+    'update-glossary-term-error': 'Error while updating glossary term!',
     'update-ingestion-error': 'Error while updating ingestion workflow',
+    'update-owner-error': 'Error while updating owner',
     'update-service-config-error': 'Error while updating ingestion workflow',
+    'update-tags-error': 'Error while updating tags!',
+    'update-task-error': 'Error while updating tasks!',
+    'update-team-error': 'Error while updating team!',
+    'update-user-error': 'Error while updating user!',
   },
   'api-success-messages': {
     'create-conversation': 'Conversation created successfully!',

openmetadata-ui/src/main/resources/ui/src/pages/UserListPage/UserListPage.test.tsx

@@ -1,6 +1,13 @@
-import { findByTestId, findByText, render } from '@testing-library/react';
+import {
+  act,
+  findByTestId,
+  findByText,
+  fireEvent,
+  render,
+} from '@testing-library/react';
 import React, { ReactNode } from 'react';
 import { MemoryRouter } from 'react-router-dom';
+import { deleteUser } from '../../axiosAPIs/userAPI';
 import UserListPage from './UserListPage';
 
 jest.mock('../../components/containers/PageContainerV1', () => {
@@ -12,13 +19,23 @@ jest.mock('../../components/containers/PageContainerV1', () => {
 });
 
 jest.mock('../../components/UserList/UserList', () => {
-  return jest.fn().mockImplementation(() => <div>UserListComponent</div>);
+  return jest
+    .fn()
+    .mockImplementation(({ deleteUser }) => (
+      <div onClick={deleteUser}>UserListComponent</div>
+    ));
 });
 
 jest.mock('../../axiosAPIs/teamsAPI', () => ({
   getTeams: jest.fn().mockImplementation(() => Promise.resolve()),
 }));
 
+jest.mock('../../axiosAPIs/userAPI', () => ({
+  deleteUser: jest.fn().mockImplementation(() => Promise.resolve()),
+}));
+
+const mockDeleteUser = jest.fn(() => Promise.resolve({}));
+
 describe('Test UserListPage component', () => {
   it('UserListPage component should render properly', async () => {
     const { container } = render(<UserListPage />, {
@@ -31,4 +48,18 @@ describe('Test UserListPage component', () => {
     expect(PageContainerV1).toBeInTheDocument();
     expect(UserListComponent).toBeInTheDocument();
   });
+
+  it('should delete users', async () => {
+    (deleteUser as jest.Mock).mockImplementationOnce(mockDeleteUser);
+
+    const { container } = render(<UserListPage />, {
+      wrapper: MemoryRouter,
+    });
+
+    await act(async () => {
+      fireEvent.click(await findByText(container, 'UserListComponent'));
+    });
+
+    expect(mockDeleteUser).toBeCalled();
+  });
 });

openmetadata-ui/src/main/resources/ui/src/pages/UserListPage/UserListPage.tsx

@@ -13,12 +13,13 @@
 
 import { AxiosError, AxiosResponse } from 'axios';
 import { Operation } from 'fast-json-patch';
+import { isNil } from 'lodash';
 import { observer } from 'mobx-react';
 import React, { useEffect, useState } from 'react';
 import { useHistory } from 'react-router-dom';
 import AppState from '../../AppState';
 import { getTeams } from '../../axiosAPIs/teamsAPI';
-import { updateUserDetail } from '../../axiosAPIs/userAPI';
+import { deleteUser, updateUserDetail } from '../../axiosAPIs/userAPI';
 import PageContainerV1 from '../../components/containers/PageContainerV1';
 import UserList from '../../components/UserList/UserList';
 import { ROUTES } from '../../constants/constants';
@@ -26,6 +27,8 @@ import { Role } from '../../generated/entity/teams/role';
 import { Team } from '../../generated/entity/teams/team';
 import { User } from '../../generated/entity/teams/user';
 import useToastContext from '../../hooks/useToastContext';
+import jsonData from '../../jsons/en';
+import { getErrorText } from '../../utils/StringsUtils';
 
 const UserListPage = () => {
   const showToast = useToastContext();
@@ -33,20 +36,30 @@ const UserListPage = () => {
 
   const [teams, setTeams] = useState<Array<Team>>([]);
   const [roles, setRoles] = useState<Array<Role>>([]);
-  const [isLoading, setIsLoading] = useState<boolean>(false);
-  const [allUsers, setAllUsers] = useState<Array<User>>([]);
+  const [isLoading, setIsLoading] = useState<boolean>(true);
+  const [allUsers, setAllUsers] = useState<Array<User>>();
+
+  const handleShowErrorToast = (message: string) => {
+    showToast({
+      variant: 'error',
+      body: message,
+    });
+  };
 
   const fetchTeams = () => {
     setIsLoading(true);
     getTeams(['users'])
       .then((res: AxiosResponse) => {
-        setTeams(res.data.data);
+        if (res.data) {
+          setTeams(res.data.data);
+        } else {
+          throw jsonData['api-error-messages']['unexpected-server-response'];
+        }
       })
       .catch((err: AxiosError) => {
-        showToast({
-          variant: 'error',
-          body: err.message || 'No teams available!',
-        });
+        handleShowErrorToast(
+          getErrorText(err, jsonData['api-error-messages']['fetch-teams-error'])
+        );
       })
       .finally(() => {
         setIsLoading(false);
@@ -63,15 +76,41 @@ const UserListPage = () => {
   const updateUser = (id: string, data: Operation[], updatedUser: User) => {
     setIsLoading(true);
     updateUserDetail(id, data)
-      .then(() => {
-        setAllUsers(
-          allUsers.map((user) => {
-            if (user.id === id) {
-              return updatedUser;
-            }
+      .then((res) => {
+        if (res.data) {
+          setAllUsers(
+            (allUsers || []).map((user) => {
+              if (user.id === id) {
+                return updatedUser;
+              }
 
-            return user;
-          })
+              return user;
+            })
+          );
+        } else {
+          throw jsonData['api-error-messages']['unexpected-server-response'];
+        }
+      })
+      .catch((err: AxiosError) => {
+        handleShowErrorToast(
+          getErrorText(err, jsonData['api-error-messages']['update-user-error'])
+        );
+      })
+      .finally(() => {
+        setIsLoading(false);
+      });
+  };
+
+  const handleDeleteUser = (id: string) => {
+    setIsLoading(true);
+    deleteUser(id)
+      .then(() => {
+        AppState.updateUsers((allUsers || []).filter((item) => item.id !== id));
+        fetchTeams();
+      })
+      .catch((err: AxiosError) => {
+        handleShowErrorToast(
+          getErrorText(err, jsonData['api-error-messages']['delete-user-error'])
         );
       })
       .finally(() => {
@@ -80,7 +119,11 @@ const UserListPage = () => {
   };
 
   useEffect(() => {
-    setAllUsers(AppState.users);
+    if (AppState.users.length) {
+      setAllUsers(AppState.users);
+    } else {
+      setAllUsers(undefined);
+    }
   }, [AppState.users]);
   useEffect(() => {
     setRoles(AppState.userRoles);
@@ -93,9 +136,10 @@ const UserListPage = () => {
   return (
     <PageContainerV1>
       <UserList
-        allUsers={allUsers}
+        allUsers={allUsers || []}
+        deleteUser={handleDeleteUser}
         handleAddUserClick={handleAddUserClick}
-        isLoading={isLoading}
+        isLoading={isLoading || isNil(allUsers)}
         roles={roles}
         teams={teams}
         updateUser={updateUser}

openmetadata-ui/src/main/resources/ui/src/pages/teams/index.tsx

@@ -797,7 +797,7 @@ const TeamsPage = () => {
                 )}
                 {deletingUser.state && (
                   <ConfirmationModal
-                    bodyText={`Are you sure want to remove ${
+                    bodyText={`Are you sure you want to remove ${
                       deletingUser.user?.displayName ?? deletingUser.user?.name
                     }?`}
                     cancelText="Cancel"