Skip to content

Commit 93fa729

Browse files
PhilippMaPhilippMa
committed
feat: only validate on import and move error display from helper text to log display
* not using form helper texts is more consistent with other forms in OpossumUI * only validating on import drastically reduces the state variables necessary in import dialog * to still get file overwrite warnings, force users to select file paths through system dialogs
1 parent 49b1920 commit 93fa729

File tree

16 files changed

+200
-449
lines changed

16 files changed

+200
-449
lines changed

‎src/ElectronBackend/main/__tests__/listeners.test.ts

-128
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
ExportSpdxDocumentJsonArgs,
1616
ExportSpdxDocumentYamlArgs,
1717
ExportType,
18-
FilePathValidity,
1918
} from '../../../shared/shared-types';
2019
import { writeFile } from '../../../shared/write-file';
2120
import { faker } from '../../../testing/Faker';
@@ -36,7 +35,6 @@ import {
3635
getImportFileListener,
3736
getImportFileSelectInputListener,
3837
getImportFileSelectSaveLocationListener,
39-
getImportFileValidatePathsListener,
4038
getOpenLinkListener,
4139
getSelectBaseURLListener,
4240
linkHasHttpSchema,
@@ -526,132 +524,6 @@ describe('getImportFileSelectSaveLocationListener', () => {
526524
});
527525
});
528526

