rfellows commented on code in PR #8053: URL: https://github.com/apache/nifi/pull/8053#discussion_r1399638818
########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/parameter-contexts/ui/parameter-context-listing/parameter-context-listing.component.ts: ########## @@ -0,0 +1,118 @@ +/* + * 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 } from '@angular/core'; +import { Store } from '@ngrx/store'; +import { ParameterContextEntity, ParameterContextListingState } from '../../state/parameter-context-listing'; +import { + selectContext, + selectParameterContextIdFromRoute, + selectParameterContextListingState, + selectSingleEditedParameterContext +} from '../../state/parameter-context-listing/parameter-context-listing.selectors'; +import { + getEffectiveParameterContextAndOpenDialog, + loadParameterContexts, + navigateToEditParameterContext, + openNewParameterContextDialog, + promptParameterContextDeletion, + selectParameterContext +} from '../../state/parameter-context-listing/parameter-context-listing.actions'; +import { initialState } from '../../state/parameter-context-listing/parameter-context-listing.reducer'; +import { filter, switchMap, take } from 'rxjs'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; + +@Component({ + selector: 'parameter-context-listing', + templateUrl: './parameter-context-listing.component.html', + styleUrls: ['./parameter-context-listing.component.scss'] +}) +export class ParameterContextListing { Review Comment: while not required, we should declare this class to `implement OnInit` as it defines an `ngOnInit` method. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/service/loading.service.ts: ########## @@ -0,0 +1,49 @@ +/* + * 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 { BehaviorSubject, delay, Observable } from 'rxjs'; + +@Injectable({ + providedIn: 'root' +}) +export class LoadingService { + status$: Observable<boolean>; + + loading: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false); + requests: Map<string, boolean> = new Map<string, boolean>(); Review Comment: These should probably be marked as `private`. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/service/storage.service.ts: ########## @@ -0,0 +1,162 @@ +/* + * 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'; + +@Injectable({ + providedIn: 'root' +}) +export class Storage { + private static readonly MILLIS_PER_DAY: number = 86400000; + private static readonly TWO_DAYS: number = Storage.MILLIS_PER_DAY * 2; + + constructor() {} + + /** + * Checks the expiration for the specified entry. + * + * @param {object} entry + * @returns {boolean} + */ + private checkExpiration(entry: any): boolean { + if (entry.expires == null) { + // get the expiration + const expires: Date = new Date(entry.expires); + const now: Date = new Date(); + + // return whether the expiration date has passed + return expires.valueOf() < now.valueOf(); + } else { + return false; + } + } Review Comment: The logic seems off to me. If the expiration is null, we are comparing `Date(null) < Date()` which are the same thing. If it isn't null, we just return false as if it will never expire. I think we want to change the condition to: ``` if (entry.expires) { ... as it is above } else { // assumes it doesn't expire? return false; } ``` ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/parameter-contexts/ui/parameter-context-listing/parameter-context-table/parameter-context-table.component.ts: ########## @@ -0,0 +1,117 @@ +/* + * 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 { AfterViewInit, Component, EventEmitter, Input, Output, ViewChild } from '@angular/core'; +import { MatTableDataSource } from '@angular/material/table'; +import { MatSort } from '@angular/material/sort'; +import { NiFiCommon } from '../../../../../service/nifi-common.service'; +import { ParameterContextEntity } from '../../../state/parameter-context-listing'; + +@Component({ + selector: 'parameter-context-table', + templateUrl: './parameter-context-table.component.html', + styleUrls: ['./parameter-context-table.component.scss'] +}) +export class ParameterContextTable implements AfterViewInit { + @Input() set parameterContexts(parameterContextEntities: ParameterContextEntity[]) { + this.dataSource = new MatTableDataSource<ParameterContextEntity>(parameterContextEntities); + this.dataSource.sort = this.sort; + this.dataSource.sortingDataAccessor = (data: ParameterContextEntity, displayColumn: string) => { + if (this.canRead(data)) { + if (displayColumn == 'name') { + return this.formatName(data); + } else if (displayColumn == 'type') { + return this.formatProvider(data); + } else if (displayColumn == 'bundle') { + return this.formatDescription(data); + } Review Comment: Could use `===` here instead of `==` ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/ui/common/controller-service/edit-controller-service/edit-controller-service.component.ts: ########## @@ -0,0 +1,156 @@ +/* + * 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 { MAT_DIALOG_DATA, MatDialogModule } from '@angular/material/dialog'; +import { AbstractControl, FormBuilder, FormControl, FormGroup, ReactiveFormsModule, Validators } from '@angular/forms'; +import { Client } from '../../../../service/client.service'; +import { + ControllerServiceEntity, + EditControllerServiceDialogRequest, + InlineServiceCreationRequest, + InlineServiceCreationResponse, + Parameter, + Property +} from '../../../../state/shared'; +import { MatInputModule } from '@angular/material/input'; +import { MatCheckboxModule } from '@angular/material/checkbox'; +import { MatButtonModule } from '@angular/material/button'; +import { AsyncPipe, NgForOf, NgIf } from '@angular/common'; +import { MatTabsModule } from '@angular/material/tabs'; +import { NiFiCommon } from '../../../../service/nifi-common.service'; +import { MatOptionModule } from '@angular/material/core'; +import { MatSelectModule } from '@angular/material/select'; +import { PropertyTable } from '../../property-table/property-table.component'; +import { ControllerServiceApi } from '../controller-service-api/controller-service-api.component'; +import { Observable } from 'rxjs'; +import { ControllerServiceReferences } from '../controller-service-references/controller-service-references.component'; +import { NifiSpinnerDirective } from '../../spinner/nifi-spinner.directive'; + +@Component({ + selector: 'edit-controller-service', + standalone: true, + templateUrl: './edit-controller-service.component.html', + imports: [ + ReactiveFormsModule, + MatDialogModule, + MatInputModule, + MatCheckboxModule, + MatButtonModule, + NgIf, + MatTabsModule, + MatOptionModule, + MatSelectModule, + NgForOf, + PropertyTable, + ControllerServiceApi, + ControllerServiceReferences, + AsyncPipe, + NifiSpinnerDirective + ], + styleUrls: ['./edit-controller-service.component.scss'] +}) +export class EditControllerService { + @Input() createNewProperty!: (existingProperties: string[], allowsSensitive: boolean) => Observable<Property>; + @Input() createNewService!: (request: InlineServiceCreationRequest) => Observable<InlineServiceCreationResponse>; + @Input() getParameters!: (sensitive: boolean) => Observable<Parameter[]>; + @Input() getServiceLink!: (serviceId: string) => Observable<string[]>; + @Input() saving$!: Observable<boolean>; + @Output() editControllerService: EventEmitter<any> = new EventEmitter<any>(); + + editControllerServiceForm: FormGroup; + + bulletinLevels = [ + { + text: 'DEBUG', + value: 'DEBUG' + }, + { + text: 'INFO', + value: 'INFO' + }, + { + text: 'WARN', + value: 'WARN' + }, + { + text: 'ERROR', + value: 'ERROR' + }, + { + text: 'NONE', + value: 'NONE' + } + ]; + + constructor( + @Inject(MAT_DIALOG_DATA) public request: EditControllerServiceDialogRequest, + private formBuilder: FormBuilder, + private client: Client, + private nifiCommon: NiFiCommon + ) { + const serviceProperties: any = request.controllerService.component.properties; + const properties: Property[] = Object.entries(serviceProperties).map((entry: any) => { + const [property, value] = entry; + return { + property, + value, + descriptor: request.controllerService.component.descriptors[property] + }; + }); + + // build the form + this.editControllerServiceForm = this.formBuilder.group({ + name: new FormControl(request.controllerService.component.name, Validators.required), + bulletinLevel: new FormControl(request.controllerService.component.bulletinLevel, Validators.required), + properties: new FormControl(properties), + comments: new FormControl(request.controllerService.component.comments) + }); + } + + formatType(entity: ControllerServiceEntity): string { + return this.nifiCommon.formatType(entity.component); + } + + formatBundle(entity: ControllerServiceEntity): string { + return this.nifiCommon.formatBundle(entity.component.bundle); + } + + submitForm() { + const payload: any = { + revision: this.client.getRevision(this.request.controllerService), + component: { + id: this.request.controllerService.id, + name: this.editControllerServiceForm.get('name')?.value, + comments: this.editControllerServiceForm.get('comments')?.value + } + }; + + const propertyControl: AbstractControl | null = this.editControllerServiceForm.get('properties'); + if (propertyControl && propertyControl.dirty) { + const properties: Property[] = propertyControl.value; + const values: { [key: string]: string | null } = {}; + properties.forEach((property) => (values[property.property] = property.value)); + payload.component.properties = values; + payload.component.config.sensitiveDynamicPropertyNames = properties + .filter((property) => property.descriptor.dynamic && property.descriptor.sensitive) + .map((property) => property.descriptor.name); Review Comment: The payload.component does not have a `config` object defined (see lines 134-141). So this fails when attempting to modify a controller service properties: `Cannot set properties of undefined (setting 'sensitiveDynamicPropertyNames')` ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/parameter-contexts/ui/parameter-context-listing/edit-parameter-context/edit-parameter-context.component.html: ########## @@ -0,0 +1,125 @@ +<!-- + ~ 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. + --> + +<h2 mat-dialog-title>{{ this.isNew ? 'Add' : 'Edit' }} Parameter Context</h2> +<form class="parameter-context-edit-form" [formGroup]="editParameterContextForm"> + <mat-dialog-content *ngIf="(updateRequest | async)! as requestEntity; else editFormContent"> + <div class="results-content flex gap-x-8"> + <div class="w-full flex flex-col"> + <div>Steps To Update Parameters</div> + <div class="flex flex-col gap-y-1.5"> + <div + *ngFor="let updateStep of requestEntity.request.updateSteps" + class="flex justify-between items-center"> + <div class="value">{{ updateStep.description }}</div> + <div *ngIf="updateStep.complete; else stepInProgress" class="fa fa-check text-green-500"></div> + <ng-template #stepInProgress> + <div class="fa fa-spin fa-circle-o-notch"></div> + </ng-template> + </div> + </div> + </div> + <div class="w-full flex flex-col gap-y-3"> + <div class="flex flex-col"> + <div>Parameter</div> + <div class="value">{{ getUpdatedParameters() }}</div> + </div> + <div class="flex-1 flex flex-col"> + <div>Referencing Components</div> + <div class="flex-1"></div> + </div> Review Comment: Maybe add a TODO here to display the referencing components while the the changes are being applied ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/state/reporting-tasks/reporting-tasks.reducer.ts: ########## @@ -0,0 +1,99 @@ +/* + * 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 { createReducer, on } from '@ngrx/store'; +import { ReportingTasksState } from './index'; +import { + createReportingTask, + createReportingTaskSuccess, + deleteReportingTaskSuccess, + loadReportingTasks, + loadReportingTasksSuccess, + reportingTasksApiError, + startReportingTaskSuccess, + stopReportingTaskSuccess +} from './reporting-tasks.actions'; +import { produce } from 'immer'; + +export const initialState: ReportingTasksState = { + reportingTasks: [], + saving: false, + loadedTimestamp: '', + error: null, + status: 'pending' +}; Review Comment: We might want to consider adding a selected property to the state or to the reporting task in the list of the one that is selected. This might be a bigger change as it would imply a refactoring of how selection is handled. At the moment, selection is not persisted and thus makes it challenging to programmatically select an item in the list as a part of another action. Specifically when an item is added to the list as a result of a creation action. The list is updated in the store, but there is no way to indicate it should be selected as a result. If I'm missing something, let me know and we can discuss. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/reporting-tasks/reporting-task-table/reporting-task-table.component.ts: ########## @@ -0,0 +1,248 @@ +/* + * 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 { AfterViewInit, Component, EventEmitter, Input, Output, ViewChild } from '@angular/core'; +import { MatTableDataSource } from '@angular/material/table'; +import { MatSort } from '@angular/material/sort'; +import { ReportingTaskEntity } from '../../../state/reporting-tasks'; +import { TextTip } from '../../../../../ui/common/tooltips/text-tip/text-tip.component'; +import { BulletinsTip } from '../../../../../ui/common/tooltips/bulletins-tip/bulletins-tip.component'; +import { ValidationErrorsTip } from '../../../../../ui/common/tooltips/validation-errors-tip/validation-errors-tip.component'; +import { NiFiCommon } from '../../../../../service/nifi-common.service'; +import { BulletinsTipInput, TextTipInput, ValidationErrorsTipInput } from '../../../../../state/shared'; + +@Component({ + selector: 'reporting-task-table', + templateUrl: './reporting-task-table.component.html', + styleUrls: ['./reporting-task-table.component.scss'] +}) +export class ReportingTaskTable implements AfterViewInit { + @Input() set reportingTasks(reportingTaskEntities: ReportingTaskEntity[]) { + this.dataSource = new MatTableDataSource<ReportingTaskEntity>(reportingTaskEntities); + this.dataSource.sort = this.sort; + this.dataSource.sortingDataAccessor = (data: ReportingTaskEntity, displayColumn: string) => { + if (displayColumn == 'name') { + return this.formatType(data); + } else if (displayColumn == 'type') { + return this.formatType(data); + } else if (displayColumn == 'bundle') { + return this.formatBundle(data); + } else if (displayColumn == 'state') { + return this.formatState(data); + } Review Comment: these could all use `===` rather than `==` -- 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]
