Skip to content

Commit

Permalink
feat: expose Promise result of the Angular navigation in RoutingServi…
Browse files Browse the repository at this point in the history
…ce (#12795)

Exposed Promise result of the Angular navigation in methods of the `RoutingService`: `go()` and `goByUrl()`.
Now Angular Router is called directly from the `RoutingService`, but not from ngrx effects anymore. Dropped the effects as well as the actions triggering them.

Added migration docs and schematics for the breaking changes.

By the way, added the missing param `NavigationExtras` in `RoutingService.goByUrl`.

BREAKING CHANGES
The following ngrx actions have been removed:
- `RoutingActions.RouteGo` (and `RoutingActions.ROUTER_GO`)
- `RoutingActions.RouteGoByUrlAction` (and `RoutingActions.ROUTER_GO_BY_URL`)
- `RoutingActions.RouteBackAction` (and `RoutingActions.ROUTER_BACK`)
- `RoutingActions.RouteForwardAction` (and `RoutingActions.ROUTER_FORWARD`).
Please use instead the methods of the `RoutingService`, respectively: `go()`, `goByUrl()`, `back()`, `forward()`.

closes #12789
  • Loading branch information
Platonn authored Jun 22, 2021
1 parent 868c420 commit 051d195
Show file tree
Hide file tree
Showing 23 changed files with 122 additions and 257 deletions.
10 changes: 10 additions & 0 deletions docs/migration/4_0.md
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ The display of the guest checkout button relies on the presence of the `forced`
- `AuthHttpHeaderService` now requires `AuthRedirectService`.
- `AsmAuthHttpHeaderService` now requires `AuthRedirectService`.
- `AuthRedirectService` now requires `AuthFlowRoutesService`.
- `RoutingService` now requires `Location` from `@angular/common`.
- `ProtectedRoutesService` now requires `UrlParsingService`.
- `EventService` no longer uses `FeatureConfigService`.
- `PageEventModule` was removed. Instead, use `NavigationEventModule` from `@spartacus/storefront`
Expand Down Expand Up @@ -1216,6 +1217,15 @@ related labels outside of checkout and the checkout lib is not used, you will ne
`RoutingService.go` - Removed 2nd argument `query`. Use `extras.queryParams` instead.
`RoutingService.navigate` - Removed 2nd argument `query`. Use `extras.queryParams` instead.
### RoutingActions ngrx
The following ngrx actions have been removed:
- `RoutingActions.RouteGo` (and `RoutingActions.ROUTER_GO`)
- `RoutingActions.RouteGoByUrlAction` (and `RoutingActions.ROUTER_GO_BY_URL`)
- `RoutingActions.RouteBackAction` (and `RoutingActions.ROUTER_BACK`)
- `RoutingActions.RouteForwardAction` (and `RoutingActions.ROUTER_FORWARD`).
Please use instead the methods of the `RoutingService`, respectively: `go()`, `goByUrl()`, `back()`, `forward()`.
### AuthRedirectService
- `#reportNotAuthGuard` - method not needed anymore. Every visited URL is now remembered automatically as redirect URL on `NavigationEnd` event.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class MockGlobalMessageService implements Partial<GlobalMessageService> {
}

class MockRoutingService implements Partial<RoutingService> {
go() {}
go = () => Promise.resolve(true);
}

class MockAsmComponentService {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class MockAsmComponentService implements Partial<AsmComponentService> {
logoutCustomerSupportAgentAndCustomer(): void {}
}
class MockRoutingService implements Partial<RoutingService> {
go() {}
go = () => Promise.resolve(true);
isNavigating() {
return of(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class MockAuthStorageService implements Partial<AuthStorageService> {
class MockOAuthLibWrapperService implements Partial<OAuthLibWrapperService> {}

class MockRoutingService implements Partial<RoutingService> {
go() {}
go = () => Promise.resolve(true);
}

class MockGlobalMessageService implements Partial<GlobalMessageService> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class MockAuthService implements Partial<AuthService> {
}

class MockRoutingService implements Partial<RoutingService> {
go(): void {}
go = () => Promise.resolve(true);
}

class MockLaunchDialogService implements Partial<LaunchDialogService> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class MockSavedCartFacade implements Partial<SavedCartFacade> {
}

class MockRoutingService implements Partial<RoutingService> {
go(): void {}
go = () => Promise.resolve(true);
}

class MockGlobalMessageService implements Partial<GlobalMessageService> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class MockSavedCartFacade implements Partial<SavedCartFacade> {
}

class MockRoutingService implements Partial<RoutingService> {
go(): void {}
go = () => Promise.resolve(true);
}
class MockGlobalMessageService implements Partial<GlobalMessageService> {
add(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class MockTranslationService {
}

class MockRoutingService implements Partial<RoutingService> {
go(): void {}
go = () => Promise.resolve(true);
}

describe('SavedCartListComponent', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class MockLaunchDialogService implements Partial<LaunchDialogService> {
}

class MockRoutingService implements Partial<RoutingService> {
go(): void {}
go = () => Promise.resolve(true);
}

class MockSavedCartFacade implements Partial<SavedCartFacade> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class MockUrlPipe implements PipeTransform {
}

class MockRoutingService implements Partial<RoutingService> {
go() {}
go = () => Promise.resolve(true);
}

let component: ConfigureProductComponent;
Expand Down
28 changes: 11 additions & 17 deletions feature-libs/storefinder/core/facade/store-finder.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,32 @@
import { inject, TestBed } from '@angular/core/testing';
import * as NgrxStore from '@ngrx/store';
import { MemoizedSelector, Store, StoreModule } from '@ngrx/store';
import { StoreFinderActions } from '../store/actions/index';
import * as fromStoreReducers from '../store/reducers/index';
import {
FindStoresState,
StoresState,
StateWithStoreFinder,
STORE_FINDER_FEATURE,
} from '../store/store-finder-state';
import { StoreFinderService } from './store-finder.service';
import { NavigationExtras } from '@angular/router';
import { StoreFinderConfig } from '../config/store-finder-config';
import {
GeoPoint,
GlobalMessageService,
PointOfService,
RoutingService,
UrlCommands,
WindowRef,
} from '@spartacus/core';
import { BehaviorSubject, EMPTY, of } from 'rxjs';
import { StoreFinderConfig } from '../config/store-finder-config';
import { StoreFinderSelectors } from '../store';
import { StoreFinderActions } from '../store/actions/index';
import * as fromStoreReducers from '../store/reducers/index';
import {
FindStoresState,
StateWithStoreFinder,
StoresState,
STORE_FINDER_FEATURE,
} from '../store/store-finder-state';
import { StoreFinderService } from './store-finder.service';

const routerParam$: BehaviorSubject<{
[key: string]: string;
}> = new BehaviorSubject({});

class MockRoutingService implements Partial<RoutingService> {
go(
_commands: any[] | UrlCommands,
_query?: object,
_extras?: NavigationExtras
): void {}
go = () => Promise.resolve(true);

getParams = () => routerParam$.asObservable();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import {
RoutingService,
} from '@spartacus/core';
import { ICON_TYPE, ModalService } from '@spartacus/storefront';
import { UserProfileFacade } from '@spartacus/user/profile/root';
import { Observable, of, throwError } from 'rxjs';
import { CloseAccountModalComponent } from './close-account-modal.component';
import { UserProfileFacade } from '@spartacus/user/profile/root';
import createSpy = jasmine.createSpy;

class MockGlobalMessageService implements Partial<GlobalMessageService> {
Expand All @@ -31,7 +31,7 @@ class MockAuthService implements Partial<AuthService> {
}

class MockRoutingService implements Partial<RoutingService> {
go() {}
go = () => Promise.resolve(true);
}

@Component({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class MockAuthRedirectService implements Partial<AuthRedirectService> {
}

class MockRoutingService implements Partial<RoutingService> {
go() {}
go = () => Promise.resolve(true);
}

describe('AuthService', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class MockOAuthLibWrapperService implements Partial<OAuthLibWrapperService> {
}

class MockRoutingService implements Partial<RoutingService> {
go() {}
go = () => Promise.resolve(true);
}

class MockOccEndpointsService implements Partial<OccEndpointsService> {
Expand Down
72 changes: 39 additions & 33 deletions projects/core/src/routing/facade/routing.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { Location } from '@angular/common';
import { TestBed } from '@angular/core/testing';
import { Router } from '@angular/router';
import { RouterTestingModule } from '@angular/router/testing';
import * as NgrxStore from '@ngrx/store';
import { Store, StoreModule } from '@ngrx/store';
Expand Down Expand Up @@ -29,12 +31,19 @@ class MockRoutingParamsService {
}
}

class MockLocation implements Partial<MockLocation> {
back = jasmine.createSpy('back');
forward = jasmine.createSpy('forward');
}

describe('RoutingService', () => {
let store: Store<RouterState>;
let service: RoutingService;
let winRef: WindowRef;
let urlService: SemanticPathService;
let routingParamsService: RoutingParamsService;
let router: Router;
let location: Location;

beforeEach(() => {
TestBed.configureTestingModule({
Expand All @@ -44,6 +53,7 @@ describe('RoutingService', () => {
WindowRef,
{ provide: SemanticPathService, useClass: MockSemanticPathService },
{ provide: RoutingParamsService, useClass: MockRoutingParamsService },
{ provide: Location, useClass: MockLocation },
],
});

Expand All @@ -52,6 +62,8 @@ describe('RoutingService', () => {
winRef = TestBed.inject(WindowRef);
urlService = TestBed.inject(SemanticPathService);
routingParamsService = TestBed.inject(RoutingParamsService);
router = TestBed.inject(Router);
location = TestBed.inject(Location);
spyOn(store, 'dispatch');
});

Expand All @@ -60,31 +72,33 @@ describe('RoutingService', () => {
});

describe('go', () => {
it('should dispatch navigation action with generated path', () => {
spyOn(urlService, 'transform').and.returnValue(['generated', 'path']);
service.go([]);
expect(store.dispatch).toHaveBeenCalledWith(
new RoutingActions.RouteGoAction({
path: ['generated', 'path'],
extras: undefined,
})
);
it('should return Promise for the Angular navigation', () => {
const navigationPromise = Promise.resolve(true);
const queryParams = { test: true };
spyOn(urlService, 'transform').and.returnValue(['url']);
spyOn(router, 'navigate').and.returnValue(navigationPromise);
const result = service.go(['url'], { queryParams });
expect(router.navigate).toHaveBeenCalledWith(['url'], { queryParams });
expect(result).toBe(navigationPromise);
});

it('should call url service service with given array of commands', () => {
spyOn(urlService, 'transform');
it('should call url service with given array of commands', () => {
const commands = ['testString', { cxRoute: 'testRoute' }];
const resultPath = ['testString', 'testPath'];
spyOn(urlService, 'transform').and.returnValue(resultPath);
service.go(commands);
expect(urlService.transform).toHaveBeenCalledWith(commands);
});
});

describe('goByUrl', () => {
it('should dispatch GoByUrl action', () => {
service.goByUrl('test');
expect(store.dispatch).toHaveBeenCalledWith(
new RoutingActions.RouteGoByUrlAction('test')
);
it('should return Promise for the Angular navigation', () => {
const navigationPromise = Promise.resolve(true);
const queryParams = { test: true };
spyOn(router, 'navigateByUrl').and.returnValue(navigationPromise);
const result = service.goByUrl('url', { queryParams });
expect(router.navigateByUrl).toHaveBeenCalledWith('url', { queryParams });
expect(result).toBe(navigationPromise);
});
});

Expand Down Expand Up @@ -125,34 +139,26 @@ describe('RoutingService', () => {
});

describe('back', () => {
it('should dispatch back action', () => {
service.back();
expect(store.dispatch).toHaveBeenCalledWith(
new RoutingActions.RouteBackAction()
);
});

it('should go to homepage on back action when referer is not from the app', () => {
spyOnProperty(document, 'referrer', 'get').and.returnValue(
'http://foobar.com'
);
spyOn(service, 'go');
spyOn(urlService, 'transform').and.callFake((x) => x);
service.back();
expect(store.dispatch).toHaveBeenCalledWith(
new RoutingActions.RouteGoAction({
path: ['/'],
extras: undefined,
})
);
expect(service.go).toHaveBeenCalledWith(['/']);
});

it('should call Location.back', () => {
service.back();
expect(location.back).toHaveBeenCalled();
});
});

describe('forward', () => {
it('should dispatch forward action', () => {
it('should call Location.forward', () => {
service.forward();
expect(store.dispatch).toHaveBeenCalledWith(
new RoutingActions.RouteForwardAction()
);
expect(location.forward).toHaveBeenCalled();
});
});

Expand Down
Loading

0 comments on commit 051d195

Please sign in to comment.