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
(cherry picked from commit a105bcd)
  • Loading branch information
Koen Pauwels authored and Jens Vannerum committed Dec 30, 2024
1 parent d578222 commit 5037c00
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { BrowseService } from '../../core/browse/browse.service';
import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service';
import { StartsWithType } from '../../shared/starts-with/starts-with-decorator';
import { PaginationService } from '../../core/pagination/pagination.service';
import { map } from 'rxjs/operators';
import { map, take } from 'rxjs/operators';
import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model';
import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model';
import { isValidDate } from '../../shared/date.util';
Expand Down Expand Up @@ -52,15 +52,16 @@ export class BrowseByDatePageComponent extends BrowseByMetadataPageComponent {
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.route.data,
this.currentPagination$, this.currentSort$]).pipe(
map(([routeParams, queryParams, data, currentPage, currentSort]) => {
return [Object.assign({}, routeParams, queryParams, data), currentPage, currentSort];
observableCombineLatest(
[ this.route.params.pipe(take(1)),
this.route.queryParams,
this.currentPagination$,
this.currentSort$]).pipe(
map(([routeParams, queryParams, currentPage, currentSort]) => {
return [Object.assign({}, routeParams, queryParams), currentPage, currentSort];
})
).subscribe(([params, currentPage, currentSort]: [Params, 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 @@ -15,7 +15,7 @@ import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.serv
import { DSpaceObject } from '../../core/shared/dspace-object.model';
import { StartsWithType } from '../../shared/starts-with/starts-with-decorator';
import { PaginationService } from '../../core/pagination/pagination.service';
import { filter, map, mergeMap } from 'rxjs/operators';
import { filter, map, mergeMap, take } from 'rxjs/operators';
import { followLink, FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
import { Bitstream } from '../../core/shared/bitstream.model';
import { Collection } from '../../core/shared/collection.model';
Expand Down Expand Up @@ -152,7 +152,11 @@ export class BrowseByMetadataPageComponent implements OnInit, 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.currentPagination$, this.currentSort$]).pipe(
observableCombineLatest(
[ this.route.params.pipe(take(1)),
this.route.queryParams,
this.currentPagination$,
this.currentSort$]).pipe(
map(([routeParams, queryParams, currentPage, currentSort]) => {
return [Object.assign({}, routeParams, queryParams),currentPage,currentSort];
})
Expand Down
Empty file.
12 changes: 8 additions & 4 deletions src/app/menu.resolver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ import { MenuServiceStub } from './shared/testing/menu-service.stub';
import { MenuID } from './shared/menu/menu-id.model';
import { BrowseService } from './core/browse/browse.service';
import { cold } from 'jasmine-marbles';
import createSpy = jasmine.createSpy;
import { createSuccessfulRemoteDataObject$ } from './shared/remote-data.utils';
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 @@ -61,6 +64,7 @@ describe('MenuResolver', () => {
{ provide: BrowseService, useValue: browseService },
{ provide: AuthorizationDataService, useValue: authorizationService },
{ provide: ScriptDataService, useValue: scriptService },
{ provide: AuthService, useValue: AuthServiceStub },
{
provide: NgbModal, useValue: {
open: () => {/*comment*/
Expand All @@ -80,19 +84,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
13 changes: 12 additions & 1 deletion src/app/menu.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
import {
ExportBatchSelectorComponent
} from './shared/dso-selector/modal-wrappers/export-batch-selector/export-batch-selector.component';
import { AuthService } from './core/auth/auth.service';

/**
* Creates all of the app's menus
Expand All @@ -61,6 +62,7 @@ export class MenuResolver implements Resolve<boolean> {
protected authorizationService: AuthorizationDataService,
protected modalService: NgbModal,
protected scriptDataService: ScriptDataService,
protected authService: AuthService,
) {
}

Expand All @@ -70,7 +72,7 @@ export class MenuResolver implements Resolve<boolean> {
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 @@ -146,6 +148,15 @@ export class MenuResolver implements Resolve<boolean> {
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 Down

0 comments on commit 5037c00

Please sign in to comment.