diff --git a/src/app/+browse-by/+browse-by-date-page/browse-by-date-page.component.spec.ts b/src/app/+browse-by/+browse-by-date-page/browse-by-date-page.component.spec.ts index a5cc69e430..5ce52d0002 100644 --- a/src/app/+browse-by/+browse-by-date-page/browse-by-date-page.component.spec.ts +++ b/src/app/+browse-by/+browse-by-date-page/browse-by-date-page.component.spec.ts @@ -107,6 +107,6 @@ describe('BrowseByDatePageComponent', () => { }); it('should create a list of startsWith options with the current year first', () => { - expect(comp.startsWithOptions[0]).toEqual(new Date().getFullYear()); + expect(comp.startsWithOptions[0]).toEqual(new Date().getUTCFullYear()); }); }); diff --git a/src/app/+browse-by/+browse-by-date-page/browse-by-date-page.component.ts b/src/app/+browse-by/+browse-by-date-page/browse-by-date-page.component.ts index a9eaa09e2f..f579085b5c 100644 --- a/src/app/+browse-by/+browse-by-date-page/browse-by-date-page.component.ts +++ b/src/app/+browse-by/+browse-by-date-page/browse-by-date-page.component.ts @@ -92,7 +92,7 @@ export class BrowseByDatePageComponent extends BrowseByMetadataPageComponent { } } const options = []; - const currentYear = new Date().getFullYear(); + const currentYear = new Date().getUTCFullYear(); const oneYearBreak = Math.floor((currentYear - environment.browseBy.oneYearLimit) / 5) * 5; const fiveYearBreak = Math.floor((currentYear - environment.browseBy.fiveYearLimit) / 10) * 10; if (lowerLimit <= fiveYearBreak) { diff --git a/src/app/+browse-by/+browse-by-metadata-page/browse-by-metadata-page.component.ts b/src/app/+browse-by/+browse-by-metadata-page/browse-by-metadata-page.component.ts index f5adefc779..5b84daab6e 100644 --- a/src/app/+browse-by/+browse-by-metadata-page/browse-by-metadata-page.component.ts +++ b/src/app/+browse-by/+browse-by-metadata-page/browse-by-metadata-page.component.ts @@ -167,7 +167,6 @@ export class BrowseByMetadataPageComponent implements OnInit { * @param value The value of the browse-entry to display items for */ updatePageWithItems(searchOptions: BrowseEntrySearchOptions, value: string) { - console.log('updatePAge', searchOptions); this.items$ = this.browseService.getBrowseItemsFor(value, searchOptions); } diff --git a/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.html b/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.html index cc4b0c22a1..e10b9da247 100644 --- a/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.html +++ b/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.html @@ -42,6 +42,7 @@ [key]="'map'" [dsoRD$]="mappedItemsRD$" [paginationOptions]="(searchOptions$ | async)?.pagination" + [featureId]="FeatureIds.CanManageMappings" [confirmButton]="'collection.edit.item-mapper.confirm'" [cancelButton]="'collection.edit.item-mapper.cancel'" (confirm)="mapItems($event)" diff --git a/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.spec.ts b/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.spec.ts index 49b6a0d63c..5ae1445cef 100644 --- a/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.spec.ts +++ b/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.spec.ts @@ -40,6 +40,7 @@ import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { createPaginatedList } from '../../shared/testing/utils.test'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; describe('CollectionItemMapperComponent', () => { let comp: CollectionItemMapperComponent; @@ -136,6 +137,10 @@ describe('CollectionItemMapperComponent', () => { } }; + const authorizationDataService = jasmine.createSpyObj('authorizationDataService', { + isAuthorized: observableOf(true) + }); + beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [CommonModule, FormsModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule], @@ -152,6 +157,7 @@ describe('CollectionItemMapperComponent', () => { { provide: HostWindowService, useValue: new HostWindowServiceStub(0) }, { provide: ObjectSelectService, useValue: new ObjectSelectServiceStub() }, { provide: RouteService, useValue: routeServiceStub }, + { provide: AuthorizationDataService, useValue: authorizationDataService } ] }).compileComponents(); })); diff --git a/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.ts b/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.ts index 571b755897..ce5ea5a142 100644 --- a/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.ts +++ b/src/app/+collection-page/collection-item-mapper/collection-item-mapper.component.ts @@ -28,6 +28,7 @@ import { PaginatedSearchOptions } from '../../shared/search/paginated-search-opt import { SearchService } from '../../core/shared/search/search.service'; import { followLink } from '../../shared/utils/follow-link-config.model'; import { NoContent } from '../../core/shared/NoContent.model'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; @Component({ selector: 'ds-collection-item-mapper', @@ -50,6 +51,8 @@ import { NoContent } from '../../core/shared/NoContent.model'; */ export class CollectionItemMapperComponent implements OnInit { + FeatureIds = FeatureID; + /** * A view on the tabset element * Used to switch tabs programmatically 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/edit-item-page/item-bitstreams/item-bitstreams.component.scss b/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.scss index 1fcbe99702..46d172dadc 100644 --- a/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.scss +++ b/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.scss @@ -19,11 +19,15 @@ .drag-handle { visibility: hidden; &:hover { - cursor: grab; + cursor: move; } } -:host ::ng-deep .bitstream-row:hover .drag-handle { +.bitstream-row-drag-handle:hover { + cursor: move; +} + +:host ::ng-deep .bitstream-row:hover .drag-handle, :host ::ng-deep .bitstream-row-drag-handle:focus .drag-handle { visibility: visible !important; } @@ -40,3 +44,9 @@ .cdk-drag-animating { transition: transform 250ms cubic-bezier(0, 0, 0.2, 1); } + +.cdk-drop-list-dragging { + .bitstream-row-drag-handle, .drag-handle { + cursor: grabbing; + } +} diff --git a/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.html b/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.html index 8f0d83bd1f..f7904f6cc8 100644 --- a/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.html +++ b/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.html @@ -21,7 +21,7 @@ -
+
diff --git a/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-drag-handle/item-edit-bitstream-drag-handle.component.html b/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-drag-handle/item-edit-bitstream-drag-handle.component.html index 0561f78e97..1bce8667ee 100644 --- a/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-drag-handle/item-edit-bitstream-drag-handle.component.html +++ b/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-drag-handle/item-edit-bitstream-drag-handle.component.html @@ -1,5 +1,5 @@ -
+
diff --git a/src/app/+item-page/edit-item-page/item-collection-mapper/item-collection-mapper.component.spec.ts b/src/app/+item-page/edit-item-page/item-collection-mapper/item-collection-mapper.component.spec.ts index d3577c3637..c15c84a647 100644 --- a/src/app/+item-page/edit-item-page/item-collection-mapper/item-collection-mapper.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-collection-mapper/item-collection-mapper.component.spec.ts @@ -40,6 +40,7 @@ import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; import { createPaginatedList } from '../../../shared/testing/utils.test'; +import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; describe('ItemCollectionMapperComponent', () => { let comp: ItemCollectionMapperComponent; @@ -110,6 +111,10 @@ describe('ItemCollectionMapperComponent', () => { onDefaultLangChange: new EventEmitter() }; + const authorizationDataService = jasmine.createSpyObj('authorizationDataService', { + isAuthorized: observableOf(true) + }); + beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [CommonModule, FormsModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule], @@ -124,7 +129,8 @@ describe('ItemCollectionMapperComponent', () => { { provide: ObjectSelectService, useValue: new ObjectSelectServiceStub() }, { provide: TranslateService, useValue: translateServiceStub }, { provide: HostWindowService, useValue: new HostWindowServiceStub(0) }, - { provide: CollectionDataService, useValue: collectionDataServiceStub } + { provide: CollectionDataService, useValue: collectionDataServiceStub }, + { provide: AuthorizationDataService, useValue: authorizationDataService } ] }).compileComponents(); })); diff --git a/src/app/+item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html b/src/app/+item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html index cf226f7733..5277fa1a3a 100644 --- a/src/app/+item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html +++ b/src/app/+item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html @@ -29,7 +29,7 @@ {{metadata?.value}}
-
@@ -40,7 +40,7 @@ {{metadata?.language}}
-
diff --git a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.html b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.html index 008708d3fb..e154487402 100644 --- a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.html +++ b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.html @@ -25,9 +25,9 @@ - - - + + + - diff --git a/src/app/+item-page/full/full-item-page.component.spec.ts b/src/app/+item-page/full/full-item-page.component.spec.ts index b4b9e77ed5..b4ab926667 100644 --- a/src/app/+item-page/full/full-item-page.component.spec.ts +++ b/src/app/+item-page/full/full-item-page.component.spec.ts @@ -1,4 +1,4 @@ -import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; +import { ComponentFixture, fakeAsync, TestBed, waitForAsync } from '@angular/core/testing'; import { ItemDataService } from '../../core/data/item-data.service'; import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock'; @@ -29,9 +29,7 @@ const mockItem: Item = Object.assign(new Item(), { ] } }); -const routeStub = Object.assign(new ActivatedRouteStub(), { - data: observableOf({ dso: createSuccessfulRemoteDataObject(mockItem) }) -}); + const metadataServiceStub = { /* tslint:disable:no-empty */ processRemoteData: () => { @@ -44,6 +42,10 @@ describe('FullItemPageComponent', () => { let fixture: ComponentFixture; let authService: AuthService; + let routeStub: ActivatedRouteStub; + let routeData; + + beforeEach(waitForAsync(() => { authService = jasmine.createSpyObj('authService', { @@ -51,6 +53,14 @@ describe('FullItemPageComponent', () => { setRedirectUrl: {} }); + routeData = { + dso: createSuccessfulRemoteDataObject(mockItem), + }; + + routeStub = Object.assign(new ActivatedRouteStub(), { + data: observableOf(routeData) + }); + TestBed.configureTestingModule({ imports: [TranslateModule.forRoot({ loader: { @@ -84,4 +94,21 @@ describe('FullItemPageComponent', () => { expect(table.nativeElement.innerHTML).toContain(metadatum.value); } }); + + it('should show simple view button when not originated from workflow item', () => { + expect(comp.fromWfi).toBe(false); + const simpleViewBtn = fixture.debugElement.query(By.css('.simple-view-link')); + expect(simpleViewBtn).toBeTruthy(); + }); + + it('should not show simple view button when originated from workflow', fakeAsync(() => { + routeData.wfi = createSuccessfulRemoteDataObject$({ id: 'wfiId'}); + comp.ngOnInit(); + fixture.detectChanges(); + fixture.whenStable().then(() => { + expect(comp.fromWfi).toBe(true); + const simpleViewBtn = fixture.debugElement.query(By.css('.simple-view-link')); + expect(simpleViewBtn).toBeFalsy(); + }); + })); }); 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..7f1b6de614 100644 --- a/src/app/+item-page/full/full-item-page.component.ts +++ b/src/app/+item-page/full/full-item-page.component.ts @@ -1,6 +1,6 @@ -import {filter, map} from 'rxjs/operators'; -import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core'; -import { ActivatedRoute, Router } from '@angular/router'; +import { filter, map } from 'rxjs/operators'; +import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from '@angular/core'; +import { ActivatedRoute, Data, Router } from '@angular/router'; import { Observable , BehaviorSubject } from 'rxjs'; @@ -11,11 +11,11 @@ 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'; +import { Location } from '@angular/common'; + /** * This component renders a full item page. @@ -29,14 +29,25 @@ import { AuthService } from '../../core/auth/auth.service'; changeDetection: ChangeDetectionStrategy.OnPush, animations: [fadeInOut] }) -export class FullItemPageComponent extends ItemPageComponent implements OnInit { +export class FullItemPageComponent extends ItemPageComponent implements OnInit, OnDestroy { itemRD$: BehaviorSubject>; metadata$: Observable; - constructor(route: ActivatedRoute, router: Router, items: ItemDataService, metadataService: MetadataService, authService: AuthService) { - super(route, router, items, metadataService, authService); + /** + * True when the itemRD has been originated from its workflowitem, false otherwise. + */ + fromWfi = false; + + subs = []; + + constructor(protected route: ActivatedRoute, + router: Router, + items: ItemDataService, + authService: AuthService, + private _location: Location) { + super(route, router, items, authService); } /*** AoT inheritance fix, will hopefully be resolved in the near future **/ @@ -46,5 +57,21 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit { map((rd: RemoteData) => rd.payload), filter((item: Item) => hasValue(item)), map((item: Item) => item.metadata),); + + this.subs.push(this.route.data.subscribe((data: Data) => { + this.fromWfi = hasValue(data.wfi); + }) + ); + } + + /** + * Navigate back in browser history. + */ + back() { + this._location.back(); + } + + ngOnDestroy() { + this.subs.filter((sub) => hasValue(sub)).forEach((sub) => sub.unsubscribe()); } } diff --git a/src/app/+item-page/simple/item-page.component.ts b/src/app/+item-page/simple/item-page.component.ts index 67e278c2fb..cc23ba86d5 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'; @@ -51,10 +49,9 @@ export class ItemPageComponent implements OnInit { itemPageRoute$: Observable; constructor( - private route: ActivatedRoute, + protected 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/+workflowitems-edit-page/item-from-workflow.resolver.spec.ts b/src/app/+workflowitems-edit-page/item-from-workflow.resolver.spec.ts new file mode 100644 index 0000000000..1ef87ad10f --- /dev/null +++ b/src/app/+workflowitems-edit-page/item-from-workflow.resolver.spec.ts @@ -0,0 +1,36 @@ +import { first } from 'rxjs/operators'; +import { WorkflowItemDataService } from '../core/submission/workflowitem-data.service'; +import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; +import { ItemFromWorkflowResolver } from './item-from-workflow.resolver'; + +describe('ItemFromWorkflowResolver', () => { + describe('resolve', () => { + let resolver: ItemFromWorkflowResolver; + let wfiService: WorkflowItemDataService; + const uuid = '1234-65487-12354-1235'; + const itemUuid = '8888-8888-8888-8888'; + const wfi = { + id: uuid, + item: createSuccessfulRemoteDataObject$({ id: itemUuid }) + }; + + + beforeEach(() => { + wfiService = { + findById: (id: string) => createSuccessfulRemoteDataObject$(wfi) + } as any; + resolver = new ItemFromWorkflowResolver(wfiService, null); + }); + + it('should resolve a an item from from the workflow item with the correct id', (done) => { + resolver.resolve({ params: { id: uuid } } as any, undefined) + .pipe(first()) + .subscribe( + (resolved) => { + expect(resolved.payload.id).toEqual(itemUuid); + done(); + } + ); + }); + }); +}); diff --git a/src/app/+workflowitems-edit-page/item-from-workflow.resolver.ts b/src/app/+workflowitems-edit-page/item-from-workflow.resolver.ts new file mode 100644 index 0000000000..2aaa762b2a --- /dev/null +++ b/src/app/+workflowitems-edit-page/item-from-workflow.resolver.ts @@ -0,0 +1,43 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot } from '@angular/router'; +import { Observable } from 'rxjs'; +import { RemoteData } from '../core/data/remote-data'; +import { Item } from '../core/shared/item.model'; +import { followLink } from '../shared/utils/follow-link-config.model'; +import { getFirstCompletedRemoteData } from '../core/shared/operators'; +import { Store } from '@ngrx/store'; +import { WorkflowItemDataService } from '../core/submission/workflowitem-data.service'; +import { WorkflowItem } from '../core/submission/models/workflowitem.model'; +import { switchMap } from 'rxjs/operators'; + +/** + * This class represents a resolver that requests a specific item before the route is activated + */ +@Injectable() +export class ItemFromWorkflowResolver implements Resolve> { + constructor( + private workflowItemService: WorkflowItemDataService, + protected store: Store + ) { + } + + /** + * Method for resolving an item based on the parameters in the current route + * @param {ActivatedRouteSnapshot} route The current ActivatedRouteSnapshot + * @param {RouterStateSnapshot} state The current RouterStateSnapshot + * @returns Observable<> Emits the found item based on the parameters in the current route, + * or an error if something went wrong + */ + resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable> { + const itemRD$ = this.workflowItemService.findById(route.params.id, + true, + false, + followLink('item'), + ).pipe( + getFirstCompletedRemoteData(), + switchMap((wfiRD: RemoteData) => wfiRD.payload.item as Observable>), + getFirstCompletedRemoteData() + ); + return itemRD$; + } +} diff --git a/src/app/+workflowitems-edit-page/workflowitems-edit-page-routing-paths.ts b/src/app/+workflowitems-edit-page/workflowitems-edit-page-routing-paths.ts index a8802e7a2f..e2d969a872 100644 --- a/src/app/+workflowitems-edit-page/workflowitems-edit-page-routing-paths.ts +++ b/src/app/+workflowitems-edit-page/workflowitems-edit-page-routing-paths.ts @@ -8,6 +8,9 @@ export function getWorkflowItemPageRoute(wfiId: string) { export function getWorkflowItemEditRoute(wfiId: string) { return new URLCombiner(getWorkflowItemModuleRoute(), wfiId, WORKFLOW_ITEM_EDIT_PATH).toString(); } +export function getWorkflowItemViewRoute(wfiId: string) { + return new URLCombiner(getWorkflowItemModuleRoute(), wfiId, WORKFLOW_ITEM_VIEW_PATH).toString(); +} export function getWorkflowItemDeleteRoute(wfiId: string) { return new URLCombiner(getWorkflowItemModuleRoute(), wfiId, WORKFLOW_ITEM_DELETE_PATH).toString(); @@ -19,4 +22,5 @@ export function getWorkflowItemSendBackRoute(wfiId: string) { export const WORKFLOW_ITEM_EDIT_PATH = 'edit'; export const WORKFLOW_ITEM_DELETE_PATH = 'delete'; +export const WORKFLOW_ITEM_VIEW_PATH = 'view'; export const WORKFLOW_ITEM_SEND_BACK_PATH = 'sendback'; diff --git a/src/app/+workflowitems-edit-page/workflowitems-edit-page-routing.module.ts b/src/app/+workflowitems-edit-page/workflowitems-edit-page-routing.module.ts index 733eebecc2..7daee0a472 100644 --- a/src/app/+workflowitems-edit-page/workflowitems-edit-page-routing.module.ts +++ b/src/app/+workflowitems-edit-page/workflowitems-edit-page-routing.module.ts @@ -6,12 +6,15 @@ import { WorkflowItemPageResolver } from './workflow-item-page.resolver'; import { WORKFLOW_ITEM_DELETE_PATH, WORKFLOW_ITEM_EDIT_PATH, - WORKFLOW_ITEM_SEND_BACK_PATH + WORKFLOW_ITEM_SEND_BACK_PATH, + WORKFLOW_ITEM_VIEW_PATH } from './workflowitems-edit-page-routing-paths'; import { ThemedSubmissionEditComponent } from '../submission/edit/themed-submission-edit.component'; import { ThemedWorkflowItemDeleteComponent } from './workflow-item-delete/themed-workflow-item-delete.component'; import { ThemedWorkflowItemSendBackComponent } from './workflow-item-send-back/themed-workflow-item-send-back.component'; import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver'; +import { ItemFromWorkflowResolver } from './item-from-workflow.resolver'; +import { ThemedFullItemPageComponent } from '../+item-page/full/themed-full-item-page.component'; @NgModule({ imports: [ @@ -29,6 +32,16 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso }, data: { title: 'workflow-item.edit.title', breadcrumbKey: 'workflow-item.edit' } }, + { + canActivate: [AuthenticatedGuard], + path: WORKFLOW_ITEM_VIEW_PATH, + component: ThemedFullItemPageComponent, + resolve: { + dso: ItemFromWorkflowResolver, + breadcrumb: I18nBreadcrumbResolver + }, + data: { title: 'workflow-item.view.title', breadcrumbKey: 'workflow-item.view' } + }, { canActivate: [AuthenticatedGuard], path: WORKFLOW_ITEM_DELETE_PATH, @@ -51,7 +64,7 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso }] ) ], - providers: [WorkflowItemPageResolver] + providers: [WorkflowItemPageResolver, ItemFromWorkflowResolver] }) /** * This module defines the default component to load when navigating to the workflowitems edit page path. diff --git a/src/app/+workflowitems-edit-page/workflowitems-edit-page.module.ts b/src/app/+workflowitems-edit-page/workflowitems-edit-page.module.ts index 6e4b3212e8..463e76e54d 100644 --- a/src/app/+workflowitems-edit-page/workflowitems-edit-page.module.ts +++ b/src/app/+workflowitems-edit-page/workflowitems-edit-page.module.ts @@ -7,6 +7,8 @@ import { WorkflowItemDeleteComponent } from './workflow-item-delete/workflow-ite import { WorkflowItemSendBackComponent } from './workflow-item-send-back/workflow-item-send-back.component'; import { ThemedWorkflowItemDeleteComponent } from './workflow-item-delete/themed-workflow-item-delete.component'; import { ThemedWorkflowItemSendBackComponent } from './workflow-item-send-back/themed-workflow-item-send-back.component'; +import { StatisticsModule } from '../statistics/statistics.module'; +import { ItemPageModule } from '../+item-page/item-page.module'; @NgModule({ imports: [ @@ -14,8 +16,15 @@ import { ThemedWorkflowItemSendBackComponent } from './workflow-item-send-back/t CommonModule, SharedModule, SubmissionModule, + StatisticsModule, + ItemPageModule ], - declarations: [WorkflowItemDeleteComponent, ThemedWorkflowItemDeleteComponent, WorkflowItemSendBackComponent, ThemedWorkflowItemSendBackComponent] + declarations: [ + WorkflowItemDeleteComponent, + ThemedWorkflowItemDeleteComponent, + WorkflowItemSendBackComponent, + ThemedWorkflowItemSendBackComponent + ] }) /** * This module handles all modules that need to access the workflowitems edit page. diff --git a/src/app/app.component.ts b/src/app/app.component.ts index c9996f275a..356025da9e 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -1,4 +1,4 @@ -import { delay, distinctUntilChanged, filter, take } from 'rxjs/operators'; +import { delay, distinctUntilChanged, filter, take, withLatestFrom } from 'rxjs/operators'; import { AfterViewInit, ChangeDetectionStrategy, @@ -38,6 +38,8 @@ import { ThemeService } from './shared/theme-support/theme.service'; import { BASE_THEME_NAME } from './shared/theme-support/theme.constants'; import { DEFAULT_THEME_CONFIG } from './shared/theme-support/theme.effects'; import { BreadcrumbsService } from './breadcrumbs/breadcrumbs.service'; +import { IdleModalComponent } from './shared/idle-modal/idle-modal.component'; +import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; @Component({ selector: 'ds-app', @@ -70,6 +72,11 @@ export class AppComponent implements OnInit, AfterViewInit { isThemeLoading$: BehaviorSubject = new BehaviorSubject(false); + /** + * Whether or not the idle modal is is currently open + */ + idleModalOpen: boolean; + constructor( @Inject(NativeWindowService) private _window: NativeWindowRef, @Inject(DOCUMENT) private document: any, @@ -87,6 +94,7 @@ export class AppComponent implements OnInit, AfterViewInit { private windowService: HostWindowService, private localeService: LocaleService, private breadcrumbsService: BreadcrumbsService, + private modalService: NgbModal, @Optional() private cookiesService: KlaroService, @Optional() private googleAnalyticsService: GoogleAnalyticsService, ) { @@ -108,6 +116,11 @@ export class AppComponent implements OnInit, AfterViewInit { } }); + if (isPlatformBrowser(this.platformId)) { + this.authService.trackTokenExpiration(); + this.trackIdleModal(); + } + // Load all the languages that are defined as active from the config file translate.addLangs(environment.languages.filter((LangConfig) => LangConfig.active === true).map((a) => a.code)); @@ -130,7 +143,6 @@ export class AppComponent implements OnInit, AfterViewInit { console.info(environment); } this.storeCSSVariables(); - } ngOnInit() { @@ -229,4 +241,23 @@ export class AppComponent implements OnInit, AfterViewInit { }; head.appendChild(link); } + + private trackIdleModal() { + const isIdle$ = this.authService.isUserIdle(); + const isAuthenticated$ = this.authService.isAuthenticated(); + isIdle$.pipe(withLatestFrom(isAuthenticated$)) + .subscribe(([userIdle, authenticated]) => { + if (userIdle && authenticated) { + if (!this.idleModalOpen) { + const modalRef = this.modalService.open(IdleModalComponent, { ariaLabelledBy: 'idle-modal.header' }); + this.idleModalOpen = true; + modalRef.componentInstance.response.pipe(take(1)).subscribe((closed: boolean) => { + if (closed) { + this.idleModalOpen = false; + } + }); + } + } + }); + } } diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 3d45ffbfc2..18b97c8e9e 100755 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -47,6 +47,7 @@ import { ThemedHeaderComponent } from './header/themed-header.component'; import { ThemedFooterComponent } from './footer/themed-footer.component'; import { ThemedBreadcrumbsComponent } from './breadcrumbs/themed-breadcrumbs.component'; import { ThemedHeaderNavbarWrapperComponent } from './header-nav-wrapper/themed-header-navbar-wrapper.component'; +import { IdleModalComponent } from './shared/idle-modal/idle-modal.component'; export function getBase() { return environment.ui.nameSpace; @@ -144,6 +145,7 @@ const DECLARATIONS = [ ThemedBreadcrumbsComponent, ForbiddenComponent, ThemedForbiddenComponent, + IdleModalComponent ]; const EXPORTS = [ diff --git a/src/app/core/auth/auth.actions.ts b/src/app/core/auth/auth.actions.ts index e2cef3562f..ad3f9a9711 100644 --- a/src/app/core/auth/auth.actions.ts +++ b/src/app/core/auth/auth.actions.ts @@ -34,7 +34,9 @@ export const AuthActionTypes = { RETRIEVE_AUTHENTICATED_EPERSON: type('dspace/auth/RETRIEVE_AUTHENTICATED_EPERSON'), RETRIEVE_AUTHENTICATED_EPERSON_SUCCESS: type('dspace/auth/RETRIEVE_AUTHENTICATED_EPERSON_SUCCESS'), RETRIEVE_AUTHENTICATED_EPERSON_ERROR: type('dspace/auth/RETRIEVE_AUTHENTICATED_EPERSON_ERROR'), - REDIRECT_AFTER_LOGIN_SUCCESS: type('dspace/auth/REDIRECT_AFTER_LOGIN_SUCCESS') + REDIRECT_AFTER_LOGIN_SUCCESS: type('dspace/auth/REDIRECT_AFTER_LOGIN_SUCCESS'), + SET_USER_AS_IDLE: type('dspace/auth/SET_USER_AS_IDLE'), + UNSET_USER_AS_IDLE: type('dspace/auth/UNSET_USER_AS_IDLE') }; /* tslint:disable:max-classes-per-file */ @@ -404,6 +406,24 @@ export class RetrieveAuthenticatedEpersonErrorAction implements Action { this.payload = payload ; } } + +/** + * Set the current user as being idle. + * @class SetUserAsIdleAction + * @implements {Action} + */ +export class SetUserAsIdleAction implements Action { + public type: string = AuthActionTypes.SET_USER_AS_IDLE; +} + +/** + * Unset the current user as being idle. + * @class UnsetUserAsIdleAction + * @implements {Action} + */ +export class UnsetUserAsIdleAction implements Action { + public type: string = AuthActionTypes.UNSET_USER_AS_IDLE; +} /* tslint:enable:max-classes-per-file */ /** @@ -434,4 +454,7 @@ export type AuthActions | RetrieveAuthenticatedEpersonErrorAction | RetrieveAuthenticatedEpersonSuccessAction | SetRedirectUrlAction - | RedirectAfterLoginSuccessAction; + | RedirectAfterLoginSuccessAction + | SetUserAsIdleAction + | UnsetUserAsIdleAction; + diff --git a/src/app/core/auth/auth.effects.ts b/src/app/core/auth/auth.effects.ts index 2ef90dd76c..8ce10c0c6b 100644 --- a/src/app/core/auth/auth.effects.ts +++ b/src/app/core/auth/auth.effects.ts @@ -1,7 +1,13 @@ -import { Injectable } from '@angular/core'; +import { Injectable, NgZone } from '@angular/core'; -import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs'; -import { catchError, filter, map, switchMap, take, tap } from 'rxjs/operators'; +import { + combineLatest as observableCombineLatest, + Observable, + of as observableOf, + timer, + asyncScheduler, queueScheduler +} from 'rxjs'; +import { catchError, filter, map, switchMap, take, tap, observeOn } from 'rxjs/operators'; // import @ngrx import { Actions, Effect, ofType } from '@ngrx/effects'; import { Action, select, Store } from '@ngrx/store'; @@ -37,9 +43,19 @@ import { RetrieveAuthMethodsAction, RetrieveAuthMethodsErrorAction, RetrieveAuthMethodsSuccessAction, - RetrieveTokenAction + RetrieveTokenAction, SetUserAsIdleAction } from './auth.actions'; import { hasValue } from '../../shared/empty.util'; +import { environment } from '../../../environments/environment'; +import { RequestActionTypes } from '../data/request.actions'; +import { NotificationsActionTypes } from '../../shared/notifications/notifications.actions'; +import { LeaveZoneScheduler } from '../utilities/leave-zone.scheduler'; +import { EnterZoneScheduler } from '../utilities/enter-zone.scheduler'; + +// Action Types that do not break/prevent the user from an idle state +const IDLE_TIMER_IGNORE_TYPES: string[] + = [...Object.values(AuthActionTypes).filter((t: string) => t !== AuthActionTypes.UNSET_USER_AS_IDLE), + ...Object.values(RequestActionTypes), ...Object.values(NotificationsActionTypes)]; @Injectable() export class AuthEffects { @@ -242,13 +258,35 @@ export class AuthEffects { }) ); + /** + * For any action that is not in {@link IDLE_TIMER_IGNORE_TYPES} that comes in => Start the idleness timer + * If the idleness timer runs out (so no un-ignored action come through for that amount of time) + * => Return the action to set the user as idle ({@link SetUserAsIdleAction}) + * @method trackIdleness + */ + @Effect() + public trackIdleness$: Observable = this.actions$.pipe( + filter((action: Action) => !IDLE_TIMER_IGNORE_TYPES.includes(action.type)), + // Using switchMap the effect will stop subscribing to the previous timer if a new action comes + // in, and start a new timer + switchMap(() => + // Start a timer outside of Angular's zone + timer(environment.auth.ui.timeUntilIdle, new LeaveZoneScheduler(this.zone, asyncScheduler)) + ), + // Re-enter the zone to dispatch the action + observeOn(new EnterZoneScheduler(this.zone, queueScheduler)), + map(() => new SetUserAsIdleAction()), + ); + /** * @constructor * @param {Actions} actions$ + * @param {NgZone} zone * @param {AuthService} authService * @param {Store} store */ constructor(private actions$: Actions, + private zone: NgZone, private authService: AuthService, private store: Store) { } diff --git a/src/app/core/auth/auth.interceptor.ts b/src/app/core/auth/auth.interceptor.ts index 7b9a08de92..a49030110b 100644 --- a/src/app/core/auth/auth.interceptor.ts +++ b/src/app/core/auth/auth.interceptor.ts @@ -1,6 +1,6 @@ import { Observable, of as observableOf, throwError as observableThrowError } from 'rxjs'; -import { catchError, filter, map } from 'rxjs/operators'; +import { catchError, map } from 'rxjs/operators'; import { Injectable, Injector } from '@angular/core'; import { HttpErrorResponse, @@ -12,14 +12,13 @@ import { HttpResponse, HttpResponseBase } from '@angular/common/http'; -import { find } from 'lodash'; import { AppState } from '../../app.reducer'; import { AuthService } from './auth.service'; import { AuthStatus } from './models/auth-status.model'; import { AuthTokenInfo } from './models/auth-token-info.model'; -import { hasValue, isNotEmpty, isNotNull, isUndefined } from '../../shared/empty.util'; -import { RedirectWhenTokenExpiredAction, RefreshTokenAction } from './auth.actions'; +import { hasValue, isNotEmpty, isNotNull } from '../../shared/empty.util'; +import { RedirectWhenTokenExpiredAction } from './auth.actions'; import { Store } from '@ngrx/store'; import { Router } from '@angular/router'; import { AuthMethod } from './models/auth.method'; @@ -28,7 +27,7 @@ import { AuthMethodType } from './models/auth.method-type'; @Injectable() export class AuthInterceptor implements HttpInterceptor { - // Intercetor is called twice per request, + // Interceptor is called twice per request, // so to prevent RefreshTokenAction is dispatched twice // we're creating a refresh token request list protected refreshTokenRequestUrls = []; @@ -216,23 +215,8 @@ export class AuthInterceptor implements HttpInterceptor { let authorization: string; if (authService.isTokenExpired()) { - authService.setRedirectUrl(this.router.url); - // The access token is expired - // Redirect to the login route - this.store.dispatch(new RedirectWhenTokenExpiredAction('auth.messages.expired')); return observableOf(null); } else if ((!this.isAuthRequest(req) || this.isLogoutResponse(req)) && isNotEmpty(token)) { - // Intercept a request that is not to the authentication endpoint - authService.isTokenExpiring().pipe( - filter((isExpiring) => isExpiring)) - .subscribe(() => { - // If the current request url is already in the refresh token request list, skip it - if (isUndefined(find(this.refreshTokenRequestUrls, req.url))) { - // When a token is about to expire, refresh it - this.store.dispatch(new RefreshTokenAction(token)); - this.refreshTokenRequestUrls.push(req.url); - } - }); // Get the auth header from the service. authorization = authService.buildAuthHeader(token); let newHeaders = req.headers.set('authorization', authorization); diff --git a/src/app/core/auth/auth.reducer.spec.ts b/src/app/core/auth/auth.reducer.spec.ts index 914a1a152d..ba271fafb5 100644 --- a/src/app/core/auth/auth.reducer.spec.ts +++ b/src/app/core/auth/auth.reducer.spec.ts @@ -23,7 +23,7 @@ import { RetrieveAuthMethodsAction, RetrieveAuthMethodsErrorAction, RetrieveAuthMethodsSuccessAction, - SetRedirectUrlAction + SetRedirectUrlAction, SetUserAsIdleAction, UnsetUserAsIdleAction } from './auth.actions'; import { AuthTokenInfo } from './models/auth-token-info.model'; import { EPersonMock } from '../../shared/testing/eperson.mock'; @@ -44,6 +44,7 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: false, + idle: false }; const action = new AuthenticateAction('user', 'password'); const newState = authReducer(initialState, action); @@ -53,7 +54,8 @@ describe('authReducer', () => { blocking: true, error: undefined, loading: true, - info: undefined + info: undefined, + idle: false }; expect(newState).toEqual(state); @@ -66,7 +68,8 @@ describe('authReducer', () => { error: undefined, blocking: true, loading: true, - info: undefined + info: undefined, + idle: false }; const action = new AuthenticationSuccessAction(mockTokenInfo); const newState = authReducer(initialState, action); @@ -81,7 +84,8 @@ describe('authReducer', () => { error: undefined, blocking: true, loading: true, - info: undefined + info: undefined, + idle: false }; const action = new AuthenticationErrorAction(mockError); const newState = authReducer(initialState, action); @@ -92,7 +96,8 @@ describe('authReducer', () => { loading: false, info: undefined, authToken: undefined, - error: 'Test error message' + error: 'Test error message', + idle: false }; expect(newState).toEqual(state); @@ -105,7 +110,8 @@ describe('authReducer', () => { loaded: false, error: undefined, loading: true, - info: undefined + info: undefined, + idle: false }; const action = new AuthenticatedAction(mockTokenInfo); const newState = authReducer(initialState, action); @@ -115,7 +121,8 @@ describe('authReducer', () => { loaded: false, error: undefined, loading: true, - info: undefined + info: undefined, + idle: false }; expect(newState).toEqual(state); }); @@ -127,7 +134,8 @@ describe('authReducer', () => { error: undefined, blocking: true, loading: true, - info: undefined + info: undefined, + idle: false }; const action = new AuthenticatedSuccessAction(true, mockTokenInfo, EPersonMock._links.self.href); const newState = authReducer(initialState, action); @@ -138,7 +146,8 @@ describe('authReducer', () => { error: undefined, blocking: true, loading: true, - info: undefined + info: undefined, + idle: false }; expect(newState).toEqual(state); }); @@ -150,7 +159,8 @@ describe('authReducer', () => { error: undefined, blocking: true, loading: true, - info: undefined + info: undefined, + idle: false }; const action = new AuthenticatedErrorAction(mockError); const newState = authReducer(initialState, action); @@ -161,7 +171,8 @@ describe('authReducer', () => { loaded: true, blocking: false, loading: false, - info: undefined + info: undefined, + idle: false }; expect(newState).toEqual(state); }); @@ -172,6 +183,7 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: false, + idle: false }; const action = new CheckAuthenticationTokenAction(); const newState = authReducer(initialState, action); @@ -180,6 +192,7 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: true, + idle: false }; expect(newState).toEqual(state); }); @@ -190,6 +203,7 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: true, + idle: false }; const action = new CheckAuthenticationTokenCookieAction(); const newState = authReducer(initialState, action); @@ -198,6 +212,7 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: true, + idle: false }; expect(newState).toEqual(state); }); @@ -211,7 +226,8 @@ describe('authReducer', () => { blocking: false, loading: false, info: undefined, - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; const action = new LogOutAction(); @@ -229,7 +245,8 @@ describe('authReducer', () => { blocking: false, loading: false, info: undefined, - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; const action = new LogOutSuccessAction(); @@ -243,7 +260,8 @@ describe('authReducer', () => { loading: true, info: undefined, refreshing: false, - userId: undefined + userId: undefined, + idle: false }; expect(newState).toEqual(state); }); @@ -257,7 +275,8 @@ describe('authReducer', () => { blocking: false, loading: false, info: undefined, - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; const action = new LogOutErrorAction(mockError); @@ -270,7 +289,8 @@ describe('authReducer', () => { blocking: false, loading: false, info: undefined, - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; expect(newState).toEqual(state); }); @@ -283,7 +303,8 @@ describe('authReducer', () => { error: undefined, blocking: true, loading: true, - info: undefined + info: undefined, + idle: false }; const action = new RetrieveAuthenticatedEpersonSuccessAction(EPersonMock.id); const newState = authReducer(initialState, action); @@ -295,7 +316,8 @@ describe('authReducer', () => { blocking: false, loading: false, info: undefined, - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; expect(newState).toEqual(state); }); @@ -307,7 +329,8 @@ describe('authReducer', () => { error: undefined, blocking: true, loading: true, - info: undefined + info: undefined, + idle: false }; const action = new RetrieveAuthenticatedEpersonErrorAction(mockError); const newState = authReducer(initialState, action); @@ -318,7 +341,8 @@ describe('authReducer', () => { loaded: true, blocking: false, loading: false, - info: undefined + info: undefined, + idle: false }; expect(newState).toEqual(state); }); @@ -332,7 +356,8 @@ describe('authReducer', () => { blocking: false, loading: false, info: undefined, - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; const newTokenInfo = new AuthTokenInfo('Refreshed token'); const action = new RefreshTokenAction(newTokenInfo); @@ -346,7 +371,8 @@ describe('authReducer', () => { loading: false, info: undefined, userId: EPersonMock.id, - refreshing: true + refreshing: true, + idle: false }; expect(newState).toEqual(state); }); @@ -361,7 +387,8 @@ describe('authReducer', () => { loading: false, info: undefined, userId: EPersonMock.id, - refreshing: true + refreshing: true, + idle: false }; const newTokenInfo = new AuthTokenInfo('Refreshed token'); const action = new RefreshTokenSuccessAction(newTokenInfo); @@ -375,7 +402,8 @@ describe('authReducer', () => { loading: false, info: undefined, userId: EPersonMock.id, - refreshing: false + refreshing: false, + idle: false }; expect(newState).toEqual(state); }); @@ -390,7 +418,8 @@ describe('authReducer', () => { loading: false, info: undefined, userId: EPersonMock.id, - refreshing: true + refreshing: true, + idle: false }; const action = new RefreshTokenErrorAction(); const newState = authReducer(initialState, action); @@ -403,7 +432,8 @@ describe('authReducer', () => { loading: false, info: undefined, refreshing: false, - userId: undefined + userId: undefined, + idle: false }; expect(newState).toEqual(state); }); @@ -417,7 +447,8 @@ describe('authReducer', () => { blocking: false, loading: false, info: undefined, - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; state = { @@ -428,7 +459,8 @@ describe('authReducer', () => { loading: false, error: undefined, info: 'Message', - userId: undefined + userId: undefined, + idle: false }; }); @@ -450,6 +482,7 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: false, + idle: false }; const action = new AddAuthenticationMessageAction('Message'); const newState = authReducer(initialState, action); @@ -458,7 +491,8 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: false, - info: 'Message' + info: 'Message', + idle: false }; expect(newState).toEqual(state); }); @@ -470,7 +504,8 @@ describe('authReducer', () => { blocking: false, loading: false, error: 'Error', - info: 'Message' + info: 'Message', + idle: false }; const action = new ResetAuthenticationMessagesAction(); const newState = authReducer(initialState, action); @@ -480,7 +515,8 @@ describe('authReducer', () => { blocking: false, loading: false, error: undefined, - info: undefined + info: undefined, + idle: false }; expect(newState).toEqual(state); }); @@ -490,7 +526,8 @@ describe('authReducer', () => { authenticated: false, loaded: false, blocking: false, - loading: false + loading: false, + idle: false }; const action = new SetRedirectUrlAction('redirect.url'); const newState = authReducer(initialState, action); @@ -499,7 +536,8 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: false, - redirectUrl: 'redirect.url' + redirectUrl: 'redirect.url', + idle: false }; expect(newState).toEqual(state); }); @@ -510,7 +548,8 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: false, - authMethods: [] + authMethods: [], + idle: false }; const action = new RetrieveAuthMethodsAction(new AuthStatus(), true); const newState = authReducer(initialState, action); @@ -519,7 +558,8 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: true, - authMethods: [] + authMethods: [], + idle: false }; expect(newState).toEqual(state); }); @@ -530,7 +570,8 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: true, - authMethods: [] + authMethods: [], + idle: false }; const authMethods = [ new AuthMethod(AuthMethodType.Password), @@ -543,7 +584,8 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: false, - authMethods: authMethods + authMethods: authMethods, + idle: false }; expect(newState).toEqual(state); }); @@ -554,7 +596,8 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: true, - authMethods: [] + authMethods: [], + idle: false }; const authMethods = [ new AuthMethod(AuthMethodType.Password), @@ -567,7 +610,8 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: false, - authMethods: authMethods + authMethods: authMethods, + idle: false }; expect(newState).toEqual(state); }); @@ -578,7 +622,8 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: true, - authMethods: [] + authMethods: [], + idle: false }; const action = new RetrieveAuthMethodsErrorAction(false); @@ -588,7 +633,50 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: false, - authMethods: [new AuthMethod(AuthMethodType.Password)] + authMethods: [new AuthMethod(AuthMethodType.Password)], + idle: false + }; + expect(newState).toEqual(state); + }); + + it('should properly set the state, in response to a SET_USER_AS_IDLE action', () => { + initialState = { + authenticated: true, + loaded: true, + blocking: false, + loading: false, + idle: false + }; + + const action = new SetUserAsIdleAction(); + const newState = authReducer(initialState, action); + state = { + authenticated: true, + loaded: true, + blocking: false, + loading: false, + idle: true + }; + expect(newState).toEqual(state); + }); + + it('should properly set the state, in response to a UNSET_USER_AS_IDLE action', () => { + initialState = { + authenticated: true, + loaded: true, + blocking: false, + loading: false, + idle: true + }; + + const action = new UnsetUserAsIdleAction(); + const newState = authReducer(initialState, action); + state = { + authenticated: true, + loaded: true, + blocking: false, + loading: false, + idle: false }; expect(newState).toEqual(state); }); @@ -599,7 +687,8 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: true, - authMethods: [] + authMethods: [], + idle: false }; const action = new RetrieveAuthMethodsErrorAction(true); @@ -609,7 +698,8 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: false, - authMethods: [new AuthMethod(AuthMethodType.Password)] + authMethods: [new AuthMethod(AuthMethodType.Password)], + idle: false }; expect(newState).toEqual(state); }); diff --git a/src/app/core/auth/auth.reducer.ts b/src/app/core/auth/auth.reducer.ts index dfe29a3ef2..ef803503c8 100644 --- a/src/app/core/auth/auth.reducer.ts +++ b/src/app/core/auth/auth.reducer.ts @@ -59,6 +59,9 @@ export interface AuthState { // all authentication Methods enabled at the backend authMethods?: AuthMethod[]; + // true when the current user is idle + idle: boolean; + } /** @@ -69,7 +72,8 @@ const initialState: AuthState = { loaded: false, blocking: true, loading: false, - authMethods: [] + authMethods: [], + idle: false }; /** @@ -189,6 +193,7 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut return Object.assign({}, state, { authToken: (action as RefreshTokenSuccessAction).payload, refreshing: false, + blocking: false }); case AuthActionTypes.ADD_MESSAGE: @@ -234,6 +239,16 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut blocking: true, }); + case AuthActionTypes.SET_USER_AS_IDLE: + return Object.assign({}, state, { + idle: true, + }); + + case AuthActionTypes.UNSET_USER_AS_IDLE: + return Object.assign({}, state, { + idle: false, + }); + default: return state; } diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index 505f925e8e..ced8bb94c8 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -27,6 +27,11 @@ import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.util import { authMethodsMock } from '../../shared/testing/auth-service.stub'; import { AuthMethod } from './models/auth.method'; import { HardRedirectService } from '../services/hard-redirect.service'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { TranslateService } from '@ngx-translate/core'; +import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; +import { NotificationsServiceStub } from '../../shared/testing/notifications-service.stub'; +import { SetUserAsIdleAction, UnsetUserAsIdleAction } from './auth.actions'; describe('AuthService test', () => { @@ -47,6 +52,7 @@ describe('AuthService test', () => { let token: AuthTokenInfo; let authenticatedState; let unAuthenticatedState; + let idleState; let linkService; let hardRedirectService; @@ -64,14 +70,24 @@ describe('AuthService test', () => { loaded: true, loading: false, authToken: token, - user: EPersonMock + user: EPersonMock, + idle: false }; unAuthenticatedState = { authenticated: false, loaded: true, loading: false, authToken: undefined, - user: undefined + user: undefined, + idle: false + }; + idleState = { + authenticated: true, + loaded: true, + loading: false, + authToken: token, + user: EPersonMock, + idle: true }; authRequest = new AuthRequestServiceStub(); routeStub = new ActivatedRouteStub(); @@ -107,6 +123,8 @@ describe('AuthService test', () => { { provide: Store, useValue: mockStore }, { provide: EPersonDataService, useValue: mockEpersonDataService }, { provide: HardRedirectService, useValue: hardRedirectService }, + { provide: NotificationsService, useValue: NotificationsServiceStub }, + { provide: TranslateService, useValue: getMockTranslateService() }, CookieService, AuthService ], @@ -180,6 +198,26 @@ describe('AuthService test', () => { expect(authMethods.length).toBe(2); }); }); + + describe('setIdle true', () => { + beforeEach(() => { + authService.setIdle(true); + }); + + it('store should dispatch SetUserAsIdleAction', () => { + expect(mockStore.dispatch).toHaveBeenCalledWith(new SetUserAsIdleAction()); + }); + }); + + describe('setIdle false', () => { + beforeEach(() => { + authService.setIdle(false); + }); + + it('store should dispatch UnsetUserAsIdleAction', () => { + expect(mockStore.dispatch).toHaveBeenCalledWith(new UnsetUserAsIdleAction()); + }); + }); }); describe('', () => { @@ -207,13 +245,13 @@ describe('AuthService test', () => { }).compileComponents(); })); - beforeEach(inject([CookieService, AuthRequestService, Store, Router, RouteService], (cookieService: CookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService) => { + beforeEach(inject([CookieService, AuthRequestService, Store, Router, RouteService], (cookieService: CookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService, notificationsService: NotificationsService, translateService: TranslateService) => { store .subscribe((state) => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService); + authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService, notificationsService, translateService); })); it('should return true when user is logged in', () => { @@ -250,6 +288,12 @@ describe('AuthService test', () => { }); }); + it('isUserIdle should return false when user is not yet idle', () => { + authService.isUserIdle().subscribe((status: boolean) => { + expect(status).toBe(false); + }); + }); + }); describe('', () => { @@ -277,7 +321,7 @@ describe('AuthService test', () => { }).compileComponents(); })); - beforeEach(inject([ClientCookieService, AuthRequestService, Store, Router, RouteService], (cookieService: ClientCookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService) => { + beforeEach(inject([ClientCookieService, AuthRequestService, Store, Router, RouteService], (cookieService: ClientCookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService, notificationsService: NotificationsService, translateService: TranslateService) => { const expiredToken: AuthTokenInfo = new AuthTokenInfo('test_token'); expiredToken.expires = Date.now() - (1000 * 60 * 60); authenticatedState = { @@ -292,7 +336,7 @@ describe('AuthService test', () => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService); + authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService, notificationsService, translateService); storage = (authService as any).storage; routeServiceMock = TestBed.inject(RouteService); routerStub = TestBed.inject(Router); @@ -493,13 +537,13 @@ describe('AuthService test', () => { }).compileComponents(); })); - beforeEach(inject([CookieService, AuthRequestService, Store, Router, RouteService], (cookieService: CookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService) => { + beforeEach(inject([CookieService, AuthRequestService, Store, Router, RouteService], (cookieService: CookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService, notificationsService: NotificationsService, translateService: TranslateService) => { store .subscribe((state) => { (state as any).core = Object.create({}); (state as any).core.auth = unAuthenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService); + authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService, notificationsService, translateService); })); it('should return null for the shortlived token', () => { @@ -508,4 +552,44 @@ describe('AuthService test', () => { }); }); }); + + describe('when user is idle', () => { + beforeEach(waitForAsync(() => { + init(); + TestBed.configureTestingModule({ + imports: [ + StoreModule.forRoot({ authReducer }, { + runtimeChecks: { + strictStateImmutability: false, + strictActionImmutability: false + } + }) + ], + providers: [ + { provide: AuthRequestService, useValue: authRequest }, + { provide: REQUEST, useValue: {} }, + { provide: Router, useValue: routerStub }, + { provide: RouteService, useValue: routeServiceStub }, + { provide: RemoteDataBuildService, useValue: linkService }, + CookieService, + AuthService + ] + }).compileComponents(); + })); + + beforeEach(inject([CookieService, AuthRequestService, Store, Router, RouteService], (cookieService: CookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService, notificationsService: NotificationsService, translateService: TranslateService) => { + store + .subscribe((state) => { + (state as any).core = Object.create({}); + (state as any).core.auth = idleState; + }); + authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService, notificationsService, translateService); + })); + + it('isUserIdle should return true when user is not idle', () => { + authService.isUserIdle().subscribe((status: boolean) => { + expect(status).toBe(true); + }); + }); + }); }); diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index ed4fca615c..a5b5d70704 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -29,14 +29,17 @@ import { getRedirectUrl, isAuthenticated, isAuthenticatedLoaded, + isIdle, isTokenRefreshing } from './selectors'; import { AppState } from '../../app.reducer'; import { - CheckAuthenticationTokenAction, + CheckAuthenticationTokenAction, RefreshTokenAction, ResetAuthenticationMessagesAction, RetrieveAuthMethodsAction, - SetRedirectUrlAction + SetRedirectUrlAction, + SetUserAsIdleAction, + UnsetUserAsIdleAction } from './auth.actions'; import { NativeWindowRef, NativeWindowService } from '../services/window.service'; import { Base64EncodeUrl } from '../../shared/utils/encode-decode.util'; @@ -46,6 +49,9 @@ import { getAllSucceededRemoteDataPayload } from '../shared/operators'; import { AuthMethod } from './models/auth.method'; import { HardRedirectService } from '../services/hard-redirect.service'; import { RemoteData } from '../data/remote-data'; +import { environment } from '../../../environments/environment'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { TranslateService } from '@ngx-translate/core'; export const LOGIN_ROUTE = '/login'; export const LOGOUT_ROUTE = '/logout'; @@ -64,6 +70,11 @@ export class AuthService { */ protected _authenticated: boolean; + /** + * Timer to track time until token refresh + */ + private tokenRefreshTimer; + constructor(@Inject(REQUEST) protected req: any, @Inject(NativeWindowService) protected _window: NativeWindowRef, @Optional() @Inject(RESPONSE) private response: any, @@ -73,7 +84,9 @@ export class AuthService { protected routeService: RouteService, protected storage: CookieService, protected store: Store, - protected hardRedirectService: HardRedirectService + protected hardRedirectService: HardRedirectService, + private notificationService: NotificationsService, + private translateService: TranslateService ) { this.store.pipe( select(isAuthenticated), @@ -187,7 +200,7 @@ export class AuthService { return this.store.pipe( select(getAuthenticatedUserId), hasValueOperator(), - switchMap((id: string) => this.epersonService.findById(id) ), + switchMap((id: string) => this.epersonService.findById(id)), getAllSucceededRemoteDataPayload() ); } @@ -298,7 +311,7 @@ export class AuthService { */ public getToken(): AuthTokenInfo { let token: AuthTokenInfo; - this.store.pipe(select(getAuthenticationToken)) + this.store.pipe(take(1), select(getAuthenticationToken)) .subscribe((authTokenInfo: AuthTokenInfo) => { // Retrieve authentication token info and check if is valid token = authTokenInfo || null; @@ -306,6 +319,44 @@ export class AuthService { return token; } + /** + * Method that checks when the session token from store expires and refreshes it when needed + */ + public trackTokenExpiration(): void { + let token: AuthTokenInfo; + let currentlyRefreshingToken = false; + this.store.pipe(select(getAuthenticationToken)).subscribe((authTokenInfo: AuthTokenInfo) => { + // If new token is undefined an it wasn't previously => Refresh failed + if (currentlyRefreshingToken && token !== undefined && authTokenInfo === undefined) { + // Token refresh failed => Error notification => 10 second wait => Page reloads & user logged out + this.notificationService.error(this.translateService.get('auth.messages.token-refresh-failed')); + setTimeout(() => this.navigateToRedirectUrl(this.hardRedirectService.getCurrentRoute()), 10000); + currentlyRefreshingToken = false; + } + // If new token.expires is different => Refresh succeeded + if (currentlyRefreshingToken && authTokenInfo !== undefined && token.expires !== authTokenInfo.expires) { + currentlyRefreshingToken = false; + } + // Check if/when token needs to be refreshed + if (!currentlyRefreshingToken) { + token = authTokenInfo || null; + if (token !== undefined && token !== null) { + let timeLeftBeforeRefresh = token.expires - new Date().getTime() - environment.auth.rest.timeLeftBeforeTokenRefresh; + if (timeLeftBeforeRefresh < 0) { + timeLeftBeforeRefresh = 0; + } + if (hasValue(this.tokenRefreshTimer)) { + clearTimeout(this.tokenRefreshTimer); + } + this.tokenRefreshTimer = setTimeout(() => { + this.store.dispatch(new RefreshTokenAction(token)); + currentlyRefreshingToken = true; + }, timeLeftBeforeRefresh); + } + } + }); + } + /** * Check if a token is next to be expired * @returns {boolean} @@ -346,7 +397,7 @@ export class AuthService { // Set the cookie expire date const expires = new Date(expireDate); - const options: CookieAttributes = { expires: expires }; + const options: CookieAttributes = {expires: expires}; // Save cookie with the token return this.storage.set(TOKENITEM, token, options); @@ -396,11 +447,14 @@ export class AuthService { * @param redirectUrl */ public navigateToRedirectUrl(redirectUrl: string) { - let url = `/reload/${new Date().getTime()}`; - if (isNotEmpty(redirectUrl) && !redirectUrl.startsWith(LOGIN_ROUTE)) { - url += `?redirect=${encodeURIComponent(redirectUrl)}`; + // Don't do redirect if already on reload url + if (!hasValue(redirectUrl) || !redirectUrl.includes('/reload/')) { + let url = `/reload/${new Date().getTime()}`; + if (isNotEmpty(redirectUrl) && !redirectUrl.startsWith(LOGIN_ROUTE)) { + url += `?redirect=${encodeURIComponent(redirectUrl)}`; + } + this.hardRedirectService.redirect(url); } - this.hardRedirectService.redirect(url); } /** @@ -435,7 +489,7 @@ export class AuthService { // Set the cookie expire date const expires = new Date(expireDate); - const options: CookieAttributes = { expires: expires }; + const options: CookieAttributes = {expires: expires}; this.storage.set(REDIRECT_COOKIE, url, options); this.store.dispatch(new SetRedirectUrlAction(isNotUndefined(url) ? url : '')); } @@ -528,4 +582,24 @@ export class AuthService { return new RetrieveAuthMethodsAction(authStatus, false); } + /** + * Determines if current user is idle + * @returns {Observable} + */ + public isUserIdle(): Observable { + return this.store.pipe(select(isIdle)); + } + + /** + * Set idle of auth state + * @returns {Observable} + */ + public setIdle(idle: boolean): void { + if (idle) { + this.store.dispatch(new SetUserAsIdleAction()); + } else { + this.store.dispatch(new UnsetUserAsIdleAction()); + } + } + } diff --git a/src/app/core/auth/selectors.ts b/src/app/core/auth/selectors.ts index c4e95a0fb3..9ee9f7eb2e 100644 --- a/src/app/core/auth/selectors.ts +++ b/src/app/core/auth/selectors.ts @@ -115,6 +115,14 @@ const _getRedirectUrl = (state: AuthState) => state.redirectUrl; const _getAuthenticationMethods = (state: AuthState) => state.authMethods; +/** + * Returns true if the user is idle. + * @function _isIdle + * @param {State} state + * @returns {boolean} + */ +const _isIdle = (state: AuthState) => state.idle; + /** * Returns the authentication methods enabled at the backend * @function getAuthenticationMethods @@ -231,3 +239,12 @@ export const getRegistrationError = createSelector(getAuthState, _getRegistratio * @return {string} */ export const getRedirectUrl = createSelector(getAuthState, _getRedirectUrl); + +/** + * Returns true if the user is idle + * @function isIdle + * @param {AuthState} state + * @param {any} props + * @return {boolean} + */ +export const isIdle = createSelector(getAuthState, _isIdle); 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..cd048d3be2 --- /dev/null +++ b/src/app/core/metadata/meta-tag.actions.ts @@ -0,0 +1,23 @@ +import { type } from '../../shared/ngrx/type'; +import { Action } from '@ngrx/store'; + +// 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.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([]); + }); +}); 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.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 18421dd489..b3404e84d5 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -1,82 +1,28 @@ -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 { NavigationEnd, Router } 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, - MockBitstream2, - MockBitstreamFormat1, - MockBitstreamFormat2 -} 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'; +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 { 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 { 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'; - -/* 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 */ +import { Bundle } from '../shared/bundle.model'; +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 } from '@ngrx/store/testing'; +import { AddMetaTagAction, ClearMetaTagAction } from './meta-tag.actions'; describe('MetadataService', () => { let metadataService: MetadataService; @@ -85,188 +31,339 @@ 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 authService: AuthService; + let bundleDataService; + let bitstreamDataService; let rootService: RootDataService; let translateService: TranslateService; + let hardRedirectService: HardRedirectService; - let location: Location; let router: Router; - let fixture: ComponentFixture; + let store; + + const initialState = { 'core': { metaTag: { tagsInUse: ['title', 'description'] }}}; - let tagStore: Map; beforeEach(() => { + 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('meta', { + 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: {} + } + } as any as Router; + hardRedirectService = jasmine.createSpyObj( { + getRequestOrigin: 'https://request.org', + }); - store = new Store(undefined, undefined, undefined); + // @ts-ignore + store = getMockStore({ initialState }); spyOn(store, 'dispatch'); - objectCacheService = new ObjectCacheService(store, undefined); - uuidService = new UUIDService(); - requestService = new RequestService(objectCacheService, uuidService, store, undefined); - remoteDataBuildService = new RemoteDataBuildService(objectCacheService, undefined, requestService); - const mockBitstreamDataService = { - 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(), [])); - } - }, - }; - const mockBitstreamFormatDataService = { - findByBitstream(bitstream: Bitstream): Observable> { - switch (bitstream) { - case MockBitstream1: - return createSuccessfulRemoteDataObject$(MockBitstreamFormat1); - break; - case MockBitstream2: - return createSuccessfulRemoteDataObject$(MockBitstreamFormat2); - break; - default: - return createSuccessfulRemoteDataObject$(new BitstreamFormat()); - } - } - }; - rootService = jasmine.createSpyObj('rootService', { - findRoot: createSuccessfulRemoteDataObject$(Object.assign(new Root(), { - dspaceVersion: 'mock-dspace-version' - })) - }); - - 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: mockBitstreamDataService }, - { provide: RootDataService, useValue: rootService }, - 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, + store, + 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_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_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'); + 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' + }); })); - }); - const mockRemoteData = (mockItem: Item): Observable> => { - return createSuccessfulRemoteDataObject$(ItemMock); - }; + describe('citation_abstract_html_url', () => { + it('should use dc.identifier.uri if available', fakeAsync(() => { + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockUri(ItemMock, 'https://ddg.gg')), + } + } + }); + tick(); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_abstract_html_url', + content: 'https://ddg.gg' + }); + })); + + it('should use current route as fallback', fakeAsync(() => { + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockUri(ItemMock)), + } + } + }); + tick(); + 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(() => { + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Thesis'))), + } + } + }); + tick(); + 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(() => { + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Technical Report'))), + } + } + }); + tick(); + 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(() => { + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(mockPublisher(mockType(ItemMock, 'Some Other Type'))), + } + } + }); + tick(); + 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(() => { + (bundleDataService.findByItemAndName as jasmine.Spy).and.returnValue(mockBundleRD$([], MockBitstream3)); + + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(ItemMock), + } + } + }); + tick(); + 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(() => { + (bundleDataService.findByItemAndName as jasmine.Spy).and.returnValue(mockBundleRD$([MockBitstream3])); + + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(ItemMock), + } + } + }); + tick(); + 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(() => { + const bitstreams = [MockBitstream3, MockBitstream3, MockBitstream1]; + (bundleDataService.findByItemAndName as jasmine.Spy).and.returnValue(mockBundleRD$(bitstreams)); + (bitstreamDataService.findAllByHref as jasmine.Spy).and.returnValues( + ...mockBitstreamPages$(bitstreams).map(bp => createSuccessfulRemoteDataObject$(bp)), + ); + + (metadataService as any).processRouteChange({ + data: { + value: { + dso: createSuccessfulRemoteDataObject(ItemMock), + } + } + }); + tick(); + expect(meta.addTag).toHaveBeenCalledWith({ + property: 'citation_pdf_url', + content: 'https://request.org/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/download' + }); + })); + }); + }); + + 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; @@ -285,4 +382,30 @@ 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(), { + 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 807f7a42ab..10e37b4282 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -5,12 +5,11 @@ 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, EMPTY, Observable, of as observableOf } from 'rxjs'; +import { expand, filter, map, switchMap, take } from 'rxjs/operators'; -import { hasValue, isNotEmpty } from '../../shared/empty.util'; +import { hasNoValue, hasValue } 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'; @@ -19,22 +18,57 @@ 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, - getFirstSucceededRemoteListPayload -} from '../shared/operators'; -import { environment } from '../../../environments/environment'; +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'; +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'; +import { MetaTagState } from './meta-tag.reducer'; +import { createSelector, select, Store } from '@ngrx/store'; +import { AddMetaTagAction, ClearMetaTagAction } from './meta-tag.actions'; +import { coreSelector } from '../core.selectors'; +import { CoreState } from '../core.reducers'; + +/** + * 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 initialized: boolean; + private currentObject: BehaviorSubject = new BehaviorSubject(undefined); - private tagStore: Map; - - private currentObject: BehaviorSubject; + /** + * 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 + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', // .docx + 'application/rtf', // .rtf + 'application/epub+zip', // .epub + ]; constructor( private router: Router, @@ -42,21 +76,19 @@ export class MetadataService { private meta: Meta, private title: Title, private dsoNameService: DSONameService, + private bundleDataService: BundleDataService, private bitstreamDataService: BitstreamDataService, private bitstreamFormatDataService: BitstreamFormatDataService, - private rootService: RootDataService + 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.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(); } 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), @@ -68,22 +100,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); @@ -98,15 +117,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 { @@ -116,16 +130,14 @@ export class MetadataService { return route; } - private setMetaTags(): void { - - this.clearMetaTags(); + private setDSOMetaTags(): void { this.setTitleTag(); this.setDescriptionTag(); this.setCitationTitleTag(); this.setCitationAuthorTags(); - this.setCitationDateTag(); + this.setCitationPublicationDateTag(); this.setCitationISSNTag(); this.setCitationISBNTag(); @@ -134,14 +146,10 @@ export class MetadataService { this.setCitationAbstractUrlTag(); this.setCitationPdfUrlTag(); + this.setCitationPublisherTag(); if (this.isDissertation()) { this.setCitationDissertationNameTag(); - this.setCitationDissertationInstitutionTag(); - } - - if (this.isTechReport()) { - this.setCitationTechReportInstitutionTag(); } // this.setCitationJournalTitleTag(); @@ -176,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); } /** @@ -196,11 +204,11 @@ export class MetadataService { } /** - * Add to the + * 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); } /** @@ -236,19 +244,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); + } } /** @@ -264,8 +270,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 = new URLCombiner(this.hardRedirectService.getRequestOrigin(), this.router.url).toString(); + } + this.addMetaTag('citation_abstract_html_url', url); } } @@ -275,35 +284,126 @@ 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', {}, 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 this.getFirstAllowedFormatBitstreamLink(bitstreamRd); + } + }) + ); } - }); + }), + take(1) + ).subscribe((link: string) => { + // Use the found link to set the tag + this.addMetaTag( + 'citation_pdf_url', + new URLCombiner(this.hardRedirectService.getRequestOrigin(), link).toString() + ); + }); } } + /** + * 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 + // 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) && this.CITATION_PDF_URL_MIMETYPES.includes(format.mimetype)), + // 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 */ private setGenerator(): void { this.rootService.findRoot().pipe(getFirstSucceededRemoteDataPayload()).subscribe((root) => { - this.addMetaTag('Generator', root.dspaceVersion); + this.meta.addTag({ property: 'Generator', content: root.dspaceVersion }); }); } @@ -351,7 +451,7 @@ export class MetadataService { if (content) { const tag = { property, content } as MetaDefinition; this.meta.addTag(tag); - this.storeTag(property, tag); + this.storeTag(property); } } @@ -361,33 +461,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; - } } diff --git a/src/app/core/utilities/enter-zone.scheduler.ts b/src/app/core/utilities/enter-zone.scheduler.ts new file mode 100644 index 0000000000..96aee7d9a5 --- /dev/null +++ b/src/app/core/utilities/enter-zone.scheduler.ts @@ -0,0 +1,19 @@ +import { SchedulerLike, Subscription } from 'rxjs'; +import { NgZone } from '@angular/core'; + +/** + * An RXJS scheduler that will re-enter the Angular zone to run what's scheduled + */ +export class EnterZoneScheduler implements SchedulerLike { + constructor(private zone: NgZone, private scheduler: SchedulerLike) { } + + schedule(...args: any[]): Subscription { + return this.zone.run(() => + this.scheduler.schedule.apply(this.scheduler, args) + ); + } + + now (): number { + return this.scheduler.now(); + } +} diff --git a/src/app/core/utilities/leave-zone.scheduler.ts b/src/app/core/utilities/leave-zone.scheduler.ts new file mode 100644 index 0000000000..2587563661 --- /dev/null +++ b/src/app/core/utilities/leave-zone.scheduler.ts @@ -0,0 +1,19 @@ +import { SchedulerLike, Subscription } from 'rxjs'; +import { NgZone } from '@angular/core'; + +/** + * An RXJS scheduler that will run what's scheduled outside of the Angular zone + */ +export class LeaveZoneScheduler implements SchedulerLike { + constructor(private zone: NgZone, private scheduler: SchedulerLike) { } + + schedule(...args: any[]): Subscription { + return this.zone.runOutsideAngular(() => + this.scheduler.schedule.apply(this.scheduler, args) + ); + } + + now (): number { + return this.scheduler.now(); + } +} diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index a7cf7c1856..bfefcb5a6c 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,4 +1,5 @@ - +
{{'item.edit.metadata.headers.field' | translate}}{{'item.edit.metadata.headers.value' | translate}}{{'item.edit.metadata.headers.language' | translate}}{{'item.edit.metadata.headers.field' | translate}}{{'item.edit.metadata.headers.value' | translate}}{{'item.edit.metadata.headers.language' | translate}} {{'item.edit.metadata.headers.edit' | translate}}
{{collection?.name}} diff --git a/src/app/shared/object-select/item-select/item-select.component.spec.ts b/src/app/shared/object-select/item-select/item-select.component.spec.ts index 224fb764b6..de52f1c3c2 100644 --- a/src/app/shared/object-select/item-select/item-select.component.spec.ts +++ b/src/app/shared/object-select/item-select/item-select.component.spec.ts @@ -11,13 +11,13 @@ import { HostWindowService } from '../../host-window.service'; import { HostWindowServiceStub } from '../../testing/host-window-service.stub'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { By } from '@angular/platform-browser'; -import { of as observableOf, of } from 'rxjs'; +import { of } from 'rxjs'; import { createSuccessfulRemoteDataObject$ } from '../../remote-data.utils'; import { createPaginatedList } from '../../testing/utils.test'; import { PaginationService } from '../../../core/pagination/pagination.service'; -import { SortDirection, SortOptions } from '../../../core/cache/models/sort-options.model'; -import { FindListOptions } from '../../../core/data/request.models'; import { PaginationServiceStub } from '../../testing/pagination-service.stub'; +import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; +import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; describe('ItemSelectComponent', () => { let comp: ItemSelectComponent; @@ -39,7 +39,8 @@ describe('ItemSelectComponent', () => { key: 'dc.type', language: null, value: 'Article' - }] + }], + _links: { self: { href: 'selfId1' } } }), Object.assign(new Item(), { id: 'id2', @@ -54,7 +55,8 @@ describe('ItemSelectComponent', () => { key: 'dc.type', language: null, value: 'Article' - }] + }], + _links: { self: { href: 'selfId2' } } }) ]; const mockItems = createSuccessfulRemoteDataObject$(createPaginatedList(mockItemList)); @@ -66,6 +68,7 @@ describe('ItemSelectComponent', () => { paginationService = new PaginationServiceStub(mockPaginationOptions); + const authorizationDataService = new AuthorizationDataService(null, null, null, null, null, null, null, null, null, null); beforeEach(waitForAsync(() => { @@ -75,7 +78,8 @@ describe('ItemSelectComponent', () => { providers: [ { provide: ObjectSelectService, useValue: new ObjectSelectServiceStub([mockItemList[1].id]) }, { provide: HostWindowService, useValue: new HostWindowServiceStub(0) }, - { provide: PaginationService, useValue: paginationService } + { provide: PaginationService, useValue: paginationService }, + { provide: AuthorizationDataService, useValue: authorizationDataService } ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); @@ -146,4 +150,21 @@ describe('ItemSelectComponent', () => { expect(comp.cancel.emit).toHaveBeenCalled(); }); }); + + describe('when the authorize feature is not authorized', () => { + + beforeEach(() => { + comp.featureId = FeatureID.CanManageMappings; + spyOn(authorizationDataService, 'isAuthorized').and.returnValue(of(false)); + }); + + it('should disable the checkbox', waitForAsync(() => { + fixture.detectChanges(); + fixture.whenStable().then(() => { + const checkbox = fixture.debugElement.query(By.css('input.item-checkbox')).nativeElement; + expect(authorizationDataService.isAuthorized).toHaveBeenCalled(); + expect(checkbox.disabled).toBeTrue(); + }); + })); + }); }); diff --git a/src/app/shared/object-select/item-select/item-select.component.ts b/src/app/shared/object-select/item-select/item-select.component.ts index b4918ada99..80808311a0 100644 --- a/src/app/shared/object-select/item-select/item-select.component.ts +++ b/src/app/shared/object-select/item-select/item-select.component.ts @@ -7,6 +7,7 @@ import { Observable } from 'rxjs'; import { getAllSucceededRemoteDataPayload } from '../../../core/shared/operators'; import { map } from 'rxjs/operators'; import { getItemPageRoute } from '../../../+item-page/item-page-routing-paths'; +import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; @Component({ selector: 'ds-item-select', @@ -33,8 +34,9 @@ export class ItemSelectComponent extends ObjectSelectComponent { [itemId: string]: string }>; - constructor(protected objectSelectService: ObjectSelectService) { - super(objectSelectService); + constructor(protected objectSelectService: ObjectSelectService, + protected authorizationService: AuthorizationDataService ) { + super(objectSelectService, authorizationService); } ngOnInit(): void { diff --git a/src/app/shared/object-select/object-select/object-select.component.ts b/src/app/shared/object-select/object-select/object-select.component.ts index f8be095719..165a8c0a4f 100644 --- a/src/app/shared/object-select/object-select/object-select.component.ts +++ b/src/app/shared/object-select/object-select/object-select.component.ts @@ -1,11 +1,15 @@ import { Component, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core'; -import { take } from 'rxjs/operators'; +import { startWith, take } from 'rxjs/operators'; import { Observable } from 'rxjs'; import { RemoteData } from '../../../core/data/remote-data'; import { PaginatedList } from '../../../core/data/paginated-list.model'; import { PaginationComponentOptions } from '../../pagination/pagination-component-options.model'; import { ObjectSelectService } from '../object-select.service'; import { SortOptions } from '../../../core/cache/models/sort-options.model'; +import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; +import { of } from 'rxjs/internal/observable/of'; +import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; +import { DSpaceObject } from '../../../core/shared/dspace-object.model'; /** * An abstract component used to select DSpaceObjects from a specific list and returning the UUIDs of the selected DSpaceObjects @@ -47,6 +51,12 @@ export abstract class ObjectSelectComponent implements OnInit, OnDestro @Input() confirmButton: string; + /** + * Authorize check to enable the selection when present. + */ + @Input() + featureId: FeatureID; + /** * The message key used for the cancel button * @type {string} @@ -79,7 +89,8 @@ export abstract class ObjectSelectComponent implements OnInit, OnDestro */ selectedIds$: Observable; - constructor(protected objectSelectService: ObjectSelectService) { + constructor(protected objectSelectService: ObjectSelectService, + protected authorizationService: AuthorizationDataService) { } ngOnInit(): void { @@ -107,6 +118,16 @@ export abstract class ObjectSelectComponent implements OnInit, OnDestro return this.objectSelectService.getSelected(this.key, id); } + /** + * Return if the item can be selected or not due to authorization check. + */ + canSelect(item: DSpaceObject): Observable { + if (!this.featureId) { + return of(true); + } + return this.authorizationService.isAuthorized(this.featureId, item.self).pipe(startWith(false)); + } + /** * Called when the confirm button is pressed * Sends the selected UUIDs to the parent component diff --git a/src/app/shared/search-form/search-form.component.html b/src/app/shared/search-form/search-form.component.html index bb75f449ff..940f3502c3 100644 --- a/src/app/shared/search-form/search-form.component.html +++ b/src/app/shared/search-form/search-form.component.html @@ -1,6 +1,6 @@
- diff --git a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.ts b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.ts index b23a2d8224..a9ad789d4d 100644 --- a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.ts +++ b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.ts @@ -56,7 +56,7 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple /** * Fallback maximum for the range */ - max = new Date().getFullYear(); + max = new Date().getUTCFullYear(); /** * The current range of the filter diff --git a/src/app/shared/search/search-switch-configuration/search-switch-configuration.component.html b/src/app/shared/search/search-switch-configuration/search-switch-configuration.component.html index 8df37214d1..e66483b645 100644 --- a/src/app/shared/search/search-switch-configuration/search-switch-configuration.component.html +++ b/src/app/shared/search/search-switch-configuration/search-switch-configuration.component.html @@ -1,7 +1,8 @@
-
{{ 'search.switch-configuration.title' | translate}}
+
{{ 'search.switch-configuration.title' | translate}}
+
+ {{dropMsg | translate}} {{'uploader.or' | translate}} + -

+
@@ -32,7 +32,7 @@ {{'uploader.queue-length' | translate}}: {{ uploader?.queue?.length }} | {{ uploader?.queue[0]?.file.name }}
-
diff --git a/src/app/submission/form/collection/submission-form-collection.component.spec.ts b/src/app/submission/form/collection/submission-form-collection.component.spec.ts index 14dfcef864..5b9946e1a4 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.spec.ts +++ b/src/app/submission/form/collection/submission-form-collection.component.spec.ts @@ -120,7 +120,7 @@ describe('SubmissionFormCollectionComponent Component', () => { }); const sectionsService: any = jasmine.createSpyObj('sectionsService', { - isSectionAvailable: of(true) + isSectionTypeAvailable: of(true) }); beforeEach(waitForAsync(() => { diff --git a/src/app/submission/form/collection/submission-form-collection.component.ts b/src/app/submission/form/collection/submission-form-collection.component.ts index ba7eb88c6f..f90814f185 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.ts +++ b/src/app/submission/form/collection/submission-form-collection.component.ts @@ -28,6 +28,7 @@ import { CollectionDataService } from '../../../core/data/collection-data.servic import { CollectionDropdownComponent } from '../../../shared/collection-dropdown/collection-dropdown.component'; import { SectionsService } from '../../sections/sections.service'; import { getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators'; +import { SectionsType } from '../../sections/sections-type'; /** * This component allows to show the current collection the submission belonging to and to change it. @@ -142,7 +143,7 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { */ ngOnInit() { this.pathCombiner = new JsonPatchOperationPathCombiner('sections', 'collection'); - this.available$ = this.sectionsService.isSectionAvailable(this.submissionId, 'collection'); + this.available$ = this.sectionsService.isSectionTypeAvailable(this.submissionId, SectionsType.collection); } /** diff --git a/src/app/submission/form/footer/submission-form-footer.component.html b/src/app/submission/form/footer/submission-form-footer.component.html index 9771e34cec..4964eb56a2 100644 --- a/src/app/submission/form/footer/submission-form-footer.component.html +++ b/src/app/submission/form/footer/submission-form-footer.component.html @@ -1,43 +1,52 @@
- -
-
-
-
Saving...
-
Depositing...
-
-
-
- - -
+
+ + {{'submission.general.info.saved' | translate}} + + + {{'submission.general.info.pending-changes' | translate}} + +
+
+
Saving...
+
Depositing...
+
+
+
+ + + +
+
diff --git a/src/app/submission/form/footer/submission-form-footer.component.spec.ts b/src/app/submission/form/footer/submission-form-footer.component.spec.ts index 57a193ad7d..dd47dad444 100644 --- a/src/app/submission/form/footer/submission-form-footer.component.spec.ts +++ b/src/app/submission/form/footer/submission-form-footer.component.spec.ts @@ -205,7 +205,7 @@ describe('SubmissionFormFooterComponent Component', () => { comp.showDepositAndDiscard = observableOf(true); compAsAny.submissionIsInvalid = observableOf(true); fixture.detectChanges(); - const depositBtn: any = fixture.debugElement.query(By.css('.btn-primary')); + const depositBtn: any = fixture.debugElement.query(By.css('.btn-success')); expect(depositBtn.nativeElement.disabled).toBeTruthy(); }); @@ -214,7 +214,7 @@ describe('SubmissionFormFooterComponent Component', () => { comp.showDepositAndDiscard = observableOf(true); compAsAny.submissionIsInvalid = observableOf(false); fixture.detectChanges(); - const depositBtn: any = fixture.debugElement.query(By.css('.btn-primary')); + const depositBtn: any = fixture.debugElement.query(By.css('.btn-success')); expect(depositBtn.nativeElement.disabled).toBeFalsy(); }); diff --git a/src/app/submission/form/submission-form.component.html b/src/app/submission/form/submission-form.component.html index 33b5d4be12..c86d4e0195 100644 --- a/src/app/submission/form/submission-form.component.html +++ b/src/app/submission/form/submission-form.component.html @@ -3,7 +3,6 @@
diff --git a/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts b/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts index bbee3b017c..bcdd01d5a1 100644 --- a/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts +++ b/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts @@ -83,7 +83,6 @@ describe('SubmissionUploadFilesComponent Component', () => { const html = ` `; testFixture = createTestComponent(html, TestComponent) as ComponentFixture; @@ -108,11 +107,11 @@ describe('SubmissionUploadFilesComponent Component', () => { compAsAny = comp; submissionServiceStub = TestBed.inject(SubmissionService as any); sectionsServiceStub = TestBed.inject(SectionsService as any); + sectionsServiceStub.isSectionTypeAvailable.and.returnValue(observableOf(true)); notificationsServiceStub = TestBed.inject(NotificationsService as any); translateService = TestBed.inject(TranslateService); comp.submissionId = submissionId; comp.collectionId = collectionId; - comp.sectionId = 'upload'; comp.uploadFilesOptions = Object.assign(new UploaderOptions(),{ url: '', authToken: null, @@ -133,7 +132,7 @@ describe('SubmissionUploadFilesComponent Component', () => { }); it('should init uploadEnabled properly', () => { - sectionsServiceStub.isSectionAvailable.and.returnValue(hot('-a-b', { + sectionsServiceStub.isSectionTypeAvailable.and.returnValue(hot('-a-b', { a: false, b: true })); @@ -149,55 +148,56 @@ describe('SubmissionUploadFilesComponent Component', () => { expect(compAsAny.uploadEnabled).toBeObservable(expected); }); - it('should show a success notification and call updateSectionData on upload complete', () => { - - const expectedErrors: any = mockUploadResponse1ParsedErrors; - compAsAny.uploadEnabled = observableOf(true); - fixture.detectChanges(); - - comp.onCompleteItem(Object.assign({}, uploadRestResponse, { sections: mockSectionsData })); - - Object.keys(mockSectionsData).forEach((sectionId) => { - expect(sectionsServiceStub.updateSectionData).toHaveBeenCalledWith( - submissionId, - sectionId, - mockSectionsData[sectionId], - expectedErrors[sectionId], - expectedErrors[sectionId] - ); + describe('on upload complete', () => { + beforeEach(() => { + sectionsServiceStub.isSectionType.and.callFake((_, sectionId, __) => observableOf(sectionId === 'upload')); + compAsAny.uploadEnabled = observableOf(true); }); - expect(notificationsServiceStub.success).toHaveBeenCalled(); + it('should show a success notification and call updateSectionData if successful', () => { + const expectedErrors: any = mockUploadResponse1ParsedErrors; + fixture.detectChanges(); - }); + comp.onCompleteItem(Object.assign({}, uploadRestResponse, { sections: mockSectionsData })); - it('should show an error notification and call updateSectionData on upload complete', () => { - - const responseErrors = mockUploadResponse2Errors; - - const expectedErrors: any = mockUploadResponse2ParsedErrors; - compAsAny.uploadEnabled = observableOf(true); - fixture.detectChanges(); - - comp.onCompleteItem(Object.assign({}, uploadRestResponse, { - sections: mockSectionsData, - errors: responseErrors.errors - })); - - Object.keys(mockSectionsData).forEach((sectionId) => { - expect(sectionsServiceStub.updateSectionData).toHaveBeenCalledWith( - submissionId, - sectionId, - mockSectionsData[sectionId], + Object.keys(mockSectionsData).forEach((sectionId) => { + expect(sectionsServiceStub.updateSectionData).toHaveBeenCalledWith( + submissionId, + sectionId, + mockSectionsData[sectionId], expectedErrors[sectionId], - expectedErrors[sectionId] - ); + expectedErrors[sectionId] + ); + }); + + expect(notificationsServiceStub.success).toHaveBeenCalled(); + }); - expect(notificationsServiceStub.success).not.toHaveBeenCalled(); + it('should show an error notification and call updateSectionData if unsuccessful', () => { + const responseErrors = mockUploadResponse2Errors; + const expectedErrors: any = mockUploadResponse2ParsedErrors; + fixture.detectChanges(); + comp.onCompleteItem(Object.assign({}, uploadRestResponse, { + sections: mockSectionsData, + errors: responseErrors.errors + })); + + Object.keys(mockSectionsData).forEach((sectionId) => { + expect(sectionsServiceStub.updateSectionData).toHaveBeenCalledWith( + submissionId, + sectionId, + mockSectionsData[sectionId], + expectedErrors[sectionId], + expectedErrors[sectionId] + ); + }); + + expect(notificationsServiceStub.success).not.toHaveBeenCalled(); + + }); }); - }); }); @@ -210,7 +210,6 @@ class TestComponent { submissionId = mockSubmissionId; collectionId = mockSubmissionCollectionId; - sectionId = 'upload'; uploadFilesOptions = Object.assign(new UploaderOptions(), { url: '', authToken: null, diff --git a/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts b/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts index ce8a4dce20..fa81fe2bbd 100644 --- a/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts +++ b/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts @@ -2,7 +2,7 @@ import { Component, Input, OnChanges } from '@angular/core'; import { TranslateService } from '@ngx-translate/core'; import { Observable, of as observableOf, Subscription } from 'rxjs'; -import { first } from 'rxjs/operators'; +import { first, take } from 'rxjs/operators'; import { SectionsService } from '../../sections/sections.service'; import { hasValue, isEmpty, isNotEmpty } from '../../../shared/empty.util'; @@ -13,6 +13,7 @@ import { UploaderOptions } from '../../../shared/uploader/uploader-options.model import parseSectionErrors from '../../utils/parseSectionErrors'; import { SubmissionJsonPatchOperationsService } from '../../../core/submission/submission-json-patch-operations.service'; import { WorkspaceItem } from '../../../core/submission/models/workspaceitem.model'; +import { SectionsType } from '../../sections/sections-type'; /** * This component represents the drop zone that provides to add files to the submission. @@ -35,12 +36,6 @@ export class SubmissionUploadFilesComponent implements OnChanges { */ @Input() submissionId: string; - /** - * The upload section id - * @type {string} - */ - @Input() sectionId: string; - /** * The uploader configuration options * @type {UploaderOptions} @@ -110,7 +105,7 @@ export class SubmissionUploadFilesComponent implements OnChanges { * Check if upload functionality is enabled */ ngOnChanges() { - this.uploadEnabled = this.sectionService.isSectionAvailable(this.submissionId, this.sectionId); + this.uploadEnabled = this.sectionService.isSectionTypeAvailable(this.submissionId, SectionsType.Upload); } /** @@ -136,14 +131,18 @@ export class SubmissionUploadFilesComponent implements OnChanges { .forEach((sectionId) => { const sectionData = normalizeSectionData(sections[sectionId]); const sectionErrors = errorsList[sectionId]; - if (sectionId === 'upload') { - // Look for errors on upload - if ((isEmpty(sectionErrors))) { - this.notificationsService.success(null, this.translate.get('submission.sections.upload.upload-successful')); - } else { - this.notificationsService.error(null, this.translate.get('submission.sections.upload.upload-failed')); - } - } + this.sectionService.isSectionType(this.submissionId, sectionId, SectionsType.Upload) + .pipe(take(1)) + .subscribe((isUpload) => { + if (isUpload) { + // Look for errors on upload + if ((isEmpty(sectionErrors))) { + this.notificationsService.success(null, this.translate.get('submission.sections.upload.upload-successful')); + } else { + this.notificationsService.error(null, this.translate.get('submission.sections.upload.upload-failed')); + } + } + }); this.sectionService.updateSectionData(this.submissionId, sectionId, sectionData, sectionErrors, sectionErrors); }); } diff --git a/src/app/submission/sections/container/section-container.component.html b/src/app/submission/sections/container/section-container.component.html index 8262d51f6f..28ed377c4a 100644 --- a/src/app/submission/sections/container/section-container.component.html +++ b/src/app/submission/sections/container/section-container.component.html @@ -1,5 +1,5 @@
- {{ 'submission.sections.'+sectionData.header | translate }} + {{ 'submission.sections.'+sectionData.header | translate }}
+ aria-hidden="true" title="{{'submission.sections.status.warnings.title' | translate}}"> + aria-hidden="true" title="{{'submission.sections.status.errors.title' | translate}}"> - - - + aria-hidden="true" title="{{'submission.sections.status.valid.title' | translate}}"> + + + - +
diff --git a/src/app/submission/sections/sections.service.spec.ts b/src/app/submission/sections/sections.service.spec.ts index 1e32969e59..9fbcedcc4d 100644 --- a/src/app/submission/sections/sections.service.spec.ts +++ b/src/app/submission/sections/sections.service.spec.ts @@ -340,6 +340,38 @@ describe('SectionsService test suite', () => { }); }); + describe('isSectionType', () => { + it('should return true if the section matches the provided type', () => { + store.select.and.returnValue(observableOf(submissionState)); + + const expected = cold('(b|)', { + b: true + }); + + expect(service.isSectionType(submissionId, 'upload', SectionsType.Upload)).toBeObservable(expected); + }); + + it('should return false if the section doesn\'t match the provided type', () => { + store.select.and.returnValue(observableOf(submissionState)); + + const expected = cold('(b|)', { + b: false + }); + + expect(service.isSectionType(submissionId, sectionId, SectionsType.Upload)).toBeObservable(expected); + }); + + it('should return false if the provided sectionId doesn\'t exist', () => { + store.select.and.returnValue(observableOf(submissionState)); + + const expected = cold('(b|)', { + b: false + }); + + expect(service.isSectionType(submissionId, 'no-such-id', SectionsType.Upload)).toBeObservable(expected); + }); + }); + describe('addSection', () => { it('should dispatch a new EnableSectionAction a move target to new section', () => { diff --git a/src/app/submission/sections/sections.service.ts b/src/app/submission/sections/sections.service.ts index 69cc79cf28..1ffcc2321c 100644 --- a/src/app/submission/sections/sections.service.ts +++ b/src/app/submission/sections/sections.service.ts @@ -366,6 +366,22 @@ export class SectionsService { distinctUntilChanged()); } + /** + * Check if given section id is of a given section type + * @param submissionId + * @param sectionId + * @param sectionType + */ + public isSectionType(submissionId: string, sectionId: string, sectionType: SectionsType): Observable { + return this.store.select(submissionObjectFromIdSelector(submissionId)).pipe( + filter((submissionState: SubmissionObjectEntry) => isNotUndefined(submissionState)), + map((submissionState: SubmissionObjectEntry) => { + return isNotUndefined(submissionState.sections) && isNotUndefined(submissionState.sections[sectionId]) + && submissionState.sections[sectionId].sectionType === sectionType; + }), + distinctUntilChanged()); + } + /** * Dispatch a new [EnableSectionAction] to add a new section and move page target to it * diff --git a/src/app/submission/sections/upload/file/edit/section-upload-file-edit.component.ts b/src/app/submission/sections/upload/file/edit/section-upload-file-edit.component.ts index 3275787984..96725f151e 100644 --- a/src/app/submission/sections/upload/file/edit/section-upload-file-edit.component.ts +++ b/src/app/submission/sections/upload/file/edit/section-upload-file-edit.component.ts @@ -244,9 +244,9 @@ export class SubmissionSectionUploadFileEditComponent implements OnChanges { if (metadataModel.type === DYNAMIC_FORM_CONTROL_TYPE_DATEPICKER) { const date = new Date(accessCondition[key]); metadataModel.value = { - year: date.getFullYear(), - month: date.getMonth() + 1, - day: date.getDate() + year: date.getUTCFullYear(), + month: date.getUTCMonth() + 1, + day: date.getUTCDate() }; } else { metadataModel.value = accessCondition[key]; @@ -302,9 +302,9 @@ export class SubmissionSectionUploadFileEditComponent implements OnChanges { const min = new Date(accessCondition.maxStartDate); startDateModel.max = { - year: min.getFullYear(), - month: min.getMonth() + 1, - day: min.getDate() + year: min.getUTCFullYear(), + month: min.getUTCMonth() + 1, + day: min.getUTCDate() }; } if (accessCondition.hasEndDate) { @@ -314,9 +314,9 @@ export class SubmissionSectionUploadFileEditComponent implements OnChanges { const max = new Date(accessCondition.maxEndDate); endDateModel.max = { - year: max.getFullYear(), - month: max.getMonth() + 1, - day: max.getDate() + year: max.getUTCFullYear(), + month: max.getUTCMonth() + 1, + day: max.getUTCDate() }; } } diff --git a/src/app/submission/sections/upload/file/section-upload-file.component.html b/src/app/submission/sections/upload/file/section-upload-file.component.html index 64df1155bf..09c062d191 100644 --- a/src/app/submission/sections/upload/file/section-upload-file.component.html +++ b/src/app/submission/sections/upload/file/section-upload-file.component.html @@ -10,10 +10,17 @@
- - - + + + + - - + +