rfellows commented on code in PR #11798:
URL: https://github.com/apache/nifi/pull/11798#discussion_r4260448204
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/state/flow-shared/index.ts:
##########
@@ -231,3 +245,306 @@ export interface SnippetDeleteRequest {
id: string;
options: SnippetDeleteOptions;
}
+
+/*
+ * Requests shared by the flow designer and the reusable component dialogs.
+ */
+
+export interface SelectedComponent {
+ id: string;
+ componentType: ComponentType;
+ entity?: any;
Review Comment:
This is still part of the promoted Create Connection dialog boundary, and
the dialog later dereferences `entity.id` and component-specific fields without
narrowing. With `any`, malformed endpoint inputs compile and then fail at
runtime. Please introduce a dialog-specific endpoint contract backed by
`CanvasConnectableEntity`—ideally a discriminated type that requires an entity
for direct endpoints and separately models process-group/remote-group
child-port selection.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/component-dialogs/create-connection/create-connection.component.ts:
##########
@@ -89,11 +91,14 @@ import { ContextErrorBanner } from
'../../../../../../../ui/common/context-error
export class CreateConnection extends CloseOnEscapeDialog {
private dialogRequest =
inject<CreateConnectionDialogRequest>(MAT_DIALOG_DATA);
private formBuilder = inject(FormBuilder);
- private store = inject<Store<NiFiState>>(Store);
- private canvasUtils = inject(CanvasUtils);
private clusterConnectionService = inject(ClusterConnectionService);
private client = inject(Client);
+ @Input() saving$: Observable<boolean> = of(false);
+ @Input() availablePrioritizers$: Observable<DocumentedType[]> = of([]);
+ @Input() breadcrumbs$: Observable<BreadcrumbEntity | null> = of(null);
+ @Output() createConnection = new EventEmitter<CreateConnectionPayload>();
+
@Input() set getChildOutputPorts(getChildOutputPorts: (groupId: string) =>
Observable<any>) {
Review Comment:
Please type this callback as `Observable<PortEntity[]>` and apply the same
type to the input-port callback and `childInputPorts$` / `childOutputPorts$`.
The dialog relies on array semantics and each item’s `id`; keeping `any` here
allows invalid reusable-host callbacks to compile and leaves the promoted input
boundary only partially typed.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/flow/flow.effects.spec.ts:
##########
@@ -1901,4 +1932,413 @@ describe('FlowEffects', () => {
});
});
});
+
+ describe('openNewConnectionDialog$', () => {
+ const request = {
+ source: { id: 'src', componentType: ComponentType.Processor },
+ destination: { id: 'dst', componentType: ComponentType.Processor }
+ };
+
+ it('assigns dialog inputs and dispatches createConnection from the
dialog output', async () => {
+ const afterClosed = new Subject<void>();
+ const createConnection = new EventEmitter<CreateConnection>();
+ const componentInstance: {
+ createConnection: EventEmitter<CreateConnection>;
+ saving$: Observable<boolean>;
+ breadcrumbs$: Observable<unknown>;
+ availablePrioritizers$: Observable<DocumentedType[]>;
+ getChildOutputPorts: (groupId: string) => Observable<unknown>;
+ getChildInputPorts: (groupId: string) => Observable<unknown>;
+ } = {
+ createConnection,
+ saving$: of(true),
+ breadcrumbs$: of(null),
+ availablePrioritizers$: of([]),
+ getChildOutputPorts: () => of([]),
+ getChildInputPorts: () => of([])
+ };
+ vi.spyOn(dialog, 'open').mockReturnValue({
+ componentInstance,
+ afterClosed: () => afterClosed.asObservable()
+ } as unknown as MatDialogRef<unknown>);
+ vi.spyOn(flowService, 'getProcessGroup').mockReturnValue(
+ of({
+ component: {
+ defaultFlowFileExpiration: '0 sec',
+ defaultBackPressureObjectThreshold: 10000,
+ defaultBackPressureDataSizeThreshold: '1 GB'
+ }
+ })
+ );
+ const outputPorts = [{ id: 'out-1' }];
+ const inputPorts = [{ id: 'in-1' }];
+ vi.spyOn(flowService, 'getFlow').mockReturnValue(
+ of({
+ processGroupFlow: {
+ flow: {
+ outputPorts,
+ inputPorts
+ }
+ }
+ })
+ );
+ const prioritizers = [{ type:
'org.apache.nifi.prioritizer.FirstInFirstOutPrioritizer' } as DocumentedType];
+ store.overrideSelector(selectCurrentProcessGroupId, 'pg-1');
+ store.overrideSelector(selectPrioritizerTypes, prioritizers);
+ store.refreshState();
+
+ effects.openNewConnectionDialog$.subscribe();
+ action$.next(FlowActions.openNewConnectionDialog({ request }));
+
+ expect(await
firstValueFrom(componentInstance.saving$)).toBe(false);
+ expect(await
firstValueFrom(componentInstance.breadcrumbs$)).toEqual(
+ fromFlow.initialState.flow.processGroupFlow.breadcrumb
+ );
+ expect(await
firstValueFrom(componentInstance.availablePrioritizers$)).toEqual(prioritizers);
+ expect(await
firstValueFrom(componentInstance.getChildOutputPorts('child-1'))).toEqual(outputPorts);
+ expect(await
firstValueFrom(componentInstance.getChildInputPorts('child-1'))).toEqual(inputPorts);
+ expect(flowService.getFlow).toHaveBeenCalledWith('child-1');
+
+ const payload: CreateConnection = {
+ payload: { revision: { version: 0, clientId: 'client-1' },
component: { name: '' } }
+ };
+ createConnection.emit(payload);
+
+
expect(store.dispatch).toHaveBeenCalledWith(FlowActions.createConnection({
request: payload }));
+ });
+
+ it('removes the temporary edge when the dialog closes', () => {
+ const afterClosed = new Subject<void>();
+ vi.spyOn(dialog, 'open').mockReturnValue({
+ componentInstance: { createConnection: new
EventEmitter<CreateConnection>() },
+ afterClosed: () => afterClosed.asObservable()
+ } as unknown as MatDialogRef<unknown>);
+ vi.spyOn(flowService, 'getProcessGroup').mockReturnValue(
+ of({
+ component: {
+ defaultFlowFileExpiration: '0 sec',
+ defaultBackPressureObjectThreshold: 10000,
+ defaultBackPressureDataSizeThreshold: '1 GB'
+ }
+ })
+ );
+ const canvasUtils = TestBed.inject(CanvasUtils);
+ vi.spyOn(canvasUtils, 'removeTempEdge');
+
+ effects.openNewConnectionDialog$.subscribe();
+ action$.next(FlowActions.openNewConnectionDialog({ request }));
+ afterClosed.next();
+
+ expect(canvasUtils.removeTempEdge).toHaveBeenCalled();
+ });
+
+ it('removes the temporary edge when process group defaults fail to
load', () => {
+ vi.spyOn(flowService, 'getProcessGroup').mockReturnValue(
+ throwError(() => new HttpErrorResponse({ status: 500 }))
+ );
+ const canvasUtils = TestBed.inject(CanvasUtils);
+ vi.spyOn(canvasUtils, 'removeTempEdge');
+
+ effects.openNewConnectionDialog$.subscribe({ error: () =>
undefined });
+ action$.next(FlowActions.openNewConnectionDialog({ request }));
+
+ expect(canvasUtils.removeTempEdge).toHaveBeenCalled();
+ });
+ });
+
+ describe('create dialog outputs', () => {
+ const componentRequest: CreateComponentRequest = {
+ type: ComponentType.Processor,
+ position: { x: 0, y: 0 },
+ revision: { version: 0, clientId: 'client-1' }
+ };
+
Review Comment:
This helper does not type-check with `EventEmitter<CreateProcessorRequest>`,
`EventEmitter<CreatePortRequest>`, and the other typed emitters because
`EventEmitter<T>` cannot be widened to `EventEmitter<unknown>`. `npx tsc -p
apps/nifi/tsconfig.spec.json --noEmit` also reports new changed-file errors for
the empty/incomplete connection fixtures and the incomplete registry-import
payload. Please make this helper generic (or otherwise preserve each emitter
type) and update all modified fixtures to satisfy the tightened contracts.
Vitest transpilation passing does not validate these TypeScript contracts.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/component-dialogs/create-process-group/create-process-group.component.spec.ts:
##########
@@ -129,6 +120,27 @@ describe('CreateProcessGroup', () => {
fixture.detectChanges();
expect(component.parameterContextsOptions.length).toEqual(2);
});
+
+ it('hides the parameter context selector when supportsParameters is
false', () => {
+ component.supportsParameters = false;
+ fixture.detectChanges();
+
+ const select = fixture.debugElement.query(
+
By.css('mat-select[formcontrolname="newProcessGroupParameterContext"]')
+ );
+ expect(select).toBeNull();
+ });
+
+ it('emits a null parameter context id when supportsParameters is
false', () => {
+ component.supportsParameters = false;
+
component.createProcessGroupForm.get('newProcessGroupName')?.setValue('child');
+ const emitted: CreateProcessGroupRequest[] = [];
+ component.createProcessGroup.subscribe((request) =>
emitted.push(request));
+
+ component.submitCreateProcessGroup();
+
+ expect(emitted[0].parameterContextId).toBeNull();
Review Comment:
This verifies `null` only when parameter contexts are unsupported. The
normal supported flow also has a valid “No parameter context” selection and
should emit `parameterContextId: null`; that path currently has no regression
assertion. Please add that case and a corresponding submit/output test in
`group-components.component.spec.ts`, which currently only verifies component
construction for its default null selection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
