From 40fa4dcdcb568faf7c8f64a0f28ef3b8ca5a5994 Mon Sep 17 00:00:00 2001 From: darth-coder00 <86726556+darth-coder00@users.noreply.github.com> Date: Wed, 30 Mar 2022 00:37:59 +0530 Subject: [PATCH] Fix #3683: Allow Admins to delete a user from Users page (#3743) * Fix #3683: Allow Admins to delete a user from Users page --- .../resources/ui/src/axiosAPIs/userAPI.ts | 4 + .../UserDataCard/UserDataCard.test.tsx | 37 ++++++- .../components/UserDataCard/UserDataCard.tsx | 96 ++++++++++++------- .../ui/src/components/UserList/UserList.tsx | 39 +++++++- .../src/main/resources/ui/src/jsons/en.ts | 23 +++-- .../pages/UserListPage/UserListPage.test.tsx | 35 ++++++- .../src/pages/UserListPage/UserListPage.tsx | 82 ++++++++++++---- .../resources/ui/src/pages/teams/index.tsx | 2 +- 8 files changed, 249 insertions(+), 69 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/axiosAPIs/userAPI.ts b/openmetadata-ui/src/main/resources/ui/src/axiosAPIs/userAPI.ts index 9d6f522ab04..9835d1f0c63 100644 --- a/openmetadata-ui/src/main/resources/ui/src/axiosAPIs/userAPI.ts +++ b/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}`); +}; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/UserDataCard/UserDataCard.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/UserDataCard/UserDataCard.test.tsx index 60f17c2230a..8146c5fa460 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/UserDataCard/UserDataCard.test.tsx +++ b/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(

Avatar

); }); +jest.mock('../../utils/SvgUtils', () => { + return { + __esModule: true, + default: jest.fn().mockReturnValue(

SVGIcons

), + Icons: { + DELETE: 'delete', + }, + }; +}); + describe('Test UserDataCard component', () => { it('Component should render', async () => { const { container } = render( - , + , { wrapper: MemoryRouter, } @@ -50,7 +77,11 @@ describe('Test UserDataCard component', () => { it('Data should render', async () => { const { container } = render( - , + , { wrapper: MemoryRouter, } diff --git a/openmetadata-ui/src/main/resources/ui/src/components/UserDataCard/UserDataCard.tsx b/openmetadata-ui/src/main/resources/ui/src/components/UserDataCard/UserDataCard.tsx index c84aeb06c58..43f41b472bd 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/UserDataCard/UserDataCard.tsx +++ b/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 (
- {item.profilePhoto ? ( -
- profile -
- ) : ( - - )} +
+ {item.profilePhoto ? ( +
+ profile +
+ ) : ( + + )} -
-
-

{ - onClick(item.id as string); - }}> - {item.description} -

- {!item.isActiveUser && ( - - Inactive - - )} +
+
+

{ + onClick(item.id as string); + }}> + {item.description} +

+ {!item.isActiveUser && ( + + Inactive + + )} +
+

{item.email}

+

Teams: {item.teamCount}

-

{item.email}

-

Teams: {item.teamCount}

+ {!isNil(onDelete) && ( +
+ + { + e.preventDefault(); + e.stopPropagation(); + onDelete(item.id as string, item.description); + }}> + + + +
+ )}
); }; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/UserList/UserList.tsx b/openmetadata-ui/src/main/resources/ui/src/components/UserList/UserList.tsx index e422298e11e..0ee22fd4e48 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/UserList/UserList.tsx +++ b/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; roles: Array; allUsers: Array; + 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 = ({ allUsers = [], isLoading, + deleteUser, updateUser, handleAddUserClick, teams = [], @@ -55,6 +63,7 @@ const UserList: FunctionComponent = ({ const [currentTab, setCurrentTab] = useState(1); const [selectedUser, setSelectedUser] = useState(); const [searchText, setSearchText] = useState(''); + const [deletingUser, setDeletingUser] = useState(); const handleSearchAction = (searchValue: string) => { setSearchText(searchValue); @@ -160,6 +169,18 @@ const UserList: FunctionComponent = ({ } }; + 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 = ({ className="tw-cursor-pointer" key={index} onClick={() => selectUser(User.id)}> - +
); })} @@ -376,6 +401,18 @@ const UserList: FunctionComponent = ({ onSave={handleSave} /> )} + {!isUndefined(deletingUser) && ( + setDeletingUser(undefined)} + onConfirm={() => { + onConfirmDeleteUser(deletingUser.id); + }} + /> + )} )} diff --git a/openmetadata-ui/src/main/resources/ui/src/jsons/en.ts b/openmetadata-ui/src/main/resources/ui/src/jsons/en.ts index e4512f699b9..06254602a7b 100644 --- a/openmetadata-ui/src/main/resources/ui/src/jsons/en.ts +++ b/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!', diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/UserListPage/UserListPage.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/UserListPage/UserListPage.test.tsx index 93eca93774a..4ce0f35a78a 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/UserListPage/UserListPage.test.tsx +++ b/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(() =>
UserListComponent
); + return jest + .fn() + .mockImplementation(({ deleteUser }) => ( +
UserListComponent
+ )); }); 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(, { @@ -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(, { + wrapper: MemoryRouter, + }); + + await act(async () => { + fireEvent.click(await findByText(container, 'UserListComponent')); + }); + + expect(mockDeleteUser).toBeCalled(); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/UserListPage/UserListPage.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/UserListPage/UserListPage.tsx index fd6c6d2d70b..203e916107c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/UserListPage/UserListPage.tsx +++ b/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>([]); const [roles, setRoles] = useState>([]); - const [isLoading, setIsLoading] = useState(false); - const [allUsers, setAllUsers] = useState>([]); + const [isLoading, setIsLoading] = useState(true); + const [allUsers, setAllUsers] = useState>(); + + 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 ( { )} {deletingUser.state && (