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

Signed-off-by: Tyler Ohlsen <[email protected]>
opensearch-project · Mar 27, 2024 · 5231f09 · 5231f09
1 parent b3e0305
commit 5231f09
Show file tree

Hide file tree

Showing 13 changed files with 195 additions and 99 deletions.
diff --git a/common/utils.ts b/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

diff --git a/public/pages/workflow_detail/component_details/component_details.tsx b/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 />
           )}

diff --git a/public/pages/workflow_detail/component_details/component_inputs.tsx b/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}
+      />
     </>
   );
 }
diff --git a/public/pages/workflow_detail/component_details/input_field_list.tsx b/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>
             );

diff --git a/public/pages/workflow_detail/component_details/input_fields/select_field.tsx b/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,

diff --git a/public/pages/workflow_detail/component_details/input_fields/text_field.tsx b/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>
         );

diff --git a/public/pages/workflow_detail/components/header.tsx b/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}

diff --git a/public/pages/workflow_detail/utils/utils.ts b/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(

diff --git a/public/pages/workflow_detail/workflow_detail.tsx b/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 && (