From 3058d8a6b22a47f3d0a3ab4aac18cf2441d83dbc Mon Sep 17 00:00:00 2001 From: Richard Cox Date: Wed, 17 Jun 2020 11:13:13 +0100 Subject: [PATCH] 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 --- .../service-tabs-base.component.html | 6 +- .../service-tabs-base.component.ts | 33 +++--- .../service-catalog/services.service.ts | 4 +- .../services-wall.component.html | 2 +- .../services-wall/services-wall.component.ts | 6 ++ ...-service-instance-base-step.component.html | 2 +- ...dd-service-instance-base-step.component.ts | 34 +++++- .../add-service-instance/csi-mode.service.ts | 51 ++++++++- .../specify-details-step.component.ts | 18 ++-- ...specify-user-provided-details.component.ts | 26 +++-- .../app-service-binding-card.component.ts | 6 +- ...app-service-binding-list-config.service.ts | 16 ++- .../cf-service-instances-list-config.base.ts | 21 ++-- .../cf-user-service-instances-list-config.ts | 8 +- ...s-service-instances-list-config.service.ts | 8 +- .../service-instances-data-source.ts | 1 + .../service-instances-list-config.service.ts | 16 ++- .../service-instance-card.component.ts | 5 +- ...vice-instances-wall-list-config.service.ts | 8 +- ...rovided-service-instance-card.component.ts | 5 +- .../create-marketplace-service-instance.po.ts | 10 +- .../create-service-instance-e2e.spec.ts | 13 ++- ...reate-service-instance-private-e2e.spec.ts | 10 +- ...-service-instance-space-scoped-e2e.spec.ts | 9 +- .../marketplace/create-service-instance.po.ts | 16 ++- ...ate-service-instances-bind-app-e2e.spec.ts | 7 +- .../create-ups-service-instance.po.ts | 9 ++ .../delete-service-instance-e2e.spec.ts | 13 ++- .../delete-ups-service-instance-e2e.spec.ts | 8 +- .../edit-service-instance-e2e.spec.ts | 5 +- ...place-create-service-instances-e2e.spec.ts | 102 ++++++++---------- .../marketplace/marketplace-instances.po.ts | 11 ++ .../marketplace-summary-e2e.spec.ts | 9 +- .../marketplace/marketplace-summary.po.ts | 1 + .../marketplace/services-wall-e2e.spec.ts | 5 +- src/test-e2e/marketplace/services-wall.po.ts | 10 +- src/test-e2e/po/page.po.ts | 25 +++-- 37 files changed, 366 insertions(+), 173 deletions(-) create mode 100644 src/test-e2e/marketplace/marketplace-instances.po.ts diff --git a/src/frontend/packages/cloud-foundry/src/features/service-catalog/service-tabs-base/service-tabs-base.component.html b/src/frontend/packages/cloud-foundry/src/features/service-catalog/service-tabs-base/service-tabs-base.component.html index f6b4c53c11..4a7bb72774 100644 --- a/src/frontend/packages/cloud-foundry/src/features/service-catalog/service-tabs-base/service-tabs-base.component.html +++ b/src/frontend/packages/cloud-foundry/src/features/service-catalog/service-tabs-base/service-tabs-base.component.html @@ -1,11 +1,11 @@
- {{ getServiceLabel() | async }} + {{ serviceLabel$ | async }}
- diff --git a/src/frontend/packages/cloud-foundry/src/features/service-catalog/service-tabs-base/service-tabs-base.component.ts b/src/frontend/packages/cloud-foundry/src/features/service-catalog/service-tabs-base/service-tabs-base.component.ts index 817f55a6e2..763af35529 100644 --- a/src/frontend/packages/cloud-foundry/src/features/service-catalog/service-tabs-base/service-tabs-base.component.ts +++ b/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; hasVisiblePlans$: Observable; servicesSubscription: Subscription; + isServiceSpaceScoped$: Observable; + addServiceInstanceLink: string[]; + serviceLabel$: Observable; 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 => { - 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() - ); + ) } } diff --git a/src/frontend/packages/cloud-foundry/src/features/service-catalog/services.service.ts b/src/frontend/packages/cloud-foundry/src/features/service-catalog/services.service.ts index a550800a57..05d65754d3 100644 --- a/src/frontend/packages/cloud-foundry/src/features/service-catalog/services.service.ts +++ b/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)) diff --git a/src/frontend/packages/cloud-foundry/src/features/services/services-wall/services-wall.component.html b/src/frontend/packages/cloud-foundry/src/features/services/services-wall/services-wall.component.html index cf6322f5e7..b47b7c7e12 100644 --- a/src/frontend/packages/cloud-foundry/src/features/services/services-wall/services-wall.component.html +++ b/src/frontend/packages/cloud-foundry/src/features/services/services-wall/services-wall.component.html @@ -3,7 +3,7 @@

Services

