fix(k8s): Fix deploy manifest (spinnaker#6833)

* fix(k8s): Artifacts in deploy manifest were nested too deep * chore(react): Apply promise-setting-react-state pattern
maggieneterval · Apr 11, 2019 · 5640431 · 5640431
1 parent ba895f1
commit 5640431
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 15 deletions.
diff --git a/app/scripts/modules/core/src/artifact/react/StageArtifactSelector.tsx b/app/scripts/modules/core/src/artifact/react/StageArtifactSelector.tsx
@@ -2,6 +2,7 @@ import * as React from 'react';
 import { module } from 'angular';
 import Select from 'react-select';
 import { react2angular } from 'react2angular';
+import { Observable, Subject } from 'rxjs';
 
 import { IArtifact, IExpectedArtifact, IPipeline, IStage } from 'core/domain';
 import { ArtifactIcon, ExpectedArtifactService } from 'core/artifact';
@@ -38,6 +39,8 @@ export class StageArtifactSelector extends React.Component<IStageArtifactSelecto
     id: DEFINE_NEW_ARTIFACT,
   };
 
+  private destroy$ = new Subject();
+
   constructor(props: IStageArtifactSelectorProps) {
     super(props);
 
@@ -46,17 +49,23 @@ export class StageArtifactSelector extends React.Component<IStageArtifactSelecto
     };
   }
 
+  public componentWillUnmount(): void {
+    this.destroy$.next();
+  }
+
   public componentDidMount(): void {
     const excludedPatterns = this.props.excludedArtifactTypePatterns;
-    AccountService.getArtifactAccounts().then(artifactAccounts => {
-      this.setState({
-        artifactAccounts: excludedPatterns
-          ? artifactAccounts.filter(
-              account => !account.types.some(typ => excludedPatterns.some(typPattern => typPattern.test(typ))),
-            )
-          : artifactAccounts,
+    Observable.fromPromise(AccountService.getArtifactAccounts())
+      .takeUntil(this.destroy$)
+      .subscribe(artifactAccounts => {
+        this.setState({
+          artifactAccounts: excludedPatterns
+            ? artifactAccounts.filter(
+                account => !account.types.some(typ => excludedPatterns.some(typPattern => typPattern.test(typ))),
+              )
+            : artifactAccounts,
+        });
       });
-    });
   }
 
   private renderArtifact = (value: IExpectedArtifact) => {
@@ -123,7 +132,7 @@ export class StageArtifactSelector extends React.Component<IStageArtifactSelecto
     return (
       <>
         <div className="sp-margin-m-bottom">{renderLabel ? renderLabel(select) : select}</div>
-        {artifact && (
+        {!!artifact && (
           <ArtifactEditor
             pipeline={pipeline}
             artifact={artifact}

diff --git a/app/scripts/modules/core/src/widgets/spelText/SpelText.tsx b/app/scripts/modules/core/src/widgets/spelText/SpelText.tsx
@@ -10,6 +10,7 @@ import { SpelAutocompleteService } from './SpelAutocompleteService';
 import { ExecutionService } from '../../pipeline/service/execution.service';
 import { StateService } from '@uirouter/core';
 import { IPipeline } from '../../domain';
+import { Observable, Subject } from 'rxjs';
 
 export interface ISpelTextProps {
   placeholder: string;
@@ -25,7 +26,8 @@ export interface ISpelTextState {
 
 export class SpelText extends React.Component<ISpelTextProps, ISpelTextState> {
   private autocompleteService: SpelAutocompleteService;
-  private spelInputRef: any;
+  private readonly spelInputRef: any;
+  private destroy$ = new Subject();
 
   constructor(props: ISpelTextProps) {
     super(props);
@@ -34,12 +36,18 @@ export class SpelText extends React.Component<ISpelTextProps, ISpelTextState> {
       $q,
       new ExecutionService($http, $q, {} as StateService, $timeout),
     );
-    this.autocompleteService.addPipelineInfo(this.props.pipeline).then(textcompleteConfig => {
-      this.setState({ textcompleteConfig: textcompleteConfig });
-    });
+    Observable.fromPromise(this.autocompleteService.addPipelineInfo(this.props.pipeline))
+      .takeUntil(this.destroy$)
+      .subscribe(textcompleteConfig => {
+        this.setState({ textcompleteConfig: textcompleteConfig });
+      });
     this.spelInputRef = React.createRef();
   }
 
+  public componentWillUnmount(): void {
+    this.destroy$.next();
+  }
+
   public componentDidMount(): void {
     this.renderSuggestions();
   }

diff --git a/...dules/kubernetes/src/v2/pipelines/stages/deployManifest/ManifestBindArtifactsSelector.tsx b/...dules/kubernetes/src/v2/pipelines/stages/deployManifest/ManifestBindArtifactsSelector.tsx
@@ -48,7 +48,7 @@ export class ManifestBindArtifactsSelector extends React.Component<IManifestBind
               pipeline={pipeline}
               stage={stage}
               expectedArtifactId={binding && binding.expectedArtifactId}
-              artifact={binding && binding.artifact}
+              artifact={!!binding && binding.artifact}
               onArtifactEdited={(artifact: IArtifact) => this.onChangeBinding(i, { artifact: artifact })}
               onExpectedArtifactSelected={(expectedArtifact: IExpectedArtifact) =>
                 this.onChangeBinding(i, { expectedArtifactId: expectedArtifact.id })

diff --git a/...ules/kubernetes/src/v2/pipelines/stages/deployManifest/deployManifestConfig.controller.ts b/...ules/kubernetes/src/v2/pipelines/stages/deployManifest/deployManifestConfig.controller.ts
@@ -42,7 +42,7 @@ export class KubernetesV2DeployManifestConfigCtrl implements IController {
     const stage = this.$scope.stage;
     this.$scope.bindings = (stage.requiredArtifactIds || [])
       .map((id: string) => ({ expectedArtifactId: id }))
-      .concat((stage.requiredArtifacts || []).map((artifact: IArtifact) => ({ artifact: artifact })));
+      .concat(stage.requiredArtifacts || []);
 
     this.$scope.excludedManifestArtifactTypes = [
       ArtifactTypePatterns.DOCKER_IMAGE,