diff --git a/src/app/+bitstream-page/bitstream-authorizations/bitstream-authorizations.component.html b/src/app/+bitstream-page/bitstream-authorizations/bitstream-authorizations.component.html new file mode 100644 index 0000000000..804bb4f891 --- /dev/null +++ b/src/app/+bitstream-page/bitstream-authorizations/bitstream-authorizations.component.html @@ -0,0 +1,10 @@ +
+ +
+
+ + {{'bitstream.edit.return' | translate}} + +
+
+
diff --git a/src/app/+bitstream-page/bitstream-authorizations/bitstream-authorizations.component.spec.ts b/src/app/+bitstream-page/bitstream-authorizations/bitstream-authorizations.component.spec.ts new file mode 100644 index 0000000000..c41351f380 --- /dev/null +++ b/src/app/+bitstream-page/bitstream-authorizations/bitstream-authorizations.component.spec.ts @@ -0,0 +1,84 @@ +import { CommonModule } from '@angular/common'; +import { ChangeDetectorRef, NO_ERRORS_SCHEMA } from '@angular/core'; +import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; +import { ActivatedRoute } from '@angular/router'; + +import { cold } from 'jasmine-marbles'; +import { of as observableOf } from 'rxjs'; +import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; + +import { DSpaceObject } from '../../core/shared/dspace-object.model'; +import { BitstreamAuthorizationsComponent } from './bitstream-authorizations.component'; +import { Bitstream } from '../../core/shared/bitstream.model'; +import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; +import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock'; + +describe('BitstreamAuthorizationsComponent', () => { + let comp: BitstreamAuthorizationsComponent; + let fixture: ComponentFixture>; + + const bitstream = Object.assign(new Bitstream(), { + sizeBytes: 10000, + metadata: { + 'dc.title': [ + { + value: 'file name', + language: null + } + ] + }, + _links: { + content: { href: 'file-selflink' } + } + }); + + const bitstreamRD = createSuccessfulRemoteDataObject(bitstream); + + const routeStub = { + data: observableOf({ + bitstream: bitstreamRD + }) + }; + + beforeEach(waitForAsync(() => { + TestBed.configureTestingModule({ + imports: [ + CommonModule, + TranslateModule.forRoot({ + loader: { + provide: TranslateLoader, + useClass: TranslateLoaderMock + } + }) + ], + declarations: [BitstreamAuthorizationsComponent], + providers: [ + { provide: ActivatedRoute, useValue: routeStub }, + ChangeDetectorRef, + BitstreamAuthorizationsComponent, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(BitstreamAuthorizationsComponent); + comp = fixture.componentInstance; + fixture.detectChanges(); + }); + + afterEach(() => { + comp = null; + fixture.destroy(); + }); + + it('should create', () => { + expect(comp).toBeTruthy(); + }); + + it('should init dso remote data properly', (done) => { + const expected = cold('(a|)', { a: bitstreamRD }); + expect(comp.dsoRD$).toBeObservable(expected); + done(); + }); +}); diff --git a/src/app/+bitstream-page/bitstream-authorizations/bitstream-authorizations.component.ts b/src/app/+bitstream-page/bitstream-authorizations/bitstream-authorizations.component.ts new file mode 100644 index 0000000000..adc0638780 --- /dev/null +++ b/src/app/+bitstream-page/bitstream-authorizations/bitstream-authorizations.component.ts @@ -0,0 +1,40 @@ +import { Component, OnInit } from '@angular/core'; +import { ActivatedRoute } from '@angular/router'; + +import { Observable } from 'rxjs'; +import { first, map } from 'rxjs/operators'; + +import { RemoteData } from '../../core/data/remote-data'; +import { DSpaceObject } from '../../core/shared/dspace-object.model'; + +@Component({ + selector: 'ds-collection-authorizations', + templateUrl: './bitstream-authorizations.component.html', +}) +/** + * Component that handles the Collection Authorizations + */ +export class BitstreamAuthorizationsComponent implements OnInit { + + /** + * The initial DSO object + */ + public dsoRD$: Observable>; + + /** + * Initialize instance variables + * + * @param {ActivatedRoute} route + */ + constructor( + private route: ActivatedRoute + ) { + } + + /** + * Initialize the component, setting up the collection + */ + ngOnInit(): void { + this.dsoRD$ = this.route.data.pipe(first(), map((data) => data.bitstream)); + } +} diff --git a/src/app/+bitstream-page/bitstream-page-routing.module.ts b/src/app/+bitstream-page/bitstream-page-routing.module.ts index bbbd65f279..284f29f7b4 100644 --- a/src/app/+bitstream-page/bitstream-page-routing.module.ts +++ b/src/app/+bitstream-page/bitstream-page-routing.module.ts @@ -4,8 +4,14 @@ import { EditBitstreamPageComponent } from './edit-bitstream-page/edit-bitstream import { AuthenticatedGuard } from '../core/auth/authenticated.guard'; import { BitstreamPageResolver } from './bitstream-page.resolver'; import { BitstreamDownloadPageComponent } from '../shared/bitstream-download-page/bitstream-download-page.component'; +import { ResourcePolicyTargetResolver } from '../shared/resource-policies/resolvers/resource-policy-target.resolver'; +import { ResourcePolicyCreateComponent } from '../shared/resource-policies/create/resource-policy-create.component'; +import { ResourcePolicyResolver } from '../shared/resource-policies/resolvers/resource-policy.resolver'; +import { ResourcePolicyEditComponent } from '../shared/resource-policies/edit/resource-policy-edit.component'; +import { BitstreamAuthorizationsComponent } from './bitstream-authorizations/bitstream-authorizations.component'; const EDIT_BITSTREAM_PATH = ':id/edit'; +const EDIT_BITSTREAM_AUTHORIZATIONS_PATH = ':id/authorizations'; /** * Routing module to help navigate Bitstream pages @@ -27,6 +33,36 @@ const EDIT_BITSTREAM_PATH = ':id/edit'; bitstream: BitstreamPageResolver }, canActivate: [AuthenticatedGuard] + }, + { + path: EDIT_BITSTREAM_AUTHORIZATIONS_PATH, + + children: [ + { + path: 'create', + resolve: { + resourcePolicyTarget: ResourcePolicyTargetResolver + }, + component: ResourcePolicyCreateComponent, + data: { title: 'resource-policies.create.page.title', showBreadcrumbs: true } + }, + { + path: 'edit', + resolve: { + resourcePolicy: ResourcePolicyResolver + }, + component: ResourcePolicyEditComponent, + data: { title: 'resource-policies.edit.page.title', showBreadcrumbs: true } + }, + { + path: '', + resolve: { + bitstream: BitstreamPageResolver + }, + component: BitstreamAuthorizationsComponent, + data: { title: 'bitstream.edit.authorizations.title', showBreadcrumbs: true } + } + ] } ]) ], diff --git a/src/app/+bitstream-page/bitstream-page.module.ts b/src/app/+bitstream-page/bitstream-page.module.ts index 24b4cd512f..80e5ad14e3 100644 --- a/src/app/+bitstream-page/bitstream-page.module.ts +++ b/src/app/+bitstream-page/bitstream-page.module.ts @@ -3,6 +3,7 @@ import { CommonModule } from '@angular/common'; import { SharedModule } from '../shared/shared.module'; import { EditBitstreamPageComponent } from './edit-bitstream-page/edit-bitstream-page.component'; import { BitstreamPageRoutingModule } from './bitstream-page-routing.module'; +import { BitstreamAuthorizationsComponent } from './bitstream-authorizations/bitstream-authorizations.component'; /** * This module handles all components that are necessary for Bitstream related pages @@ -14,6 +15,7 @@ import { BitstreamPageRoutingModule } from './bitstream-page-routing.module'; BitstreamPageRoutingModule ], declarations: [ + BitstreamAuthorizationsComponent, EditBitstreamPageComponent ] }) diff --git a/src/app/+bitstream-page/bitstream-page.resolver.ts b/src/app/+bitstream-page/bitstream-page.resolver.ts index a876b22d5e..fd9d5b351b 100644 --- a/src/app/+bitstream-page/bitstream-page.resolver.ts +++ b/src/app/+bitstream-page/bitstream-page.resolver.ts @@ -35,7 +35,7 @@ export class BitstreamPageResolver implements Resolve> { */ get followLinks(): FollowLinkConfig[] { return [ - followLink('bundle', undefined, true, true, true, followLink('item')), + followLink('bundle', {}, followLink('item')), followLink('format') ]; } diff --git a/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.html b/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.html index fd13e249a0..cbb587cca4 100644 --- a/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.html +++ b/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.html @@ -19,7 +19,11 @@ [submitLabel]="'form.save'" (submitForm)="onSubmit()" (cancel)="onCancel()" - (dfChange)="onChange($event)"> + (dfChange)="onChange($event)"> +
+ {{'bitstream.edit.authorizations.link' | translate}} +
+ diff --git a/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.spec.ts b/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.spec.ts index 2e7eb4e1d1..9c2cb3a093 100644 --- a/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.spec.ts +++ b/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.spec.ts @@ -18,12 +18,8 @@ import { hasValue } from '../../shared/empty.util'; import { FormControl, FormGroup } from '@angular/forms'; import { FileSizePipe } from '../../shared/utils/file-size-pipe'; import { VarDirective } from '../../shared/utils/var.directive'; -import { - createSuccessfulRemoteDataObject, - createSuccessfulRemoteDataObject$ -} from '../../shared/remote-data.utils'; -import { RouterStub } from '../../shared/testing/router.stub'; -import { getEntityEditRoute, getItemEditRoute } from '../../+item-page/item-page-routing-paths'; +import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { getEntityEditRoute } from '../../+item-page/item-page-routing-paths'; import { createPaginatedList } from '../../shared/testing/utils.test'; import { Item } from '../../core/shared/item.model'; @@ -39,7 +35,6 @@ let bitstream: Bitstream; let selectedFormat: BitstreamFormat; let allFormats: BitstreamFormat[]; let router: Router; -let routerStub; describe('EditBitstreamPageComponent', () => { let comp: EditBitstreamPageComponent; @@ -129,10 +124,6 @@ describe('EditBitstreamPageComponent', () => { findAll: createSuccessfulRemoteDataObject$(createPaginatedList(allFormats)) }); - const itemPageUrl = `fake-url/some-uuid`; - routerStub = Object.assign(new RouterStub(), { - url: `${itemPageUrl}` - }); TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), RouterTestingModule], declarations: [EditBitstreamPageComponent, FileSizePipe, VarDirective], @@ -142,7 +133,6 @@ describe('EditBitstreamPageComponent', () => { { provide: ActivatedRoute, useValue: { data: observableOf({ bitstream: createSuccessfulRemoteDataObject(bitstream) }), snapshot: { queryParams: {} } } }, { provide: BitstreamDataService, useValue: bitstreamService }, { provide: BitstreamFormatDataService, useValue: bitstreamFormatService }, - { provide: Router, useValue: routerStub }, ChangeDetectorRef ], schemas: [NO_ERRORS_SCHEMA] @@ -154,7 +144,8 @@ describe('EditBitstreamPageComponent', () => { fixture = TestBed.createComponent(EditBitstreamPageComponent); comp = fixture.componentInstance; fixture.detectChanges(); - router = (comp as any).router; + router = TestBed.inject(Router); + spyOn(router, 'navigate'); }); describe('on startup', () => { @@ -241,14 +232,14 @@ describe('EditBitstreamPageComponent', () => { it('should redirect to the item edit page on the bitstreams tab with the itemId from the component', () => { comp.itemId = 'some-uuid1'; comp.navigateToItemEditBitstreams(); - expect(routerStub.navigate).toHaveBeenCalledWith([getEntityEditRoute(null, 'some-uuid1'), 'bitstreams']); + expect(router.navigate).toHaveBeenCalledWith([getEntityEditRoute(null, 'some-uuid1'), 'bitstreams']); }); }); describe('when navigateToItemEditBitstreams is called, and the component does not have an itemId', () => { it('should redirect to the item edit page on the bitstreams tab with the itemId from the bundle links ', () => { comp.itemId = undefined; comp.navigateToItemEditBitstreams(); - expect(routerStub.navigate).toHaveBeenCalledWith([getEntityEditRoute(null, 'some-uuid'), 'bitstreams']); + expect(router.navigate).toHaveBeenCalledWith([getEntityEditRoute(null, 'some-uuid'), 'bitstreams']); }); }); }); diff --git a/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.ts b/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.ts index 8a4d584647..4ad0aac7ef 100644 --- a/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.ts +++ b/src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.ts @@ -19,10 +19,10 @@ import { cloneDeep } from 'lodash'; import { BitstreamDataService } from '../../core/data/bitstream-data.service'; import { getAllSucceededRemoteDataPayload, - getFirstSucceededRemoteDataPayload, - getRemoteDataPayload, + getFirstCompletedRemoteData, getFirstSucceededRemoteData, - getFirstCompletedRemoteData + getFirstSucceededRemoteDataPayload, + getRemoteDataPayload } from '../../core/shared/operators'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { BitstreamFormatDataService } from '../../core/data/bitstream-format-data.service'; @@ -131,15 +131,6 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { rows: 10 }); - /** - * The Dynamic Input Model for the file's embargo (disabled on this page) - */ - embargoModel = new DynamicInputModel({ - id: 'embargo', - name: 'embargo', - disabled: true - }); - /** * The Dynamic Input Model for the selected format */ @@ -159,7 +150,7 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { /** * All input models in a simple array for easier iterations */ - inputModels = [this.fileNameModel, this.primaryBitstreamModel, this.descriptionModel, this.embargoModel, this.selectedFormatModel, this.newFormatModel]; + inputModels = [this.fileNameModel, this.primaryBitstreamModel, this.descriptionModel, this.selectedFormatModel, this.newFormatModel]; /** * The dynamic form fields used for editing the information of a bitstream @@ -179,12 +170,6 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { this.descriptionModel ] }), - new DynamicFormGroupModel({ - id: 'embargoContainer', - group: [ - this.embargoModel - ] - }), new DynamicFormGroupModel({ id: 'formatContainer', group: [ @@ -243,11 +228,6 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { host: 'row' } }, - embargoContainer: { - grid: { - host: 'row' - } - }, formatContainer: { grid: { host: 'row' 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-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/+collection-page/collection-page.resolver.ts b/src/app/+collection-page/collection-page.resolver.ts index f6f87f117c..d476a180d3 100644 --- a/src/app/+collection-page/collection-page.resolver.ts +++ b/src/app/+collection-page/collection-page.resolver.ts @@ -14,7 +14,7 @@ import { ResolvedAction } from '../core/resolving/resolver.actions'; * Requesting them as embeds will limit the number of requests */ export const COLLECTION_PAGE_LINKS_TO_FOLLOW: FollowLinkConfig[] = [ - followLink('parentCommunity', undefined, true, true, true, + followLink('parentCommunity', {}, followLink('parentCommunity') ), followLink('logo') diff --git a/src/app/+collection-page/delete-collection-page/delete-collection-page.component.html b/src/app/+collection-page/delete-collection-page/delete-collection-page.component.html index 81ba60e51b..4abb149498 100644 --- a/src/app/+collection-page/delete-collection-page/delete-collection-page.component.html +++ b/src/app/+collection-page/delete-collection-page/delete-collection-page.component.html @@ -6,11 +6,12 @@

{{ 'collection.delete.text' | translate:{ dso: dso.name } }}

- -
diff --git a/src/app/+community-page/delete-community-page/delete-community-page.component.html b/src/app/+community-page/delete-community-page/delete-community-page.component.html index 85aa8b1bce..658f3da436 100644 --- a/src/app/+community-page/delete-community-page/delete-community-page.component.html +++ b/src/app/+community-page/delete-community-page/delete-community-page.component.html @@ -6,11 +6,12 @@

{{ 'community.delete.text' | translate:{ dso: dso.name } }}

- -
diff --git a/src/app/+item-page/edit-item-page/item-authorizations/item-authorizations.component.ts b/src/app/+item-page/edit-item-page/item-authorizations/item-authorizations.component.ts index fb193b24d4..76597a135b 100644 --- a/src/app/+item-page/edit-item-page/item-authorizations/item-authorizations.component.ts +++ b/src/app/+item-page/edit-item-page/item-authorizations/item-authorizations.component.ts @@ -4,10 +4,9 @@ import { ActivatedRoute } from '@angular/router'; import { BehaviorSubject, Observable, of as observableOf, Subscription } from 'rxjs'; import { catchError, filter, first, map, mergeMap, take } from 'rxjs/operators'; -import { PaginatedList, buildPaginatedList } from '../../../core/data/paginated-list.model'; +import { buildPaginatedList, PaginatedList } from '../../../core/data/paginated-list.model'; import { - getFirstSucceededRemoteDataPayload, - getFirstSucceededRemoteDataWithNotEmptyPayload + getFirstSucceededRemoteDataPayload, getFirstSucceededRemoteDataWithNotEmptyPayload, } from '../../../core/shared/operators'; import { Item } from '../../../core/shared/item.model'; import { followLink } from '../../../shared/utils/follow-link-config.model'; @@ -15,7 +14,6 @@ import { LinkService } from '../../../core/cache/builders/link.service'; import { Bundle } from '../../../core/shared/bundle.model'; import { hasValue, isNotEmpty } from '../../../shared/empty.util'; import { Bitstream } from '../../../core/shared/bitstream.model'; -import { FindListOptions } from '../../../core/data/request.models'; /** * Interface for a bundle's bitstream map entry @@ -79,7 +77,7 @@ export class ItemAuthorizationsComponent implements OnInit, OnDestroy { getFirstSucceededRemoteDataWithNotEmptyPayload(), map((item: Item) => this.linkService.resolveLink( item, - followLink('bundles', new FindListOptions(), true, true, true, followLink('bitstreams')) + followLink('bundles', {}, followLink('bitstreams')) )) ) as Observable; diff --git a/src/app/+item-page/edit-item-page/item-move/item-move.component.html b/src/app/+item-page/edit-item-page/item-move/item-move.component.html index 74ca9aae4e..f68cfb0685 100644 --- a/src/app/+item-page/edit-item-page/item-move/item-move.component.html +++ b/src/app/+item-page/edit-item-page/item-move/item-move.component.html @@ -5,19 +5,16 @@

{{'item.edit.move.description' | translate}}

- - - +
+
{{'dso-selector.placeholder' | translate: { type: 'collection' } }}
+
+ + +
+
+
@@ -33,16 +30,24 @@
- - +
+
+ + + +
+
diff --git a/src/app/+item-page/edit-item-page/item-move/item-move.component.spec.ts b/src/app/+item-page/edit-item-page/item-move/item-move.component.spec.ts index dd91c65e1e..d200891629 100644 --- a/src/app/+item-page/edit-item-page/item-move/item-move.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-move/item-move.component.spec.ts @@ -21,6 +21,8 @@ import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; import { createPaginatedList } from '../../../shared/testing/utils.test'; +import { RequestService } from '../../../core/data/request.service'; +import { getMockRequestService } from '../../../shared/mocks/request.service.mock'; describe('ItemMoveComponent', () => { let comp: ItemMoveComponent; @@ -47,18 +49,25 @@ describe('ItemMoveComponent', () => { name: 'Test collection 2' }); - const mockItemDataService = jasmine.createSpyObj({ - moveToCollection: createSuccessfulRemoteDataObject$(collection1) + let itemDataService; + + const mockItemDataServiceSuccess = jasmine.createSpyObj({ + moveToCollection: createSuccessfulRemoteDataObject$(collection1), + findById: createSuccessfulRemoteDataObject$(mockItem), }); const mockItemDataServiceFail = jasmine.createSpyObj({ - moveToCollection: createFailedRemoteDataObject$('Internal server error', 500) + moveToCollection: createFailedRemoteDataObject$('Internal server error', 500), + findById: createSuccessfulRemoteDataObject$(mockItem), }); const routeStub = { data: observableOf({ dso: createSuccessfulRemoteDataObject(Object.assign(new Item(), { - id: 'item1' + id: 'item1', + owningCollection: createSuccessfulRemoteDataObject$(Object.assign(new Collection(), { + id: 'originalOwningCollection', + })) })) }) }; @@ -79,43 +88,40 @@ describe('ItemMoveComponent', () => { const notificationsServiceStub = new NotificationsServiceStub(); + const init = (mockItemDataService) => { + itemDataService = mockItemDataService; + + TestBed.configureTestingModule({ + imports: [CommonModule, FormsModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule], + declarations: [ItemMoveComponent], + providers: [ + { provide: ActivatedRoute, useValue: routeStub }, + { provide: Router, useValue: routerStub }, + { provide: ItemDataService, useValue: mockItemDataService }, + { provide: NotificationsService, useValue: notificationsServiceStub }, + { provide: SearchService, useValue: mockSearchService }, + { provide: RequestService, useValue: getMockRequestService() }, + ], schemas: [ + CUSTOM_ELEMENTS_SCHEMA + ] + }).compileComponents(); + fixture = TestBed.createComponent(ItemMoveComponent); + comp = fixture.componentInstance; + fixture.detectChanges(); + }; + describe('ItemMoveComponent success', () => { beforeEach(() => { - TestBed.configureTestingModule({ - imports: [CommonModule, FormsModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule], - declarations: [ItemMoveComponent], - providers: [ - { provide: ActivatedRoute, useValue: routeStub }, - { provide: Router, useValue: routerStub }, - { provide: ItemDataService, useValue: mockItemDataService }, - { provide: NotificationsService, useValue: notificationsServiceStub }, - { provide: SearchService, useValue: mockSearchService }, - ], schemas: [ - CUSTOM_ELEMENTS_SCHEMA - ] - }).compileComponents(); - fixture = TestBed.createComponent(ItemMoveComponent); - comp = fixture.componentInstance; - fixture.detectChanges(); + init(mockItemDataServiceSuccess); }); - it('should load suggestions', () => { - const expected = [ - collection1, - collection2 - ]; - comp.collectionSearchResults.subscribe((value) => { - expect(value).toEqual(expected); - } - ); - }); it('should get current url ', () => { expect(comp.getCurrentUrl()).toEqual('fake-url/fake-id/edit'); }); - it('should on click select the correct collection name and id', () => { + it('should select the correct collection name and id on click', () => { const data = collection1; - comp.onClick(data); + comp.selectDso(data); expect(comp.selectedCollectionName).toEqual('Test collection 1'); expect(comp.selectedCollection).toEqual(collection1); @@ -128,12 +134,12 @@ describe('ItemMoveComponent', () => { }); comp.selectedCollectionName = 'selected-collection-id'; comp.selectedCollection = collection1; - comp.moveCollection(); + comp.moveToCollection(); - expect(mockItemDataService.moveToCollection).toHaveBeenCalledWith('item-id', collection1); + expect(itemDataService.moveToCollection).toHaveBeenCalledWith('item-id', collection1); }); it('should call notificationsService success message on success', () => { - comp.moveCollection(); + comp.moveToCollection(); expect(notificationsServiceStub.success).toHaveBeenCalled(); }); @@ -142,26 +148,11 @@ describe('ItemMoveComponent', () => { describe('ItemMoveComponent fail', () => { beforeEach(() => { - TestBed.configureTestingModule({ - imports: [CommonModule, FormsModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule], - declarations: [ItemMoveComponent], - providers: [ - { provide: ActivatedRoute, useValue: routeStub }, - { provide: Router, useValue: routerStub }, - { provide: ItemDataService, useValue: mockItemDataServiceFail }, - { provide: NotificationsService, useValue: notificationsServiceStub }, - { provide: SearchService, useValue: mockSearchService }, - ], schemas: [ - CUSTOM_ELEMENTS_SCHEMA - ] - }).compileComponents(); - fixture = TestBed.createComponent(ItemMoveComponent); - comp = fixture.componentInstance; - fixture.detectChanges(); + init(mockItemDataServiceFail); }); it('should call notificationsService error message on fail', () => { - comp.moveCollection(); + comp.moveToCollection(); expect(notificationsServiceStub.error).toHaveBeenCalled(); }); diff --git a/src/app/+item-page/edit-item-page/item-move/item-move.component.ts b/src/app/+item-page/edit-item-page/item-move/item-move.component.ts index b1ed121b40..b7ab761fe5 100644 --- a/src/app/+item-page/edit-item-page/item-move/item-move.component.ts +++ b/src/app/+item-page/edit-item-page/item-move/item-move.component.ts @@ -1,25 +1,21 @@ import { Component, OnInit } from '@angular/core'; -import { first, map } from 'rxjs/operators'; +import { map, switchMap } from 'rxjs/operators'; import { DSpaceObjectType } from '../../../core/shared/dspace-object-type.model'; import { RemoteData } from '../../../core/data/remote-data'; -import { DSpaceObject } from '../../../core/shared/dspace-object.model'; -import { PaginatedList } from '../../../core/data/paginated-list.model'; import { Item } from '../../../core/shared/item.model'; import { ActivatedRoute, Router } from '@angular/router'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; import { - getFirstSucceededRemoteData, - getFirstCompletedRemoteData, getAllSucceededRemoteDataPayload + getAllSucceededRemoteDataPayload, getFirstCompletedRemoteData, getFirstSucceededRemoteData, getRemoteDataPayload, } from '../../../core/shared/operators'; import { ItemDataService } from '../../../core/data/item-data.service'; -import { Observable, of as observableOf } from 'rxjs'; +import { Observable } from 'rxjs'; import { Collection } from '../../../core/shared/collection.model'; -import { PaginationComponentOptions } from '../../../shared/pagination/pagination-component-options.model'; import { SearchService } from '../../../core/shared/search/search.service'; -import { PaginatedSearchOptions } from '../../../shared/search/paginated-search-options.model'; -import { SearchResult } from '../../../shared/search/search-result.model'; import { getItemEditRoute, getItemPageRoute } from '../../item-page-routing-paths'; +import { followLink } from '../../../shared/utils/follow-link-config.model'; +import { RequestService } from '../../../core/data/request.service'; @Component({ selector: 'ds-item-move', @@ -38,7 +34,8 @@ export class ItemMoveComponent implements OnInit { inheritPolicies = false; itemRD$: Observable>; - collectionSearchResults: Observable = observableOf([]); + originalCollection: Collection; + selectedCollectionName: string; selectedCollection: Collection; canSubmit = false; @@ -46,23 +43,26 @@ export class ItemMoveComponent implements OnInit { item: Item; processing = false; - pagination = new PaginationComponentOptions(); - /** * Route to the item's page */ itemPageRoute$: Observable; + COLLECTIONS = [DSpaceObjectType.COLLECTION]; + constructor(private route: ActivatedRoute, private router: Router, private notificationsService: NotificationsService, private itemDataService: ItemDataService, private searchService: SearchService, - private translateService: TranslateService) { - } + private translateService: TranslateService, + private requestService: RequestService, + ) {} ngOnInit(): void { - this.itemRD$ = this.route.data.pipe(map((data) => data.dso), getFirstSucceededRemoteData()) as Observable>; + this.itemRD$ = this.route.data.pipe( + map((data) => data.dso), getFirstSucceededRemoteData() + ) as Observable>; this.itemPageRoute$ = this.itemRD$.pipe( getAllSucceededRemoteDataPayload(), map((item) => getItemPageRoute(item)) @@ -71,43 +71,22 @@ export class ItemMoveComponent implements OnInit { this.item = rd.payload; } ); - this.pagination.pageSize = 5; - this.loadSuggestions(''); - } - - /** - * Find suggestions based on entered query - * @param query - Search query - */ - findSuggestions(query): void { - this.loadSuggestions(query); - } - - /** - * Load all available collections to move the item to. - * TODO: When the API support it, only fetch collections where user has ADD rights to. - */ - loadSuggestions(query): void { - this.collectionSearchResults = this.searchService.search(new PaginatedSearchOptions({ - pagination: this.pagination, - dsoTypes: [DSpaceObjectType.COLLECTION], - query: query - })).pipe( - first(), - map((rd: RemoteData>>) => { - return rd.payload.page.map((searchResult) => { - return searchResult.indexableObject; - }); - }) , - ); - + this.itemRD$.pipe( + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + switchMap((item) => item.owningCollection), + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + ).subscribe((collection) => { + this.originalCollection = collection; + }); } /** * Set the collection name and id based on the selected value * @param data - obtained from the ds-input-suggestions component */ - onClick(data: any): void { + selectDso(data: any): void { this.selectedCollection = data; this.selectedCollectionName = data.name; this.canSubmit = true; @@ -123,26 +102,41 @@ export class ItemMoveComponent implements OnInit { /** * Moves the item to a new collection based on the selected collection */ - moveCollection() { + moveToCollection() { this.processing = true; - this.itemDataService.moveToCollection(this.item.id, this.selectedCollection).pipe(getFirstCompletedRemoteData()).subscribe( - (response: RemoteData) => { - this.router.navigate([getItemEditRoute(this.item)]); - if (response.hasSucceeded) { - this.notificationsService.success(this.translateService.get('item.edit.move.success')); - } else { - this.notificationsService.error(this.translateService.get('item.edit.move.error')); - } - this.processing = false; + const move$ = this.itemDataService.moveToCollection(this.item.id, this.selectedCollection) + .pipe(getFirstCompletedRemoteData()); + + move$.subscribe((response: RemoteData) => { + if (response.hasSucceeded) { + this.notificationsService.success(this.translateService.get('item.edit.move.success')); + } else { + this.notificationsService.error(this.translateService.get('item.edit.move.error')); } - ); + }); + + move$.pipe( + switchMap(() => this.requestService.setStaleByHrefSubstring(this.item.id)), + switchMap(() => + this.itemDataService.findById( + this.item.id, + false, + true, + followLink('owningCollection') + )), + getFirstCompletedRemoteData() + ).subscribe(() => { + this.processing = false; + this.router.navigate([getItemEditRoute(this.item)]); + }); } - /** - * Resets the can submit when the user changes the content of the input field - * @param data - */ - resetCollection(data: any) { + discard(): void { + this.selectedCollection = null; this.canSubmit = false; } + + get canMove(): boolean { + return this.canSubmit && this.selectedCollection?.id !== this.originalCollection.id; + } } diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html index 5583de5fd5..2185108c8f 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html @@ -6,21 +6,30 @@ - + - +
+ - -
{{"item.edit.relationships.no-relationships" | translate}}
+ +
+ +
{{"item.edit.relationships.no-relationships" | translate}}
 - + diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts index 90fad00a45..6742234058 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts @@ -16,6 +16,12 @@ import { SharedModule } from '../../../../shared/shared.module'; import { EditRelationshipListComponent } from './edit-relationship-list.component'; import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; import { createPaginatedList } from '../../../../shared/testing/utils.test'; +import { PaginationService } from '../../../../core/pagination/pagination.service'; +import { PaginationServiceStub } from '../../../../shared/testing/pagination-service.stub'; +import { HostWindowService } from '../../../../shared/host-window.service'; +import { HostWindowServiceStub } from '../../../../shared/testing/host-window-service.stub'; +import { PaginationComponent } from '../../../../shared/pagination/pagination.component'; +import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model'; let comp: EditRelationshipListComponent; let fixture: ComponentFixture; @@ -25,6 +31,8 @@ let linkService; let objectUpdatesService; let relationshipService; let selectableListService; +let paginationService; +let hostWindowService; const url = 'http://test-url.com/test-url'; @@ -37,9 +45,21 @@ let fieldUpdate1; let fieldUpdate2; let relationships; let relationshipType; +let paginationOptions; describe('EditRelationshipListComponent', () => { + const resetComponent = () => { + fixture = TestBed.createComponent(EditRelationshipListComponent); + comp = fixture.componentInstance; + de = fixture.debugElement; + comp.item = item; + comp.itemType = entityType; + comp.url = url; + comp.relationshipType = relationshipType; + fixture.detectChanges(); + }; + beforeEach(waitForAsync(() => { entityType = Object.assign(new ItemType(), { @@ -63,6 +83,12 @@ describe('EditRelationshipListComponent', () => { rightwardType: 'isPublicationOfAuthor', }); + paginationOptions = Object.assign(new PaginationComponentOptions(), { + id: `er${relationshipType.id}`, + pageSize: 5, + currentPage: 1, + }); + author1 = Object.assign(new Item(), { id: 'author1', uuid: 'author1' @@ -141,6 +167,10 @@ describe('EditRelationshipListComponent', () => { resolveLinks: () => null, }; + paginationService = new PaginationServiceStub(paginationOptions); + + hostWindowService = new HostWindowServiceStub(1200); + TestBed.configureTestingModule({ imports: [SharedModule, TranslateModule.forRoot()], declarations: [EditRelationshipListComponent], @@ -149,22 +179,15 @@ describe('EditRelationshipListComponent', () => { { provide: RelationshipService, useValue: relationshipService }, { provide: SelectableListService, useValue: selectableListService }, { provide: LinkService, useValue: linkService }, + { provide: PaginationService, useValue: paginationService }, + { provide: HostWindowService, useValue: hostWindowService }, ], schemas: [ NO_ERRORS_SCHEMA ] }).compileComponents(); - })); - beforeEach(() => { - fixture = TestBed.createComponent(EditRelationshipListComponent); - comp = fixture.componentInstance; - de = fixture.debugElement; - comp.item = item; - comp.itemType = entityType; - comp.url = url; - comp.relationshipType = relationshipType; - fixture.detectChanges(); - }); + resetComponent(); + })); describe('changeType is REMOVE', () => { beforeEach(() => { @@ -176,4 +199,82 @@ describe('EditRelationshipListComponent', () => { expect(element.classList).toContain('alert-danger'); }); }); + + describe('pagination component', () => { + let paginationComp: PaginationComponent; + + beforeEach(() => { + paginationComp = de.query(By.css('ds-pagination')).componentInstance; + }); + + it('should receive the correct pagination config', () => { + expect(paginationComp.paginationOptions).toEqual(paginationOptions); + }); + + it('should receive correct collection size', () => { + expect(paginationComp.collectionSize).toEqual(relationships.length); + }); + + }); + + describe('relationshipService.getItemRelationshipsByLabel', () => { + it('should receive the correct pagination info', () => { + expect(relationshipService.getItemRelationshipsByLabel).toHaveBeenCalledTimes(1); + + const callArgs = relationshipService.getItemRelationshipsByLabel.calls.mostRecent().args; + const findListOptions = callArgs[2]; + + expect(findListOptions.elementsPerPage).toEqual(paginationOptions.pageSize); + expect(findListOptions.currentPage).toEqual(paginationOptions.currentPage); + }); + + describe('when the publication is on the left side of the relationship', () => { + beforeEach(() => { + relationshipType = Object.assign(new RelationshipType(), { + id: '1', + uuid: '1', + leftType: createSuccessfulRemoteDataObject$(entityType), // publication + rightType: createSuccessfulRemoteDataObject$(relatedEntityType), // author + leftwardType: 'isAuthorOfPublication', + rightwardType: 'isPublicationOfAuthor', + }); + relationshipService.getItemRelationshipsByLabel.calls.reset(); + resetComponent(); + }); + + it('should fetch isAuthorOfPublication', () => { + expect(relationshipService.getItemRelationshipsByLabel).toHaveBeenCalledTimes(1); + + const callArgs = relationshipService.getItemRelationshipsByLabel.calls.mostRecent().args; + const label = callArgs[1]; + + expect(label).toEqual('isAuthorOfPublication'); + }); + }); + + describe('when the publication is on the right side of the relationship', () => { + beforeEach(() => { + relationshipType = Object.assign(new RelationshipType(), { + id: '1', + uuid: '1', + leftType: createSuccessfulRemoteDataObject$(relatedEntityType), // author + rightType: createSuccessfulRemoteDataObject$(entityType), // publication + leftwardType: 'isPublicationOfAuthor', + rightwardType: 'isAuthorOfPublication', + }); + relationshipService.getItemRelationshipsByLabel.calls.reset(); + resetComponent(); + }); + + it('should fetch isAuthorOfPublication', () => { + expect(relationshipService.getItemRelationshipsByLabel).toHaveBeenCalledTimes(1); + + const callArgs = relationshipService.getItemRelationshipsByLabel.calls.mostRecent().args; + const label = callArgs[1]; + + expect(label).toEqual('isAuthorOfPublication'); + }); + }); + }); + }); diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts index 3f9637cdc9..9f417ab799 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts @@ -1,9 +1,14 @@ -import { Component, Input, OnInit } from '@angular/core'; +import { Component, Input, OnInit, OnDestroy } from '@angular/core'; import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; import { LinkService } from '../../../../core/cache/builders/link.service'; import { FieldChangeType } from '../../../../core/data/object-updates/object-updates.actions'; import { ObjectUpdatesService } from '../../../../core/data/object-updates/object-updates.service'; -import { combineLatest as observableCombineLatest, Observable, of } from 'rxjs'; +import { + combineLatest as observableCombineLatest, + Observable, + of as observableOf, + from as observableFrom +} from 'rxjs'; import { FieldUpdate, FieldUpdates, @@ -11,14 +16,24 @@ import { } from '../../../../core/data/object-updates/object-updates.reducer'; import { RelationshipService } from '../../../../core/data/relationship.service'; import { Item } from '../../../../core/shared/item.model'; -import { defaultIfEmpty, map, mergeMap, switchMap, take, startWith } from 'rxjs/operators'; -import { hasValue, hasValueOperator } from '../../../../shared/empty.util'; +import { + defaultIfEmpty, + map, + mergeMap, + switchMap, + take, + startWith, + toArray, + tap +} from 'rxjs/operators'; +import { hasValue, hasValueOperator, hasNoValue } from '../../../../shared/empty.util'; import { Relationship } from '../../../../core/shared/item-relationships/relationship.model'; import { RelationshipType } from '../../../../core/shared/item-relationships/relationship-type.model'; import { - getAllSucceededRemoteData, getRemoteDataPayload, - getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload, + getFirstSucceededRemoteData, + getFirstSucceededRemoteDataPayload, + getAllSucceededRemoteData, } from '../../../../core/shared/operators'; import { ItemType } from '../../../../core/shared/item-relationships/item-type.model'; import { DsDynamicLookupRelationModalComponent } from '../../../../shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component'; @@ -30,6 +45,10 @@ import { followLink } from '../../../../shared/utils/follow-link-config.model'; import { PaginatedList } from '../../../../core/data/paginated-list.model'; import { RemoteData } from '../../../../core/data/remote-data'; import { Collection } from '../../../../core/shared/collection.model'; +import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; +import { Subscription } from 'rxjs/internal/Subscription'; +import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model'; +import { PaginationService } from '../../../../core/pagination/pagination.service'; @Component({ selector: 'ds-edit-relationship-list', @@ -40,7 +59,7 @@ import { Collection } from '../../../../core/shared/collection.model'; * A component creating a list of editable relationships of a certain type * The relationships are rendered as a list of related items */ -export class EditRelationshipListComponent implements OnInit { +export class EditRelationshipListComponent implements OnInit, OnDestroy { /** * The item to display related items for @@ -60,6 +79,17 @@ export class EditRelationshipListComponent implements OnInit { */ @Input() relationshipType: RelationshipType; + /** + * Observable that emits the left and right item type of {@link relationshipType} simultaneously. + */ + private relationshipLeftAndRightType$: Observable<[ItemType, ItemType]>; + + /** + * Observable that emits true if {@link itemType} is on the left-hand side of {@link relationshipType}, + * false if it is on the right-hand side and undefined in the rare case that it is on neither side. + */ + private currentItemIsLeftItem$: Observable; + private relatedEntityType$: Observable; /** @@ -70,7 +100,38 @@ export class EditRelationshipListComponent implements OnInit { /** * The FieldUpdates for the relationships in question */ - updates$: Observable; + updates$: BehaviorSubject = new BehaviorSubject(undefined); + + /** + * The RemoteData for the relationships + */ + relationshipsRd$: BehaviorSubject>> = new BehaviorSubject(undefined); + + /** + * Whether the current page is the last page + */ + isLastPage$: BehaviorSubject = new BehaviorSubject(true); + + /** + * Whether we're loading + */ + loading$: BehaviorSubject = new BehaviorSubject(true); + + /** + * The number of added fields that haven't been saved yet + */ + nbAddedFields$: BehaviorSubject = new BehaviorSubject(0); + + /** + * Array to track all subscriptions and unsubscribe them onDestroy + * @type {Array} + */ + private subs: Subscription[] = []; + + /** + * The pagination config + */ + paginationConfig: PaginationComponentOptions; /** * A reference to the lookup window @@ -82,6 +143,7 @@ export class EditRelationshipListComponent implements OnInit { protected linkService: LinkService, protected relationshipService: RelationshipService, protected modalService: NgbModal, + protected paginationService: PaginationService, protected selectableListService: SelectableListService, ) { } @@ -172,6 +234,10 @@ export class EditRelationshipListComponent implements OnInit { this.objectUpdatesService.saveAddFieldUpdate(this.url, update); }); } + + this.loading$.next(true); + // emit the last page again to trigger a fieldupdates refresh + this.relationshipsRd$.next(this.relationshipsRd$.getValue()); }); }); }; @@ -186,6 +252,10 @@ export class EditRelationshipListComponent implements OnInit { ) ); }); + + this.loading$.next(true); + // emit the last page again to trigger a fieldupdates refresh + this.relationshipsRd$.next(this.relationshipsRd$.getValue()); }; this.relatedEntityType$ .pipe(take(1)) @@ -212,10 +282,10 @@ export class EditRelationshipListComponent implements OnInit { if (field.relationship) { return this.getRelatedItem(field.relationship); } else { - return of(field.relatedItem); + return observableOf(field.relatedItem); } }) - ) : of([]) + ) : observableOf([]) ), take(1), map((items) => items.map((item) => { @@ -267,18 +337,19 @@ export class EditRelationshipListComponent implements OnInit { } ngOnInit(): void { + // store the left and right type of the relationship in a single observable + this.relationshipLeftAndRightType$ = observableCombineLatest([ + this.relationshipType.leftType, + this.relationshipType.rightType, + ].map((type) => type.pipe( + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + ))) as Observable<[ItemType, ItemType]>; - this.relatedEntityType$ = - observableCombineLatest([ - this.relationshipType.leftType, - this.relationshipType.rightType, - ].map((type) => type.pipe( - getFirstSucceededRemoteData(), - getRemoteDataPayload(), - ))).pipe( - map((relatedTypes: ItemType[]) => relatedTypes.find((relatedType) => relatedType.uuid !== this.itemType.uuid)), - hasValueOperator() - ); + this.relatedEntityType$ = this.relationshipLeftAndRightType$.pipe( + map((relatedTypes: ItemType[]) => relatedTypes.find((relatedType) => relatedType.uuid !== this.itemType.uuid)), + hasValueOperator() + ); this.relatedEntityType$.pipe( take(1) @@ -286,65 +357,142 @@ export class EditRelationshipListComponent implements OnInit { (relatedEntityType) => this.listId = `edit-relationship-${this.itemType.id}-${relatedEntityType.id}` ); - this.updates$ = this.getItemRelationships().pipe( - switchMap((relationships) => - observableCombineLatest( - relationships.map((relationship) => this.relationshipService.isLeftItem(relationship, this.item)) - ).pipe( - defaultIfEmpty([]), - map((isLeftItemArray) => isLeftItemArray.map((isLeftItem, index) => { - const relationship = relationships[index]; - const nameVariant = isLeftItem ? relationship.rightwardValue : relationship.leftwardValue; + this.currentItemIsLeftItem$ = this.relationshipLeftAndRightType$.pipe( + map(([leftType, rightType]: [ItemType, ItemType]) => { + if (leftType.id === this.itemType.id) { + return true; + } + + if (rightType.id === this.itemType.id) { + return false; + } + + // should never happen... + console.warn(`The item ${this.item.uuid} is not on the right or the left side of relationship type ${this.relationshipType.uuid}`); + return undefined; + }) + ); + + // initialize the pagination options + this.paginationConfig = new PaginationComponentOptions(); + this.paginationConfig.id = `er${this.relationshipType.id}`; + this.paginationConfig.pageSize = 5; + this.paginationConfig.currentPage = 1; + + // get the pagination params from the route + const currentPagination$ = this.paginationService.getCurrentPagination( + this.paginationConfig.id, + this.paginationConfig + ).pipe( + tap(() => this.loading$.next(true)) + ); + + this.subs.push( + observableCombineLatest([ + currentPagination$, + this.currentItemIsLeftItem$, + ]).pipe( + switchMap(([currentPagination, currentItemIsLeftItem]: [PaginationComponentOptions, boolean]) => + // get the relationships for the current item, relationshiptype and page + this.relationshipService.getItemRelationshipsByLabel( + this.item, + currentItemIsLeftItem ? this.relationshipType.leftwardType : this.relationshipType.rightwardType, + { + elementsPerPage: currentPagination.pageSize, + currentPage: currentPagination.currentPage, + }, + false, + true, + followLink('leftItem'), + followLink('rightItem'), + )), + ).subscribe((rd: RemoteData>) => { + this.relationshipsRd$.next(rd); + }) + ); + + // keep isLastPage$ up to date based on relationshipsRd$ + this.subs.push(this.relationshipsRd$.pipe( + hasValueOperator(), + getAllSucceededRemoteData() + ).subscribe((rd: RemoteData>) => { + this.isLastPage$.next(hasNoValue(rd.payload._links.next)); + })); + + this.subs.push(this.relationshipsRd$.pipe( + hasValueOperator(), + getAllSucceededRemoteData(), + switchMap((rd: RemoteData>) => + // emit each relationship in the page separately + observableFrom(rd.payload.page).pipe( + mergeMap((relationship: Relationship) => + // check for each relationship whether it's the left item + this.relationshipService.isLeftItem(relationship, this.item).pipe( + // emit an array containing both the relationship and whether it's the left item, + // as we'll need both + map((isLeftItem: boolean) => [relationship, isLeftItem]) + ) + ), + map(([relationship, isLeftItem]: [Relationship, boolean]) => { + // turn it into a RelationshipIdentifiable, an + const nameVariant = + isLeftItem ? relationship.rightwardValue : relationship.leftwardValue; return { uuid: relationship.id, type: this.relationshipType, relationship, nameVariant, } as RelationshipIdentifiable; - })), - )), - switchMap((initialFields) => this.objectUpdatesService.getFieldUpdates(this.url, initialFields).pipe( - map((fieldUpdates) => { - const fieldUpdatesFiltered: FieldUpdates = {}; - Object.keys(fieldUpdates).forEach((uuid) => { - if (hasValue(fieldUpdates[uuid])) { - const field = fieldUpdates[uuid].field; - if ((field as RelationshipIdentifiable).type.id === this.relationshipType.id) { - fieldUpdatesFiltered[uuid] = fieldUpdates[uuid]; - } - } - }); - return fieldUpdatesFiltered; - }), + }), + // wait until all relationships have been processed, and emit them all as a single array + toArray(), + // if the pipe above completes without emitting anything, emit an empty array instead + defaultIfEmpty([]) )), + switchMap((nextFields: RelationshipIdentifiable[]) => { + // Get a list that contains the unsaved changes for the page, as well as the page of + // RelationshipIdentifiables, as a single list of FieldUpdates + return this.objectUpdatesService.getFieldUpdates(this.url, nextFields).pipe( + map((fieldUpdates: FieldUpdates) => { + const fieldUpdatesFiltered: FieldUpdates = {}; + this.nbAddedFields$.next(0); + // iterate over the fieldupdates and filter out the ones that pertain to this + // relationshiptype + Object.keys(fieldUpdates).forEach((uuid) => { + if (hasValue(fieldUpdates[uuid])) { + const field = fieldUpdates[uuid].field as RelationshipIdentifiable; + // only include fieldupdates regarding this RelationshipType + if (field.type.id === this.relationshipType.id) { + // if it's a newly added relationship + if (fieldUpdates[uuid].changeType === FieldChangeType.ADD) { + // increase the counter that tracks new relationships + this.nbAddedFields$.next(this.nbAddedFields$.getValue() + 1); + if (this.isLastPage$.getValue() === true) { + // only include newly added relationships to the output if we're on the last + // page + fieldUpdatesFiltered[uuid] = fieldUpdates[uuid]; + } + } else { + // include all others + fieldUpdatesFiltered[uuid] = fieldUpdates[uuid]; + } + } + } + }); + return fieldUpdatesFiltered; + }), + ); + }), startWith({}), - ); + ).subscribe((updates: FieldUpdates) => { + this.loading$.next(false); + this.updates$.next(updates); + })); } - private getItemRelationships() { - this.linkService.resolveLink(this.item, - followLink('relationships', undefined, true, true, true, - followLink('relationshipType'), - followLink('leftItem'), - followLink('rightItem'), - )); - return this.item.relationships.pipe( - getAllSucceededRemoteData(), - map((relationships: RemoteData>) => relationships.payload.page.filter((relationship: Relationship) => hasValue(relationship))), - switchMap((itemRelationships: Relationship[]) => - observableCombineLatest( - itemRelationships - .map((relationship) => relationship.relationshipType.pipe( - getFirstSucceededRemoteData(), - getRemoteDataPayload(), - )) - ).pipe( - defaultIfEmpty([]), - map((relationshipTypes) => itemRelationships.filter( - (relationship, index) => relationshipTypes[index].id === this.relationshipType.id) - ), - ) - ), - ); + ngOnDestroy(): void { + this.subs + .filter((subscription) => hasValue(subscription)) + .forEach((subscription) => subscription.unsubscribe()); } } diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 1c5ed3c02b..e22ad8ddcb 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -227,7 +227,6 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { * Sends all initial values of this item to the object updates service */ public initializeOriginalFields() { - console.log('init'); return this.relationshipService.getRelatedItems(this.item).pipe( take(1), ).subscribe((items: Item[]) => { diff --git a/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts b/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts index 214484120e..e21c1a32eb 100644 --- a/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts +++ b/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts @@ -68,7 +68,8 @@ export class FullFileSectionComponent extends FileSectionComponent implements On {elementsPerPage: options.pageSize, currentPage: options.currentPage}, true, true, - followLink('format') + followLink('format'), + followLink('thumbnail'), )), tap((rd: RemoteData>) => { if (hasValue(rd.errorMessage)) { @@ -85,7 +86,8 @@ export class FullFileSectionComponent extends FileSectionComponent implements On {elementsPerPage: options.pageSize, currentPage: options.currentPage}, true, true, - followLink('format') + followLink('format'), + followLink('thumbnail'), )), tap((rd: RemoteData>) => { if (hasValue(rd.errorMessage)) { diff --git a/src/app/+item-page/full/full-item-page.component.ts b/src/app/+item-page/full/full-item-page.component.ts index aea350e58e..da16e134cb 100644 --- a/src/app/+item-page/full/full-item-page.component.ts +++ b/src/app/+item-page/full/full-item-page.component.ts @@ -1,8 +1,8 @@ -import {filter, map} from 'rxjs/operators'; +import { filter, map } from 'rxjs/operators'; import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { Observable , BehaviorSubject } from 'rxjs'; +import { Observable, BehaviorSubject } from 'rxjs'; import { ItemPageComponent } from '../simple/item-page.component'; import { MetadataMap } from '../../core/shared/metadata.models'; @@ -11,8 +11,6 @@ import { ItemDataService } from '../../core/data/item-data.service'; import { RemoteData } from '../../core/data/remote-data'; import { Item } from '../../core/shared/item.model'; -import { MetadataService } from '../../core/metadata/metadata.service'; - import { fadeInOut } from '../../shared/animations/fade'; import { hasValue } from '../../shared/empty.util'; import { AuthService } from '../../core/auth/auth.service'; @@ -35,8 +33,8 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit { metadata$: Observable; - constructor(route: ActivatedRoute, router: Router, items: ItemDataService, metadataService: MetadataService, authService: AuthService) { - super(route, router, items, metadataService, authService); + constructor(route: ActivatedRoute, router: Router, items: ItemDataService, authService: AuthService) { + super(route, router, items, authService); } /*** AoT inheritance fix, will hopefully be resolved in the near future **/ diff --git a/src/app/+item-page/item.resolver.ts b/src/app/+item-page/item.resolver.ts index 99b96511fe..ca6a6c5958 100644 --- a/src/app/+item-page/item.resolver.ts +++ b/src/app/+item-page/item.resolver.ts @@ -5,7 +5,6 @@ import { RemoteData } from '../core/data/remote-data'; import { ItemDataService } from '../core/data/item-data.service'; import { Item } from '../core/shared/item.model'; import { followLink, FollowLinkConfig } from '../shared/utils/follow-link-config.model'; -import { FindListOptions } from '../core/data/request.models'; import { getFirstCompletedRemoteData } from '../core/shared/operators'; import { Store } from '@ngrx/store'; import { ResolvedAction } from '../core/resolving/resolver.actions'; @@ -15,13 +14,13 @@ import { ResolvedAction } from '../core/resolving/resolver.actions'; * Requesting them as embeds will limit the number of requests */ export const ITEM_PAGE_LINKS_TO_FOLLOW: FollowLinkConfig[] = [ - followLink('owningCollection', undefined, true, true, true, - followLink('parentCommunity', undefined, true, true, true, + followLink('owningCollection', {}, + followLink('parentCommunity', {}, followLink('parentCommunity')) ), - followLink('bundles', new FindListOptions(), true, true, true, followLink('bitstreams')), followLink('relationships'), - followLink('version', undefined, true, true, true, followLink('versionhistory')), + followLink('version', {}, followLink('versionhistory')), + followLink('thumbnail') ]; /** diff --git a/src/app/+item-page/simple/item-page.component.ts b/src/app/+item-page/simple/item-page.component.ts index 67e278c2fb..d2c238b5e6 100644 --- a/src/app/+item-page/simple/item-page.component.ts +++ b/src/app/+item-page/simple/item-page.component.ts @@ -8,8 +8,6 @@ import { RemoteData } from '../../core/data/remote-data'; import { Item } from '../../core/shared/item.model'; -import { MetadataService } from '../../core/metadata/metadata.service'; - import { fadeInOut } from '../../shared/animations/fade'; import { getAllSucceededRemoteDataPayload, redirectOn4xx } from '../../core/shared/operators'; import { ViewMode } from '../../core/shared/view-mode.model'; @@ -54,7 +52,6 @@ export class ItemPageComponent implements OnInit { private route: ActivatedRoute, private router: Router, private items: ItemDataService, - private metadataService: MetadataService, private authService: AuthService, ) { } @@ -66,7 +63,6 @@ export class ItemPageComponent implements OnInit { map((data) => data.dso as RemoteData), redirectOn4xx(this.router, this.authService) ); - this.metadataService.processRemoteData(this.itemRD$); this.itemPageRoute$ = this.itemRD$.pipe( getAllSucceededRemoteDataPayload(), map((item) => getItemPageRoute(item)) diff --git a/src/app/+item-page/simple/item-types/publication/publication.component.html b/src/app/+item-page/simple/item-types/publication/publication.component.html index a5282bfa7f..9e61f00e48 100644 --- a/src/app/+item-page/simple/item-types/publication/publication.component.html +++ b/src/app/+item-page/simple/item-types/publication/publication.component.html @@ -10,7 +10,7 @@
- + diff --git a/src/app/+item-page/simple/item-types/shared/item.component.ts b/src/app/+item-page/simple/item-types/shared/item.component.ts index 130f67edc7..793af180c9 100644 --- a/src/app/+item-page/simple/item-types/shared/item.component.ts +++ b/src/app/+item-page/simple/item-types/shared/item.component.ts @@ -1,12 +1,7 @@ import { Component, Input, OnInit } from '@angular/core'; import { environment } from '../../../../../environments/environment'; -import { BitstreamDataService } from '../../../../core/data/bitstream-data.service'; -import { Bitstream } from '../../../../core/shared/bitstream.model'; import { Item } from '../../../../core/shared/item.model'; -import { takeUntilCompletedRemoteData } from '../../../../core/shared/operators'; import { getItemPageRoute } from '../../../item-page-routing-paths'; -import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; -import { RemoteData } from '../../../../core/data/remote-data'; @Component({ selector: 'ds-item', @@ -18,28 +13,14 @@ import { RemoteData } from '../../../../core/data/remote-data'; export class ItemComponent implements OnInit { @Input() object: Item; - /** - * The Item's thumbnail - */ - thumbnail$: BehaviorSubject>; - /** * Route to the item page */ itemPageRoute: string; - mediaViewer = environment.mediaViewer; - constructor(protected bitstreamDataService: BitstreamDataService) { - } + mediaViewer = environment.mediaViewer; ngOnInit(): void { this.itemPageRoute = getItemPageRoute(this.object); - - this.thumbnail$ = new BehaviorSubject>(undefined); - this.bitstreamDataService.getThumbnailFor(this.object).pipe( - takeUntilCompletedRemoteData(), - ).subscribe((rd: RemoteData) => { - this.thumbnail$.next(rd); - }); } } diff --git a/src/app/+item-page/simple/item-types/untyped-item/untyped-item.component.html b/src/app/+item-page/simple/item-types/untyped-item/untyped-item.component.html index bacffbf526..b0157dcfee 100644 --- a/src/app/+item-page/simple/item-types/untyped-item/untyped-item.component.html +++ b/src/app/+item-page/simple/item-types/untyped-item/untyped-item.component.html @@ -10,7 +10,7 @@
- + diff --git a/src/app/+search-page/search.component.ts b/src/app/+search-page/search.component.ts index b817c82a57..c1caf27b9a 100644 --- a/src/app/+search-page/search.component.ts +++ b/src/app/+search-page/search.component.ts @@ -16,9 +16,11 @@ import { SearchResult } from '../shared/search/search-result.model'; import { SearchConfigurationService } from '../core/shared/search/search-configuration.service'; import { SearchService } from '../core/shared/search/search.service'; import { currentPath } from '../shared/utils/route.utils'; -import { Router} from '@angular/router'; +import { Router } from '@angular/router'; import { Context } from '../core/shared/context.model'; import { SortOptions } from '../core/cache/models/sort-options.model'; +import { followLink } from '../shared/utils/follow-link-config.model'; +import { Item } from '../core/shared/item.model'; @Component({ selector: 'ds-search', @@ -128,8 +130,11 @@ export class SearchComponent implements OnInit { this.searchLink = this.getSearchLink(); this.searchOptions$ = this.getSearchOptions(); this.sub = this.searchOptions$.pipe( - switchMap((options) => this.service.search(options).pipe(getFirstSucceededRemoteData(), startWith(undefined)))) - .subscribe((results) => { + switchMap((options) => this.service.search( + options, undefined, true, true, followLink('thumbnail', { isOptional: true }) + ).pipe(getFirstSucceededRemoteData(), startWith(undefined)) + ) + ).subscribe((results) => { this.resultsRD$.next(results); }); this.scopeListRD$ = this.searchConfigService.getCurrentScope('').pipe( 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 b5feffe03a..4a119036bc 100755 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -48,6 +48,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'; import { UUIDService } from './core/shared/uuid.service'; import { CookieService } from './core/services/cookie.service'; @@ -169,6 +170,7 @@ const DECLARATIONS = [ ThemedBreadcrumbsComponent, ForbiddenComponent, ThemedForbiddenComponent, + IdleModalComponent ]; const EXPORTS = [ diff --git a/src/app/community-list-page/community-list-service.ts b/src/app/community-list-page/community-list-service.ts index cbf70ca39a..e882ae5902 100644 --- a/src/app/community-list-page/community-list-service.ts +++ b/src/app/community-list-page/community-list-service.ts @@ -174,8 +174,8 @@ export class CommunityListService { direction: options.sort.direction } }, - followLink('subcommunities', this.configOnePage, true, true), - followLink('collections', this.configOnePage, true, true)) + followLink('subcommunities', { findListOptions: this.configOnePage }), + followLink('collections', { findListOptions: this.configOnePage })) .pipe( getFirstSucceededRemoteData(), map((results) => results.payload), @@ -242,8 +242,8 @@ export class CommunityListService { elementsPerPage: MAX_COMCOLS_PER_PAGE, currentPage: i }, - followLink('subcommunities', this.configOnePage, true, true), - followLink('collections', this.configOnePage, true, true)) + followLink('subcommunities', { findListOptions: this.configOnePage }), + followLink('collections', { findListOptions: this.configOnePage })) .pipe( getFirstCompletedRemoteData(), switchMap((rd: RemoteData>) => { diff --git a/src/app/core/auth/auth.actions.ts b/src/app/core/auth/auth.actions.ts index f80be89034..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 */ @@ -292,10 +294,13 @@ export class ResetAuthenticationMessagesAction implements Action { export class RetrieveAuthMethodsAction implements Action { public type: string = AuthActionTypes.RETRIEVE_AUTH_METHODS; - payload: AuthStatus; + payload: { + status: AuthStatus; + blocking: boolean; + }; - constructor(authStatus: AuthStatus) { - this.payload = authStatus; + constructor(status: AuthStatus, blocking: boolean) { + this.payload = { status, blocking }; } } @@ -306,10 +311,14 @@ export class RetrieveAuthMethodsAction implements Action { */ export class RetrieveAuthMethodsSuccessAction implements Action { public type: string = AuthActionTypes.RETRIEVE_AUTH_METHODS_SUCCESS; - payload: AuthMethod[]; - constructor(authMethods: AuthMethod[] ) { - this.payload = authMethods; + payload: { + authMethods: AuthMethod[]; + blocking: boolean; + }; + + constructor(authMethods: AuthMethod[], blocking: boolean ) { + this.payload = { authMethods, blocking }; } } @@ -320,6 +329,12 @@ export class RetrieveAuthMethodsSuccessAction implements Action { */ export class RetrieveAuthMethodsErrorAction implements Action { public type: string = AuthActionTypes.RETRIEVE_AUTH_METHODS_ERROR; + + payload: boolean; + + constructor(blocking: boolean) { + this.payload = blocking; + } } /** @@ -391,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 */ /** @@ -421,4 +454,7 @@ export type AuthActions | RetrieveAuthenticatedEpersonErrorAction | RetrieveAuthenticatedEpersonSuccessAction | SetRedirectUrlAction - | RedirectAfterLoginSuccessAction; + | RedirectAfterLoginSuccessAction + | SetUserAsIdleAction + | UnsetUserAsIdleAction; + diff --git a/src/app/core/auth/auth.effects.spec.ts b/src/app/core/auth/auth.effects.spec.ts index 5d530f39a9..cd4f456b44 100644 --- a/src/app/core/auth/auth.effects.spec.ts +++ b/src/app/core/auth/auth.effects.spec.ts @@ -43,10 +43,12 @@ describe('AuthEffects', () => { let initialState; let token; let store: MockStore; + let authStatus; function init() { authServiceStub = new AuthServiceStub(); token = authServiceStub.getToken(); + authStatus = Object.assign(new AuthStatus(), {}); initialState = { core: { auth: { @@ -217,16 +219,38 @@ describe('AuthEffects', () => { expect(authEffects.checkTokenCookie$).toBeObservable(expected); }); - it('should return a RETRIEVE_AUTH_METHODS action in response to a CHECK_AUTHENTICATION_TOKEN_COOKIE action when authenticated is false', () => { - spyOn((authEffects as any).authService, 'checkAuthenticationCookie').and.returnValue( - observableOf( - { authenticated: false }) - ); - actions = hot('--a-', { a: { type: AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_COOKIE } }); + describe('on CSR', () => { + it('should return a RETRIEVE_AUTH_METHODS action in response to a CHECK_AUTHENTICATION_TOKEN_COOKIE action when authenticated is false', () => { + spyOn((authEffects as any).authService, 'checkAuthenticationCookie').and.returnValue( + observableOf( + { authenticated: false }) + ); + spyOn((authEffects as any).authService, 'getRetrieveAuthMethodsAction').and.returnValue( + new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus, false) + ); + actions = hot('--a-', { a: { type: AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_COOKIE } }); - const expected = cold('--b-', { b: new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus) }); + const expected = cold('--b-', { b: new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus, false) }); - expect(authEffects.checkTokenCookie$).toBeObservable(expected); + expect(authEffects.checkTokenCookie$).toBeObservable(expected); + }); + }); + + describe('on SSR', () => { + it('should return a RETRIEVE_AUTH_METHODS action in response to a CHECK_AUTHENTICATION_TOKEN_COOKIE action when authenticated is false', () => { + spyOn((authEffects as any).authService, 'checkAuthenticationCookie').and.returnValue( + observableOf( + { authenticated: false }) + ); + spyOn((authEffects as any).authService, 'getRetrieveAuthMethodsAction').and.returnValue( + new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus, true) + ); + actions = hot('--a-', { a: { type: AuthActionTypes.CHECK_AUTHENTICATION_TOKEN_COOKIE } }); + + const expected = cold('--b-', { b: new RetrieveAuthMethodsAction({ authenticated: false } as AuthStatus, true) }); + + expect(authEffects.checkTokenCookie$).toBeObservable(expected); + }); }); }); @@ -359,27 +383,74 @@ describe('AuthEffects', () => { describe('retrieveMethods$', () => { - describe('when retrieve authentication methods succeeded', () => { - it('should return a RETRIEVE_AUTH_METHODS_SUCCESS action in response to a RETRIEVE_AUTH_METHODS action', () => { - actions = hot('--a-', { a: { type: AuthActionTypes.RETRIEVE_AUTH_METHODS } }); + describe('on CSR', () => { + describe('when retrieve authentication methods succeeded', () => { + it('should return a RETRIEVE_AUTH_METHODS_SUCCESS action in response to a RETRIEVE_AUTH_METHODS action', () => { + actions = hot('--a-', { a: + { + type: AuthActionTypes.RETRIEVE_AUTH_METHODS, + payload: { status: authStatus, blocking: false} + } + }); - const expected = cold('--b-', { b: new RetrieveAuthMethodsSuccessAction(authMethodsMock) }); + const expected = cold('--b-', { b: new RetrieveAuthMethodsSuccessAction(authMethodsMock, false) }); - expect(authEffects.retrieveMethods$).toBeObservable(expected); + expect(authEffects.retrieveMethods$).toBeObservable(expected); + }); + }); + + describe('when retrieve authentication methods failed', () => { + it('should return a RETRIEVE_AUTH_METHODS_ERROR action in response to a RETRIEVE_AUTH_METHODS action', () => { + spyOn((authEffects as any).authService, 'retrieveAuthMethodsFromAuthStatus').and.returnValue(observableThrow('')); + + actions = hot('--a-', { a: + { + type: AuthActionTypes.RETRIEVE_AUTH_METHODS, + payload: { status: authStatus, blocking: false} + } + }); + + const expected = cold('--b-', { b: new RetrieveAuthMethodsErrorAction(false) }); + + expect(authEffects.retrieveMethods$).toBeObservable(expected); + }); }); }); - describe('when retrieve authentication methods failed', () => { - it('should return a RETRIEVE_AUTH_METHODS_ERROR action in response to a RETRIEVE_AUTH_METHODS action', () => { - spyOn((authEffects as any).authService, 'retrieveAuthMethodsFromAuthStatus').and.returnValue(observableThrow('')); + describe('on SSR', () => { + describe('when retrieve authentication methods succeeded', () => { + it('should return a RETRIEVE_AUTH_METHODS_SUCCESS action in response to a RETRIEVE_AUTH_METHODS action', () => { + actions = hot('--a-', { a: + { + type: AuthActionTypes.RETRIEVE_AUTH_METHODS, + payload: { status: authStatus, blocking: true} + } + }); - actions = hot('--a-', { a: { type: AuthActionTypes.RETRIEVE_AUTH_METHODS } }); + const expected = cold('--b-', { b: new RetrieveAuthMethodsSuccessAction(authMethodsMock, true) }); - const expected = cold('--b-', { b: new RetrieveAuthMethodsErrorAction() }); + expect(authEffects.retrieveMethods$).toBeObservable(expected); + }); + }); - expect(authEffects.retrieveMethods$).toBeObservable(expected); + describe('when retrieve authentication methods failed', () => { + it('should return a RETRIEVE_AUTH_METHODS_ERROR action in response to a RETRIEVE_AUTH_METHODS action', () => { + spyOn((authEffects as any).authService, 'retrieveAuthMethodsFromAuthStatus').and.returnValue(observableThrow('')); + + actions = hot('--a-', { a: + { + type: AuthActionTypes.RETRIEVE_AUTH_METHODS, + payload: { status: authStatus, blocking: true} + } + }); + + const expected = cold('--b-', { b: new RetrieveAuthMethodsErrorAction(true) }); + + expect(authEffects.retrieveMethods$).toBeObservable(expected); + }); }); }); + }); describe('clearInvalidTokenOnRehydrate$', () => { diff --git a/src/app/core/auth/auth.effects.ts b/src/app/core/auth/auth.effects.ts index 9452af1fb8..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 { @@ -145,7 +161,7 @@ export class AuthEffects { if (response.authenticated) { return new RetrieveTokenAction(); } else { - return new RetrieveAuthMethodsAction(response); + return this.authService.getRetrieveAuthMethodsAction(response); } }), catchError((error) => observableOf(new AuthenticatedErrorAction(error))) @@ -234,21 +250,43 @@ export class AuthEffects { .pipe( ofType(AuthActionTypes.RETRIEVE_AUTH_METHODS), switchMap((action: RetrieveAuthMethodsAction) => { - return this.authService.retrieveAuthMethodsFromAuthStatus(action.payload) + return this.authService.retrieveAuthMethodsFromAuthStatus(action.payload.status) .pipe( - map((authMethodModels: AuthMethod[]) => new RetrieveAuthMethodsSuccessAction(authMethodModels)), - catchError((error) => observableOf(new RetrieveAuthMethodsErrorAction())) + map((authMethodModels: AuthMethod[]) => new RetrieveAuthMethodsSuccessAction(authMethodModels, action.payload.blocking)), + catchError((error) => observableOf(new RetrieveAuthMethodsErrorAction(action.payload.blocking))) ); }) ); + /** + * 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 4c6f1e2a25..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,16 +548,18 @@ describe('authReducer', () => { loaded: false, blocking: false, loading: false, - authMethods: [] + authMethods: [], + idle: false }; - const action = new RetrieveAuthMethodsAction(new AuthStatus()); + const action = new RetrieveAuthMethodsAction(new AuthStatus(), true); const newState = authReducer(initialState, action); state = { authenticated: false, loaded: false, blocking: true, loading: true, - authMethods: [] + authMethods: [], + idle: false }; expect(newState).toEqual(state); }); @@ -530,41 +570,136 @@ describe('authReducer', () => { loaded: false, blocking: true, loading: true, - authMethods: [] + authMethods: [], + idle: false }; const authMethods = [ new AuthMethod(AuthMethodType.Password), new AuthMethod(AuthMethodType.Shibboleth, 'location') ]; - const action = new RetrieveAuthMethodsSuccessAction(authMethods); + const action = new RetrieveAuthMethodsSuccessAction(authMethods, false); const newState = authReducer(initialState, action); state = { authenticated: false, loaded: false, blocking: false, loading: false, - authMethods: authMethods + authMethods: authMethods, + idle: false }; expect(newState).toEqual(state); }); - it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS_ERROR action', () => { + it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS_SUCCESS action with blocking as true', () => { initialState = { authenticated: false, loaded: false, blocking: true, loading: true, - authMethods: [] + authMethods: [], + idle: false + }; + const authMethods = [ + new AuthMethod(AuthMethodType.Password), + new AuthMethod(AuthMethodType.Shibboleth, 'location') + ]; + const action = new RetrieveAuthMethodsSuccessAction(authMethods, true); + const newState = authReducer(initialState, action); + state = { + authenticated: false, + loaded: false, + blocking: true, + loading: false, + authMethods: authMethods, + idle: false + }; + expect(newState).toEqual(state); + }); + + it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS_ERROR action ', () => { + initialState = { + authenticated: false, + loaded: false, + blocking: true, + loading: true, + authMethods: [], + idle: false }; - const action = new RetrieveAuthMethodsErrorAction(); + const action = new RetrieveAuthMethodsErrorAction(false); const newState = authReducer(initialState, action); state = { authenticated: false, 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); + }); + + it('should properly set the state, in response to a RETRIEVE_AUTH_METHODS_ERROR action with blocking as true', () => { + initialState = { + authenticated: false, + loaded: false, + blocking: true, + loading: true, + authMethods: [], + idle: false + }; + + const action = new RetrieveAuthMethodsErrorAction(true); + const newState = authReducer(initialState, action); + state = { + authenticated: false, + loaded: false, + blocking: true, + loading: false, + 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 6d5635f263..ef803503c8 100644 --- a/src/app/core/auth/auth.reducer.ts +++ b/src/app/core/auth/auth.reducer.ts @@ -10,6 +10,7 @@ import { RedirectWhenTokenExpiredAction, RefreshTokenSuccessAction, RetrieveAuthenticatedEpersonSuccessAction, + RetrieveAuthMethodsErrorAction, RetrieveAuthMethodsSuccessAction, SetRedirectUrlAction } from './auth.actions'; @@ -58,6 +59,9 @@ export interface AuthState { // all authentication Methods enabled at the backend authMethods?: AuthMethod[]; + // true when the current user is idle + idle: boolean; + } /** @@ -68,7 +72,8 @@ const initialState: AuthState = { loaded: false, blocking: true, loading: false, - authMethods: [] + authMethods: [], + idle: false }; /** @@ -188,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: @@ -211,14 +217,14 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut case AuthActionTypes.RETRIEVE_AUTH_METHODS_SUCCESS: return Object.assign({}, state, { loading: false, - blocking: false, - authMethods: (action as RetrieveAuthMethodsSuccessAction).payload + blocking: (action as RetrieveAuthMethodsSuccessAction).payload.blocking, + authMethods: (action as RetrieveAuthMethodsSuccessAction).payload.authMethods }); case AuthActionTypes.RETRIEVE_AUTH_METHODS_ERROR: return Object.assign({}, state, { loading: false, - blocking: false, + blocking: (action as RetrieveAuthMethodsErrorAction).payload, authMethods: [new AuthMethod(AuthMethodType.Password)] }); @@ -233,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 fa29f1bc36..a5b5d70704 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -29,13 +29,17 @@ import { getRedirectUrl, isAuthenticated, isAuthenticatedLoaded, + isIdle, isTokenRefreshing } from './selectors'; import { AppState } from '../../app.reducer'; import { - CheckAuthenticationTokenAction, + CheckAuthenticationTokenAction, RefreshTokenAction, ResetAuthenticationMessagesAction, - SetRedirectUrlAction + RetrieveAuthMethodsAction, + SetRedirectUrlAction, + SetUserAsIdleAction, + UnsetUserAsIdleAction } from './auth.actions'; import { NativeWindowRef, NativeWindowService } from '../services/window.service'; import { Base64EncodeUrl } from '../../shared/utils/encode-decode.util'; @@ -45,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'; @@ -63,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, @@ -72,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), @@ -186,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() ); } @@ -297,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; @@ -305,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} @@ -345,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); @@ -395,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); } /** @@ -434,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 : '')); } @@ -518,4 +573,33 @@ export class AuthService { ); } + /** + * Return a new instance of RetrieveAuthMethodsAction + * + * @param authStatus The auth status + */ + getRetrieveAuthMethodsAction(authStatus: AuthStatus): RetrieveAuthMethodsAction { + 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/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 9840b22267..cccc1490f8 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -10,6 +10,7 @@ import { AuthService } from './auth.service'; import { AuthStatus } from './models/auth-status.model'; import { AuthTokenInfo } from './models/auth-token-info.model'; import { RemoteData } from '../data/remote-data'; +import { RetrieveAuthMethodsAction } from './auth.actions'; /** * The auth service. @@ -60,4 +61,13 @@ export class ServerAuthService extends AuthService { map((rd: RemoteData) => Object.assign(new AuthStatus(), rd.payload)) ); } + + /** + * Return a new instance of RetrieveAuthMethodsAction + * + * @param authStatus The auth status + */ + getRetrieveAuthMethodsAction(authStatus: AuthStatus): RetrieveAuthMethodsAction { + return new RetrieveAuthMethodsAction(authStatus, true); + } } diff --git a/src/app/core/cache/builders/link.service.spec.ts b/src/app/core/cache/builders/link.service.spec.ts index a6d9c59492..f567c39314 100644 --- a/src/app/core/cache/builders/link.service.spec.ts +++ b/src/app/core/cache/builders/link.service.spec.ts @@ -102,7 +102,7 @@ describe('LinkService', () => { describe('resolveLink', () => { describe(`when the linkdefinition concerns a single object`, () => { beforeEach(() => { - service.resolveLink(testModel, followLink('predecessor', {}, true, true, true, followLink('successor'))); + service.resolveLink(testModel, followLink('predecessor', {}, followLink('successor'))); }); it('should call dataservice.findByHref with the correct href and nested links', () => { expect(testDataService.findByHref).toHaveBeenCalledWith(testModel._links.predecessor.href, true, true, followLink('successor')); @@ -116,7 +116,7 @@ describe('LinkService', () => { propertyName: 'predecessor', isList: true }); - service.resolveLink(testModel, followLink('predecessor', { some: 'options ' } as any, true, true, true, followLink('successor'))); + service.resolveLink(testModel, followLink('predecessor', { findListOptions: { some: 'options ' } as any }, followLink('successor'))); }); it('should call dataservice.findAllByHref with the correct href, findListOptions, and nested links', () => { expect(testDataService.findAllByHref).toHaveBeenCalledWith(testModel._links.predecessor.href, { some: 'options ' } as any, true, true, followLink('successor')); @@ -124,7 +124,7 @@ describe('LinkService', () => { }); describe('either way', () => { beforeEach(() => { - result = service.resolveLink(testModel, followLink('predecessor', {}, true, true, true, followLink('successor'))); + result = service.resolveLink(testModel, followLink('predecessor', {}, followLink('successor'))); }); it('should call getLinkDefinition with the correct model and link', () => { @@ -149,7 +149,7 @@ describe('LinkService', () => { }); it('should throw an error', () => { expect(() => { - service.resolveLink(testModel, followLink('predecessor', {}, true, true, true, followLink('successor'))); + service.resolveLink(testModel, followLink('predecessor', {}, followLink('successor'))); }).toThrow(); }); }); @@ -160,7 +160,7 @@ describe('LinkService', () => { }); it('should throw an error', () => { expect(() => { - service.resolveLink(testModel, followLink('predecessor', {}, true, true, true, followLink('successor'))); + service.resolveLink(testModel, followLink('predecessor', {}, followLink('successor'))); }).toThrow(); }); }); diff --git a/src/app/core/cache/builders/link.service.ts b/src/app/core/cache/builders/link.service.ts index 29f8633da5..66f91dbbd6 100644 --- a/src/app/core/cache/builders/link.service.ts +++ b/src/app/core/cache/builders/link.service.ts @@ -39,7 +39,7 @@ export class LinkService { */ public resolveLinks(model: T, ...linksToFollow: FollowLinkConfig[]): T { linksToFollow.forEach((linkToFollow: FollowLinkConfig) => { - this.resolveLink(model, linkToFollow); + this.resolveLink(model, linkToFollow); }); return model; } @@ -55,9 +55,7 @@ export class LinkService { public resolveLinkWithoutAttaching(model, linkToFollow: FollowLinkConfig): Observable> { const matchingLinkDef = this.getLinkDefinition(model.constructor, linkToFollow.name); - if (hasNoValue(matchingLinkDef)) { - throw new Error(`followLink('${linkToFollow.name}') was used for a ${model.constructor.name}, but there is no property on ${model.constructor.name} models with an @link() for ${linkToFollow.name}`); - } else { + if (hasValue(matchingLinkDef)) { const provider = this.getDataServiceFor(matchingLinkDef.resourceType); if (hasNoValue(provider)) { @@ -84,7 +82,10 @@ export class LinkService { throw e; } } + } else if (!linkToFollow.isOptional) { + throw new Error(`followLink('${linkToFollow.name}') was used as a required link for a ${model.constructor.name}, but there is no property on ${model.constructor.name} models with an @link() for ${linkToFollow.name}`); } + return EMPTY; } diff --git a/src/app/core/cache/builders/remote-data-build.service.spec.ts b/src/app/core/cache/builders/remote-data-build.service.spec.ts index cb193724a7..0cb45733a6 100644 --- a/src/app/core/cache/builders/remote-data-build.service.spec.ts +++ b/src/app/core/cache/builders/remote-data-build.service.spec.ts @@ -523,7 +523,7 @@ describe('RemoteDataBuildService', () => { let paginatedLinksToFollow; beforeEach(() => { paginatedLinksToFollow = [ - followLink('page', undefined, true, true, true, ...linksToFollow), + followLink('page', {}, ...linksToFollow), ...linksToFollow ]; }); diff --git a/src/app/core/cache/builders/remote-data-build.service.ts b/src/app/core/cache/builders/remote-data-build.service.ts index 11815c133b..6b67549f2d 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -271,7 +271,7 @@ export class RemoteDataBuildService { * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ buildList(href$: string | Observable, ...linksToFollow: FollowLinkConfig[]): Observable>> { - return this.buildFromHref>(href$, followLink('page', undefined, false, true, true, ...linksToFollow)); + return this.buildFromHref>(href$, followLink('page', { shouldEmbed: false }, ...linksToFollow)); } /** diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index 615c2b3977..3c34e5ec35 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -162,6 +162,7 @@ import { UsageReport } from './statistics/models/usage-report.model'; import { RootDataService } from './data/root-data.service'; import { Root } from './data/root.model'; import { SearchConfig } from './shared/search/search-filters/search-config.model'; +import { SequenceService } from './shared/sequence.service'; /** * When not in production, endpoint responses can be mocked for testing purposes @@ -282,7 +283,8 @@ const PROVIDERS = [ FilteredDiscoveryPageResponseParsingService, { provide: NativeWindowService, useFactory: NativeWindowFactory }, VocabularyService, - VocabularyTreeviewService + VocabularyTreeviewService, + SequenceService, ]; /** 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/data/bitstream-data.service.ts b/src/app/core/data/bitstream-data.service.ts index 1a16abc47f..3890f4e663 100644 --- a/src/app/core/data/bitstream-data.service.ts +++ b/src/app/core/data/bitstream-data.service.ts @@ -3,7 +3,7 @@ import { Injectable } from '@angular/core'; import { Store } from '@ngrx/store'; import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; import { map, switchMap, take } from 'rxjs/operators'; -import { hasValue, isNotEmpty } from '../../shared/empty.util'; +import { hasValue } from '../../shared/empty.util'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; import { dataService } from '../cache/builders/build-decorators'; @@ -18,7 +18,7 @@ import { Item } from '../shared/item.model'; import { BundleDataService } from './bundle-data.service'; import { DataService } from './data.service'; import { DSOChangeAnalyzer } from './dso-change-analyzer.service'; -import { PaginatedList, buildPaginatedList } from './paginated-list.model'; +import { buildPaginatedList, PaginatedList } from './paginated-list.model'; import { RemoteData } from './remote-data'; import { FindListOptions, PutRequest } from './request.models'; import { RequestService } from './request.service'; @@ -28,7 +28,6 @@ import { HttpOptions } from '../dspace-rest/dspace-rest.service'; import { sendRequest } from '../shared/operators'; import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { PageInfo } from '../shared/page-info.model'; -import { RequestEntryState } from './request.reducer'; /** * A service to retrieve {@link Bitstream}s from the REST API @@ -75,92 +74,6 @@ export class BitstreamDataService extends DataService { return this.findAllByHref(bundle._links.bitstreams.href, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } - /** - * Retrieves the thumbnail for the given item - * @returns {Observable>} the first bitstream in the THUMBNAIL bundle - */ - // TODO should be implemented rest side. {@link Item} should get a thumbnail link - public getThumbnailFor(item: Item): Observable> { - return this.bundleService.findByItemAndName(item, 'THUMBNAIL').pipe( - switchMap((bundleRD: RemoteData) => { - if (isNotEmpty(bundleRD.payload)) { - return this.findAllByBundle(bundleRD.payload, { elementsPerPage: 1 }).pipe( - map((bitstreamRD: RemoteData>) => { - if (hasValue(bitstreamRD.payload) && hasValue(bitstreamRD.payload.page)) { - return new RemoteData( - bitstreamRD.timeCompleted, - bitstreamRD.msToLive, - bitstreamRD.lastUpdated, - bitstreamRD.state, - bitstreamRD.errorMessage, - bitstreamRD.payload.page[0], - bitstreamRD.statusCode - ); - } else { - return bitstreamRD as any; - } - }) - ); - } else { - return [bundleRD as any]; - } - }) - ); - } - - /** - * Retrieve the matching thumbnail for a {@link Bitstream}. - * - * The {@link Item} is technically redundant, but is available - * in all current use cases, and having it simplifies this method - * - * @param item The {@link Item} the {@link Bitstream} and its thumbnail are a part of - * @param bitstreamInOriginal The original {@link Bitstream} to find the thumbnail for - */ - // TODO should be implemented rest side - public getMatchingThumbnail(item: Item, bitstreamInOriginal: Bitstream): Observable> { - return this.bundleService.findByItemAndName(item, 'THUMBNAIL').pipe( - switchMap((bundleRD: RemoteData) => { - if (isNotEmpty(bundleRD.payload)) { - return this.findAllByBundle(bundleRD.payload, { elementsPerPage: 9999 }).pipe( - map((bitstreamRD: RemoteData>) => { - if (hasValue(bitstreamRD.payload) && hasValue(bitstreamRD.payload.page)) { - const matchingThumbnail = bitstreamRD.payload.page.find((thumbnail: Bitstream) => - thumbnail.name.startsWith(bitstreamInOriginal.name) - ); - if (hasValue(matchingThumbnail)) { - return new RemoteData( - bitstreamRD.timeCompleted, - bitstreamRD.msToLive, - bitstreamRD.lastUpdated, - bitstreamRD.state, - bitstreamRD.errorMessage, - matchingThumbnail, - bitstreamRD.statusCode - ); - } else { - return new RemoteData( - bitstreamRD.timeCompleted, - bitstreamRD.msToLive, - bitstreamRD.lastUpdated, - RequestEntryState.Error, - 'No matching thumbnail found', - undefined, - 404 - ); - } - } else { - return bitstreamRD as any; - } - }) - ); - } else { - return [bundleRD as any]; - } - }) - ); - } - /** * Retrieve all {@link Bitstream}s in a certain {@link Bundle}. * diff --git a/src/app/core/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index 88b15754af..5bc7423824 100644 --- a/src/app/core/data/data.service.spec.ts +++ b/src/app/core/data/data.service.spec.ts @@ -233,7 +233,7 @@ describe('DataService', () => { const config: FindListOptions = Object.assign(new FindListOptions(), { elementsPerPage: 5 }); - (service as any).getFindAllHref({}, null, followLink('bundles', config, true, true, true)).subscribe((value) => { + (service as any).getFindAllHref({}, null, followLink('bundles', { findListOptions: config })).subscribe((value) => { expect(value).toBe(expected); }); }); @@ -253,7 +253,7 @@ describe('DataService', () => { elementsPerPage: 2 }); - (service as any).getFindAllHref({}, null, followLink('bundles'), followLink('owningCollection', config, true, true, true), followLink('templateItemOf')).subscribe((value) => { + (service as any).getFindAllHref({}, null, followLink('bundles'), followLink('owningCollection', { findListOptions: config }), followLink('templateItemOf')).subscribe((value) => { expect(value).toBe(expected); }); }); @@ -261,7 +261,13 @@ describe('DataService', () => { it('should not include linksToFollow with shouldEmbed = false', () => { const expected = `${endpoint}?embed=templateItemOf`; - (service as any).getFindAllHref({}, null, followLink('bundles', undefined, false), followLink('owningCollection', undefined, false), followLink('templateItemOf')).subscribe((value) => { + (service as any).getFindAllHref( + {}, + null, + followLink('bundles', { shouldEmbed: false }), + followLink('owningCollection', { shouldEmbed: false }), + followLink('templateItemOf') + ).subscribe((value) => { expect(value).toBe(expected); }); }); @@ -269,7 +275,7 @@ describe('DataService', () => { it('should include nested linksToFollow 3lvl', () => { const expected = `${endpoint}?embed=owningCollection/itemtemplate/relationships`; - (service as any).getFindAllHref({}, null, followLink('owningCollection', undefined, true, true, true, followLink('itemtemplate', undefined, true, true, true, followLink('relationships')))).subscribe((value) => { + (service as any).getFindAllHref({}, null, followLink('owningCollection', {}, followLink('itemtemplate', {}, followLink('relationships')))).subscribe((value) => { expect(value).toBe(expected); }); }); @@ -279,7 +285,7 @@ describe('DataService', () => { const config: FindListOptions = Object.assign(new FindListOptions(), { elementsPerPage: 4 }); - (service as any).getFindAllHref({}, null, followLink('owningCollection', undefined, true, true, true, followLink('itemtemplate', config, true, true, true))).subscribe((value) => { + (service as any).getFindAllHref({}, null, followLink('owningCollection', {}, followLink('itemtemplate', { findListOptions: config }))).subscribe((value) => { expect(value).toBe(expected); }); }); @@ -308,13 +314,19 @@ describe('DataService', () => { it('should not include linksToFollow with shouldEmbed = false', () => { const expected = `${endpointMock}/${resourceIdMock}?embed=templateItemOf`; - const result = (service as any).getIDHref(endpointMock, resourceIdMock, followLink('bundles', undefined, false), followLink('owningCollection', undefined, false), followLink('templateItemOf')); + const result = (service as any).getIDHref( + endpointMock, + resourceIdMock, + followLink('bundles', { shouldEmbed: false }), + followLink('owningCollection', { shouldEmbed: false }), + followLink('templateItemOf') + ); expect(result).toEqual(expected); }); it('should include nested linksToFollow 3lvl', () => { const expected = `${endpointMock}/${resourceIdMock}?embed=owningCollection/itemtemplate/relationships`; - const result = (service as any).getIDHref(endpointMock, resourceIdMock, followLink('owningCollection', undefined, true, true, true,followLink('itemtemplate', undefined, true, true, true, followLink('relationships')))); + const result = (service as any).getIDHref(endpointMock, resourceIdMock, followLink('owningCollection', {}, followLink('itemtemplate', {}, followLink('relationships')))); expect(result).toEqual(expected); }); }); diff --git a/src/app/core/data/dso-redirect-data.service.spec.ts b/src/app/core/data/dso-redirect-data.service.spec.ts index d64f37ad78..bcd25487c2 100644 --- a/src/app/core/data/dso-redirect-data.service.spec.ts +++ b/src/app/core/data/dso-redirect-data.service.spec.ts @@ -119,7 +119,7 @@ describe('DsoRedirectDataService', () => { }); it('should navigate to entities route with the corresponding entity type', () => { remoteData.payload.type = 'item'; - remoteData.payload.metadata = { + remoteData.payload.metadata = { 'dspace.entity.type': [ { language: 'en_US', @@ -174,13 +174,29 @@ describe('DsoRedirectDataService', () => { it('should not include linksToFollow with shouldEmbed = false', () => { const expected = `${requestUUIDURL}&embed=templateItemOf`; - const result = (service as any).getIDHref(pidLink, dsoUUID, followLink('bundles', undefined, false), followLink('owningCollection', undefined, false), followLink('templateItemOf')); + const result = (service as any).getIDHref( + pidLink, + dsoUUID, + followLink('bundles', { shouldEmbed: false }), + followLink('owningCollection', { shouldEmbed: false }), + followLink('templateItemOf') + ); expect(result).toEqual(expected); }); it('should include nested linksToFollow 3lvl', () => { const expected = `${requestUUIDURL}&embed=owningCollection/itemtemplate/relationships`; - const result = (service as any).getIDHref(pidLink, dsoUUID, followLink('owningCollection', undefined, true, true, true, followLink('itemtemplate', undefined, true, true, true, followLink('relationships')))); + const result = (service as any).getIDHref( + pidLink, + dsoUUID, + followLink('owningCollection', + {}, + followLink('itemtemplate', + {}, + followLink('relationships') + ) + ) + ); expect(result).toEqual(expected); }); }); diff --git a/src/app/core/data/item-data.service.ts b/src/app/core/data/item-data.service.ts index e56f9f2b0c..7a0116fe86 100644 --- a/src/app/core/data/item-data.service.ts +++ b/src/app/core/data/item-data.service.ts @@ -23,14 +23,7 @@ import { DataService } from './data.service'; import { DSOChangeAnalyzer } from './dso-change-analyzer.service'; import { PaginatedList } from './paginated-list.model'; import { RemoteData } from './remote-data'; -import { - DeleteRequest, - FindListOptions, - GetRequest, - PostRequest, - PutRequest, - RestRequest -} from './request.models'; +import { DeleteRequest, FindListOptions, GetRequest, PostRequest, PutRequest, RestRequest } from './request.models'; import { RequestService } from './request.service'; import { PaginatedSearchOptions } from '../../shared/search/paginated-search-options.model'; import { Bundle } from '../shared/bundle.model'; @@ -38,6 +31,9 @@ import { MetadataMap } from '../shared/metadata.models'; import { BundleDataService } from './bundle-data.service'; import { Operation } from 'fast-json-patch'; import { NoContent } from '../shared/NoContent.model'; +import { GenericConstructor } from '../shared/generic-constructor'; +import { ResponseParsingService } from './parsing.service'; +import { StatusCodeOnlyResponseParsingService } from './status-code-only-response-parsing.service'; @Injectable() @dataService(ITEM) @@ -229,7 +225,7 @@ export class ItemDataService extends DataService { * @param itemId * @param collection */ - public moveToCollection(itemId: string, collection: Collection): Observable> { + public moveToCollection(itemId: string, collection: Collection): Observable> { const options: HttpOptions = Object.create({}); let headers = new HttpHeaders(); headers = headers.append('Content-Type', 'text/uri-list'); @@ -242,9 +238,17 @@ export class ItemDataService extends DataService { find((href: string) => hasValue(href)), map((href: string) => { const request = new PutRequest(requestId, href, collection._links.self.href, options); - this.requestService.send(request); + Object.assign(request, { + // TODO: for now, the move Item endpoint returns a malformed collection -- only look at the status code + getResponseParser(): GenericConstructor { + return StatusCodeOnlyResponseParsingService; + } + }); + return request; }) - ).subscribe(); + ).subscribe((request) => { + this.requestService.send(request); + }); return this.rdbService.buildFromRequestUUID(requestId); } diff --git a/src/app/core/json-patch/builder/json-patch-operations-builder.ts b/src/app/core/json-patch/builder/json-patch-operations-builder.ts index ced3750834..d3896c4a6c 100644 --- a/src/app/core/json-patch/builder/json-patch-operations-builder.ts +++ b/src/app/core/json-patch/builder/json-patch-operations-builder.ts @@ -9,7 +9,7 @@ import { import { JsonPatchOperationPathObject } from './json-patch-operation-path-combiner'; import { Injectable } from '@angular/core'; import { hasNoValue, hasValue, isEmpty, isNotEmpty } from '../../../shared/empty.util'; -import { dateToISOFormat } from '../../../shared/date.util'; +import { dateToISOFormat, dateToString, isNgbDateStruct } from '../../../shared/date.util'; import { VocabularyEntry } from '../../submission/vocabularies/models/vocabulary-entry.model'; import { FormFieldMetadataValueObject } from '../../../shared/form/builder/models/form-field-metadata-value.model'; import { FormFieldLanguageValueObject } from '../../../shared/form/builder/models/form-field-language-value.model'; @@ -136,6 +136,8 @@ export class JsonPatchOperationsBuilder { operationValue = new FormFieldMetadataValueObject(value.value, value.language); } else if (value.hasOwnProperty('authority')) { operationValue = new FormFieldMetadataValueObject(value.value, value.language, value.authority); + } else if (isNgbDateStruct(value)) { + operationValue = new FormFieldMetadataValueObject(dateToString(value)); } else if (value.hasOwnProperty('value')) { operationValue = new FormFieldMetadataValueObject(value.value); } else { diff --git a/src/app/core/json-patch/json-patch-operations.actions.ts b/src/app/core/json-patch/json-patch-operations.actions.ts index 4c26703472..91fc6cb0da 100644 --- a/src/app/core/json-patch/json-patch-operations.actions.ts +++ b/src/app/core/json-patch/json-patch-operations.actions.ts @@ -20,6 +20,7 @@ export const JsonPatchOperationsActionTypes = { ROLLBACK_JSON_PATCH_OPERATIONS: type('dspace/core/patch/ROLLBACK_JSON_PATCH_OPERATIONS'), FLUSH_JSON_PATCH_OPERATIONS: type('dspace/core/patch/FLUSH_JSON_PATCH_OPERATIONS'), START_TRANSACTION_JSON_PATCH_OPERATIONS: type('dspace/core/patch/START_TRANSACTION_JSON_PATCH_OPERATIONS'), + DELETE_PENDING_JSON_PATCH_OPERATIONS: type('dspace/core/patch/DELETE_PENDING_JSON_PATCH_OPERATIONS'), }; /* tslint:disable:max-classes-per-file */ @@ -261,6 +262,13 @@ export class NewPatchReplaceOperationAction implements Action { } } +/** + * An ngrx action to delete all pending JSON Patch Operations. + */ +export class DeletePendingJsonPatchOperationsAction implements Action { + type = JsonPatchOperationsActionTypes.DELETE_PENDING_JSON_PATCH_OPERATIONS; +} + /* tslint:enable:max-classes-per-file */ /** @@ -276,4 +284,5 @@ export type PatchOperationsActions | NewPatchRemoveOperationAction | NewPatchReplaceOperationAction | RollbacktPatchOperationsAction - | StartTransactionPatchOperationsAction; + | StartTransactionPatchOperationsAction + | DeletePendingJsonPatchOperationsAction; diff --git a/src/app/core/json-patch/json-patch-operations.reducer.spec.ts b/src/app/core/json-patch/json-patch-operations.reducer.spec.ts index 34378b819b..1f98cf0920 100644 --- a/src/app/core/json-patch/json-patch-operations.reducer.spec.ts +++ b/src/app/core/json-patch/json-patch-operations.reducer.spec.ts @@ -1,7 +1,7 @@ import * as deepFreeze from 'deep-freeze'; import { - CommitPatchOperationsAction, + CommitPatchOperationsAction, DeletePendingJsonPatchOperationsAction, FlushPatchOperationsAction, NewPatchAddOperationAction, NewPatchRemoveOperationAction, @@ -323,4 +323,19 @@ describe('jsonPatchOperationsReducer test suite', () => { }); + describe('When DeletePendingJsonPatchOperationsAction has been dispatched', () => { + it('should set set the JsonPatchOperationsState to null ', () => { + const action = new DeletePendingJsonPatchOperationsAction(); + initState = Object.assign({}, testState, { + [testJsonPatchResourceType]: Object.assign({}, testState[testJsonPatchResourceType], { + transactionStartTime: startTimestamp, + commitPending: true + }) + }); + const newState = jsonPatchOperationsReducer(initState, action); + + expect(newState).toBeNull(); + }); + }); + }); diff --git a/src/app/core/json-patch/json-patch-operations.reducer.ts b/src/app/core/json-patch/json-patch-operations.reducer.ts index 8d630dbfa1..5e00027edb 100644 --- a/src/app/core/json-patch/json-patch-operations.reducer.ts +++ b/src/app/core/json-patch/json-patch-operations.reducer.ts @@ -11,7 +11,8 @@ import { NewPatchReplaceOperationAction, CommitPatchOperationsAction, StartTransactionPatchOperationsAction, - RollbacktPatchOperationsAction + RollbacktPatchOperationsAction, + DeletePendingJsonPatchOperationsAction } from './json-patch-operations.actions'; import { JsonPatchOperationModel, JsonPatchOperationType } from './json-patch.model'; @@ -101,6 +102,10 @@ export function jsonPatchOperationsReducer(state = initialState, action: PatchOp return startTransactionPatchOperations(state, action as StartTransactionPatchOperationsAction); } + case JsonPatchOperationsActionTypes.DELETE_PENDING_JSON_PATCH_OPERATIONS: { + return deletePendingOperations(state, action as DeletePendingJsonPatchOperationsAction); + } + default: { return state; } @@ -178,6 +183,20 @@ function rollbackOperations(state: JsonPatchOperationsState, action: RollbacktPa } } +/** + * Set the JsonPatchOperationsState to its initial value. + * + * @param state + * the current state + * @param action + * an DeletePendingJsonPatchOperationsAction + * @return JsonPatchOperationsState + * the new state. + */ +function deletePendingOperations(state: JsonPatchOperationsState, action: DeletePendingJsonPatchOperationsAction): JsonPatchOperationsState { + return null; +} + /** * Add new JSON patch operation list. * diff --git a/src/app/core/json-patch/json-patch-operations.service.spec.ts b/src/app/core/json-patch/json-patch-operations.service.spec.ts index 97aba52e56..d1b2948777 100644 --- a/src/app/core/json-patch/json-patch-operations.service.spec.ts +++ b/src/app/core/json-patch/json-patch-operations.service.spec.ts @@ -17,6 +17,7 @@ import { HALEndpointService } from '../shared/hal-endpoint.service'; import { JsonPatchOperationsEntry, JsonPatchOperationsResourceEntry } from './json-patch-operations.reducer'; import { CommitPatchOperationsAction, + DeletePendingJsonPatchOperationsAction, RollbacktPatchOperationsAction, StartTransactionPatchOperationsAction } from './json-patch-operations.actions'; @@ -288,4 +289,19 @@ describe('JsonPatchOperationsService test suite', () => { }); }); + describe('deletePendingJsonPatchOperations', () => { + beforeEach(() => { + store.dispatch.and.callFake(() => { /* */ }); + }); + + it('should dispatch a new DeletePendingJsonPatchOperationsAction', () => { + + const expectedAction = new DeletePendingJsonPatchOperationsAction(); + scheduler.schedule(() => service.deletePendingJsonPatchOperations()); + scheduler.flush(); + + expect(store.dispatch).toHaveBeenCalledWith(expectedAction); + }); + }); + }); diff --git a/src/app/core/json-patch/json-patch-operations.service.ts b/src/app/core/json-patch/json-patch-operations.service.ts index 84d946daba..c3363f4db4 100644 --- a/src/app/core/json-patch/json-patch-operations.service.ts +++ b/src/app/core/json-patch/json-patch-operations.service.ts @@ -10,7 +10,7 @@ import { CoreState } from '../core.reducers'; import { jsonPatchOperationsByResourceType } from './selectors'; import { JsonPatchOperationsResourceEntry } from './json-patch-operations.reducer'; import { - CommitPatchOperationsAction, + CommitPatchOperationsAction, DeletePendingJsonPatchOperationsAction, RollbacktPatchOperationsAction, StartTransactionPatchOperationsAction } from './json-patch-operations.actions'; @@ -105,6 +105,13 @@ export abstract class JsonPatchOperationsService { + 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/shared/bitstream.model.ts b/src/app/core/shared/bitstream.model.ts index 314818b482..baf2f82635 100644 --- a/src/app/core/shared/bitstream.model.ts +++ b/src/app/core/shared/bitstream.model.ts @@ -43,13 +43,14 @@ export class Bitstream extends DSpaceObject implements HALResource { bundle: HALLink; format: HALLink; content: HALLink; + thumbnail: HALLink; }; /** * The thumbnail for this Bitstream - * Needs to be resolved first, but isn't available as a {@link HALLink} yet - * Use BitstreamDataService.getThumbnailFor(…) for now. + * Will be undefined unless the thumbnail {@link HALLink} has been resolved. */ + @link(BITSTREAM, false, 'thumbnail') thumbnail?: Observable>; /** diff --git a/src/app/core/shared/item.model.ts b/src/app/core/shared/item.model.ts index 53eb5e3ce2..d98c22225e 100644 --- a/src/app/core/shared/item.model.ts +++ b/src/app/core/shared/item.model.ts @@ -1,4 +1,4 @@ -import { autoserialize, autoserializeAs, deserialize, inheritSerialization } from 'cerialize'; +import { autoserialize, autoserializeAs, deserialize, deserializeAs, inheritSerialization } from 'cerialize'; import { Observable } from 'rxjs'; import { isEmpty } from '../../shared/empty.util'; import { ListableObject } from '../../shared/object-collection/shared/listable-object.model'; @@ -19,6 +19,8 @@ import { ITEM } from './item.resource-type'; import { ChildHALResource } from './child-hal-resource.model'; import { Version } from './version.model'; import { VERSION } from './version.resource-type'; +import { BITSTREAM } from './bitstream.resource-type'; +import { Bitstream } from './bitstream.model'; /** * Class representing a DSpace Item @@ -37,7 +39,7 @@ export class Item extends DSpaceObject implements ChildHALResource { /** * The Date of the last modification of this Item */ - @deserialize + @deserializeAs(Date) lastModified: Date; /** @@ -69,6 +71,7 @@ export class Item extends DSpaceObject implements ChildHALResource { owningCollection: HALLink; templateItemOf: HALLink; version: HALLink; + thumbnail: HALLink; self: HALLink; }; @@ -100,6 +103,13 @@ export class Item extends DSpaceObject implements ChildHALResource { @link(RELATIONSHIP, true) relationships?: Observable>>; + /** + * The thumbnail for this Item + * Will be undefined unless the thumbnail {@link HALLink} has been resolved. + */ + @link(BITSTREAM, false, 'thumbnail') + thumbnail?: Observable>; + /** * Method that returns as which type of object this object should be rendered */ diff --git a/src/app/core/shared/search/search.service.ts b/src/app/core/shared/search/search.service.ts index 9cb284db32..75723366bc 100644 --- a/src/app/core/shared/search/search.service.ts +++ b/src/app/core/shared/search/search.service.ts @@ -41,6 +41,43 @@ import { SearchConfig } from './search-filters/search-config.model'; import { PaginationService } from '../../pagination/pagination.service'; import { SearchConfigurationService } from './search-configuration.service'; import { PaginationComponentOptions } from '../../../shared/pagination/pagination-component-options.model'; +import { DataService } from '../../data/data.service'; +import { Store } from '@ngrx/store'; +import { CoreState } from '../../core.reducers'; +import { ObjectCacheService } from '../../cache/object-cache.service'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { HttpClient } from '@angular/common/http'; +import { DSOChangeAnalyzer } from '../../data/dso-change-analyzer.service'; + +/* tslint:disable:max-classes-per-file */ +/** + * A class that lets us delegate some methods to DataService + */ +class DataServiceImpl extends DataService { + protected linkPath = 'discover'; + + constructor( + protected requestService: RequestService, + protected rdbService: RemoteDataBuildService, + protected store: Store, + protected objectCache: ObjectCacheService, + protected halService: HALEndpointService, + protected notificationsService: NotificationsService, + protected http: HttpClient, + protected comparator: DSOChangeAnalyzer) { + super(); + } + + /** + * Adds the embed options to the link for the request + * @param href The href the params are to be added to + * @param args params for the query string + * @param linksToFollow links we want to embed in query string if shouldEmbed is true + */ + public addEmbedParams(href: string, args: string[], ...linksToFollow: FollowLinkConfig[]) { + return super.addEmbedParams(href, args, ...linksToFollow); + } +} /** * Service that performs all general actions that have to do with the search page @@ -78,6 +115,11 @@ export class SearchService implements OnDestroy { */ private sub; + /** + * Instance of DataServiceImpl that lets us delegate some methods to DataService + */ + private searchDataService: DataServiceImpl; + constructor(private router: Router, private routeService: RouteService, protected requestService: RequestService, @@ -89,6 +131,16 @@ export class SearchService implements OnDestroy { private paginationService: PaginationService, private searchConfigurationService: SearchConfigurationService ) { + this.searchDataService = new DataServiceImpl( + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined + ); } /** @@ -131,7 +183,17 @@ export class SearchService implements OnDestroy { search(searchOptions?: PaginatedSearchOptions, responseMsToLive?: number, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig[]): Observable>> { const href$ = this.getEndpoint(searchOptions); - href$.pipe(take(1)).subscribe((url: string) => { + href$.pipe( + take(1), + map((href: string) => { + const args = this.searchDataService.addEmbedParams(href, [], ...linksToFollow); + if (isNotEmpty(args)) { + return new URLCombiner(href, `?${args.join('&')}`).toString(); + } else { + return href; + } + }) + ).subscribe((url: string) => { const request = new this.request(this.requestService.generateRequestId(), url); const getResponseParserFn: () => GenericConstructor = () => { @@ -152,7 +214,7 @@ export class SearchService implements OnDestroy { ); return this.directlyAttachIndexableObjects(sqr$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); - } + } /** * Method to retrieve request entries for search results from the server @@ -399,9 +461,9 @@ export class SearchService implements OnDestroy { let pageParams = { page: 1 }; const queryParams = { view: viewMode }; if (viewMode === ViewMode.DetailedListElement) { - pageParams = Object.assign(pageParams, {pageSize: 1}); + pageParams = Object.assign(pageParams, { pageSize: 1 }); } else if (config.pageSize === 1) { - pageParams = Object.assign(pageParams, {pageSize: 10}); + pageParams = Object.assign(pageParams, { pageSize: 10 }); } this.paginationService.updateRouteWithUrl(this.searchConfigurationService.paginationID, hasValue(searchLinkParts) ? searchLinkParts : [this.getSearchLink()], pageParams, queryParams); }); @@ -413,7 +475,7 @@ export class SearchService implements OnDestroy { * @param {string} configurationName the name of the configuration * @returns {Observable>} The found configuration */ - getSearchConfigurationFor(scope?: string, configurationName?: string ): Observable> { + getSearchConfigurationFor(scope?: string, configurationName?: string): Observable> { const href$ = this.halService.getEndpoint(this.configurationLinkPath).pipe( map((url: string) => this.getConfigUrl(url, scope, configurationName)), ); diff --git a/src/app/core/shared/sequence.service.spec.ts b/src/app/core/shared/sequence.service.spec.ts new file mode 100644 index 0000000000..e48ad3efcc --- /dev/null +++ b/src/app/core/shared/sequence.service.spec.ts @@ -0,0 +1,22 @@ +/** + * 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 { SequenceService } from './sequence.service'; + +let service: SequenceService; + +describe('SequenceService', () => { + beforeEach(() => { + service = new SequenceService(); + }); + + it('should return sequential numbers on next(), starting with 1', () => { + const NUMBERS = [1,2,3,4,5]; + const sequence = NUMBERS.map(() => service.next()); + expect(sequence).toEqual(NUMBERS); + }); +}); diff --git a/src/app/core/shared/sequence.service.ts b/src/app/core/shared/sequence.service.ts new file mode 100644 index 0000000000..2340ffb259 --- /dev/null +++ b/src/app/core/shared/sequence.service.ts @@ -0,0 +1,24 @@ +/** + * 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 { Injectable } from '@angular/core'; + +@Injectable() +/** + * Provides unique sequential numbers + */ +export class SequenceService { + private value: number; + + constructor() { + this.value = 0; + } + + public next(): number { + return ++this.value; + } +} diff --git a/src/app/core/submission/vocabularies/vocabulary.service.ts b/src/app/core/submission/vocabularies/vocabulary.service.ts index d82ef01087..da58512441 100644 --- a/src/app/core/submission/vocabularies/vocabulary.service.ts +++ b/src/app/core/submission/vocabularies/vocabulary.service.ts @@ -173,7 +173,7 @@ export class VocabularyService { ); // TODO remove false for the entries embed when https://github.com/DSpace/DSpace/issues/3096 is solved - return this.findVocabularyById(vocabularyOptions.name, true, true, followLink('entries', options, false)).pipe( + return this.findVocabularyById(vocabularyOptions.name, true, true, followLink('entries', { findListOptions: options, shouldEmbed: false })).pipe( getFirstSucceededRemoteDataPayload(), switchMap((vocabulary: Vocabulary) => vocabulary.entries), ); @@ -200,7 +200,7 @@ export class VocabularyService { ); // TODO remove false for the entries embed when https://github.com/DSpace/DSpace/issues/3096 is solved - return this.findVocabularyById(vocabularyOptions.name, true, true, followLink('entries', options, false)).pipe( + return this.findVocabularyById(vocabularyOptions.name, true, true, followLink('entries', { findListOptions: options, shouldEmbed: false })).pipe( getFirstSucceededRemoteDataPayload(), switchMap((vocabulary: Vocabulary) => vocabulary.entries), ); @@ -249,7 +249,7 @@ export class VocabularyService { ); // TODO remove false for the entries embed when https://github.com/DSpace/DSpace/issues/3096 is solved - return this.findVocabularyById(vocabularyOptions.name, true, true, followLink('entries', options, false)).pipe( + return this.findVocabularyById(vocabularyOptions.name, true, true, followLink('entries', { findListOptions: options, shouldEmbed: false })).pipe( getFirstSucceededRemoteDataPayload(), switchMap((vocabulary: Vocabulary) => vocabulary.entries), getFirstSucceededRemoteListPayload(), 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/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal-issue/journal-issue-search-result-grid-element.component.html b/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal-issue/journal-issue-search-result-grid-element.component.html index feb282d3a7..028876b3d0 100644 --- a/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal-issue/journal-issue-search-result-grid-element.component.html +++ b/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal-issue/journal-issue-search-result-grid-element.component.html @@ -8,13 +8,13 @@ rel="noopener noreferrer" [routerLink]="[itemPageRoute]" class="card-img-top full-width">
- +
- +
diff --git a/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal-volume/journal-volume-search-result-grid-element.component.html b/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal-volume/journal-volume-search-result-grid-element.component.html index aa2352b284..65ff75a731 100644 --- a/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal-volume/journal-volume-search-result-grid-element.component.html +++ b/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal-volume/journal-volume-search-result-grid-element.component.html @@ -8,13 +8,13 @@ rel="noopener noreferrer" [routerLink]="[itemPageRoute]" class="card-img-top full-width">
- +
- +
diff --git a/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal/journal-search-result-grid-element.component.html b/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal/journal-search-result-grid-element.component.html index 8fdad59827..0c5824c6d6 100644 --- a/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal/journal-search-result-grid-element.component.html +++ b/src/app/entity-groups/journal-entities/item-grid-elements/search-result-grid-elements/journal/journal-search-result-grid-element.component.html @@ -8,13 +8,13 @@ rel="noopener noreferrer" [routerLink]="[itemPageRoute]" class="card-img-top full-width">
- +
- +
diff --git a/src/app/entity-groups/journal-entities/item-pages/journal-issue/journal-issue.component.html b/src/app/entity-groups/journal-entities/item-pages/journal-issue/journal-issue.component.html index 8e357140d8..5847be7dd2 100644 --- a/src/app/entity-groups/journal-entities/item-pages/journal-issue/journal-issue.component.html +++ b/src/app/entity-groups/journal-entities/item-pages/journal-issue/journal-issue.component.html @@ -9,7 +9,7 @@
- +
- +
- +
- +
- +
diff --git a/src/app/entity-groups/research-entities/item-grid-elements/search-result-grid-elements/person/person-search-result-grid-element.component.html b/src/app/entity-groups/research-entities/item-grid-elements/search-result-grid-elements/person/person-search-result-grid-element.component.html index 23de8b134f..680a9909bc 100644 --- a/src/app/entity-groups/research-entities/item-grid-elements/search-result-grid-elements/person/person-search-result-grid-element.component.html +++ b/src/app/entity-groups/research-entities/item-grid-elements/search-result-grid-elements/person/person-search-result-grid-element.component.html @@ -8,13 +8,13 @@ rel="noopener noreferrer" [routerLink]="[itemPageRoute]" class="card-img-top full-width">
- +
- +
diff --git a/src/app/entity-groups/research-entities/item-grid-elements/search-result-grid-elements/project/project-search-result-grid-element.component.html b/src/app/entity-groups/research-entities/item-grid-elements/search-result-grid-elements/project/project-search-result-grid-element.component.html index 88498a4d67..204f8fc8cb 100644 --- a/src/app/entity-groups/research-entities/item-grid-elements/search-result-grid-elements/project/project-search-result-grid-element.component.html +++ b/src/app/entity-groups/research-entities/item-grid-elements/search-result-grid-elements/project/project-search-result-grid-element.component.html @@ -8,13 +8,13 @@ rel="noopener noreferrer" [routerLink]="[itemPageRoute]" class="card-img-top full-width">
- +
- +
diff --git a/src/app/entity-groups/research-entities/item-pages/org-unit/org-unit.component.html b/src/app/entity-groups/research-entities/item-pages/org-unit/org-unit.component.html index d328e93b15..c9ea8fb549 100644 --- a/src/app/entity-groups/research-entities/item-pages/org-unit/org-unit.component.html +++ b/src/app/entity-groups/research-entities/item-pages/org-unit/org-unit.component.html @@ -9,7 +9,7 @@
-
- diff --git a/src/app/entity-groups/research-entities/item-pages/project/project.component.html b/src/app/entity-groups/research-entities/item-pages/project/project.component.html index 181a8dc44e..8f2ff6adcd 100644 --- a/src/app/entity-groups/research-entities/item-pages/project/project.component.html +++ b/src/app/entity-groups/research-entities/item-pages/project/project.component.html @@ -10,7 +10,7 @@
diff --git a/src/app/entity-groups/research-entities/submission/item-list-elements/org-unit/org-unit-search-result-list-submission-element.component.html b/src/app/entity-groups/research-entities/submission/item-list-elements/org-unit/org-unit-search-result-list-submission-element.component.html index 063e1393cc..13787bb925 100644 --- a/src/app/entity-groups/research-entities/submission/item-list-elements/org-unit/org-unit-search-result-list-submission-element.component.html +++ b/src/app/entity-groups/research-entities/submission/item-list-elements/org-unit/org-unit-search-result-list-submission-element.component.html @@ -1,6 +1,6 @@
- +
{ - return this.bitstreamDataService.getThumbnailFor(this.dso).pipe( - getFirstSucceededRemoteDataPayload() - ); - } } diff --git a/src/app/entity-groups/research-entities/submission/item-list-elements/org-unit/org-unit-suggestions/org-unit-input-suggestions.component.html b/src/app/entity-groups/research-entities/submission/item-list-elements/org-unit/org-unit-suggestions/org-unit-input-suggestions.component.html index e177b2b561..87a422e7db 100644 --- a/src/app/entity-groups/research-entities/submission/item-list-elements/org-unit/org-unit-suggestions/org-unit-input-suggestions.component.html +++ b/src/app/entity-groups/research-entities/submission/item-list-elements/org-unit/org-unit-suggestions/org-unit-input-suggestions.component.html @@ -21,4 +21,4 @@
- \ No newline at end of file + diff --git a/src/app/entity-groups/research-entities/submission/item-list-elements/person/person-search-result-list-submission-element.component.ts b/src/app/entity-groups/research-entities/submission/item-list-elements/person/person-search-result-list-submission-element.component.ts index 64cf73cfb9..13de40e015 100644 --- a/src/app/entity-groups/research-entities/submission/item-list-elements/person/person-search-result-list-submission-element.component.ts +++ b/src/app/entity-groups/research-entities/submission/item-list-elements/person/person-search-result-list-submission-element.component.ts @@ -1,8 +1,5 @@ import { Component, OnInit } from '@angular/core'; -import { Observable } from 'rxjs'; import { BitstreamDataService } from '../../../../../core/data/bitstream-data.service'; -import { Bitstream } from '../../../../../core/shared/bitstream.model'; -import { getFirstSucceededRemoteDataPayload } from '../../../../../core/shared/operators'; import { SearchResultListElementComponent } from '../../../../../shared/object-list/search-result-list-element/search-result-list-element.component'; import { ItemSearchResult } from '../../../../../shared/object-collection/shared/item-search-result.model'; import { listableObjectComponent } from '../../../../../shared/object-collection/shared/listable-object/listable-object.decorator'; @@ -108,11 +105,4 @@ export class PersonSearchResultListSubmissionElementComponent extends SearchResu modalComp.value = value; return modalRef.result; } - - // TODO refactor to return RemoteData, and thumbnail template to deal with loading - getThumbnail(): Observable { - return this.bitstreamDataService.getThumbnailFor(this.dso).pipe( - getFirstSucceededRemoteDataPayload() - ); - } } diff --git a/src/app/root/root.component.ts b/src/app/root/root.component.ts index 576a0152be..209f17b520 100644 --- a/src/app/root/root.component.ts +++ b/src/app/root/root.component.ts @@ -1,5 +1,5 @@ import { map } from 'rxjs/operators'; -import { Component, Inject, OnInit, Optional, Input } from '@angular/core'; +import { Component, Inject, OnInit, Input } from '@angular/core'; import { Router } from '@angular/router'; import { combineLatest as combineLatestObservable, Observable, of } from 'rxjs'; @@ -18,8 +18,6 @@ import { HostWindowService } from '../shared/host-window.service'; import { ThemeConfig } from '../../config/theme.model'; import { Angulartics2DSpace } from '../statistics/angulartics/dspace-provider'; import { environment } from '../../environments/environment'; -import { LocaleService } from '../core/locale/locale.service'; -import { KlaroService } from '../shared/cookies/klaro.service'; import { slideSidebarPadding } from '../shared/animations/slide'; @Component({ @@ -58,9 +56,7 @@ export class RootComponent implements OnInit { private router: Router, private cssService: CSSVariableService, private menuService: MenuService, - private windowService: HostWindowService, - private localeService: LocaleService, - @Optional() private cookiesService: KlaroService + private windowService: HostWindowService ) { } diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts index 185cf3e92a..6aad3dd636 100644 --- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts +++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts @@ -44,7 +44,8 @@ describe('AuthNavMenuComponent', () => { authenticated: false, loaded: false, blocking: false, - loading: false + loading: false, + idle: false }; authState = { authenticated: true, @@ -52,7 +53,8 @@ describe('AuthNavMenuComponent', () => { blocking: false, loading: false, authToken: new AuthTokenInfo('test_token'), - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; } diff --git a/src/app/shared/auth-nav-menu/user-menu/user-menu.component.spec.ts b/src/app/shared/auth-nav-menu/user-menu/user-menu.component.spec.ts index c0f5f13e8f..983fe68274 100644 --- a/src/app/shared/auth-nav-menu/user-menu/user-menu.component.spec.ts +++ b/src/app/shared/auth-nav-menu/user-menu/user-menu.component.spec.ts @@ -37,7 +37,8 @@ describe('UserMenuComponent', () => { blocking: false, loading: false, authToken: new AuthTokenInfo('test_token'), - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; authStateLoading = { authenticated: true, @@ -45,7 +46,8 @@ describe('UserMenuComponent', () => { blocking: false, loading: true, authToken: null, - userId: EPersonMock.id + userId: EPersonMock.id, + idle: false }; } diff --git a/src/app/shared/comcol-forms/delete-comcol-page/delete-comcol-page.component.ts b/src/app/shared/comcol-forms/delete-comcol-page/delete-comcol-page.component.ts index ad3cf9f9fa..8579becc83 100644 --- a/src/app/shared/comcol-forms/delete-comcol-page/delete-comcol-page.component.ts +++ b/src/app/shared/comcol-forms/delete-comcol-page/delete-comcol-page.component.ts @@ -1,5 +1,5 @@ import { Component, OnInit } from '@angular/core'; -import { Observable } from 'rxjs'; +import { BehaviorSubject, Observable } from 'rxjs'; import { ActivatedRoute, Router } from '@angular/router'; import { RemoteData } from '../../../core/data/remote-data'; import { first, map } from 'rxjs/operators'; @@ -29,6 +29,12 @@ export class DeleteComColPageComponent i */ public dsoRD$: Observable>; + /** + * A boolean representing if a delete operation is pending + * @type {BehaviorSubject} + */ + public processing$: BehaviorSubject = new BehaviorSubject(false); + public constructor( protected dsoDataService: ComColDataService, protected router: Router, @@ -48,6 +54,7 @@ export class DeleteComColPageComponent i * Deletes an existing DSO and redirects to the home page afterwards, showing a notification that states whether or not the deletion was successful */ onConfirm(dso: TDomain) { + this.processing$.next(true); this.dsoDataService.delete(dso.id) .pipe(getFirstCompletedRemoteData()) .subscribe((response: RemoteData) => { diff --git a/src/app/shared/date.util.ts b/src/app/shared/date.util.ts index 063820784c..44afdd10a4 100644 --- a/src/app/shared/date.util.ts +++ b/src/app/shared/date.util.ts @@ -3,7 +3,7 @@ import { NgbDateStruct } from '@ng-bootstrap/ng-bootstrap'; import { isObject } from 'lodash'; import * as moment from 'moment'; -import { isNull } from './empty.util'; +import { isNull, isUndefined } from './empty.util'; /** * Returns true if the passed value is a NgbDateStruct. @@ -27,8 +27,9 @@ export function isNgbDateStruct(value: object): boolean { * @return string * the formatted date */ -export function dateToISOFormat(date: Date | NgbDateStruct): string { - const dateObj: Date = (date instanceof Date) ? date : ngbDateStructToDate(date); +export function dateToISOFormat(date: Date | NgbDateStruct | string): string { + const dateObj: Date = (date instanceof Date) ? date : + ((typeof date === 'string') ? ngbDateStructToDate(stringToNgbDateStruct(date)) : ngbDateStructToDate(date)); let year = dateObj.getFullYear().toString(); let month = (dateObj.getMonth() + 1).toString(); @@ -80,7 +81,7 @@ export function stringToNgbDateStruct(date: string): NgbDateStruct { * the NgbDateStruct object */ export function dateToNgbDateStruct(date?: Date): NgbDateStruct { - if (isNull(date)) { + if (isNull(date) || isUndefined(date)) { date = new Date(); } diff --git a/src/app/shared/dso-selector/dso-selector/dso-selector.component.html b/src/app/shared/dso-selector/dso-selector/dso-selector.component.html index 122f37b031..ab2ea6cd8b 100644 --- a/src/app/shared/dso-selector/dso-selector/dso-selector.component.html +++ b/src/app/shared/dso-selector/dso-selector/dso-selector.component.html @@ -14,12 +14,12 @@ [infiniteScrollContainer]="'.scrollable-menu'" [fromRoot]="true" (scrolled)="onScrollDown()"> - + - +
+
+

{{ "idle-modal.info" | translate:{timeToExpire: timeToExpire} }}

+
+
+ + +
+
diff --git a/src/app/shared/idle-modal/idle-modal.component.spec.ts b/src/app/shared/idle-modal/idle-modal.component.spec.ts new file mode 100644 index 0000000000..847bf6ac4f --- /dev/null +++ b/src/app/shared/idle-modal/idle-modal.component.spec.ts @@ -0,0 +1,135 @@ +import { DebugElement, NO_ERRORS_SCHEMA } from '@angular/core'; +import { ComponentFixture, fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing'; +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { TranslateModule } from '@ngx-translate/core'; +import { IdleModalComponent } from './idle-modal.component'; +import { AuthService } from '../../core/auth/auth.service'; +import { By } from '@angular/platform-browser'; +import { Store } from '@ngrx/store'; +import { LogOutAction } from '../../core/auth/auth.actions'; + +describe('IdleModalComponent', () => { + let component: IdleModalComponent; + let fixture: ComponentFixture; + let debugElement: DebugElement; + + let modalStub; + let authServiceStub; + let storeStub; + + beforeEach(waitForAsync(() => { + modalStub = jasmine.createSpyObj('modalStub', ['close']); + authServiceStub = jasmine.createSpyObj('authService', ['setIdle']); + storeStub = jasmine.createSpyObj('store', ['dispatch']); + TestBed.configureTestingModule({ + imports: [TranslateModule.forRoot()], + declarations: [IdleModalComponent], + providers: [ + { provide: NgbActiveModal, useValue: modalStub }, + { provide: AuthService, useValue: authServiceStub }, + { provide: Store, useValue: storeStub } + ], + schemas: [NO_ERRORS_SCHEMA] + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(IdleModalComponent); + component = fixture.componentInstance; + debugElement = fixture.debugElement; + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); + + describe('extendSessionPressed', () => { + beforeEach(fakeAsync(() => { + spyOn(component.response, 'next'); + component.extendSessionPressed(); + })); + it('should set idle to false', () => { + expect(authServiceStub.setIdle).toHaveBeenCalledWith(false); + }); + it('should close the modal', () => { + expect(modalStub.close).toHaveBeenCalled(); + }); + it('response \'closed\' should have true as next', () => { + expect(component.response.next).toHaveBeenCalledWith(true); + }); + }); + + describe('logOutPressed', () => { + beforeEach(() => { + component.logOutPressed(); + }); + it('should close the modal', () => { + expect(modalStub.close).toHaveBeenCalled(); + }); + it('should send logout action', () => { + expect(storeStub.dispatch).toHaveBeenCalledWith(new LogOutAction()); + }); + }); + + describe('closePressed', () => { + beforeEach(fakeAsync(() => { + spyOn(component.response, 'next'); + component.closePressed(); + })); + it('should set idle to false', () => { + expect(authServiceStub.setIdle).toHaveBeenCalledWith(false); + }); + it('should close the modal', () => { + expect(modalStub.close).toHaveBeenCalled(); + }); + it('response \'closed\' should have true as next', () => { + expect(component.response.next).toHaveBeenCalledWith(true); + }); + }); + + describe('when the click method emits on extend session button', () => { + beforeEach(fakeAsync(() => { + spyOn(component, 'extendSessionPressed'); + debugElement.query(By.css('button.confirm')).triggerEventHandler('click', { + preventDefault: () => {/**/ + } + }); + tick(); + fixture.detectChanges(); + })); + it('should call the extendSessionPressed method on the component', () => { + expect(component.extendSessionPressed).toHaveBeenCalled(); + }); + }); + + describe('when the click method emits on log out button', () => { + beforeEach(fakeAsync(() => { + spyOn(component, 'logOutPressed'); + debugElement.query(By.css('button.cancel')).triggerEventHandler('click', { + preventDefault: () => {/**/ + } + }); + tick(); + fixture.detectChanges(); + })); + it('should call the logOutPressed method on the component', () => { + expect(component.logOutPressed).toHaveBeenCalled(); + }); + }); + + describe('when the click method emits on close button', () => { + beforeEach(fakeAsync(() => { + spyOn(component, 'closePressed'); + debugElement.query(By.css('.close')).triggerEventHandler('click', { + preventDefault: () => {/**/ + } + }); + tick(); + fixture.detectChanges(); + })); + it('should call the closePressed method on the component', () => { + expect(component.closePressed).toHaveBeenCalled(); + }); + }); +}); diff --git a/src/app/shared/idle-modal/idle-modal.component.ts b/src/app/shared/idle-modal/idle-modal.component.ts new file mode 100644 index 0000000000..35fafcf5cf --- /dev/null +++ b/src/app/shared/idle-modal/idle-modal.component.ts @@ -0,0 +1,89 @@ +import { Component, OnInit, Output } from '@angular/core'; +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { environment } from '../../../environments/environment'; +import { AuthService } from '../../core/auth/auth.service'; +import { Subject } from 'rxjs'; +import { hasValue } from '../empty.util'; +import { Store } from '@ngrx/store'; +import { AppState } from '../../app.reducer'; +import { LogOutAction } from '../../core/auth/auth.actions'; + +@Component({ + selector: 'ds-idle-modal', + templateUrl: 'idle-modal.component.html', +}) +export class IdleModalComponent implements OnInit { + + /** + * Total time of idleness before session expires (in minutes) + * (environment.auth.ui.timeUntilIdle + environment.auth.ui.idleGracePeriod / 1000 / 60) + */ + timeToExpire: number; + + /** + * Timer to track time grace period + */ + private graceTimer; + + /** + * An event fired when the modal is closed + */ + @Output() + response: Subject = new Subject(); + + constructor(private activeModal: NgbActiveModal, + private authService: AuthService, + private store: Store) { + this.timeToExpire = (environment.auth.ui.timeUntilIdle + environment.auth.ui.idleGracePeriod) / 1000 / 60; // ms => min + } + + ngOnInit() { + if (hasValue(this.graceTimer)) { + clearTimeout(this.graceTimer); + } + this.graceTimer = setTimeout(() => { + this.logOutPressed(); + }, environment.auth.ui.idleGracePeriod); + } + + /** + * When extend session is pressed + */ + extendSessionPressed() { + this.extendSessionAndCloseModal(); + } + + /** + * Close modal and logout + */ + logOutPressed() { + this.closeModal(); + this.store.dispatch(new LogOutAction()); + } + + /** + * When close is pressed + */ + closePressed() { + this.extendSessionAndCloseModal(); + } + + /** + * Close the modal and extend session + */ + extendSessionAndCloseModal() { + if (hasValue(this.graceTimer)) { + clearTimeout(this.graceTimer); + } + this.authService.setIdle(false); + this.closeModal(); + } + + /** + * Close the modal and set the response to true so RootComponent knows the modal was closed + */ + closeModal() { + this.activeModal.close(); + this.response.next(true); + } +} diff --git a/src/app/shared/input-suggestions/filter-suggestions/filter-input-suggestions.component.html b/src/app/shared/input-suggestions/filter-suggestions/filter-input-suggestions.component.html index 7a9481f2f1..886c8f31c0 100644 --- a/src/app/shared/input-suggestions/filter-suggestions/filter-input-suggestions.component.html +++ b/src/app/shared/input-suggestions/filter-suggestions/filter-input-suggestions.component.html @@ -3,13 +3,27 @@ (keydown.arrowdown)="shiftFocusDown($event)" (keydown.arrowup)="shiftFocusUp($event)" (keydown.esc)="close()" (dsClickOutside)="close();"> - - +
+ +
+ + + +
@@ -19,4 +33,4 @@
- \ No newline at end of file + diff --git a/src/app/shared/input-suggestions/input-suggestions.component.ts b/src/app/shared/input-suggestions/input-suggestions.component.ts index c48dcfb831..7b5c9f34f2 100644 --- a/src/app/shared/input-suggestions/input-suggestions.component.ts +++ b/src/app/shared/input-suggestions/input-suggestions.component.ts @@ -53,6 +53,11 @@ export class InputSuggestionsComponent implements ControlValueAccessor, OnChange */ @Input() valid = true; + /** + * Label for the input field. Used for screen readers. + */ + @Input() label? = ''; + /** * Output for when the form is submitted */ diff --git a/src/app/shared/mocks/auth.service.mock.ts b/src/app/shared/mocks/auth.service.mock.ts index 98878bd6c1..bb39d08284 100644 --- a/src/app/shared/mocks/auth.service.mock.ts +++ b/src/app/shared/mocks/auth.service.mock.ts @@ -19,4 +19,11 @@ export class AuthServiceMock { public setRedirectUrl(url: string) { } + + public trackTokenExpiration(): void { + } + + public isUserIdle(): Observable { + return observableOf(false); + } } diff --git a/src/app/shared/mocks/item.mock.ts b/src/app/shared/mocks/item.mock.ts index 945b0f7816..10eab2da00 100644 --- a/src/app/shared/mocks/item.mock.ts +++ b/src/app/shared/mocks/item.mock.ts @@ -5,6 +5,7 @@ import { Bitstream } from '../../core/shared/bitstream.model'; import { Item } from '../../core/shared/item.model'; import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils'; import { createPaginatedList } from '../testing/utils.test'; +import { Bundle } from '../../core/shared/bundle.model'; export const MockBitstreamFormat1: BitstreamFormat = Object.assign(new BitstreamFormat(), { shortDescription: 'Microsoft Word XML', @@ -34,11 +35,25 @@ export const MockBitstreamFormat2: BitstreamFormat = Object.assign(new Bitstream } }); +export const MockBitstreamFormat3: BitstreamFormat = Object.assign(new BitstreamFormat(), { + shortDescription: 'Binary', + description: 'Some scary unknown binary file', + mimetype: 'application/octet-stream', + supportLevel: 0, + internal: false, + extensions: null, + _links:{ + self: { + href: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/17' + } + } +}); + export const MockBitstream1: Bitstream = Object.assign(new Bitstream(), { sizeBytes: 10201, content: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/cf9b0c8e-a1eb-4b65-afd0-567366448713/content', - format: observableOf(MockBitstreamFormat1), + format: createSuccessfulRemoteDataObject$(MockBitstreamFormat1), bundleName: 'ORIGINAL', _links:{ self: { @@ -61,7 +76,7 @@ export const MockBitstream1: Bitstream = Object.assign(new Bitstream(), export const MockBitstream2: Bitstream = Object.assign(new Bitstream(), { sizeBytes: 31302, content: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/content', - format: observableOf(MockBitstreamFormat2), + format: createSuccessfulRemoteDataObject$(MockBitstreamFormat2), bundleName: 'ORIGINAL', id: '99b00f3c-1cc6-4689-8158-91965bee6b28', uuid: '99b00f3c-1cc6-4689-8158-91965bee6b28', @@ -82,6 +97,68 @@ export const MockBitstream2: Bitstream = Object.assign(new Bitstream(), { } }); +export const MockBitstream3: Bitstream = Object.assign(new Bitstream(), { + sizeBytes: 4975123, + content: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/content', + format: createSuccessfulRemoteDataObject$(MockBitstreamFormat3), + bundleName: 'ORIGINAL', + id: '4db100c1-e1f5-4055-9404-9bc3e2d15f29', + uuid: '4db100c1-e1f5-4055-9404-9bc3e2d15f29', + type: 'bitstream', + _links: { + self: { href: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29' }, + content: { href: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/4db100c1-e1f5-4055-9404-9bc3e2d15f29/content' }, + format: { href: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/17' }, + bundle: { href: '' } + }, + metadata: { + 'dc.title': [ + { + language: null, + value: 'scary' + } + ] + } +}); + +export const MockOriginalBundle: Bundle = Object.assign(new Bundle(), { + name: 'ORIGINAL', + primaryBitstream: createSuccessfulRemoteDataObject$(MockBitstream2), + bitstreams: observableOf(Object.assign({ + _links: { + self: { + href: 'dspace-angular://aggregated/object/1507836003548', + } + }, + requestPending: false, + responsePending: false, + isSuccessful: true, + errorMessage: '', + state: '', + error: undefined, + isRequestPending: false, + isResponsePending: false, + isLoading: false, + hasFailed: false, + hasSucceeded: true, + statusCode: '202', + pageInfo: {}, + payload: { + pageInfo: { + elementsPerPage: 20, + totalElements: 3, + totalPages: 1, + currentPage: 2 + }, + page: [ + MockBitstream1, + MockBitstream2 + ] + } + })) +}); + + /* tslint:disable:no-shadowed-variable */ export const ItemMock: Item = Object.assign(new Item(), { handle: '10673/6', @@ -90,41 +167,7 @@ export const ItemMock: Item = Object.assign(new Item(), { isDiscoverable: true, isWithdrawn: false, bundles: createSuccessfulRemoteDataObject$(createPaginatedList([ - { - name: 'ORIGINAL', - bitstreams: observableOf(Object.assign({ - _links: { - self: { - href: 'dspace-angular://aggregated/object/1507836003548', - } - }, - requestPending: false, - responsePending: false, - isSuccessful: true, - errorMessage: '', - state: '', - error: undefined, - isRequestPending: false, - isResponsePending: false, - isLoading: false, - hasFailed: false, - hasSucceeded: true, - statusCode: '202', - pageInfo: {}, - payload: { - pageInfo: { - elementsPerPage: 20, - totalElements: 3, - totalPages: 1, - currentPage: 2 - }, - page: [ - MockBitstream1, - MockBitstream2 - ] - } - })) - } + MockOriginalBundle, ])), _links:{ self: { diff --git a/src/app/shared/mocks/submission.mock.ts b/src/app/shared/mocks/submission.mock.ts index 1ee097af71..eaebb38df8 100644 --- a/src/app/shared/mocks/submission.mock.ts +++ b/src/app/shared/mocks/submission.mock.ts @@ -1519,83 +1519,87 @@ export const mockFileFormData = { }, accessConditions: [ { - name: [ - { - value: 'openaccess', - language: null, - authority: null, - display: 'openaccess', - confidence: -1, - place: 0, - otherInformation: null - } - ], - } - , + accessConditionGroup: { + name: [ + { + value: 'openaccess', + language: null, + authority: null, + display: 'openaccess', + confidence: -1, + place: 0, + otherInformation: null + } + ], + }, + }, { - name: [ - { - value: 'lease', - language: null, - authority: null, - display: 'lease', - confidence: -1, - place: 0, - otherInformation: null - } - ], - endDate: [ - { - value: { - year: 2019, - month: 1, - day: 16 - }, - language: null, - authority: null, - display: { - year: 2019, - month: 1, - day: 16 - }, - confidence: -1, - place: 0, - otherInformation: null - } - ], - } - , + accessConditionGroup:{ + name: [ + { + value: 'lease', + language: null, + authority: null, + display: 'lease', + confidence: -1, + place: 0, + otherInformation: null + } + ], + endDate: [ + { + value: { + year: 2019, + month: 1, + day: 16 + }, + language: null, + authority: null, + display: { + year: 2019, + month: 1, + day: 16 + }, + confidence: -1, + place: 0, + otherInformation: null + } + ], + } + }, { - name: [ - { - value: 'embargo', - language: null, - authority: null, - display: 'lease', - confidence: -1, - place: 0, - otherInformation: null - } - ], - startDate: [ - { - value: { - year: 2019, - month: 1, - day: 16 - }, - language: null, - authority: null, - display: { - year: 2019, - month: 1, - day: 16 - }, - confidence: -1, - place: 0, - otherInformation: null - } - ], + accessConditionGroup: { + name: [ + { + value: 'embargo', + language: null, + authority: null, + display: 'lease', + confidence: -1, + place: 0, + otherInformation: null + } + ], + startDate: [ + { + value: { + year: 2019, + month: 1, + day: 16 + }, + language: null, + authority: null, + display: { + year: 2019, + month: 1, + day: 16 + }, + confidence: -1, + place: 0, + otherInformation: null + } + ], + } } ] }; diff --git a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts index 3442b044a2..458272c606 100644 --- a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts +++ b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts @@ -1,5 +1,5 @@ -import { waitForAsync, ComponentFixture, TestBed, fakeAsync, tick } from '@angular/core/testing'; -import { ChangeDetectionStrategy, ComponentFactoryResolver, NO_ERRORS_SCHEMA } from '@angular/core'; +import { ComponentFixture, fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing'; +import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; import { ListableObjectComponentLoaderComponent } from './listable-object-component-loader.component'; import { ListableObject } from '../listable-object.model'; import { GenericConstructor } from '../../../../core/shared/generic-constructor'; @@ -117,17 +117,33 @@ describe('ListableObjectComponentLoaderComponent', () => { }); describe('When a reloadedObject is emitted', () => { + let listableComponent; + let reloadedObject: any; - it('should re-instantiate the listable component ', fakeAsync(() => { + beforeEach(() => { + spyOn((comp as any), 'connectInputsAndOutputs').and.returnValue(null); + spyOn((comp as any).contentChange, 'emit').and.returnValue(null); - spyOn((comp as any), 'instantiateComponent').and.returnValue(null); + listableComponent = fixture.debugElement.query(By.css('ds-item-list-element')).componentInstance; + reloadedObject = 'object'; + }); + + it('should pass it on connectInputsAndOutputs', fakeAsync(() => { + expect((comp as any).connectInputsAndOutputs).not.toHaveBeenCalled(); - const listableComponent = fixture.debugElement.query(By.css('ds-item-list-element')).componentInstance; - const reloadedObject: any = 'object'; (listableComponent as any).reloadedObject.emit(reloadedObject); tick(); - expect((comp as any).instantiateComponent).toHaveBeenCalledWith(reloadedObject); + expect((comp as any).connectInputsAndOutputs).toHaveBeenCalled(); + })); + + it('should re-emit it as a contentChange', fakeAsync(() => { + expect((comp as any).contentChange.emit).not.toHaveBeenCalled(); + + (listableComponent as any).reloadedObject.emit(reloadedObject); + tick(); + + expect((comp as any).contentChange.emit).toHaveBeenCalledWith(reloadedObject); })); }); diff --git a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.ts b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.ts index 30ad91c1e2..4c6206cb43 100644 --- a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.ts +++ b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.ts @@ -3,10 +3,14 @@ import { ComponentFactoryResolver, ElementRef, Input, - OnDestroy, OnInit, - Output, ViewChild -, - EventEmitter + OnDestroy, + OnInit, + Output, + ViewChild, + EventEmitter, + SimpleChanges, + OnChanges, + ComponentRef } from '@angular/core'; import { ListableObject } from '../listable-object.model'; import { ViewMode } from '../../../../core/shared/view-mode.model'; @@ -15,7 +19,7 @@ import { getListableObjectComponent } from './listable-object.decorator'; import { GenericConstructor } from '../../../../core/shared/generic-constructor'; import { ListableObjectDirective } from './listable-object.directive'; import { CollectionElementLinkType } from '../../collection-element-link.type'; -import { hasValue } from '../../../empty.util'; +import { hasValue, isNotEmpty } from '../../../empty.util'; import { Subscription } from 'rxjs/internal/Subscription'; import { DSpaceObject } from '../../../../core/shared/dspace-object.model'; import { take } from 'rxjs/operators'; @@ -29,7 +33,7 @@ import { ThemeService } from '../../../theme-support/theme.service'; /** * Component for determining what component to use depending on the item's entity type (dspace.entity.type) */ -export class ListableObjectComponentLoaderComponent implements OnInit, OnDestroy { +export class ListableObjectComponentLoaderComponent implements OnInit, OnChanges, OnDestroy { /** * The item or metadata to determine the component for */ @@ -107,6 +111,25 @@ export class ListableObjectComponentLoaderComponent implements OnInit, OnDestroy */ protected subs: Subscription[] = []; + /** + * The reference to the dynamic component + */ + protected compRef: ComponentRef; + + /** + * The list of input and output names for the dynamic component + */ + protected inAndOutputNames: string[] = [ + 'object', + 'index', + 'linkType', + 'listID', + 'showLabel', + 'context', + 'viewMode', + 'value', + ]; + constructor( private componentFactoryResolver: ComponentFactoryResolver, private themeService: ThemeService @@ -120,6 +143,15 @@ export class ListableObjectComponentLoaderComponent implements OnInit, OnDestroy this.instantiateComponent(this.object); } + /** + * Whenever the inputs change, update the inputs of the dynamic component + */ + ngOnChanges(changes: SimpleChanges): void { + if (this.inAndOutputNames.some((name: any) => hasValue(changes[name]))) { + this.connectInputsAndOutputs(); + } + } + ngOnDestroy() { this.subs .filter((subscription) => hasValue(subscription)) @@ -137,28 +169,22 @@ export class ListableObjectComponentLoaderComponent implements OnInit, OnDestroy const viewContainerRef = this.listableObjectDirective.viewContainerRef; viewContainerRef.clear(); - const componentRef = viewContainerRef.createComponent( + this.compRef = viewContainerRef.createComponent( componentFactory, 0, undefined, [ [this.badges.nativeElement], ]); - (componentRef.instance as any).object = object; - (componentRef.instance as any).index = this.index; - (componentRef.instance as any).linkType = this.linkType; - (componentRef.instance as any).listID = this.listID; - (componentRef.instance as any).showLabel = this.showLabel; - (componentRef.instance as any).context = this.context; - (componentRef.instance as any).viewMode = this.viewMode; - (componentRef.instance as any).value = this.value; - if ((componentRef.instance as any).reloadedObject) { - (componentRef.instance as any).reloadedObject.pipe(take(1)).subscribe((reloadedObject: DSpaceObject) => { + this.connectInputsAndOutputs(); + + if ((this.compRef.instance as any).reloadedObject) { + (this.compRef.instance as any).reloadedObject.pipe(take(1)).subscribe((reloadedObject: DSpaceObject) => { if (reloadedObject) { - componentRef.destroy(); + this.compRef.destroy(); this.object = reloadedObject; - this.instantiateComponent(reloadedObject); + this.connectInputsAndOutputs(); this.contentChange.emit(reloadedObject); } }); @@ -187,4 +213,17 @@ export class ListableObjectComponentLoaderComponent implements OnInit, OnDestroy context: Context): GenericConstructor { return getListableObjectComponent(renderTypes, viewMode, context, this.themeService.getThemeName()); } + + /** + * Connect the in and outputs of this component to the dynamic component, + * to ensure they're in sync + */ + protected connectInputsAndOutputs(): void { + if (isNotEmpty(this.inAndOutputNames) && hasValue(this.compRef) && hasValue(this.compRef.instance)) { + this.inAndOutputNames.forEach((name: any) => { + this.compRef.instance[name] = this[name]; + }); + } + } + } diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/claimed-task-search-result/claimed-task-search-result-detail-element.component.ts b/src/app/shared/object-detail/my-dspace-result-detail-element/claimed-task-search-result/claimed-task-search-result-detail-element.component.ts index b729307443..74411d2341 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/claimed-task-search-result/claimed-task-search-result-detail-element.component.ts +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/claimed-task-search-result/claimed-task-search-result-detail-element.component.ts @@ -49,8 +49,8 @@ export class ClaimedTaskSearchResultDetailElementComponent extends SearchResultD */ ngOnInit() { super.ngOnInit(); - this.linkService.resolveLinks(this.dso, followLink('workflowitem', null, true, true, true, - followLink('item', null, true, true, true, followLink('bundles')), + this.linkService.resolveLinks(this.dso, followLink('workflowitem', {}, + followLink('item', {}, followLink('bundles')), followLink('submitter') ), followLink('action')); this.workflowitemRD$ = this.dso.workflowitem as Observable>; diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.html b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.html index e4d2526eb2..61e2955deb 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.html +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.html @@ -10,7 +10,7 @@
- + diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.spec.ts b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.spec.ts index 2b38b58598..07acf3ea75 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.spec.ts +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.spec.ts @@ -126,13 +126,6 @@ describe('ItemDetailPreviewComponent', () => { })); - it('should get item thumbnail', (done) => { - component.getThumbnail().subscribe((thumbnail) => { - expect(thumbnail).toBeDefined(); - done(); - }); - }); - it('should get item bitstreams', (done) => { component.getFiles().subscribe((bitstreams) => { expect(bitstreams).toBeDefined(); diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.ts b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.ts index a4dc0a1d3d..92c1afcb59 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.ts +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/item-detail-preview/item-detail-preview.component.ts @@ -5,10 +5,7 @@ import { first } from 'rxjs/operators'; import { BitstreamDataService } from '../../../../core/data/bitstream-data.service'; import { Item } from '../../../../core/shared/item.model'; -import { - getFirstSucceededRemoteDataPayload, - getFirstSucceededRemoteListPayload -} from '../../../../core/shared/operators'; +import { getFirstSucceededRemoteListPayload } from '../../../../core/shared/operators'; import { MyDspaceItemStatusType } from '../../../object-collection/shared/mydspace-item-status/my-dspace-item-status-type'; import { fadeInOut } from '../../../animations/fade'; import { Bitstream } from '../../../../core/shared/bitstream.model'; @@ -57,11 +54,6 @@ export class ItemDetailPreviewComponent { */ public separator = ', '; - /** - * The item's thumbnail - */ - public thumbnail$: Observable; - /** * Initialize instance variables * @@ -86,13 +78,6 @@ export class ItemDetailPreviewComponent { }); } - // TODO refactor this method to return RemoteData, and the template to deal with loading and errors - public getThumbnail(): Observable { - return this.bitstreamDataService.getThumbnailFor(this.item).pipe( - getFirstSucceededRemoteDataPayload() - ); - } - // TODO refactor this method to return RemoteData, and the template to deal with loading and errors public getFiles(): Observable { return this.bitstreamDataService diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/pool-search-result/pool-search-result-detail-element.component.ts b/src/app/shared/object-detail/my-dspace-result-detail-element/pool-search-result/pool-search-result-detail-element.component.ts index a8b2514ffb..df27abd42e 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/pool-search-result/pool-search-result-detail-element.component.ts +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/pool-search-result/pool-search-result-detail-element.component.ts @@ -48,8 +48,8 @@ export class PoolSearchResultDetailElementComponent extends SearchResultDetailEl */ ngOnInit() { super.ngOnInit(); - this.linkService.resolveLinks(this.dso, followLink('workflowitem', null, true, true, true, - followLink('item', null, true, true, true, followLink('bundles')), + this.linkService.resolveLinks(this.dso, followLink('workflowitem', {}, + followLink('item', {}, followLink('bundles')), followLink('submitter') ), followLink('action')); this.workflowitemRD$ = this.dso.workflowitem as Observable>; diff --git a/src/app/shared/object-grid/search-result-grid-element/item-search-result/item/item-search-result-grid-element.component.html b/src/app/shared/object-grid/search-result-grid-element/item-search-result/item/item-search-result-grid-element.component.html index bc16853721..d2454b28e6 100644 --- a/src/app/shared/object-grid/search-result-grid-element/item-search-result/item/item-search-result-grid-element.component.html +++ b/src/app/shared/object-grid/search-result-grid-element/item-search-result/item/item-search-result-grid-element.component.html @@ -6,13 +6,13 @@
- +
- +
@@ -43,4 +43,3 @@
- diff --git a/src/app/shared/object-grid/search-result-grid-element/search-result-grid-element.component.ts b/src/app/shared/object-grid/search-result-grid-element/search-result-grid-element.component.ts index 7436d2922e..da1f0ea11b 100644 --- a/src/app/shared/object-grid/search-result-grid-element/search-result-grid-element.component.ts +++ b/src/app/shared/object-grid/search-result-grid-element/search-result-grid-element.component.ts @@ -3,13 +3,11 @@ import { Observable } from 'rxjs'; import { SearchResult } from '../../search/search-result.model'; import { BitstreamDataService } from '../../../core/data/bitstream-data.service'; -import { Bitstream } from '../../../core/shared/bitstream.model'; import { DSpaceObject } from '../../../core/shared/dspace-object.model'; import { Metadata } from '../../../core/shared/metadata.utils'; import { hasValue } from '../../empty.util'; import { AbstractListableElementComponent } from '../../object-collection/shared/object-collection-element/abstract-listable-element.component'; import { TruncatableService } from '../../truncatable/truncatable.service'; -import { getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators'; @Component({ selector: 'ds-search-result-grid-element', @@ -66,11 +64,4 @@ export class SearchResultGridElementComponent, K exten private isCollapsed(): Observable { return this.truncatableService.isCollapsed(this.dso.id); } - - // TODO refactor to return RemoteData, and thumbnail template to deal with loading - getThumbnail(): Observable { - return this.bitstreamDataService.getThumbnailFor(this.dso as any).pipe( - getFirstSucceededRemoteDataPayload() - ); - } } diff --git a/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-approved-search-result/claimed-approved-search-result-list-element.component.ts b/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-approved-search-result/claimed-approved-search-result-list-element.component.ts index 5571782ce2..eaf407d787 100644 --- a/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-approved-search-result/claimed-approved-search-result-list-element.component.ts +++ b/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-approved-search-result/claimed-approved-search-result-list-element.component.ts @@ -55,10 +55,7 @@ export class ClaimedApprovedSearchResultListElementComponent extends SearchResul super.ngOnInit(); this.linkService.resolveLinks(this.dso, followLink('workflowitem', - null, - true, - false, - true, + { useCachedVersionIfAvailable: false }, followLink('item'), followLink('submitter') ), diff --git a/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-declined-search-result/claimed-declined-search-result-list-element.component.ts b/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-declined-search-result/claimed-declined-search-result-list-element.component.ts index 630aa699a7..0b9a925dbf 100644 --- a/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-declined-search-result/claimed-declined-search-result-list-element.component.ts +++ b/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-declined-search-result/claimed-declined-search-result-list-element.component.ts @@ -56,10 +56,7 @@ export class ClaimedDeclinedSearchResultListElementComponent extends SearchResul super.ngOnInit(); this.linkService.resolveLinks(this.dso, followLink('workflowitem', - null, - true, - false, - true, + { useCachedVersionIfAvailable: false }, followLink('item'), followLink('submitter') ), diff --git a/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-search-result-list-element.component.ts b/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-search-result-list-element.component.ts index dae3272889..2cf8f9a231 100644 --- a/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-search-result-list-element.component.ts +++ b/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-search-result-list-element.component.ts @@ -50,7 +50,7 @@ export class ClaimedSearchResultListElementComponent extends SearchResultListEle */ ngOnInit() { super.ngOnInit(); - this.linkService.resolveLinks(this.dso, followLink('workflowitem', null, true, true, true, + this.linkService.resolveLinks(this.dso, followLink('workflowitem', {}, followLink('item'), followLink('submitter') ), followLink('action')); this.workflowitemRD$ = this.dso.workflowitem as Observable>; diff --git a/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.ts b/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.ts index fe4fa715ee..e9d64db572 100644 --- a/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.ts +++ b/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.ts @@ -60,7 +60,7 @@ export class PoolSearchResultListElementComponent extends SearchResultListElemen */ ngOnInit() { super.ngOnInit(); - this.linkService.resolveLinks(this.dso, followLink('workflowitem', null, true, true, true, + this.linkService.resolveLinks(this.dso, followLink('workflowitem', {}, followLink('item'), followLink('submitter') ), followLink('action')); this.workflowitemRD$ = this.dso.workflowitem as Observable>; diff --git a/src/app/shared/search/search-filters/search-filter/search-authority-filter/search-authority-filter.component.html b/src/app/shared/search/search-filters/search-filter/search-authority-filter/search-authority-filter.component.html index 5e6bcfaf8b..44aed494e3 100644 --- a/src/app/shared/search/search-filters/search-filter/search-authority-filter/search-authority-filter.component.html +++ b/src/app/shared/search/search-filters/search-filter/search-authority-filter/search-authority-filter.component.html @@ -8,15 +8,18 @@
{{"search.filters.filter.show-more" - | translate}} + (click)="showMore()" href="javascript:void(0);"> + {{"search.filters.filter.show-more" | translate}} + {{"search.filters.filter.show-less" - | translate}} + (click)="showFirstPageOnly()" href="javascript:void(0);"> + {{"search.filters.filter.show-less" | translate}} +
{{"search.filters.filter.show-more" - | translate}} + (click)="showMore()" href="javascript:void(0);"> + {{"search.filters.filter.show-more" | translate}} + {{"search.filters.filter.show-less" - | translate}} + (click)="showFirstPageOnly()" href="javascript:void(0);"> + {{"search.filters.filter.show-less" | translate}} +
diff --git a/src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html b/src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html index cf4876e34f..e2e57e7370 100644 --- a/src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html +++ b/src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html @@ -1,10 +1,13 @@ - - + + {{filterValue.count}} diff --git a/src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html b/src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html index 4bcfc02966..d6cb7a3d79 100644 --- a/src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html +++ b/src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html @@ -1,8 +1,11 @@ + diff --git a/src/app/shared/search/search-filters/search-filter/search-filter.component.html b/src/app/shared/search/search-filters/search-filter/search-filter.component.html index eb2105f4e7..230f072772 100644 --- a/src/app/shared/search/search-filters/search-filter/search-filter.component.html +++ b/src/app/shared/search/search-filters/search-filter/search-filter.component.html @@ -1,17 +1,21 @@ -
-
+
+
+
+ class="search-filter-wrapper" [ngClass]="{ 'closed' : closed, 'notab': notab }"> diff --git a/src/app/shared/search/search-filters/search-filter/search-filter.component.scss b/src/app/shared/search/search-filters/search-filter/search-filter.component.scss index 518e7c9d5f..7e2631b55f 100644 --- a/src/app/shared/search/search-filters/search-filter/search-filter.component.scss +++ b/src/app/shared/search/search-filters/search-filter/search-filter.component.scss @@ -1,10 +1,36 @@ :host .facet-filter { - border: 1px solid var(--bs-light); - cursor: pointer; - .search-filter-wrapper.closed { - overflow: hidden; + border: 1px solid var(--bs-light); + cursor: pointer; + line-height: 0; + + .search-filter-wrapper { + line-height: var(--bs-line-height-base); + &.closed { + overflow: hidden; } - .filter-toggle { - line-height: var(--bs-line-height-base); + &.notab { + visibility: hidden; } + } + + .filter-toggle { + line-height: var(--bs-line-height-base); + text-align: right; + position: relative; + top: -0.125rem; // Fix weird outline shape in Chrome + } + + > button { + appearance: none; + border: 0; + padding: 0; + background: transparent; + width: 100%; + outline: none !important; + } + + &.focus { + outline: none; + box-shadow: var(--bs-input-btn-focus-box-shadow); + } } diff --git a/src/app/shared/search/search-filters/search-filter/search-filter.component.spec.ts b/src/app/shared/search/search-filters/search-filter/search-filter.component.spec.ts index 228eef9a20..5e0077e11d 100644 --- a/src/app/shared/search/search-filters/search-filter/search-filter.component.spec.ts +++ b/src/app/shared/search/search-filters/search-filter/search-filter.component.spec.ts @@ -12,6 +12,7 @@ import { SearchFilterConfig } from '../../search-filter-config.model'; import { FilterType } from '../../filter-type.model'; import { SearchConfigurationServiceStub } from '../../../testing/search-configuration-service.stub'; import { SEARCH_CONFIG_SERVICE } from '../../../../+my-dspace-page/my-dspace-page.component'; +import { SequenceService } from '../../../../core/shared/sequence.service'; describe('SearchFilterComponent', () => { let comp: SearchFilterComponent; @@ -50,12 +51,15 @@ describe('SearchFilterComponent', () => { }; let filterService; + let sequenceService; const mockResults = observableOf(['test', 'data']); const searchServiceStub = { getFacetValuesFor: (filter) => mockResults }; beforeEach(waitForAsync(() => { + sequenceService = jasmine.createSpyObj('sequenceService', { next: 17 }); + TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([]), NoopAnimationsModule], declarations: [SearchFilterComponent], @@ -65,7 +69,8 @@ describe('SearchFilterComponent', () => { provide: SearchFilterService, useValue: mockFilterService }, - { provide: SEARCH_CONFIG_SERVICE, useValue: new SearchConfigurationServiceStub() } + { provide: SEARCH_CONFIG_SERVICE, useValue: new SearchConfigurationServiceStub() }, + { provide: SequenceService, useValue: sequenceService }, ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(SearchFilterComponent, { @@ -81,6 +86,12 @@ describe('SearchFilterComponent', () => { filterService = (comp as any).filterService; }); + it('should generate unique IDs', () => { + expect(sequenceService.next).toHaveBeenCalled(); + expect(comp.toggleId).toContain('17'); + expect(comp.regionId).toContain('17'); + }); + describe('when the toggle method is triggered', () => { beforeEach(() => { spyOn(filterService, 'toggle'); diff --git a/src/app/shared/search/search-filters/search-filter/search-filter.component.ts b/src/app/shared/search/search-filters/search-filter/search-filter.component.ts index 31ace10a7d..0f7f763b45 100644 --- a/src/app/shared/search/search-filters/search-filter/search-filter.component.ts +++ b/src/app/shared/search/search-filters/search-filter/search-filter.component.ts @@ -10,6 +10,7 @@ import { isNotEmpty } from '../../../empty.util'; import { SearchService } from '../../../../core/shared/search/search.service'; import { SearchConfigurationService } from '../../../../core/shared/search/search-configuration.service'; import { SEARCH_CONFIG_SERVICE } from '../../../../+my-dspace-page/my-dspace-page.component'; +import { SequenceService } from '../../../../core/shared/sequence.service'; @Component({ selector: 'ds-search-filter', @@ -37,6 +38,16 @@ export class SearchFilterComponent implements OnInit { */ closed: boolean; + /** + * True when the filter controls should be hidden & removed from the tablist + */ + notab: boolean; + + /** + * True when the filter toggle button is focused + */ + focusBox = false; + /** * Emits true when the filter is currently collapsed in the store */ @@ -52,10 +63,15 @@ export class SearchFilterComponent implements OnInit { */ active$: Observable; + private readonly sequenceId: number; + constructor( private filterService: SearchFilterService, private searchService: SearchService, - @Inject(SEARCH_CONFIG_SERVICE) private searchConfigService: SearchConfigurationService) { + @Inject(SEARCH_CONFIG_SERVICE) private searchConfigService: SearchConfigurationService, + private sequenceService: SequenceService, + ) { + this.sequenceId = this.sequenceService.next(); } /** @@ -112,6 +128,9 @@ export class SearchFilterComponent implements OnInit { if (event.fromState === 'collapsed') { this.closed = false; } + if (event.toState === 'collapsed') { + this.notab = true; + } } /** @@ -122,6 +141,17 @@ export class SearchFilterComponent implements OnInit { if (event.toState === 'collapsed') { this.closed = true; } + if (event.fromState === 'collapsed') { + this.notab = false; + } + } + + get regionId(): string { + return `search-filter-region-${this.sequenceId}`; + } + + get toggleId(): string { + return `search-filter-toggle-${this.sequenceId}`; } /** diff --git a/src/app/shared/search/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html b/src/app/shared/search/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html index 06b60b5ecd..49ca6fe3fd 100644 --- a/src/app/shared/search/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html +++ b/src/app/shared/search/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html @@ -8,15 +8,18 @@
{{"search.filters.filter.show-more" - | translate}} + (click)="showMore()" href="javascript:void(0);"> + {{"search.filters.filter.show-more" | translate}} + {{"search.filters.filter.show-less" - | translate}} + (click)="showFirstPageOnly()" href="javascript:void(0);"> + {{"search.filters.filter.show-less" | translate}} +
-
- +
+
- +
- + - + [dsDebounce]="250" (onDebounce)="onSubmit()" + (keydown)="startKeyboardControl()" (keyup)="stopKeyboardControl()" + [(ngModel)]="range" ngDefaultControl> +
diff --git a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss index 2c98280e7f..f26806abfb 100644 --- a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss +++ b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss @@ -21,6 +21,7 @@ } &:focus { outline: none; + box-shadow: var(--bs-input-btn-focus-box-shadow); } } 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 62b1cb98a6..b23a2d8224 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 @@ -68,6 +68,12 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple */ sub: Subscription; + /** + * Whether the sider is being controlled by the keyboard. + * Supresses any changes until the key is released. + */ + keyboardControl: boolean; + constructor(protected searchService: SearchService, protected filterService: SearchFilterService, protected router: Router, @@ -104,6 +110,10 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple * Submits new custom range values to the range filter from the widget */ onSubmit() { + if (this.keyboardControl) { + return; // don't submit if a key is being held down + } + const newMin = this.range[0] !== this.min ? [this.range[0]] : null; const newMax = this.range[1] !== this.max ? [this.range[1]] : null; this.router.navigate(this.getSearchLinkParts(), { @@ -117,6 +127,14 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple this.filter = ''; } + startKeyboardControl(): void { + this.keyboardControl = true; + } + + stopKeyboardControl(): void { + this.keyboardControl = false; + } + /** * TODO when upgrading nouislider, verify that this check is still needed. * Prevents AoT bug diff --git a/src/app/shared/search/search-filters/search-filter/search-text-filter/search-text-filter.component.html b/src/app/shared/search/search-filters/search-filter/search-text-filter/search-text-filter.component.html index 43134014e1..fdf154bc04 100644 --- a/src/app/shared/search/search-filters/search-filter/search-text-filter/search-text-filter.component.html +++ b/src/app/shared/search/search-filters/search-filter/search-text-filter/search-text-filter.component.html @@ -8,15 +8,18 @@
{{"search.filters.filter.show-more" - | translate}} + (click)="showMore()" href="javascript:void(0);"> + {{"search.filters.filter.show-more" | translate}} + {{"search.filters.filter.show-less" - | translate}} + (click)="showFirstPageOnly()" href="javascript:void(0);"> + {{"search.filters.filter.show-less" | translate}} +
{ * Defaults to true */ reRequestOnStale? = true; + + /** + * If this is false an error will be thrown if the link doesn't exist on the model it is used on + * Defaults to false + */ + isOptional? = false; } /** @@ -57,23 +64,35 @@ export class FollowLinkConfig { * no valid cached version. Defaults * @param reRequestOnStale: Whether or not the link should automatically be re-requested after the * response becomes stale + * @param isOptional: Whether or not to fail if the link doesn't exist * @param linksToFollow: a list of {@link FollowLinkConfig}s to * use on the retrieved object. */ export const followLink = ( linkName: keyof R['_links'], - findListOptions?: FindListOptions, - shouldEmbed = true, - useCachedVersionIfAvailable = true, - reRequestOnStale = true, - ...linksToFollow: FollowLinkConfig[] -): FollowLinkConfig => { - return { - name: linkName, + { findListOptions, shouldEmbed, useCachedVersionIfAvailable, reRequestOnStale, + isOptional + }: { + findListOptions?: FindListOptions, + shouldEmbed?: boolean, + useCachedVersionIfAvailable?: boolean, + reRequestOnStale?: boolean, + isOptional?: boolean, + } = {}, + ...linksToFollow: FollowLinkConfig[] +): FollowLinkConfig => { + const followLinkConfig = { + name: linkName, + findListOptions: hasValue(findListOptions) ? findListOptions : new FindListOptions(), + shouldEmbed: hasValue(shouldEmbed) ? shouldEmbed : true, + useCachedVersionIfAvailable: hasValue(useCachedVersionIfAvailable) ? useCachedVersionIfAvailable : true, + reRequestOnStale: hasValue(reRequestOnStale) ? reRequestOnStale : true, + isOptional: hasValue(isOptional) ? isOptional : false, linksToFollow }; + return followLinkConfig; }; diff --git a/src/app/submission/edit/submission-edit.component.spec.ts b/src/app/submission/edit/submission-edit.component.spec.ts index 7b730b7a73..8013162d85 100644 --- a/src/app/submission/edit/submission-edit.component.spec.ts +++ b/src/app/submission/edit/submission-edit.component.spec.ts @@ -18,6 +18,8 @@ import { ActivatedRouteStub } from '../../shared/testing/active-router.stub'; import { mockSubmissionObject } from '../../shared/mocks/submission.mock'; import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { ItemDataService } from '../../core/data/item-data.service'; +import { SubmissionJsonPatchOperationsServiceStub } from '../../shared/testing/submission-json-patch-operations-service.stub'; +import { SubmissionJsonPatchOperationsService } from '../../core/submission/submission-json-patch-operations.service'; describe('SubmissionEditComponent Component', () => { @@ -25,6 +27,7 @@ describe('SubmissionEditComponent Component', () => { let fixture: ComponentFixture; let submissionServiceStub: SubmissionServiceStub; let itemDataService: ItemDataService; + let submissionJsonPatchOperationsServiceStub: SubmissionJsonPatchOperationsServiceStub; let router: RouterStub; const submissionId = '826'; @@ -46,6 +49,7 @@ describe('SubmissionEditComponent Component', () => { providers: [ { provide: NotificationsService, useClass: NotificationsServiceStub }, { provide: SubmissionService, useClass: SubmissionServiceStub }, + { provide: SubmissionJsonPatchOperationsService, useClass: SubmissionJsonPatchOperationsServiceStub }, { provide: ItemDataService, useValue: itemDataService }, { provide: TranslateService, useValue: getMockTranslateService() }, { provide: Router, useValue: new RouterStub() }, @@ -60,6 +64,7 @@ describe('SubmissionEditComponent Component', () => { fixture = TestBed.createComponent(SubmissionEditComponent); comp = fixture.componentInstance; submissionServiceStub = TestBed.inject(SubmissionService as any); + submissionJsonPatchOperationsServiceStub = TestBed.inject(SubmissionJsonPatchOperationsService as any); router = TestBed.inject(Router as any); }); @@ -112,4 +117,16 @@ describe('SubmissionEditComponent Component', () => { expect(comp.submissionDefinition).toBeUndefined(); }); + describe('ngOnDestroy', () => { + it('should call delete pending json patch operations', fakeAsync(() => { + + submissionJsonPatchOperationsServiceStub.deletePendingJsonPatchOperations.and.callFake(() => { /* */ }); + comp.ngOnDestroy(); + + expect(submissionJsonPatchOperationsServiceStub.deletePendingJsonPatchOperations).toHaveBeenCalled(); + })); + + }); + + }); diff --git a/src/app/submission/edit/submission-edit.component.ts b/src/app/submission/edit/submission-edit.component.ts index 34fdcba104..154e73a765 100644 --- a/src/app/submission/edit/submission-edit.component.ts +++ b/src/app/submission/edit/submission-edit.component.ts @@ -17,6 +17,7 @@ import { Item } from '../../core/shared/item.model'; import { getAllSucceededRemoteData } from '../../core/shared/operators'; import { ItemDataService } from '../../core/data/item-data.service'; import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; +import { SubmissionJsonPatchOperationsService } from '../../core/submission/submission-json-patch-operations.service'; /** * This component allows to edit an existing workspaceitem/workflowitem. @@ -92,7 +93,8 @@ export class SubmissionEditComponent implements OnDestroy, OnInit { private router: Router, private itemDataService: ItemDataService, private submissionService: SubmissionService, - private translate: TranslateService) { + private translate: TranslateService, + private submissionJsonPatchOperationsService: SubmissionJsonPatchOperationsService) { } /** @@ -149,5 +151,7 @@ export class SubmissionEditComponent implements OnDestroy, OnInit { this.subs .filter((sub) => hasValue(sub)) .forEach((sub) => sub.unsubscribe()); + + this.submissionJsonPatchOperationsService.deletePendingJsonPatchOperations(); } } 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 b4ff31aaf6..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(() => { @@ -261,9 +261,10 @@ describe('SubmissionFormCollectionComponent Component', () => { expect(comp.toggled).toHaveBeenCalled(); }); - it('should ', () => { + it('should change collection properly', () => { spyOn(comp.collectionChange, 'emit').and.callThrough(); jsonPatchOpServiceStub.jsonPatchByResourceID.and.returnValue(of(submissionRestResponse)); + submissionServiceStub.retrieveSubmission.and.returnValue(createSuccessfulRemoteDataObject$(submissionRestResponse[0])); comp.ngOnInit(); comp.onSelect(mockCollectionList[1]); fixture.detectChanges(); 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 854a2f1db0..f90814f185 100644 --- a/src/app/submission/form/collection/submission-form-collection.component.ts +++ b/src/app/submission/form/collection/submission-form-collection.component.ts @@ -13,7 +13,7 @@ import { import { BehaviorSubject, Observable, of as observableOf, Subscription } from 'rxjs'; import { find, - map + map, mergeMap } from 'rxjs/operators'; import { Collection } from '../../../core/shared/collection.model'; @@ -27,6 +27,8 @@ import { SubmissionJsonPatchOperationsService } from '../../../core/submission/s import { CollectionDataService } from '../../../core/data/collection-data.service'; 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. @@ -141,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); } /** @@ -164,11 +166,17 @@ export class SubmissionFormCollectionComponent implements OnChanges, OnInit { this.submissionService.getSubmissionObjectLinkName(), this.submissionId, 'sections', - 'collection') - .subscribe((submissionObject: SubmissionObject[]) => { + 'collection').pipe( + mergeMap((submissionObject: SubmissionObject[]) => { + // retrieve the full submission object with embeds + return this.submissionService.retrieveSubmission(submissionObject[0].id).pipe( + getFirstSucceededRemoteDataPayload() + ); + }) + ).subscribe((submissionObject: SubmissionObject) => { this.selectedCollectionId = event.collection.id; this.selectedCollectionName$ = observableOf(event.collection.name); - this.collectionChange.emit(submissionObject[0]); + this.collectionChange.emit(submissionObject); this.submissionService.changeSubmissionCollection(this.submissionId, event.collection.id); this.processingChange$.next(false); this.cdr.detectChanges(); 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-form.component.ts b/src/app/submission/form/submission-form.component.ts index 8df0ab1658..6d4ddb4ca0 100644 --- a/src/app/submission/form/submission-form.component.ts +++ b/src/app/submission/form/submission-form.component.ts @@ -122,7 +122,7 @@ export class SubmissionFormComponent implements OnChanges, OnDestroy { * Initialize all instance variables and retrieve form configuration */ ngOnChanges(changes: SimpleChanges) { - if (this.collectionId && this.submissionId) { + if ((changes.collectionId && this.collectionId) && (changes.submissionId && this.submissionId)) { this.isActive = true; // retrieve submission's section list 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 4a943276e5..823cbb5d82 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,53 +148,54 @@ 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] - ); + 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', () => { + Object.keys(mockSectionsData).forEach((sectionId) => { + expect(sectionsServiceStub.updateSectionData).toHaveBeenCalledWith( + submissionId, + sectionId, + mockSectionsData[sectionId], + expectedErrors[sectionId] + ); + }); - const responseErrors = mockUploadResponse2Errors; + expect(notificationsServiceStub.success).toHaveBeenCalled(); - 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], - expectedErrors[sectionId] - ); }); - 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] + ); + }); + + expect(notificationsServiceStub.success).not.toHaveBeenCalled(); + + }); }); - }); }); @@ -208,7 +208,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 2c348d2c87..b1b5051458 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); }); } diff --git a/src/app/submission/sections/form/section-form-operations.service.spec.ts b/src/app/submission/sections/form/section-form-operations.service.spec.ts index c76a15abcb..d5798b82c8 100644 --- a/src/app/submission/sections/form/section-form-operations.service.spec.ts +++ b/src/app/submission/sections/form/section-form-operations.service.spec.ts @@ -4,7 +4,8 @@ import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; import { DYNAMIC_FORM_CONTROL_TYPE_ARRAY, DYNAMIC_FORM_CONTROL_TYPE_GROUP, - DynamicFormControlEvent + DynamicFormControlEvent, + DynamicInputModel } from '@ng-dynamic-forms/core'; import { FormBuilderService } from '../../../shared/form/builder/form-builder.service'; @@ -28,6 +29,7 @@ import { } from '../../../shared/mocks/form-models.mock'; import { FormFieldMetadataValueObject } from '../../../shared/form/builder/models/form-field-metadata-value.model'; import { VocabularyEntry } from '../../../core/submission/vocabularies/models/vocabulary-entry.model'; +import { DynamicRowArrayModel } from '../../../shared/form/builder/ds-dynamic-form-ui/models/ds-dynamic-row-array-model'; describe('SectionFormOperationsService test suite', () => { let formBuilderService: any; @@ -83,6 +85,11 @@ describe('SectionFormOperationsService test suite', () => { formBuilderService = TestBed.inject(FormBuilderService); }); + afterEach(() => { + jsonPatchOpBuilder.add.calls.reset(); + jsonPatchOpBuilder.remove.calls.reset(); + }); + describe('dispatchOperationsFromEvent', () => { it('should call dispatchOperationsFromRemoveEvent on remove event', () => { const previousValue = new FormFieldPreviousValueObject(['path', 'test'], 'value'); @@ -567,7 +574,7 @@ describe('SectionFormOperationsService test suite', () => { }); it('should dispatch a json-path remove operation when has a stored value', () => { - const previousValue = new FormFieldPreviousValueObject(['path', 'test'], 'value'); + let previousValue = new FormFieldPreviousValueObject(['path', 'test'], 'value'); const event = Object.assign({}, dynamicFormControlChangeEvent, { model: { parent: mockRowGroupModel @@ -590,6 +597,7 @@ describe('SectionFormOperationsService test suite', () => { spyIndex.and.returnValue(1); spyPath.and.returnValue('path/1'); + previousValue = new FormFieldPreviousValueObject(['path', 'test'], 'value'); serviceAsAny.dispatchOperationsFromChangeEvent(pathCombiner, event, previousValue, true); expect(jsonPatchOpBuilder.remove).toHaveBeenCalledWith(pathCombiner.getPath('path/1')); @@ -620,6 +628,32 @@ describe('SectionFormOperationsService test suite', () => { new FormFieldMetadataValueObject('test')); }); + it('should dispatch a json-path add operation when has a stored value but previous value is empty', () => { + const previousValue = new FormFieldPreviousValueObject(['path', 'test'], null); + const event = Object.assign({}, dynamicFormControlChangeEvent, { + model: { + parent: mockRowGroupModel + } + }); + spyOn(service, 'getFieldPathFromEvent').and.returnValue('path/0'); + spyOn(service, 'getFieldPathSegmentedFromChangeEvent').and.returnValue('path'); + spyOn(service, 'getFieldValueFromChangeEvent').and.returnValue(new FormFieldMetadataValueObject('test')); + spyOn(service, 'getArrayIndexFromEvent').and.returnValue(0); + spyOn(serviceAsAny, 'getValueMap'); + spyOn(serviceAsAny, 'dispatchOperationsFromMap'); + formBuilderService.isQualdropGroup.and.returnValue(false); + formBuilderService.isRelationGroup.and.returnValue(false); + formBuilderService.hasArrayGroupValue.and.returnValue(false); + spyOn(previousValue, 'isPathEqual').and.returnValue(false); + + serviceAsAny.dispatchOperationsFromChangeEvent(pathCombiner, event, previousValue, true); + + expect(jsonPatchOpBuilder.add).toHaveBeenCalledWith( + pathCombiner.getPath('path'), + new FormFieldMetadataValueObject('test'), + true); + }); + it('should dispatch a json-path add operation when has a value and field index is zero or undefined', () => { const previousValue = new FormFieldPreviousValueObject(['path', 'test'], 'value'); const event = Object.assign({}, dynamicFormControlChangeEvent, { @@ -760,4 +794,86 @@ describe('SectionFormOperationsService test suite', () => { }); }); + describe('handleArrayGroupPatch', () => { + let arrayModel; + let previousValue; + beforeEach(() => { + arrayModel = new DynamicRowArrayModel( + { + id: 'testFormRowArray', + initialCount: 5, + notRepeatable: false, + relationshipConfig: undefined, + submissionId: '1234', + isDraggable: true, + groupFactory: () => { + return [ + new DynamicInputModel({ id: 'testFormRowArrayGroupInput' }) + ]; + }, + required: false, + metadataKey: 'dc.contributor.author', + metadataFields: ['dc.contributor.author'], + hasSelectableMetadata: true + } + ); + spyOn(serviceAsAny, 'getFieldPathSegmentedFromChangeEvent').and.returnValue('path'); + previousValue = new FormFieldPreviousValueObject(['path'], null); + }); + + it('should not dispatch a json-path operation when a array value is empty', () => { + formBuilderService.getValueFromModel.and.returnValue({}); + spyOn(previousValue, 'isPathEqual').and.returnValue(false); + + serviceAsAny.handleArrayGroupPatch( + pathCombiner, + dynamicFormControlChangeEvent, + arrayModel, + previousValue + ); + + expect(jsonPatchOpBuilder.add).not.toHaveBeenCalled(); + expect(jsonPatchOpBuilder.remove).not.toHaveBeenCalled(); + }); + + it('should dispatch a json-path add operation when a array value is not empty', () => { + const pathValue = [ + new FormFieldMetadataValueObject('test'), + new FormFieldMetadataValueObject('test two') + ]; + formBuilderService.getValueFromModel.and.returnValue({ + path:pathValue + }); + spyOn(previousValue, 'isPathEqual').and.returnValue(false); + + serviceAsAny.handleArrayGroupPatch( + pathCombiner, + dynamicFormControlChangeEvent, + arrayModel, + previousValue + ); + + expect(jsonPatchOpBuilder.add).toHaveBeenCalledWith( + pathCombiner.getPath('path'), + pathValue, + false + ); + expect(jsonPatchOpBuilder.remove).not.toHaveBeenCalled(); + }); + + it('should dispatch a json-path remove operation when a array value is empty and has previous value', () => { + formBuilderService.getValueFromModel.and.returnValue({}); + spyOn(previousValue, 'isPathEqual').and.returnValue(true); + + serviceAsAny.handleArrayGroupPatch( + pathCombiner, + dynamicFormControlChangeEvent, + arrayModel, + previousValue + ); + + expect(jsonPatchOpBuilder.add).not.toHaveBeenCalled(); + expect(jsonPatchOpBuilder.remove).toHaveBeenCalledWith(pathCombiner.getPath('path')); + }); + }); }); diff --git a/src/app/submission/sections/form/section-form-operations.service.ts b/src/app/submission/sections/form/section-form-operations.service.ts index a1bb99e3cd..adba46bf3a 100644 --- a/src/app/submission/sections/form/section-form-operations.service.ts +++ b/src/app/submission/sections/form/section-form-operations.service.ts @@ -6,7 +6,8 @@ import { DYNAMIC_FORM_CONTROL_TYPE_GROUP, DynamicFormArrayGroupModel, DynamicFormControlEvent, - DynamicFormControlModel, isDynamicFormControlEvent + DynamicFormControlModel, + isDynamicFormControlEvent } from '@ng-dynamic-forms/core'; import { hasValue, isNotEmpty, isNotNull, isNotUndefined, isNull, isUndefined } from '../../../shared/empty.util'; @@ -297,17 +298,14 @@ export class SectionFormOperationsService { event: DynamicFormControlEvent, previousValue: FormFieldPreviousValueObject): void { - if (event.context && event.context instanceof DynamicFormArrayGroupModel) { - // Model is a DynamicRowArrayModel - this.handleArrayGroupPatch(pathCombiner, event, (event as any).context.context); - return; - } - const path = this.getFieldPathFromEvent(event); const value = this.getFieldValueFromChangeEvent(event); console.log(value); if (this.formBuilder.isQualdropGroup(event.model as DynamicFormControlModel)) { this.dispatchOperationsFromMap(this.getQualdropValueMap(event), pathCombiner, event, previousValue); + } else if (event.context && event.context instanceof DynamicFormArrayGroupModel) { + // Model is a DynamicRowArrayModel + this.handleArrayGroupPatch(pathCombiner, event, (event as any).context.context, previousValue); } else if ((isNotEmpty(value) && typeof value === 'string') || (isNotEmpty(value) && value instanceof FormFieldMetadataValueObject && value.hasValue())) { this.operationsBuilder.remove(pathCombiner.getPath(path)); } @@ -368,7 +366,7 @@ export class SectionFormOperationsService { if (event.context && event.context instanceof DynamicFormArrayGroupModel) { // Model is a DynamicRowArrayModel - this.handleArrayGroupPatch(pathCombiner, event, (event as any).context.context); + this.handleArrayGroupPatch(pathCombiner, event, (event as any).context.context, previousValue); return; } @@ -388,7 +386,7 @@ export class SectionFormOperationsService { this.operationsBuilder.add( pathCombiner.getPath(segmentedPath), value, true); - } else if (previousValue.isPathEqual(this.formBuilder.getPath(event.model)) || hasStoredValue) { + } else if (previousValue.isPathEqual(this.formBuilder.getPath(event.model)) || (hasStoredValue && isNotEmpty(previousValue.value)) ) { // Here model has a previous value changed or stored in the server if (hasValue(event.$event) && hasValue(event.$event.previousIndex)) { if (event.$event.previousIndex < 0) { @@ -421,7 +419,7 @@ export class SectionFormOperationsService { previousValue.delete(); } else if (value.hasValue()) { // Here model has no previous value but a new one - if (isUndefined(this.getArrayIndexFromEvent(event)) || this.getArrayIndexFromEvent(event) === 0) { + if (isUndefined(this.getArrayIndexFromEvent(event)) || this.getArrayIndexFromEvent(event) === 0) { // Model is single field or is part of an array model but is the first item, // so dispatch an add operation that initialize the values of a specific metadata this.operationsBuilder.add( @@ -498,23 +496,37 @@ export class SectionFormOperationsService { event: DynamicFormControlEvent, previousValue: FormFieldPreviousValueObject) { - return this.handleArrayGroupPatch(pathCombiner, event.$event, (event as any).$event.arrayModel); + return this.handleArrayGroupPatch(pathCombiner, event.$event, (event as any).$event.arrayModel, previousValue); } /** * Specific patch handler for a DynamicRowArrayModel. * Configure a Patch ADD with the current array value. * @param pathCombiner + * the [[JsonPatchOperationPathCombiner]] object for the specified operation * @param event + * the [[DynamicFormControlEvent]] for the specified operation * @param model + * the [[DynamicRowArrayModel]] model + * @param previousValue + * the [[FormFieldPreviousValueObject]] for the specified operation */ private handleArrayGroupPatch(pathCombiner: JsonPatchOperationPathCombiner, event, - model: DynamicRowArrayModel) { + model: DynamicRowArrayModel, + previousValue: FormFieldPreviousValueObject) { + const arrayValue = this.formBuilder.getValueFromModel([model]); - const segmentedPath2 = this.getFieldPathSegmentedFromChangeEvent(event); - this.operationsBuilder.add( - pathCombiner.getPath(segmentedPath2), - arrayValue[segmentedPath2], false); + const segmentedPath = this.getFieldPathSegmentedFromChangeEvent(event); + if (isNotEmpty(arrayValue)) { + this.operationsBuilder.add( + pathCombiner.getPath(segmentedPath), + arrayValue[segmentedPath], + false + ); + } else if (previousValue.isPathEqual(this.formBuilder.getPath(event.model))) { + this.operationsBuilder.remove(pathCombiner.getPath(segmentedPath)); + } + } } diff --git a/src/app/submission/sections/sections.service.spec.ts b/src/app/submission/sections/sections.service.spec.ts index 0d63beeaea..d199f1beae 100644 --- a/src/app/submission/sections/sections.service.spec.ts +++ b/src/app/submission/sections/sections.service.spec.ts @@ -334,6 +334,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 d8d1491cb7..05e9a96267 100644 --- a/src/app/submission/sections/sections.service.ts +++ b/src/app/submission/sections/sections.service.ts @@ -328,6 +328,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.scss b/src/app/submission/sections/upload/file/edit/section-upload-file-edit.component.scss new file mode 100644 index 0000000000..b443db711b --- /dev/null +++ b/src/app/submission/sections/upload/file/edit/section-upload-file-edit.component.scss @@ -0,0 +1,6 @@ + +::ng-deep .access-condition-group { + position: relative; + top: -2.3rem; + margin-bottom: -2.3rem; +} 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 512453d84e..3275787984 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 @@ -18,6 +18,8 @@ import { import { WorkspaceitemSectionUploadFileObject } from '../../../../../core/submission/models/workspaceitem-section-upload-file.model'; import { FormBuilderService } from '../../../../../shared/form/builder/form-builder.service'; import { + BITSTREAM_ACCESS_CONDITION_GROUP_CONFIG, + BITSTREAM_ACCESS_CONDITION_GROUP_LAYOUT, BITSTREAM_ACCESS_CONDITIONS_FORM_ARRAY_CONFIG, BITSTREAM_ACCESS_CONDITIONS_FORM_ARRAY_LAYOUT, BITSTREAM_FORM_ACCESS_CONDITION_END_DATE_CONFIG, @@ -43,6 +45,7 @@ import { FormComponent } from '../../../../../shared/form/form.component'; */ @Component({ selector: 'ds-submission-section-upload-file-edit', + styleUrls: ['./section-upload-file-edit.component.scss'], templateUrl: './section-upload-file-edit.component.html', }) export class SubmissionSectionUploadFileEditComponent implements OnChanges { @@ -209,8 +212,9 @@ export class SubmissionSectionUploadFileEditComponent implements OnChanges { const startDate = new DynamicDatePickerModel(startDateConfig, BITSTREAM_FORM_ACCESS_CONDITION_START_DATE_LAYOUT); const endDate = new DynamicDatePickerModel(endDateConfig, BITSTREAM_FORM_ACCESS_CONDITION_END_DATE_LAYOUT); - - return [type, startDate, endDate]; + const accessConditionGroupConfig = Object.assign({}, BITSTREAM_ACCESS_CONDITION_GROUP_CONFIG); + accessConditionGroupConfig.group = [type, startDate, endDate]; + return [new DynamicFormGroupModel(accessConditionGroupConfig, BITSTREAM_ACCESS_CONDITION_GROUP_LAYOUT)]; }; // Number of access conditions blocks in form @@ -233,7 +237,7 @@ export class SubmissionSectionUploadFileEditComponent implements OnChanges { public initModelData(formModel: DynamicFormControlModel[]) { this.fileData.accessConditions.forEach((accessCondition, index) => { Array.of('name', 'startDate', 'endDate') - .filter((key) => accessCondition.hasOwnProperty(key)) + .filter((key) => accessCondition.hasOwnProperty(key) && isNotEmpty(accessCondition[key])) .forEach((key) => { const metadataModel: any = this.formBuilderService.findById(key, formModel, index); if (metadataModel) { diff --git a/src/app/submission/sections/upload/file/edit/section-upload-file-edit.model.ts b/src/app/submission/sections/upload/file/edit/section-upload-file-edit.model.ts index 096954659e..300a4b461f 100644 --- a/src/app/submission/sections/upload/file/edit/section-upload-file-edit.model.ts +++ b/src/app/submission/sections/upload/file/edit/section-upload-file-edit.model.ts @@ -15,12 +15,24 @@ export const BITSTREAM_METADATA_FORM_GROUP_CONFIG: DynamicFormGroupModelConfig = export const BITSTREAM_METADATA_FORM_GROUP_LAYOUT: DynamicFormControlLayout = { element: { container: 'form-group', - label: 'col-form-label' + label: 'col-form-label' }, grid: { label: 'col-sm-3' } }; +export const BITSTREAM_ACCESS_CONDITION_GROUP_CONFIG: DynamicFormGroupModelConfig = { + id: 'accessConditionGroup', + group: [] +}; + +export const BITSTREAM_ACCESS_CONDITION_GROUP_LAYOUT: DynamicFormControlLayout = { + element: { + host: 'form-group flex-fill access-condition-group', + container: 'pl-1 pr-1', + control: 'form-row ' + } +}; export const BITSTREAM_ACCESS_CONDITIONS_FORM_ARRAY_CONFIG: DynamicFormArrayModelConfig = { id: 'accessConditions', @@ -28,7 +40,7 @@ export const BITSTREAM_ACCESS_CONDITIONS_FORM_ARRAY_CONFIG: DynamicFormArrayMode }; export const BITSTREAM_ACCESS_CONDITIONS_FORM_ARRAY_LAYOUT: DynamicFormControlLayout = { grid: { - group: 'form-row' + group: 'form-row pt-4', } }; @@ -39,11 +51,8 @@ export const BITSTREAM_FORM_ACCESS_CONDITION_TYPE_CONFIG: DynamicSelectModelConf }; export const BITSTREAM_FORM_ACCESS_CONDITION_TYPE_LAYOUT: DynamicFormControlLayout = { element: { - container: 'p-0', - label: 'col-form-label' - }, - grid: { - host: 'col-md-10' + host: 'col-12', + label: 'col-form-label name-label' } }; @@ -70,11 +79,10 @@ export const BITSTREAM_FORM_ACCESS_CONDITION_START_DATE_CONFIG: DynamicDatePicke }; export const BITSTREAM_FORM_ACCESS_CONDITION_START_DATE_LAYOUT: DynamicFormControlLayout = { element: { - container: 'p-0', label: 'col-form-label' }, grid: { - host: 'col-md-4' + host: 'col-6' } }; @@ -101,10 +109,9 @@ export const BITSTREAM_FORM_ACCESS_CONDITION_END_DATE_CONFIG: DynamicDatePickerM }; export const BITSTREAM_FORM_ACCESS_CONDITION_END_DATE_LAYOUT: DynamicFormControlLayout = { element: { - container: 'p-0', label: 'col-form-label' }, grid: { - host: 'col-md-4' + host: 'col-6' } }; 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..221d396a39 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,8 +10,9 @@
- - + + +