529-
describe('getImportFileValidatePathsListener', () => {
530-
it('recognizes empty strings', async () => {
531-
const mainWindow = await initWindowAndBackendState();
532-
533-
const inputFilePath = '';
534-
const extensions = ['json', 'json.gz'];
535-
const opossumFilePath = ' \t';
536-
537-
const listener = getImportFileValidatePathsListener(mainWindow);
538-
539-
jest.spyOn(fs, 'existsSync').mockReturnValue(true);
540-
541-
const result = await listener(
542-
{} as Electron.IpcMainInvokeEvent,
543-
inputFilePath,
544-
extensions,
545-
opossumFilePath,
546-
);
547-
548-
expect(result).toStrictEqual([
549-
FilePathValidity.EMPTY_STRING,
550-
FilePathValidity.EMPTY_STRING,
551-
]);
552-
});
553-
554-
it('recognizes non-existent file paths', async () => {
555-
const mainWindow = await initWindowAndBackendState();
556-
557-
const inputFilePath = '/home/input.json';
558-
const extensions = ['json', 'json.gz'];
559-
const opossumFilePath = '/hom/input.opossum';
560-
561-
const listener = getImportFileValidatePathsListener(mainWindow);
562-
563-
jest.spyOn(fs, 'existsSync').mockReturnValue(false);
564-
565-
const result = await listener(
566-
{} as Electron.IpcMainInvokeEvent,
567-
inputFilePath,
568-
extensions,
569-
opossumFilePath,
570-
);
571-
572-
expect(result).toStrictEqual([
573-
FilePathValidity.PATH_DOESNT_EXIST,
574-
FilePathValidity.PATH_DOESNT_EXIST,
575-
]);
576-
});
577-
578-
it('recognizes invalid file extensions', async () => {
579-
const mainWindow = await initWindowAndBackendState();
580-
581-
const inputFilePath = '/home/input.txt';
582-
const extensions = ['json', 'json.gz'];
583-
const opossumFilePath = '/home/input.oposs';
584-
585-
const listener = getImportFileValidatePathsListener(mainWindow);
586-
587-
jest.spyOn(fs, 'existsSync').mockReturnValue(true);
588-
589-
const result = await listener(
590-
{} as Electron.IpcMainInvokeEvent,
591-
inputFilePath,
592-
extensions,
593-
opossumFilePath,
594-
);
595-
596-
expect(result).toStrictEqual([
597-
FilePathValidity.WRONG_EXTENSION,
598-
FilePathValidity.WRONG_EXTENSION,
599-
]);
600-
});
601-
602-
it('returns valid when no problem is found', async () => {
603-
const mainWindow = await initWindowAndBackendState();
604-
605-
const inputFilePath = '/home/input.json.gz';
606-
const extensions = ['json', 'json.gz'];
607-
const opossumFilePath = '/home/input.opossum';
608-
609-
const listener = getImportFileValidatePathsListener(mainWindow);
610-
611-
jest
612-
.spyOn(fs, 'existsSync')
613-
.mockReturnValueOnce(true)
614-
.mockReturnValueOnce(true)
615-
.mockReturnValueOnce(false);
616-
617-
const result = await listener(
618-
{} as Electron.IpcMainInvokeEvent,
619-
inputFilePath,
620-
extensions,
621-
opossumFilePath,
622-
);
623-
624-
expect(result).toStrictEqual([
625-
FilePathValidity.VALID,
626-
FilePathValidity.VALID,
627-
]);
628-
});
629-
630-
it('gives overwrite warning when opossum file already exists', async () => {
631-
const mainWindow = await initWindowAndBackendState();
632-
633-
const inputFilePath = '/home/input.json.gz';
634-
const extensions = ['json', 'json.gz'];
635-
const opossumFilePath = '/home/input.opossum';
636-
637-
const listener = getImportFileValidatePathsListener(mainWindow);
638-
639-
jest.spyOn(fs, 'existsSync').mockReturnValue(true);
640-
641-
const result = await listener(
642-
{} as Electron.IpcMainInvokeEvent,
643-
inputFilePath,
644-
extensions,
645-
opossumFilePath,
646-
);
647-
648-
expect(result).toStrictEqual([
649-
FilePathValidity.VALID,
650-
FilePathValidity.OVERWRITE_WARNING,
651-
]);
652-
});
653-
});
654-
655527
function initWindowAndBackendState(): Promise<BrowserWindow> {
656528
(writeCsvToFile as jest.Mock).mockReset();
657529
setGlobalBackendState({});

‎src/ElectronBackend/main/listeners.ts

+13-64
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import {
1919
ExportSpdxDocumentYamlArgs,
2020
ExportType,
2121
FileFormatInfo,
22-
FilePathValidity,
2322
OpenLinkArgs,
2423
PackageInfo,
2524
SaveFileArgs,
@@ -188,11 +187,23 @@ export function getImportFileConvertAndLoadListener(
188187
resourceFilePath: string,
189188
opossumFilePath: string,
190189
) => {
191-
if (!fs.existsSync(resourceFilePath)) {
190+
if (!resourceFilePath.trim() || !fs.existsSync(resourceFilePath)) {
191+
logger.error('Input file does not exist');
192+
return false;
193+
}
194+
195+
if (!opossumFilePath.trim()) {
196+
logger.error('No .opossum save location selected');
197+
return false;
198+
}
199+
200+
if (!opossumFilePath.endsWith('.opossum')) {
201+
logger.error('Output file name must have .opossum extension');
192202
return false;
193203
}
194204

195205
if (!fs.existsSync(path.dirname(opossumFilePath))) {
206+
logger.error('Output directory does not exist');
196207
return false;
197208
}
198209

@@ -218,68 +229,6 @@ export function getImportFileConvertAndLoadListener(
218229
);
219230
}
220231

221-
export function getImportFileValidatePathsListener(
222-
mainWindow: BrowserWindow,
223-
): (
224-
_: Electron.IpcMainInvokeEvent,
225-
inputFilePath: string,
226-
extensions: Array<string>,
227-
opossumFilePath: string,
228-
) => Promise<[FilePathValidity, FilePathValidity] | null> {
229-
return createListenerCallbackWithErrorHandling(
230-
mainWindow,
231-
(
232-
_: Electron.IpcMainInvokeEvent,
233-
inputFilePath: string,
234-
extensions: Array<string>,
235-
opossumFilePath: string,
236-
): [FilePathValidity, FilePathValidity] => {
237-
const inputFilePathExists = fs.existsSync(inputFilePath);
238-
const opossumFileDirectoryExists = fs.existsSync(
239-
path.dirname(opossumFilePath),
240-
);
241-
const opossumFileAlreadyExists = fs.existsSync(opossumFilePath);
242-
243-
const inputFilePathValidity = validateFilePathFormat(
244-
inputFilePath,
245-
extensions,
246-
inputFilePathExists,
247-
);
248-
249-
const opossumFilePathValidity = validateFilePathFormat(
250-
opossumFilePath,
251-
['opossum'],
252-
opossumFileDirectoryExists,
253-
);
254-
const opossumFilePathValidityWithWarning =
255-
opossumFilePathValidity === FilePathValidity.VALID &&
256-
opossumFileAlreadyExists
257-
? FilePathValidity.OVERWRITE_WARNING
258-
: opossumFilePathValidity;
259-
260-
return [inputFilePathValidity, opossumFilePathValidityWithWarning];
261-
},
262-
);
263-
}
264-
265-
function validateFilePathFormat(
266-
filePath: string,
267-
expectedExtensions: Array<string>,
268-
filePathExists: boolean,
269-
): FilePathValidity {
270-
if (!filePath.trim()) {
271-
return FilePathValidity.EMPTY_STRING;
272-
} else if (!filePathExists) {
273-
return FilePathValidity.PATH_DOESNT_EXIST;
274-
} else if (
275-
!expectedExtensions.some((extension) => filePath.endsWith(`.${extension}`))
276-
) {
277-
return FilePathValidity.WRONG_EXTENSION;
278-
}
279-
280-
return FilePathValidity.VALID;
281-
}
282-
283232
function initializeGlobalBackendState(
284233
filePath: string,
285234
isOpossumFormat: boolean,

‎src/ElectronBackend/main/main.ts

-5
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
getImportFileConvertAndLoadListener,
1515
getImportFileSelectInputListener,
1616
getImportFileSelectSaveLocationListener,
17-
getImportFileValidatePathsListener,
1817
getOpenFileListener,
1918
getOpenLinkListener,
2019
getSaveFileListener,
@@ -80,10 +79,6 @@ export async function main(): Promise<void> {
8079
IpcChannel.ImportFileConvertAndLoad,
8180
getImportFileConvertAndLoadListener(mainWindow, activateMenuItems),
8281
);
83-
ipcMain.handle(
84-
IpcChannel.ImportFileValidatePaths,
85-
getImportFileValidatePathsListener(mainWindow),
86-
);
8782
ipcMain.handle(IpcChannel.SaveFile, getSaveFileListener(mainWindow));
8883
ipcMain.handle(
8984
IpcChannel.DeleteFile,

‎src/ElectronBackend/preload.ts

-7
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,6 @@ const electronAPI: ElectronAPI = {
2323
inputFilePath,
2424
opossumFilePath,
2525
),
26-
importFileValidatePaths: (inputFilePath, extensions, opossumFilePath) =>
27-
ipcRenderer.invoke(
28-
IpcChannel.ImportFileValidatePaths,
29-
inputFilePath,
30-
extensions,
31-
opossumFilePath,
32-
),
3326
exportFile: (args) => ipcRenderer.invoke(IpcChannel.ExportFile, args),
3427
saveFile: (saveFileArgs) =>
3528
ipcRenderer.invoke(IpcChannel.SaveFile, saveFileArgs),

‎src/Frontend/Components/FilePathInput/FilePathInput.tsx

+12-12
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//
44
// SPDX-License-Identifier: Apache-2.0
55
import { Folder } from '@mui/icons-material';
6-
import { FormControl, FormHelperText } from '@mui/material';
6+
import { FormControl } from '@mui/material';
77
import MuiBox from '@mui/system/Box';
88

99
import { IconButton } from '../IconButton/IconButton';
@@ -13,25 +13,28 @@ interface FilePathInputProps {
1313
label: string;
1414
text: string;
1515
buttonTooltip: string;
16-
onEdit: (filePath: string) => void;
17-
onBlur?: () => void;
16+
onEdit?: (filePath: string) => void;
1817
onButtonClick: () => void;
19-
errorMessage: string | null;
20-
warnMessage?: string | null;
18+
readOnly?: boolean;
2119
}
2220

2321
export const FilePathInput: React.FC<FilePathInputProps> = (props) => {
2422
return (
25-
<FormControl sx={{ display: 'flex', flexDirection: 'column' }}>
23+
<FormControl
24+
sx={{ display: 'flex', flexDirection: 'column', marginTop: '10px' }}
25+
>
2626
<MuiBox
2727
sx={{ display: 'flex', alignItems: 'center', paddingTop: '10px' }}
2828
>
2929
<TextBox
3030
title={props.label}
31+
readOnly={props.readOnly}
3132
text={props.text}
32-
error={props.errorMessage !== null}
33-
handleChange={(event) => props.onEdit(event.target.value)}
34-
onBlur={props.onBlur}
33+
handleChange={(event) => {
34+
if (props.onEdit) {
35+
props.onEdit(event.target.value);
36+
}
37+
}}
3538
sx={{ width: 600, marginRight: '10px' }}
3639
/>
3740
<IconButton
@@ -40,9 +43,6 @@ export const FilePathInput: React.FC<FilePathInputProps> = (props) => {
4043
tooltipTitle={props.buttonTooltip}
4144
/>
4245
</MuiBox>
43-
<FormHelperText aria-label={'file path helper text'} error={true}>
44-
{props.errorMessage ?? props.warnMessage ?? ' '}
45-
</FormHelperText>
4646
</FormControl>
4747
);
4848
};

0 commit comments

Comments
 (0)