- diff --git a/src/frontend/packages/cloud-foundry/src/features/services/services-wall/services-wall.component.ts b/src/frontend/packages/cloud-foundry/src/features/services/services-wall/services-wall.component.ts index 96d806b0c5..be3e78240d 100644 --- a/src/frontend/packages/cloud-foundry/src/features/services/services-wall/services-wall.component.ts +++ b/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; + 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 { diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance-base-step.component.html b/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance-base-step.component.html index bdb623280b..20a2ba742b 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance-base-step.component.html +++ b/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 @@ Choose Service Type - +

Choose service type to {{ bindApp ? 'bind' : 'create' }}

diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance-base-step.component.ts b/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance-base-step.component.ts index 06f9214b75..2daa17cb4a 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance-base-step.component.ts +++ b/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( @@ -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) { + + private cfId: string; + private appId: string; + + constructor( + private route: ActivatedRoute, + public store: Store + ) { 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' + } + + } diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/csi-mode.service.ts b/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/csi-mode.service.ts index 3c4fa72df5..ad65ce4410 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/csi-mode.service.ts +++ b/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(); + } + } + } + } diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/specify-details-step/specify-details-step.component.ts b/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/specify-details-step/specify-details-step.component.ts index 80ed214a56..ae2ed8a868 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/specify-details-step/specify-details-step.component.ts +++ b/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[]; }; }) => diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/specify-user-provided-details/specify-user-provided-details.component.ts b/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/specify-user-provided-details/specify-user-provided-details.component.ts index a5d2b02b09..258727a9cc 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/specify-user-provided-details/specify-user-provided-details.component.ts +++ b/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, @@ -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}` + }; + }) ); } diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-card/app-service-binding-card.component.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-card/app-service-binding-card.component.ts index 5f15687925..f72b232704 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-card/app-service-binding-card.component.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-card/app-service-binding-card.component.ts @@ -34,6 +34,7 @@ import { import { AppEnvVarsState } from '../../../../../../store/types/app-metadata.types'; import { CfCurrentUserPermissions } from '../../../../../../user-permissions/cf-user-permissions-checkers'; import { ServiceActionHelperService } from '../../../../../data-services/service-action-helper.service'; +import { CSI_CANCEL_URL } from '../../../../add-service-instance/csi-mode.service'; import { EnvVarViewComponent } from '../../../../env-var-view/env-var-view.component'; @@ -241,7 +242,10 @@ export class AppServiceBindingCardComponent extends CardCell this.serviceActionHelperService.startEditServiceBindingStepper( this.row.entity.service_instance_guid, this.appService.cfGuid, - { appId: this.appService.appGuid }, + { + appId: this.appService.appGuid, + [CSI_CANCEL_URL]: `/applications/${this.appService.cfGuid}/${this.appService.appGuid}/services` + }, this.isUserProvidedServiceInstance ) } diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-list-config.service.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-list-config.service.ts index 68288fa9ec..2d39cd38df 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-list-config.service.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-list-config.service.ts @@ -19,12 +19,12 @@ import { import { ListView } from '../../../../../../../store/src/actions/list.actions'; import { RouterNav } from '../../../../../../../store/src/actions/router.actions'; import { APIResource } from '../../../../../../../store/src/types/api.types'; -import { GetAppServiceBindings } from '../../../../../actions/application-service-routes.actions'; import { IServiceBinding } from '../../../../../cf-api-svc.types'; import { ApplicationService } from '../../../../../features/applications/application.service'; import { isServiceInstance, isUserProvidedServiceInstance } from '../../../../../features/cloud-foundry/cf.helpers'; import { CfCurrentUserPermissions } from '../../../../../user-permissions/cf-user-permissions-checkers'; import { ServiceActionHelperService } from '../../../../data-services/service-action-helper.service'; +import { CSI_CANCEL_URL } from '../../../add-service-instance/csi-mode.service'; import { BaseCfListConfig } from '../base-cf/base-cf-list-config'; import { TableCellServiceInstanceTagsComponent, @@ -57,17 +57,15 @@ export class AppServiceBindingListConfigService extends BaseCfListConfig> = { action: (item) => { - // FIXME: If the user cancels stepper this leaks #4295 this.serviceActionHelperService.startEditServiceBindingStepper( item.entity.service_instance_guid, this.appService.cfGuid, - { appId: this.appService.appGuid }, + { + appId: this.appService.appGuid, + [CSI_CANCEL_URL]: `/applications/${this.appService.cfGuid}/${this.appService.appGuid}/services` + }, !!isUserProvidedServiceInstance(item.entity.service_instance.entity) - ).subscribe(res => { - if (!res.error) { - this.store.dispatch(new GetAppServiceBindings(this.appService.appGuid, this.appService.cfGuid)); - } - }); + ); }, label: 'Edit', createVisible: () => this.appService.waitForAppEntity$.pipe( @@ -104,7 +102,7 @@ export class AppServiceBindingListConfigService extends BaseCfListConfig 'Service Instances', + headerCell: () => 'Name', cellDefinition: { getValue: (row) => row.entity.service_instance.entity.name }, diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-service-instances-list-config.base.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-service-instances-list-config.base.ts index 52b60a12ee..b1ecf9ef80 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-service-instances-list-config.base.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-service-instances-list-config.base.ts @@ -21,9 +21,10 @@ import { import { ListView } from '../../../../../../../store/src/actions/list.actions'; import { APIResource } from '../../../../../../../store/src/types/api.types'; import { IServiceInstance } from '../../../../../cf-api-svc.types'; +import { isUserProvidedServiceInstance } from '../../../../../features/cloud-foundry/cf.helpers'; import { CfCurrentUserPermissions } from '../../../../../user-permissions/cf-user-permissions-checkers'; import { ServiceActionHelperService } from '../../../../data-services/service-action-helper.service'; -import { CANCEL_ORG_ID_PARAM, CANCEL_SPACE_ID_PARAM } from '../../../add-service-instance/csi-mode.service'; +import { CANCEL_ORG_ID_PARAM, CANCEL_SPACE_ID_PARAM, CSI_CANCEL_URL } from '../../../add-service-instance/csi-mode.service'; import { TableCellAppCfOrgSpaceHeaderComponent, } from '../app/table-cell-app-cforgspace-header/table-cell-app-cforgspace-header.component'; @@ -61,7 +62,7 @@ export class CfServiceInstancesListConfigBase implements IListConfig>[] = [ { columnId: 'name', - headerCell: () => 'Service Instance', + headerCell: () => 'Name', cellDefinition: { getValue: (row) => `${row.entity.name}` }, @@ -149,10 +150,15 @@ export class CfServiceInstancesListConfigBase implements IListConfig = { action: (item: APIResource) => - this.serviceActionHelperService.startEditServiceBindingStepper(item.metadata.guid, item.entity.cfGuid, { - [CANCEL_SPACE_ID_PARAM]: item.entity.space_guid, - [CANCEL_ORG_ID_PARAM]: item.entity.space.entity.organization_guid - }), + this.serviceActionHelperService.startEditServiceBindingStepper( + item.metadata.guid, + item.entity.cfGuid, + { + [CANCEL_SPACE_ID_PARAM]: item.entity.space_guid, + [CANCEL_ORG_ID_PARAM]: item.entity.space.entity.organization_guid, + [CSI_CANCEL_URL]: this.rootLocation + }, + !!isUserProvidedServiceInstance(item.entity)), label: 'Edit', description: 'Edit Service Instance', createVisible: (row$: Observable>) => @@ -176,7 +182,8 @@ export class CfServiceInstancesListConfigBase implements IListConfig, protected datePipe: DatePipe, protected currentUserPermissionsService: CurrentUserPermissionsService, - private serviceActionHelperService: ServiceActionHelperService + private serviceActionHelperService: ServiceActionHelperService, + private rootLocation: string ) { } diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-user-service-instances-list-config.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-user-service-instances-list-config.ts index 9f653f851f..4a9ee537ec 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-user-service-instances-list-config.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-user-service-instances-list-config.ts @@ -28,6 +28,7 @@ import { CANCEL_ORG_ID_PARAM, CANCEL_SPACE_ID_PARAM, CANCEL_USER_PROVIDED, + CSI_CANCEL_URL, } from '../../../add-service-instance/csi-mode.service'; import { CfSpacesUserServiceInstancesDataSource, @@ -64,7 +65,7 @@ export class CfUserServiceInstancesListConfigBase implements IListConfig>[] = [ { columnId: 'name', - headerCell: () => 'Service Instance', + headerCell: () => 'Name', cellDefinition: { getValue: (row) => `${row.entity.name}` }, @@ -152,7 +153,8 @@ export class CfUserServiceInstancesListConfigBase implements IListConfig, - cfSpaceService: CloudFoundrySpaceService, + private cfSpaceService: CloudFoundrySpaceService, protected datePipe: DatePipe, protected currentUserPermissionsService: CurrentUserPermissionsService, private serviceActionHelperService: ServiceActionHelperService diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-spaces-service-instances/cf-spaces-service-instances-list-config.service.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-spaces-service-instances/cf-spaces-service-instances-list-config.service.ts index e87ed733c5..f747341abe 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-spaces-service-instances/cf-spaces-service-instances-list-config.service.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-spaces-service-instances/cf-spaces-service-instances-list-config.service.ts @@ -30,7 +30,13 @@ export class CfSpacesServiceInstancesListConfigService extends CfServiceInstance datePipe: DatePipe, currentUserPermissionsService: CurrentUserPermissionsService, serviceActionHelperService: ServiceActionHelperService) { - super(store, datePipe, currentUserPermissionsService, serviceActionHelperService); + super( + store, + datePipe, + currentUserPermissionsService, + serviceActionHelperService, + `/cloud-foundry/${cfSpaceService.cfGuid}/organizations/${cfSpaceService.orgGuid}/spaces/${cfSpaceService.spaceGuid}/service-instances` + ); this.dataSource = new CfSpacesServiceInstancesDataSource(cfSpaceService.cfGuid, cfSpaceService.spaceGuid, this.store, this); this.serviceInstanceColumns.find(column => column.columnId === 'attachedApps').cellConfig = { breadcrumbs: 'space-services' diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/service-instances/service-instances-data-source.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/service-instances/service-instances-data-source.ts index b99fead8d1..7c3cc59981 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/service-instances/service-instances-data-source.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/service-instances/service-instances-data-source.ts @@ -31,6 +31,7 @@ export class ServiceInstancesDataSource extends ListDataSource { paginationKey, isLocal: true, transformEntities: [ + { type: 'filter', field: 'entity.name' }, (entities: APIResource[], paginationState: PaginationEntityState) => { return entities.filter(e => e.entity.service_guid === serviceGuid); } diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/service-instances/service-instances-list-config.service.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/service-instances/service-instances-list-config.service.ts index 20878f7924..89c39ee4fa 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/service-instances/service-instances-list-config.service.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/service-instances/service-instances-list-config.service.ts @@ -6,6 +6,7 @@ import { CFAppState } from '../../../../../../../cloud-foundry/src/cf-app-state' import { CurrentUserPermissionsService, } from '../../../../../../../core/src/core/permissions/current-user-permissions.service'; +import { ITableText } from '../../../../../../../core/src/shared/components/list/list-table/table.types'; import { ServicesService } from '../../../../../features/service-catalog/services.service'; import { ServiceActionHelperService } from '../../../../data-services/service-action-helper.service'; import { CfServiceInstancesListConfigBase } from '../cf-services/cf-service-instances-list-config.base'; @@ -20,13 +21,26 @@ import { ServiceInstancesDataSource } from './service-instances-data-source'; @Injectable() export class ServiceInstancesListConfigService extends CfServiceInstancesListConfigBase { + enableTextFilter = true; + text: ITableText = { + title: null, + filter: 'Search by name', + noEntries: 'There are no service instances', + }; + constructor( store: Store, servicesService: ServicesService, datePipe: DatePipe, currentUserPermissionsService: CurrentUserPermissionsService, serviceActionHelperService: ServiceActionHelperService) { - super(store, datePipe, currentUserPermissionsService, serviceActionHelperService); + super( + store, + datePipe, + currentUserPermissionsService, + serviceActionHelperService, + `/marketplace/${servicesService.cfGuid}/${servicesService.serviceGuid}/instances` + ); // Remove 'Service' column this.serviceInstanceColumns.splice(1, 1); this.dataSource = new ServiceInstancesDataSource(servicesService.cfGuid, servicesService.serviceGuid, store, this); diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/service-instance-card/service-instance-card.component.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/service-instance-card/service-instance-card.component.ts index 649162074c..e9ac71ae99 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/service-instance-card/service-instance-card.component.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/service-instance-card/service-instance-card.component.ts @@ -25,6 +25,7 @@ import { import { CfCurrentUserPermissions } from '../../../../../../user-permissions/cf-user-permissions-checkers'; import { ServiceActionHelperService } from '../../../../../data-services/service-action-helper.service'; import { CfOrgSpaceLabelService } from '../../../../../services/cf-org-space-label.service'; +import { CSI_CANCEL_URL } from '../../../../add-service-instance/csi-mode.service'; @Component({ selector: 'app-service-instance-card', @@ -130,7 +131,9 @@ export class ServiceInstanceCardComponent extends CardCell this.serviceActionHelperService.startEditServiceBindingStepper( this.serviceInstanceEntity.metadata.guid, this.serviceInstanceEntity.entity.cfGuid, - null + { + [CSI_CANCEL_URL]: '/services' + } ) getServiceName = () => { diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/service-instances-wall-list-config.service.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/service-instances-wall-list-config.service.ts index f3a4c57132..0f597ef543 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/service-instances-wall-list-config.service.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/service-instances-wall-list-config.service.ts @@ -70,7 +70,13 @@ export class ServiceInstancesWallListConfigService extends CfServiceInstancesLis currentUserPermissionsService: CurrentUserPermissionsService, serviceActionHelperService: ServiceActionHelperService ) { - super(store, datePipe, currentUserPermissionsService, serviceActionHelperService); + super( + store, + datePipe, + currentUserPermissionsService, + serviceActionHelperService, + `/services` + ); const multiFilterConfigs = [ createCfOrgSpaceFilterConfig('cf', 'Cloud Foundry', this.cfOrgSpaceService.cf), createCfOrgSpaceFilterConfig('org', 'Organization', this.cfOrgSpaceService.org), diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/user-provided-service-instance-card/user-provided-service-instance-card.component.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/user-provided-service-instance-card/user-provided-service-instance-card.component.ts index 0b2fdcf274..8c2564b26c 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/user-provided-service-instance-card/user-provided-service-instance-card.component.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/services-wall/user-provided-service-instance-card/user-provided-service-instance-card.component.ts @@ -19,6 +19,7 @@ import { cfEntityFactory } from '../../../../../../cf-entity-factory'; import { CfCurrentUserPermissions } from '../../../../../../user-permissions/cf-user-permissions-checkers'; import { ServiceActionHelperService } from '../../../../../data-services/service-action-helper.service'; import { CfOrgSpaceLabelService } from '../../../../../services/cf-org-space-label.service'; +import { CSI_CANCEL_URL } from '../../../../add-service-instance/csi-mode.service'; @Component({ @@ -120,7 +121,9 @@ export class UserProvidedServiceInstanceCardComponent extends CardCell this.serviceActionHelperService.startEditServiceBindingStepper( this.serviceInstanceEntity.metadata.guid, this.serviceInstanceEntity.entity.cfGuid, - null, + { + [CSI_CANCEL_URL]: '/services' + }, true ) diff --git a/src/test-e2e/marketplace/create-marketplace-service-instance.po.ts b/src/test-e2e/marketplace/create-marketplace-service-instance.po.ts index 993fed4acf..7be967f6d1 100644 --- a/src/test-e2e/marketplace/create-marketplace-service-instance.po.ts +++ b/src/test-e2e/marketplace/create-marketplace-service-instance.po.ts @@ -6,9 +6,17 @@ export class CreateMarketplaceServiceInstance extends Page { public stepper: CreateServiceInstanceStepper; - constructor(url = '/services/new/service?base-previous-redirect=%2Fservices%2Fnew') { + constructor(url = '/services/new/service') { super(url); this.stepper = new CreateServiceInstanceStepper(); } + isActivePage() { + return super.isActivePage(true); + } + + waitForPage() { + return super.waitForPage(undefined, true); + } + } diff --git a/src/test-e2e/marketplace/create-service-instance-e2e.spec.ts b/src/test-e2e/marketplace/create-service-instance-e2e.spec.ts index 8c1d7d7ec3..b326fe3f48 100644 --- a/src/test-e2e/marketplace/create-service-instance-e2e.spec.ts +++ b/src/test-e2e/marketplace/create-service-instance-e2e.spec.ts @@ -29,10 +29,7 @@ describe('Create Service Instance', () => { createServiceInstance.waitForPage(); createMarketplaceServiceInstance = createServiceInstance.selectMarketplace(); servicesHelperE2E = new ServicesHelperE2E(e2eSetup, createMarketplaceServiceInstance); - }); - - it('- should reach create service instance page', () => { - expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); + createMarketplaceServiceInstance.waitForPage(); }); describe('Long running tests - ', () => { @@ -40,6 +37,8 @@ describe('Create Service Instance', () => { extendE2ETestTime(timeout); it('- should be able to create a service instance', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); + serviceInstanceName = servicesHelperE2E.createServiceInstanceName(); servicesHelperE2E.createService(e2e.secrets.getDefaultCFEndpoint().services.publicService.name, serviceInstanceName); @@ -51,16 +50,17 @@ describe('Create Service Instance', () => { }); it('- should return user to Service summary when cancelled on CFOrgSpace selection', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); // Select CF/Org/Space servicesHelperE2E.setCfOrgSpace(); createMarketplaceServiceInstance.stepper.cancel(); servicesWall.waitForPage(); - }); it('- should return user to Service summary when cancelled on Service selection', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); // Select CF/Org/Space servicesHelperE2E.setCfOrgSpace(); @@ -77,6 +77,7 @@ describe('Create Service Instance', () => { }); it('- should return user to Service summary when cancelled on App binding selection', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); // Select CF/Org/Space servicesHelperE2E.setCfOrgSpace(); @@ -97,6 +98,8 @@ describe('Create Service Instance', () => { }); it('- should return user to Service summary when cancelled on service instance details', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); + // Select CF/Org/Space servicesHelperE2E.setCfOrgSpace(); createMarketplaceServiceInstance.stepper.next(); diff --git a/src/test-e2e/marketplace/create-service-instance-private-e2e.spec.ts b/src/test-e2e/marketplace/create-service-instance-private-e2e.spec.ts index 35716683f9..3f81f5b3f0 100644 --- a/src/test-e2e/marketplace/create-service-instance-private-e2e.spec.ts +++ b/src/test-e2e/marketplace/create-service-instance-private-e2e.spec.ts @@ -28,10 +28,7 @@ describe('Create Service Instance of Private Service', () => { createServiceInstance.navigateTo(); createServiceInstance.waitForPage(); createMarketplaceServiceInstance = createServiceInstance.selectMarketplace(); - }); - - it('- should reach create service instance page', () => { - expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); + createMarketplaceServiceInstance.waitForPage(); }); describe('Long running tests - ', () => { @@ -39,6 +36,8 @@ describe('Create Service Instance of Private Service', () => { extendE2ETestTime(timeout); it('- should be able to to create a service instance', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); + serviceInstanceName = servicesHelperE2E.createServiceInstanceName(); servicesHelperE2E.createService(e2e.secrets.getDefaultCFEndpoint().services.privateService.name, serviceInstanceName, false); @@ -51,6 +50,7 @@ describe('Create Service Instance of Private Service', () => { }); it('- should return user to Service summary when cancelled on CFOrgSpace selection', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); // Select CF/Org/Space servicesHelperE2E.setCfOrgSpace(); @@ -61,6 +61,7 @@ describe('Create Service Instance of Private Service', () => { }); it('- should return user to Service summary when cancelled on Service selection', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); // Select CF/Org/Space servicesHelperE2E.setCfOrgSpace(); @@ -77,6 +78,7 @@ describe('Create Service Instance of Private Service', () => { }); it('- should not show service plan if wrong org/space are selected', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); // Select CF/Org/Space servicesHelperE2E.setCfOrgSpace(e2e.secrets.getDefaultCFEndpoint().services.privateService.invalidOrgName, diff --git a/src/test-e2e/marketplace/create-service-instance-space-scoped-e2e.spec.ts b/src/test-e2e/marketplace/create-service-instance-space-scoped-e2e.spec.ts index 612e48cdb1..42a9a2f6c0 100644 --- a/src/test-e2e/marketplace/create-service-instance-space-scoped-e2e.spec.ts +++ b/src/test-e2e/marketplace/create-service-instance-space-scoped-e2e.spec.ts @@ -27,11 +27,7 @@ describe('Create Service Instance of Space Scoped Service', () => { createServiceInstance.waitForPage(); createMarketplaceServiceInstance = createServiceInstance.selectMarketplace(); servicesHelperE2E = new ServicesHelperE2E(e2eSetup, createMarketplaceServiceInstance); - }); - - - it('- should reach create service instance page', () => { - expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); + createMarketplaceServiceInstance.waitForPage(); }); describe('Long running tests - ', () => { @@ -39,6 +35,8 @@ describe('Create Service Instance of Space Scoped Service', () => { extendE2ETestTime(timeout); it('- should be able to to create a service instance', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); + serviceInstanceName = servicesHelperE2E.createServiceInstanceName(); servicesHelperE2E.createService(e2e.secrets.getDefaultCFEndpoint().services.spaceScopedService.name, serviceInstanceName); @@ -50,6 +48,7 @@ describe('Create Service Instance of Space Scoped Service', () => { }); it('- should not show service plan if wrong org/space are selected', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); // Select CF/Org/Space servicesHelperE2E.setCfOrgSpace(e2e.secrets.getDefaultCFEndpoint().services.spaceScopedService.invalidOrgName, diff --git a/src/test-e2e/marketplace/create-service-instance.po.ts b/src/test-e2e/marketplace/create-service-instance.po.ts index ee3b61997e..a4cdc26f0d 100644 --- a/src/test-e2e/marketplace/create-service-instance.po.ts +++ b/src/test-e2e/marketplace/create-service-instance.po.ts @@ -1,10 +1,10 @@ - +import { + SERVICE_INSTANCE_TYPES, +} from '../../frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance.types'; import { Page } from '../po/page.po'; import { BaseCreateServiceInstanceStepper } from './base-create-service-instance-stepper.po'; import { CreateMarketplaceServiceInstance } from './create-marketplace-service-instance.po'; -import { - SERVICE_INSTANCE_TYPES -} from '../../frontend/packages/cloud-foundry/src/shared/components/add-service-instance/add-service-instance-base-step/add-service-instance.types'; + export class CreateServiceInstance extends Page { @@ -23,4 +23,12 @@ export class CreateServiceInstance extends Page { super(url); } + isActivePage() { + return super.isActivePage(true); + } + + waitForPage() { + return super.waitForPage(undefined, true); + } + } diff --git a/src/test-e2e/marketplace/create-service-instances-bind-app-e2e.spec.ts b/src/test-e2e/marketplace/create-service-instances-bind-app-e2e.spec.ts index 2608a7a66e..91eb24e945 100644 --- a/src/test-e2e/marketplace/create-service-instances-bind-app-e2e.spec.ts +++ b/src/test-e2e/marketplace/create-service-instances-bind-app-e2e.spec.ts @@ -30,16 +30,15 @@ describe('Create Service Instance with binding', () => { createServiceInstance.waitForPage(); createMarketplaceServiceInstance = createServiceInstance.selectMarketplace(); servicesHelperE2E = new ServicesHelperE2E(e2eSetup, createMarketplaceServiceInstance, servicesHelperE2E); - }); - - it('- should reach create service instance page', () => { - expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); + createMarketplaceServiceInstance.waitForPage(); }); describe('Long running tests - ', () => { extendE2ETestTime(100000); it('- should be able to to create a service instance with binding', () => { + expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); + serviceInstanceName = servicesHelperE2E.createServiceInstanceName(); const servicesSecrets = e2e.secrets.getDefaultCFEndpoint().services; diff --git a/src/test-e2e/marketplace/create-ups-service-instance.po.ts b/src/test-e2e/marketplace/create-ups-service-instance.po.ts index 70801bb031..55c68241a7 100644 --- a/src/test-e2e/marketplace/create-ups-service-instance.po.ts +++ b/src/test-e2e/marketplace/create-ups-service-instance.po.ts @@ -10,4 +10,13 @@ export class CreateUserProvidedServiceInstance extends Page { super(url); this.stepper = new CreateServiceInstanceStepper(); } + + isActivePage() { + return super.isActivePage(true); + } + + waitForPage() { + return super.waitForPage(undefined, true); + } + } diff --git a/src/test-e2e/marketplace/delete-service-instance-e2e.spec.ts b/src/test-e2e/marketplace/delete-service-instance-e2e.spec.ts index 98300f1425..aa74438545 100644 --- a/src/test-e2e/marketplace/delete-service-instance-e2e.spec.ts +++ b/src/test-e2e/marketplace/delete-service-instance-e2e.spec.ts @@ -1,16 +1,16 @@ import { e2e } from '../e2e'; import { ConsoleUserType } from '../helpers/e2e-helpers'; import { extendE2ETestTime } from '../helpers/extend-test-helpers'; +import { ConfirmDialogComponent } from '../po/confirm-dialog'; +import { ListComponent } from '../po/list.po'; +import { MetaCardTitleType } from '../po/meta-card.po'; import { CreateMarketplaceServiceInstance } from './create-marketplace-service-instance.po'; import { CreateServiceInstance } from './create-service-instance.po'; import { ServicesHelperE2E } from './services-helper-e2e'; import { ServicesWallPage } from './services-wall.po'; -import { ConfirmDialogComponent } from '../po/confirm-dialog'; -import { MetaCardTitleType } from '../po/meta-card.po'; -import { ListComponent } from '../po/list.po'; // Regression test for: -// - Deleting a service instance in the services list will delete the service but the list incorerctly shows no services +// - Deleting a service instance in the services list will delete the service but the list incorrectly shows no services describe('Delete Service Instance', () => { const createServiceInstance = new CreateServiceInstance(); let createMarketplaceServiceInstance: CreateMarketplaceServiceInstance; @@ -41,13 +41,12 @@ describe('Delete Service Instance', () => { createServiceInstance.waitForPage(); createMarketplaceServiceInstance = createServiceInstance.selectMarketplace(); servicesHelperE2E = new ServicesHelperE2E(e2eSetup, createMarketplaceServiceInstance); + createMarketplaceServiceInstance.waitForPage(); }); - it('should go to create service instance view', () => { + it('should be able to create a service instance', () => { expect(createMarketplaceServiceInstance.isActivePage()).toBeTruthy(); - }); - it('should be able to create a service instance', () => { const serviceInstanceName = servicesHelperE2E.createServiceInstanceName(); names.push(serviceInstanceName); servicesHelperE2E.createService(e2e.secrets.getDefaultCFEndpoint().services.publicService.name, serviceInstanceName); diff --git a/src/test-e2e/marketplace/delete-ups-service-instance-e2e.spec.ts b/src/test-e2e/marketplace/delete-ups-service-instance-e2e.spec.ts index eff4c5be7e..112dec4037 100644 --- a/src/test-e2e/marketplace/delete-ups-service-instance-e2e.spec.ts +++ b/src/test-e2e/marketplace/delete-ups-service-instance-e2e.spec.ts @@ -1,13 +1,13 @@ import { e2e } from '../e2e'; import { ConsoleUserType } from '../helpers/e2e-helpers'; import { extendE2ETestTime } from '../helpers/extend-test-helpers'; -import { CreateServiceInstance } from './create-service-instance.po'; -import { ServicesHelperE2E } from './services-helper-e2e'; -import { ServicesWallPage } from './services-wall.po'; import { ConfirmDialogComponent } from '../po/confirm-dialog'; -import { MetaCardTitleType } from '../po/meta-card.po'; import { ListComponent } from '../po/list.po'; +import { MetaCardTitleType } from '../po/meta-card.po'; +import { CreateServiceInstance } from './create-service-instance.po'; import { CreateUserProvidedServiceInstance } from './create-ups-service-instance.po'; +import { ServicesHelperE2E } from './services-helper-e2e'; +import { ServicesWallPage } from './services-wall.po'; describe('Delete Service Instance (User Provided Service)', () => { const createServiceInstance = new CreateServiceInstance(); diff --git a/src/test-e2e/marketplace/edit-service-instance-e2e.spec.ts b/src/test-e2e/marketplace/edit-service-instance-e2e.spec.ts index af583a4409..a169e8ab19 100644 --- a/src/test-e2e/marketplace/edit-service-instance-e2e.spec.ts +++ b/src/test-e2e/marketplace/edit-service-instance-e2e.spec.ts @@ -65,7 +65,10 @@ describe('Edit Service Instance', () => { menu.waitUntilNotShown(); return browser.getCurrentUrl().then(url => { - expect(url.endsWith('edit')).toBeTruthy(); + const query = url.indexOf('?'); + const urlWithoutQuery = query >= 0 ? url.substring(0, query) : url; + expect(urlWithoutQuery.endsWith('edit')).toBeTruthy(); + servicesHelperE2E.setServicePlan(true); servicesHelperE2E.createServiceInstance.stepper.next(); diff --git a/src/test-e2e/marketplace/marketplace-create-service-instances-e2e.spec.ts b/src/test-e2e/marketplace/marketplace-create-service-instances-e2e.spec.ts index d520f0da8f..a20621fcfb 100644 --- a/src/test-e2e/marketplace/marketplace-create-service-instances-e2e.spec.ts +++ b/src/test-e2e/marketplace/marketplace-create-service-instances-e2e.spec.ts @@ -4,6 +4,7 @@ import { e2e, E2ESetup } from '../e2e'; import { ConsoleUserType } from '../helpers/e2e-helpers'; import { extendE2ETestTime } from '../helpers/extend-test-helpers'; import { CreateMarketplaceServiceInstance } from './create-marketplace-service-instance.po'; +import { MarketplaceInstancesPage } from './marketplace-instances.po'; import { MarketplaceSummaryPage } from './marketplace-summary.po'; import { ServicesHelperE2E } from './services-helper-e2e'; import { ServicesWallPage } from './services-wall.po'; @@ -11,6 +12,7 @@ import { ServicesWallPage } from './services-wall.po'; describe('Service Instance from Marketplace', () => { let setup: E2ESetup; const servicesWall = new ServicesWallPage(); + let servicesInstances: MarketplaceInstancesPage; const timeout = 60000; let serviceInstanceName: string; @@ -29,7 +31,6 @@ describe('Service Instance from Marketplace', () => { beforeAll(() => init(setup, serviceName).then(res => { servicesHelperE2E = res.servicesHelper; marketplaceSummaryPage = res.summaryPage; - })); beforeEach(() => { @@ -37,10 +38,6 @@ describe('Service Instance from Marketplace', () => { marketplaceSummaryPage.waitForPage(); }); - it('- should have an Add Service Instance button', () => { - expect(marketplaceSummaryPage.getAddServiceInstanceButton().isPresent()).toBeTruthy(); - }); - describe('Long running test', () => { extendE2ETestTime(timeout); it('- should be able to create a new service instance', () => { @@ -66,10 +63,6 @@ describe('Service Instance from Marketplace', () => { marketplaceSummaryPage.waitForPage(); }); - it('- should have an Add Service Instance button', () => { - expect(marketplaceSummaryPage.getAddServiceInstanceButton().isPresent()).toBeTruthy(); - }); - describe('Long running test', () => { extendE2ETestTime(timeout); it('- should be able to create a new service instance', () => { @@ -95,11 +88,8 @@ describe('Service Instance from Marketplace', () => { marketplaceSummaryPage.waitForPage(); }); - it('- should have an Add Service Instance button', () => { - expect(marketplaceSummaryPage.getAddServiceInstanceButton().isPresent()).toBeTruthy(); - }); - describe('Long running test', () => { + extendE2ETestTime(timeout); it('- should be able to create a new service instance', () => { serviceInstanceName = servicesHelperE2E.createServiceInstanceName(); @@ -109,48 +99,46 @@ describe('Service Instance from Marketplace', () => { afterAll(() => servicesHelperE2E.cleanUpServiceInstance(serviceInstanceName)); }); -}); - -function createService( - marketplaceSummaryPage: MarketplaceSummaryPage, - servicesHelperE2E: ServicesHelperE2E, - serviceName: string, - servicesWall: ServicesWallPage, - serviceInstanceName: string -) { - const button = marketplaceSummaryPage.header.getIconButton('add'); - expect(button).toBeDefined(); - button.then(bt => bt.click()); - browser.getCurrentUrl().then(url => { - expect(url.endsWith('create?isSpaceScoped=false')).toBeTruthy(); - // Proceed to create a service instance - servicesHelperE2E.createService(serviceName, serviceInstanceName, true); - - servicesWall.waitForPage(); - - servicesHelperE2E.getServiceCardWithTitle(servicesWall.serviceInstancesList, serviceInstanceName); - - }); -} - -function init( - setup: E2ESetup, - serviceName: string, - spaceScoped = false -) { - const defaultCf = e2e.secrets.getDefaultCFEndpoint(); - const endpointGuid = e2e.helper.getEndpointGuid(e2e.info, defaultCf.name); - - const servicesHelperE2E = new ServicesHelperE2E(setup); - return servicesHelperE2E.fetchServices(endpointGuid).then(response => { - serviceName = e2e.secrets.getDefaultCFEndpoint().services.publicService.name; - const service = response.resources.find(e => e.entity.label === serviceName); - const serviceGuid = service.metadata.guid; - servicesHelperE2E.setCreateServiceInstance( - new CreateMarketplaceServiceInstance('/marketplace/' + endpointGuid + '/' + serviceGuid + - '/create?isSpaceScoped=' + (spaceScoped ? 'true' : 'false'))); - const marketplaceSummaryPage = new MarketplaceSummaryPage(endpointGuid, serviceGuid); - return { servicesHelper: servicesHelperE2E, summaryPage: marketplaceSummaryPage }; - }); -} + function createService( + marketplaceSummaryPage: MarketplaceSummaryPage, + servicesHelperE2E: ServicesHelperE2E, + serviceName: string, + servicesWall: ServicesWallPage, + serviceInstanceName: string + ) { + expect(marketplaceSummaryPage.getAddServiceInstanceButton().isPresent()).toBeTruthy(); + marketplaceSummaryPage.getAddServiceInstanceButton().click(); + servicesHelperE2E.createServiceInstance.waitForPage(); + browser.getCurrentUrl().then(url => { + expect(url.indexOf('isSpaceScoped=false') >= 0).toBeTruthy(); + // Proceed to create a service instance + servicesHelperE2E.createService(serviceName, serviceInstanceName, true); + + servicesInstances.waitForPage(); + servicesInstances.list.header.setSearchText(serviceInstanceName); + servicesInstances.list.table.findRow('name', serviceInstanceName, true); + }); + } + + function init( + setup: E2ESetup, + serviceName: string, + ) { + const defaultCf = e2e.secrets.getDefaultCFEndpoint(); + const endpointGuid = e2e.helper.getEndpointGuid(e2e.info, defaultCf.name); + + const servicesHelperE2E = new ServicesHelperE2E(setup); + return servicesHelperE2E.fetchServices(endpointGuid).then(response => { + serviceName = e2e.secrets.getDefaultCFEndpoint().services.publicService.name; + const service = response.resources.find(e => e.entity.label === serviceName); + const serviceGuid = service.metadata.guid; + servicesHelperE2E.setCreateServiceInstance( + new CreateMarketplaceServiceInstance('/marketplace/' + endpointGuid + '/' + serviceGuid + '/create') + ); + servicesInstances = new MarketplaceInstancesPage(endpointGuid, serviceGuid); + const marketplaceSummaryPage = new MarketplaceSummaryPage(endpointGuid, serviceGuid); + return { servicesHelper: servicesHelperE2E, summaryPage: marketplaceSummaryPage }; + }); + } +}); diff --git a/src/test-e2e/marketplace/marketplace-instances.po.ts b/src/test-e2e/marketplace/marketplace-instances.po.ts new file mode 100644 index 0000000000..7dc3791d3e --- /dev/null +++ b/src/test-e2e/marketplace/marketplace-instances.po.ts @@ -0,0 +1,11 @@ +import { ListComponent } from '../po/list.po'; +import { Page } from '../po/page.po'; + +export class MarketplaceInstancesPage extends Page { + public list = new ListComponent(); + + constructor(cfGuid: string, serviceGuid: string) { + super(`/marketplace/${cfGuid}/${serviceGuid}/instances`); + } + +} diff --git a/src/test-e2e/marketplace/marketplace-summary-e2e.spec.ts b/src/test-e2e/marketplace/marketplace-summary-e2e.spec.ts index c48a61a7eb..c3aac96883 100644 --- a/src/test-e2e/marketplace/marketplace-summary-e2e.spec.ts +++ b/src/test-e2e/marketplace/marketplace-summary-e2e.spec.ts @@ -1,7 +1,8 @@ -import { MarketplaceSummaryPage } from './marketplace-summary.po'; -import { ConsoleUserType } from '../helpers/e2e-helpers'; -import { e2e } from '../e2e'; import { browser } from 'protractor'; + +import { e2e } from '../e2e'; +import { ConsoleUserType } from '../helpers/e2e-helpers'; +import { MarketplaceSummaryPage } from './marketplace-summary.po'; import { ServicesHelperE2E } from './services-helper-e2e'; describe('Marketplace Summary', () => { @@ -56,7 +57,7 @@ describe('Marketplace Summary', () => { expect(button).toBeDefined(); button.then(bt => bt.click()); browser.getCurrentUrl().then(url => { - expect(url.endsWith('create?isSpaceScoped=false')).toBeTruthy(); + expect(url.indexOf('create?isSpaceScoped=false') >= 0).toBeTruthy(); }); }); }); diff --git a/src/test-e2e/marketplace/marketplace-summary.po.ts b/src/test-e2e/marketplace/marketplace-summary.po.ts index 1f1ffe0c5f..1fc186e42d 100644 --- a/src/test-e2e/marketplace/marketplace-summary.po.ts +++ b/src/test-e2e/marketplace/marketplace-summary.po.ts @@ -23,4 +23,5 @@ export class MarketplaceSummaryPage extends Page { return element(by.name('add-service-instance')); } + } diff --git a/src/test-e2e/marketplace/services-wall-e2e.spec.ts b/src/test-e2e/marketplace/services-wall-e2e.spec.ts index 1fa6c985d8..398471326f 100644 --- a/src/test-e2e/marketplace/services-wall-e2e.spec.ts +++ b/src/test-e2e/marketplace/services-wall-e2e.spec.ts @@ -128,8 +128,11 @@ describe('Service Instances Wall', () => { expect(editMenuItem.getText()).toEqual('Edit'); expect(editMenuItem.isEnabled()).toBeTruthy(); editMenuItem.click(); + browser.getCurrentUrl().then(url => { - expect(url.endsWith('edit')).toBeTruthy(); + const query = url.indexOf('?'); + const urlWithoutQuery = query >= 0 ? url.substring(0, query) : url; + expect(urlWithoutQuery.endsWith('edit')).toBeTruthy(); }); const createMarketplaceServiceInstance = new CreateMarketplaceServiceInstance(); createMarketplaceServiceInstance.stepper.cancel(); diff --git a/src/test-e2e/marketplace/services-wall.po.ts b/src/test-e2e/marketplace/services-wall.po.ts index 9f1229751a..c61219b05c 100644 --- a/src/test-e2e/marketplace/services-wall.po.ts +++ b/src/test-e2e/marketplace/services-wall.po.ts @@ -1,4 +1,4 @@ -import { ElementArrayFinder, ElementFinder, promise, element, by } from 'protractor'; +import { by, element, ElementArrayFinder, ElementFinder, promise } from 'protractor'; import { ListComponent } from '../po/list.po'; import { MetaCard, MetaCardTitleType } from '../po/meta-card.po'; @@ -43,4 +43,12 @@ export class ServicesWallPage extends Page { applicationsAttached: items[3].value, })); } + + isActivePage() { + return super.isActivePage(true); + } + + waitForPage() { + return super.waitForPage(undefined, true); + } } diff --git a/src/test-e2e/po/page.po.ts b/src/test-e2e/po/page.po.ts index 533bc147cd..c7bc7d919a 100644 --- a/src/test-e2e/po/page.po.ts +++ b/src/test-e2e/po/page.po.ts @@ -43,13 +43,11 @@ export abstract class Page { return browser.get(this.navLink); } - isActivePage(): promise.Promise { - return browser.getCurrentUrl().then(url => url === this.getUrl()); - } - - isActivePageOrChildPage(): promise.Promise { + isActivePage(ignoreQuery = false): promise.Promise { return browser.getCurrentUrl().then(url => { - return url.startsWith(this.getUrl()); + const browserUrl = ignoreQuery ? this.stripQuery(url) : url; + const expectedUrl = ignoreQuery ? this.stripQuery(this.getUrl()) : this.getUrl(); + return browserUrl === expectedUrl; }); } @@ -62,9 +60,15 @@ export abstract class Page { }); } - waitForPage(timeout = 20000) { + waitForPage(timeout = 20000, ignoreQuery = false) { expect(this.navLink.startsWith('/')).toBeTruthy('navLink should start with a /'); - return browser.wait(until.urlIs(this.getUrl()), timeout, `Failed to wait for page with navlink '${this.navLink}'`); + if (ignoreQuery) { + return browser.wait(() => { + return this.isActivePage(true); + }); + } else { + return browser.wait(until.urlIs(this.getUrl()), timeout, `Failed to wait for page with navlink '${this.navLink}'`); + } } waitForPageDataLoaded(timeout = 20000) { @@ -82,4 +86,9 @@ export abstract class Page { browser.wait(until.urlContains(browser.baseUrl + this.navLink + childPath), 20000); } private getUrl = () => browser.baseUrl + this.navLink; + + private stripQuery(url: string): string { + const queryStarts = url.indexOf('?'); + return queryStarts >= 0 ? url.substring(0, queryStarts) : url; + } }