From dde57b93871c544a903721e526d3d3e430cb2cb7 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Mon, 31 May 2021 16:19:44 +0200 Subject: [PATCH 01/20] update citation_pdf_url logic --- src/app/core/metadata/metadata.service.ts | 133 ++++++++++++++++++---- 1 file changed, 111 insertions(+), 22 deletions(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 807f7a42ab..268d361567 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -5,10 +5,10 @@ import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; -import { BehaviorSubject, combineLatest, Observable } from 'rxjs'; -import { catchError, distinctUntilKeyChanged, filter, first, map, take } from 'rxjs/operators'; +import { BehaviorSubject, combineLatest, Observable, of as observableOf, EMPTY } from 'rxjs'; +import { distinctUntilKeyChanged, filter, map, take, switchMap, expand } from 'rxjs/operators'; -import { hasValue, isNotEmpty } from '../../shared/empty.util'; +import { hasValue, hasNoValue } from '../../shared/empty.util'; import { DSONameService } from '../breadcrumbs/dso-name.service'; import { CacheableObject } from '../cache/object-cache.reducer'; import { BitstreamDataService } from '../data/bitstream-data.service'; @@ -21,11 +21,15 @@ import { DSpaceObject } from '../shared/dspace-object.model'; import { Item } from '../shared/item.model'; import { getFirstSucceededRemoteDataPayload, - getFirstSucceededRemoteListPayload + getFirstCompletedRemoteData } from '../shared/operators'; import { environment } from '../../../environments/environment'; import { RootDataService } from '../data/root-data.service'; import { getBitstreamDownloadRoute } from '../../app-routing-paths'; +import { BundleDataService } from '../data/bundle-data.service'; +import { followLink } from '../../shared/utils/follow-link-config.model'; +import { Bundle } from '../shared/bundle.model'; +import { PaginatedList } from '../data/paginated-list.model'; @Injectable() export class MetadataService { @@ -42,6 +46,7 @@ export class MetadataService { private meta: Meta, private title: Title, private dsoNameService: DSONameService, + private bundleDataService: BundleDataService, private bitstreamDataService: BitstreamDataService, private bitstreamFormatDataService: BitstreamFormatDataService, private rootService: RootDataService @@ -275,26 +280,110 @@ export class MetadataService { private setCitationPdfUrlTag(): void { if (this.currentObject.value instanceof Item) { const item = this.currentObject.value as Item; - this.bitstreamDataService.findAllByItemAndBundleName(item, 'ORIGINAL') - .pipe( - getFirstSucceededRemoteListPayload(), - first((files) => isNotEmpty(files)), - catchError((error) => { - console.debug(error.message); - return []; - })) - .subscribe((bitstreams: Bitstream[]) => { - for (const bitstream of bitstreams) { - this.bitstreamFormatDataService.findByBitstream(bitstream).pipe( - getFirstSucceededRemoteDataPayload() - ).subscribe((format: BitstreamFormat) => { - if (format.mimetype === 'application/pdf') { - const bitstreamLink = getBitstreamDownloadRoute(bitstream); - this.addMetaTag('citation_pdf_url', bitstreamLink); + + // Retrieve the ORIGINAL bundle for the item + this.bundleDataService.findByItemAndName( + item, + 'ORIGINAL', + true, + true, + followLink('primaryBitstream'), + followLink('bitstreams', + undefined, + true, + true, + true, + followLink('format') + ) + ).pipe( + getFirstSucceededRemoteDataPayload(), + switchMap((bundle: Bundle) => + + // First try the primary bitstream + bundle.primaryBitstream.pipe( + getFirstCompletedRemoteData(), + map((rd: RemoteData) => { + if (hasValue(rd.payload)) { + return rd.payload; + } else { + return null; } - }); + }), + // return the bundle as well so we can use it again if there's no primary bitstream + map((bitstream: Bitstream) => [bundle, bitstream]) + ) + ), + switchMap(([bundle, primaryBitstream]: [Bundle, Bitstream]) => { + if (hasValue(primaryBitstream)) { + // If there was a primary bitstream, emit its link + return [getBitstreamDownloadRoute(primaryBitstream)]; + } else { + // Otherwise consider the regular bitstreams in the bundle + return bundle.bitstreams.pipe( + getFirstCompletedRemoteData(), + switchMap((bitstreamRd: RemoteData>) => { + if (hasValue(bitstreamRd.payload) && bitstreamRd.payload.totalElements === 1) { + // If there's only one bitstream in the bundle, emit its link + return [getBitstreamDownloadRoute(bitstreamRd.payload.page[0])]; + } else { + // Otherwise check all bitstreams to see if one matches the format whitelist + return observableOf(bitstreamRd.payload).pipe( + // Because there can be more than one page of bitstreams, this expand operator + // will retrieve them in turn due to the take(1) at the bottom, it will only + // retrieve pages until a match is found + expand((paginatedList: PaginatedList) => { + if (hasNoValue(paginatedList.next)) { + // If there's no next page, stop. + return EMPTY; + } else { + // Otherwise retrieve the next page + return this.bitstreamDataService.findAllByHref( + paginatedList.next, + undefined, + true, + true, + followLink('format') + ).pipe( + getFirstCompletedRemoteData(), + map((next: RemoteData>) => { + if (hasValue(next.payload)) { + return next.payload; + } else { + return EMPTY; + } + }) + ); + } + }), + // Return the array of bitstreams inside each paginated list + map((paginatedList: PaginatedList) => paginatedList.page), + // Emit the bitstreams in the list one at a time + switchMap((bitstreams: Bitstream[]) => bitstreams), + // Retrieve the format for each bitstream + switchMap((bitstream: Bitstream) => bitstream.format.pipe( + getFirstSucceededRemoteDataPayload(), + // Keep the original bitstream, because it, not the format, is what we'll need + // for the link at the end + map((format: BitstreamFormat) => [bitstream, format]) + )), + // Filter out only pairs with whitelisted formats + filter(([, format]: [Bitstream, BitstreamFormat]) => + hasValue(format) && format.mimetype === 'application/pdf'), // TODO change to check map of mimetypes + // We only need 1 + take(1), + // Emit the link of the match + map(([bitstream, ]: [Bitstream, BitstreamFormat]) => getBitstreamDownloadRoute(bitstream)) + ); + } + }) + ); } - }); + }), + take(1) + ).subscribe((link: string) => { + // Use the found link to set the tag + this.addMetaTag('citation_pdf_url', link); + }); } } From 9fe5a91bc2894b7c6fb04e28190323c4efc34d19 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 3 Jun 2021 16:30:57 +0200 Subject: [PATCH 02/20] 79768: Extract download route method for Bundles with multiple Bitstreams --- src/app/core/metadata/metadata.service.ts | 98 ++++++++++++----------- 1 file changed, 51 insertions(+), 47 deletions(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 268d361567..fa4e81a5d7 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -327,53 +327,7 @@ export class MetadataService { return [getBitstreamDownloadRoute(bitstreamRd.payload.page[0])]; } else { // Otherwise check all bitstreams to see if one matches the format whitelist - return observableOf(bitstreamRd.payload).pipe( - // Because there can be more than one page of bitstreams, this expand operator - // will retrieve them in turn due to the take(1) at the bottom, it will only - // retrieve pages until a match is found - expand((paginatedList: PaginatedList) => { - if (hasNoValue(paginatedList.next)) { - // If there's no next page, stop. - return EMPTY; - } else { - // Otherwise retrieve the next page - return this.bitstreamDataService.findAllByHref( - paginatedList.next, - undefined, - true, - true, - followLink('format') - ).pipe( - getFirstCompletedRemoteData(), - map((next: RemoteData>) => { - if (hasValue(next.payload)) { - return next.payload; - } else { - return EMPTY; - } - }) - ); - } - }), - // Return the array of bitstreams inside each paginated list - map((paginatedList: PaginatedList) => paginatedList.page), - // Emit the bitstreams in the list one at a time - switchMap((bitstreams: Bitstream[]) => bitstreams), - // Retrieve the format for each bitstream - switchMap((bitstream: Bitstream) => bitstream.format.pipe( - getFirstSucceededRemoteDataPayload(), - // Keep the original bitstream, because it, not the format, is what we'll need - // for the link at the end - map((format: BitstreamFormat) => [bitstream, format]) - )), - // Filter out only pairs with whitelisted formats - filter(([, format]: [Bitstream, BitstreamFormat]) => - hasValue(format) && format.mimetype === 'application/pdf'), // TODO change to check map of mimetypes - // We only need 1 - take(1), - // Emit the link of the match - map(([bitstream, ]: [Bitstream, BitstreamFormat]) => getBitstreamDownloadRoute(bitstream)) - ); + return this.getBitstreamDownloadRoute(bitstreamRd); } }) ); @@ -387,6 +341,56 @@ export class MetadataService { } } + private getBitstreamDownloadRoute(bitstreamRd: RemoteData>): Observable { + return observableOf(bitstreamRd.payload).pipe( + // Because there can be more than one page of bitstreams, this expand operator + // will retrieve them in turn due to the take(1) at the bottom, it will only + // retrieve pages until a match is found + expand((paginatedList: PaginatedList) => { + if (hasNoValue(paginatedList.next)) { + // If there's no next page, stop. + return EMPTY; + } else { + // Otherwise retrieve the next page + return this.bitstreamDataService.findAllByHref( + paginatedList.next, + undefined, + true, + true, + followLink('format') + ).pipe( + getFirstCompletedRemoteData(), + map((next: RemoteData>) => { + if (hasValue(next.payload)) { + return next.payload; + } else { + return EMPTY; + } + }) + ); + } + }), + // Return the array of bitstreams inside each paginated list + map((paginatedList: PaginatedList) => paginatedList.page), + // Emit the bitstreams in the list one at a time + switchMap((bitstreams: Bitstream[]) => bitstreams), + // Retrieve the format for each bitstream + switchMap((bitstream: Bitstream) => bitstream.format.pipe( + getFirstSucceededRemoteDataPayload(), + // Keep the original bitstream, because it, not the format, is what we'll need + // for the link at the end + map((format: BitstreamFormat) => [bitstream, format]) + )), + // Filter out only pairs with whitelisted formats + filter(([, format]: [Bitstream, BitstreamFormat]) => + hasValue(format) && format.mimetype === 'application/pdf'), // TODO change to check map of mimetypes + // We only need 1 + take(1), + // Emit the link of the match + map(([bitstream, ]: [Bitstream, BitstreamFormat]) => getBitstreamDownloadRoute(bitstream)) + ); + } + /** * Add to the containing the current DSpace version */ From 1caba78b4df6b9fcec8a99c8d10010c0596da0d8 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 3 Jun 2021 16:33:58 +0200 Subject: [PATCH 03/20] 79768: Check against list of allowed mimetypes --- src/app/core/metadata/metadata.service.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index fa4e81a5d7..0b63e13976 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -40,6 +40,15 @@ export class MetadataService { private currentObject: BehaviorSubject; + private readonly ALLOWED_MIMETYPES = [ + 'application/pdf', // .pdf + 'application/postscript', // .ps + 'application/msword', // .doc + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document ', // .docx + 'application/rtf', // .rtf + 'application/epub+zip', // .epub + ]; + constructor( private router: Router, private translate: TranslateService, @@ -383,7 +392,7 @@ export class MetadataService { )), // Filter out only pairs with whitelisted formats filter(([, format]: [Bitstream, BitstreamFormat]) => - hasValue(format) && format.mimetype === 'application/pdf'), // TODO change to check map of mimetypes + hasValue(format) && this.ALLOWED_MIMETYPES.includes(format.mimetype)), // We only need 1 take(1), // Emit the link of the match From cb4446b79d17e7acc20b82f4855ef94e39307592 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 4 Jun 2021 13:57:30 +0200 Subject: [PATCH 04/20] 79768: Update & add unit tests --- .../core/metadata/metadata.service.spec.ts | 83 +++++++++++-- src/app/core/metadata/metadata.service.ts | 2 +- src/app/shared/mocks/item.mock.ts | 117 ++++++++++++------ 3 files changed, 154 insertions(+), 48 deletions(-) diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 18421dd489..0a4377e1b7 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -15,11 +15,9 @@ import { RemoteData } from '../data/remote-data'; import { Item } from '../shared/item.model'; import { - ItemMock, - MockBitstream1, - MockBitstream2, - MockBitstreamFormat1, - MockBitstreamFormat2 + ItemMock, MockBitstream1, MockBitstream2, MockBitstream3, MockBitstreamFormat1, MockBitstreamFormat2, + MockBitstreamFormat3, + MockOriginalBundle, } from '../../shared/mocks/item.mock'; import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock'; import { NotificationsService } from '../../shared/notifications/notifications.service'; @@ -51,10 +49,11 @@ import { UUIDService } from '../shared/uuid.service'; import { MetadataService } from './metadata.service'; import { environment } from '../../../environments/environment'; import { storeModuleConfig } from '../../app.reducer'; -import { HardRedirectService } from '../services/hard-redirect.service'; -import { URLCombiner } from '../url-combiner/url-combiner'; import { RootDataService } from '../data/root-data.service'; import { Root } from '../data/root.model'; +import { Bundle } from '../shared/bundle.model'; +import { BundleDataService } from '../data/bundle-data.service'; +import { createPaginatedList } from '../../shared/testing/utils.test'; /* tslint:disable:max-classes-per-file */ @Component({ @@ -92,6 +91,8 @@ describe('MetadataService', () => { let uuidService: UUIDService; let remoteDataBuildService: RemoteDataBuildService; let itemDataService: ItemDataService; + let bundleDataService; + let bitstreamDataService; let authService: AuthService; let rootService: RootDataService; let translateService: TranslateService; @@ -111,7 +112,7 @@ describe('MetadataService', () => { uuidService = new UUIDService(); requestService = new RequestService(objectCacheService, uuidService, store, undefined); remoteDataBuildService = new RemoteDataBuildService(objectCacheService, undefined, requestService); - const mockBitstreamDataService = { + bitstreamDataService = { findAllByItemAndBundleName(item: Item, bundleName: string, options?: FindListOptions, ...linksToFollow: FollowLinkConfig[]): Observable>> { if (item.equals(ItemMock)) { return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [MockBitstream1, MockBitstream2])); @@ -119,6 +120,7 @@ describe('MetadataService', () => { return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [])); } }, + findAllByHref: jasmine.createSpy(), }; const mockBitstreamFormatDataService = { findByBitstream(bitstream: Bitstream): Observable> { @@ -129,11 +131,17 @@ describe('MetadataService', () => { case MockBitstream2: return createSuccessfulRemoteDataObject$(MockBitstreamFormat2); break; + case MockBitstream3: + return createSuccessfulRemoteDataObject$(MockBitstreamFormat3); + break; default: return createSuccessfulRemoteDataObject$(new BitstreamFormat()); } } }; + bundleDataService = jasmine.createSpyObj('bundleDataService', { + findByItemAndName: createSuccessfulRemoteDataObject$(MockOriginalBundle), + }); rootService = jasmine.createSpyObj('rootService', { findRoot: createSuccessfulRemoteDataObject$(Object.assign(new Root(), { dspaceVersion: 'mock-dspace-version' @@ -176,7 +184,8 @@ describe('MetadataService', () => { { provide: CommunityDataService, useValue: {} }, { provide: DefaultChangeAnalyzer, useValue: {} }, { provide: BitstreamFormatDataService, useValue: mockBitstreamFormatDataService }, - { provide: BitstreamDataService, useValue: mockBitstreamDataService }, + { provide: BitstreamDataService, useValue: bitstreamDataService }, + { provide: BundleDataService, useValue: bundleDataService }, { provide: RootDataService, useValue: rootService }, Meta, Title, @@ -264,8 +273,42 @@ describe('MetadataService', () => { }); + describe('citation_pdf_url', () => { + it('should link to primary Bitstream URL regardless of format', fakeAsync(() => { + spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); + bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$([], MockBitstream3)); + router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + tick(); + expect(tagStore.get('citation_pdf_url')[0].content).toEqual('/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'); + })); + + describe('no primary Bitstream', () => { + it('should link to first and only Bitstream regardless of format', fakeAsync(() => { + spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); + bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$([MockBitstream3])); + router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + tick(); + expect(tagStore.get('citation_pdf_url')[0].content).toEqual('/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'); + })); + + it('should link to first Bitstream with allowed format', fakeAsync(() => { + spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); + + const bitstreams = [MockBitstream3, MockBitstream3, MockBitstream1]; + bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$(bitstreams)); + bitstreamDataService.findAllByHref.and.returnValues( + ...mockBitstreamPages$(bitstreams).map(bp => createSuccessfulRemoteDataObject$(bp)), + ); + + router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + tick(); + expect(tagStore.get('citation_pdf_url')[0].content).toEqual('/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/download'); + })); + }); + }); + const mockRemoteData = (mockItem: Item): Observable> => { - return createSuccessfulRemoteDataObject$(ItemMock); + return createSuccessfulRemoteDataObject$(mockItem); }; const mockType = (mockItem: Item, type: string): Item => { @@ -285,4 +328,24 @@ describe('MetadataService', () => { return publishedMockItem; }; + const mockBundleRD$ = (bitstreams: Bitstream[], primary?: Bitstream): Observable> => { + return createSuccessfulRemoteDataObject$( + Object.assign(new Bundle(), { + name: 'ORIGINAL', + bitstreams: createSuccessfulRemoteDataObject$(mockBitstreamPages$(bitstreams)[0]), + primaryBitstream: createSuccessfulRemoteDataObject$(primary), + }) + ); + }; + + const mockBitstreamPages$ = (bitstreams: Bitstream[]): PaginatedList[] => { + return bitstreams.map((bitstream, index) => Object.assign(createPaginatedList([bitstream]), { + pageInfo: { + totalElements: bitstreams.length, // announce multiple elements/pages + }, + _links: index < bitstreams.length - 1 + ? { next: { href: 'not empty' }} // fake link to the next bitstream page + : { next: { href: undefined }}, // last page has no link + })); + }; }); diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 0b63e13976..848b48b8c5 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -44,7 +44,7 @@ export class MetadataService { 'application/pdf', // .pdf 'application/postscript', // .ps 'application/msword', // .doc - 'application/vnd.openxmlformats-officedocument.wordprocessingml.document ', // .docx + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', // .docx 'application/rtf', // .rtf 'application/epub+zip', // .epub ]; diff --git a/src/app/shared/mocks/item.mock.ts b/src/app/shared/mocks/item.mock.ts index 945b0f7816..10eab2da00 100644 --- a/src/app/shared/mocks/item.mock.ts +++ b/src/app/shared/mocks/item.mock.ts @@ -5,6 +5,7 @@ import { Bitstream } from '../../core/shared/bitstream.model'; import { Item } from '../../core/shared/item.model'; import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils'; import { createPaginatedList } from '../testing/utils.test'; +import { Bundle } from '../../core/shared/bundle.model'; export const MockBitstreamFormat1: BitstreamFormat = Object.assign(new BitstreamFormat(), { shortDescription: 'Microsoft Word XML', @@ -34,11 +35,25 @@ export const MockBitstreamFormat2: BitstreamFormat = Object.assign(new Bitstream } }); +export const MockBitstreamFormat3: BitstreamFormat = Object.assign(new BitstreamFormat(), { + shortDescription: 'Binary', + description: 'Some scary unknown binary file', + mimetype: 'application/octet-stream', + supportLevel: 0, + internal: false, + extensions: null, + _links:{ + self: { + href: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/17' + } + } +}); + export const MockBitstream1: Bitstream = Object.assign(new Bitstream(), { sizeBytes: 10201, content: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/content', - format: observableOf(MockBitstreamFormat1), + format: createSuccessfulRemoteDataObject$(MockBitstreamFormat1), bundleName: 'ORIGINAL', _links:{ self: { @@ -61,7 +76,7 @@ export const MockBitstream1: Bitstream = Object.assign(new Bitstream(), export const MockBitstream2: Bitstream = Object.assign(new Bitstream(), { sizeBytes: 31302, content: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/content', - format: observableOf(MockBitstreamFormat2), + format: createSuccessfulRemoteDataObject$(MockBitstreamFormat2), bundleName: 'ORIGINAL', id: '99b00f3c-1cc6-4689-8158-91965bee6b28', uuid: '99b00f3c-1cc6-4689-8158-91965bee6b28', @@ -82,6 +97,68 @@ export const MockBitstream2: Bitstream = Object.assign(new Bitstream(), { } }); +export const MockBitstream3: Bitstream = Object.assign(new Bitstream(), { + sizeBytes: 4975123, + content: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/content', + format: createSuccessfulRemoteDataObject$(MockBitstreamFormat3), + bundleName: 'ORIGINAL', + id: '4db100c1-e1f5-4055-9404-9bc3e2d15f29', + uuid: '4db100c1-e1f5-4055-9404-9bc3e2d15f29', + type: 'bitstream', + _links: { + self: { href: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29' }, + content: { href: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/content' }, + format: { href: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/17' }, + bundle: { href: '' } + }, + metadata: { + 'dc.title': [ + { + language: null, + value: 'scary' + } + ] + } +}); + +export const MockOriginalBundle: Bundle = Object.assign(new Bundle(), { + name: 'ORIGINAL', + primaryBitstream: createSuccessfulRemoteDataObject$(MockBitstream2), + bitstreams: observableOf(Object.assign({ + _links: { + self: { + href: 'dspace-angular://aggregated/object/1507836003548', + } + }, + requestPending: false, + responsePending: false, + isSuccessful: true, + errorMessage: '', + state: '', + error: undefined, + isRequestPending: false, + isResponsePending: false, + isLoading: false, + hasFailed: false, + hasSucceeded: true, + statusCode: '202', + pageInfo: {}, + payload: { + pageInfo: { + elementsPerPage: 20, + totalElements: 3, + totalPages: 1, + currentPage: 2 + }, + page: [ + MockBitstream1, + MockBitstream2 + ] + } + })) +}); + + /* tslint:disable:no-shadowed-variable */ export const ItemMock: Item = Object.assign(new Item(), { handle: '10673/6', @@ -90,41 +167,7 @@ export const ItemMock: Item = Object.assign(new Item(), { isDiscoverable: true, isWithdrawn: false, bundles: createSuccessfulRemoteDataObject$(createPaginatedList([ - { - name: 'ORIGINAL', - bitstreams: observableOf(Object.assign({ - _links: { - self: { - href: 'dspace-angular://aggregated/object/1507836003548', - } - }, - requestPending: false, - responsePending: false, - isSuccessful: true, - errorMessage: '', - state: '', - error: undefined, - isRequestPending: false, - isResponsePending: false, - isLoading: false, - hasFailed: false, - hasSucceeded: true, - statusCode: '202', - pageInfo: {}, - payload: { - pageInfo: { - elementsPerPage: 20, - totalElements: 3, - totalPages: 1, - currentPage: 2 - }, - page: [ - MockBitstream1, - MockBitstream2 - ] - } - })) - } + MockOriginalBundle, ])), _links:{ self: { From 6f7b76ec39efa00676065bb50ef23c66804d96ca Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 4 Jun 2021 14:02:44 +0200 Subject: [PATCH 05/20] 79768: Remove og:title & og:description --- src/app/core/metadata/metadata.service.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 848b48b8c5..7caa7fe6cf 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -62,10 +62,6 @@ export class MetadataService { ) { // TODO: determine what open graph meta tags are needed and whether // the differ per route. potentially add image based on DSpaceObject - this.meta.addTags([ - { property: 'og:title', content: 'DSpace Angular Universal' }, - { property: 'og:description', content: 'The modern front-end for DSpace 7.' } - ]); this.initialized = false; this.tagStore = new Map(); } From 67f8ce7849ad222acb8bf171056e2a27cd66f7b8 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 4 Jun 2021 14:02:19 +0200 Subject: [PATCH 06/20] 79768: Rename citation_date to citation_publication_date --- src/app/core/metadata/metadata.service.spec.ts | 2 +- src/app/core/metadata/metadata.service.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 0a4377e1b7..749074d0a7 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -218,7 +218,7 @@ describe('MetadataService', () => { expect(title.getTitle()).toEqual('Test PowerPoint Document'); expect(tagStore.get('citation_title')[0].content).toEqual('Test PowerPoint Document'); expect(tagStore.get('citation_author')[0].content).toEqual('Doe, Jane'); - expect(tagStore.get('citation_date')[0].content).toEqual('1650-06-26'); + expect(tagStore.get('citation_publication_date')[0].content).toEqual('1650-06-26'); expect(tagStore.get('citation_issn')[0].content).toEqual('123456789'); expect(tagStore.get('citation_language')[0].content).toEqual('en'); expect(tagStore.get('citation_keywords')[0].content).toEqual('keyword1; keyword2; keyword3'); diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 7caa7fe6cf..3ccc9e5f09 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -135,7 +135,7 @@ export class MetadataService { this.setCitationTitleTag(); this.setCitationAuthorTags(); - this.setCitationDateTag(); + this.setCitationPublicationDateTag(); this.setCitationISSNTag(); this.setCitationISBNTag(); @@ -208,9 +208,9 @@ export class MetadataService { /** * Add to the */ - private setCitationDateTag(): void { + private setCitationPublicationDateTag(): void { const value = this.getFirstMetaTagValue(['dc.date.copyright', 'dc.date.issued', 'dc.date.available', 'dc.date.accessioned']); - this.addMetaTag('citation_date', value); + this.addMetaTag('citation_publication_date', value); } /** From 304d8f73862fd577d687813bdaad07d6f7d32b3d Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 4 Jun 2021 14:08:20 +0200 Subject: [PATCH 07/20] 79768: Fix tag: citation_abstract_html_url --- .../core/metadata/metadata.service.spec.ts | 23 ++++++++++++++++++- src/app/core/metadata/metadata.service.ts | 7 ++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 749074d0a7..30ee7f99ea 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -230,7 +230,6 @@ describe('MetadataService', () => { tick(); expect(tagStore.get('citation_dissertation_name')[0].content).toEqual('Test PowerPoint Document'); expect(tagStore.get('citation_dissertation_institution')[0].content).toEqual('Mock Publisher'); - expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual([environment.ui.baseUrl, router.url].join('')); expect(tagStore.get('citation_pdf_url')[0].content).toEqual('/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/download'); })); @@ -273,6 +272,22 @@ describe('MetadataService', () => { }); + describe('citation_abstract_html_url', () => { + it('should use dc.identifier.uri if available', fakeAsync(() => { + spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockUri(ItemMock, 'https://ddg.gg'))); + router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + tick(); + expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('https://ddg.gg'); + })); + + it('should use current route as fallback', fakeAsync(() => { + spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockUri(ItemMock))); + router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + tick(); + expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357'); + })); + }); + describe('citation_pdf_url', () => { it('should link to primary Bitstream URL regardless of format', fakeAsync(() => { spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); @@ -328,6 +343,12 @@ describe('MetadataService', () => { return publishedMockItem; }; + const mockUri = (mockItem: Item, uri?: string): Item => { + const publishedMockItem = Object.assign(new Item(), mockItem) as Item; + publishedMockItem.metadata['dc.identifier.uri'] = [{ value: uri }] as MetadataValue[]; + return publishedMockItem; + }; + const mockBundleRD$ = (bitstreams: Bitstream[], primary?: Bitstream): Observable> => { return createSuccessfulRemoteDataObject$( Object.assign(new Bundle(), { diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 3ccc9e5f09..bc26f36fd1 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -274,8 +274,11 @@ export class MetadataService { */ private setCitationAbstractUrlTag(): void { if (this.currentObject.value instanceof Item) { - const value = [environment.ui.baseUrl, this.router.url].join(''); - this.addMetaTag('citation_abstract_html_url', value); + let url = this.getMetaTagValue('dc.identifier.uri'); + if (hasNoValue(url)) { + url = this.router.url; // Google should handle relative URL + } + this.addMetaTag('citation_abstract_html_url', url); } } From 81b76dd32774d68bcf2ce87a5ab9e4deeddd71d7 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 4 Jun 2021 14:42:16 +0200 Subject: [PATCH 08/20] 79768: Fix tag: citation_publisher / citation_*_institution --- .../core/metadata/metadata.service.spec.ts | 32 +++++++++++++++++-- src/app/core/metadata/metadata.service.ts | 26 ++++++--------- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 30ee7f99ea..d2656d6c70 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -229,7 +229,6 @@ describe('MetadataService', () => { router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); tick(); expect(tagStore.get('citation_dissertation_name')[0].content).toEqual('Test PowerPoint Document'); - expect(tagStore.get('citation_dissertation_institution')[0].content).toEqual('Mock Publisher'); expect(tagStore.get('citation_pdf_url')[0].content).toEqual('/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/download'); })); @@ -284,7 +283,36 @@ describe('MetadataService', () => { spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockUri(ItemMock))); router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); tick(); - expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357'); + expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357'); + })); + }); + + describe('citation_*_institution / citation_publisher', () => { + it('should use citation_dissertation_institution tag for dissertations', fakeAsync(() => { + spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Thesis')))); + router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + tick(); + expect(tagStore.get('citation_dissertation_institution')[0].content).toEqual('Mock Publisher'); + expect(tagStore.get('citation_technical_report_institution')).toBeFalsy(); + expect(tagStore.get('citation_publisher')).toBeFalsy(); + })); + + it('should use citation_tech_report_institution tag for tech reports', fakeAsync(() => { + spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Technical Report')))); + router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + tick(); + expect(tagStore.get('citation_dissertation_institution')).toBeFalsy(); + expect(tagStore.get('citation_technical_report_institution')[0].content).toEqual('Mock Publisher'); + expect(tagStore.get('citation_publisher')).toBeFalsy(); + })); + + it('should use citation_publisher for other item types', fakeAsync(() => { + spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Some Other Type')))); + router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + tick(); + expect(tagStore.get('citation_dissertation_institution')).toBeFalsy(); + expect(tagStore.get('citation_technical_report_institution')).toBeFalsy(); + expect(tagStore.get('citation_publisher')[0].content).toEqual('Mock Publisher'); })); }); diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index bc26f36fd1..f4b7b7ca10 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -144,14 +144,10 @@ export class MetadataService { this.setCitationAbstractUrlTag(); this.setCitationPdfUrlTag(); + this.setCitationPublisherTag(); if (this.isDissertation()) { this.setCitationDissertationNameTag(); - this.setCitationDissertationInstitutionTag(); - } - - if (this.isTechReport()) { - this.setCitationTechReportInstitutionTag(); } // this.setCitationJournalTitleTag(); @@ -246,19 +242,17 @@ export class MetadataService { } /** - * Add to the + * Add dc.publisher to the . The tag name depends on the item type. */ - private setCitationDissertationInstitutionTag(): void { + private setCitationPublisherTag(): void { const value = this.getMetaTagValue('dc.publisher'); - this.addMetaTag('citation_dissertation_institution', value); - } - - /** - * Add to the - */ - private setCitationTechReportInstitutionTag(): void { - const value = this.getMetaTagValue('dc.publisher'); - this.addMetaTag('citation_technical_report_institution', value); + if (this.isDissertation()) { + this.addMetaTag('citation_dissertation_institution', value); + } else if (this.isTechReport()) { + this.addMetaTag('citation_technical_report_institution', value); + } else { + this.addMetaTag('citation_publisher', value); + } } /** From 6a4e56322f70768b52301adaaf5f903fa5572937 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 4 Jun 2021 15:14:57 +0200 Subject: [PATCH 09/20] 79768: Rename method --- src/app/core/metadata/metadata.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index f4b7b7ca10..7b886d9067 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -329,7 +329,7 @@ export class MetadataService { return [getBitstreamDownloadRoute(bitstreamRd.payload.page[0])]; } else { // Otherwise check all bitstreams to see if one matches the format whitelist - return this.getBitstreamDownloadRoute(bitstreamRd); + return this.getFirstAllowedFormatBitstreamLink(bitstreamRd); } }) ); @@ -343,7 +343,7 @@ export class MetadataService { } } - private getBitstreamDownloadRoute(bitstreamRd: RemoteData>): Observable { + private getFirstAllowedFormatBitstreamLink(bitstreamRd: RemoteData>): Observable { return observableOf(bitstreamRd.payload).pipe( // Because there can be more than one page of bitstreams, this expand operator // will retrieve them in turn due to the take(1) at the bottom, it will only From 2ed16aa66ecf81ad05c85d2fb21f65774d554fa9 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 14 Jun 2021 09:34:23 +0200 Subject: [PATCH 10/20] 79768: Update typedocs --- src/app/core/metadata/metadata.service.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 7b886d9067..fa1022e4f6 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -40,7 +40,13 @@ export class MetadataService { private currentObject: BehaviorSubject; - private readonly ALLOWED_MIMETYPES = [ + /** + * When generating the citation_pdf_url meta tag for Items with more than one Bitstream (and no primary Bitstream), + * the first Bitstream to match one of the following MIME types is selected. + * See {@linkcode getFirstAllowedFormatBitstreamLink} + * @private + */ + private readonly CITATION_PDF_URL_MIMETYPES = [ 'application/pdf', // .pdf 'application/postscript', // .ps 'application/msword', // .doc @@ -202,7 +208,7 @@ export class MetadataService { } /** - * Add to the + * Add to the */ private setCitationPublicationDateTag(): void { const value = this.getFirstMetaTagValue(['dc.date.copyright', 'dc.date.issued', 'dc.date.available', 'dc.date.accessioned']); @@ -343,6 +349,12 @@ export class MetadataService { } } + /** + * For Items with more than one Bitstream (and no primary Bitstream), link to the first Bitstream with a MIME type + * included in {@linkcode CITATION_PDF_URL_MIMETYPES} + * @param bitstreamRd + * @private + */ private getFirstAllowedFormatBitstreamLink(bitstreamRd: RemoteData>): Observable { return observableOf(bitstreamRd.payload).pipe( // Because there can be more than one page of bitstreams, this expand operator @@ -385,7 +397,7 @@ export class MetadataService { )), // Filter out only pairs with whitelisted formats filter(([, format]: [Bitstream, BitstreamFormat]) => - hasValue(format) && this.ALLOWED_MIMETYPES.includes(format.mimetype)), + hasValue(format) && this.CITATION_PDF_URL_MIMETYPES.includes(format.mimetype)), // We only need 1 take(1), // Emit the link of the match From 64049fdcf770b43cff44cb7228b003bc879c2a5b Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 14 Jun 2021 09:48:33 +0200 Subject: [PATCH 11/20] 79768: Make relative URLs absolute --- src/app/core/metadata/metadata.service.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index fa1022e4f6..cef3a67054 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -30,6 +30,8 @@ import { BundleDataService } from '../data/bundle-data.service'; import { followLink } from '../../shared/utils/follow-link-config.model'; import { Bundle } from '../shared/bundle.model'; import { PaginatedList } from '../data/paginated-list.model'; +import { URLCombiner } from '../url-combiner/url-combiner'; +import { HardRedirectService } from '../services/hard-redirect.service'; @Injectable() export class MetadataService { @@ -64,7 +66,8 @@ export class MetadataService { private bundleDataService: BundleDataService, private bitstreamDataService: BitstreamDataService, private bitstreamFormatDataService: BitstreamFormatDataService, - private rootService: RootDataService + private rootService: RootDataService, + private hardRedirectService: HardRedirectService, ) { // TODO: determine what open graph meta tags are needed and whether // the differ per route. potentially add image based on DSpaceObject @@ -276,7 +279,7 @@ export class MetadataService { if (this.currentObject.value instanceof Item) { let url = this.getMetaTagValue('dc.identifier.uri'); if (hasNoValue(url)) { - url = this.router.url; // Google should handle relative URL + url = new URLCombiner(this.hardRedirectService.getRequestOrigin(), this.router.url).toString(); } this.addMetaTag('citation_abstract_html_url', url); } @@ -344,7 +347,10 @@ export class MetadataService { take(1) ).subscribe((link: string) => { // Use the found link to set the tag - this.addMetaTag('citation_pdf_url', link); + this.addMetaTag( + 'citation_pdf_url', + new URLCombiner(this.hardRedirectService.getRequestOrigin(), link).toString() + ); }); } } From 04b4f1cf58dc2f606f4eb120d66f072e2587bf8b Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Mon, 14 Jun 2021 10:30:17 +0200 Subject: [PATCH 12/20] Fix comment punctuation --- src/app/core/metadata/metadata.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index cef3a67054..4e3da11bf2 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -364,7 +364,7 @@ export class MetadataService { private getFirstAllowedFormatBitstreamLink(bitstreamRd: RemoteData>): Observable { return observableOf(bitstreamRd.payload).pipe( // Because there can be more than one page of bitstreams, this expand operator - // will retrieve them in turn due to the take(1) at the bottom, it will only + // will retrieve them in turn. Due to the take(1) at the bottom, it will only // retrieve pages until a match is found expand((paginatedList: PaginatedList) => { if (hasNoValue(paginatedList.next)) { From 34b117efe3a4918a81c4d3ee904808f29885fe4d Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 14 Jun 2021 10:50:59 +0200 Subject: [PATCH 13/20] 80084: Fix unit test & LGTM issues --- src/app/core/metadata/metadata.service.spec.ts | 17 +++++++++++------ src/app/core/metadata/metadata.service.ts | 1 - 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index d2656d6c70..a2f2272219 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -47,13 +47,13 @@ import { PageInfo } from '../shared/page-info.model'; import { UUIDService } from '../shared/uuid.service'; import { MetadataService } from './metadata.service'; -import { environment } from '../../../environments/environment'; import { storeModuleConfig } from '../../app.reducer'; import { RootDataService } from '../data/root-data.service'; import { Root } from '../data/root.model'; import { Bundle } from '../shared/bundle.model'; import { BundleDataService } from '../data/bundle-data.service'; import { createPaginatedList } from '../../shared/testing/utils.test'; +import { HardRedirectService } from '../services/hard-redirect.service'; /* tslint:disable:max-classes-per-file */ @Component({ @@ -96,6 +96,7 @@ describe('MetadataService', () => { let authService: AuthService; let rootService: RootDataService; let translateService: TranslateService; + let hardRedirectService: HardRedirectService; let location: Location; let router: Router; @@ -147,6 +148,9 @@ describe('MetadataService', () => { dspaceVersion: 'mock-dspace-version' })) }); + hardRedirectService = jasmine.createSpyObj('hardRedirectService', { + getRequestOrigin: 'https://request.org', + }); TestBed.configureTestingModule({ imports: [ @@ -187,6 +191,7 @@ describe('MetadataService', () => { { provide: BitstreamDataService, useValue: bitstreamDataService }, { provide: BundleDataService, useValue: bundleDataService }, { provide: RootDataService, useValue: rootService }, + { provide: HardRedirectService, useValue: hardRedirectService }, Meta, Title, // tslint:disable-next-line:no-empty @@ -229,7 +234,7 @@ describe('MetadataService', () => { router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); tick(); expect(tagStore.get('citation_dissertation_name')[0].content).toEqual('Test PowerPoint Document'); - expect(tagStore.get('citation_pdf_url')[0].content).toEqual('/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/download'); + expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/download'); })); it('items page should set meta tags as published Technical Report', fakeAsync(() => { @@ -283,7 +288,7 @@ describe('MetadataService', () => { spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockUri(ItemMock))); router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); tick(); - expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357'); + expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('https://request.org/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357'); })); }); @@ -322,7 +327,7 @@ describe('MetadataService', () => { bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$([], MockBitstream3)); router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); tick(); - expect(tagStore.get('citation_pdf_url')[0].content).toEqual('/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'); + expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'); })); describe('no primary Bitstream', () => { @@ -331,7 +336,7 @@ describe('MetadataService', () => { bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$([MockBitstream3])); router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); tick(); - expect(tagStore.get('citation_pdf_url')[0].content).toEqual('/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'); + expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'); })); it('should link to first Bitstream with allowed format', fakeAsync(() => { @@ -345,7 +350,7 @@ describe('MetadataService', () => { router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); tick(); - expect(tagStore.get('citation_pdf_url')[0].content).toEqual('/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/download'); + expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/download'); })); }); }); diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 4e3da11bf2..d6518b6164 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -23,7 +23,6 @@ import { getFirstSucceededRemoteDataPayload, getFirstCompletedRemoteData } from '../shared/operators'; -import { environment } from '../../../environments/environment'; import { RootDataService } from '../data/root-data.service'; import { getBitstreamDownloadRoute } from '../../app-routing-paths'; import { BundleDataService } from '../data/bundle-data.service'; From 03e2e30510a4ab151f17d158ca8010e99ed09da6 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 30 Jun 2021 14:09:18 +0200 Subject: [PATCH 14/20] fix issue where meta tags wouldn't be updated properly based on the route --- .../collection-page.component.ts | 14 +- .../full/full-item-page.component.ts | 10 +- .../+item-page/simple/item-page.component.ts | 4 - .../core/metadata/metadata.service.spec.ts | 503 +++++++++--------- src/app/core/metadata/metadata.service.ts | 47 +- 5 files changed, 261 insertions(+), 317 deletions(-) diff --git a/src/app/+collection-page/collection-page.component.ts b/src/app/+collection-page/collection-page.component.ts index 9eba2e4ab2..366e1da7b1 100644 --- a/src/app/+collection-page/collection-page.component.ts +++ b/src/app/+collection-page/collection-page.component.ts @@ -1,6 +1,11 @@ import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, Subject } from 'rxjs'; +import { + BehaviorSubject, + combineLatest as observableCombineLatest, + Observable, + Subject +} from 'rxjs'; import { filter, map, mergeMap, startWith, switchMap, take } from 'rxjs/operators'; import { PaginatedSearchOptions } from '../shared/search/paginated-search-options.model'; import { SearchService } from '../core/shared/search/search.service'; @@ -8,8 +13,6 @@ import { SortDirection, SortOptions } from '../core/cache/models/sort-options.mo import { CollectionDataService } from '../core/data/collection-data.service'; import { PaginatedList } from '../core/data/paginated-list.model'; import { RemoteData } from '../core/data/remote-data'; - -import { MetadataService } from '../core/metadata/metadata.service'; import { Bitstream } from '../core/shared/bitstream.model'; import { Collection } from '../core/shared/collection.model'; @@ -65,7 +68,6 @@ export class CollectionPageComponent implements OnInit { constructor( private collectionDataService: CollectionDataService, private searchService: SearchService, - private metadata: MetadataService, private route: ActivatedRoute, private router: Router, private authService: AuthService, @@ -122,10 +124,6 @@ export class CollectionPageComponent implements OnInit { getAllSucceededRemoteDataPayload(), map((collection) => getCollectionPageRoute(collection.id)) ); - - this.route.queryParams.pipe(take(1)).subscribe((params) => { - this.metadata.processRemoteData(this.collectionRD$); - }); } isNotEmpty(object: any) { diff --git a/src/app/+item-page/full/full-item-page.component.ts b/src/app/+item-page/full/full-item-page.component.ts index aea350e58e..da16e134cb 100644 --- a/src/app/+item-page/full/full-item-page.component.ts +++ b/src/app/+item-page/full/full-item-page.component.ts @@ -1,8 +1,8 @@ -import {filter, map} from 'rxjs/operators'; +import { filter, map } from 'rxjs/operators'; import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { Observable , BehaviorSubject } from 'rxjs'; +import { Observable, BehaviorSubject } from 'rxjs'; import { ItemPageComponent } from '../simple/item-page.component'; import { MetadataMap } from '../../core/shared/metadata.models'; @@ -11,8 +11,6 @@ import { ItemDataService } from '../../core/data/item-data.service'; import { RemoteData } from '../../core/data/remote-data'; import { Item } from '../../core/shared/item.model'; -import { MetadataService } from '../../core/metadata/metadata.service'; - import { fadeInOut } from '../../shared/animations/fade'; import { hasValue } from '../../shared/empty.util'; import { AuthService } from '../../core/auth/auth.service'; @@ -35,8 +33,8 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit { metadata$: Observable; - constructor(route: ActivatedRoute, router: Router, items: ItemDataService, metadataService: MetadataService, authService: AuthService) { - super(route, router, items, metadataService, authService); + constructor(route: ActivatedRoute, router: Router, items: ItemDataService, authService: AuthService) { + super(route, router, items, authService); } /*** AoT inheritance fix, will hopefully be resolved in the near future **/ diff --git a/src/app/+item-page/simple/item-page.component.ts b/src/app/+item-page/simple/item-page.component.ts index 67e278c2fb..d2c238b5e6 100644 --- a/src/app/+item-page/simple/item-page.component.ts +++ b/src/app/+item-page/simple/item-page.component.ts @@ -8,8 +8,6 @@ import { RemoteData } from '../../core/data/remote-data'; import { Item } from '../../core/shared/item.model'; -import { MetadataService } from '../../core/metadata/metadata.service'; - import { fadeInOut } from '../../shared/animations/fade'; import { getAllSucceededRemoteDataPayload, redirectOn4xx } from '../../core/shared/operators'; import { ViewMode } from '../../core/shared/view-mode.model'; @@ -54,7 +52,6 @@ export class ItemPageComponent implements OnInit { private route: ActivatedRoute, private router: Router, private items: ItemDataService, - private metadataService: MetadataService, private authService: AuthService, ) { } @@ -66,7 +63,6 @@ export class ItemPageComponent implements OnInit { map((data) => data.dso as RemoteData), redirectOn4xx(this.router, this.authService) ); - this.metadataService.processRemoteData(this.itemRD$); this.itemPageRoute$ = this.itemRD$.pipe( getAllSucceededRemoteDataPayload(), map((item) => getItemPageRoute(item)) diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index a2f2272219..d18897cc55 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -1,82 +1,30 @@ -import { CommonModule, Location } from '@angular/common'; -import { HttpClient } from '@angular/common/http'; -import { Component, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; -import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; -import { Meta, MetaDefinition, Title } from '@angular/platform-browser'; -import { ActivatedRoute, Router } from '@angular/router'; -import { RouterTestingModule } from '@angular/router/testing'; +import { fakeAsync, tick } from '@angular/core/testing'; +import { Meta, Title } from '@angular/platform-browser'; +import { Router, NavigationEnd } from '@angular/router'; -import { Store, StoreModule } from '@ngrx/store'; - -import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core'; -import { EmptyError, Observable, of } from 'rxjs'; +import { TranslateService } from '@ngx-translate/core'; +import { Observable, of } from 'rxjs'; import { RemoteData } from '../data/remote-data'; import { Item } from '../shared/item.model'; +import { ItemMock, MockBitstream1, MockBitstream3, } from '../../shared/mocks/item.mock'; import { - ItemMock, MockBitstream1, MockBitstream2, MockBitstream3, MockBitstreamFormat1, MockBitstreamFormat2, - MockBitstreamFormat3, - MockOriginalBundle, -} from '../../shared/mocks/item.mock'; -import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock'; -import { NotificationsService } from '../../shared/notifications/notifications.service'; -import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; -import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; -import { AuthService } from '../auth/auth.service'; -import { BrowseService } from '../browse/browse.service'; -import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; -import { ObjectCacheService } from '../cache/object-cache.service'; - -import { CoreState } from '../core.reducers'; -import { BitstreamDataService } from '../data/bitstream-data.service'; -import { BitstreamFormatDataService } from '../data/bitstream-format-data.service'; -import { CommunityDataService } from '../data/community-data.service'; -import { DefaultChangeAnalyzer } from '../data/default-change-analyzer.service'; -import { DSOChangeAnalyzer } from '../data/dso-change-analyzer.service'; - -import { ItemDataService } from '../data/item-data.service'; -import { buildPaginatedList, PaginatedList } from '../data/paginated-list.model'; -import { FindListOptions } from '../data/request.models'; -import { RequestService } from '../data/request.service'; -import { BitstreamFormat } from '../shared/bitstream-format.model'; + createSuccessfulRemoteDataObject$, + createSuccessfulRemoteDataObject +} from '../../shared/remote-data.utils'; +import { PaginatedList } from '../data/paginated-list.model'; import { Bitstream } from '../shared/bitstream.model'; -import { HALEndpointService } from '../shared/hal-endpoint.service'; import { MetadataValue } from '../shared/metadata.models'; -import { PageInfo } from '../shared/page-info.model'; -import { UUIDService } from '../shared/uuid.service'; import { MetadataService } from './metadata.service'; -import { storeModuleConfig } from '../../app.reducer'; import { RootDataService } from '../data/root-data.service'; -import { Root } from '../data/root.model'; import { Bundle } from '../shared/bundle.model'; -import { BundleDataService } from '../data/bundle-data.service'; import { createPaginatedList } from '../../shared/testing/utils.test'; +import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; +import { DSONameService } from '../breadcrumbs/dso-name.service'; import { HardRedirectService } from '../services/hard-redirect.service'; -/* tslint:disable:max-classes-per-file */ -@Component({ - template: ` - ` -}) -class TestComponent { - constructor(private metadata: MetadataService) { - metadata.listenForRouteChange(); - } -} - -@Component({ template: '' }) -class DummyItemComponent { - constructor(private route: ActivatedRoute, private items: ItemDataService, private metadata: MetadataService) { - this.route.params.subscribe((params) => { - this.metadata.processRemoteData(this.items.findById(params.id)); - }); - } -} - -/* tslint:enable:max-classes-per-file */ - describe('MetadataService', () => { let metadataService: MetadataService; @@ -84,281 +32,306 @@ describe('MetadataService', () => { let title: Title; - let store: Store; + let dsoNameService: DSONameService; - let objectCacheService: ObjectCacheService; - let requestService: RequestService; - let uuidService: UUIDService; - let remoteDataBuildService: RemoteDataBuildService; - let itemDataService: ItemDataService; let bundleDataService; let bitstreamDataService; - let authService: AuthService; let rootService: RootDataService; let translateService: TranslateService; let hardRedirectService: HardRedirectService; - let location: Location; let router: Router; - let fixture: ComponentFixture; - - let tagStore: Map; beforeEach(() => { - - store = new Store(undefined, undefined, undefined); - spyOn(store, 'dispatch'); - - objectCacheService = new ObjectCacheService(store, undefined); - uuidService = new UUIDService(); - requestService = new RequestService(objectCacheService, uuidService, store, undefined); - remoteDataBuildService = new RemoteDataBuildService(objectCacheService, undefined, requestService); - bitstreamDataService = { - findAllByItemAndBundleName(item: Item, bundleName: string, options?: FindListOptions, ...linksToFollow: FollowLinkConfig[]): Observable>> { - if (item.equals(ItemMock)) { - return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [MockBitstream1, MockBitstream2])); - } else { - return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [])); - } - }, - findAllByHref: jasmine.createSpy(), - }; - const mockBitstreamFormatDataService = { - findByBitstream(bitstream: Bitstream): Observable> { - switch (bitstream) { - case MockBitstream1: - return createSuccessfulRemoteDataObject$(MockBitstreamFormat1); - break; - case MockBitstream2: - return createSuccessfulRemoteDataObject$(MockBitstreamFormat2); - break; - case MockBitstream3: - return createSuccessfulRemoteDataObject$(MockBitstreamFormat3); - break; - default: - return createSuccessfulRemoteDataObject$(new BitstreamFormat()); - } + rootService = jasmine.createSpyObj({ + findRoot: createSuccessfulRemoteDataObject$({ dspaceVersion: 'mock-dspace-version' }) + }); + bitstreamDataService = jasmine.createSpyObj({ + findAllByHref: createSuccessfulRemoteDataObject$(createPaginatedList([MockBitstream3])) + }); + bundleDataService = jasmine.createSpyObj({ + findByItemAndName: mockBundleRD$([MockBitstream3]) + }); + translateService = getMockTranslateService(); + meta = jasmine.createSpyObj({ + addTag: {}, + removeTag: {} + }); + title = jasmine.createSpyObj({ + setTitle: {} + }); + dsoNameService = jasmine.createSpyObj({ + getName: ItemMock.firstMetadataValue('dc.title') + }); + router = { + url: '/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357', + events: of(new NavigationEnd(1, '', '')), + routerState: { + root: {} } - }; - bundleDataService = jasmine.createSpyObj('bundleDataService', { - findByItemAndName: createSuccessfulRemoteDataObject$(MockOriginalBundle), - }); - rootService = jasmine.createSpyObj('rootService', { - findRoot: createSuccessfulRemoteDataObject$(Object.assign(new Root(), { - dspaceVersion: 'mock-dspace-version' - })) - }); - hardRedirectService = jasmine.createSpyObj('hardRedirectService', { + } as any as Router; + hardRedirectService = jasmine.createSpyObj( { getRequestOrigin: 'https://request.org', }); - - TestBed.configureTestingModule({ - imports: [ - CommonModule, - StoreModule.forRoot({}, storeModuleConfig), - TranslateModule.forRoot({ - loader: { - provide: TranslateLoader, - useClass: TranslateLoaderMock - } - }), - RouterTestingModule.withRoutes([ - { path: 'items/:id', component: DummyItemComponent, pathMatch: 'full' }, - { - path: 'other', - component: DummyItemComponent, - pathMatch: 'full', - data: { title: 'Dummy Title', description: 'This is a dummy item component for testing!' } - } - ]) - ], - declarations: [ - TestComponent, - DummyItemComponent - ], - providers: [ - { provide: ObjectCacheService, useValue: objectCacheService }, - { provide: RequestService, useValue: requestService }, - { provide: RemoteDataBuildService, useValue: remoteDataBuildService }, - { provide: HALEndpointService, useValue: {} }, - { provide: AuthService, useValue: {} }, - { provide: NotificationsService, useValue: {} }, - { provide: HttpClient, useValue: {} }, - { provide: DSOChangeAnalyzer, useValue: {} }, - { provide: CommunityDataService, useValue: {} }, - { provide: DefaultChangeAnalyzer, useValue: {} }, - { provide: BitstreamFormatDataService, useValue: mockBitstreamFormatDataService }, - { provide: BitstreamDataService, useValue: bitstreamDataService }, - { provide: BundleDataService, useValue: bundleDataService }, - { provide: RootDataService, useValue: rootService }, - { provide: HardRedirectService, useValue: hardRedirectService }, - Meta, - Title, - // tslint:disable-next-line:no-empty - { provide: ItemDataService, useValue: { findById: () => {} } }, - BrowseService, - MetadataService - ], - schemas: [CUSTOM_ELEMENTS_SCHEMA] - }); - meta = TestBed.inject(Meta); - title = TestBed.inject(Title); - itemDataService = TestBed.inject(ItemDataService); - metadataService = TestBed.inject(MetadataService); - authService = TestBed.inject(AuthService); - translateService = TestBed.inject(TranslateService); - - router = TestBed.inject(Router); - location = TestBed.inject(Location); - - fixture = TestBed.createComponent(TestComponent); - - tagStore = metadataService.getTagStore(); + metadataService = new MetadataService( + router, + translateService, + meta, + title, + dsoNameService, + bundleDataService, + bitstreamDataService, + undefined, + rootService, + hardRedirectService + ); }); it('items page should set meta tags', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(ItemMock), + } + } + }); tick(); - expect(title.getTitle()).toEqual('Test PowerPoint Document'); - expect(tagStore.get('citation_title')[0].content).toEqual('Test PowerPoint Document'); - expect(tagStore.get('citation_author')[0].content).toEqual('Doe, Jane'); - expect(tagStore.get('citation_publication_date')[0].content).toEqual('1650-06-26'); - expect(tagStore.get('citation_issn')[0].content).toEqual('123456789'); - expect(tagStore.get('citation_language')[0].content).toEqual('en'); - expect(tagStore.get('citation_keywords')[0].content).toEqual('keyword1; keyword2; keyword3'); + expect(title.setTitle).toHaveBeenCalledWith('Test PowerPoint Document'); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_title', + content: 'Test PowerPoint Document' + }); + expect(meta.addTag).toHaveBeenCalledWith({ property: 'citation_author', content: 'Doe, Jane' }); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_publication_date', + content: '1650-06-26' + }); + expect(meta.addTag).toHaveBeenCalledWith({ property: 'citation_issn', content: '123456789' }); + expect(meta.addTag).toHaveBeenCalledWith({ property: 'citation_language', content: 'en' }); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_keywords', + content: 'keyword1; keyword2; keyword3' + }); })); it('items page should set meta tags as published Thesis', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Thesis')))); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Thesis'))), + } + } + }); tick(); - expect(tagStore.get('citation_dissertation_name')[0].content).toEqual('Test PowerPoint Document'); - expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/download'); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_dissertation_name', + content: 'Test PowerPoint Document' + }); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_pdf_url', + content: 'https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download' + }); })); it('items page should set meta tags as published Technical Report', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Technical Report')))); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); - tick(); - expect(tagStore.get('citation_technical_report_institution')[0].content).toEqual('Mock Publisher'); - })); - - it('other navigation should add title, description and Generator', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); - spyOn(translateService, 'get').and.returnValues(of('DSpace :: '), of('Dummy Title'), of('This is a dummy item component for testing!')); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); - tick(); - expect(tagStore.size).toBeGreaterThan(0); - router.navigate(['/other']); - tick(); - expect(tagStore.size).toEqual(3); - expect(title.getTitle()).toEqual('DSpace :: Dummy Title'); - expect(tagStore.get('title')[0].content).toEqual('DSpace :: Dummy Title'); - expect(tagStore.get('description')[0].content).toEqual('This is a dummy item component for testing!'); - expect(tagStore.get('Generator')[0].content).toEqual('mock-dspace-version'); - })); - - describe('when the item has no bitstreams', () => { - - beforeEach(() => { - // this.bitstreamDataService.findAllByItemAndBundleName(this.item, 'ORIGINAL') - // spyOn(MockItem, 'getFiles').and.returnValue(observableOf([])); + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Technical Report'))), + } + } }); + tick(); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_technical_report_institution', + content: 'Mock Publisher' + }); + })); - it('processRemoteData should not produce an EmptyError', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); - spyOn(metadataService, 'processRemoteData').and.callThrough(); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + it('other navigation should add title and description', fakeAsync(() => { + (translateService.get as jasmine.Spy).and.returnValues(of('DSpace :: '), of('Dummy Title'), of('This is a dummy item component for testing!')); + (metadataService as any).processRouteChange({ + data: { + value: { + title: 'Dummy Title', + description: 'This is a dummy item component for testing!' + } + } + }); + tick(); + expect(title.setTitle).toHaveBeenCalledWith('DSpace :: Dummy Title'); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'title', + content: 'DSpace :: Dummy Title' + }); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'description', + content: 'This is a dummy item component for testing!' + }); + })); + + describe(`listenForRouteChange`, () => { + it(`should call processRouteChange`, fakeAsync(() => { + spyOn(metadataService as any, 'processRouteChange').and.callFake(() => undefined); + metadataService.listenForRouteChange(); tick(); - expect(metadataService.processRemoteData).not.toThrow(new EmptyError()); + expect((metadataService as any).processRouteChange).toHaveBeenCalled(); + })); + it(`should add Generator`, fakeAsync(() => { + spyOn(metadataService as any, 'processRouteChange').and.callFake(() => undefined); + metadataService.listenForRouteChange(); + tick(); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'Generator', + content: 'mock-dspace-version' + }); })); - }); describe('citation_abstract_html_url', () => { it('should use dc.identifier.uri if available', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockUri(ItemMock, 'https://ddg.gg'))); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockUri(ItemMock, 'https://ddg.gg')), + } + } + }); tick(); - expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('https://ddg.gg'); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_abstract_html_url', + content: 'https://ddg.gg' + }); })); it('should use current route as fallback', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockUri(ItemMock))); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockUri(ItemMock)), + } + } + }); tick(); - expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual('https://request.org/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357'); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_abstract_html_url', + content: 'https://request.org/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357' + }); })); }); describe('citation_*_institution / citation_publisher', () => { it('should use citation_dissertation_institution tag for dissertations', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Thesis')))); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Thesis'))), + } + } + }); tick(); - expect(tagStore.get('citation_dissertation_institution')[0].content).toEqual('Mock Publisher'); - expect(tagStore.get('citation_technical_report_institution')).toBeFalsy(); - expect(tagStore.get('citation_publisher')).toBeFalsy(); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_dissertation_institution', + content: 'Mock Publisher' + }); + expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_technical_report_institution' })); + expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_publisher' })); })); it('should use citation_tech_report_institution tag for tech reports', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Technical Report')))); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Technical Report'))), + } + } + }); tick(); - expect(tagStore.get('citation_dissertation_institution')).toBeFalsy(); - expect(tagStore.get('citation_technical_report_institution')[0].content).toEqual('Mock Publisher'); - expect(tagStore.get('citation_publisher')).toBeFalsy(); + expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_dissertation_institution' })); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_technical_report_institution', + content: 'Mock Publisher' + }); + expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_publisher' })); })); it('should use citation_publisher for other item types', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(mockPublisher(mockType(ItemMock, 'Some Other Type')))); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Some Other Type'))), + } + } + }); tick(); - expect(tagStore.get('citation_dissertation_institution')).toBeFalsy(); - expect(tagStore.get('citation_technical_report_institution')).toBeFalsy(); - expect(tagStore.get('citation_publisher')[0].content).toEqual('Mock Publisher'); + expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_dissertation_institution' })); + expect(meta.addTag).not.toHaveBeenCalledWith(jasmine.objectContaining({ property: 'citation_technical_report_institution' })); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_publisher', + content: 'Mock Publisher' + }); })); }); describe('citation_pdf_url', () => { it('should link to primary Bitstream URL regardless of format', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); - bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$([], MockBitstream3)); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (bundleDataService.findByItemAndName as jasmine.Spy).and.returnValue(mockBundleRD$([], MockBitstream3)); + + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(ItemMock), + } + } + }); tick(); - expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_pdf_url', + content: 'https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download' + }); })); describe('no primary Bitstream', () => { it('should link to first and only Bitstream regardless of format', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); - bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$([MockBitstream3])); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (bundleDataService.findByItemAndName as jasmine.Spy).and.returnValue(mockBundleRD$([MockBitstream3])); + + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(ItemMock), + } + } + }); tick(); - expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download'); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_pdf_url', + content: 'https://request.org/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/download' + }); })); it('should link to first Bitstream with allowed format', fakeAsync(() => { - spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(ItemMock)); - const bitstreams = [MockBitstream3, MockBitstream3, MockBitstream1]; - bundleDataService.findByItemAndName.and.returnValue(mockBundleRD$(bitstreams)); - bitstreamDataService.findAllByHref.and.returnValues( + (bundleDataService.findByItemAndName as jasmine.Spy).and.returnValue(mockBundleRD$(bitstreams)); + (bitstreamDataService.findAllByHref as jasmine.Spy).and.returnValues( ...mockBitstreamPages$(bitstreams).map(bp => createSuccessfulRemoteDataObject$(bp)), ); - router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']); + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(ItemMock), + } + } + }); tick(); - expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://request.org/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/download'); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_pdf_url', + content: 'https://request.org/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/download' + }); })); }); }); - const mockRemoteData = (mockItem: Item): Observable> => { - return createSuccessfulRemoteDataObject$(mockItem); - }; - const mockType = (mockItem: Item, type: string): Item => { const typedMockItem = Object.assign(new Item(), mockItem) as Item; typedMockItem.metadata['dc.type'] = [{ value: type }] as MetadataValue[]; diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index d6518b6164..ed17fad2d8 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -6,11 +6,10 @@ import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; import { BehaviorSubject, combineLatest, Observable, of as observableOf, EMPTY } from 'rxjs'; -import { distinctUntilKeyChanged, filter, map, take, switchMap, expand } from 'rxjs/operators'; +import { filter, map, take, switchMap, expand } from 'rxjs/operators'; import { hasValue, hasNoValue } from '../../shared/empty.util'; import { DSONameService } from '../breadcrumbs/dso-name.service'; -import { CacheableObject } from '../cache/object-cache.reducer'; import { BitstreamDataService } from '../data/bitstream-data.service'; import { BitstreamFormatDataService } from '../data/bitstream-format-data.service'; @@ -35,11 +34,9 @@ import { HardRedirectService } from '../services/hard-redirect.service'; @Injectable() export class MetadataService { - private initialized: boolean; - private tagStore: Map; - private currentObject: BehaviorSubject; + private currentObject: BehaviorSubject = new BehaviorSubject(undefined); /** * When generating the citation_pdf_url meta tag for Items with more than one Bitstream (and no primary Bitstream), @@ -70,11 +67,13 @@ export class MetadataService { ) { // TODO: determine what open graph meta tags are needed and whether // the differ per route. potentially add image based on DSpaceObject - this.initialized = false; this.tagStore = new Map(); } public listenForRouteChange(): void { + // This never changes, set it only once + this.setGenerator(); + this.router.events.pipe( filter((event) => event instanceof NavigationEnd), map(() => this.router.routerState.root), @@ -86,22 +85,9 @@ export class MetadataService { }); } - public processRemoteData(remoteData: Observable>): void { - remoteData.pipe(map((rd: RemoteData) => rd.payload), - filter((co: CacheableObject) => hasValue(co)), - take(1)) - .subscribe((dspaceObject: DSpaceObject) => { - if (!this.initialized) { - this.initialize(dspaceObject); - } - this.currentObject.next(dspaceObject); - }); - } - private processRouteChange(routeInfo: any): void { - if (routeInfo.params.value.id === undefined) { - this.clearMetaTags(); - } + this.clearMetaTags(); + if (routeInfo.data.value.title) { const titlePrefix = this.translate.get('repository.title.prefix'); const title = this.translate.get(routeInfo.data.value.title, routeInfo.data.value); @@ -116,15 +102,10 @@ export class MetadataService { }); } - this.setGenerator(); - } - - private initialize(dspaceObject: DSpaceObject): void { - this.currentObject = new BehaviorSubject(dspaceObject); - this.currentObject.asObservable().pipe(distinctUntilKeyChanged('uuid')).subscribe(() => { - this.setMetaTags(); - }); - this.initialized = true; + if (hasValue(routeInfo.data.value.dso) && hasValue(routeInfo.data.value.dso.payload)) { + this.currentObject.next(routeInfo.data.value.dso.payload); + this.setDSOMetaTags(); + } } private getCurrentRoute(route: ActivatedRoute): ActivatedRoute { @@ -134,9 +115,7 @@ export class MetadataService { return route; } - private setMetaTags(): void { - - this.clearMetaTags(); + private setDSOMetaTags(): void { this.setTitleTag(); this.setDescriptionTag(); @@ -415,7 +394,7 @@ export class MetadataService { */ private setGenerator(): void { this.rootService.findRoot().pipe(getFirstSucceededRemoteDataPayload()).subscribe((root) => { - this.addMetaTag('Generator', root.dspaceVersion); + this.meta.addTag({ property: 'Generator', content: root.dspaceVersion }); }); } From c86f163cb7e8c6de6da273b3c4c198699339c7cc Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 1 Jul 2021 11:36:12 +0200 Subject: [PATCH 15/20] move tagstore to ngrx --- src/app/core/core.reducers.ts | 3 ++ src/app/core/metadata/meta-tag.actions.ts | 24 +++++++++ src/app/core/metadata/meta-tag.reducer.ts | 38 ++++++++++++++ src/app/core/metadata/metadata.service.ts | 63 +++++++++++++---------- 4 files changed, 100 insertions(+), 28 deletions(-) create mode 100644 src/app/core/metadata/meta-tag.actions.ts create mode 100644 src/app/core/metadata/meta-tag.reducer.ts diff --git a/src/app/core/core.reducers.ts b/src/app/core/core.reducers.ts index 077aa3dc95..448c1b8641 100644 --- a/src/app/core/core.reducers.ts +++ b/src/app/core/core.reducers.ts @@ -13,6 +13,7 @@ import { BitstreamFormatRegistryState } from '../+admin/admin-registries/bitstream-formats/bitstream-format.reducers'; import { historyReducer, HistoryState } from './history/history.reducer'; +import { metaTagReducer, MetaTagState } from './metadata/meta-tag.reducer'; export interface CoreState { 'bitstreamFormats': BitstreamFormatRegistryState; @@ -24,6 +25,7 @@ export interface CoreState { 'index': MetaIndexState; 'auth': AuthState; 'json/patch': JsonPatchOperationsState; + 'metaTag': MetaTagState; 'route': RouteState; } @@ -37,5 +39,6 @@ export const coreReducers: ActionReducerMap = { 'index': indexReducer, 'auth': authReducer, 'json/patch': jsonPatchOperationsReducer, + 'metaTag': metaTagReducer, 'route': routeReducer }; diff --git a/src/app/core/metadata/meta-tag.actions.ts b/src/app/core/metadata/meta-tag.actions.ts new file mode 100644 index 0000000000..6451e58da2 --- /dev/null +++ b/src/app/core/metadata/meta-tag.actions.ts @@ -0,0 +1,24 @@ +import { type } from '../../shared/ngrx/type'; +import { Action } from '@ngrx/store'; +import { MetaDefinition } from '@angular/platform-browser'; + +// tslint:disable:max-classes-per-file +export const MetaTagTypes = { + ADD: type('dspace/meta-tag/ADD'), + CLEAR: type('dspace/meta-tag/CLEAR') +}; + +export class AddMetaTagAction implements Action { + type = MetaTagTypes.ADD; + payload: string; + + constructor(property: string) { + this.payload = property; + } +} + +export class ClearMetaTagAction implements Action { + type = MetaTagTypes.CLEAR; +} + +export type MetaTagAction = AddMetaTagAction | ClearMetaTagAction; diff --git a/src/app/core/metadata/meta-tag.reducer.ts b/src/app/core/metadata/meta-tag.reducer.ts new file mode 100644 index 0000000000..0af6fb0aab --- /dev/null +++ b/src/app/core/metadata/meta-tag.reducer.ts @@ -0,0 +1,38 @@ +import { + MetaTagAction, + MetaTagTypes, + AddMetaTagAction, + ClearMetaTagAction, +} from './meta-tag.actions'; + +export interface MetaTagState { + tagsInUse: string[]; +} + +const initialstate: MetaTagState = { + tagsInUse: [] +}; + +export const metaTagReducer = (state: MetaTagState = initialstate, action: MetaTagAction): MetaTagState => { + switch (action.type) { + case MetaTagTypes.ADD: { + return addMetaTag(state, action as AddMetaTagAction); + } + case MetaTagTypes.CLEAR: { + return clearMetaTags(state, action as ClearMetaTagAction); + } + default: { + return state; + } + } +}; + +const addMetaTag = (state: MetaTagState, action: AddMetaTagAction): MetaTagState => { + return { + tagsInUse: [...state.tagsInUse, action.payload] + }; +}; + +const clearMetaTags = (state: MetaTagState, action: ClearMetaTagAction): MetaTagState => { + return Object.assign({}, initialstate); +}; diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index ed17fad2d8..8c1e1027dd 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -30,12 +30,33 @@ import { Bundle } from '../shared/bundle.model'; import { PaginatedList } from '../data/paginated-list.model'; import { URLCombiner } from '../url-combiner/url-combiner'; import { HardRedirectService } from '../services/hard-redirect.service'; +import { MetaTagState } from './meta-tag.reducer'; +import { Store, createSelector, select, MemoizedSelector } from '@ngrx/store'; +import { AddMetaTagAction, ClearMetaTagAction } from './meta-tag.actions'; +import { coreSelector } from '../core.selectors'; +import { CoreState } from '../core.reducers'; +import { ObjectCacheEntry, ObjectCacheState } from '../cache/object-cache.reducer'; + +/** + * The base selector function to select the metaTag section in the store + */ +const metaTagSelector = createSelector( + coreSelector, + (state: CoreState) => state.metaTag +); + +/** + * Selector function to select the tags in use from the MetaTagState + */ +const tagsInUseSelector = + createSelector( + metaTagSelector, + (state: MetaTagState) => state.tagsInUse, + ); @Injectable() export class MetadataService { - private tagStore: Map; - private currentObject: BehaviorSubject = new BehaviorSubject(undefined); /** @@ -63,11 +84,9 @@ export class MetadataService { private bitstreamDataService: BitstreamDataService, private bitstreamFormatDataService: BitstreamFormatDataService, private rootService: RootDataService, + private store: Store, private hardRedirectService: HardRedirectService, ) { - // TODO: determine what open graph meta tags are needed and whether - // the differ per route. potentially add image based on DSpaceObject - this.tagStore = new Map(); } public listenForRouteChange(): void { @@ -442,7 +461,7 @@ export class MetadataService { if (content) { const tag = { property, content } as MetaDefinition; this.meta.addTag(tag); - this.storeTag(property, tag); + this.storeTag(property); } } @@ -452,33 +471,21 @@ export class MetadataService { } } - private storeTag(key: string, tag: MetaDefinition): void { - const tags: MetaDefinition[] = this.getTags(key); - tags.push(tag); - this.setTags(key, tags); - } - - private getTags(key: string): MetaDefinition[] { - let tags: MetaDefinition[] = this.tagStore.get(key); - if (tags === undefined) { - tags = []; - } - return tags; - } - - private setTags(key: string, tags: MetaDefinition[]): void { - this.tagStore.set(key, tags); + private storeTag(key: string): void { + this.store.dispatch(new AddMetaTagAction(key)); } public clearMetaTags() { - this.tagStore.forEach((tags: MetaDefinition[], property: string) => { - this.meta.removeTag('property=\'' + property + '\''); + this.store.pipe( + select(tagsInUseSelector), + take(1) + ).subscribe((tagsInUse: string[]) => { + for (const property of tagsInUse) { + this.meta.removeTag('property=\'' + property + '\''); + } + this.store.dispatch(new ClearMetaTagAction()); }); - this.tagStore.clear(); } - public getTagStore(): Map { - return this.tagStore; - } } From 50400895de6aca9ebe85068a12092d788e7d71ba Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 1 Jul 2021 22:36:02 +0200 Subject: [PATCH 16/20] 79768: Add unit tests for metaTagReducer --- src/app/core/metadata/meta-tag.actions.ts | 1 - .../core/metadata/meta-tag.reducer.spec.ts | 50 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 src/app/core/metadata/meta-tag.reducer.spec.ts diff --git a/src/app/core/metadata/meta-tag.actions.ts b/src/app/core/metadata/meta-tag.actions.ts index 6451e58da2..cd048d3be2 100644 --- a/src/app/core/metadata/meta-tag.actions.ts +++ b/src/app/core/metadata/meta-tag.actions.ts @@ -1,6 +1,5 @@ import { type } from '../../shared/ngrx/type'; import { Action } from '@ngrx/store'; -import { MetaDefinition } from '@angular/platform-browser'; // tslint:disable:max-classes-per-file export const MetaTagTypes = { diff --git a/src/app/core/metadata/meta-tag.reducer.spec.ts b/src/app/core/metadata/meta-tag.reducer.spec.ts new file mode 100644 index 0000000000..1fcd7d83e3 --- /dev/null +++ b/src/app/core/metadata/meta-tag.reducer.spec.ts @@ -0,0 +1,50 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ + +import { metaTagReducer } from './meta-tag.reducer'; +import { AddMetaTagAction, ClearMetaTagAction } from './meta-tag.actions'; + +const nullAction = { type: null }; + +describe('metaTagReducer', () => { + it('should start with an empty array', () => { + const state0 = metaTagReducer(undefined, nullAction); + expect(state0.tagsInUse).toEqual([]); + }); + + it('should return the current state on invalid action', () => { + const state0 = { + tagsInUse: ['foo', 'bar'], + }; + + const state1 = metaTagReducer(state0, nullAction); + expect(state1).toEqual(state0); + }); + + it('should add tags on AddMetaTagAction', () => { + const state0 = { + tagsInUse: ['foo'], + }; + + const state1 = metaTagReducer(state0, new AddMetaTagAction('bar')); + const state2 = metaTagReducer(state1, new AddMetaTagAction('baz')); + + expect(state1.tagsInUse).toEqual(['foo', 'bar']); + expect(state2.tagsInUse).toEqual(['foo', 'bar', 'baz']); + }); + + it('should clear tags on ClearMetaTagAction', () => { + const state0 = { + tagsInUse: ['foo', 'bar'], + }; + + const state1 = metaTagReducer(state0, new ClearMetaTagAction()); + + expect(state1.tagsInUse).toEqual([]); + }); +}); From fb8f28f17d1047986e3ace7331e3f32111b7c46a Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 1 Jul 2021 22:37:07 +0200 Subject: [PATCH 17/20] 79768: Update & add MetadataService unit tests --- .../core/metadata/metadata.service.spec.ts | 50 ++++++++++++++++--- src/app/core/metadata/metadata.service.ts | 16 +++--- 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index d18897cc55..f946120cd2 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -1,6 +1,6 @@ import { fakeAsync, tick } from '@angular/core/testing'; import { Meta, Title } from '@angular/platform-browser'; -import { Router, NavigationEnd } from '@angular/router'; +import { NavigationEnd, Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; import { Observable, of } from 'rxjs'; @@ -8,11 +8,8 @@ import { Observable, of } from 'rxjs'; import { RemoteData } from '../data/remote-data'; import { Item } from '../shared/item.model'; -import { ItemMock, MockBitstream1, MockBitstream3, } from '../../shared/mocks/item.mock'; -import { - createSuccessfulRemoteDataObject$, - createSuccessfulRemoteDataObject -} from '../../shared/remote-data.utils'; +import { ItemMock, MockBitstream1, MockBitstream3 } from '../../shared/mocks/item.mock'; +import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { PaginatedList } from '../data/paginated-list.model'; import { Bitstream } from '../shared/bitstream.model'; import { MetadataValue } from '../shared/metadata.models'; @@ -24,6 +21,11 @@ import { createPaginatedList } from '../../shared/testing/utils.test'; import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; import { DSONameService } from '../breadcrumbs/dso-name.service'; import { HardRedirectService } from '../services/hard-redirect.service'; +import { getMockStore, MockStore } from '@ngrx/store/testing'; +import { CoreState } from '../core.reducers'; +import { MetaTagState } from './meta-tag.reducer'; +import { AddMetaTagAction, ClearMetaTagAction } from './meta-tag.actions'; +import { Community } from '../shared/community.model'; describe('MetadataService', () => { let metadataService: MetadataService; @@ -41,6 +43,10 @@ describe('MetadataService', () => { let hardRedirectService: HardRedirectService; let router: Router; + let store; + + const initialState = { 'core': { metaTag: { tagsInUse: ['title', 'description'] }}}; + beforeEach(() => { rootService = jasmine.createSpyObj({ @@ -53,7 +59,7 @@ describe('MetadataService', () => { findByItemAndName: mockBundleRD$([MockBitstream3]) }); translateService = getMockTranslateService(); - meta = jasmine.createSpyObj({ + meta = jasmine.createSpyObj('meta', { addTag: {}, removeTag: {} }); @@ -73,6 +79,11 @@ describe('MetadataService', () => { hardRedirectService = jasmine.createSpyObj( { getRequestOrigin: 'https://request.org', }); + + //@ts-ignore + store = getMockStore({ initialState }); + spyOn(store, 'dispatch'); + metadataService = new MetadataService( router, translateService, @@ -83,6 +94,7 @@ describe('MetadataService', () => { bitstreamDataService, undefined, rootService, + store, hardRedirectService ); }); @@ -332,6 +344,30 @@ describe('MetadataService', () => { }); }); + describe('tagstore', () => { + beforeEach(fakeAsync(() => { + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(ItemMock), + } + } + }); + tick(); + })); + + it('should remove previous tags on route change', fakeAsync(() => { + expect(meta.removeTag).toHaveBeenCalledWith('property=\'title\''); + expect(meta.removeTag).toHaveBeenCalledWith('property=\'description\''); + })); + + it('should clear all tags and add new ones on route change', () => { + expect(store.dispatch.calls.argsFor(0)).toEqual([new ClearMetaTagAction()]); + expect(store.dispatch.calls.argsFor(1)).toEqual([new AddMetaTagAction('title')]); + expect(store.dispatch.calls.argsFor(2)).toEqual([new AddMetaTagAction('description')]); + }); + }); + const mockType = (mockItem: Item, type: string): Item => { const typedMockItem = Object.assign(new Item(), mockItem) as Item; typedMockItem.metadata['dc.type'] = [{ value: type }] as MetadataValue[]; diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 8c1e1027dd..f42747fbc5 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -5,10 +5,10 @@ import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; -import { BehaviorSubject, combineLatest, Observable, of as observableOf, EMPTY } from 'rxjs'; -import { filter, map, take, switchMap, expand } from 'rxjs/operators'; +import { BehaviorSubject, combineLatest, EMPTY, Observable, of as observableOf } from 'rxjs'; +import { expand, filter, map, switchMap, take } from 'rxjs/operators'; -import { hasValue, hasNoValue } from '../../shared/empty.util'; +import { hasNoValue, hasValue } from '../../shared/empty.util'; import { DSONameService } from '../breadcrumbs/dso-name.service'; import { BitstreamDataService } from '../data/bitstream-data.service'; import { BitstreamFormatDataService } from '../data/bitstream-format-data.service'; @@ -18,10 +18,7 @@ import { BitstreamFormat } from '../shared/bitstream-format.model'; import { Bitstream } from '../shared/bitstream.model'; import { DSpaceObject } from '../shared/dspace-object.model'; import { Item } from '../shared/item.model'; -import { - getFirstSucceededRemoteDataPayload, - getFirstCompletedRemoteData -} from '../shared/operators'; +import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../shared/operators'; import { RootDataService } from '../data/root-data.service'; import { getBitstreamDownloadRoute } from '../../app-routing-paths'; import { BundleDataService } from '../data/bundle-data.service'; @@ -31,11 +28,10 @@ import { PaginatedList } from '../data/paginated-list.model'; import { URLCombiner } from '../url-combiner/url-combiner'; import { HardRedirectService } from '../services/hard-redirect.service'; import { MetaTagState } from './meta-tag.reducer'; -import { Store, createSelector, select, MemoizedSelector } from '@ngrx/store'; +import { createSelector, select, Store } from '@ngrx/store'; import { AddMetaTagAction, ClearMetaTagAction } from './meta-tag.actions'; import { coreSelector } from '../core.selectors'; import { CoreState } from '../core.reducers'; -import { ObjectCacheEntry, ObjectCacheState } from '../cache/object-cache.reducer'; /** * The base selector function to select the metaTag section in the store @@ -84,7 +80,7 @@ export class MetadataService { private bitstreamDataService: BitstreamDataService, private bitstreamFormatDataService: BitstreamFormatDataService, private rootService: RootDataService, - private store: Store, + private store: Store, private hardRedirectService: HardRedirectService, ) { } From 8caa9163165df0b669eb8ad9dd4f5d23b9540529 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 1 Jul 2021 22:37:55 +0200 Subject: [PATCH 18/20] 79768: Fix typo --- src/app/core/metadata/metadata.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index f42747fbc5..bd0ad66c7b 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -184,7 +184,7 @@ export class MetadataService { private setDescriptionTag(): void { // TODO: truncate abstract const value = this.getMetaTagValue('dc.description.abstract'); - this.addMetaTag('desciption', value); + this.addMetaTag('description', value); } /** From 5ed41b3f9bc8a611fe034697efc49c1127580e37 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 1 Jul 2021 22:47:26 +0200 Subject: [PATCH 19/20] 79768: Fix unused imports & lint issue --- src/app/core/metadata/metadata.service.spec.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index f946120cd2..b3404e84d5 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -21,11 +21,8 @@ import { createPaginatedList } from '../../shared/testing/utils.test'; import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; import { DSONameService } from '../breadcrumbs/dso-name.service'; import { HardRedirectService } from '../services/hard-redirect.service'; -import { getMockStore, MockStore } from '@ngrx/store/testing'; -import { CoreState } from '../core.reducers'; -import { MetaTagState } from './meta-tag.reducer'; +import { getMockStore } from '@ngrx/store/testing'; import { AddMetaTagAction, ClearMetaTagAction } from './meta-tag.actions'; -import { Community } from '../shared/community.model'; describe('MetadataService', () => { let metadataService: MetadataService; @@ -80,7 +77,7 @@ describe('MetadataService', () => { getRequestOrigin: 'https://request.org', }); - //@ts-ignore + // @ts-ignore store = getMockStore({ initialState }); spyOn(store, 'dispatch'); From a91f16ed62ab4bf050a21c900de9f5d6b8108c91 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 1 Jul 2021 23:11:57 +0200 Subject: [PATCH 20/20] 79768: Fix followLink syntax --- src/app/core/metadata/metadata.service.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index bd0ad66c7b..10e37b4282 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -292,13 +292,7 @@ export class MetadataService { true, true, followLink('primaryBitstream'), - followLink('bitstreams', - undefined, - true, - true, - true, - followLink('format') - ) + followLink('bitstreams', {}, followLink('format')), ).pipe( getFirstSucceededRemoteDataPayload(), switchMap((bundle: Bundle) =>