Merge pull request janus-idp#70 from parodos-dev/pc/fix-recursion

fix recursive bug by refactoring transformWorkToStep to use javascript generators
mareklibra · Mar 29, 2023 · 41e3c32 · 41e3c32
2 parents d7ebd6f + 38a9ce0
commit 41e3c32
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 82 deletions.
diff --git a/plugins/parodos/src/components/Form/Form.tsx b/plugins/parodos/src/components/Form/Form.tsx
@@ -104,7 +104,7 @@ export function Form({
       {stepLess ? (
         children
       ) : (
-        <ButtonGroup className={styles.buttonContainer}>
+        <ButtonGroup className={styles.buttonContainer} variant="contained">
           <Button
             disabled={activeStep === 0}
             className={styles.previous}

diff --git a/plugins/parodos/src/components/Form/Templates/ArrayFieldTemplate.tsx b/plugins/parodos/src/components/Form/Templates/ArrayFieldTemplate.tsx
@@ -115,48 +115,50 @@ export default function ArrayFieldTemplate<
         className={styles.stepper}
       >
         {items &&
-          items.map(
-            (
-              { key, ...itemProps }: ArrayFieldTemplateItemType<T, S, F>,
-              index,
-            ) => {
-              return (
-                <Step key={key}>
-                  <StepLabel
-                    StepIconProps={{ icon: String.fromCharCode(65 + index) }}
-                    className={formStyles.stepLabel}
-                  >
-                    {uiOptions.title || itemProps.schema.title}
-                  </StepLabel>
-                  <StepContent key={key}>
-                    <>
-                      <ArrayFieldItemTemplate key={key} {...itemProps} />
-                      <ButtonGroup>
-                        <Button
-                          disabled={activeItem === 0}
-                          className={formStyles.previous}
-                          onClick={() =>
-                            setActiveItem(a => (activeItem === 0 ? 0 : a - 1))
-                          }
-                        >
-                          PREVIOUS
-                        </Button>
-                        <Button
-                          variant="contained"
-                          type="button"
-                          color="primary"
-                          onClick={handleNext}
-                          className={formStyles.next}
-                        >
-                          NEXT
-                        </Button>
-                      </ButtonGroup>
-                    </>
-                  </StepContent>
-                </Step>
-              );
-            },
-          )}
+          items
+            .filter(Boolean)
+            .map(
+              (
+                { key, ...itemProps }: ArrayFieldTemplateItemType<T, S, F>,
+                index,
+              ) => {
+                return (
+                  <Step key={key}>
+                    <StepLabel
+                      StepIconProps={{ icon: String.fromCharCode(65 + index) }}
+                      className={formStyles.stepLabel}
+                    >
+                      {uiOptions.title || itemProps.schema.title}
+                    </StepLabel>
+                    <StepContent key={key}>
+                      <>
+                        <ArrayFieldItemTemplate key={key} {...itemProps} />
+                        <ButtonGroup variant="contained">
+                          <Button
+                            disabled={activeItem === 0}
+                            className={formStyles.previous}
+                            onClick={() =>
+                              setActiveItem(a => (activeItem === 0 ? 0 : a - 1))
+                            }
+                          >
+                            PREVIOUS
+                          </Button>
+                          <Button
+                            variant="contained"
+                            type="button"
+                            color="primary"
+                            onClick={handleNext}
+                            className={formStyles.next}
+                          >
+                            NEXT
+                          </Button>
+                        </ButtonGroup>
+                      </>
+                    </StepContent>
+                  </Step>
+                );
+              },
+            )}
       </Stepper>
     </>
   );

diff --git a/plugins/parodos/src/components/Form/styles.ts b/plugins/parodos/src/components/Form/styles.ts
@@ -7,13 +7,7 @@ export const useStyles = makeStyles(theme => ({
     },
   },
   previous: {
-    border: `1px solid ${theme.palette.primary.main}`,
-    color: theme.palette.text.primary,
-    marginRight: theme.spacing(1),
     textTransform: 'uppercase',
-    '&:disabled': {
-      border: `1px solid ${theme.palette.text.disabled}`,
-    },
   },
   next: {
     paddingRight: theme.spacing(4),

diff --git a/...s/parodos/src/hooks/useWorkflowDefinitionToJsonSchema/jsonSchemaFromWorkflowDefinition.ts b/...s/parodos/src/hooks/useWorkflowDefinitionToJsonSchema/jsonSchemaFromWorkflowDefinition.ts
@@ -75,7 +75,7 @@ export function getUiSchema(type: ParameterFormat) {
   }
 }
 
-function transformWorkToStep(work: WorkType, nestedSteps: Step[] = []) {
+function* transformWorkToStep(work: WorkType) {
   const title = taskDisplayName(work.name); // TODO: task label would be good here
 
   const schema: Record<string, any> = {
@@ -153,28 +153,31 @@ function transformWorkToStep(work: WorkType, nestedSteps: Step[] = []) {
     const childWorks = work.works ?? [];
 
     for (const [index, childWork] of childWorks.entries()) {
-      const children: Step[] = [];
-
-      const childStep = transformWorkToStep(childWork, children);
-
-      // We don't want to nest the recusive structure many levels deep
-      // so instead we flatten the structure and only allow one level of nesting
-      if (childWork.workType === 'WORKFLOW') {
-        nestedSteps.push(childStep);
-        continue;
+      let childStep: Step;
+      for (childStep of transformWorkToStep(childWork)) {
+        // We don't want to nest the recusive structure many levels deep
+        // so instead we flatten the structure and only allow one level of nesting
+        if (childWork.workType === 'WORKFLOW') {
+          yield childStep;
+          continue;
+        }
+
+        const nextSchemaKey = `properties.${key}.properties.works.items[${index}]`;
+        const nextUiSchemaKey = `${key}.works.items[${index}]`;
+
+        set(schema, nextSchemaKey, childStep.schema);
+
+        set(uiSchema, `${key}.works.['ui:hidden']`, true);
+        set(uiSchema, nextUiSchemaKey, childStep.uiSchema);
       }
-
-      const nextSchemaKey = `properties.${key}.properties.works.items[${index}]`;
-      const nextUiSchemaKey = `${key}.works.items[${index}]`;
-
-      set(schema, nextSchemaKey, childStep.schema);
-
-      set(uiSchema, `${key}.works.['ui:hidden']`, true);
-      set(uiSchema, nextUiSchemaKey, childStep.uiSchema);
     }
   }
 
-  return { schema, uiSchema, title, mergedSchema: schema, parent: undefined };
+  yield { schema, uiSchema, title, mergedSchema: schema, parent: undefined };
+}
+
+export function* getAllSteps(work: WorkType) {
+  yield* transformWorkToStep(work);
 }
 
 export function jsonSchemaFromWorkflowDefinition(
@@ -187,26 +190,22 @@ export function jsonSchemaFromWorkflowDefinition(
   const parameters = workflowDefinition.parameters ?? {};
 
   if (Object.keys(parameters).length > 0) {
-    const step = transformWorkToStep({
-      name: workflowDefinition.name,
-      parameters,
-    } as WorkType);
-
-    result.steps.push(step);
+    const masterStep = [
+      ...transformWorkToStep({
+        name: workflowDefinition.name,
+        parameters,
+      } as WorkType),
+    ][0];
+
+    result.steps.push(masterStep);
   }
 
   for (const work of workflowDefinition.works.filter(
     w =>
       Object.keys(w.parameters ?? {}).length > 0 || (w?.works ?? []).length > 0,
   )) {
-    const nestedSteps: Step[] = [];
-    const step = transformWorkToStep(work, nestedSteps);
-
-    result.steps.push(step);
-
-    for (const nestedStep of nestedSteps) {
-      nestedStep.parent = step;
-      result.steps.push(nestedStep);
+    for (const step of [...getAllSteps(work)].reverse()) {
+      result.steps.push(step);
     }
   }