Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
scottyaslan merged PR #8607: URL: https://github.com/apache/nifi/pull/8607 -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
scottyaslan commented on code in PR #8607: URL: https://github.com/apache/nifi/pull/8607#discussion_r1559434165 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-progress-dialog/change-version-progress-dialog.html: ## @@ -0,0 +1,51 @@ + + +Change Flow Version + + + +@if (flowUpdateRequest$ | async; as versionChangeRequest) { + +@if (versionChangeRequest.request.complete) { +This Process Group version has changed. +} @else { +{{ versionChangeRequest.request.state }} +} + + + Review Comment: actually this was a mistake. I thought it was applying the value class. Disregard. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
rfellows commented on code in PR #8607: URL: https://github.com/apache/nifi/pull/8607#discussion_r1559396797 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-dialog/change-version-dialog.html: ## @@ -0,0 +1,89 @@ + + +Change Version + + + + +Registry +{{ versionControlInformation.registryName }} Review Comment: Good call, how's this? https://github.com/apache/nifi/assets/713866/6eb46d1a-c7f5-427e-a9ac-0c8db2872b1a";> -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
rfellows commented on code in PR #8607: URL: https://github.com/apache/nifi/pull/8607#discussion_r1559374929 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-progress-dialog/change-version-progress-dialog.html: ## @@ -0,0 +1,51 @@ + + +Change Flow Version + + + +@if (flowUpdateRequest$ | async; as versionChangeRequest) { + +@if (versionChangeRequest.request.complete) { +This Process Group version has changed. +} @else { +{{ versionChangeRequest.request.state }} +} + + + Review Comment: I'll assume you meant this to be on the line below where we display the actual percentage value as text below the progress bar. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
rfellows commented on code in PR #8607: URL: https://github.com/apache/nifi/pull/8607#discussion_r1559370570 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-progress-dialog/change-version-progress-dialog.html: ## @@ -0,0 +1,51 @@ + + +Change Flow Version + + + +@if (flowUpdateRequest$ | async; as versionChangeRequest) { + +@if (versionChangeRequest.request.complete) { +This Process Group version has changed. +} @else { +{{ versionChangeRequest.request.state }} +} + + + Review Comment: I don't understand this suggestion. the `[value]` here is an input into the component to it knows the percent complete to display. It isn't a class/style. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
rfellows commented on code in PR #8607: URL: https://github.com/apache/nifi/pull/8607#discussion_r1559348402 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/local-changes-dialog/local-changes-table/local-changes-table.html: ## @@ -0,0 +1,80 @@ + + + + +Displaying {{ filteredCount }} of {{ totalCount }} + + + + +Filter + Review Comment: Yeah, it is on the left everywhere else. I'd say we leave it alone. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
scottyaslan commented on code in PR #8607: URL: https://github.com/apache/nifi/pull/8607#discussion_r1558341192 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-dialog/change-version-dialog.html: ## @@ -0,0 +1,89 @@ + + +Change Version + + + + +Registry +{{ versionControlInformation.registryName }} Review Comment: There is a lot of white space on the right hand side of this dialog... maybe we could move the flow name and current version up to balance it out? -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
scottyaslan commented on code in PR #8607: URL: https://github.com/apache/nifi/pull/8607#discussion_r1558337332 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-progress-dialog/change-version-progress-dialog.html: ## @@ -0,0 +1,51 @@ + + +Change Flow Version + + + +@if (flowUpdateRequest$ | async; as versionChangeRequest) { + +@if (versionChangeRequest.request.complete) { +This Process Group version has changed. +} @else { +{{ versionChangeRequest.request.state }} +} + + + Review Comment: ```suggestion [mat-accent]="versionChangeRequest.request.percentCompleted" [font-medium]="versionChangeRequest.request.percentCompleted"> ``` ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-dialog/change-version-dialog.html: ## @@ -0,0 +1,89 @@ + + +Change Version + + + + +Registry +{{ versionControlInformation.registryName }} Review Comment: Since you merged my semantic color PR you need to update this to be: ```suggestion {{ versionControlInformation.registryName }} ``` ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/local-changes-dialog/local-changes-table/local-changes-table.html: ## @@ -0,0 +1,80 @@ + + + + +Displaying {{ filteredCount }} of {{ totalCount }} + + + + +Filter + Review Comment: Of course as I typed this I see that the summary tables have the filter on the left so it is probably fine. ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-progress-dialog/change-version-progress-dialog.html: ## @@ -0,0 +1,51 @@ + + +Change Flow Version + + + +@if (flowUpdateRequest$ | async; as versionChangeRequest) { + Review Comment: ```suggestion ``` -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
scottyaslan commented on code in PR #8607: URL: https://github.com/apache/nifi/pull/8607#discussion_r1558332295 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/local-changes-dialog/local-changes-table/local-changes-table.html: ## @@ -0,0 +1,80 @@ + + + + +Displaying {{ filteredCount }} of {{ totalCount }} + + + + +Filter + Review Comment: I know that in current nifi this input field is also located on the left but I think it would be more consistent with the other use cases like this where users can filter a table is to have the filter on the top right. What do you think? -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
scottyaslan commented on code in PR #8607: URL: https://github.com/apache/nifi/pull/8607#discussion_r1558329228 ## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/local-changes-dialog/local-changes-table/local-changes-table.ts: ## @@ -0,0 +1,174 @@ +/* + * 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, DestroyRef, EventEmitter, inject, Input, Output } from '@angular/core'; +import { MatFormField, MatLabel } from '@angular/material/form-field'; +import { MatInput } from '@angular/material/input'; +import { FormBuilder, FormGroup, ReactiveFormsModule } from '@angular/forms'; +import { MatTableDataSource, MatTableModule } from '@angular/material/table'; +import { MatSortModule, Sort } from '@angular/material/sort'; +import { NiFiCommon } from '../../../../../../../../service/nifi-common.service'; +import { ComponentDifference, NavigateToComponentRequest } from '../../../../../../state/flow'; +import { debounceTime } from 'rxjs'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { ComponentType } from '../../../../../../../../state/shared'; + +interface LocalChange { +componentType: ComponentType; +componentId: string; +componentName: string; +processGroupId: string; +differenceType: string; +difference: string; +} + +@Component({ +selector: 'local-changes-table', +standalone: true, +imports: [MatFormField, MatInput, MatLabel, ReactiveFormsModule, MatTableModule, MatSortModule], +templateUrl: './local-changes-table.html', +styleUrl: './local-changes-table.scss' +}) +export class LocalChangesTable implements AfterViewInit { +private destroyRef: DestroyRef = inject(DestroyRef); +initialSortColumn: 'componentName' | 'changeType' | 'difference' = 'componentName'; +initialSortDirection: 'asc' | 'desc' = 'asc'; + +filterTerm = ''; +totalCount = 0; +filteredCount = 0; + +activeSort: Sort = { +active: this.initialSortColumn, +direction: this.initialSortDirection +}; + +displayedColumns: string[] = ['componentName', 'changeType', 'difference', 'actions']; +dataSource: MatTableDataSource = new MatTableDataSource(); +filterForm: FormGroup; + +@Input() set differences(differences: ComponentDifference[]) { +const localChanges: LocalChange[] = this.explodeDifferences(differences); +this.dataSource.data = this.sortEntities(localChanges, this.activeSort); +this.dataSource.filterPredicate = (data: LocalChange, filter: string) => { +const { filterTerm } = JSON.parse(filter); +// check the filter term in both the name and type columns +return ( +this.nifiCommon.stringContains(data.componentName, filterTerm, true) || +this.nifiCommon.stringContains(data.differenceType, filterTerm, true) +); +}; +this.totalCount = localChanges.length; +this.filteredCount = localChanges.length; + +// apply any filtering to the new data +const filterTerm = this.filterForm.get('filterTerm')?.value; +if (filterTerm?.length > 0) { +this.applyFilter(filterTerm); +} +} + +@Output() goToChange: EventEmitter = new EventEmitter(); + +constructor( +private formBuilder: FormBuilder, +private nifiCommon: NiFiCommon +) { +this.filterForm = this.formBuilder.group({ filterTerm: '', filterColumn: 'componentName' }); +} + +ngAfterViewInit(): void { +this.filterForm +.get('filterTerm') +?.valueChanges.pipe(debounceTime(500), takeUntilDestroyed(this.destroyRef)) +.subscribe((filterTerm: string) => { +this.applyFilter(filterTerm); +}); +} + +applyFilter(filterTerm: string) { +this.dataSource.filter = JSON.stringify({ filterTerm }); +this.filteredCount = this.dataSource.filteredData.length; +} + +formatComponentName(item: LocalChange): string { +return item.componentName; Review Comment: For a fun
Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]
scottyaslan commented on PR #8607: URL: https://github.com/apache/nifi/pull/8607#issuecomment-2046084942 reviewing... -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org