diff --git a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx index 2269ba90b89..f6d66f688fb 100644 --- a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx +++ b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx @@ -153,3 +153,28 @@ MultipleModelings.args = { methodCanBeModeled: true, viewState, }; + +export const ValidationError = Template.bind({}); +ValidationError.args = { + method, + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "source" }, + ], + methodCanBeModeled: true, + viewState, +}; + +export const MultipleValidationErrors = Template.bind({}); +MultipleValidationErrors.args = { + method, + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "sink" }, + { ...modeledMethod, type: "sink" }, + { ...modeledMethod, type: "neutral", kind: "source" }, + ], + methodCanBeModeled: true, + viewState, +}; diff --git a/extensions/ql-vscode/src/view/method-modeling/ModeledMethodAlert.tsx b/extensions/ql-vscode/src/view/method-modeling/ModeledMethodAlert.tsx index 111109d3534..7a638a22c8d 100644 --- a/extensions/ql-vscode/src/view/method-modeling/ModeledMethodAlert.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/ModeledMethodAlert.tsx @@ -6,12 +6,12 @@ import { useCallback } from "react"; type Props = { error: ModeledMethodValidationError; - setSelectedIndex: (index: number) => void; + setSelectedIndex?: (index: number) => void; }; export const ModeledMethodAlert = ({ error, setSelectedIndex }: Props) => { const handleClick = useCallback(() => { - setSelectedIndex(error.index); + setSelectedIndex?.(error.index); }, [error.index, setSelectedIndex]); return ( @@ -22,7 +22,11 @@ export const ModeledMethodAlert = ({ error, setSelectedIndex }: Props) => { message={ <> {error.message}{" "} - {error.actionText} + {setSelectedIndex ? ( + {error.actionText} + ) : ( + error.actionText + )} } /> diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index 1125d86c9ec..13c50fdcdc9 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -31,6 +31,8 @@ import { ModelEditorViewState } from "../../model-editor/shared/view-state"; import { Codicon } from "../common"; import { canAddNewModeledMethod } from "../../model-editor/shared/multiple-modeled-methods"; import { DataGridCell, DataGridRow } from "../common/DataGrid"; +import { validateModeledMethods } from "../../model-editor/shared/validation"; +import { ModeledMethodAlert } from "../method-modeling/ModeledMethodAlert"; const ApiOrMethodRow = styled.div` min-height: calc(var(--input-height) * 1px); @@ -111,6 +113,11 @@ const ModelableMethodRow = forwardRef( [modeledMethodsProp, method, viewState], ); + const validationErrors = useMemo( + () => validateModeledMethods(modeledMethods), + [modeledMethods], + ); + const modeledMethodChangedHandlers = useMemo( () => modeledMethods.map((_, index) => (modeledMethod: ModeledMethod) => { @@ -163,7 +170,9 @@ const ModelableMethodRow = forwardRef( ref={ref} focused={revealedMethodSignature === method.signature} > - + @@ -200,61 +209,69 @@ const ModelableMethodRow = forwardRef( )} )} - {!props.modelingInProgress && - modeledMethods.map((modeledMethod, index) => ( - - - - - - - - - - - - - - {viewState.showMultipleModels && ( + {!props.modelingInProgress && ( + <> + {modeledMethods.map((modeledMethod, index) => ( + + + + + + + - {index === modeledMethods.length - 1 ? ( - - - - ) : ( - - - - )} + - )} - - ))} + + + + {viewState.showMultipleModels && ( + + {index === modeledMethods.length - 1 ? ( + + + + ) : ( + + + + )} + + )} + + ))} + {validationErrors.map((error, index) => ( + + + + ))} + + )} ); }, diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx index bd690ce8cff..d798b291fd9 100644 --- a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx +++ b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx @@ -438,4 +438,62 @@ describe(MethodRow.name, () => { { ...modeledMethod, type: "summary" }, ]); }); + + it("does not display validation errors when everything is valid", () => { + render({ + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "sink" }, + ], + viewState: { + ...viewState, + showMultipleModels: true, + }, + }); + + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + }); + + it("displays a single validation error", () => { + render({ + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "source" }, + ], + viewState: { + ...viewState, + showMultipleModels: true, + }, + }); + + expect(screen.getByRole("alert")).toBeInTheDocument(); + expect( + screen.getByText("Error: Duplicated classification"), + ).toBeInTheDocument(); + expect( + screen.queryByText("Error: Conflicting classification"), + ).not.toBeInTheDocument(); + }); + + it("displays multiple validation errors", () => { + render({ + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "neutral", kind: "source" }, + ], + viewState: { + ...viewState, + showMultipleModels: true, + }, + }); + + expect(screen.getAllByRole("alert").length).toBe(2); + expect( + screen.getByText("Error: Duplicated classification"), + ).toBeInTheDocument(); + expect( + screen.getByText("Error: Conflicting classification"), + ).toBeInTheDocument(); + }); });