Skip to content

NIFI-14777 Restore capability from NiFi 1.x to view upstream/downstre… - #11582

Open
markobean wants to merge 8 commits into
apache:mainfrom
markobean:NIFI-14777
Open

markobean wants to merge 8 commits into
apache:mainfrom
markobean:NIFI-14777

Conversation

@markobean

Copy link
Copy Markdown
Contributor

…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

  • 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

Verification was performed by interacting with the graph and testing a variety of scenarios including:

  • viewing connections from processors, input/output ports, process groups, remote process groups and funnels
  • disabling access to upstream and downstream connected components via access policies
  • renaming connected components to verify the View Connections table is updated similarly
  • clicking on each item in the View Connections table navigates to the selected component or connection

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

@pvillard31 pvillard31 added the ui Pull requests for work relating to the user interface label Aug 24, 2026
@Freedom9339

Copy link
Copy Markdown
Contributor

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.

@markobean

Copy link
Copy Markdown
Contributor Author

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.

@rfellows

Copy link
Copy Markdown
Contributor

Will review...

@rfellows rfellows left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. The new dialog tests fail (empty mock store + assertions still targeting goTo() / a partial row shape).
  2. Remote-port cells navigate with the port id and ComponentType.RemoteProcessGroup.
  3. 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.

@markobean

Copy link
Copy Markdown
Contributor Author

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:

  1. The new dialog tests fail (empty mock store + assertions still targeting goTo() / a partial row shape).
  2. Remote-port cells navigate with the port id and ComponentType.RemoteProcessGroup.
  3. 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.

@rfellows Thanks for taking the time to review and catch a few things that needed attention.

I'm made numerous updates.

  1. The tests were refactored completely. The original submission had tests leftover from the original implementation. They are now current and all pass with npx nx test nifi --runInBand.
  2. Remote port components are no longer clickable in the table since they are not rendered on the local canvas.
  3. Any source/destination process group which is the current process group is non-clickable
  4. Table "buttons" were replaced with more native and elements.
  5. Syntax for styles were updated.

…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.
…in the parent group to support port-to-port connection in sibling PGs

Update table formatting with fixed header and scrollable
…to match current state of upstream/downstream dialog functionality; original was from a older implementation

@rfellows rfellows left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow-up. Tests, current-group non-navigation, the fetched name map, removal of .html.orig, column SCSS, <a>/<span> cells, and the sticky/scroll table look good. npx nx test nifi --runInBand is green (2872).

Still blocking:

  1. Source/Destination Process Group cells always call navigateTo(..., processGroupType), so an RPG endpoint becomes /process-groups/{group}/ProcessGroup/{rpgId}.
  2. nx run nifi:lint fails (trailing comma in flow.effects.ts, unused import in flow.selectors.ts).

Also please see the notes on reusing component-context for the header, empty-canvas View Connections, table sorting, and whitespace inside the links.

Comment on lines +22 to +25
<div class="tertiary-color font-medium">Selected Component<br>
<i class="icon component-type-icon" [class]="componentIcon(componentType)"></i>
{{ componentName }}
</div>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The “Selected Component” header (icon + name) is a good place to reuse the shared component-context widget instead of a one-off icon/name block.

That widget already maps ComponentType to the canvas icon (including icon-group-remote), shows the type label, and offers a copyable id. Cluster Summary uses it the same way at the top of a dialog:

<component-context
    [type]="componentType"
    [name]="componentName"
    [id]="componentId"></component-context>

(cluster-summary-dialog.component.html; also the Operation panel.) Import ComponentContext from @nifi/shared and add it to this component’s imports.

ViewComponentConnectionsRequest already has id / name / type. Those just need to be forwarded on ComponentConnectionsDialogRequest so the header can bind [id] as well. The name fallback you already have in requestComponentConnections (readable name, else the component id) is the right [name]component-context does not itself handle canRead.

