Merge pull request #2924 from github/koesie10/modeling-panel-multiple-models-add-remove

Add ability to add/remove modelings to method modeling panel

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

@@ -0,0 +1,71 @@
+import * as React from "react";
+import { useCallback, useEffect, useState } from "react";
+
+import { Meta, StoryFn } from "@storybook/react";
+
+import { MultipleModeledMethodsPanel as MultipleModeledMethodsPanelComponent } from "../../view/method-modeling/MultipleModeledMethodsPanel";
+import { createMethod } from "../../../test/factories/model-editor/method-factories";
+import { createModeledMethod } from "../../../test/factories/model-editor/modeled-method-factories";
+import { ModeledMethod } from "../../model-editor/modeled-method";
+
+export default {
+  title: "Method Modeling/Multiple Modeled Methods Panel",
+  component: MultipleModeledMethodsPanelComponent,
+} as Meta<typeof MultipleModeledMethodsPanelComponent>;
+
+const Template: StoryFn<typeof MultipleModeledMethodsPanelComponent> = (
+  args,
+) => {
+  const [modeledMethods, setModeledMethods] = useState<ModeledMethod[]>(
+    args.modeledMethods,
+  );
+
+  useEffect(() => {
+    setModeledMethods(args.modeledMethods);
+  }, [args.modeledMethods]);
+
+  const handleChange = useCallback(
+    (modeledMethods: ModeledMethod[]) => {
+      args.onChange(modeledMethods);
+      setModeledMethods(modeledMethods);
+    },
+    [args],
+  );
+
+  return (
+    <MultipleModeledMethodsPanelComponent
+      {...args}
+      modeledMethods={modeledMethods}
+      onChange={handleChange}
+    />
+  );
+};
+
+const method = createMethod();
+
+export const Unmodeled = Template.bind({});
+Unmodeled.args = {
+  method,
+  modeledMethods: [],
+};
+
+export const Single = Template.bind({});
+Single.args = {
+  method,
+  modeledMethods: [createModeledMethod(method)],
+};
+
+export const Multiple = Template.bind({});
+Multiple.args = {
+  method,
+  modeledMethods: [
+    createModeledMethod(method),
+    createModeledMethod({
+      ...method,
+      type: "source",
+      input: "",
+      output: "ReturnValue",
+      kind: "remote",
+    }),
+  ],
+};

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

@@ -1,4 +1,5 @@
 import * as React from "react";
+import { useCallback } from "react";
 import { ModeledMethod } from "../../model-editor/modeled-method";
 import { MethodModelingInputs } from "./MethodModelingInputs";
 import { Method } from "../../model-editor/method";
