From a105bcd6f45d8e85844cd473964670e1461b29cf Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Mon, 2 Dec 2024 14:51:08 +0100 Subject: [PATCH] 121787: Get rid of multiple unnecessary requests on browse by pages --- .../browse-by-date.component.ts | 21 ++++++++++++------- .../browse-by-metadata.component.ts | 13 ++++++++++-- .../browse-by-page.component.ts | 4 ++-- .../browse-by-switcher.component.spec.ts | 2 +- .../browse-by-switcher.component.ts | 4 ++-- .../browse-by-title.component.ts | 16 +++++++++----- src/app/menu-resolver.service.ts | 17 ++++++++++++--- src/app/menu.resolver.spec.ts | 9 +++++--- 8 files changed, 60 insertions(+), 26 deletions(-) diff --git a/src/app/browse-by/browse-by-date/browse-by-date.component.ts b/src/app/browse-by/browse-by-date/browse-by-date.component.ts index ee33ee533c8..11818ff5f15 100644 --- a/src/app/browse-by/browse-by-date/browse-by-date.component.ts +++ b/src/app/browse-by/browse-by-date/browse-by-date.component.ts @@ -18,7 +18,10 @@ import { combineLatest as observableCombineLatest, Observable, } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { + map, + take, +} from 'rxjs/operators'; import { ThemedBrowseByComponent } from 'src/app/shared/browse-by/themed-browse-by.component'; import { @@ -53,7 +56,6 @@ import { VarDirective } from '../../shared/utils/var.directive'; import { BrowseByMetadataComponent, browseParamsToOptions, - getBrowseSearchOptions, } from '../browse-by-metadata/browse-by-metadata.component'; @Component({ @@ -104,15 +106,18 @@ export class BrowseByDateComponent extends BrowseByMetadataComponent implements ngOnInit(): void { const sortConfig = new SortOptions('default', SortDirection.ASC); this.startsWithType = StartsWithType.date; - // include the thumbnail configuration in browse search options - this.updatePage(getBrowseSearchOptions(this.defaultBrowseId, this.paginationConfig, sortConfig, this.fetchThumbnails)); this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig); this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig); this.subs.push( - observableCombineLatest([this.route.params, this.route.queryParams, this.scope$, this.route.data, - this.currentPagination$, this.currentSort$]).pipe( - map(([routeParams, queryParams, scope, data, currentPage, currentSort]) => { - return [Object.assign({}, routeParams, queryParams, data), scope, currentPage, currentSort]; + observableCombineLatest( + [ this.route.params.pipe(take(1)), + this.route.queryParams, + this.scope$, + this.currentPagination$, + this.currentSort$, + ]).pipe( + map(([routeParams, queryParams, scope, currentPage, currentSort]) => { + return [Object.assign({}, routeParams, queryParams), scope, currentPage, currentSort]; }), ).subscribe(([params, scope, currentPage, currentSort]: [Params, string, PaginationComponentOptions, SortOptions]) => { const metadataKeys = params.browseDefinition ? params.browseDefinition.metadataKeys : this.defaultMetadataKeys; diff --git a/src/app/browse-by/browse-by-metadata/browse-by-metadata.component.ts b/src/app/browse-by/browse-by-metadata/browse-by-metadata.component.ts index 29bc48b1bdc..d17c2a1a7b4 100644 --- a/src/app/browse-by/browse-by-metadata/browse-by-metadata.component.ts +++ b/src/app/browse-by/browse-by-metadata/browse-by-metadata.component.ts @@ -23,7 +23,10 @@ import { of as observableOf, Subscription, } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { + map, + take, +} from 'rxjs/operators'; import { ThemedBrowseByComponent } from 'src/app/shared/browse-by/themed-browse-by.component'; import { @@ -216,7 +219,13 @@ export class BrowseByMetadataComponent implements OnInit, OnChanges, OnDestroy { this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig); this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig); this.subs.push( - observableCombineLatest([this.route.params, this.route.queryParams, this.scope$, this.currentPagination$, this.currentSort$]).pipe( + observableCombineLatest( + [ this.route.params.pipe(take(1)), + this.route.queryParams, + this.scope$, + this.currentPagination$, + this.currentSort$, + ]).pipe( map(([routeParams, queryParams, scope, currentPage, currentSort]) => { return [Object.assign({}, routeParams, queryParams), scope, currentPage, currentSort]; }), diff --git a/src/app/browse-by/browse-by-page/browse-by-page.component.ts b/src/app/browse-by/browse-by-page/browse-by-page.component.ts index 1a204264d15..670b5a7711c 100644 --- a/src/app/browse-by/browse-by-page/browse-by-page.component.ts +++ b/src/app/browse-by/browse-by-page/browse-by-page.component.ts @@ -23,7 +23,7 @@ import { BrowseBySwitcherComponent } from '../browse-by-switcher/browse-by-switc }) export class BrowseByPageComponent implements OnInit { - browseByType$: Observable; + browseByType$: Observable<{type: BrowseByDataType }>; constructor( protected route: ActivatedRoute, @@ -35,7 +35,7 @@ export class BrowseByPageComponent implements OnInit { */ ngOnInit(): void { this.browseByType$ = this.route.data.pipe( - map((data: { browseDefinition: BrowseDefinition }) => data.browseDefinition.getRenderType()), + map((data: { browseDefinition: BrowseDefinition }) => ({ type: data.browseDefinition.getRenderType() })), ); } diff --git a/src/app/browse-by/browse-by-switcher/browse-by-switcher.component.spec.ts b/src/app/browse-by/browse-by-switcher/browse-by-switcher.component.spec.ts index d56671577ed..8539365f668 100644 --- a/src/app/browse-by/browse-by-switcher/browse-by-switcher.component.spec.ts +++ b/src/app/browse-by/browse-by-switcher/browse-by-switcher.component.spec.ts @@ -85,7 +85,7 @@ describe('BrowseBySwitcherComponent', () => { types.forEach((type: NonHierarchicalBrowseDefinition) => { describe(`when switching to a browse-by page for "${type.id}"`, () => { beforeEach(async () => { - comp.browseByType = type.dataType; + comp.browseByType = type as any; comp.ngOnChanges({ browseByType: new SimpleChange(undefined, type.dataType, true), }); diff --git a/src/app/browse-by/browse-by-switcher/browse-by-switcher.component.ts b/src/app/browse-by/browse-by-switcher/browse-by-switcher.component.ts index c1e3dae79f0..53be5fa786d 100644 --- a/src/app/browse-by/browse-by-switcher/browse-by-switcher.component.ts +++ b/src/app/browse-by/browse-by-switcher/browse-by-switcher.component.ts @@ -24,7 +24,7 @@ export class BrowseBySwitcherComponent extends AbstractComponentLoaderComponent< @Input() context: Context; - @Input() browseByType: BrowseByDataType; + @Input() browseByType: { type: BrowseByDataType }; @Input() displayTitle: boolean; @@ -43,7 +43,7 @@ export class BrowseBySwitcherComponent extends AbstractComponentLoaderComponent< ]; public getComponent(): GenericConstructor { - return getComponentByBrowseByType(this.browseByType, this.context, this.themeService.getThemeName()); + return getComponentByBrowseByType(this.browseByType.type, this.context, this.themeService.getThemeName()); } } diff --git a/src/app/browse-by/browse-by-title/browse-by-title.component.ts b/src/app/browse-by/browse-by-title/browse-by-title.component.ts index aa5daa31661..296386d9d94 100644 --- a/src/app/browse-by/browse-by-title/browse-by-title.component.ts +++ b/src/app/browse-by/browse-by-title/browse-by-title.component.ts @@ -9,7 +9,10 @@ import { import { Params } from '@angular/router'; import { TranslateModule } from '@ngx-translate/core'; import { combineLatest as observableCombineLatest } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { + map, + take, +} from 'rxjs/operators'; import { SortDirection, @@ -28,7 +31,6 @@ import { VarDirective } from '../../shared/utils/var.directive'; import { BrowseByMetadataComponent, browseParamsToOptions, - getBrowseSearchOptions, } from '../browse-by-metadata/browse-by-metadata.component'; @Component({ @@ -58,12 +60,16 @@ export class BrowseByTitleComponent extends BrowseByMetadataComponent implements ngOnInit(): void { const sortConfig = new SortOptions('dc.title', SortDirection.ASC); - // include the thumbnail configuration in browse search options - this.updatePage(getBrowseSearchOptions(this.defaultBrowseId, this.paginationConfig, sortConfig, this.fetchThumbnails)); this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig); this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig); this.subs.push( - observableCombineLatest([this.route.params, this.route.queryParams, this.scope$, this.currentPagination$, this.currentSort$]).pipe( + observableCombineLatest( + [ this.route.params.pipe(take(1)), + this.route.queryParams, + this.scope$, + this.currentPagination$, + this.currentSort$, + ]).pipe( map(([routeParams, queryParams, scope, currentPage, currentSort]) => { return [Object.assign({}, routeParams, queryParams), scope, currentPage, currentSort]; }), diff --git a/src/app/menu-resolver.service.ts b/src/app/menu-resolver.service.ts index 144cad4547c..683111b32ad 100644 --- a/src/app/menu-resolver.service.ts +++ b/src/app/menu-resolver.service.ts @@ -7,7 +7,9 @@ import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { combineLatest, combineLatest as observableCombineLatest, + mergeMap, Observable, + of as observableOf, } from 'rxjs'; import { filter, @@ -17,6 +19,7 @@ import { } from 'rxjs/operators'; import { PUBLICATION_CLAIMS_PATH } from './admin/admin-notifications/admin-notifications-routing-paths'; +import { AuthService } from './core/auth/auth.service'; import { BrowseService } from './core/browse/browse.service'; import { ConfigurationDataService } from './core/data/configuration-data.service'; import { AuthorizationDataService } from './core/data/feature-authorization/authorization-data.service'; @@ -62,6 +65,7 @@ export class MenuResolverService { protected modalService: NgbModal, protected scriptDataService: ScriptDataService, protected configurationDataService: ConfigurationDataService, + protected authService: AuthService, ) { } @@ -71,7 +75,7 @@ export class MenuResolverService { resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { return combineLatest([ this.createPublicMenu$(), - this.createAdminMenu$(), + this.createAdminMenuIfLoggedIn$(), ]).pipe( map((menusDone: boolean[]) => menusDone.every(Boolean)), ); @@ -147,6 +151,15 @@ export class MenuResolverService { return this.waitForMenu$(MenuID.PUBLIC); } + /** + * Initialize all menu sections and items for {@link MenuID.ADMIN}, only if the user is logged in. + */ + createAdminMenuIfLoggedIn$() { + return this.authService.isAuthenticated().pipe( + mergeMap((isAuthenticated) => isAuthenticated ? this.createAdminMenu$() : observableOf(true)), + ); + } + /** * Initialize all menu sections and items for {@link MenuID.ADMIN} */ @@ -156,8 +169,6 @@ export class MenuResolverService { this.createExportMenuSections(); this.createImportMenuSections(); this.createAccessControlMenuSections(); - this.createReportMenuSections(); - return this.waitForMenu$(MenuID.ADMIN); } diff --git a/src/app/menu.resolver.spec.ts b/src/app/menu.resolver.spec.ts index 386bbb0cae4..720c0711044 100644 --- a/src/app/menu.resolver.spec.ts +++ b/src/app/menu.resolver.spec.ts @@ -26,7 +26,9 @@ import { ConfigurationDataServiceStub } from './shared/testing/configuration-dat import { MenuServiceStub } from './shared/testing/menu-service.stub'; import { createPaginatedList } from './shared/testing/utils.test'; import createSpy = jasmine.createSpy; +import { AuthService } from './core/auth/auth.service'; import { MenuResolverService } from './menu-resolver.service'; +import { AuthServiceStub } from './shared/testing/auth-service.stub'; const BOOLEAN = { t: true, f: false }; const MENU_STATE = { @@ -80,6 +82,7 @@ describe('menuResolver', () => { { provide: ScriptDataService, useValue: scriptService }, { provide: ConfigurationDataService, useValue: configurationDataService }, { provide: NgbModal, useValue: mockNgbModal }, + { provide: AuthService, useValue: AuthServiceStub }, MenuResolverService, ], schemas: [NO_ERRORS_SCHEMA], @@ -94,19 +97,19 @@ describe('menuResolver', () => { describe('resolve', () => { it('should create all menus', (done) => { spyOn(resolver, 'createPublicMenu$').and.returnValue(observableOf(true)); - spyOn(resolver, 'createAdminMenu$').and.returnValue(observableOf(true)); + spyOn(resolver, 'createAdminMenuIfLoggedIn$').and.returnValue(observableOf(true)); resolver.resolve(null, null).subscribe(resolved => { expect(resolved).toBeTrue(); expect(resolver.createPublicMenu$).toHaveBeenCalled(); - expect(resolver.createAdminMenu$).toHaveBeenCalled(); + expect(resolver.createAdminMenuIfLoggedIn$).toHaveBeenCalled(); done(); }); }); it('should return an Observable that emits true as soon as all menus are created', () => { spyOn(resolver, 'createPublicMenu$').and.returnValue(cold('--(t|)', BOOLEAN)); - spyOn(resolver, 'createAdminMenu$').and.returnValue(cold('----(t|)', BOOLEAN)); + spyOn(resolver, 'createAdminMenuIfLoggedIn$').and.returnValue(cold('----(t|)', BOOLEAN)); expect(resolver.resolve(null, null)).toBeObservable(cold('----(t|)', BOOLEAN)); });