Skip to content

Commit ee1da3b

Browse files
fix(lightspeeed): Enable only supported file types while uploading attachments (#913)
* fix(lightspeeed): Enable only supported file types while uploading attachments * fix unit tests * add xml to supported file types * add xml to supported file types * update playwright tests * update chatbot placeholder text * fix e2e tests * move alerts to the top of the page * add onAttachRejected handler to surface invalid file type errors
1 parent 01e48e1 commit ee1da3b

13 files changed

Lines changed: 140 additions & 51 deletions

File tree

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+
enable only supported file types in file picker

workspaces/lightspeed/packages/app/e2e-tests/lightspeed.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ import { openLightspeed, sendMessage } from './utils/testHelper';
3030
import {
3131
uploadFile,
3232
uploadAndAssertDuplicate,
33-
validateFailedUpload,
3433
supportedFileTypes,
34+
validateFailedUpload,
3535
} from './utils/fileUpload';
3636
import {
3737
assertChatDialogInitialState,
@@ -142,6 +142,12 @@ test.describe('File Attachment Validation', () => {
142142
await uploadAndAssertDuplicate(page, path, name);
143143
} else {
144144
await validateFailedUpload(page);
145+
// Unsupported files will not be available to preview.
146+
const filePreview = page
147+
.locator('span', { hasText: name.split('.')[0] })
148+
.first();
149+
150+
await expect(filePreview).not.toBeVisible();
145151
}
146152
});
147153
}

workspaces/lightspeed/packages/app/e2e-tests/utils/fileUpload.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
import { Page, Locator, FileChooser, expect } from '@playwright/test';
1717

18-
export const supportedFileTypes = ['.txt', '.yaml', '.json'];
18+
export const supportedFileTypes = ['.txt', '.yaml', '.json', '.xml'];
1919

2020
export async function triggerFileChooser(
2121
page: Page,
@@ -76,14 +76,14 @@ export async function validateSuccessfulUpload(page: Page, fileName: string) {
7676
await page.getByRole('button', { name: 'Save' }).click();
7777
await page
7878
.getByRole('contentinfo')
79-
.getByRole('button', { name: 'Close' })
79+
.locator('role=button[name="Close"]')
8080
.click();
8181
}
8282

8383
export async function validateFailedUpload(page: Page) {
8484
const alertHeader = page.getByText('File upload failed');
8585
const alertText = page.getByText(
86-
'Unsupported file type. Supported types are: .txt, .yaml, .json.',
86+
'Unsupported file type. Supported types are: .txt, .yaml, .json and .xml.',
8787
);
8888

8989
await expect(alertHeader).toBeVisible();

workspaces/lightspeed/packages/app/e2e-tests/utils/testHelper.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ export const openLightspeed = async (page: Page) => {
2424
};
2525

2626
export const sendMessage = async (message: string, page: Page) => {
27-
const inputLocator = page.getByRole('textbox', { name: 'Send a message...' });
27+
const inputLocator = page.getByRole('textbox', {
28+
name: 'Send a message and optionally upload a JSON, YAML, TXT, or XML file...',
29+
});
2830
await inputLocator.fill(message);
2931
const sendButton = page.getByRole('button', { name: 'Send' });
3032
await sendButton.click();

workspaces/lightspeed/plugins/lightspeed/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"@material-ui/core": "^4.9.13",
4545
"@material-ui/lab": "^4.0.0-alpha.61",
4646
"@mui/icons-material": "^6.1.8",
47-
"@patternfly/chatbot": "6.3.0-prerelease.20",
47+
"@patternfly/chatbot": "6.3.0-prerelease.23",
4848
"@patternfly/react-core": "6.3.0-prerelease.17",
4949
"@red-hat-developer-hub/backstage-plugin-lightspeed-common": "workspace:^",
5050
"@tanstack/react-query": "^5.59.15",

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ interface FileAttachmentContextType {
2727
handleFileUpload: (files: File[]) => void;
2828
setFileContents: React.Dispatch<React.SetStateAction<FileContent[]>>;
2929
setUploadError: React.Dispatch<React.SetStateAction<UploadError>>;
30+
setShowAlert: React.Dispatch<React.SetStateAction<boolean>>;
3031
currentFileContent?: FileContent;
3132
setCurrentFileContent: React.Dispatch<
3233
React.SetStateAction<FileContent | undefined>
@@ -85,7 +86,7 @@ const FileAttachmentContextProvider: React.FC<{
8586
setShowAlert(true);
8687
setUploadError({
8788
message:
88-
'Unsupported file type. Supported types are: .txt, .yaml, .json.',
89+
'Unsupported file type. Supported types are: .txt, .yaml, .json and .xml.',
8990
});
9091
return;
9192
}
@@ -138,6 +139,7 @@ const FileAttachmentContextProvider: React.FC<{
138139
handleFileUpload,
139140
isLoadingFile,
140141
showAlert,
142+
setShowAlert,
141143
uploadError,
142144
setUploadError,
143145
currentFileContent,

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

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,16 @@
1616
import React from 'react';
1717

1818
import { Divider } from '@material-ui/core';
19-
import { ChatbotAlert, FileDetailsLabel } from '@patternfly/chatbot';
19+
import { FileDetailsLabel } from '@patternfly/chatbot';
2020

2121
import { useFileAttachmentContext } from './AttachmentContext';
2222

2323
const FilePreview = () => {
2424
const {
25-
showAlert,
2625
fileContents,
2726
isLoadingFile,
28-
uploadError,
2927
modalState,
3028
setFileContents,
31-
setUploadError,
3229
setCurrentFileContent,
3330
} = useFileAttachmentContext();
3431

@@ -71,17 +68,6 @@ const FilePreview = () => {
7168
))}
7269
</div>
7370
)}
74-
{showAlert && uploadError.message && (
75-
<ChatbotAlert
76-
component="h4"
77-
title="File upload failed"
78-
variant={uploadError.type ?? 'danger'}
79-
isInline
80-
onClose={() => setUploadError({ message: null })}
81-
>
82-
{uploadError.message}
83-
</ChatbotAlert>
84-
)}
8571
</>
8672
);
8773
};

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

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@
1515
*/
1616

