Skip to content

Commit

Permalink
Merge pull request #3228 from cloudfoundry-incubator/fix-add-route-ca…
Browse files Browse the repository at this point in the history
…ncel-bug

Fix for cancel broken on add route
  • Loading branch information
richard-cox authored Dec 3, 2018
2 parents ccb9b6c + 83ac4be commit e0d5b8e
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 20 deletions.
4 changes: 2 additions & 2 deletions src/frontend/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { RouterStateSerializer, StoreRouterConnectingModule } from '@ngrx/router
import { AppComponent } from './app.component';
import { RouteModule } from './app.routing';
import { CoreModule } from './core/core.module';
import { DynamicExtenstionRoutes } from './core/extension/dynamic-extension-routes';
import { DynamicExtensionRoutes } from './core/extension/dynamic-extension-routes';
import { ExtensionService } from './core/extension/extension-service';
import { getGitHubAPIURL, GITHUB_API_URL } from './core/github.helpers';
import { CustomImportModule } from './custom-import.module';
Expand Down Expand Up @@ -83,7 +83,7 @@ export class CustomRouterStateSerializer
providers: [
LoggedInService,
ExtensionService,
DynamicExtenstionRoutes,
DynamicExtensionRoutes,
{ provide: GITHUB_API_URL, useFactory: getGitHubAPIURL },
{ provide: RouterStateSerializer, useClass: CustomRouterStateSerializer } // Create action for router navigation
],
Expand Down
10 changes: 6 additions & 4 deletions src/frontend/app/core/extension/dynamic-extension-routes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Injectable } from '@angular/core';
import { CanActivate, Router, RouterStateSnapshot, ActivatedRouteSnapshot, Route } from '@angular/router';
import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from '@angular/router';
import { Observable } from 'rxjs';

import { getRoutesFromExtensions, StratosRouteType } from './extension-service';

/**
Expand All @@ -18,13 +19,13 @@ import { getRoutesFromExtensions, StratosRouteType } from './extension-service';
*/

