MAT-7791: Address feedback, add code coverage

src/CqlBuilderPanel/functionsSection/argumentSection/ArguementsSection.scss

@@ -0,0 +1,5 @@
+#clear-function-argument-btn {
+  &:focus {
+    margin-right: 16px;
+  }
+}

src/CqlBuilderPanel/functionsSection/argumentSection/ArgumentSection.tsx

@@ -16,6 +16,7 @@ import Arguments from "./Arguments";
 import { FunctionArgument } from "../../../model/CqlBuilderLookup";
 import ConfirmationDialog from "../../common/ConfirmationDialog";
 import { FunctionArgumentSchemaValidator } from "../../../validations/FunctionArgumentSchemaValidator";
+import "./ArguementsSection.scss";
 
 interface ArgumentsProps {
   functionArguments?: FunctionArgument[];
@@ -119,6 +120,7 @@ export default function ArgumentSection(props: ArgumentsProps) {
             onChange={(evt) => {
               setFunctionDataType(evt.target.value);
               formik.setFieldValue("dataType", evt.target.value);
+              formik.setFieldValue("other", "");
             }}
           />
         </div>
@@ -137,8 +139,9 @@ export default function ArgumentSection(props: ArgumentsProps) {
               inputProps={{
                 "data-testid": "other-type-input",
               }}
+              required
               {...formik.getFieldProps("other")}
-              error={Boolean(formik.errors.other)}
+              error={formik.touched.other && Boolean(formik.errors.other)}
               helperText={formik.errors.other}
             />
           </div>
@@ -166,6 +169,7 @@ export default function ArgumentSection(props: ArgumentsProps) {
         </Button>
       </div>
       <div style={{ paddingTop: "24px" }}>
+        {/* tableData with pagination. */}
         <Arguments
           functionArguments={functionArguments}
           canEdit={canEdit}

src/CqlBuilderPanel/functionsSection/argumentSection/Arguments.scss

@@ -2,15 +2,22 @@
   .arrow-container {
     display: flex;
     flex-direction: column;
+
     > button {
       display: flex;
       align-self: center;
+
       &:hover > svg {
         color: #197b9e;
       }
+
+      // prevent hover from showing on disabled
+      &:disabled > svg {
+        color: #717171 !important;
+      }
     }
     > button > svg {
-      color: #000; // Default color
+      color: #000;
       display: flex;
     }
   }

src/CqlBuilderPanel/functionsSection/argumentSection/Arguments.tsx

@@ -74,6 +74,7 @@ const Arguments = ({
   const [currentPage, setCurrentPage] = useState<number>(1);
 
   const managePagination = useCallback(() => {
+    //  page zero for no pages available
     if (functionArguments.length < currentLimit) {
       setOffset(0);
       setVisibleArguments([...functionArguments]);
@@ -109,6 +110,15 @@ const Arguments = ({
   const handlePageChange = (e, v) => {
     setCurrentPage(v);
   };
+  // maybe an additional useEffect if functionArguments. letting this happen outside of useCallback to prevent us having to compare against old values using refs.
+  // slightly less performant, but much less complicated then a bunch of refs and checking prev state to delegate pagination correctly
+  useEffect(() => {
+    if (functionArguments?.length > currentLimit) {
+      const newTotalPages = Math.ceil(functionArguments.length / currentLimit);
+      handlePageChange(null, newTotalPages);
+    }
+  }, [functionArguments, currentLimit]);
+
   const handleLimitChange = (e) => {
     setCurrentLimit(e.target.value);
     setCurrentPage(1);
@@ -163,11 +173,15 @@ const Arguments = ({
               <div className="arrow-container">
                 <button
                   onClick={() => moveItem(row.row.index, row.row.index - 1)}
+                  disabled={row.row.index == 0}
+                  data-testId={`arg-order-up-index-${row.row.index}`}
                 >
                   <ArrowDropUpOutlinedIcon />
                 </button>
                 <button
                   onClick={() => moveItem(row.row.index, row.row.index + 1)}
+                  disabled={row.row.index === functionArguments.length - 1}
+                  data-testId={`arg-order-down-index-${row.row.index}`}
                 >
                   <ArrowDropDownOutlinedIcon />
                 </button>

src/CqlBuilderPanel/functionsSection/functionBuilder/FunctionBuilder.test.tsx

@@ -171,6 +171,13 @@ describe("CQL Function Builder Tests", () => {
       name: "Other",
     });
     expect(otherTextbox).toBeInTheDocument();
+    userEvent.click(otherTextbox);
+    userEvent.click(argumentDataTypeTextBox);
+    await waitFor(() => {
+      screen
+        .getByTestId("other-field-helper-text")
+        .classList.contains("Mui-error");
+    });
   });
 
   it("Should clear argument section", async () => {
@@ -504,10 +511,6 @@ describe("CQL Function Builder Tests", () => {
     )) as HTMLInputElement;
     expect(definitionName.value).toBe("IP");
     // args
-    const argumentSectionButton = await within(argumentsSection).findByRole(
-      "button"
-    );
-    fireEvent.click(argumentSectionButton);
 
     const argumentNameInput = (await screen.findByTestId(
       "argument-name-input"
@@ -644,10 +647,6 @@ describe("CQL Function Builder Tests", () => {
     )) as HTMLInputElement;
     expect(definitionName.value).toBe("IP");
     // args
-    const argumentSectionButton = await within(argumentsSection).findByRole(
-      "button"
-    );
-    fireEvent.click(argumentSectionButton);
 
     const argumentNameInput = (await screen.findByTestId(
       "argument-name-input"
@@ -699,6 +698,152 @@ describe("CQL Function Builder Tests", () => {
       );
     });
   });
+
+  it("Should handle many table changes", async () => {
+    const handleApplyFn = jest.fn();
+    render(
+      <FunctionBuilder
+        canEdit={true}
+        handleApplyFunction={handleApplyFn}
+        cqlBuilderLookupsTypes={cqlBuilderLookup}
+        funct={{
+          functionName: "",
+          comment: "",
+          functionsArguments: [
+            { dataType: "Integer", argumentName: "Test" },
+            { dataType: "Integer", argumentName: "Test" },
+            { dataType: "Integer", argumentName: "Test" },
+            { dataType: "Integer", argumentName: "Test" },
+            { dataType: "Integer", argumentName: "Test" },
+          ],
+        }}
+      />
+    );
+    // name, insert, except args
+    const functionNameInput = (await screen.findByTestId(
+      "function-name-text-input"
+    )) as HTMLInputElement;
+    const argumentsSection = screen.getByTestId(
+      "terminology-section-Arguments-sub-heading"
+    );
+    expect(functionNameInput).toBeInTheDocument();
+    expect(functionNameInput.value).toBe("");
+    fireEvent.change(functionNameInput, {
+      target: { value: "IP" },
+    });
+    expect(functionNameInput.value).toBe("IP");
+    expect(
+      screen.getByTestId("terminology-section-Expression Editor-sub-heading")
+    ).toBeInTheDocument();
+    const typeInput = screen.getByTestId(
+      "type-selector-input"
+    ) as HTMLInputElement;
+    expect(typeInput).toBeInTheDocument();
+    expect(typeInput.value).toBe("");
+
+    fireEvent.change(typeInput, {
+      target: { value: "Timing" },
+    });
+    expect(typeInput.value).toBe("Timing");
+
+    const nameAutoComplete = screen.getByTestId("name-selector");
+    expect(nameAutoComplete).toBeInTheDocument();
+    const nameComboBox = within(nameAutoComplete).getByRole("combobox");
+    //name dropdown is populated with values based on type
+    await waitFor(() => expect(nameComboBox).toBeEnabled());
+
+    const nameDropDown = await screen.findByTestId("name-selector");
+    fireEvent.keyDown(nameDropDown, { key: "ArrowDown" });
+
+    const nameOptions = await screen.findAllByRole("option");
+    expect(nameOptions).toHaveLength(70);
+    const insertBtn = screen.getByTestId("expression-insert-btn");
+
+    expect(insertBtn).toBeInTheDocument();
+    expect(insertBtn).toBeDisabled();
+
+    fireEvent.click(nameOptions[0]);
+    expect(insertBtn).toBeEnabled();
+
+    fireEvent.click(insertBtn);
+    const definitionName = (await screen.findByTestId(
+      "function-name-text-input"
+    )) as HTMLInputElement;
+    expect(definitionName.value).toBe("IP");
+    // args
+
+    const argumentNameInput = (await screen.findByTestId(
+      "argument-name-input"
+    )) as HTMLInputElement;
+    expect(argumentNameInput).toBeInTheDocument();
+    expect(argumentNameInput.value).toBe("");
+    fireEvent.change(argumentNameInput, {
+      target: { value: "newName" },
+    });
+    expect(argumentNameInput.value).toBe("newName");
+
+    const dataTypeDropdown = await screen.findByTestId(
+      "arg-type-selector-input"
+    );
+    fireEvent.change(dataTypeDropdown, {
+      target: { value: "Boolean" },
+    });
+
+    const addButton = screen.getByTestId("function-argument-add-btn");
+    expect(addButton).toBeInTheDocument();
+    expect(addButton).toBeEnabled();
+
+    fireEvent.click(addButton);
+
+    const functionArgumentTable = screen.getByTestId("function-argument-tbl");
+    expect(functionArgumentTable).toBeInTheDocument();
+    const tableRow = functionArgumentTable.querySelector("tbody").children[0];
+    expect(tableRow.children[1].textContent).toEqual("newName");
+    // we got to the last page lets go back and trigger a change to page 0
+    const pageButton = await screen.findByRole("button", {
+      name: /Go to page 1/i,
+    });
+    expect(pageButton).toHaveTextContent("1");
+
+    userEvent.click(pageButton);
+    const page1Row0 = functionArgumentTable.querySelector("tbody").children[0];
+    expect(page1Row0.children[1].textContent).toEqual("Test");
+    // trigger the limit to prove we can see our new entry
+
+    const comboBoxes = await screen.findAllByRole("combobox");
+    const limitChoice = comboBoxes[1];
+
+    expect(limitChoice).toHaveTextContent("5");
+
+    userEvent.click(limitChoice);
+
+    const optionTen = await screen.findByRole("option", {
+      name: /10/i,
+    });
+    await waitFor(() => {
+      expect(optionTen).toBeDefined();
+    });
+
+    userEvent.click(optionTen);
+    await waitFor(() => {
+      expect(screen.findAllByText("newName")).toBeDefined();
+    });
+    // now we're back on first page with ten items. Lets move index 5 -> 4, 4 -> 5
+    const sixthUp = await screen.findByTestId("arg-order-up-index-5");
+    userEvent.click(sixthUp);
+    await waitFor(() => {
+      const page1Row = functionArgumentTable.querySelector("tbody").children[5];
+      expect(page1Row.children[1].textContent).toEqual("Test");
+    });
+    const fifthDown = await screen.findByTestId("arg-order-down-index-4");
+    userEvent.click(fifthDown);
+
+    await waitFor(() => {
+      const page1Row5 =
+        functionArgumentTable.querySelector("tbody").children[5];
+      expect(page1Row5.children[1].textContent).toEqual("newName");
+    });
+  });
 });
 
 describe("getNewExpressionsAndLines", () => {

src/CqlBuilderPanel/functionsSection/functionBuilder/FunctionBuilder.tsx

@@ -50,7 +50,6 @@ export default function FunctionBuilder({
     useState<boolean>(false);
   const textAreaRef = useRef(null);
   const [confirmationDialog, setConfirmationDialog] = useState<boolean>(false);
-
   const [expressionEditorValue, setExpressionEditorValue] = useState("");
   const [cursorPosition, setCursorPosition] = useState(null);
   const [autoInsert, setAutoInsert] = useState(false);
@@ -135,6 +134,9 @@ export default function FunctionBuilder({
                 if (e.target.value && !expressionEditorOpen) {
                   setExpressionEditorOpen(true);
                 }
+                if (e.target.value && !argumentsEditorOpen) {
+                  setArgumentsEditorOpen(true);
+                }
               }}
             />
           </div>
@@ -177,6 +179,7 @@ export default function FunctionBuilder({
           title="Arguments"
           showHeaderContent={argumentsEditorOpen}
         >
+          {/* functional input fields */}
           <ArgumentSection
             canEdit={canEdit}
             addArgumentToFunctionsArguments={addArgumentToFunctionsArguments}

src/validations/FunctionArgumentSchemaValidator.ts

@@ -6,5 +6,10 @@ export const FunctionArgumentSchemaValidator = Yup.object().shape({
     "No spaces or special characters besides underscore are allowed"
   ),
   dataType: Yup.string(),
-  other: Yup.string(),
+  other: Yup.string().when("dataType", {
+    is: (value: any) => value === "Other",
+    then: (schema) =>
+      schema.required("Other is required when dataType is Other."),
+    otherwise: (schema) => schema,
+  }),
 });