Skip to content

Commit

Permalink
121787: Get rid of multiple unnecessary requests on browse by pages
Browse files Browse the repository at this point in the history
  • Loading branch information
Koen Pauwels authored and Jens Vannerum committed Dec 19, 2024
1 parent b454e49 commit a105bcd
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 26 deletions.
21 changes: 13 additions & 8 deletions src/app/browse-by/browse-by-date/browse-by-date.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -53,7 +56,6 @@ import { VarDirective } from '../../shared/utils/var.directive';
import {
BrowseByMetadataComponent,
browseParamsToOptions,
getBrowseSearchOptions,
} from '../browse-by-metadata/browse-by-metadata.component';

@Component({
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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];
}),
Expand Down
4 changes: 2 additions & 2 deletions src/app/browse-by/browse-by-page/browse-by-page.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { BrowseBySwitcherComponent } from '../browse-by-switcher/browse-by-switc
})
export class BrowseByPageComponent implements OnInit {

browseByType$: Observable<BrowseByDataType>;
browseByType$: Observable<{type: BrowseByDataType }>;

constructor(
protected route: ActivatedRoute,
Expand All @@ -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() })),
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class BrowseBySwitcherComponent extends AbstractComponentLoaderComponent<

@Input() context: Context;

@Input() browseByType: BrowseByDataType;
@Input() browseByType: { type: BrowseByDataType };

@Input() displayTitle: boolean;

Expand All @@ -43,7 +43,7 @@ export class BrowseBySwitcherComponent extends AbstractComponentLoaderComponent<
];

public getComponent(): GenericConstructor<Component> {
return getComponentByBrowseByType(this.browseByType, this.context, this.themeService.getThemeName());
return getComponentByBrowseByType(this.browseByType.type, this.context, this.themeService.getThemeName());
}

}
16 changes: 11 additions & 5 deletions src/app/browse-by/browse-by-title/browse-by-title.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -28,7 +31,6 @@ import { VarDirective } from '../../shared/utils/var.directive';
import {
BrowseByMetadataComponent,
browseParamsToOptions,
getBrowseSearchOptions,
} from '../browse-by-metadata/browse-by-metadata.component';

@Component({
Expand Down Expand Up @@ -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];
}),
Expand Down
17 changes: 14 additions & 3 deletions src/app/menu-resolver.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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';
Expand Down Expand Up @@ -62,6 +65,7 @@ export class MenuResolverService {
protected modalService: NgbModal,
protected scriptDataService: ScriptDataService,
protected configurationDataService: ConfigurationDataService,
protected authService: AuthService,
) {
}

Expand All @@ -71,7 +75,7 @@ export class MenuResolverService {
resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean> {
return combineLatest([
this.createPublicMenu$(),
this.createAdminMenu$(),
this.createAdminMenuIfLoggedIn$(),
]).pipe(
map((menusDone: boolean[]) => menusDone.every(Boolean)),
);
Expand Down Expand Up @@ -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}
*/
Expand All @@ -156,8 +169,6 @@ export class MenuResolverService {
this.createExportMenuSections();
this.createImportMenuSections();
this.createAccessControlMenuSections();
this.createReportMenuSections();

return this.waitForMenu$(MenuID.ADMIN);
}

Expand Down
9 changes: 6 additions & 3 deletions src/app/menu.resolver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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],
Expand All @@ -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));
});
Expand Down

0 comments on commit a105bcd

Please sign in to comment.