From dfbbd988620e40c687ac9989c8da69acb5908973 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Mon, 21 Sep 2020 13:18:41 +0200 Subject: [PATCH 1/3] 73249: Fix issues with bitstream pagination on item pages. --- .../full-file-section.component.html | 146 +++++++++--------- .../full-file-section.component.spec.ts | 5 +- .../full-file-section.component.ts | 55 +++++-- .../file-section.component.spec.ts | 5 +- .../file-section/file-section.component.ts | 32 ++-- src/app/core/data/bitstream-data.service.ts | 6 +- src/app/core/data/bundle-data.service.ts | 19 ++- 7 files changed, 159 insertions(+), 109 deletions(-) diff --git a/src/app/+item-page/full/field-components/file-section/full-file-section.component.html b/src/app/+item-page/full/field-components/file-section/full-file-section.component.html index c6f9f8e944..00218b66d1 100644 --- a/src/app/+item-page/full/field-components/file-section/full-file-section.component.html +++ b/src/app/+item-page/full/field-components/file-section/full-file-section.component.html @@ -1,87 +1,87 @@
-
{{"item.page.filesection.original.bundle" | translate}}
- +
+
{{"item.page.filesection.original.bundle" | translate}}
+ +
+
+ +
+
+
+
{{"item.page.filesection.name" | translate}}
+
{{file.name}}
-
-
- +
{{"item.page.filesection.size" | translate}}
+
{{(file.sizeBytes) | dsFileSize }}
+ + +
{{"item.page.filesection.format" | translate}}
+
{{(file.format | async)?.payload?.description}}
+ + +
{{"item.page.filesection.description" | translate}}
+
{{file.firstMetadataValue("dc.description")}}
+
+
+
+ + {{"item.page.filesection.download" | translate}} + +
-
-
-
{{"item.page.filesection.name" | translate}}
-
{{file.name}}
- -
{{"item.page.filesection.size" | translate}}
-
{{(file.sizeBytes) | dsFileSize }}
- - -
{{"item.page.filesection.format" | translate}}
-
{{(file.format | async)?.payload?.description}}
- - -
{{"item.page.filesection.description" | translate}}
-
{{file.firstMetadataValue("dc.description")}}
-
-
-
- - {{"item.page.filesection.download" | translate}} - -
-
- + +
-
-
{{"item.page.filesection.license.bundle" | translate}}
- +
+
{{"item.page.filesection.license.bundle" | translate}}
+ +
+
+ +
+
+
+
{{"item.page.filesection.name" | translate}}
+
{{file.name}}
-
-
- +
{{"item.page.filesection.size" | translate}}
+
{{(file.sizeBytes) | dsFileSize }}
+ +
{{"item.page.filesection.format" | translate}}
+
{{(file.format | async)?.payload?.description}}
+ + +
{{"item.page.filesection.description" | translate}}
+
{{file.firstMetadataValue("dc.description")}}
+
+
+
+ + {{"item.page.filesection.download" | translate}} + +
-
-
-
{{"item.page.filesection.name" | translate}}
-
{{file.name}}
- -
{{"item.page.filesection.size" | translate}}
-
{{(file.sizeBytes) | dsFileSize }}
- - -
{{"item.page.filesection.format" | translate}}
-
{{(file.format | async)?.payload?.description}}
- - -
{{"item.page.filesection.description" | translate}}
-
{{file.firstMetadataValue("dc.description")}}
-
-
-
- - {{"item.page.filesection.download" | translate}} - -
-
- + +
diff --git a/src/app/+item-page/full/field-components/file-section/full-file-section.component.spec.ts b/src/app/+item-page/full/field-components/file-section/full-file-section.component.spec.ts index 970420f252..4d4b713648 100644 --- a/src/app/+item-page/full/field-components/file-section/full-file-section.component.spec.ts +++ b/src/app/+item-page/full/field-components/file-section/full-file-section.component.spec.ts @@ -14,6 +14,8 @@ import {Bitstream} from '../../../../core/shared/bitstream.model'; import {of as observableOf} from 'rxjs'; import {MockBitstreamFormat1} from '../../../../shared/mocks/item.mock'; import {By} from '@angular/platform-browser'; +import { NotificationsService } from '../../../../shared/notifications/notifications.service'; +import { NotificationsServiceStub } from '../../../../shared/testing/notifications-service.stub'; describe('FullFileSectionComponent', () => { let comp: FullFileSectionComponent; @@ -61,7 +63,8 @@ describe('FullFileSectionComponent', () => { }), BrowserAnimationsModule], declarations: [FullFileSectionComponent, VarDirective, FileSizePipe, MetadataFieldWrapperComponent], providers: [ - {provide: BitstreamDataService, useValue: bitstreamDataService} + {provide: BitstreamDataService, useValue: bitstreamDataService}, + {provide: NotificationsService, useValue: new NotificationsServiceStub()} ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts b/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts index fdbe662ed9..d3260f7ec3 100644 --- a/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts +++ b/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts @@ -10,6 +10,10 @@ import { PaginationComponentOptions } from '../../../../shared/pagination/pagina import { PaginatedList } from '../../../../core/data/paginated-list'; import { RemoteData } from '../../../../core/data/remote-data'; import { switchMap } from 'rxjs/operators'; +import { NotificationsService } from '../../../../shared/notifications/notifications.service'; +import { TranslateService } from '@ngx-translate/core'; +import { hasValue, isEmpty } from '../../../../shared/empty.util'; +import { tap } from 'rxjs/internal/operators/tap'; /** * This component renders the file section of the item @@ -31,14 +35,14 @@ export class FullFileSectionComponent extends FileSectionComponent implements On licenses$: Observable>>; pageSize = 5; - originalOptions = Object.assign(new PaginationComponentOptions(),{ + originalOptions = Object.assign(new PaginationComponentOptions(), { id: 'original-bitstreams-options', currentPage: 1, pageSize: this.pageSize }); originalCurrentPage$ = new BehaviorSubject(1); - licenseOptions = Object.assign(new PaginationComponentOptions(),{ + licenseOptions = Object.assign(new PaginationComponentOptions(), { id: 'license-bitstreams-options', currentPage: 1, pageSize: this.pageSize @@ -46,9 +50,11 @@ export class FullFileSectionComponent extends FileSectionComponent implements On licenseCurrentPage$ = new BehaviorSubject(1); constructor( - bitstreamDataService: BitstreamDataService + bitstreamDataService: BitstreamDataService, + protected notificationsService: NotificationsService, + protected translateService: TranslateService ) { - super(bitstreamDataService); + super(bitstreamDataService, notificationsService, translateService); } ngOnInit(): void { @@ -57,21 +63,33 @@ export class FullFileSectionComponent extends FileSectionComponent implements On initialize(): void { this.originals$ = this.originalCurrentPage$.pipe( - switchMap((pageNumber: number) => this.bitstreamDataService.findAllByItemAndBundleName( - this.item, - 'ORIGINAL', - { elementsPerPage: this.pageSize, currentPage: pageNumber }, - followLink( 'format') - )) + switchMap((pageNumber: number) => this.bitstreamDataService.findAllByItemAndBundleName( + this.item, + 'ORIGINAL', + {elementsPerPage: this.pageSize, currentPage: pageNumber}, + followLink('format') + )), + tap((rd: RemoteData>) => { + if (hasValue(rd.error)) { + this.notificationsService.error(this.translateService.get('file-section.error.header'), `${rd.error.statusCode} ${rd.error.message}`); + } + } + ) ); this.licenses$ = this.licenseCurrentPage$.pipe( - switchMap((pageNumber: number) => this.bitstreamDataService.findAllByItemAndBundleName( - this.item, - 'LICENSE', - { elementsPerPage: this.pageSize, currentPage: pageNumber }, - followLink( 'format') - )) + switchMap((pageNumber: number) => this.bitstreamDataService.findAllByItemAndBundleName( + this.item, + 'LICENSE', + {elementsPerPage: this.pageSize, currentPage: pageNumber}, + followLink('format') + )), + tap((rd: RemoteData>) => { + if (hasValue(rd.error)) { + this.notificationsService.error(this.translateService.get('file-section.error.header'), `${rd.error.statusCode} ${rd.error.message}`); + } + } + ) ); } @@ -93,4 +111,9 @@ export class FullFileSectionComponent extends FileSectionComponent implements On this.licenseOptions.currentPage = page; this.licenseCurrentPage$.next(page); } + + hasValuesInBundle(bundle: PaginatedList) { + console.log(bundle, hasValue(bundle), hasValue(bundle) && !isEmpty(bundle.page)); + return hasValue(bundle) && !isEmpty(bundle.page); + } } diff --git a/src/app/+item-page/simple/field-components/file-section/file-section.component.spec.ts b/src/app/+item-page/simple/field-components/file-section/file-section.component.spec.ts index 1b7fa75ce5..330aaadfe0 100644 --- a/src/app/+item-page/simple/field-components/file-section/file-section.component.spec.ts +++ b/src/app/+item-page/simple/field-components/file-section/file-section.component.spec.ts @@ -15,6 +15,8 @@ import {FileSizePipe} from '../../../../shared/utils/file-size-pipe'; import {PageInfo} from '../../../../core/shared/page-info.model'; import {MetadataFieldWrapperComponent} from '../../../field-components/metadata-field-wrapper/metadata-field-wrapper.component'; import {createPaginatedList} from '../../../../shared/testing/utils.test'; +import { NotificationsService } from '../../../../shared/notifications/notifications.service'; +import { NotificationsServiceStub } from '../../../../shared/testing/notifications-service.stub'; describe('FileSectionComponent', () => { let comp: FileSectionComponent; @@ -62,7 +64,8 @@ describe('FileSectionComponent', () => { }), BrowserAnimationsModule], declarations: [FileSectionComponent, VarDirective, FileSizePipe, MetadataFieldWrapperComponent], providers: [ - {provide: BitstreamDataService, useValue: bitstreamDataService} + {provide: BitstreamDataService, useValue: bitstreamDataService}, + {provide: NotificationsService, useValue: new NotificationsServiceStub()} ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/+item-page/simple/field-components/file-section/file-section.component.ts b/src/app/+item-page/simple/field-components/file-section/file-section.component.ts index 25b214e200..a668767c99 100644 --- a/src/app/+item-page/simple/field-components/file-section/file-section.component.ts +++ b/src/app/+item-page/simple/field-components/file-section/file-section.component.ts @@ -4,10 +4,12 @@ import { BitstreamDataService } from '../../../../core/data/bitstream-data.servi import { Bitstream } from '../../../../core/shared/bitstream.model'; import { Item } from '../../../../core/shared/item.model'; -import { filter, takeWhile } from 'rxjs/operators'; +import { filter, take } from 'rxjs/operators'; import { RemoteData } from '../../../../core/data/remote-data'; -import { hasNoValue, hasValue } from '../../../../shared/empty.util'; +import { hasValue } from '../../../../shared/empty.util'; import { PaginatedList } from '../../../../core/data/paginated-list'; +import { NotificationsService } from '../../../../shared/notifications/notifications.service'; +import { TranslateService } from '@ngx-translate/core'; /** * This component renders the file section of the item @@ -36,7 +38,9 @@ export class FileSectionComponent implements OnInit { pageSize = 5; constructor( - protected bitstreamDataService: BitstreamDataService + protected bitstreamDataService: BitstreamDataService, + protected notificationsService: NotificationsService, + protected translateService: TranslateService ) { } @@ -58,14 +62,22 @@ export class FileSectionComponent implements OnInit { } else { this.currentPage++; } - this.bitstreamDataService.findAllByItemAndBundleName(this.item, 'ORIGINAL', { currentPage: this.currentPage, elementsPerPage: this.pageSize }).pipe( - filter((bitstreamsRD: RemoteData>) => hasValue(bitstreamsRD)), - takeWhile((bitstreamsRD: RemoteData>) => hasNoValue(bitstreamsRD.payload) && hasNoValue(bitstreamsRD.error), true) + this.bitstreamDataService.findAllByItemAndBundleName(this.item, 'ORIGINAL', { + currentPage: this.currentPage, + elementsPerPage: this.pageSize + }).pipe( + filter((bitstreamsRD: RemoteData>) => hasValue(bitstreamsRD)), + filter((bitstreamsRD: RemoteData>) => hasValue(!bitstreamsRD.isLoading)), + take(1), ).subscribe((bitstreamsRD: RemoteData>) => { - const current: Bitstream[] = this.bitstreams$.getValue(); - this.bitstreams$.next([...current, ...bitstreamsRD.payload.page]); - this.isLoading = false; - this.isLastPage = this.currentPage === bitstreamsRD.payload.totalPages; + if (bitstreamsRD.error) { + this.notificationsService.error(this.translateService.get('file-section.error.header'), `${bitstreamsRD.error.statusCode} ${bitstreamsRD.error.message}`); + } else if (hasValue(bitstreamsRD.payload)) { + const current: Bitstream[] = this.bitstreams$.getValue(); + this.bitstreams$.next([...current, ...bitstreamsRD.payload.page]); + this.isLoading = false; + this.isLastPage = this.currentPage === bitstreamsRD.payload.totalPages; + } }); } } diff --git a/src/app/core/data/bitstream-data.service.ts b/src/app/core/data/bitstream-data.service.ts index 4c24f5d78b..ca0338116f 100644 --- a/src/app/core/data/bitstream-data.service.ts +++ b/src/app/core/data/bitstream-data.service.ts @@ -30,6 +30,8 @@ import { RestResponse } from '../cache/response.models'; import { HttpOptions } from '../dspace-rest-v2/dspace-rest-v2.service'; import { configureRequest, getResponseFromEntry } from '../shared/operators'; import { combineLatest as observableCombineLatest } from 'rxjs'; +import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { PageInfo } from '../shared/page-info.model'; /** * A service to retrieve {@link Bitstream}s from the REST API @@ -165,8 +167,10 @@ export class BitstreamDataService extends DataService { public findAllByItemAndBundleName(item: Item, bundleName: string, options?: FindListOptions, ...linksToFollow: Array>): Observable>> { return this.bundleService.findByItemAndName(item, bundleName).pipe( switchMap((bundleRD: RemoteData) => { - if (hasValue(bundleRD.payload)) { + if (bundleRD.hasSucceeded && hasValue(bundleRD.payload)) { return this.findAllByBundle(bundleRD.payload, options, ...linksToFollow); + } else if (!bundleRD.hasSucceeded && bundleRD.error.statusCode === 404) { + return createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), [])) } else { return [bundleRD as any]; } diff --git a/src/app/core/data/bundle-data.service.ts b/src/app/core/data/bundle-data.service.ts index de0e8a4337..e651ed354f 100644 --- a/src/app/core/data/bundle-data.service.ts +++ b/src/app/core/data/bundle-data.service.ts @@ -22,6 +22,7 @@ import { FindListOptions, GetRequest } from './request.models'; import { RequestService } from './request.service'; import { PaginatedSearchOptions } from '../../shared/search/paginated-search-options.model'; import { Bitstream } from '../shared/bitstream.model'; +import { RemoteDataError } from './remote-data-error'; /** * A service to retrieve {@link Bundle}s from the REST API @@ -71,13 +72,17 @@ export class BundleDataService extends DataService { if (hasValue(rd.payload) && hasValue(rd.payload.page)) { const matchingBundle = rd.payload.page.find((bundle: Bundle) => bundle.name === bundleName); - return new RemoteData( - false, - false, - true, - undefined, - matchingBundle - ); + if (hasValue(matchingBundle)) { + return new RemoteData( + false, + false, + true, + undefined, + matchingBundle + ); + } else { + return new RemoteData(false, false, false, new RemoteDataError(404, 'Not found', `The bundle with name ${bundleName} was not found.` )) + } } else { return rd as any; } From cfadb4314ead392fbef7edfbc8838dba50175d99 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Mon, 21 Sep 2020 14:24:43 +0200 Subject: [PATCH 2/3] Remove console.log --- .../field-components/file-section/full-file-section.component.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts b/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts index d3260f7ec3..bd3b2f7063 100644 --- a/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts +++ b/src/app/+item-page/full/field-components/file-section/full-file-section.component.ts @@ -113,7 +113,6 @@ export class FullFileSectionComponent extends FileSectionComponent implements On } hasValuesInBundle(bundle: PaginatedList) { - console.log(bundle, hasValue(bundle), hasValue(bundle) && !isEmpty(bundle.page)); return hasValue(bundle) && !isEmpty(bundle.page); } } From ddc1cbbd7339681e465d62ce750daa5ea1cd1441 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Wed, 23 Sep 2020 11:36:10 +0200 Subject: [PATCH 3/3] 73249: Fix message and filter issue --- .../field-components/file-section/file-section.component.ts | 3 +-- src/assets/i18n/en.json5 | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/app/+item-page/simple/field-components/file-section/file-section.component.ts b/src/app/+item-page/simple/field-components/file-section/file-section.component.ts index a668767c99..4b60691e09 100644 --- a/src/app/+item-page/simple/field-components/file-section/file-section.component.ts +++ b/src/app/+item-page/simple/field-components/file-section/file-section.component.ts @@ -66,8 +66,7 @@ export class FileSectionComponent implements OnInit { currentPage: this.currentPage, elementsPerPage: this.pageSize }).pipe( - filter((bitstreamsRD: RemoteData>) => hasValue(bitstreamsRD)), - filter((bitstreamsRD: RemoteData>) => hasValue(!bitstreamsRD.isLoading)), + filter((bitstreamsRD: RemoteData>) => hasValue(bitstreamsRD) && (hasValue(bitstreamsRD.error) || hasValue(bitstreamsRD.payload))), take(1), ).subscribe((bitstreamsRD: RemoteData>) => { if (bitstreamsRD.error) { diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 4148c6e5c9..97a9faf861 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1045,6 +1045,10 @@ + "file-section.error.header": "Error obtaining files for this item", + + + "footer.copyright": "copyright © 2002-{{ year }}", "footer.link.dspace": "DSpace software",