NIFI-13710: Addressing review feedback.

- Adding editor hint so users know how to use EL. - Fixing issue with rule payload when managing conditions and actions. - Accounting for user permissions when setting the editable flag. - Preventing editors from displaying offscreen. - Showing snackbars following successfully saving. - Adding validators to the condition and actions table.
apache · Dec 11, 2024 · ff670ad · ff670ad
1 parent 7abeb71
commit ff670ad
Show file tree

Hide file tree

Showing 42 changed files with 643 additions and 406 deletions.
diff --git a/...pp/pages/flow-designer/service/resolver/controller-service-advanced-ui-params.resolver.ts b/...pp/pages/flow-designer/service/resolver/controller-service-advanced-ui-params.resolver.ts
@@ -68,7 +68,7 @@ export const controllerServiceAdvancedUiParamsResolver: ResolveFn<AdvancedUiPara
         map((entity) => {
             const revision = client.getRevision(entity);
 
-            const editable = entity.status.runStatus === 'DISABLED';
+            const editable = entity.status.runStatus === 'DISABLED' && entity.permissions.canWrite;
 
             return {
                 url: entity.component.customUiUrl,

diff --git a/...ifi/src/app/pages/flow-designer/service/resolver/processor-advanced-ui-params.resolver.ts b/...ifi/src/app/pages/flow-designer/service/resolver/processor-advanced-ui-params.resolver.ts
@@ -66,10 +66,11 @@ export const processorAdvancedUiParamsResolver: ResolveFn<AdvancedUiParams> = (r
         map((entity) => {
             const revision = client.getRevision(entity);
 
-            const editable = !(
-                entity.status.aggregateSnapshot.runStatus === 'Running' ||
-                entity.status.aggregateSnapshot.activeThreadCount > 0
-            );
+            const editable =
+                !(
+                    entity.status.aggregateSnapshot.runStatus === 'Running' ||
+                    entity.status.aggregateSnapshot.activeThreadCount > 0
+                ) && entity.permissions.canWrite;
 
             return {
                 url: entity.component.config.customUiUrl,

diff --git a/.../pages/flow-designer/ui/canvas/items/processor/edit-processor/edit-processor.component.ts b/.../pages/flow-designer/ui/canvas/items/processor/edit-processor/edit-processor.component.ts
@@ -37,13 +37,11 @@ import { MatOptionModule } from '@angular/material/core';
 import { MatSelectModule } from '@angular/material/select';
 import { Observable, of } from 'rxjs';
 import {
-    BulletinEntity,
     BulletinsTipInput,
     InlineServiceCreationRequest,
     InlineServiceCreationResponse,
     ParameterContextEntity,
-    Property,
-    Revision
+    Property
 } from '../../../../../../../state/shared';
 import { Client } from '../../../../../../../service/client.service';
 import {
@@ -56,7 +54,7 @@ import {
 } from '../../../../../state/flow';
 import { PropertyTable } from '../../../../../../../ui/common/property-table/property-table.component';
 import { NifiSpinnerDirective } from '../../../../../../../ui/common/spinner/nifi-spinner.directive';
-import { NifiTooltipDirective, NiFiCommon, TextTip, CopyDirective } from '@nifi/shared';
+import { NifiTooltipDirective, NiFiCommon, TextTip, CopyDirective, Revision, BulletinEntity } from '@nifi/shared';
 import { RunDurationSlider } from './run-duration-slider/run-duration-slider.component';
 import {
     RelationshipConfiguration,

diff --git a/...src/app/pages/settings/service/resolver/controller-service-advanced-ui-params.resolver.ts b/...src/app/pages/settings/service/resolver/controller-service-advanced-ui-params.resolver.ts
@@ -70,7 +70,7 @@ export const controllerServiceAdvancedUiParamsResolver: ResolveFn<AdvancedUiPara
         map((entity) => {
             const revision = client.getRevision(entity);
 
-            const editable = entity.status.runStatus === 'DISABLED';
+            const editable = entity.status.runStatus === 'DISABLED' && entity.permissions.canWrite;
 
             return {
                 url: entity.component.customUiUrl,

diff --git a/...src/app/pages/settings/service/resolver/parameter-provider-advanced-ui-params.resolver.ts b/...src/app/pages/settings/service/resolver/parameter-provider-advanced-ui-params.resolver.ts
@@ -73,7 +73,7 @@ export const parameterProviderAdvancedUiParamsResolver: ResolveFn<AdvancedUiPara
                 id: entity.id,
                 clientId: revision.clientId,
                 revision: revision.version,
-                editable: true,
+                editable: entity.permissions.canWrite,
                 disconnectedNodeAcknowledged: clusterConnectionService.isDisconnectionAcknowledged()
             } as AdvancedUiParams;
         }),

diff --git a/...ifi/src/app/pages/settings/service/resolver/reporting-task-advanced-ui-params.resolver.ts b/...ifi/src/app/pages/settings/service/resolver/reporting-task-advanced-ui-params.resolver.ts
@@ -66,7 +66,9 @@ export const reportingTaskAdvancedUiParamsResolver: ResolveFn<AdvancedUiParams>
         map((entity) => {
             const revision = client.getRevision(entity);
 
-            const editable = entity.status.runStatus === 'STOPPED' || entity.status.runStatus === 'DISABLED';
+            const editable =
+                (entity.status.runStatus === 'STOPPED' || entity.status.runStatus === 'DISABLED') &&
+                entity.permissions.canWrite;
 
             return {
                 url: entity.component.customUiUrl,

diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/state/shared/index.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/state/shared/index.ts
@@ -252,12 +252,6 @@ export interface PropertyTipInput {
     propertyHistory?: PropertyHistory;
 }
 
-export interface PropertyHintTipInput {
-    supportsEl: boolean;
-    supportsParameters: boolean;
-    hasParameterContext: boolean;
-}
-
 export interface RestrictionsTipInput {
     usageRestriction: string;
     explicitRestrictions: ExplicitRestriction[];

diff --git a/...end/apps/nifi/src/app/ui/common/property-table/editors/nf-editor/nf-editor.component.html b/...end/apps/nifi/src/app/ui/common/property-table/editors/nf-editor/nf-editor.component.html
@@ -24,27 +24,11 @@
     <form class="h-full" [formGroup]="nfEditorForm" cdkTrapFocus [cdkTrapFocusAutoCapture]="!readonly">
         <div class="flex flex-col gap-y-3 relative h-full">
             <div class="flex justify-end">
-                <div
-                    class="property-hint flex gap-x-4 italic text-gray-500"
-                    nifiTooltip
-                    [tooltipComponentType]="PropertyHintTip"
-                    [tooltipInputData]="getPropertyHintTipData()"
-                    (mousedown)="preventDrag($event)">
-                    <div class="flex items-baseline gap-x-1">
-                        <div>EL</div>
-                        <ng-container *ngTemplateOutlet="supportsEl ? supported : notSupported"></ng-container>
-                    </div>
-                    <div class="flex items-baseline gap-x-1">
-                        <div>PARAM</div>
-                        <ng-container *ngTemplateOutlet="supportsParameters ? supported : notSupported"></ng-container>
-                    </div>
-                </div>
-                <ng-template #supported>
-                    <div class="fa fa-check"></div>
-                </ng-template>
-                <ng-template #notSupported>
-                    <div class="fa fa-ban"></div>
-                </ng-template>
+                <property-hint
+                    [supportsEl]="supportsEl"
+                    [showParameters]="true"
+                    [supportsParameters]="supportsParameters"
+                    [hasParameterContext]="parameters !== null"></property-hint>
             </div>
             <div class="flex flex-col gap-y-0.5 flex-1">
                 <div

diff --git a/...end/apps/nifi/src/app/ui/common/property-table/editors/nf-editor/nf-editor.component.scss b/...end/apps/nifi/src/app/ui/common/property-table/editors/nf-editor/nf-editor.component.scss
@@ -25,8 +25,4 @@
     max-height: 100vh;
     max-width: 100vw;
     cursor: move;
-
-    .property-hint {
-        cursor: default;
-    }
 }
diff --git a/...ntend/apps/nifi/src/app/ui/common/property-table/editors/nf-editor/nf-editor.component.ts b/...ntend/apps/nifi/src/app/ui/common/property-table/editors/nf-editor/nf-editor.component.ts
@@ -23,15 +23,13 @@ import { MatDialogModule } from '@angular/material/dialog';
 import { MatInputModule } from '@angular/material/input';
 import { MatButtonModule } from '@angular/material/button';
 import { MatCheckboxModule } from '@angular/material/checkbox';
-import { NgTemplateOutlet } from '@angular/common';
-import { NifiTooltipDirective, Resizable, Parameter } from '@nifi/shared';
-import { PropertyHintTip } from '../../../tooltips/property-hint-tip/property-hint-tip.component';
-import { ParameterConfig, PropertyHintTipInput } from '../../../../../state/shared';
+import { Resizable, Parameter, PropertyHint } from '@nifi/shared';
+import { ParameterConfig } from '../../../../../state/shared';
 import { A11yModule } from '@angular/cdk/a11y';
 import { CodemirrorModule } from '@ctrl/ngx-codemirror';
 import { Editor } from 'codemirror';
-import { NfEl } from "./modes/nfel";
-import { NfPr } from "./modes/nfpr";
+import { NfEl } from './modes/nfel';
+import { NfPr } from './modes/nfpr';
 
 @Component({
     selector: 'nf-editor',
@@ -44,11 +42,10 @@ import { NfPr } from "./modes/nfpr";
         MatInputModule,
         MatButtonModule,
         MatCheckboxModule,
-        NgTemplateOutlet,
-        NifiTooltipDirective,
         A11yModule,
         CodemirrorModule,
-        Resizable
+        Resizable,
+        PropertyHint
     ],
     styleUrls: ['./nf-editor.component.scss']
 })
@@ -91,8 +88,6 @@ export class NfEditor implements OnDestroy {
     @Output() ok: EventEmitter<string | null> = new EventEmitter<string | null>();
     @Output() cancel: EventEmitter<void> = new EventEmitter<void>();
 
-    protected readonly PropertyHintTip = PropertyHintTip;
-
     itemSet = false;
     getParametersSet = false;
 
@@ -197,14 +192,6 @@ export class NfEditor implements OnDestroy {
         };
     }
 
-    getPropertyHintTipData(): PropertyHintTipInput {
-        return {
-            supportsEl: this.supportsEl,
-            supportsParameters: this.supportsParameters,
-            hasParameterContext: this.parameters !== null
-        };
-    }
-
     resized(event: any): void {
         // Note: We calculate the height of the codemirror to fit into an `.nf-editor` overlay. The
         // height of the codemirror needs to be set in order to handle large amounts of text in the codemirror editor.

diff --git a/...rc/main/frontend/apps/nifi/src/app/ui/common/property-table/property-table.component.html b/...rc/main/frontend/apps/nifi/src/app/ui/common/property-table/property-table.component.html
@@ -173,6 +173,7 @@
                 cdkConnectedOverlay
                 [cdkConnectedOverlayOrigin]="editorTrigger"
                 [cdkConnectedOverlayPositions]="editorPositions"
+                [cdkConnectedOverlayPush]="true"
                 [cdkConnectedOverlayHasBackdrop]="true"
                 [cdkConnectedOverlayBackdropClass]="'cdk-overlay-transparent-backdrop'"
                 [cdkConnectedOverlayOpen]="editorOpen"

diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/styles.scss b/nifi-frontend/src/main/frontend/apps/nifi/src/styles.scss
@@ -51,7 +51,7 @@
 @use 'app/ui/common/navigation/navigation.component-theme' as navigation;
 @use 'app/ui/common/property-table/editors/nf-editor/nf-editor.component-theme' as nf-editor;
 @use 'app/ui/common/status-history/status-history.component-theme' as status-history;
-@use 'app/ui/common/tooltips/property-hint-tip/property-hint-tip.component-theme' as property-hint-tip;
+@use 'libs/shared/src/components/tooltips/property-hint-tip/property-hint-tip.component-theme' as property-hint-tip;
 @use 'app/pages/summary/ui/processor-status-listing/processor-status-table/processor-status-table.component-theme' as
     processor-status-table;
 @use 'app/pages/flow-designer/ui/canvas/change-color-dialog/change-color-dialog.component-theme' as change-color-dialog;

diff --git a/...s/update-attribute/src/app/pages/update-attribute/feature/update-attribute.component.html b/...s/update-attribute/src/app/pages/update-attribute/feature/update-attribute.component.html
@@ -19,7 +19,7 @@
     <div class="p-5 flex-1 flex flex-col">
         <h3 class="primary-color">Update Attribute</h3>
         @if (rulesState(); as rulesState) {
-            @if (rulesState.error) {
+            @if (rulesState.error && rulesState.rules === null) {
                 <div class="flex flex-1 justify-center items-center">
                     <i class="fa fa-warning caution-color mr-2"></i>{{ rulesState.error }}
                 </div>
@@ -29,7 +29,7 @@ <h3 class="primary-color">Update Attribute</h3>
                 </div>
             } @else if (rulesState.rules) {
                 @if (evaluationContextState(); as evaluationContextState) {
-                    @if (evaluationContextState.error) {
+                    @if (evaluationContextState.error && evaluationContextState.evaluationContext === null) {
                         <div class="flex flex-1 justify-center items-center">
                             <i class="fa fa-warning caution-color mr-2"></i>{{ evaluationContextState.error }}
                         </div>

diff --git a/...ute/src/app/pages/update-attribute/state/evaluation-context/evaluation-context.actions.ts b/...ute/src/app/pages/update-attribute/state/evaluation-context/evaluation-context.actions.ts
@@ -29,7 +29,7 @@ export const loadEvaluationContextSuccess = createAction(
     props<{ evaluationContextEntity: EvaluationContextEntity }>()
 );
 
-export const laodEvaluationContextFailure = createAction(
+export const loadEvaluationContextFailure = createAction(
     `${EVALUATION_CONTEXT_PREFIX} Load Evaluation Context Failure`,
     props<{ error: string }>()
 );
@@ -48,3 +48,7 @@ export const saveEvaluationContextFailure = createAction(
     `${EVALUATION_CONTEXT_PREFIX} Save Evaluation Context Failure`,
     props<{ error: string }>()
 );
+
+export const resetEvaluationContextFailure = createAction(
+    `${EVALUATION_CONTEXT_PREFIX} Reset Evaluation Context Failure`
+);
diff --git a/...ute/src/app/pages/update-attribute/state/evaluation-context/evaluation-context.effects.ts b/...ute/src/app/pages/update-attribute/state/evaluation-context/evaluation-context.effects.ts
@@ -28,6 +28,7 @@ import { selectAdvancedUiParameters } from '../advanced-ui-parameters/advanced-u
 import { EvaluationContextEntity } from './index';
 import { updateRevision } from '../advanced-ui-parameters/advanced-ui-parameters.actions';
 import { MatSnackBar } from '@angular/material/snack-bar';
+import { resetEvaluationContextFailure } from './evaluation-context.actions';
 
 @Injectable()
 export class EvaluationContextEffects {
@@ -52,7 +53,7 @@ export class EvaluationContextEffects {
                     catchError((errorResponse: HttpErrorResponse) => {
                         const error = this.updateAttributeService.getErrorString(errorResponse);
                         return of(
-                            EvaluationContextActions.laodEvaluationContextFailure({
+                            EvaluationContextActions.loadEvaluationContextFailure({
                                 error
                             })
                         );
@@ -91,6 +92,9 @@ export class EvaluationContextEffects {
         this.actions$.pipe(
             ofType(EvaluationContextActions.saveEvaluationContextSuccess),
             map((action) => action.evaluationContextEntity),
+            tap(() => {
+                this.snackBar.open('Evaluation criteria successfully saved', 'Ok', { duration: 30000 });
+            }),
             switchMap((evaluationContextEntity) =>
                 of(
                     updateRevision({
@@ -101,15 +105,14 @@ export class EvaluationContextEffects {
         )
     );
 
-    saveEvaluationContextFailure$ = createEffect(
-        () =>
-            this.actions$.pipe(
-                ofType(EvaluationContextActions.saveEvaluationContextFailure),
-                map((action) => action.error),
-                tap((error) => {
-                    this.snackBar.open(error, 'Dismiss', { duration: 30000 });
-                })
-            ),
-        { dispatch: false }
+    saveEvaluationContextFailure$ = createEffect(() =>
+        this.actions$.pipe(
+            ofType(EvaluationContextActions.saveEvaluationContextFailure),
+            map((action) => action.error),
+            tap((error) => {
+                this.snackBar.open(error, 'Dismiss', { duration: 30000 });
+            }),
+            switchMap(() => of(resetEvaluationContextFailure()))
+        )
     );
 }
diff --git a/...ute/src/app/pages/update-attribute/state/evaluation-context/evaluation-context.reducer.ts b/...ute/src/app/pages/update-attribute/state/evaluation-context/evaluation-context.reducer.ts
@@ -19,7 +19,9 @@ import { EvaluationContextState } from './index';
 import { createReducer, on } from '@ngrx/store';
 import {
     loadEvaluationContext,
+    loadEvaluationContextFailure,
     loadEvaluationContextSuccess,
+    resetEvaluationContextFailure,
     resetEvaluationContextState,
     saveEvaluationContext,
     saveEvaluationContextFailure,
@@ -50,6 +52,11 @@ export const evaluationContextReducer = createReducer(
             flowFilePolicy: evaluationContextEntity.flowFilePolicy
         }
     })),
+    on(loadEvaluationContextFailure, (state, { error }) => ({
+        ...state,
+        error,
+        loading: false
+    })),
     on(saveEvaluationContext, (state) => ({
         ...state,
         saving: true
@@ -62,8 +69,13 @@ export const evaluationContextReducer = createReducer(
             flowFilePolicy: evaluationContextEntity.flowFilePolicy
         }
     })),
-    on(saveEvaluationContextFailure, (state) => ({
+    on(saveEvaluationContextFailure, (state, { error }) => ({
         ...state,
+        error,
         saving: false
+    })),
+    on(resetEvaluationContextFailure, (state) => ({
+        ...state,
+        error: null
     }))
 );
diff --git a/...e/src/app/pages/update-attribute/state/evaluation-context/evaluation-context.selectors.ts b/...e/src/app/pages/update-attribute/state/evaluation-context/evaluation-context.selectors.ts
@@ -17,9 +17,14 @@
 
 import { createSelector } from '@ngrx/store';
 import { selectUpdateAttributeState, UpdateAttributeState } from '../index';
-import { evaluationContextFeatureKey } from './index';
+import { evaluationContextFeatureKey, EvaluationContextState } from './index';
 
 export const selectEvaluationContextState = createSelector(
     selectUpdateAttributeState,
     (state: UpdateAttributeState) => state[evaluationContextFeatureKey]
 );
+
+export const selectEvaluationContextError = createSelector(
+    selectEvaluationContextState,
+    (state: EvaluationContextState) => state.error
+);
diff --git a/...rontend/apps/update-attribute/src/app/pages/update-attribute/state/rules/rules.effects.ts b/...rontend/apps/update-attribute/src/app/pages/update-attribute/state/rules/rules.effects.ts
@@ -115,6 +115,17 @@ export class RulesEffects {
         )
     );
 
+    editRuleSuccess$ = createEffect(
+        () =>
+            this.actions$.pipe(
+                ofType(RulesActions.editRuleSuccess),
+                tap(() => {
+                    this.snackBar.open('Rule successfully saved', 'Ok', { duration: 30000 });
+                })
+            ),
+        { dispatch: false }
+    );
+
     promptRuleDeletion$ = createEffect(
         () =>
             this.actions$.pipe(

diff --git a/...rontend/apps/update-attribute/src/app/pages/update-attribute/state/rules/rules.reducer.ts b/...rontend/apps/update-attribute/src/app/pages/update-attribute/state/rules/rules.reducer.ts
@@ -67,23 +67,8 @@ export const rulesReducer = createReducer(
         newRule: {
             name: rule ? `Copy of ${rule.name}` : '',
             comments: rule ? rule.comments : '',
-            conditions: rule
-                ? rule.conditions
-                : [
-                      {
-                          id: uuidv4(),
-                          expression: ''
-                      }
-                  ],
-            actions: rule
-                ? rule.actions
-                : [
-                      {
-                          id: uuidv4(),
-                          attribute: '',
-                          value: ''
-                      }
-                  ]
+            conditions: rule ? rule.conditions : [],
+            actions: rule ? rule.actions : []
         }
     })),
     on(resetNewRule, (state) => ({