Skip to content

Commit

Permalink
[DSC-1337] Improve redirecting code and add unit test
Browse files Browse the repository at this point in the history
  • Loading branch information
atarix83 committed Nov 28, 2023
1 parent d9db5ca commit 0564b8e
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 36 deletions.
152 changes: 152 additions & 0 deletions src/app/item-page/cris-item-page-tab.resolver.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
import { take } from 'rxjs/operators';
import { ItemDataService } from '../core/data/item-data.service';
import { Item } from '../core/shared/item.model';
import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
import { TestBed } from '@angular/core/testing';
import { RouterTestingModule } from '@angular/router/testing';
import { Router } from '@angular/router';
import { HardRedirectService } from '../core/services/hard-redirect.service';
import { CrisItemPageTabResolver } from './cris-item-page-tab.resolver';
import { TabDataService } from '../core/layout/tab-data.service';
import { createPaginatedList } from '../shared/testing/utils.test';
import { tabDetailsTest, tabPublicationsTest } from '../shared/testing/layout-tab.mocks';

describe('CrisItemPageTabResolver', () => {
beforeEach(() => {
TestBed.configureTestingModule({
imports: [RouterTestingModule.withRoutes([{
path: 'entities/:entity-type/:id/:tab',
component: {} as any
}])]
});
});

describe('when item exists', () => {
let resolver: CrisItemPageTabResolver;
const itemService: jasmine.SpyObj<ItemDataService> = jasmine.createSpyObj('ItemDataService', {
'findById': jasmine.createSpy('findById')
});
const tabService: jasmine.SpyObj<TabDataService> = jasmine.createSpyObj('TabDataService', {
'findByItem': jasmine.createSpy('findByItem')
});
let hardRedirectService: HardRedirectService;

let router;

const uuid = '1234-65487-12354-1235';
const item = Object.assign(new Item(), {
id: uuid,
uuid: uuid,
metadata: {
'dspace.entity.type': [
{
value: 'Publication'
}
]
}
});

const tabsRD = createSuccessfulRemoteDataObject(createPaginatedList([tabPublicationsTest, tabDetailsTest]));
const tabsRD$ = createSuccessfulRemoteDataObject$(createPaginatedList([tabPublicationsTest, tabDetailsTest]));

const noTabsRD = createSuccessfulRemoteDataObject(createPaginatedList([]));
const noTabsRD$ = createSuccessfulRemoteDataObject$(createPaginatedList([]));

beforeEach(() => {
router = TestBed.inject(Router);

itemService.findById.and.returnValue(createSuccessfulRemoteDataObject$(item));

hardRedirectService = jasmine.createSpyObj('HardRedirectService', {
'redirect': jasmine.createSpy('redirect')
});
});

describe('and there tabs', () => {
beforeEach(() => {

(tabService as any).findByItem.and.returnValue(tabsRD$);

spyOn(router, 'navigateByUrl');

resolver = new CrisItemPageTabResolver(hardRedirectService, tabService, itemService, router);
});

it('should redirect to root route if given tab is the first one', (done) => {
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235/publications' } as any)
.pipe(take(1))
.subscribe(
(resolved) => {
expect(router.navigateByUrl).not.toHaveBeenCalled();
expect(hardRedirectService.redirect).toHaveBeenCalledWith('/entities/publication/1234-65487-12354-1235', 302);
expect(resolved).toEqual(tabsRD);
done();
}
);
});

it('should not redirect to root route if tab different than the main one is given', (done) => {
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235/details' } as any)
.pipe(take(1))
.subscribe(
(resolved) => {
expect(router.navigateByUrl).not.toHaveBeenCalled();
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
expect(resolved).toEqual(tabsRD);
done();
}
);
});

it('should not redirect to root route if no tab is given', (done) => {
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235' } as any)
.pipe(take(1))
.subscribe(
(resolved) => {
expect(router.navigateByUrl).not.toHaveBeenCalled();
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
expect(resolved).toEqual(tabsRD);
done();
}
);
});

it('should navigate to 404 if a wrong tab is given', (done) => {
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235/test' } as any)
.pipe(take(1))
.subscribe(
(resolved) => {
expect(router.navigateByUrl).toHaveBeenCalled();
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
expect(resolved).toEqual(tabsRD);
done();
}
);
});
});

describe('and there no tabs', () => {
beforeEach(() => {

(tabService as any).findByItem.and.returnValue(noTabsRD$);

spyOn(router, 'navigateByUrl');

resolver = new CrisItemPageTabResolver(hardRedirectService, tabService, itemService, router);
});

it('should not redirect nor navigate', (done) => {
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235' } as any)
.pipe(take(1))
.subscribe(
(resolved) => {
expect(router.navigateByUrl).not.toHaveBeenCalled();
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
expect(resolved).toEqual(noTabsRD);
done();
}
);
});
});
});
});
27 changes: 11 additions & 16 deletions src/app/item-page/cris-item-page-tab.resolver.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Injectable, Inject, PLATFORM_ID} from '@angular/core';
import { Injectable} from '@angular/core';
import { ActivatedRouteSnapshot, Resolve, Router, RouterStateSnapshot } from '@angular/router';
import { isPlatformServer } from '@angular/common';

