Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ workflows:
- feat/v6
- pm-2074_1
- feat/ai-workflows
- delete_user

- deployQa:
context: org-global
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
@import '@libs/ui/styles/includes';

.container {
display: flex;
flex-direction: column;
gap: $sp-4;
}

.description {
white-space: pre-line;
}

.actions {
display: flex;
justify-content: flex-end;
gap: $sp-3;
margin-top: $sp-4;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { ChangeEvent, FC, useCallback, useEffect, useState } from 'react'
import classNames from 'classnames'

import { BaseModal, Button, InputText } from '~/libs/ui'

import { UserInfo } from '../../models'

import styles from './DialogDeleteUser.module.scss'

interface Props {
className?: string
open: boolean
setOpen: (isOpen: boolean) => void
userInfo: UserInfo
isLoading?: boolean
onDelete: (ticketUrl: string) => void
}

export const DialogDeleteUser: FC<Props> = (props: Props) => {
const [ticketUrl, setTicketUrl] = useState('')
const [error, setError] = useState('')

useEffect(() => {
if (props.open) {
setTicketUrl('')
setError('')
}
}, [props.open])

const handleClose = useCallback(() => {
if (!props.isLoading) {
props.setOpen(false)
}
}, [props.isLoading, props.setOpen])

const handleConfirm = useCallback(() => {
if (!ticketUrl.trim()) {
setError('Delete ticket URL is required')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 readability]
Consider using a more specific error message to guide the user, such as 'Please enter a valid delete ticket URL.' This can improve user experience by providing clearer instructions.

return
}

setError('')
props.onDelete(ticketUrl.trim())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
Ensure that props.onDelete properly handles potential errors or exceptions that might occur during the deletion process. Consider wrapping this call in a try-catch block or handling errors in the parent component to prevent unhandled exceptions.

}, [props, ticketUrl])

const handleTicketUrlChange = useCallback(
(event: ChangeEvent<HTMLInputElement>) => {
if (error) {
setError('')
}

setTicketUrl(event.target.value)
},
[error],
)

const description
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 maintainability]
The concatenation of strings for the description could be improved for readability and maintainability by using template literals entirely. This would make it easier to read and modify in the future.

= `Are you sure you want to DELETE user ${props.userInfo.handle} with email address ${props.userInfo.email}. `
+ 'If you are sure, please enter the associated delete request ticket URL below'

return (
<BaseModal
allowBodyScroll
blockScroll
title={`Delete ${props.userInfo.handle}`}
onClose={handleClose}
open={props.open}
focusTrapped={false}
>
<div className={classNames(styles.container, props.className)}>
<p className={styles.description}>{description}</p>
<InputText
name='ticketUrl'
label='Delete request ticket URL'
placeholder='https://'
type='text'
value={ticketUrl}
error={error}
onChange={handleTicketUrlChange}
disabled={props.isLoading}
/>

<div className={styles.actions}>
<Button
secondary
size='lg'
onClick={handleClose}
disabled={props.isLoading}
>
Cancel
</Button>
<Button
primary
variant='danger'
size='lg'
onClick={handleConfirm}
disabled={props.isLoading}
>
DELETE
</Button>
</div>
</div>
</BaseModal>
)
}

export default DialogDeleteUser
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { DialogDeleteUser } from './DialogDeleteUser'
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
}

