From 97ce951bd8bd06c519670acbc81ae63cd47d3368 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Wed, 5 Aug 2020 11:01:56 +0200 Subject: [PATCH 01/30] FollowLink addition to bitstream/bundle models --- src/app/+bitstream-page/bitstream-page.resolver.ts | 13 ++++++++++++- src/app/core/shared/bitstream.model.ts | 8 ++++++++ src/app/core/shared/bundle.model.ts | 10 ++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/app/+bitstream-page/bitstream-page.resolver.ts b/src/app/+bitstream-page/bitstream-page.resolver.ts index 8e9f64fcc1..74cf54fb3a 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,19 @@ 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')) + ]; + } } 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>; } From 82587b5ff841ae8571bcbe650675da23ac6b0997 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Wed, 5 Aug 2020 11:14:21 +0200 Subject: [PATCH 02/30] Move followLink for format to bitstream-page.resolver --- src/app/+bitstream-page/bitstream-page.resolver.ts | 3 ++- .../edit-bitstream-page/edit-bitstream-page.component.ts | 7 +------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/app/+bitstream-page/bitstream-page.resolver.ts b/src/app/+bitstream-page/bitstream-page.resolver.ts index 74cf54fb3a..a47183e049 100644 --- a/src/app/+bitstream-page/bitstream-page.resolver.ts +++ b/src/app/+bitstream-page/bitstream-page.resolver.ts @@ -36,7 +36,8 @@ export class BitstreamPageResolver implements Resolve> { */ get followLinks(): Array> { return [ - followLink('bundle', undefined, true, followLink('item')) + followLink('bundle', undefined, true, followLink('item')), + followLink('format') ]; } } 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 cce6932cd1..66569dd363 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 @@ -299,12 +299,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( From e78a1ee63e634bb14392eeab1fe0f566a0f719c7 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Fri, 7 Aug 2020 09:51:33 +0200 Subject: [PATCH 03/30] Routing back to the item page from bitstream edit --- .../edit-bitstream-page.component.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) 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 66569dd363..9e6b0babac 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 {filter, map, mergeMap, switchMap} from 'rxjs/operators'; import { combineLatest as observableCombineLatest, of as observableOf } from 'rxjs'; import { Subscription } from 'rxjs/internal/Subscription'; import { @@ -36,7 +36,9 @@ 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 { getItemEditPath } from '../../+item-page/item-page-routing.module'; +import {getItemEditPath, getItemPageRoute} from '../../+item-page/item-page-routing.module'; +import {Bundle} from "../../core/shared/bundle.model"; +import {Item} from "../../core/shared/item.model"; @Component({ selector: 'ds-edit-bitstream-page', @@ -503,7 +505,11 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { if (hasValue(this.itemId)) { this.router.navigate([getItemEditPath(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(([getItemPageRoute(item)])); + }); } } From 11d81d8afc82f4a230ed4c877eac178e7421ac96 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Fri, 21 Aug 2020 09:30:06 +0200 Subject: [PATCH 04/30] Initial tests added (Still failing) --- .../edit-bitstream-page.component.spec.ts | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) 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..4ed733d7d6 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,12 @@ 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 { + createFailedRemoteDataObject$, + createSuccessfulRemoteDataObject$ +} from "../../shared/remote-data.utils"; +import {getItemPageRoute} from "../../+item-page/item-page-routing.module"; +import {RouterStub} from "../../shared/testing/router.stub"; const infoNotification: INotification = new Notification('id', NotificationType.Info, 'info'); const warningNotification: INotification = new Notification('id', NotificationType.Warning, 'warning'); @@ -34,6 +40,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 +113,12 @@ describe('EditBitstreamPageComponent', () => { format: observableOf(new RemoteData(false, false, true, null, selectedFormat)), _links: { self: 'bitstream-selflink' - } + }, + bundle: createFailedRemoteDataObject$({ + item: createSuccessfulRemoteDataObject$({ + uuid: 'some-uuid' + }) + }) }); bitstreamService = jasmine.createSpyObj('bitstreamService', { findById: observableOf(new RemoteData(false, false, true, null, bitstream)), @@ -118,6 +131,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 +144,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 +156,7 @@ describe('EditBitstreamPageComponent', () => { fixture = TestBed.createComponent(EditBitstreamPageComponent); comp = fixture.componentInstance; fixture.detectChanges(); + router = (comp as any).router; }); describe('on startup', () => { @@ -213,4 +232,11 @@ describe('EditBitstreamPageComponent', () => { }); }); }); + describe('when the cancel button is clicked', () => { + it('should redirect to the item page', () => { + comp.onCancel(); + spyOn(routerStub, 'navigate'); + expect(routerStub.navigate).toHaveBeenCalledWith([getItemPageRoute('some-uuid')]); + }); + }); }); From 95de75dbf924b84a904bd837351a3f0012329357 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Fri, 21 Aug 2020 11:28:16 +0200 Subject: [PATCH 05/30] Failing test fixing + additional tests for the call that handles the actual navigation routing --- .../edit-bitstream-page.component.spec.ts | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) 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 4ed733d7d6..722b7c8833 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 @@ -23,11 +23,10 @@ import { FileSizePipe } from '../../shared/utils/file-size-pipe'; import { RestResponse } from '../../core/cache/response.models'; import { VarDirective } from '../../shared/utils/var.directive'; import { - createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from "../../shared/remote-data.utils"; -import {getItemPageRoute} from "../../+item-page/item-page-routing.module"; import {RouterStub} from "../../shared/testing/router.stub"; +import {getItemEditPath, getItemPageRoute} from "../../+item-page/item-page-routing.module"; const infoNotification: INotification = new Notification('id', NotificationType.Info, 'info'); const warningNotification: INotification = new Notification('id', NotificationType.Warning, 'warning'); @@ -114,7 +113,7 @@ describe('EditBitstreamPageComponent', () => { _links: { self: 'bitstream-selflink' }, - bundle: createFailedRemoteDataObject$({ + bundle: createSuccessfulRemoteDataObject$({ item: createSuccessfulRemoteDataObject$({ uuid: 'some-uuid' }) @@ -233,9 +232,23 @@ describe('EditBitstreamPageComponent', () => { }); }); describe('when the cancel button is clicked', () => { - it('should redirect to the item page', () => { + it('should call navigateToItemEditBitstreams method', () => { + spyOn(comp, 'navigateToItemEditBitstreams'); comp.onCancel(); - spyOn(routerStub, 'navigate'); + expect(comp.navigateToItemEditBitstreams).toHaveBeenCalled(); + }); + }); + describe('when navigateToItemEditBitstreams is called, and the component has an itemId', () => { + it('should redirect to the item edit path bitstream page', () => { + comp.itemId = 'some-uuid' + comp.navigateToItemEditBitstreams(); + expect(routerStub.navigate).toHaveBeenCalledWith([getItemEditPath('some-uuid'), 'bitstreams']); + }); + }); + describe('when navigateToItemEditBitstreams is called, and the component does not have an itemId', () => { + it('should redirect to the item page', () => { + comp.itemId = undefined; + comp.navigateToItemEditBitstreams(); expect(routerStub.navigate).toHaveBeenCalledWith([getItemPageRoute('some-uuid')]); }); }); From dddf8df2b2fbb068c954571dfeb25a00406d21b2 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Fri, 25 Sep 2020 11:38:11 +0200 Subject: [PATCH 06/30] Merge conflict fixes --- .../edit-bitstream-page/edit-bitstream-page.component.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 6f0a60c554..7f002478fe 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 @@ -16,6 +16,7 @@ import { import { FormGroup } from '@angular/forms'; import { TranslateService } from '@ngx-translate/core'; import { DynamicCustomSwitchModel } from '../../shared/form/builder/ds-dynamic-form-ui/models/custom-switch/custom-switch.model'; +import { cloneDeep } from 'lodash'; import { BitstreamDataService } from '../../core/data/bitstream-data.service'; import { getAllSucceededRemoteDataPayload, @@ -34,8 +35,7 @@ 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 {getItemEditPath, getItemPageRoute} from '../../+item-page/item-page-routing.module'; -import { getItemEditRoute } from '../../+item-page/item-page-routing-paths'; +import { getItemEditRoute, getItemPageRoute } from '../../+item-page/item-page-routing-paths'; import {Bundle} from '../../core/shared/bundle.model'; import {Item} from '../../core/shared/item.model'; From 988079caed77eaf1eb4aa29e766336fd37d87ce2 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Wed, 30 Sep 2020 12:34:03 +0200 Subject: [PATCH 07/30] Merge conflict fixes --- .../edit-bitstream-page.component.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 722b7c8833..d7349894a9 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 @@ -24,9 +24,9 @@ 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 {getItemEditPath, getItemPageRoute} from "../../+item-page/item-page-routing.module"; +} from '../../shared/remote-data.utils'; +import {RouterStub} from '../../shared/testing/router.stub'; +import { getItemEditRoute, getItemPageRoute } 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'); @@ -242,7 +242,7 @@ describe('EditBitstreamPageComponent', () => { it('should redirect to the item edit path bitstream page', () => { comp.itemId = 'some-uuid' comp.navigateToItemEditBitstreams(); - expect(routerStub.navigate).toHaveBeenCalledWith([getItemEditPath('some-uuid'), 'bitstreams']); + expect(routerStub.navigate).toHaveBeenCalledWith([getItemEditRoute('some-uuid'), 'bitstreams']); }); }); describe('when navigateToItemEditBitstreams is called, and the component does not have an itemId', () => { From aa757cfe231dc62552c8903cff1856549a639362 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Wed, 30 Sep 2020 13:04:34 +0200 Subject: [PATCH 08/30] Redirecting to item edit page on the bitstream tab, instead of the item viewing page --- .../edit-bitstream-page.component.spec.ts | 18 +++++++++--------- .../edit-bitstream-page.component.ts | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) 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 d7349894a9..d35f2d5c21 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 @@ -239,17 +239,17 @@ describe('EditBitstreamPageComponent', () => { }); }); describe('when navigateToItemEditBitstreams is called, and the component has an itemId', () => { - it('should redirect to the item edit path bitstream page', () => { - comp.itemId = 'some-uuid' + 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']); }); }); - describe('when navigateToItemEditBitstreams is called, and the component does not have an itemId', () => { - it('should redirect to the item page', () => { - comp.itemId = undefined; - comp.navigateToItemEditBitstreams(); - expect(routerStub.navigate).toHaveBeenCalledWith([getItemPageRoute('some-uuid')]); - }); - }); }); 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 7f002478fe..a8bb321087 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 @@ -507,7 +507,7 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { this.bitstream.bundle.pipe(getFirstSucceededRemoteDataPayload(), mergeMap((bundle: Bundle) => bundle.item.pipe(getFirstSucceededRemoteDataPayload(), map((item: Item) => item.uuid)))) .subscribe((item) => { - this.router.navigate(([getItemPageRoute(item)])); + this.router.navigate(([getItemEditRoute(item), 'bitstreams'])); }); } } From ce553e3272bd20cc5b92961fa29ccced0b66542f Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Thu, 1 Oct 2020 09:09:05 +0200 Subject: [PATCH 09/30] Fixing lint error(s) --- src/app/+bitstream-page/bitstream-page.resolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/+bitstream-page/bitstream-page.resolver.ts b/src/app/+bitstream-page/bitstream-page.resolver.ts index a47183e049..9c85688c39 100644 --- a/src/app/+bitstream-page/bitstream-page.resolver.ts +++ b/src/app/+bitstream-page/bitstream-page.resolver.ts @@ -6,7 +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"; +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 From a45bc349cc81760eb19f20228c2929ec52b56da7 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Thu, 1 Oct 2020 09:11:25 +0200 Subject: [PATCH 10/30] Small doc update --- .../edit-bitstream-page/edit-bitstream-page.component.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 a8bb321087..cd4e47cd18 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 @@ -497,8 +497,8 @@ 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)) { From be2bfbe066f8c4035b5ddf6951b5d2f5b84de557 Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Thu, 1 Oct 2020 09:33:38 +0200 Subject: [PATCH 11/30] Fixing LGTM - Unused imports --- .../edit-bitstream-page/edit-bitstream-page.component.spec.ts | 2 +- .../edit-bitstream-page/edit-bitstream-page.component.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 d35f2d5c21..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 @@ -26,7 +26,7 @@ import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import {RouterStub} from '../../shared/testing/router.stub'; -import { getItemEditRoute, getItemPageRoute } from '../../+item-page/item-page-routing-paths'; +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'); 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 cd4e47cd18..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 @@ -35,7 +35,7 @@ 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 { getItemEditRoute, getItemPageRoute } from '../../+item-page/item-page-routing-paths'; +import { getItemEditRoute } from '../../+item-page/item-page-routing-paths'; import {Bundle} from '../../core/shared/bundle.model'; import {Item} from '../../core/shared/item.model'; From 5e21bfa5ca2369b4659efeb6e34daa9d23fbf10a Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 13 Oct 2020 13:28:47 +0200 Subject: [PATCH 12/30] 74053: fix issue #865 --- .../core/breadcrumbs/dso-breadcrumb.resolver.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts index 09292fec21..9d04083749 100644 --- a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts +++ b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts @@ -4,11 +4,12 @@ import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot } from '@angular/r import { DSOBreadcrumbsService } from './dso-breadcrumbs.service'; import { DataService } from '../data/data.service'; import { getRemoteDataPayload, getSucceededRemoteData } from '../shared/operators'; -import { map } from 'rxjs/operators'; +import { filter, find, 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; + } }) ); } From d4bd128b673a071c6ba27586627b1db3f76c3466 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 13 Oct 2020 14:19:56 +0200 Subject: [PATCH 13/30] 74053: Remove unused imports --- src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts index 9d04083749..03d4db3f5d 100644 --- a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts +++ b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts @@ -3,8 +3,8 @@ 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 { filter, find, map, take } 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'; From f8afc6355b5433181a43aff1174bc1efd40b4c26 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 15 Oct 2020 12:06:32 +0200 Subject: [PATCH 14/30] [CSTPER-138] Fixed issue that blocked route change within the import-external page --- .../import-external/submission-import-external.component.ts | 1 + 1 file changed, 1 insertion(+) 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..a230d12caf 100644 --- a/src/app/submission/import-external/submission-import-external.component.ts +++ b/src/app/submission/import-external/submission-import-external.component.ts @@ -107,6 +107,7 @@ export class SubmissionImportExternalComponent implements OnInit { this.routeService.getQueryParameterValue('source'), this.routeService.getQueryParameterValue('query') ]).pipe( + take(1), filter(([source, query]) => source && query && source !== '' && query !== ''), filter(([source, query]) => source !== this.routeData.sourceId || query !== this.routeData.query), switchMap(([source, query]) => { From f1256b37952154241c6da3613f134d458e3cae7d Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 15 Oct 2020 12:07:47 +0200 Subject: [PATCH 15/30] [CSTPER-138] Fixed issue with request pagination while retrieving external source providers --- .../submission-import-external-searchbar.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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..367279d930 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 @@ -101,7 +101,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(() => { From ed1e93f73d72d8b34b4bc7f758d4115e55f6ccfd Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 15 Oct 2020 12:09:05 +0200 Subject: [PATCH 16/30] [CSTPER-138] Fixed issue with infinite scroll within the dropdown of the external source providers --- .../submission-import-external-searchbar.component.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; } From 75999385164fd38cb0412bc4fa6789b7ef9236c1 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 15 Oct 2020 12:47:12 +0200 Subject: [PATCH 17/30] [CSTPER-138] Added label for arXiv external providers --- src/assets/i18n/en.json5 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 84d874388c..e7335694f1 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -2924,6 +2924,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", From 988747e392afc694754301f29b636fa4aabb53b6 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 15 Oct 2020 12:50:02 +0200 Subject: [PATCH 18/30] [CSTPER-138] Fixed issue with request pagination while retrieving external source providers --- .../submission-import-external-searchbar.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 367279d930..743f3ea73a 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 @@ -139,7 +139,7 @@ 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, From f7883492c6b44cd8bd9ceeaebe50efb2311000ec Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Thu, 15 Oct 2020 15:41:08 -0500 Subject: [PATCH 19/30] Correct syntax for referencing related issues/PRs --- .github/pull_request_template.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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). From 622871fcdee59cdb6d8294d1107a73c0add41b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20P=C3=A9ter=20Sipos?= Date: Fri, 16 Oct 2020 14:53:58 +0200 Subject: [PATCH 20/30] #895 fix grid layout --- src/app/shared/object-grid/object-grid.component.html | 8 ++++---- src/app/shared/object-grid/object-grid.component.scss | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/app/shared/object-grid/object-grid.component.html b/src/app/shared/object-grid/object-grid.component.html index 0afd623d86..b3ad9cc27c 100644 --- a/src/app/shared/object-grid/object-grid.component.html +++ b/src/app/shared/object-grid/object-grid.component.html @@ -11,10 +11,10 @@ (sortDirectionChange)="onSortDirectionChange($event)" (sortFieldChange)="onSortFieldChange($event)" (paginationChange)="onPaginationChange($event)"> -
-
-
- +
+
+
+
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; From 3d1ffa6738b8def998cf73b5d123a481f116e272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20P=C3=A9ter=20Sipos?= Date: Fri, 16 Oct 2020 17:05:46 +0200 Subject: [PATCH 21/30] #895 fix colums sorting --- src/app/shared/object-grid/object-grid.component.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/app/shared/object-grid/object-grid.component.ts b/src/app/shared/object-grid/object-grid.component.ts index c6f8347217..c4aaebed72 100644 --- a/src/app/shared/object-grid/object-grid.component.ts +++ b/src/app/shared/object-grid/object-grid.component.ts @@ -172,13 +172,18 @@ export class ObjectGridComponent implements OnInit { const result = []; + let index = 0; page.forEach((obj: ListableObject, i: number) => { - const colNb = i % nbColumns; - let col = result[colNb]; + + if (i % nbColumns === 0 && i > 0) { + index++; + } + + let col = result[index]; if (hasNoValue(col)) { col = []; } - result[colNb] = [...col, obj]; + result[index] = [...col, obj]; }); return result; } else { From 9a324ba45621dd594a2d3a9e200d65ff2e753c98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20P=C3=A9ter=20Sipos?= Date: Fri, 16 Oct 2020 17:12:25 +0200 Subject: [PATCH 22/30] #895 revert changes fix layout --- src/app/shared/object-grid/object-grid.component.html | 8 ++++---- src/app/shared/object-grid/object-grid.component.ts | 11 +++-------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/app/shared/object-grid/object-grid.component.html b/src/app/shared/object-grid/object-grid.component.html index b3ad9cc27c..0afd623d86 100644 --- a/src/app/shared/object-grid/object-grid.component.html +++ b/src/app/shared/object-grid/object-grid.component.html @@ -11,10 +11,10 @@ (sortDirectionChange)="onSortDirectionChange($event)" (sortFieldChange)="onSortFieldChange($event)" (paginationChange)="onPaginationChange($event)"> -
-
-
- +
+
+
+
diff --git a/src/app/shared/object-grid/object-grid.component.ts b/src/app/shared/object-grid/object-grid.component.ts index c4aaebed72..c6f8347217 100644 --- a/src/app/shared/object-grid/object-grid.component.ts +++ b/src/app/shared/object-grid/object-grid.component.ts @@ -172,18 +172,13 @@ export class ObjectGridComponent implements OnInit { const result = []; - let index = 0; page.forEach((obj: ListableObject, i: number) => { - - if (i % nbColumns === 0 && i > 0) { - index++; - } - - let col = result[index]; + const colNb = i % nbColumns; + let col = result[colNb]; if (hasNoValue(col)) { col = []; } - result[index] = [...col, obj]; + result[colNb] = [...col, obj]; }); return result; } else { From 4e4ef36c3484dba80bf28c8fe56fc6a8bae5b505 Mon Sep 17 00:00:00 2001 From: Sascha Szott Date: Sat, 17 Oct 2020 21:42:47 +0200 Subject: [PATCH 23/30] bugfix of issue #904 --- ...-search-result-list-element.component.html | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) 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 @@ - - (, ) - - - - ; - - - - + + + ( + ) + + + + + ; + + + +
From 1dcfb729a827e867f8f0120f2bb223a7cee7b5be Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 21 Oct 2020 10:23:56 +0200 Subject: [PATCH 24/30] fix issue where item search results would always have a shadow, instead of only when they're focused --- .../search-result-grid-element.component.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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(); } } From c4687116e5c237a5daf4523ccd5835381e9c4399 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 21 Oct 2020 11:35:05 +0200 Subject: [PATCH 25/30] add isCollapsed to mock TruncatableService --- ...arch-result-admin-workflow-grid-element.component.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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] From 826957a66d59f4cf2d8dde81a1dab75f82cc0030 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 21 Oct 2020 18:01:29 +0200 Subject: [PATCH 26/30] [CSTPER-138] Fixed issue where results are the same when pagination changed while retrieving external source providers --- ...ion-import-external-searchbar.component.ts | 25 +++- .../submission-import-external.component.html | 5 +- ...bmission-import-external.component.spec.ts | 48 +++++--- .../submission-import-external.component.ts | 109 ++++++++++++------ 4 files changed, 129 insertions(+), 58 deletions(-) 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 743f3ea73a..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 @@ -145,7 +151,7 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit { 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..919eec3f4a 100644 --- a/src/app/submission/import-external/submission-import-external.component.html +++ b/src/app/submission/import-external/submission-import-external.component.html @@ -4,7 +4,7 @@ + (externalSourceData) = "getExternalSourceData($event)">
@@ -20,7 +20,8 @@ [context]="context" [importable]="true" [importConfig]="importConfig" - (importObject)="import($event)"> + (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..fdfa972cc0 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,15 +1,19 @@ 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'; @@ -18,17 +22,20 @@ import { PaginationComponentOptions } from '../../shared/pagination/pagination-c import { 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'; +import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; 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 @@ -83,6 +90,7 @@ describe('SubmissionImportExternalComponent test suite', () => { fixture = TestBed.createComponent(SubmissionImportExternalComponent); comp = fixture.componentInstance; compAsAny = comp; + scheduler = getTestScheduler(); }); afterEach(() => { @@ -102,25 +110,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(compAsAny.retrieveExternalSources).toHaveBeenCalledWith('source', 'dummy'); + }); + + it('Should call \'getExternalSourceEntries\' properly', () => { + comp.routeData = { sourceId: '', query: '' }; + comp.isLoading$ = new BehaviorSubject(false); + scheduler.schedule(() => compAsAny.retrieveExternalSources('orcidV2', 'test')); + scheduler.flush(); + + expect(comp.routeData).toEqual({ sourceId: 'orcidV2', query: 'test' }); expect(comp.isLoading$.value).toBe(true); - expect(comp.entriesRD$.value).toEqual(expectedEntries); - expect(compAsAny.externalService.getExternalSourceEntries).toHaveBeenCalledWith('dummy', searchOptions); + expect(compAsAny.externalService.getExternalSourceEntries).toHaveBeenCalled(); }); it('Should call \'router.navigate\'', () => { + spyOn(compAsAny, 'retrieveExternalSources'); + 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 a230d12caf..110a399287 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. */ @@ -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,58 +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( - take(1), - 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)); } /** @@ -151,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); + }) + ); + } + } + } From c374627c27f9db8b03273761d1d6bb05ccfb88c7 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 22 Oct 2020 17:24:18 +0200 Subject: [PATCH 27/30] [CSTPER-138] Fixed failed test --- .../shared/mocks/external-source.service.mock.ts | 3 +-- .../submission-import-external.component.html | 2 +- .../submission-import-external.component.spec.ts | 13 +++++++------ .../submission-import-external.component.ts | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) 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/submission/import-external/submission-import-external.component.html b/src/app/submission/import-external/submission-import-external.component.html index 919eec3f4a..bee5f5d872 100644 --- a/src/app/submission/import-external/submission-import-external.component.html +++ b/src/app/submission/import-external/submission-import-external.component.html @@ -11,7 +11,7 @@
-

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

+

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

{ let comp: SubmissionImportExternalComponent; @@ -40,6 +39,7 @@ describe('SubmissionImportExternalComponent test suite', () => { const searchConfigServiceStub = { paginatedSearchOptions: mockSearchOptions }; + const mockExternalSourceService: any = getMockExternalSourceService(); beforeEach(async (() => { TestBed.configureTestingModule({ @@ -52,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() }, @@ -91,6 +91,7 @@ describe('SubmissionImportExternalComponent test suite', () => { comp = fixture.componentInstance; compAsAny = comp; scheduler = getTestScheduler(); + mockExternalSourceService.getExternalSourceEntries.and.returnValue(createSuccessfulRemoteDataObject$(createPaginatedList([]))) }); afterEach(() => { @@ -119,17 +120,17 @@ describe('SubmissionImportExternalComponent test suite', () => { it('Should call \'getExternalSourceEntries\' properly', () => { comp.routeData = { sourceId: '', query: '' }; - comp.isLoading$ = new BehaviorSubject(false); scheduler.schedule(() => compAsAny.retrieveExternalSources('orcidV2', 'test')); scheduler.flush(); expect(comp.routeData).toEqual({ sourceId: 'orcidV2', query: 'test' }); - expect(comp.isLoading$.value).toBe(true); + expect(comp.isLoading$.value).toBe(false); expect(compAsAny.externalService.getExternalSourceEntries).toHaveBeenCalled(); }); it('Should call \'router.navigate\'', () => { - spyOn(compAsAny, 'retrieveExternalSources'); + comp.routeData = { sourceId: '', query: '' }; + spyOn(compAsAny, 'retrieveExternalSources').and.callFake(() => null); compAsAny.router.navigate.and.returnValue( new Promise(() => {return;})) const event = { sourceId: 'orcidV2', query: 'dummy' }; 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 110a399287..d28186a703 100644 --- a/src/app/submission/import-external/submission-import-external.component.ts +++ b/src/app/submission/import-external/submission-import-external.component.ts @@ -39,11 +39,11 @@ export class SubmissionImportExternalComponent implements OnInit, OnDestroy { /** * 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 */ From 736704aae8fc2ee22a2559625a57ca17e9ab0fcd Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 27 Oct 2020 11:25:12 +0100 Subject: [PATCH 28/30] 74053: redirectOn404Or401 --- .../collection-page.component.ts | 4 ++-- .../community-page.component.ts | 4 ++-- .../+item-page/simple/item-page.component.ts | 4 ++-- src/app/app-routing-paths.ts | 2 +- src/app/core/shared/operators.spec.ts | 24 ++++++++++++------- src/app/core/shared/operators.ts | 14 +++++++---- .../detail/process-detail.component.ts | 4 ++-- .../statistics-page.component.ts | 4 ++-- 8 files changed, 36 insertions(+), 24 deletions(-) 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..f9fc448b99 100644 --- a/src/app/app-routing-paths.ts +++ b/src/app/app-routing-paths.ts @@ -55,7 +55,7 @@ export function getDSORoute(dso: DSpaceObject): string { } } -export const UNAUTHORIZED_PATH = 'unauthorized'; +export const UNAUTHORIZED_PATH = '401'; export function getUnauthorizedRoute() { return `/${UNAUTHORIZED_PATH}`; diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index a19419259d..401efae760 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'; @@ -23,7 +24,7 @@ import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; -describe('Core Module - RxJS Operators', () => { +fdescribe('Core Module - RxJS Operators', () => { let scheduler: TestScheduler; let requestService: RequestService; const testRequestHref = 'https://rest.api/'; @@ -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..41e9c7e18a 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -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('/404', {skipLocationChange: true}); + } else if (rd.error.statusCode === 401) { + router.navigateByUrl('/401', {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/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(), ); From 2d67b0b13ef2bccd74987ee4a98d88a1a6ec85e1 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 27 Oct 2020 12:29:39 +0100 Subject: [PATCH 29/30] 74053: remove fdescribe + use consts for routing paths --- src/app/app-routing-paths.ts | 6 ++++++ src/app/core/shared/operators.spec.ts | 2 +- src/app/core/shared/operators.ts | 6 +++--- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/app/app-routing-paths.ts b/src/app/app-routing-paths.ts index f9fc448b99..2b57a1957c 100644 --- a/src/app/app-routing-paths.ts +++ b/src/app/app-routing-paths.ts @@ -61,6 +61,12 @@ 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/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index 401efae760..8acf5ea860 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -24,7 +24,7 @@ import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; -fdescribe('Core Module - RxJS Operators', () => { +describe('Core Module - RxJS Operators', () => { let scheduler: TestScheduler; let requestService: RequestService; const testRequestHref = 'https://rest.api/'; diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 41e9c7e18a..ecc1f53933 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'; /** @@ -181,9 +181,9 @@ export const redirectOn404Or401 = (router: Router) => tap((rd: RemoteData) => { if (rd.hasFailed) { if (rd.error.statusCode === 404) { - router.navigateByUrl('/404', {skipLocationChange: true}); + router.navigateByUrl(`/${getPageNotFoundRoute()}`, {skipLocationChange: true}); } else if (rd.error.statusCode === 401) { - router.navigateByUrl('/401', {skipLocationChange: true}); + router.navigateByUrl(`/${getUnauthorizedRoute()}`, {skipLocationChange: true}); } } })); From d52da292a65e6add90c914e6e5d758d873643d69 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 27 Oct 2020 13:14:01 +0100 Subject: [PATCH 30/30] 74053: Fix double slash in route --- src/app/core/shared/operators.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index ecc1f53933..29e41907e1 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -181,9 +181,9 @@ export const redirectOn404Or401 = (router: Router) => tap((rd: RemoteData) => { if (rd.hasFailed) { if (rd.error.statusCode === 404) { - router.navigateByUrl(`/${getPageNotFoundRoute()}`, {skipLocationChange: true}); + router.navigateByUrl(getPageNotFoundRoute(), {skipLocationChange: true}); } else if (rd.error.statusCode === 401) { - router.navigateByUrl(`/${getUnauthorizedRoute()}`, {skipLocationChange: true}); + router.navigateByUrl(getUnauthorizedRoute(), {skipLocationChange: true}); } } }));