@@ -23,6 +24,13 @@ export const ModeledMethodsPanel = ({
   showMultipleModels,
   onChange,
 }: ModeledMethodsPanelProps) => {
+  const handleMultipleChange = useCallback(
+    (modeledMethods: ModeledMethod[]) => {
+      onChange(modeledMethods[0]);
+    },
+    [onChange],
+  );
+
   if (!showMultipleModels) {
     return (
       <SingleMethodModelingInputs
@@ -37,7 +45,7 @@ export const ModeledMethodsPanel = ({
     <MultipleModeledMethodsPanel
       method={method}
       modeledMethods={modeledMethods}
-      onChange={onChange}
+      onChange={handleMultipleChange}
     />
   );
 };

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

@@ -1,5 +1,5 @@
 import * as React from "react";
-import { useCallback, useState } from "react";
+import { useCallback, useMemo, useState } from "react";
 import { Method } from "../../model-editor/method";
 import { ModeledMethod } from "../../model-editor/modeled-method";
 import { styled } from "styled-components";
@@ -10,7 +10,7 @@ import { Codicon } from "../common";
 export type MultipleModeledMethodsPanelProps = {
   method: Method;
   modeledMethods: ModeledMethod[];
-  onChange: (modeledMethod: ModeledMethod) => void;
+  onChange: (modeledMethods: ModeledMethod[]) => void;
 };
 
 const Container = styled.div`
@@ -25,6 +25,7 @@ const Container = styled.div`
 const Footer = styled.div`
   display: flex;
   flex-direction: row;
+  justify-content: space-between;
 `;
 
 const PaginationActions = styled.div`
@@ -33,6 +34,12 @@ const PaginationActions = styled.div`
   gap: 0.5rem;
 `;
 
+const ModificationActions = styled.div`
+  display: flex;
+  flex-direction: row;
+  gap: 0.5rem;
+`;
+
 export const MultipleModeledMethodsPanel = ({
   method,
   modeledMethods,
@@ -47,19 +54,73 @@ export const MultipleModeledMethodsPanel = ({
     setSelectedIndex((previousIndex) => previousIndex + 1);
   }, []);
 
+  const handleAddClick = useCallback(() => {
+    const newModeledMethod: ModeledMethod = {
+      type: "none",
+      input: "",
+      output: "",
+      kind: "",
+      provenance: "manual",
+      signature: method.signature,
+      packageName: method.packageName,
+      typeName: method.typeName,
+      methodName: method.methodName,
+      methodParameters: method.methodParameters,
+    };
+
+    const newModeledMethods = [...modeledMethods, newModeledMethod];
+
+    onChange(newModeledMethods);
+    setSelectedIndex(newModeledMethods.length - 1);
+  }, [onChange, modeledMethods, method]);
+
+  const handleRemoveClick = useCallback(() => {
+    const newModeledMethods = modeledMethods.filter(
+      (_, index) => index !== selectedIndex,
+    );
+
+    const newSelectedIndex =
+      selectedIndex === newModeledMethods.length
+        ? selectedIndex - 1
+        : selectedIndex;
+
+    onChange(newModeledMethods);
+    setSelectedIndex(newSelectedIndex);
+  }, [onChange, modeledMethods, selectedIndex]);
+
+  const anyUnmodeled = useMemo(
+    () =>
+      modeledMethods.length === 0 ||
+      modeledMethods.some((m) => m.type === "none"),
+    [modeledMethods],
+  );
+
+  const handleChange = useCallback(
+    (modeledMethod: ModeledMethod) => {
+      if (modeledMethods.length > 0) {
+        const newModeledMethods = [...modeledMethods];
+        newModeledMethods[selectedIndex] = modeledMethod;
+        onChange(newModeledMethods);
+      } else {
+        onChange([modeledMethod]);
+      }
+    },
+    [modeledMethods, selectedIndex, onChange],
+  );
+
   return (
     <Container>
       {modeledMethods.length > 0 ? (
         <MethodModelingInputs
           method={method}
           modeledMethod={modeledMethods[selectedIndex]}
-          onChange={onChange}
+          onChange={handleChange}
         />
       ) : (
         <MethodModelingInputs
           method={method}
           modeledMethod={undefined}
-          onChange={onChange}
+          onChange={handleChange}
         />
       )}
       <Footer>
@@ -89,6 +150,24 @@ export const MultipleModeledMethodsPanel = ({
             <Codicon name="chevron-right" />
           </VSCodeButton>
         </PaginationActions>
+        <ModificationActions>
+          <VSCodeButton
+            appearance="icon"
+            aria-label="Delete modeling"
+            onClick={handleRemoveClick}
+            disabled={modeledMethods.length < 2}
+          >
+            <Codicon name="trash" />
+          </VSCodeButton>
+          <VSCodeButton
+            appearance="icon"
+            aria-label="Add modeling"
+            onClick={handleAddClick}
+            disabled={anyUnmodeled}
+          >
+            <Codicon name="add" />
+          </VSCodeButton>
+        </ModificationActions>
       </Footer>
     </Container>
   );

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();
+  const onChange = jest.fn<void, [ModeledMethod[]]>();
 
   describe("with no modeled methods", () => {
     const modeledMethods: ModeledMethod[] = [];
@@ -52,6 +52,23 @@ describe(MultipleModeledMethodsPanel.name, () => {
       expect(screen.queryByText("0/0")).not.toBeInTheDocument();
       expect(screen.queryByText("1/0")).not.toBeInTheDocument();
     });
+
+    it("cannot add or delete modeling", () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      expect(
+        screen
+          .getByLabelText("Delete modeling")
+          .getElementsByTagName("input")[0],
+      ).toBeDisabled();
+      expect(
+        screen.getByLabelText("Add modeling").getElementsByTagName("input")[0],
+      ).toBeDisabled();
+    });
   });
 
   describe("with one modeled method", () => {
@@ -97,6 +114,46 @@ describe(MultipleModeledMethodsPanel.name, () => {
       ).toBeDisabled();
       expect(screen.queryByText("1/1")).not.toBeInTheDocument();
     });
+
+    it("cannot delete modeling", () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      expect(
+        screen
+          .getByLabelText("Delete modeling")
+          .getElementsByTagName("input")[0],
+      ).toBeDisabled();
+    });
+
+    it("can add modeling", async () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      await userEvent.click(screen.getByLabelText("Add modeling"));
+
+      expect(onChange).toHaveBeenCalledWith([
+        ...modeledMethods,
+        {
+          signature: method.signature,
+          packageName: method.packageName,
+          typeName: method.typeName,
+          methodName: method.methodName,
+          methodParameters: method.methodParameters,
+          type: "none",
+          input: "",
+          output: "",
+          kind: "",
+          provenance: "manual",
+        },
+      ]);
+    });
   });
 
   describe("with two modeled methods", () => {
@@ -194,6 +251,106 @@ describe(MultipleModeledMethodsPanel.name, () => {
         }),
       ).toHaveValue("source");
     });
+
+    it("can update the first modeling", async () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      const modelTypeDropdown = screen.getByRole("combobox", {
+        name: "Model type",
+      });
+
+      await userEvent.selectOptions(modelTypeDropdown, "source");
+
+      expect(onChange).toHaveBeenCalledWith([
+        {
+          signature: method.signature,
+          packageName: method.packageName,
+          typeName: method.typeName,
+          methodName: method.methodName,
+          methodParameters: method.methodParameters,
+          type: "source",
+          input: "Argument[this]",
+          output: "ReturnValue",
+          kind: "value",
+          provenance: "manual",
+        },
+        ...modeledMethods.slice(1),
+      ]);
+    });
+
+    it("can update the second modeling", async () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      await userEvent.click(screen.getByLabelText("Next modeling"));
+
+      const modelTypeDropdown = screen.getByRole("combobox", {
+        name: "Model type",
+      });
+
+      await userEvent.selectOptions(modelTypeDropdown, "sink");
+
+      expect(onChange).toHaveBeenCalledWith([
+        ...modeledMethods.slice(0, 1),
+        {
+          signature: method.signature,
+          packageName: method.packageName,
+          typeName: method.typeName,
+          methodName: method.methodName,
+          methodParameters: method.methodParameters,
+          type: "sink",
+          input: "Argument[this]",
+          output: "ReturnValue",
+          kind: "value",
+          provenance: "manual",
+        },
+      ]);
+    });
+
+    it("can delete modeling", async () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      await userEvent.click(screen.getByLabelText("Delete modeling"));
+
+      expect(onChange).toHaveBeenCalledWith(modeledMethods.slice(1));
+    });
+
+    it("can add modeling", async () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      await userEvent.click(screen.getByLabelText("Add modeling"));
+
+      expect(onChange).toHaveBeenCalledWith([
+        ...modeledMethods,
+        {
+          signature: method.signature,
+          packageName: method.packageName,
+          typeName: method.typeName,
+          methodName: method.methodName,
+          methodParameters: method.methodParameters,
+          type: "none",
+          input: "",
+          output: "",
+          kind: "",
+          provenance: "manual",
+        },
+      ]);
+    });
   });
 
   describe("with three modeled methods", () => {
@@ -309,4 +466,100 @@ describe(MultipleModeledMethodsPanel.name, () => {
       ).toHaveValue("remote");
     });
   });
+
+  describe("with 1 modeled and 1 unmodeled method", () => {
+    const modeledMethods = [
+      createModeledMethod({
+        ...method,
+        type: "sink",
+        input: "Argument[this]",
+        output: "",
+        kind: "path-injection",
+      }),
+      createModeledMethod({
+        ...method,
+        type: "none",
+        input: "",
+        output: "",
+        kind: "",
+      }),
+    ];
+
+    it("cannot add modeling", () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      expect(
+        screen.getByLabelText("Add modeling").getElementsByTagName("input")[0],
+      ).toBeDisabled();
+    });
+
+    it("can delete first modeling", async () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      await userEvent.click(screen.getByLabelText("Delete modeling"));
+
+      expect(onChange).toHaveBeenCalledWith(modeledMethods.slice(1));
+    });
+
+    it("can delete second modeling", async () => {
+      render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      await userEvent.click(screen.getByLabelText("Next modeling"));
+      await userEvent.click(screen.getByLabelText("Delete modeling"));
+
+      expect(onChange).toHaveBeenCalledWith(modeledMethods.slice(0, 1));
+    });
+
+    it("can add modeling after deleting second modeling", async () => {
+      const { rerender } = render({
+        method,
+        modeledMethods,
+        onChange,
+      });
+
+      await userEvent.click(screen.getByLabelText("Next modeling"));
+      await userEvent.click(screen.getByLabelText("Delete modeling"));
+
+      expect(onChange).toHaveBeenCalledWith(modeledMethods.slice(0, 1));
+
+      rerender(
+        <MultipleModeledMethodsPanel
+          method={method}
+          modeledMethods={modeledMethods.slice(0, 1)}
+          onChange={onChange}
+        />,
+      );
+
+      onChange.mockReset();
+      await userEvent.click(screen.getByLabelText("Add modeling"));
+
+      expect(onChange).toHaveBeenCalledWith([
+        ...modeledMethods.slice(0, 1),
+        {
+          signature: method.signature,
+          packageName: method.packageName,
+          typeName: method.typeName,
+          methodName: method.methodName,
+          methodParameters: method.methodParameters,
+          type: "none",
+          input: "",
+          output: "",
+          kind: "",
+          provenance: "manual",
+        },
+      ]);
+    });
+  });
 });