fix(#2815): Fix pipeline validation in UI (#2816)

* fix(#2815): Fix pipeline validation in UI * Fix formatting * Fix formatting * fix(#2815): Fix linting * Fix e2e tests --------- Co-authored-by: Philipp Zehnder <tenthe@users.noreply.github.com>
apache · May 7, 2024 · cd5a7b4 · cd5a7b4
1 parent bbaed41
commit cd5a7b4
Show file tree

Hide file tree

Showing 9 changed files with 177 additions and 163 deletions.
diff --git a/...ion/adapter-settings/adapter-deployment-settings/adapter-deployment-settings.component.ts b/...ion/adapter-settings/adapter-deployment-settings/adapter-deployment-settings.component.ts
@@ -79,7 +79,9 @@ export class SpAdapterDeploymentSettingsComponent implements OnInit {
             );
         }
 
-        event.chipInput!.clear();
+        if (event.chipInput) {
+            event.chipInput.clear();
+        }
 
         this.serviceTagCtrl.setValue(null);
     }

diff --git a/ui/src/app/editor/components/pipeline-assembly/pipeline-assembly.component.ts b/ui/src/app/editor/components/pipeline-assembly/pipeline-assembly.component.ts
@@ -246,7 +246,7 @@ export class PipelineAssemblyComponent implements OnInit, AfterViewInit {
      */
     submit() {
         const pipelineModel = this.pipelineComponent.rawPipelineModel;
-        const pipeline = this.objectProvider.makeFinalPipeline(pipelineModel);
+        const pipeline = this.objectProvider.makePipeline(pipelineModel);
         this.pipelinePositioningService.collectPipelineElementPositions(
             this.pipelineCanvasMetadata,
             pipelineModel,

diff --git a/...rc/app/editor/components/pipeline-element-options/pipeline-element-options.component.html b/...rc/app/editor/components/pipeline-element-options/pipeline-element-options.component.html
@@ -16,110 +16,114 @@
   ~
   -->
 <div>
-    <div
-        [ngStyle]="
-            currentMouseOverElement === pipelineElement.payload.dom
-                ? { opacity: 1 }
-                : { opacity: 1 }
-        "
-        class="sp-fade-options"
-    >
-        <span
-            class="options-button customize-button"
-            *ngIf="pipelineElement.type !== 'stream'"
-            style="z-index: 10"
+    <div>
+        <div
+            [ngStyle]="
+                currentMouseOverElement === pipelineElement.payload.dom
+                    ? { opacity: 1 }
+                    : { opacity: 1 }
+            "
+            class="sp-fade-options"
         >
-            <button
-                class="options-icon-button"
-                mat-icon-button
-                matTooltip="Configure Element"
-                [matTooltipPosition]="'above'"
-                (click)="customizeElement(pipelineElement)"
-                data-cy="settings-pipeline-element-button"
+            <span
+                class="options-button customize-button"
+                *ngIf="pipelineElement.type !== 'stream'"
+                style="z-index: 10"
             >
-                <i class="material-icons options-icon-size">settings</i>
-            </button>
-        </span>
-        <span
-            class="options-button customize-button"
-            *ngIf="pipelineElement.type === 'stream' && isWildcardTopic()"
-            style="z-index: 10"
-        >
-            <button
-                class="options-icon-button"
-                mat-icon-button
-                matTooltip="Configure Element"
-                [matTooltipPosition]="'above'"
-                (click)="openCustomizeStreamDialog()"
+                <button
+                    class="options-icon-button"
+                    mat-icon-button
+                    matTooltip="Configure Element"
+                    [matTooltipPosition]="'above'"
+                    (click)="customizeElement(pipelineElement)"
+                    data-cy="settings-pipeline-element-button"
+                >
+                    <i class="material-icons options-icon-size">settings</i>
+                </button>
+            </span>
+            <span
+                class="options-button customize-button"
+                *ngIf="pipelineElement.type === 'stream' && isWildcardTopic()"
+                style="z-index: 10"
             >
-                <i class="material-icons options-icon-size">settings</i>
-            </button>
-        </span>
-        <span class="options-button delete-button" style="z-index: 10">
-            <button
-                class="options-icon-button"
-                mat-icon-button
-                matTooltip="Delete Element"
-                [matTooltipPosition]="'above'"
-                (click)="removeElement(pipelineElement)"
+                <button
+                    class="options-icon-button"
+                    mat-icon-button
+                    matTooltip="Configure Element"
+                    [matTooltipPosition]="'above'"
+                    (click)="openCustomizeStreamDialog()"
+                >
+                    <i class="material-icons options-icon-size">settings</i>
+                </button>
+            </span>
+            <span class="options-button delete-button" style="z-index: 10">
+                <button
+                    class="options-icon-button"
+                    mat-icon-button
+                    matTooltip="Delete Element"
+                    [matTooltipPosition]="'above'"
+                    (click)="removeElement(pipelineElement)"
+                >
+                    <i class="material-icons options-icon-size">clear</i>
+                </button>
+            </span>
+            <span
+                class="options-button possible-button"
+                *ngIf="pipelineElement.type !== 'action'"
+                style="z-index: 10"
             >
-                <i class="material-icons options-icon-size">clear</i>
-            </button>
-        </span>
-        <span
-            class="options-button possible-button"
-            *ngIf="pipelineElement.type !== 'action'"
-            style="z-index: 10"
-        >
-            <button
-                class="options-icon-button"
-                mat-icon-button
-                matTooltip="Compatible Elements"
-                [matTooltipPosition]="'below'"
-                [disabled]="!possibleElements || possibleElements.length === 0"
-                (click)="openPossibleElementsDialog()"
-                [attr.data-cy]="
-                    'sp-possible-elements-' +
-                    pipelineElement.payload.name
-                        .toLowerCase()
-                        .replaceAll(' ', '_')
+                <button
+                    class="options-icon-button"
+                    mat-icon-button
+                    matTooltip="Compatible Elements"
+                    [matTooltipPosition]="'below'"
+                    [disabled]="
+                        !possibleElements || possibleElements.length === 0
+                    "
+                    (click)="openPossibleElementsDialog()"
+                    [attr.data-cy]="
+                        'sp-possible-elements-' +
+                        pipelineElement.payload.name
+                            .toLowerCase()
+                            .replaceAll(' ', '_')
+                    "
+                    _
+                >
+                    <i class="material-icons options-icon-size">account_tree</i>
+                </button>
+            </span>
+            <span
+                class="options-button recommended-button"
+                *ngIf="
+                    pipelineElement.type !== 'action' &&
+                    recommendationsAvailable &&
+                    recommendedElements.length > 0
                 "
-                _
+                style="z-index: 10"
             >
-                <i class="material-icons options-icon-size">account_tree</i>
-            </button>
-        </span>
-        <span
-            class="options-button recommended-button"
-            *ngIf="
-                pipelineElement.type !== 'action' &&
-                recommendationsAvailable &&
-                recommendedElements.length > 0
-            "
-            style="z-index: 10"
-        >
-            <button
-                class="options-icon-button"
-                mat-icon-button
-                matTooltip="Recommended Elements"
-                [matTooltipPosition]="'below'"
-                (click)="showRecommendations($event)"
-                [disabled]="!recommendationsAvailable"
-            >
-                <i class="material-icons options-icon-size">add</i>
-            </button>
-        </span>
-        <span class="options-button help-button" style="z-index: 10">
-            <button
-                class="options-icon-button"
-                matTooltip="Help"
-                [matTooltipPosition]="'below'"
-                mat-icon-button
-                (click)="openHelpDialog()"
-            >
-                <i class="material-icons options-icon-size">help</i>
-            </button>
-        </span>
+                <button
+                    class="options-icon-button"
+                    mat-icon-button
+                    matTooltip="Recommended Elements"
+                    [matTooltipPosition]="'below'"
+                    (click)="showRecommendations($event)"
+                    [disabled]="!recommendationsAvailable"
+                >
+                    <i class="material-icons options-icon-size">add</i>
+                </button>
+            </span>
+            <span class="options-button help-button" style="z-index: 10">
+                <button
+                    class="options-icon-button"
+                    matTooltip="Help"
+                    [matTooltipPosition]="'below'"
+                    mat-icon-button
+                    (click)="openHelpDialog()"
+                >
+                    <i class="material-icons options-icon-size">help</i>
+                </button>
+            </span>
+        </div>
         <div
             class="editor-pe-info"
             [ngClass]="'pe-info-' + pipelineElementCssType"

diff --git a/ui/src/app/editor/components/pipeline-element-options/pipeline-element-options.component.ts b/ui/src/app/editor/components/pipeline-element-options/pipeline-element-options.component.ts
@@ -111,7 +111,6 @@ export class PipelineElementOptionsComponent implements OnInit, OnDestroy {
         this.pipelineElementConfiguredObservable =
             this.editorService.pipelineElementConfigured$.subscribe(
                 pipelineElementDomId => {
-                    this.pipelineElement.settings.openCustomize = false;
                     this.restApi.updateCachedPipeline(this.rawPipelineModel);
                     if (
                         pipelineElementDomId ===

diff --git a/ui/src/app/editor/components/pipeline/pipeline.component.ts b/ui/src/app/editor/components/pipeline/pipeline.component.ts
@@ -118,6 +118,8 @@ export class PipelineComponent implements OnInit, OnDestroy {
     previewModeActive = false;
     pipelinePreview: PipelinePreviewModel;
 
+    shouldOpenCustomizeSettings = false;
+
     constructor(
         private jsplumbService: JsplumbService,
         private pipelineEditorService: PipelineEditorService,
@@ -168,11 +170,11 @@ export class PipelineComponent implements OnInit, OnDestroy {
         this.jsplumbFactoryService.destroy(this.preview);
     }
 
-    updateMouseover(elementId) {
+    updateMouseover(elementId: string) {
         this.currentMouseOverElement = elementId;
     }
 
-    updateOptionsClick(elementId) {
+    updateOptionsClick(elementId: string) {
         this.currentMouseOverElement =
             this.currentMouseOverElement === elementId ? '' : elementId;
     }
@@ -191,11 +193,10 @@ export class PipelineComponent implements OnInit, OnDestroy {
         );
     }
 
-    getElementCssClasses(currentPipelineElement) {
+    getElementCssClasses(currentPipelineElement: PipelineElementConfig) {
         return (
             currentPipelineElement.type +
             ' ' +
-            (currentPipelineElement.settings.openCustomize ? '' : '') +
             currentPipelineElement.settings.connectable +
             ' ' +
             currentPipelineElement.settings.displaySettings
@@ -210,7 +211,7 @@ export class PipelineComponent implements OnInit, OnDestroy {
         return this.isInPipeline('set');
     }
 
-    isInPipeline(type) {
+    isInPipeline(type: string) {
         return this.rawPipelineModel.some(
             x => x.type === type && !x.settings.disabled,
         );
@@ -336,6 +337,14 @@ export class PipelineComponent implements OnInit, OnDestroy {
     initPlumb() {
         this.JsplumbBridge.unbind(EVENT_CONNECTION);
 
+        this.JsplumbBridge.bind(EVENT_CONNECTION_DRAG, () => {
+            this.shouldOpenCustomizeSettings = true;
+        });
+
+        this.JsplumbBridge.bind(EVENT_CONNECTION_ABORT, () => {
+            this.shouldOpenCustomizeSettings = false;
+        });
+
         this.JsplumbBridge.bind(EVENT_CONNECTION_MOVED, info => {
             const pe = this.objectProvider.findElement(
                 info.newTargetEndpoint.elementId,
@@ -355,7 +364,6 @@ export class PipelineComponent implements OnInit, OnDestroy {
                 this.rawPipelineModel,
             );
             (pe.payload as InvocablePipelineElementUnion).configured = false;
-            pe.settings.openCustomize = true;
             info.targetEndpoint.setType('empty');
             this.JsplumbBridge.repaintEverything();
             this.validatePipeline();
@@ -383,10 +391,15 @@ export class PipelineComponent implements OnInit, OnDestroy {
                 info.target.id,
                 this.rawPipelineModel,
             );
-            if (pe.settings.openCustomize) {
+
+            if (
+                this.shouldOpenCustomizeSettings ||
+                info.connection.data.openCustomize
+            ) {
                 this.currentPipelineModel = this.objectProvider.makePipeline(
                     this.rawPipelineModel,
                 );
+
                 pe.settings.loadingStatus = true;
                 this.objectProvider
                     .updatePipeline(this.currentPipelineModel)
@@ -399,10 +412,7 @@ export class PipelineComponent implements OnInit, OnDestroy {
                                     info.target.id,
                                 );
                             const currentConnectionValid =
-                                this.currentConnectionValid(
-                                    pe,
-                                    edgeValidations,
-                                );
+                                this.currentConnectionValid(edgeValidations);
                             if (currentConnectionValid) {
                                 this.validatePipeline(
                                     pipelineModificationMessage,
@@ -467,11 +477,7 @@ export class PipelineComponent implements OnInit, OnDestroy {
         };
     }
 
-    currentConnectionValid(
-        pe: PipelineElementConfig,
-        targetEdges: PipelineEdgeValidation[],
-    ) {
-        const entity = pe.payload as InvocablePipelineElementUnion;
+    currentConnectionValid(targetEdges: PipelineEdgeValidation[]) {
         return targetEdges.every(
             e => e.status.validationStatusType === 'COMPLETE',
         );
@@ -511,11 +517,18 @@ export class PipelineComponent implements OnInit, OnDestroy {
                             ).outputStream = modification.outputStream;
                         }
                     }
-                    if (modification.inputStreams) {
+                    if (
+                        modification.inputStreams &&
+                        modification.inputStreams.length > 0
+                    ) {
                         (
                             pe.payload as InvocablePipelineElementUnion
                         ).inputStreams = modification.inputStreams;
                     }
+                    if (modification.pipelineElementValid) {
+                        pe.settings.completed =
+                            PipelineElementConfigurationStatus.OK;
+                    }
                     if (modification.validationInfos.length > 0) {
                         this.pipelineStyleService.updatePeConfigurationStatus(
                             pe,
@@ -536,7 +549,7 @@ export class PipelineComponent implements OnInit, OnDestroy {
         }
     }
 
-    isCustomOutput(pe) {
+    isCustomOutput(pe: PipelineElementConfig) {
         let custom = false;
         if (pe.payload instanceof DataProcessorInvocation) {
             pe.payload.outputStrategies.forEach(strategy => {
@@ -576,7 +589,7 @@ export class PipelineComponent implements OnInit, OnDestroy {
         });
     }
 
-    showErrorDialog(title, description) {
+    showErrorDialog(title: string, description: string) {
         this.dialog.open(ConfirmDialogComponent, {
             width: '500px',
             data: {
@@ -610,7 +623,6 @@ export class PipelineComponent implements OnInit, OnDestroy {
 
         dialogRef.afterClosed().subscribe(c => {
             if (c) {
-                pipelineElementConfig.settings.openCustomize = false;
                 (
                     pipelineElementConfig.payload as InvocablePipelineElementUnion
                 ).configured = true;