[NIFI-13053] Cluster page #8685
Conversation
rfellows
added
the
new ui
|
Will review... |
There was a problem hiding this comment.
Nice work @rfellows! I've left a few comments below. Most are minor but did notice one bigger issue. In the existing NiFi UI, the tabs that are populated through system diagnostics are hidden when the user lacks those permissions.
| const routes: Routes = [ | ||
| { | ||
| path: '', | ||
| component: Cluster, |
There was a problem hiding this comment.
Based on the changes to navigation.component.html we should be using the authorizationGuard to ensure the user has:
user.controllerPermissions.canRead
| from(this.clusterService.disconnectNode(request.nodeId)).pipe( | ||
| map((entity) => { | ||
| this.dialog.closeAll(); | ||
| return ClusterListingActions.updateNodeSuccess({ response: entity }); | ||
| }), | ||
| catchError((errorResponse: HttpErrorResponse) => { | ||
| this.dialog.closeAll(); | ||
| return of(ClusterListingActions.clusterNodeSnackbarError({ error: errorResponse.error })); | ||
| }) | ||
| ) |
There was a problem hiding this comment.
The this.dialog.closeAll() in this effect is unnecessary. The YesNoDialog does not stay open when the user presses Yes or No.
| from(this.clusterService.connectNode(request.nodeId)).pipe( | ||
| map((entity) => { | ||
| this.dialog.closeAll(); | ||
| return ClusterListingActions.updateNodeSuccess({ response: entity }); | ||
| }), | ||
| catchError((errorResponse: HttpErrorResponse) => { | ||
| this.dialog.closeAll(); | ||
| return of(ClusterListingActions.clusterNodeSnackbarError({ error: errorResponse.error })); | ||
| }) | ||
| ) |
There was a problem hiding this comment.
The this.dialog.closeAll() in this effect is unnecessary. The YesNoDialog does not stay open when the user presses Yes or No.
| from(this.clusterService.offloadNode(request.nodeId)).pipe( | ||
| map((entity) => { | ||
| this.dialog.closeAll(); | ||
| return ClusterListingActions.updateNodeSuccess({ response: entity }); | ||
| }), | ||
| catchError((errorResponse: HttpErrorResponse) => { | ||
| this.dialog.closeAll(); | ||
| return of(ClusterListingActions.clusterNodeSnackbarError({ error: errorResponse.error })); | ||
| }) | ||
| ) |
There was a problem hiding this comment.
The this.dialog.closeAll() in this effect is unnecessary. The YesNoDialog does not stay open when the user presses Yes or No.
| from(this.clusterService.removeNode(request.nodeId)).pipe( | ||
| map(() => { | ||
| this.dialog.closeAll(); | ||
| return ClusterListingActions.removeNodeSuccess({ response: request }); | ||
| }), | ||
| catchError((errorResponse: HttpErrorResponse) => { | ||
| this.dialog.closeAll(); | ||
| return of(ClusterListingActions.clusterNodeSnackbarError({ error: errorResponse.error })); | ||
| }) | ||
| ) |
There was a problem hiding this comment.
The this.dialog.closeAll() in this effect is unnecessary. The YesNoDialog does not stay open when the user presses Yes or No.
| return this._initialSortDirection; | ||
| } | ||
|
|
||
| @Input({}) set components(components: T[]) { |
There was a problem hiding this comment.
Is this {} empty object needed here?
| <!-- Node Address --> | ||
| <ng-container matColumnDef="address"> | ||
| <th mat-header-cell *matHeaderCellDef mat-sort-header title="Node Address"> | ||
| <div class="flex-1 overflow-ellipsis overflow-hidden whitespace-nowrap"> |
There was a problem hiding this comment.
I don't think that flex-1 isn needed in our column headers. This would apply to all headers in all tables in this PR.
| .cluster-table-filter-container { | ||
| } |
| import { ClusterVersionTable } from './cluster-version-table.component'; | ||
| import { NoopAnimationsModule } from '@angular/platform-browser/animations'; | ||
|
|
||
| describe('ClusterVersionTableComponent', () => { |
There was a problem hiding this comment.
| describe('ClusterVersionTableComponent', () => { | |
| describe('ClusterVersionTable', () => { |
| { label: 'System', link: 'system' }, | ||
| { label: 'JVM', link: 'jvm' }, | ||
| { label: 'FlowFile Storage', link: 'flowfile-storage' }, | ||
| { label: 'Content Storage', link: 'content-storage' }, | ||
| { label: 'Provenance Storage', link: 'provenance-storage' }, | ||
| { label: 'Versions', link: 'versions' } |
There was a problem hiding this comment.
The presence of these tabs should be conditional on the users permissions to System Diagnostics.
rfellows
force-pushed
the
NIFI-13053-nodeTable
branch
from
0cbc3a7
to
6a6fe89
Compare
* Node listing
…the user has the proper permission.
…state when user loses permission to it while on the cluster page.
rfellows
force-pushed
the
NIFI-13053-nodeTable
branch
from
58c1bf2
to
6c00aaf
Compare
NIFI-13053
All table here extend a base table
ClusterTableand leverage the same filter component. The "Storage" pages all share the same same table implementation.