Skip to content

Commit d621f93

Browse files
fix(lightspeed): improve notebook upload modal and document overwrite UX (#2936)
* fix(lightspeed): improve notebook upload modal and MessageBar UX Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> * toast message auto dismiss Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> * fix(lightspeed): fix overwrite flow Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> * updating changeset Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> * showing model from app-config for displaying response Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> * disabling upload when 10 files are there Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> * updating changeset Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> * hadnling duplicates file Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> * updating api report Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> --------- Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
1 parent 1876c00 commit d621f93

18 files changed

Lines changed: 864 additions & 89 deletions
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-lightspeed': patch
3+
---
4+
5+
Improved notebook upload modal and MessageBar UX.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-lightspeed': patch
3+
---
4+
5+
Fixed overwrite flow to add duplicate files to the Add Document modal instead of uploading immediately. Reduced notebook delete toast timeout to 2 seconds.

workspaces/lightspeed/plugins/lightspeed/report-alpha.api.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,17 +230,22 @@ export const lightspeedTranslationRef: TranslationRef<
230230
readonly 'notebook.view.upload.heading': string;
231231
readonly 'notebook.view.upload.action': string;
232232
readonly 'notebook.view.input.placeholder': string;
233+
readonly 'notebook.view.input.disabledTooltip': string;
233234
readonly 'notebook.view.sidebar.collapse': string;
234235
readonly 'notebook.view.sidebar.expand': string;
235236
readonly 'notebook.view.sidebar.resize': string;
236237
readonly 'notebook.view.documents.uploading': string;
238+
readonly 'notebook.view.documents.maxReached': string;
237239
readonly 'notebook.upload.success': string;
238240
readonly 'notebook.upload.failed': string;
239241
readonly 'notebook.upload.modal.title': string;
240242
readonly 'notebook.upload.modal.dragDropTitle': string;
241243
readonly 'notebook.upload.modal.browseButton': string;
242244
readonly 'notebook.upload.modal.separator': string;
243245
readonly 'notebook.upload.modal.infoText': string;
246+
readonly 'notebook.upload.modal.selectedFiles': string;
247+
readonly 'notebook.upload.modal.addButton': string;
248+
readonly 'notebook.upload.modal.removeFile': string;
244249
readonly 'notebook.upload.error.unsupportedType': string;
245250
readonly 'notebook.upload.error.fileTooLarge': string;
246251
readonly 'notebook.upload.error.tooManyFiles': string;

workspaces/lightspeed/plugins/lightspeed/src/components/LightSpeedChat.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1570,7 +1570,7 @@ export const LightspeedChat = ({
15701570
variant={AlertVariant[variant ?? 'success']}
15711571
title={title}
15721572
className={classes.toastAlert}
1573-
timeout={8000}
1573+
timeout={2000}
15741574
onTimeout={() => handleRemoveNotebookAlert(key as React.Key)}
15751575
actionClose={
15761576
<AlertActionCloseButton
@@ -1768,6 +1768,7 @@ export const LightspeedChat = ({
17681768
avatar={avatar}
17691769
profileLoading={profileLoading}
17701770
topicRestrictionEnabled={topicRestrictionEnabled}
1771+
selectedModel={selectedModel}
17711772
onClose={handleCloseNotebook}
17721773
/>
17731774
)}
Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
/*
2+
* Copyright Red Hat, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
18+
19+
import { mockUseTranslation } from '../../test-utils/mockTranslations';
20+
import { AddDocumentModal } from '../notebooks/AddDocumentModal';
21+
22+
jest.mock('../../hooks/useTranslation', () => ({
23+
useTranslation: jest.fn(() => mockUseTranslation()),
24+
}));
25+
26+
const mockMutateAsync = jest.fn();
27+
jest.mock('../../hooks/notebooks/useUploadDocument', () => ({
28+
useUploadDocument: () => ({
29+
mutateAsync: mockMutateAsync,
30+
}),
31+
}));
32+
33+
describe('AddDocumentModal', () => {
34+
const defaultProps = {
35+
isOpen: true,
36+
onClose: jest.fn(),
37+
sessionId: 'test-session-id',
38+
existingDocumentNames: [],
39+
onFilesUploading: jest.fn(),
40+
onUploadStarted: jest.fn(),
41+
onUploadFailed: jest.fn(),
42+
onDuplicatesFound: jest.fn(),
43+
};
44+
45+
beforeEach(() => {
46+
jest.clearAllMocks();
47+
mockMutateAsync.mockResolvedValue({ document_id: 'test-doc-id' });
48+
});
49+
50+
it('should render the modal when open', () => {
51+
render(<AddDocumentModal {...defaultProps} />);
52+
53+
expect(screen.getByText('Add a document to Notebook')).toBeInTheDocument();
54+
expect(screen.getByText('Drag and drop files here')).toBeInTheDocument();
55+
});
56+
57+
it('should not render when isOpen is false', () => {
58+
render(<AddDocumentModal {...defaultProps} isOpen={false} />);
59+
60+
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
61+
});
62+
63+
it('should render Cancel and Add buttons', () => {
64+
render(<AddDocumentModal {...defaultProps} />);
65+
66+
expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument();
67+
expect(screen.getByRole('button', { name: 'Add (0)' })).toBeInTheDocument();
68+
});
69+
70+
it('should have Add button disabled when no files selected', () => {
71+
render(<AddDocumentModal {...defaultProps} />);
72+
73+
const addButton = screen.getByRole('button', { name: 'Add (0)' });
74+
expect(addButton).toBeDisabled();
75+
});
76+
77+
it('should call onClose when Cancel button is clicked', () => {
78+
render(<AddDocumentModal {...defaultProps} />);
79+
80+
const cancelButton = screen.getByRole('button', { name: 'Cancel' });
81+
fireEvent.click(cancelButton);
82+
83+
expect(defaultProps.onClose).toHaveBeenCalledTimes(1);
84+
});
85+
86+
it('should call onClose when close icon is clicked', () => {
87+
render(<AddDocumentModal {...defaultProps} />);
88+
89+
const closeButton = screen.getByRole('button', { name: 'Close' });
90+
fireEvent.click(closeButton);
91+
92+
expect(defaultProps.onClose).toHaveBeenCalledTimes(1);
93+
});
94+
95+
it('should display file list when files are dropped', async () => {
96+
render(<AddDocumentModal {...defaultProps} />);
97+
98+
const dropzone = screen
99+
.getByText('Drag and drop files here')
100+
.closest('div');
101+
const file = new File(['content'], 'test-file.txt', { type: 'text/plain' });
102+
103+
fireEvent.drop(dropzone!, {
104+
dataTransfer: {
105+
files: [file],
106+
types: ['Files'],
107+
},
108+
});
109+
110+
await waitFor(() => {
111+
expect(screen.getByText('test-file.txt')).toBeInTheDocument();
112+
});
113+
});
114+
115+
it('should update Add button count when files are selected', async () => {
116+
render(<AddDocumentModal {...defaultProps} />);
117+
118+
const dropzone = screen
119+
.getByText('Drag and drop files here')
120+
.closest('div');
121+
const file = new File(['content'], 'test-file.txt', { type: 'text/plain' });
122+
123+
fireEvent.drop(dropzone!, {
124+
dataTransfer: {
125+
files: [file],
126+
types: ['Files'],
127+
},
128+
});
129+
130+
await waitFor(() => {
131+
expect(
132+
screen.getByRole('button', { name: 'Add (1)' }),
133+
).toBeInTheDocument();
134+
});
135+
});
136+
137+
it('should not auto-close modal after file drop', async () => {
138+
render(<AddDocumentModal {...defaultProps} />);
139+
140+
const dropzone = screen
141+
.getByText('Drag and drop files here')
142+
.closest('div');
143+
const file = new File(['content'], 'test-file.txt', { type: 'text/plain' });
144+
145+
fireEvent.drop(dropzone!, {
146+
dataTransfer: {
147+
files: [file],
148+
types: ['Files'],
149+
},
150+
});
151+
152+
await waitFor(() => {
153+
expect(screen.getByText('test-file.txt')).toBeInTheDocument();
154+
});
155+
156+
expect(defaultProps.onClose).not.toHaveBeenCalled();
157+
});
158+
159+
it('should trigger upload and close when Add button is clicked', async () => {
160+
render(<AddDocumentModal {...defaultProps} />);
161+
162+
const dropzone = screen
163+
.getByText('Drag and drop files here')
164+
.closest('div');
165+
const file = new File(['content'], 'test-file.txt', { type: 'text/plain' });
166+
167+
fireEvent.drop(dropzone!, {
168+
dataTransfer: {
169+
files: [file],
170+
types: ['Files'],
171+
},
172+
});
173+
174+
await waitFor(() => {
175+
expect(
176+
screen.getByRole('button', { name: 'Add (1)' }),
177+
).not.toBeDisabled();
178+
});
179+
180+
fireEvent.click(screen.getByRole('button', { name: 'Add (1)' }));
181+
182+
await waitFor(() => {
183+
expect(defaultProps.onFilesUploading).toHaveBeenCalledWith([file]);
184+
expect(mockMutateAsync).toHaveBeenCalledWith({
185+
sessionId: 'test-session-id',
186+
file,
187+
});
188+
expect(defaultProps.onClose).toHaveBeenCalled();
189+
});
190+
});
191+
192+
it('should allow removing files from the list', async () => {
193+
render(<AddDocumentModal {...defaultProps} />);
194+
195+
const dropzone = screen
196+
.getByText('Drag and drop files here')
197+
.closest('div');
198+
const file = new File(['content'], 'test-file.txt', { type: 'text/plain' });
199+
200+
fireEvent.drop(dropzone!, {
201+
dataTransfer: {
202+
files: [file],
203+
types: ['Files'],
204+
},
205+
});
206+
207+
await waitFor(() => {
208+
expect(screen.getByText('test-file.txt')).toBeInTheDocument();
209+
});
210+
211+
const removeButton = screen.getByRole('button', {
212+
name: 'Remove test-file.txt',
213+
});
214+
fireEvent.click(removeButton);
215+
216+
await waitFor(() => {
217+
expect(screen.queryByText('test-file.txt')).not.toBeInTheDocument();
218+
expect(screen.getByRole('button', { name: 'Add (0)' })).toBeDisabled();
219+
});
220+
});
221+
222+
it('should clear selected files when modal is closed', async () => {
223+
const { rerender } = render(<AddDocumentModal {...defaultProps} />);
224+
225+
const dropzone = screen
226+
.getByText('Drag and drop files here')
227+
.closest('div');
228+
const file = new File(['content'], 'test-file.txt', { type: 'text/plain' });
229+
230+
fireEvent.drop(dropzone!, {
231+
dataTransfer: {
232+
files: [file],
233+
types: ['Files'],
234+
},
235+
});
236+
237+
await waitFor(() => {
238+
expect(screen.getByText('test-file.txt')).toBeInTheDocument();
239+
});
240+
241+
fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
242+
243+
rerender(<AddDocumentModal {...defaultProps} isOpen />);
244+
245+
expect(screen.queryByText('test-file.txt')).not.toBeInTheDocument();
246+
});
247+
248+
it('should call onDuplicatesFound for files that already exist', async () => {
249+
render(
250+
<AddDocumentModal
251+
{...defaultProps}
252+
existingDocumentNames={['existing-file.txt']}
253+
/>,
254+
);
255+
256+
const dropzone = screen
257+
.getByText('Drag and drop files here')
258+
.closest('div');
259+
const existingFile = new File(['content'], 'existing-file.txt', {
260+
type: 'text/plain',
261+
});
262+
const newFile = new File(['content'], 'new-file.txt', {
263+
type: 'text/plain',
264+
});
265+
266+
fireEvent.drop(dropzone!, {
267+
dataTransfer: {
268+
files: [existingFile, newFile],
269+
types: ['Files'],
270+
},
271+
});
272+
273+
await waitFor(() => {
274+
expect(defaultProps.onDuplicatesFound).toHaveBeenCalledWith([
275+
existingFile,
276+
]);
277+
expect(screen.getByText('new-file.txt')).toBeInTheDocument();
278+
expect(screen.queryByText('existing-file.txt')).not.toBeInTheDocument();
279+
});
280+
});
281+
});

0 commit comments

Comments
 (0)