Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Enforce fine-grained form and flow validation #117

Merged
merged 1 commit into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading