fix(platform): fix the exception that occurs in fdp-approval-flow for multiple blocks

[zhuwentao0724]

…ltiple parallel blocks.

Related Issue(s)

#14085

closes

Description

When there are two or more parallel paths within a serial flow, the workflow is not displaying.

Screenshots

[netlify]

✅ Deploy Preview for fundamental-ngx ready!

Name	Link
🔨 Latest commit	b2d3c35
🔍 Latest deploy log	https://app.netlify.com/projects/fundamental-ngx/deploys/6a1e234f8a61c900085ee822
😎 Deploy Preview	https://deploy-preview-14228--fundamental-ngx.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[github-actions]

♿ Accessibility Report

✅ 236/236 routes passed axe-core audit

[droshev]

Findings

• pipeline is failing

• File: libs/platform/approval-flow/approval-flow-graph.ts:268
  When a node appears at index 0 in a short path but at index ≥ 1 in a longer path (e.g. [[A, D], [B, C, A, D]]), the < mostFar branch reads path[nodeIndex - 1].status with nodeIndex === 0, so path[-1] is undefined → Cannot read properties of undefined (reading 'status').
  The fix lands the function in a slightly more rigorous state, which makes the next crash more visible. @yixuan-liu Do you think this should be fixed too?

• nodeIndex >= longestPathLength - 1 → continue is subtly off-by-one in the wrong direction
  File: libs/platform/approval-flow/approval-flow-graph.ts:251 (new code)
  The guard is meant to be a "we're already at target length, stop" brake. The condition nodeIndex >= longestPathLength - 1 fires one step too early — at nodeIndex == longestPathLength - 1 the path could still legitimately be shorter than longestPathLength if the iteration just visited a real node and the trailing-blanks branch (nodeIndex === path.length - 1) hasn't yet had a chance to fire.
  Concretely: imagine a single-node path [A] against longest=3. At nodeIndex=0: guard says 0 >= 2 → false, ok, body runs the trailing-blanks branch (because path.length===1 and mostFar(A)===0), splices in 2 blanks. After splice path.length===3. Loop continues to nodeIndex=1: guard 1 >= 2 → false, body runs against blank node — getBlankNodesAfterNode returns [], mostFar is -1 (since blanks aren't in any path), so nodeIndex < mostFar is 0 < -1 == false, and nodeIndex === mostFar && nodeIndex === path.length - 1 is 1 === -1 false. Falls through harmlessly. At nodeIndex=2: guard 2 >= 2 → true, continue. OK.
  I traced through and it works in the cases I tested (Edge 1, Edge 3, Edge 5 all pass), but the condition reads as "stop one before length is reached" rather than the more obvious "stop when length is reached." The author probably meant path.length >= longestPathLength as the early exit. Both happen to work because of the structure of the surrounding branches, but the current expression is harder to reason about than:
  if (path.length >= longestPathLength) break;
  at the top of the loop body. @yixuan-liu What do you think?

• Commit format violation. libs/platform/approval-flow/approval-flow-graph.ts:246-285

  • Both commits and the PR title violate .claude/rules/commit-format.md. Required form: fix(platform): resolve crash in fdp-approval-flow when serial flow contains parallel sub-paths. The follow-up commit (Removed unnecessary continue.) should be squashed or amended into a single conventional commit. Without fix(platform) there's no version bump trigger and the changelog will miss the entry.

• No regression test added. libs/platform/approval-flow/approval-flow.component.spec.ts

  • The bug is data-shape sensitive (parallel paths inside a serial flow with shared join/split nodes) and there is zero existing coverage for generateApprovalFlowGraph / makePathsSameLength (grep found no references). Without a test using the issue's 6-node graph, this exact regression can return at any future refactor of makePathsSameLength.

• Stylistic: removed blank lines reduce readability. approval-flow-graph.ts:246-284

  • The diff strips every blank line inside the function. Functional code stays the same, but the dense block makes the three control-flow branches harder to scan. Consider keeping the original blank-line spacing between the three if blocks for consistency with the rest of the file.
  • Inline comment opportunity. The new if (nodeIndex >= longestPathLength - 1) continue; is non-obvious — it's the safety brake that prevents over-padding once a path reaches longestPathLength.
  • path[nodeIndex - 1].status still implicitly assumes nodeIndex > 0 in the < mostFar branch. Pre-existing behavior, not introduced by this PR — but worth noting since nodeIndex === 0 && nodeIndex < mostFar would crash on path[-1].status. Probe didn't hit it on the issue's graph; could happen if a root node is shared between paths of different rank. Out of scope to fix here, but worth a follow-up issue.

[droshev]

see my comments

[zhuwentao0724]

Hi @droshev ,

1. Changed to use break. Please review again.
2. The root cause is forEach loops fail to iterate over newly added elements.

[droshev]

Hi @droshev ,

1. Changed to use break. Please review again.
2. The root cause is forEach loops fail to iterate over newly added elements.

@zhuwentao0724 that is just one change, what about the others?

[zhuwentao0724]

Hi @droshev ,
I don't think the rest points are valid. Let me know which AI comments need to incorporate.
The code also worked fine before the 'break' change.

[yixuan-liu]

No major concerns. Suggest keeping the blank lines for better readability.

[zhuwentao0724]

Added unit tests.

[mikerodonnell89]

@zhuwentao0724 are you able to give a list of steps to recreate the original problem, in the example documentation? i.e. when to add an approver, the type, etc

[zhuwentao0724]

Hi @mikerodonnell89 ,
I reproduced this issue in Sourcing system.

1. Create a task in a template with approval flow like in the screenshot.
2. Create a Guided Sourcing project with above template.
3. Check the created task, exception happens.

You can check the parameter paths value in unit tests, I captured that from browser dev tool.

[mikerodonnell89]

I reproduced this issue in Sourcing system.

The problem should be reproducible in the fundamental-ngx library itself, and you'd be able to demonstrate it with a video/stackblitz/etc

[zhuwentao0724]

Hi @mikerodonnell89.

1. I can't reproduce this issue in NGX library UI, because the process is different. NGX library UI add nodes one by one, and the system checks whether a blank trailing node is needed each time a new node is added. However, in our scenario, all flow nodes are provided at once, the program will check all nodes then determines where to add blank nodes.

2. I reproduced this issue in StackBltiz by replacing this complexGraph nodes data with 'buildPRNodes' in the file approval-flow.component.spec.ts, same exception occurs.

[mikerodonnell89]

Hi @mikerodonnell89.

2. I reproduced this issue in StackBltiz

Care to share a public link to this stackblitz? I tried typing the URL manually and getting access denied

[zhuwentao0724]

Hi @mikerodonnell89 ,

1. Here is the link: https://stackblitz.com/edit/yve7ceqr-2ngdweee?file=src%2Fapp%2Fplatform-approval-flow-example.component.ts
2. You also can replacing this complexGraph nodes data with 'buildPRNodes' in PR file approval-flow.component.spec.ts