From cbedbfff5887ece2f0c15612f0ec4168ff9bc8eb Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Mon, 7 Jul 2025 08:14:21 -0300 Subject: [PATCH] feat(ws): prepare frontend for validation errors during WorkspaceKind creation (#471) * feat(ws): prepare frontend for validation errors during WorkspaceKind creation Signed-off-by: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> * feat(ws): extract validation alert to its own component Signed-off-by: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> * fix(ws): use error icon for helper text Signed-off-by: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> --------- Signed-off-by: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> --- workspaces/frontend/src/app/NavSidebar.tsx | 2 +- .../app/components/ValidationErrorAlert.tsx | 24 ++++++ .../WorkspaceKinds/Form/WorkspaceKindForm.tsx | 45 ++++++++-- .../fileUpload/WorkspaceKindFileUpload.tsx | 85 ++++++++++--------- .../shared/api/__tests__/errorUtils.spec.ts | 17 ++-- .../api/__tests__/notebookService.spec.ts | 18 ++-- .../frontend/src/shared/api/apiUtils.ts | 41 +++++++++ .../src/shared/api/backendApiTypes.ts | 33 +++++++ .../frontend/src/shared/api/errorUtils.ts | 35 +++----- .../src/shared/api/notebookService.ts | 75 +++++----------- workspaces/frontend/src/shared/api/types.ts | 7 -- .../src/shared/mock/mockNotebookService.ts | 35 +++++++- .../frontend/src/shared/mock/mockUtils.ts | 7 ++ .../frontend/src/shared/style/MUI-theme.scss | 1 - 14 files changed, 273 insertions(+), 152 deletions(-) create mode 100644 workspaces/frontend/src/app/components/ValidationErrorAlert.tsx create mode 100644 workspaces/frontend/src/shared/mock/mockUtils.ts diff --git a/workspaces/frontend/src/app/NavSidebar.tsx b/workspaces/frontend/src/app/NavSidebar.tsx index f1deb892..0bd4ac81 100644 --- a/workspaces/frontend/src/app/NavSidebar.tsx +++ b/workspaces/frontend/src/app/NavSidebar.tsx @@ -17,7 +17,7 @@ const NavHref: React.FC<{ item: NavDataHref }> = ({ item }) => { const location = useTypedLocation(); // With the redirect in place, we can now use a simple path comparison. - const isActive = location.pathname === item.path; + const isActive = location.pathname === item.path || location.pathname.startsWith(`${item.path}/`); return ( diff --git a/workspaces/frontend/src/app/components/ValidationErrorAlert.tsx b/workspaces/frontend/src/app/components/ValidationErrorAlert.tsx new file mode 100644 index 00000000..e2691b2a --- /dev/null +++ b/workspaces/frontend/src/app/components/ValidationErrorAlert.tsx @@ -0,0 +1,24 @@ +import React from 'react'; +import { Alert, List, ListItem } from '@patternfly/react-core'; +import { ValidationError } from '~/shared/api/backendApiTypes'; + +interface ValidationErrorAlertProps { + title: string; + errors: ValidationError[]; +} + +export const ValidationErrorAlert: React.FC = ({ title, errors }) => { + if (errors.length === 0) { + return null; + } + + return ( + + + {errors.map((error, index) => ( + {error.message} + ))} + + + ); +}; diff --git a/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/WorkspaceKindForm.tsx b/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/WorkspaceKindForm.tsx index 2666f54b..feb1ba68 100644 --- a/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/WorkspaceKindForm.tsx +++ b/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/WorkspaceKindForm.tsx @@ -8,12 +8,17 @@ import { PageGroup, PageSection, Stack, + StackItem, } from '@patternfly/react-core'; +import { t_global_spacer_sm as SmallPadding } from '@patternfly/react-tokens'; +import { ValidationErrorAlert } from '~/app/components/ValidationErrorAlert'; import { useTypedNavigate } from '~/app/routerHelper'; import { useCurrentRouteKey } from '~/app/hooks/useCurrentRouteKey'; import useGenericObjectState from '~/app/hooks/useGenericObjectState'; import { useNotebookAPI } from '~/app/hooks/useNotebookAPI'; import { WorkspaceKindFormData } from '~/app/types'; +import { ErrorEnvelopeException } from '~/shared/api/apiUtils'; +import { ValidationError } from '~/shared/api/backendApiTypes'; import { WorkspaceKindFileUpload } from './fileUpload/WorkspaceKindFileUpload'; import { WorkspaceKindFormProperties } from './properties/WorkspaceKindFormProperties'; import { WorkspaceKindFormImage } from './image/WorkspaceKindFormImage'; @@ -34,6 +39,8 @@ export const WorkspaceKindForm: React.FC = () => { const [isSubmitting, setIsSubmitting] = useState(false); const [validated, setValidated] = useState('default'); const mode = useCurrentRouteKey() === 'workspaceKindCreate' ? 'create' : 'edit'; + const [specErrors, setSpecErrors] = useState([]); + const [data, setData, resetData] = useGenericObjectState({ properties: { displayName: '', @@ -60,14 +67,24 @@ export const WorkspaceKindForm: React.FC = () => { try { if (mode === 'create') { const newWorkspaceKind = await api.createWorkspaceKind({}, yamlValue); + // TODO: alert user about success console.info('New workspace kind created:', JSON.stringify(newWorkspaceKind)); + navigate('workspaceKinds'); } } catch (err) { + if (err instanceof ErrorEnvelopeException) { + const validationErrors = err.envelope.error?.cause?.validation_errors; + if (validationErrors && validationErrors.length > 0) { + setSpecErrors(validationErrors); + setValidated('error'); + return; + } + } + // TODO: alert user about error console.error(`Error ${mode === 'edit' ? 'editing' : 'creating'} workspace kind: ${err}`); } finally { setIsSubmitting(false); } - navigate('workspaceKinds'); }, [navigate, mode, api, yamlValue]); const canSubmit = useMemo( @@ -102,13 +119,25 @@ export const WorkspaceKindForm: React.FC = () => { {mode === 'create' && ( - + + {specErrors.length > 0 && ( + + + + )} + + { + setSpecErrors([]); + }} + /> + + )} {mode === 'edit' && ( <> diff --git a/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/fileUpload/WorkspaceKindFileUpload.tsx b/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/fileUpload/WorkspaceKindFileUpload.tsx index eae9c918..bbd78ee2 100644 --- a/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/fileUpload/WorkspaceKindFileUpload.tsx +++ b/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/fileUpload/WorkspaceKindFileUpload.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useRef, useState } from 'react'; +import React, { useCallback, useState } from 'react'; import yaml, { YAMLException } from 'js-yaml'; import { FileUpload, @@ -7,6 +7,7 @@ import { HelperText, HelperTextItem, Content, + DropzoneErrorCode, } from '@patternfly/react-core'; import { isValidWorkspaceKindYaml } from '~/app/pages/WorkspaceKinds/Form/helpers'; import { ValidationStatus } from '~/app/pages/WorkspaceKinds/Form/WorkspaceKindForm'; @@ -17,60 +18,52 @@ interface WorkspaceKindFileUploadProps { resetData: () => void; validated: ValidationStatus; setValidated: (type: ValidationStatus) => void; + onClear: () => void; } +const YAML_MIME_TYPE = 'application/x-yaml'; +const YAML_EXTENSIONS = ['.yml', '.yaml']; + export const WorkspaceKindFileUpload: React.FC = ({ resetData, value, setValue, validated, setValidated, + onClear, }) => { - const isYamlFileRef = useRef(false); const [filename, setFilename] = useState(''); const [isLoading, setIsLoading] = useState(false); - const [fileUploadHelperText, setFileUploadHelperText] = useState(''); + const [fileUploadHelperText, setFileUploadHelperText] = useState(); const handleFileInputChange = useCallback( (_: unknown, file: File) => { - const fileName = file.name; + onClear(); setFilename(file.name); - // if extension is not yaml or yml, raise a flag - const ext = fileName.split('.').pop(); - const isYaml = ext === 'yml' || ext === 'yaml'; - isYamlFileRef.current = isYaml; - if (!isYaml) { - setFileUploadHelperText('Invalid file. Only YAML files are allowed.'); - resetData(); - setValidated('error'); - } else { - setFileUploadHelperText(''); - setValidated('success'); - } + setFileUploadHelperText(undefined); + setValidated('success'); }, - [resetData, setValidated], + [setValidated, onClear], ); // TODO: Use zod or another TS type coercion/schema for file upload const handleDataChange = useCallback( (_: DropEvent, v: string) => { setValue(v); - if (isYamlFileRef.current) { - try { - const parsed = yaml.load(v); - if (!isValidWorkspaceKindYaml(parsed)) { - setFileUploadHelperText('YAML is invalid: must follow WorkspaceKind format.'); - setValidated('error'); - resetData(); - } else { - setValidated('success'); - setFileUploadHelperText(''); - } - } catch (e) { - console.error('Error parsing YAML:', e); - setFileUploadHelperText(`Error parsing YAML: ${e as YAMLException['reason']}`); + try { + const parsed = yaml.load(v); + if (!isValidWorkspaceKindYaml(parsed)) { + setFileUploadHelperText('YAML is invalid: must follow WorkspaceKind format.'); setValidated('error'); + resetData(); + } else { + setValidated('success'); + setFileUploadHelperText(''); } + } catch (e) { + console.error('Error parsing YAML:', e); + setFileUploadHelperText(`Error parsing YAML: ${e as YAMLException['reason']}`); + setValidated('error'); } }, [setValue, setValidated, resetData], @@ -82,7 +75,8 @@ export const WorkspaceKindFileUpload: React.FC = ( setFileUploadHelperText(''); setValidated('default'); resetData(); - }, [resetData, setValidated, setValue]); + onClear(); + }, [resetData, setValidated, setValue, onClear]); const handleFileReadStarted = useCallback(() => { setIsLoading(true); @@ -110,14 +104,27 @@ export const WorkspaceKindFileUpload: React.FC = ( validated={validated} allowEditingUploadedText={false} browseButtonText="Choose File" + dropzoneProps={{ + accept: { [YAML_MIME_TYPE]: YAML_EXTENSIONS }, + onDropRejected: (rejections) => { + const error = rejections[0]?.errors?.[0] ?? {}; + setFileUploadHelperText( + error.code === DropzoneErrorCode.FileInvalidType + ? 'Invalid file. Only YAML files are allowed.' + : error.message, + ); + }, + }} > - - - - {fileUploadHelperText} - - - + {fileUploadHelperText && ( + + + + {fileUploadHelperText} + + + + )} ); diff --git a/workspaces/frontend/src/shared/api/__tests__/errorUtils.spec.ts b/workspaces/frontend/src/shared/api/__tests__/errorUtils.spec.ts index 16870890..099905e5 100644 --- a/workspaces/frontend/src/shared/api/__tests__/errorUtils.spec.ts +++ b/workspaces/frontend/src/shared/api/__tests__/errorUtils.spec.ts @@ -1,8 +1,9 @@ -import { NotReadyError } from '~/shared/utilities/useFetchState'; -import { APIError } from '~/shared/api/types'; -import { handleRestFailures } from '~/shared/api/errorUtils'; import { mockNamespaces } from '~/__mocks__/mockNamespaces'; import { mockBFFResponse } from '~/__mocks__/utils'; +import { ErrorEnvelopeException } from '~/shared/api/apiUtils'; +import { ErrorEnvelope } from '~/shared/api/backendApiTypes'; +import { handleRestFailures } from '~/shared/api/errorUtils'; +import { NotReadyError } from '~/shared/utilities/useFetchState'; describe('handleRestFailures', () => { it('should successfully return namespaces', async () => { @@ -11,14 +12,14 @@ describe('handleRestFailures', () => { }); it('should handle and throw notebook errors', async () => { - const statusMock: APIError = { + const errorEnvelope: ErrorEnvelope = { error: { - code: '', - message: 'error', + code: '', + message: '', }, }; - - await expect(handleRestFailures(Promise.resolve(statusMock))).rejects.toThrow('error'); + const expectedError = new ErrorEnvelopeException(errorEnvelope); + await expect(handleRestFailures(Promise.reject(errorEnvelope))).rejects.toThrow(expectedError); }); it('should handle common state errors ', async () => { diff --git a/workspaces/frontend/src/shared/api/__tests__/notebookService.spec.ts b/workspaces/frontend/src/shared/api/__tests__/notebookService.spec.ts index d84aa9a9..27a650b5 100644 --- a/workspaces/frontend/src/shared/api/__tests__/notebookService.spec.ts +++ b/workspaces/frontend/src/shared/api/__tests__/notebookService.spec.ts @@ -1,10 +1,9 @@ import { BFF_API_VERSION } from '~/app/const'; -import { restGET } from '~/shared/api/apiUtils'; -import { handleRestFailures } from '~/shared/api/errorUtils'; +import { restGET, wrapRequest } from '~/shared/api/apiUtils'; import { listNamespaces } from '~/shared/api/notebookService'; -const mockRestPromise = Promise.resolve({ data: {} }); -const mockRestResponse = {}; +const mockRestResponse = { data: {} }; +const mockRestPromise = Promise.resolve(mockRestResponse); jest.mock('~/shared/api/apiUtils', () => ({ restCREATE: jest.fn(() => mockRestPromise), @@ -12,13 +11,10 @@ jest.mock('~/shared/api/apiUtils', () => ({ restPATCH: jest.fn(() => mockRestPromise), isNotebookResponse: jest.fn(() => true), extractNotebookResponse: jest.fn(() => mockRestResponse), + wrapRequest: jest.fn(() => mockRestPromise), })); -jest.mock('~/shared/api/errorUtils', () => ({ - handleRestFailures: jest.fn(() => mockRestPromise), -})); - -const handleRestFailuresMock = jest.mocked(handleRestFailures); +const wrapRequestMock = jest.mocked(wrapRequest); const restGETMock = jest.mocked(restGET); const APIOptionsMock = {}; @@ -33,7 +29,7 @@ describe('getNamespaces', () => { {}, APIOptionsMock, ); - expect(handleRestFailuresMock).toHaveBeenCalledTimes(1); - expect(handleRestFailuresMock).toHaveBeenCalledWith(mockRestPromise); + expect(wrapRequestMock).toHaveBeenCalledTimes(1); + expect(wrapRequestMock).toHaveBeenCalledWith(mockRestPromise); }); }); diff --git a/workspaces/frontend/src/shared/api/apiUtils.ts b/workspaces/frontend/src/shared/api/apiUtils.ts index e1c5c6da..6993cc45 100644 --- a/workspaces/frontend/src/shared/api/apiUtils.ts +++ b/workspaces/frontend/src/shared/api/apiUtils.ts @@ -1,3 +1,5 @@ +import { ErrorEnvelope } from '~/shared/api/backendApiTypes'; +import { handleRestFailures } from '~/shared/api/errorUtils'; import { APIOptions, ResponseBody } from '~/shared/api/types'; import { EitherOrNone } from '~/shared/typeHelpers'; import { AUTH_HEADER, DEV_MODE } from '~/shared/utilities/const'; @@ -222,9 +224,48 @@ export const isNotebookResponse = (response: unknown): response is ResponseBo return false; }; +export const isErrorEnvelope = (e: unknown): e is ErrorEnvelope => + typeof e === 'object' && + e !== null && + 'error' in e && + typeof (e as Record).error === 'object' && + (e as { error: unknown }).error !== null && + typeof (e as { error: { message: unknown } }).error.message === 'string'; + export function extractNotebookResponse(response: unknown): T { if (isNotebookResponse(response)) { return response.data; } throw new Error('Invalid response format'); } + +export function extractErrorEnvelope(error: unknown): ErrorEnvelope { + if (isErrorEnvelope(error)) { + return error; + } + + const message = + error instanceof Error ? error.message : typeof error === 'string' ? error : 'Unexpected error'; + + return { + error: { + message, + code: 'UNKNOWN_ERROR', + }, + }; +} + +export async function wrapRequest(promise: Promise, extractData = true): Promise { + try { + const res = await handleRestFailures(promise); + return extractData ? extractNotebookResponse(res) : res; + } catch (error) { + throw new ErrorEnvelopeException(extractErrorEnvelope(error)); + } +} + +export class ErrorEnvelopeException extends Error { + constructor(public envelope: ErrorEnvelope) { + super(envelope.error?.message ?? 'Unknown error'); + } +} diff --git a/workspaces/frontend/src/shared/api/backendApiTypes.ts b/workspaces/frontend/src/shared/api/backendApiTypes.ts index ca42d666..e84ca6b5 100644 --- a/workspaces/frontend/src/shared/api/backendApiTypes.ts +++ b/workspaces/frontend/src/shared/api/backendApiTypes.ts @@ -282,3 +282,36 @@ export interface WorkspacePauseState { workspaceName: string; paused: boolean; } + +export enum FieldErrorType { + FieldValueRequired = 'FieldValueRequired', + FieldValueInvalid = 'FieldValueInvalid', + FieldValueNotSupported = 'FieldValueNotSupported', + FieldValueDuplicate = 'FieldValueDuplicate', + FieldValueTooLong = 'FieldValueTooLong', + FieldValueForbidden = 'FieldValueForbidden', + FieldValueNotFound = 'FieldValueNotFound', + FieldValueConflict = 'FieldValueConflict', + FieldValueTooShort = 'FieldValueTooShort', + FieldValueUnknown = 'FieldValueUnknown', +} + +export interface ValidationError { + type: FieldErrorType; + field: string; + message: string; +} + +export interface ErrorCause { + validation_errors?: ValidationError[]; // TODO: backend is not using camelCase for this field +} + +export type HTTPError = { + code: string; + message: string; + cause?: ErrorCause; +}; + +export type ErrorEnvelope = { + error: HTTPError | null; +}; diff --git a/workspaces/frontend/src/shared/api/errorUtils.ts b/workspaces/frontend/src/shared/api/errorUtils.ts index 47046554..9205a8f0 100644 --- a/workspaces/frontend/src/shared/api/errorUtils.ts +++ b/workspaces/frontend/src/shared/api/errorUtils.ts @@ -1,25 +1,16 @@ -import { APIError } from '~/shared/api/types'; +import { ErrorEnvelopeException, isErrorEnvelope } from '~/shared/api//apiUtils'; import { isCommonStateError } from '~/shared/utilities/useFetchState'; -const isError = (e: unknown): e is APIError => typeof e === 'object' && e !== null && 'error' in e; - export const handleRestFailures = (promise: Promise): Promise => - promise - .then((result) => { - if (isError(result)) { - throw result; - } - return result; - }) - .catch((e) => { - if (isError(e)) { - throw new Error(e.error.message); - } - if (isCommonStateError(e)) { - // Common state errors are handled by useFetchState at storage level, let them deal with it - throw e; - } - // eslint-disable-next-line no-console - console.error('Unknown API error', e); - throw new Error('Error communicating with server'); - }); + promise.catch((e) => { + if (isErrorEnvelope(e)) { + throw new ErrorEnvelopeException(e); + } + if (isCommonStateError(e)) { + // Common state errors are handled by useFetchState at storage level, let them deal with it + throw e; + } + // eslint-disable-next-line no-console + console.error('Unknown API error', e); + throw new Error('Error communicating with server'); + }); diff --git a/workspaces/frontend/src/shared/api/notebookService.ts b/workspaces/frontend/src/shared/api/notebookService.ts index 81fdbfcc..5cdd91bb 100644 --- a/workspaces/frontend/src/shared/api/notebookService.ts +++ b/workspaces/frontend/src/shared/api/notebookService.ts @@ -1,19 +1,12 @@ import { - extractNotebookResponse, restCREATE, restDELETE, restGET, restPATCH, restUPDATE, restYAML, + wrapRequest, } from '~/shared/api/apiUtils'; -import { handleRestFailures } from '~/shared/api/errorUtils'; -import { - Namespace, - Workspace, - WorkspaceKind, - WorkspacePauseState, -} from '~/shared/api/backendApiTypes'; import { CreateWorkspaceAPI, CreateWorkspaceKindAPI, @@ -32,86 +25,60 @@ import { StartWorkspaceAPI, UpdateWorkspaceAPI, UpdateWorkspaceKindAPI, -} from './callTypes'; +} from '~/shared/api/callTypes'; export const getHealthCheck: GetHealthCheckAPI = (hostPath) => (opts) => - handleRestFailures(restGET(hostPath, `/healthcheck`, {}, opts)); + wrapRequest(restGET(hostPath, `/healthcheck`, {}, opts), false); export const listNamespaces: ListNamespacesAPI = (hostPath) => (opts) => - handleRestFailures(restGET(hostPath, `/namespaces`, {}, opts)).then((response) => - extractNotebookResponse(response), - ); + wrapRequest(restGET(hostPath, `/namespaces`, {}, opts)); export const listAllWorkspaces: ListAllWorkspacesAPI = (hostPath) => (opts) => - handleRestFailures(restGET(hostPath, `/workspaces`, {}, opts)).then((response) => - extractNotebookResponse(response), - ); + wrapRequest(restGET(hostPath, `/workspaces`, {}, opts)); export const listWorkspaces: ListWorkspacesAPI = (hostPath) => (opts, namespace) => - handleRestFailures(restGET(hostPath, `/workspaces/${namespace}`, {}, opts)).then((response) => - extractNotebookResponse(response), - ); + wrapRequest(restGET(hostPath, `/workspaces/${namespace}`, {}, opts)); export const getWorkspace: GetWorkspaceAPI = (hostPath) => (opts, namespace, workspace) => - handleRestFailures(restGET(hostPath, `/workspaces/${namespace}/${workspace}`, {}, opts)).then( - (response) => extractNotebookResponse(response), - ); + wrapRequest(restGET(hostPath, `/workspaces/${namespace}/${workspace}`, {}, opts)); export const createWorkspace: CreateWorkspaceAPI = (hostPath) => (opts, namespace, data) => - handleRestFailures(restCREATE(hostPath, `/workspaces/${namespace}`, data, {}, opts)).then( - (response) => extractNotebookResponse(response), - ); + wrapRequest(restCREATE(hostPath, `/workspaces/${namespace}`, data, {}, opts)); export const updateWorkspace: UpdateWorkspaceAPI = (hostPath) => (opts, namespace, workspace, data) => - handleRestFailures( - restUPDATE(hostPath, `/workspaces/${namespace}/${workspace}`, data, {}, opts), - ).then((response) => extractNotebookResponse(response)); + wrapRequest(restUPDATE(hostPath, `/workspaces/${namespace}/${workspace}`, data, {}, opts)); export const patchWorkspace: PatchWorkspaceAPI = (hostPath) => (opts, namespace, workspace, data) => - handleRestFailures(restPATCH(hostPath, `/workspaces/${namespace}/${workspace}`, data, opts)).then( - (response) => extractNotebookResponse(response), - ); + wrapRequest(restPATCH(hostPath, `/workspaces/${namespace}/${workspace}`, data, opts)); export const deleteWorkspace: DeleteWorkspaceAPI = (hostPath) => (opts, namespace, workspace) => - handleRestFailures(restDELETE(hostPath, `/workspaces/${namespace}/${workspace}`, {}, {}, opts)); + wrapRequest(restDELETE(hostPath, `/workspaces/${namespace}/${workspace}`, {}, {}, opts), false); export const pauseWorkspace: PauseWorkspaceAPI = (hostPath) => (opts, namespace, workspace) => - handleRestFailures( + wrapRequest( restCREATE(hostPath, `/workspaces/${namespace}/${workspace}/actions/pause`, {}, opts), - ).then((response) => extractNotebookResponse(response)); + ); export const startWorkspace: StartWorkspaceAPI = (hostPath) => (opts, namespace, workspace) => - handleRestFailures( + wrapRequest( restCREATE(hostPath, `/workspaces/${namespace}/${workspace}/actions/start`, {}, opts), - ).then((response) => extractNotebookResponse(response)); + ); export const listWorkspaceKinds: ListWorkspaceKindsAPI = (hostPath) => (opts) => - handleRestFailures(restGET(hostPath, `/workspacekinds`, {}, opts)).then((response) => - extractNotebookResponse(response), - ); + wrapRequest(restGET(hostPath, `/workspacekinds`, {}, opts)); export const getWorkspaceKind: GetWorkspaceKindAPI = (hostPath) => (opts, kind) => - handleRestFailures(restGET(hostPath, `/workspacekinds/${kind}`, {}, opts)).then((response) => - extractNotebookResponse(response), - ); + wrapRequest(restGET(hostPath, `/workspacekinds/${kind}`, {}, opts)); export const createWorkspaceKind: CreateWorkspaceKindAPI = (hostPath) => (opts, data) => - handleRestFailures(restYAML(hostPath, `/workspacekinds`, data, {}, opts)).then((response) => - extractNotebookResponse(response), - ); + wrapRequest(restYAML(hostPath, `/workspacekinds`, data, {}, opts)); export const updateWorkspaceKind: UpdateWorkspaceKindAPI = (hostPath) => (opts, kind, data) => - handleRestFailures(restUPDATE(hostPath, `/workspacekinds/${kind}`, data, {}, opts)).then( - (response) => extractNotebookResponse(response), - ); + wrapRequest(restUPDATE(hostPath, `/workspacekinds/${kind}`, data, {}, opts)); export const patchWorkspaceKind: PatchWorkspaceKindAPI = (hostPath) => (opts, kind, data) => - handleRestFailures(restPATCH(hostPath, `/workspacekinds/${kind}`, data, opts)).then((response) => - extractNotebookResponse(response), - ); + wrapRequest(restPATCH(hostPath, `/workspacekinds/${kind}`, data, opts)); export const deleteWorkspaceKind: DeleteWorkspaceKindAPI = (hostPath) => (opts, kind) => - handleRestFailures(restDELETE(hostPath, `/workspacekinds/${kind}`, {}, {}, opts)).then( - (response) => extractNotebookResponse(response), - ); + wrapRequest(restDELETE(hostPath, `/workspacekinds/${kind}`, {}, {}, opts), false); diff --git a/workspaces/frontend/src/shared/api/types.ts b/workspaces/frontend/src/shared/api/types.ts index 6f1ac507..71389f36 100644 --- a/workspaces/frontend/src/shared/api/types.ts +++ b/workspaces/frontend/src/shared/api/types.ts @@ -5,13 +5,6 @@ export type APIOptions = { headers?: Record; }; -export type APIError = { - error: { - code: string; - message: string; - }; -}; - export type APIState = { /** If API will successfully call */ apiAvailable: boolean; diff --git a/workspaces/frontend/src/shared/mock/mockNotebookService.ts b/workspaces/frontend/src/shared/mock/mockNotebookService.ts index 91397c9e..0bb63e16 100644 --- a/workspaces/frontend/src/shared/mock/mockNotebookService.ts +++ b/workspaces/frontend/src/shared/mock/mockNotebookService.ts @@ -1,3 +1,5 @@ +import { ErrorEnvelopeException } from '~/shared/api/apiUtils'; +import { FieldErrorType } from '~/shared/api/backendApiTypes'; import { CreateWorkspaceAPI, CreateWorkspaceKindAPI, @@ -27,6 +29,7 @@ import { mockWorkspaceKind1, mockWorkspaceKinds, } from '~/shared/mock/mockNotebookServiceData'; +import { isInvalidYaml } from '~/shared/mock/mockUtils'; const delay = (ms: number) => new Promise((resolve) => { @@ -70,7 +73,37 @@ export const mockListWorkspaceKinds: ListWorkspaceKindsAPI = () => async () => m export const mockGetWorkspaceKind: GetWorkspaceKindAPI = () => async (_opts, kind) => mockWorkspaceKinds.find((w) => w.name === kind)!; -export const mockCreateWorkspaceKind: CreateWorkspaceKindAPI = () => async () => mockWorkspaceKind1; +export const mockCreateWorkspaceKind: CreateWorkspaceKindAPI = () => async (_opts, data) => { + if (isInvalidYaml(data)) { + throw new ErrorEnvelopeException({ + error: { + code: 'invalid_yaml', + message: 'Invalid YAML provided', + cause: { + // eslint-disable-next-line camelcase + validation_errors: [ + { + type: FieldErrorType.FieldValueRequired, + field: 'spec.spawner.displayName', + message: "Missing required 'spec.spawner.displayName' property", + }, + { + type: FieldErrorType.FieldValueUnknown, + field: 'spec.spawner.xyz', + message: "Unknown property 'spec.spawner.xyz'", + }, + { + type: FieldErrorType.FieldValueNotSupported, + field: 'spec.spawner.hidden', + message: "Invalid data type for 'spec.spawner.hidden', expected 'boolean'", + }, + ], + }, + }, + }); + } + return mockWorkspaceKind1; +}; export const mockUpdateWorkspaceKind: UpdateWorkspaceKindAPI = () => async () => mockWorkspaceKind1; diff --git a/workspaces/frontend/src/shared/mock/mockUtils.ts b/workspaces/frontend/src/shared/mock/mockUtils.ts new file mode 100644 index 00000000..c4af8e1a --- /dev/null +++ b/workspaces/frontend/src/shared/mock/mockUtils.ts @@ -0,0 +1,7 @@ +import yaml from 'js-yaml'; + +// For testing purposes, a YAML string is considered invalid if it contains a specific pattern in the metadata name. +export function isInvalidYaml(yamlString: string): boolean { + const parsed = yaml.load(yamlString) as { metadata?: { name?: string } }; + return parsed.metadata?.name?.includes('-invalid') ?? false; +} diff --git a/workspaces/frontend/src/shared/style/MUI-theme.scss b/workspaces/frontend/src/shared/style/MUI-theme.scss index 6084f158..ebc7263b 100644 --- a/workspaces/frontend/src/shared/style/MUI-theme.scss +++ b/workspaces/frontend/src/shared/style/MUI-theme.scss @@ -124,7 +124,6 @@ .mui-theme .pf-v6-c-alert { --pf-v6-c-alert--m-warning__title--Color: var(--pf-t--global--text--color--status--warning--default); --pf-v6-c-alert__icon--MarginInlineEnd: var(--mui-alert__icon--MarginInlineEnd); - --pf-v6-c-alert__title--FontWeight: var(--mui-alert-warning-font-weight); --pf-v6-c-alert__icon--MarginBlockStart: var(--mui-alert__icon--MarginBlockStart); --pf-v6-c-alert__icon--FontSize: var(--mui-alert__icon--FontSize); --pf-v6-c-alert--BoxShadow: var(--mui-alert--BoxShadow);