Repository navigation
NIFI-16488 Add component creation and deletion to the Connector canvas - #11900
Conversation
- create processors, ports, process groups, remote process groups, funnels, labels, and connections on the Connector canvas while the Connector is in Troubleshooting and writable - delete the selection from the context menu or the Delete key, skipping components that cannot be deleted and removing outgoing connections of deleted components - reload the endpoints of a connection after it is created, deleted, or moved to a new destination - reconcile created components with any copy a poll already added - enable canvas editing from the Connector's current Troubleshooting state - move the new canvas item chip to ui/common so it emits outputs, and update the flow designer header to use it - extract the Connector canvas context menu into a service - keep selection and double-click ahead of drag on funnels, ports, process groups, remote process groups, and connection labels - keep a connection label on the same bend point when bend points are added or removed, and attach the label drag once so it works after the label is selected - validate dynamic path segments in create and delete requests
|
reviewing... |
There was a problem hiding this comment.
Thanks Matt, this is a really implementation. The state reconciliation, the outgoing-connection cleanup on delete, and the move of the new canvas item chip to ui/common all look good. The specs are thorough as well.
A few suggestions inline. The first one (canEdit and write permission) is the only one I'd consider important. I ran nx test nifi locally at e8ceec2, and all 3,556 tests passed.
| ]), | ||
| filter(([, connectorEntity]) => connectorEntity !== null), | ||
| map(([, connectorEntity, features]) => ({ | ||
| canEdit: connectorEntity!.component.state === 'TROUBLESHOOTING', |
There was a problem hiding this comment.
canEdit here is set from state === 'TROUBLESHOOTING' alone. The palette, the Delete context-menu item and the Delete key all gate on ConnectorCanvasContextMenu.canEdit (connector-canvas-context-menu.service.ts:78), which also requires canvasPermissions()?.canWrite.
This canvas feature flag is what enables component drag and the connection-drawing handles. So a user who can read but not write the flow can still start a drag or draw a connection during Troubleshooting. The server rejects it, but the user sees an error banner and the component snaps back.
Could this use the same condition as the context menu? It would also be worth adding a spec for "in Troubleshooting but not writable".
| createProcessor(processGroupId: string, request: CreateProcessorRequest): Observable<ProcessorEntity> { | ||
| return defer(() => | ||
| this.httpClient.post<ProcessorEntity>( | ||
| `${ConnectorEditService.API}/process-groups/${safeApiPath(processGroupId)}/processors`, |
There was a problem hiding this comment.
The new create, get and delete calls in this service go through safeApiPath, but two raw segments remain earlier in the file (outside this diff):
updateComponent(line 71):${path}/${request.id}createConnection(line 75):process-groups/${processGroupId}/connections
The PR description says every dynamic path segment in create and delete requests is validated, and createConnection is a create. Could both be wrapped the same way as these new methods (safeApiPath(...) inside defer)?
| () => | ||
| this.actions$.pipe( | ||
| ofType(ConnectorCanvasActions.createCanvasComponentSuccess), | ||
| tap(() => this.dialog.closeAll()) |
There was a problem hiding this comment.
The flow designer selects a newly created component (createComponentComplete$), and this PR already selects new connections. Processors, ports, funnels, labels and process groups created here aren't selected, though.
Would it make sense to dispatch selectComponents for response.payload.id / response.type alongside closeAll(), for parity?
| ([{ request }, connectorId, processGroupId]) => | ||
| request.length > 0 && connectorId !== '' && processGroupId !== null | ||
| ), | ||
| mergeMap(([{ request }, connectorId, processGroupId, connections]) => { |
There was a problem hiding this comment.
Two small things on this effect:
- With
mergeMap, pressing Delete twice quickly sends the same revisions twice. The second request gets a 409, which shows a banner and reloads the flow.exhaustMapwould make this consistent withcreateCanvasComponent$. - If the
filterabove drops a request, the reducer has already setsaving: trueand nothing clears it. In practicecanEditmeans the flow is always loaded by then, so this is defensive only.
| expect(result.connections).toEqual([]); | ||
| }); | ||
|
|
||
| it('does not restore a connection leaving a deleted remote process group from an older snapshot', () => { |
There was a problem hiding this comment.
The reducer correctly maps a connection's source to the child process group when sourceGroupId !== processGroupId. These specs cover the remote process group case, but not deleting a child process group whose output ports have outgoing connections. Could you add a case for that?
Summary
NIFI-16488
Adds component creation and deletion to the Connector canvas. When a Connector is in Troubleshooting and the user has write permission, the canvas supports the same create and delete interactions as the flow designer.
Creation
ui/commonand now emits outputs instead of dispatching flow designer actions. The flow designer header uses the relocated chip.Deletion
deletion.utilshelpers.Canvas interaction
ConnectorCanvasContextMenuService, and the Delete command was added to it.Security
safeApiPath.Testing
npx nx test nifi: 357 test files, 3,556 tests passingConnectorEditService, the Connector canvas actions, reducer, selectors, and effects, the context menu and commands services, the Connector canvas component, the canvas component (label anchoring on bend points), the funnel, port, process group, remote process group, and connection renderers,deletion.utils, the relocated new canvas item chip, and the flow designer headerTracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation