NIFI-14191: Add the Reference Process Group to the URL when navigatin…

[Freedom9339]

…g to the Parent

Summary

NIFI-14191 - Added the Reference Process Group to the URL when navigating to the Parent. I took into consideration the issues brought up with the previous closed PR for this issue and made sure these changes only affect the behavior when leaving the process-group.

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000
• Pull request contains commits signed with a registered key indicating Verified status

Pull Request Formatting

• Pull Request based on current revision of the main branch
• Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• Build completed using ./mvnw clean install -P contrib-check
  • JDK 21
  • JDK 25

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[Freedom9339]

Rebased. has anyone looked into this PR?

[mcgilman]

Rebased. has anyone looked into this PR?

I can give this a review today.

[mcgilman]

Thanks for the PR @Freedom9339 and for taking the feedback from the previous PR into consideration.

The approach of navigating to just /process-groups/{parentId} (without the component in the URL) and using a store flag to trigger selection afterward avoids the regression that affected the previous PR -- search-based centering is preserved. That's a good improvement.

One concern I have is the same issue I flagged in the previous PR: the subscription chains in canvas.component.ts fire twice during cross-PG navigation -- once when the route changes (against stale store data) and again when the store finishes loading. I've confirmed this with runtime instrumentation: during a leave-PG operation, the viewport-restore subscriber fires first while selectCurrentProcessGroupId still holds the child PG ID (stale), and then fires again after the parent PG loads with the correct ID. This isn't something introduced by this PR -- it's a latent issue in the existing code. But this PR's leavingProcessGroupId guard implicitly relies on that double-fire to work correctly: the first stale firing is filtered out because currentProcessGroupId still equals leavingProcessGroupId, and only the second firing passes the guard. This means restoreViewport() is also dispatched twice per leave -- once against stale state and once against the correct state. Building on top of that double-fire makes it harder to address later.

I think it would be worth addressing the double-fire directly as part of this change. All three subscription chains in canvas.component.ts (viewport restore, single-component selection, and bulk selection) share the same pattern:

switchMap(() => this.store.select(selectCurrentProcessGroupId)),
distinctUntilChanged(),
switchMap(() => this.store.select(/* chain-specific selector */)),

A sync guard can be inserted after distinctUntilChanged() to ensure the store and route agree before proceeding:

switchMap(() => this.store.select(selectCurrentProcessGroupId)),
distinctUntilChanged(),
concatLatestFrom(() => this.store.select(selectProcessGroupIdFromRoute)),
filter(([storeId, routeId]) => storeId === routeId),
switchMap(() => this.store.select(/* chain-specific selector */)),

Both selectors already exist. This prevents any of the three chains from acting on stale data during cross-PG navigation and eliminates the redundant restoreViewport() dispatch.

With that in place, the child PG selection could be handled entirely in the leaveProcessGroup$ effect as a two-step navigation: first navigate to the parent PG (letting the viewport restore as usual), then listen for loadProcessGroupComplete and dispatch navigateWithoutTransform to select the child PG. This would avoid needing the new store state (leavingProcessGroupId, the action, the reducer case, and the selector) since the effect can capture the current PG ID at the time leaveProcessGroup is dispatched and use it after the load completes.

Happy to discuss further or help work through the details.

[Freedom9339]

Thank you for reviewing @mcgilman. I did try the method you mentioned, but it didn't work, the subscribe was still being fired twice. However, I was able to prevent the stale firing by wrapping the route filter inside a switchmap. I removed leavingProcessGroupId from the state and instead did what you suggested and handled everything from the leaveProcessGroup$ effect.

I know you mentioned doing the same thing for single-component selection and bulk selection, but I wasn't able to recreate the double firing like how the viewport restore was. Is there a specific scenario where that is happening?