1717
import React from 'react';
18+
import { FileRejection } from 'react-dropzone/.';
1819

1920
import { ErrorPanel } from '@backstage/core-components';
2021

2122
import { Box, makeStyles } from '@material-ui/core';
2223
import {
2324
Chatbot,
25+
ChatbotAlert,
2426
ChatbotContent,
2527
ChatbotDisplayMode,
2628
ChatbotFooter,
@@ -71,6 +73,9 @@ const useStyles = makeStyles(theme => ({
7173
header: {
7274
padding: `${theme.spacing(3)}px !important`,
7375
},
76+
errorContainer: {
77+
padding: theme.spacing(3),
78+
},
7479
headerMenu: {
7580
// align hamburger icon with title
7681
'& .pf-v6-c-button': {
@@ -129,8 +134,15 @@ export const LightspeedChat = ({
129134
const { isReady, lastOpenedId, setLastOpenedId, clearLastOpenedId } =
130135
useLastOpenedConversation(user);
131136

132-
const { fileContents, setFileContents, handleFileUpload } =
133-
useFileAttachmentContext();
137+
const {
138+
uploadError,
139+
showAlert,
140+
fileContents,
141+
setShowAlert,
142+
setFileContents,
143+
handleFileUpload,
144+
setUploadError,
145+
} = useFileAttachmentContext();
134146

135147
// Sync conversationId with lastOpenedId whenever lastOpenedId changes
136148
React.useEffect(() => {
@@ -346,6 +358,18 @@ export const LightspeedChat = ({
346358
handleFileUpload(data);
347359
};
348360

361+
const onAttachRejected = (data: FileRejection[]) => {
362+
data.forEach(attachment => {
363+
if (!!attachment.errors.find(e => e.code === 'file-invalid-type')) {
364+
setShowAlert(true);
365+
setUploadError({
366+
message:
367+
'Unsupported file type. Supported types are: .txt, .yaml, .json and .xml.',
368+
});
369+
}
370+
});
371+
};
372+
349373
if (error) {
350374
return (
351375
<Box padding={1}>
@@ -401,6 +425,19 @@ export const LightspeedChat = ({
401425
handleTextInputChange={handleFilter}
402426
drawerContent={
403427
<>
428+
{showAlert && uploadError.message && (
429+
<div className={classes.errorContainer}>
430+
<ChatbotAlert
431+
component="h4"
432+
title="File upload failed"
433+
variant={uploadError.type ?? 'danger'}
434+
isInline
435+
onClose={() => setUploadError({ message: null })}
436+
>
437+
{uploadError.message}
438+
</ChatbotAlert>
439+
</div>
440+
)}
404441
<ChatbotContent>
405442
<LightspeedChatBox
406443
userName={userName}
@@ -419,6 +456,19 @@ export const LightspeedChat = ({
419456
hasAttachButton
420457
handleAttach={handleAttach}
421458
hasMicrophoneButton
459+
buttonProps={{
460+
attach: {
461+
inputTestId: 'attachment-input',
462+
},
463+
}}
464+
allowedFileTypes={{
465+
'text/plain': ['.txt'],
466+
'application/json': ['.json'],
467+
'application/yaml': ['.yaml', '.yml'],
468+
'application/xml': ['.xml'],
469+
}}
470+
onAttachRejected={onAttachRejected}
471+
placeholder="Send a message and optionally upload a JSON, YAML, TXT, or XML file..."
422472
/>
423473
<ChatbotFootnote {...getFootnoteProps(classes.footerPopover)} />
424474
</ChatbotFooter>

workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/LightspeedChat.test.tsx

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ import { mockApis, TestApiProvider } from '@backstage/test-utils';
2525

2626
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
2727
import { render, screen, waitFor } from '@testing-library/react';
28+
import userEvent from '@testing-library/user-event';
2829

30+
import { useConversations } from '../../hooks';
2931
import FileAttachmentContextProvider from '../AttachmentContext';
3032
import { LightspeedChat } from '../LightSpeedChat';
3133

@@ -80,6 +82,7 @@ jest.mock('@patternfly/chatbot', () => {
8082
};
8183
});
8284

85+
const mockUseConversations = useConversations as jest.Mock;
8386
const mockUsePermission = usePermission as jest.MockedFunction<
8487
typeof usePermission
8588
>;
@@ -111,6 +114,13 @@ const setupLightspeedChat = () => (
111114
describe('LightspeedChat', () => {
112115
beforeEach(() => {
113116
mockUsePermission.mockReturnValue({ loading: true, allowed: true });
117+
mockUseConversations.mockReturnValue({
118+
data: [],
119+
isRefetching: false,
120+
isLoading: false,
121+
});
122+
123+
localStorage.clear();
114124
});
115125
const localStorageKey = 'lastOpenedConversation';
116126
const mockUser = 'user:test';
@@ -124,19 +134,17 @@ describe('LightspeedChat', () => {
124134
});
125135

126136
it('should not reset localstorage if the conversations are available', async () => {
127-
jest.mock('../../hooks/useConversations', () => ({
128-
useConversations: jest.fn().mockReturnValue({
129-
data: [
130-
{
131-
conversation_id: 'test-conversation-id',
132-
topic_summary: 'Greetings',
133-
last_message_timestamp: 1749023603.806369,
134-
},
135-
],
136-
isRefetching: false,
137-
isLoading: false,
138-
}),
139-
}));
137+
mockUseConversations.mockReturnValue({
138+
data: [
139+
{
140+
conversation_id: 'test-conversation-id',
141+
topic_summary: 'Greetings',
142+
last_message_timestamp: 1749023603.806369,
143+
},
144+
],
145+
isRefetching: false,
146+
isLoading: false,
147+
});
140148

141149
const storedData = JSON.stringify({ [mockUser]: 'test-conversation-id' });
142150
localStorage.setItem(localStorageKey, storedData);
@@ -146,14 +154,17 @@ describe('LightspeedChat', () => {
146154
await waitFor(() => {
147155
expect(screen.getByText('Developer Hub Lightspeed')).toBeInTheDocument();
148156

149-
expect(screen.queryByText('New chat')).not.toBeInTheDocument();
150-
expect(JSON.parse(localStorage.getItem(localStorageKey)!)).toEqual({});
157+
expect(screen.queryByText('New chat')).toBeInTheDocument();
158+
159+
expect(JSON.parse(localStorage.getItem(localStorageKey)!)).toEqual({
160+
'user:test': 'test-conversation-id',
161+
});
151162
});
152163
});
153164

154165
it('should reset localstorage if the conversations are empty', async () => {
155-
const storedData = JSON.stringify({ [mockUser]: 'test-conversation-id' });
156-
localStorage.setItem(localStorageKey, storedData);
166+
const initialData = JSON.stringify({ [mockUser]: 'test-conversation-id' });
167+
localStorage.setItem(localStorageKey, initialData);
157168

158169
render(setupLightspeedChat());
159170

@@ -164,4 +175,15 @@ describe('LightspeedChat', () => {
164175
expect(JSON.parse(localStorage.getItem(localStorageKey)!)).toEqual({});
165176
});
166177
});
178+
179+
it('should set correct accept attribute on file input', async () => {
180+
render(setupLightspeedChat());
181+
182+
await userEvent.click(screen.getByRole('button', { name: 'Attach' }));
183+
const input = screen.getByTestId('attachment-input') as HTMLInputElement;
184+
expect(input).toHaveAttribute(
185+
'accept',
186+
'text/plain,.txt,application/json,.json,application/yaml,.yaml,.yml,application/xml,.xml',
187+
);
188+
});
167189
});

workspaces/lightspeed/plugins/lightspeed/src/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export enum SupportedFileType {
5555
JSON = 'application/json',
5656
YAML = 'application/x-yaml',
5757
TEXT = 'text/plain',
58+
XML = 'text/xml',
5859
}
5960
export interface FileContent {
6061
content: string;

0 commit comments

Comments
 (0)