Skip to content
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
7 changes: 1 addition & 6 deletions extensions/ql-vscode/src/common/interface-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -572,11 +572,6 @@ interface HideModeledMethodsMessage {
hideModeledMethods: boolean;
}

interface SetModeledMethodMessage {
t: "setModeledMethod";
method: ModeledMethod;
}

interface SetMultipleModeledMethodsMessage {
t: "setMultipleModeledMethods";
methodSignature: string;
Expand Down Expand Up @@ -627,7 +622,7 @@ interface StartModelingMessage {

export type FromMethodModelingMessage =
| CommonFromViewMessages
| SetModeledMethodMessage
| SetMultipleModeledMethodsMessage
| RevealInEditorMessage
| StartModelingMessage;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { assertNever } from "../../common/helpers-pure";
import { ModelEditorViewTracker } from "../model-editor-view-tracker";
import { ModelConfigListener } from "../../config";
import { DatabaseItem } from "../../databases/local-databases";
import { convertFromLegacyModeledMethod } from "../shared/modeled-methods-legacy";

export class MethodModelingViewProvider extends AbstractWebviewViewProvider<
ToMethodModelingMessage,
Expand Down Expand Up @@ -108,19 +107,19 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider<
);
break;

case "setModeledMethod": {
case "setMultipleModeledMethods": {
if (!this.databaseItem) {
return;
}

this.modelingStore.updateModeledMethods(
this.databaseItem,
msg.method.signature,
convertFromLegacyModeledMethod(msg.method),
msg.methodSignature,
msg.modeledMethods,
);
this.modelingStore.addModifiedMethod(
this.databaseItem,
msg.method.signature,
msg.methodSignature,
);
break;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,5 @@
import { ModeledMethod } from "../modeled-method";

/**
* Converts a single ModeledMethod to a ModeledMethod[] for legacy usage. This function should always be used instead
* of the trivial conversion to track usages of this conversion.
*
* This method should only be called inside a `onMessage` function (or its equivalent). If it's used anywhere else,
* consider whether the boundary is correct: the boundary should as close as possible to the webview -> extension host
* boundary.
*
* @param modeledMethod The single ModeledMethod
*/
export function convertFromLegacyModeledMethod(
modeledMethod: ModeledMethod | undefined,
): ModeledMethod[] {
return modeledMethod ? [modeledMethod] : [];
}

/**
* Converts a ModeledMethod[] to a single ModeledMethod for legacy usage. This function should always be used instead
* of the trivial conversion to track usages of this conversion.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ const Template: StoryFn<typeof MultipleModeledMethodsPanelComponent> = (
}, [args.modeledMethods]);

const handleChange = useCallback(
(modeledMethods: ModeledMethod[]) => {
args.onChange(modeledMethods);
(methodSignature: string, modeledMethods: ModeledMethod[]) => {
args.onChange(methodSignature, modeledMethods);
setModeledMethods(modeledMethods);
},
[args],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export type MethodModelingProps = {
method: Method;
modeledMethods: ModeledMethod[];
showMultipleModels?: boolean;
onChange: (modeledMethod: ModeledMethod) => void;
onChange: (methodSignature: string, modeledMethods: ModeledMethod[]) => void;
};

export const MethodModeling = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,14 @@ export function MethodModelingView({ initialViewState }: Props): JSX.Element {
return <MethodAlreadyModeled />;
}

const onChange = (modeledMethod: ModeledMethod) => {
const onChange = (
methodSignature: string,
modeledMethods: ModeledMethod[],
) => {
vscode.postMessage({
t: "setModeledMethod",
method: modeledMethod,
t: "setMultipleModeledMethods",
methodSignature,
modeledMethods,
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export type ModeledMethodsPanelProps = {
method: Method;
modeledMethods: ModeledMethod[];
showMultipleModels: boolean;
onChange: (modeledMethod: ModeledMethod) => void;
onChange: (methodSignature: string, modeledMethods: ModeledMethod[]) => void;
};

const SingleMethodModelingInputs = styled(MethodModelingInputs)`
Expand All @@ -24,9 +24,9 @@ export const ModeledMethodsPanel = ({
showMultipleModels,
onChange,
}: ModeledMethodsPanelProps) => {
const handleMultipleChange = useCallback(
(modeledMethods: ModeledMethod[]) => {
onChange(modeledMethods[0]);
const handleSingleChange = useCallback(
(modeledMethod: ModeledMethod) => {
onChange(modeledMethod.signature, [modeledMethod]);
},
[onChange],
);
Expand All @@ -36,7 +36,7 @@ export const ModeledMethodsPanel = ({
<SingleMethodModelingInputs
method={method}
modeledMethod={convertToLegacyModeledMethod(modeledMethods)}
onChange={onChange}
onChange={handleSingleChange}
/>
);
}
Expand All @@ -45,7 +45,7 @@ export const ModeledMethodsPanel = ({
<MultipleModeledMethodsPanel
method={method}
modeledMethods={modeledMethods}
onChange={handleMultipleChange}
onChange={onChange}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Codicon } from "../common";
export type MultipleModeledMethodsPanelProps = {
method: Method;
modeledMethods: ModeledMethod[];
onChange: (modeledMethods: ModeledMethod[]) => void;
onChange: (methodSignature: string, modeledMethods: ModeledMethod[]) => void;
};

const Container = styled.div`
Expand Down Expand Up @@ -70,7 +70,7 @@ export const MultipleModeledMethodsPanel = ({

const newModeledMethods = [...modeledMethods, newModeledMethod];

onChange(newModeledMethods);
onChange(method.signature, newModeledMethods);
setSelectedIndex(newModeledMethods.length - 1);
}, [onChange, modeledMethods, method]);

Expand All @@ -84,9 +84,9 @@ export const MultipleModeledMethodsPanel = ({
? selectedIndex - 1
: selectedIndex;

onChange(newModeledMethods);
onChange(method.signature, newModeledMethods);
setSelectedIndex(newSelectedIndex);
}, [onChange, modeledMethods, selectedIndex]);
}, [onChange, modeledMethods, selectedIndex, method]);

const anyUnmodeled = useMemo(
() =>
Expand All @@ -100,12 +100,12 @@ export const MultipleModeledMethodsPanel = ({
if (modeledMethods.length > 0) {
const newModeledMethods = [...modeledMethods];
newModeledMethods[selectedIndex] = modeledMethod;
onChange(newModeledMethods);
onChange(method.signature, newModeledMethods);
} else {
onChange([modeledMethod]);
onChange(method.signature, [modeledMethod]);
}
},
[modeledMethods, selectedIndex, onChange],
[modeledMethods, selectedIndex, onChange, method],
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe(MultipleModeledMethodsPanel.name, () => {
reactRender(<MultipleModeledMethodsPanel {...props} />);

const method = createMethod();
const onChange = jest.fn<void, [ModeledMethod[]]>();
const onChange = jest.fn<void, [string, ModeledMethod[]]>();

describe("with no modeled methods", () => {
const modeledMethods: ModeledMethod[] = [];
Expand Down Expand Up @@ -138,7 +138,7 @@ describe(MultipleModeledMethodsPanel.name, () => {

await userEvent.click(screen.getByLabelText("Add modeling"));

expect(onChange).toHaveBeenCalledWith([
expect(onChange).toHaveBeenCalledWith(method.signature, [
...modeledMethods,
{
signature: method.signature,
Expand Down Expand Up @@ -265,7 +265,7 @@ describe(MultipleModeledMethodsPanel.name, () => {

await userEvent.selectOptions(modelTypeDropdown, "source");

expect(onChange).toHaveBeenCalledWith([
expect(onChange).toHaveBeenCalledWith(method.signature, [
{
signature: method.signature,
packageName: method.packageName,
Expand Down Expand Up @@ -297,7 +297,7 @@ describe(MultipleModeledMethodsPanel.name, () => {

await userEvent.selectOptions(modelTypeDropdown, "sink");

expect(onChange).toHaveBeenCalledWith([
expect(onChange).toHaveBeenCalledWith(method.signature, [
...modeledMethods.slice(0, 1),
{
signature: method.signature,
Expand All @@ -323,7 +323,10 @@ describe(MultipleModeledMethodsPanel.name, () => {

await userEvent.click(screen.getByLabelText("Delete modeling"));

expect(onChange).toHaveBeenCalledWith(modeledMethods.slice(1));
expect(onChange).toHaveBeenCalledWith(
method.signature,
modeledMethods.slice(1),
);
});

it("can add modeling", async () => {
Expand All @@ -335,7 +338,7 @@ describe(MultipleModeledMethodsPanel.name, () => {

await userEvent.click(screen.getByLabelText("Add modeling"));

expect(onChange).toHaveBeenCalledWith([
expect(onChange).toHaveBeenCalledWith(method.signature, [
...modeledMethods,
{
signature: method.signature,
Expand Down Expand Up @@ -506,7 +509,10 @@ describe(MultipleModeledMethodsPanel.name, () => {

await userEvent.click(screen.getByLabelText("Delete modeling"));

expect(onChange).toHaveBeenCalledWith(modeledMethods.slice(1));
expect(onChange).toHaveBeenCalledWith(
method.signature,
modeledMethods.slice(1),
);
});

it("can delete second modeling", async () => {
Expand All @@ -519,7 +525,10 @@ describe(MultipleModeledMethodsPanel.name, () => {
await userEvent.click(screen.getByLabelText("Next modeling"));
await userEvent.click(screen.getByLabelText("Delete modeling"));

expect(onChange).toHaveBeenCalledWith(modeledMethods.slice(0, 1));
expect(onChange).toHaveBeenCalledWith(
method.signature,
modeledMethods.slice(0, 1),
);
});

it("can add modeling after deleting second modeling", async () => {
Expand All @@ -532,7 +541,10 @@ describe(MultipleModeledMethodsPanel.name, () => {
await userEvent.click(screen.getByLabelText("Next modeling"));
await userEvent.click(screen.getByLabelText("Delete modeling"));

expect(onChange).toHaveBeenCalledWith(modeledMethods.slice(0, 1));
expect(onChange).toHaveBeenCalledWith(
method.signature,
modeledMethods.slice(0, 1),
);

rerender(
<MultipleModeledMethodsPanel
Expand All @@ -545,7 +557,7 @@ describe(MultipleModeledMethodsPanel.name, () => {
onChange.mockReset();
await userEvent.click(screen.getByLabelText("Add modeling"));

expect(onChange).toHaveBeenCalledWith([
expect(onChange).toHaveBeenCalledWith(method.signature, [
...modeledMethods.slice(0, 1),
{
signature: method.signature,
Expand Down