scottyaslan commented on code in PR #9855: URL: https://github.com/apache/nifi/pull/9855#discussion_r2162727963
########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/service/droplets.service.ts: ########## @@ -0,0 +1,64 @@ +/* + * 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 } from 'rxjs'; +import { HttpClient, HttpHeaders } from '@angular/common/http'; + +@Injectable({ providedIn: 'root' }) +export class DropletsService { + private static readonly API: string = '../nifi-registry-api'; + + constructor(private httpClient: HttpClient) {} + + getDroplets(bucketId?: string): Observable<any> { + if (bucketId) { + return this.httpClient.get(`${DropletsService.API}/items/${bucketId}`); + } + return this.httpClient.get(`${DropletsService.API}/items`); + } + + deleteDroplet(href: string): Observable<any> { + return this.httpClient.delete(`${DropletsService.API}/${href}?version=0`); + } + + createNewFlow(bucketUri: string, name: string, description: string): Observable<any> { + return this.httpClient.post(`${DropletsService.API}/${bucketUri}/flows`, { + name: name, + description: description + }); + } + + uploadFlow(flowUri: string, file: File, description: string): Observable<any> { + return this.httpClient.post(`${DropletsService.API}/${flowUri}/versions/import`, file, { + headers: { Comments: description } + }); + } + + exportDropletVersionedSnapshot(dropletUri: string, versionNumber: number): Observable<any> { Review Comment: Can we do better than Observable<any> here? It would be better if we had some types defined. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/ui/import-new-flow-dialog/import-new-flow-dialog.component.html: ########## @@ -0,0 +1,92 @@ +<!-- +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>Import New Flow</h2> +<div class="import-new-flow-dialog"> + <form class="import-new-flow-form" [formGroup]="importNewFlowForm"> + <mat-dialog-content> + <context-error-banner [context]="ErrorContextKey.DROPLETS"></context-error-banner> Review Comment: Needs its own context. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/service/current-user.service.ts: ########## @@ -0,0 +1,31 @@ +/* + * 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 } from 'rxjs'; +import { HttpClient } from '@angular/common/http'; + +@Injectable({ providedIn: 'root' }) +export class CurrentUserService { + private static readonly API: string = '../nifi-registry-api'; + + constructor(private httpClient: HttpClient) {} + + getUser(): Observable<any> { Review Comment: Can we do better than Observable<any> here? It would be better if we had some types defined. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/ui/import-new-flow-dialog/import-new-flow-dialog.component.html: ########## @@ -0,0 +1,92 @@ +<!-- +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>Import New Flow</h2> +<div class="import-new-flow-dialog"> + <form class="import-new-flow-form" [formGroup]="importNewFlowForm"> + <mat-dialog-content> + <context-error-banner [context]="ErrorContextKey.DROPLETS"></context-error-banner> + <div class="flex justify-between items-center"> + <div class="w-full flex flex-1 items-center"> + <mat-form-field class="flex-1"> Review Comment: ```suggestion <mat-form-field class="w-full"> ``` ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/ui/import-new-flow-dialog/import-new-flow-dialog.component.html: ########## @@ -0,0 +1,92 @@ +<!-- +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>Import New Flow</h2> +<div class="import-new-flow-dialog"> + <form class="import-new-flow-form" [formGroup]="importNewFlowForm"> + <mat-dialog-content> + <context-error-banner [context]="ErrorContextKey.DROPLETS"></context-error-banner> + <div class="flex justify-between items-center"> + <div class="w-full flex flex-1 items-center"> + <mat-form-field class="flex-1"> + <mat-label>Flow Name</mat-label> + <input #flowName id="new-flow-name-input-field" class="mb-2 w-full" matInput Review Comment: ```suggestion <input matInput ``` ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/ui/header/header.component.ts: ########## @@ -0,0 +1,44 @@ +/* + * 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, OnInit, Signal } from '@angular/core'; +import { CommonModule, NgOptimizedImage } from '@angular/common'; +import { Store } from '@ngrx/store'; +import { selectCurrentUser } from '../../state/current-user/current-user.selectors'; +import { CurrentUser } from '../../state/current-user'; +import { loadCurrentUser, startCurrentUserPolling } from '../../state/current-user/current-user.actions'; +import { RouterModule } from '@angular/router'; + +@Component({ + selector: 'app-header', + standalone: true, + imports: [CommonModule, NgOptimizedImage, RouterModule], + templateUrl: './header.component.html', + styleUrl: './header.component.scss' +}) +export class HeaderComponent implements OnInit { + currentUser: Signal<CurrentUser>; + + constructor(private store: Store) { + this.currentUser = this.store.selectSignal(selectCurrentUser); + } + + ngOnInit(): void { + this.store.dispatch(loadCurrentUser()); + this.store.dispatch(startCurrentUserPolling()); Review Comment: I am trying to help get this PR wrapped up. As such, I think we can delete these lines and we can delete the state for the Current User. You aren't using it for anything yet and it is not needed for this PR so just stash it away until we address the current user etc. Then we can figure out if we need polling or if we can just load the user once. I think legacy NiFi Registry only loads the user once. Anyway, to simplify this PR can you please remove all that? ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/state/error/error.actions.ts: ########## @@ -0,0 +1,37 @@ +/* + * 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 { createAction, props } from '@ngrx/store'; +import { ErrorContext, ErrorContextKey, ErrorDetail } from './index'; + +export const fullScreenError = createAction( + '[Error] Full Screen Error', + props<{ errorDetail: ErrorDetail; skipReplaceUrl?: boolean }>() +); + +export const snackBarError = createAction('[Error] Snackbar Error', props<{ error: string }>()); + +export const addBannerError = createAction('[Error] Add Banner Error', props<{ errorContext: ErrorContext }>()); + +export const clearBannerErrors = createAction('[Error] Clear Banner Errors', props<{ context: ErrorContextKey }>()); + +export const resetErrorState = createAction('[Error] Reset Error State'); + +export const setRoutedToFullScreenError = createAction( + '[Error] Set Routed To Full Screen Error', + props<{ routedToFullScreenError: boolean }>() +); Review Comment: Are you really using all of these actions in this PR? If not, let's delete the ones we aren't using and we can bring them back when we need them for the next feature we work. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/ui/header/common/context-error-banner/context-error-banner.component.ts: ########## @@ -0,0 +1,60 @@ +/* + * 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, DestroyRef, inject, Input, OnDestroy } from '@angular/core'; +import { CommonModule } from '@angular/common'; +import { NiFiState } from '../../../../state'; +import { Store } from '@ngrx/store'; +import { clearBannerErrors } from '../../../../state/error/error.actions'; +import { Observable } from 'rxjs'; +import { selectBannerErrors } from '../../../../state/error/error.selectors'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { ErrorBanner } from '@nifi/shared'; +import { ErrorContextKey } from '../../../../state/error'; + +@Component({ + selector: 'context-error-banner', + imports: [CommonModule, ErrorBanner], + templateUrl: './context-error-banner.component.html', + styleUrl: './context-error-banner.component.scss' +}) +export class ContextErrorBanner implements OnDestroy { Review Comment: It appears that you have copied this component from the nifi application. I think we may be able to just move the context-error-banner.component into the shared lib so we can benefit from reuse. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/service/droplets.service.ts: ########## @@ -0,0 +1,64 @@ +/* + * 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 } from 'rxjs'; +import { HttpClient, HttpHeaders } from '@angular/common/http'; + +@Injectable({ providedIn: 'root' }) +export class DropletsService { + private static readonly API: string = '../nifi-registry-api'; + + constructor(private httpClient: HttpClient) {} + + getDroplets(bucketId?: string): Observable<any> { + if (bucketId) { + return this.httpClient.get(`${DropletsService.API}/items/${bucketId}`); + } + return this.httpClient.get(`${DropletsService.API}/items`); + } + + deleteDroplet(href: string): Observable<any> { + return this.httpClient.delete(`${DropletsService.API}/${href}?version=0`); + } + + createNewFlow(bucketUri: string, name: string, description: string): Observable<any> { + return this.httpClient.post(`${DropletsService.API}/${bucketUri}/flows`, { + name: name, + description: description + }); + } + + uploadFlow(flowUri: string, file: File, description: string): Observable<any> { + return this.httpClient.post(`${DropletsService.API}/${flowUri}/versions/import`, file, { + headers: { Comments: description } + }); + } + + exportDropletVersionedSnapshot(dropletUri: string, versionNumber: number): Observable<any> { + const headers = new HttpHeaders({ 'Content-Type': 'application/json' }); + return this.httpClient.get(`${DropletsService.API}/${dropletUri}/versions/${versionNumber}/export`, { + headers, + observe: 'response', + responseType: 'text' + }); + } + + getDropletSnapshotMetadata(dropletUri: string): Observable<any> { Review Comment: Can we do better than Observable<any> here? It would be better if we had some types defined. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/ui/import-new-flow-version-dialog/import-new-flow-version-dialog.component.html: ########## @@ -0,0 +1,82 @@ +<!-- +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>Import New Flow Version</h2> +<div class="import-new-flow-version-dialog"> + <form class="import-new-flow-version-form" [formGroup]="importNewFlowVersionForm"> + <context-error-banner [context]="ErrorContextKey.DROPLETS"></context-error-banner> Review Comment: Here I see you have used the same context for each of the dialogs. This works in the current UX because none of these dialog are open at the same time. However, we could improve this an the best practice here is to create a unique context for each error. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/ui/delete-droplet-dialog/delete-droplet-dialog.component.html: ########## @@ -0,0 +1,30 @@ +<!-- +~ 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>Delete Flow?</h2> +<div class="delete-droplet-dialog"> + <context-error-banner [context]="ErrorContextKey.DROPLETS"></context-error-banner> Review Comment: Needs its own context. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/ui/header/header.component.scss: ########## @@ -0,0 +1,22 @@ +/* + * 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. + */ + +.logo-container { + background-color: #2c3e44; // legacy registry header background color not part of theme + height: calc(4rem - 4px); + width: calc(4rem - 4px); Review Comment: I was thinking more like: ```suggestion height: 52px; width: 52px; ``` ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/service/error-helper.service.ts: ########## @@ -0,0 +1,97 @@ +/* + * 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 { HttpErrorResponse } from '@angular/common/http'; +import * as ErrorActions from '../state/error/error.actions'; +import { Action } from '@ngrx/store'; + +@Injectable({ providedIn: 'root' }) +export class ErrorHelper { Review Comment: Is this just a copy from the nifi application? Again, could this be moved to the shared lib? ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/explorer.component.html: ########## @@ -0,0 +1,54 @@ +<!-- +~ 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="pb-5 px-5 flex flex-col h-full"> + <div class="h-full flex flex-col"> + <h3 class="primary-color">Flows</h3> Review Comment: "Registry - a subproject of Apache NiFi - is a complementary application that provides a central location for storage and management of shared resources across one or more instances of NiFi or MiNiFi." - https://nifi.apache.org/projects/registry/ Flows doesn't really make sense here as the page title. Maybe ```suggestion <h3 class="primary-color">Resources</h3> ``` ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/pages/expolorer/feature/explorer.component.ts: ########## @@ -0,0 +1,237 @@ +/* + * 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, OnInit } from '@angular/core'; +import { + selectDropletIdFromRoute, + selectDroplets, + selectDropletState +} from '../../../state/droplets/droplets.selectors'; +import { + loadDroplets, + openDeleteDropletDialog, + openExportFlowVersionDialog, + openFlowVersionsDialog, + openImportNewFlowDialog, + openImportNewFlowVersionDialog, + selectDroplet +} from '../../../state/droplets/droplets.actions'; +import { Droplet } from '../../../state/droplets'; +import { Store } from '@ngrx/store'; +import { MatTableDataSource } from '@angular/material/table'; +import { Observable } from 'rxjs'; +import { Sort } from '@angular/material/sort'; +import { NiFiCommon, selectQueryParams } from '@nifi/shared'; +import { loadBuckets } from '../../../state/buckets/buckets.actions'; +import { Bucket } from '../../../state/buckets'; +import { selectBuckets } from '../../../state/buckets/buckets.selectors'; +import { DropletTableFilterContext } from './ui/droplet-table-filter/droplet-table-filter.component'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { ActivatedRoute, Router } from '@angular/router'; + +import { CommonModule } from '@angular/common'; +import { MatTableModule } from '@angular/material/table'; +import { MatSortModule } from '@angular/material/sort'; +import { MatMenuModule } from '@angular/material/menu'; +import { DropletTableFilterComponent } from './ui/droplet-table-filter/droplet-table-filter.component'; +import { MatButtonModule } from '@angular/material/button'; +import { DropletTableComponent } from './ui/droplet-table/droplet-table.component'; + +@Component({ + selector: 'explorer', + imports: [ + CommonModule, + MatTableModule, + MatSortModule, + MatMenuModule, + DropletTableFilterComponent, + MatButtonModule, + MatButtonModule, + DropletTableComponent + ], + templateUrl: './explorer.component.html', + styleUrl: './explorer.component.scss', + standalone: true +}) +export class ExplorerComponent implements OnInit { + droplets$: Observable<Droplet[]>; + buckets$: Observable<Bucket[]>; + buckets: Bucket[] = []; + dataSource: MatTableDataSource<Droplet> = new MatTableDataSource<Droplet>(); + displayedColumns: string[] = [ + 'name', + 'type', + 'bucketName', + 'bucketIdentifier', + 'identifier', + 'versions', + 'actions' + ]; + sort: Sort = { + active: 'name', + direction: 'asc' + }; + filterTerm = ''; + filterBucket = 'All'; + filterColumn = ''; + dropletsState$ = this.store.select(selectDropletState); + selectedDropletId$ = this.store.select(selectDropletIdFromRoute); + + constructor( + private store: Store, + private nifiCommon: NiFiCommon, + private router: Router, + private activatedRoute: ActivatedRoute + ) { + this.droplets$ = this.store.select(selectDroplets).pipe(takeUntilDestroyed()); + this.buckets$ = this.store.select(selectBuckets).pipe(takeUntilDestroyed()); + this.store + .select(selectQueryParams) + .pipe(takeUntilDestroyed()) + .subscribe((queryParams) => { + if (queryParams) { + this.filterTerm = queryParams['filterTerm'] || ''; + this.filterBucket = queryParams['filterBucket'] || 'All'; + this.filterColumn = queryParams['filterColumn'] || ''; + this.dataSource.filter = JSON.stringify(queryParams); + } + }); + } Review Comment: I have a few thoughts here: First, no other listing in nifi preserves the filters, sortings, etc via query params. I see a few issues currently with having these query params with back/forward navigation and row selection while keeping the query params in sync. Also the handling of erroneous values for query params, handling erroneous query params, etc. seems to be missing. Secondly, the routes in this new NiFi registry application explorer view do not behave the same way as the legacy UI and I think we need to honor the existing routes. Users from the existing NiFi Registry may have bookmarks etc and expect those to work and since NiFi Registry is part of the NiFi release I don't think we want to have breaking changes like this in a minor release. It is kind of like an api. Both of these thoughts lead me to request that you remove the query params from the url for filtering, sorting, etc. Just let the user land at the explorer perspective and let them apply a filter, sort, etc. Instead we can use deep links like the legacy UI. Now, if we look at the routes from legacy NiFi Registry legacy UI then `/explorer/grid-list` takes the user to the listing of all resources. `explorer/grid-list/buckets/<bucket-id>` takes the user to the listing of all resources in <bucket-id>. And `explorer/grid-list/buckets/<bucket-id>/flows/<resource-id>` takes the user to a single resource in the listing. I think for this effort we can support the same routes and deep links. `/explorer/grid-list` would route to the explorer.component and would take the user to the listing of all resources. `explorer/grid-list/buckets/<bucket-id>` would also route to the explorer.component but would set the `formcontrolname="filterBucket"` to the bucket-id which would then show only the resources in that bucket in the listing. Then for `explorer/grid-list/buckets/<bucket-id>/flows/<resource-id>` it would also route to the explorer.component, set the `formcontrolname="filterBucket"` to the bucket-id which would then show only the resources in that bucket in the listing, and would select the <resource-id> in the listing. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/service/buckets.service.ts: ########## @@ -0,0 +1,31 @@ +/* + * 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 } from 'rxjs'; +import { HttpClient } from '@angular/common/http'; + +@Injectable({ providedIn: 'root' }) +export class BucketsService { + private static readonly API: string = '../nifi-registry-api'; + + constructor(private httpClient: HttpClient) {} + + getBuckets(): Observable<any> { Review Comment: Can we do better than Observable<any> here? It would be better if we had some types defined. ########## nifi-frontend/src/main/frontend/apps/nifi-registry/src/app/ui/header/header.component.html: ########## @@ -0,0 +1,30 @@ +<!-- +~ 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. +--> + +<header class="mb-5 h-16 registry-header flex items-center"> + <div class="flex items-center ml-2 mr-2 logo-container relative rounded-full"> Review Comment: ```suggestion <div class="flex items-center ml-2 mr-2 logo-container relative rounded-md"> ``` -- 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]
