rfellows commented on code in PR #11487:
URL: https://github.com/apache/nifi/pull/11487#discussion_r3692863373
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/connector.service.ts:
##########
@@ -124,6 +124,14 @@ export class ConnectorService {
);
}
+ getControllerService(connectorId: string, controllerServiceId: string):
Observable<any> {
+ const uiOnly: any = { uiOnly: true };
+ return this.httpClient.get(
+
`${ConnectorService.API}/connectors/${connectorId}/controller-services/${controllerServiceId}`,
+ { params: uiOnly }
+ );
+ }
Review Comment:
Prefer typing this off `any`. `ControllerServiceEntity` already exists and
is imported in the effect specs, so the response can be
`Observable<ControllerServiceEntity>`, and the params can mirror
`cancelConnectorDrain` above (`Record<string, ...>`) instead of `const uiOnly:
any`:
```ts
getControllerService(connectorId: string, controllerServiceId: string):
Observable<ControllerServiceEntity> {
const params: Record<string, boolean> = { uiOnly: true };
return this.httpClient.get<ControllerServiceEntity>(
`${ConnectorService.API}/connectors/${connectorId}/controller-services/${controllerServiceId}`,
{ params }
);
}
```
This satisfies the strict-typing / no-`any` guidance and, as a bonus, makes
the effect's `serviceEntity` strongly typed so `component`'s optionality is
visible at the call site. The sibling
`getConnectorFlow`/`getConnectorControllerServices` also return
`Observable<any>`, so if you'd rather keep parity within the file that's
defensible — but tightening at least this new method is the better move.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/state/connector-canvas/connector-canvas.effects.ts:
##########
@@ -720,7 +721,49 @@ export class ConnectorCanvasEffects {
instance.createNewProperty = () => NEVER;
instance.createNewService = () => NEVER;
instance.convertToParameter = () => NEVER;
- instance.goToService = () => undefined;
+ instance.goToService = (serviceId: string) => {
+ this.store
+ .select(selectConnectorIdFromRoute)
+ .pipe(take(1))
+ .subscribe((connectorId) => {
+ if (!connectorId) {
+ this.store.dispatch(
+ ErrorActions.addBannerError({
+ errorContext: {
+ errors: ['Unable to determine Connector id
for navigation.'],
+ context: ErrorContextKey.CONNECTOR_CANVAS
+ }
+ })
+ );
+ return;
+ }
+
+ this.connectorService
+ .getControllerService(connectorId, serviceId)
+ .pipe(takeUntil(dialogRef.afterClosed()))
+ .subscribe({
+ next: (serviceEntity) => {
+ this.store.dispatch(
+
ConnectorCanvasActions.navigateToControllerService({
+ processGroupId:
serviceEntity.component.parentGroupId,
+ serviceId: serviceEntity.id
+ })
+ );
+ dialogRef.close();
+ },
+ error: (errorResponse: HttpErrorResponse) => {
+ this.store.dispatch(
+ ErrorActions.addBannerError({
+ errorContext: {
+ errors:
[this.errorHelper.getErrorString(errorResponse)],
+ context:
ErrorContextKey.CONNECTOR_CANVAS
+ }
+ })
+ );
+ }
+ });
+ });
+ };
Review Comment:
This ~40-line `goToService` handler is duplicated almost verbatim in
`connector-controller-services.effects.ts` (lines 154-196); the two copies
differ only in the `ErrorContextKey` (`CONNECTOR_CANVAS` vs
`CONTROLLER_SERVICES`). Consider extracting a shared helper next to
`bind-connector-parameter-context.ts`, e.g.:
```ts
bindGoToService(this.store, this.connectorService, this.errorHelper,
dialogRef, ErrorContextKey.CONNECTOR_CANVAS);
```
That keeps the two canvases from drifting — as written, any fix to one
branch has to be mirrored by hand in the other.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/state/connector-controller-services/connector-controller-services.effects.ts:
##########
@@ -148,7 +151,49 @@ export class ConnectorControllerServicesEffects {
// values still render in the value tip.
instance.createNewService = () => NEVER;
instance.convertToParameter = () => NEVER;
- instance.goToService = () => undefined;
+ instance.goToService = (serviceId: string) => {
+ this.store
+ .select(selectConnectorIdFromRoute)
+ .pipe(take(1))
+ .subscribe((connectorId) => {
+ if (!connectorId) {
+ this.store.dispatch(
+ ErrorActions.addBannerError({
+ errorContext: {
+ errors: ['Unable to determine
Connector id for navigation.'],
+ context:
ErrorContextKey.CONTROLLER_SERVICES
+ }
+ })
+ );
+ return;
+ }
+
+ this.connectorService
+ .getControllerService(connectorId,
serviceId)
+ .pipe(takeUntil(dialogRef.afterClosed()))
+ .subscribe({
+ next: (serviceEntity) => {
+ this.store.dispatch(
+
ConnectorCanvasActions.navigateToControllerService({
+ processGroupId:
serviceEntity.component.parentGroupId,
+ serviceId: serviceEntity.id
+ })
+ );
+ dialogRef.close();
+ },
+ error: (errorResponse:
HttpErrorResponse) => {
+ this.store.dispatch(
+ ErrorActions.addBannerError({
+ errorContext: {
+ errors:
[this.errorHelper.getErrorString(errorResponse)],
+ context:
ErrorContextKey.CONTROLLER_SERVICES
+ }
+ })
+ );
+ }
+ });
+ });
+ };
Review Comment:
Duplicate of the `goToService` handler in `connector-canvas.effects.ts`
(lines 724-766) — identical except for `ErrorContextKey.CONTROLLER_SERVICES`.
See the suggestion there to extract a shared `bindGoToService(...)` helper so
the two implementations can't drift.
--
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]
