Improve alternative step display in FlowDiagram (#28699) (#28700)

* Refactor FlowDiagram to explicitly link concurrent groupings

Signed-off-by: Anil Dhurjaty <anil.dhurjaty@appfolio.com>

* Remove subflow nodes from FlowDiagram

Signed-off-by: Anil Dhurjaty <anil.dhurjaty@appfolio.com>

* Remove disabled steps and subflows from FlowDiagram

Signed-off-by: Anil Dhurjaty <anil.dhurjaty@appfolio.com>

* Change alternative steps to be serial w/ labels

Signed-off-by: Anil Dhurjaty <anil.dhurjaty@appfolio.com>

---------

Signed-off-by: Anil Dhurjaty <anil.dhurjaty@appfolio.com>

js/apps/admin-ui/cypress/e2e/authentication_test.spec.ts

@@ -220,16 +220,13 @@ describe("Authentication test", () => {
     diagramView.edgesExist([
       { from: "Start", to: "Cookie" },
       { from: "Cookie", to: "End" },
-      { from: "Start", to: "Kerberos" },
+      { from: "Cookie", to: "Identity Provider Redirector" },
-      { from: "Kerberos", to: "End" },
-      { from: "Start", to: "Identity Provider Redirector" },
       { from: "Identity Provider Redirector", to: "End" },
-      { from: "Start", to: "Start forms" },
+      { from: "Identity Provider Redirector", to: "Username Password Form" },
-      { from: "Start forms", to: "Username Password Form" },
       { from: "Username Password Form", to: "Condition - user configured" },
       { from: "Condition - user configured", to: "OTP Form" },
-      { from: "Condition - user configured", to: "End forms" },
+      { from: "Condition - user configured", to: "End" },
-      { from: "End forms", to: "End" },
+      { from: "OTP Form", to: "End" },
     ]);
   });
 });

js/apps/admin-ui/src/authentication/__tests__/FlowDiagram.test.tsx