.blockColumnAction {
width: 240px;
width: 320px;

@include ltelg {
width: 60px;
Expand Down
47 changes: 44 additions & 3 deletions src/apps/admin/src/lib/components/UsersTable/UsersTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { DialogEditUserSSOLogin } from '../DialogEditUserSSOLogin'
import { DialogEditUserTerms } from '../DialogEditUserTerms'
import { DialogEditUserStatus } from '../DialogEditUserStatus'
import { DialogUserStatusHistory } from '../DialogUserStatusHistory'
import { DialogDeleteUser } from '../DialogDeleteUser'
import { DropdownMenuButton } from '../common/DropdownMenuButton'
import { useTableFilterLocal, useTableFilterLocalProps } from '../../hooks'
import { TABLE_DATE_FORMAT } from '../../../config/index.config'
Expand All @@ -43,12 +44,18 @@ interface Props {
totalPages: number
onPageChange: (page: number) => void
updatingStatus: { [key: string]: boolean }
deletingUsers: { [key: string]: boolean }
doUpdateStatus: (
userInfo: UserInfo,
newStatus: string,
comment: string,
onSuccess?: () => void,
) => void
doDeleteUser: (
userInfo: UserInfo,
ticketUrl: string,
onSuccess?: () => void,
) => void
}

export const UsersTable: FC<Props> = props => {
Expand Down Expand Up @@ -100,6 +107,9 @@ export const UsersTable: FC<Props> = props => {
const [showDialogStatusHistory, setShowDialogStatusHistory] = useState<
UserInfo | undefined
>()
const [showDialogDeleteUser, setShowDialogDeleteUser] = useState<
UserInfo | undefined
>()
const { width: screenWidth }: WindowSize = useWindowSize()

const updatingStatusBool = useMemo(
Expand Down Expand Up @@ -294,6 +304,8 @@ export const UsersTable: FC<Props> = props => {
columnId: 'Action',
label: 'Action',
renderer: (data: UserInfo) => {
const isDeleting = props.deletingUsers?.[data.id] === true

function onSelectOption(item: string): void {
if (item === 'Primary Email') {
setShowDialogEditUserEmail(data)
Expand Down Expand Up @@ -321,6 +333,8 @@ export const UsersTable: FC<Props> = props => {
data,
message: confirmation,
})
} else if (item === 'Delete') {
setShowDialogDeleteUser(data)
}
}

Expand All @@ -335,8 +349,8 @@ export const UsersTable: FC<Props> = props => {
'Terms',
'SSO Logins',
...(data.active
? ['Deactivate']
: ['Activate']),
? ['Deactivate', 'Delete']
: ['Activate', 'Delete']),
]}
onSelectOption={onSelectOption}
>
Expand Down Expand Up @@ -374,6 +388,7 @@ export const UsersTable: FC<Props> = props => {
onClick={function onClick() {
onSelectOption('Deactivate')
}}
disabled={isDeleting}
/>
) : (
<Button
Expand All @@ -385,6 +400,15 @@ export const UsersTable: FC<Props> = props => {
}}
/>
)}
<Button
primary
variant='danger'
label='Delete'
onClick={function onClick() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 maintainability]
The onClick handler for the 'Delete' button directly calls onSelectOption('Delete'). Consider extracting this logic into a named function for better readability and maintainability, especially if the logic becomes more complex in the future.

onSelectOption('Delete')
}}
disabled={isDeleting}
/>
</>
)}
</div>
Expand All @@ -393,7 +417,7 @@ export const UsersTable: FC<Props> = props => {
type: 'action',
},
],
[isTablet, isMobile],
[isTablet, isMobile, props.deletingUsers, props.updatingStatus],
)

return (
Expand Down Expand Up @@ -473,6 +497,23 @@ export const UsersTable: FC<Props> = props => {
isLoading={updatingStatusBool}
/>
)}
{showDialogDeleteUser && (
<DialogDeleteUser
open
setOpen={function setOpen() {
setShowDialogDeleteUser(undefined)
}}
userInfo={showDialogDeleteUser}
isLoading={props.deletingUsers?.[showDialogDeleteUser.id]}
onDelete={function onDelete(ticketUrl: string) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ security]
Ensure that ticketUrl is properly validated before being used in doDeleteUser. If ticketUrl is user-generated or comes from an untrusted source, it could lead to security vulnerabilities.

props.doDeleteUser(
showDialogDeleteUser,
ticketUrl,
() => setShowDialogDeleteUser(undefined),
)
}}
/>
)}
{showDialogStatusHistory && (
<DialogUserStatusHistory
open
Expand Down
Loading
Loading