import { Observable } from 'rxjs';
import { map, switchMap } from 'rxjs/operators';
Expand All @@ -15,6 +14,7 @@ import { Item } from '../core/shared/item.model';
import { getItemPageRoute } from './item-page-routing-paths';
import { createFailedRemoteDataObject$ } from '../shared/remote-data.utils';
import { HardRedirectService } from '../core/services/hard-redirect.service';
import { getPageNotFoundRoute } from '../app-routing-paths';

/**
* This class represents a resolver that requests the tabs of specific
Expand All @@ -24,7 +24,6 @@ import { HardRedirectService } from '../core/services/hard-redirect.service';
export class CrisItemPageTabResolver implements Resolve<RemoteData<PaginatedList<CrisLayoutTab>>> {

constructor(
@Inject(PLATFORM_ID) private platformId: any,
private hardRedirectService: HardRedirectService,
private tabService: TabDataService,
private itemDataService: ItemDataService,
Expand Down Expand Up @@ -52,20 +51,16 @@ export class CrisItemPageTabResolver implements Resolve<RemoteData<PaginatedList
if (tabsRD.hasSucceeded && tabsRD?.payload?.page?.length > 0) {
// By splitting the url with uuid we can understand if the item is primary item page or a tab
const urlSplit = state.url.split(route.params.id);
const givenTab = urlSplit[1];
const itemPageRoute = getItemPageRoute(itemRD.payload);
const isValidTab = urlSplit[1] && tabsRD.payload.page.some(tab => `/${tab.shortname}` === urlSplit[1]);
const selectedTab = tabsRD.payload.page.filter((tab) => !tab.leading)[0];
// If a no or wrong tab is given redirect to the first tab available
if (!!tabsRD.payload && !!tabsRD.payload.page && tabsRD.payload.page.length > 0 && !isValidTab) {
if (urlSplit[1]) {
if (isPlatformServer(this.platformId)) {
this.hardRedirectService.redirect(itemPageRoute, 301);
} else {
this.router.navigateByUrl(itemPageRoute);
}
}
} else if (urlSplit[1] === `/${selectedTab.shortname}` && isPlatformServer(this.platformId)) {
this.hardRedirectService.redirect(itemPageRoute, 301);
const isValidTab = tabsRD.payload.page.some((tab) => !givenTab || `/${tab.shortname}` === givenTab);
const mainTab = tabsRD.payload.page.filter((tab) => !tab.leading)[0];
if (!isValidTab) {
// If wrong tab is given redirect to 404 page
this.router.navigateByUrl(getPageNotFoundRoute(), { skipLocationChange: true, replaceUrl: false });
} else if (givenTab === `/${mainTab.shortname}`) {
// If first tab is given redirect to root item page
this.hardRedirectService.redirect(itemPageRoute, 302);
}
}
return tabsRD;
Expand Down
13 changes: 11 additions & 2 deletions src/app/item-page/item-page-routing.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,17 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver';
resolve: {
dso: ItemPageResolver,
breadcrumb: ItemBreadcrumbResolver,
menu: DSOEditMenuResolver,
tabs: CrisItemPageTabResolver
menu: DSOEditMenuResolver
},
runGuardsAndResolvers: 'always',
children: [
{
path: '',
component: ThemedItemPageComponent,
pathMatch: 'full',
resolve: {
tabs: CrisItemPageTabResolver
}
},
{
path: 'full',
Expand All @@ -63,6 +65,13 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver';
path: ORCID_PATH,
component: OrcidPageComponent,
canActivate: [AuthenticatedGuard, OrcidPageGuard]
},
{
path: ':tab',
component: ThemedItemPageComponent,
resolve: {
tabs: CrisItemPageTabResolver
},
}
],
data: {
Expand Down
36 changes: 26 additions & 10 deletions src/app/item-page/item-page.resolver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { TestBed } from '@angular/core/testing';
import { RouterTestingModule } from '@angular/router/testing';
import { Router } from '@angular/router';
import { HardRedirectService } from '../core/services/hard-redirect.service';
import { PLATFORM_ID } from '@angular/core';

describe('ItemPageResolver', () => {
beforeEach(() => {
Expand All @@ -25,8 +24,7 @@ describe('ItemPageResolver', () => {

let store;
let router;
let hardRedirectService: HardRedirectService;
let platformId;
let hardRedirectService: HardRedirectService ;
const uuid = '1234-65487-12354-1235';
const item = Object.assign(new Item(), {
id: uuid,
Expand Down Expand Up @@ -60,7 +58,6 @@ describe('ItemPageResolver', () => {

beforeEach(() => {
router = TestBed.inject(Router);
platformId = TestBed.inject(PLATFORM_ID);
itemService = {
findById: (id: string) => createSuccessfulRemoteDataObject$(item)
} as any;
Expand All @@ -69,8 +66,12 @@ describe('ItemPageResolver', () => {
dispatch: {},
});

hardRedirectService = jasmine.createSpyObj('HardRedirectService', {
'redirect': jasmine.createSpy('redirect')
});

spyOn(router, 'navigateByUrl');
resolver = new ItemPageResolver(platformId, hardRedirectService, itemService, store, router);
resolver = new ItemPageResolver(hardRedirectService, itemService, store, router);
});

it('should resolve a an item from from the item with the url redirect', (done) => {
Expand Down Expand Up @@ -111,7 +112,7 @@ describe('ItemPageResolver', () => {
.pipe(first())
.subscribe(
(resolved) => {
expect(router.navigateByUrl).not.toHaveBeenCalledWith('/entities/person/customurl/edit');
expect(hardRedirectService.redirect).not.toHaveBeenCalledWith('/entities/person/customurl/edit');
done();
}
);
Expand All @@ -131,16 +132,31 @@ describe('ItemPageResolver', () => {
dispatch: {},
});

hardRedirectService = jasmine.createSpyObj('HardRedirectService', {
'redirect': jasmine.createSpy('redirect')
});

spyOn(router, 'navigateByUrl');
resolver = new ItemPageResolver(platformId, hardRedirectService, itemService, store, router);
resolver = new ItemPageResolver(hardRedirectService, itemService, store, router);
});

it('should not call custom url', (done) => {
resolver.resolve({ params: { id: uuid } } as any, { url: 'test-url/1234-65487-12354-1235/edit' } as any)
it('should redirect if it has not the new item url', (done) => {
resolver.resolve({ params: { id: uuid } } as any, { url: '/items/1234-65487-12354-1235/edit' } as any)
.pipe(first())
.subscribe(
(resolved) => {
expect(hardRedirectService.redirect).toHaveBeenCalledWith('/entities/person/1234-65487-12354-1235/edit', 301);
done();
}
);
});

it('should not redirect if it has the new item url', (done) => {
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/person/1234-65487-12354-1235/edit' } as any)
.pipe(first())
.subscribe(
(resolved) => {
expect(router.navigateByUrl).toHaveBeenCalledWith('/entities/person/1234-65487-12354-1235/edit');
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
done();
}
);
Expand Down
10 changes: 2 additions & 8 deletions src/app/item-page/item-page.resolver.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Inject, Injectable, PLATFORM_ID } from '@angular/core';
import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
import { Observable } from 'rxjs';
import { RemoteData } from '../core/data/remote-data';
Expand All @@ -10,7 +10,6 @@ import { hasValue, isNotEmpty } from '../shared/empty.util';
import { getItemPageRoute } from './item-page-routing-paths';
import { ItemResolver } from './item.resolver';
import { HardRedirectService } from '../core/services/hard-redirect.service';
import { isPlatformServer } from '@angular/common';

/**
* This class represents a resolver that requests a specific item before the route is activated and will redirect to the
Expand All @@ -19,7 +18,6 @@ import { isPlatformServer } from '@angular/common';
@Injectable()
export class ItemPageResolver extends ItemResolver {
constructor(
@Inject(PLATFORM_ID) protected platformId: any,
protected hardRedirectService: HardRedirectService,
protected itemService: ItemDataService,
protected store: Store<any>,
Expand Down Expand Up @@ -57,11 +55,7 @@ export class ItemPageResolver extends ItemResolver {
if (!thisRoute.startsWith(itemRoute)) {
const itemId = rd.payload.uuid;
const subRoute = thisRoute.substring(thisRoute.indexOf(itemId) + itemId.length, thisRoute.length);
if (isPlatformServer(this.platformId)) {
this.hardRedirectService.redirect(itemRoute + subRoute, 301);
} else {
this.router.navigateByUrl(itemRoute + subRoute);
}
this.hardRedirectService.redirect(itemRoute + subRoute, 301);
}
}
}
Expand Down

0 comments on commit 0564b8e

Please sign in to comment.