rfellows commented on code in PR #8142: URL: https://github.com/apache/nifi/pull/8142#discussion_r1426906730
########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/registry-clients/edit-registry-client/edit-registry-client.component.html: ########## @@ -0,0 +1,70 @@ +<!-- + ~ 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>Edit Registry Client</h2> +<form class="edit-registry-client-form" [formGroup]="editRegistryClientForm"> + <mat-dialog-content> + <mat-tab-group> + <mat-tab label="Settings"> + <div class="tab-content py-4 flex flex-col"> + <div class="flex flex-col mb-5"> + <div>Id</div> + <div class="value">{{ request.registryClient.id }}</div> + </div> + <div> + <mat-form-field> + <mat-label>Name</mat-label> + <input matInput formControlName="name" type="text" /> + </mat-form-field> + </div> + <div class="flex flex-col mb-5"> + <div>Id</div> + <div class="value">{{ request.registryClient.component.type }}</div> + </div> + <div> + <mat-form-field> + <mat-label>Description</mat-label> + <textarea matInput formControlName="description" type="text"></textarea> + </mat-form-field> + </div> + </div> + </mat-tab> + <mat-tab label="Properties"> + <property-table + formControlName="properties" + [createNewProperty]="createNewProperty" + [createNewService]="createNewService" + [getParameters]="getParameters" + [getServiceLink]="getServiceLink" + [supportsSensitiveDynamicProperties]=" + request.registryClient.component.supportsSensitiveDynamicProperties + "></property-table> Review Comment: Should nest this in a `<div class="tab-content py-4 flex flex-col">` to give it consistent vertical height and padding as the first tab. As it stands now, the add button is jammed up against the tab and the size of the dialog changes when you flip between the tabs: <img width="775" alt="Screenshot 2023-12-14 at 10 50 43 AM" src="https://github.com/apache/nifi/assets/713866/3d154900-1387-4c49-9eb3-cd2479fe856f";> ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/registry-clients/registry-client-table/registry-client-table.component.ts: ########## @@ -0,0 +1,136 @@ +/* + * 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, ValidationErrorsTipInput } from '../../../../../state/shared'; +import { RegistryClientEntity } from '../../../state/registry-clients'; + +@Component({ + selector: 'registry-client-table', + templateUrl: './registry-client-table.component.html', + styleUrls: ['./registry-client-table.component.scss', '../../../../../../assets/styles/listing-table.scss'] +}) +export class RegistryClientTable implements AfterViewInit { + @Input() set registryClients(registryClientEntities: RegistryClientEntity[]) { + this.dataSource = new MatTableDataSource<RegistryClientEntity>(registryClientEntities); + this.dataSource.sort = this.sort; + this.dataSource.sortingDataAccessor = (data: RegistryClientEntity, 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); + } + return ''; + }; Review Comment: sorting on name and description doesn't work... ```suggestion this.dataSource.sortingDataAccessor = (data: RegistryClientEntity, displayColumn: string) => { if (displayColumn === 'name') { return data.component.name; } else if (displayColumn === 'description') { return data.component.description; } else if (displayColumn === 'type') { return this.formatType(data); } else if (displayColumn === 'bundle') { return this.formatBundle(data); } return ''; }; ``` ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerResource.java: ########## @@ -1483,6 +1483,7 @@ public Response getFlowRegistryClients() { final Set<FlowRegistryClientEntity> flowRegistryClients = serviceFacade.getRegistryClients(); final FlowRegistryClientsEntity flowRegistryClientEntities = new FlowRegistryClientsEntity(); + flowRegistryClientEntities.setCurrentTime(new Date()); Review Comment: FlowResource.java also instantiates a new `FlowRegistryClientEntity`. We should set the current date there as well to be consistent. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/registry-clients/registry-client-table/registry-client-table.component.html: ########## @@ -0,0 +1,119 @@ +<!-- + ~ 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="relative h-full border"> + <div class="registry-client-table listing-table absolute inset-0 overflow-y-auto"> + <table mat-table [dataSource]="dataSource" matSort matSortDisableClear> + <!-- More Details Column --> + <ng-container matColumnDef="moreDetails"> + <th mat-header-cell *matHeaderCellDef></th> + <td mat-cell *matCellDef="let item"> + <ng-container *ngIf="canRead(item)"> + <div class="flex items-center"> + <!-- TODO - handle read only in configure component? --> + <div + class="mr-3 pointer fa fa-warning has-errors" + *ngIf="hasErrors(item)" + nifiTooltip + [tooltipComponentType]="ValidationErrorsTip" + [tooltipInputData]="getValidationErrorsTipData(item)"></div> + <div + class="mr-3 pointer fa fa-sticky-note-o" + *ngIf="hasBulletins(item)" + nifiTooltip + [tooltipComponentType]="BulletinsTip" + [tooltipInputData]="getBulletinsTipData(item)"></div> + </div> + </ng-container> + </td> + </ng-container> + + <!-- Name Column --> + <ng-container matColumnDef="name"> + <th mat-header-cell *matHeaderCellDef mat-sort-header>Name</th> + <td mat-cell *matCellDef="let item"> + <ng-container *ngIf="canRead(item); else nameNoPermissions"> + {{ item.component.name }} + </ng-container> + <ng-template #nameNoPermissions> + <div class="unset">{{ item.id }}</div> + </ng-template> + </td> + </ng-container> + + <!-- Description Column --> + <ng-container matColumnDef="description"> + <th mat-header-cell *matHeaderCellDef mat-sort-header>Description</th> + <td mat-cell *matCellDef="let item"> + <ng-container *ngIf="canRead(item); else descriptionNoPermissions"> + {{ item.component.description }} + </ng-container> + <ng-template #descriptionNoPermissions> + <div class="unset">{{ item.id }}</div> + </ng-template> + </td> + </ng-container> + + <!-- Type Column --> + <ng-container matColumnDef="type"> + <th mat-header-cell *matHeaderCellDef mat-sort-header>Type</th> + <td mat-cell *matCellDef="let item"> + <ng-container *ngIf="canRead(item)"> + {{ formatType(item) }} + </ng-container> + </td> + </ng-container> + + <!-- Bundle Column --> + <ng-container matColumnDef="bundle"> + <th mat-header-cell *matHeaderCellDef mat-sort-header>Bundle</th> + <td mat-cell *matCellDef="let item"> + <ng-container *ngIf="canRead(item)"> + {{ formatBundle(item) }} + </ng-container> + </td> + </ng-container> + + <!-- Actions Column --> + <ng-container matColumnDef="actions"> + <th mat-header-cell *matHeaderCellDef></th> + <td mat-cell *matCellDef="let item"> + <div class="flex items-center gap-x-3"> + <div + class="pointer fa fa-pencil" + *ngIf="canConfigure(item)" + (click)="configureClicked(item, $event)" + title="Delete"></div> Review Comment: this is the wrong title for the configure action. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/registry-clients/registry-client-table/registry-client-table.component.html: ########## @@ -0,0 +1,119 @@ +<!-- + ~ 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="relative h-full border"> + <div class="registry-client-table listing-table absolute inset-0 overflow-y-auto"> + <table mat-table [dataSource]="dataSource" matSort matSortDisableClear> Review Comment: probably should add an initial sort: ```suggestion <table mat-table [dataSource]="dataSource" matSort matSortDisableClear matSortActive="name" matSortDirection="asc"> ``` but, of course that would introduce ExpressionChangedAfterItHasBeenCheckedError if we continue to use the `even` helper in the row def on line 116 below. So we should probably implement the sort function here. -- 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]
