Skip to content

Commit 220b30b

Browse files
atarix83atarix83
committed
[CST-5729] Address review feedback
1 parent e12da0f commit 220b30b

9 files changed

Lines changed: 136 additions & 45 deletions

‎src/app/bitstream-page/bitstream-download-page/bitstream-download-page.component.spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { TranslateModule } from '@ngx-translate/core';
1313
import { CommonModule } from '@angular/common';
1414
import { SignpostingDataService } from '../../core/data/signposting-data.service';
1515
import { ServerResponseService } from '../../core/services/server-response.service';
16+
import { PLATFORM_ID } from '@angular/core';
1617

1718
describe('BitstreamDownloadPageComponent', () => {
1819
let component: BitstreamDownloadPageComponent;
@@ -99,7 +100,8 @@ describe('BitstreamDownloadPageComponent', () => {
99100
{ provide: FileService, useValue: fileService },
100101
{ provide: HardRedirectService, useValue: hardRedirectService },
101102
{ provide: ServerResponseService, useValue: serverResponseService },
102-
{ provide: SignpostingDataService, useValue: signpostingDataService }
103+
{ provide: SignpostingDataService, useValue: signpostingDataService },
104+
{ provide: PLATFORM_ID, useValue: 'server' }
103105
]
104106
})
105107
.compileComponents();

‎src/app/bitstream-page/bitstream-download-page/bitstream-download-page.component.ts‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Component, OnInit } from '@angular/core';
1+
import { Component, Inject, OnInit, PLATFORM_ID } from '@angular/core';
22
import { filter, map, switchMap, take } from 'rxjs/operators';
33
import { ActivatedRoute, Router } from '@angular/router';
44
import { hasValue, isNotEmpty } from '../../shared/empty.util';
@@ -13,7 +13,7 @@ import { HardRedirectService } from '../../core/services/hard-redirect.service';
1313
import { getForbiddenRoute } from '../../app-routing-paths';
1414
import { RemoteData } from '../../core/data/remote-data';
1515
import { redirectOn4xx } from '../../core/shared/authorized.operators';
16-
import { Location } from '@angular/common';
16+
import { isPlatformServer, Location } from '@angular/common';
1717
import { SignpostingDataService } from 'src/app/core/data/signposting-data.service';
1818
import { ServerResponseService } from 'src/app/core/services/server-response.service';
1919
import { SignpostingLink } from '../../core/data/signposting-links.model';
@@ -39,19 +39,10 @@ export class BitstreamDownloadPageComponent implements OnInit {
3939
private hardRedirectService: HardRedirectService,
4040
private location: Location,
4141
private signpostingDataService: SignpostingDataService,
42-
private responseService: ServerResponseService
42+
private responseService: ServerResponseService,
43+
@Inject(PLATFORM_ID) protected platformId: string
4344
) {
44-
this.route.params.subscribe(params => {
45-
this.signpostingDataService.getLinks(params.id).pipe(take(1)).subscribe((signpostingLinks: SignpostingLink[]) => {
46-
let links = '';
47-
48-
signpostingLinks.forEach((link: SignpostingLink) => {
49-
links = links + (isNotEmpty(links) ? ', ' : '') + `<${link.href}> ; rel="${link.rel}" ; type="${link.type}" `;
50-
});
51-
52-
this.responseService.setHeader('Link', links);
53-
});
54-
});
45+
this.initPageLinks();
5546
}
5647

5748
back(): void {
@@ -101,4 +92,25 @@ export class BitstreamDownloadPageComponent implements OnInit {
10192
}
10293
});
10394
}
95+
96+
/**
97+
* Create page links if any are retrieved by signposting endpoint
98+
*
99+
* @private
100+
*/
101+
private initPageLinks(): void {
102+
if (isPlatformServer(this.platformId)) {
103+
this.route.params.subscribe(params => {
104+
this.signpostingDataService.getLinks(params.id).pipe(take(1)).subscribe((signpostingLinks: SignpostingLink[]) => {
105+
let links = '';
106+
107+
signpostingLinks.forEach((link: SignpostingLink) => {
108+
links = links + (isNotEmpty(links) ? ', ' : '') + `<${link.href}> ; rel="${link.rel}" ; type="${link.type}" `;
109+
});
110+
111+
this.responseService.setHeader('Link', links);
112+
});
113+
});
114+
}
115+
}
104116
}

