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 <[email protected]>

* feat(ws): extract validation alert to its own component

Signed-off-by: Guilherme Caponetto <[email protected]>

* fix(ws): use error icon for helper text

Signed-off-by: Guilherme Caponetto <[email protected]>

---------

Signed-off-by: Guilherme Caponetto <[email protected]>

Author: caponetto

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 (
     <NavItem isActive={isActive} key={item.label} data-id={item.label} itemId={item.label}>

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<ValidationErrorAlertProps> = ({ title, errors }) => {
+  if (errors.length === 0) {
+    return null;
+  }
+
+  return (
+    <Alert variant="danger" title={title} isInline>
+      <List>
+        {errors.map((error, index) => (
+          <ListItem key={index}>{error.message}</ListItem>
+        ))}
+      </List>
+    </Alert>
+  );
+};

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<ValidationStatus>('default');
   const mode = useCurrentRouteKey() === 'workspaceKindCreate' ? 'create' : 'edit';
+  const [specErrors, setSpecErrors] = useState<ValidationError[]>([]);
+
   const [data, setData, resetData] = useGenericObjectState<WorkspaceKindFormData>({
     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 = () => {
       </PageGroup>
       <PageSection isFilled>
         {mode === 'create' && (
-          <WorkspaceKindFileUpload
-            resetData={resetData}
-            value={yamlValue}
-            setValue={setYamlValue}
-            validated={validated}
-            setValidated={setValidated}
-          />
+          <Stack>
+            {specErrors.length > 0 && (
+              <StackItem style={{ padding: SmallPadding.value }}>
+                <ValidationErrorAlert title="Error creating workspace kind" errors={specErrors} />
+              </StackItem>
+            )}
+            <StackItem style={{ height: '100%' }}>
+              <WorkspaceKindFileUpload
+                resetData={resetData}
+                value={yamlValue}
+                setValue={setYamlValue}
+                validated={validated}
+                setValidated={setValidated}
+                onClear={() => {
+                  setSpecErrors([]);
+                }}
+              />
+            </StackItem>
+          </Stack>
         )}
         {mode === 'edit' && (
           <>

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<WorkspaceKindFileUploadProps> = ({
   resetData,
   value,
   setValue,
   validated,
   setValidated,
+  onClear,
 }) => {
-  const isYamlFileRef = useRef(false);
   const [filename, setFilename] = useState('');
   const [isLoading, setIsLoading] = useState(false);
-  const [fileUploadHelperText, setFileUploadHelperText] = useState<string>('');
+  const [fileUploadHelperText, setFileUploadHelperText] = useState<string | undefined>();
 
   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<WorkspaceKindFileUploadProps> = (
     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<WorkspaceKindFileUploadProps> = (
         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>
-          <HelperText>
-            <HelperTextItem id="helper-text-example-helpText">
-              {fileUploadHelperText}
-            </HelperTextItem>
-          </HelperText>
-        </FileUploadHelperText>
+        {fileUploadHelperText && (
+          <FileUploadHelperText>
+            <HelperText>
+              <HelperTextItem id="helper-text-example-helpText" variant="error">
+                {fileUploadHelperText}
+              </HelperTextItem>
+            </HelperText>
+          </FileUploadHelperText>
+        )}
       </FileUpload>
     </Content>
   );

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: '<error_code>',
+        message: '<error_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 () => {

workspaces/frontend/src/shared/api/__tests__/notebookService.spec.ts

@@ -1,24 +1,20 @@
 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),
   restGET: jest.fn(() => mockRestPromise),
   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);
   });
 });

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 = <T>(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<string, unknown>).error === 'object' &&
+  (e as { error: unknown }).error !== null &&
+  typeof (e as { error: { message: unknown } }).error.message === 'string';
+
 export function extractNotebookResponse<T>(response: unknown): T {
   if (isNotebookResponse<T>(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<T>(promise: Promise<T>, extractData = true): Promise<T> {
+  try {
+    const res = await handleRestFailures<T>(promise);
+    return extractData ? extractNotebookResponse<T>(res) : res;
+  } catch (error) {
+    throw new ErrorEnvelopeException(extractErrorEnvelope(error));
+  }
+}
+
+export class ErrorEnvelopeException extends Error {
+  constructor(public envelope: ErrorEnvelope) {
+    super(envelope.error?.message ?? 'Unknown error');
+  }
+}