From 18061ba3a9ccb46b65b6baabd1420a5a586dd8fd Mon Sep 17 00:00:00 2001 From: Jeff Phillips Date: Fri, 30 Aug 2024 14:09:33 -0400 Subject: [PATCH] [RHOAIENG-11530] Add file field advanced settings for connection type fields (#3132) --- frontend/package-lock.json | 1 + frontend/package.json | 1 + .../connectionTypes/fields/FileFormField.tsx | 121 ++++++++++++++---- .../fields/__tests__/FileFormField.spec.tsx | 4 + .../connectionTypes/fields/fieldUtils.ts | 5 + .../src/concepts/connectionTypes/types.ts | 8 +- .../manage/ConnectionTypeDataFieldModal.tsx | 3 +- .../DataFieldAdvancedPropertiesForm.tsx | 3 +- .../FileUploadAdvancedPropertiesForm.tsx | 92 +++++++++++++ .../advanced/FileUploadExtensionRow.tsx | 95 ++++++++++++++ .../FileUploadAdvancedPropertiesForm.spec.tsx | 114 +++++++++++++++++ .../manage/manageFieldUtils.ts | 21 +++ 12 files changed, 440 insertions(+), 28 deletions(-) create mode 100644 frontend/src/concepts/connectionTypes/fields/fieldUtils.ts create mode 100644 frontend/src/pages/connectionTypes/manage/advanced/FileUploadAdvancedPropertiesForm.tsx create mode 100644 frontend/src/pages/connectionTypes/manage/advanced/FileUploadExtensionRow.tsx create mode 100644 frontend/src/pages/connectionTypes/manage/advanced/__tests__/FileUploadAdvancedPropertiesForm.spec.tsx create mode 100644 frontend/src/pages/connectionTypes/manage/manageFieldUtils.ts diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 378efe232a..c8c15bd814 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -42,6 +42,7 @@ "react": "^18.2.0", "react-cool-dimensions": "^2.0.5", "react-dom": "^18.2.0", + "react-dropzone": "^14.2.3", "react-redux": "^8.0.4", "react-router": "^6.4.1", "react-router-dom": "^6.4.1", diff --git a/frontend/package.json b/frontend/package.json index b7434a4088..ab119aa176 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -85,6 +85,7 @@ "react": "^18.2.0", "react-cool-dimensions": "^2.0.5", "react-dom": "^18.2.0", + "react-dropzone": "^14.2.3", "react-redux": "^8.0.4", "react-router": "^6.4.1", "react-router-dom": "^6.4.1", diff --git a/frontend/src/concepts/connectionTypes/fields/FileFormField.tsx b/frontend/src/concepts/connectionTypes/fields/FileFormField.tsx index 340a876aea..5abb04ba55 100644 --- a/frontend/src/concepts/connectionTypes/fields/FileFormField.tsx +++ b/frontend/src/concepts/connectionTypes/fields/FileFormField.tsx @@ -1,7 +1,12 @@ import * as React from 'react'; -import { FileUpload } from '@patternfly/react-core'; +import { ErrorCode, FileError } from 'react-dropzone'; +import { FileUpload, FormHelperText, HelperText, HelperTextItem } from '@patternfly/react-core'; +import { ExclamationCircleIcon } from '@patternfly/react-icons'; import { FileField } from '~/concepts/connectionTypes/types'; import { FieldProps } from '~/concepts/connectionTypes/fields/types'; +import { EXTENSION_REGEX, isDuplicateExtension } from './fieldUtils'; + +const MAX_SIZE = 1024 * 1024; // 1 MB as bytes const FileFormField: React.FC> = ({ id, @@ -14,31 +19,97 @@ const FileFormField: React.FC> = ({ const isPreview = mode === 'preview'; const [isLoading, setIsLoading] = React.useState(false); const [filename, setFilename] = React.useState(''); - const readOnly = isPreview || field.properties.defaultReadOnly; + const [rejectedReason, setRejectedReason] = React.useState(); + const readOnly = isPreview || (mode !== 'default' && field.properties.defaultReadOnly); + const extensions = + field.properties.extensions + ?.filter( + (ext, index) => + !isDuplicateExtension(index, field.properties.extensions || []) && + EXTENSION_REGEX.test(ext), + ) + .map((ext) => ext.toLowerCase()) ?? []; + + React.useEffect(() => { + setRejectedReason(undefined); + }, [field.properties.extensions]); + + const formatString = extensions.length + ? `File format must be ${extensions.slice(0, -1).join(', ')}${ + extensions.length > 1 + ? `${extensions.length > 2 ? ',' : ''} or ${extensions[extensions.length - 1]}` + : extensions[0] + }` + : ''; + + const getRejectionMessage = (error?: FileError): string => { + switch (error?.code) { + case ErrorCode.FileTooLarge: + return 'File is larger than 1MB'; + case ErrorCode.FileInvalidType: + return formatString; + case ErrorCode.TooManyFiles: + return 'Only a single file may be uploaded'; + default: + return 'Unable to upload the file'; + } + }; + return ( - onChange(content)} - onFileInputChange={(_e, file) => setFilename(file.name)} - onReadStarted={() => { - setIsLoading(true); - }} - onReadFinished={() => { - setIsLoading(false); - }} - /> + <> + onChange(content)} + onFileInputChange={(_e, file) => setFilename(file.name)} + isClearButtonDisabled={rejectedReason ? false : undefined} + onClearClick={() => { + if (onChange) { + onChange(''); + } + setFilename(''); + setRejectedReason(undefined); + }} + onReadStarted={() => { + setRejectedReason(undefined); + setIsLoading(true); + }} + onReadFinished={() => { + setIsLoading(false); + }} + dropzoneProps={{ + accept: extensions.length ? { '': extensions } : undefined, + maxSize: MAX_SIZE, + onDropRejected: (reason) => { + setRejectedReason(getRejectionMessage(reason[0]?.errors?.[0])); + }, + }} + /> + + + : undefined} + variant={rejectedReason ? 'error' : 'default'} + data-testid="file-form-field-helper-text" + > + {rejectedReason || formatString} + + + + ); }; diff --git a/frontend/src/concepts/connectionTypes/fields/__tests__/FileFormField.spec.tsx b/frontend/src/concepts/connectionTypes/fields/__tests__/FileFormField.spec.tsx index a01b6d19a4..66fe84af4a 100644 --- a/frontend/src/concepts/connectionTypes/fields/__tests__/FileFormField.spec.tsx +++ b/frontend/src/concepts/connectionTypes/fields/__tests__/FileFormField.spec.tsx @@ -14,6 +14,7 @@ describe('FileFormField', () => { envVar: 'test-envVar', properties: { defaultValue: 'default-value', + extensions: ['.jpg', '.svg', '.png'], }, }; @@ -24,6 +25,9 @@ describe('FileFormField', () => { expect(screen.getByRole('button', { name: 'Upload' })).not.toBeDisabled(); expect(screen.getByRole('button', { name: 'Clear' })).not.toBeDisabled(); + const helperText = screen.getByTestId('file-form-field-helper-text'); + expect(helperText).toHaveTextContent('.jpg, .svg, or .png'); + act(() => { fireEvent.change(contentInput, { target: { value: 'new-value' } }); }); diff --git a/frontend/src/concepts/connectionTypes/fields/fieldUtils.ts b/frontend/src/concepts/connectionTypes/fields/fieldUtils.ts new file mode 100644 index 0000000000..021f382033 --- /dev/null +++ b/frontend/src/concepts/connectionTypes/fields/fieldUtils.ts @@ -0,0 +1,5 @@ +export const EXTENSION_REGEX = /^\.[a-zA-Z0-9]+(^.[a-zA-Z0-9]+)?$/; + +export const isDuplicateExtension = (index: number, values: string[]): boolean => + index !== 0 && + !!values.slice(0, index).find((val) => values[index].toLowerCase() === val.toLowerCase()); diff --git a/frontend/src/concepts/connectionTypes/types.ts b/frontend/src/concepts/connectionTypes/types.ts index 7ffecda491..5104715333 100644 --- a/frontend/src/concepts/connectionTypes/types.ts +++ b/frontend/src/concepts/connectionTypes/types.ts @@ -59,10 +59,16 @@ export type DataField; export type HiddenField = DataField; -export type FileField = DataField; export type ShortTextField = DataField; export type TextField = DataField; export type UriField = DataField; +export type FileField = DataField< + ConnectionTypeFieldType.File | 'file', + string, + { + extensions?: string[]; + } +>; export type BooleanField = DataField< ConnectionTypeFieldType.Boolean | 'boolean', boolean, diff --git a/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx b/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx index 3c9b27d2bc..06694ab98d 100644 --- a/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx +++ b/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx @@ -32,6 +32,7 @@ import { fieldNameToEnvVar, fieldTypeToString } from '~/concepts/connectionTypes import { isEnumMember } from '~/utilities/utils'; import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconButton'; import DataFieldPropertiesForm from '~/pages/connectionTypes/manage/DataFieldPropertiesForm'; +import { prepareFieldForSave } from '~/pages/connectionTypes/manage/manageFieldUtils'; const ENV_VAR_NAME_REGEX = new RegExp('^[-._a-zA-Z][-._a-zA-Z0-9]*$'); @@ -98,7 +99,7 @@ export const ConnectionTypeDataFieldModal: React.FC = ({ const handleSubmit = () => { if (isValid) { if (newField) { - onSubmit(newField); + onSubmit(prepareFieldForSave(newField)); } onClose(); } diff --git a/frontend/src/pages/connectionTypes/manage/advanced/DataFieldAdvancedPropertiesForm.tsx b/frontend/src/pages/connectionTypes/manage/advanced/DataFieldAdvancedPropertiesForm.tsx index 30935b3c14..f166afcbeb 100644 --- a/frontend/src/pages/connectionTypes/manage/advanced/DataFieldAdvancedPropertiesForm.tsx +++ b/frontend/src/pages/connectionTypes/manage/advanced/DataFieldAdvancedPropertiesForm.tsx @@ -3,6 +3,7 @@ import { ConnectionTypeDataField, ConnectionTypeFieldType } from '~/concepts/con import BooleanAdvancedPropertiesForm from '~/pages/connectionTypes/manage/advanced/BooleanAdvancedPropertiesForm'; import { AdvancedFieldProps } from '~/pages/connectionTypes/manage/advanced/types'; import NumericAdvancedPropertiesForm from '~/pages/connectionTypes/manage/advanced/NumericAdvancedPropertiesForm'; +import FileUploadAdvancedPropertiesForm from '~/pages/connectionTypes/manage/advanced/FileUploadAdvancedPropertiesForm'; const CustomFieldPropertiesForm = ( props: AdvancedFieldProps, @@ -11,7 +12,7 @@ const CustomFieldPropertiesForm = ( // TODO define advanced forms switch (props.field.type) { case ConnectionTypeFieldType.File: - return () => null; + return FileUploadAdvancedPropertiesForm; case ConnectionTypeFieldType.Boolean: return BooleanAdvancedPropertiesForm; diff --git a/frontend/src/pages/connectionTypes/manage/advanced/FileUploadAdvancedPropertiesForm.tsx b/frontend/src/pages/connectionTypes/manage/advanced/FileUploadAdvancedPropertiesForm.tsx new file mode 100644 index 0000000000..f27996a43f --- /dev/null +++ b/frontend/src/pages/connectionTypes/manage/advanced/FileUploadAdvancedPropertiesForm.tsx @@ -0,0 +1,92 @@ +import * as React from 'react'; +import { Button, FormGroup } from '@patternfly/react-core'; +import { PlusCircleIcon } from '@patternfly/react-icons'; +import { FileField } from '~/concepts/connectionTypes/types'; +import { + EXTENSION_REGEX, + isDuplicateExtension, +} from '~/concepts/connectionTypes/fields/fieldUtils'; +import { AdvancedFieldProps } from '~/pages/connectionTypes/manage/advanced/types'; +import ExpandableFormSection from '~/components/ExpandableFormSection'; +import FileUploadExtensionRow from '~/pages/connectionTypes/manage/advanced/FileUploadExtensionRow'; + +const FileUploadAdvancedPropertiesForm: React.FC> = ({ + properties, + onChange, + onValidate, +}) => { + const displayedExtensions = React.useMemo( + () => [...(properties.extensions?.length ? properties.extensions : [''])], + [properties.extensions], + ); + const lastTextRef = React.useRef(null); + + React.useEffect(() => { + if (!properties.extensions?.length) { + onValidate(true); + return; + } + const valid = properties.extensions.every( + (extension, i) => + !extension || + (EXTENSION_REGEX.test(extension) && !isDuplicateExtension(i, properties.extensions || [])), + ); + onValidate(valid); + // do not run when callback changes + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [properties.extensions]); + + return ( + + + {displayedExtensions.map((extension, index) => ( + { + onChange({ + ...properties, + extensions: [ + ...displayedExtensions.slice(0, index), + val, + ...displayedExtensions.slice(index + 1), + ], + }); + }} + onRemove={() => + onChange({ + ...properties, + extensions: [ + ...displayedExtensions.slice(0, index), + ...displayedExtensions.slice(index + 1), + ], + }) + } + allowRemove={displayedExtensions.length > 1 || !!displayedExtensions[0]} + /> + ))} + + + + ); +}; + +export default FileUploadAdvancedPropertiesForm; diff --git a/frontend/src/pages/connectionTypes/manage/advanced/FileUploadExtensionRow.tsx b/frontend/src/pages/connectionTypes/manage/advanced/FileUploadExtensionRow.tsx new file mode 100644 index 0000000000..96e9c9a585 --- /dev/null +++ b/frontend/src/pages/connectionTypes/manage/advanced/FileUploadExtensionRow.tsx @@ -0,0 +1,95 @@ +import * as React from 'react'; +import { + Button, + FormHelperText, + HelperText, + HelperTextItem, + InputGroup, + InputGroupItem, + TextInput, +} from '@patternfly/react-core'; +import { ExclamationCircleIcon, MinusCircleIcon } from '@patternfly/react-icons'; +import { EXTENSION_REGEX } from '~/concepts/connectionTypes/fields/fieldUtils'; + +type FileUploadExtensionRowProps = { + extension?: string; + isDuplicate?: boolean; + onChange: (newValue: string) => void; + onRemove?: () => void; + allowRemove: boolean; + textRef?: React.RefObject; +}; + +const FileUploadExtensionRow: React.FC = ({ + extension, + isDuplicate, + onRemove, + allowRemove, + onChange, + textRef, +}) => { + const [isValid, setIsValid] = React.useState(!isDuplicate); + + React.useEffect( + () => setIsValid(extension ? !isDuplicate && EXTENSION_REGEX.test(extension) : true), + // only run on entry or if a duplicate, otherwise wait for blur + // eslint-disable-next-line react-hooks/exhaustive-deps + [isDuplicate], + ); + + return ( + <> + + + { + if (!isValid && !isDuplicate && EXTENSION_REGEX.test(val)) { + setIsValid(true); + } + onChange(val); + }} + onBlur={() => { + setIsValid(extension ? !isDuplicate && EXTENSION_REGEX.test(extension) : true); + }} + /> + + + + + + {!isValid ? ( + + + } + variant="error" + data-testid="file-upload-extension-row-error" + > + {isDuplicate + ? 'Extension has already been specified.' + : `Please enter a valid extension starting with '.'`} + + + + ) : null} + + ); +}; + +export default FileUploadExtensionRow; diff --git a/frontend/src/pages/connectionTypes/manage/advanced/__tests__/FileUploadAdvancedPropertiesForm.spec.tsx b/frontend/src/pages/connectionTypes/manage/advanced/__tests__/FileUploadAdvancedPropertiesForm.spec.tsx new file mode 100644 index 0000000000..7b3af8b3fc --- /dev/null +++ b/frontend/src/pages/connectionTypes/manage/advanced/__tests__/FileUploadAdvancedPropertiesForm.spec.tsx @@ -0,0 +1,114 @@ +import * as React from 'react'; +import '@testing-library/jest-dom'; +import { fireEvent, render, screen, within } from '@testing-library/react'; +import { act } from 'react-dom/test-utils'; +import { ConnectionTypeFieldType, FileField } from '~/concepts/connectionTypes/types'; +import FileUploadAdvancedPropertiesForm from '~/pages/connectionTypes/manage/advanced/FileUploadAdvancedPropertiesForm'; + +let onChange: jest.Mock; +let onValidate: jest.Mock; +let field: FileField; + +describe('FileUploadAdvancedPropertiesForm', () => { + beforeEach(() => { + onChange = jest.fn(); + onValidate = jest.fn(); + field = { + type: ConnectionTypeFieldType.File, + name: 'Test Number', + envVar: 'TEST_NUMBER', + properties: {}, + }; + }); + + it('should show the empty state', () => { + render( + , + ); + + expect(onValidate).toHaveBeenCalledWith(true); + + const advancedToggle = screen.getByTestId('advanced-settings-toggle'); + const toggleButton = within(advancedToggle).getByRole('button'); + + act(() => { + toggleButton.click(); + }); + + // there should only be one row + const extensionRow = screen.getByTestId('file-upload-extension-row'); + expect(extensionRow).toBeVisible(); + // there should only be one remove button and it should be disabled + const removeButton = screen.getByTestId('file-upload-extension-row-remove'); + expect(removeButton).toBeDisabled(); + + const input = screen.getByTestId('file-upload-extension-row-input'); + act(() => { + fireEvent.change(input, { target: { value: '.jsp' } }); + }); + expect(onChange).toHaveBeenCalledWith({ extensions: ['.jsp'] }); + }); + + it('should show the given extensions', () => { + render( + , + ); + + expect(onValidate).toHaveBeenCalledWith(true); + + // there should be 3 visible rows + const extensionRows = screen.getAllByTestId('file-upload-extension-row'); + expect(extensionRows).toHaveLength(3); + extensionRows.forEach((extensionRow) => expect(extensionRow).toBeVisible()); + + // there should only be three remove buttons and all should be enabled + const removeButtons = screen.getAllByTestId('file-upload-extension-row-remove'); + expect(removeButtons).toHaveLength(3); + removeButtons.forEach((removeButton) => expect(removeButton).toBeEnabled()); + + act(() => { + removeButtons[0].click(); + }); + expect(onChange).toHaveBeenCalledWith({ extensions: ['.jpg', '.png'] }); + + act(() => { + removeButtons[1].click(); + }); + expect(onChange).toHaveBeenCalledWith({ extensions: ['.svg', '.png'] }); + + act(() => { + removeButtons[2].click(); + }); + expect(onChange).toHaveBeenCalledWith({ extensions: ['.svg', '.jpg'] }); + + const addButton = screen.getByTestId('add-variable-button'); + act(() => { + addButton.click(); + }); + expect(onChange).toHaveBeenCalledWith({ extensions: ['.svg', '.jpg', '.png', ''] }); + }); + + it('should invalidate invalid extensions', () => { + render( + , + ); + + expect(onValidate).toHaveBeenCalledWith(false); + expect(screen.getByTestId('file-upload-extension-row-error')).toBeVisible(); + }); +}); diff --git a/frontend/src/pages/connectionTypes/manage/manageFieldUtils.ts b/frontend/src/pages/connectionTypes/manage/manageFieldUtils.ts new file mode 100644 index 0000000000..8a6a4b7206 --- /dev/null +++ b/frontend/src/pages/connectionTypes/manage/manageFieldUtils.ts @@ -0,0 +1,21 @@ +import { + ConnectionTypeDataField, + ConnectionTypeFieldType, + FileField, +} from '~/concepts/connectionTypes/types'; + +const cleanupFileUploadField = (field: FileField): FileField => ({ + ...field, + properties: { + ...field.properties, + extensions: field.properties.extensions?.filter((ext) => !!ext).map((ext) => ext.toLowerCase()), + }, +}); + +export const prepareFieldForSave = (field: ConnectionTypeDataField): ConnectionTypeDataField => { + switch (field.type) { + case ConnectionTypeFieldType.File: + return cleanupFileUploadField(field); + } + return field; +};