From f2dab663ca6bfddac106b9f9b7c0965626adf8be Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Mon, 18 Dec 2023 00:01:59 +0100 Subject: [PATCH 1/2] 108588: Fixed browse by issue date show loading icon indefinitely when empty (cherry picked from commit 6f51bd866ccccd17e70557b82028effa183263df) --- .../browse-by-date-page.component.spec.ts | 7 ++++++- .../browse-by-date-page.component.ts | 21 +++++++++++++------ .../browse-by-metadata-page.component.html | 4 ++-- .../browse-by-metadata-page.component.spec.ts | 7 ++++++- .../browse-by-metadata-page.component.ts | 13 +++++++++++- 5 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.spec.ts b/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.spec.ts index e41d3a45b25..b19250edae8 100644 --- a/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.spec.ts +++ b/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.spec.ts @@ -23,6 +23,7 @@ import { PaginationServiceStub } from '../../shared/testing/pagination-service.s import { APP_CONFIG } from '../../../config/app-config.interface'; import { environment } from '../../../environments/environment'; import { SortDirection } from '../../core/cache/models/sort-options.model'; +import { cold } from 'jasmine-marbles'; describe('BrowseByDatePageComponent', () => { let comp: BrowseByDatePageComponent; @@ -112,9 +113,13 @@ describe('BrowseByDatePageComponent', () => { fixture.detectChanges(); }); - it('should initialize the list of items', () => { + it('should initialize the list of items', (done: DoneFn) => { + expect(comp.loading$).toBeObservable(cold('(a|)', { + a: false, + })); comp.items$.subscribe((result) => { expect(result.payload.page).toEqual([firstItem]); + done(); }); }); diff --git a/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts b/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts index c52731a421f..d3c832d3e2e 100644 --- a/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts +++ b/src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts @@ -4,7 +4,7 @@ import { browseParamsToOptions, getBrowseSearchOptions } from '../browse-by-metadata-page/browse-by-metadata-page.component'; -import { combineLatest as observableCombineLatest } from 'rxjs'; +import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { ActivatedRoute, Params, Router } from '@angular/router'; import { BrowseService } from '../../core/browse/browse.service'; @@ -85,12 +85,21 @@ export class BrowseByDatePageComponent extends BrowseByMetadataPageComponent { * @param scope The scope under which to fetch the earliest item for */ updateStartsWithOptions(definition: string, metadataKeys: string[], scope?: string) { - const firstItemRD = this.browseService.getFirstItemFor(definition, scope, SortDirection.ASC); - const lastItemRD = this.browseService.getFirstItemFor(definition, scope, SortDirection.DESC); + const firstItemRD$: Observable> = this.browseService.getFirstItemFor(definition, scope, SortDirection.ASC); + const lastItemRD$: Observable> = this.browseService.getFirstItemFor(definition, scope, SortDirection.DESC); + this.loading$ = observableCombineLatest([ + firstItemRD$, + lastItemRD$, + ]).pipe( + map(([firstItemRD, lastItemRD]: [RemoteData, RemoteData]) => firstItemRD.isLoading || lastItemRD.isLoading) + ); this.subs.push( - observableCombineLatest([firstItemRD, lastItemRD]).subscribe(([firstItem, lastItem]) => { - let lowerLimit: number = this.getLimit(firstItem, metadataKeys, this.appConfig.browseBy.defaultLowerLimit); - let upperLimit: number = this.getLimit(lastItem, metadataKeys, new Date().getUTCFullYear()); + observableCombineLatest([ + firstItemRD$, + lastItemRD$, + ]).subscribe(([firstItemRD, lastItemRD]: [RemoteData, RemoteData]) => { + let lowerLimit: number = this.getLimit(firstItemRD, metadataKeys, this.appConfig.browseBy.defaultLowerLimit); + let upperLimit: number = this.getLimit(lastItemRD, metadataKeys, new Date().getUTCFullYear()); const options: number[] = []; const oneYearBreak: number = Math.floor((upperLimit - this.appConfig.browseBy.oneYearLimit) / 5) * 5; const fiveYearBreak: number = Math.floor((upperLimit - this.appConfig.browseBy.fiveYearLimit) / 10) * 10; diff --git a/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.html b/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.html index cfc2cbe3056..a6abd5a7a47 100644 --- a/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.html +++ b/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.html @@ -32,7 +32,7 @@
- - +
diff --git a/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.spec.ts b/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.spec.ts index a5beeb8a452..2bdecc26704 100644 --- a/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.spec.ts +++ b/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.spec.ts @@ -30,6 +30,7 @@ import { PaginationService } from '../../core/pagination/pagination.service'; import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { PaginationServiceStub } from '../../shared/testing/pagination-service.stub'; import { APP_CONFIG } from '../../../config/app-config.interface'; +import { cold } from 'jasmine-marbles'; describe('BrowseByMetadataPageComponent', () => { let comp: BrowseByMetadataPageComponent; @@ -149,9 +150,13 @@ describe('BrowseByMetadataPageComponent', () => { fixture.detectChanges(); }); - it('should fetch items', () => { + it('should fetch items', (done: DoneFn) => { + expect(comp.loading$).toBeObservable(cold('(a|)', { + a: false, + })); comp.items$.subscribe((result) => { expect(result.payload.page).toEqual(mockItems); + done(); }); }); diff --git a/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts b/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts index 75dbfcfc82a..c03cf03429f 100644 --- a/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts +++ b/src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts @@ -1,4 +1,4 @@ -import { combineLatest as observableCombineLatest, Observable, Subscription } from 'rxjs'; +import { combineLatest as observableCombineLatest, Observable, Subscription, of as observableOf } from 'rxjs'; import { Component, Inject, OnInit, OnDestroy } from '@angular/core'; import { RemoteData } from '../../core/data/remote-data'; import { PaginatedList } from '../../core/data/paginated-list.model'; @@ -122,6 +122,11 @@ export class BrowseByMetadataPageComponent implements OnInit, OnDestroy { */ fetchThumbnails: boolean; + /** + * Observable determining if the loading animation needs to be shown + */ + loading$ = observableOf(true); + public constructor(protected route: ActivatedRoute, protected browseService: BrowseService, protected dsoService: DSpaceObjectDataService, @@ -200,6 +205,9 @@ export class BrowseByMetadataPageComponent implements OnInit, OnDestroy { */ updatePage(searchOptions: BrowseEntrySearchOptions) { this.browseEntries$ = this.browseService.getBrowseEntriesFor(searchOptions); + this.loading$ = this.browseEntries$.pipe( + map((browseEntriesRD: RemoteData>) => browseEntriesRD.isLoading), + ); this.items$ = undefined; } @@ -214,6 +222,9 @@ export class BrowseByMetadataPageComponent implements OnInit, OnDestroy { */ updatePageWithItems(searchOptions: BrowseEntrySearchOptions, value: string, authority: string) { this.items$ = this.browseService.getBrowseItemsFor(value, authority, searchOptions); + this.loading$ = this.items$.pipe( + map((itemsRD: RemoteData>) => itemsRD.isLoading), + ); } /** From 25cb0455e2de428cd2bdabd7e30a74b2fc676a44 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Tue, 12 Dec 2023 00:34:12 +0100 Subject: [PATCH 2/2] Fixed bug in BrowseService where findListByHref was called with null instead of undefined, which prevented its default values from being used (cherry picked from commit e6bf2f0ca71adf02dca64a7a1a07de46c309a8d1) --- src/app/core/browse/browse.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index b210b349494..58bbc0b8700 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -105,7 +105,7 @@ export class BrowseService { }) ); if (options.fetchThumbnail ) { - return this.hrefOnlyDataService.findListByHref(href$, {}, null, null, ...BROWSE_LINKS_TO_FOLLOW); + return this.hrefOnlyDataService.findListByHref(href$, {}, undefined, undefined, ...BROWSE_LINKS_TO_FOLLOW); } return this.hrefOnlyDataService.findListByHref(href$); } @@ -153,7 +153,7 @@ export class BrowseService { }), ); if (options.fetchThumbnail) { - return this.hrefOnlyDataService.findListByHref(href$, {}, null, null, ...BROWSE_LINKS_TO_FOLLOW); + return this.hrefOnlyDataService.findListByHref(href$, {}, undefined, undefined, ...BROWSE_LINKS_TO_FOLLOW); } return this.hrefOnlyDataService.findListByHref(href$); }