From 03e2e30510a4ab151f17d158ca8010e99ed09da6 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 30 Jun 2021 14:09:18 +0200 Subject: [PATCH] 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 }); }); }