From 2dc67bdf06b49e6093f5831bba4268a5a37df320 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 26 Jan 2026 16:58:01 -0600 Subject: [PATCH] Merge pull request #3741 from atmire/w2p-122064_browse-pages-ignore-sort-config-fix-UI-main browse pages should not ignore sort config from back end --- .../browse-by-date.component.spec.ts | 1 + .../browse-by-date.component.ts | 34 ++++++++---------- .../browse-by-metadata.component.spec.ts | 1 + .../browse-by-metadata.component.ts | 34 +++++++----------- .../browse-by-title.component.spec.ts | 2 ++ .../browse-by-title.component.ts | 36 ++++++++----------- src/app/core/browse/browse.service.ts | 23 ++++++++++++ .../core/shared/browse-definition.model.ts | 4 +++ 8 files changed, 72 insertions(+), 63 deletions(-) diff --git a/src/app/browse-by/browse-by-date/browse-by-date.component.spec.ts b/src/app/browse-by/browse-by-date/browse-by-date.component.spec.ts index a40ffde2263..6547e7ae557 100644 --- a/src/app/browse-by/browse-by-date/browse-by-date.component.spec.ts +++ b/src/app/browse-by/browse-by-date/browse-by-date.component.spec.ts @@ -89,6 +89,7 @@ describe('BrowseByDateComponent', () => { getBrowseEntriesFor: (options: BrowseEntrySearchOptions) => toRemoteData([]), getBrowseItemsFor: (value: string, options: BrowseEntrySearchOptions) => toRemoteData([firstItem]), getFirstItemFor: (definition: string, scope?: string, sortDirection?: SortDirection) => null, + getConfiguredSortDirection: () => of(SortDirection.DESC), }; const mockDsoService = { 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 0e11c765156..50bd1343da0 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 @@ -11,7 +11,6 @@ import { } from '@angular/core'; import { ActivatedRoute, - Params, Router, } from '@angular/router'; import { TranslateModule } from '@ngx-translate/core'; @@ -21,8 +20,8 @@ import { of, } from 'rxjs'; import { - distinctUntilChanged, map, + switchMap, } from 'rxjs/operators'; import { ThemedBrowseByComponent } from 'src/app/shared/browse-by/themed-browse-by.component'; @@ -47,7 +46,6 @@ import { isNotEmpty, } from '../../shared/empty.util'; import { ThemedLoadingComponent } from '../../shared/loading/themed-loading.component'; -import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { StartsWithType } from '../../shared/starts-with/starts-with-type'; import { BrowseByMetadataComponent, @@ -96,27 +94,23 @@ export class BrowseByDateComponent extends BrowseByMetadataComponent implements this.loading$ = of(false); return; } - const sortConfig = new SortOptions('default', SortDirection.ASC); + this.browseId = this.route.snapshot.params.id; this.startsWithType = StartsWithType.date; - this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig); - this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig); - const routeParams$: Observable = observableCombineLatest([ - this.route.params, - this.route.queryParams, - ]).pipe( - map(([params, queryParams]: [Params, Params]) => Object.assign({}, params, queryParams)), - distinctUntilChanged((prev: Params, curr: Params) => prev.id === curr.id && prev.startsWith === curr.startsWith), - ); + this.subs.push( - observableCombineLatest([ - routeParams$, - this.scope$, - this.currentPagination$, - this.currentSort$, - ]).subscribe(([params, scope, currentPage, currentSort]: [Params, string, PaginationComponentOptions, SortOptions]) => { + this.browseService.getConfiguredSortDirection(this.browseId, SortDirection.ASC).pipe( + map((sortDir) => new SortOptions(this.browseId, sortDir)), + switchMap((sortConfig) => { + this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig); + this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig, false); + return observableCombineLatest([this.route.params, this.route.queryParams, this.scope$, this.route.data, this.currentPagination$, this.currentSort$]).pipe( + map(([routeParams, queryParams, scope, data, currentPage, currentSort]) => ({ + params: Object.assign({}, routeParams, queryParams, data), scope, currentPage, currentSort, + }))); + })).subscribe(({ params, scope, currentPage, currentSort }) => { const metadataKeys = params.browseDefinition ? params.browseDefinition.metadataKeys : this.defaultMetadataKeys; - this.browseId = params.id; this.startsWith = +params.startsWith || params.startsWith; + this.browseId = params.id; const searchOptions = browseParamsToOptions(params, scope, currentPage, currentSort, this.browseId, this.fetchThumbnails); this.updatePageWithItems(searchOptions, this.value, undefined); this.updateStartsWithOptions(this.browseId, metadataKeys, params.scope); diff --git a/src/app/browse-by/browse-by-metadata/browse-by-metadata.component.spec.ts b/src/app/browse-by/browse-by-metadata/browse-by-metadata.component.spec.ts index 46df8b1c0c3..c68979fa1aa 100644 --- a/src/app/browse-by/browse-by-metadata/browse-by-metadata.component.spec.ts +++ b/src/app/browse-by/browse-by-metadata/browse-by-metadata.component.spec.ts @@ -117,6 +117,7 @@ describe('BrowseByMetadataComponent', () => { const mockBrowseService = { getBrowseEntriesFor: (options: BrowseEntrySearchOptions) => toRemoteData(mockEntries), getBrowseItemsFor: (value: string, options: BrowseEntrySearchOptions) => toRemoteData(mockItems), + getConfiguredSortDirection: () => of(SortDirection.ASC), }; const mockDsoService = { 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 6f627b2da37..d30e959f5db 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 @@ -14,7 +14,6 @@ import { } from '@angular/core'; import { ActivatedRoute, - Params, Router, } from '@angular/router'; import { TranslateModule } from '@ngx-translate/core'; @@ -26,8 +25,8 @@ import { Subscription, } from 'rxjs'; import { - distinctUntilChanged, map, + switchMap, } from 'rxjs/operators'; import { ThemedBrowseByComponent } from 'src/app/shared/browse-by/themed-browse-by.component'; @@ -209,24 +208,18 @@ export class BrowseByMetadataComponent implements OnInit, OnChanges, OnDestroy { this.loading$ = of(false); return; } - const sortConfig = new SortOptions('default', SortDirection.ASC); - this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig); - this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig); - const routeParams$: Observable = observableCombineLatest([ - this.route.params, - this.route.queryParams, - ]).pipe( - map(([params, queryParams]: [Params, Params]) => Object.assign({}, params, queryParams)), - distinctUntilChanged((prev: Params, curr: Params) => prev.id === curr.id && prev.authority === curr.authority && prev.value === curr.value && prev.startsWith === curr.startsWith), - ); + this.browseId = this.route.snapshot.params.id; this.subs.push( - observableCombineLatest([ - routeParams$, - this.scope$, - this.currentPagination$, - this.currentSort$, - ]).subscribe(([params, scope, currentPage, currentSort]: [Params, string, PaginationComponentOptions, SortOptions]) => { - this.browseId = params.id; + this.browseService.getConfiguredSortDirection(this.browseId, SortDirection.ASC).pipe( + map((sortDir) => new SortOptions(this.browseId, sortDir)), + switchMap((sortConfig) => { + this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig, false); + this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig); + return observableCombineLatest([this.route.params, this.route.queryParams, this.scope$, this.currentPagination$, this.currentSort$]).pipe( + map(([routeParams, queryParams, scope, currentPage, currentSort]) => ({ + params: Object.assign({}, routeParams, queryParams), scope, currentPage, currentSort, + }))); + })).subscribe(({ params, scope, currentPage, currentSort }) => { this.authority = params.authority; if (typeof params.value === 'string') { @@ -250,9 +243,8 @@ export class BrowseByMetadataComponent implements OnInit, OnChanges, OnDestroy { } else { this.updatePage(browseParamsToOptions(params, scope, currentPage, currentSort, this.browseId, false)); } + this.updateStartsWithTextOptions(); })); - this.updateStartsWithTextOptions(); - } ngOnChanges(changes: SimpleChanges): void { diff --git a/src/app/browse-by/browse-by-title/browse-by-title.component.spec.ts b/src/app/browse-by/browse-by-title/browse-by-title.component.spec.ts index dbcb70f5176..5c648150e1c 100644 --- a/src/app/browse-by/browse-by-title/browse-by-title.component.spec.ts +++ b/src/app/browse-by/browse-by-title/browse-by-title.component.spec.ts @@ -22,6 +22,7 @@ import { of } from 'rxjs'; import { APP_CONFIG } from '../../../config/app-config.interface'; import { environment } from '../../../environments/environment'; import { BrowseService } from '../../core/browse/browse.service'; +import { SortDirection } from '../../core/cache/models/sort-options.model'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; import { ItemDataService } from '../../core/data/item-data.service'; import { PaginationService } from '../../core/pagination/pagination.service'; @@ -76,6 +77,7 @@ describe('BrowseByTitleComponent', () => { const mockBrowseService = { getBrowseItemsFor: () => toRemoteData(mockItems), getBrowseEntriesFor: () => toRemoteData([]), + getConfiguredSortDirection: () => of(SortDirection.ASC), }; const mockDsoService = { 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 cddc98d385f..58a12a94a50 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 @@ -6,16 +6,14 @@ import { Component, OnInit, } from '@angular/core'; -import { Params } from '@angular/router'; import { TranslateModule } from '@ngx-translate/core'; import { combineLatest as observableCombineLatest, - Observable, of, } from 'rxjs'; import { - distinctUntilChanged, map, + switchMap, } from 'rxjs/operators'; import { environment } from '../../../environments/environment'; @@ -25,7 +23,6 @@ import { } from '../../core/cache/models/sort-options.model'; import { ThemedBrowseByComponent } from '../../shared/browse-by/themed-browse-by.component'; import { ThemedLoadingComponent } from '../../shared/loading/themed-loading.component'; -import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { BrowseByMetadataComponent, browseParamsToOptions, @@ -52,28 +49,23 @@ export class BrowseByTitleComponent extends BrowseByMetadataComponent implements this.loading$ = of(false); return; } - const sortConfig = new SortOptions('dc.title', SortDirection.ASC); - this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig); - this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig); - const routeParams$: Observable = observableCombineLatest([ - this.route.params, - this.route.queryParams, - ]).pipe( - map(([params, queryParams]: [Params, Params]) => Object.assign({}, params, queryParams)), - distinctUntilChanged((prev: Params, curr: Params) => prev.id === curr.id && prev.startsWith === curr.startsWith), - ); + this.browseId = this.route.snapshot.params.id; this.subs.push( - observableCombineLatest([ - routeParams$, - this.scope$, - this.currentPagination$, - this.currentSort$, - ]).subscribe(([params, scope, currentPage, currentSort]: [Params, string, PaginationComponentOptions, SortOptions]) => { + this.browseService.getConfiguredSortDirection(this.browseId, SortDirection.ASC).pipe( + map((sortDir) => new SortOptions(this.browseId, sortDir)), + switchMap((sortConfig) => { + this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig, false); + this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig); + return observableCombineLatest([this.route.params, this.route.queryParams, this.scope$, this.currentPagination$, this.currentSort$]).pipe( + map(([routeParams, queryParams, scope, currentPage, currentSort]) => ({ + params: Object.assign({}, routeParams, queryParams), scope, currentPage, currentSort, + })), + ); + })).subscribe(({ params, scope, currentPage, currentSort }) => { this.startsWith = +params.startsWith || params.startsWith; - this.browseId = params.id; this.updatePageWithItems(browseParamsToOptions(params, scope, currentPage, currentSort, this.browseId, this.fetchThumbnails), undefined, undefined); + this.updateStartsWithTextOptions(); })); - this.updateStartsWithTextOptions(); } } diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index 5fe06a700e5..28c0fa0a439 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -127,6 +127,29 @@ export class BrowseService { return this.hrefOnlyDataService.findListByHref(href$); } + /* + * Get the sort direction for a browse index based on its unique id + * @param browseId The unique id of the browse index + * @param defaultDirection The default sort direction to return if the browse index has no sort direction configured + * @returns {Observable} The sort direction of the browse index + */ + getConfiguredSortDirection(browseId: string, defaultDirection: SortDirection): Observable { + return this.getBrowseDefinitions().pipe( + getRemoteDataPayload(), + getPaginatedListPayload(), + map((browseDefinitions: BrowseDefinition[]) => browseDefinitions + .find((def: BrowseDefinition) => def.id === browseId), + ), + map((browseDef: BrowseDefinition) => { + if (browseDef.order === SortDirection.ASC || browseDef.order === SortDirection.DESC) { + return browseDef.order; + } else { + return defaultDirection; + } + }), + ); + } + /** * Get all items linked to a certain metadata value * @param {string} filterValue metadata value to filter by (e.g. author's name) diff --git a/src/app/core/shared/browse-definition.model.ts b/src/app/core/shared/browse-definition.model.ts index 5fe5d02ecb4..3f5e6e1e041 100644 --- a/src/app/core/shared/browse-definition.model.ts +++ b/src/app/core/shared/browse-definition.model.ts @@ -5,6 +5,7 @@ import { import { BrowseByDataType } from '../../browse-by/browse-by-switcher/browse-by-data-type'; import { CacheableObject } from '../cache/cacheable-object.model'; +import { SortDirection } from '../cache/models/sort-options.model'; /** * Base class for BrowseDefinition models @@ -17,6 +18,9 @@ export abstract class BrowseDefinition extends CacheableObject { @autoserializeAs('metadata') metadataKeys: string[]; + @autoserialize + order: SortDirection; + /** * Get the render type of the BrowseDefinition model */