From c10e660e0b1da868065f1c8ecdd4709636a1e5b9 Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Wed, 10 May 2023 10:11:06 +0200 Subject: [PATCH 01/14] 101623: Refactor BrowseDefinition model/resource-type to FlatBrowseDefinition --- src/app/browse-by/browse-by-guard.spec.ts | 4 ++-- src/app/browse-by/browse-by-guard.ts | 4 ++-- .../browse-by-switcher.component.spec.ts | 14 +++++++------- .../browse-by-switcher.component.ts | 4 ++-- .../core/browse/browse-definition-data.service.ts | 8 ++++---- src/app/core/browse/browse.service.spec.ts | 6 +++--- src/app/core/browse/browse.service.ts | 10 +++++----- src/app/core/core.module.ts | 4 ++-- ...on.model.ts => flat-browse-definition.model.ts} | 9 ++++----- .../shared/flat-browse-definition.resource-type.ts | 9 +++++++++ src/app/core/shared/operators.ts | 12 ++++++------ src/app/menu.resolver.ts | 8 ++++---- src/app/navbar/navbar.component.spec.ts | 10 +++++----- .../comcol-page-browse-by.component.ts | 8 ++++---- 14 files changed, 59 insertions(+), 51 deletions(-) rename src/app/core/shared/{browse-definition.model.ts => flat-browse-definition.model.ts} (81%) create mode 100644 src/app/core/shared/flat-browse-definition.resource-type.ts diff --git a/src/app/browse-by/browse-by-guard.spec.ts b/src/app/browse-by/browse-by-guard.spec.ts index 933c95a3cb..8a9f9b8c50 100644 --- a/src/app/browse-by/browse-by-guard.spec.ts +++ b/src/app/browse-by/browse-by-guard.spec.ts @@ -2,7 +2,7 @@ import { first } from 'rxjs/operators'; import { BrowseByGuard } from './browse-by-guard'; import { of as observableOf } from 'rxjs'; import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; -import { BrowseDefinition } from '../core/shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../core/shared/flat-browse-definition.model'; import { BrowseByDataType } from './browse-by-switcher/browse-by-decorator'; describe('BrowseByGuard', () => { @@ -18,7 +18,7 @@ describe('BrowseByGuard', () => { const id = 'author'; const scope = '1234-65487-12354-1235'; const value = 'Filter'; - const browseDefinition = Object.assign(new BrowseDefinition(), { type: BrowseByDataType.Metadata, metadataKeys: ['dc.contributor'] }); + const browseDefinition = Object.assign(new FlatBrowseDefinition(), { type: BrowseByDataType.Metadata, metadataKeys: ['dc.contributor'] }); beforeEach(() => { dsoService = { diff --git a/src/app/browse-by/browse-by-guard.ts b/src/app/browse-by/browse-by-guard.ts index e4582cb77a..f42359b56b 100644 --- a/src/app/browse-by/browse-by-guard.ts +++ b/src/app/browse-by/browse-by-guard.ts @@ -7,7 +7,7 @@ import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from import { TranslateService } from '@ngx-translate/core'; import { Observable, of as observableOf } from 'rxjs'; import { BrowseDefinitionDataService } from '../core/browse/browse-definition-data.service'; -import { BrowseDefinition } from '../core/shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../core/shared/flat-browse-definition.model'; @Injectable() /** @@ -23,7 +23,7 @@ export class BrowseByGuard implements CanActivate { canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) { const title = route.data.title; const id = route.params.id || route.queryParams.id || route.data.id; - let browseDefinition$: Observable; + let browseDefinition$: Observable; if (hasNoValue(route.data.browseDefinition) && hasValue(id)) { browseDefinition$ = this.browseDefinitionService.findById(id).pipe(getFirstSucceededRemoteDataPayload()); } else { 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 c2e1c9cb68..91c6c29252 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 @@ -3,7 +3,7 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { BROWSE_BY_COMPONENT_FACTORY, BrowseByDataType } from './browse-by-decorator'; -import { BrowseDefinition } from '../../core/shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../../core/shared/flat-browse-definition.model'; import { BehaviorSubject } from 'rxjs'; import { ThemeService } from '../../shared/theme-support/theme.service'; @@ -13,33 +13,33 @@ describe('BrowseBySwitcherComponent', () => { const types = [ Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'title', dataType: BrowseByDataType.Title, } ), Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'dateissued', dataType: BrowseByDataType.Date, metadataKeys: ['dc.date.issued'] } ), Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'author', dataType: BrowseByDataType.Metadata, } ), Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'subject', dataType: BrowseByDataType.Metadata, } ), ]; - const data = new BehaviorSubject(createDataWithBrowseDefinition(new BrowseDefinition())); + const data = new BehaviorSubject(createDataWithBrowseDefinition(new FlatBrowseDefinition())); const activatedRouteStub = { data @@ -70,7 +70,7 @@ describe('BrowseBySwitcherComponent', () => { comp = fixture.componentInstance; })); - types.forEach((type: BrowseDefinition) => { + types.forEach((type: FlatBrowseDefinition) => { describe(`when switching to a browse-by page for "${type.id}"`, () => { beforeEach(() => { data.next(createDataWithBrowseDefinition(type)); 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 0d3a35bebf..3cfc735135 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 @@ -4,7 +4,7 @@ import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { BROWSE_BY_COMPONENT_FACTORY } from './browse-by-decorator'; import { GenericConstructor } from '../../core/shared/generic-constructor'; -import { BrowseDefinition } from '../../core/shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../../core/shared/flat-browse-definition.model'; import { ThemeService } from '../../shared/theme-support/theme.service'; @Component({ @@ -31,7 +31,7 @@ export class BrowseBySwitcherComponent implements OnInit { */ ngOnInit(): void { this.browseByComponent = this.route.data.pipe( - map((data: { browseDefinition: BrowseDefinition }) => this.getComponentByBrowseByType(data.browseDefinition.dataType, this.themeService.getThemeName())) + map((data: { browseDefinition: FlatBrowseDefinition }) => this.getComponentByBrowseByType(data.browseDefinition.dataType, this.themeService.getThemeName())) ); } diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index 32c3b44e14..d9c401fc3f 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@angular/core'; import { BROWSE_DEFINITION } from '../shared/browse-definition.resource-type'; -import { BrowseDefinition } from '../shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; import { RequestService } from '../data/request.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { ObjectCacheService } from '../cache/object-cache.service'; @@ -21,8 +21,8 @@ import { dataService } from '../data/base/data-service.decorator'; providedIn: 'root', }) @dataService(BROWSE_DEFINITION) -export class BrowseDefinitionDataService extends IdentifiableDataService implements FindAllData { - private findAllData: FindAllDataImpl; +export class BrowseDefinitionDataService extends IdentifiableDataService implements FindAllData { + private findAllData: FindAllDataImpl; constructor( protected requestService: RequestService, @@ -49,7 +49,7 @@ export class BrowseDefinitionDataService extends IdentifiableDataService>>} * Return an observable that emits object list */ - findAll(options: FindListOptions = {}, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig[]): Observable>> { + findAll(options: FindListOptions = {}, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig[]): Observable>> { return this.findAllData.findAll(options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } } diff --git a/src/app/core/browse/browse.service.spec.ts b/src/app/core/browse/browse.service.spec.ts index 46ac8c44f4..9e0615385b 100644 --- a/src/app/core/browse/browse.service.spec.ts +++ b/src/app/core/browse/browse.service.spec.ts @@ -6,7 +6,7 @@ import { getMockRequestService } from '../../shared/mocks/request.service.mock'; import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RequestService } from '../data/request.service'; -import { BrowseDefinition } from '../shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; import { BrowseEntrySearchOptions } from './browse-entry-search-options.model'; import { BrowseService } from './browse.service'; import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; @@ -23,7 +23,7 @@ describe('BrowseService', () => { const browsesEndpointURL = 'https://rest.api/browses'; const halService: any = new HALEndpointServiceStub(browsesEndpointURL); const browseDefinitions = [ - Object.assign(new BrowseDefinition(), { + Object.assign(new FlatBrowseDefinition(), { id: 'date', metadataBrowse: false, sortOptions: [ @@ -50,7 +50,7 @@ describe('BrowseService', () => { items: { href: 'https://rest.api/discover/browses/dateissued/items' } } }), - Object.assign(new BrowseDefinition(), { + Object.assign(new FlatBrowseDefinition(), { id: 'author', metadataBrowse: true, sortOptions: [ diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index 2fab189254..9e5589e3d8 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -6,7 +6,7 @@ import { RemoteDataBuildService } from '../cache/builders/remote-data-build.serv import { PaginatedList } from '../data/paginated-list.model'; import { RemoteData } from '../data/remote-data'; import { RequestService } from '../data/request.service'; -import { BrowseDefinition } from '../shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; import { BrowseEntry } from '../shared/browse-entry.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { Item } from '../shared/item.model'; @@ -60,7 +60,7 @@ export class BrowseService { /** * Get all BrowseDefinitions */ - getBrowseDefinitions(): Observable>> { + getBrowseDefinitions(): Observable>> { // TODO properly support pagination return this.browseDefinitionDataService.findAll({ elementsPerPage: 9999 }).pipe( getFirstSucceededRemoteData(), @@ -233,13 +233,13 @@ export class BrowseService { return this.getBrowseDefinitions().pipe( getRemoteDataPayload(), getPaginatedListPayload(), - map((browseDefinitions: BrowseDefinition[]) => browseDefinitions - .find((def: BrowseDefinition) => { + map((browseDefinitions: FlatBrowseDefinition[]) => browseDefinitions + .find((def: FlatBrowseDefinition) => { const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); return isNotEmpty(matchingKeys); }) ), - map((def: BrowseDefinition) => { + map((def: FlatBrowseDefinition) => { if (isEmpty(def) || isEmpty(def._links) || isEmpty(def._links[linkPath])) { throw new Error(`A browse endpoint for ${linkPath} on ${metadataKey} isn't configured`); } else { diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index ede23ba43b..81b70e42c2 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -73,7 +73,7 @@ import { ServerResponseService } from './services/server-response.service'; import { NativeWindowFactory, NativeWindowService } from './services/window.service'; import { BitstreamFormat } from './shared/bitstream-format.model'; import { Bitstream } from './shared/bitstream.model'; -import { BrowseDefinition } from './shared/browse-definition.model'; +import { FlatBrowseDefinition } from './shared/flat-browse-definition.model'; import { BrowseEntry } from './shared/browse-entry.model'; import { Bundle } from './shared/bundle.model'; import { Collection } from './shared/collection.model'; @@ -324,7 +324,7 @@ export const models = SubmissionUploadsModel, AuthStatus, BrowseEntry, - BrowseDefinition, + FlatBrowseDefinition, ClaimedTask, TaskObject, PoolTask, diff --git a/src/app/core/shared/browse-definition.model.ts b/src/app/core/shared/flat-browse-definition.model.ts similarity index 81% rename from src/app/core/shared/browse-definition.model.ts rename to src/app/core/shared/flat-browse-definition.model.ts index 863f454422..38df4a562d 100644 --- a/src/app/core/shared/browse-definition.model.ts +++ b/src/app/core/shared/flat-browse-definition.model.ts @@ -1,7 +1,7 @@ import { autoserialize, autoserializeAs, deserialize } from 'cerialize'; import { typedObject } from '../cache/builders/build-decorators'; import { excludeFromEquals } from '../utilities/equals.decorators'; -import { BROWSE_DEFINITION } from './browse-definition.resource-type'; +import { FLAT_BROWSE_DEFINITION } from './flat-browse-definition.resource-type'; import { HALLink } from './hal-link.model'; import { ResourceType } from './resource-type'; import { SortOption } from './sort-option.model'; @@ -9,15 +9,14 @@ import { CacheableObject } from '../cache/cacheable-object.model'; import { BrowseByDataType } from '../../browse-by/browse-by-switcher/browse-by-decorator'; @typedObject -export class BrowseDefinition extends CacheableObject { - static type = BROWSE_DEFINITION; +export class FlatBrowseDefinition extends CacheableObject { + static type = FLAT_BROWSE_DEFINITION; /** * The object type */ @excludeFromEquals - @autoserialize - type: ResourceType; + type: ResourceType = FLAT_BROWSE_DEFINITION; @autoserialize id: string; diff --git a/src/app/core/shared/flat-browse-definition.resource-type.ts b/src/app/core/shared/flat-browse-definition.resource-type.ts new file mode 100644 index 0000000000..bfb01cd98c --- /dev/null +++ b/src/app/core/shared/flat-browse-definition.resource-type.ts @@ -0,0 +1,9 @@ +import { ResourceType } from './resource-type'; + +/** + * The resource type for FlatBrowseDefinition + * + * Needs to be in a separate file to prevent circular + * dependencies in webpack. + */ +export const FLAT_BROWSE_DEFINITION = new ResourceType('flatBrowse'); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 32610c82fd..dd0ade112a 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -6,7 +6,7 @@ import { PaginatedList } from '../data/paginated-list.model'; import { RemoteData } from '../data/remote-data'; import { MetadataField } from '../metadata/metadata-field.model'; import { MetadataSchema } from '../metadata/metadata-schema.model'; -import { BrowseDefinition } from './browse-definition.model'; +import { FlatBrowseDefinition } from './flat-browse-definition.model'; import { DSpaceObject } from './dspace-object.model'; import { InjectionToken } from '@angular/core'; import { MonoTypeOperatorFunction, SchedulerLike } from 'rxjs/internal/types'; @@ -171,17 +171,17 @@ export const toDSpaceObjectListRD = () => /** * Get the browse links from a definition by ID given an array of all definitions * @param {string} definitionID - * @returns {(source: Observable>) => Observable} + * @returns {(source: Observable>) => Observable} */ export const getBrowseDefinitionLinks = (definitionID: string) => - (source: Observable>>): Observable => + (source: Observable>>): Observable => source.pipe( getRemoteDataPayload(), getPaginatedListPayload(), - map((browseDefinitions: BrowseDefinition[]) => browseDefinitions - .find((def: BrowseDefinition) => def.id === definitionID) + map((browseDefinitions: FlatBrowseDefinition[]) => browseDefinitions + .find((def: FlatBrowseDefinition) => def.id === definitionID) ), - map((def: BrowseDefinition) => { + map((def: FlatBrowseDefinition) => { if (isNotEmpty(def)) { return def._links; } else { diff --git a/src/app/menu.resolver.ts b/src/app/menu.resolver.ts index f771ef8b27..d423d3bc33 100644 --- a/src/app/menu.resolver.ts +++ b/src/app/menu.resolver.ts @@ -7,7 +7,7 @@ import { MenuItemType } from './shared/menu/menu-item-type.model'; import { LinkMenuItemModel } from './shared/menu/menu-item/models/link.model'; import { getFirstCompletedRemoteData } from './core/shared/operators'; import { PaginatedList } from './core/data/paginated-list.model'; -import { BrowseDefinition } from './core/shared/browse-definition.model'; +import { FlatBrowseDefinition } from './core/shared/flat-browse-definition.model'; import { RemoteData } from './core/data/remote-data'; import { TextMenuItemModel } from './shared/menu/menu-item/models/text.model'; import { BrowseService } from './core/browse/browse.service'; @@ -108,10 +108,10 @@ export class MenuResolver implements Resolve { ]; // Read the different Browse-By types from config and add them to the browse menu this.browseService.getBrowseDefinitions() - .pipe(getFirstCompletedRemoteData>()) - .subscribe((browseDefListRD: RemoteData>) => { + .pipe(getFirstCompletedRemoteData>()) + .subscribe((browseDefListRD: RemoteData>) => { if (browseDefListRD.hasSucceeded) { - browseDefListRD.payload.page.forEach((browseDef: BrowseDefinition) => { + browseDefListRD.payload.page.forEach((browseDef: FlatBrowseDefinition) => { menuList.push({ id: `browse_global_by_${browseDef.id}`, parentID: 'browse_global', diff --git a/src/app/navbar/navbar.component.spec.ts b/src/app/navbar/navbar.component.spec.ts index ada9be9d0b..084047fc46 100644 --- a/src/app/navbar/navbar.component.spec.ts +++ b/src/app/navbar/navbar.component.spec.ts @@ -16,7 +16,7 @@ import { RouterTestingModule } from '@angular/router/testing'; import { BrowseService } from '../core/browse/browse.service'; import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; import { buildPaginatedList } from '../core/data/paginated-list.model'; -import { BrowseDefinition } from '../core/shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../core/shared/flat-browse-definition.model'; import { BrowseByDataType } from '../browse-by/browse-by-switcher/browse-by-decorator'; import { Item } from '../core/shared/item.model'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; @@ -66,26 +66,26 @@ describe('NavbarComponent', () => { beforeEach(waitForAsync(() => { browseDefinitions = [ Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'title', dataType: BrowseByDataType.Title, } ), Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'dateissued', dataType: BrowseByDataType.Date, metadataKeys: ['dc.date.issued'] } ), Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'author', dataType: BrowseByDataType.Metadata, } ), Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'subject', dataType: BrowseByDataType.Metadata, } diff --git a/src/app/shared/comcol/comcol-page-browse-by/comcol-page-browse-by.component.ts b/src/app/shared/comcol/comcol-page-browse-by/comcol-page-browse-by.component.ts index 0527d283f0..668bd138c8 100644 --- a/src/app/shared/comcol/comcol-page-browse-by/comcol-page-browse-by.component.ts +++ b/src/app/shared/comcol/comcol-page-browse-by/comcol-page-browse-by.component.ts @@ -6,7 +6,7 @@ import { getCommunityPageRoute } from '../../../community-page/community-page-ro import { getCollectionPageRoute } from '../../../collection-page/collection-page-routing-paths'; import { getFirstCompletedRemoteData } from '../../../core/shared/operators'; import { PaginatedList } from '../../../core/data/paginated-list.model'; -import { BrowseDefinition } from '../../../core/shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../../../core/shared/flat-browse-definition.model'; import { RemoteData } from '../../../core/data/remote-data'; import { BrowseService } from '../../../core/browse/browse.service'; @@ -46,11 +46,11 @@ export class ComcolPageBrowseByComponent implements OnInit { ngOnInit(): void { this.browseService.getBrowseDefinitions() - .pipe(getFirstCompletedRemoteData>()) - .subscribe((browseDefListRD: RemoteData>) => { + .pipe(getFirstCompletedRemoteData>()) + .subscribe((browseDefListRD: RemoteData>) => { if (browseDefListRD.hasSucceeded) { this.allOptions = browseDefListRD.payload.page - .map((config: BrowseDefinition) => ({ + .map((config: FlatBrowseDefinition) => ({ id: config.id, label: `browse.comcol.by.${config.id}`, routerLink: `/browse/${config.id}`, From c2f2cb5b3abecdd358876a4462efaea7ae686be4 Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Wed, 10 May 2023 11:01:15 +0200 Subject: [PATCH 02/14] 101623: Add HierarchicalBrowseDefinition model + base BrowseDefinition class --- src/app/core/shared/browse-definition.ts | 10 ++++ .../shared/flat-browse-definition.model.ts | 7 ++- .../hierarchical-browse-definition.model.ts | 47 +++++++++++++++++++ ...rchical-browse-definition.resource-type.ts | 9 ++++ 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 src/app/core/shared/browse-definition.ts create mode 100644 src/app/core/shared/hierarchical-browse-definition.model.ts create mode 100644 src/app/core/shared/hierarchical-browse-definition.resource-type.ts diff --git a/src/app/core/shared/browse-definition.ts b/src/app/core/shared/browse-definition.ts new file mode 100644 index 0000000000..788ef65739 --- /dev/null +++ b/src/app/core/shared/browse-definition.ts @@ -0,0 +1,10 @@ +/** + * Base class for BrowseDefinition models + */ +export abstract class BrowseDefinition { + + /** + * Get the render type of the BrowseDefinition model + */ + abstract getRenderType(): string; +} diff --git a/src/app/core/shared/flat-browse-definition.model.ts b/src/app/core/shared/flat-browse-definition.model.ts index 38df4a562d..2002d6bbab 100644 --- a/src/app/core/shared/flat-browse-definition.model.ts +++ b/src/app/core/shared/flat-browse-definition.model.ts @@ -7,9 +7,10 @@ import { ResourceType } from './resource-type'; import { SortOption } from './sort-option.model'; import { CacheableObject } from '../cache/cacheable-object.model'; import { BrowseByDataType } from '../../browse-by/browse-by-switcher/browse-by-decorator'; +import { BrowseDefinition } from './browse-definition'; @typedObject -export class FlatBrowseDefinition extends CacheableObject { +export class FlatBrowseDefinition extends CacheableObject implements BrowseDefinition { static type = FLAT_BROWSE_DEFINITION; /** @@ -46,4 +47,8 @@ export class FlatBrowseDefinition extends CacheableObject { entries: HALLink; items: HALLink; }; + + getRenderType(): string { + return this.dataType; + } } diff --git a/src/app/core/shared/hierarchical-browse-definition.model.ts b/src/app/core/shared/hierarchical-browse-definition.model.ts new file mode 100644 index 0000000000..a57550e1b3 --- /dev/null +++ b/src/app/core/shared/hierarchical-browse-definition.model.ts @@ -0,0 +1,47 @@ +import { autoserialize, autoserializeAs, deserialize } from 'cerialize'; +import { typedObject } from '../cache/builders/build-decorators'; +import { excludeFromEquals } from '../utilities/equals.decorators'; +import { HIERARCHICAL_BROWSE_DEFINITION } from './hierarchical-browse-definition.resource-type'; +import { HALLink } from './hal-link.model'; +import { ResourceType } from './resource-type'; +import { CacheableObject } from '../cache/cacheable-object.model'; +import { BrowseDefinition } from './browse-definition'; + +@typedObject +export class HierarchicalBrowseDefinition extends CacheableObject implements BrowseDefinition { + static type = HIERARCHICAL_BROWSE_DEFINITION; + + /** + * The object type + */ + @excludeFromEquals + type: ResourceType = HIERARCHICAL_BROWSE_DEFINITION; + + @autoserialize + id: string; + + @autoserialize + facetType: string; + + @autoserialize + vocabulary: string; + + @autoserializeAs('metadata') + metadataKeys: string[]; + + get self(): string { + return this._links.self.href; + } + + @deserialize + _links: { + self: HALLink; + entries: HALLink; + items: HALLink; + vocabulary: HALLink; + }; + + getRenderType(): string { + return 'hierarchy'; + } +} diff --git a/src/app/core/shared/hierarchical-browse-definition.resource-type.ts b/src/app/core/shared/hierarchical-browse-definition.resource-type.ts new file mode 100644 index 0000000000..df06d67c7a --- /dev/null +++ b/src/app/core/shared/hierarchical-browse-definition.resource-type.ts @@ -0,0 +1,9 @@ +import { ResourceType } from './resource-type'; + +/** + * The resource type for HierarchicalBrowseDefinition + * + * Needs to be in a separate file to prevent circular + * dependencies in webpack. + */ +export const HIERARCHICAL_BROWSE_DEFINITION = new ResourceType('hierarchicalBrowse'); From 8c6e3b0c1ff6187afa10d57d3607e2c4353d1229 Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Wed, 10 May 2023 13:28:49 +0200 Subject: [PATCH 03/14] 101623: Add new BrowseResponseParsingService and BrowseDefinitionRestRequest --- .../browse/browse-definition-data.service.ts | 34 ++++++++++++++++++- .../data/browse-response-parsing.service.ts | 33 ++++++++++++++++++ src/app/core/data/request.models.ts | 7 ++++ 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 src/app/core/data/browse-response-parsing.service.ts diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index d9c401fc3f..465c5b44b4 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -6,13 +6,16 @@ import { RemoteDataBuildService } from '../cache/builders/remote-data-build.serv import { ObjectCacheService } from '../cache/object-cache.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; -import { Observable } from 'rxjs'; +import { Observable, of as observableOf } from 'rxjs'; import { RemoteData } from '../data/remote-data'; import { PaginatedList } from '../data/paginated-list.model'; import { FindListOptions } from '../data/find-list-options.model'; import { IdentifiableDataService } from '../data/base/identifiable-data.service'; import { FindAllData, FindAllDataImpl } from '../data/base/find-all-data'; import { dataService } from '../data/base/data-service.decorator'; +import { isNotEmpty, isNotEmptyOperator, hasValue } from '../../shared/empty.util'; +import { take } from 'rxjs/operators'; +import { BrowseDefinitionRestRequest } from '../data/request.models'; /** * Data service responsible for retrieving browse definitions from the REST server @@ -52,5 +55,34 @@ export class BrowseDefinitionDataService extends IdentifiableDataService[]): Observable>> { return this.findAllData.findAll(options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } + + /** + * Create a GET request for the given href, and send it. + * Use a GET request specific for BrowseDefinitions. + * + * @param href$ The url of browse we want to retrieve. Can be a string or + * an Observable + * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's + * no valid cached version. Defaults to true + */ + createAndSendGetRequest(href$: string | Observable, useCachedVersionIfAvailable: boolean = true) { + if (isNotEmpty(href$)) { + if (typeof href$ === 'string') { + href$ = observableOf(href$); + } + + href$.pipe( + isNotEmptyOperator(), + take(1) + ).subscribe((href: string) => { + const requestId = this.requestService.generateRequestId(); + const request = new BrowseDefinitionRestRequest(requestId, href); + if (hasValue(this.responseMsToLive)) { + request.responseMsToLive = this.responseMsToLive; + } + this.requestService.send(request, useCachedVersionIfAvailable); + }); + } + } } diff --git a/src/app/core/data/browse-response-parsing.service.ts b/src/app/core/data/browse-response-parsing.service.ts new file mode 100644 index 0000000000..aecba9cb50 --- /dev/null +++ b/src/app/core/data/browse-response-parsing.service.ts @@ -0,0 +1,33 @@ +import { Injectable } from '@angular/core'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { hasValue } from '../../shared/empty.util'; +import { HIERARCHICAL_BROWSE_DEFINITION } from '../shared/hierarchical-browse-definition.resource-type'; +import { FLAT_BROWSE_DEFINITION } from '../shared/flat-browse-definition.resource-type'; +import { HierarchicalBrowseDefinition } from '../shared/hierarchical-browse-definition.model'; +import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; +import { DSOResponseParsingService } from './dso-response-parsing.service'; + +/** + * A ResponseParsingService used to parse RawRestResponse coming from the REST API to a BrowseDefinition object + */ +@Injectable() +export class BrowseResponseParsingService extends DSOResponseParsingService { + protected objectCache: ObjectCacheService; + protected toCache: boolean; + + protected deserialize(obj): any { + const browseType: string = obj.browseType; + if (hasValue(browseType)) { + if (browseType === HIERARCHICAL_BROWSE_DEFINITION.value) { + const serializer = new this.serializerConstructor(HierarchicalBrowseDefinition); + return serializer.deserialize(obj); + } else if (browseType === FLAT_BROWSE_DEFINITION.value) { + const serializer = new this.serializerConstructor(FlatBrowseDefinition); + return serializer.deserialize(obj); + } + } else { + console.warn('cannot deserialize type ' + browseType); + return null; + } + } +} diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index 6ab3f180d3..79505a3dee 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -11,6 +11,7 @@ import { TaskResponseParsingService } from '../tasks/task-response-parsing.servi import { ContentSourceResponseParsingService } from './content-source-response-parsing.service'; import { RestRequestWithResponseParser } from './rest-request-with-response-parser.model'; import { DspaceRestResponseParsingService } from './dspace-rest-response-parsing.service'; +import { BrowseResponseParsingService } from './browse-response-parsing.service'; import { FindListOptions } from './find-list-options.model'; @@ -118,6 +119,12 @@ export class PatchRequest extends DSpaceRestRequest { } } +export class BrowseDefinitionRestRequest extends DSpaceRestRequest { + getResponseParser(): GenericConstructor { + return BrowseResponseParsingService; + } +} + export class FindListRequest extends GetRequest { constructor( uuid: string, From 195ab41aac32bedbb00e5c5705588785202fc3c9 Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Wed, 10 May 2023 13:39:57 +0200 Subject: [PATCH 04/14] 101623: Themed BrowseByTaxonomyPageComponent --- src/app/browse-by/browse-by-routing.module.ts | 4 +-- ...hemed-browse-by-taxonomy-page.component.ts | 26 +++++++++++++++++++ src/app/browse-by/browse-by.module.ts | 2 ++ .../browse-by-taxonomy-page.component.html | 0 .../browse-by-taxonomy-page.component.scss | 0 .../browse-by-taxonomy-page.component.ts | 15 +++++++++++ src/themes/custom/lazy-theme.module.ts | 2 ++ 7 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 src/app/browse-by/browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component.ts create mode 100644 src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.html create mode 100644 src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.scss create mode 100644 src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.ts diff --git a/src/app/browse-by/browse-by-routing.module.ts b/src/app/browse-by/browse-by-routing.module.ts index 9e078dd4eb..b555390225 100644 --- a/src/app/browse-by/browse-by-routing.module.ts +++ b/src/app/browse-by/browse-by-routing.module.ts @@ -4,7 +4,7 @@ import { BrowseByGuard } from './browse-by-guard'; import { BrowseByDSOBreadcrumbResolver } from './browse-by-dso-breadcrumb.resolver'; import { BrowseByI18nBreadcrumbResolver } from './browse-by-i18n-breadcrumb.resolver'; import { ThemedBrowseBySwitcherComponent } from './browse-by-switcher/themed-browse-by-switcher.component'; -import { BrowseByTaxonomyPageComponent } from './browse-by-taxonomy-page/browse-by-taxonomy-page.component'; +import { ThemedBrowseByTaxonomyPageComponent } from './browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component'; import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver'; @NgModule({ @@ -16,7 +16,7 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso children: [ { path: 'srsc', - component: BrowseByTaxonomyPageComponent, + component: ThemedBrowseByTaxonomyPageComponent, canActivate: [BrowseByGuard], resolve: { breadcrumb: I18nBreadcrumbResolver }, data: { title: 'browse.title.page', breadcrumbKey: 'browse.metadata.srsc' } diff --git a/src/app/browse-by/browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component.ts b/src/app/browse-by/browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component.ts new file mode 100644 index 0000000000..9e491aeaac --- /dev/null +++ b/src/app/browse-by/browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component.ts @@ -0,0 +1,26 @@ +import { Component } from '@angular/core'; +import { ThemedComponent } from '../../shared/theme-support/themed.component'; +import { BrowseByTaxonomyPageComponent } from './browse-by-taxonomy-page.component'; + +@Component({ + selector: 'ds-themed-browse-by-taxonomy-page', + templateUrl: './browse-by-taxonomy-page.component.html', + styleUrls: [] +}) +/** + * Themed wrapper for BrowseByTaxonomyPageComponent + */ +export class ThemedBrowseByTaxonomyPageComponent extends ThemedComponent{ + + protected getComponentName(): string { + return 'BrowseByTaxonomyPageComponent'; + } + + protected importThemedComponent(themeName: string): Promise { + return import(`../../../themes/${themeName}/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component`); + } + + protected importUnthemedComponent(): Promise { + return import(`./browse-by-taxonomy-page.component`); + } +} diff --git a/src/app/browse-by/browse-by.module.ts b/src/app/browse-by/browse-by.module.ts index 8cd52c887d..25a297eb14 100644 --- a/src/app/browse-by/browse-by.module.ts +++ b/src/app/browse-by/browse-by.module.ts @@ -11,6 +11,7 @@ import { ComcolModule } from '../shared/comcol/comcol.module'; import { ThemedBrowseByMetadataPageComponent } from './browse-by-metadata-page/themed-browse-by-metadata-page.component'; import { ThemedBrowseByDatePageComponent } from './browse-by-date-page/themed-browse-by-date-page.component'; import { ThemedBrowseByTitlePageComponent } from './browse-by-title-page/themed-browse-by-title-page.component'; +import { ThemedBrowseByTaxonomyPageComponent } from './browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component'; import { FormModule } from '../shared/form/form.module'; const ENTRY_COMPONENTS = [ @@ -36,6 +37,7 @@ const ENTRY_COMPONENTS = [ BrowseBySwitcherComponent, ThemedBrowseBySwitcherComponent, BrowseByTaxonomyPageComponent, + ThemedBrowseByTaxonomyPageComponent, ...ENTRY_COMPONENTS ], exports: [ diff --git a/src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.html b/src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.html new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.scss b/src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.ts b/src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.ts new file mode 100644 index 0000000000..34d80a0cb8 --- /dev/null +++ b/src/themes/custom/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.ts @@ -0,0 +1,15 @@ +import { Component } from '@angular/core'; +import { BrowseByTaxonomyPageComponent as BaseComponent } from '../../../../../app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component'; + +@Component({ + selector: 'ds-browse-by-taxonomy-page', + // templateUrl: './browse-by-taxonomy-page.component.html', + templateUrl: '../../../../../app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.html', + // styleUrls: ['./browse-by-taxonomy-page.component.scss'], + styleUrls: ['../../../../../app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.scss'], +}) +/** + * Component for browsing items by metadata in a hierarchical controlled vocabulary + */ +export class BrowseByTaxonomyPageComponent extends BaseComponent { +} diff --git a/src/themes/custom/lazy-theme.module.ts b/src/themes/custom/lazy-theme.module.ts index 871764bc66..c39e6a82a1 100644 --- a/src/themes/custom/lazy-theme.module.ts +++ b/src/themes/custom/lazy-theme.module.ts @@ -114,6 +114,7 @@ import { ObjectListComponent } from './app/shared/object-list/object-list.compon import { BrowseByMetadataPageComponent } from './app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component'; import { BrowseByDatePageComponent } from './app/browse-by/browse-by-date-page/browse-by-date-page.component'; import { BrowseByTitlePageComponent } from './app/browse-by/browse-by-title-page/browse-by-title-page.component'; +import { BrowseByTaxonomyPageComponent } from './app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component'; import { ExternalSourceEntryImportModalComponent } from './app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/external-source-tab/external-source-entry-import-modal/external-source-entry-import-modal.component'; @@ -173,6 +174,7 @@ const DECLARATIONS = [ BrowseByMetadataPageComponent, BrowseByDatePageComponent, BrowseByTitlePageComponent, + BrowseByTaxonomyPageComponent, ExternalSourceEntryImportModalComponent, From 67976299de7ab657ccbdb60b7f4efde2e112d63f Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Fri, 12 May 2023 08:12:04 +0200 Subject: [PATCH 05/14] 101623: Fixes for rendersBrowseBy, BrowseResponseParsingService, BrowseByRoutingModule --- src/app/browse-by/browse-by-routing.module.ts | 7 ------- .../browse-by/browse-by-switcher/browse-by-decorator.ts | 2 +- .../browse-by-switcher/browse-by-switcher.component.ts | 2 +- .../themed-browse-by-taxonomy-page.component.ts | 4 +++- src/app/core/data/browse-response-parsing.service.ts | 4 +++- 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/app/browse-by/browse-by-routing.module.ts b/src/app/browse-by/browse-by-routing.module.ts index b555390225..5d3697f391 100644 --- a/src/app/browse-by/browse-by-routing.module.ts +++ b/src/app/browse-by/browse-by-routing.module.ts @@ -14,13 +14,6 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso path: '', resolve: { breadcrumb: BrowseByDSOBreadcrumbResolver }, children: [ - { - path: 'srsc', - component: ThemedBrowseByTaxonomyPageComponent, - canActivate: [BrowseByGuard], - resolve: { breadcrumb: I18nBreadcrumbResolver }, - data: { title: 'browse.title.page', breadcrumbKey: 'browse.metadata.srsc' } - }, { path: ':id', component: ThemedBrowseBySwitcherComponent, diff --git a/src/app/browse-by/browse-by-switcher/browse-by-decorator.ts b/src/app/browse-by/browse-by-switcher/browse-by-decorator.ts index ceb4c6a6c6..b59a46cae1 100644 --- a/src/app/browse-by/browse-by-switcher/browse-by-decorator.ts +++ b/src/app/browse-by/browse-by-switcher/browse-by-decorator.ts @@ -26,7 +26,7 @@ const map = new Map(); * @param browseByType The type of page * @param theme The optional theme for the component */ -export function rendersBrowseBy(browseByType: BrowseByDataType, theme = DEFAULT_THEME) { +export function rendersBrowseBy(browseByType: string, theme = DEFAULT_THEME) { return function decorator(component: any) { if (hasNoValue(map.get(browseByType))) { map.set(browseByType, new Map()); 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 3cfc735135..e4746129dc 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 @@ -31,7 +31,7 @@ export class BrowseBySwitcherComponent implements OnInit { */ ngOnInit(): void { this.browseByComponent = this.route.data.pipe( - map((data: { browseDefinition: FlatBrowseDefinition }) => this.getComponentByBrowseByType(data.browseDefinition.dataType, this.themeService.getThemeName())) + map((data: { browseDefinition: FlatBrowseDefinition }) => this.getComponentByBrowseByType(data.browseDefinition.getRenderType(), this.themeService.getThemeName())) ); } diff --git a/src/app/browse-by/browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component.ts b/src/app/browse-by/browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component.ts index 9e491aeaac..212044b853 100644 --- a/src/app/browse-by/browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component.ts +++ b/src/app/browse-by/browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component.ts @@ -1,15 +1,17 @@ import { Component } from '@angular/core'; import { ThemedComponent } from '../../shared/theme-support/themed.component'; +import { rendersBrowseBy } from '../browse-by-switcher/browse-by-decorator'; import { BrowseByTaxonomyPageComponent } from './browse-by-taxonomy-page.component'; @Component({ selector: 'ds-themed-browse-by-taxonomy-page', - templateUrl: './browse-by-taxonomy-page.component.html', + templateUrl: '../../shared/theme-support/themed.component.html', styleUrls: [] }) /** * Themed wrapper for BrowseByTaxonomyPageComponent */ +@rendersBrowseBy('hierarchy') export class ThemedBrowseByTaxonomyPageComponent extends ThemedComponent{ protected getComponentName(): string { diff --git a/src/app/core/data/browse-response-parsing.service.ts b/src/app/core/data/browse-response-parsing.service.ts index aecba9cb50..9aa508ccf0 100644 --- a/src/app/core/data/browse-response-parsing.service.ts +++ b/src/app/core/data/browse-response-parsing.service.ts @@ -10,7 +10,9 @@ import { DSOResponseParsingService } from './dso-response-parsing.service'; /** * A ResponseParsingService used to parse RawRestResponse coming from the REST API to a BrowseDefinition object */ -@Injectable() +@Injectable({ + providedIn: 'root', + }) export class BrowseResponseParsingService extends DSOResponseParsingService { protected objectCache: ObjectCacheService; protected toCache: boolean; From f3d4754d5bbdf0731231e2d98d8cd1a021723a51 Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Fri, 12 May 2023 10:06:03 +0200 Subject: [PATCH 06/14] 101623: Add BrowseDefinitionModels to CoreModule, Fix FlatBrowseDefinition --- src/app/core/core.module.ts | 2 ++ src/app/core/shared/flat-browse-definition.model.ts | 3 --- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index 81b70e42c2..c00cfd6002 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -170,6 +170,7 @@ import { OrcidHistory } from './orcid/model/orcid-history.model'; import { OrcidAuthService } from './orcid/orcid-auth.service'; import { VocabularyDataService } from './submission/vocabularies/vocabulary.data.service'; import { VocabularyEntryDetailsDataService } from './submission/vocabularies/vocabulary-entry-details.data.service'; +import { HierarchicalBrowseDefinition } from './shared/hierarchical-browse-definition.model'; /** * When not in production, endpoint responses can be mocked for testing purposes @@ -325,6 +326,7 @@ export const models = AuthStatus, BrowseEntry, FlatBrowseDefinition, + HierarchicalBrowseDefinition, ClaimedTask, TaskObject, PoolTask, diff --git a/src/app/core/shared/flat-browse-definition.model.ts b/src/app/core/shared/flat-browse-definition.model.ts index 2002d6bbab..fa664c7747 100644 --- a/src/app/core/shared/flat-browse-definition.model.ts +++ b/src/app/core/shared/flat-browse-definition.model.ts @@ -22,9 +22,6 @@ export class FlatBrowseDefinition extends CacheableObject implements BrowseDefin @autoserialize id: string; - @autoserialize - metadataBrowse: boolean; - @autoserialize sortOptions: SortOption[]; From 3d7e61f57f5ab5693b6a310b593ab335072f32d8 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 17 May 2023 18:31:46 +0200 Subject: [PATCH 07/14] add a custom FindDataImpl for browsedefinitions --- src/app/browse-by/browse-by-guard.spec.ts | 4 +- src/app/browse-by/browse-by-guard.ts | 4 +- .../browse-by-switcher.component.spec.ts | 14 ++-- .../browse-by-switcher.component.ts | 4 +- .../browse/browse-definition-data.service.ts | 81 ++++++++++--------- src/app/core/browse/browse.service.spec.ts | 6 +- src/app/core/browse/browse.service.ts | 16 ++-- src/app/core/core.module.ts | 4 +- .../data/browse-response-parsing.service.ts | 33 +++++--- .../core/shared/browse-definition.model.ts | 16 ++++ src/app/core/shared/browse-definition.ts | 10 --- .../shared/flat-browse-definition.model.ts | 11 +-- .../hierarchical-browse-definition.model.ts | 11 +-- src/app/core/shared/operators.ts | 12 +-- src/app/menu.resolver.ts | 8 +- src/app/navbar/navbar.component.spec.ts | 10 +-- .../comcol-page-browse-by.component.ts | 8 +- 17 files changed, 134 insertions(+), 118 deletions(-) create mode 100644 src/app/core/shared/browse-definition.model.ts delete mode 100644 src/app/core/shared/browse-definition.ts diff --git a/src/app/browse-by/browse-by-guard.spec.ts b/src/app/browse-by/browse-by-guard.spec.ts index 8a9f9b8c50..933c95a3cb 100644 --- a/src/app/browse-by/browse-by-guard.spec.ts +++ b/src/app/browse-by/browse-by-guard.spec.ts @@ -2,7 +2,7 @@ import { first } from 'rxjs/operators'; import { BrowseByGuard } from './browse-by-guard'; import { of as observableOf } from 'rxjs'; import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; -import { FlatBrowseDefinition } from '../core/shared/flat-browse-definition.model'; +import { BrowseDefinition } from '../core/shared/browse-definition.model'; import { BrowseByDataType } from './browse-by-switcher/browse-by-decorator'; describe('BrowseByGuard', () => { @@ -18,7 +18,7 @@ describe('BrowseByGuard', () => { const id = 'author'; const scope = '1234-65487-12354-1235'; const value = 'Filter'; - const browseDefinition = Object.assign(new FlatBrowseDefinition(), { type: BrowseByDataType.Metadata, metadataKeys: ['dc.contributor'] }); + const browseDefinition = Object.assign(new BrowseDefinition(), { type: BrowseByDataType.Metadata, metadataKeys: ['dc.contributor'] }); beforeEach(() => { dsoService = { diff --git a/src/app/browse-by/browse-by-guard.ts b/src/app/browse-by/browse-by-guard.ts index f42359b56b..e4582cb77a 100644 --- a/src/app/browse-by/browse-by-guard.ts +++ b/src/app/browse-by/browse-by-guard.ts @@ -7,7 +7,7 @@ import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from import { TranslateService } from '@ngx-translate/core'; import { Observable, of as observableOf } from 'rxjs'; import { BrowseDefinitionDataService } from '../core/browse/browse-definition-data.service'; -import { FlatBrowseDefinition } from '../core/shared/flat-browse-definition.model'; +import { BrowseDefinition } from '../core/shared/browse-definition.model'; @Injectable() /** @@ -23,7 +23,7 @@ export class BrowseByGuard implements CanActivate { canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) { const title = route.data.title; const id = route.params.id || route.queryParams.id || route.data.id; - let browseDefinition$: Observable; + let browseDefinition$: Observable; if (hasNoValue(route.data.browseDefinition) && hasValue(id)) { browseDefinition$ = this.browseDefinitionService.findById(id).pipe(getFirstSucceededRemoteDataPayload()); } else { 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 91c6c29252..c2e1c9cb68 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 @@ -3,7 +3,7 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { BROWSE_BY_COMPONENT_FACTORY, BrowseByDataType } from './browse-by-decorator'; -import { FlatBrowseDefinition } from '../../core/shared/flat-browse-definition.model'; +import { BrowseDefinition } from '../../core/shared/browse-definition.model'; import { BehaviorSubject } from 'rxjs'; import { ThemeService } from '../../shared/theme-support/theme.service'; @@ -13,33 +13,33 @@ describe('BrowseBySwitcherComponent', () => { const types = [ Object.assign( - new FlatBrowseDefinition(), { + new BrowseDefinition(), { id: 'title', dataType: BrowseByDataType.Title, } ), Object.assign( - new FlatBrowseDefinition(), { + new BrowseDefinition(), { id: 'dateissued', dataType: BrowseByDataType.Date, metadataKeys: ['dc.date.issued'] } ), Object.assign( - new FlatBrowseDefinition(), { + new BrowseDefinition(), { id: 'author', dataType: BrowseByDataType.Metadata, } ), Object.assign( - new FlatBrowseDefinition(), { + new BrowseDefinition(), { id: 'subject', dataType: BrowseByDataType.Metadata, } ), ]; - const data = new BehaviorSubject(createDataWithBrowseDefinition(new FlatBrowseDefinition())); + const data = new BehaviorSubject(createDataWithBrowseDefinition(new BrowseDefinition())); const activatedRouteStub = { data @@ -70,7 +70,7 @@ describe('BrowseBySwitcherComponent', () => { comp = fixture.componentInstance; })); - types.forEach((type: FlatBrowseDefinition) => { + types.forEach((type: BrowseDefinition) => { describe(`when switching to a browse-by page for "${type.id}"`, () => { beforeEach(() => { data.next(createDataWithBrowseDefinition(type)); 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 e4746129dc..35e4edf900 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 @@ -4,7 +4,7 @@ import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { BROWSE_BY_COMPONENT_FACTORY } from './browse-by-decorator'; import { GenericConstructor } from '../../core/shared/generic-constructor'; -import { FlatBrowseDefinition } from '../../core/shared/flat-browse-definition.model'; +import { BrowseDefinition } from '../../core/shared/browse-definition.model'; import { ThemeService } from '../../shared/theme-support/theme.service'; @Component({ @@ -31,7 +31,7 @@ export class BrowseBySwitcherComponent implements OnInit { */ ngOnInit(): void { this.browseByComponent = this.route.data.pipe( - map((data: { browseDefinition: FlatBrowseDefinition }) => this.getComponentByBrowseByType(data.browseDefinition.getRenderType(), this.themeService.getThemeName())) + map((data: { browseDefinition: BrowseDefinition }) => this.getComponentByBrowseByType(data.browseDefinition.getRenderType(), this.themeService.getThemeName())) ); } diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index 465c5b44b4..5380bb7ec4 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -1,6 +1,5 @@ import { Injectable } from '@angular/core'; import { BROWSE_DEFINITION } from '../shared/browse-definition.resource-type'; -import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; import { RequestService } from '../data/request.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { ObjectCacheService } from '../cache/object-cache.service'; @@ -16,46 +15,10 @@ import { dataService } from '../data/base/data-service.decorator'; import { isNotEmpty, isNotEmptyOperator, hasValue } from '../../shared/empty.util'; import { take } from 'rxjs/operators'; import { BrowseDefinitionRestRequest } from '../data/request.models'; +import { BrowseDefinition } from '../shared/browse-definition.model'; -/** - * Data service responsible for retrieving browse definitions from the REST server - */ -@Injectable({ - providedIn: 'root', -}) -@dataService(BROWSE_DEFINITION) -export class BrowseDefinitionDataService extends IdentifiableDataService implements FindAllData { - private findAllData: FindAllDataImpl; - - constructor( - protected requestService: RequestService, - protected rdbService: RemoteDataBuildService, - protected objectCache: ObjectCacheService, - protected halService: HALEndpointService, - ) { - super('browses', requestService, rdbService, objectCache, halService); - - this.findAllData = new FindAllDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, this.responseMsToLive); - } - - /** - * Returns {@link RemoteData} of all object with a list of {@link FollowLinkConfig}, to indicate which embedded - * info should be added to the objects - * - * @param options Find list options object - * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's - * no valid cached version. Defaults to true - * @param reRequestOnStale Whether or not the request should automatically be re- - * requested after the response becomes stale - * @param linksToFollow List of {@link FollowLinkConfig} that indicate which - * {@link HALLink}s should be automatically resolved - * @return {Observable>>} - * Return an observable that emits object list - */ - findAll(options: FindListOptions = {}, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig[]): Observable>> { - return this.findAllData.findAll(options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); - } +class BrowseDefinitionDataImpl extends FindAllDataImpl { /** * Create a GET request for the given href, and send it. * Use a GET request specific for BrowseDefinitions. @@ -86,3 +49,43 @@ export class BrowseDefinitionDataService extends IdentifiableDataService implements FindAllData { + private findAllData: BrowseDefinitionDataImpl; + + constructor( + protected requestService: RequestService, + protected rdbService: RemoteDataBuildService, + protected objectCache: ObjectCacheService, + protected halService: HALEndpointService, + ) { + super('browses', requestService, rdbService, objectCache, halService); + + this.findAllData = new BrowseDefinitionDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, this.responseMsToLive); + } + + /** + * Returns {@link RemoteData} of all object with a list of {@link FollowLinkConfig}, to indicate which embedded + * info should be added to the objects + * + * @param options Find list options object + * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's + * no valid cached version. Defaults to true + * @param reRequestOnStale Whether or not the request should automatically be re- + * requested after the response becomes stale + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which + * {@link HALLink}s should be automatically resolved + * @return {Observable>>} + * Return an observable that emits object list + */ + findAll(options: FindListOptions = {}, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig[]): Observable>> { + return this.findAllData.findAll(options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); + } +} + diff --git a/src/app/core/browse/browse.service.spec.ts b/src/app/core/browse/browse.service.spec.ts index 9e0615385b..46ac8c44f4 100644 --- a/src/app/core/browse/browse.service.spec.ts +++ b/src/app/core/browse/browse.service.spec.ts @@ -6,7 +6,7 @@ import { getMockRequestService } from '../../shared/mocks/request.service.mock'; import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RequestService } from '../data/request.service'; -import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; +import { BrowseDefinition } from '../shared/browse-definition.model'; import { BrowseEntrySearchOptions } from './browse-entry-search-options.model'; import { BrowseService } from './browse.service'; import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; @@ -23,7 +23,7 @@ describe('BrowseService', () => { const browsesEndpointURL = 'https://rest.api/browses'; const halService: any = new HALEndpointServiceStub(browsesEndpointURL); const browseDefinitions = [ - Object.assign(new FlatBrowseDefinition(), { + Object.assign(new BrowseDefinition(), { id: 'date', metadataBrowse: false, sortOptions: [ @@ -50,7 +50,7 @@ describe('BrowseService', () => { items: { href: 'https://rest.api/discover/browses/dateissued/items' } } }), - Object.assign(new FlatBrowseDefinition(), { + Object.assign(new BrowseDefinition(), { id: 'author', metadataBrowse: true, sortOptions: [ diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index 9e5589e3d8..a9a030bf57 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -6,6 +6,7 @@ import { RemoteDataBuildService } from '../cache/builders/remote-data-build.serv import { PaginatedList } from '../data/paginated-list.model'; import { RemoteData } from '../data/remote-data'; import { RequestService } from '../data/request.service'; +import { BrowseDefinition } from '../shared/browse-definition.model'; import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; import { BrowseEntry } from '../shared/browse-entry.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; @@ -60,7 +61,7 @@ export class BrowseService { /** * Get all BrowseDefinitions */ - getBrowseDefinitions(): Observable>> { + getBrowseDefinitions(): Observable>> { // TODO properly support pagination return this.browseDefinitionDataService.findAll({ elementsPerPage: 9999 }).pipe( getFirstSucceededRemoteData(), @@ -233,13 +234,18 @@ export class BrowseService { return this.getBrowseDefinitions().pipe( getRemoteDataPayload(), getPaginatedListPayload(), - map((browseDefinitions: FlatBrowseDefinition[]) => browseDefinitions - .find((def: FlatBrowseDefinition) => { - const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); + map((browseDefinitions: BrowseDefinition[]) => browseDefinitions + .find((def: BrowseDefinition) => { + let matchingKeys = ''; + + if (Array.isArray((def as FlatBrowseDefinition).metadataKeys)) { + matchingKeys = (def as FlatBrowseDefinition).metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); + } + return isNotEmpty(matchingKeys); }) ), - map((def: FlatBrowseDefinition) => { + map((def: BrowseDefinition) => { if (isEmpty(def) || isEmpty(def._links) || isEmpty(def._links[linkPath])) { throw new Error(`A browse endpoint for ${linkPath} on ${metadataKey} isn't configured`); } else { diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index c00cfd6002..5365de0c4b 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -73,7 +73,7 @@ import { ServerResponseService } from './services/server-response.service'; import { NativeWindowFactory, NativeWindowService } from './services/window.service'; import { BitstreamFormat } from './shared/bitstream-format.model'; import { Bitstream } from './shared/bitstream.model'; -import { FlatBrowseDefinition } from './shared/flat-browse-definition.model'; +import { BrowseDefinition } from './shared/browse-definition.model'; import { BrowseEntry } from './shared/browse-entry.model'; import { Bundle } from './shared/bundle.model'; import { Collection } from './shared/collection.model'; @@ -325,7 +325,7 @@ export const models = SubmissionUploadsModel, AuthStatus, BrowseEntry, - FlatBrowseDefinition, + BrowseDefinition, HierarchicalBrowseDefinition, ClaimedTask, TaskObject, diff --git a/src/app/core/data/browse-response-parsing.service.ts b/src/app/core/data/browse-response-parsing.service.ts index 9aa508ccf0..cc1a4a64a1 100644 --- a/src/app/core/data/browse-response-parsing.service.ts +++ b/src/app/core/data/browse-response-parsing.service.ts @@ -1,35 +1,42 @@ import { Injectable } from '@angular/core'; import { ObjectCacheService } from '../cache/object-cache.service'; import { hasValue } from '../../shared/empty.util'; -import { HIERARCHICAL_BROWSE_DEFINITION } from '../shared/hierarchical-browse-definition.resource-type'; +import { + HIERARCHICAL_BROWSE_DEFINITION +} from '../shared/hierarchical-browse-definition.resource-type'; import { FLAT_BROWSE_DEFINITION } from '../shared/flat-browse-definition.resource-type'; import { HierarchicalBrowseDefinition } from '../shared/hierarchical-browse-definition.model'; import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; import { DSOResponseParsingService } from './dso-response-parsing.service'; +import { Serializer } from '../serializer'; +import { BrowseDefinition } from '../shared/browse-definition.model'; +import { BROWSE_DEFINITION } from '../shared/browse-definition.resource-type'; /** * A ResponseParsingService used to parse RawRestResponse coming from the REST API to a BrowseDefinition object */ @Injectable({ - providedIn: 'root', - }) + providedIn: 'root', +}) export class BrowseResponseParsingService extends DSOResponseParsingService { - protected objectCache: ObjectCacheService; - protected toCache: boolean; + constructor( + protected objectCache: ObjectCacheService, + ) { + super(objectCache); + } protected deserialize(obj): any { const browseType: string = obj.browseType; - if (hasValue(browseType)) { + if (obj.type === BROWSE_DEFINITION.value && hasValue(browseType)) { + let serializer: Serializer; if (browseType === HIERARCHICAL_BROWSE_DEFINITION.value) { - const serializer = new this.serializerConstructor(HierarchicalBrowseDefinition); - return serializer.deserialize(obj); - } else if (browseType === FLAT_BROWSE_DEFINITION.value) { - const serializer = new this.serializerConstructor(FlatBrowseDefinition); - return serializer.deserialize(obj); + serializer = new this.serializerConstructor(HierarchicalBrowseDefinition); + } else { + serializer = new this.serializerConstructor(FlatBrowseDefinition); } + return serializer.deserialize(obj); } else { - console.warn('cannot deserialize type ' + browseType); - return null; + return super.deserialize(obj); } } } diff --git a/src/app/core/shared/browse-definition.model.ts b/src/app/core/shared/browse-definition.model.ts new file mode 100644 index 0000000000..a5bed53c9f --- /dev/null +++ b/src/app/core/shared/browse-definition.model.ts @@ -0,0 +1,16 @@ +import { autoserialize } from 'cerialize'; +import { CacheableObject } from '../cache/cacheable-object.model'; + +/** + * Base class for BrowseDefinition models + */ +export abstract class BrowseDefinition extends CacheableObject { + + @autoserialize + id: string; + + /** + * Get the render type of the BrowseDefinition model + */ + abstract getRenderType(): string; +} diff --git a/src/app/core/shared/browse-definition.ts b/src/app/core/shared/browse-definition.ts deleted file mode 100644 index 788ef65739..0000000000 --- a/src/app/core/shared/browse-definition.ts +++ /dev/null @@ -1,10 +0,0 @@ -/** - * Base class for BrowseDefinition models - */ -export abstract class BrowseDefinition { - - /** - * Get the render type of the BrowseDefinition model - */ - abstract getRenderType(): string; -} diff --git a/src/app/core/shared/flat-browse-definition.model.ts b/src/app/core/shared/flat-browse-definition.model.ts index fa664c7747..9660a3bac9 100644 --- a/src/app/core/shared/flat-browse-definition.model.ts +++ b/src/app/core/shared/flat-browse-definition.model.ts @@ -1,16 +1,16 @@ -import { autoserialize, autoserializeAs, deserialize } from 'cerialize'; +import { autoserialize, autoserializeAs, deserialize, inheritSerialization } from 'cerialize'; import { typedObject } from '../cache/builders/build-decorators'; import { excludeFromEquals } from '../utilities/equals.decorators'; import { FLAT_BROWSE_DEFINITION } from './flat-browse-definition.resource-type'; import { HALLink } from './hal-link.model'; import { ResourceType } from './resource-type'; import { SortOption } from './sort-option.model'; -import { CacheableObject } from '../cache/cacheable-object.model'; import { BrowseByDataType } from '../../browse-by/browse-by-switcher/browse-by-decorator'; -import { BrowseDefinition } from './browse-definition'; +import { BrowseDefinition } from './browse-definition.model'; @typedObject -export class FlatBrowseDefinition extends CacheableObject implements BrowseDefinition { +@inheritSerialization(BrowseDefinition) +export class FlatBrowseDefinition extends BrowseDefinition { static type = FLAT_BROWSE_DEFINITION; /** @@ -19,9 +19,6 @@ export class FlatBrowseDefinition extends CacheableObject implements BrowseDefin @excludeFromEquals type: ResourceType = FLAT_BROWSE_DEFINITION; - @autoserialize - id: string; - @autoserialize sortOptions: SortOption[]; diff --git a/src/app/core/shared/hierarchical-browse-definition.model.ts b/src/app/core/shared/hierarchical-browse-definition.model.ts index a57550e1b3..427f5e1947 100644 --- a/src/app/core/shared/hierarchical-browse-definition.model.ts +++ b/src/app/core/shared/hierarchical-browse-definition.model.ts @@ -1,14 +1,14 @@ -import { autoserialize, autoserializeAs, deserialize } from 'cerialize'; +import { autoserialize, autoserializeAs, deserialize, inheritSerialization } from 'cerialize'; import { typedObject } from '../cache/builders/build-decorators'; import { excludeFromEquals } from '../utilities/equals.decorators'; import { HIERARCHICAL_BROWSE_DEFINITION } from './hierarchical-browse-definition.resource-type'; import { HALLink } from './hal-link.model'; import { ResourceType } from './resource-type'; -import { CacheableObject } from '../cache/cacheable-object.model'; -import { BrowseDefinition } from './browse-definition'; +import { BrowseDefinition } from './browse-definition.model'; @typedObject -export class HierarchicalBrowseDefinition extends CacheableObject implements BrowseDefinition { +@inheritSerialization(BrowseDefinition) +export class HierarchicalBrowseDefinition extends BrowseDefinition { static type = HIERARCHICAL_BROWSE_DEFINITION; /** @@ -17,9 +17,6 @@ export class HierarchicalBrowseDefinition extends CacheableObject implements Bro @excludeFromEquals type: ResourceType = HIERARCHICAL_BROWSE_DEFINITION; - @autoserialize - id: string; - @autoserialize facetType: string; diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index dd0ade112a..32610c82fd 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -6,7 +6,7 @@ import { PaginatedList } from '../data/paginated-list.model'; import { RemoteData } from '../data/remote-data'; import { MetadataField } from '../metadata/metadata-field.model'; import { MetadataSchema } from '../metadata/metadata-schema.model'; -import { FlatBrowseDefinition } from './flat-browse-definition.model'; +import { BrowseDefinition } from './browse-definition.model'; import { DSpaceObject } from './dspace-object.model'; import { InjectionToken } from '@angular/core'; import { MonoTypeOperatorFunction, SchedulerLike } from 'rxjs/internal/types'; @@ -171,17 +171,17 @@ export const toDSpaceObjectListRD = () => /** * Get the browse links from a definition by ID given an array of all definitions * @param {string} definitionID - * @returns {(source: Observable>) => Observable} + * @returns {(source: Observable>) => Observable} */ export const getBrowseDefinitionLinks = (definitionID: string) => - (source: Observable>>): Observable => + (source: Observable>>): Observable => source.pipe( getRemoteDataPayload(), getPaginatedListPayload(), - map((browseDefinitions: FlatBrowseDefinition[]) => browseDefinitions - .find((def: FlatBrowseDefinition) => def.id === definitionID) + map((browseDefinitions: BrowseDefinition[]) => browseDefinitions + .find((def: BrowseDefinition) => def.id === definitionID) ), - map((def: FlatBrowseDefinition) => { + map((def: BrowseDefinition) => { if (isNotEmpty(def)) { return def._links; } else { diff --git a/src/app/menu.resolver.ts b/src/app/menu.resolver.ts index d423d3bc33..f771ef8b27 100644 --- a/src/app/menu.resolver.ts +++ b/src/app/menu.resolver.ts @@ -7,7 +7,7 @@ import { MenuItemType } from './shared/menu/menu-item-type.model'; import { LinkMenuItemModel } from './shared/menu/menu-item/models/link.model'; import { getFirstCompletedRemoteData } from './core/shared/operators'; import { PaginatedList } from './core/data/paginated-list.model'; -import { FlatBrowseDefinition } from './core/shared/flat-browse-definition.model'; +import { BrowseDefinition } from './core/shared/browse-definition.model'; import { RemoteData } from './core/data/remote-data'; import { TextMenuItemModel } from './shared/menu/menu-item/models/text.model'; import { BrowseService } from './core/browse/browse.service'; @@ -108,10 +108,10 @@ export class MenuResolver implements Resolve { ]; // Read the different Browse-By types from config and add them to the browse menu this.browseService.getBrowseDefinitions() - .pipe(getFirstCompletedRemoteData>()) - .subscribe((browseDefListRD: RemoteData>) => { + .pipe(getFirstCompletedRemoteData>()) + .subscribe((browseDefListRD: RemoteData>) => { if (browseDefListRD.hasSucceeded) { - browseDefListRD.payload.page.forEach((browseDef: FlatBrowseDefinition) => { + browseDefListRD.payload.page.forEach((browseDef: BrowseDefinition) => { menuList.push({ id: `browse_global_by_${browseDef.id}`, parentID: 'browse_global', diff --git a/src/app/navbar/navbar.component.spec.ts b/src/app/navbar/navbar.component.spec.ts index 084047fc46..ada9be9d0b 100644 --- a/src/app/navbar/navbar.component.spec.ts +++ b/src/app/navbar/navbar.component.spec.ts @@ -16,7 +16,7 @@ import { RouterTestingModule } from '@angular/router/testing'; import { BrowseService } from '../core/browse/browse.service'; import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; import { buildPaginatedList } from '../core/data/paginated-list.model'; -import { FlatBrowseDefinition } from '../core/shared/flat-browse-definition.model'; +import { BrowseDefinition } from '../core/shared/browse-definition.model'; import { BrowseByDataType } from '../browse-by/browse-by-switcher/browse-by-decorator'; import { Item } from '../core/shared/item.model'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; @@ -66,26 +66,26 @@ describe('NavbarComponent', () => { beforeEach(waitForAsync(() => { browseDefinitions = [ Object.assign( - new FlatBrowseDefinition(), { + new BrowseDefinition(), { id: 'title', dataType: BrowseByDataType.Title, } ), Object.assign( - new FlatBrowseDefinition(), { + new BrowseDefinition(), { id: 'dateissued', dataType: BrowseByDataType.Date, metadataKeys: ['dc.date.issued'] } ), Object.assign( - new FlatBrowseDefinition(), { + new BrowseDefinition(), { id: 'author', dataType: BrowseByDataType.Metadata, } ), Object.assign( - new FlatBrowseDefinition(), { + new BrowseDefinition(), { id: 'subject', dataType: BrowseByDataType.Metadata, } diff --git a/src/app/shared/comcol/comcol-page-browse-by/comcol-page-browse-by.component.ts b/src/app/shared/comcol/comcol-page-browse-by/comcol-page-browse-by.component.ts index 668bd138c8..0527d283f0 100644 --- a/src/app/shared/comcol/comcol-page-browse-by/comcol-page-browse-by.component.ts +++ b/src/app/shared/comcol/comcol-page-browse-by/comcol-page-browse-by.component.ts @@ -6,7 +6,7 @@ import { getCommunityPageRoute } from '../../../community-page/community-page-ro import { getCollectionPageRoute } from '../../../collection-page/collection-page-routing-paths'; import { getFirstCompletedRemoteData } from '../../../core/shared/operators'; import { PaginatedList } from '../../../core/data/paginated-list.model'; -import { FlatBrowseDefinition } from '../../../core/shared/flat-browse-definition.model'; +import { BrowseDefinition } from '../../../core/shared/browse-definition.model'; import { RemoteData } from '../../../core/data/remote-data'; import { BrowseService } from '../../../core/browse/browse.service'; @@ -46,11 +46,11 @@ export class ComcolPageBrowseByComponent implements OnInit { ngOnInit(): void { this.browseService.getBrowseDefinitions() - .pipe(getFirstCompletedRemoteData>()) - .subscribe((browseDefListRD: RemoteData>) => { + .pipe(getFirstCompletedRemoteData>()) + .subscribe((browseDefListRD: RemoteData>) => { if (browseDefListRD.hasSucceeded) { this.allOptions = browseDefListRD.payload.page - .map((config: FlatBrowseDefinition) => ({ + .map((config: BrowseDefinition) => ({ id: config.id, label: `browse.comcol.by.${config.id}`, routerLink: `/browse/${config.id}`, From bf31c76c885669d1215491dc2acbd5e3a052f663 Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Fri, 19 May 2023 16:36:13 +0200 Subject: [PATCH 08/14] 101623: Add ValueListBrowseDefinition model + super NonHierarchicalBrowse class --- .../browse/browse-definition-data.service.ts | 6 ++-- src/app/core/core.module.ts | 6 ++++ .../data/browse-response-parsing.service.ts | 10 ++++-- .../shared/flat-browse-definition.model.ts | 30 +++-------------- .../non-hierarchical-browse-definition.ts | 32 +++++++++++++++++++ .../value-list-browse-definition.model.ts | 26 +++++++++++++++ ...ue-list-browse-definition.resource-type.ts | 9 ++++++ 7 files changed, 88 insertions(+), 31 deletions(-) create mode 100644 src/app/core/shared/non-hierarchical-browse-definition.ts create mode 100644 src/app/core/shared/value-list-browse-definition.model.ts create mode 100644 src/app/core/shared/value-list-browse-definition.resource-type.ts diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index 5380bb7ec4..a4c7bf1353 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -18,7 +18,7 @@ import { BrowseDefinitionRestRequest } from '../data/request.models'; import { BrowseDefinition } from '../shared/browse-definition.model'; -class BrowseDefinitionDataImpl extends FindAllDataImpl { +class BrowseDefinitionFindAllDataImpl extends FindAllDataImpl { /** * Create a GET request for the given href, and send it. * Use a GET request specific for BrowseDefinitions. @@ -57,7 +57,7 @@ class BrowseDefinitionDataImpl extends FindAllDataImpl { }) @dataService(BROWSE_DEFINITION) export class BrowseDefinitionDataService extends IdentifiableDataService implements FindAllData { - private findAllData: BrowseDefinitionDataImpl; + private findAllData: BrowseDefinitionFindAllDataImpl; constructor( protected requestService: RequestService, @@ -67,7 +67,7 @@ export class BrowseDefinitionDataService extends IdentifiableDataService; if (browseType === HIERARCHICAL_BROWSE_DEFINITION.value) { serializer = new this.serializerConstructor(HierarchicalBrowseDefinition); - } else { + } else if (browseType === FLAT_BROWSE_DEFINITION.value) { serializer = new this.serializerConstructor(FlatBrowseDefinition); + } else if (browseType === VALUE_LIST_BROWSE_DEFINITION.value) { + serializer = new this.serializerConstructor(ValueListBrowseDefinition); + } else { + throw new Error('An error occurred while retrieving the browse definitions.'); } return serializer.deserialize(obj); } else { - return super.deserialize(obj); + throw new Error('An error occurred while retrieving the browse definitions.'); } } } diff --git a/src/app/core/shared/flat-browse-definition.model.ts b/src/app/core/shared/flat-browse-definition.model.ts index 9660a3bac9..f94fa75b91 100644 --- a/src/app/core/shared/flat-browse-definition.model.ts +++ b/src/app/core/shared/flat-browse-definition.model.ts @@ -1,16 +1,13 @@ -import { autoserialize, autoserializeAs, deserialize, inheritSerialization } from 'cerialize'; +import { inheritSerialization } from 'cerialize'; import { typedObject } from '../cache/builders/build-decorators'; import { excludeFromEquals } from '../utilities/equals.decorators'; import { FLAT_BROWSE_DEFINITION } from './flat-browse-definition.resource-type'; -import { HALLink } from './hal-link.model'; import { ResourceType } from './resource-type'; -import { SortOption } from './sort-option.model'; -import { BrowseByDataType } from '../../browse-by/browse-by-switcher/browse-by-decorator'; -import { BrowseDefinition } from './browse-definition.model'; +import { NonHierarchicalBrowseDefinition } from './non-hierarchical-browse-definition'; @typedObject -@inheritSerialization(BrowseDefinition) -export class FlatBrowseDefinition extends BrowseDefinition { +@inheritSerialization(NonHierarchicalBrowseDefinition) +export class FlatBrowseDefinition extends NonHierarchicalBrowseDefinition { static type = FLAT_BROWSE_DEFINITION; /** @@ -19,29 +16,10 @@ export class FlatBrowseDefinition extends BrowseDefinition { @excludeFromEquals type: ResourceType = FLAT_BROWSE_DEFINITION; - @autoserialize - sortOptions: SortOption[]; - - @autoserializeAs('order') - defaultSortOrder: string; - - @autoserializeAs('metadata') - metadataKeys: string[]; - - @autoserialize - dataType: BrowseByDataType; - get self(): string { return this._links.self.href; } - @deserialize - _links: { - self: HALLink; - entries: HALLink; - items: HALLink; - }; - getRenderType(): string { return this.dataType; } diff --git a/src/app/core/shared/non-hierarchical-browse-definition.ts b/src/app/core/shared/non-hierarchical-browse-definition.ts new file mode 100644 index 0000000000..a4f6df43d9 --- /dev/null +++ b/src/app/core/shared/non-hierarchical-browse-definition.ts @@ -0,0 +1,32 @@ +import { autoserialize, autoserializeAs, deserialize, inheritSerialization } from 'cerialize'; +import { SortOption } from './sort-option.model'; +import { BrowseByDataType } from '../../browse-by/browse-by-switcher/browse-by-decorator'; +import { HALLink } from './hal-link.model'; +import { BrowseDefinition } from './browse-definition.model'; + +/** + * Super class for NonHierarchicalBrowseDefinition models, + * e.g. FlatBrowseDefinition and ValueListBrowseDefinition + */ +@inheritSerialization(BrowseDefinition) +export abstract class NonHierarchicalBrowseDefinition extends BrowseDefinition { + + @autoserialize + sortOptions: SortOption[]; + + @autoserializeAs('order') + defaultSortOrder: string; + + @autoserializeAs('metadata') + metadataKeys: string[]; + + @autoserialize + dataType: BrowseByDataType; + + @deserialize + _links: { + self: HALLink; + entries: HALLink; + items: HALLink; + }; +} diff --git a/src/app/core/shared/value-list-browse-definition.model.ts b/src/app/core/shared/value-list-browse-definition.model.ts new file mode 100644 index 0000000000..a42a702940 --- /dev/null +++ b/src/app/core/shared/value-list-browse-definition.model.ts @@ -0,0 +1,26 @@ +import { inheritSerialization } from 'cerialize'; +import { typedObject } from '../cache/builders/build-decorators'; +import { excludeFromEquals } from '../utilities/equals.decorators'; +import { VALUE_LIST_BROWSE_DEFINITION } from './value-list-browse-definition.resource-type'; +import { ResourceType } from './resource-type'; +import { NonHierarchicalBrowseDefinition } from './non-hierarchical-browse-definition'; + +@typedObject +@inheritSerialization(NonHierarchicalBrowseDefinition) +export class ValueListBrowseDefinition extends NonHierarchicalBrowseDefinition { + static type = VALUE_LIST_BROWSE_DEFINITION; + + /** + * The object type + */ + @excludeFromEquals + type: ResourceType = VALUE_LIST_BROWSE_DEFINITION; + + get self(): string { + return this._links.self.href; + } + + getRenderType(): string { + return this.dataType; + } +} diff --git a/src/app/core/shared/value-list-browse-definition.resource-type.ts b/src/app/core/shared/value-list-browse-definition.resource-type.ts new file mode 100644 index 0000000000..8904dc472f --- /dev/null +++ b/src/app/core/shared/value-list-browse-definition.resource-type.ts @@ -0,0 +1,9 @@ +import { ResourceType } from './resource-type'; + +/** + * The resource type for ValueListBrowseDefinition + * + * Needs to be in a separate file to prevent circular + * dependencies in webpack. + */ +export const VALUE_LIST_BROWSE_DEFINITION = new ResourceType('valueList'); From f01c58e84dd4a7a907cc90ce45a28ee50e3eef41 Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Tue, 23 May 2023 14:13:05 +0200 Subject: [PATCH 09/14] 101623: Fix BrowseResponseParsingService + Add tests for it --- .../browse-response-parsing.service.spec.ts | 64 +++++++++++++++++++ .../data/browse-response-parsing.service.ts | 4 +- .../core/data/dso-response-parsing.service.ts | 4 ++ 3 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 src/app/core/data/browse-response-parsing.service.spec.ts diff --git a/src/app/core/data/browse-response-parsing.service.spec.ts b/src/app/core/data/browse-response-parsing.service.spec.ts new file mode 100644 index 0000000000..9fa7239ef7 --- /dev/null +++ b/src/app/core/data/browse-response-parsing.service.spec.ts @@ -0,0 +1,64 @@ +import { getMockObjectCacheService } from '../../shared/mocks/object-cache.service.mock'; +import { BrowseResponseParsingService } from './browse-response-parsing.service'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { HIERARCHICAL_BROWSE_DEFINITION } from '../shared/hierarchical-browse-definition.resource-type'; +import { FLAT_BROWSE_DEFINITION } from '../shared/flat-browse-definition.resource-type'; +import { VALUE_LIST_BROWSE_DEFINITION } from '../shared/value-list-browse-definition.resource-type'; + +class TestService extends BrowseResponseParsingService { + constructor(protected objectCache: ObjectCacheService) { + super(objectCache); + } + + // Overwrite method to make it public for testing + public deserialize(obj): any { + return super.deserialize(obj); + } +} + +describe('BrowseResponseParsingService', () => { + let service: TestService; + + + beforeEach(() => { + service = new TestService(getMockObjectCacheService()); + }); + + describe('', () => { + const mockFlatBrowse = { + id: 'title', + browseType: 'flatBrowse', + type: 'browse', + }; + + const mockValueList = { + id: 'author', + browseType: 'valueList', + type: 'browse', + }; + + const mockHierarchicalBrowse = { + id: 'srsc', + browseType: 'hierarchicalBrowse', + type: 'browse', + }; + + it('should deserialize flatBrowses correctly', () => { + let deserialized = service.deserialize(mockFlatBrowse); + expect(deserialized.type).toBe(FLAT_BROWSE_DEFINITION); + expect(deserialized.id).toBe(mockFlatBrowse.id); + }); + + it('should deserialize valueList browses correctly', () => { + let deserialized = service.deserialize(mockValueList); + expect(deserialized.type).toBe(VALUE_LIST_BROWSE_DEFINITION); + expect(deserialized.id).toBe(mockValueList.id); + }); + + it('should deserialize hierarchicalBrowses correctly', () => { + let deserialized = service.deserialize(mockHierarchicalBrowse); + expect(deserialized.type).toBe(HIERARCHICAL_BROWSE_DEFINITION); + expect(deserialized.id).toBe(mockHierarchicalBrowse.id); + }); + }); +}); diff --git a/src/app/core/data/browse-response-parsing.service.ts b/src/app/core/data/browse-response-parsing.service.ts index 57b76c72a4..3681198406 100644 --- a/src/app/core/data/browse-response-parsing.service.ts +++ b/src/app/core/data/browse-response-parsing.service.ts @@ -7,7 +7,7 @@ import { import { FLAT_BROWSE_DEFINITION } from '../shared/flat-browse-definition.resource-type'; import { HierarchicalBrowseDefinition } from '../shared/hierarchical-browse-definition.model'; import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; -import { DSOResponseParsingService } from './dso-response-parsing.service'; +import { DspaceRestResponseParsingService } from './dspace-rest-response-parsing.service'; import { Serializer } from '../serializer'; import { BrowseDefinition } from '../shared/browse-definition.model'; import { BROWSE_DEFINITION } from '../shared/browse-definition.resource-type'; @@ -20,7 +20,7 @@ import { VALUE_LIST_BROWSE_DEFINITION } from '../shared/value-list-browse-defini @Injectable({ providedIn: 'root', }) -export class BrowseResponseParsingService extends DSOResponseParsingService { +export class BrowseResponseParsingService extends DspaceRestResponseParsingService { constructor( protected objectCache: ObjectCacheService, ) { diff --git a/src/app/core/data/dso-response-parsing.service.ts b/src/app/core/data/dso-response-parsing.service.ts index fd5a22fae9..74117e79d3 100644 --- a/src/app/core/data/dso-response-parsing.service.ts +++ b/src/app/core/data/dso-response-parsing.service.ts @@ -10,6 +10,10 @@ import { hasNoValue, hasValue } from '../../shared/empty.util'; import { DSpaceObject } from '../shared/dspace-object.model'; import { RestRequest } from './rest-request.model'; +/** + * @deprecated use DspaceRestResponseParsingService for new code, this is only left to support a + * few legacy use cases, and should get removed eventually + */ @Injectable() export class DSOResponseParsingService extends BaseResponseParsingService implements ResponseParsingService { protected toCache = true; From d1c91b8bc2bc21b7478a6c46ea700a9a86fdf7eb Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Tue, 23 May 2023 17:02:10 +0200 Subject: [PATCH 10/14] 101623: Override createAndSendGetRequest in BrowseDefinitionDataService --- src/app/browse-by/browse-by.module.ts | 5 +- .../browse/browse-definition-data.service.ts | 65 +++++++++++-------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/app/browse-by/browse-by.module.ts b/src/app/browse-by/browse-by.module.ts index 25a297eb14..826868f288 100644 --- a/src/app/browse-by/browse-by.module.ts +++ b/src/app/browse-by/browse-by.module.ts @@ -19,11 +19,12 @@ const ENTRY_COMPONENTS = [ BrowseByTitlePageComponent, BrowseByMetadataPageComponent, BrowseByDatePageComponent, + BrowseByTaxonomyPageComponent, ThemedBrowseByMetadataPageComponent, ThemedBrowseByDatePageComponent, ThemedBrowseByTitlePageComponent, - + ThemedBrowseByTaxonomyPageComponent, ]; @NgModule({ @@ -36,8 +37,6 @@ const ENTRY_COMPONENTS = [ declarations: [ BrowseBySwitcherComponent, ThemedBrowseBySwitcherComponent, - BrowseByTaxonomyPageComponent, - ThemedBrowseByTaxonomyPageComponent, ...ENTRY_COMPONENTS ], exports: [ diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index a4c7bf1353..f5e67cd297 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -1,3 +1,4 @@ +// eslint-disable-next-line max-classes-per-file import { Injectable } from '@angular/core'; import { BROWSE_DEFINITION } from '../shared/browse-definition.resource-type'; import { RequestService } from '../data/request.service'; @@ -17,35 +18,39 @@ import { take } from 'rxjs/operators'; import { BrowseDefinitionRestRequest } from '../data/request.models'; import { BrowseDefinition } from '../shared/browse-definition.model'; - -class BrowseDefinitionFindAllDataImpl extends FindAllDataImpl { - /** - * Create a GET request for the given href, and send it. - * Use a GET request specific for BrowseDefinitions. - * - * @param href$ The url of browse we want to retrieve. Can be a string or - * an Observable - * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's - * no valid cached version. Defaults to true - */ - createAndSendGetRequest(href$: string | Observable, useCachedVersionIfAvailable: boolean = true) { - if (isNotEmpty(href$)) { - if (typeof href$ === 'string') { - href$ = observableOf(href$); - } - - href$.pipe( - isNotEmptyOperator(), - take(1) - ).subscribe((href: string) => { - const requestId = this.requestService.generateRequestId(); - const request = new BrowseDefinitionRestRequest(requestId, href); - if (hasValue(this.responseMsToLive)) { - request.responseMsToLive = this.responseMsToLive; - } - this.requestService.send(request, useCachedVersionIfAvailable); - }); +/** + * Create a GET request for the given href, and send it. + * Use a GET request specific for BrowseDefinitions. + */ +export const createAndSendBrowseDefinitionGetRequest = (requestService: RequestService, + responseMsToLive: number, + href$: string | Observable, + useCachedVersionIfAvailable: boolean = true): void => { + if (isNotEmpty(href$)) { + if (typeof href$ === 'string') { + href$ = observableOf(href$); } + + href$.pipe( + isNotEmptyOperator(), + take(1) + ).subscribe((href: string) => { + const requestId = requestService.generateRequestId(); + const request = new BrowseDefinitionRestRequest(requestId, href); + if (hasValue(responseMsToLive)) { + request.responseMsToLive = responseMsToLive; + } + requestService.send(request, useCachedVersionIfAvailable); + }); + } +}; + +/** + * Custom extension of {@link FindAllDataImpl} to be able to send BrowseDefinitionRestRequests + */ +class BrowseDefinitionFindAllDataImpl extends FindAllDataImpl { + createAndSendGetRequest(href$: string | Observable, useCachedVersionIfAvailable: boolean = true) { + createAndSendBrowseDefinitionGetRequest(this.requestService, this.responseMsToLive, href$, useCachedVersionIfAvailable); } } @@ -87,5 +92,9 @@ export class BrowseDefinitionDataService extends IdentifiableDataService[]): Observable>> { return this.findAllData.findAll(options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } + + createAndSendGetRequest(href$: string | Observable, useCachedVersionIfAvailable: boolean = true) { + createAndSendBrowseDefinitionGetRequest(this.requestService, this.responseMsToLive, href$, useCachedVersionIfAvailable); + } } From aee76913aac0aaf812c019ca641f7e311b0bafbf Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Tue, 23 May 2023 17:05:00 +0200 Subject: [PATCH 11/14] 101623: Replace hardcoded vocabulary with BrowseDefinition in BrowseByTaxonomyPage --- .../browse-by-taxonomy-page.component.html | 2 +- .../browse-by-taxonomy-page.component.ts | 76 ++++++++++++++++++- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.html b/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.html index 149e1e6b33..87c7937b1b 100644 --- a/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.html +++ b/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.html @@ -6,5 +6,5 @@ (deselect)="onDeselect($event)"> - {{ 'browse.taxonomy.button' | translate }} + {{ 'browse.taxonomy.button' | translate }} diff --git a/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.ts b/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.ts index b132f299d6..d568a97fd7 100644 --- a/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.ts +++ b/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.ts @@ -1,6 +1,14 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, OnInit, Inject, OnDestroy } from '@angular/core'; import { VocabularyOptions } from '../../core/submission/vocabularies/models/vocabulary-options.model'; import { VocabularyEntryDetail } from '../../core/submission/vocabularies/models/vocabulary-entry-detail.model'; +import { ActivatedRoute } from '@angular/router'; +import { Observable, Subscription } from 'rxjs'; +import { BrowseDefinition } from '../../core/shared/browse-definition.model'; +import { GenericConstructor } from '../../core/shared/generic-constructor'; +import { BROWSE_BY_COMPONENT_FACTORY } from '../browse-by-switcher/browse-by-decorator'; +import { map } from 'rxjs/operators'; +import { ThemeService } from 'src/app/shared/theme-support/theme.service'; +import { HierarchicalBrowseDefinition } from '../../core/shared/hierarchical-browse-definition.model'; @Component({ selector: 'ds-browse-by-taxonomy-page', @@ -10,7 +18,7 @@ import { VocabularyEntryDetail } from '../../core/submission/vocabularies/models /** * Component for browsing items by metadata in a hierarchical controlled vocabulary */ -export class BrowseByTaxonomyPageComponent implements OnInit { +export class BrowseByTaxonomyPageComponent implements OnInit, OnDestroy { /** * The {@link VocabularyOptions} object @@ -27,8 +35,48 @@ export class BrowseByTaxonomyPageComponent implements OnInit { */ filterValues: string[]; - ngOnInit() { - this.vocabularyOptions = { name: 'srsc', closed: true }; + /** + * The facet the use when filtering + */ + facetType: string; + + /** + * The used vocabulary + */ + vocabularyName: string; + + /** + * The parameters used in the URL + */ + queryParams: any; + + /** + * Resolved browse-by component + */ + browseByComponent: Observable; + + /** + * Subscriptions to track + */ + browseByComponentSubs: Subscription[] = []; + + public constructor( protected route: ActivatedRoute, + protected themeService: ThemeService, + @Inject(BROWSE_BY_COMPONENT_FACTORY) private getComponentByBrowseByType: (browseByType, theme) => GenericConstructor) { + } + + ngOnInit(): void { + this.browseByComponent = this.route.data.pipe( + map((data: { browseDefinition: BrowseDefinition }) => { + this.getComponentByBrowseByType(data.browseDefinition.getRenderType(), this.themeService.getThemeName()); + return data.browseDefinition; + }) + ); + this.browseByComponentSubs.push(this.browseByComponent.subscribe((browseDefinition: HierarchicalBrowseDefinition) => { + this.facetType = browseDefinition.facetType; + this.vocabularyName = browseDefinition.vocabulary; + this.vocabularyOptions = { name: this.vocabularyName, closed: true }; + })); } /** @@ -41,10 +89,30 @@ export class BrowseByTaxonomyPageComponent implements OnInit { this.selectedItems.push(detail); this.filterValues = this.selectedItems .map((item: VocabularyEntryDetail) => `${item.value},equals`); + this.updateQueryParams(); } + /** + * Removes detail from selectedItems and filterValues. + * + * @param detail VocabularyEntryDetail to be removed + */ onDeselect(detail: VocabularyEntryDetail): void { this.selectedItems = this.selectedItems.filter((entry: VocabularyEntryDetail) => { return entry !== detail; }); this.filterValues = this.filterValues.filter((value: string) => { return value !== `${detail.value},equals`; }); + this.updateQueryParams(); + } + + /** + * Updates queryParams based on the current facetType and filterValues. + */ + private updateQueryParams(): void { + this.queryParams = { + ['f.' + this.facetType]: this.filterValues + }; + } + + ngOnDestroy(): void { + this.browseByComponentSubs.forEach((sub: Subscription) => sub.unsubscribe()); } } From d9b0eebc18154aaddc827bfc38a80f09ca7d95ef Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Tue, 23 May 2023 17:08:00 +0200 Subject: [PATCH 12/14] 101623: Remove hardcoded 'browse by srsc' menu option in MenuResolver --- src/app/menu.resolver.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/app/menu.resolver.ts b/src/app/menu.resolver.ts index f771ef8b27..8630150c58 100644 --- a/src/app/menu.resolver.ts +++ b/src/app/menu.resolver.ts @@ -137,20 +137,6 @@ export class MenuResolver implements Resolve { } as TextMenuItemModel, } ); - menuList.push( - { - id: 'browse_global_by_srsc', - parentID: 'browse_global', - active: false, - visible: true, - index: 99, - model: { - type: MenuItemType.LINK, - text: `menu.section.browse_global_by_srsc`, - link: `/browse/srsc` - } as LinkMenuItemModel - } - ); } menuList.forEach((menuSection) => this.menuService.addSection(MenuID.PUBLIC, Object.assign(menuSection, { shouldPersistOnRouteChange: true From 2f0f69710e32412084fd94ee5a034d940146c340 Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Tue, 23 May 2023 17:40:07 +0200 Subject: [PATCH 13/14] 101623: Fix browse-related tests by replacing abstract class --- src/app/browse-by/browse-by-guard.spec.ts | 4 +-- .../browse-by-switcher.component.spec.ts | 16 +++++++----- .../browse-by-taxonomy-page.component.spec.ts | 19 ++++++++++++++ src/app/core/browse/browse.service.spec.ts | 26 ++++++++++++++++--- src/app/navbar/navbar.component.spec.ts | 17 ++++++++---- 5 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/app/browse-by/browse-by-guard.spec.ts b/src/app/browse-by/browse-by-guard.spec.ts index 933c95a3cb..7fca9b15c0 100644 --- a/src/app/browse-by/browse-by-guard.spec.ts +++ b/src/app/browse-by/browse-by-guard.spec.ts @@ -2,8 +2,8 @@ import { first } from 'rxjs/operators'; import { BrowseByGuard } from './browse-by-guard'; import { of as observableOf } from 'rxjs'; import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; -import { BrowseDefinition } from '../core/shared/browse-definition.model'; import { BrowseByDataType } from './browse-by-switcher/browse-by-decorator'; +import { ValueListBrowseDefinition } from '../core/shared/value-list-browse-definition.model'; describe('BrowseByGuard', () => { describe('canActivate', () => { @@ -18,7 +18,7 @@ describe('BrowseByGuard', () => { const id = 'author'; const scope = '1234-65487-12354-1235'; const value = 'Filter'; - const browseDefinition = Object.assign(new BrowseDefinition(), { type: BrowseByDataType.Metadata, metadataKeys: ['dc.contributor'] }); + const browseDefinition = Object.assign(new ValueListBrowseDefinition(), { type: BrowseByDataType.Metadata, metadataKeys: ['dc.contributor'] }); beforeEach(() => { dsoService = { 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 c2e1c9cb68..c13405dd4d 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 @@ -3,9 +3,11 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { BROWSE_BY_COMPONENT_FACTORY, BrowseByDataType } from './browse-by-decorator'; -import { BrowseDefinition } from '../../core/shared/browse-definition.model'; import { BehaviorSubject } from 'rxjs'; import { ThemeService } from '../../shared/theme-support/theme.service'; +import { FlatBrowseDefinition } from '../../core/shared/flat-browse-definition.model'; +import { ValueListBrowseDefinition } from '../../core/shared/value-list-browse-definition.model'; +import { NonHierarchicalBrowseDefinition } from '../../core/shared/non-hierarchical-browse-definition'; describe('BrowseBySwitcherComponent', () => { let comp: BrowseBySwitcherComponent; @@ -13,33 +15,33 @@ describe('BrowseBySwitcherComponent', () => { const types = [ Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'title', dataType: BrowseByDataType.Title, } ), Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'dateissued', dataType: BrowseByDataType.Date, metadataKeys: ['dc.date.issued'] } ), Object.assign( - new BrowseDefinition(), { + new ValueListBrowseDefinition(), { id: 'author', dataType: BrowseByDataType.Metadata, } ), Object.assign( - new BrowseDefinition(), { + new ValueListBrowseDefinition(), { id: 'subject', dataType: BrowseByDataType.Metadata, } ), ]; - const data = new BehaviorSubject(createDataWithBrowseDefinition(new BrowseDefinition())); + const data = new BehaviorSubject(createDataWithBrowseDefinition(new FlatBrowseDefinition())); const activatedRouteStub = { data @@ -70,7 +72,7 @@ describe('BrowseBySwitcherComponent', () => { comp = fixture.componentInstance; })); - types.forEach((type: BrowseDefinition) => { + types.forEach((type: NonHierarchicalBrowseDefinition) => { describe(`when switching to a browse-by page for "${type.id}"`, () => { beforeEach(() => { data.next(createDataWithBrowseDefinition(type)); diff --git a/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.spec.ts b/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.spec.ts index bc9380d7ad..484992afbf 100644 --- a/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.spec.ts +++ b/src/app/browse-by/browse-by-taxonomy-page/browse-by-taxonomy-page.component.spec.ts @@ -4,17 +4,36 @@ import { BrowseByTaxonomyPageComponent } from './browse-by-taxonomy-page.compone import { VocabularyEntryDetail } from '../../core/submission/vocabularies/models/vocabulary-entry-detail.model'; import { TranslateModule } from '@ngx-translate/core'; import { NO_ERRORS_SCHEMA } from '@angular/core'; +import { ActivatedRoute } from '@angular/router'; +import { BehaviorSubject } from 'rxjs'; +import { createDataWithBrowseDefinition } from '../browse-by-switcher/browse-by-switcher.component.spec'; +import { HierarchicalBrowseDefinition } from '../../core/shared/hierarchical-browse-definition.model'; +import { ThemeService } from '../../shared/theme-support/theme.service'; describe('BrowseByTaxonomyPageComponent', () => { let component: BrowseByTaxonomyPageComponent; let fixture: ComponentFixture; + let themeService: ThemeService; let detail1: VocabularyEntryDetail; let detail2: VocabularyEntryDetail; + const data = new BehaviorSubject(createDataWithBrowseDefinition(new HierarchicalBrowseDefinition())); + const activatedRouteStub = { + data + }; + beforeEach(async () => { + themeService = jasmine.createSpyObj('themeService', { + getThemeName: 'dspace', + }); + await TestBed.configureTestingModule({ imports: [ TranslateModule.forRoot() ], declarations: [ BrowseByTaxonomyPageComponent ], + providers: [ + { provide: ActivatedRoute, useValue: activatedRouteStub }, + { provide: ThemeService, useValue: themeService }, + ], schemas: [NO_ERRORS_SCHEMA] }) .compileComponents(); diff --git a/src/app/core/browse/browse.service.spec.ts b/src/app/core/browse/browse.service.spec.ts index 46ac8c44f4..0e39e53e43 100644 --- a/src/app/core/browse/browse.service.spec.ts +++ b/src/app/core/browse/browse.service.spec.ts @@ -6,13 +6,15 @@ import { getMockRequestService } from '../../shared/mocks/request.service.mock'; import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RequestService } from '../data/request.service'; -import { BrowseDefinition } from '../shared/browse-definition.model'; import { BrowseEntrySearchOptions } from './browse-entry-search-options.model'; import { BrowseService } from './browse.service'; import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { createPaginatedList, getFirstUsedArgumentOfSpyMethod } from '../../shared/testing/utils.test'; import { getMockHrefOnlyDataService } from '../../shared/mocks/href-only-data.service.mock'; import { RequestEntry } from '../data/request-entry.model'; +import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model'; +import { ValueListBrowseDefinition } from '../shared/value-list-browse-definition.model'; +import { HierarchicalBrowseDefinition } from '../shared/hierarchical-browse-definition.model'; describe('BrowseService', () => { let scheduler: TestScheduler; @@ -23,7 +25,7 @@ describe('BrowseService', () => { const browsesEndpointURL = 'https://rest.api/browses'; const halService: any = new HALEndpointServiceStub(browsesEndpointURL); const browseDefinitions = [ - Object.assign(new BrowseDefinition(), { + Object.assign(new FlatBrowseDefinition(), { id: 'date', metadataBrowse: false, sortOptions: [ @@ -50,7 +52,7 @@ describe('BrowseService', () => { items: { href: 'https://rest.api/discover/browses/dateissued/items' } } }), - Object.assign(new BrowseDefinition(), { + Object.assign(new ValueListBrowseDefinition(), { id: 'author', metadataBrowse: true, sortOptions: [ @@ -78,7 +80,23 @@ describe('BrowseService', () => { entries: { href: 'https://rest.api/discover/browses/author/entries' }, items: { href: 'https://rest.api/discover/browses/author/items' } } - }) + }), + Object.assign(new HierarchicalBrowseDefinition(), { + id: 'srsc', + browseType: 'hierarchicalBrowse', + facetType: 'subject', + vocabulary: 'srsc', + type: 'browse', + metadata: [ + 'dc.subject' + ], + _links: { + vocabulary: { 'href': 'https://rest.api/submission/vocabularies/srsc/' }, + items: { 'href': 'https://rest.api/discover/browses/srsc/items' }, + entries: { 'href': 'https://rest.api/discover/browses/srsc/entries' }, + self: { 'href': 'https://rest.api/discover/browses/srsc' } + } + }), ]; let browseDefinitionDataService; diff --git a/src/app/navbar/navbar.component.spec.ts b/src/app/navbar/navbar.component.spec.ts index ada9be9d0b..983eace055 100644 --- a/src/app/navbar/navbar.component.spec.ts +++ b/src/app/navbar/navbar.component.spec.ts @@ -16,7 +16,6 @@ import { RouterTestingModule } from '@angular/router/testing'; import { BrowseService } from '../core/browse/browse.service'; import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; import { buildPaginatedList } from '../core/data/paginated-list.model'; -import { BrowseDefinition } from '../core/shared/browse-definition.model'; import { BrowseByDataType } from '../browse-by/browse-by-switcher/browse-by-decorator'; import { Item } from '../core/shared/item.model'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; @@ -28,6 +27,9 @@ import { authReducer } from '../core/auth/auth.reducer'; import { provideMockStore } from '@ngrx/store/testing'; import { AuthTokenInfo } from '../core/auth/models/auth-token-info.model'; import { EPersonMock } from '../shared/testing/eperson.mock'; +import { FlatBrowseDefinition } from '../core/shared/flat-browse-definition.model'; +import { ValueListBrowseDefinition } from '../core/shared/value-list-browse-definition.model'; +import { HierarchicalBrowseDefinition } from '../core/shared/hierarchical-browse-definition.model'; let comp: NavbarComponent; let fixture: ComponentFixture; @@ -66,30 +68,35 @@ describe('NavbarComponent', () => { beforeEach(waitForAsync(() => { browseDefinitions = [ Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'title', dataType: BrowseByDataType.Title, } ), Object.assign( - new BrowseDefinition(), { + new FlatBrowseDefinition(), { id: 'dateissued', dataType: BrowseByDataType.Date, metadataKeys: ['dc.date.issued'] } ), Object.assign( - new BrowseDefinition(), { + new ValueListBrowseDefinition(), { id: 'author', dataType: BrowseByDataType.Metadata, } ), Object.assign( - new BrowseDefinition(), { + new ValueListBrowseDefinition(), { id: 'subject', dataType: BrowseByDataType.Metadata, } ), + Object.assign( + new HierarchicalBrowseDefinition(), { + id: 'srsc', + } + ), ]; initialState = { core: { From 78d5116cdb7d1d6afaef8d8b9e35b792b551e03d Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Tue, 23 May 2023 17:54:33 +0200 Subject: [PATCH 14/14] 101623: Add missing docs + Fix lint issues --- src/app/browse-by/browse-by-routing.module.ts | 2 -- src/app/core/data/browse-response-parsing.service.ts | 2 +- src/app/core/data/request.models.ts | 3 +++ src/app/core/shared/flat-browse-definition.model.ts | 3 +++ src/app/core/shared/hierarchical-browse-definition.model.ts | 3 +++ src/app/core/shared/value-list-browse-definition.model.ts | 3 +++ .../form/vocabulary-treeview/vocabulary-treeview.component.ts | 2 +- 7 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/app/browse-by/browse-by-routing.module.ts b/src/app/browse-by/browse-by-routing.module.ts index 5d3697f391..5788d3cc70 100644 --- a/src/app/browse-by/browse-by-routing.module.ts +++ b/src/app/browse-by/browse-by-routing.module.ts @@ -4,8 +4,6 @@ import { BrowseByGuard } from './browse-by-guard'; import { BrowseByDSOBreadcrumbResolver } from './browse-by-dso-breadcrumb.resolver'; import { BrowseByI18nBreadcrumbResolver } from './browse-by-i18n-breadcrumb.resolver'; import { ThemedBrowseBySwitcherComponent } from './browse-by-switcher/themed-browse-by-switcher.component'; -import { ThemedBrowseByTaxonomyPageComponent } from './browse-by-taxonomy-page/themed-browse-by-taxonomy-page.component'; -import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver'; @NgModule({ imports: [ diff --git a/src/app/core/data/browse-response-parsing.service.ts b/src/app/core/data/browse-response-parsing.service.ts index 3681198406..a568cdb617 100644 --- a/src/app/core/data/browse-response-parsing.service.ts +++ b/src/app/core/data/browse-response-parsing.service.ts @@ -15,7 +15,7 @@ import { ValueListBrowseDefinition } from '../shared/value-list-browse-definitio import { VALUE_LIST_BROWSE_DEFINITION } from '../shared/value-list-browse-definition.resource-type'; /** - * A ResponseParsingService used to parse RawRestResponse coming from the REST API to a BrowseDefinition object + * A ResponseParsingService used to parse a REST API response to a BrowseDefinition object */ @Injectable({ providedIn: 'root', diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index 79505a3dee..9809bc0fde 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -119,6 +119,9 @@ export class PatchRequest extends DSpaceRestRequest { } } +/** + * Class representing a BrowseDefinition HTTP Rest request object + */ export class BrowseDefinitionRestRequest extends DSpaceRestRequest { getResponseParser(): GenericConstructor { return BrowseResponseParsingService; diff --git a/src/app/core/shared/flat-browse-definition.model.ts b/src/app/core/shared/flat-browse-definition.model.ts index f94fa75b91..308f0c7a5b 100644 --- a/src/app/core/shared/flat-browse-definition.model.ts +++ b/src/app/core/shared/flat-browse-definition.model.ts @@ -5,6 +5,9 @@ import { FLAT_BROWSE_DEFINITION } from './flat-browse-definition.resource-type'; import { ResourceType } from './resource-type'; import { NonHierarchicalBrowseDefinition } from './non-hierarchical-browse-definition'; +/** + * BrowseDefinition model for browses of type 'flatBrowse' + */ @typedObject @inheritSerialization(NonHierarchicalBrowseDefinition) export class FlatBrowseDefinition extends NonHierarchicalBrowseDefinition { diff --git a/src/app/core/shared/hierarchical-browse-definition.model.ts b/src/app/core/shared/hierarchical-browse-definition.model.ts index 427f5e1947..ca3ed7bff0 100644 --- a/src/app/core/shared/hierarchical-browse-definition.model.ts +++ b/src/app/core/shared/hierarchical-browse-definition.model.ts @@ -6,6 +6,9 @@ import { HALLink } from './hal-link.model'; import { ResourceType } from './resource-type'; import { BrowseDefinition } from './browse-definition.model'; +/** + * BrowseDefinition model for browses of type 'hierarchicalBrowse' + */ @typedObject @inheritSerialization(BrowseDefinition) export class HierarchicalBrowseDefinition extends BrowseDefinition { diff --git a/src/app/core/shared/value-list-browse-definition.model.ts b/src/app/core/shared/value-list-browse-definition.model.ts index a42a702940..33cce82cac 100644 --- a/src/app/core/shared/value-list-browse-definition.model.ts +++ b/src/app/core/shared/value-list-browse-definition.model.ts @@ -5,6 +5,9 @@ import { VALUE_LIST_BROWSE_DEFINITION } from './value-list-browse-definition.res import { ResourceType } from './resource-type'; import { NonHierarchicalBrowseDefinition } from './non-hierarchical-browse-definition'; +/** + * BrowseDefinition model for browses of type 'valueList' + */ @typedObject @inheritSerialization(NonHierarchicalBrowseDefinition) export class ValueListBrowseDefinition extends NonHierarchicalBrowseDefinition { diff --git a/src/app/shared/form/vocabulary-treeview/vocabulary-treeview.component.ts b/src/app/shared/form/vocabulary-treeview/vocabulary-treeview.component.ts index 2199de920e..891a825745 100644 --- a/src/app/shared/form/vocabulary-treeview/vocabulary-treeview.component.ts +++ b/src/app/shared/form/vocabulary-treeview/vocabulary-treeview.component.ts @@ -248,7 +248,7 @@ export class VocabularyTreeviewComponent implements OnDestroy, OnInit { } /** - * Method called on entry select + * Method called on entry select/deselect */ onSelect(item: VocabularyEntryDetail) { if (!this.selectedItems.includes(item.id)) {