Comment on lines +292 to 294
condition: (selection: d3.Selection<any, any, any, any>) => {
return this.canvasUtils.hasUpstream(selection);
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View Connections never appears on empty-canvas right-click. hasUpstream / hasDownstream require selection.size() === 1, so a click on the canvas (no component selected) yields an empty submenu and the parent item is hidden (context-menu.component.ts keeps a submenu only when it has visible children).

Empty canvas is the current process group — the Operation panel already treats selection.size() === 0 that way (getContextTypeProcessGroup, name from breadcrumbs). Several canvas actions do the same, for example Enable/Disable All Controller Services:

condition: (selection) => {
    return this.canvasUtils.isProcessGroup(selection) || this.canvasUtils.emptySelection(selection);
}

If the intent is to restore 1.x “view connections for the group I’m in,” allow empty selection here and, when selection.empty(), use the current process group id (canvasUtils.getProcessGroupId()) instead of selection.datum(). That matches how those other current-PG actions are wired.

Comment on lines +41 to +47
<a
class="component-connection-cell neutral-contrast"
[matTooltip]="resolveGroupName(row.source.groupId)"
(click)="navigateTo(row.source.groupId, dialogRequestGroupId, processGroupType)">
<i class="icon component-type-icon flex-none" [class]="componentIcon(processGroupType)"></i>
{{ resolveGroupName(row.source.groupId) }}
</a>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remote ports are correctly non-clickable now, but the Source/Destination Process Group cells still always call navigateTo(..., processGroupType):

(click)="navigateTo(row.source.groupId, dialogRequestGroupId, processGroupType)"

For REMOTE_INPUT_PORT / REMOTE_OUTPUT_PORT, groupId is the remote process group id, so this still routes to /process-groups/{current}/ProcessGroup/{rpgId} instead of RemoteProcessGroup/{rpgId} — the same class of bug as navigating the port itself. The icon is also icon-group rather than icon-group-remote.

Go To Source already uses ComponentType.RemoteProcessGroup when the connectable is remote (canvas-context-menu.service.ts). remoteProcessGroupIds is already on the dialog request and never read; wire that (or endpoint.type === RemoteProcessGroup) for the group-cell type and icon, and add a click test for that cell. The destination process-group link (lines 103–109) has the same issue.

} @else {
<div class="listing-table component-connections-table flex-1 relative">
<div class="absolute inset-0 overflow-y-auto overflow-x-hidden">
<table mat-table [dataSource]="rows">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This listing table is not sortable. Other dialog tables (Change Version, Local Changes) use Angular Material sort on the mat-table. A compact version of that pattern:

<table
    mat-table
    [dataSource]="dataSource"
    matSort
    matSortDisableClear
    (matSortChange)="sortData($event)"
    [matSortActive]="initialSortColumn"
    [matSortDirection]="initialSortDirection">
    ...
    <th mat-header-cell *matHeaderCellDef mat-sort-header>Connection</th>

See change-version-dialog.html (73–80, 99–100) and local-changes-table.html / local-changes-table.ts (sortData + nifiCommon.compareString).

Practical defaults for this dialog:

  • Use MatTableDataSource (or keep the array and re-assign on sort).
  • Import MatSortModule.
  • Default [matSortActive]="'connection'" and [matSortDirection]="'asc'".
  • Sort each column by the visible label (resolveGroupName(...), endpoint name, connection name), with nifiCommon.compareString, so unauthorized / unnamed rows still have a stable key.

Comment on lines +66 to +72
<a
class="component-connection-cell neutral-contrast"
[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>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underline on these links starts a character before the icon/text. That comes from whitespace in the template becoming text nodes inside the <a>. For example:

<a
    class="component-connection-cell neutral-contrast"
    ...>
    <i class="icon component-type-icon" [class]="componentIcon(row.source.type)"></i>
    {{ row.source.name }}
</a>

The newline/indent between > / <i> / {{ ... }} is rendered as spaces, so the underline is longer than the visible content. Icon-to-label spacing is already handled in SCSS (.component-type-icon { margin-right: 0.25rem; }), so the extra DOM spaces are not needed.

Please keep the opening tag, icon, interpolation, and closing tag flush (all five link/span cells):

<a
    class="component-connection-cell neutral-contrast"
    [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>

direction: request.direction,
connections: flowEntity.processGroupFlow.flow.connections.filter(attachedTo),
groupIdToName: this.buildProcessGroupIdToNameMap(flowEntity),
remoteProcessGroupIds: this.buildRemoteProcessGroupIdSet(flowEntity),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npx nx run nifi:lint fails on this change set:

  1. Here — prettier wants the trailing comma after remoteProcessGroupIds: this.buildRemoteProcessGroupIdSet(flowEntity) removed.
  2. flow.selectors.ts:22 — unused BreadcrumbEntity import (@typescript-eslint/no-unused-vars), leftover from moving the name map onto the dialog request.

Please fix both so CI lint stays green.

import { createSelector } from '@ngrx/store';
import { CanvasState, selectCanvasState } from '../index';
import { ComponentType, selectCurrentRoute } from '@nifi/shared';
import { BreadcrumbEntity } from '../../../../state/shared';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused BreadcrumbEntity import (@typescript-eslint/no-unused-vars), leftover from moving the name map onto the dialog request. Please drop it (same lint pass as the trailing comma in flow.effects.ts).


<h2 mat-dialog-title>{{ title }}</h2>
<mat-dialog-content>
<!-- <div class="flex flex-col gap-y-4">-->

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of leftovers from the iteration:

  1. This commented-out wrapper: <!-- <div class="flex flex-col gap-y-4">-->. Please delete it rather than leaving it in the shipped template.
  2. component-connections-dialog.component.spec.ts 674–697 and 699–722 are the same test (navigates to the connection in the group that defines the dialog request). Please keep one.

Comment on lines +699 to +722
it('navigates to the connection in the group that defines the dialog request', () => {
const connection = readableConnection({
id: 'connection-to-navigate-to',
name: 'Connection To Navigate To'
});

const { fixture, store, dialogRef } = createDialog('upstream', [connection]);
const dispatch = vi.spyOn(store, 'dispatch');

const connectionCell = getCells(fixture, 'mat-column-connection')[0];
const link = connectionCell.querySelector('a') as HTMLAnchorElement;
link.click();

expect(dispatch).toHaveBeenCalledWith(
navigateToComponent({
request: {
id: 'connection-to-navigate-to',
processGroupId: REQUEST_GROUP_ID,
type: ComponentType.Connection
}
})
);
expect(dialogRef.close).toHaveBeenCalled();
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is a duplicate of the one immediately above (same name and assertions). Please keep one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui Pull requests for work relating to the user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants