Fix service stepper navigation (#4366)

* Fix service stepper navigation on create/edit cancel/sumbit
- create service stepper from app service tab, marketplace service summary, service wall
- edit service stepper from marketplace service instances, service wall instances, space service instances and app service instances lists
- fixes #4052, contrinbutes to #4079, #4051

* Fix subscription leak
- fixes #4295
- code no-longer needed

* Force return location of service stepper, fix table edit of upsi and other improvements

* Fix unit tests

* Fix e2e tests

* Fix e2e tests, add search to marketplace service instances table

* Changes following review

src/frontend/packages/cloud-foundry/src/features/service-catalog/service-tabs-base/service-tabs-base.component.html

@@ -1,11 +1,11 @@
 <app-page-header [tabs]="tabLinks" tabsHeader="Service" [breadcrumbs]="breadcrumbs">
   <div class="app-page-header">
-    {{ getServiceLabel() | async }}
+    {{ serviceLabel$ | async }}
   </div>
   <div class="page-header-right">
     <ng-container *appCfUserPermission="canCreateServiceInstance">
-      <button mat-icon-button name="add-service-instance" [routerLink]="addServiceInstanceLink()"
-        [queryParams]="isServiceSpaceScoped() | async" [disabled]="!(hasVisiblePlans$ | async)">
+      <button mat-icon-button name="add-service-instance" [routerLink]="addServiceInstanceLink"
+        [queryParams]="isServiceSpaceScoped$ | async" [disabled]="!(hasVisiblePlans$ | async)">
         <mat-icon [matTooltip]="toolTipText$ | async">add</mat-icon>
       </button>
     </ng-container>

src/frontend/packages/cloud-foundry/src/features/service-catalog/service-tabs-base/service-tabs-base.component.ts

@@ -6,6 +6,7 @@ import { map, publishReplay, refCount } from 'rxjs/operators';
 import { CFAppState } from '../../../../../cloud-foundry/src/cf-app-state';
 import { IPageSideNavTab } from '../../../../../core/src/features/dashboard/page-side-nav/page-side-nav.component';
 import { IHeaderBreadcrumb } from '../../../../../core/src/shared/components/page-header/page-header.types';
+import { CSI_CANCEL_URL } from '../../../shared/components/add-service-instance/csi-mode.service';
 import { CfCurrentUserPermissions } from '../../../user-permissions/cf-user-permissions-checkers';
 import { getServiceName } from '../services-helper';
 import { ServicesService } from '../services.service';
@@ -20,6 +21,9 @@ export class ServiceTabsBaseComponent {
   toolTipText$: Observable<string>;
   hasVisiblePlans$: Observable<boolean>;
   servicesSubscription: Subscription;
+  isServiceSpaceScoped$: Observable<any>;
+  addServiceInstanceLink: string[];
+  serviceLabel$: Observable<string>;
 
   tabLinks: IPageSideNavTab[] = [
     {
@@ -59,24 +63,23 @@ export class ServiceTabsBaseComponent {
           return 'Cannot create service instance (no public or visible plans exist for service)';
         }
       }));
-
-  }
-
-  addServiceInstanceLink = () => [
-    '/marketplace',
-    this.servicesService.cfGuid,
-    this.servicesService.serviceGuid,
-    'create'
-  ]
-
-  isServiceSpaceScoped = () => this.servicesService.isSpaceScoped$;
-
-  getServiceLabel = (): Observable<string> => {
-    return this.servicesService.service$.pipe(
+    this.isServiceSpaceScoped$ = this.servicesService.isSpaceScoped$.pipe(
+      map(queryParams => ({
+        ...queryParams,
+        [CSI_CANCEL_URL]: `/marketplace/${this.servicesService.cfGuid}/${this.servicesService.serviceGuid}/instances`
+      }))
+    )
+    this.addServiceInstanceLink = [
+      '/marketplace',
+      this.servicesService.cfGuid,
+      this.servicesService.serviceGuid,
+      'create'
+    ]
+    this.serviceLabel$ = this.servicesService.service$.pipe(
       map(getServiceName),
       publishReplay(1),
       refCount()
-    );
+    )
   }
 
 }

src/frontend/packages/cloud-foundry/src/features/service-catalog/services.service.ts

@@ -132,6 +132,7 @@ export class ServicesService {
       map(o => (o.length === 0 ? null : o[0]))
     );
     this.isSpaceScoped$ = this.serviceBroker$.pipe(
+      first(),
       map(o => o ? o.entity.space_guid : null),
       switchMap(spaceGuid => {
         if (!spaceGuid) {
@@ -151,7 +152,8 @@ export class ServicesService {
             })),
           );
         }
-      })
+      }),
+      first()
     );
     this.serviceInstances$ = this.allServiceInstances$.pipe(
       map(instances => instances.filter(instance => instance.entity.service_guid === this.serviceGuid))

src/frontend/packages/cloud-foundry/src/features/services/services-wall/services-wall.component.html

@@ -3,7 +3,7 @@ <h1>Services</h1>
   <div class="page-header-right">
     <ng-container *ngIf="(haveConnectedCf$ | async)">
       <ng-container *appCfUserPermission="canCreateServiceInstance">
-        <button mat-icon-button [routerLink]="'/services/new/'">
+        <button mat-icon-button [routerLink]="'/services/new/'" [queryParams]="location">
           <mat-icon>add</mat-icon>
         </button>
       </ng-container>

src/frontend/packages/cloud-foundry/src/features/services/services-wall/services-wall.component.ts

@@ -14,6 +14,7 @@ import {
 } from '../../../../../cloud-foundry/src/shared/data-services/cf-org-space-service.service';
 import { CloudFoundryService } from '../../../../../cloud-foundry/src/shared/data-services/cloud-foundry.service';
 import { ListConfig } from '../../../../../core/src/shared/components/list/list.component.types';
+import { CSI_CANCEL_URL } from '../../../shared/components/add-service-instance/csi-mode.service';
 import { CfCurrentUserPermissions } from '../../../user-permissions/cf-user-permissions-checkers';
 
 @Component({
@@ -35,6 +36,7 @@ export class ServicesWallComponent implements OnDestroy {
   canCreateServiceInstance: CfCurrentUserPermissions;
   initCfOrgSpaceService: Subscription;
   cfIds$: Observable<string[]>;
+  location: { [CSI_CANCEL_URL]: string };
 
   constructor(
     public cloudFoundryService: CloudFoundryService,
@@ -57,6 +59,10 @@ export class ServicesWallComponent implements OnDestroy {
     this.haveConnectedCf$ = cloudFoundryService.connectedCFEndpoints$.pipe(
       map(endpoints => !!endpoints && endpoints.length > 0)
     );
+
+    this.location = {
+      [CSI_CANCEL_URL]: `/services`
+    }
   }
 
   ngOnDestroy(): void {

src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance-base-step.component.html

@@ -1,7 +1,7 @@
 <app-page-header>
   Choose Service Type
 </app-page-header>
-<app-steppers [cancel]="'/services'">
+<app-steppers [cancel]="cancelUrl">
   <app-step [hideNextButton]="true">
     <div class="select-service-step">
       <h3>Choose service type to {{ bindApp ? 'bind' : 'create' }}</h3>

src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance-base-step.component.ts

@@ -1,12 +1,14 @@
 import { Component } from '@angular/core';
-import { ActivatedRoute, Router } from '@angular/router';
+import { ActivatedRoute } from '@angular/router';
 import { Store } from '@ngrx/store';
 
 import { CFAppState } from '../../../../../../cloud-foundry/src/cf-app-state';
+import { getIdFromRoute } from '../../../../../../core/src/core/utils.service';
 import { BASE_REDIRECT_QUERY } from '../../../../../../core/src/shared/components/stepper/stepper.types';
 import { TileConfigManager } from '../../../../../../core/src/shared/components/tile/tile-selector.helpers';
 import { ITileConfig, ITileData } from '../../../../../../core/src/shared/components/tile/tile-selector.types';
 import { RouterNav } from '../../../../../../store/src/actions/router.actions';
+import { CSI_CANCEL_URL } from '../csi-mode.service';
 import { SERVICE_INSTANCE_TYPES } from './add-service-instance.types';
 
 interface ICreateServiceTilesData extends ITileData {
@@ -21,6 +23,7 @@ interface ICreateServiceTilesData extends ITileData {
 export class AddServiceInstanceBaseStepComponent {
   private tileManager = new TileConfigManager();
   public serviceType: string;
+  public cancelUrl = '/services';
 
   public tileSelectorConfig = [
     this.tileManager.getNextTileConfig<ICreateServiceTilesData>(
@@ -44,16 +47,39 @@ export class AddServiceInstanceBaseStepComponent {
     this.serviceType = tile ? tile.data.type : null;
     this.pSelectedTile = tile;
     if (tile) {
-      const baseUrl = this.bindApp ? this.router.routerState.snapshot.url : '/services/new';
+      const baseUrl = this.createServiceTileUrl();
       this.store.dispatch(new RouterNav({
         path: `${baseUrl}/${this.serviceType}`,
         query: {
-          [BASE_REDIRECT_QUERY]: baseUrl
+          [BASE_REDIRECT_QUERY]: baseUrl, // 'previous' destination
+          [CSI_CANCEL_URL]: this.cancelUrl // 'cancel' + 'success' destination
         }
       }));
     }
   }
-  constructor(private route: ActivatedRoute, private router: Router, public store: Store<CFAppState>) {
+
+  private cfId: string;
+  private appId: string;
+
+  constructor(
+    private route: ActivatedRoute,
+    public store: Store<CFAppState>
+  ) {
     this.bindApp = !!this.route.snapshot.data.bind;
+    if (this.bindApp) {
+      this.cfId = getIdFromRoute(this.route, 'endpointId');
+      this.appId = getIdFromRoute(this.route, 'id');
+    }
+    this.cancelUrl = this.createCancelUrl();
+  }
+
+  private createServiceTileUrl(): string {
+    return this.bindApp ? `/applications/${this.cfId}/${this.appId}/bind` : '/services/new'
   }
+
+  private createCancelUrl(): string {
+    return this.bindApp ? `/applications/${this.cfId}/${this.appId}/services` : '/services'
+  }
+
+
 }

src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/csi-mode.service.ts

@@ -1,5 +1,5 @@
 import { Injectable } from '@angular/core';
-import { ActivatedRoute } from '@angular/router';
+import { ActivatedRoute, Router } from '@angular/router';
 import { filter, map, pairwise } from 'rxjs/operators';
 
 import { SpaceScopedService } from '../../../../../cloud-foundry/src/features/service-catalog/services.service';
@@ -19,8 +19,23 @@ export const enum CreateServiceFormMode {
   BindServiceInstance = 'bind-service-instance',
 }
 
+/**
+ * Where should the user be taken on cancel (and success). If not supplied will fall back on previous location and then deduced from
+ * params
+ */
+export const CSI_CANCEL_URL = 'cancel'
+
+/**
+ * Used when `CSI_CANCEL_URL` is not supplied
+ */
 export const CANCEL_SPACE_ID_PARAM = 'space-guid';
+/**
+ * Used when `CSI_CANCEL_URL` is not supplied
+ */
 export const CANCEL_ORG_ID_PARAM = 'org-guid';
+/**
+ * Used when `CSI_CANCEL_URL` is not supplied
+ */
 export const CANCEL_USER_PROVIDED = 'up';
 
 interface ViewDetail {
@@ -44,12 +59,16 @@ export class CsiModeService {
 
   private mode: string;
   public viewDetail: ViewDetail;
+  /**
+   * Where should the user be taken on cancel (and success). Taken from url param, previous location or deduced
+   */
   public cancelUrl: string;
   // This property is only used when launching the Create Service Instance Wizard from the Marketplace
   spaceScopedDetails: SpaceScopedService = { isSpaceScoped: false };
 
   constructor(
     private activatedRoute: ActivatedRoute,
+    router: Router
   ) {
     const serviceId = getIdFromRoute(activatedRoute, 'serviceId');
     const serviceInstanceId = getIdFromRoute(activatedRoute, 'serviceInstanceId');
@@ -118,6 +137,7 @@ export class CsiModeService {
         `/cloud-foundry/${cfId}/organizations/${orgGuid}/spaces/${spaceGuid}/${isUserProvided ? 'user-service-instances' : 'service-instances'}`;
     }
 
+    this.updateCancelUrl(this.activatedRoute, router);
   }
 
   getViewDetail = () => this.viewDetail;
@@ -148,4 +168,33 @@ export class CsiModeService {
     );
   }
 
+  private updateCancelUrl(
+    activatedRoute: ActivatedRoute,
+    router: Router
+  ) {
+    // cancelUrl determines where we go on cancel AND success
+    const cancelUrl = activatedRoute.snapshot.queryParamMap.get(CSI_CANCEL_URL);
+    if (cancelUrl) {
+      // Override cancelUrl with what's been passed in (probably came from the service selection pre-step)
+      this.cancelUrl = cancelUrl;
+    } else {
+      // There's some holes with the way cancelUrl in ctor is calculated
+      // - marketplace/service/instances list --> cancel goes to space service instance list
+      // - marketplace/service create instance --> cancel goes to marketplace/service/instance regardless of starting tab
+      // - .. others??
+      // For simplicity always go back to the previous location
+      // - good catch all
+      // - doesn't work that well for marketplace/service create instance --> success (should go to marketplace/service/instance)
+      // - if user has refreshed on stepper (previous url was login) use the old cancelUrl best-guess value
+      const currentNavigation = router.getCurrentNavigation();
+      if (currentNavigation &&
+        currentNavigation.previousNavigation &&
+        currentNavigation.previousNavigation.finalUrl &&
+        currentNavigation.previousNavigation.finalUrl.toString() !== '/login'
+      ) {
+        this.cancelUrl = currentNavigation.previousNavigation.finalUrl.toString();
+      }
+    }
+  }
+
 }

src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/specify-details-step/specify-details-step.component.ts

@@ -31,7 +31,6 @@ import {
 } from '../../../../../../cloud-foundry/src/actions/create-service-instance.actions';
 import { pathGet, safeStringToObj } from '../../../../../../core/src/core/utils.service';
 import { StepOnNextResult } from '../../../../../../core/src/shared/components/stepper/step/step.component';
-import { RouterNav } from '../../../../../../store/src/actions/router.actions';
 import { getDefaultRequestState, RequestInfoState } from '../../../../../../store/src/reducers/api-request-reducer/types';
 import { APIResource, NormalizedResponse } from '../../../../../../store/src/types/api.types';
 import { UpdateServiceInstance } from '../../../../actions/service-instances.actions';
@@ -297,7 +296,7 @@ export class SpecifyDetailsStepComponent implements OnDestroy, AfterContentInit
       this.longRunningOpService.handleLongRunningCreateService(bindApp);
       // Return to app page instead of falling through to service page
       if (bindApp) {
-        return observableOf(this.routeToServices(state.cfGuid, state.bindAppGuid));
+        return observableOf(this.routeToServices());
       }
     } else if (request.error) {
       // The request has errored, report this back
@@ -318,7 +317,7 @@ export class SpecifyDetailsStepComponent implements OnDestroy, AfterContentInit
           } else {
             // Refetch env vars for app, since they have been changed by CF
             cfEntityCatalog.appEnvVar.api.getMultiple(state.bindAppGuid, state.cfGuid);
-            return this.routeToServices(state.cfGuid, state.bindAppGuid);
+            return this.routeToServices();
           }
         })
       );
@@ -356,13 +355,12 @@ export class SpecifyDetailsStepComponent implements OnDestroy, AfterContentInit
     );
   }
 
-  routeToServices = (cfGuid: string = null, appGuid: string = null): StepOnNextResult => {
-    if (this.modeService.isAppServicesMode()) {
-      this.store.dispatch(new RouterNav({ path: ['/applications', cfGuid, appGuid, 'services'] }));
-    } else {
-      this.store.dispatch(new RouterNav({ path: ['/services'] }));
-    }
-    return { success: true };
+  routeToServices = (): StepOnNextResult => {
+    return {
+      success: true,
+      // We should always go back to where we came from, aka 'cancel' location.
+      redirect: true,
+    };
   }
 
   private setServiceInstanceGuid = (request: { creating: boolean; error: boolean; response: { result: any[]; }; }) =>

src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/specify-user-provided-details/specify-user-provided-details.component.ts

@@ -27,6 +27,7 @@ import { IUserProvidedServiceInstance } from '../../../../cf-api-svc.types';
 import { cfEntityCatalog } from '../../../../cf-entity-catalog';
 import { AppNameUniqueChecking } from '../../../directives/app-name-unique.directive/app-name-unique.directive';
 import { CloudFoundryUserProvidedServicesService } from '../../../services/cloud-foundry-user-provided-services.service';
+import { AppServiceBindingDataSource } from '../../list/list-types/app-sevice-bindings/app-service-binding-data-source';
 import { CreateServiceFormMode, CsiModeService } from './../csi-mode.service';
 
 const { proxyAPIVersion, cfAPIVersion } = environment;
@@ -38,7 +39,7 @@ const { proxyAPIVersion, cfAPIVersion } = environment;
 export class SpecifyUserProvidedDetailsComponent implements OnDestroy {
 
   constructor(
-    route: ActivatedRoute,
+    private route: ActivatedRoute,
     private upsService: CloudFoundryUserProvidedServicesService,
     public modeService: CsiModeService,
     private store: Store<CFAppState>,
@@ -297,11 +298,24 @@ export class SpecifyUserProvidedDetailsComponent implements OnDestroy {
       this.serviceInstanceId,
       updateData
     ).pipe(
-      map(er => ({
-        success: !er.error,
-        redirect: !er.error,
-        message: `Failed to update service instance: ${er.message}`
-      }))
+      map(er => {
+        if (!er.error) {
+          // Update the application binding list
+          const appId = this.appId || this.route.snapshot.queryParamMap.get('appId');
+          if (appId) {
+            this.store.dispatch(AppServiceBindingDataSource.createGetAllServiceBindings(appId, this.cfGuid));
+          }
+          return {
+            success: true,
+            redirect: true,
+          };
+        }
+        return {
+          success: false,
+          redirect: false,
+          message: `Failed to update service instance: ${er.message}`
+        };
+      })
     );
   }