rfellows commented on code in PR #9613:
URL: https://github.com/apache/nifi/pull/9613#discussion_r1924286846
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/process-group/edit-process-group/edit-process-group.component.html:
##########
@@ -42,18 +54,29 @@ <h2 mat-dialog-title>{{ readonly ? 'Process Group Details'
: 'Edit Process Group
[tooltipComponentType]="TextTip"
[tooltipInputData]="option.description"
[delayClose]="false"
- >{{ option.text }}</mat-option
- >
+ >{{ option.text }}
+ </mat-option>
} @else {
<mat-option
[value]="option.value"
[disabled]="readonly ||
option.disabled"
- >{{ option.text }}</mat-option
- >
+ nifiTooltip
+
[tooltipComponentType]="TextTip"
+
[tooltipInputData]="option.text"
+ [delayClose]="false"
+ >{{ option.text }}
+ </mat-option>
}
}
</mat-select>
</mat-form-field>
+ <button
+ mat-icon-button
+ class="primary-icon-button mt-1 ml-1"
+ (click)="openNewParameterContextDialog()"
+ title="Create parameter context">
+ <i class="fa fa-plus"></i>
+ </button>
Review Comment:
This should only be available if the current user has `canWrite` for
`parameterContextPermissions`. If the user can't modify parameter contexts,
creating one will fail. We should prevent the user from getting into a
situation where we know the desired outcome will fail.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/process-group/create-process-group/create-process-group.component.html:
##########
@@ -32,25 +32,45 @@ <h2 mat-dialog-title>Create Process Group</h2>
</button>
</mat-form-field>
@if (!flowDefinition) {
- <mat-form-field>
- <mat-label>Parameter Context</mat-label>
- <mat-select formControlName="newProcessGroupParameterContext">
- @for (option of parameterContextsOptions; track option) {
- @if (option.description) {
- <mat-option
- [value]="option.value"
- nifiTooltip
- [tooltipComponentType]="TextTip"
- [tooltipInputData]="option.description"
- [delayClose]="false"
- >{{ option.text }}</mat-option
- >
- } @else {
- <mat-option [value]="option.value">{{ option.text
}}</mat-option>
+ <div class="flex flew-row w-full justify-start items-start">
+ <mat-form-field>
+ <mat-label>Parameter Context</mat-label>
+ <mat-select
formControlName="newProcessGroupParameterContext">
+ <mat-option
+ [value]="null"
+ nifiTooltip
+ [tooltipComponentType]="TextTip"
+ [tooltipInputData]="'No parameter context'"
+ [delayClose]="false"
+ >No parameter context
+ </mat-option>
+ @for (option of parameterContextsOptions |
sortObjectByProperty: 'text'; track option.value) {
+ @if (option.description) {
+ <mat-option
+ [value]="option.value"
+ nifiTooltip
+ [disabled]="option.disabled"
+ [tooltipComponentType]="TextTip"
+ [tooltipInputData]="option.description"
+ [delayClose]="false"
+ >{{ option.text }}</mat-option
+ >
+ } @else {
+ <mat-option [value]="option.value"
[disabled]="option.disabled">{{
+ option.text
+ }}</mat-option>
+ }
}
- }
- </mat-select>
- </mat-form-field>
+ </mat-select>
+ </mat-form-field>
+ <button
+ mat-icon-button
+ class="primary-icon-button mt-1 ml-1"
+ (click)="openNewParameterContextDialog()"
+ title="Create parameter context">
+ <i class="fa fa-plus"></i>
+ </button>
Review Comment:
This should only be available if the current user has `canWrite` for
`parameterContextPermissions`. If the user can't modify parameter contexts,
creating one will fail. We should prevent the user from getting into a
situation where we know the desired outcome will fail.
##########
nifi-frontend/src/main/frontend/libs/shared/src/pipes/sort-by-property.pipe.spec.ts:
##########
@@ -0,0 +1,139 @@
+/*
+ * 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 { SortableBy, SortObjectByPropertyPipe } from './sort-by-property.pipe';
+
+describe('SortObjectByPipe', () => {
+ let pipe: SortObjectByPropertyPipe;
+
+ beforeEach(() => {
+ pipe = new SortObjectByPropertyPipe();
+ });
+
+ it('should sort objects alphabetically by name', () => {
+ const items: SortableBy[] = [
+ { name: 'Banana', value: 'Fruit' },
+ { name: 'Apple', value: 'Fruit' },
+ { name: 'Carrot', value: 'Vegetable' }
+ ];
+ const result = pipe.transform(items);
+ expect(result).toEqual([
+ { name: 'Apple', value: 'Fruit' },
+ { name: 'Banana', value: 'Fruit' },
+ { name: 'Carrot', value: 'Vegetable' }
+ ]);
+ });
+
+ it('should handle case-insensitive sorting', () => {
+ const items: SortableBy[] = [{ name: 'banana' }, { name: 'Apple' }, {
name: 'carrot' }];
+ const result = pipe.transform(items);
+ expect(result).toEqual([{ name: 'Apple' }, { name: 'banana' }, { name:
'carrot' }]);
+ });
+
+ it('should return an empty array if input is empty', () => {
+ const items: SortableBy[] = [];
+ const result = pipe.transform(items);
+ expect(result).toEqual([]);
+ });
+
+ it('should handle a single-item array', () => {
+ const items: SortableBy[] = [{ name: 'Apple' }];
+ const result = pipe.transform(items);
+ expect(result).toEqual([{ name: 'Apple' }]);
+ });
+
+ it('should not modify the original array', () => {
+ const items: SortableBy[] = [{ name: 'Banana' }, { name: 'Apple' }];
+ const itemsCopy = [...items];
+ pipe.transform(items);
+ expect(items).not.toEqual(itemsCopy);
+ });
Review Comment:
I'm confused by this test. It suggests that the original array should not
get modified however, the underlying transform uses array.sort which sorts
array elements in place. In many paces we depend on it modifying the array that
is passed in as we don't bother assigning a variable to the result (which is
just a reference to the same array).
--
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]
