mcgilman commented on code in PR #8857:
URL: https://github.com/apache/nifi/pull/8857#discussion_r1608849065
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/flow-analysis-rules/edit-flow-analysis-rule/edit-flow-analysis-rule.component.ts:
##########
@@ -75,6 +78,10 @@ export class EditFlowAnalysisRule extends
CloseOnEscapeDialog {
@Input() createNewService!: (request: InlineServiceCreationRequest) =>
Observable<InlineServiceCreationResponse>;
@Input() saving$!: Observable<boolean>;
@Input() goToService!: (serviceId: string) => void;
+ @Input() propertyVerificationResults$!:
Observable<ConfigVerificationResult[]>;
+ @Input() propertyVerificationStatus$: Observable<'pending' | 'loading' |
'success'> = of('pending');
+
+ @Output() verify: EventEmitter<any> = new EventEmitter();
Review Comment:
Can we use `FlowAnalysisRuleEntity` instead of `any`?
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/processor/edit-processor/edit-processor.component.html:
##########
@@ -249,6 +250,12 @@ <h2 mat-dialog-title>
[supportsSensitiveDynamicProperties]="
request.entity.component.supportsSensitiveDynamicProperties
"></property-table>
+ <property-verification
+ class="verification w-1/3"
Review Comment:
It doesn't look like the `verification` style is used anywhere. This is
present on all usage of the `property-verification` component.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/ui/common/property-table/editors/nf-editor/nf-editor.component.ts:
##########
@@ -75,7 +75,7 @@ export class NfEditor implements OnDestroy {
this.loadParameters();
}
- @Input() set parameters(parameters: Parameter[]) {
+ @Input() set parameters(parameters: Parameter[] | null) {
Review Comment:
Thanks for catching this one! 🎉
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/reporting-tasks/edit-reporting-task/edit-reporting-task.component.ts:
##########
@@ -79,6 +82,10 @@ export class EditReportingTask extends CloseOnEscapeDialog {
@Input() goToService!: (serviceId: string) => void;
@Input() goToReferencingComponent!: (component:
ControllerServiceReferencingComponent) => void;
@Input() saving$!: Observable<boolean>;
+ @Input() propertyVerificationResults$!:
Observable<ConfigVerificationResult[]>;
+ @Input() propertyVerificationStatus$: Observable<'pending' | 'loading' |
'success'> = of('pending');
+
+ @Output() verify: EventEmitter<any> = new EventEmitter();
Review Comment:
Can we use `ReportingTaskEntity` instead of `any`?
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/ui/common/property-table/property-table.component.ts:
##########
@@ -114,6 +114,8 @@ export class PropertyTable implements AfterViewInit,
ControlValueAccessor {
@Input() goToService!: (serviceId: string) => void;
@Input() supportsSensitiveDynamicProperties = false;
@Input() propertyHistory: ComponentHistory | undefined;
+ @Input() supportsParameters: boolean = true;
+ @Input() forReferencedAttributes: boolean = false;
Review Comment:
I don't think the changes introduced here for the `forReferencedAttributes`
Input are being used. Assuming that is the case, we should be able to revert
the changes in the `PropertyTable` component.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/ui/common/property-verification/common/referenced-attributes-dialog/referenced-attributes-dialog.component.ts:
##########
@@ -0,0 +1,91 @@
+/*
+ * 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 { Component, EventEmitter, Inject, Input, Output } from '@angular/core';
+import { CommonModule } from '@angular/common';
+import {
+ MAT_DIALOG_DATA,
+ MatDialogActions,
+ MatDialogClose,
+ MatDialogContent,
+ MatDialogTitle
+} from '@angular/material/dialog';
+import { FormBuilder, FormControl, FormGroup, ReactiveFormsModule } from
'@angular/forms';
+import { Observable } from 'rxjs';
+import { MapTableEntry } from '../../../../../state/shared';
+import { MatButton, MatIconButton } from '@angular/material/button';
+import { NifiSpinnerDirective } from '../../../spinner/nifi-spinner.directive';
+import { MapTable } from '../../../map-table/map-table.component';
+import { NifiTooltipDirective } from
'../../../tooltips/nifi-tooltip.directive';
+import { TextTip } from '../../../tooltips/text-tip/text-tip.component';
+import { CloseOnEscapeDialog } from
'../../../close-on-escape-dialog/close-on-escape-dialog.component';
+
+export interface ReferencedAttributesDialogData {
+ attributes: MapTableEntry[];
+}
+
+@Component({
+ selector: 'referenced-attributes-dialog',
+ standalone: true,
+ imports: [
+ CommonModule,
+ MatDialogTitle,
+ ReactiveFormsModule,
+ MatDialogContent,
+ MatDialogActions,
+ MatButton,
+ MatDialogClose,
+ NifiSpinnerDirective,
+ MapTable,
+ NifiTooltipDirective,
+ MatIconButton
+ ],
+ templateUrl: './referenced-attributes-dialog.component.html',
+ styleUrl: './referenced-attributes-dialog.component.scss'
+})
+export class ReferencedAttributesDialog extends CloseOnEscapeDialog {
+ referencedAttributesForm: FormGroup;
+
+ @Input() createNew!: (existingProperties: string[]) =>
Observable<MapTableEntry>;
Review Comment:
```suggestion
@Input() createNew!: (existingEntries: string[]) =>
Observable<MapTableEntry>;
```
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/service/property-verification.service.ts:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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 { HttpClient } from '@angular/common/http';
+import { Client } from './client.service';
+import {
+ ConfigurationAnalysisResponse,
+ InitiateVerificationRequest,
+ PropertyVerificationResponse,
+ VerifyPropertiesRequestContext
+} from '../state/property-verification';
+import { Observable, of } from 'rxjs';
+import { NiFiCommon } from './nifi-common.service';
+import { PropertyDescriptorEntity, PropertyDescriptorRetriever } from
'../state/shared';
+
+@Injectable({ providedIn: 'root' })
+export class PropertyVerificationService implements
PropertyDescriptorRetriever {
+ private static readonly API: string = '../nifi-api';
+
+ constructor(
+ private httpClient: HttpClient,
+ private client: Client,
+ private nifiCommon: NiFiCommon
+ ) {}
+
+ getAnalysis(request: VerifyPropertiesRequestContext):
Observable<ConfigurationAnalysisResponse> {
+ const body = {
+ configurationAnalysis: {
+ componentId: request.entity.id,
+ properties: request.properties
+ }
+ };
+ return this.httpClient.post(
+
`${this.nifiCommon.stripProtocol(request.entity.uri)}/config/analysis`,
+ body
+ ) as Observable<ConfigurationAnalysisResponse>;
+ }
+
+ initiatePropertyVerification(request: InitiateVerificationRequest):
Observable<PropertyVerificationResponse> {
+ return this.httpClient.post(
+
`${this.nifiCommon.stripProtocol(request.uri)}/config/verification-requests`,
+ request.request
+ ) as Observable<PropertyVerificationResponse>;
+ }
+
+ getPropertyVerificationRequest(requestId: string, uri: string):
Observable<PropertyVerificationResponse> {
+ return this.httpClient.get(
+
`${this.nifiCommon.stripProtocol(uri)}/config/verification-requests/${requestId}`
+ ) as Observable<PropertyVerificationResponse>;
+ }
+
+ deletePropertyVerificationRequest(requestId: string, uri: string) {
+ return this.httpClient.delete(
+
`${this.nifiCommon.stripProtocol(uri)}/config/verification-requests/${requestId}`
+ );
+ }
+
+ getPropertyDescriptor(id: string, propertyName: string, sensitive:
boolean): Observable<PropertyDescriptorEntity> {
Review Comment:
This is not used.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/parameter-providers/edit-parameter-provider/edit-parameter-provider.component.ts:
##########
@@ -75,7 +78,10 @@ export class EditParameterProvider extends
CloseOnEscapeDialog {
@Input() goToService!: (serviceId: string) => void;
@Input() goToReferencingParameterContext!: (parameterContextId: string) =>
void;
@Input() saving$!: Observable<boolean>;
+ @Input() propertyVerificationResults$!:
Observable<ConfigVerificationResult[]>;
+ @Input() propertyVerificationStatus$: Observable<'pending' | 'loading' |
'success'> = of('pending');
+ @Output() verify: EventEmitter<any> = new EventEmitter();
Review Comment:
Can we use `ParameterProviderEntity` instead of `any`?
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/ui/common/property-verification/property-verification.component.spec.ts:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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 { ComponentFixture, TestBed } from '@angular/core/testing';
+import { PropertyVerification } from './property-verification.component';
+
+describe('PropertyVerificationComponent', () => {
Review Comment:
```suggestion
describe('PropertyVerification', () => {
```
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/ui/common/property-verification/property-verification.component.html:
##########
@@ -0,0 +1,58 @@
+<!--
+ ~ 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.
+ -->
+
+<div class="property-verification flex flex-col h-full gap-y-3">
+ <div class="flex justify-between items-center">
+ <div class="font-bold">Verification</div>
+ <div>
+ <button mat-icon-button color="primary" type="button"
[disabled]="disabled" (click)="verifyClicked()">
+ <i class="fa fa-check"></i>
+ </button>
+ </div>
+ </div>
+ @if (disabled) {
+ <div class="verification-disabled unset">Property verification is
disabled</div>
+ } @else if (isVerifying) {
+ <div class="empty">Verifying properties...</div>
+ } @else if (results.length > 0) {
+ <div class="verification-results flex flex-col gap-y-2
overflow-y-auto">
Review Comment:
IMO I think there should be a border around the verification like we do for
Service references or Fetch parameters. Happy to discuss this more. Generally
it looks good but my primary concern in when the results scroll like in the pic
below. The border helps when the text clips.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/reporting-tasks/edit-reporting-task/edit-reporting-task.component.ts:
##########
@@ -79,6 +82,10 @@ export class EditReportingTask extends CloseOnEscapeDialog {
@Input() goToService!: (serviceId: string) => void;
@Input() goToReferencingComponent!: (component:
ControllerServiceReferencingComponent) => void;
@Input() saving$!: Observable<boolean>;
+ @Input() propertyVerificationResults$!:
Observable<ConfigVerificationResult[]>;
+ @Input() propertyVerificationStatus$: Observable<'pending' | 'loading' |
'success'> = of('pending');
+
+ @Output() verify: EventEmitter<any> = new EventEmitter();
Review Comment:
Thoughts on changing the type being emitted to include the
`ReportingTaskEntity` and the modified properties? If we did that we wouldn't
need to call this method from the Effects. This question/comment applies to all
new `verify` Output emitters.
--
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]