@Injectable()
export class DynamicExtenstionRoutes implements CanActivate {
constructor(private router: Router) {}
export class DynamicExtensionRoutes implements CanActivate {
constructor(private router: Router) { }

canActivate(
route: ActivatedRouteSnapshot,
state: RouterStateSnapshot
): Observable<boolean>|Promise<boolean>|boolean {
): Observable<boolean> | Promise<boolean> | boolean {
const childRoutes = this.getChildRoutes(route.parent.routeConfig);
// Remove the last route (which is us, the '**' route)
let newChildRoutes = childRoutes.splice(0, childRoutes.length - 1);
Expand All @@ -42,6 +43,7 @@ export class DynamicExtenstionRoutes implements CanActivate {
// Update the route config and navigate again to the same route that was intercepted
this.setChildRoutes(route.parent.routeConfig, newChildRoutes);
this.router.navigateByUrl(state.url);

return false;
}

Expand Down
12 changes: 6 additions & 6 deletions src/frontend/app/features/applications/applications.routing.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { NgModule } from '@angular/core';
import { RouterModule, Routes } from '@angular/router';

import { DynamicExtensionRoutes } from '../../core/extension/dynamic-extension-routes';
import { StratosActionType, StratosTabType } from '../../core/extension/extension-service';
import { PageNotFoundComponentComponent } from '../../core/page-not-found-component/page-not-found-component.component';
import {
AddServiceInstanceComponent,
} from '../../shared/components/add-service-instance/add-service-instance/add-service-instance.component';
Expand All @@ -25,9 +28,6 @@ import { DeployApplicationModule } from './deploy-application/deploy-application
import { EditApplicationComponent } from './edit-application/edit-application.component';
import { AddRouteStepperComponent } from './routes/add-route-stepper/add-route-stepper.component';
import { SshApplicationComponent } from './ssh-application/ssh-application.component';
import { DynamicExtenstionRoutes } from '../../core/extension/dynamic-extension-routes';
import { StratosActionType, StratosTabType, extensionsActionRouteKey } from '../../core/extension/extension-service';
import { PageNotFoundComponentComponent } from '../../core/page-not-found-component/page-not-found-component.component';

const applicationsRoutes: Routes = [
{
Expand Down Expand Up @@ -94,7 +94,7 @@ const applicationsRoutes: Routes = [
{
path: '**',
component: PageNotFoundComponentComponent,
canActivate: [DynamicExtenstionRoutes],
canActivate: [DynamicExtensionRoutes],
data: {
stratosRouteGroup: StratosTabType.Application
}
Expand All @@ -108,7 +108,7 @@ const applicationsRoutes: Routes = [
{
path: '**',
component: PageNotFoundComponentComponent,
canActivate: [DynamicExtenstionRoutes],
canActivate: [DynamicExtensionRoutes],
data: {
stratosRouteGroup: StratosActionType.Application
}
Expand All @@ -120,7 +120,7 @@ const applicationsRoutes: Routes = [
{
path: '**',
component: PageNotFoundComponentComponent,
canActivate: [DynamicExtenstionRoutes],
canActivate: [DynamicExtensionRoutes],
data: {
stratosRouteGroup: StratosActionType.Applications
}
Expand Down
10 changes: 5 additions & 5 deletions src/frontend/app/features/cloud-foundry/cloud-foundry.routing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ import { CloudFoundryStacksComponent } from './tabs/cloud-foundry-stacks/cloud-f
import { CloudFoundrySummaryTabComponent } from './tabs/cloud-foundry-summary-tab/cloud-foundry-summary-tab.component';
import { CloudFoundryUsersComponent } from './tabs/cloud-foundry-users/cloud-foundry-users.component';
import { UsersRolesComponent } from './users/manage-users/manage-users.component';
import { DynamicExtenstionRoutes } from '../../core/extension/dynamic-extension-routes';
import { DynamicExtensionRoutes } from '../../core/extension/dynamic-extension-routes';
import { PageNotFoundComponentComponent } from '../../core/page-not-found-component/page-not-found-component.component';
import { StratosActionType } from '../../core/extension/extension-service';

Expand Down Expand Up @@ -184,7 +184,7 @@ const cloudFoundry: Routes = [{
{
path: '**',
component: PageNotFoundComponentComponent,
canActivate: [DynamicExtenstionRoutes],
canActivate: [DynamicExtensionRoutes],
data: {
stratosRouteGroup: 'cfTabs'
}
Expand Down Expand Up @@ -249,7 +249,7 @@ const cloudFoundry: Routes = [{
{
path: '**',
component: PageNotFoundComponentComponent,
canActivate: [DynamicExtenstionRoutes],
canActivate: [DynamicExtensionRoutes],
data: {
stratosRouteGroup: 'cfOrgTabs'
}
Expand Down Expand Up @@ -292,7 +292,7 @@ const cloudFoundry: Routes = [{
{
path: '**',
component: PageNotFoundComponentComponent,
canActivate: [DynamicExtenstionRoutes],
canActivate: [DynamicExtensionRoutes],
data: {
stratosRouteGroup: 'cfSpaceTabs'
}
Expand All @@ -305,7 +305,7 @@ const cloudFoundry: Routes = [{
{
path: '**',
component: PageNotFoundComponentComponent,
canActivate: [DynamicExtenstionRoutes],
canActivate: [DynamicExtensionRoutes],
data: {
stratosRouteGroup: StratosActionType.CloudFoundry
}
Expand Down
4 changes: 2 additions & 2 deletions src/frontend/app/features/endpoints/endpoints.routing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Routes, RouterModule } from '@angular/router';

import { EndpointsPageComponent } from './endpoints-page/endpoints-page.component';
import { CreateEndpointComponent } from './create-endpoint/create-endpoint.component';
import { DynamicExtenstionRoutes } from '../../core/extension/dynamic-extension-routes';
import { DynamicExtensionRoutes } from '../../core/extension/dynamic-extension-routes';
import { PageNotFoundComponentComponent } from '../../core/page-not-found-component/page-not-found-component.component';
import { StratosActionType } from '../../core/extension/extension-service';

Expand All @@ -18,7 +18,7 @@ const endpointsRoutes: Routes = [
{
path: '**',
component: PageNotFoundComponentComponent,
canActivate: [DynamicExtenstionRoutes],
canActivate: [DynamicExtensionRoutes],
data: {
stratosRouteGroup: StratosActionType.Endpoints
}
Expand Down
12 changes: 11 additions & 1 deletion src/frontend/app/store/reducers/routing.reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,22 @@ export interface RouterRedirect {
export function routingReducer(state: RoutingHistory = defaultRoutingState, action: RouterNavigationAction) {
switch (action.type) {
case ROUTER_NAVIGATION:
// Check that the route actually changed - don't update the state if it did not
// This catches the case for the Dynamic Extensions where we have to redirect to the route
// Which would otherwise set the previous state to the same state as the current state
const destUrl = action.payload.event.url;
const currentUrl = state.currentState ? state.currentState.url : null;
if (destUrl === currentUrl) {
return state;
}

return {
// ATM don't track change of route history
// history: state.history.concat([action.payload.event]),
previousState: state.currentState ? state.currentState : null,
currentState: action.payload.event

};

default:
return state;
}
Expand Down
16 changes: 16 additions & 0 deletions src/test-e2e/application/application-view-e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { ApplicationPageInstancesTab } from './po/application-page-instances.po'
import { ApplicationPageRoutesTab } from './po/application-page-routes.po';
import { ApplicationPageSummaryTab } from './po/application-page-summary.po';
import { ApplicationPageVariablesTab } from './po/application-page-variables.po';
import { CreateRoutesPage } from './po/routes-create-page.po';

describe('Application View -', function () {
let cfHelper: CFHelpers;
Expand Down Expand Up @@ -182,6 +183,21 @@ describe('Application View -', function () {
expect(appRoutes.list.empty.getCustomLineOne()).toBe('This application has no routes');
});

it('Should be able to cancel from Add Route', () => {
expect(appRoutes.list.header.getAdd().isDisplayed()).toBeTruthy();
appRoutes.list.header.getAdd().click();

const addRoutePage = new CreateRoutesPage(CFHelpers.cachedDefaultCfGuid, app.metadata.guid, app.entity.space_guid);
expect(addRoutePage.isActivePage()).toBeTruthy();
expect(addRoutePage.header.getTitleText()).toBe('Create Route');
expect(addRoutePage.type.getSelected().getText()).toBe('Create and map new route');

expect(addRoutePage.stepper.canCancel()).toBeTruthy();
addRoutePage.stepper.cancel();

// Should return back to App Routes
appRoutes.waitForPage();
});
});

describe('Variables Tab -', () => {
Expand Down

0 comments on commit e0d5b8e

Please sign in to comment.