Merge pull request #2957 from github/koesie10/modeled-methods-panel-multiple-save

Use `SetMultipleModeledMethodsMessage` in modeled methods panel

extensions/ql-vscode/src/common/interface-types.ts

@@ -572,11 +572,6 @@ interface HideModeledMethodsMessage {
   hideModeledMethods: boolean;
 }
 
-interface SetModeledMethodMessage {
-  t: "setModeledMethod";
-  method: ModeledMethod;
-}
-
 interface SetMultipleModeledMethodsMessage {
   t: "setMultipleModeledMethods";
   methodSignature: string;
@@ -627,7 +622,7 @@ interface StartModelingMessage {
 
 export type FromMethodModelingMessage =
   | CommonFromViewMessages
-  | SetModeledMethodMessage
+  | SetMultipleModeledMethodsMessage
   | RevealInEditorMessage
   | StartModelingMessage;
 

extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts

@@ -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,
@@ -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;
       }

extensions/ql-vscode/src/model-editor/shared/modeled-methods-legacy.ts

@@ -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.

extensions/ql-vscode/src/stories/method-modeling/MultipleModeledMethodsPanel.stories.tsx

@@ -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],

extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx

@@ -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 = ({

extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx

@@ -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,
     });
   };
 

extensions/ql-vscode/src/view/method-modeling/ModeledMethodsPanel.tsx

@@ -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)`
@@ -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],
   );
@@ -36,7 +36,7 @@ export const ModeledMethodsPanel = ({
       <SingleMethodModelingInputs
         method={method}
         modeledMethod={convertToLegacyModeledMethod(modeledMethods)}
-        onChange={onChange}
+        onChange={handleSingleChange}
       />
     );
   }
@@ -45,7 +45,7 @@ export const ModeledMethodsPanel = ({
     <MultipleModeledMethodsPanel
       method={method}
       modeledMethods={modeledMethods}
-      onChange={handleMultipleChange}
+      onChange={onChange}
     />
   );
 };

extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx

@@ -12,7 +12,7 @@ import { ModeledMethodAlert } from "./ModeledMethodAlert";
 export type MultipleModeledMethodsPanelProps = {
   method: Method;
   modeledMethods: ModeledMethod[];
-  onChange: (modeledMethods: ModeledMethod[]) => void;
+  onChange: (methodSignature: string, modeledMethods: ModeledMethod[]) => void;
 };
 
 const Container = styled.div`
@@ -82,7 +82,7 @@ export const MultipleModeledMethodsPanel = ({
 
     const newModeledMethods = [...modeledMethods, newModeledMethod];
 
-    onChange(newModeledMethods);
+    onChange(method.signature, newModeledMethods);
     setSelectedIndex(newModeledMethods.length - 1);
   }, [onChange, modeledMethods, method]);
 
@@ -96,21 +96,21 @@ export const MultipleModeledMethodsPanel = ({
         ? selectedIndex - 1
         : selectedIndex;
 
-    onChange(newModeledMethods);
+    onChange(method.signature, newModeledMethods);
     setSelectedIndex(newSelectedIndex);
-  }, [onChange, modeledMethods, selectedIndex]);
+  }, [onChange, modeledMethods, selectedIndex, method]);
 
   const handleChange = useCallback(
     (modeledMethod: ModeledMethod) => {
       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 (

extensions/ql-vscode/src/view/method-modeling/__tests__/MultipleModeledMethodsPanel.spec.tsx

@@ -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[] = [];
@@ -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,
@@ -275,7 +275,7 @@ describe(MultipleModeledMethodsPanel.name, () => {
 
       await userEvent.selectOptions(modelTypeDropdown, "source");
 
-      expect(onChange).toHaveBeenCalledWith([
+      expect(onChange).toHaveBeenCalledWith(method.signature, [
         {
           signature: method.signature,
           packageName: method.packageName,
@@ -307,7 +307,7 @@ describe(MultipleModeledMethodsPanel.name, () => {
 
       await userEvent.selectOptions(modelTypeDropdown, "sink");
 
-      expect(onChange).toHaveBeenCalledWith([
+      expect(onChange).toHaveBeenCalledWith(method.signature, [
         ...modeledMethods.slice(0, 1),
         {
           signature: method.signature,
@@ -333,7 +333,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 () => {
@@ -345,7 +348,7 @@ describe(MultipleModeledMethodsPanel.name, () => {
 
       await userEvent.click(screen.getByLabelText("Add modeling"));
 
-      expect(onChange).toHaveBeenCalledWith([
+      expect(onChange).toHaveBeenCalledWith(method.signature, [
         ...modeledMethods,
         {
           signature: method.signature,
@@ -557,7 +560,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 () => {
@@ -570,7 +576,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 () => {
@@ -583,7 +592,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
@@ -596,7 +608,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,