Skip to content

Commit 896bfb5

Browse files
authored
RI-7298: Rework delete confirmation (#4831)
1 parent 5aa7be6 commit 896bfb5

File tree

5 files changed

+221
-56
lines changed

5 files changed

+221
-56
lines changed
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import React from 'react'
2+
import userEvent from '@testing-library/user-event'
3+
import DeleteConfirmationButton from './DeleteConfirmationButton'
4+
import { render, screen } from 'uiSrc/utils/test-utils'
5+
6+
describe('DeleteConfirmationButton', () => {
7+
it('renders the trigger IconButton', () => {
8+
const onConfirm = jest.fn()
9+
render(<DeleteConfirmationButton onConfirm={onConfirm} />)
10+
11+
const trigger = screen.getByTestId('manage-index-delete-btn')
12+
expect(trigger).toBeInTheDocument()
13+
})
14+
15+
it('opens the popover on trigger click and shows content', async () => {
16+
const user = userEvent.setup()
17+
const onConfirm = jest.fn()
18+
render(<DeleteConfirmationButton onConfirm={onConfirm} />)
19+
20+
expect(
21+
screen.queryByText(/are you sure you want to delete this index\?/i),
22+
).not.toBeInTheDocument()
23+
24+
await user.click(screen.getByTestId('manage-index-delete-btn'))
25+
26+
expect(
27+
screen.getByText(/are you sure you want to delete this index\?/i),
28+
).toBeInTheDocument()
29+
30+
const deleteBtn = screen.getByTestId('manage-index-delete-confirmation-btn')
31+
expect(deleteBtn).toBeInTheDocument()
32+
expect(deleteBtn).toHaveTextContent(/delete/i)
33+
})
34+
35+
it('calls onConfirm when the Delete button is clicked', async () => {
36+
const onConfirm = jest.fn()
37+
38+
render(<DeleteConfirmationButton onConfirm={onConfirm} isOpen />)
39+
40+
// In JSDOM, Popover content may be rendered with `pointer-events: none`
41+
// or `visibility: hidden` due to missing layout measurements.
42+
// This disables userEvent's pointer-events check so the button can be clicked in tests.
43+
const user = userEvent.setup({ pointerEventsCheck: 0 })
44+
45+
await user.click(screen.getByTestId('manage-index-delete-btn'))
46+
const deleteBtn = await screen.findByTestId(
47+
'manage-index-delete-confirmation-btn',
48+
)
49+
await user.click(deleteBtn)
50+
51+
expect(onConfirm).toHaveBeenCalledTimes(1)
52+
})
53+
54+
it('forwards RiPopover props via ...rest and keeps hardcoded ones', async () => {
55+
const user = userEvent.setup()
56+
const onConfirm = jest.fn()
57+
58+
render(
59+
<DeleteConfirmationButton
60+
onConfirm={onConfirm}
61+
id="test-id-manage-index"
62+
anchorPosition="upLeft"
63+
/>,
64+
)
65+
66+
await user.click(screen.getByTestId('manage-index-delete-btn'))
67+
68+
const popoverRoot = document.querySelector('#test-id-manage-index')
69+
expect(popoverRoot).not.toBeNull()
70+
71+
expect(
72+
screen.getByText(/are you sure you want to delete this index\?/i),
73+
).toBeInTheDocument()
74+
})
75+
})
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import React from 'react'
2+
import { RiPopover, RiPopoverProps } from 'uiSrc/components'
3+
import { Button, IconButton } from 'uiSrc/components/base/forms/buttons'
4+
import {
5+
ButtonWrapper,
6+
IconAndTitleWrapper,
7+
IconWrapper,
8+
PopoverContent,
9+
Title,
10+
} from './styles'
11+
import { DeleteIcon, RiIcon } from 'uiSrc/components/base/icons'
12+
13+
export type DeleteConfirmationButtonProps = Omit<
14+
RiPopoverProps,
15+
'children' | 'button'
16+
> & {
17+
onConfirm: () => void
18+
}
19+
20+
const DeleteConfirmationButton = ({
21+
onConfirm,
22+
...rest
23+
}: DeleteConfirmationButtonProps) => (
24+
<RiPopover
25+
id="manage-index-delete-confirmation"
26+
panelPaddingSize="none"
27+
anchorPosition="downCenter"
28+
{...rest}
29+
button={
30+
<IconButton
31+
icon={DeleteIcon}
32+
data-testid="manage-index-delete-btn"
33+
></IconButton>
34+
}
35+
>
36+
<PopoverContent>
37+
<IconAndTitleWrapper>
38+
<IconWrapper>
39+
<RiIcon color="danger600" type="ToastDangerIcon" />
40+
</IconWrapper>
41+
42+
<Title color="danger">
43+
Are you sure you want to delete this index?
44+
</Title>
45+
</IconAndTitleWrapper>
46+
47+
<ButtonWrapper>
48+
<Button
49+
variant="destructive"
50+
onClick={onConfirm}
51+
data-testid="manage-index-delete-confirmation-btn"
52+
>
53+
Delete
54+
</Button>
55+
</ButtonWrapper>
56+
</PopoverContent>
57+
</RiPopover>
58+
)
59+
60+
export default DeleteConfirmationButton

redisinsight/ui/src/pages/vector-search/manage-indexes/IndexSection.spec.tsx

Lines changed: 27 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
initialStateDefault,
1111
userEvent,
1212
getMswURL,
13+
waitForRiPopoverVisible,
1314
fireEvent,
1415
} from 'uiSrc/utils/test-utils'
1516
import { INSTANCE_ID_MOCK } from 'uiSrc/mocks/handlers/instances/instancesHandlers'
@@ -292,36 +293,38 @@ describe('IndexSection', () => {
292293
})
293294

