Skip to content

Commit

Permalink
Enforce fine-grained form and flow validation (#114)
Browse files Browse the repository at this point in the history
Signed-off-by: Tyler Ohlsen <[email protected]>
(cherry picked from commit 5231f09)
  • Loading branch information
ohltyler authored and github-actions[bot] committed Mar 27, 2024
1 parent 74714ce commit 23ea277
Show file tree
Hide file tree
Showing 13 changed files with 195 additions and 99 deletions.
1 change: 1 addition & 0 deletions common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { EmptyComponentInputs } from './empty_component_inputs';
import '../workspace/workspace-styles.scss';

interface ComponentDetailsProps {
onFormChange: () => void;
selectedComponent?: ReactFlowComponent;
}

Expand All @@ -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 />
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { ReactFlowComponent } from '../../../../common';

interface ComponentInputsProps {
selectedComponent: ReactFlowComponent;
onFormChange: () => void;
}

export function ComponentInputs(props: ComponentInputsProps) {
Expand All @@ -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}
/>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { ReactFlowComponent } from '../../../../common';

interface InputFieldListProps {
selectedComponent: ReactFlowComponent;
onFormChange: () => void;
}

export function InputFieldList(props: InputFieldListProps) {
Expand All @@ -30,6 +31,7 @@ export function InputFieldList(props: InputFieldListProps) {
<TextField
field={field}
componentId={props.selectedComponent.id}
onFormChange={props.onFormChange}
/>
<EuiSpacer size="s" />
</EuiFlexItem>
Expand All @@ -42,6 +44,7 @@ export function InputFieldList(props: InputFieldListProps) {
<SelectField
field={field}
componentId={props.selectedComponent.id}
onFormChange={props.onFormChange}
/>
</EuiFlexItem>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const existingIndices = [
interface SelectFieldProps {
field: IComponentField;
componentId: string;
onFormChange: () => void;
}

/**
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
interface TextFieldProps {
field: IComponentField;
componentId: string;
onFormChange: () => void;
}

/**
Expand Down Expand Up @@ -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>
);
Expand Down
18 changes: 3 additions & 15 deletions public/pages/workflow_detail/components/header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
37 changes: 8 additions & 29 deletions public/pages/workflow_detail/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
16 changes: 4 additions & 12 deletions public/pages/workflow_detail/workflow_detail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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 && (
Expand Down
Loading

0 comments on commit 23ea277

Please sign in to comment.