scottyaslan commented on code in PR #10473:
URL: https://github.com/apache/nifi/pull/10473#discussion_r2478349220
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-dialog/change-version-dialog.ts:
##########
@@ -47,16 +49,18 @@ export class ChangeVersionDialog extends
CloseOnEscapeDialog {
private nifiCommon = inject(NiFiCommon);
private store = inject<Store<CanvasState>>(Store);
- displayedColumns: string[] = ['current', 'version', 'created', 'comments'];
+ displayedColumns: string[] = ['actions', 'current', 'version', 'created',
'comments'];
Review Comment:
The actions column should be displayed as the last column in the table
listing.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.scss:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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.
+ */
+
+@use '@angular/material' as mat;
+
+.flow-diff {
+ @include mat.button-density(-1);
+
+ .flow-diff-message {
+ color: var(--mat-sys-on-surface-variant);
+
+ .flow-diff-message-title {
+ font-weight: 600;
+ margin-bottom: 4px;
+ }
+
+ .flow-diff-message-list {
+ margin: 0;
+ padding-left: 16px;
+ list-style: disc;
+
+ li {
+ margin-bottom: 2px;
+
+ .flow-diff-version {
+ font-weight: 500;
+ }
+
+ .flow-diff-created {
+ margin-left: 4px;
+ color: var(--mat-sys-on-surface-variant);
+ }
+ }
+ }
Review Comment:
Again, all typography should be handled by the design system so these should
not be needed. Similarly the li styles should already be set by the design
system. If we truly need the margins then please update the template to use
tailwind declarative styles rather than setting them here in the SASS partial.
The reason is that all of the spacing is configurable and handled by tailwiind.
This is really powerful as it allows us to update the tailwind config in a
single place and the new spacing will be applied application wide.
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.html:
##########
@@ -0,0 +1,123 @@
+<!--
+ ~ 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>
+ Flow Version Diff - {{ flowName }}
+</h2>
+<div class="flow-diff">
+ <mat-dialog-content>
+ <div class="dialog-content flex flex-col gap-y-4">
+ <div
+ class="flow-diff-message"
+ data-qa="flow-diff-message"
+ *ngIf="comparisonSummary.length > 0">
+ <div class="flow-diff-message-title">Comparing versions:</div>
+ <ul class="flow-diff-message-list">
Review Comment:
An unordered list is not the correct way to display this information. Please
follow the label/value UX used throughout the application to display values.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.html:
##########
@@ -0,0 +1,123 @@
+<!--
+ ~ 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>
+ Flow Version Diff - {{ flowName }}
+</h2>
+<div class="flow-diff">
+ <mat-dialog-content>
+ <div class="dialog-content flex flex-col gap-y-4">
+ <div
+ class="flow-diff-message"
+ data-qa="flow-diff-message"
+ *ngIf="comparisonSummary.length > 0">
+ <div class="flow-diff-message-title">Comparing versions:</div>
+ <ul class="flow-diff-message-list">
+ <li *ngFor="let summary of comparisonSummary">
+ <span class="flow-diff-version">Version {{
summary.version }}</span>
+ <span *ngIf="summary.created"
class="flow-diff-created">
+ ({{ summary.created }})
+ </span>
+ </li>
+ </ul>
+ </div>
+
+ <div class="flow-diff-controls flex flex-wrap gap-4">
+ <mat-form-field class="flow-diff-select" appearance="outline">
+ <mat-label>Current Version</mat-label>
+ <mat-select [formControl]="currentVersionControl"
data-qa="current-version-select">
+ <mat-option *ngFor="let version of versionOptions"
[value]="version">
+ {{ formatVersionOption(version) }}
+ </mat-option>
+ </mat-select>
+ </mat-form-field>
+
+ <mat-form-field class="flow-diff-select" appearance="outline">
+ <mat-label>Selected Version</mat-label>
+ <mat-select [formControl]="selectedVersionControl"
data-qa="selected-version-select">
+ <mat-option *ngFor="let version of versionOptions"
[value]="version">
+ {{ formatVersionOption(version) }}
+ </mat-option>
+ </mat-select>
+ </mat-form-field>
+
+ <mat-form-field class="flow-diff-filter" appearance="outline">
+ <mat-label>Filter</mat-label>
+ <input
+ matInput
+ type="text"
+ [formControl]="filterControl"
+ placeholder="Filter differences"
+ data-qa="flow-diff-filter" />
+ </mat-form-field>
+ </div>
+
+ <div class="flow-diff-table flex-1">
+ <div class="flow-diff-loading" *ngIf="isLoading">
+ <mat-progress-spinner diameter="36"
mode="indeterminate"></mat-progress-spinner>
+ </div>
Review Comment:
The listings in nifi do not show mat-progress-spinner or any other loading
UX.
The listings in n
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-dialog/change-version-dialog.scss:
##########
@@ -22,6 +22,10 @@
.listing-table {
table {
+ .mat-column-actions {
+ width: 54px;
+ }
+
Review Comment:
actions column is already set for all listings so we don't need this here
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/change-version-dialog/change-version-dialog.html:
##########
@@ -78,6 +78,24 @@ <h2 mat-dialog-title>
(matSortChange)="sortData($event)"
[matSortActive]="sort.active"
[matSortDirection]="sort.direction">
+ <!-- Actions Column -->
Review Comment:
The actions column should be displayed as the last column in the table
listing.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.html:
##########
@@ -0,0 +1,123 @@
+<!--
+ ~ 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>
+ Flow Version Diff - {{ flowName }}
+</h2>
+<div class="flow-diff">
+ <mat-dialog-content>
+ <div class="dialog-content flex flex-col gap-y-4">
+ <div
+ class="flow-diff-message"
+ data-qa="flow-diff-message"
+ *ngIf="comparisonSummary.length > 0">
+ <div class="flow-diff-message-title">Comparing versions:</div>
+ <ul class="flow-diff-message-list">
+ <li *ngFor="let summary of comparisonSummary">
+ <span class="flow-diff-version">Version {{
summary.version }}</span>
+ <span *ngIf="summary.created"
class="flow-diff-created">
+ ({{ summary.created }})
+ </span>
+ </li>
+ </ul>
+ </div>
+
+ <div class="flow-diff-controls flex flex-wrap gap-4">
+ <mat-form-field class="flow-diff-select" appearance="outline">
+ <mat-label>Current Version</mat-label>
+ <mat-select [formControl]="currentVersionControl"
data-qa="current-version-select">
+ <mat-option *ngFor="let version of versionOptions"
[value]="version">
+ {{ formatVersionOption(version) }}
+ </mat-option>
+ </mat-select>
+ </mat-form-field>
+
+ <mat-form-field class="flow-diff-select" appearance="outline">
+ <mat-label>Selected Version</mat-label>
+ <mat-select [formControl]="selectedVersionControl"
data-qa="selected-version-select">
+ <mat-option *ngFor="let version of versionOptions"
[value]="version">
+ {{ formatVersionOption(version) }}
+ </mat-option>
+ </mat-select>
+ </mat-form-field>
+
+ <mat-form-field class="flow-diff-filter" appearance="outline">
+ <mat-label>Filter</mat-label>
+ <input
+ matInput
+ type="text"
+ [formControl]="filterControl"
+ placeholder="Filter differences"
+ data-qa="flow-diff-filter" />
+ </mat-form-field>
+ </div>
+
+ <div class="flow-diff-table flex-1">
+ <div class="flow-diff-loading" *ngIf="isLoading">
+ <mat-progress-spinner diameter="36"
mode="indeterminate"></mat-progress-spinner>
+ </div>
+
+ <div *ngIf="!isLoading">
+ <div *ngIf="errorMessage" class="flow-diff-error"
data-qa="flow-diff-error">
+ {{ errorMessage }}
+ </div>
+
+ <ng-container *ngIf="!errorMessage">
+ <div *ngIf="noDifferences" class="flow-diff-empty"
data-qa="flow-diff-empty">
+ No differences to display.
+ </div>
+
+ <table
Review Comment:
Ths styles for this table do not match the style for other listings
throughout the app.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.scss:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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.
+ */
+
+@use '@angular/material' as mat;
+
+.flow-diff {
+ @include mat.button-density(-1);
+
+ .flow-diff-message {
+ color: var(--mat-sys-on-surface-variant);
Review Comment:
I am curious as to why this is needed? Typography should already be properly
styled.
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.scss:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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.
+ */
+
+@use '@angular/material' as mat;
+
+.flow-diff {
+ @include mat.button-density(-1);
+
+ .flow-diff-message {
+ color: var(--mat-sys-on-surface-variant);
+
+ .flow-diff-message-title {
+ font-weight: 600;
+ margin-bottom: 4px;
+ }
Review Comment:
These styles are set as part of the design system and should not be needed
here.
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.scss:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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.
+ */
+
+@use '@angular/material' as mat;
+
+.flow-diff {
+ @include mat.button-density(-1);
+
+ .flow-diff-message {
+ color: var(--mat-sys-on-surface-variant);
+
+ .flow-diff-message-title {
+ font-weight: 600;
+ margin-bottom: 4px;
+ }
+
+ .flow-diff-message-list {
+ margin: 0;
+ padding-left: 16px;
+ list-style: disc;
+
+ li {
+ margin-bottom: 2px;
+
+ .flow-diff-version {
+ font-weight: 500;
+ }
+
+ .flow-diff-created {
+ margin-left: 4px;
+ color: var(--mat-sys-on-surface-variant);
+ }
+ }
+ }
+ }
+
+ .flow-diff-controls {
+ .mat-mdc-form-field {
+ min-width: 200px;
+ }
+
+ .flow-diff-filter {
+ flex: 1 1 240px;
+ }
+ }
+
+ .flow-diff-table {
+ min-height: 240px;
+
+ table {
+ width: 100%;
+
+ .mat-column-componentName,
+ .mat-column-changeType {
+ width: 200px;
+ }
+
+ .mat-column-difference {
+ width: auto;
+ }
+ }
+
+ .flow-diff-loading {
+ display: flex;
+ align-items: center;
+ justify-content: center;
+ min-height: 200px;
+ }
+
+ .flow-diff-empty,
+ .flow-diff-error {
+ padding: 12px 0;
+ color: var(--mat-sys-on-surface-variant);
+ }
+
+ .flow-diff-error {
+ color: var(--mat-sys-error);
+ }
+ }
Review Comment:
Please use tailwind declarative styles to apply these rules in the template
if they are actually needed but I don't think they are.
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.ts:
##########
@@ -0,0 +1,298 @@
+/*
+ * 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,
+ ViewChild,
+ inject
+} from '@angular/core';
+import { MAT_DIALOG_DATA, MatDialogModule } from '@angular/material/dialog';
+import { MatTableDataSource, MatTableModule } from '@angular/material/table';
+import { MatSort, MatSortModule } from '@angular/material/sort';
+import { FormBuilder, FormControl, FormGroup, ReactiveFormsModule } from
'@angular/forms';
+import { CommonModule } from '@angular/common';
+import { MatFormField, MatLabel } from '@angular/material/form-field';
+import { MatSelectModule } from '@angular/material/select';
+import { MatOption } from '@angular/material/core';
+import { MatInput } from '@angular/material/input';
+import { MatButton } from '@angular/material/button';
+import { MatProgressSpinner } from '@angular/material/progress-spinner';
+import { combineLatest, of } from 'rxjs';
+import { catchError, debounceTime, distinctUntilChanged, filter, map,
startWith, switchMap, take } from 'rxjs/operators';
+import { FlowComparisonEntity, VersionControlInformation } from
'../../../../../state/flow';
+import { VersionedFlowSnapshotMetadata } from
'../../../../../../../state/shared';
+import { RegistryService } from '../../../../../service/registry.service';
+import { CloseOnEscapeDialog, NiFiCommon } from '@nifi/shared';
+import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
+import { Store } from '@ngrx/store';
+import { CanvasState } from '../../../../../state';
+import { selectTimeOffset } from
'../../../../../../../state/flow-configuration/flow-configuration.selectors';
+
+export interface FlowDiffDialogData {
+ versionControlInformation: VersionControlInformation;
+ versions: VersionedFlowSnapshotMetadata[];
+ currentVersion: string;
+ selectedVersion: string;
+}
+
+interface FlowDiffRow {
+ componentName: string;
+ changeType: string;
+ difference: string;
+}
+
+@Component({
+ selector: 'flow-diff-dialog',
+ standalone: true,
+ imports: [
+ CommonModule,
+ MatDialogModule,
+ MatTableModule,
+ MatSortModule,
+ ReactiveFormsModule,
+ MatFormField,
+ MatLabel,
+ MatSelectModule,
+ MatOption,
+ MatInput,
+ MatButton,
+ MatProgressSpinner
+ ],
+ templateUrl: './flow-diff-dialog.html',
+ styleUrl: './flow-diff-dialog.scss'
+})
+export class FlowDiffDialog extends CloseOnEscapeDialog implements
AfterViewInit {
+ private data = inject<FlowDiffDialogData>(MAT_DIALOG_DATA);
+ private registryService = inject(RegistryService);
+ private destroyRef = inject(DestroyRef);
+ private formBuilder = inject(FormBuilder);
+ private nifiCommon = inject(NiFiCommon);
+ private store = inject<Store<CanvasState>>(Store);
+ private timeOffset = this.store.selectSignal(selectTimeOffset);
+
+ @ViewChild(MatSort)
+ set matSort(sort: MatSort | undefined) {
+ if (sort) {
+ this.dataSource.sort = sort;
+ }
+ }
+
+ displayedColumns: string[] = ['componentName', 'changeType', 'difference'];
+ dataSource: MatTableDataSource<FlowDiffRow> = new
MatTableDataSource<FlowDiffRow>();
+ comparisonForm: FormGroup;
+ filterControl: FormControl<string> = new FormControl<string>('', {
nonNullable: true });
+ currentVersionControl: FormControl<string>;
+ selectedVersionControl: FormControl<string>;
+
+ versionOptions: string[];
+ flowName: string;
+ comparisonSummary: { version: string; created?: string }[] = [];
+ isLoading = false;
+ errorMessage: string | null = null;
+ noDifferences = false;
+ private versionMetadataByVersion: Map<string,
VersionedFlowSnapshotMetadata> = new Map();
+
+ constructor() {
+ super();
+ const versions = this.sortVersions(this.data.versions);
+ this.versionOptions = versions.map((version) => version.version);
+ this.versionMetadataByVersion = new Map(versions.map((metadata) =>
[metadata.version, metadata]));
+ const vci = this.data.versionControlInformation;
+ this.flowName = vci.flowName || vci.flowId;
+
+ this.currentVersionControl = new
FormControl<string>(this.data.currentVersion, { nonNullable: true });
+ this.selectedVersionControl = new
FormControl<string>(this.data.selectedVersion, { nonNullable: true });
+ this.comparisonForm = this.formBuilder.group({
+ currentVersion: this.currentVersionControl,
+ selectedVersion: this.selectedVersionControl
+ });
+
+ this.dataSource.filterPredicate = (row: FlowDiffRow, filterTerm:
string) => {
+ if (!filterTerm) {
+ return true;
+ }
+
+ const normalizedFilter = filterTerm.toLowerCase();
+ return (
+ (row.componentName ||
'').toLowerCase().includes(normalizedFilter) ||
+ (row.changeType ||
'').toLowerCase().includes(normalizedFilter) ||
+ (row.difference || '').toLowerCase().includes(normalizedFilter)
+ );
+ };
+
+ this.dataSource.sortingDataAccessor = (row: FlowDiffRow, property:
string) => {
+ switch (property) {
+ case 'componentName':
+ return (row.componentName || '').toLowerCase();
+ case 'changeType':
+ return (row.changeType || '').toLowerCase();
+ case 'difference':
+ return (row.difference || '').toLowerCase();
+ default:
+ return '';
+ }
+ };
+
+ this.configureFiltering();
+ this.configureComparisonChanges();
+ this.setComparisonSummary(this.data.currentVersion,
this.data.selectedVersion);
+ }
+
+ formatVersionOption(version: string): string {
+ const metadata = this.versionMetadataByVersion.get(version);
+ const formattedVersion =
+ version.length > 5 ? `${version.substring(0, 5)}...` : version;
+ const created = metadata ? this.formatTimestamp(metadata) : undefined;
+ return created ? `${formattedVersion} (${created})` : formattedVersion;
+ }
+
+ ngAfterViewInit(): void {
+ // Sorting handled automatically when MatSort is available via the
ViewChild setter
Review Comment:
I haven't really dug in here yet but it seems sorting is handled differently
for this listing then other listings throughout nifi. I am not saying this is
incorrect but it is a different pattern so at the very least it would need some
testing and verification. It would be nice it we could just update this to
follow the pattern used in other listings. We should not need to use a
ViewChild to sort an angular material table.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.scss:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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.
+ */
+
+@use '@angular/material' as mat;
+
+.flow-diff {
+ @include mat.button-density(-1);
+
+ .flow-diff-message {
+ color: var(--mat-sys-on-surface-variant);
+
+ .flow-diff-message-title {
+ font-weight: 600;
+ margin-bottom: 4px;
+ }
+
+ .flow-diff-message-list {
+ margin: 0;
+ padding-left: 16px;
+ list-style: disc;
+
+ li {
+ margin-bottom: 2px;
+
+ .flow-diff-version {
+ font-weight: 500;
+ }
+
+ .flow-diff-created {
+ margin-left: 4px;
+ color: var(--mat-sys-on-surface-variant);
+ }
+ }
+ }
+ }
+
+ .flow-diff-controls {
+ .mat-mdc-form-field {
+ min-width: 200px;
+ }
+
+ .flow-diff-filter {
+ flex: 1 1 240px;
+ }
Review Comment:
Please use tailwind declaritive styles to apply these rules in the template.
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.html:
##########
@@ -0,0 +1,123 @@
+<!--
+ ~ 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>
+ Flow Version Diff - {{ flowName }}
+</h2>
+<div class="flow-diff">
+ <mat-dialog-content>
+ <div class="dialog-content flex flex-col gap-y-4">
+ <div
+ class="flow-diff-message"
+ data-qa="flow-diff-message"
+ *ngIf="comparisonSummary.length > 0">
+ <div class="flow-diff-message-title">Comparing versions:</div>
+ <ul class="flow-diff-message-list">
+ <li *ngFor="let summary of comparisonSummary">
+ <span class="flow-diff-version">Version {{
summary.version }}</span>
+ <span *ngIf="summary.created"
class="flow-diff-created">
+ ({{ summary.created }})
+ </span>
+ </li>
+ </ul>
+ </div>
+
+ <div class="flow-diff-controls flex flex-wrap gap-4">
+ <mat-form-field class="flow-diff-select" appearance="outline">
+ <mat-label>Current Version</mat-label>
+ <mat-select [formControl]="currentVersionControl"
data-qa="current-version-select">
+ <mat-option *ngFor="let version of versionOptions"
[value]="version">
+ {{ formatVersionOption(version) }}
+ </mat-option>
+ </mat-select>
+ </mat-form-field>
+
+ <mat-form-field class="flow-diff-select" appearance="outline">
+ <mat-label>Selected Version</mat-label>
+ <mat-select [formControl]="selectedVersionControl"
data-qa="selected-version-select">
+ <mat-option *ngFor="let version of versionOptions"
[value]="version">
+ {{ formatVersionOption(version) }}
+ </mat-option>
+ </mat-select>
+ </mat-form-field>
+
+ <mat-form-field class="flow-diff-filter" appearance="outline">
+ <mat-label>Filter</mat-label>
+ <input
+ matInput
+ type="text"
+ [formControl]="filterControl"
+ placeholder="Filter differences"
+ data-qa="flow-diff-filter" />
+ </mat-form-field>
+ </div>
+
+ <div class="flow-diff-table flex-1">
+ <div class="flow-diff-loading" *ngIf="isLoading">
+ <mat-progress-spinner diameter="36"
mode="indeterminate"></mat-progress-spinner>
+ </div>
+
+ <div *ngIf="!isLoading">
+ <div *ngIf="errorMessage" class="flow-diff-error"
data-qa="flow-diff-error">
+ {{ errorMessage }}
+ </div>
Review Comment:
The listings in nifi do not show errors like this. If there is an error when
requesting the diff it should be displayed in a context error banner at the top
of the dialog like we do in other cases.
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.scss:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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.
+ */
+
+@use '@angular/material' as mat;
+
+.flow-diff {
+ @include mat.button-density(-1);
+
+ .flow-diff-message {
+ color: var(--mat-sys-on-surface-variant);
+
+ .flow-diff-message-title {
+ font-weight: 600;
+ margin-bottom: 4px;
+ }
+
+ .flow-diff-message-list {
+ margin: 0;
+ padding-left: 16px;
+ list-style: disc;
+
+ li {
+ margin-bottom: 2px;
+
+ .flow-diff-version {
+ font-weight: 500;
+ }
+
+ .flow-diff-created {
+ margin-left: 4px;
+ color: var(--mat-sys-on-surface-variant);
+ }
+ }
+ }
+ }
+
+ .flow-diff-controls {
+ .mat-mdc-form-field {
+ min-width: 200px;
+ }
Review Comment:
This does not seem correct. Form fields should all be correctly styles by
the design system and should not need width set.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.html:
##########
@@ -0,0 +1,123 @@
+<!--
+ ~ 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>
+ Flow Version Diff - {{ flowName }}
+</h2>
+<div class="flow-diff">
+ <mat-dialog-content>
+ <div class="dialog-content flex flex-col gap-y-4">
+ <div
+ class="flow-diff-message"
+ data-qa="flow-diff-message"
+ *ngIf="comparisonSummary.length > 0">
+ <div class="flow-diff-message-title">Comparing versions:</div>
+ <ul class="flow-diff-message-list">
+ <li *ngFor="let summary of comparisonSummary">
+ <span class="flow-diff-version">Version {{
summary.version }}</span>
+ <span *ngIf="summary.created"
class="flow-diff-created">
+ ({{ summary.created }})
+ </span>
+ </li>
+ </ul>
+ </div>
+
+ <div class="flow-diff-controls flex flex-wrap gap-4">
+ <mat-form-field class="flow-diff-select" appearance="outline">
+ <mat-label>Current Version</mat-label>
+ <mat-select [formControl]="currentVersionControl"
data-qa="current-version-select">
+ <mat-option *ngFor="let version of versionOptions"
[value]="version">
+ {{ formatVersionOption(version) }}
+ </mat-option>
+ </mat-select>
+ </mat-form-field>
+
+ <mat-form-field class="flow-diff-select" appearance="outline">
+ <mat-label>Selected Version</mat-label>
+ <mat-select [formControl]="selectedVersionControl"
data-qa="selected-version-select">
+ <mat-option *ngFor="let version of versionOptions"
[value]="version">
+ {{ formatVersionOption(version) }}
+ </mat-option>
+ </mat-select>
+ </mat-form-field>
+
+ <mat-form-field class="flow-diff-filter" appearance="outline">
+ <mat-label>Filter</mat-label>
+ <input
+ matInput
+ type="text"
+ [formControl]="filterControl"
+ placeholder="Filter differences"
+ data-qa="flow-diff-filter" />
+ </mat-form-field>
+ </div>
+
+ <div class="flow-diff-table flex-1">
+ <div class="flow-diff-loading" *ngIf="isLoading">
+ <mat-progress-spinner diameter="36"
mode="indeterminate"></mat-progress-spinner>
+ </div>
+
+ <div *ngIf="!isLoading">
+ <div *ngIf="errorMessage" class="flow-diff-error"
data-qa="flow-diff-error">
+ {{ errorMessage }}
+ </div>
+
+ <ng-container *ngIf="!errorMessage">
+ <div *ngIf="noDifferences" class="flow-diff-empty"
data-qa="flow-diff-empty">
+ No differences to display.
+ </div>
+
+ <table
+ mat-table
+ *ngIf="dataSource.data.length > 0"
Review Comment:
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.ts:
##########
@@ -0,0 +1,298 @@
+/*
+ * 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,
+ ViewChild,
+ inject
+} from '@angular/core';
+import { MAT_DIALOG_DATA, MatDialogModule } from '@angular/material/dialog';
+import { MatTableDataSource, MatTableModule } from '@angular/material/table';
+import { MatSort, MatSortModule } from '@angular/material/sort';
+import { FormBuilder, FormControl, FormGroup, ReactiveFormsModule } from
'@angular/forms';
+import { CommonModule } from '@angular/common';
+import { MatFormField, MatLabel } from '@angular/material/form-field';
+import { MatSelectModule } from '@angular/material/select';
+import { MatOption } from '@angular/material/core';
+import { MatInput } from '@angular/material/input';
+import { MatButton } from '@angular/material/button';
+import { MatProgressSpinner } from '@angular/material/progress-spinner';
+import { combineLatest, of } from 'rxjs';
+import { catchError, debounceTime, distinctUntilChanged, filter, map,
startWith, switchMap, take } from 'rxjs/operators';
+import { FlowComparisonEntity, VersionControlInformation } from
'../../../../../state/flow';
+import { VersionedFlowSnapshotMetadata } from
'../../../../../../../state/shared';
+import { RegistryService } from '../../../../../service/registry.service';
+import { CloseOnEscapeDialog, NiFiCommon } from '@nifi/shared';
+import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
+import { Store } from '@ngrx/store';
+import { CanvasState } from '../../../../../state';
+import { selectTimeOffset } from
'../../../../../../../state/flow-configuration/flow-configuration.selectors';
+
+export interface FlowDiffDialogData {
+ versionControlInformation: VersionControlInformation;
+ versions: VersionedFlowSnapshotMetadata[];
+ currentVersion: string;
+ selectedVersion: string;
+}
+
+interface FlowDiffRow {
+ componentName: string;
+ changeType: string;
+ difference: string;
+}
+
+@Component({
+ selector: 'flow-diff-dialog',
+ standalone: true,
+ imports: [
+ CommonModule,
+ MatDialogModule,
+ MatTableModule,
+ MatSortModule,
+ ReactiveFormsModule,
+ MatFormField,
+ MatLabel,
+ MatSelectModule,
+ MatOption,
+ MatInput,
+ MatButton,
+ MatProgressSpinner
+ ],
+ templateUrl: './flow-diff-dialog.html',
+ styleUrl: './flow-diff-dialog.scss'
+})
+export class FlowDiffDialog extends CloseOnEscapeDialog implements
AfterViewInit {
+ private data = inject<FlowDiffDialogData>(MAT_DIALOG_DATA);
+ private registryService = inject(RegistryService);
+ private destroyRef = inject(DestroyRef);
+ private formBuilder = inject(FormBuilder);
+ private nifiCommon = inject(NiFiCommon);
+ private store = inject<Store<CanvasState>>(Store);
+ private timeOffset = this.store.selectSignal(selectTimeOffset);
+
+ @ViewChild(MatSort)
Review Comment:
You can follow how the change version dialog implemented sorting instead of
using a ViewChild here.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.ts:
##########
@@ -0,0 +1,298 @@
+/*
+ * 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,
+ ViewChild,
+ inject
+} from '@angular/core';
+import { MAT_DIALOG_DATA, MatDialogModule } from '@angular/material/dialog';
+import { MatTableDataSource, MatTableModule } from '@angular/material/table';
+import { MatSort, MatSortModule } from '@angular/material/sort';
+import { FormBuilder, FormControl, FormGroup, ReactiveFormsModule } from
'@angular/forms';
+import { CommonModule } from '@angular/common';
+import { MatFormField, MatLabel } from '@angular/material/form-field';
+import { MatSelectModule } from '@angular/material/select';
+import { MatOption } from '@angular/material/core';
+import { MatInput } from '@angular/material/input';
+import { MatButton } from '@angular/material/button';
+import { MatProgressSpinner } from '@angular/material/progress-spinner';
+import { combineLatest, of } from 'rxjs';
+import { catchError, debounceTime, distinctUntilChanged, filter, map,
startWith, switchMap, take } from 'rxjs/operators';
+import { FlowComparisonEntity, VersionControlInformation } from
'../../../../../state/flow';
+import { VersionedFlowSnapshotMetadata } from
'../../../../../../../state/shared';
+import { RegistryService } from '../../../../../service/registry.service';
+import { CloseOnEscapeDialog, NiFiCommon } from '@nifi/shared';
+import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
+import { Store } from '@ngrx/store';
+import { CanvasState } from '../../../../../state';
+import { selectTimeOffset } from
'../../../../../../../state/flow-configuration/flow-configuration.selectors';
+
+export interface FlowDiffDialogData {
+ versionControlInformation: VersionControlInformation;
+ versions: VersionedFlowSnapshotMetadata[];
+ currentVersion: string;
+ selectedVersion: string;
+}
+
+interface FlowDiffRow {
+ componentName: string;
+ changeType: string;
+ difference: string;
+}
+
+@Component({
+ selector: 'flow-diff-dialog',
+ standalone: true,
+ imports: [
+ CommonModule,
+ MatDialogModule,
+ MatTableModule,
+ MatSortModule,
+ ReactiveFormsModule,
+ MatFormField,
+ MatLabel,
+ MatSelectModule,
+ MatOption,
+ MatInput,
+ MatButton,
+ MatProgressSpinner
+ ],
+ templateUrl: './flow-diff-dialog.html',
+ styleUrl: './flow-diff-dialog.scss'
+})
+export class FlowDiffDialog extends CloseOnEscapeDialog implements
AfterViewInit {
+ private data = inject<FlowDiffDialogData>(MAT_DIALOG_DATA);
+ private registryService = inject(RegistryService);
+ private destroyRef = inject(DestroyRef);
+ private formBuilder = inject(FormBuilder);
+ private nifiCommon = inject(NiFiCommon);
+ private store = inject<Store<CanvasState>>(Store);
Review Comment:
We should not be injecting the store in a dialog. Data should be passed into
the dialog when opened via MAT_DIALOG_DATA.
```suggestion
```
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/flow-diff-dialog/flow-diff-dialog.ts:
##########
@@ -0,0 +1,298 @@
+/*
+ * 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,
+ ViewChild,
+ inject
+} from '@angular/core';
+import { MAT_DIALOG_DATA, MatDialogModule } from '@angular/material/dialog';
+import { MatTableDataSource, MatTableModule } from '@angular/material/table';
+import { MatSort, MatSortModule } from '@angular/material/sort';
+import { FormBuilder, FormControl, FormGroup, ReactiveFormsModule } from
'@angular/forms';
+import { CommonModule } from '@angular/common';
+import { MatFormField, MatLabel } from '@angular/material/form-field';
+import { MatSelectModule } from '@angular/material/select';
+import { MatOption } from '@angular/material/core';
+import { MatInput } from '@angular/material/input';
+import { MatButton } from '@angular/material/button';
+import { MatProgressSpinner } from '@angular/material/progress-spinner';
+import { combineLatest, of } from 'rxjs';
+import { catchError, debounceTime, distinctUntilChanged, filter, map,
startWith, switchMap, take } from 'rxjs/operators';
+import { FlowComparisonEntity, VersionControlInformation } from
'../../../../../state/flow';
+import { VersionedFlowSnapshotMetadata } from
'../../../../../../../state/shared';
+import { RegistryService } from '../../../../../service/registry.service';
+import { CloseOnEscapeDialog, NiFiCommon } from '@nifi/shared';
+import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
+import { Store } from '@ngrx/store';
+import { CanvasState } from '../../../../../state';
+import { selectTimeOffset } from
'../../../../../../../state/flow-configuration/flow-configuration.selectors';
+
+export interface FlowDiffDialogData {
+ versionControlInformation: VersionControlInformation;
+ versions: VersionedFlowSnapshotMetadata[];
+ currentVersion: string;
+ selectedVersion: string;
+}
+
+interface FlowDiffRow {
+ componentName: string;
+ changeType: string;
+ difference: string;
+}
+
+@Component({
+ selector: 'flow-diff-dialog',
+ standalone: true,
+ imports: [
+ CommonModule,
+ MatDialogModule,
+ MatTableModule,
+ MatSortModule,
+ ReactiveFormsModule,
+ MatFormField,
+ MatLabel,
+ MatSelectModule,
+ MatOption,
+ MatInput,
+ MatButton,
+ MatProgressSpinner
+ ],
+ templateUrl: './flow-diff-dialog.html',
+ styleUrl: './flow-diff-dialog.scss'
+})
+export class FlowDiffDialog extends CloseOnEscapeDialog implements
AfterViewInit {
+ private data = inject<FlowDiffDialogData>(MAT_DIALOG_DATA);
+ private registryService = inject(RegistryService);
+ private destroyRef = inject(DestroyRef);
+ private formBuilder = inject(FormBuilder);
+ private nifiCommon = inject(NiFiCommon);
+ private store = inject<Store<CanvasState>>(Store);
+ private timeOffset = this.store.selectSignal(selectTimeOffset);
+
+ @ViewChild(MatSort)
+ set matSort(sort: MatSort | undefined) {
+ if (sort) {
+ this.dataSource.sort = sort;
+ }
+ }
+
+ displayedColumns: string[] = ['componentName', 'changeType', 'difference'];
+ dataSource: MatTableDataSource<FlowDiffRow> = new
MatTableDataSource<FlowDiffRow>();
+ comparisonForm: FormGroup;
+ filterControl: FormControl<string> = new FormControl<string>('', {
nonNullable: true });
+ currentVersionControl: FormControl<string>;
+ selectedVersionControl: FormControl<string>;
+
+ versionOptions: string[];
+ flowName: string;
+ comparisonSummary: { version: string; created?: string }[] = [];
+ isLoading = false;
+ errorMessage: string | null = null;
+ noDifferences = false;
+ private versionMetadataByVersion: Map<string,
VersionedFlowSnapshotMetadata> = new Map();
+
+ constructor() {
+ super();
+ const versions = this.sortVersions(this.data.versions);
+ this.versionOptions = versions.map((version) => version.version);
+ this.versionMetadataByVersion = new Map(versions.map((metadata) =>
[metadata.version, metadata]));
+ const vci = this.data.versionControlInformation;
+ this.flowName = vci.flowName || vci.flowId;
+
+ this.currentVersionControl = new
FormControl<string>(this.data.currentVersion, { nonNullable: true });
+ this.selectedVersionControl = new
FormControl<string>(this.data.selectedVersion, { nonNullable: true });
+ this.comparisonForm = this.formBuilder.group({
+ currentVersion: this.currentVersionControl,
+ selectedVersion: this.selectedVersionControl
+ });
+
+ this.dataSource.filterPredicate = (row: FlowDiffRow, filterTerm:
string) => {
+ if (!filterTerm) {
+ return true;
+ }
+
+ const normalizedFilter = filterTerm.toLowerCase();
+ return (
+ (row.componentName ||
'').toLowerCase().includes(normalizedFilter) ||
+ (row.changeType ||
'').toLowerCase().includes(normalizedFilter) ||
+ (row.difference || '').toLowerCase().includes(normalizedFilter)
+ );
+ };
+
+ this.dataSource.sortingDataAccessor = (row: FlowDiffRow, property:
string) => {
+ switch (property) {
+ case 'componentName':
+ return (row.componentName || '').toLowerCase();
+ case 'changeType':
+ return (row.changeType || '').toLowerCase();
+ case 'difference':
+ return (row.difference || '').toLowerCase();
+ default:
+ return '';
+ }
+ };
+
+ this.configureFiltering();
+ this.configureComparisonChanges();
+ this.setComparisonSummary(this.data.currentVersion,
this.data.selectedVersion);
+ }
+
+ formatVersionOption(version: string): string {
+ const metadata = this.versionMetadataByVersion.get(version);
+ const formattedVersion =
+ version.length > 5 ? `${version.substring(0, 5)}...` : version;
+ const created = metadata ? this.formatTimestamp(metadata) : undefined;
+ return created ? `${formattedVersion} (${created})` : formattedVersion;
+ }
+
+ ngAfterViewInit(): void {
+ // Sorting handled automatically when MatSort is available via the
ViewChild setter
+ }
+
+ private configureFiltering(): void {
+ this.filterControl.valueChanges
+ .pipe(
+ startWith(this.filterControl.value),
+ debounceTime(200),
+ takeUntilDestroyed(this.destroyRef)
+ )
+ .subscribe((value) => {
+ const filterTerm = value ? value.trim() : '';
+ this.dataSource.filter = filterTerm.toLowerCase();
+ });
+ }
+
+ private configureComparisonChanges(): void {
+ const currentVersion$ =
this.currentVersionControl.valueChanges.pipe(startWith(this.currentVersionControl.value));
+ const selectedVersion$ = this.selectedVersionControl.valueChanges.pipe(
+ startWith(this.selectedVersionControl.value)
+ );
+
+ combineLatest([currentVersion$, selectedVersion$])
+ .pipe(
+ takeUntilDestroyed(this.destroyRef),
+ map(([current, selected]) => [current, selected] as [string |
null, string | null]),
+ filter(([current, selected]) => !!current && !!selected),
+ distinctUntilChanged(
+ ([currentA, selectedA], [currentB, selectedB]) =>
+ currentA === currentB && selectedA === selectedB
+ ),
+ switchMap(([current, selected]) => {
+ this.isLoading = true;
+ this.errorMessage = null;
+ this.noDifferences = false;
+ return this.fetchFlowDiff(current as string, selected as
string).pipe(
+ catchError((error) => {
+ this.isLoading = false;
+ this.errorMessage = 'Unable to retrieve version
differences.';
+ console.error('Failed to load flow version diff',
error);
+ this.dataSource.data = [];
+ this.noDifferences = true;
+ return of(null);
+ })
+ );
+ })
+ )
+ .subscribe((comparison) => {
+ if (!comparison) {
+ return;
+ }
+
+ this.isLoading = false;
+ this.setComparisonSummary(this.currentVersionControl.value,
this.selectedVersionControl.value);
+ const rows = this.toRows(comparison);
+ this.dataSource.data = rows;
+ this.noDifferences = rows.length === 0;
+ });
+ }
+
+ private fetchFlowDiff(versionA: string, versionB: string) {
+ this.setComparisonSummary(versionA, versionB);
+ const vci = this.data.versionControlInformation;
+ const branch = vci.branch ?? null;
+
+ return this.registryService
+ .getFlowDiff(vci.registryId, vci.bucketId, vci.flowId, versionA,
versionB, branch)
+ .pipe(take(1));
+ }
+
+ private sortVersions(versions: VersionedFlowSnapshotMetadata[]):
VersionedFlowSnapshotMetadata[] {
+ return versions
+ .slice()
+ .sort((a, b) => {
+ const timestampA =
this.nifiCommon.isDefinedAndNotNull(a.timestamp) ? a.timestamp : 0;
+ const timestampB =
this.nifiCommon.isDefinedAndNotNull(b.timestamp) ? b.timestamp : 0;
+ const timestampComparison =
this.nifiCommon.compareNumber(timestampB, timestampA);
+ if (timestampComparison !== 0) {
+ return timestampComparison;
+ }
+
+ if (this.nifiCommon.isNumber(a.version) &&
this.nifiCommon.isNumber(b.version)) {
+ return this.nifiCommon.compareNumber(parseInt(b.version,
10), parseInt(a.version, 10));
+ }
+
+ return this.nifiCommon.compareString(b.version, a.version);
+ });
+ }
+
+ private toRows(comparison: FlowComparisonEntity): FlowDiffRow[] {
+ if (!comparison || !comparison.componentDifferences) {
+ return [];
+ }
+
+ const rows: FlowDiffRow[] = [];
+ comparison.componentDifferences.forEach((component) => {
+ component.differences.forEach((difference) => {
+ rows.push({
+ componentName: component.componentName || '',
+ changeType: difference.differenceType,
+ difference: difference.difference
+ });
+ });
+ });
+
+ return rows;
+ }
+
+ private setComparisonSummary(versionA: string, versionB: string): void {
+ this.comparisonSummary = [versionA, versionB].map((version) => {
+ const metadata = this.versionMetadataByVersion.get(version);
+ return {
+ version,
+ created: metadata ? this.formatTimestamp(metadata) : undefined
+ };
+ });
+ }
+
+ private formatTimestamp(metadata: VersionedFlowSnapshotMetadata): string |
undefined {
+ if (!metadata.timestamp) {
+ return undefined;
+ }
+
+ const now = new Date();
+ const userOffset = now.getTimezoneOffset() * 60 * 1000;
+ const date = new Date(metadata.timestamp + userOffset +
this.getTimezoneOffset());
+ return this.nifiCommon.formatDateTime(date);
+ }
+
+ private getTimezoneOffset(): number {
+ return this.timeOffset() || 0;
+ }
Review Comment:
I don't think we typically format timestamps like this. Is there a reason to
here? I now see that we do have two implementations of:
```
formatTimestamp(flowVersion: VersionedFlowSnapshotMetadata) {
// get the current user time to properly convert the server time
const now: Date = new Date();
// convert the user offset to millis
const userTimeOffset: number = now.getTimezoneOffset() * 60 * 1000;
// create the proper date by adjusting by the offsets
const date: Date = new Date(flowVersion.timestamp + userTimeOffset +
this.timeOffset);
return this.nifiCommon.formatDateTime(date);
}
```
One in the select-flow-version.componet.ts and one in the
import-from-registry.component.ts. If we really have a need for this formatting
we should have a single implementation in a shared area. A new pipe in
https://github.com/apache/nifi/tree/main/nifi-frontend/src/main/frontend/libs/shared/src/pipes
would work well for this use case.
--
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]