294295
describe('delete index', () => {
295-
let confirmSpy: jest.SpyInstance
296296
let telemetryMock: jest.Mock
297297

298298
beforeEach(() => {
299-
// Mock window.confirm to return true (user confirms deletion)
300-
confirmSpy = jest.spyOn(window, 'confirm').mockReturnValue(true)
301-
302299
// Mock the telemetry function
303300
telemetryMock = sendEventTelemetry as jest.Mock
304301
telemetryMock.mockClear()
305302
})
306303

307304
afterEach(() => {
308-
confirmSpy.mockRestore()
309305
mswServer.resetHandlers()
310306
})
311307

312308
it('should delete an index when the delete button is clicked and confirmed', async () => {
313309
renderComponent()
314310

315-
const deleteButton = screen.getByText('Delete')
311+
// Trigger confirm dialog
312+
const deleteButton = screen.getByTestId('manage-index-delete-btn')
316313
expect(deleteButton).toBeInTheDocument()
314+
fireEvent.click(deleteButton)
317315

318-
// Click the delete button
319-
await userEvent.click(deleteButton)
316+
// Confirm dialog is visible
317+
await waitForRiPopoverVisible()
318+
expect(
319+
screen.getByText('Are you sure you want to delete this index?'),
320+
).toBeInTheDocument()
320321

321-
// Verify that confirm was called with the correct message
322-
expect(confirmSpy).toHaveBeenCalledWith(
323-
'Are you sure you want to delete this index?',
322+
// Confirm actual delete
323+
const confirmDeleteButton = screen.getByTestId(
324+
'manage-index-delete-confirmation-btn',
324325
)
326+
expect(confirmDeleteButton).toBeInTheDocument()
327+
fireEvent.click(confirmDeleteButton)
325328

326329
// Wait for the success notification to appear
327330
const successNotification = await screen.findByText(
@@ -338,30 +341,6 @@ describe('IndexSection', () => {
338341
})
339342
})
340343

341-
it('should not delete an index when the deletion is cancelled', async () => {
342-
// Mock window.confirm to return false (user cancels deletion)
343-
confirmSpy.mockReturnValueOnce(false)
344-
345-
renderComponent()
346-
347-
const deleteButton = screen.getByText('Delete')
348-
expect(deleteButton).toBeInTheDocument()
349-
350-
// Click the delete button
351-
await userEvent.click(deleteButton)
352-
353-
// Verify that confirm was called with the correct message
354-
expect(confirmSpy).toHaveBeenCalledWith(
355-
'Are you sure you want to delete this index?',
356-
)
357-
358-
// Verify that no API call was made and no notification appears
359-
expect(telemetryMock).not.toHaveBeenCalled()
360-
361-
const successNotification = screen.queryByText('Index has been deleted')
362-
expect(successNotification).not.toBeInTheDocument()
363-
})
364-
365344
it('should handle deletion failure gracefully', async () => {
366345
// Override the MSW handler to return an error for this test
367346
mswServer.use(
@@ -381,16 +360,23 @@ describe('IndexSection', () => {
381360

382361
renderComponent()
383362

384-
const deleteButton = screen.getByText('Delete')
363+
// Trigger confirm dialog
364+
const deleteButton = screen.getByTestId('manage-index-delete-btn')
385365
expect(deleteButton).toBeInTheDocument()
366+
fireEvent.click(deleteButton)
386367

387-
// Click the delete button
388-
await userEvent.click(deleteButton)
368+
// Confirm dialog is visible
369+
await waitForRiPopoverVisible()
370+
expect(
371+
screen.getByText('Are you sure you want to delete this index?'),
372+
).toBeInTheDocument()
389373

390-
// Verify that confirm was called with the correct message
391-
expect(confirmSpy).toHaveBeenCalledWith(
392-
'Are you sure you want to delete this index?',
374+
// Confirm actual delete
375+
const confirmDeleteButton = screen.getByTestId(
376+
'manage-index-delete-confirmation-btn',
393377
)
378+
expect(confirmDeleteButton).toBeInTheDocument()
379+
fireEvent.click(confirmDeleteButton)
394380

395381
// Wait for the error notification to appear
396382
const errorNotification = await screen.findByText(

redisinsight/ui/src/pages/vector-search/manage-indexes/IndexSection.tsx

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
collectManageIndexesDeleteTelemetry,
1919
collectManageIndexesDetailsToggleTelemetry,
2020
} from '../telemetry'
21+
import DeleteConfirmationButton from './DeleteConfirmationButton'
2122

2223
export interface IndexSectionProps extends Omit<SectionProps, 'label'> {
2324
index: RedisString
@@ -26,6 +27,7 @@ export interface IndexSectionProps extends Omit<SectionProps, 'label'> {
2627
export const IndexSection = ({ index, ...rest }: IndexSectionProps) => {
2728
const dispatch = useDispatch()
2829
const indexName = bufferToString(index)
30+
const [isPopoverOpen, setIsPopoverOpen] = useState(false)
2931
const { id: instanceId } = useSelector(connectedInstanceSelector)
3032

3133
const [indexInfo, setIndexInfo] = useState<IndexInfoDto>()
@@ -45,17 +47,11 @@ export const IndexSection = ({ index, ...rest }: IndexSectionProps) => {
4547
}, [indexName, dispatch])
4648

4749
const handleDelete = () => {
48-
// TODO: Replace with confirmation popup once the design is ready
49-
// eslint-disable-next-line no-restricted-globals
50-
const result = confirm('Are you sure you want to delete this index?')
51-
52-
if (result) {
53-
const data: IndexDeleteRequestBodyDto = {
54-
index: stringToBuffer(indexName),
55-
}
56-
57-
dispatch(deleteRedisearchIndexAction(data, onDeletedIndexSuccess))
50+
const data: IndexDeleteRequestBodyDto = {
51+
index: stringToBuffer(indexName),
5852
}
53+
54+
dispatch(deleteRedisearchIndexAction(data, onDeletedIndexSuccess))
5955
}
6056

6157
const onDeletedIndexSuccess = () => {
@@ -71,20 +67,40 @@ export const IndexSection = ({ index, ...rest }: IndexSectionProps) => {
7167
})
7268
}
7369

70+
// TODO: Add FieldTag component to list the types of the different fields
7471
return (
75-
<Section
72+
<Section.Compose
7673
collapsible
7774
collapsedInfo={<CategoryValueList categoryValueList={indexSummaryInfo} />}
7875
content={<IndexAttributesList indexInfo={indexInfo} />}
7976
// TODO: Add FieldTag component to list the types of the different fields
80-
label={formatLongName(indexName)}
8177
defaultOpen={false}
82-
actionButtonText="Delete" // TODO: Replace with an icon of a trash can
83-
onAction={handleDelete}
8478
onOpenChange={handleOpenChange}
79+
onAction={() => setIsPopoverOpen(true)}
8580
data-testid={`manage-indexes-list--item--${indexName}`}
8681
{...rest}
87-
/>
82+
>
83+
<Section.Header.Compose
84+
collapsedInfo={
85+
<CategoryValueList categoryValueList={indexSummaryInfo} />
86+
}
87+
>
88+
<Section.Header.Group>
89+
<Section.Header.Label label={formatLongName(indexName)} />
90+
</Section.Header.Group>
91+
<Section.Header.Group>
92+
<Section.Header.ActionButton onClick={() => setIsPopoverOpen(true)}>
93+
<DeleteConfirmationButton
94+
isOpen={isPopoverOpen}
95+
closePopover={() => setIsPopoverOpen(false)}
96+
onConfirm={handleDelete}
97+
/>
98+
</Section.Header.ActionButton>
99+
<Section.Header.CollapseIndicator />
100+
</Section.Header.Group>
101+
</Section.Header.Compose>
102+
<Section.Body content={<IndexAttributesList indexInfo={indexInfo} />} />
103+
</Section.Compose>
88104
)
89105
}
90106

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import styled from 'styled-components'
2+
import { Text } from 'uiSrc/components/base/text'
3+
4+
export const PopoverContent = styled.div`
5+
padding: ${({ theme }) => theme.core?.space.space200};
6+
`
7+
8+
export const Title = styled(Text)`
9+
margin-top: ${({ theme }) => theme.core?.space.space100};
10+
margin-bottom: ${({ theme }) => theme.core?.space.space100};
11+
font-weight: bold;
12+
color: ${({ theme }) => theme.color.danger500};
13+
`
14+
15+
export const IconWrapper = styled.div`
16+
text-align: center;
17+
`
18+
export const ButtonWrapper = styled.div`
19+
margin-top: ${({ theme }) => theme.core?.space.space100};
20+
display: flex;
21+
justify-content: flex-end;
22+
`
23+
24+
export const IconAndTitleWrapper = styled.div`
25+
display: flex;
26+
gap: ${({ theme }) => theme.core?.space.space100};
27+
align-items: center;
28+
`

0 commit comments

Comments
 (0)