‎src/app/core/data/signposting-data.service.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import { HALEndpointService } from '../shared/hal-endpoint.service';
88
import { RawRestResponse } from '../dspace-rest/raw-rest-response.model';
99
import { SignpostingLink } from './signposting-links.model';
1010

11+
/**
12+
* Service responsible for handling requests related to the Signposting endpoint
13+
*/
1114
@Injectable({
1215
providedIn: 'root'
1316
})
@@ -16,6 +19,11 @@ export class SignpostingDataService {
1619
constructor(private restService: DspaceRestService, protected halService: HALEndpointService) {
1720
}
1821

22+
/**
23+
* Retrieve the list of signposting links related to the given resource's id
24+
*
25+
* @param uuid
26+
*/
1927
getLinks(uuid: string): Observable<SignpostingLink[]> {
2028
const baseUrl = this.halService.getRootHref().replace('/api', '');
2129

‎src/app/core/services/server-response.service.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { RESPONSE } from '@nguniversal/express-engine/tokens';
22
import { Inject, Injectable, Optional } from '@angular/core';
3+
34
import { Response } from 'express';
45

6+
/**
7+
* Service responsible to provide method to manage the response object
8+
*/
59
@Injectable()
610
export class ServerResponseService {
711
private response: Response;
@@ -10,6 +14,12 @@ export class ServerResponseService {
1014
this.response = response;
1115
}
1216

17+
/**
18+
* Set a status code to response
19+
*
20+
* @param code
21+
* @param message
22+
*/
1323
setStatus(code: number, message?: string): this {
1424
if (this.response) {
1525
this.response.statusCode = code;
@@ -20,22 +30,48 @@ export class ServerResponseService {
2030
return this;
2131
}
2232

33+
/**
34+
* Set Unauthorized status
35+
*
36+
* @param message
37+
*/
2338
setUnauthorized(message = 'Unauthorized'): this {
2439
return this.setStatus(401, message);
2540
}
2641

42+
/**
43+
* Set Forbidden status
44+
*
45+
* @param message
46+
*/
2747
setForbidden(message = 'Forbidden'): this {
2848
return this.setStatus(403, message);
2949
}
3050

51+
/**
52+
* Set Not found status
53+
*
54+
* @param message
55+
*/
3156
setNotFound(message = 'Not found'): this {
3257
return this.setStatus(404, message);
3358
}
3459

60+
/**
61+
* Set Internal Server Error status
62+
*
63+
* @param message
64+
*/
3565
setInternalServerError(message = 'Internal Server Error'): this {
3666
return this.setStatus(500, message);
3767
}
3868

69+
/**
70+
* Set a response's header
71+
*
72+
* @param header
73+
* @param content
74+
*/
3975
setHeader(header: string, content: string) {
4076
if (this.response) {
4177
this.response.setHeader(header, content);

‎src/app/init.service.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@ export abstract class InitService {
188188
this.breadcrumbsService.listenForRouteChanges();
189189
this.themeService.listenForRouteChanges();
190190
this.menuService.listenForRouteChanges();
191-
// this.metadataItem.checkCurrentRoute();
192191
}
193192

194193
/**

‎src/app/item-page/full/full-item-page.component.spec.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ComponentFixture, fakeAsync, TestBed, waitForAsync } from '@angular/cor
22
import { ItemDataService } from '../../core/data/item-data.service';
33
import { TranslateLoader, TranslateModule } from '@ngx-translate/core';
44
import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock';
5-
import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core';
5+
import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA, PLATFORM_ID } from '@angular/core';
66
import { TruncatePipe } from '../../shared/utils/truncate.pipe';
77
import { FullItemPageComponent } from './full-item-page.component';
88
import { MetadataService } from '../../core/metadata/metadata.service';
@@ -122,6 +122,7 @@ describe('FullItemPageComponent', () => {
122122
{ provide: ServerResponseService, useValue: serverResponseService },
123123
{ provide: SignpostingDataService, useValue: signpostingDataService },
124124
{ provide: LinkHeadService, useValue: linkHeadService },
125+
{ provide: PLATFORM_ID, useValue: 'server' }
125126
],
126127
schemas: [NO_ERRORS_SCHEMA]
127128
}).overrideComponent(FullItemPageComponent, {

‎src/app/item-page/full/full-item-page.component.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { filter, map } from 'rxjs/operators';
2-
import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from '@angular/core';
2+
import { ChangeDetectionStrategy, Component, Inject, OnDestroy, OnInit, PLATFORM_ID } from '@angular/core';
33
import { ActivatedRoute, Data, Router } from '@angular/router';
44

55
import { BehaviorSubject, Observable } from 'rxjs';
@@ -54,9 +54,10 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit,
5454
protected _location: Location,
5555
protected responseService: ServerResponseService,
5656
protected signpostingDataService: SignpostingDataService,
57-
protected linkHeadService: LinkHeadService
57+
protected linkHeadService: LinkHeadService,
58+
@Inject(PLATFORM_ID) protected platformId: string,
5859
) {
59-
super(route, router, items, authService, authorizationService, responseService, signpostingDataService, linkHeadService);
60+
super(route, router, items, authService, authorizationService, responseService, signpostingDataService, linkHeadService, platformId);
6061
}
6162

6263
/*** AoT inheritance fix, will hopefully be resolved in the near future **/

‎src/app/item-page/simple/item-page.component.spec.ts‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
22
import { TranslateLoader, TranslateModule } from '@ngx-translate/core';
33
import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock';
44
import { ItemDataService } from '../../core/data/item-data.service';
5-
import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core';
5+
import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA, PLATFORM_ID } from '@angular/core';
66
import { ItemPageComponent } from './item-page.component';
77
import { ActivatedRoute, Router } from '@angular/router';
88
import { ActivatedRouteStub } from '../../shared/testing/active-router.stub';
@@ -24,7 +24,8 @@ import { createPaginatedList } from '../../shared/testing/utils.test';
2424
import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
2525
import { ServerResponseService } from '../../core/services/server-response.service';
2626
import { SignpostingDataService } from '../../core/data/signposting-data.service';
27-
import { LinkHeadService } from '../../core/services/link-head.service';
27+
import { LinkDefinition, LinkHeadService } from '../../core/services/link-head.service';
28+
import { SignpostingLink } from '../../core/data/signposting-links.model';
2829

2930
const mockItem: Item = Object.assign(new Item(), {
3031
bundles: createSuccessfulRemoteDataObject$(createPaginatedList([])),
@@ -41,16 +42,18 @@ const mockWithdrawnItem: Item = Object.assign(new Item(), {
4142

4243
const mocklink = {
4344
href: 'http://test.org',
44-
rel: 'test',
45-
type: 'test'
45+
rel: 'rel1',
46+
type: 'type1'
4647
};
4748

4849
const mocklink2 = {
4950
href: 'http://test2.org',
50-
rel: 'test',
51-
type: 'test'
51+
rel: 'rel2',
52+
type: 'type2'
5253
};
5354

55+
const mockSignpostingLinks: SignpostingLink[] = [mocklink, mocklink2];
56+
5457
describe('ItemPageComponent', () => {
5558
let comp: ItemPageComponent;
5659
let fixture: ComponentFixture<ItemPageComponent>;
@@ -109,6 +112,7 @@ describe('ItemPageComponent', () => {
109112
{ provide: ServerResponseService, useValue: serverResponseService },
110113
{ provide: SignpostingDataService, useValue: signpostingDataService },
111114
{ provide: LinkHeadService, useValue: linkHeadService },
115+
{ provide: PLATFORM_ID, useValue: 'server' },
112116
],
113117

114118
schemas: [NO_ERRORS_SCHEMA]
@@ -165,6 +169,22 @@ describe('ItemPageComponent', () => {
165169
expect(linkHeadService.addTag).toHaveBeenCalledTimes(2);
166170
});
167171

172+
173+
it('should add link tags correctly', () => {
174+
175+
expect(comp.signpostingLinks).toEqual([mocklink, mocklink2]);
176+
177+
// Check if linkHeadService.addTag() was called with the correct arguments
178+
expect(linkHeadService.addTag).toHaveBeenCalledTimes(mockSignpostingLinks.length);
179+
expect(linkHeadService.addTag).toHaveBeenCalledWith(mockSignpostingLinks[0] as LinkDefinition);
180+
expect(linkHeadService.addTag).toHaveBeenCalledWith(mockSignpostingLinks[1] as LinkDefinition);
181+
});
182+
183+
it('should set Link header on the server', () => {
184+
185+
expect(serverResponseService.setHeader).toHaveBeenCalledWith('Link', '<http://test.org> ; rel="rel1" ; type="type1" , <http://test2.org> ; rel="rel2" ; type="type2" ');
186+
});
187+
168188
});
169189
describe('when the item is withdrawn and the user is not an admin', () => {
170190
beforeEach(() => {

‎src/app/item-page/simple/item-page.component.ts‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from '@angular/core';
1+
import { ChangeDetectionStrategy, Component, Inject, OnDestroy, OnInit, PLATFORM_ID } from '@angular/core';
22
import { ActivatedRoute, Router } from '@angular/router';
3+
import { isPlatformServer } from '@angular/common';
34

45
import { Observable } from 'rxjs';
56
import { map, take } from 'rxjs/operators';
@@ -75,25 +76,10 @@ export class ItemPageComponent implements OnInit, OnDestroy {
7576
protected authorizationService: AuthorizationDataService,
7677
protected responseService: ServerResponseService,
7778
protected signpostingDataService: SignpostingDataService,
78-
protected linkHeadService: LinkHeadService
79+
protected linkHeadService: LinkHeadService,
80+
@Inject(PLATFORM_ID) protected platformId: string
7981
) {
80-
this.route.params.subscribe(params => {
81-
this.signpostingDataService.getLinks(params.id).pipe(take(1)).subscribe((signpostingLinks: SignpostingLink[]) => {
82-
let links = '';
83-
this.signpostingLinks = signpostingLinks;
84-
85-
signpostingLinks.forEach((link: SignpostingLink) => {
86-
links = links + (isNotEmpty(links) ? ', ' : '') + `<${link.href}> ; rel="${link.rel}" ; type="${link.type}" `;
87-
this.linkHeadService.addTag({
88-
href: link.href,
89-
type: link.type,
90-
rel: link.rel
91-
});
92-
});
93-
94-
this.responseService.setHeader('Link', links);
95-
});
96-
});
82+
this.initPageLinks();
9783
}
9884

9985
/**
@@ -113,6 +99,32 @@ export class ItemPageComponent implements OnInit, OnDestroy {
11399

114100
}
115101

102+
/**
103+
* Create page links if any are retrieved by signposting endpoint
104+
*
105+
* @private
106+
*/
107+
private initPageLinks(): void {
108+
this.route.params.subscribe(params => {
109+
this.signpostingDataService.getLinks(params.id).pipe(take(1)).subscribe((signpostingLinks: SignpostingLink[]) => {
110+
let links = '';
111+
this.signpostingLinks = signpostingLinks;
112+
113+
signpostingLinks.forEach((link: SignpostingLink) => {
114+
links = links + (isNotEmpty(links) ? ', ' : '') + `<${link.href}> ; rel="${link.rel}" ; type="${link.type}" `;
115+
this.linkHeadService.addTag({
116+
href: link.href,
117+
type: link.type,
118+
rel: link.rel
119+
});
120+
});
121+
122+
if (isPlatformServer(this.platformId)) {
123+
this.responseService.setHeader('Link', links);
124+
}
125+
});
126+
});
127+
}
116128

117129
ngOnDestroy(): void {
118130
this.signpostingLinks.forEach((link: SignpostingLink) => {

0 commit comments

Comments
 (0)