JSONSchema table - fixes (#5432)

* handle undefined currentValue in slider param Signed-off-by: Antonio Gamez Diaz <[email protected]> * move submitForm to a useEffect Signed-off-by: Antonio Gamez Diaz <[email protected]> * remove "from: true" Signed-off-by: Antonio Gamez Diaz <[email protected]> * fix test Signed-off-by: Antonio Gamez Diaz <[email protected]> Signed-off-by: Antonio Gamez Diaz <[email protected]>
vmware-tanzu · Oct 5, 2022 · c6280ae · c6280ae
1 parent 7260592
commit c6280ae
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 90 deletions.
diff --git a/dashboard/src/components/DeploymentForm/DeploymentForm.test.tsx b/dashboard/src/components/DeploymentForm/DeploymentForm.test.tsx
@@ -271,9 +271,8 @@ describe("renders an error", () => {
     spyOnUseHistory = jest.spyOn(ReactRouter, "useHistory").mockReturnValue({ push } as any);
 
     const appValues = "foo: bar";
-    const schema = {
-      properties: { foo: { type: "string", form: true } },
-    } as unknown as JSONSchemaType<any>;
+    const newAppValues = "foo: modified";
+    const schema = { properties: { foo: { type: "string" } } } as unknown as JSONSchemaType<any>;
     const selected = { ...defaultSelectedPkg, values: appValues, schema: schema };
 
     const wrapper = mountWrapper(
@@ -289,8 +288,9 @@ describe("renders an error", () => {
     const handleValuesChange: (v: string) => void = wrapper
       .find(DeploymentFormBody)
       .prop("setValues");
+
     act(() => {
-      handleValuesChange("foo: bar");
+      handleValuesChange(newAppValues);
     });
 
     wrapper
@@ -299,7 +299,7 @@ describe("renders an error", () => {
 
     wrapper.update();
 
-    expect(wrapper.find(DeploymentFormBody).prop("appValues")).toBe("foo: bar");
+    expect(wrapper.find(DeploymentFormBody).prop("appValues")).toBe(newAppValues);
     expect(wrapper.find(DeploymentForm).find("#releaseName").prop("value")).toBe(
       defaultProps.releaseName,
     );
@@ -316,7 +316,7 @@ describe("renders an error", () => {
       defaultProps.namespace,
       defaultSelectedPkg.availablePackageDetail,
       defaultProps.releaseName,
-      appValues,
+      newAppValues,
       schema,
       {} as ReconciliationOptions,
     );

diff --git a/...rm/DeploymentFormBody/BasicDeploymentForm/TabularSchemaEditorTable/Params/SliderParam.tsx b/...rm/DeploymentFormBody/BasicDeploymentForm/TabularSchemaEditorTable/Params/SliderParam.tsx
@@ -25,7 +25,9 @@ export interface ISliderParamProps {
 export default function SliderParam(props: ISliderParamProps) {
   const { handleBasicFormParamChange, id, label, param, step } = props;
 
-  const [currentValue, setCurrentValue] = useState(toNumber(param.currentValue) || param.minimum);
+  const [currentValue, setCurrentValue] = useState(
+    toNumber(param.currentValue) || toNumber(param.minimum) || 0,
+  );
   const [isValueModified, setIsValueModified] = useState(false);
   const [timeout, setThisTimeout] = useState({} as NodeJS.Timeout);
 
@@ -45,6 +47,9 @@ export default function SliderParam(props: ISliderParamProps) {
     setThisTimeout(setTimeout(() => func(targetCopy), basicFormsDebounceTime));
   };
 
+  const defaultMin = 0;
+  const defaultMax = 1000;
+
   const unsavedMessage = isValueModified ? "Unsaved" : "";
   const isModified =
     isValueModified ||
@@ -57,8 +62,8 @@ export default function SliderParam(props: ISliderParamProps) {
         aria-label={label}
         id={id + "_range"}
         type="range"
-        min={Math.min(param.minimum || 0, currentValue)}
-        max={Math.max(param.maximum || 1000, currentValue)}
+        min={Math.min(param.minimum || defaultMin, currentValue)}
+        max={Math.max(param.maximum || defaultMax, currentValue)}
         step={step}
         onChange={onChange}
         value={currentValue}

diff --git a/dashboard/src/components/DeploymentForm/DeploymentFormBody/DeploymentFormBody.test.tsx b/dashboard/src/components/DeploymentForm/DeploymentFormBody/DeploymentFormBody.test.tsx
@@ -124,7 +124,7 @@ it("should modify the original values of the differential component if parsed as
 c: d
 `;
   const schema = {
-    properties: { a: { type: "string", form: true } },
+    properties: { a: { type: "string" } },
   } as unknown as JSONSchemaType<any>;
   const selected = {
     values: oldValues,

diff --git a/dashboard/src/components/DeploymentForm/DeploymentFormBody/DeploymentFormBody.tsx b/dashboard/src/components/DeploymentForm/DeploymentFormBody/DeploymentFormBody.tsx
@@ -64,11 +64,13 @@ function DeploymentFormBody({
     {} as YAML.Document.Parsed<YAML.ParsedNode>,
   );
   const [restoreModalIsOpen, setRestoreModalOpen] = useState(false);
-  const [isLoaded, setIsloaded] = useState(false);
-  const [isLoading, setIsloading] = useState(true);
+  const [isLoaded, setIsLoaded] = useState(false);
+  const [isLoading, setIsLoading] = useState(true);
   const [unsavedChangesMap] = useState(new Map<string, any>());
+  const [shouldSubmitForm, setShouldSubmitForm] = useState(false);
 
-  // setBasicFormParameters when basicFormParameters changes
+  // whenever the parsed values change (for instance, when a new pkg version is selected),
+  // we need to force a new extraction of the params from the schema
   useEffect(() => {
     if (!isLoaded && schemaFromTheAvailablePackage && !isEmpty(valuesFromTheParentContainerNodes)) {
       const initialParamsFromContainer = retrieveBasicFormParams(
@@ -79,8 +81,8 @@ function DeploymentFormBody({
         valuesFromTheDeployedPackageNodes,
       );
       setParamsFromComponentState(initialParamsFromContainer);
-      setIsloaded(true);
-      setIsloading(false);
+      setIsLoaded(true);
+      setIsLoading(false);
     }
   }, [
     deploymentEvent,
@@ -92,42 +94,48 @@ function DeploymentFormBody({
     valuesFromTheParentContainerNodes,
   ]);
 
-  // setDefaultValues when defaultValues changes
+  // parse and store in the component state the values from the available package
   useEffect(() => {
     if (valuesFromTheAvailablePackage) {
       setValuesFromTheAvailablePackageNodes(parseToYamlNode(valuesFromTheAvailablePackage));
     }
   }, [isLoaded, valuesFromTheAvailablePackage]);
 
+  // parse and store in the component state the current values (which come from the parent container)
   useEffect(() => {
     if (valuesFromTheParentContainer) {
       setValuesFromTheParentContainerNodes(parseToYamlNode(valuesFromTheParentContainer));
     }
   }, [isLoaded, valuesFromTheParentContainer]);
 
+  // parse and store in the component state the values from the deployed package
   useEffect(() => {
     if (valuesFromTheDeployedPackage) {
       setValuesFromTheDeployedPackageNodes(parseToYamlNode(valuesFromTheDeployedPackage));
     }
   }, [isLoaded, valuesFromTheDeployedPackage, valuesFromTheParentContainer]);
 
-  const handleYAMLEditorChange = (value: string) => {
-    setValuesFromTheParentContainer(value);
-    setValuesModified();
-  };
-
-  const forceSubmit = useCallback(() => {
-    // the API was added recently, but should replace the manual dispatch of a submit event with bubbles:true (react>=17)
-    formRef?.current?.requestSubmit();
-  }, [formRef]);
+  // when the shouldSubmitForm flag is enabled, the form will be submitted, but using a native
+  // form submit event (to trigger the browser validations) instead of just calling its handler function
+  useEffect(() => {
+    if (shouldSubmitForm) {
+      // the requestSubmit API was added recently,
+      // but should replace the manual dispatch of a submit event with "bubbles:true" (react>=17)
+      formRef?.current?.requestSubmit();
+      setShouldSubmitForm(false);
+    }
+  }, [formRef, shouldSubmitForm]);
 
+  // for each unsaved change in the component state, we need to update the values,
+  // so that both the table and the yaml editor get the updated values
   const saveAllChanges = () => {
     let newValuesFromTheParentContainer, newParamsFromComponentState;
     unsavedChangesMap.forEach((value, key) => {
-      setIsloading(true);
+      setIsLoading(true);
       setValuesModified();
-      const aa = updateCurrentConfigByKey(paramsFromComponentState, key, value);
-      newParamsFromComponentState = [...aa];
+      newParamsFromComponentState = [
+        ...updateCurrentConfigByKey(paramsFromComponentState, key, value),
+      ];
       setParamsFromComponentState(newParamsFromComponentState);
 
       newValuesFromTheParentContainer = toStringYamlNode(
@@ -136,13 +144,19 @@ function DeploymentFormBody({
       setValuesFromTheParentContainer(newValuesFromTheParentContainer);
     });
     unsavedChangesMap.clear();
-    setIsloading(false);
+    setIsLoading(false);
   };
 
-  // save the pending changes and fire the submit event (via useEffect, to actually get the saved changes)
+  // save the pending changes and fire the submit event
+  // via an useEffect, to actually get the most recent saved changes
   const handleDeployClick = () => {
     saveAllChanges();
-    forceSubmit();
+    setShouldSubmitForm(true);
+  };
+
+  const handleYAMLEditorChange = (value: string) => {
+    setValuesFromTheParentContainer(value);
+    setValuesModified();
   };
 
   // re-build the table based on the new YAML