NIFI-14777 Restore capability from NiFi 1.x to view upstream/downstre… - #11582
NIFI-14777 Restore capability from NiFi 1.x to view upstream/downstre…#11582markobean wants to merge 2 commits into
Conversation
|
I tested the functionality of this and it looks good. The pop up window shows accurate connections and the buttons redirect to the appropriate component. I would suggest using different Icons for the context menu options so that the arrows are pointing up and down, to correspond with "Upstream" and "Downstream". Maybe slanted up/down arrows? Just a thought. |
Thanks @Freedom9339. I updated the arrows, as suggested. The upstream context menu item shows an arrow pointing up and to the left, and downstream shows down and to the right. It's slightly more intuitive and looks good. |
…am connections. View connections > upstream/downstream is available in the context menu from a variety of components: input/output ports, processors, process groups, remote process groups and funnels.
0d9d316 to
c56d29a
Compare
|
Will review... |
rfellows
left a comment
There was a problem hiding this comment.
Thanks for restoring View Connections. The effect/filter path and unauthorized/empty handling look directionally right, but this isn’t merge-ready yet.
Verified locally on this branch (npx nx test nifi --runInBand): 6 failed / 2,825 passed, all in component-connections-dialog.component.spec.ts. Lint and a development build passed.
Must-fix before merge:
- The new dialog tests fail (empty mock store + assertions still targeting
goTo()/ a partial row shape). - Remote-port cells navigate with the port id and
ComponentType.RemoteProcessGroup. - Source/Destination Process Group cells navigate to
/process-groups/{groupId}/ProcessGroup/{groupId}when the endpoint is the group currently on the canvas.
The five-column clickable table is a reasonable UX, but it should follow the existing dialog listing-table pattern (bounded scroll, sticky header, striped rows) used by Local Changes and Change Version, and use <a> for in-cell navigation rather than mat-button.
| { provide: MAT_DIALOG_DATA, useValue: dialogRequest }, | ||
| { provide: MatDialogRef, useValue: dialogRef }, | ||
| { provide: CanvasUtils, useValue: canvasUtils }, | ||
| provideMockStore({}) |
There was a problem hiding this comment.
These tests currently fail (npx nx test nifi: 6 failed in this file).
provideMockStore({}) does not satisfy selectProcessGroupIdToNameMap, so detectChanges() throws Cannot read properties of undefined (reading 'flowState') from flow.selectors.ts. The row assertions also omit groupId/type, and the navigation test still calls component.goTo(...), which this component no longer has (navigateTo is the method).
Please override selectProcessGroupIdToNameMap before detectChanges(), update the expected row shape, and call navigateTo(...) for the connection (and ideally the other clickable cells).
| private static readonly TYPE_MAP: Record<string, ComponentType> = { | ||
| PROCESSOR: ComponentType.Processor, | ||
| INPUT_PORT: ComponentType.InputPort, | ||
| OUTPUT_PORT: ComponentType.OutputPort, | ||
| REMOTE_INPUT_PORT: ComponentType.RemoteProcessGroup, | ||
| REMOTE_OUTPUT_PORT: ComponentType.RemoteProcessGroup, | ||
| FUNNEL: ComponentType.Funnel | ||
| }; |
There was a problem hiding this comment.
REMOTE_INPUT_PORT / REMOTE_OUTPUT_PORT are mapped to ComponentType.RemoteProcessGroup, but buildRow still stores connection.sourceId / destinationId (the remote port id). The source/destination cells then call navigateTo(row.*.id, row.*.groupId, row.*.type), which becomes /process-groups/{rpgId}/RemoteProcessGroup/{remotePortId}.
Existing connectable mapping in apps/nifi/src/app/ui/common/utils/component-state.utils.ts (getComponentTypeForSource / getComponentTypeForDestination) treats those types as the containing RPG. Canvas endpoint resolution (CanvasUtils.getConnectionSourceComponentId / getConnectionDestinationComponentId) likewise collapses to the group/RPG id.
For remote ports, navigate with the RPG id as id and the process group that contains that RPG as processGroupId (or open Manage Remote Ports). Please add a remote-port navigation test.
| <button | ||
| type="button" | ||
| mat-button | ||
| class="link-cell" | ||
| [matTooltip]="row.source.name" | ||
| (click)="navigateTo(row.source.id, row.source.groupId, row.source.type)"> | ||
| <i class="icon component-type-icon" [class]="componentIcon(row.source.type)"></i> | ||
| {{ row.source.name }} | ||
| </button> |
There was a problem hiding this comment.
Listing tables don’t put Material buttons in data cells. mat-button adds padded chrome and fights the ellipsis / table-layout: fixed treatment that .listing-table already applies to cell text.
When a name is itself the navigation target, existing UIs use an <a>:
apps/nifi/src/app/ui/common/controller-service/controller-service-references/controller-service-references.component.htmlapps/nifi/src/app/ui/common/parameter-references/parameter-references.component.htmlapps/nifi/src/app/ui/common/process-group-references/process-group-references.component.html
Row actions belong in an overflow menu (local-changes-table.html). Please replace all five mat-button cells with <a> (or routerLink) and drop class="link-cell".
| <button | |
| type="button" | |
| mat-button | |
| class="link-cell" | |
| [matTooltip]="row.source.name" | |
| (click)="navigateTo(row.source.id, row.source.groupId, row.source.type)"> | |
| <i class="icon component-type-icon" [class]="componentIcon(row.source.type)"></i> | |
| {{ row.source.name }} | |
| </button> | |
| <a | |
| [matTooltip]="row.source.name" | |
| (click)="navigateTo(row.source.id, row.source.groupId, row.source.type)"> | |
| <i class="icon component-type-icon" [class]="componentIcon(row.source.type)"></i> | |
| {{ row.source.name }} | |
| </a> |
| <button | ||
| type="button" | ||
| mat-button | ||
| class="link-cell" | ||
| [matTooltip]="resolveGroupName(row.source.groupId)" | ||
| (click)="navigateTo(row.source.groupId, dialogRequestGroupId, processGroupType)"> |
There was a problem hiding this comment.
This always navigates with (id, processGroupId, ProcessGroup) = (row.source.groupId, dialogRequestGroupId, ProcessGroup). For connections whose endpoint lives in the group currently on the canvas, those two ids are the same, so navigateToComponent routes to /process-groups/{groupId}/ProcessGroup/{groupId}.
The current process group is not a child in that group’s processGroups collection, so nothing is selected or centered. The destination process-group cell (lines 87–92) has the same problem.
Please treat “this is the group we’re already in” as non-interactive, or navigate to /process-groups/{groupId} without a selected component. Also drop the leftover TODO on line 30 — resolveGroupName already does that lookup.
| // groups it contains. Remote process groups are included alongside process groups because a connection to | ||
| // one reports the remote process group's own id as the source/destination group id of the remote port it | ||
| // terminates at, so the two kinds of id are looked up the same way. | ||
| export const selectProcessGroupIdToNameMap = createSelector( |
There was a problem hiding this comment.
This map only includes the current breadcrumb chain plus children of the currently loaded canvas group. For Input Port upstream / Output Port downstream, viewComponentConnections$ fetches the parent flow (flow.effects.ts ~3213–3221) and never puts that parent’s child groups/RPGs into canvas state.
resolveGroupName() then falls back to the raw UUID (component-connections-dialog.component.ts 147–148). That is exactly the cross-boundary case this feature is meant to clarify.
Please build the name map from the fetched ProcessGroupFlowEntity (plus breadcrumbs as needed) and pass it into the dialog request, rather than reading only current canvas state.
| @if (rows.length === 0) { | ||
| <div class="unset neutral-color">{{ emptyMessage }}</div> | ||
| } @else { | ||
| <div class="listing-table component-connections-table"> |
There was a problem hiding this comment.
This table is an unbounded listing-table with no scroll viewport, no sticky header, and no striped rows. With more than a handful of connections, the header and “Selected Component” block scroll away.
Please match the existing dialog/listing-table pattern:
- Bounded scroll + sticky header:
apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/local-changes-dialog/local-changes-table/local-changes-table.html(44–45, 104) andapps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-dialog/change-version-dialog.html(71–72, 124). Pattern:listing-table flex-1 relativewrappingabsolute inset-0 overflow-y-auto, and*matHeaderRowDef="displayedColumns; sticky: true". - Striped rows (
let even = even+[class.even]="even"), same two files plusapps/nifi/src/app/pages/summary/ui/common/cluster-summary-dialog/connection-cluster-table/connection-cluster-table.component.html(18–19, 121–125).
Keep truncation/tooltips; put the table in that scroll container.
| <tr mat-header-row *matHeaderRowDef="displayedColumns"></tr> | ||
| <tr mat-row *matRowDef="let row; columns: displayedColumns"></tr> |
There was a problem hiding this comment.
Add the sticky header and striped rows used by other listing tables.
| <tr mat-header-row *matHeaderRowDef="displayedColumns"></tr> | |
| <tr mat-row *matRowDef="let row; columns: displayedColumns"></tr> | |
| <tr mat-header-row *matHeaderRowDef="displayedColumns; sticky: true"></tr> | |
| <tr mat-row *matRowDef="let row; let even = even; columns: displayedColumns" [class.even]="even"></tr> |
| @@ -0,0 +1,90 @@ | |||
| <!-- | |||
There was a problem hiding this comment.
This looks like a leftover backup of an earlier template (Go To column, no per-cell navigation). Please remove it from the PR so it is not shipped.
| // flexible columns: truncate instead of pushing siblings out | ||
| .mat-column-source, | ||
| .mat-column-destination, | ||
| .mat-column-name { | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| max-width: 0; // works with table-layout: fixed to force shrinking | ||
| } |
There was a problem hiding this comment.
These selectors (.mat-column-source, .mat-column-destination, .mat-column-name) do not match displayedColumns (sourceProcessGroup, sourceComponent, connection, destinationProcessGroup, destinationComponent), so the truncation rules never apply.
Global .listing-table in libs/shared/src/assets/themes/components/_table.scss already sets table-layout: fixed and cell ellipsis. Either retarget the real mat-column-* classes or drop the dead rules. The commented-out .link-cell / .cell-text remnants below should go too.
| // flexible columns: truncate instead of pushing siblings out | |
| .mat-column-source, | |
| .mat-column-destination, | |
| .mat-column-name { | |
| overflow: hidden; | |
| text-overflow: ellipsis; | |
| white-space: nowrap; | |
| max-width: 0; // works with table-layout: fixed to force shrinking | |
| } |
…am connections. View connections > upstream/downstream is available in the context menu from a variety of components: input/output ports, processors, process groups, remote process groups and funnels.
Summary
In NiFi 1.x, there was a context menu option for View Connections > upstream/downstream. This is available from any component, but it is particularly useful for input/output ports. The reason is that the connections to ports are not visible on the graph at the same level as the ports themselves. The user must go to the parent process group, and then there is ambiguity which connections are connected to which ports in cases where there are multiple connections and ports on the process group.
A key feature of the View Connections table is that each item in the table is clickable and will navigate to the selected component or connection.
Restoring this feature makes it far easier to navigate the graph and identify complex routing/connectivity issues in the flow.
NIFI-14777
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Verification was performed by interacting with the graph and testing a variety of scenarios including:
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation