From 928157f994b6e617d1037c7931627372a90b7190 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Tue, 8 Nov 2022 16:43:24 +1300 Subject: [PATCH 01/20] [TLC-380] Support browse links and regex links in metadata display (resolved conflicts jan 2023) --- .../browse/browse-link-data.service.spec.ts | 28 +++++++ .../core/browse/browse-link-data.service.ts | 81 +++++++++++++++++++ src/app/core/browse/browse.service.ts | 36 ++++++++- src/app/core/core.module.ts | 2 + .../metadata-representation.model.ts | 11 ++- .../metadatum-representation.model.ts | 11 ++- .../metadata-values.component.html | 23 +++++- .../metadata-values.component.ts | 28 +++++++ .../item-page-author-field.component.spec.ts | 4 +- .../item-page-date-field.component.spec.ts | 4 +- .../generic-item-page-field.component.spec.ts | 4 +- .../generic-item-page-field.component.ts | 5 ++ .../item-page-field.component.html | 2 + .../item-page-field.component.spec.ts | 60 ++++++++++++-- .../item-page-field.component.ts | 20 +++++ .../item-page-title-field.component.spec.ts | 4 +- .../uri/item-page-uri-field.component.spec.ts | 4 +- .../item-types/shared/item.component.ts | 12 ++- .../versioned-item.component.ts | 4 +- ...-link-metadata-list-element.component.html | 8 ++ ...nk-metadata-list-element.component.spec.ts | 36 +++++++++ ...se-link-metadata-list-element.component.ts | 18 +++++ ...a-representation-list-element.component.ts | 7 ++ ...-text-metadata-list-element.component.html | 15 +++- ...xt-metadata-list-element.component.spec.ts | 10 ++- src/app/shared/shared.module.ts | 3 + .../testing/browse-link-data-service.stub.ts | 77 ++++++++++++++++++ 27 files changed, 491 insertions(+), 26 deletions(-) create mode 100644 src/app/core/browse/browse-link-data.service.spec.ts create mode 100644 src/app/core/browse/browse-link-data.service.ts create mode 100644 src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.html create mode 100644 src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts create mode 100644 src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.ts create mode 100644 src/app/shared/testing/browse-link-data-service.stub.ts diff --git a/src/app/core/browse/browse-link-data.service.spec.ts b/src/app/core/browse/browse-link-data.service.spec.ts new file mode 100644 index 0000000000..bb34c24900 --- /dev/null +++ b/src/app/core/browse/browse-link-data.service.spec.ts @@ -0,0 +1,28 @@ +import { followLink } from '../../shared/utils/follow-link-config.model'; +import { EMPTY } from 'rxjs'; +import { FindListOptions } from '../data/find-list-options.model'; +import { BrowseLinkDataService } from './browse-link-data.service'; + +describe(`BrowseLinkDataService`, () => { + let service: BrowseLinkDataService; + const findAllDataSpy = jasmine.createSpyObj('findAllData', { + findAll: EMPTY, + }); + const options = new FindListOptions(); + const linksToFollow = [ + followLink('entries'), + followLink('items') + ]; + + beforeEach(() => { + service = new BrowseLinkDataService(null, null, null, null, null); + (service as any).findAllData = findAllDataSpy; + }); + + describe(`getBrowseLinkFor`, () => { + it(`should call findAll on findAllData`, () => { + service.getBrowseLinkFor(['dc.test']); + expect(findAllDataSpy.findAll).toHaveBeenCalledWith({ elementsPerPage: 9999 }); + }); + }); +}); diff --git a/src/app/core/browse/browse-link-data.service.ts b/src/app/core/browse/browse-link-data.service.ts new file mode 100644 index 0000000000..0f4820bb19 --- /dev/null +++ b/src/app/core/browse/browse-link-data.service.ts @@ -0,0 +1,81 @@ +import { Injectable } from '@angular/core'; +import { BrowseDefinition } from '../shared/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'; +import { HALEndpointService } from '../shared/hal-endpoint.service'; +import { Observable } from 'rxjs'; +import { RemoteData } from '../data/remote-data'; +import { PaginatedList } from '../data/paginated-list.model'; +import { IdentifiableDataService } from '../data/base/identifiable-data.service'; +import { FindAllDataImpl } from '../data/base/find-all-data'; +import { BrowseDefinitionDataService } from './browse-definition-data.service'; +import { + getFirstSucceededRemoteData, getPaginatedListPayload, getRemoteDataPayload +} from '../shared/operators'; +import { distinctUntilChanged, map, startWith } from 'rxjs/operators'; +import { isEmpty, isNotEmpty } from '../../shared/empty.util'; +import { BrowseService } from './browse.service'; + +/** + * Data service responsible for retrieving browse definitions from the REST server, IF AND ONLY IF + * they are configured as browse links (webui.browse.link.) + */ +@Injectable() +export class BrowseLinkDataService extends IdentifiableDataService { + private findAllData: FindAllDataImpl; + + constructor( + protected requestService: RequestService, + protected rdbService: RemoteDataBuildService, + protected objectCache: ObjectCacheService, + protected halService: HALEndpointService, + protected browseDefinitionDataService: BrowseDefinitionDataService + ) { + super('browselinks', requestService, rdbService, objectCache, halService); + this.findAllData = new FindAllDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, this.responseMsToLive); + } + + /** + * Get all BrowseDefinitions + */ + getBrowseLinks(): Observable>> { + return this.findAllData.findAll({ elementsPerPage: 9999 }).pipe( + getFirstSucceededRemoteData(), + ); + } + + + /** + * Get the browse URL by providing a list of metadata keys + * @param metadatumKey + * @param linkPath + */ + getBrowseLinkFor(metadataKeys: string[]): Observable { + let searchKeyArray: string[] = []; + metadataKeys.forEach((metadataKey) => { + searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); + }) + return this.getBrowseLinks().pipe( + getRemoteDataPayload(), + getPaginatedListPayload(), + map((browseDefinitions: BrowseDefinition[]) => browseDefinitions + .find((def: BrowseDefinition) => { + const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); + return isNotEmpty(matchingKeys); + }) + ), + map((def: BrowseDefinition) => { + if (isEmpty(def) || isEmpty(def.id)) { + //throw new Error(`A browse definition for field ${metadataKey} isn't configured`); + } else { + return def; + } + }), + startWith(undefined), + distinctUntilChanged() + ); + } + +} + diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index 2fab189254..fd8e68430d 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -35,7 +35,7 @@ export const BROWSE_LINKS_TO_FOLLOW: FollowLinkConfig[] = [ export class BrowseService { protected linkPath = 'browses'; - private static toSearchKeyArray(metadataKey: string): string[] { + public static toSearchKeyArray(metadataKey: string): string[] { const keyParts = metadataKey.split('.'); const searchFor = []; searchFor.push('*'); @@ -229,6 +229,7 @@ export class BrowseService { * @param linkPath */ getBrowseURLFor(metadataKey: string, linkPath: string): Observable { + console.log("Looking for " + metadataKey + " in link path " + linkPath); const searchKeyArray = BrowseService.toSearchKeyArray(metadataKey); return this.getBrowseDefinitions().pipe( getRemoteDataPayload(), @@ -251,4 +252,37 @@ export class BrowseService { ); } + /** + * Get the browse URL by providing a metadatum key and linkPath + * @param metadatumKey + * @param linkPath + */ + getBrowseDefinitionFor(metadataKeys: string[]): Observable { + let searchKeyArray: string[] = []; + metadataKeys.forEach((metadataKey) => { + searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); + }) + return this.getBrowseDefinitions().pipe( + getRemoteDataPayload(), + getPaginatedListPayload(), + map((browseDefinitions: BrowseDefinition[]) => browseDefinitions + .find((def: BrowseDefinition) => { + //console.dir(def.metadataKeys); + const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); + //console.dir(matchingKeys); + return isNotEmpty(matchingKeys); + }) + ), + map((def: BrowseDefinition) => { + if (isEmpty(def) || isEmpty(def.id)) { + //throw new Error(`A browse definition for field ${metadataKey} isn't configured`); + } else { + return def; + } + }), + startWith(undefined), + distinctUntilChanged() + ); + } + } diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index ede23ba43b..d90ad8da77 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -24,6 +24,7 @@ import { SidebarService } from '../shared/sidebar/sidebar.service'; import { AuthenticatedGuard } from './auth/authenticated.guard'; import { AuthStatus } from './auth/models/auth-status.model'; import { BrowseService } from './browse/browse.service'; +import { BrowseLinkDataService } from './browse/browse-link-data.service'; import { RemoteDataBuildService } from './cache/builders/remote-data-build.service'; import { ObjectCacheService } from './cache/object-cache.service'; import { SubmissionDefinitionsModel } from './config/models/config-submission-definitions.model'; @@ -221,6 +222,7 @@ const PROVIDERS = [ MyDSpaceResponseParsingService, ServerResponseService, BrowseService, + BrowseLinkDataService, AccessStatusDataService, SubmissionCcLicenseDataService, SubmissionCcLicenseUrlDataService, diff --git a/src/app/core/shared/metadata-representation/metadata-representation.model.ts b/src/app/core/shared/metadata-representation/metadata-representation.model.ts index 06387966f7..379a3d1be8 100644 --- a/src/app/core/shared/metadata-representation/metadata-representation.model.ts +++ b/src/app/core/shared/metadata-representation/metadata-representation.model.ts @@ -1,11 +1,14 @@ /** * An Enum defining the representation type of metadata */ +import { BrowseDefinition } from '../browse-definition.model'; + export enum MetadataRepresentationType { None = 'none', Item = 'item', AuthorityControlled = 'authority_controlled', - PlainText = 'plain_text' + PlainText = 'plain_text', + BrowseLink = 'browse_link' } /** @@ -24,8 +27,14 @@ export interface MetadataRepresentation { */ representationType: MetadataRepresentationType; + /** + * The browse definition (optional) + */ + browseDefinition?: BrowseDefinition; + /** * Fetches the value to be displayed */ getValue(): string; + } diff --git a/src/app/core/shared/metadata-representation/metadatum/metadatum-representation.model.ts b/src/app/core/shared/metadata-representation/metadatum/metadatum-representation.model.ts index 595147f3e6..a09de12ae4 100644 --- a/src/app/core/shared/metadata-representation/metadatum/metadatum-representation.model.ts +++ b/src/app/core/shared/metadata-representation/metadatum/metadatum-representation.model.ts @@ -1,6 +1,7 @@ import { MetadataRepresentation, MetadataRepresentationType } from '../metadata-representation.model'; import { hasValue } from '../../../../shared/empty.util'; import { MetadataValue } from '../../metadata.models'; +import { BrowseDefinition } from '../../browse-definition.model'; /** * This class defines the way the metadatum it extends should be represented @@ -12,9 +13,15 @@ export class MetadatumRepresentation extends MetadataValue implements MetadataRe */ itemType: string; - constructor(itemType: string) { + /** + * The browse definition ID passed in with the metadatum, if any + */ + browseDefinition?: BrowseDefinition; + + constructor(itemType: string, browseDefinition?: BrowseDefinition) { super(); this.itemType = itemType; + this.browseDefinition = browseDefinition; } /** @@ -23,6 +30,8 @@ export class MetadatumRepresentation extends MetadataValue implements MetadataRe get representationType(): MetadataRepresentationType { if (hasValue(this.authority)) { return MetadataRepresentationType.AuthorityControlled; + } else if (hasValue(this.browseDefinition)) { + return MetadataRepresentationType.BrowseLink; } else { return MetadataRepresentationType.PlainText; } diff --git a/src/app/item-page/field-components/metadata-values/metadata-values.component.html b/src/app/item-page/field-components/metadata-values/metadata-values.component.html index becb35e4bd..e3ee1cefbc 100644 --- a/src/app/item-page/field-components/metadata-values/metadata-values.component.html +++ b/src/app/item-page/field-components/metadata-values/metadata-values.component.html @@ -1,16 +1,37 @@ - + + + + + + + {{value}} + + + + {{value}} + + + + + {{value}} + + diff --git a/src/app/item-page/field-components/metadata-values/metadata-values.component.ts b/src/app/item-page/field-components/metadata-values/metadata-values.component.ts index 3f0c918796..5b7e937d95 100644 --- a/src/app/item-page/field-components/metadata-values/metadata-values.component.ts +++ b/src/app/item-page/field-components/metadata-values/metadata-values.component.ts @@ -1,6 +1,8 @@ import { Component, Inject, Input, OnChanges, SimpleChanges } from '@angular/core'; import { MetadataValue } from '../../../core/shared/metadata.models'; import { APP_CONFIG, AppConfig } from '../../../../config/app-config.interface'; +import { BrowseDefinition } from '../../../core/shared/browse-definition.model'; +import { hasValue } from '../../../shared/empty.util'; /** * This component renders the configured 'values' into the ds-metadata-field-wrapper component. @@ -40,12 +42,38 @@ export class MetadataValuesComponent implements OnChanges { */ @Input() enableMarkdown = false; + /** + * Whether any valid HTTP(S) URL should be rendered as a link + */ + @Input() urlRegex?; + /** * This variable will be true if both {@link environment.markdown.enabled} and {@link enableMarkdown} are true. */ renderMarkdown; + @Input() browseDefinition?: BrowseDefinition; + ngOnChanges(changes: SimpleChanges): void { this.renderMarkdown = !!this.appConfig.markdown.enabled && this.enableMarkdown; } + + /** + * Does this metadata value have a configured link to a browse definition? + */ + hasBrowseDefinition(): boolean { + return hasValue(this.browseDefinition); + } + + /** + * Does this metadata value have a valid URL that should be rendered as a link? + * @param value + */ + hasLink(value): boolean { + if (hasValue(this.urlRegex)) { + const pattern: RegExp = new RegExp(this.urlRegex); + return pattern.test(value.value); + } + return false; + } } diff --git a/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts index 7f8d6fb812..5536d54632 100644 --- a/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts @@ -3,7 +3,7 @@ import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { MetadataValuesComponent } from '../../../../field-components/metadata-values/metadata-values.component'; -import { mockItemWithMetadataFieldAndValue } from '../item-page-field.component.spec'; +import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component.spec'; import { ItemPageAuthorFieldComponent } from './item-page-author-field.component'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; @@ -37,7 +37,7 @@ describe('ItemPageAuthorFieldComponent', () => { beforeEach(waitForAsync(() => { fixture = TestBed.createComponent(ItemPageAuthorFieldComponent); comp = fixture.componentInstance; - comp.item = mockItemWithMetadataFieldAndValue(field, mockValue); + comp.item = mockItemWithMetadataFieldsAndValue([field], mockValue); fixture.detectChanges(); })); diff --git a/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts index ebd37e8b8a..9b60cdd3d6 100644 --- a/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts @@ -3,7 +3,7 @@ import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { MetadataValuesComponent } from '../../../../field-components/metadata-values/metadata-values.component'; -import { mockItemWithMetadataFieldAndValue } from '../item-page-field.component.spec'; +import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component.spec'; import { ItemPageDateFieldComponent } from './item-page-date-field.component'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; @@ -36,7 +36,7 @@ describe('ItemPageDateFieldComponent', () => { beforeEach(waitForAsync(() => { fixture = TestBed.createComponent(ItemPageDateFieldComponent); comp = fixture.componentInstance; - comp.item = mockItemWithMetadataFieldAndValue(mockField, mockValue); + comp.item = mockItemWithMetadataFieldsAndValue([mockField], mockValue); fixture.detectChanges(); })); diff --git a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts index f055101f2a..053976b374 100644 --- a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts @@ -3,7 +3,7 @@ import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { MetadataValuesComponent } from '../../../../field-components/metadata-values/metadata-values.component'; -import { mockItemWithMetadataFieldAndValue } from '../item-page-field.component.spec'; +import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component.spec'; import { GenericItemPageFieldComponent } from './generic-item-page-field.component'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; @@ -38,7 +38,7 @@ describe('GenericItemPageFieldComponent', () => { beforeEach(waitForAsync(() => { fixture = TestBed.createComponent(GenericItemPageFieldComponent); comp = fixture.componentInstance; - comp.item = mockItemWithMetadataFieldAndValue(mockField, mockValue); + comp.item = mockItemWithMetadataFieldsAndValue([mockField], mockValue); comp.fields = mockFields; comp.label = mockLabel; fixture.detectChanges(); diff --git a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.ts b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.ts index 9d818e2f7d..c9c16724e1 100644 --- a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.ts +++ b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.ts @@ -40,5 +40,10 @@ export class GenericItemPageFieldComponent extends ItemPageFieldComponent { */ @Input() enableMarkdown = false; + /** + * Whether any valid HTTP(S) URL should be rendered as a link + */ + @Input() urlRegex?; + } diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.html b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.html index 8b017c77f5..91d40b0ad7 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.html +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.html @@ -4,5 +4,7 @@ [separator]="separator" [label]="label" [enableMarkdown]="enableMarkdown" + [urlRegex]="urlRegex" + [browseDefinition]="browseDefinition|async" > diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts index e41fd1b8a7..48d5a0f29e 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts @@ -1,6 +1,6 @@ import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; -import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; +import { ComponentFixture, inject, TestBed, waitForAsync } from '@angular/core/testing'; import { Item } from '../../../../core/shared/item.model'; import { TranslateLoaderMock } from '../../../../shared/mocks/translate-loader.mock'; import { ItemPageFieldComponent } from './item-page-field.component'; @@ -12,6 +12,9 @@ import { environment } from '../../../../../environments/environment'; import { MarkdownPipe } from '../../../../shared/utils/markdown.pipe'; import { SharedModule } from '../../../../shared/shared.module'; import { APP_CONFIG } from '../../../../../config/app-config.interface'; +import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; +import { browseLinkDataServiceStub } from '../../../../shared/testing/browse-link-data-service.stub'; +import { By } from '@angular/platform-browser'; let comp: ItemPageFieldComponent; let fixture: ComponentFixture; @@ -20,7 +23,9 @@ let markdownSpy; const mockValue = 'test value'; const mockField = 'dc.test'; const mockLabel = 'test label'; -const mockFields = [mockField]; +const mockAuthorField = 'dc.contributor.author'; +const mockDateIssuedField = 'dc.date.issued'; +const mockFields = [mockField, mockAuthorField, mockDateIssuedField]; describe('ItemPageFieldComponent', () => { @@ -44,6 +49,7 @@ describe('ItemPageFieldComponent', () => { ], providers: [ { provide: APP_CONFIG, useValue: appConfig }, + { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } ], declarations: [ItemPageFieldComponent, MetadataValuesComponent], schemas: [NO_ERRORS_SCHEMA] @@ -53,7 +59,7 @@ describe('ItemPageFieldComponent', () => { markdownSpy = spyOn(MarkdownPipe.prototype, 'transform'); fixture = TestBed.createComponent(ItemPageFieldComponent); comp = fixture.componentInstance; - comp.item = mockItemWithMetadataFieldAndValue(mockField, mockValue); + comp.item = mockItemWithMetadataFieldsAndValue(mockFields, mockValue); comp.fields = mockFields; comp.label = mockLabel; fixture.detectChanges(); @@ -126,17 +132,55 @@ describe('ItemPageFieldComponent', () => { expect(markdownSpy).toHaveBeenCalled(); }); }); + }); + + describe("test rendering of configured browse links", () => { + beforeEach(() => { + fixture.detectChanges(); + }); + it('should have a browse link', () => { + expect(fixture.debugElement.query(By.css('a.ds-browse-link')).nativeElement.innerHTML).toContain(mockValue); + }) + }); + + describe("test rendering of configured regex-based links", () => { + beforeEach(() => { + comp.urlRegex = '^test' + fixture.detectChanges(); + }); + beforeEach(waitForAsync(() => { + it('should have a rendered (non-browse) link since the value matches ^test', () => { + expect(fixture.debugElement.query(By.css('a.ds-simple-metadata-link')).nativeElement.innerHTML).toContain(mockValue); + }) + })) + }); + + describe("test skipping of configured links that do NOT match regex", () => { + beforeEach(() => { + comp.urlRegex = '^nope' + fixture.detectChanges(); + }); + beforeEach(waitForAsync(() => { + it('should NOT have a rendered (non-browse) link since the value matches ^test', () => { + expect(fixture.debugElement.query(By.css('a.ds-simple-metadata-link'))).toBeNull() + }) + })) + }); + + }); -export function mockItemWithMetadataFieldAndValue(field: string, value: string): Item { +export function mockItemWithMetadataFieldsAndValue(fields: string[], value: string): Item { const item = Object.assign(new Item(), { bundles: createSuccessfulRemoteDataObject$(createPaginatedList([])), metadata: new MetadataMap() }); - item.metadata[field] = [{ - language: 'en_US', - value: value - }] as MetadataValue[]; + fields.forEach((field: string) => { + item.metadata[field] = [{ + language: 'en_US', + value: value + }] as MetadataValue[]; + }) return item; } diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts index 681c5e16bc..0b6805d857 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts @@ -1,5 +1,9 @@ import { Component, Input } from '@angular/core'; import { Item } from '../../../../core/shared/item.model'; +import { map } from 'rxjs/operators'; +import { Observable } from 'rxjs'; +import { BrowseDefinition } from '../../../../core/shared/browse-definition.model'; +import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; /** * This component can be used to represent metadata on a simple item page. @@ -12,6 +16,9 @@ import { Item } from '../../../../core/shared/item.model'; }) export class ItemPageFieldComponent { + constructor(protected browseLinkDataService: BrowseLinkDataService) { + } + /** * The item to display metadata for */ @@ -38,4 +45,17 @@ export class ItemPageFieldComponent { */ separator = '
'; + /** + * Whether any valid HTTP(S) URL should be rendered as a link + */ + urlRegex?: string; + + /** + * Return browse definition that matches any field used in this component if it is configured as a browse + * link in dspace.cfg (webui.browse.link.) + */ + get browseDefinition(): Observable { + return this.browseLinkDataService.getBrowseLinkFor(this.fields).pipe( + map((def) => def)); + } } diff --git a/src/app/item-page/simple/field-components/specific-field/title/item-page-title-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/title/item-page-title-field.component.spec.ts index bc661e81c9..316e08e564 100644 --- a/src/app/item-page/simple/field-components/specific-field/title/item-page-title-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/title/item-page-title-field.component.spec.ts @@ -3,7 +3,7 @@ import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { MetadataValuesComponent } from '../../../../field-components/metadata-values/metadata-values.component'; -import { mockItemWithMetadataFieldAndValue } from '../item-page-field.component.spec'; +import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component.spec'; import { ItemPageTitleFieldComponent } from './item-page-title-field.component'; let comp: ItemPageTitleFieldComponent; @@ -31,7 +31,7 @@ describe('ItemPageTitleFieldComponent', () => { beforeEach(waitForAsync(() => { fixture = TestBed.createComponent(ItemPageTitleFieldComponent); comp = fixture.componentInstance; - comp.item = mockItemWithMetadataFieldAndValue(mockField, mockValue); + comp.item = mockItemWithMetadataFieldsAndValue([mockField], mockValue); fixture.detectChanges(); })); diff --git a/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts index 7c766252a3..1faa18de22 100644 --- a/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts @@ -2,7 +2,7 @@ import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; -import { mockItemWithMetadataFieldAndValue } from '../item-page-field.component.spec'; +import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component.spec'; import { ItemPageUriFieldComponent } from './item-page-uri-field.component'; import { MetadataUriValuesComponent } from '../../../../field-components/metadata-uri-values/metadata-uri-values.component'; import { environment } from '../../../../../../environments/environment'; @@ -37,7 +37,7 @@ describe('ItemPageUriFieldComponent', () => { beforeEach(waitForAsync(() => { fixture = TestBed.createComponent(ItemPageUriFieldComponent); comp = fixture.componentInstance; - comp.item = mockItemWithMetadataFieldAndValue(mockField, mockValue); + comp.item = mockItemWithMetadataFieldsAndValue([mockField], mockValue); comp.fields = [mockField]; comp.label = mockLabel; fixture.detectChanges(); diff --git a/src/app/item-page/simple/item-types/shared/item.component.ts b/src/app/item-page/simple/item-types/shared/item.component.ts index 93e6a0b346..59681705ef 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.ts @@ -4,9 +4,11 @@ import { Item } from '../../../../core/shared/item.model'; import { getItemPageRoute } from '../../../item-page-routing-paths'; import { RouteService } from '../../../../core/services/route.service'; import { Observable } from 'rxjs'; +import { BrowseService } from '../../../../../app/core/browse/browse.service'; import { getDSpaceQuery, isIiifEnabled, isIiifSearchEnabled } from './item-iiif-utils'; import { filter, map, take } from 'rxjs/operators'; import { Router } from '@angular/router'; +import { BrowseDefinition } from '../../../../core/shared/browse-definition.model'; @Component({ selector: 'ds-item', @@ -49,10 +51,14 @@ export class ItemComponent implements OnInit { */ iiifQuery$: Observable; + browseDefinitions: BrowseDefinition[]; + browseDefinitions$: Observable; + mediaViewer; constructor(protected routeService: RouteService, - protected router: Router) { + protected router: Router, + protected browseService: BrowseService) { this.mediaViewer = environment.mediaViewer; } @@ -84,5 +90,9 @@ export class ItemComponent implements OnInit { if (this.iiifSearchEnabled) { this.iiifQuery$ = getDSpaceQuery(this.object, this.routeService); } + // get browse definitions + this.browseDefinitions$ = this.browseService.getBrowseDefinitions().pipe( + map((data) => data.payload.page as BrowseDefinition[]) + ); } } diff --git a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.ts b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.ts index a1f4cebd77..016ac26d0c 100644 --- a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.ts +++ b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.ts @@ -17,6 +17,7 @@ import { Item } from '../../../../core/shared/item.model'; import { ItemDataService } from '../../../../core/data/item-data.service'; import { WorkspaceItem } from '../../../../core/submission/models/workspaceitem.model'; import { RouteService } from '../../../../core/services/route.service'; +import { BrowseService } from '../../../../core/browse/browse.service'; @Component({ selector: 'ds-versioned-item', @@ -36,8 +37,9 @@ export class VersionedItemComponent extends ItemComponent { private searchService: SearchService, private itemService: ItemDataService, protected routeService: RouteService, + protected browseService: BrowseService, ) { - super(routeService, router); + super(routeService, router, browseService); } /** diff --git a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.html b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.html new file mode 100644 index 0000000000..24f500ac60 --- /dev/null +++ b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.html @@ -0,0 +1,8 @@ + diff --git a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts new file mode 100644 index 0000000000..ec3551ee09 --- /dev/null +++ b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts @@ -0,0 +1,36 @@ +import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; +import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; +import { BrowseLinkMetadataListElementComponent } from './browse-link-metadata-list-element.component'; +import { MetadatumRepresentation } from '../../../../core/shared/metadata-representation/metadatum/metadatum-representation.model'; + +const mockMetadataRepresentation = Object.assign(new MetadatumRepresentation('type'), { + key: 'dc.contributor.author', + value: 'Test Author' +}); + +describe('BrowseLinkMetadataListElementComponent', () => { + let comp: BrowseLinkMetadataListElementComponent; + let fixture: ComponentFixture; + + beforeEach(waitForAsync(() => { + TestBed.configureTestingModule({ + imports: [], + declarations: [BrowseLinkMetadataListElementComponent], + schemas: [NO_ERRORS_SCHEMA] + }).overrideComponent(BrowseLinkMetadataListElementComponent, { + set: { changeDetection: ChangeDetectionStrategy.Default } + }).compileComponents(); + })); + + beforeEach(waitForAsync(() => { + fixture = TestBed.createComponent(BrowseLinkMetadataListElementComponent); + comp = fixture.componentInstance; + comp.metadataRepresentation = mockMetadataRepresentation; + fixture.detectChanges(); + })); + + it('should contain the value as a browse link', () => { + expect(fixture.debugElement.nativeElement.textContent).toContain(mockMetadataRepresentation.value); + }); + +}); diff --git a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.ts b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.ts new file mode 100644 index 0000000000..616ef09ae3 --- /dev/null +++ b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.ts @@ -0,0 +1,18 @@ +import { MetadataRepresentationType } from '../../../../core/shared/metadata-representation/metadata-representation.model'; +import { Component } from '@angular/core'; +import { MetadataRepresentationListElementComponent } from '../metadata-representation-list-element.component'; +import { metadataRepresentationComponent } from '../../../metadata-representation/metadata-representation.decorator'; +//@metadataRepresentationComponent('Publication', MetadataRepresentationType.PlainText) +// For now, authority controlled fields are rendered the same way as plain text fields +//@metadataRepresentationComponent('Publication', MetadataRepresentationType.AuthorityControlled) +@metadataRepresentationComponent('Publication', MetadataRepresentationType.BrowseLink) +@Component({ + selector: 'ds-browse-link-metadata-list-element', + templateUrl: './browse-link-metadata-list-element.component.html' +}) +/** + * A component for displaying MetadataRepresentation objects in the form of plain text + * It will simply use the value retrieved from MetadataRepresentation.getValue() to display as plain text + */ +export class BrowseLinkMetadataListElementComponent extends MetadataRepresentationListElementComponent { +} diff --git a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts index 2e14485fbb..a51232d1f6 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts @@ -13,4 +13,11 @@ export class MetadataRepresentationListElementComponent { * The metadata representation of this component */ metadataRepresentation: MetadataRepresentation; + + isLink(): boolean { + // Match any http:// or https:// + const linkPattern: RegExp = /^https?\/\//; + return linkPattern.test(this.metadataRepresentation.getValue()); + } + } diff --git a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.html b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.html index 31b670b1a3..93dd993ce4 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.html +++ b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.html @@ -1,3 +1,16 @@
- {{metadataRepresentation.getValue()}} + + + {{metadataRepresentation.getValue()}} + + + {{metadataRepresentation.getValue()}} + + {{metadataRepresentation.getValue()}} + + {{metadataRepresentation.getValue()}} +
diff --git a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.spec.ts b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.spec.ts index af09d3c204..f05f7bb566 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.spec.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.spec.ts @@ -2,8 +2,12 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; import { PlainTextMetadataListElementComponent } from './plain-text-metadata-list-element.component'; import { MetadatumRepresentation } from '../../../../core/shared/metadata-representation/metadatum/metadatum-representation.model'; +import { By } from '@angular/platform-browser'; +import { mockData } from '../../../testing/browse-link-data-service.stub'; -const mockMetadataRepresentation = Object.assign(new MetadatumRepresentation('type'), { +// Render the mock representation with the default mock author browse definition so it is also rendered as a link +// without affecting other tests +const mockMetadataRepresentation = Object.assign(new MetadatumRepresentation('type', mockData[1]), { key: 'dc.contributor.author', value: 'Test Author' }); @@ -33,4 +37,8 @@ describe('PlainTextMetadataListElementComponent', () => { expect(fixture.debugElement.nativeElement.textContent).toContain(mockMetadataRepresentation.value); }); + it('should contain the browse link as plain text', () => { + expect(fixture.debugElement.query(By.css('a.ds-browse-link')).nativeElement.innerHTML).toContain(mockMetadataRepresentation.value); + }); + }); diff --git a/src/app/shared/shared.module.ts b/src/app/shared/shared.module.ts index dfe8768014..bd46380452 100644 --- a/src/app/shared/shared.module.ts +++ b/src/app/shared/shared.module.ts @@ -84,6 +84,8 @@ import { LangSwitchComponent } from './lang-switch/lang-switch.component'; import { PlainTextMetadataListElementComponent } from './object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component'; +import { BrowseLinkMetadataListElementComponent } + from './object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component'; import { ItemMetadataListElementComponent } from './object-list/metadata-representation-list-element/item/item-metadata-list-element.component'; @@ -383,6 +385,7 @@ const ENTRY_COMPONENTS = [ EditItemSelectorComponent, ThemedEditItemSelectorComponent, PlainTextMetadataListElementComponent, + BrowseLinkMetadataListElementComponent, ItemMetadataListElementComponent, MetadataRepresentationListElementComponent, ItemMetadataRepresentationListElementComponent, diff --git a/src/app/shared/testing/browse-link-data-service.stub.ts b/src/app/shared/testing/browse-link-data-service.stub.ts new file mode 100644 index 0000000000..c3eb806791 --- /dev/null +++ b/src/app/shared/testing/browse-link-data-service.stub.ts @@ -0,0 +1,77 @@ +import { EMPTY, Observable, of as observableOf } from 'rxjs'; +import { RemoteData } from '../../core/data/remote-data'; +import { buildPaginatedList, PaginatedList } from '../../core/data/paginated-list.model'; +import { BrowseDefinition } from '../../core/shared/browse-definition.model'; +import { + getPaginatedListPayload, + getRemoteDataPayload +} from '../../core/shared/operators'; +import { BrowseService } from '../../core/browse/browse.service'; +import { distinctUntilChanged, map, startWith } from 'rxjs/operators'; +import { isEmpty, isNotEmpty } from '../empty.util'; +import { createSuccessfulRemoteDataObject } from '../remote-data.utils'; +import { PageInfo } from '../../core/shared/page-info.model'; + +// This data is in post-serialized form (metadata -> metadataKeys) +export const mockData: BrowseDefinition[] = [ + Object.assign(new BrowseDefinition, { + "id" : "dateissued", + "metadataBrowse" : false, + "dataType" : "date", + "sortOptions" : EMPTY, + "order" : "ASC", + "type" : "browse", + "metadataKeys" : [ "dc.date.issued" ], + "_links" : EMPTY + }), + Object.assign(new BrowseDefinition, { + "id" : "author", + "metadataBrowse" : true, + "dataType" : "text", + "sortOptions" : EMPTY, + "order" : "ASC", + "type" : "browse", + "metadataKeys" : [ "dc.contributor.*", "dc.creator" ], + "_links" : EMPTY + }) +]; + + +export const browseLinkDataServiceStub: any = { + /** + * Get all BrowseDefinitions + */ + getBrowseLinks(): Observable>> { + return observableOf(createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), mockData))); + }, + /** + * Get the browse URL by providing a list of metadata keys + * @param metadatumKey + * @param linkPath + */ + getBrowseLinkFor(metadataKeys: string[]): Observable { + let searchKeyArray: string[] = []; + metadataKeys.forEach((metadataKey) => { + searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); + }) + return this.getBrowseLinks().pipe( + getRemoteDataPayload(), + getPaginatedListPayload(), + map((browseDefinitions: BrowseDefinition[]) => browseDefinitions + .find((def: BrowseDefinition) => { + const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); + return isNotEmpty(matchingKeys); + }) + ), + map((def: BrowseDefinition) => { + if (isEmpty(def) || isEmpty(def.id)) { + //throw new Error(`A browse definition for field ${metadataKey} isn't configured`); + } else { + return def; + } + }), + startWith(undefined), + distinctUntilChanged() + ); + } +} From 3c48df6fe5fcf65284d792e8788bec45d307bab6 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 23 Nov 2022 13:36:55 +1300 Subject: [PATCH 02/20] [TLC-380] Lint fixes --- .../core/browse/browse-link-data.service.ts | 2 +- src/app/core/browse/browse.service.ts | 3 +- .../metadata-values.component.ts | 2 +- .../item-page-field.component.spec.ts | 24 ++++++------- ...a-representation-list-element.component.ts | 2 +- .../testing/browse-link-data-service.stub.ts | 36 +++++++++---------- 6 files changed, 34 insertions(+), 35 deletions(-) diff --git a/src/app/core/browse/browse-link-data.service.ts b/src/app/core/browse/browse-link-data.service.ts index 0f4820bb19..d1790101e3 100644 --- a/src/app/core/browse/browse-link-data.service.ts +++ b/src/app/core/browse/browse-link-data.service.ts @@ -55,7 +55,7 @@ export class BrowseLinkDataService extends IdentifiableDataService { searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); - }) + }); return this.getBrowseLinks().pipe( getRemoteDataPayload(), getPaginatedListPayload(), diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index fd8e68430d..e17cc5e830 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -229,7 +229,6 @@ export class BrowseService { * @param linkPath */ getBrowseURLFor(metadataKey: string, linkPath: string): Observable { - console.log("Looking for " + metadataKey + " in link path " + linkPath); const searchKeyArray = BrowseService.toSearchKeyArray(metadataKey); return this.getBrowseDefinitions().pipe( getRemoteDataPayload(), @@ -261,7 +260,7 @@ export class BrowseService { let searchKeyArray: string[] = []; metadataKeys.forEach((metadataKey) => { searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); - }) + }); return this.getBrowseDefinitions().pipe( getRemoteDataPayload(), getPaginatedListPayload(), diff --git a/src/app/item-page/field-components/metadata-values/metadata-values.component.ts b/src/app/item-page/field-components/metadata-values/metadata-values.component.ts index 5b7e937d95..2ef160e74e 100644 --- a/src/app/item-page/field-components/metadata-values/metadata-values.component.ts +++ b/src/app/item-page/field-components/metadata-values/metadata-values.component.ts @@ -71,7 +71,7 @@ export class MetadataValuesComponent implements OnChanges { */ hasLink(value): boolean { if (hasValue(this.urlRegex)) { - const pattern: RegExp = new RegExp(this.urlRegex); + const pattern = new RegExp(this.urlRegex); return pattern.test(value.value); } return false; diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts index 48d5a0f29e..5834a026eb 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts @@ -135,37 +135,37 @@ describe('ItemPageFieldComponent', () => { }); - describe("test rendering of configured browse links", () => { + describe('test rendering of configured browse links', () => { beforeEach(() => { fixture.detectChanges(); }); it('should have a browse link', () => { expect(fixture.debugElement.query(By.css('a.ds-browse-link')).nativeElement.innerHTML).toContain(mockValue); - }) + }); }); - describe("test rendering of configured regex-based links", () => { + describe('test rendering of configured regex-based links', () => { beforeEach(() => { - comp.urlRegex = '^test' + comp.urlRegex = '^test'; fixture.detectChanges(); }); beforeEach(waitForAsync(() => { it('should have a rendered (non-browse) link since the value matches ^test', () => { expect(fixture.debugElement.query(By.css('a.ds-simple-metadata-link')).nativeElement.innerHTML).toContain(mockValue); - }) - })) + }); + })); }); - describe("test skipping of configured links that do NOT match regex", () => { + describe('test skipping of configured links that do NOT match regex', () => { beforeEach(() => { - comp.urlRegex = '^nope' + comp.urlRegex = '^nope'; fixture.detectChanges(); }); beforeEach(waitForAsync(() => { it('should NOT have a rendered (non-browse) link since the value matches ^test', () => { - expect(fixture.debugElement.query(By.css('a.ds-simple-metadata-link'))).toBeNull() - }) - })) + expect(fixture.debugElement.query(By.css('a.ds-simple-metadata-link'))).toBeNull(); + }); + })); }); @@ -181,6 +181,6 @@ export function mockItemWithMetadataFieldsAndValue(fields: string[], value: stri language: 'en_US', value: value }] as MetadataValue[]; - }) + }); return item; } diff --git a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts index a51232d1f6..2b9fbab4ee 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts @@ -16,7 +16,7 @@ export class MetadataRepresentationListElementComponent { isLink(): boolean { // Match any http:// or https:// - const linkPattern: RegExp = /^https?\/\//; + const linkPattern = /^https?\/\//; return linkPattern.test(this.metadataRepresentation.getValue()); } diff --git a/src/app/shared/testing/browse-link-data-service.stub.ts b/src/app/shared/testing/browse-link-data-service.stub.ts index c3eb806791..4deff2bf05 100644 --- a/src/app/shared/testing/browse-link-data-service.stub.ts +++ b/src/app/shared/testing/browse-link-data-service.stub.ts @@ -15,24 +15,24 @@ import { PageInfo } from '../../core/shared/page-info.model'; // This data is in post-serialized form (metadata -> metadataKeys) export const mockData: BrowseDefinition[] = [ Object.assign(new BrowseDefinition, { - "id" : "dateissued", - "metadataBrowse" : false, - "dataType" : "date", - "sortOptions" : EMPTY, - "order" : "ASC", - "type" : "browse", - "metadataKeys" : [ "dc.date.issued" ], - "_links" : EMPTY + 'id' : 'dateissued', + 'metadataBrowse' : false, + 'dataType' : 'date', + 'sortOptions' : EMPTY, + 'order' : 'ASC', + 'type' : 'browse', + 'metadataKeys' : [ 'dc.date.issued' ], + '_links' : EMPTY }), Object.assign(new BrowseDefinition, { - "id" : "author", - "metadataBrowse" : true, - "dataType" : "text", - "sortOptions" : EMPTY, - "order" : "ASC", - "type" : "browse", - "metadataKeys" : [ "dc.contributor.*", "dc.creator" ], - "_links" : EMPTY + 'id' : 'author', + 'metadataBrowse' : true, + 'dataType' : 'text', + 'sortOptions' : EMPTY, + 'order' : 'ASC', + 'type' : 'browse', + 'metadataKeys' : [ 'dc.contributor.*', 'dc.creator' ], + '_links' : EMPTY }) ]; @@ -53,7 +53,7 @@ export const browseLinkDataServiceStub: any = { let searchKeyArray: string[] = []; metadataKeys.forEach((metadataKey) => { searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); - }) + }); return this.getBrowseLinks().pipe( getRemoteDataPayload(), getPaginatedListPayload(), @@ -74,4 +74,4 @@ export const browseLinkDataServiceStub: any = { distinctUntilChanged() ); } -} +}; From 4b5b438968a73decf8cbd694dbccfbe4078e2ba6 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 23 Nov 2022 13:50:59 +1300 Subject: [PATCH 03/20] [TLC-380] Lint fixes --- .../specific-field/item-page-field.component.spec.ts | 2 +- src/app/item-page/simple/item-types/shared/item.component.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts index 5834a026eb..7d4d9cb508 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts @@ -1,6 +1,6 @@ import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; -import { ComponentFixture, inject, TestBed, waitForAsync } from '@angular/core/testing'; +import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { Item } from '../../../../core/shared/item.model'; import { TranslateLoaderMock } from '../../../../shared/mocks/translate-loader.mock'; import { ItemPageFieldComponent } from './item-page-field.component'; diff --git a/src/app/item-page/simple/item-types/shared/item.component.ts b/src/app/item-page/simple/item-types/shared/item.component.ts index 59681705ef..8e9cd389a7 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.ts @@ -4,7 +4,7 @@ import { Item } from '../../../../core/shared/item.model'; import { getItemPageRoute } from '../../../item-page-routing-paths'; import { RouteService } from '../../../../core/services/route.service'; import { Observable } from 'rxjs'; -import { BrowseService } from '../../../../../app/core/browse/browse.service'; +import { BrowseService } from '../../../../core/browse/browse.service'; import { getDSpaceQuery, isIiifEnabled, isIiifSearchEnabled } from './item-iiif-utils'; import { filter, map, take } from 'rxjs/operators'; import { Router } from '@angular/router'; From e74b77840c895fcd44c5eff6f8f8b267c07d703d Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 23 Nov 2022 14:42:13 +1300 Subject: [PATCH 04/20] [TLC-380] Unit test (provider injection) fixes (resolved conflict jan 16) --- .../abstract/item-page-abstract-field.component.spec.ts | 3 +++ .../author/item-page-author-field.component.spec.ts | 3 +++ .../date/item-page-date-field.component.spec.ts | 3 +++ .../generic/generic-item-page-field.component.spec.ts | 3 +++ .../uri/item-page-uri-field.component.spec.ts | 3 +++ .../item-types/publication/publication.component.spec.ts | 5 ++++- .../simple/item-types/shared/item.component.spec.ts | 5 ++++- .../untyped-item/untyped-item.component.spec.ts | 5 ++++- .../browse-link-metadata-list-element.component.spec.ts | 8 +++++--- 9 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts index 53f0522f39..c98ea09079 100644 --- a/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts @@ -7,6 +7,8 @@ import { SharedModule } from '../../../../../shared/shared.module'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; import { By } from '@angular/platform-browser'; +import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; +import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; let comp: ItemPageAbstractFieldComponent; let fixture: ComponentFixture; @@ -25,6 +27,7 @@ describe('ItemPageAbstractFieldComponent', () => { ], providers: [ { provide: APP_CONFIG, useValue: environment }, + { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } ], declarations: [ItemPageAbstractFieldComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts index 5536d54632..d85754438c 100644 --- a/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts @@ -7,6 +7,8 @@ import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component import { ItemPageAuthorFieldComponent } from './item-page-author-field.component'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; +import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; +import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; let comp: ItemPageAuthorFieldComponent; let fixture: ComponentFixture; @@ -25,6 +27,7 @@ describe('ItemPageAuthorFieldComponent', () => { })], providers: [ { provide: APP_CONFIG, useValue: environment }, + { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } ], declarations: [ItemPageAuthorFieldComponent, MetadataValuesComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts index 9b60cdd3d6..f4b491dd22 100644 --- a/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts @@ -7,6 +7,8 @@ import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component import { ItemPageDateFieldComponent } from './item-page-date-field.component'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; +import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; +import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; let comp: ItemPageDateFieldComponent; let fixture: ComponentFixture; @@ -25,6 +27,7 @@ describe('ItemPageDateFieldComponent', () => { })], providers: [ { provide: APP_CONFIG, useValue: environment }, + { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } ], declarations: [ItemPageDateFieldComponent, MetadataValuesComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts index 053976b374..69fc84957d 100644 --- a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts @@ -7,6 +7,8 @@ import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component import { GenericItemPageFieldComponent } from './generic-item-page-field.component'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; +import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; +import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; let comp: GenericItemPageFieldComponent; let fixture: ComponentFixture; @@ -27,6 +29,7 @@ describe('GenericItemPageFieldComponent', () => { })], providers: [ { provide: APP_CONFIG, useValue: environment }, + { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } ], declarations: [GenericItemPageFieldComponent, MetadataValuesComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts index 1faa18de22..50d697fb70 100644 --- a/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts @@ -7,6 +7,8 @@ import { ItemPageUriFieldComponent } from './item-page-uri-field.component'; import { MetadataUriValuesComponent } from '../../../../field-components/metadata-uri-values/metadata-uri-values.component'; import { environment } from '../../../../../../environments/environment'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; +import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; +import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; let comp: ItemPageUriFieldComponent; let fixture: ComponentFixture; @@ -26,6 +28,7 @@ describe('ItemPageUriFieldComponent', () => { })], providers: [ { provide: APP_CONFIG, useValue: environment }, + { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } ], declarations: [ItemPageUriFieldComponent, MetadataUriValuesComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/item-types/publication/publication.component.spec.ts b/src/app/item-page/simple/item-types/publication/publication.component.spec.ts index 3c2ff4f844..d1ea4fbe56 100644 --- a/src/app/item-page/simple/item-types/publication/publication.component.spec.ts +++ b/src/app/item-page/simple/item-types/publication/publication.component.spec.ts @@ -36,6 +36,8 @@ import { VersionDataService } from '../../../../core/data/version-data.service'; import { RouterTestingModule } from '@angular/router/testing'; import { WorkspaceitemDataService } from '../../../../core/submission/workspaceitem-data.service'; import { SearchService } from '../../../../core/shared/search/search.service'; +import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; +import { browseLinkDataServiceStub } from '../../../../shared/testing/browse-link-data-service.stub'; const noMetadata = new MetadataMap(); @@ -87,7 +89,8 @@ describe('PublicationComponent', () => { { provide: BitstreamDataService, useValue: mockBitstreamDataService }, { provide: WorkspaceitemDataService, useValue: {} }, { provide: SearchService, useValue: {} }, - { provide: RouteService, useValue: mockRouteService } + { provide: RouteService, useValue: mockRouteService }, + { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/item-types/shared/item.component.spec.ts b/src/app/item-page/simple/item-types/shared/item.component.spec.ts index cb91a31b06..40666a4d33 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.spec.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.spec.ts @@ -38,6 +38,8 @@ import { VersionHistoryDataService } from '../../../../core/data/version-history import { RouterTestingModule } from '@angular/router/testing'; import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; import { ResearcherProfileDataService } from '../../../../core/profile/researcher-profile-data.service'; +import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; +import { browseLinkDataServiceStub } from '../../../../shared/testing/browse-link-data-service.stub'; import { buildPaginatedList } from '../../../../core/data/paginated-list.model'; import { PageInfo } from '../../../../core/shared/page-info.model'; @@ -125,7 +127,8 @@ export function getItemPageFieldsTest(mockItem: Item, component) { { provide: SearchService, useValue: {} }, { provide: RouteService, useValue: mockRouteService }, { provide: AuthorizationDataService, useValue: authorizationService }, - { provide: ResearcherProfileDataService, useValue: {} } + { provide: ResearcherProfileDataService, useValue: {} }, + { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts b/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts index 3581694a5e..4b8d581d75 100644 --- a/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts +++ b/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts @@ -36,6 +36,8 @@ import { VersionDataService } from '../../../../core/data/version-data.service'; import { RouterTestingModule } from '@angular/router/testing'; import { WorkspaceitemDataService } from '../../../../core/submission/workspaceitem-data.service'; import { SearchService } from '../../../../core/shared/search/search.service'; +import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; +import { browseLinkDataServiceStub } from '../../../../shared/testing/browse-link-data-service.stub'; import { ItemVersionsSharedService } from '../../../versions/item-versions-shared.service'; const noMetadata = new MetadataMap(); @@ -90,7 +92,8 @@ describe('UntypedItemComponent', () => { { provide: SearchService, useValue: {} }, { provide: ItemDataService, useValue: {} }, { provide: ItemVersionsSharedService, useValue: {} }, - { provide: RouteService, useValue: mockRouteService } + { provide: RouteService, useValue: mockRouteService }, + { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(UntypedItemComponent, { diff --git a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts index ec3551ee09..88a262c389 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts @@ -29,8 +29,10 @@ describe('BrowseLinkMetadataListElementComponent', () => { fixture.detectChanges(); })); - it('should contain the value as a browse link', () => { - expect(fixture.debugElement.nativeElement.textContent).toContain(mockMetadataRepresentation.value); - }); + waitForAsync(() => { + it('should contain the value as a browse link', () => { + expect(fixture.debugElement.nativeElement.textContent).toContain(mockMetadataRepresentation.value); + }); + }) }); From bc73032e458f96de170aa21e42750850203a62f1 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 23 Nov 2022 14:51:23 +1300 Subject: [PATCH 05/20] [TLC-380] lint fix --- .../browse-link-metadata-list-element.component.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts index 88a262c389..5c9c436537 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts @@ -33,6 +33,6 @@ describe('BrowseLinkMetadataListElementComponent', () => { it('should contain the value as a browse link', () => { expect(fixture.debugElement.nativeElement.textContent).toContain(mockMetadataRepresentation.value); }); - }) + }); }); From b80545642f171ef7d8f364a6a76b73b05f8a0bb1 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 23 Nov 2022 15:12:17 +1300 Subject: [PATCH 06/20] [TLC-380] further browse link unit test fixes (waitForAsync) --- .../specific-field/item-page-field.component.spec.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts index 7d4d9cb508..2a4e60b393 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts @@ -139,8 +139,10 @@ describe('ItemPageFieldComponent', () => { beforeEach(() => { fixture.detectChanges(); }); - it('should have a browse link', () => { - expect(fixture.debugElement.query(By.css('a.ds-browse-link')).nativeElement.innerHTML).toContain(mockValue); + waitForAsync(() => { + it('should have a browse link', () => { + expect(fixture.debugElement.query(By.css('a.ds-browse-link')).nativeElement.innerHTML).toContain(mockValue); + }); }); }); @@ -149,11 +151,11 @@ describe('ItemPageFieldComponent', () => { comp.urlRegex = '^test'; fixture.detectChanges(); }); - beforeEach(waitForAsync(() => { + waitForAsync(() => { it('should have a rendered (non-browse) link since the value matches ^test', () => { expect(fixture.debugElement.query(By.css('a.ds-simple-metadata-link')).nativeElement.innerHTML).toContain(mockValue); }); - })); + }); }); describe('test skipping of configured links that do NOT match regex', () => { From 5ad635fb5a8d1330d8c3455bd77601186b98a156 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Wed, 25 Jan 2023 15:43:20 +1300 Subject: [PATCH 07/20] [TLC-380] Refactor browse links to use new /browses endpoint --- .../browse/browse-definition-data.service.ts | 95 ++++++++++++++++++- .../core/browse/browse-link-data.service.ts | 1 + 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index 32c3b44e14..3ffc821b79 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -13,6 +13,12 @@ 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 { getPaginatedListPayload, getRemoteDataPayload } from '../shared/operators'; +import { RequestParam } from '../cache/models/request-param.model'; +import { SearchData, SearchDataImpl } from '../data/base/search-data'; +import { BrowseService } from './browse.service'; +import { distinctUntilChanged, map, startWith } from 'rxjs/operators'; +import { isEmpty, isNotEmpty } from '../../shared/empty.util'; /** * Data service responsible for retrieving browse definitions from the REST server @@ -21,8 +27,9 @@ import { dataService } from '../data/base/data-service.decorator'; providedIn: 'root', }) @dataService(BROWSE_DEFINITION) -export class BrowseDefinitionDataService extends IdentifiableDataService implements FindAllData { +export class BrowseDefinitionDataService extends IdentifiableDataService implements FindAllData, SearchData { private findAllData: FindAllDataImpl; + private searchData: SearchDataImpl; constructor( protected requestService: RequestService, @@ -52,5 +59,91 @@ export class BrowseDefinitionDataService extends IdentifiableDataService[]): Observable>> { return this.findAllData.findAll(options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } + + /** + * Make a new FindListRequest with given search method + * + * @param searchMethod The search method for the object + * @param options The [[FindListOptions]] 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 response from the server + */ + public searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig[]): Observable>> { + return this.searchData.searchBy(searchMethod, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); + } + + /** + * Create the HREF for a specific object's search method with given options object + * + * @param searchMethod The search method for the object + * @param options The [[FindListOptions]] object + * @return {Observable} + * Return an observable that emits created HREF + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved + */ + public getSearchByHref(searchMethod: string, options?: FindListOptions, ...linksToFollow: FollowLinkConfig[]): Observable { + return this.searchData.getSearchByHref(searchMethod, options, ...linksToFollow); + } + + findByField( + field: string, + useCachedVersionIfAvailable = true, + reRequestOnStale = true, + ...linksToFollow: FollowLinkConfig[] + ): Observable> { + const searchParams = []; + searchParams.push(new RequestParam('field', field)); + + const hrefObs = this.getSearchByHref( + 'byField', + { searchParams }, + ...linksToFollow + ); + + return this.findByHref( + hrefObs, + useCachedVersionIfAvailable, + reRequestOnStale, + ...linksToFollow, + ); + } + + /** + * Get the browse URL by providing a list of metadata keys + * @param metadatumKey + * @param linkPath + */ + findByFields(metadataKeys: string[]): Observable { + let searchKeyArray: string[] = []; + metadataKeys.forEach((metadataKey) => { + searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); + }); + return this.findAll().pipe( + getRemoteDataPayload(), + getPaginatedListPayload(), + map((browseDefinitions: BrowseDefinition[]) => browseDefinitions + .find((def: BrowseDefinition) => { + const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); + return isNotEmpty(matchingKeys); + }) + ), + map((def: BrowseDefinition) => { + if (isEmpty(def) || isEmpty(def.id)) { + //throw new Error(`A browse definition for field ${metadataKey} isn't configured`); + } else { + return def; + } + }), + startWith(undefined), + distinctUntilChanged() + ); + } + } diff --git a/src/app/core/browse/browse-link-data.service.ts b/src/app/core/browse/browse-link-data.service.ts index d1790101e3..c7dd31e4d3 100644 --- a/src/app/core/browse/browse-link-data.service.ts +++ b/src/app/core/browse/browse-link-data.service.ts @@ -36,6 +36,7 @@ export class BrowseLinkDataService extends IdentifiableDataService Date: Thu, 26 Jan 2023 12:35:26 +1300 Subject: [PATCH 08/20] [TLC-249] Fix circular dependency in browse services --- .../browse/browse-definition-data.service.ts | 16 ++++++++++++++-- src/app/core/browse/browse.service.ts | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index 3ffc821b79..e4d3718806 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -16,7 +16,6 @@ import { dataService } from '../data/base/data-service.decorator'; import { getPaginatedListPayload, getRemoteDataPayload } from '../shared/operators'; import { RequestParam } from '../cache/models/request-param.model'; import { SearchData, SearchDataImpl } from '../data/base/search-data'; -import { BrowseService } from './browse.service'; import { distinctUntilChanged, map, startWith } from 'rxjs/operators'; import { isEmpty, isNotEmpty } from '../../shared/empty.util'; @@ -31,6 +30,19 @@ export class BrowseDefinitionDataService extends IdentifiableDataService; private searchData: SearchDataImpl; + public static toSearchKeyArray(metadataKey: string): string[] { + const keyParts = metadataKey.split('.'); + const searchFor = []; + searchFor.push('*'); + for (let i = 0; i < keyParts.length - 1; i++) { + const prevParts = keyParts.slice(0, i + 1); + const nextPart = [...prevParts, '*'].join('.'); + searchFor.push(nextPart); + } + searchFor.push(metadataKey); + return searchFor; + } + constructor( protected requestService: RequestService, protected rdbService: RemoteDataBuildService, @@ -122,7 +134,7 @@ export class BrowseDefinitionDataService extends IdentifiableDataService { let searchKeyArray: string[] = []; metadataKeys.forEach((metadataKey) => { - searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); + searchKeyArray = searchKeyArray.concat(BrowseDefinitionDataService.toSearchKeyArray(metadataKey)); }); return this.findAll().pipe( getRemoteDataPayload(), diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index e17cc5e830..280e8e6f5a 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -19,9 +19,9 @@ import { } from '../shared/operators'; import { URLCombiner } from '../url-combiner/url-combiner'; import { BrowseEntrySearchOptions } from './browse-entry-search-options.model'; -import { BrowseDefinitionDataService } from './browse-definition-data.service'; import { HrefOnlyDataService } from '../data/href-only-data.service'; import { followLink, FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; +import { BrowseDefinitionDataService } from './browse-definition-data.service'; export const BROWSE_LINKS_TO_FOLLOW: FollowLinkConfig[] = [ From bdda84f884c08de07d7c38a0cf7fc145fd4892ce Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Thu, 26 Jan 2023 14:47:35 +1300 Subject: [PATCH 09/20] [TLC-249] Larger refactor to field, item components for browse links --- .../browse/browse-definition-data.service.ts | 66 +++++++++------ .../browse/browse-link-data.service.spec.ts | 28 ------- .../core/browse/browse-link-data.service.ts | 82 ------------------- src/app/core/core.module.ts | 2 - .../journal/journal.component.spec.ts | 10 ++- ...item-page-abstract-field.component.spec.ts | 6 +- .../item-page-author-field.component.spec.ts | 6 +- .../item-page-date-field.component.spec.ts | 6 +- .../generic-item-page-field.component.spec.ts | 6 +- .../item-page-field.component.spec.ts | 6 +- .../item-page-field.component.ts | 11 ++- .../uri/item-page-uri-field.component.spec.ts | 6 +- .../publication/publication.component.spec.ts | 11 ++- .../item-types/shared/item.component.spec.ts | 28 +++++-- .../untyped-item.component.spec.ts | 11 ++- .../versioned-item.component.spec.ts | 7 +- ...xt-metadata-list-element.component.spec.ts | 2 +- ...=> browse-definition-data-service.stub.ts} | 22 ++++- 18 files changed, 139 insertions(+), 177 deletions(-) delete mode 100644 src/app/core/browse/browse-link-data.service.spec.ts delete mode 100644 src/app/core/browse/browse-link-data.service.ts rename src/app/shared/testing/{browse-link-data-service.stub.ts => browse-definition-data-service.stub.ts} (77%) diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index e4d3718806..aa3e095608 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -50,7 +50,7 @@ export class BrowseDefinitionDataService extends IdentifiableDataService[] + ): Observable> { + const searchParams = []; + + const hrefObs = this.getSearchByHref( + 'allLinked', + { searchParams }, + ...linksToFollow + ); + + return this.findByHref( + hrefObs, + useCachedVersionIfAvailable, + reRequestOnStale, + ...linksToFollow, + ); + } + /** * Get the browse URL by providing a list of metadata keys * @param metadatumKey * @param linkPath */ - findByFields(metadataKeys: string[]): Observable { - let searchKeyArray: string[] = []; - metadataKeys.forEach((metadataKey) => { - searchKeyArray = searchKeyArray.concat(BrowseDefinitionDataService.toSearchKeyArray(metadataKey)); - }); - return this.findAll().pipe( - getRemoteDataPayload(), - getPaginatedListPayload(), - map((browseDefinitions: BrowseDefinition[]) => browseDefinitions - .find((def: BrowseDefinition) => { - const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); - return isNotEmpty(matchingKeys); - }) - ), - map((def: BrowseDefinition) => { - if (isEmpty(def) || isEmpty(def.id)) { - //throw new Error(`A browse definition for field ${metadataKey} isn't configured`); - } else { - return def; - } - }), - startWith(undefined), - distinctUntilChanged() + findByFields( + fields: string[], + useCachedVersionIfAvailable = true, + reRequestOnStale = true, + ...linksToFollow: FollowLinkConfig[] + ): Observable> { + const searchParams = []; + searchParams.push(new RequestParam('fields', fields)); + + const hrefObs = this.getSearchByHref( + 'byFields', + { searchParams }, + ...linksToFollow + ); + + return this.findByHref( + hrefObs, + useCachedVersionIfAvailable, + reRequestOnStale, + ...linksToFollow, ); } diff --git a/src/app/core/browse/browse-link-data.service.spec.ts b/src/app/core/browse/browse-link-data.service.spec.ts deleted file mode 100644 index bb34c24900..0000000000 --- a/src/app/core/browse/browse-link-data.service.spec.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { followLink } from '../../shared/utils/follow-link-config.model'; -import { EMPTY } from 'rxjs'; -import { FindListOptions } from '../data/find-list-options.model'; -import { BrowseLinkDataService } from './browse-link-data.service'; - -describe(`BrowseLinkDataService`, () => { - let service: BrowseLinkDataService; - const findAllDataSpy = jasmine.createSpyObj('findAllData', { - findAll: EMPTY, - }); - const options = new FindListOptions(); - const linksToFollow = [ - followLink('entries'), - followLink('items') - ]; - - beforeEach(() => { - service = new BrowseLinkDataService(null, null, null, null, null); - (service as any).findAllData = findAllDataSpy; - }); - - describe(`getBrowseLinkFor`, () => { - it(`should call findAll on findAllData`, () => { - service.getBrowseLinkFor(['dc.test']); - expect(findAllDataSpy.findAll).toHaveBeenCalledWith({ elementsPerPage: 9999 }); - }); - }); -}); diff --git a/src/app/core/browse/browse-link-data.service.ts b/src/app/core/browse/browse-link-data.service.ts deleted file mode 100644 index c7dd31e4d3..0000000000 --- a/src/app/core/browse/browse-link-data.service.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { Injectable } from '@angular/core'; -import { BrowseDefinition } from '../shared/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'; -import { HALEndpointService } from '../shared/hal-endpoint.service'; -import { Observable } from 'rxjs'; -import { RemoteData } from '../data/remote-data'; -import { PaginatedList } from '../data/paginated-list.model'; -import { IdentifiableDataService } from '../data/base/identifiable-data.service'; -import { FindAllDataImpl } from '../data/base/find-all-data'; -import { BrowseDefinitionDataService } from './browse-definition-data.service'; -import { - getFirstSucceededRemoteData, getPaginatedListPayload, getRemoteDataPayload -} from '../shared/operators'; -import { distinctUntilChanged, map, startWith } from 'rxjs/operators'; -import { isEmpty, isNotEmpty } from '../../shared/empty.util'; -import { BrowseService } from './browse.service'; - -/** - * Data service responsible for retrieving browse definitions from the REST server, IF AND ONLY IF - * they are configured as browse links (webui.browse.link.) - */ -@Injectable() -export class BrowseLinkDataService extends IdentifiableDataService { - private findAllData: FindAllDataImpl; - - constructor( - protected requestService: RequestService, - protected rdbService: RemoteDataBuildService, - protected objectCache: ObjectCacheService, - protected halService: HALEndpointService, - protected browseDefinitionDataService: BrowseDefinitionDataService - ) { - super('browselinks', requestService, rdbService, objectCache, halService); - this.findAllData = new FindAllDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, this.responseMsToLive); - } - - - /** - * Get all BrowseDefinitions - */ - getBrowseLinks(): Observable>> { - return this.findAllData.findAll({ elementsPerPage: 9999 }).pipe( - getFirstSucceededRemoteData(), - ); - } - - - /** - * Get the browse URL by providing a list of metadata keys - * @param metadatumKey - * @param linkPath - */ - getBrowseLinkFor(metadataKeys: string[]): Observable { - let searchKeyArray: string[] = []; - metadataKeys.forEach((metadataKey) => { - searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); - }); - return this.getBrowseLinks().pipe( - getRemoteDataPayload(), - getPaginatedListPayload(), - map((browseDefinitions: BrowseDefinition[]) => browseDefinitions - .find((def: BrowseDefinition) => { - const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); - return isNotEmpty(matchingKeys); - }) - ), - map((def: BrowseDefinition) => { - if (isEmpty(def) || isEmpty(def.id)) { - //throw new Error(`A browse definition for field ${metadataKey} isn't configured`); - } else { - return def; - } - }), - startWith(undefined), - distinctUntilChanged() - ); - } - -} - diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index d90ad8da77..ede23ba43b 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -24,7 +24,6 @@ import { SidebarService } from '../shared/sidebar/sidebar.service'; import { AuthenticatedGuard } from './auth/authenticated.guard'; import { AuthStatus } from './auth/models/auth-status.model'; import { BrowseService } from './browse/browse.service'; -import { BrowseLinkDataService } from './browse/browse-link-data.service'; import { RemoteDataBuildService } from './cache/builders/remote-data-build.service'; import { ObjectCacheService } from './cache/object-cache.service'; import { SubmissionDefinitionsModel } from './config/models/config-submission-definitions.model'; @@ -222,7 +221,6 @@ const PROVIDERS = [ MyDSpaceResponseParsingService, ServerResponseService, BrowseService, - BrowseLinkDataService, AccessStatusDataService, SubmissionCcLicenseDataService, SubmissionCcLicenseUrlDataService, diff --git a/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts b/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts index 7e20edca6b..c4afdcb850 100644 --- a/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts +++ b/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts @@ -35,6 +35,12 @@ import { VersionDataService } from '../../../../core/data/version-data.service'; import { WorkspaceitemDataService } from '../../../../core/submission/workspaceitem-data.service'; import { SearchService } from '../../../../core/shared/search/search.service'; import { mockRouteService } from '../../../../item-page/simple/item-types/shared/item.component.spec'; +import { + BrowseDefinitionDataServiceStub, + browseServiceStub, +} from '../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseService } from '../../../../core/browse/browse.service'; +import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; let comp: JournalComponent; let fixture: ComponentFixture; @@ -100,7 +106,9 @@ describe('JournalComponent', () => { { provide: BitstreamDataService, useValue: mockBitstreamDataService }, { provide: WorkspaceitemDataService, useValue: {} }, { provide: SearchService, useValue: {} }, - { provide: RouteService, useValue: mockRouteService } + { provide: RouteService, useValue: mockRouteService }, + { provide: BrowseService, useValue: browseServiceStub }, + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub } ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts index c98ea09079..bfed3847c5 100644 --- a/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts @@ -7,8 +7,8 @@ import { SharedModule } from '../../../../../shared/shared.module'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; import { By } from '@angular/platform-browser'; -import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; -import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; +import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; +import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; let comp: ItemPageAbstractFieldComponent; let fixture: ComponentFixture; @@ -27,7 +27,7 @@ describe('ItemPageAbstractFieldComponent', () => { ], providers: [ { provide: APP_CONFIG, useValue: environment }, - { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub } ], declarations: [ItemPageAbstractFieldComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts index d85754438c..855a995142 100644 --- a/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts @@ -7,8 +7,8 @@ import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component import { ItemPageAuthorFieldComponent } from './item-page-author-field.component'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; -import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; -import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; +import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; +import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; let comp: ItemPageAuthorFieldComponent; let fixture: ComponentFixture; @@ -27,7 +27,7 @@ describe('ItemPageAuthorFieldComponent', () => { })], providers: [ { provide: APP_CONFIG, useValue: environment }, - { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub } ], declarations: [ItemPageAuthorFieldComponent, MetadataValuesComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts index f4b491dd22..be124dab82 100644 --- a/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts @@ -7,8 +7,8 @@ import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component import { ItemPageDateFieldComponent } from './item-page-date-field.component'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; -import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; -import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; +import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; +import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; let comp: ItemPageDateFieldComponent; let fixture: ComponentFixture; @@ -27,7 +27,7 @@ describe('ItemPageDateFieldComponent', () => { })], providers: [ { provide: APP_CONFIG, useValue: environment }, - { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub } ], declarations: [ItemPageDateFieldComponent, MetadataValuesComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts index 69fc84957d..fdf5ac1bb5 100644 --- a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts @@ -7,8 +7,8 @@ import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component import { GenericItemPageFieldComponent } from './generic-item-page-field.component'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; -import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; -import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; +import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; +import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; let comp: GenericItemPageFieldComponent; let fixture: ComponentFixture; @@ -29,7 +29,7 @@ describe('GenericItemPageFieldComponent', () => { })], providers: [ { provide: APP_CONFIG, useValue: environment }, - { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub } ], declarations: [GenericItemPageFieldComponent, MetadataValuesComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts index 2a4e60b393..b1579b1cda 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts @@ -12,9 +12,9 @@ import { environment } from '../../../../../environments/environment'; import { MarkdownPipe } from '../../../../shared/utils/markdown.pipe'; import { SharedModule } from '../../../../shared/shared.module'; import { APP_CONFIG } from '../../../../../config/app-config.interface'; -import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; -import { browseLinkDataServiceStub } from '../../../../shared/testing/browse-link-data-service.stub'; import { By } from '@angular/platform-browser'; +import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; +import { BrowseDefinitionDataServiceStub } from '../../../../shared/testing/browse-definition-data-service.stub'; let comp: ItemPageFieldComponent; let fixture: ComponentFixture; @@ -49,7 +49,7 @@ describe('ItemPageFieldComponent', () => { ], providers: [ { provide: APP_CONFIG, useValue: appConfig }, - { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub } ], declarations: [ItemPageFieldComponent, MetadataValuesComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts index 0b6805d857..5d835f97dd 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts @@ -3,7 +3,8 @@ import { Item } from '../../../../core/shared/item.model'; import { map } from 'rxjs/operators'; import { Observable } from 'rxjs'; import { BrowseDefinition } from '../../../../core/shared/browse-definition.model'; -import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; +import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; +import { getPaginatedListPayload, getRemoteDataPayload } from '../../../../core/shared/operators'; /** * This component can be used to represent metadata on a simple item page. @@ -16,7 +17,7 @@ import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data. }) export class ItemPageFieldComponent { - constructor(protected browseLinkDataService: BrowseLinkDataService) { + constructor(protected browseDefinitionDataService: BrowseDefinitionDataService) { } /** @@ -55,7 +56,9 @@ export class ItemPageFieldComponent { * link in dspace.cfg (webui.browse.link.) */ get browseDefinition(): Observable { - return this.browseLinkDataService.getBrowseLinkFor(this.fields).pipe( - map((def) => def)); + return this.browseDefinitionDataService.findByFields(this.fields).pipe( + getRemoteDataPayload(), + map((def) => def) + ); } } diff --git a/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts index 50d697fb70..cc55b76e3e 100644 --- a/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts @@ -7,8 +7,8 @@ import { ItemPageUriFieldComponent } from './item-page-uri-field.component'; import { MetadataUriValuesComponent } from '../../../../field-components/metadata-uri-values/metadata-uri-values.component'; import { environment } from '../../../../../../environments/environment'; import { APP_CONFIG } from '../../../../../../config/app-config.interface'; -import { BrowseLinkDataService } from '../../../../../core/browse/browse-link-data.service'; -import { browseLinkDataServiceStub } from '../../../../../shared/testing/browse-link-data-service.stub'; +import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; +import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; let comp: ItemPageUriFieldComponent; let fixture: ComponentFixture; @@ -28,7 +28,7 @@ describe('ItemPageUriFieldComponent', () => { })], providers: [ { provide: APP_CONFIG, useValue: environment }, - { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub } ], declarations: [ItemPageUriFieldComponent, MetadataUriValuesComponent], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/item-types/publication/publication.component.spec.ts b/src/app/item-page/simple/item-types/publication/publication.component.spec.ts index d1ea4fbe56..b56471fbb9 100644 --- a/src/app/item-page/simple/item-types/publication/publication.component.spec.ts +++ b/src/app/item-page/simple/item-types/publication/publication.component.spec.ts @@ -36,8 +36,12 @@ import { VersionDataService } from '../../../../core/data/version-data.service'; import { RouterTestingModule } from '@angular/router/testing'; import { WorkspaceitemDataService } from '../../../../core/submission/workspaceitem-data.service'; import { SearchService } from '../../../../core/shared/search/search.service'; -import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; -import { browseLinkDataServiceStub } from '../../../../shared/testing/browse-link-data-service.stub'; +import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; +import { + BrowseDefinitionDataServiceStub, + browseServiceStub, +} from '../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseService } from '../../../../core/browse/browse.service'; const noMetadata = new MetadataMap(); @@ -90,7 +94,8 @@ describe('PublicationComponent', () => { { provide: WorkspaceitemDataService, useValue: {} }, { provide: SearchService, useValue: {} }, { provide: RouteService, useValue: mockRouteService }, - { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: browseServiceStub }, ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/item-types/shared/item.component.spec.ts b/src/app/item-page/simple/item-types/shared/item.component.spec.ts index 40666a4d33..5849053c0b 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.spec.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.spec.ts @@ -23,7 +23,10 @@ import { UUIDService } from '../../../../core/shared/uuid.service'; import { isNotEmpty } from '../../../../shared/empty.util'; import { TranslateLoaderMock } from '../../../../shared/mocks/translate-loader.mock'; import { NotificationsService } from '../../../../shared/notifications/notifications.service'; -import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; +import { + createSuccessfulRemoteDataObject, + createSuccessfulRemoteDataObject$ +} from '../../../../shared/remote-data.utils'; import { TruncatableService } from '../../../../shared/truncatable/truncatable.service'; import { TruncatePipe } from '../../../../shared/utils/truncate.pipe'; import { GenericItemPageFieldComponent } from '../../field-components/specific-field/generic/generic-item-page-field.component'; @@ -38,13 +41,18 @@ import { VersionHistoryDataService } from '../../../../core/data/version-history import { RouterTestingModule } from '@angular/router/testing'; import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; import { ResearcherProfileDataService } from '../../../../core/profile/researcher-profile-data.service'; -import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; -import { browseLinkDataServiceStub } from '../../../../shared/testing/browse-link-data-service.stub'; +import { BrowseService } from '../../../../core/browse/browse.service'; +import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; +import { + BrowseDefinitionDataServiceStub, + browseServiceStub, +} from '../../../../shared/testing/browse-definition-data-service.stub'; -import { buildPaginatedList } from '../../../../core/data/paginated-list.model'; +import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model'; import { PageInfo } from '../../../../core/shared/page-info.model'; import { Router } from '@angular/router'; import { ItemComponent } from './item.component'; +import { BrowseDefinition } from '../../../../core/shared/browse-definition.model'; export function getIIIFSearchEnabled(enabled: boolean): MetadataValue { return Object.assign(new MetadataValue(), { @@ -72,6 +80,12 @@ export const mockRouteService = { } }; +export const mockBrowseService = { + getBrowseDefinitions(): Observable>> { + return observableOf(createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), []))); + }, +} + /** * Create a generic test for an item-page-fields component using a mockItem and the type of component * @param {Item} mockItem The item to use for testing. The item needs to contain just the metadata necessary to @@ -128,7 +142,8 @@ export function getItemPageFieldsTest(mockItem: Item, component) { { provide: RouteService, useValue: mockRouteService }, { provide: AuthorizationDataService, useValue: authorizationService }, { provide: ResearcherProfileDataService, useValue: {} }, - { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: browseServiceStub } ], schemas: [NO_ERRORS_SCHEMA] @@ -447,7 +462,8 @@ describe('ItemComponent', () => { { provide: SearchService, useValue: {} }, { provide: RouteService, useValue: mockRouteService }, { provide: AuthorizationDataService, useValue: {} }, - { provide: ResearcherProfileDataService, useValue: {} } + { provide: ResearcherProfileDataService, useValue: {} }, + { provide: BrowseService, useValue: browseServiceStub } ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(ItemComponent, { diff --git a/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts b/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts index 4b8d581d75..f085400cb9 100644 --- a/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts +++ b/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts @@ -36,9 +36,13 @@ import { VersionDataService } from '../../../../core/data/version-data.service'; import { RouterTestingModule } from '@angular/router/testing'; import { WorkspaceitemDataService } from '../../../../core/submission/workspaceitem-data.service'; import { SearchService } from '../../../../core/shared/search/search.service'; -import { BrowseLinkDataService } from '../../../../core/browse/browse-link-data.service'; -import { browseLinkDataServiceStub } from '../../../../shared/testing/browse-link-data-service.stub'; import { ItemVersionsSharedService } from '../../../versions/item-versions-shared.service'; +import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; +import { + BrowseDefinitionDataServiceStub, + browseServiceStub, +} from '../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseService } from '../../../../core/browse/browse.service'; const noMetadata = new MetadataMap(); @@ -93,7 +97,8 @@ describe('UntypedItemComponent', () => { { provide: ItemDataService, useValue: {} }, { provide: ItemVersionsSharedService, useValue: {} }, { provide: RouteService, useValue: mockRouteService }, - { provide: BrowseLinkDataService, useValue: browseLinkDataServiceStub } + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: browseServiceStub }, ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(UntypedItemComponent, { diff --git a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts index fcd82ce678..5b4c5171fc 100644 --- a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts +++ b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts @@ -21,6 +21,10 @@ import { Version } from '../../../../core/shared/version.model'; import { RouteService } from '../../../../core/services/route.service'; import { TranslateLoaderMock } from '../../../../shared/testing/translate-loader.mock'; import { ItemSharedModule } from '../../../item-shared.module'; +import { + browseServiceStub, +} from '../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseService } from '../../../../core/browse/browse.service'; const mockItem: Item = Object.assign(new Item(), { bundles: createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [])), @@ -77,7 +81,8 @@ describe('VersionedItemComponent', () => { { provide: WorkspaceitemDataService, useValue: {} }, { provide: SearchService, useValue: {} }, { provide: ItemDataService, useValue: {} }, - { provide: RouteService, useValue: mockRouteService } + { provide: RouteService, useValue: mockRouteService }, + { provide: BrowseService, useValue: browseServiceStub }, ] }).compileComponents(); versionService = TestBed.inject(VersionDataService); diff --git a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.spec.ts b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.spec.ts index f05f7bb566..cfb812a475 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.spec.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.spec.ts @@ -3,7 +3,7 @@ import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; import { PlainTextMetadataListElementComponent } from './plain-text-metadata-list-element.component'; import { MetadatumRepresentation } from '../../../../core/shared/metadata-representation/metadatum/metadatum-representation.model'; import { By } from '@angular/platform-browser'; -import { mockData } from '../../../testing/browse-link-data-service.stub'; +import { mockData } from '../../../testing/browse-definition-data-service.stub'; // Render the mock representation with the default mock author browse definition so it is also rendered as a link // without affecting other tests diff --git a/src/app/shared/testing/browse-link-data-service.stub.ts b/src/app/shared/testing/browse-definition-data-service.stub.ts similarity index 77% rename from src/app/shared/testing/browse-link-data-service.stub.ts rename to src/app/shared/testing/browse-definition-data-service.stub.ts index 4deff2bf05..5428947c35 100644 --- a/src/app/shared/testing/browse-link-data-service.stub.ts +++ b/src/app/shared/testing/browse-definition-data-service.stub.ts @@ -36,25 +36,39 @@ export const mockData: BrowseDefinition[] = [ }) ]; +export const browseServiceStub = { + getBrowseDefinitions(): Observable>> { + return observableOf(createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), mockData))); + }, +} + +export const BrowseDefinitionDataServiceStub: any = { -export const browseLinkDataServiceStub: any = { /** * Get all BrowseDefinitions */ - getBrowseLinks(): Observable>> { + findAll(): Observable>> { return observableOf(createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), mockData))); }, + + /** + * Get all BrowseDefinitions with any link configuration + */ + findAllLinked(): Observable>> { + return observableOf(createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), mockData))); + }, + /** * Get the browse URL by providing a list of metadata keys * @param metadatumKey * @param linkPath */ - getBrowseLinkFor(metadataKeys: string[]): Observable { + findByFields(metadataKeys: string[]): Observable { let searchKeyArray: string[] = []; metadataKeys.forEach((metadataKey) => { searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); }); - return this.getBrowseLinks().pipe( + return this.findAllLinked().pipe( getRemoteDataPayload(), getPaginatedListPayload(), map((browseDefinitions: BrowseDefinition[]) => browseDefinitions From 58673ed9abc6b1c50cc667e278ded980489668dd Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Thu, 26 Jan 2023 15:15:44 +1300 Subject: [PATCH 10/20] [TLC-249] Lint fixes --- src/app/core/browse/browse-definition-data.service.ts | 3 --- .../specific-field/item-page-field.component.ts | 2 +- .../simple/item-types/shared/item.component.spec.ts | 6 ------ .../shared/testing/browse-definition-data-service.stub.ts | 2 +- 4 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index aa3e095608..af2d387b71 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -13,11 +13,8 @@ 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 { getPaginatedListPayload, getRemoteDataPayload } from '../shared/operators'; import { RequestParam } from '../cache/models/request-param.model'; import { SearchData, SearchDataImpl } from '../data/base/search-data'; -import { distinctUntilChanged, map, startWith } from 'rxjs/operators'; -import { isEmpty, isNotEmpty } from '../../shared/empty.util'; /** * Data service responsible for retrieving browse definitions from the REST server diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts index 5d835f97dd..fc526dabcc 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts @@ -4,7 +4,7 @@ import { map } from 'rxjs/operators'; import { Observable } from 'rxjs'; import { BrowseDefinition } from '../../../../core/shared/browse-definition.model'; import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; -import { getPaginatedListPayload, getRemoteDataPayload } from '../../../../core/shared/operators'; +import { getRemoteDataPayload } from '../../../../core/shared/operators'; /** * This component can be used to represent metadata on a simple item page. diff --git a/src/app/item-page/simple/item-types/shared/item.component.spec.ts b/src/app/item-page/simple/item-types/shared/item.component.spec.ts index 5849053c0b..07aa47b882 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.spec.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.spec.ts @@ -80,12 +80,6 @@ export const mockRouteService = { } }; -export const mockBrowseService = { - getBrowseDefinitions(): Observable>> { - return observableOf(createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), []))); - }, -} - /** * Create a generic test for an item-page-fields component using a mockItem and the type of component * @param {Item} mockItem The item to use for testing. The item needs to contain just the metadata necessary to diff --git a/src/app/shared/testing/browse-definition-data-service.stub.ts b/src/app/shared/testing/browse-definition-data-service.stub.ts index 5428947c35..f56b01e26a 100644 --- a/src/app/shared/testing/browse-definition-data-service.stub.ts +++ b/src/app/shared/testing/browse-definition-data-service.stub.ts @@ -40,7 +40,7 @@ export const browseServiceStub = { getBrowseDefinitions(): Observable>> { return observableOf(createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), mockData))); }, -} +}; export const BrowseDefinitionDataServiceStub: any = { From 6ead2f0a7da4d398c6ad58f8ee550fda3c2d2b58 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Thu, 26 Jan 2023 15:23:25 +1300 Subject: [PATCH 11/20] [TLC-249] Lint fixes --- .../item-page/simple/item-types/shared/item.component.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/app/item-page/simple/item-types/shared/item.component.spec.ts b/src/app/item-page/simple/item-types/shared/item.component.spec.ts index 07aa47b882..13e3b9686d 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.spec.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.spec.ts @@ -24,7 +24,6 @@ import { isNotEmpty } from '../../../../shared/empty.util'; import { TranslateLoaderMock } from '../../../../shared/mocks/translate-loader.mock'; import { NotificationsService } from '../../../../shared/notifications/notifications.service'; import { - createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; import { TruncatableService } from '../../../../shared/truncatable/truncatable.service'; @@ -48,11 +47,10 @@ import { browseServiceStub, } from '../../../../shared/testing/browse-definition-data-service.stub'; -import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model'; +import { buildPaginatedList } from '../../../../core/data/paginated-list.model'; import { PageInfo } from '../../../../core/shared/page-info.model'; import { Router } from '@angular/router'; import { ItemComponent } from './item.component'; -import { BrowseDefinition } from '../../../../core/shared/browse-definition.model'; export function getIIIFSearchEnabled(enabled: boolean): MetadataValue { return Object.assign(new MetadataValue(), { From bf9041f25ffa4f6b97e502dc1da65b7249846762 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Thu, 26 Jan 2023 16:21:15 +1300 Subject: [PATCH 12/20] [TLC-380] Fix mock service to return proper payload --- .../browse-definition-data-service.stub.ts | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/src/app/shared/testing/browse-definition-data-service.stub.ts b/src/app/shared/testing/browse-definition-data-service.stub.ts index f56b01e26a..7dfbd0d804 100644 --- a/src/app/shared/testing/browse-definition-data-service.stub.ts +++ b/src/app/shared/testing/browse-definition-data-service.stub.ts @@ -11,6 +11,8 @@ import { distinctUntilChanged, map, startWith } from 'rxjs/operators'; import { isEmpty, isNotEmpty } from '../empty.util'; import { createSuccessfulRemoteDataObject } from '../remote-data.utils'; import { PageInfo } from '../../core/shared/page-info.model'; +import { FollowLinkConfig } from '../utils/follow-link-config.model'; +import { RequestParam } from '../../core/cache/models/request-param.model'; // This data is in post-serialized form (metadata -> metadataKeys) export const mockData: BrowseDefinition[] = [ @@ -63,29 +65,13 @@ export const BrowseDefinitionDataServiceStub: any = { * @param metadatumKey * @param linkPath */ - findByFields(metadataKeys: string[]): Observable { + findByFields(metadataKeys: string[]): Observable> { let searchKeyArray: string[] = []; metadataKeys.forEach((metadataKey) => { searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); }); - return this.findAllLinked().pipe( - getRemoteDataPayload(), - getPaginatedListPayload(), - map((browseDefinitions: BrowseDefinition[]) => browseDefinitions - .find((def: BrowseDefinition) => { - const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); - return isNotEmpty(matchingKeys); - }) - ), - map((def: BrowseDefinition) => { - if (isEmpty(def) || isEmpty(def.id)) { - //throw new Error(`A browse definition for field ${metadataKey} isn't configured`); - } else { - return def; - } - }), - startWith(undefined), - distinctUntilChanged() - ); + // Return just the first, as a pretend match + return observableOf(createSuccessfulRemoteDataObject(mockData[0])); } + }; From 72afd0cafd62c8545c5278e63860f1a40c43ff06 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Thu, 26 Jan 2023 16:33:37 +1300 Subject: [PATCH 13/20] [TLC-380] Simplify / strip browse service from components --- .../item-pages/journal/journal.component.spec.ts | 5 +---- .../publication/publication.component.spec.ts | 5 +---- .../simple/item-types/shared/item.component.spec.ts | 6 +----- .../simple/item-types/shared/item.component.ts | 12 +----------- .../untyped-item/untyped-item.component.spec.ts | 5 +---- .../versioned-item/versioned-item.component.spec.ts | 5 ----- .../versioned-item/versioned-item.component.ts | 6 ++---- .../testing/browse-definition-data-service.stub.ts | 6 ------ 8 files changed, 7 insertions(+), 43 deletions(-) diff --git a/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts b/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts index c4afdcb850..6e2ded334b 100644 --- a/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts +++ b/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts @@ -36,10 +36,8 @@ import { WorkspaceitemDataService } from '../../../../core/submission/workspacei import { SearchService } from '../../../../core/shared/search/search.service'; import { mockRouteService } from '../../../../item-page/simple/item-types/shared/item.component.spec'; import { - BrowseDefinitionDataServiceStub, - browseServiceStub, + BrowseDefinitionDataServiceStub } from '../../../../shared/testing/browse-definition-data-service.stub'; -import { BrowseService } from '../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; let comp: JournalComponent; @@ -107,7 +105,6 @@ describe('JournalComponent', () => { { provide: WorkspaceitemDataService, useValue: {} }, { provide: SearchService, useValue: {} }, { provide: RouteService, useValue: mockRouteService }, - { provide: BrowseService, useValue: browseServiceStub }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub } ], diff --git a/src/app/item-page/simple/item-types/publication/publication.component.spec.ts b/src/app/item-page/simple/item-types/publication/publication.component.spec.ts index b56471fbb9..211ec102bc 100644 --- a/src/app/item-page/simple/item-types/publication/publication.component.spec.ts +++ b/src/app/item-page/simple/item-types/publication/publication.component.spec.ts @@ -38,10 +38,8 @@ import { WorkspaceitemDataService } from '../../../../core/submission/workspacei import { SearchService } from '../../../../core/shared/search/search.service'; import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; import { - BrowseDefinitionDataServiceStub, - browseServiceStub, + BrowseDefinitionDataServiceStub } from '../../../../shared/testing/browse-definition-data-service.stub'; -import { BrowseService } from '../../../../core/browse/browse.service'; const noMetadata = new MetadataMap(); @@ -95,7 +93,6 @@ describe('PublicationComponent', () => { { provide: SearchService, useValue: {} }, { provide: RouteService, useValue: mockRouteService }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, - { provide: BrowseService, useValue: browseServiceStub }, ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/item-page/simple/item-types/shared/item.component.spec.ts b/src/app/item-page/simple/item-types/shared/item.component.spec.ts index 13e3b9686d..5bf08fc004 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.spec.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.spec.ts @@ -40,11 +40,9 @@ import { VersionHistoryDataService } from '../../../../core/data/version-history import { RouterTestingModule } from '@angular/router/testing'; import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; import { ResearcherProfileDataService } from '../../../../core/profile/researcher-profile-data.service'; -import { BrowseService } from '../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; import { - BrowseDefinitionDataServiceStub, - browseServiceStub, + BrowseDefinitionDataServiceStub } from '../../../../shared/testing/browse-definition-data-service.stub'; import { buildPaginatedList } from '../../../../core/data/paginated-list.model'; @@ -135,7 +133,6 @@ export function getItemPageFieldsTest(mockItem: Item, component) { { provide: AuthorizationDataService, useValue: authorizationService }, { provide: ResearcherProfileDataService, useValue: {} }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, - { provide: BrowseService, useValue: browseServiceStub } ], schemas: [NO_ERRORS_SCHEMA] @@ -455,7 +452,6 @@ describe('ItemComponent', () => { { provide: RouteService, useValue: mockRouteService }, { provide: AuthorizationDataService, useValue: {} }, { provide: ResearcherProfileDataService, useValue: {} }, - { provide: BrowseService, useValue: browseServiceStub } ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(ItemComponent, { diff --git a/src/app/item-page/simple/item-types/shared/item.component.ts b/src/app/item-page/simple/item-types/shared/item.component.ts index 8e9cd389a7..93e6a0b346 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.ts @@ -4,11 +4,9 @@ import { Item } from '../../../../core/shared/item.model'; import { getItemPageRoute } from '../../../item-page-routing-paths'; import { RouteService } from '../../../../core/services/route.service'; import { Observable } from 'rxjs'; -import { BrowseService } from '../../../../core/browse/browse.service'; import { getDSpaceQuery, isIiifEnabled, isIiifSearchEnabled } from './item-iiif-utils'; import { filter, map, take } from 'rxjs/operators'; import { Router } from '@angular/router'; -import { BrowseDefinition } from '../../../../core/shared/browse-definition.model'; @Component({ selector: 'ds-item', @@ -51,14 +49,10 @@ export class ItemComponent implements OnInit { */ iiifQuery$: Observable; - browseDefinitions: BrowseDefinition[]; - browseDefinitions$: Observable; - mediaViewer; constructor(protected routeService: RouteService, - protected router: Router, - protected browseService: BrowseService) { + protected router: Router) { this.mediaViewer = environment.mediaViewer; } @@ -90,9 +84,5 @@ export class ItemComponent implements OnInit { if (this.iiifSearchEnabled) { this.iiifQuery$ = getDSpaceQuery(this.object, this.routeService); } - // get browse definitions - this.browseDefinitions$ = this.browseService.getBrowseDefinitions().pipe( - map((data) => data.payload.page as BrowseDefinition[]) - ); } } diff --git a/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts b/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts index f085400cb9..4b7da40abe 100644 --- a/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts +++ b/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts @@ -39,10 +39,8 @@ import { SearchService } from '../../../../core/shared/search/search.service'; import { ItemVersionsSharedService } from '../../../versions/item-versions-shared.service'; import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; import { - BrowseDefinitionDataServiceStub, - browseServiceStub, + BrowseDefinitionDataServiceStub } from '../../../../shared/testing/browse-definition-data-service.stub'; -import { BrowseService } from '../../../../core/browse/browse.service'; const noMetadata = new MetadataMap(); @@ -98,7 +96,6 @@ describe('UntypedItemComponent', () => { { provide: ItemVersionsSharedService, useValue: {} }, { provide: RouteService, useValue: mockRouteService }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, - { provide: BrowseService, useValue: browseServiceStub }, ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(UntypedItemComponent, { diff --git a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts index 5b4c5171fc..b29c7e58f3 100644 --- a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts +++ b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts @@ -21,10 +21,6 @@ import { Version } from '../../../../core/shared/version.model'; import { RouteService } from '../../../../core/services/route.service'; import { TranslateLoaderMock } from '../../../../shared/testing/translate-loader.mock'; import { ItemSharedModule } from '../../../item-shared.module'; -import { - browseServiceStub, -} from '../../../../shared/testing/browse-definition-data-service.stub'; -import { BrowseService } from '../../../../core/browse/browse.service'; const mockItem: Item = Object.assign(new Item(), { bundles: createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [])), @@ -82,7 +78,6 @@ describe('VersionedItemComponent', () => { { provide: SearchService, useValue: {} }, { provide: ItemDataService, useValue: {} }, { provide: RouteService, useValue: mockRouteService }, - { provide: BrowseService, useValue: browseServiceStub }, ] }).compileComponents(); versionService = TestBed.inject(VersionDataService); diff --git a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.ts b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.ts index 016ac26d0c..6855d9c4dc 100644 --- a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.ts +++ b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.ts @@ -17,7 +17,6 @@ import { Item } from '../../../../core/shared/item.model'; import { ItemDataService } from '../../../../core/data/item-data.service'; import { WorkspaceItem } from '../../../../core/submission/models/workspaceitem.model'; import { RouteService } from '../../../../core/services/route.service'; -import { BrowseService } from '../../../../core/browse/browse.service'; @Component({ selector: 'ds-versioned-item', @@ -36,10 +35,9 @@ export class VersionedItemComponent extends ItemComponent { private workspaceItemDataService: WorkspaceitemDataService, private searchService: SearchService, private itemService: ItemDataService, - protected routeService: RouteService, - protected browseService: BrowseService, + protected routeService: RouteService ) { - super(routeService, router, browseService); + super(routeService, router); } /** diff --git a/src/app/shared/testing/browse-definition-data-service.stub.ts b/src/app/shared/testing/browse-definition-data-service.stub.ts index 7dfbd0d804..42f4020483 100644 --- a/src/app/shared/testing/browse-definition-data-service.stub.ts +++ b/src/app/shared/testing/browse-definition-data-service.stub.ts @@ -38,12 +38,6 @@ export const mockData: BrowseDefinition[] = [ }) ]; -export const browseServiceStub = { - getBrowseDefinitions(): Observable>> { - return observableOf(createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), mockData))); - }, -}; - export const BrowseDefinitionDataServiceStub: any = { /** From 6883b8c7823742adc28d9182c6e74f70f17ed202 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Thu, 26 Jan 2023 16:39:28 +1300 Subject: [PATCH 14/20] [TLC-380] Lint fixes for mock browse def service --- .../shared/testing/browse-definition-data-service.stub.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/app/shared/testing/browse-definition-data-service.stub.ts b/src/app/shared/testing/browse-definition-data-service.stub.ts index 42f4020483..47b2bb982a 100644 --- a/src/app/shared/testing/browse-definition-data-service.stub.ts +++ b/src/app/shared/testing/browse-definition-data-service.stub.ts @@ -2,17 +2,9 @@ import { EMPTY, Observable, of as observableOf } from 'rxjs'; import { RemoteData } from '../../core/data/remote-data'; import { buildPaginatedList, PaginatedList } from '../../core/data/paginated-list.model'; import { BrowseDefinition } from '../../core/shared/browse-definition.model'; -import { - getPaginatedListPayload, - getRemoteDataPayload -} from '../../core/shared/operators'; import { BrowseService } from '../../core/browse/browse.service'; -import { distinctUntilChanged, map, startWith } from 'rxjs/operators'; -import { isEmpty, isNotEmpty } from '../empty.util'; import { createSuccessfulRemoteDataObject } from '../remote-data.utils'; import { PageInfo } from '../../core/shared/page-info.model'; -import { FollowLinkConfig } from '../utils/follow-link-config.model'; -import { RequestParam } from '../../core/cache/models/request-param.model'; // This data is in post-serialized form (metadata -> metadataKeys) export const mockData: BrowseDefinition[] = [ From d86e8ed0a86e5691274a412337ec11a8ebfe007d Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Mon, 6 Feb 2023 13:01:33 +1300 Subject: [PATCH 15/20] [TLC-380] Template link, spec test, doc fixup as per review --- .../browse-definition-data.service.spec.ts | 47 +++++++++++-- .../browse/browse-definition-data.service.ts | 69 +++---------------- .../metadata-values.component.spec.ts | 6 ++ .../metadata-values.component.ts | 4 +- .../generic-item-page-field.component.ts | 2 +- ...-link-metadata-list-element.component.html | 3 +- ...nk-metadata-list-element.component.spec.ts | 24 +++++++ ...se-link-metadata-list-element.component.ts | 11 +++ ...a-representation-list-element.component.ts | 3 + ...-text-metadata-list-element.component.html | 5 +- ...in-text-metadata-list-element.component.ts | 11 +++ .../browse-definition-data-service.stub.ts | 4 +- 12 files changed, 117 insertions(+), 72 deletions(-) diff --git a/src/app/core/browse/browse-definition-data.service.spec.ts b/src/app/core/browse/browse-definition-data.service.spec.ts index 9377dc715f..f321c2551c 100644 --- a/src/app/core/browse/browse-definition-data.service.spec.ts +++ b/src/app/core/browse/browse-definition-data.service.spec.ts @@ -2,27 +2,66 @@ import { BrowseDefinitionDataService } from './browse-definition-data.service'; import { followLink } from '../../shared/utils/follow-link-config.model'; import { EMPTY } from 'rxjs'; import { FindListOptions } from '../data/find-list-options.model'; +import { getMockRemoteDataBuildService } from '../../shared/mocks/remote-data-build.service.mock'; +import { RequestService } from '../data/request.service'; +import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub'; +import { getMockObjectCacheService } from '../../shared/mocks/object-cache.service.mock'; describe(`BrowseDefinitionDataService`, () => { + let requestService: RequestService; let service: BrowseDefinitionDataService; - const findAllDataSpy = jasmine.createSpyObj('findAllData', { - findAll: EMPTY, - }); + let findAllDataSpy; + let searchDataSpy; + const browsesEndpointURL = 'https://rest.api/browses'; + const halService: any = new HALEndpointServiceStub(browsesEndpointURL); + const options = new FindListOptions(); const linksToFollow = [ followLink('entries'), followLink('items') ]; + function initTestService() { + return new BrowseDefinitionDataService( + requestService, + getMockRemoteDataBuildService(), + getMockObjectCacheService(), + halService, + ); + } + beforeEach(() => { - service = new BrowseDefinitionDataService(null, null, null, null); + service = initTestService(); + findAllDataSpy = jasmine.createSpyObj('findAllData', { + findAll: EMPTY, + }); + searchDataSpy = jasmine.createSpyObj('searchData', { + searchBy: EMPTY, + getSearchByHref: EMPTY, + }); (service as any).findAllData = findAllDataSpy; + (service as any).searchData = searchDataSpy; }); + describe('findByFields', () => { + it(`should call searchByHref on searchData`, () => { + service.findByFields(['test'], true, false, ...linksToFollow); + expect(searchDataSpy.getSearchByHref).toHaveBeenCalled(); + }); + }); + describe('searchBy', () => { + it(`should call searchBy on searchData`, () => { + service.searchBy('test', options, true, false, ...linksToFollow); + expect(searchDataSpy.searchBy).toHaveBeenCalledWith('test', options, true, false, ...linksToFollow); + }); + }); describe(`findAll`, () => { it(`should call findAll on findAllData`, () => { service.findAll(options, true, false, ...linksToFollow); expect(findAllDataSpy.findAll).toHaveBeenCalledWith(options, true, false, ...linksToFollow); }); }); + + + }); diff --git a/src/app/core/browse/browse-definition-data.service.ts b/src/app/core/browse/browse-definition-data.service.ts index af2d387b71..88d070000e 100644 --- a/src/app/core/browse/browse-definition-data.service.ts +++ b/src/app/core/browse/browse-definition-data.service.ts @@ -27,19 +27,6 @@ export class BrowseDefinitionDataService extends IdentifiableDataService; private searchData: SearchDataImpl; - public static toSearchKeyArray(metadataKey: string): string[] { - const keyParts = metadataKey.split('.'); - const searchFor = []; - searchFor.push('*'); - for (let i = 0; i < keyParts.length - 1; i++) { - const prevParts = keyParts.slice(0, i + 1); - const nextPart = [...prevParts, '*'].join('.'); - searchFor.push(nextPart); - } - searchFor.push(metadataKey); - return searchFor; - } - constructor( protected requestService: RequestService, protected rdbService: RemoteDataBuildService, @@ -100,54 +87,16 @@ export class BrowseDefinitionDataService extends IdentifiableDataService[] - ): Observable> { - const searchParams = []; - searchParams.push(new RequestParam('field', field)); - - const hrefObs = this.getSearchByHref( - 'byField', - { searchParams }, - ...linksToFollow - ); - - return this.findByHref( - hrefObs, - useCachedVersionIfAvailable, - reRequestOnStale, - ...linksToFollow, - ); - } - - findAllLinked( - useCachedVersionIfAvailable = true, - reRequestOnStale = true, - ...linksToFollow: FollowLinkConfig[] - ): Observable> { - const searchParams = []; - - const hrefObs = this.getSearchByHref( - 'allLinked', - { searchParams }, - ...linksToFollow - ); - - return this.findByHref( - hrefObs, - useCachedVersionIfAvailable, - reRequestOnStale, - ...linksToFollow, - ); - } - /** - * Get the browse URL by providing a list of metadata keys - * @param metadatumKey - * @param linkPath + * Get the browse URL by providing a list of metadata keys. The first matching browse index definition + * for any of the fields is returned. This is used in eg. item page field component, which can be configured + * with several fields for a component like 'Author', and needs to know if and how to link the values + * to configured browse indices. + * + * @param fields an array of field strings, eg. ['dc.contributor.author', 'dc.creator'] + * @param useCachedVersionIfAvailable Override the data service useCachedVersionIfAvailable parameter (default: true) + * @param reRequestOnStale Override the data service reRequestOnStale parameter (default: true) + * @param linksToFollow Override the data service linksToFollow parameter (default: empty array) */ findByFields( fields: string[], diff --git a/src/app/item-page/field-components/metadata-values/metadata-values.component.spec.ts b/src/app/item-page/field-components/metadata-values/metadata-values.component.spec.ts index 9e599b1294..c996169e6d 100644 --- a/src/app/item-page/field-components/metadata-values/metadata-values.component.spec.ts +++ b/src/app/item-page/field-components/metadata-values/metadata-values.component.spec.ts @@ -52,6 +52,7 @@ describe('MetadataValuesComponent', () => { comp.mdValues = mockMetadata; comp.separator = mockSeperator; comp.label = mockLabel; + comp.urlRegex = /^.*test.*$/; fixture.detectChanges(); })); @@ -67,4 +68,9 @@ describe('MetadataValuesComponent', () => { expect(separators.length).toBe(mockMetadata.length - 1); }); + it('should correctly detect a pattern on string containing "test"', () => { + const mdValue = {value: "This is a test value"} as MetadataValue; + expect(comp.hasLink(mdValue)).toBe(true); + }); + }); diff --git a/src/app/item-page/field-components/metadata-values/metadata-values.component.ts b/src/app/item-page/field-components/metadata-values/metadata-values.component.ts index 2ef160e74e..b487844e21 100644 --- a/src/app/item-page/field-components/metadata-values/metadata-values.component.ts +++ b/src/app/item-page/field-components/metadata-values/metadata-values.component.ts @@ -67,9 +67,9 @@ export class MetadataValuesComponent implements OnChanges { /** * Does this metadata value have a valid URL that should be rendered as a link? - * @param value + * @param value A MetadataValue being displayed */ - hasLink(value): boolean { + hasLink(value: MetadataValue): boolean { if (hasValue(this.urlRegex)) { const pattern = new RegExp(this.urlRegex); return pattern.test(value.value); diff --git a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.ts b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.ts index c9c16724e1..53d2f6aa20 100644 --- a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.ts +++ b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.ts @@ -43,7 +43,7 @@ export class GenericItemPageFieldComponent extends ItemPageFieldComponent { /** * Whether any valid HTTP(S) URL should be rendered as a link */ - @Input() urlRegex?; + @Input() urlRegex?: string; } diff --git a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.html b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.html index 24f500ac60..8d3afea273 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.html +++ b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.html @@ -1,7 +1,8 @@
+ [routerLink]="['/browse/', metadataRepresentation.browseDefinition.id]" + [queryParams]="getQueryParams()"> {{metadataRepresentation.getValue()}} (new browse link page) diff --git a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts index 5c9c436537..3d7fb5f862 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts @@ -8,6 +8,11 @@ const mockMetadataRepresentation = Object.assign(new MetadatumRepresentation('ty value: 'Test Author' }); +const mockMetadataRepresentationWithUrl = Object.assign(new MetadatumRepresentation('type'), { + key: 'dc.subject', + value: 'http://purl.org/test/subject' +}); + describe('BrowseLinkMetadataListElementComponent', () => { let comp: BrowseLinkMetadataListElementComponent; let fixture: ComponentFixture; @@ -33,6 +38,25 @@ describe('BrowseLinkMetadataListElementComponent', () => { it('should contain the value as a browse link', () => { expect(fixture.debugElement.nativeElement.textContent).toContain(mockMetadataRepresentation.value); }); + it('should NOT match isLink', () => { + expect(comp.isLink).toBe(false); + }) + }); + + beforeEach(waitForAsync(() => { + fixture = TestBed.createComponent(BrowseLinkMetadataListElementComponent); + comp = fixture.componentInstance; + comp.metadataRepresentation = mockMetadataRepresentationWithUrl; + fixture.detectChanges(); + })); + + waitForAsync(() => { + it('should contain the value expected', () => { + expect(fixture.debugElement.nativeElement.textContent).toContain(mockMetadataRepresentationWithUrl.value); + }); + it('should match isLink', () => { + expect(comp.isLink).toBe(true); + }) }); }); diff --git a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.ts b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.ts index 616ef09ae3..0eb0ce05b0 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.ts @@ -15,4 +15,15 @@ import { metadataRepresentationComponent } from '../../../metadata-representatio * It will simply use the value retrieved from MetadataRepresentation.getValue() to display as plain text */ export class BrowseLinkMetadataListElementComponent extends MetadataRepresentationListElementComponent { + /** + * Get the appropriate query parameters for this browse link, depending on whether the browse definition + * expects 'startsWith' (eg browse by date) or 'value' (eg browse by title) + */ + getQueryParams() { + let queryParams = {startsWith: this.metadataRepresentation.getValue()}; + if (this.metadataRepresentation.browseDefinition.metadataBrowse) { + return {value: this.metadataRepresentation.getValue()}; + } + return queryParams; + } } diff --git a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts index 2b9fbab4ee..5b667ed74a 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts @@ -14,6 +14,9 @@ export class MetadataRepresentationListElementComponent { */ metadataRepresentation: MetadataRepresentation; + /** + * Returns true if this component's value matches a basic regex "Is this an HTTP URL" test + */ isLink(): boolean { // Match any http:// or https:// const linkPattern = /^https?\/\//; diff --git a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.html b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.html index 93dd993ce4..7b611a7d1f 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.html +++ b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.html @@ -4,13 +4,14 @@ {{metadataRepresentation.getValue()}} + target="_blank" [href]="metadataRepresentation.getValue()"> {{metadataRepresentation.getValue()}} {{metadataRepresentation.getValue()}} + [routerLink]="['/browse/', metadataRepresentation.browseDefinition.id]" + [queryParams]="getQueryParams()"> {{metadataRepresentation.getValue()}}
diff --git a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.ts b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.ts index 198c3712d9..2d21a7afe8 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.ts @@ -15,4 +15,15 @@ import { metadataRepresentationComponent } from '../../../metadata-representatio * It will simply use the value retrieved from MetadataRepresentation.getValue() to display as plain text */ export class PlainTextMetadataListElementComponent extends MetadataRepresentationListElementComponent { + /** + * Get the appropriate query parameters for this browse link, depending on whether the browse definition + * expects 'startsWith' (eg browse by date) or 'value' (eg browse by title) + */ + getQueryParams() { + let queryParams = {startsWith: this.metadataRepresentation.getValue()}; + if (this.metadataRepresentation.browseDefinition.metadataBrowse) { + return {value: this.metadataRepresentation.getValue()}; + } + return queryParams; + } } diff --git a/src/app/shared/testing/browse-definition-data-service.stub.ts b/src/app/shared/testing/browse-definition-data-service.stub.ts index 47b2bb982a..ec1fc2f05e 100644 --- a/src/app/shared/testing/browse-definition-data-service.stub.ts +++ b/src/app/shared/testing/browse-definition-data-service.stub.ts @@ -48,8 +48,8 @@ export const BrowseDefinitionDataServiceStub: any = { /** * Get the browse URL by providing a list of metadata keys - * @param metadatumKey - * @param linkPath + * + * @param metadataKeys a list of fields eg. ['dc.contributor.author', 'dc.creator'] */ findByFields(metadataKeys: string[]): Observable> { let searchKeyArray: string[] = []; From 63af2e079c809ee9ef4c983e6cd2bc13e616835c Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Mon, 6 Feb 2023 14:12:38 +1300 Subject: [PATCH 16/20] [TLC-380] Refactor metadata rep list comp after rebase --- ...data-representation-list.component.spec.ts | 5 +++- .../metadata-representation-list.component.ts | 26 +++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.spec.ts b/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.spec.ts index 54420721b8..180eaaa2be 100644 --- a/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.spec.ts +++ b/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.spec.ts @@ -11,6 +11,8 @@ import { MetadataValue } from '../../../core/shared/metadata.models'; import { DSpaceObject } from '../../../core/shared/dspace-object.model'; import { ItemMetadataRepresentation } from '../../../core/shared/metadata-representation/item/item-metadata-representation.model'; import { MetadatumRepresentation } from '../../../core/shared/metadata-representation/metadatum/metadatum-representation.model'; +import { BrowseDefinitionDataService } from '../../../core/browse/browse-definition-data.service'; +import { BrowseDefinitionDataServiceStub } from '../../../shared/testing/browse-definition-data-service.stub'; const itemType = 'Person'; const metadataFields = ['dc.contributor.author', 'dc.creator']; @@ -104,7 +106,8 @@ describe('MetadataRepresentationListComponent', () => { imports: [TranslateModule.forRoot()], declarations: [MetadataRepresentationListComponent, VarDirective], providers: [ - { provide: RelationshipDataService, useValue: relationshipService } + { provide: RelationshipDataService, useValue: relationshipService }, + { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub } ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(MetadataRepresentationListComponent, { diff --git a/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.ts b/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.ts index 16dcf72cd4..3965be8705 100644 --- a/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.ts +++ b/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.ts @@ -8,6 +8,13 @@ import { RelationshipDataService } from '../../../core/data/relationship-data.se import { MetadataValue } from '../../../core/shared/metadata.models'; import { Item } from '../../../core/shared/item.model'; import { AbstractIncrementalListComponent } from '../abstract-incremental-list/abstract-incremental-list.component'; +import { map } from 'rxjs/operators'; +import { getRemoteDataPayload } from '../../../core/shared/operators'; +import { + MetadatumRepresentation +} from '../../../core/shared/metadata-representation/metadatum/metadatum-representation.model'; +import { BrowseService } from '../../../core/browse/browse.service'; +import { BrowseDefinitionDataService } from '../../../core/browse/browse-definition-data.service'; @Component({ selector: 'ds-metadata-representation-list', @@ -52,7 +59,8 @@ export class MetadataRepresentationListComponent extends AbstractIncrementalList */ total: number; - constructor(public relationshipService: RelationshipDataService) { + constructor(public relationshipService: RelationshipDataService, + private browseDefinitionDataService: BrowseDefinitionDataService) { super(); } @@ -76,7 +84,21 @@ export class MetadataRepresentationListComponent extends AbstractIncrementalList ...metadata .slice((this.objects.length * this.incrementBy), (this.objects.length * this.incrementBy) + this.incrementBy) .map((metadatum: any) => Object.assign(new MetadataValue(), metadatum)) - .map((metadatum: MetadataValue) => this.relationshipService.resolveMetadataRepresentation(metadatum, this.parentItem, this.itemType)), + .map((metadatum: MetadataValue) => { + if (metadatum.isVirtual) { + return this.relationshipService.resolveMetadataRepresentation(metadatum, this.parentItem, this.itemType) + } else { + // Check for a configured browse link and return a standard metadata representation + let searchKeyArray: string[] = []; + this.metadataFields.forEach((field: string) => { + searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(field)); + }); + return this.browseDefinitionDataService.findByFields(this.metadataFields).pipe( + getRemoteDataPayload(), + map((def) => Object.assign(new MetadatumRepresentation(this.itemType, def), metadatum)) + ); + } + }), ); } } From 5aab1af5f77f49faf492e3ac7912543f7b17e520 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Mon, 6 Feb 2023 14:23:33 +1300 Subject: [PATCH 17/20] [TLC-380] Lint fixes --- .../metadata-values/metadata-values.component.spec.ts | 2 +- .../metadata-representation-list.component.ts | 2 +- .../browse-link-metadata-list-element.component.spec.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/item-page/field-components/metadata-values/metadata-values.component.spec.ts b/src/app/item-page/field-components/metadata-values/metadata-values.component.spec.ts index c996169e6d..23f8098207 100644 --- a/src/app/item-page/field-components/metadata-values/metadata-values.component.spec.ts +++ b/src/app/item-page/field-components/metadata-values/metadata-values.component.spec.ts @@ -69,7 +69,7 @@ describe('MetadataValuesComponent', () => { }); it('should correctly detect a pattern on string containing "test"', () => { - const mdValue = {value: "This is a test value"} as MetadataValue; + const mdValue = {value: 'This is a test value'} as MetadataValue; expect(comp.hasLink(mdValue)).toBe(true); }); diff --git a/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.ts b/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.ts index 3965be8705..d5e6547778 100644 --- a/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.ts +++ b/src/app/item-page/simple/metadata-representation-list/metadata-representation-list.component.ts @@ -86,7 +86,7 @@ export class MetadataRepresentationListComponent extends AbstractIncrementalList .map((metadatum: any) => Object.assign(new MetadataValue(), metadatum)) .map((metadatum: MetadataValue) => { if (metadatum.isVirtual) { - return this.relationshipService.resolveMetadataRepresentation(metadatum, this.parentItem, this.itemType) + return this.relationshipService.resolveMetadataRepresentation(metadatum, this.parentItem, this.itemType); } else { // Check for a configured browse link and return a standard metadata representation let searchKeyArray: string[] = []; diff --git a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts index 3d7fb5f862..32919d9758 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts @@ -40,7 +40,7 @@ describe('BrowseLinkMetadataListElementComponent', () => { }); it('should NOT match isLink', () => { expect(comp.isLink).toBe(false); - }) + }); }); beforeEach(waitForAsync(() => { @@ -56,7 +56,7 @@ describe('BrowseLinkMetadataListElementComponent', () => { }); it('should match isLink', () => { expect(comp.isLink).toBe(true); - }) + }); }); }); From 793411882336234c6684910a92f259b10173b7e8 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Tue, 7 Feb 2023 11:51:18 +1300 Subject: [PATCH 18/20] [TLC-380] Template link fixes, spec test fixes Correct use of routerLink and queryParams Removed unused method from browse service, specs New spec tests for MetadataRepresentationListElementComponent --- src/app/core/browse/browse.service.ts | 33 ----------- .../metadata-values.component.html | 3 +- .../metadata-values.component.ts | 13 ++++ ...resentation-list-element.component.spec.ts | 59 +++++++++++++++++++ ...a-representation-list-element.component.ts | 4 +- 5 files changed, 76 insertions(+), 36 deletions(-) create mode 100644 src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.spec.ts diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index 280e8e6f5a..be28015069 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -251,37 +251,4 @@ export class BrowseService { ); } - /** - * Get the browse URL by providing a metadatum key and linkPath - * @param metadatumKey - * @param linkPath - */ - getBrowseDefinitionFor(metadataKeys: string[]): Observable { - let searchKeyArray: string[] = []; - metadataKeys.forEach((metadataKey) => { - searchKeyArray = searchKeyArray.concat(BrowseService.toSearchKeyArray(metadataKey)); - }); - return this.getBrowseDefinitions().pipe( - getRemoteDataPayload(), - getPaginatedListPayload(), - map((browseDefinitions: BrowseDefinition[]) => browseDefinitions - .find((def: BrowseDefinition) => { - //console.dir(def.metadataKeys); - const matchingKeys = def.metadataKeys.find((key: string) => searchKeyArray.indexOf(key) >= 0); - //console.dir(matchingKeys); - return isNotEmpty(matchingKeys); - }) - ), - map((def: BrowseDefinition) => { - if (isEmpty(def) || isEmpty(def.id)) { - //throw new Error(`A browse definition for field ${metadataKey} isn't configured`); - } else { - return def; - } - }), - startWith(undefined), - distinctUntilChanged() - ); - } - } diff --git a/src/app/item-page/field-components/metadata-values/metadata-values.component.html b/src/app/item-page/field-components/metadata-values/metadata-values.component.html index e3ee1cefbc..61088edd16 100644 --- a/src/app/item-page/field-components/metadata-values/metadata-values.component.html +++ b/src/app/item-page/field-components/metadata-values/metadata-values.component.html @@ -31,7 +31,8 @@ + [routerLink]="['/browse', browseDefinition.id]" + [queryParams]="getQueryParams(value)"> {{value}} diff --git a/src/app/item-page/field-components/metadata-values/metadata-values.component.ts b/src/app/item-page/field-components/metadata-values/metadata-values.component.ts index b487844e21..49ce453fbe 100644 --- a/src/app/item-page/field-components/metadata-values/metadata-values.component.ts +++ b/src/app/item-page/field-components/metadata-values/metadata-values.component.ts @@ -76,4 +76,17 @@ export class MetadataValuesComponent implements OnChanges { } return false; } + + /** + * Return a queryparams object for use in a link, with the key dependent on whether this browse + * definition is metadata browse, or item browse + * @param value the specific metadata value being linked + */ + getQueryParams(value) { + let queryParams = {startsWith: value}; + if (this.browseDefinition.metadataBrowse) { + return {value: value}; + } + return queryParams; + } } diff --git a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.spec.ts b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.spec.ts new file mode 100644 index 0000000000..8e9b1ca37f --- /dev/null +++ b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.spec.ts @@ -0,0 +1,59 @@ +import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; +import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; +import { MetadatumRepresentation } from '../../../core/shared/metadata-representation/metadatum/metadatum-representation.model'; +import { mockData } from '../../testing/browse-definition-data-service.stub'; +import { MetadataRepresentationListElementComponent } from './metadata-representation-list-element.component'; + +// Mock metadata representation values +const mockMetadataRepresentation = Object.assign(new MetadatumRepresentation('type', mockData[1]), { + key: 'dc.contributor.author', + value: 'Test Author' +}); +const mockMetadataRepresentationUrl = Object.assign(new MetadatumRepresentation('type', mockData[1]), { + key: 'dc.subject', + value: 'https://www.google.com' +}); + +describe('MetadataRepresentationListElementComponent', () => { + let comp: MetadataRepresentationListElementComponent; + let fixture: ComponentFixture; + + beforeEach(waitForAsync(() => { + TestBed.configureTestingModule({ + imports: [], + declarations: [MetadataRepresentationListElementComponent], + schemas: [NO_ERRORS_SCHEMA] + }).overrideComponent(MetadataRepresentationListElementComponent, { + set: { changeDetection: ChangeDetectionStrategy.Default } + }).compileComponents(); + })); + + beforeEach(waitForAsync(() => { + fixture = TestBed.createComponent(MetadataRepresentationListElementComponent); + comp = fixture.componentInstance; + fixture.detectChanges(); + })); + + describe('when the value is not a URL', () => { + beforeEach(() => { + comp.metadataRepresentation = mockMetadataRepresentation; + }); + it('isLink correctly detects a non-URL string as false', () => { + waitForAsync(() => { + expect(comp.isLink()).toBe(false); + }); + }); + }) + + describe('when the value is a URL', () => { + beforeEach(() => { + comp.metadataRepresentation = mockMetadataRepresentationUrl; + }); + it('isLink correctly detects a URL string as true', () => { + waitForAsync(() => { + expect(comp.isLink()).toBe(true); + }); + }); + }) + +}); diff --git a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts index 5b667ed74a..b69f6b37dc 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts @@ -18,8 +18,8 @@ export class MetadataRepresentationListElementComponent { * Returns true if this component's value matches a basic regex "Is this an HTTP URL" test */ isLink(): boolean { - // Match any http:// or https:// - const linkPattern = /^https?\/\//; + // Match any string that begins with http:// or https:// + const linkPattern = new RegExp(/^https?\/\/.*/); return linkPattern.test(this.metadataRepresentation.getValue()); } From ca6f799ee555a88575e3f38ac3cf90c936626124 Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Tue, 7 Feb 2023 14:59:56 +1300 Subject: [PATCH 19/20] [TLC-380] Lint fixes on spec test --- .../metadata-representation-list-element.component.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.spec.ts b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.spec.ts index 8e9b1ca37f..f0cc150b3e 100644 --- a/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.spec.ts +++ b/src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.spec.ts @@ -43,7 +43,7 @@ describe('MetadataRepresentationListElementComponent', () => { expect(comp.isLink()).toBe(false); }); }); - }) + }); describe('when the value is a URL', () => { beforeEach(() => { @@ -54,6 +54,6 @@ describe('MetadataRepresentationListElementComponent', () => { expect(comp.isLink()).toBe(true); }); }); - }) + }); }); From e8a53da766e664fe2a4e684ad8d85b2bef35496c Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Tue, 7 Feb 2023 15:28:18 +1300 Subject: [PATCH 20/20] [TLC-380] Fix item page field test to supply router --- .../specific-field/item-page-field.component.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts index b1579b1cda..15b7a9df21 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts @@ -15,6 +15,7 @@ import { APP_CONFIG } from '../../../../../config/app-config.interface'; import { By } from '@angular/platform-browser'; import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; import { BrowseDefinitionDataServiceStub } from '../../../../shared/testing/browse-definition-data-service.stub'; +import { RouterTestingModule } from '@angular/router/testing'; let comp: ItemPageFieldComponent; let fixture: ComponentFixture; @@ -39,6 +40,7 @@ describe('ItemPageFieldComponent', () => { const buildTestEnvironment = async () => { await TestBed.configureTestingModule({ imports: [ + RouterTestingModule.withRoutes([]), TranslateModule.forRoot({ loader: { provide: TranslateLoader,