Re: [PR] [NIFI-12995]- Change Flow Version, Show Local Changes, Revert local changes [nifi]

2024-04-10 Thread via GitHub


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]

2024-04-10 Thread via GitHub


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]

2024-04-10 Thread via GitHub


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]

2024-04-10 Thread via GitHub


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]

2024-04-10 Thread via GitHub


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]

2024-04-10 Thread via GitHub


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]

2024-04-09 Thread via GitHub


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]

2024-04-09 Thread via GitHub


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]

2024-04-09 Thread via GitHub


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]

2024-04-09 Thread via GitHub


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]

2024-04-09 Thread via GitHub


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