Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

browse pages should not ignore sort config from back end #3741

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ describe('BrowseByDateComponent', () => {
getBrowseEntriesFor: (options: BrowseEntrySearchOptions) => toRemoteData([]),
getBrowseItemsFor: (value: string, options: BrowseEntrySearchOptions) => toRemoteData([firstItem]),
getFirstItemFor: (definition: string, scope?: string, sortDirection?: SortDirection) => null,
getConfiguredSortDirection: () => observableOf(SortDirection.DESC),
};

const mockDsoService = {
Expand Down
33 changes: 17 additions & 16 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 @@ -10,15 +10,17 @@ import {
} from '@angular/core';
import {
ActivatedRoute,
Params,
Router,
} from '@angular/router';
import { TranslateModule } from '@ngx-translate/core';
import {
combineLatest as observableCombineLatest,
Observable,
} from 'rxjs';
import { map } from 'rxjs/operators';
import {
map,
switchMap,
} from 'rxjs/operators';
import { ThemedBrowseByComponent } from 'src/app/shared/browse-by/themed-browse-by.component';

import {
Expand Down Expand Up @@ -47,13 +49,11 @@ 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 { VarDirective } from '../../shared/utils/var.directive';
import {
BrowseByMetadataComponent,
browseParamsToOptions,
getBrowseSearchOptions,
} from '../browse-by-metadata/browse-by-metadata.component';

@Component({
Expand Down Expand Up @@ -102,22 +102,23 @@ export class BrowseByDateComponent extends BrowseByMetadataComponent implements
}

ngOnInit(): void {
const sortConfig = new SortOptions('default', SortDirection.ASC);
this.browseId = this.route.snapshot.params.id;
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];
}),
).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.defaultBrowseId;
this.startsWith = +params.startsWith || params.startsWith;
this.browseId = params.id || this.defaultBrowseId;
const searchOptions = browseParamsToOptions(params, scope, currentPage, currentSort, this.browseId, this.fetchThumbnails);
this.updatePageWithItems(searchOptions, this.value, undefined);
this.updateStartsWithOptions(this.browseId, metadataKeys, params.scope);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ describe('BrowseByMetadataComponent', () => {
const mockBrowseService = {
getBrowseEntriesFor: (options: BrowseEntrySearchOptions) => toRemoteData(mockEntries),
getBrowseItemsFor: (value: string, options: BrowseEntrySearchOptions) => toRemoteData(mockItems),
getConfiguredSortDirection: () => observableOf(SortDirection.ASC),
};

const mockDsoService = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
} from '@angular/core';
import {
ActivatedRoute,
Params,
Router,
} from '@angular/router';
import { TranslateModule } from '@ngx-translate/core';
Expand All @@ -23,7 +22,10 @@ import {
of as observableOf,
Subscription,
} from 'rxjs';
import { map } from 'rxjs/operators';
import {
map,
switchMap,
} from 'rxjs/operators';
import { ThemedBrowseByComponent } from 'src/app/shared/browse-by/themed-browse-by.component';

import {
Expand Down Expand Up @@ -210,18 +212,18 @@ export class BrowseByMetadataComponent implements OnInit, OnChanges, OnDestroy {


ngOnInit(): void {

const sortConfig = new SortOptions('default', SortDirection.ASC);
this.updatePage(getBrowseSearchOptions(this.defaultBrowseId, this.paginationConfig, sortConfig));
this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig);
this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig);
this.browseId = this.route.snapshot.params.id;
this.subs.push(
observableCombineLatest([this.route.params, 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]) => {
this.browseId = params.id || this.defaultBrowseId;
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') {
Expand All @@ -243,9 +245,8 @@ export class BrowseByMetadataComponent implements OnInit, OnChanges, OnDestroy {
} else {
this.updatePage(browseParamsToOptions(params, scope, currentPage, currentSort, this.browseId, false));
}
this.updateStartsWithTextOptions();
}));
this.updateStartsWithTextOptions();

}

ngOnChanges(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { of as observableOf } 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';
Expand Down Expand Up @@ -73,6 +74,7 @@ describe('BrowseByTitleComponent', () => {
const mockBrowseService = {
getBrowseItemsFor: () => toRemoteData(mockItems),
getBrowseEntriesFor: () => toRemoteData([]),
getConfiguredSortDirection: () => observableOf(SortDirection.ASC),
};

const mockDsoService = {
Expand Down
33 changes: 17 additions & 16 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 @@ -6,10 +6,12 @@ import {
Component,
OnInit,
} from '@angular/core';
import { Params } from '@angular/router';
import { TranslateModule } from '@ngx-translate/core';
import { combineLatest as observableCombineLatest } from 'rxjs';
import { map } from 'rxjs/operators';
import {
map,
switchMap,
} from 'rxjs/operators';

import {
SortDirection,
Expand All @@ -23,12 +25,10 @@ import { ComcolPageHeaderComponent } from '../../shared/comcol/comcol-page-heade
import { ComcolPageLogoComponent } from '../../shared/comcol/comcol-page-logo/comcol-page-logo.component';
import { DsoEditMenuComponent } from '../../shared/dso-page/dso-edit-menu/dso-edit-menu.component';
import { ThemedLoadingComponent } from '../../shared/loading/themed-loading.component';
import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model';
import { VarDirective } from '../../shared/utils/var.directive';
import {
BrowseByMetadataComponent,
browseParamsToOptions,
getBrowseSearchOptions,
} from '../browse-by-metadata/browse-by-metadata.component';

@Component({
Expand Down Expand Up @@ -57,22 +57,23 @@ import {
export class BrowseByTitleComponent extends BrowseByMetadataComponent implements OnInit {

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.browseId = this.route.snapshot.params.id;
this.subs.push(
observableCombineLatest([this.route.params, 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]) => {
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.defaultBrowseId;
this.updatePageWithItems(browseParamsToOptions(params, scope, currentPage, currentSort, this.browseId, this.fetchThumbnails), undefined, undefined);
this.updateStartsWithTextOptions();
}));
this.updateStartsWithTextOptions();
}

}
23 changes: 23 additions & 0 deletions src/app/core/browse/browse.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,29 @@
return this.hrefOnlyDataService.findListByHref<BrowseEntry>(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<SortDirection>} The sort direction of the browse index
*/
getConfiguredSortDirection(browseId: string, defaultDirection: SortDirection): Observable<SortDirection> {
return this.getBrowseDefinitions().pipe(

Check warning on line 137 in src/app/core/browse/browse.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/browse/browse.service.ts#L137

Added line #L137 was not covered by tests
getRemoteDataPayload(),
getPaginatedListPayload(),
map((browseDefinitions: BrowseDefinition[]) => browseDefinitions
.find((def: BrowseDefinition) => def.id === browseId),

Check warning on line 141 in src/app/core/browse/browse.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/browse/browse.service.ts#L140-L141

Added lines #L140 - L141 were not covered by tests
),
map((browseDef: BrowseDefinition) => {
if (browseDef.order === SortDirection.ASC || browseDef.order === SortDirection.DESC) {
return browseDef.order;

Check warning on line 145 in src/app/core/browse/browse.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/browse/browse.service.ts#L145

Added line #L145 was not covered by tests
} else {
return defaultDirection;

Check warning on line 147 in src/app/core/browse/browse.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/browse/browse.service.ts#L147

Added line #L147 was not covered by tests
}
}),
);
}

/**
* Get all items linked to a certain metadata value
* @param {string} filterValue metadata value to filter by (e.g. author's name)
Expand Down
4 changes: 4 additions & 0 deletions src/app/core/shared/browse-definition.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { autoserialize } from 'cerialize';

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
Expand All @@ -11,6 +12,9 @@ export abstract class BrowseDefinition extends CacheableObject {
@autoserialize
id: string;

@autoserialize
order: SortDirection;

/**
* Get the render type of the BrowseDefinition model
*/
Expand Down
Loading