Skip to content

Commit a1edb01

Browse files
atarix83vins01-4science
authored andcommitted
Merged in dspace-cris-2023_02_x-DSC-1511 (pull request DSpace#1494)
[DSC-1511] Fix issue for which navigating to item page could have lead to and hard redirect of the page when it shouldn't Approved-by: Vincenzo Mecca
2 parents 07b6d05 + a75af3e commit a1edb01

2 files changed

Lines changed: 74 additions & 22 deletions

File tree

src/app/item-page/item-page.resolver.spec.ts

Lines changed: 66 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { TestBed } from '@angular/core/testing';
77
import { RouterTestingModule } from '@angular/router/testing';
88
import { Router } from '@angular/router';
99
import { HardRedirectService } from '../core/services/hard-redirect.service';
10+
import { PLATFORM_ID } from '@angular/core';
1011

1112
describe('ItemPageResolver', () => {
1213
beforeEach(() => {
@@ -25,6 +26,7 @@ describe('ItemPageResolver', () => {
2526
let store;
2627
let router;
2728
let hardRedirectService: HardRedirectService ;
29+
let platformId;
2830
const uuid = '1234-65487-12354-1235';
2931
const item = Object.assign(new Item(), {
3032
id: uuid,
@@ -58,6 +60,7 @@ describe('ItemPageResolver', () => {
5860

5961
beforeEach(() => {
6062
router = TestBed.inject(Router);
63+
platformId = TestBed.inject(PLATFORM_ID);
6164
itemService = {
6265
findById: (id: string) => createSuccessfulRemoteDataObject$(item)
6366
} as any;
@@ -71,7 +74,7 @@ describe('ItemPageResolver', () => {
7174
});
7275

7376
spyOn(router, 'navigateByUrl');
74-
resolver = new ItemPageResolver(hardRedirectService, itemService, store, router);
77+
resolver = new ItemPageResolver(platformId, hardRedirectService, itemService, store, router);
7578
});
7679

7780
it('should resolve a an item from from the item with the url redirect', (done) => {
@@ -137,31 +140,74 @@ describe('ItemPageResolver', () => {
137140
});
138141

139142
spyOn(router, 'navigateByUrl');
140-
resolver = new ItemPageResolver(hardRedirectService, itemService, store, router);
141143
});
142144

143-
it('should redirect if it has not the new item url', (done) => {
144-
resolver.resolve({ params: { id: uuid } } as any, { url: '/items/1234-65487-12354-1235/edit' } as any)
145-
.pipe(first())
146-
.subscribe(
147-
(resolved) => {
148-
expect(hardRedirectService.redirect).toHaveBeenCalledWith('/entities/person/1234-65487-12354-1235/edit', 301);
149-
done();
150-
}
151-
);
145+
describe(' and platform is server', () => {
146+
147+
beforeEach(() => {
148+
platformId = 'server';
149+
resolver = new ItemPageResolver(platformId, hardRedirectService, itemService, store, router);
150+
});
151+
152+
it('should redirect if it has not the new item url', (done) => {
153+
resolver.resolve({ params: { id: uuid } } as any, { url: '/items/1234-65487-12354-1235/edit' } as any)
154+
.pipe(first())
155+
.subscribe(
156+
(resolved) => {
157+
expect(hardRedirectService.redirect).toHaveBeenCalledWith('/entities/person/1234-65487-12354-1235/edit', 301);
158+
expect(router.navigateByUrl).not.toHaveBeenCalled();
159+
done();
160+
}
161+
);
162+
});
163+
164+
it('should not redirect if it has the new item url', (done) => {
165+
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/person/1234-65487-12354-1235/edit' } as any)
166+
.pipe(first())
167+
.subscribe(
168+
(resolved) => {
169+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
170+
expect(router.navigateByUrl).not.toHaveBeenCalled();
171+
done();
172+
}
173+
);
174+
});
152175
});
153176

154-
it('should not redirect if it has the new item url', (done) => {
155-
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/person/1234-65487-12354-1235/edit' } as any)
156-
.pipe(first())
157-
.subscribe(
158-
(resolved) => {
159-
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
160-
done();
161-
}
162-
);
177+
describe(' and platform is browser', () => {
178+
179+
beforeEach(() => {
180+
platformId = 'browser';
181+
resolver = new ItemPageResolver(platformId, hardRedirectService, itemService, store, router);
182+
});
183+
184+
it('should redirect if it has not the new item url', (done) => {
185+
resolver.resolve({ params: { id: uuid } } as any, { url: '/items/1234-65487-12354-1235/edit' } as any)
186+
.pipe(first())
187+
.subscribe(
188+
(resolved) => {
189+
expect(router.navigateByUrl).toHaveBeenCalledWith('/entities/person/1234-65487-12354-1235/edit');
190+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
191+
done();
192+
}
193+
);
194+
});
195+
196+
it('should not redirect if it has the new item url', (done) => {
197+
resolver.resolve({ params: { id: uuid } } as any, { url: '/entities/person/1234-65487-12354-1235/edit' } as any)
198+
.pipe(first())
199+
.subscribe(
200+
(resolved) => {
201+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
202+
expect(router.navigateByUrl).not.toHaveBeenCalled();
203+
done();
204+
}
205+
);
206+
});
163207
});
164208

209+
210+
165211
});
166212

167213
});

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable } from '@angular/core';
1+
import { Inject, Injectable, PLATFORM_ID } from '@angular/core';
22
import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
33
import { Observable } from 'rxjs';
44
import { RemoteData } from '../core/data/remote-data';
@@ -10,6 +10,7 @@ import { hasValue, isNotEmpty } from '../shared/empty.util';
1010
import { getItemPageRoute } from './item-page-routing-paths';
1111
import { ItemResolver } from './item.resolver';
1212
import { HardRedirectService } from '../core/services/hard-redirect.service';
13+
import { isPlatformServer } from '@angular/common';
1314

1415
/**
1516
* This class represents a resolver that requests a specific item before the route is activated and will redirect to the
@@ -18,6 +19,7 @@ import { HardRedirectService } from '../core/services/hard-redirect.service';
1819
@Injectable()
1920
export class ItemPageResolver extends ItemResolver {
2021
constructor(
22+
@Inject(PLATFORM_ID) protected platformId: any,
2123
protected hardRedirectService: HardRedirectService,
2224
protected itemService: ItemDataService,
2325
protected store: Store<any>,
@@ -55,7 +57,11 @@ export class ItemPageResolver extends ItemResolver {
5557
if (!thisRoute.startsWith(itemRoute)) {
5658
const itemId = rd.payload.uuid;
5759
const subRoute = thisRoute.substring(thisRoute.indexOf(itemId) + itemId.length, thisRoute.length);
58-
this.hardRedirectService.redirect(itemRoute + subRoute, 301);
60+
if (isPlatformServer(this.platformId)) {
61+
this.hardRedirectService.redirect(itemRoute + subRoute, 301);
62+
} else {
63+
this.router.navigateByUrl(itemRoute + subRoute);
64+
}
5965
}
6066
}
6167
}

0 commit comments

Comments
 (0)