Skip to content

Commit 23ea277

Browse files
ohltylergithub-actions[bot]
authored andcommitted
Enforce fine-grained form and flow validation (#114)
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com> (cherry picked from commit 5231f09)
1 parent 74714ce commit 23ea277

File tree

13 files changed

+195
-99
lines changed

13 files changed

+195
-99
lines changed

common/utils.ts

+1
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ export function toWorkspaceFlow(
7676
/**
7777
* Validates the UI workflow state.
7878
* Note we don't have to validate connections since that is done via input/output handlers.
79+
* But we need to validate there are no open connections
7980
*/
8081
export function validateWorkspaceFlow(
8182
workspaceFlow: WorkspaceFlowState

public/pages/workflow_detail/component_details/component_details.tsx

+5-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { EmptyComponentInputs } from './empty_component_inputs';
1313
import '../workspace/workspace-styles.scss';
1414

1515
interface ComponentDetailsProps {
16+
onFormChange: () => void;
1617
selectedComponent?: ReactFlowComponent;
1718
}
1819

@@ -31,7 +32,10 @@ export function ComponentDetails(props: ComponentDetailsProps) {
3132
<EuiFlexItem>
3233
<EuiPanel paddingSize="m">
3334
{props.selectedComponent ? (
34-
<ComponentInputs selectedComponent={props.selectedComponent} />
35+
<ComponentInputs
36+
selectedComponent={props.selectedComponent}
37+
onFormChange={props.onFormChange}
38+
/>
3539
) : (
3640
<EmptyComponentInputs />
3741
)}

public/pages/workflow_detail/component_details/component_inputs.tsx

+5-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { ReactFlowComponent } from '../../../../common';
1010

1111
interface ComponentInputsProps {
1212
selectedComponent: ReactFlowComponent;
13+
onFormChange: () => void;
1314
}
1415

1516
export function ComponentInputs(props: ComponentInputsProps) {
@@ -19,7 +20,10 @@ export function ComponentInputs(props: ComponentInputsProps) {
1920
<h2>{props.selectedComponent.data.label || ''}</h2>
2021
</EuiTitle>
2122
<EuiSpacer size="s" />
22-
<InputFieldList selectedComponent={props.selectedComponent} />
23+
<InputFieldList
24+
selectedComponent={props.selectedComponent}
25+
onFormChange={props.onFormChange}
26+
/>
2327
</>
2428
);
2529
}

public/pages/workflow_detail/component_details/input_field_list.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { ReactFlowComponent } from '../../../../common';
1515

1616
interface InputFieldListProps {
1717
selectedComponent: ReactFlowComponent;
18+
onFormChange: () => void;
1819
}
1920

2021
export function InputFieldList(props: InputFieldListProps) {
@@ -30,6 +31,7 @@ export function InputFieldList(props: InputFieldListProps) {
3031
<TextField
3132
field={field}
3233
componentId={props.selectedComponent.id}
34+
onFormChange={props.onFormChange}
3335
/>
3436
<EuiSpacer size="s" />
3537
</EuiFlexItem>
@@ -42,6 +44,7 @@ export function InputFieldList(props: InputFieldListProps) {
4244
<SelectField
4345
field={field}
4446
componentId={props.selectedComponent.id}
47+
onFormChange={props.onFormChange}
4548
/>
4649
</EuiFlexItem>
4750
);

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const existingIndices = [
3636
interface SelectFieldProps {
3737
field: IComponentField;
3838
componentId: string;
39+
onFormChange: () => void;
3940
}
4041

4142
/**
@@ -56,8 +57,8 @@ export function SelectField(props: SelectFieldProps) {
5657
options={options}
5758
valueOfSelected={field.value || getInitialValue(props.field.type)}
5859
onChange={(option) => {
59-
field.onChange(option);
6060
form.setFieldValue(formField, option);
61+
props.onFormChange();
6162
}}
6263
isInvalid={isFieldInvalid(
6364
props.componentId,

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

+7
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
interface TextFieldProps {
1818
field: IComponentField;
1919
componentId: string;
20+
onFormChange: () => void;
2021
}
2122

2223
/**
@@ -46,6 +47,12 @@ export function TextField(props: TextFieldProps) {
4647
placeholder={props.field.placeholder || ''}
4748
compressed={false}
4849
value={field.value || getInitialValue(props.field.type)}
50+
onChange={(e) => form.setFieldValue(formField, e.target.value)}
51+
// This is a design decision to only trigger form updates onBlur() instead
52+
// of onChange(). This is to rate limit the number of updates & re-renders made, as users
53+
// typically rapidly type things into a text box, which would consequently trigger
54+
// onChange() much more often.
55+
onBlur={() => props.onFormChange()}
4956
/>
5057
</EuiFormRow>
5158
);

public/pages/workflow_detail/components/header.tsx

+3-15
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,9 @@ export function WorkflowDetailHeader(props: WorkflowDetailHeaderProps) {
3333
)
3434
}
3535
rightSideItems={[
36-
// TODO: add launch logic
37-
<EuiButton fill={false} onClick={() => {}}>
38-
Launch
39-
</EuiButton>,
40-
<EuiButton
41-
fill={false}
42-
disabled={!props.workflow || !isDirty}
43-
// TODO: if isNewWorkflow is true, clear the workflow cache if saving is successful.
44-
onClick={() => {
45-
// @ts-ignore
46-
saveWorkflow(props.workflow, reactFlowInstance);
47-
dispatch(removeDirty());
48-
}}
49-
>
50-
Save
36+
// TODO: finalize if this is needed
37+
<EuiButton fill={false} color="danger" onClick={() => {}}>
38+
Delete
5139
</EuiButton>,
5240
]}
5341
tabs={props.tabs}

public/pages/workflow_detail/utils/utils.ts

+8-29
Original file line numberDiff line numberDiff line change
@@ -3,41 +3,20 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import {
7-
WorkspaceFlowState,
8-
Workflow,
9-
ReactFlowComponent,
10-
toTemplateFlows,
11-
validateWorkspaceFlow,
12-
} from '../../../../common';
6+
import { Workflow, ReactFlowComponent } from '../../../../common';
137

14-
export function saveWorkflow(workflow: Workflow, rfInstance: any): void {
15-
let curFlowState = rfInstance.toObject();
16-
17-
curFlowState = {
18-
...curFlowState,
19-
nodes: processNodes(curFlowState.nodes),
20-
};
21-
22-
const isValid = validateWorkspaceFlow(curFlowState);
23-
if (isValid) {
24-
const updatedWorkflow = {
25-
...workflow,
26-
workspaceFlowState: curFlowState,
27-
workflows: toTemplateFlows(curFlowState),
28-
} as Workflow;
29-
if (workflow.id) {
30-
// TODO: implement connection to update workflow API
31-
} else {
32-
// TODO: implement connection to create workflow API
33-
}
8+
export function saveWorkflow(workflow?: Workflow): void {
9+
if (workflow && workflow.id) {
10+
// TODO: implement connection to update workflow API
3411
} else {
35-
return;
12+
// TODO: implement connection to create workflow API
3613
}
3714
}
3815

3916
// Process the raw ReactFlow nodes to only persist the fields we need
40-
function processNodes(nodes: ReactFlowComponent[]): ReactFlowComponent[] {
17+
export function processNodes(
18+
nodes: ReactFlowComponent[]
19+
): ReactFlowComponent[] {
4120
return nodes
4221
.map((node: ReactFlowComponent) => {
4322
return Object.fromEntries(

public/pages/workflow_detail/workflow_detail.tsx

+4-12
Original file line numberDiff line numberDiff line change
@@ -98,22 +98,11 @@ export function WorkflowDetail(props: WorkflowDetailProps) {
9898

9999
// On initial load:
100100
// - fetch workflow, if there is an existing workflow ID
101-
// - add a window listener to warn users if they exit/refresh
102-
// without saving latest changes
103101
useEffect(() => {
104102
if (!isNewWorkflow) {
105103
// TODO: can optimize to only fetch a single workflow
106104
dispatch(searchWorkflows({ query: { match_all: {} } }));
107105
}
108-
109-
// TODO: below has the following issue:
110-
// 1. user starts to create new unsaved workflow changes
111-
// 2. user navigates to other parts of the plugin without refreshing - no warning happens
112-
// 3. user refreshes at any later time: if isDirty is still true, shows browser warning
113-
// tune to only handle the check if still on the workflow details page, or consider adding a check / warning
114-
// if navigating away from the details page without refreshing (where it is currently not being triggered)
115-
// window.onbeforeunload = (e) =>
116-
// isDirty || isNewWorkflow ? true : undefined;
117106
}, []);
118107

119108
const tabs = [
@@ -156,7 +145,10 @@ export function WorkflowDetail(props: WorkflowDetailProps) {
156145
tabs={tabs}
157146
/>
158147
{selectedTabId === WORKFLOW_DETAILS_TAB.EDITOR && (
159-
<ResizableWorkspace workflow={workflow} />
148+
<ResizableWorkspace
149+
isNewWorkflow={isNewWorkflow}
150+
workflow={workflow}
151+
/>
160152
)}
161153
{selectedTabId === WORKFLOW_DETAILS_TAB.LAUNCHES && <Launches />}
162154
{selectedTabId === WORKFLOW_DETAILS_TAB.PROTOTYPE && (

0 commit comments

Comments
 (0)