You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "mcgilman (via GitHub)" <gi...@apache.org> on 2023/12/14 16:27:42 UTC

[PR] NIFI-12502: Handle additional Property Table capabilities [nifi]

mcgilman opened a new pull request, #8159:
URL: https://github.com/apache/nifi/pull/8159

   NIFI-12502:
   - Go To Service.
   - Save before navigating if form is dirty.
   - Go To Parameter.
   - Convert To Parameter.


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-12502: Handle additional Property Table capabilities [nifi]

Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #8159:
URL: https://github.com/apache/nifi/pull/8159#discussion_r1430362779


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.effects.ts:
##########
@@ -234,18 +253,134 @@ export class ControllerServicesEffects {
                         );
                     };
 
-                    editDialogReference.componentInstance.getServiceLink = (serviceId: string) => {
-                        return this.controllerServiceService.getControllerService(serviceId).pipe(
-                            take(1),
-                            map((serviceEntity) => {
-                                return [
+                    const goTo = (commands: string[], destination: string): void => {
+                        if (editDialogReference.componentInstance.editControllerServiceForm.dirty) {
+                            const saveChangesDialogReference = this.dialog.open(YesNoDialog, {
+                                data: {
+                                    title: 'Controller Service Configuration',
+                                    message: `Save changes before going to this ${destination}?`
+                                },
+                                panelClass: 'small-dialog'
+                            });
+
+                            saveChangesDialogReference.componentInstance.yes.pipe(take(1)).subscribe(() => {
+                                editDialogReference.componentInstance.submitForm(commands);
+                            });
+
+                            saveChangesDialogReference.componentInstance.no.pipe(take(1)).subscribe(() => {
+                                editDialogReference.close('ROUTED');
+                                this.router.navigate(commands);
+                            });
+                        } else {
+                            editDialogReference.close('ROUTED');
+                            this.router.navigate(commands);
+                        }
+                    };
+
+                    if (parameterContext != null) {
+                        editDialogReference.componentInstance.getParameters = (sensitive: boolean) => {
+                            return this.flowService.getParameterContext(parameterContext.id).pipe(
+                                take(1),
+                                map((response) => response.component.parameters),
+                                map((parameterEntities) => {
+                                    return parameterEntities
+                                        .map((parameterEntity: ParameterEntity) => parameterEntity.parameter)
+                                        .filter((parameter: Parameter) => parameter.sensitive == sensitive);
+                                })
+                            );
+                        };
+
+                        editDialogReference.componentInstance.parameterContext = parameterContext;
+                        editDialogReference.componentInstance.goToParameter = (parameter: string) => {
+                            const commands: string[] = ['/parameter-contexts', parameterContext.id];
+                            goTo(commands, 'Parameter');
+                        };
+
+                        editDialogReference.componentInstance.convertToParameter = (
+                            name: string,
+                            sensitive: boolean,
+                            value: string | null
+                        ) => {
+                            return this.parameterService.getParameterContext(parameterContext.id, false).pipe(
+                                switchMap((parameterContextEntity) => {
+                                    const existingParameters: string[] =
+                                        parameterContextEntity.component.parameters.map(
+                                            (parameterEntity: ParameterEntity) => parameterEntity.parameter.name
+                                        );
+                                    const convertToParameterDialogRequest: EditParameterRequest = {
+                                        parameter: {
+                                            name,
+                                            value,
+                                            sensitive,
+                                            description: ''
+                                        },
+                                        existingParameters
+                                    };
+                                    const convertToParameterDialogReference = this.dialog.open(EditParameterDialog, {
+                                        data: convertToParameterDialogRequest,
+                                        panelClass: 'medium-dialog'
+                                    });
+
+                                    convertToParameterDialogReference.componentInstance.saving$ =
+                                        this.store.select(selectParameterSaving);
+
+                                    convertToParameterDialogReference.componentInstance.cancel.pipe(
+                                        takeUntil(convertToParameterDialogReference.afterClosed()),
+                                        tap(() => ParameterActions.stopPollingParameterContextUpdateRequest())
+                                    );
+
+                                    return convertToParameterDialogReference.componentInstance.editParameter.pipe(
+                                        takeUntil(convertToParameterDialogReference.afterClosed()),
+                                        switchMap((dialogResponse: EditParameterResponse) => {
+                                            this.store.dispatch(
+                                                ParameterActions.submitParameterContextUpdateRequest({
+                                                    request: {
+                                                        id: parameterContext.id,
+                                                        payload: {
+                                                            revision: this.client.getRevision(parameterContextEntity),
+                                                            component: {
+                                                                id: parameterContextEntity.id,
+                                                                parameters: [{ parameter: dialogResponse.parameter }]
+                                                            }
+                                                        }
+                                                    }
+                                                })
+                                            );
+
+                                            return this.store.select(selectParameterSaving).pipe(
+                                                takeUntil(convertToParameterDialogReference.afterClosed()),
+                                                filter((parameterSaving) => parameterSaving === false),
+                                                map(() => {
+                                                    convertToParameterDialogReference.close();
+                                                    return `#{${dialogResponse.parameter.name}}`;
+                                                })
+                                            );
+                                        })
+                                    );
+                                }),
+                                catchError((error) => {
+                                    // TODO handle error
+                                    return NEVER;
+                                })
+                            );
+                        };
+                    }

Review Comment:
   Thanks for calling this out. I agree that we should investigate ways to improve this. The block is identical but does reference a method that is different and the dialog instances that are different. From an offline discussion, we will investigate opportunities to pull some of this logic into separate services allowing the Effects to be more concise and helpful with some re-use in follow on efforts. 



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-12502: Handle additional Property Table capabilities [nifi]

Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on PR #8159:
URL: https://github.com/apache/nifi/pull/8159#issuecomment-1856240098

   Will review...


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-12502: Handle additional Property Table capabilities [nifi]

Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on code in PR #8159:
URL: https://github.com/apache/nifi/pull/8159#discussion_r1428412617


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/ui/common/property-table/property-table.component.html:
##########
@@ -103,19 +103,26 @@
                 <th mat-header-cell *matHeaderCellDef></th>
                 <td mat-cell *matCellDef="let item">
                     <div class="flex items-center gap-x-3">
-                        <!-- TODO - convert to parameter, go to parameter -->
                         <div
                             class="pointer fa fa-plus"
                             *ngIf="item.descriptor.identifiesControllerService"
                             (click)="createNewControllerService(item)"
                             title="Create new service"></div>
-                        <!-- TODO - need to prevent navigation if outstanding changes -->
+                        <div
+                            class="pointer fa fa-level-up"
+                            *ngIf="canConvertToParameter(item)"
+                            (click)="convertToParameterClicked(item)"
+                            title="Go to Parameter"></div>

Review Comment:
   should be "Convert to Parameter"



##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/service/parameter-context.service.ts:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Injectable } from '@angular/core';
+import { Observable, throwError } from 'rxjs';
+import { HttpClient } from '@angular/common/http';
+import { NiFiCommon } from '../../../service/nifi-common.service';
+import { ParameterContextUpdateRequest, SubmitParameterContextUpdate } from '../../../state/shared';
+
+@Injectable({ providedIn: 'root' })
+export class ParameterContextService {

Review Comment:
   This class is almost an exact copy of `ParameterService`. Do we need both?



##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.effects.ts:
##########
@@ -234,18 +253,134 @@ export class ControllerServicesEffects {
                         );
                     };
 
-                    editDialogReference.componentInstance.getServiceLink = (serviceId: string) => {
-                        return this.controllerServiceService.getControllerService(serviceId).pipe(
-                            take(1),
-                            map((serviceEntity) => {
-                                return [
+                    const goTo = (commands: string[], destination: string): void => {
+                        if (editDialogReference.componentInstance.editControllerServiceForm.dirty) {
+                            const saveChangesDialogReference = this.dialog.open(YesNoDialog, {
+                                data: {
+                                    title: 'Controller Service Configuration',
+                                    message: `Save changes before going to this ${destination}?`
+                                },
+                                panelClass: 'small-dialog'
+                            });
+
+                            saveChangesDialogReference.componentInstance.yes.pipe(take(1)).subscribe(() => {
+                                editDialogReference.componentInstance.submitForm(commands);
+                            });
+
+                            saveChangesDialogReference.componentInstance.no.pipe(take(1)).subscribe(() => {
+                                editDialogReference.close('ROUTED');
+                                this.router.navigate(commands);
+                            });
+                        } else {
+                            editDialogReference.close('ROUTED');
+                            this.router.navigate(commands);
+                        }
+                    };
+
+                    if (parameterContext != null) {
+                        editDialogReference.componentInstance.getParameters = (sensitive: boolean) => {
+                            return this.flowService.getParameterContext(parameterContext.id).pipe(
+                                take(1),
+                                map((response) => response.component.parameters),
+                                map((parameterEntities) => {
+                                    return parameterEntities
+                                        .map((parameterEntity: ParameterEntity) => parameterEntity.parameter)
+                                        .filter((parameter: Parameter) => parameter.sensitive == sensitive);
+                                })
+                            );
+                        };
+
+                        editDialogReference.componentInstance.parameterContext = parameterContext;
+                        editDialogReference.componentInstance.goToParameter = (parameter: string) => {
+                            const commands: string[] = ['/parameter-contexts', parameterContext.id];
+                            goTo(commands, 'Parameter');
+                        };
+
+                        editDialogReference.componentInstance.convertToParameter = (
+                            name: string,
+                            sensitive: boolean,
+                            value: string | null
+                        ) => {
+                            return this.parameterService.getParameterContext(parameterContext.id, false).pipe(
+                                switchMap((parameterContextEntity) => {
+                                    const existingParameters: string[] =
+                                        parameterContextEntity.component.parameters.map(
+                                            (parameterEntity: ParameterEntity) => parameterEntity.parameter.name
+                                        );
+                                    const convertToParameterDialogRequest: EditParameterRequest = {
+                                        parameter: {
+                                            name,
+                                            value,
+                                            sensitive,
+                                            description: ''
+                                        },
+                                        existingParameters
+                                    };
+                                    const convertToParameterDialogReference = this.dialog.open(EditParameterDialog, {
+                                        data: convertToParameterDialogRequest,
+                                        panelClass: 'medium-dialog'
+                                    });
+
+                                    convertToParameterDialogReference.componentInstance.saving$ =
+                                        this.store.select(selectParameterSaving);
+
+                                    convertToParameterDialogReference.componentInstance.cancel.pipe(
+                                        takeUntil(convertToParameterDialogReference.afterClosed()),
+                                        tap(() => ParameterActions.stopPollingParameterContextUpdateRequest())
+                                    );
+
+                                    return convertToParameterDialogReference.componentInstance.editParameter.pipe(
+                                        takeUntil(convertToParameterDialogReference.afterClosed()),
+                                        switchMap((dialogResponse: EditParameterResponse) => {
+                                            this.store.dispatch(
+                                                ParameterActions.submitParameterContextUpdateRequest({
+                                                    request: {
+                                                        id: parameterContext.id,
+                                                        payload: {
+                                                            revision: this.client.getRevision(parameterContextEntity),
+                                                            component: {
+                                                                id: parameterContextEntity.id,
+                                                                parameters: [{ parameter: dialogResponse.parameter }]
+                                                            }
+                                                        }
+                                                    }
+                                                })
+                                            );
+
+                                            return this.store.select(selectParameterSaving).pipe(
+                                                takeUntil(convertToParameterDialogReference.afterClosed()),
+                                                filter((parameterSaving) => parameterSaving === false),
+                                                map(() => {
+                                                    convertToParameterDialogReference.close();
+                                                    return `#{${dialogResponse.parameter.name}}`;
+                                                })
+                                            );
+                                        })
+                                    );
+                                }),
+                                catchError((error) => {
+                                    // TODO handle error
+                                    return NEVER;
+                                })
+                            );
+                        };
+                    }

Review Comment:
   At some point in the near future we might want to consider refactoring this. This block (plus more) are identical to code in FlowService. Maintenance-wise, this isn't ideal.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-12502: Handle additional Property Table capabilities [nifi]

Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows merged PR #8159:
URL: https://github.com/apache/nifi/pull/8159


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org