Enforce fine-grained form and flow validation (#114) (#117)

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
(cherry picked from commit 5231f09)

Co-authored-by: Tyler Ohlsen <ohltyler@amazon.com>

Author: opensearch-trigger-bot[bot]

common/utils.ts

@@ -76,6 +76,7 @@ export function toWorkspaceFlow(
 /**
  * Validates the UI workflow state.
  * Note we don't have to validate connections since that is done via input/output handlers.
+ * But we need to validate there are no open connections
  */
 export function validateWorkspaceFlow(
   workspaceFlow: WorkspaceFlowState

public/pages/workflow_detail/component_details/component_details.tsx

@@ -13,6 +13,7 @@ import { EmptyComponentInputs } from './empty_component_inputs';
 import '../workspace/workspace-styles.scss';
 
 interface ComponentDetailsProps {
+  onFormChange: () => void;
   selectedComponent?: ReactFlowComponent;
 }
 
@@ -31,7 +32,10 @@ export function ComponentDetails(props: ComponentDetailsProps) {
       <EuiFlexItem>
         <EuiPanel paddingSize="m">
           {props.selectedComponent ? (
-            <ComponentInputs selectedComponent={props.selectedComponent} />
+            <ComponentInputs
+              selectedComponent={props.selectedComponent}
+              onFormChange={props.onFormChange}
+            />
           ) : (
             <EmptyComponentInputs />
           )}

public/pages/workflow_detail/component_details/component_inputs.tsx

@@ -10,6 +10,7 @@ import { ReactFlowComponent } from '../../../../common';
 
 interface ComponentInputsProps {
   selectedComponent: ReactFlowComponent;
+  onFormChange: () => void;
 }
 
 export function ComponentInputs(props: ComponentInputsProps) {
@@ -19,7 +20,10 @@ export function ComponentInputs(props: ComponentInputsProps) {
         <h2>{props.selectedComponent.data.label || ''}</h2>
       </EuiTitle>
       <EuiSpacer size="s" />
-      <InputFieldList selectedComponent={props.selectedComponent} />
+      <InputFieldList
+        selectedComponent={props.selectedComponent}
+        onFormChange={props.onFormChange}
+      />
     </>
   );
 }

public/pages/workflow_detail/component_details/input_field_list.tsx

@@ -15,6 +15,7 @@ import { ReactFlowComponent } from '../../../../common';
 
 interface InputFieldListProps {
   selectedComponent: ReactFlowComponent;
+  onFormChange: () => void;
 }
 
 export function InputFieldList(props: InputFieldListProps) {
@@ -30,6 +31,7 @@ export function InputFieldList(props: InputFieldListProps) {
                 <TextField
                   field={field}
                   componentId={props.selectedComponent.id}
+                  onFormChange={props.onFormChange}
                 />
                 <EuiSpacer size="s" />
               </EuiFlexItem>
@@ -42,6 +44,7 @@ export function InputFieldList(props: InputFieldListProps) {
                 <SelectField
                   field={field}
                   componentId={props.selectedComponent.id}
+                  onFormChange={props.onFormChange}
                 />
               </EuiFlexItem>
             );

public/pages/workflow_detail/component_details/input_fields/select_field.tsx

@@ -36,6 +36,7 @@ const existingIndices = [
 interface SelectFieldProps {
   field: IComponentField;
   componentId: string;
+  onFormChange: () => void;
 }
 
 /**
@@ -56,8 +57,8 @@ export function SelectField(props: SelectFieldProps) {
               options={options}
               valueOfSelected={field.value || getInitialValue(props.field.type)}
               onChange={(option) => {
-                field.onChange(option);
                 form.setFieldValue(formField, option);
+                props.onFormChange();
               }}
               isInvalid={isFieldInvalid(
                 props.componentId,

public/pages/workflow_detail/component_details/input_fields/text_field.tsx

@@ -17,6 +17,7 @@ import {
 interface TextFieldProps {
   field: IComponentField;
   componentId: string;
+  onFormChange: () => void;
 }
 
 /**
@@ -46,6 +47,12 @@ export function TextField(props: TextFieldProps) {
               placeholder={props.field.placeholder || ''}
               compressed={false}
               value={field.value || getInitialValue(props.field.type)}
+              onChange={(e) => form.setFieldValue(formField, e.target.value)}
+              // This is a design decision to only trigger form updates onBlur() instead
+              // of onChange(). This is to rate limit the number of updates & re-renders made, as users
+              // typically rapidly type things into a text box, which would consequently trigger
+              // onChange() much more often.
+              onBlur={() => props.onFormChange()}
             />
           </EuiFormRow>
         );

public/pages/workflow_detail/components/header.tsx

@@ -33,21 +33,9 @@ export function WorkflowDetailHeader(props: WorkflowDetailHeaderProps) {
         )
       }
       rightSideItems={[
-        // TODO: add launch logic
-        <EuiButton fill={false} onClick={() => {}}>
-          Launch
-        </EuiButton>,
-        <EuiButton
-          fill={false}
-          disabled={!props.workflow || !isDirty}
-          // TODO: if isNewWorkflow is true, clear the workflow cache if saving is successful.
-          onClick={() => {
-            // @ts-ignore
-            saveWorkflow(props.workflow, reactFlowInstance);
-            dispatch(removeDirty());
-          }}
-        >
-          Save
+        // TODO: finalize if this is needed
+        <EuiButton fill={false} color="danger" onClick={() => {}}>
+          Delete
         </EuiButton>,
       ]}
       tabs={props.tabs}

public/pages/workflow_detail/utils/utils.ts

@@ -3,41 +3,20 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import {
-  WorkspaceFlowState,
-  Workflow,
-  ReactFlowComponent,
-  toTemplateFlows,
-  validateWorkspaceFlow,
-} from '../../../../common';
+import { Workflow, ReactFlowComponent } from '../../../../common';
 
-export function saveWorkflow(workflow: Workflow, rfInstance: any): void {
-  let curFlowState = rfInstance.toObject();
-
-  curFlowState = {
-    ...curFlowState,
-    nodes: processNodes(curFlowState.nodes),
-  };
-
-  const isValid = validateWorkspaceFlow(curFlowState);
-  if (isValid) {
-    const updatedWorkflow = {
-      ...workflow,
-      workspaceFlowState: curFlowState,
-      workflows: toTemplateFlows(curFlowState),
-    } as Workflow;
-    if (workflow.id) {
-      // TODO: implement connection to update workflow API
-    } else {
-      // TODO: implement connection to create workflow API
-    }
+export function saveWorkflow(workflow?: Workflow): void {
+  if (workflow && workflow.id) {
+    // TODO: implement connection to update workflow API
   } else {
-    return;
+    // TODO: implement connection to create workflow API
   }
 }
 
 // Process the raw ReactFlow nodes to only persist the fields we need
-function processNodes(nodes: ReactFlowComponent[]): ReactFlowComponent[] {
+export function processNodes(
+  nodes: ReactFlowComponent[]
+): ReactFlowComponent[] {
   return nodes
     .map((node: ReactFlowComponent) => {
       return Object.fromEntries(

public/pages/workflow_detail/workflow_detail.tsx

@@ -98,22 +98,11 @@ export function WorkflowDetail(props: WorkflowDetailProps) {
 
   // On initial load:
   // - fetch workflow, if there is an existing workflow ID
-  // - add a window listener to warn users if they exit/refresh
-  //   without saving latest changes
   useEffect(() => {
     if (!isNewWorkflow) {
       // TODO: can optimize to only fetch a single workflow
       dispatch(searchWorkflows({ query: { match_all: {} } }));
     }
-
-    // TODO: below has the following issue:
-    // 1. user starts to create new unsaved workflow changes
-    // 2. user navigates to other parts of the plugin without refreshing - no warning happens
-    // 3. user refreshes at any later time: if isDirty is still true, shows browser warning
-    // tune to only handle the check if still on the workflow details page, or consider adding a check / warning
-    // if navigating away from the details page without refreshing (where it is currently not being triggered)
-    // window.onbeforeunload = (e) =>
-    //   isDirty || isNewWorkflow ? true : undefined;
   }, []);
 
   const tabs = [
@@ -156,7 +145,10 @@ export function WorkflowDetail(props: WorkflowDetailProps) {
             tabs={tabs}
           />
           {selectedTabId === WORKFLOW_DETAILS_TAB.EDITOR && (
-            <ResizableWorkspace workflow={workflow} />
+            <ResizableWorkspace
+              isNewWorkflow={isNewWorkflow}
+              workflow={workflow}
+            />
           )}
           {selectedTabId === WORKFLOW_DETAILS_TAB.LAUNCHES && <Launches />}
           {selectedTabId === WORKFLOW_DETAILS_TAB.PROTOTYPE && (

public/pages/workflow_detail/workspace/resizable_workspace.tsx

@@ -4,11 +4,17 @@
  */
 
 import React, { useRef, useState, useEffect, useContext } from 'react';
+import { useDispatch, useSelector } from 'react-redux';
 import { useOnSelectionChange } from 'reactflow';
 import { Form, Formik } from 'formik';
 import * as yup from 'yup';
 import { cloneDeep } from 'lodash';
-import { EuiButton, EuiResizableContainer } from '@elastic/eui';
+import {
+  EuiButton,
+  EuiCallOut,
+  EuiPageHeader,
+  EuiResizableContainer,
+} from '@elastic/eui';
 import {
   Workflow,
   WorkspaceFormValues,
@@ -17,12 +23,18 @@ import {
   WorkspaceSchemaObj,
   componentDataToFormik,
   getComponentSchema,
+  toWorkspaceFlow,
+  validateWorkspaceFlow,
+  WorkspaceFlowState,
+  toTemplateFlows,
 } from '../../../../common';
-import { rfContext } from '../../../store';
+import { AppState, removeDirty, setDirty, rfContext } from '../../../store';
 import { Workspace } from './workspace';
 import { ComponentDetails } from '../component_details';
+import { processNodes, saveWorkflow } from '../utils';
 
 interface ResizableWorkspaceProps {
+  isNewWorkflow: boolean;
   workflow?: Workflow;
 }
 
@@ -33,6 +45,27 @@ const COMPONENT_DETAILS_PANEL_ID = 'component_details_panel_id';
  * panels - the ReactFlow workspace panel and the selected component details panel.
  */
 export function ResizableWorkspace(props: ResizableWorkspaceProps) {
+  const dispatch = useDispatch();
+
+  // Overall workspace state
+  const isDirty = useSelector((state: AppState) => state.workspace.isDirty);
+  const [isFirstSave, setIsFirstSave] = useState<boolean>(props.isNewWorkflow);
+  const isSaveable = isFirstSave ? true : isDirty;
+
+  // Workflow state
+  const [workflow, setWorkflow] = useState<Workflow | undefined>(
+    props.workflow
+  );
+
+  // Formik form state
+  const [formValues, setFormValues] = useState<WorkspaceFormValues>({});
+  const [formSchema, setFormSchema] = useState<WorkspaceSchema>(yup.object({}));
+
+  // Validation states. Maintain separate state for form vs. overall flow so
+  // we can have fine-grained errors and action items for users
+  const [formValidOnSubmit, setFormValidOnSubmit] = useState<boolean>(true);
+  const [flowValidOnSubmit, setFlowValidOnSubmit] = useState<boolean>(true);
+
   // Component details side panel state
   const [isDetailsPanelOpen, setisDetailsPanelOpen] = useState<boolean>(true);
   const collapseFn = useRef(
@@ -68,6 +101,25 @@ export function ResizableWorkspace(props: ResizableWorkspaceProps) {
     },
   });
 
+  // Hook to update the workflow's flow state, if applicable. It may not exist if
+  // it is a backend-only-created workflow, or a new, unsaved workflow. If so,
+  // generate a default one based on the 'workflows' JSON field.
+  useEffect(() => {
+    const workflowCopy = { ...props.workflow } as Workflow;
+    if (workflowCopy) {
+      if (!workflowCopy.workspaceFlowState) {
+        workflowCopy.workspaceFlowState = toWorkspaceFlow(
+          workflowCopy.workflows
+        );
+        console.debug(
+          `There is no saved UI flow for workflow: ${workflowCopy.name}. Generating a default one.`
+        );
+      }
+      setWorkflow(workflowCopy);
+    }
+  }, [props.workflow]);
+
+  // Hook to updated the selected ReactFlow component
   useEffect(() => {
     reactFlowInstance?.setNodes((nodes: ReactFlowComponent[]) =>
       nodes.map((node) => {
@@ -80,24 +132,20 @@ export function ResizableWorkspace(props: ResizableWorkspaceProps) {
     );
   }, [selectedComponent]);
 
-  // Formik form state
-  const [formValues, setFormValues] = useState<WorkspaceFormValues>({});
-  const [formSchema, setFormSchema] = useState<WorkspaceSchema>(yup.object({}));
-
   // Initialize the form state to an existing workflow, if applicable.
   useEffect(() => {
-    if (props.workflow?.workspaceFlowState) {
+    if (workflow?.workspaceFlowState) {
       const initFormValues = {} as WorkspaceFormValues;
       const initSchemaObj = {} as WorkspaceSchemaObj;
-      props.workflow.workspaceFlowState.nodes.forEach((node) => {
+      workflow.workspaceFlowState.nodes.forEach((node) => {
         initFormValues[node.id] = componentDataToFormik(node.data);
         initSchemaObj[node.id] = getComponentSchema(node.data);
       });
       const initFormSchema = yup.object(initSchemaObj) as WorkspaceSchema;
       setFormValues(initFormValues);
       setFormSchema(initFormSchema);
     }
-  }, [props.workflow]);
+  }, [workflow]);
 
   // Update the form values and validation schema when a node is added
   // or removed from the workspace.
@@ -129,6 +177,16 @@ export function ResizableWorkspace(props: ResizableWorkspaceProps) {
     setFormSchema(updatedSchema);
   }
 
+  /**
+   * Function to pass down to the Formik <Form> components as a listener to propagate
+   * form changes to this parent component to re-enable save button, etc.
+   */
+  function onFormChange() {
+    if (!isDirty) {
+      dispatch(setDirty());
+    }
+  }
+
   return (
     <Formik
       enableReinitialize={true}
@@ -139,9 +197,84 @@ export function ResizableWorkspace(props: ResizableWorkspaceProps) {
     >
       {(formikProps) => (
         <Form>
+          {!formValidOnSubmit && (
+            <EuiCallOut
+              title="There are empty or invalid fields"
+              color="danger"
+              iconType="alert"
+              style={{ marginBottom: '16px' }}
+            >
+              Please address the highlighted fields and try saving again.
+            </EuiCallOut>
+          )}
+          {!flowValidOnSubmit && (
+            <EuiCallOut
+              title="The configured flow is invalid"
+              color="danger"
+              iconType="alert"
+              style={{ marginBottom: '16px' }}
+            >
+              Please ensure there are no open connections between the
+              components.
+            </EuiCallOut>
+          )}
+          <EuiPageHeader
+            style={{ marginBottom: '8px' }}
+            rightSideItems={[
+              // TODO: add launch logic
+              <EuiButton fill={false} onClick={() => {}}>
+                Launch
+              </EuiButton>,
+              <EuiButton
+                fill={false}
+                disabled={!isSaveable}
+                // TODO: if props.isNewWorkflow is true, clear the workflow cache if saving is successful.
+                onClick={() => {
+                  dispatch(removeDirty());
+                  if (isFirstSave) {
+                    setIsFirstSave(false);
+                  }
+                  // Submit the form to bubble up any errors.
+                  // Ideally we handle Promise accept/rejects with submitForm(), but there is
+                  // open issues for that - see https://github.com/jaredpalmer/formik/issues/2057
+                  // The workaround is to additionally execute validateForm() which will return any errors found.
+                  formikProps.submitForm();
+                  formikProps.validateForm().then((validationResults: {}) => {
+                    if (Object.keys(validationResults).length > 0) {
+                      setFormValidOnSubmit(false);
+                    } else {
+                      setFormValidOnSubmit(true);
+                      // @ts-ignore
+                      let curFlowState = reactFlowInstance.toObject() as WorkspaceFlowState;
+                      curFlowState = {
+                        ...curFlowState,
+                        nodes: processNodes(curFlowState.nodes),
+                      };
+                      if (validateWorkspaceFlow(curFlowState)) {
+                        setFlowValidOnSubmit(true);
+                        const updatedWorkflow = {
+                          ...workflow,
+                          workspaceFlowState: curFlowState,
+                          workflows: toTemplateFlows(curFlowState),
+                        } as Workflow;
+                        saveWorkflow(updatedWorkflow);
+                      } else {
+                        setFlowValidOnSubmit(false);
+                      }
+                    }
+                  });
+                }}
+              >
+                Save
+              </EuiButton>,
+            ]}
+            bottomBorder={false}
+          />
           <EuiResizableContainer
             direction="horizontal"
-            style={{ marginLeft: '-8px', marginTop: '-8px' }}
+            style={{
+              marginLeft: '-8px',
+            }}
           >
             {(EuiResizablePanel, EuiResizableButton, { togglePanel }) => {
               if (togglePanel) {
@@ -153,33 +286,34 @@ export function ResizableWorkspace(props: ResizableWorkspaceProps) {
                 <>
                   <EuiResizablePanel
                     mode="main"
-                    initialSize={75}
+                    initialSize={80}
                     minSize="50%"
                     paddingSize="s"
                   >
                     <Workspace
-                      workflow={props.workflow}
+                      workflow={workflow}
                       onNodesChange={onNodesChange}
                     />
                   </EuiResizablePanel>
                   <EuiResizableButton />
                   <EuiResizablePanel
+                    style={{ marginRight: '-16px' }}
                     id={COMPONENT_DETAILS_PANEL_ID}
                     mode="collapsible"
                     initialSize={25}
                     minSize="10%"
                     paddingSize="s"
                     onToggleCollapsedInternal={() => onToggleChange()}
                   >
-                    <ComponentDetails selectedComponent={selectedComponent} />
+                    <ComponentDetails
+                      selectedComponent={selectedComponent}
+                      onFormChange={onFormChange}
+                    />
                   </EuiResizablePanel>
                 </>
               );
             }}
           </EuiResizableContainer>
-          <EuiButton onClick={() => formikProps.handleSubmit()}>
-            Submit
-          </EuiButton>
         </Form>
       )}
     </Formik>

public/pages/workflow_detail/workspace/workspace.tsx

@@ -21,7 +21,6 @@ import {
   IComponentData,
   ReactFlowComponent,
   Workflow,
-  toWorkspaceFlow,
 } from '../../../../common';
 import { generateId, initComponentData } from '../../../utils';
 import { WorkspaceComponent } from '../workspace_component';
@@ -116,20 +115,10 @@ export function Workspace(props: WorkspaceProps) {
     [reactFlowInstance]
   );
 
-  // Initialization. Set the nodes and edges to an existing workflow,
-  // if applicable.
+  // Initialization. Set the nodes and edges to an existing workflow state,
   useEffect(() => {
     const workflow = { ...props.workflow };
-    if (workflow) {
-      if (!workflow.workspaceFlowState) {
-        // No existing workspace state. This could be due to it being a backend-only-created
-        // workflow, or a new, unsaved workflow
-        // @ts-ignore
-        workflow.workspaceFlowState = toWorkspaceFlow(workflow.workflows);
-        console.debug(
-          `There is no saved UI flow for workflow: ${workflow.name}. Generating a default one.`
-        );
-      }
+    if (workflow && workflow.workspaceFlowState) {
       setNodes(workflow.workspaceFlowState.nodes);
       setEdges(workflow.workspaceFlowState.edges);
     }

public/pages/workflows/workflows.test.tsx

@@ -40,7 +40,7 @@ const renderWithRouter = () => ({
 
 describe('Workflows', () => {
   test('renders the page', () => {
-    const { getByText } = renderWithRouter();
-    expect(getByText('Workflows')).not.toBeNull();
+    const { getAllByText } = renderWithRouter();
+    expect(getAllByText('Workflows').length).toBeGreaterThan(0);
   });
 });

public/pages/workflows/workflows.tsx

@@ -58,20 +58,14 @@ export function Workflows(props: WorkflowsProps) {
   ] as WORKFLOWS_TAB;
   const [selectedTabId, setSelectedTabId] = useState<WORKFLOWS_TAB>(tabFromUrl);
 
-  // If there is no selected tab or invalid tab, default to a tab depending
-  // on if user has existing created workflows or not.
+  // If there is no selected tab or invalid tab, default to manage tab
   useEffect(() => {
     if (
       !selectedTabId ||
       !Object.values(WORKFLOWS_TAB).includes(selectedTabId)
     ) {
-      if (Object.keys(workflows).length > 0) {
-        setSelectedTabId(WORKFLOWS_TAB.MANAGE);
-        replaceActiveTab(WORKFLOWS_TAB.MANAGE, props);
-      } else {
-        setSelectedTabId(WORKFLOWS_TAB.CREATE);
-        replaceActiveTab(WORKFLOWS_TAB.CREATE, props);
-      }
+      setSelectedTabId(WORKFLOWS_TAB.MANAGE);
+      replaceActiveTab(WORKFLOWS_TAB.MANAGE, props);
     }
   }, [selectedTabId, workflows]);