diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 5c0163bf8e..be15b0a507 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,7 +1,7 @@ ## References _Add references/links to any related issues or PRs. These may include:_ -* Fixes [GitHub issue](https://github.com/DSpace/dspace-angular/issues), if any -* Requires [REST API PR](https://github.com/DSpace/DSpace/pulls), if any +* Fixes #[issue-number] +* Requires DSpace/DSpace#[pr-number] (if a REST API PR is required to test this) ## Description Short summary of changes (1-2 sentences). diff --git a/src/app/+admin/admin-workflow-page/admin-workflow-search-results/admin-workflow-search-result-grid-element/workflow-item/workflow-item-search-result-admin-workflow-grid-element.component.spec.ts b/src/app/+admin/admin-workflow-page/admin-workflow-search-results/admin-workflow-search-result-grid-element/workflow-item/workflow-item-search-result-admin-workflow-grid-element.component.spec.ts index 2f3f88fa70..6436f2d873 100644 --- a/src/app/+admin/admin-workflow-page/admin-workflow-search-results/admin-workflow-search-result-grid-element/workflow-item/workflow-item-search-result-admin-workflow-grid-element.component.spec.ts +++ b/src/app/+admin/admin-workflow-page/admin-workflow-search-results/admin-workflow-search-result-grid-element/workflow-item/workflow-item-search-result-admin-workflow-grid-element.component.spec.ts @@ -18,6 +18,7 @@ import { WorkflowItemSearchResult } from '../../../../../shared/object-collectio import { BitstreamDataService } from '../../../../../core/data/bitstream-data.service'; import { createSuccessfulRemoteDataObject$ } from '../../../../../shared/remote-data.utils'; import { getMockLinkService } from '../../../../../shared/mocks/link-service.mock'; +import { of as observableOf } from 'rxjs'; describe('WorkflowItemAdminWorkflowGridElementComponent', () => { let component: WorkflowItemSearchResultAdminWorkflowGridElementComponent; @@ -50,7 +51,9 @@ describe('WorkflowItemAdminWorkflowGridElementComponent', () => { ], providers: [ { provide: LinkService, useValue: linkService }, - { provide: TruncatableService, useValue: {} }, + { provide: TruncatableService, useValue: { + isCollapsed: () => observableOf(true), + } }, { provide: BitstreamDataService, useValue: {} }, ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/+bitstream-page/bitstream-page.resolver.ts b/src/app/+bitstream-page/bitstream-page.resolver.ts index 8e9f64fcc1..9c85688c39 100644 --- a/src/app/+bitstream-page/bitstream-page.resolver.ts +++ b/src/app/+bitstream-page/bitstream-page.resolver.ts @@ -6,6 +6,7 @@ import { find } from 'rxjs/operators'; import { hasValue } from '../shared/empty.util'; import { Bitstream } from '../core/shared/bitstream.model'; import { BitstreamDataService } from '../core/data/bitstream-data.service'; +import {followLink, FollowLinkConfig} from '../shared/utils/follow-link-config.model'; /** * This class represents a resolver that requests a specific bitstream before the route is activated @@ -23,9 +24,20 @@ export class BitstreamPageResolver implements Resolve> { * or an error if something went wrong */ resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable> { - return this.bitstreamService.findById(route.params.id) + return this.bitstreamService.findById(route.params.id, ...this.followLinks) .pipe( find((RD) => hasValue(RD.error) || RD.hasSucceeded), ); } + /** + * Method that returns the follow links to already resolve + * The self links defined in this list are expected to be requested somewhere in the near future + * Requesting them as embeds will limit the number of requests + */ + get followLinks(): Array> { + return [ + followLink('bundle', undefined, true, followLink('item')), + followLink('format') + ]; + } } 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 c802622dc4..ce46c2a7b3 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 @@ -4,7 +4,7 @@ import { TranslateModule } from '@ngx-translate/core'; import { RouterTestingModule } from '@angular/router/testing'; import { RemoteData } from '../../core/data/remote-data'; import { of as observableOf } from 'rxjs/internal/observable/of'; -import { ActivatedRoute } from '@angular/router'; +import {ActivatedRoute, Router} from '@angular/router'; import { DynamicFormControlModel, DynamicFormService } from '@ng-dynamic-forms/core'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { BitstreamDataService } from '../../core/data/bitstream-data.service'; @@ -22,6 +22,11 @@ import { PageInfo } from '../../core/shared/page-info.model'; import { FileSizePipe } from '../../shared/utils/file-size-pipe'; import { RestResponse } from '../../core/cache/response.models'; import { VarDirective } from '../../shared/utils/var.directive'; +import { + createSuccessfulRemoteDataObject$ +} from '../../shared/remote-data.utils'; +import {RouterStub} from '../../shared/testing/router.stub'; +import { getItemEditRoute } from '../../+item-page/item-page-routing-paths'; const infoNotification: INotification = new Notification('id', NotificationType.Info, 'info'); const warningNotification: INotification = new Notification('id', NotificationType.Warning, 'warning'); @@ -34,6 +39,8 @@ let bitstreamFormatService: BitstreamFormatDataService; let bitstream: Bitstream; let selectedFormat: BitstreamFormat; let allFormats: BitstreamFormat[]; +let router: Router; +let routerStub; describe('EditBitstreamPageComponent', () => { let comp: EditBitstreamPageComponent; @@ -105,7 +112,12 @@ describe('EditBitstreamPageComponent', () => { format: observableOf(new RemoteData(false, false, true, null, selectedFormat)), _links: { self: 'bitstream-selflink' - } + }, + bundle: createSuccessfulRemoteDataObject$({ + item: createSuccessfulRemoteDataObject$({ + uuid: 'some-uuid' + }) + }) }); bitstreamService = jasmine.createSpyObj('bitstreamService', { findById: observableOf(new RemoteData(false, false, true, null, bitstream)), @@ -118,6 +130,10 @@ describe('EditBitstreamPageComponent', () => { findAll: observableOf(new RemoteData(false, false, true, null, new PaginatedList(new PageInfo(), allFormats))) }); + const itemPageUrl = `fake-url/some-uuid`; + routerStub = Object.assign(new RouterStub(), { + url: `${itemPageUrl}` + }); TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), RouterTestingModule], declarations: [EditBitstreamPageComponent, FileSizePipe, VarDirective], @@ -127,6 +143,7 @@ describe('EditBitstreamPageComponent', () => { { provide: ActivatedRoute, useValue: { data: observableOf({ bitstream: new RemoteData(false, false, true, null, bitstream) }), snapshot: { queryParams: {} } } }, { provide: BitstreamDataService, useValue: bitstreamService }, { provide: BitstreamFormatDataService, useValue: bitstreamFormatService }, + { provide: Router, useValue: routerStub }, ChangeDetectorRef ], schemas: [NO_ERRORS_SCHEMA] @@ -138,6 +155,7 @@ describe('EditBitstreamPageComponent', () => { fixture = TestBed.createComponent(EditBitstreamPageComponent); comp = fixture.componentInstance; fixture.detectChanges(); + router = (comp as any).router; }); describe('on startup', () => { @@ -213,4 +231,25 @@ describe('EditBitstreamPageComponent', () => { }); }); }); + describe('when the cancel button is clicked', () => { + it('should call navigateToItemEditBitstreams method', () => { + spyOn(comp, 'navigateToItemEditBitstreams'); + comp.onCancel(); + expect(comp.navigateToItemEditBitstreams).toHaveBeenCalled(); + }); + }); + describe('when navigateToItemEditBitstreams is called, and the component has an itemId', () => { + 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([getItemEditRoute('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([getItemEditRoute('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 3e8b686e48..ad64739dac 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 @@ -1,7 +1,7 @@ import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from '@angular/core'; import { Bitstream } from '../../core/shared/bitstream.model'; import { ActivatedRoute, Router } from '@angular/router'; -import { filter, map, switchMap } from 'rxjs/operators'; +import { map, mergeMap, switchMap} from 'rxjs/operators'; import { combineLatest as observableCombineLatest, of as observableOf } from 'rxjs'; import { Subscription } from 'rxjs/internal/Subscription'; import { @@ -19,7 +19,7 @@ import { DynamicCustomSwitchModel } from '../../shared/form/builder/ds-dynamic-f import { cloneDeep } from 'lodash'; import { BitstreamDataService } from '../../core/data/bitstream-data.service'; import { - getAllSucceededRemoteData, getAllSucceededRemoteDataPayload, + getAllSucceededRemoteDataPayload, getFirstSucceededRemoteDataPayload, getRemoteDataPayload, getSucceededRemoteData @@ -35,8 +35,9 @@ import { Location } from '@angular/common'; import { Observable } from 'rxjs/internal/Observable'; import { RemoteData } from '../../core/data/remote-data'; import { PaginatedList } from '../../core/data/paginated-list'; -import { followLink } from '../../shared/utils/follow-link-config.model'; import { getItemEditRoute } from '../../+item-page/item-page-routing-paths'; +import {Bundle} from '../../core/shared/bundle.model'; +import {Item} from '../../core/shared/item.model'; @Component({ selector: 'ds-edit-bitstream-page', @@ -299,12 +300,7 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { const bitstream$ = this.bitstreamRD$.pipe( getSucceededRemoteData(), - getRemoteDataPayload(), - switchMap((bitstream: Bitstream) => this.bitstreamService.findById(bitstream.id, followLink('format')).pipe( - getAllSucceededRemoteData(), - getRemoteDataPayload(), - filter((bs: Bitstream) => hasValue(bs))) - ) + getRemoteDataPayload() ); const allFormats$ = this.bitstreamFormatsRD$.pipe( @@ -501,14 +497,18 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { } /** - * When the item ID is present, navigate back to the item's edit bitstreams page, otherwise go back to the previous - * page the user came from + * When the item ID is present, navigate back to the item's edit bitstreams page, + * otherwise retrieve the item ID based on the owning bundle's link */ navigateToItemEditBitstreams() { if (hasValue(this.itemId)) { this.router.navigate([getItemEditRoute(this.itemId), 'bitstreams']); } else { - this.location.back(); + this.bitstream.bundle.pipe(getFirstSucceededRemoteDataPayload(), + mergeMap((bundle: Bundle) => bundle.item.pipe(getFirstSucceededRemoteDataPayload(), map((item: Item) => item.uuid)))) + .subscribe((item) => { + this.router.navigate(([getItemEditRoute(item), 'bitstreams'])); + }); } } diff --git a/src/app/+collection-page/collection-page.component.ts b/src/app/+collection-page/collection-page.component.ts index c7d287ed6a..a390bda4cb 100644 --- a/src/app/+collection-page/collection-page.component.ts +++ b/src/app/+collection-page/collection-page.component.ts @@ -17,7 +17,7 @@ import { DSpaceObjectType } from '../core/shared/dspace-object-type.model'; import { Item } from '../core/shared/item.model'; import { getSucceededRemoteData, - redirectToPageNotFoundOn404, + redirectOn404Or401, toDSpaceObjectListRD } from '../core/shared/operators'; @@ -63,7 +63,7 @@ export class CollectionPageComponent implements OnInit { ngOnInit(): void { this.collectionRD$ = this.route.data.pipe( map((data) => data.dso as RemoteData), - redirectToPageNotFoundOn404(this.router), + redirectOn404Or401(this.router), take(1) ); this.logoRD$ = this.collectionRD$.pipe( diff --git a/src/app/+community-page/community-page.component.ts b/src/app/+community-page/community-page.component.ts index 3621829927..7c93b1bf4e 100644 --- a/src/app/+community-page/community-page.component.ts +++ b/src/app/+community-page/community-page.component.ts @@ -13,7 +13,7 @@ import { MetadataService } from '../core/metadata/metadata.service'; import { fadeInOut } from '../shared/animations/fade'; import { hasValue } from '../shared/empty.util'; -import { redirectToPageNotFoundOn404 } from '../core/shared/operators'; +import { redirectOn404Or401 } from '../core/shared/operators'; @Component({ selector: 'ds-community-page', @@ -47,7 +47,7 @@ export class CommunityPageComponent implements OnInit { ngOnInit(): void { this.communityRD$ = this.route.data.pipe( map((data) => data.dso as RemoteData), - redirectToPageNotFoundOn404(this.router) + redirectOn404Or401(this.router) ); this.logoRD$ = this.communityRD$.pipe( map((rd: RemoteData) => rd.payload), diff --git a/src/app/+item-page/simple/item-page.component.ts b/src/app/+item-page/simple/item-page.component.ts index 10deef23e4..87d2294ff9 100644 --- a/src/app/+item-page/simple/item-page.component.ts +++ b/src/app/+item-page/simple/item-page.component.ts @@ -11,7 +11,7 @@ import { Item } from '../../core/shared/item.model'; import { MetadataService } from '../../core/metadata/metadata.service'; import { fadeInOut } from '../../shared/animations/fade'; -import { redirectToPageNotFoundOn404 } from '../../core/shared/operators'; +import { redirectOn404Or401 } from '../../core/shared/operators'; import { ViewMode } from '../../core/shared/view-mode.model'; /** @@ -56,7 +56,7 @@ export class ItemPageComponent implements OnInit { ngOnInit(): void { this.itemRD$ = this.route.data.pipe( map((data) => data.item as RemoteData), - redirectToPageNotFoundOn404(this.router) + redirectOn404Or401(this.router) ); this.metadataService.processRemoteData(this.itemRD$); } diff --git a/src/app/app-routing-paths.ts b/src/app/app-routing-paths.ts index 4e64a4a552..2b57a1957c 100644 --- a/src/app/app-routing-paths.ts +++ b/src/app/app-routing-paths.ts @@ -55,12 +55,18 @@ export function getDSORoute(dso: DSpaceObject): string { } } -export const UNAUTHORIZED_PATH = 'unauthorized'; +export const UNAUTHORIZED_PATH = '401'; export function getUnauthorizedRoute() { return `/${UNAUTHORIZED_PATH}`; } +export const PAGE_NOT_FOUND_PATH = '404'; + +export function getPageNotFoundRoute() { + return `/${PAGE_NOT_FOUND_PATH}`; +} + export const INFO_MODULE_PATH = 'info'; export function getInfoModulePath() { return `/${INFO_MODULE_PATH}`; diff --git a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts index 09292fec21..03d4db3f5d 100644 --- a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts +++ b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts @@ -3,12 +3,13 @@ import { Injectable } from '@angular/core'; import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot } from '@angular/router'; import { DSOBreadcrumbsService } from './dso-breadcrumbs.service'; import { DataService } from '../data/data.service'; -import { getRemoteDataPayload, getSucceededRemoteData } from '../shared/operators'; -import { map } from 'rxjs/operators'; +import { getRemoteDataPayload } from '../shared/operators'; +import { filter, map, take } from 'rxjs/operators'; import { Observable } from 'rxjs'; import { DSpaceObject } from '../shared/dspace-object.model'; import { ChildHALResource } from '../shared/child-hal-resource.model'; import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; +import { hasValue } from '../../shared/empty.util'; /** * The class that resolves the BreadcrumbConfig object for a DSpaceObject @@ -29,12 +30,17 @@ export abstract class DSOBreadcrumbResolver> { const uuid = route.params.id; return this.dataService.findById(uuid, ...this.followLinks).pipe( - getSucceededRemoteData(), + filter((rd) => hasValue(rd.error) || hasValue(rd.payload)), + take(1), getRemoteDataPayload(), map((object: T) => { - const fullPath = state.url; - const url = fullPath.substr(0, fullPath.indexOf(uuid)) + uuid; - return { provider: this.breadcrumbService, key: object, url: url }; + if (hasValue(object)) { + const fullPath = state.url; + const url = fullPath.substr(0, fullPath.indexOf(uuid)) + uuid; + return {provider: this.breadcrumbService, key: object, url: url}; + } else { + return undefined; + } }) ); } diff --git a/src/app/core/shared/bitstream.model.ts b/src/app/core/shared/bitstream.model.ts index ab9d1548b7..314818b482 100644 --- a/src/app/core/shared/bitstream.model.ts +++ b/src/app/core/shared/bitstream.model.ts @@ -8,6 +8,8 @@ import { BITSTREAM } from './bitstream.resource-type'; import { DSpaceObject } from './dspace-object.model'; import { HALLink } from './hal-link.model'; import { HALResource } from './hal-resource.model'; +import {BUNDLE} from './bundle.resource-type'; +import {Bundle} from './bundle.model'; @typedObject @inheritSerialization(DSpaceObject) @@ -57,4 +59,10 @@ export class Bitstream extends DSpaceObject implements HALResource { @link(BITSTREAM_FORMAT, false, 'format') format?: Observable>; + /** + * The owning bundle for this Bitstream + * Will be undefined unless the bundle{@link HALLink} has been resolved. + */ + @link(BUNDLE) + bundle?: Observable>; } diff --git a/src/app/core/shared/bundle.model.ts b/src/app/core/shared/bundle.model.ts index 1e5c14d486..c84b1f691f 100644 --- a/src/app/core/shared/bundle.model.ts +++ b/src/app/core/shared/bundle.model.ts @@ -10,6 +10,8 @@ import { RemoteData } from '../data/remote-data'; import { PaginatedList } from '../data/paginated-list'; import { BITSTREAM } from './bitstream.resource-type'; import { Bitstream } from './bitstream.model'; +import {ITEM} from './item.resource-type'; +import {Item} from './item.model'; @typedObject @inheritSerialization(DSpaceObject) @@ -24,6 +26,7 @@ export class Bundle extends DSpaceObject { self: HALLink; primaryBitstream: HALLink; bitstreams: HALLink; + item: HALLink; }; /** @@ -39,4 +42,11 @@ export class Bundle extends DSpaceObject { */ @link(BITSTREAM, true) bitstreams?: Observable>>; + + /** + * The owning item for this Bundle + * Will be undefined unless the Item{@link HALLink} has been resolved. + */ + @link(ITEM) + item?: Observable>; } diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index a19419259d..8acf5ea860 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -13,7 +13,8 @@ import { getRequestFromRequestUUID, getResourceLinksFromResponse, getResponseFromEntry, - getSucceededRemoteData, redirectToPageNotFoundOn404 + getSucceededRemoteData, + redirectOn404Or401 } from './operators'; import { RemoteData } from '../data/remote-data'; import { RemoteDataError } from '../data/remote-data-error'; @@ -199,7 +200,7 @@ describe('Core Module - RxJS Operators', () => { }); }); - describe('redirectToPageNotFoundOn404', () => { + describe('redirectOn404Or401', () => { let router; beforeEach(() => { router = jasmine.createSpyObj('router', ['navigateByUrl']); @@ -208,21 +209,28 @@ describe('Core Module - RxJS Operators', () => { it('should call navigateByUrl to a 404 page, when the remote data contains a 404 error', () => { const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(404, 'Not Found', 'Object was not found')); - observableOf(testRD).pipe(redirectToPageNotFoundOn404(router)).subscribe(); + observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); expect(router.navigateByUrl).toHaveBeenCalledWith('/404', { skipLocationChange: true }); }); - it('should not call navigateByUrl to a 404 page, when the remote data contains another error than a 404', () => { + it('should call navigateByUrl to a 401 page, when the remote data contains a 401 error', () => { + const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(401, 'Unauthorized', 'The current user is unauthorized')); + + observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/401', { skipLocationChange: true }); + }); + + it('should not call navigateByUrl to a 404 or 401 page, when the remote data contains another error than a 404 or 401', () => { const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(500, 'Server Error', 'Something went wrong')); - observableOf(testRD).pipe(redirectToPageNotFoundOn404(router)).subscribe(); + observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); expect(router.navigateByUrl).not.toHaveBeenCalled(); }); - it('should not call navigateByUrl to a 404 page, when the remote data contains no error', () => { + it('should not call navigateByUrl to a 404 or 401 page, when the remote data contains no error', () => { const testRD = createSuccessfulRemoteDataObject(undefined); - observableOf(testRD).pipe(redirectToPageNotFoundOn404(router)).subscribe(); + observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); expect(router.navigateByUrl).not.toHaveBeenCalled(); }); }); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index ad2588f2b9..29e41907e1 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -13,7 +13,7 @@ import { MetadataField } from '../metadata/metadata-field.model'; import { MetadataSchema } from '../metadata/metadata-schema.model'; import { BrowseDefinition } from './browse-definition.model'; import { DSpaceObject } from './dspace-object.model'; -import { getUnauthorizedRoute } from '../../app-routing-paths'; +import { getPageNotFoundRoute, getUnauthorizedRoute } from '../../app-routing-paths'; import { getEndUserAgreementPath } from '../../info/info-routing-paths'; /** @@ -171,16 +171,20 @@ export const getAllSucceededRemoteListPayload = () => ); /** - * Operator that checks if a remote data object contains a page not found error - * When it does contain such an error, it will redirect the user to a page not found, without altering the current URL + * Operator that checks if a remote data object returned a 401 or 404 error + * When it does contain such an error, it will redirect the user to the related error page, without altering the current URL * @param router The router used to navigate to a new page */ -export const redirectToPageNotFoundOn404 = (router: Router) => +export const redirectOn404Or401 = (router: Router) => (source: Observable>): Observable> => source.pipe( tap((rd: RemoteData) => { - if (rd.hasFailed && rd.error.statusCode === 404) { - router.navigateByUrl('/404', { skipLocationChange: true }); + if (rd.hasFailed) { + if (rd.error.statusCode === 404) { + router.navigateByUrl(getPageNotFoundRoute(), {skipLocationChange: true}); + } else if (rd.error.statusCode === 401) { + router.navigateByUrl(getUnauthorizedRoute(), {skipLocationChange: true}); + } } })); diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index b0e2c7e378..c4610b70e9 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -4,7 +4,7 @@ import { Observable } from 'rxjs/internal/Observable'; import { RemoteData } from '../../core/data/remote-data'; import { Process } from '../processes/process.model'; import { map, switchMap } from 'rxjs/operators'; -import { getFirstSucceededRemoteDataPayload, redirectToPageNotFoundOn404 } from '../../core/shared/operators'; +import { getFirstSucceededRemoteDataPayload, redirectOn404Or401 } from '../../core/shared/operators'; import { AlertType } from '../../shared/alert/aletr-type'; import { ProcessDataService } from '../../core/data/processes/process-data.service'; import { PaginatedList } from '../../core/data/paginated-list'; @@ -49,7 +49,7 @@ export class ProcessDetailComponent implements OnInit { ngOnInit(): void { this.processRD$ = this.route.data.pipe( map((data) => data.process as RemoteData), - redirectToPageNotFoundOn404(this.router) + redirectOn404Or401(this.router) ); this.filesRD$ = this.processRD$.pipe( diff --git a/src/app/shared/mocks/external-source.service.mock.ts b/src/app/shared/mocks/external-source.service.mock.ts index 85d63189e5..fd6d7cdc46 100644 --- a/src/app/shared/mocks/external-source.service.mock.ts +++ b/src/app/shared/mocks/external-source.service.mock.ts @@ -50,8 +50,7 @@ export const externalSourceMyStaffDb: ExternalSource = { /** * Mock for [[ExternalSourceService]] */ -export function getMockExternalSourceService(): -ExternalSourceService { +export function getMockExternalSourceService(): ExternalSourceService { return jasmine.createSpyObj('ExternalSourceService', { findAll: jasmine.createSpy('findAll'), getExternalSourceEntries: jasmine.createSpy('getExternalSourceEntries'), diff --git a/src/app/shared/object-grid/object-grid.component.scss b/src/app/shared/object-grid/object-grid.component.scss index fb883a1f07..53d64bc60c 100644 --- a/src/app/shared/object-grid/object-grid.component.scss +++ b/src/app/shared/object-grid/object-grid.component.scss @@ -19,6 +19,7 @@ $ds-wrapper-grid-spacing: $spacer/2; .card-columns { margin-left: -$ds-wrapper-grid-spacing; margin-right: -$ds-wrapper-grid-spacing; + column-gap: 0; .card-column { padding-left: $ds-wrapper-grid-spacing; 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 dc05f78e40..c8c413a81b 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 @@ -32,9 +32,6 @@ export class SearchResultGridElementComponent, K exten protected bitstreamDataService: BitstreamDataService ) { super(); - if (hasValue(this.object)) { - this.isCollapsed$ = this.isCollapsed(); - } } /** @@ -43,6 +40,7 @@ export class SearchResultGridElementComponent, K exten ngOnInit(): void { if (hasValue(this.object)) { this.dso = this.object.indexableObject; + this.isCollapsed$ = this.isCollapsed(); } } diff --git a/src/app/shared/object-list/search-result-list-element/item-search-result/item-types/publication/publication-search-result-list-element.component.html b/src/app/shared/object-list/search-result-list-element/item-search-result/item-types/publication/publication-search-result-list-element.component.html index bd00e4aff1..1bc723200a 100644 --- a/src/app/shared/object-list/search-result-list-element/item-search-result/item-types/publication/publication-search-result-list-element.component.html +++ b/src/app/shared/object-list/search-result-list-element/item-search-result/item-types/publication/publication-search-result-list-element.component.html @@ -7,20 +7,19 @@ - - (, ) - - - - ; - - - - + + + ( + ) + + + + + ; + + + +
diff --git a/src/app/statistics-page/statistics-page/statistics-page.component.ts b/src/app/statistics-page/statistics-page/statistics-page.component.ts index e034a35dca..31cb74eb08 100644 --- a/src/app/statistics-page/statistics-page/statistics-page.component.ts +++ b/src/app/statistics-page/statistics-page/statistics-page.component.ts @@ -4,7 +4,7 @@ import { UsageReportService } from '../../core/statistics/usage-report-data.serv import { map, switchMap } from 'rxjs/operators'; import { UsageReport } from '../../core/statistics/models/usage-report.model'; import { RemoteData } from '../../core/data/remote-data'; -import { getRemoteDataPayload, getSucceededRemoteData, redirectToPageNotFoundOn404 } from '../../core/shared/operators'; +import { getRemoteDataPayload, getSucceededRemoteData, redirectOn404Or401 } from '../../core/shared/operators'; import { DSpaceObject } from '../../core/shared/dspace-object.model'; import { ActivatedRoute, Router } from '@angular/router'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; @@ -55,7 +55,7 @@ export abstract class StatisticsPageComponent implements protected getScope$(): Observable { return this.route.data.pipe( map((data) => data.scope as RemoteData), - redirectToPageNotFoundOn404(this.router), + redirectOn404Or401(this.router), getSucceededRemoteData(), getRemoteDataPayload(), ); diff --git a/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.scss b/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.scss index 0d2a79b056..136bf12e0e 100644 --- a/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.scss +++ b/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.scss @@ -13,7 +13,7 @@ .scrollable-menu { height: auto; - max-height: $dropdown-menu-max-height; + max-height: $dropdown-menu-max-height / 2; overflow-x: hidden; } diff --git a/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.ts b/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.ts index bb156e1878..58fadacdcc 100644 --- a/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.ts +++ b/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.ts @@ -1,6 +1,6 @@ -import { ChangeDetectorRef, Component, EventEmitter, Input, OnInit, Output } from '@angular/core'; +import { ChangeDetectorRef, Component, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core'; -import { of as observableOf, Observable } from 'rxjs'; +import { Observable, of as observableOf, Subscription } from 'rxjs'; import { catchError, tap } from 'rxjs/operators'; import { ExternalSourceService } from '../../../core/data/external-source.service'; @@ -12,6 +12,7 @@ import { createSuccessfulRemoteDataObject } from '../../../shared/remote-data.ut import { FindListOptions } from '../../../core/data/request.models'; import { getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators'; import { HostWindowService } from '../../../shared/host-window.service'; +import { hasValue } from '../../../shared/empty.util'; /** * Interface for the selected external source element. @@ -37,7 +38,7 @@ export interface ExternalSourceData { styleUrls: ['./submission-import-external-searchbar.component.scss'], templateUrl: './submission-import-external-searchbar.component.html' }) -export class SubmissionImportExternalSearchbarComponent implements OnInit { +export class SubmissionImportExternalSearchbarComponent implements OnInit, OnDestroy { /** * The init external source value. */ @@ -76,6 +77,11 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit { */ protected findListOptions: FindListOptions; + /** + * The subscription to unsubscribe + */ + protected sub: Subscription; + /** * Initialize the component variables. * @param {ExternalSourceService} externalService @@ -101,7 +107,7 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit { this.sourceList = []; this.findListOptions = Object.assign({}, new FindListOptions(), { elementsPerPage: 5, - currentPage: 0, + currentPage: 1, }); this.externalService.findAll(this.findListOptions).pipe( catchError(() => { @@ -139,13 +145,13 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit { * Load the next pages of external sources. */ public onScroll(): void { - if (!this.sourceListLoading && this.pageInfo.currentPage <= this.pageInfo.totalPages) { + if (!this.sourceListLoading && ((this.pageInfo.currentPage + 1) <= this.pageInfo.totalPages)) { this.sourceListLoading = true; this.findListOptions = Object.assign({}, new FindListOptions(), { elementsPerPage: 5, currentPage: this.findListOptions.currentPage + 1, }); - this.externalService.findAll(this.findListOptions).pipe( + this.sub = this.externalService.findAll(this.findListOptions).pipe( catchError(() => { const pageInfo = new PageInfo(); const paginatedList = new PaginatedList(pageInfo, []); @@ -159,7 +165,7 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit { }) this.pageInfo = externalSource.payload.pageInfo; this.cdr.detectChanges(); - }) + }); } } @@ -169,4 +175,13 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit { public search(): void { this.externalSourceData.emit({ sourceId: this.selectedElement.id, query: this.searchString }); } + + /** + * Unsubscribe from all subscriptions + */ + ngOnDestroy(): void { + if (hasValue(this.sub)) { + this.sub.unsubscribe(); + } + } } diff --git a/src/app/submission/import-external/submission-import-external.component.html b/src/app/submission/import-external/submission-import-external.component.html index c4de97b934..bee5f5d872 100644 --- a/src/app/submission/import-external/submission-import-external.component.html +++ b/src/app/submission/import-external/submission-import-external.component.html @@ -4,14 +4,14 @@ + (externalSourceData) = "getExternalSourceData($event)">
-

{{ 'submission.sections.describe.relationship-lookup.selection-tab.title.' + routeData.sourceId | translate}}

+

{{ 'submission.sections.describe.relationship-lookup.selection-tab.title.' + routeData.sourceId | translate}}

+ (importObject)="import($event)" + (pageChange)="paginationChange();"> diff --git a/src/app/submission/import-external/submission-import-external.component.spec.ts b/src/app/submission/import-external/submission-import-external.component.spec.ts index 4cbe204a91..10370d5e77 100644 --- a/src/app/submission/import-external/submission-import-external.component.spec.ts +++ b/src/app/submission/import-external/submission-import-external.component.spec.ts @@ -1,21 +1,25 @@ import { Component, NO_ERRORS_SCHEMA } from '@angular/core'; -import { async, TestBed, ComponentFixture, inject } from '@angular/core/testing'; +import { async, ComponentFixture, inject, TestBed } from '@angular/core/testing'; + +import { getTestScheduler } from 'jasmine-marbles'; import { TranslateModule } from '@ngx-translate/core'; import { Router } from '@angular/router'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; -import { of as observableOf, of } from 'rxjs/internal/observable/of'; +import { of as observableOf } from 'rxjs'; +import { TestScheduler } from 'rxjs/testing'; + import { SubmissionImportExternalComponent } from './submission-import-external.component'; import { ExternalSourceService } from '../../core/data/external-source.service'; import { getMockExternalSourceService } from '../../shared/mocks/external-source.service.mock'; import { SearchConfigurationService } from '../../core/shared/search/search-configuration.service'; import { RouteService } from '../../core/services/route.service'; -import { createTestComponent, createPaginatedList } from '../../shared/testing/utils.test'; +import { createPaginatedList, createTestComponent } from '../../shared/testing/utils.test'; import { RouterStub } from '../../shared/testing/router.stub'; import { VarDirective } from '../../shared/utils/var.directive'; import { routeServiceStub } from '../../shared/testing/route-service.stub'; import { PaginatedSearchOptions } from '../../shared/search/paginated-search-options.model'; import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; -import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; +import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { ExternalSourceEntry } from '../../core/shared/external-source-entry.model'; import { SubmissionImportExternalPreviewComponent } from './import-external-preview/submission-import-external-preview.component'; @@ -23,16 +27,19 @@ describe('SubmissionImportExternalComponent test suite', () => { let comp: SubmissionImportExternalComponent; let compAsAny: any; let fixture: ComponentFixture; + let scheduler: TestScheduler; const ngbModal = jasmine.createSpyObj('modal', ['open']); - const mockSearchOptions = of(new PaginatedSearchOptions({ + const mockSearchOptions = observableOf(new PaginatedSearchOptions({ pagination: Object.assign(new PaginationComponentOptions(), { pageSize: 10, currentPage: 0 - }) + }), + query: 'test' })); const searchConfigServiceStub = { paginatedSearchOptions: mockSearchOptions }; + const mockExternalSourceService: any = getMockExternalSourceService(); beforeEach(async (() => { TestBed.configureTestingModule({ @@ -45,7 +52,7 @@ describe('SubmissionImportExternalComponent test suite', () => { VarDirective ], providers: [ - { provide: ExternalSourceService, useClass: getMockExternalSourceService }, + { provide: ExternalSourceService, useValue: mockExternalSourceService }, { provide: SearchConfigurationService, useValue: searchConfigServiceStub }, { provide: RouteService, useValue: routeServiceStub }, { provide: Router, useValue: new RouterStub() }, @@ -83,6 +90,8 @@ describe('SubmissionImportExternalComponent test suite', () => { fixture = TestBed.createComponent(SubmissionImportExternalComponent); comp = fixture.componentInstance; compAsAny = comp; + scheduler = getTestScheduler(); + mockExternalSourceService.getExternalSourceEntries.and.returnValue(createSuccessfulRemoteDataObject$(createPaginatedList([]))) }); afterEach(() => { @@ -102,25 +111,31 @@ describe('SubmissionImportExternalComponent test suite', () => { }); it('Should init component properly (with route data)', () => { - const expectedEntries = createSuccessfulRemoteDataObject(createPaginatedList([])); - const searchOptions = new PaginatedSearchOptions({ - pagination: Object.assign(new PaginationComponentOptions(), { - pageSize: 10, - currentPage: 0 - }) - }); - spyOn(compAsAny.routeService, 'getQueryParameterValue').and.returnValue(observableOf('dummy')); + spyOn(compAsAny, 'retrieveExternalSources'); + spyOn(compAsAny.routeService, 'getQueryParameterValue').and.returnValues(observableOf('source'), observableOf('dummy')); fixture.detectChanges(); - expect(comp.routeData).toEqual({ sourceId: 'dummy', query: 'dummy' }); - expect(comp.isLoading$.value).toBe(true); - expect(comp.entriesRD$.value).toEqual(expectedEntries); - expect(compAsAny.externalService.getExternalSourceEntries).toHaveBeenCalledWith('dummy', searchOptions); + expect(compAsAny.retrieveExternalSources).toHaveBeenCalledWith('source', 'dummy'); + }); + + it('Should call \'getExternalSourceEntries\' properly', () => { + comp.routeData = { sourceId: '', query: '' }; + scheduler.schedule(() => compAsAny.retrieveExternalSources('orcidV2', 'test')); + scheduler.flush(); + + expect(comp.routeData).toEqual({ sourceId: 'orcidV2', query: 'test' }); + expect(comp.isLoading$.value).toBe(false); + expect(compAsAny.externalService.getExternalSourceEntries).toHaveBeenCalled(); }); it('Should call \'router.navigate\'', () => { + comp.routeData = { sourceId: '', query: '' }; + spyOn(compAsAny, 'retrieveExternalSources').and.callFake(() => null); + compAsAny.router.navigate.and.returnValue( new Promise(() => {return;})) const event = { sourceId: 'orcidV2', query: 'dummy' }; - comp.getExternalsourceData(event); + + scheduler.schedule(() => comp.getExternalSourceData(event)); + scheduler.flush(); expect(compAsAny.router.navigate).toHaveBeenCalledWith([], { queryParams: { source: event.sourceId, query: event.query }, replaceUrl: true }); }); diff --git a/src/app/submission/import-external/submission-import-external.component.ts b/src/app/submission/import-external/submission-import-external.component.ts index a369863a74..d28186a703 100644 --- a/src/app/submission/import-external/submission-import-external.component.ts +++ b/src/app/submission/import-external/submission-import-external.component.ts @@ -1,22 +1,25 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, OnDestroy, OnInit } from '@angular/core'; import { Router } from '@angular/router'; -import { combineLatest, BehaviorSubject } from 'rxjs'; + +import { BehaviorSubject, combineLatest, Subscription } from 'rxjs'; +import { filter, flatMap, take } from 'rxjs/operators'; +import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; + import { ExternalSourceService } from '../../core/data/external-source.service'; import { ExternalSourceData } from './import-external-searchbar/submission-import-external-searchbar.component'; import { RemoteData } from '../../core/data/remote-data'; import { PaginatedList } from '../../core/data/paginated-list'; import { ExternalSourceEntry } from '../../core/shared/external-source-entry.model'; import { SearchConfigurationService } from '../../core/shared/search/search-configuration.service'; -import { switchMap, filter, take } from 'rxjs/operators'; -import { PaginatedSearchOptions } from '../../shared/search/paginated-search-options.model'; import { Context } from '../../core/shared/context.model'; import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { RouteService } from '../../core/services/route.service'; import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; -import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; import { SubmissionImportExternalPreviewComponent } from './import-external-preview/submission-import-external-preview.component'; import { fadeIn } from '../../shared/animations/fade'; import { PageInfo } from '../../core/shared/page-info.model'; +import { hasValue, isNotEmpty } from '../../shared/empty.util'; +import { getFinishedRemoteData } from '../../core/shared/operators'; /** * This component allows to submit a new workspaceitem importing the data from an external source. @@ -25,9 +28,10 @@ import { PageInfo } from '../../core/shared/page-info.model'; selector: 'ds-submission-import-external', styleUrls: ['./submission-import-external.component.scss'], templateUrl: './submission-import-external.component.html', - animations: [ fadeIn ] + animations: [fadeIn] }) -export class SubmissionImportExternalComponent implements OnInit { +export class SubmissionImportExternalComponent implements OnInit, OnDestroy { + /** * The external source search data from the routing service. */ @@ -35,11 +39,11 @@ export class SubmissionImportExternalComponent implements OnInit { /** * The displayed list of entries */ - public entriesRD$: BehaviorSubject>>; + public entriesRD$: BehaviorSubject>> = new BehaviorSubject>>(null); /** * TRUE if the REST service is called to retrieve the external source items */ - public isLoading$: BehaviorSubject; + public isLoading$: BehaviorSubject = new BehaviorSubject(false); /** * Configuration to use for the import buttons */ @@ -61,7 +65,7 @@ export class SubmissionImportExternalComponent implements OnInit { */ public initialPagination = Object.assign(new PaginationComponentOptions(), { id: 'submission-external-source-relation-list', - pageSize: 5 + pageSize: 10 }); /** * The context to displaying lists for @@ -72,6 +76,11 @@ export class SubmissionImportExternalComponent implements OnInit { */ public modalRef: NgbModalRef; + /** + * The subscription to unsubscribe + */ + protected subs: Subscription[] = []; + /** * Initialize the component variables. * @param {SearchConfigurationService} searchConfigService @@ -86,57 +95,45 @@ export class SubmissionImportExternalComponent implements OnInit { private routeService: RouteService, private router: Router, private modalService: NgbModal, - ) { } + ) { + } /** * Get the entries for the selected external source and set initial configuration. */ ngOnInit(): void { - this.label = 'Journal'; - this.listId = 'list-submission-external-sources'; - this.context = Context.EntitySearchModalWithNameVariants; + this.label = 'Journal'; + this.listId = 'list-submission-external-sources'; + this.context = Context.EntitySearchModalWithNameVariants; this.repeatable = false; - this.routeData = { sourceId: '', query: '' }; + this.routeData = { sourceId: '', query: '' }; this.importConfig = { buttonLabel: 'submission.sections.describe.relationship-lookup.external-source.import-button-title.' + this.label }; this.entriesRD$ = new BehaviorSubject(createSuccessfulRemoteDataObject(new PaginatedList(new PageInfo(), []))); this.isLoading$ = new BehaviorSubject(false); - combineLatest( + this.subs.push(combineLatest( [ this.routeService.getQueryParameterValue('source'), this.routeService.getQueryParameterValue('query') ]).pipe( - filter(([source, query]) => source && query && source !== '' && query !== ''), - filter(([source, query]) => source !== this.routeData.sourceId || query !== this.routeData.query), - switchMap(([source, query]) => { - this.routeData.sourceId = source; - this.routeData.query = query; - this.isLoading$.next(true); - return this.searchConfigService.paginatedSearchOptions.pipe( - switchMap((searchOptions: PaginatedSearchOptions) => { - return this.externalService.getExternalSourceEntries(this.routeData.sourceId, searchOptions); - }), - take(1) - ) - }), - ).subscribe((rdData) => { - this.entriesRD$.next(rdData); - this.isLoading$.next(false); - }); + take(1) + ).subscribe(([source, query]: [string, string]) => { + this.retrieveExternalSources(source, query); + })); } /** * Get the data from the searchbar and changes the router data. */ - public getExternalsourceData(event: ExternalSourceData): void { + public getExternalSourceData(event: ExternalSourceData): void { this.router.navigate( [], { queryParams: { source: event.sourceId, query: event.query }, replaceUrl: true } - ); + ).then(() => this.retrieveExternalSources(event.sourceId, event.query)); } /** @@ -150,4 +147,49 @@ export class SubmissionImportExternalComponent implements OnInit { const modalComp = this.modalRef.componentInstance; modalComp.externalSourceEntry = entry; } + + /** + * Retrieve external sources on pagination change + */ + paginationChange() { + this.retrieveExternalSources(this.routeData.sourceId, this.routeData.query); + } + + /** + * Unsubscribe from all subscriptions + */ + ngOnDestroy(): void { + this.subs + .filter((sub) => hasValue(sub)) + .forEach((sub) => sub.unsubscribe()); + } + + /** + * Retrieve external source entries + * + * @param source The source tupe + * @param query The query string to search + */ + private retrieveExternalSources(source: string, query: string): void { + if (isNotEmpty(source) && isNotEmpty(query)) { + this.routeData.sourceId = source; + this.routeData.query = query; + this.isLoading$.next(true); + this.subs.push( + this.searchConfigService.paginatedSearchOptions.pipe( + filter((searchOptions) => searchOptions.query === query), + take(1), + flatMap((searchOptions) => this.externalService.getExternalSourceEntries(this.routeData.sourceId, searchOptions).pipe( + getFinishedRemoteData(), + take(1) + )), + take(1) + ).subscribe((rdData) => { + this.entriesRD$.next(rdData); + this.isLoading$.next(false); + }) + ); + } + } + } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index c93944f929..0ab4e29b69 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -2931,6 +2931,8 @@ "submission.import-external.search.source.hint": "Pick an external source", + "submission.import-external.source.arxiv": "arXiv", + "submission.import-external.source.loading": "Loading ...", "submission.import-external.source.sherpaJournal": "SHERPA Journals",