Skip to content

Commit 85a295e

Browse files
vNovskiatarix83
authored andcommitted
Merged in DSC-1337-change-urls-behaviour-for-cris-item-page (pull request DSpace#993)
[DSC-1337] feature: changed urls behaviour for cris item page Approved-by: Giuseppe Digilio
2 parents e274d96 + 036d03a commit 85a295e

File tree

11 files changed

+218
-97
lines changed

11 files changed

+218
-97
lines changed

src/app/cris-item-page/cris-item-page-routing.module.ts

Lines changed: 0 additions & 68 deletions
This file was deleted.

src/app/cris-item-page/cris-item-page.module.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { CrisItemPageRoutingModule } from './cris-item-page-routing.module';
21
import { NgModule } from '@angular/core';
32
import { CommonModule } from '@angular/common';
43
import { SharedModule } from '../shared/shared.module';
@@ -17,7 +16,6 @@ import { ItemSharedModule } from '../item-page/item-shared.module';
1716
SharedModule,
1817
CrisLayoutModule,
1918
StatisticsModule,
20-
CrisItemPageRoutingModule,
2119
ItemSharedModule
2220
],
2321
exports: [

src/app/cris-layout/cris-layout-loader/shared/cris-layout-tabs/cris-layout-tabs.component.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,13 @@ export abstract class CrisLayoutTabsComponent {
108108
abstract emitSelected(selectedTab): void;
109109

110110
setActiveTab(tab) {
111+
const itemPageRoute = getItemPageRoute(this.item);
111112
this.activeTab$.next(tab);
112113
this.emitSelected(tab);
113-
if (isNotNull(this.route.snapshot.paramMap.get('tab'))) {
114-
this.location.replaceState(getItemPageRoute(this.item) + '/' + tab.shortname);
114+
if (this.tabs[0].shortname === tab.shortname) {
115+
this.location.replaceState(itemPageRoute);
116+
} else {
117+
this.location.replaceState(itemPageRoute + '/' + tab.shortname);
115118
}
116119
}
117120

src/app/cris-layout/cris-layout-matrix/cris-layout-box-container/boxes/relation/cris-layout-relation-box.component.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<ds-configuration-search-page *ngIf="configuration" class="w-100"
22
[fixedFilterQuery]="searchFilter"
33
[configuration]="configuration"
4+
[renderOnServerSide]="false"
45
[searchEnabled]="searchEnabled"
56
[showCharts]="true"
67
[showScopeSelector]="false"
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
import { take } from 'rxjs/operators';
2+
import { ItemDataService } from '../core/data/item-data.service';
3+
import { Item } from '../core/shared/item.model';
4+
import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
5+
import { TestBed } from '@angular/core/testing';
6+
import { RouterTestingModule } from '@angular/router/testing';
7+
import { Router } from '@angular/router';
8+
import { HardRedirectService } from '../core/services/hard-redirect.service';
9+
import { CrisItemPageTabResolver } from './cris-item-page-tab.resolver';
10+
import { TabDataService } from '../core/layout/tab-data.service';
11+
import { createPaginatedList } from '../shared/testing/utils.test';
12+
import { tabDetailsTest, tabPublicationsTest } from '../shared/testing/layout-tab.mocks';
13+
14+
describe('CrisItemPageTabResolver', () => {
15+
beforeEach(() => {
16+
TestBed.configureTestingModule({
17+
imports: [RouterTestingModule.withRoutes([{
18+
path: 'entities/:entity-type/:id/:tab',
19+
component: {} as any
20+
}])]
21+
});
22+
});
23+
24+
describe('when item exists', () => {
25+
let resolver: CrisItemPageTabResolver;
26+
const itemService: jasmine.SpyObj<ItemDataService> = jasmine.createSpyObj('ItemDataService', {
27+
'findById': jasmine.createSpy('findById')
28+
});
29+
const tabService: jasmine.SpyObj<TabDataService> = jasmine.createSpyObj('TabDataService', {
30+
'findByItem': jasmine.createSpy('findByItem')
31+
});
32+
let hardRedirectService: HardRedirectService;
33+
34+
let router;
35+
36+
const uuid = '1234-65487-12354-1235';
37+
const item = Object.assign(new Item(), {
38+
id: uuid,
39+
uuid: uuid,
40+
metadata: {
41+
'dspace.entity.type': [
42+
{
43+
value: 'Publication'
44+
}
45+
]
46+
}
47+
});
48+
49+
const tabsRD = createSuccessfulRemoteDataObject(createPaginatedList([tabPublicationsTest, tabDetailsTest]));
50+
const tabsRD$ = createSuccessfulRemoteDataObject$(createPaginatedList([tabPublicationsTest, tabDetailsTest]));
51+
52+
const noTabsRD = createSuccessfulRemoteDataObject(createPaginatedList([]));
53+
const noTabsRD$ = createSuccessfulRemoteDataObject$(createPaginatedList([]));
54+
55+
beforeEach(() => {
56+
router = TestBed.inject(Router);
57+
58+
itemService.findById.and.returnValue(createSuccessfulRemoteDataObject$(item));
59+
60+
hardRedirectService = jasmine.createSpyObj('HardRedirectService', {
61+
'redirect': jasmine.createSpy('redirect')
62+
});
63+
});
64+
65+
describe('and there tabs', () => {
66+
beforeEach(() => {
67+
68+
(tabService as any).findByItem.and.returnValue(tabsRD$);
69+
70+
spyOn(router, 'navigateByUrl');
71+
72+
resolver = new CrisItemPageTabResolver(hardRedirectService, tabService, itemService, router);
73+
});
74+
75+
it('should redirect to root route if given tab is the first one', (done) => {
76+
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235/publications' } as any)
77+
.pipe(take(1))
78+
.subscribe(
79+
(resolved) => {
80+
expect(router.navigateByUrl).not.toHaveBeenCalled();
81+
expect(hardRedirectService.redirect).toHaveBeenCalledWith('/entities/publication/1234-65487-12354-1235', 302);
82+
expect(resolved).toEqual(tabsRD);
83+
done();
84+
}
85+
);
86+
});
87+
88+
it('should not redirect to root route if tab different than the main one is given', (done) => {
89+
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235/details' } as any)
90+
.pipe(take(1))
91+
.subscribe(
92+
(resolved) => {
93+
expect(router.navigateByUrl).not.toHaveBeenCalled();
94+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
95+
expect(resolved).toEqual(tabsRD);
96+
done();
97+
}
98+
);
99+
});
100+
101+
it('should not redirect to root route if no tab is given', (done) => {
102+
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235' } as any)
103+
.pipe(take(1))
104+
.subscribe(
105+
(resolved) => {
106+
expect(router.navigateByUrl).not.toHaveBeenCalled();
107+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
108+
expect(resolved).toEqual(tabsRD);
109+
done();
110+
}
111+
);
112+
});
113+
114+
it('should navigate to 404 if a wrong tab is given', (done) => {
115+
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235/test' } as any)
116+
.pipe(take(1))
117+
.subscribe(
118+
(resolved) => {
119+
expect(router.navigateByUrl).toHaveBeenCalled();
120+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
121+
expect(resolved).toEqual(tabsRD);
122+
done();
123+
}
124+
);
125+
});
126+
});
127+
128+
describe('and there no tabs', () => {
129+
beforeEach(() => {
130+
131+
(tabService as any).findByItem.and.returnValue(noTabsRD$);
132+
133+
spyOn(router, 'navigateByUrl');
134+
135+
resolver = new CrisItemPageTabResolver(hardRedirectService, tabService, itemService, router);
136+
});
137+
138+
it('should not redirect nor navigate', (done) => {
139+
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235' } as any)
140+
.pipe(take(1))
141+
.subscribe(
142+
(resolved) => {
143+
expect(router.navigateByUrl).not.toHaveBeenCalled();
144+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
145+
expect(resolved).toEqual(noTabsRD);
146+
done();
147+
}
148+
);
149+
});
150+
});
151+
});
152+
});

src/app/item-page/cris-item-page-tab.resolver.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable } from '@angular/core';
1+
import { Injectable} from '@angular/core';
22
import { ActivatedRouteSnapshot, Resolve, Router, RouterStateSnapshot } from '@angular/router';
33

44
import { Observable } from 'rxjs';
@@ -13,6 +13,8 @@ import { getFirstCompletedRemoteData } from '../core/shared/operators';
1313
import { Item } from '../core/shared/item.model';
1414
import { getItemPageRoute } from './item-page-routing-paths';
1515
import { createFailedRemoteDataObject$ } from '../shared/remote-data.utils';
16+
import { HardRedirectService } from '../core/services/hard-redirect.service';
17+
import { getPageNotFoundRoute } from '../app-routing-paths';
1618

1719
/**
1820
* This class represents a resolver that requests the tabs of specific
@@ -21,7 +23,11 @@ import { createFailedRemoteDataObject$ } from '../shared/remote-data.utils';
2123
@Injectable()
2224
export class CrisItemPageTabResolver implements Resolve<RemoteData<PaginatedList<CrisLayoutTab>>> {
2325

24-
constructor(private tabService: TabDataService, private itemDataService: ItemDataService, private router: Router) { }
26+
constructor(
27+
private hardRedirectService: HardRedirectService,
28+
private tabService: TabDataService,
29+
private itemDataService: ItemDataService,
30+
private router: Router) { }
2531

2632
/**
2733
* Method for resolving the tabs of item based on the parameters in the current route
@@ -45,18 +51,16 @@ export class CrisItemPageTabResolver implements Resolve<RemoteData<PaginatedList
4551
if (tabsRD.hasSucceeded && tabsRD?.payload?.page?.length > 0) {
4652
// By splitting the url with uuid we can understand if the item is primary item page or a tab
4753
const urlSplit = state.url.split(route.params.id);
48-
// If a no or wrong tab is given redirect to the first tab available
49-
if (!!tabsRD.payload && !!tabsRD.payload.page && tabsRD.payload.page.length > 0 && !urlSplit[1]) {
50-
const selectedTab = tabsRD.payload.page.filter((tab) => !tab.leading)[0];
51-
if (!!selectedTab) {
52-
let tabName = selectedTab.shortname;
53-
54-
if (tabName.includes('::')) {
55-
tabName = tabName.split('::')[1];
56-
}
57-
58-
this.router.navigateByUrl(getItemPageRoute(itemRD.payload) + '/' + tabName);
59-
}
54+
const givenTab = urlSplit[1];
55+
const itemPageRoute = getItemPageRoute(itemRD.payload);
56+
const isValidTab = tabsRD.payload.page.some((tab) => !givenTab || `/${tab.shortname}` === givenTab);
57+
const mainTab = tabsRD.payload.page.filter((tab) => !tab.leading)[0];
58+
if (!isValidTab) {
59+
// If wrong tab is given redirect to 404 page
60+
this.router.navigateByUrl(getPageNotFoundRoute(), { skipLocationChange: true, replaceUrl: false });
61+
} else if (givenTab === `/${mainTab.shortname}`) {
62+
// If first tab is given redirect to root item page
63+
this.hardRedirectService.redirect(itemPageRoute, 302);
6064
}
6165
}
6266
return tabsRD;

src/app/item-page/item-page-routing.module.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,17 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver';
2929
resolve: {
3030
dso: ItemPageResolver,
3131
breadcrumb: ItemBreadcrumbResolver,
32-
menu: DSOEditMenuResolver,
33-
tabs: CrisItemPageTabResolver
32+
menu: DSOEditMenuResolver
3433
},
3534
runGuardsAndResolvers: 'always',
3635
children: [
3736
{
3837
path: '',
3938
component: ThemedItemPageComponent,
4039
pathMatch: 'full',
40+
resolve: {
41+
tabs: CrisItemPageTabResolver
42+
}
4143
},
4244
{
4345
path: 'full',
@@ -63,6 +65,13 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver';
6365
path: ORCID_PATH,
6466
component: OrcidPageComponent,
6567
canActivate: [AuthenticatedGuard, OrcidPageGuard]
68+
},
69+
{
70+
path: ':tab',
71+
component: ThemedItemPageComponent,
72+
resolve: {
73+
tabs: CrisItemPageTabResolver
74+
},
6675
}
6776
],
6877
data: {

0 commit comments

Comments
 (0)