@@ -144,7 +144,7 @@ describe("<FlowDiagram />", () => {
     const expectedEdges = [
       "Edge from start to alt1",
       "Edge from alt1 to end",
-      "Edge from start to alt2",
+      "Edge from alt1 to alt2",
       "Edge from alt2 to end",
     ];
     testHelper.expectEdgeLabels(expectedEdges);
@@ -178,24 +178,15 @@ describe("<FlowDiagram />", () => {
     const { container } = render(<FlowDiagram executionList={executionList} />);
 
     const testHelper = reactFlowTester(container);
+    const expectedNodes = ["start", "requiredElement", "subElement", "end"];
+    testHelper.expectNodeIds(expectedNodes);
+
     const expectedEdges = [
       "Edge from start to requiredElement",
-      "Edge from requiredElement to subflow",
+      "Edge from requiredElement to subElement",
-      "Edge from subflow to subElement",
+      "Edge from subElement to end",
-      "Edge from subElement to flow-end-subflow",
-      "Edge from flow-end-subflow to end",
     ];
     testHelper.expectEdgeLabels(expectedEdges);
-
-    const expectedNodes = [
-      "start",
-      "requiredElement",
-      "subflow",
-      "subElement",
-      "flow-end-subflow",
-      "end",
-    ];
-    testHelper.expectNodeIds(expectedNodes);
   });
 
   it("should render a flow with a subflow with alternative steps", () => {
@@ -231,22 +222,18 @@ describe("<FlowDiagram />", () => {
     const testHelper = reactFlowTester(container);
     const expectedEdges = [
       "Edge from start to requiredElement",
-      "Edge from requiredElement to subflow",
+      "Edge from requiredElement to subElement1",
-      "Edge from subflow to subElement1",
+      "Edge from subElement1 to end",
-      "Edge from subElement1 to flow-end-subflow",
+      "Edge from subElement1 to subElement2",
-      "Edge from subflow to subElement2",
+      "Edge from subElement2 to end",
-      "Edge from subElement2 to flow-end-subflow",
-      "Edge from flow-end-subflow to end",
     ];
     testHelper.expectEdgeLabels(expectedEdges);
 
     const expectedNodes = [
       "start",
       "requiredElement",
-      "subflow",
       "subElement1",
       "subElement2",
-      "flow-end-subflow",
       "end",
     ];
     testHelper.expectNodeIds(expectedNodes);
@@ -291,12 +278,10 @@ describe("<FlowDiagram />", () => {
     const testHelper = reactFlowTester(container);
     const expectedEdges = [
       "Edge from start to requiredElement",
-      "Edge from requiredElement to subflow",
+      "Edge from requiredElement to subElement1",
-      "Edge from subflow to subElement1",
+      "Edge from subElement1 to finalStep",
-      "Edge from subElement1 to flow-end-subflow",
+      "Edge from subElement1 to subElement2",
-      "Edge from subflow to subElement2",
+      "Edge from subElement2 to finalStep",
-      "Edge from subElement2 to flow-end-subflow",
-      "Edge from flow-end-subflow to finalStep",
       "Edge from finalStep to end",
     ];
     testHelper.expectEdgeLabels(expectedEdges);
@@ -304,10 +289,8 @@ describe("<FlowDiagram />", () => {
     const expectedNodes = [
       "start",
       "requiredElement",
-      "subflow",
       "subElement1",
       "subElement2",
-      "flow-end-subflow",
       "finalStep",
       "end",
     ];
@@ -357,6 +340,17 @@ describe("<FlowDiagram />", () => {
     const { container } = render(<FlowDiagram executionList={executionList} />);
 
     const testHelper = reactFlowTester(container);
+    const expectedNodes = [
+      "start",
+      "chooseUser",
+      "sendReset",
+      "conditionOtpConfigured",
+      "otpForm",
+      "resetPassword",
+      "end",
+    ];
+    testHelper.expectNodeIds(expectedNodes);
+
     const expectedEdges = [
       "Edge from start to chooseUser",
       "Edge from chooseUser to sendReset",
@@ -437,37 +431,207 @@ describe("<FlowDiagram />", () => {
     const { container } = render(<FlowDiagram executionList={executionList} />);
 
     const testHelper = reactFlowTester(container);
-    const expectedEdges = [
-      "Edge from start to exampleForms",
-      "Edge from exampleForms to usernamePasswordForm",
-      "Edge from usernamePasswordForm to conditionUserConfigured",
-      "Edge from conditionUserConfigured to conditionUserAttribute",
-      "Edge from conditionUserConfigured to flow-end-exampleForms",
-      "Edge from conditionUserAttribute to otpForm",
-      "Edge from conditionUserAttribute to flow-end-exampleForms",
-      "Edge from otpForm to confirmLink",
-      "Edge from confirmLink to flow-end-exampleForms",
-      "Edge from flow-end-exampleForms to end",
-      "Edge from start to conditionLoa",
-      "Edge from conditionLoa to reviewProfile",
-      "Edge from conditionLoa to end",
-      "Edge from reviewProfile to end",
-    ];
-    testHelper.expectEdgeLabels(expectedEdges);
 
     const expectedNodes = [
       "start",
-      "exampleForms",
       "usernamePasswordForm",
       "conditionUserConfigured",
       "conditionUserAttribute",
       "otpForm",
       "confirmLink",
-      "flow-end-exampleForms",
       "conditionLoa",
       "reviewProfile",
       "end",
     ];
     testHelper.expectNodeIds(expectedNodes);
+
+    const expectedEdges = [
+      "Edge from start to usernamePasswordForm",
+      "Edge from usernamePasswordForm to conditionUserConfigured",
+      "Edge from conditionUserConfigured to conditionUserAttribute",
+      "Edge from conditionUserConfigured to end",
+      "Edge from conditionUserAttribute to otpForm",
+      "Edge from conditionUserAttribute to end",
+      "Edge from otpForm to confirmLink",
+      "Edge from confirmLink to end",
+      "Edge from usernamePasswordForm to conditionLoa",
+      "Edge from conditionLoa to reviewProfile",
+      "Edge from conditionLoa to end",
+      "Edge from reviewProfile to end",
+    ];
+    testHelper.expectEdgeLabels(expectedEdges);
+  });
+
+  it("should render the default first broker login flow", () => {
+    const executionList = new ExecutionList([
+      {
+        id: "reviewProfile",
+        displayName: "Review Profile",
+        requirement: "REQUIRED",
+        level: 0,
+      },
+      {
+        id: "createOrLink",
+        displayName: "User creation or linking",
+        requirement: "REQUIRED",
+        level: 0,
+      },
+      {
+        id: "createUnique",
+        displayName: "Create User If Unique",
+        requirement: "ALTERNATIVE",
+        level: 1,
+      },
+      {
+        id: "existingAccount",
+        displayName: "Handle Existing Account",
+        requirement: "ALTERNATIVE",
+        level: 1,
+      },
+      {
+        id: "confirmLink",
+        displayName: "Confirm link existing account",
+        requirement: "REQUIRED",
+        level: 2,
+      },
+      {
+        id: "accountVerification",
+        displayName: "Account verification options",
+        requirement: "REQUIRED",
+        level: 2,
+      },
+      {
+        id: "emailVerify",
+        displayName: "Verify existing account by Email",
+        requirement: "ALTERNATIVE",
+        level: 3,
+      },
+      {
+        id: "reauthVerify",
+        displayName: "Verify Existing Account by Re-authentication",
+        requirement: "ALTERNATIVE",
+        level: 3,
+      },
+      {
+        id: "usernamePassword",
+        displayName:
+          "Username Password Form for identity provider reauthentication",
+        requirement: "REQUIRED",
+        level: 4,
+      },
+      {
+        id: "conditionalOtp",
+        displayName: "First broker login - Conditional OTP",
+        requirement: "CONDITIONAL",
+        level: 4,
+      },
+      {
+        id: "conditionUserConfigured",
+        displayName: "Condition - user configured",
+        requirement: "REQUIRED",
+        level: 5,
+      },
+      {
+        id: "otpForm",
+        displayName: "OTP Form",
+        requirement: "REQUIRED",
+        level: 5,
+      },
+    ]);
+
+    const { container } = render(<FlowDiagram executionList={executionList} />);
+
+    const testHelper = reactFlowTester(container);
+
+    const expectedNodes = [
+      "start",
+      "reviewProfile",
+      "createUnique",
+      "confirmLink",
+      "usernamePassword",
+      "conditionUserConfigured",
+      "otpForm",
+      "emailVerify",
+      "end",
+    ];
+    testHelper.expectNodeIds(expectedNodes);
+
+    const expectedEdges = [
+      "Edge from start to reviewProfile",
+      "Edge from reviewProfile to createUnique",
+      "Edge from createUnique to confirmLink",
+      "Edge from createUnique to end",
+      "Edge from confirmLink to emailVerify",
+      "Edge from emailVerify to usernamePassword",
+      "Edge from usernamePassword to conditionUserConfigured",
+      "Edge from conditionUserConfigured to otpForm",
+      "Edge from conditionUserConfigured to end",
+      "Edge from otpForm to end",
+      "Edge from emailVerify to end",
+    ];
+    testHelper.expectEdgeLabels(expectedEdges);
+  });
+
+  it("should hide disabled steps", () => {
+    const executionList = new ExecutionList([
+      {
+        id: "disabled",
+        displayName: "Disabled",
+        requirement: "DISABLED",
+      },
+      {
+        id: "required",
+        displayName: "Required",
+        requirement: "REQUIRED",
+      },
+    ]);
+
+    const { container } = render(<FlowDiagram executionList={executionList} />);
+
+    const testHelper = reactFlowTester(container);
+
+    const expectedNodes = ["start", "required", "end"];
+    testHelper.expectNodeIds(expectedNodes);
+
+    const expectedEdges = [
+      "Edge from start to required",
+      "Edge from required to end",
+    ];
+    testHelper.expectEdgeLabels(expectedEdges);
+  });
+
+  it("should hide disabled subflow", () => {
+    const executionList = new ExecutionList([
+      {
+        id: "required",
+        displayName: "Required",
+        requirement: "REQUIRED",
+        level: 0,
+      },
+      {
+        id: "subflow",
+        displayName: "Subflow",
+        requirement: "DISABLED",
+        level: 0,
+      },
+      {
+        id: "subElement",
+        displayName: "Sub Element",
+        requirement: "REQUIRED",
+        level: 1,
+      },
+    ]);
+
+    const { container } = render(<FlowDiagram executionList={executionList} />);
+
+    const testHelper = reactFlowTester(container);
+    const expectedNodes = ["start", "required", "end"];
+    testHelper.expectNodeIds(expectedNodes);
+
+    const expectedEdges = [
+      "Edge from start to required",
+      "Edge from required to end",
+    ];
+    testHelper.expectEdgeLabels(expectedEdges);
   });
 });

js/apps/admin-ui/src/authentication/components/FlowDiagram.tsx

@@ -29,19 +29,44 @@ type FlowDiagramProps = {
   executionList: ExecutionList;
 };
 
-type ConditionLabel = "true" | "false";
+type ConditionLabel = "true" | "false" | "success" | "attempted";
 
 const nodeTypes = {
   conditional: ConditionalNode,
   startSubFlow: StartSubFlowNode,
   endSubFlow: EndSubFlowNode,
-} as const;
+};
 
-type NodeType = keyof typeof nodeTypes;
+const inOutClasses = new Map<string, string>([
+  ["input", "keycloak__authentication__input_node"],
+  ["output", "keycloak__authentication__output_node"],
+]);
+
+type NodeType =
+  | "conditional"
+  | "startSubFlow"
+  | "endSubFlow"
+  | "input"
+  | "output";
+
+type IntermediateFlowResult = {
+  startId: string;
+  nodes: Node[];
+  edges: Edge[];
+  nextLinkFns: ((id: string) => Edge)[];
+};
+
+function pairwise<T, U>(fn: (x: T, y: T) => U, arr: T[]): U[] {
+  const result: U[] = [];
+  for (let index = 0; index < arr.length - 1; index++) {
+    result.push(fn(arr[index], arr[index + 1]));
+  }
+  return result;
+}
 
 const isBypassable = (execution: ExpandableExecution) =>
   execution.requirement === "ALTERNATIVE" ||
-  execution.requirement === "DISABLED";
+  execution.requirement === "CONDITIONAL";
 
 const createEdge = (
   fromNode: string,
@@ -71,190 +96,177 @@ const createNode = (
   return {
     id: ex.id!,
     type: nodeType,
-    sourcePosition: Position.Right,
+    sourcePosition: nodeType === "output" ? undefined : Position.Right,
-    targetPosition: Position.Left,
+    targetPosition: nodeType === "input" ? undefined : Position.Left,
     data: { label: ex.displayName! },
     position: { x: 0, y: 0 },
+    className: inOutClasses.get(nodeType || ""),
   };
 };
 
-const renderSubFlowNodes = (execution: ExpandableExecution): Node[] => {
+const consecutiveBypassableFlows = (
-  const nodes: Node[] = [];
+  executionList: ExpandableExecution[],
-
+): ExpandableExecution[] => {
-  if (execution.requirement !== "CONDITIONAL") {
+  const result = [];
-    nodes.push(createNode(execution, "startSubFlow"));
-
-    const endSubFlowId = `flow-end-${execution.id}`;
-    nodes.push(
-      createNode(
-        {
-          id: endSubFlowId,
-          displayName: execution.displayName!,
-        },
-        "endSubFlow",
-      ),
-    );
-  }
-
-  return nodes.concat(renderFlowNodes(execution.executionList || []));
-};
-
-const renderFlowNodes = (executionList: ExpandableExecution[]): Node[] => {
-  let elements: Node[] = [];
-
   for (let index = 0; index < executionList.length; index++) {
     const execution = executionList[index];
-    if (execution.executionList) {
+    if (!isBypassable(execution)) {
-      elements = elements.concat(renderSubFlowNodes(execution));
+      break;
-    } else {
-      elements.push(
-        createNode(
-          execution,
-          providerConditionFilter(execution) ? "conditional" : undefined,
-        ),
-      );
     }
+    result.push(execution);
   }
-
+  return result;
-  return elements;
 };
 
-const renderSubFlowEdges = (
+const borderStep = (
+  node: Node,
+  continuing: boolean = true,
+): IntermediateFlowResult => ({
+  startId: node.id,
+  nodes: [node],
+  edges: [],
+  nextLinkFns: continuing ? [(id: string) => createEdge(node.id, id)] : [],
+});
+
+const renderSubFlow = (
   execution: ExpandableExecution,
-  flowEndId: string,
+): IntermediateFlowResult => {
-): { startId: string; edges: Edge[]; endId: string } => {
   if (!execution.executionList)
     throw new Error("Execution list is required for subflow");
 
-  if (execution.requirement === "CONDITIONAL") {
+  const graph = createGraph(createConcurrentGroupings(execution.executionList));
-    const startId = execution.executionList![0].id!;
 
-    return {
+  graph.nextLinkFns.push(
-      startId: startId,
+    ...execution.executionList
-      edges: renderFlowEdges(startId, execution.executionList!, flowEndId),
+      .filter((e) => providerConditionFilter(e))
-      endId: execution.executionList![execution.executionList!.length - 1].id!,
+      .map((e) => (id: string) => createEdge(e.id!, id, "false")),
-    };
+  );
-  }
+  return graph;
-  const elements: Edge[] = [];
-  const subFlowEndId = `flow-end-${execution.id}`;
-
-  return {
-    startId: execution.id!,
-    edges: elements.concat(
-      renderFlowEdges(execution.id!, execution.executionList!, subFlowEndId),
-    ),
-    endId: subFlowEndId,
-  };
 };
 
-const renderFlowEdges = (
+const groupConcurrentSteps = (
-  startId: string,
   executionList: ExpandableExecution[],
-  endId: string,
+): ExpandableExecution[] => {
-): Edge[] => {
+  const executions = consecutiveBypassableFlows(executionList);
-  let elements: Edge[] = [];
+  if (executions.length > 0) {
-  let prevExecutionId = startId;
+    return executions;
-  let isLastExecutionBypassable = false;
+  }
-  const conditionals = [];
+  return [executionList[0]];
+};
 
-  for (let index = 0; index < executionList.length; index++) {
+const createConcurrentSteps = (
-    const execution = executionList[index];
+  executionList: ExpandableExecution[],
-    let executionId = execution.id!;
+): IntermediateFlowResult[] => {
-    const isPrevConditional =
+  if (executionList.length === 0) {
-      conditionals[conditionals.length - 1] === prevExecutionId;
+    return [];
-    const connectToPrevious = (id: string) =>
+  }
-      elements.push(
-        createEdge(prevExecutionId, id, isPrevConditional ? "true" : undefined),
-      );
-
-    if (providerConditionFilter(execution)) {
-      conditionals.push(executionId);
-    }
-    if (startId === executionId) {
-      continue;
-    }
 
+  const executions = groupConcurrentSteps(executionList);
+  return executions.map((execution) => {
     if (execution.executionList) {
-      const nextRequired =
+      return renderSubFlow(execution);
-        executionList.slice(index + 1).find((e) => !isBypassable(e))?.id ??
-        endId;
-      const {
-        startId: subFlowStartId,
-        edges,
-        endId: subflowEndId,
-      } = renderSubFlowEdges(execution, nextRequired);
-
-      connectToPrevious(subFlowStartId);
-      elements = elements.concat(edges);
-      executionId = subflowEndId;
-    } else {
-      connectToPrevious(executionId);
     }
 
-    const isExecutionBypassable = isBypassable(execution);
+    const isConditional = providerConditionFilter(execution);
+    const edgeLabel = (() => {
+      if (isConditional) {
+        return "true";
+      }
+      if (execution.requirement === "ALTERNATIVE") {
+        return "success";
+      }
+    })();
 
-    if (isExecutionBypassable) {
+    return {
-      elements.push(createEdge(executionId, endId));
+      startId: execution.id!,
-    } else {
+      nodes: [createNode(execution, isConditional ? "conditional" : undefined)],
-      prevExecutionId = executionId;
+      edges: [],
-    }
+      nextLinkFns: [(id: string) => createEdge(execution.id!, id, edgeLabel)],
+    };
+  });
+};
 
-    isLastExecutionBypassable = isExecutionBypassable;
+const createConcurrentGroupings = (
+  executionList: ExpandableExecution[],
+): IntermediateFlowResult[][] => {
+  if (executionList.length === 0) {
+    return [];
+  }
+  const steps = createConcurrentSteps(executionList);
+  return [
+    steps,
+    ...createConcurrentGroupings(executionList.slice(steps.length)),
+  ];
+};
+
+const createGraph = (
+  groupings: IntermediateFlowResult[][],
+): IntermediateFlowResult => {
+  const nodes: Node[] = [];
+  const edges: Edge[] = [];
+  let nextLinkFns: ((id: string) => Edge)[] = [];
+
+  for (const group of groupings) {
+    nodes.push(...group.flatMap((g) => g.nodes));
+    edges.push(
+      ...group.flatMap((g) => g.edges),
+      ...nextLinkFns.map((fn) => fn(group[0].startId)),
+      ...pairwise(
+        (prev, current) =>
+          createEdge(prev.startId, current.startId, "attempted"),
+        group,
+      ),
+    );
+    nextLinkFns = group.flatMap((g) => g.nextLinkFns);
   }
 
-  // subflows with conditionals automatically connect to the end, so don't do it twice
+  return {
-  if (!isLastExecutionBypassable && conditionals.length === 0) {
+    startId: groupings[0][0].startId,
-    elements.push(createEdge(prevExecutionId, endId));
+    nodes,
-  }
+    edges,
-  elements = elements.concat(
+    nextLinkFns,
-    conditionals.map((id) => createEdge(id, endId, "false")),
+  };
-  );
-
-  return elements;
 };
 
 const edgeTypes: ButtonEdges = {
   buttonEdge: ButtonEdge,
 };
 
-function renderNodes(expandableList: ExpandableExecution[]) {
+function renderGraph(executionList: ExpandableExecution[]): [Node[], Edge[]] {
-  return getLayoutedNodes([
+  const executionListNoDisabled = executionList.filter(
-    {
+    (e) => e.requirement !== "DISABLED",
-      id: "start",
+  );
-      sourcePosition: Position.Right,
+  const groupings = [
-      type: "input",
+    [borderStep(createNode({ id: "start", displayName: "Start" }, "input"))],
-      data: { label: "Start" },
+    ...createConcurrentGroupings(executionListNoDisabled),
-      position: { x: 0, y: 0 },
+    [
-      className: "keycloak__authentication__input_node",
+      borderStep(
-    },
+        createNode({ id: "end", displayName: "End" }, "output"),
-    {
+        false,
-      id: "end",
+      ),
-      targetPosition: Position.Left,
+    ],
-      type: "output",
+  ];
-      data: { label: "End" },
-      position: { x: 0, y: 0 },
-      className: "keycloak__authentication__output_node",
-    },
-    ...renderFlowNodes(expandableList),
-  ]);
-}
 
-function renderEdges(expandableList: ExpandableExecution[]): Edge[] {
+  const { nodes, edges } = createGraph(groupings);
-  return getLayoutedEdges(renderFlowEdges("start", expandableList, "end"));
+
+  return [getLayoutedNodes(nodes), getLayoutedEdges(edges)];
 }
 
 export const FlowDiagram = ({
   executionList: { expandableList },
 }: FlowDiagramProps) => {
   const [expandDrawer, setExpandDrawer] = useState(false);
-  const initialNodes = useMemo(() => renderNodes(expandableList), []);
+  const [initialNodes, initialEdges] = useMemo(
-  const initialEdges = useMemo(() => renderEdges(expandableList), []);
+    () => renderGraph(expandableList),
+    [],
+  );
   const [nodes, setNodes, onNodesChange] = useNodesState(initialNodes);
   const [edges, setEdges, onEdgesChange] = useEdgesState(initialEdges);
 
   useUpdateEffect(() => {
-    setNodes(renderNodes(expandableList));
+    const [nodes, edges] = renderGraph(expandableList);
-    setEdges(renderEdges(expandableList));
+    setNodes(nodes);
+    setEdges(edges);
   }, [expandableList]);
 
   const onInit = (reactFlowInstance: ReactFlowInstance) =>