From f81d416daf385fdf2c502794c78c4c2f185d7761 Mon Sep 17 00:00:00 2001 From: Joost Date: Wed, 7 Dec 2022 17:28:36 +0100 Subject: [PATCH 01/59] [task 97298][wip] showing domains for which registration can be done --- .../register-email-form.component.html | 4 ++- .../register-email-form.component.ts | 32 ++++++++++++------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/app/register-email-form/register-email-form.component.html b/src/app/register-email-form/register-email-form.component.html index e47eedb6ae..68d0a08d26 100644 --- a/src/app/register-email-form/register-email-form.component.html +++ b/src/app/register-email-form/register-email-form.component.html @@ -1,6 +1,8 @@

{{MESSAGE_PREFIX + '.header'|translate}}

{{MESSAGE_PREFIX + '.info' | translate}}

+ {{valid_mail_domains.length}} +

Accounts can be registered for mail addresses of the domanins {{valid_domain}}

@@ -29,7 +31,7 @@
- + diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index d40629f597..20b79168d1 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -1,11 +1,13 @@ -import { Component, Input, OnInit } from '@angular/core'; -import { EpersonRegistrationService } from '../core/data/eperson-registration.service'; -import { NotificationsService } from '../shared/notifications/notifications.service'; -import { TranslateService } from '@ngx-translate/core'; -import { Router } from '@angular/router'; -import { FormBuilder, FormControl, FormGroup, Validators } from '@angular/forms'; -import { Registration } from '../core/shared/registration.model'; -import { RemoteData } from '../core/data/remote-data'; +import {Component, Input, OnInit} from '@angular/core'; +import {EpersonRegistrationService} from '../core/data/eperson-registration.service'; +import {NotificationsService} from '../shared/notifications/notifications.service'; +import {TranslateService} from '@ngx-translate/core'; +import {Router} from '@angular/router'; +import {FormBuilder, FormControl, FormGroup, Validators} from '@angular/forms'; +import {Registration} from '../core/shared/registration.model'; +import {RemoteData} from '../core/data/remote-data'; +import {ConfigurationDataService} from "../core/data/configuration-data.service"; +import {getFirstCompletedRemoteData} from "../core/shared/operators"; @Component({ selector: 'ds-register-email-form', @@ -27,14 +29,16 @@ export class RegisterEmailFormComponent implements OnInit { @Input() MESSAGE_PREFIX: string; + valid_mail_domains: string[]; + constructor( private epersonRegistrationService: EpersonRegistrationService, private notificationService: NotificationsService, private translateService: TranslateService, private router: Router, - private formBuilder: FormBuilder + private formBuilder: FormBuilder, + private configurationService: ConfigurationDataService ) { - } ngOnInit(): void { @@ -45,7 +49,13 @@ export class RegisterEmailFormComponent implements OnInit { ], }) }); - + this.valid_mail_domains = []; + this.configurationService.findByPropertyName('authentication-password.domain.valid') + .pipe(getFirstCompletedRemoteData()) + .subscribe((remoteData) =>{ + this.valid_mail_domains.push(remoteData.payload.values[0]); + } + ); } /** From e0deed785ef4a21c926c1fbaa17ff224a98504ff Mon Sep 17 00:00:00 2001 From: Joost Date: Mon, 12 Dec 2022 17:27:54 +0100 Subject: [PATCH 02/59] [task 97298] error message for not correct domain in registration mail --- .../register-email-form.component.html | 3 +- .../register-email-form.component.ts | 29 +++++++++++-------- src/assets/i18n/en.json5 | 2 ++ 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/app/register-email-form/register-email-form.component.html b/src/app/register-email-form/register-email-form.component.html index 68d0a08d26..b76fa521e2 100644 --- a/src/app/register-email-form/register-email-form.component.html +++ b/src/app/register-email-form/register-email-form.component.html @@ -1,8 +1,7 @@

{{MESSAGE_PREFIX + '.header'|translate}}

{{MESSAGE_PREFIX + '.info' | translate}}

- {{valid_mail_domains.length}} -

Accounts can be registered for mail addresses of the domanins {{valid_domain}}

+

Accounts can be registered for mail addresses of the domanins {{ valid_mail_domains.join(", ")}}

diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index 20b79168d1..09548def92 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -7,7 +7,7 @@ import {FormBuilder, FormControl, FormGroup, Validators} from '@angular/forms'; import {Registration} from '../core/shared/registration.model'; import {RemoteData} from '../core/data/remote-data'; import {ConfigurationDataService} from "../core/data/configuration-data.service"; -import {getFirstCompletedRemoteData} from "../core/shared/operators"; +import {getAllCompletedRemoteData} from "../core/shared/operators"; @Component({ selector: 'ds-register-email-form', @@ -51,9 +51,11 @@ export class RegisterEmailFormComponent implements OnInit { }); this.valid_mail_domains = []; this.configurationService.findByPropertyName('authentication-password.domain.valid') - .pipe(getFirstCompletedRemoteData()) - .subscribe((remoteData) =>{ - this.valid_mail_domains.push(remoteData.payload.values[0]); + .pipe(getAllCompletedRemoteData()) + .subscribe((remoteData) => { + for (const remoteValue of remoteData.payload.values) { + this.valid_mail_domains.push(remoteValue); + } } ); } @@ -64,14 +66,17 @@ export class RegisterEmailFormComponent implements OnInit { register() { if (!this.form.invalid) { this.epersonRegistrationService.registerEmail(this.email.value).subscribe((response: RemoteData) => { - if (response.hasSucceeded) { - this.notificationService.success(this.translateService.get(`${this.MESSAGE_PREFIX}.success.head`), - this.translateService.get(`${this.MESSAGE_PREFIX}.success.content`, {email: this.email.value})); - this.router.navigate(['/home']); - } else { - this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), - this.translateService.get(`${this.MESSAGE_PREFIX}.error.content`, {email: this.email.value})); - } + if (response.hasSucceeded) { + this.notificationService.success(this.translateService.get(`${this.MESSAGE_PREFIX}.success.head`), + this.translateService.get(`${this.MESSAGE_PREFIX}.success.content`, {email: this.email.value})); + this.router.navigate(['/home']); + } else if (response.statusCode == 400) { + this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.error.maildomain`, { domains: this.valid_mail_domains.join(", ")})); + } else { + this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), + this.translateService.get(`${this.MESSAGE_PREFIX}.error.content`, {email: this.email.value})); + this.notificationService.error("title", response.errorMessage); + } } ); } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index f742273edb..a98eb50cac 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3027,6 +3027,8 @@ "register-page.registration.error.content": "An error occured when registering the following email address: {{ email }}", + "register-page.registration.error.maildomain": "this email address is not on the list of domains who can register. Allowed domains are {{ domains }}", + "relationships.add.error.relationship-type.content": "No suitable match could be found for relationship type {{ type }} between the two items", From 03fe9a3ab9c385d595094a9c39b9610b1cd7c673 Mon Sep 17 00:00:00 2001 From: Joost Date: Tue, 13 Dec 2022 11:11:42 +0100 Subject: [PATCH 03/59] typo fix --- src/app/register-email-form/register-email-form.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/register-email-form/register-email-form.component.html b/src/app/register-email-form/register-email-form.component.html index 9a438d3e92..81d425c468 100644 --- a/src/app/register-email-form/register-email-form.component.html +++ b/src/app/register-email-form/register-email-form.component.html @@ -1,7 +1,7 @@

{{MESSAGE_PREFIX + '.header'|translate}}

{{MESSAGE_PREFIX + '.info' | translate}}

-

Accounts can be registered for mail addresses of the domanins {{ valid_mail_domains.join(", ")}}

+

Accounts can be registered for mail addresses of the domains {{ valid_mail_domains.join(", ")}}

From 7917f612106b63810eb3ed5556eb1ccc005e0ce7 Mon Sep 17 00:00:00 2001 From: Joost Date: Tue, 13 Dec 2022 11:11:42 +0100 Subject: [PATCH 04/59] typo fix --- src/app/register-email-form/register-email-form.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/register-email-form/register-email-form.component.html b/src/app/register-email-form/register-email-form.component.html index b76fa521e2..15c68d322f 100644 --- a/src/app/register-email-form/register-email-form.component.html +++ b/src/app/register-email-form/register-email-form.component.html @@ -1,7 +1,7 @@

{{MESSAGE_PREFIX + '.header'|translate}}

{{MESSAGE_PREFIX + '.info' | translate}}

-

Accounts can be registered for mail addresses of the domanins {{ valid_mail_domains.join(", ")}}

+

Accounts can be registered for mail addresses of the domains {{ valid_mail_domains.join(", ")}}

From 8f6db1f6a095c6eeb57bf5f859c2e2ca15b1553f Mon Sep 17 00:00:00 2001 From: Joost Date: Mon, 19 Dec 2022 16:43:02 +0100 Subject: [PATCH 05/59] lint fixes --- .../register-email-form.component.html | 2 +- .../register-email-form.component.ts | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/app/register-email-form/register-email-form.component.html b/src/app/register-email-form/register-email-form.component.html index 15c68d322f..ee620e1e6d 100644 --- a/src/app/register-email-form/register-email-form.component.html +++ b/src/app/register-email-form/register-email-form.component.html @@ -1,7 +1,7 @@

{{MESSAGE_PREFIX + '.header'|translate}}

{{MESSAGE_PREFIX + '.info' | translate}}

-

Accounts can be registered for mail addresses of the domains {{ valid_mail_domains.join(", ")}}

+

Accounts can be registered for mail addresses of the domains {{ validMailDomains.join(", ")}}

diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index 09548def92..6c83e9a4de 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -6,8 +6,8 @@ import {Router} from '@angular/router'; import {FormBuilder, FormControl, FormGroup, Validators} from '@angular/forms'; import {Registration} from '../core/shared/registration.model'; import {RemoteData} from '../core/data/remote-data'; -import {ConfigurationDataService} from "../core/data/configuration-data.service"; -import {getAllCompletedRemoteData} from "../core/shared/operators"; +import {ConfigurationDataService} from '../core/data/configuration-data.service'; +import {getAllCompletedRemoteData} from '../core/shared/operators'; @Component({ selector: 'ds-register-email-form', @@ -29,7 +29,7 @@ export class RegisterEmailFormComponent implements OnInit { @Input() MESSAGE_PREFIX: string; - valid_mail_domains: string[]; + validMailDomains: string[]; constructor( private epersonRegistrationService: EpersonRegistrationService, @@ -49,12 +49,12 @@ export class RegisterEmailFormComponent implements OnInit { ], }) }); - this.valid_mail_domains = []; + this.validMailDomains = []; this.configurationService.findByPropertyName('authentication-password.domain.valid') .pipe(getAllCompletedRemoteData()) .subscribe((remoteData) => { for (const remoteValue of remoteData.payload.values) { - this.valid_mail_domains.push(remoteValue); + this.validMailDomains.push(remoteValue); } } ); @@ -70,12 +70,12 @@ export class RegisterEmailFormComponent implements OnInit { this.notificationService.success(this.translateService.get(`${this.MESSAGE_PREFIX}.success.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.success.content`, {email: this.email.value})); this.router.navigate(['/home']); - } else if (response.statusCode == 400) { - this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.error.maildomain`, { domains: this.valid_mail_domains.join(", ")})); + } else if (response.statusCode === 400) { + this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.error.maildomain`, { domains: this.validMailDomains.join(', ')})); } else { this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.error.content`, {email: this.email.value})); - this.notificationService.error("title", response.errorMessage); + this.notificationService.error('title', response.errorMessage); } } ); From 3690dceb75dc027830df25345ec5a6537aeaf75f Mon Sep 17 00:00:00 2001 From: Joost Date: Tue, 20 Dec 2022 14:54:54 +0100 Subject: [PATCH 06/59] [task 97298] sending the type for registration --- src/app/core/data/eperson-registration.service.ts | 7 +++++-- .../register-email-form/register-email-form.component.ts | 6 +++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.ts b/src/app/core/data/eperson-registration.service.ts index adf01b0ce9..e1bdde262a 100644 --- a/src/app/core/data/eperson-registration.service.ts +++ b/src/app/core/data/eperson-registration.service.ts @@ -12,6 +12,7 @@ import { GenericConstructor } from '../shared/generic-constructor'; import { RegistrationResponseParsingService } from './registration-response-parsing.service'; import { RemoteData } from './remote-data'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import {HttpParams} from "@angular/common/http"; @Injectable( { @@ -54,7 +55,7 @@ export class EpersonRegistrationService { * Register a new email address * @param email */ - registerEmail(email: string): Observable> { + registerEmail(email: string, type?:string): Observable> { const registration = new Registration(); registration.email = email; @@ -62,10 +63,12 @@ export class EpersonRegistrationService { const href$ = this.getRegistrationEndpoint(); + let options = type? {params: new HttpParams({fromString:'type='+type})}: {} + href$.pipe( find((href: string) => hasValue(href)), map((href: string) => { - const request = new PostRequest(requestId, href, registration); + const request = new PostRequest(requestId, href, registration, options); this.requestService.send(request); }) ).subscribe(); diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index 6c83e9a4de..5daaf1d41c 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -64,8 +64,12 @@ export class RegisterEmailFormComponent implements OnInit { * Register an email address */ register() { + let typeMap = new Map([ + ["register-page.registration", "register"], + ["forgot-email.form", "forgot"] + ]); if (!this.form.invalid) { - this.epersonRegistrationService.registerEmail(this.email.value).subscribe((response: RemoteData) => { + this.epersonRegistrationService.registerEmail(this.email.value, typeMap.get(this.MESSAGE_PREFIX)).subscribe((response: RemoteData) => { if (response.hasSucceeded) { this.notificationService.success(this.translateService.get(`${this.MESSAGE_PREFIX}.success.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.success.content`, {email: this.email.value})); From 3f7bdb284fae305239adb0946c2a1402847b8045 Mon Sep 17 00:00:00 2001 From: Joost Date: Tue, 20 Dec 2022 15:01:53 +0100 Subject: [PATCH 07/59] [task 97298] displaying mail domains only on register --- src/app/register-email-form/register-email-form.component.html | 3 ++- src/assets/i18n/en.json5 | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/register-email-form/register-email-form.component.html b/src/app/register-email-form/register-email-form.component.html index ee620e1e6d..a1eb0cef98 100644 --- a/src/app/register-email-form/register-email-form.component.html +++ b/src/app/register-email-form/register-email-form.component.html @@ -1,7 +1,8 @@

{{MESSAGE_PREFIX + '.header'|translate}}

{{MESSAGE_PREFIX + '.info' | translate}}

-

Accounts can be registered for mail addresses of the domains {{ validMailDomains.join(", ")}}

+

{{ MESSAGE_PREFIX + '.info.maildomain' | translate}} {{ validMailDomains.join(", ")}}

diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index a98eb50cac..8737bed9b3 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3029,7 +3029,7 @@ "register-page.registration.error.maildomain": "this email address is not on the list of domains who can register. Allowed domains are {{ domains }}", - + "register-page.registration.info.maildomain": "Accounts can be registered for mail addresses of the domains", "relationships.add.error.relationship-type.content": "No suitable match could be found for relationship type {{ type }} between the two items", From 8b71ad6735c9c51fea06ad5557f83567aabb551c Mon Sep 17 00:00:00 2001 From: Joost Date: Tue, 20 Dec 2022 15:36:15 +0100 Subject: [PATCH 08/59] [task 97198] fix merge mistakes --- .../register-email-form.component.ts | 46 +++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index 42a015b3d7..d9c54d211e 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -1,21 +1,21 @@ -import { ChangeDetectorRef, Component, Input, OnInit, Optional } from '@angular/core'; -import { EpersonRegistrationService } from '../core/data/eperson-registration.service'; -import { NotificationsService } from '../shared/notifications/notifications.service'; -import { TranslateService } from '@ngx-translate/core'; -import { Router } from '@angular/router'; -import { FormBuilder, FormControl, FormGroup, Validators } from '@angular/forms'; -import { Registration } from '../core/shared/registration.model'; -import { RemoteData } from '../core/data/remote-data'; -import { ConfigurationDataService } from '../core/data/configuration-data.service'; +import {ChangeDetectorRef, Component, Input, OnInit, Optional} from '@angular/core'; +import {EpersonRegistrationService} from '../core/data/eperson-registration.service'; +import {NotificationsService} from '../shared/notifications/notifications.service'; +import {TranslateService} from '@ngx-translate/core'; +import {Router} from '@angular/router'; +import {FormBuilder, FormControl, FormGroup, Validators} from '@angular/forms'; +import {Registration} from '../core/shared/registration.model'; +import {RemoteData} from '../core/data/remote-data'; +import {ConfigurationDataService} from '../core/data/configuration-data.service'; import {getAllCompletedRemoteData, getFirstSucceededRemoteDataPayload} from '../core/shared/operators'; -import { ConfigurationProperty } from '../core/shared/configuration-property.model'; -import { isNotEmpty } from '../shared/empty.util'; -import { BehaviorSubject, combineLatest, Observable, of, switchMap } from 'rxjs'; -import { map, startWith, take } from 'rxjs/operators'; -import { CAPTCHA_NAME, GoogleRecaptchaService } from '../core/google-recaptcha/google-recaptcha.service'; -import { AlertType } from '../shared/alert/aletr-type'; -import { KlaroService } from '../shared/cookies/klaro.service'; -import { CookieService } from '../core/services/cookie.service'; +import {ConfigurationProperty} from '../core/shared/configuration-property.model'; +import {isNotEmpty} from '../shared/empty.util'; +import {BehaviorSubject, combineLatest, Observable, of, switchMap} from 'rxjs'; +import {map, startWith, take} from 'rxjs/operators'; +import {CAPTCHA_NAME, GoogleRecaptchaService} from '../core/google-recaptcha/google-recaptcha.service'; +import {AlertType} from '../shared/alert/aletr-type'; +import {KlaroService} from '../shared/cookies/klaro.service'; +import {CookieService} from '../core/services/cookie.service'; @Component({ selector: 'ds-register-email-form', @@ -86,6 +86,14 @@ export class RegisterEmailFormComponent implements OnInit { }) }); this.validMailDomains = []; + this.configService.findByPropertyName('authentication-password.domain.valid') + .pipe(getAllCompletedRemoteData()) + .subscribe((remoteData) => { + for (const remoteValue of remoteData.payload.values) { + this.validMailDomains.push(remoteValue); + } + } + ); this.configService.findByPropertyName('registration.verification.enabled').pipe( getFirstSucceededRemoteDataPayload(), map((res: ConfigurationProperty) => res?.values[0].toLowerCase() === 'true') @@ -151,12 +159,14 @@ export class RegisterEmailFormComponent implements OnInit { ]); let registerEmail$ = captchaToken ? this.epersonRegistrationService.registerEmail(this.email.value, captchaToken, typeMap.get(this.MESSAGE_PREFIX)) : - this.epersonRegistrationService.registerEmail(this.email.value,typeMap.get(this.MESSAGE_PREFIX)); + this.epersonRegistrationService.registerEmail(this.email.value, null, typeMap.get(this.MESSAGE_PREFIX)); registerEmail$.subscribe((response: RemoteData) => { if (response.hasSucceeded) { this.notificationService.success(this.translateService.get(`${this.MESSAGE_PREFIX}.success.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.success.content`, {email: this.email.value})); this.router.navigate(['/home']); + } else if (response.statusCode === 400) { + this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.error.maildomain`, {domains: this.validMailDomains.join(', ')})); } else { this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.error.content`, {email: this.email.value})); From c4f9bc4b1ae01c0b14ccfd932b7f16c459746155 Mon Sep 17 00:00:00 2001 From: Joost Date: Tue, 20 Dec 2022 15:45:51 +0100 Subject: [PATCH 09/59] lint fixes --- src/app/core/data/eperson-registration.service.ts | 6 +++--- .../register-email-form/register-email-form.component.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.ts b/src/app/core/data/eperson-registration.service.ts index e1bdde262a..24c2263fd1 100644 --- a/src/app/core/data/eperson-registration.service.ts +++ b/src/app/core/data/eperson-registration.service.ts @@ -12,7 +12,7 @@ import { GenericConstructor } from '../shared/generic-constructor'; import { RegistrationResponseParsingService } from './registration-response-parsing.service'; import { RemoteData } from './remote-data'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; -import {HttpParams} from "@angular/common/http"; +import {HttpParams} from '@angular/common/http'; @Injectable( { @@ -55,7 +55,7 @@ export class EpersonRegistrationService { * Register a new email address * @param email */ - registerEmail(email: string, type?:string): Observable> { + registerEmail(email: string, type?: string): Observable> { const registration = new Registration(); registration.email = email; @@ -63,7 +63,7 @@ export class EpersonRegistrationService { const href$ = this.getRegistrationEndpoint(); - let options = type? {params: new HttpParams({fromString:'type='+type})}: {} + const options = type ? {params: new HttpParams({fromString:'type=' + type})} : {}; href$.pipe( find((href: string) => hasValue(href)), diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index 5daaf1d41c..9e7b783544 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -64,9 +64,9 @@ export class RegisterEmailFormComponent implements OnInit { * Register an email address */ register() { - let typeMap = new Map([ - ["register-page.registration", "register"], - ["forgot-email.form", "forgot"] + const typeMap = new Map([ + ['register-page.registration', 'register'], + ['forgot-email.form', 'forgot'] ]); if (!this.form.invalid) { this.epersonRegistrationService.registerEmail(this.email.value, typeMap.get(this.MESSAGE_PREFIX)).subscribe((response: RemoteData) => { From cbf6f24d8710a92f12a66cef5aaca4414d4fa3b5 Mon Sep 17 00:00:00 2001 From: Joost Date: Tue, 20 Dec 2022 15:50:31 +0100 Subject: [PATCH 10/59] lint fixes --- src/app/core/data/eperson-registration.service.ts | 4 ++-- src/app/register-email-form/register-email-form.component.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.ts b/src/app/core/data/eperson-registration.service.ts index 80bc56c037..436291f34a 100644 --- a/src/app/core/data/eperson-registration.service.ts +++ b/src/app/core/data/eperson-registration.service.ts @@ -14,7 +14,7 @@ import { RemoteData } from './remote-data'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { HttpOptions } from '../dspace-rest/dspace-rest.service'; import { HttpHeaders } from '@angular/common/http'; -import {HttpParams} from "@angular/common/http"; +import {HttpParams} from '@angular/common/http'; @Injectable({ providedIn: 'root', @@ -70,7 +70,7 @@ export class EpersonRegistrationService { headers = headers.append('x-recaptcha-token', captchaToken); } options.headers = headers; - options.params = type? new HttpParams({fromString:'type='+type}): new HttpParams(); + options.params = type ? new HttpParams({fromString:'type=' + type}) : new HttpParams(); href$.pipe( find((href: string) => hasValue(href)), diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index d9c54d211e..252fb88953 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -154,8 +154,8 @@ export class RegisterEmailFormComponent implements OnInit { */ registration(captchaToken = null) { let typeMap = new Map([ - ["register-page.registration", "register"], - ["forgot-email.form", "forgot"] + ['register-page.registration', 'register'], + ['forgot-email.form', 'forgot'] ]); let registerEmail$ = captchaToken ? this.epersonRegistrationService.registerEmail(this.email.value, captchaToken, typeMap.get(this.MESSAGE_PREFIX)) : From 4f960231da938292843f6c4c29910c767a0c528e Mon Sep 17 00:00:00 2001 From: Joost Date: Tue, 20 Dec 2022 15:57:54 +0100 Subject: [PATCH 11/59] lint fixes --- src/app/register-email-form/register-email-form.component.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index 252fb88953..0da026ac1a 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -51,6 +51,9 @@ export class RegisterEmailFormComponent implements OnInit { disableUntilChecked = true; + validMailDomains: string[]; + + captchaVersion(): Observable { return this.googleRecaptchaService.captchaVersion(); } @@ -58,7 +61,6 @@ export class RegisterEmailFormComponent implements OnInit { captchaMode(): Observable { return this.googleRecaptchaService.captchaMode(); } - validMailDomains: string[]; constructor( private epersonRegistrationService: EpersonRegistrationService, From e4ce91fa73fa2d5bb90be43198d2b6baef8e2a53 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Wed, 28 Dec 2022 11:57:48 +0100 Subject: [PATCH 12/59] 97298: #3281 Self-register - type request query param --- .../forgot-email.component.html | 4 ++-- .../register-email-form.component.ts | 12 +++++++----- .../register-email/register-email.component.html | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/app/forgot-password/forgot-password-email/forgot-email.component.html b/src/app/forgot-password/forgot-password-email/forgot-email.component.html index 263f142c2e..8be86f17ae 100644 --- a/src/app/forgot-password/forgot-password-email/forgot-email.component.html +++ b/src/app/forgot-password/forgot-password-email/forgot-email.component.html @@ -1,3 +1,3 @@ - \ No newline at end of file + [MESSAGE_PREFIX]="'forgot-email.form'" [typeRequest]="'forgot'"> + diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index 9e7b783544..f2a338537d 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -29,6 +29,12 @@ export class RegisterEmailFormComponent implements OnInit { @Input() MESSAGE_PREFIX: string; + /** + * Type of register request to be done, register new email or forgot password (same endpoint) + */ + @Input() + typeRequest: string = null; + validMailDomains: string[]; constructor( @@ -64,12 +70,8 @@ export class RegisterEmailFormComponent implements OnInit { * Register an email address */ register() { - const typeMap = new Map([ - ['register-page.registration', 'register'], - ['forgot-email.form', 'forgot'] - ]); if (!this.form.invalid) { - this.epersonRegistrationService.registerEmail(this.email.value, typeMap.get(this.MESSAGE_PREFIX)).subscribe((response: RemoteData) => { + this.epersonRegistrationService.registerEmail(this.email.value, this.typeRequest).subscribe((response: RemoteData) => { if (response.hasSucceeded) { this.notificationService.success(this.translateService.get(`${this.MESSAGE_PREFIX}.success.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.success.content`, {email: this.email.value})); diff --git a/src/app/register-page/register-email/register-email.component.html b/src/app/register-page/register-email/register-email.component.html index a60dc4c31e..80b6885272 100644 --- a/src/app/register-page/register-email/register-email.component.html +++ b/src/app/register-page/register-email/register-email.component.html @@ -1,3 +1,3 @@ + [MESSAGE_PREFIX]="'register-page.registration'" [typeRequest]="'register'"> From 31b17731f25403f79b04b2d2966f340efbda8198 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Wed, 28 Dec 2022 12:28:27 +0100 Subject: [PATCH 13/59] 97298: #3281 Self-register - test fixes --- .../data/eperson-registration.service.spec.ts | 4 +++- .../core/data/eperson-registration.service.ts | 6 +++++- .../register-email-form.component.spec.ts | 19 ++++++++++++++++--- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.spec.ts b/src/app/core/data/eperson-registration.service.spec.ts index 2407249615..32688d050b 100644 --- a/src/app/core/data/eperson-registration.service.spec.ts +++ b/src/app/core/data/eperson-registration.service.spec.ts @@ -9,6 +9,7 @@ import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-servic import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; import { of as observableOf } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; +import { HttpOptions } from '../dspace-rest/dspace-rest.service'; describe('EpersonRegistrationService', () => { let testScheduler; @@ -79,8 +80,9 @@ describe('EpersonRegistrationService', () => { it('should send an email registration', () => { const expected = service.registerEmail('test@mail.org'); + const options: HttpOptions = Object.create({}); - expect(requestService.send).toHaveBeenCalledWith(new PostRequest('request-id', 'rest-url/registrations', registration)); + expect(requestService.send).toHaveBeenCalledWith(new PostRequest('request-id', 'rest-url/registrations', registration, options)); expect(expected).toBeObservable(cold('(a|)', { a: rd })); }); }); diff --git a/src/app/core/data/eperson-registration.service.ts b/src/app/core/data/eperson-registration.service.ts index 24c2263fd1..6a8d9c94f7 100644 --- a/src/app/core/data/eperson-registration.service.ts +++ b/src/app/core/data/eperson-registration.service.ts @@ -13,6 +13,7 @@ import { RegistrationResponseParsingService } from './registration-response-pars import { RemoteData } from './remote-data'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import {HttpParams} from '@angular/common/http'; +import { HttpOptions } from '../dspace-rest/dspace-rest.service'; @Injectable( { @@ -63,7 +64,10 @@ export class EpersonRegistrationService { const href$ = this.getRegistrationEndpoint(); - const options = type ? {params: new HttpParams({fromString:'type=' + type})} : {}; + const options: HttpOptions = Object.create({}); + if (hasValue(type)) { + options.params = type ? new HttpParams({ fromString: 'type=' + type }) : new HttpParams(); + } href$.pipe( find((href: string) => hasValue(href)), diff --git a/src/app/register-email-form/register-email-form.component.spec.ts b/src/app/register-email-form/register-email-form.component.spec.ts index a415ef4808..585e69405a 100644 --- a/src/app/register-email-form/register-email-form.component.spec.ts +++ b/src/app/register-email-form/register-email-form.component.spec.ts @@ -14,8 +14,10 @@ import { RouterStub } from '../shared/testing/router.stub'; import { NotificationsServiceStub } from '../shared/testing/notifications-service.stub'; import { RegisterEmailFormComponent } from './register-email-form.component'; import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; +import { ConfigurationDataService } from '../core/data/configuration-data.service'; +import { ConfigurationProperty } from '../core/shared/configuration-property.model'; -describe('RegisterEmailComponent', () => { +describe('RegisterEmailFormComponent', () => { let comp: RegisterEmailFormComponent; let fixture: ComponentFixture; @@ -23,6 +25,7 @@ describe('RegisterEmailComponent', () => { let router; let epersonRegistrationService: EpersonRegistrationService; let notificationsService; + let configurationDataService: ConfigurationDataService; beforeEach(waitForAsync(() => { @@ -33,6 +36,15 @@ describe('RegisterEmailComponent', () => { registerEmail: createSuccessfulRemoteDataObject$({}) }); + configurationDataService = jasmine.createSpyObj('configurationDataService', { + findByPropertyName: createSuccessfulRemoteDataObject$(Object.assign(new ConfigurationProperty(), { + name: 'authentication-password.domain.valid', + values: [ + 'example.com, @gmail.com' + ] + })) + }); + TestBed.configureTestingModule({ imports: [CommonModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), ReactiveFormsModule], declarations: [RegisterEmailFormComponent], @@ -41,6 +53,7 @@ describe('RegisterEmailComponent', () => { {provide: EpersonRegistrationService, useValue: epersonRegistrationService}, {provide: FormBuilder, useValue: new FormBuilder()}, {provide: NotificationsService, useValue: notificationsService}, + {provide: ConfigurationDataService, useValue: configurationDataService}, ], schemas: [CUSTOM_ELEMENTS_SCHEMA] }).compileComponents(); @@ -75,7 +88,7 @@ describe('RegisterEmailComponent', () => { comp.form.patchValue({email: 'valid@email.org'}); comp.register(); - expect(epersonRegistrationService.registerEmail).toHaveBeenCalledWith('valid@email.org'); + expect(epersonRegistrationService.registerEmail).toHaveBeenCalledWith('valid@email.org', null); expect(notificationsService.success).toHaveBeenCalled(); expect(router.navigate).toHaveBeenCalledWith(['/home']); }); @@ -85,7 +98,7 @@ describe('RegisterEmailComponent', () => { comp.form.patchValue({email: 'valid@email.org'}); comp.register(); - expect(epersonRegistrationService.registerEmail).toHaveBeenCalledWith('valid@email.org'); + expect(epersonRegistrationService.registerEmail).toHaveBeenCalledWith('valid@email.org', null); expect(notificationsService.error).toHaveBeenCalled(); expect(router.navigate).not.toHaveBeenCalled(); }); From 66c6872b0051c86867afade4de7d0f8daa97eb0f Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Wed, 25 Jan 2023 15:38:48 +0100 Subject: [PATCH 14/59] 97061: type request param name change to avoid confusion with rest object type & non valid email domain error code changed --- src/app/core/data/eperson-registration.service.ts | 3 ++- src/app/register-email-form/register-email-form.component.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.ts b/src/app/core/data/eperson-registration.service.ts index 6a8d9c94f7..67f5116953 100644 --- a/src/app/core/data/eperson-registration.service.ts +++ b/src/app/core/data/eperson-registration.service.ts @@ -66,7 +66,8 @@ export class EpersonRegistrationService { const options: HttpOptions = Object.create({}); if (hasValue(type)) { - options.params = type ? new HttpParams({ fromString: 'type=' + type }) : new HttpParams(); + options.params = type ? + new HttpParams({ fromString: 'accountRequestType=' + type }) : new HttpParams(); } href$.pipe( diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index f2a338537d..dc742d6760 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -76,7 +76,7 @@ export class RegisterEmailFormComponent implements OnInit { this.notificationService.success(this.translateService.get(`${this.MESSAGE_PREFIX}.success.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.success.content`, {email: this.email.value})); this.router.navigate(['/home']); - } else if (response.statusCode === 400) { + } else if (response.statusCode === 422) { this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), this.translateService.get(`${this.MESSAGE_PREFIX}.error.maildomain`, { domains: this.validMailDomains.join(', ')})); } else { this.notificationService.error(this.translateService.get(`${this.MESSAGE_PREFIX}.error.head`), From 0d7a030960116e6635f5dacd9c007c992dbd9d9d Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Wed, 25 Jan 2023 17:46:21 +0100 Subject: [PATCH 15/59] 98863: Fixed typo and added domain name validation on input field --- .../register-email-form.component.ts | 23 ++++++++++++++----- src/assets/i18n/en.json5 | 2 +- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index dc742d6760..9269a0cb2d 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -3,11 +3,12 @@ import {EpersonRegistrationService} from '../core/data/eperson-registration.serv import {NotificationsService} from '../shared/notifications/notifications.service'; import {TranslateService} from '@ngx-translate/core'; import {Router} from '@angular/router'; -import {FormBuilder, FormControl, FormGroup, Validators} from '@angular/forms'; +import { FormBuilder, FormControl, FormGroup, Validators, ValidatorFn } from '@angular/forms'; import {Registration} from '../core/shared/registration.model'; import {RemoteData} from '../core/data/remote-data'; import {ConfigurationDataService} from '../core/data/configuration-data.service'; import {getAllCompletedRemoteData} from '../core/shared/operators'; +import { ConfigurationProperty } from '../core/shared/configuration-property.model'; @Component({ selector: 'ds-register-email-form', @@ -48,22 +49,32 @@ export class RegisterEmailFormComponent implements OnInit { } ngOnInit(): void { + const validators: ValidatorFn[] = [ + Validators.required, + Validators.pattern('^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,4}$') + ]; this.form = this.formBuilder.group({ email: new FormControl('', { - validators: [Validators.required, - Validators.pattern('^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,4}$') - ], + validators: validators, }) }); this.validMailDomains = []; this.configurationService.findByPropertyName('authentication-password.domain.valid') .pipe(getAllCompletedRemoteData()) - .subscribe((remoteData) => { + .subscribe((remoteData: RemoteData) => { + if (remoteData.payload) { for (const remoteValue of remoteData.payload.values) { this.validMailDomains.push(remoteValue); + if (this.validMailDomains.length !== 0 && this.MESSAGE_PREFIX === 'register-page.registration') { + this.form.get('email').setValidators([ + ...validators, + Validators.pattern(this.validMailDomains.map((domain: string) => '(^.*@' + domain.replace(new RegExp('\\.', 'g'), '\\.') + '$)').join('|')), + ]); + this.form.updateValueAndValidity(); + } } } - ); + }); } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 8737bed9b3..511d923f7d 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3027,7 +3027,7 @@ "register-page.registration.error.content": "An error occured when registering the following email address: {{ email }}", - "register-page.registration.error.maildomain": "this email address is not on the list of domains who can register. Allowed domains are {{ domains }}", + "register-page.registration.error.maildomain": "This email address is not on the list of domains who can register. Allowed domains are {{ domains }}", "register-page.registration.info.maildomain": "Accounts can be registered for mail addresses of the domains", From 31d86eeb8cac289085d61477ce31f753e9111f91 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Thu, 26 Jan 2023 14:24:52 +0100 Subject: [PATCH 16/59] 98863: Domain validator fix + error message --- .../forgot-email.component.html | 2 +- .../forgot-email.component.ts | 3 ++- .../register-email-form.component.html | 7 +++++-- .../register-email-form.component.ts | 17 +++++++++++++++-- .../register-email.component.html | 2 +- .../register-email/register-email.component.ts | 3 ++- src/assets/i18n/en.json5 | 6 ++++-- 7 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/app/forgot-password/forgot-password-email/forgot-email.component.html b/src/app/forgot-password/forgot-password-email/forgot-email.component.html index 8be86f17ae..995108cdbc 100644 --- a/src/app/forgot-password/forgot-password-email/forgot-email.component.html +++ b/src/app/forgot-password/forgot-password-email/forgot-email.component.html @@ -1,3 +1,3 @@ + [MESSAGE_PREFIX]="'forgot-email.form'" [typeRequest]="typeRequest"> diff --git a/src/app/forgot-password/forgot-password-email/forgot-email.component.ts b/src/app/forgot-password/forgot-password-email/forgot-email.component.ts index af482bdb67..66a61ed7ee 100644 --- a/src/app/forgot-password/forgot-password-email/forgot-email.component.ts +++ b/src/app/forgot-password/forgot-password-email/forgot-email.component.ts @@ -1,4 +1,5 @@ import { Component } from '@angular/core'; +import { TYPE_REQUEST_FORGOT } from '../../register-email-form/register-email-form.component'; @Component({ selector: 'ds-forgot-email', @@ -9,5 +10,5 @@ import { Component } from '@angular/core'; * Component responsible the forgot password email step */ export class ForgotEmailComponent { - + typeRequest = TYPE_REQUEST_FORGOT; } diff --git a/src/app/register-email-form/register-email-form.component.html b/src/app/register-email-form/register-email-form.component.html index a1eb0cef98..8cc0d293cf 100644 --- a/src/app/register-email-form/register-email-form.component.html +++ b/src/app/register-email-form/register-email-form.component.html @@ -18,8 +18,11 @@ {{ MESSAGE_PREFIX + '.email.error.required' | translate }} - - {{ MESSAGE_PREFIX + '.email.error.pattern' | translate }} + + {{ MESSAGE_PREFIX + '.email.error.not-email-form' | translate }} + + + {{ MESSAGE_PREFIX + '.email.error.not-valid-domain' | translate: { domains: validMailDomains } }}
diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index 9269a0cb2d..83053bd345 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -10,6 +10,9 @@ import {ConfigurationDataService} from '../core/data/configuration-data.service' import {getAllCompletedRemoteData} from '../core/shared/operators'; import { ConfigurationProperty } from '../core/shared/configuration-property.model'; +export const TYPE_REQUEST_FORGOT = 'forgot'; +export const TYPE_REQUEST_REGISTER = 'register'; + @Component({ selector: 'ds-register-email-form', templateUrl: './register-email-form.component.html' @@ -37,6 +40,15 @@ export class RegisterEmailFormComponent implements OnInit { typeRequest: string = null; validMailDomains: string[]; + TYPE_REQUEST_REGISTER = TYPE_REQUEST_REGISTER; + + captchaVersion(): Observable { + return this.googleRecaptchaService.captchaVersion(); + } + + captchaMode(): Observable { + return this.googleRecaptchaService.captchaMode(); + } constructor( private epersonRegistrationService: EpersonRegistrationService, @@ -51,6 +63,7 @@ export class RegisterEmailFormComponent implements OnInit { ngOnInit(): void { const validators: ValidatorFn[] = [ Validators.required, + Validators.email, Validators.pattern('^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,4}$') ]; this.form = this.formBuilder.group({ @@ -65,10 +78,10 @@ export class RegisterEmailFormComponent implements OnInit { if (remoteData.payload) { for (const remoteValue of remoteData.payload.values) { this.validMailDomains.push(remoteValue); - if (this.validMailDomains.length !== 0 && this.MESSAGE_PREFIX === 'register-page.registration') { + if (this.validMailDomains.length !== 0 && this.typeRequest === TYPE_REQUEST_REGISTER) { this.form.get('email').setValidators([ ...validators, - Validators.pattern(this.validMailDomains.map((domain: string) => '(^.*@' + domain.replace(new RegExp('\\.', 'g'), '\\.') + '$)').join('|')), + Validators.pattern(this.validMailDomains.map((domain: string) => '(^.*' + domain.replace(new RegExp('\\.', 'g'), '\\.') + '$)').join('|')), ]); this.form.updateValueAndValidity(); } diff --git a/src/app/register-page/register-email/register-email.component.html b/src/app/register-page/register-email/register-email.component.html index 80b6885272..1829bb2914 100644 --- a/src/app/register-page/register-email/register-email.component.html +++ b/src/app/register-page/register-email/register-email.component.html @@ -1,3 +1,3 @@ + [MESSAGE_PREFIX]="'register-page.registration'" [typeRequest]="typeRequest"> diff --git a/src/app/register-page/register-email/register-email.component.ts b/src/app/register-page/register-email/register-email.component.ts index 7b7b0f631b..228e8c56a0 100644 --- a/src/app/register-page/register-email/register-email.component.ts +++ b/src/app/register-page/register-email/register-email.component.ts @@ -1,4 +1,5 @@ import { Component } from '@angular/core'; +import { TYPE_REQUEST_REGISTER } from '../../register-email-form/register-email-form.component'; @Component({ selector: 'ds-register-email', @@ -9,5 +10,5 @@ import { Component } from '@angular/core'; * Component responsible the email registration step when registering as a new user */ export class RegisterEmailComponent { - + typeRequest = TYPE_REQUEST_REGISTER; } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 511d923f7d..7a011ecff5 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1406,7 +1406,7 @@ "forgot-email.form.email.error.required": "Please fill in an email address", - "forgot-email.form.email.error.pattern": "Please fill in a valid email address", + "forgot-email.form.email.error.not-email-form": "Please fill in a valid email address", "forgot-email.form.email.hint": "This address will be verified and used as your login name.", @@ -3013,7 +3013,9 @@ "register-page.registration.email.error.required": "Please fill in an email address", - "register-page.registration.email.error.pattern": "Please fill in a valid email address", + "register-page.registration.email.error.not-email-form": "Please fill in a valid email address. ", + + "register-page.registration.email.error.not-valid-domain": "Use email with allowed domains: {{ domains }}", "register-page.registration.email.hint": "This address will be verified and used as your login name.", From 81d21e25c226a18528ee1d1ba6e07f235add3ce5 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Thu, 26 Jan 2023 16:13:21 +0100 Subject: [PATCH 17/59] 98863: IT fix --- .../register-email-form/register-email-form.component.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/app/register-email-form/register-email-form.component.ts b/src/app/register-email-form/register-email-form.component.ts index 83053bd345..bb10aaed38 100644 --- a/src/app/register-email-form/register-email-form.component.ts +++ b/src/app/register-email-form/register-email-form.component.ts @@ -42,14 +42,6 @@ export class RegisterEmailFormComponent implements OnInit { validMailDomains: string[]; TYPE_REQUEST_REGISTER = TYPE_REQUEST_REGISTER; - captchaVersion(): Observable { - return this.googleRecaptchaService.captchaVersion(); - } - - captchaMode(): Observable { - return this.googleRecaptchaService.captchaMode(); - } - constructor( private epersonRegistrationService: EpersonRegistrationService, private notificationService: NotificationsService, From 14dfd6d9b855ff1acaf0138da2fe17d0ac1183bc Mon Sep 17 00:00:00 2001 From: cris Date: Mon, 30 Jan 2023 21:44:38 +0000 Subject: [PATCH 18/59] aria-selected added to item edit tab --- src/app/item-page/edit-item-page/edit-item-page.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/item-page/edit-item-page/edit-item-page.component.html b/src/app/item-page/edit-item-page/edit-item-page.component.html index 9458df2249..6465026532 100644 --- a/src/app/item-page/edit-item-page/edit-item-page.component.html +++ b/src/app/item-page/edit-item-page/edit-item-page.component.html @@ -4,7 +4,7 @@

{{'item.edit.head' | translate}}

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 35/59] [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 36/59] [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 37/59] [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 38/59] [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 39/59] [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 40/59] [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 41/59] [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 42/59] [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 43/59] [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 44/59] [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 45/59] [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 46/59] [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 47/59] [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 48/59] [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 49/59] [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 50/59] [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 51/59] [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 52/59] [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 53/59] [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, From 06de559974c085e0098831054cc9e0f6b446502c Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Tue, 7 Feb 2023 14:49:22 +0100 Subject: [PATCH 54/59] Retrieve the XSRF token first, and set it as both the XSRF header and cookie --- src/app/core/auth/auth-request.service.ts | 8 ++- .../core/auth/browser-auth-request.service.ts | 9 ++-- .../core/auth/server-auth-request.service.ts | 54 +++++++++++++------ src/app/core/services/server-xhr.service.ts | 16 ++++++ src/app/core/xsrf/xsrf.interceptor.ts | 2 + src/modules/app/server-app.module.ts | 6 +++ 6 files changed, 69 insertions(+), 26 deletions(-) create mode 100644 src/app/core/services/server-xhr.service.ts diff --git a/src/app/core/auth/auth-request.service.ts b/src/app/core/auth/auth-request.service.ts index 5c0c3340c7..7c1f17dec2 100644 --- a/src/app/core/auth/auth-request.service.ts +++ b/src/app/core/auth/auth-request.service.ts @@ -100,14 +100,12 @@ export abstract class AuthRequestService { ); } /** - * Factory function to create the request object to send. This needs to be a POST client side and - * a GET server side. Due to CSRF validation, the server isn't allowed to send a POST, so we allow - * only the server IP to send a GET to this endpoint. + * Factory function to create the request object to send. * * @param href The href to send the request to * @protected */ - protected abstract createShortLivedTokenRequest(href: string): GetRequest | PostRequest; + protected abstract createShortLivedTokenRequest(href: string): Observable; /** * Send a request to retrieve a short-lived token which provides download access of restricted files @@ -117,7 +115,7 @@ export abstract class AuthRequestService { filter((href: string) => isNotEmpty(href)), distinctUntilChanged(), map((href: string) => new URLCombiner(href, this.shortlivedtokensEndpoint).toString()), - map((endpointURL: string) => this.createShortLivedTokenRequest(endpointURL)), + switchMap((endpointURL: string) => this.createShortLivedTokenRequest(endpointURL)), tap((request: RestRequest) => this.requestService.send(request)), switchMap((request: RestRequest) => this.rdbService.buildFromRequestUUID(request.uuid)), getFirstCompletedRemoteData(), diff --git a/src/app/core/auth/browser-auth-request.service.ts b/src/app/core/auth/browser-auth-request.service.ts index 85d5f54340..485e2ef9c4 100644 --- a/src/app/core/auth/browser-auth-request.service.ts +++ b/src/app/core/auth/browser-auth-request.service.ts @@ -4,6 +4,7 @@ import { PostRequest } from '../data/request.models'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { RequestService } from '../data/request.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import { Observable, of as observableOf } from 'rxjs'; /** * Client side version of the service to send authentication requests @@ -20,15 +21,13 @@ export class BrowserAuthRequestService extends AuthRequestService { } /** - * Factory function to create the request object to send. This needs to be a POST client side and - * a GET server side. Due to CSRF validation, the server isn't allowed to send a POST, so we allow - * only the server IP to send a GET to this endpoint. + * Factory function to create the request object to send. * * @param href The href to send the request to * @protected */ - protected createShortLivedTokenRequest(href: string): PostRequest { - return new PostRequest(this.requestService.generateRequestId(), href); + protected createShortLivedTokenRequest(href: string): Observable { + return observableOf(new PostRequest(this.requestService.generateRequestId(), href)); } } diff --git a/src/app/core/auth/server-auth-request.service.ts b/src/app/core/auth/server-auth-request.service.ts index 9954e7c7ef..05131b7375 100644 --- a/src/app/core/auth/server-auth-request.service.ts +++ b/src/app/core/auth/server-auth-request.service.ts @@ -4,8 +4,19 @@ import { PostRequest } from '../data/request.models'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { RequestService } from '../data/request.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; -import { HttpHeaders, HttpXsrfTokenExtractor } from '@angular/common/http'; -import { XSRF_REQUEST_HEADER } from '../xsrf/xsrf.interceptor'; +import { + HttpHeaders, + HttpXsrfTokenExtractor, + HttpClient, + HttpResponse +} from '@angular/common/http'; +import { + XSRF_REQUEST_HEADER, + XSRF_RESPONSE_HEADER, + DSPACE_XSRF_COOKIE +} from '../xsrf/xsrf.interceptor'; +import { map } from 'rxjs/operators'; +import { Observable } from 'rxjs'; /** * Server side version of the service to send authentication requests @@ -17,29 +28,40 @@ export class ServerAuthRequestService extends AuthRequestService { halService: HALEndpointService, requestService: RequestService, rdbService: RemoteDataBuildService, - protected tokenExtractor: HttpXsrfTokenExtractor, + protected httpClient: HttpClient, ) { super(halService, requestService, rdbService); } /** - * Factory function to create the request object to send. This needs to be a POST client side and - * a GET server side. Due to CSRF validation, the server isn't allowed to send a POST, so we allow - * only the server IP to send a GET to this endpoint. + * Factory function to create the request object to send. * * @param href The href to send the request to * @protected */ - protected createShortLivedTokenRequest(href: string): PostRequest { - let options = new HttpHeaders(); - options = options.set('Content-Type', 'application/json; charset=utf-8'); - options = options.set(XSRF_REQUEST_HEADER, this.tokenExtractor.getToken()); - let requestOptions = { - headers: options, - }; - return Object.assign(new PostRequest(this.requestService.generateRequestId(), href, {}, requestOptions), { - responseMsToLive: 2 * 1000 // A short lived token is only valid for 2 seconds. - }); + protected createShortLivedTokenRequest(href: string): Observable { + // First do a call to the root endpoint in order to get an XSRF token + return this.httpClient.get(this.halService.getRootHref(), { observe: 'response' }).pipe( + // retrieve the XSRF token from the response header + map((response: HttpResponse) => response.headers.get(XSRF_RESPONSE_HEADER)), + // Use that token to create an HttpHeaders object + map((xsrfToken: string) => new HttpHeaders() + .set('Content-Type', 'application/json; charset=utf-8') + // set the token as the XSRF header + .set(XSRF_REQUEST_HEADER, xsrfToken) + // and as the DSPACE-XSRF-COOKIE + .set('Cookie', `${DSPACE_XSRF_COOKIE}=${xsrfToken}`)), + map((headers: HttpHeaders) => + // Create a new PostRequest using those headers and the given href + new PostRequest( + this.requestService.generateRequestId(), + href, + {}, + { + headers: headers, + } + )) + ) } } diff --git a/src/app/core/services/server-xhr.service.ts b/src/app/core/services/server-xhr.service.ts new file mode 100644 index 0000000000..835072bbd3 --- /dev/null +++ b/src/app/core/services/server-xhr.service.ts @@ -0,0 +1,16 @@ +import { XhrFactory } from '@angular/common'; +import { Injectable } from '@angular/core'; +import * as xhr2 from 'xhr2'; + +/** + * Overrides the default XhrFactoru server side, to allow us to set cookies in requests to the + * backend. This was added to be able to perform a working XSRF request from the node server, as it + * needs to set a cookie for the XSRF token + */ +@Injectable() +export class ServerXhrService implements XhrFactory { + build(): XMLHttpRequest { + xhr2.prototype._restrictedHeaders.cookie = false; + return new xhr2.XMLHttpRequest(); + } +} diff --git a/src/app/core/xsrf/xsrf.interceptor.ts b/src/app/core/xsrf/xsrf.interceptor.ts index d527924a28..cded432397 100644 --- a/src/app/core/xsrf/xsrf.interceptor.ts +++ b/src/app/core/xsrf/xsrf.interceptor.ts @@ -19,6 +19,8 @@ export const XSRF_REQUEST_HEADER = 'X-XSRF-TOKEN'; export const XSRF_RESPONSE_HEADER = 'DSPACE-XSRF-TOKEN'; // Name of cookie where we store the XSRF token export const XSRF_COOKIE = 'XSRF-TOKEN'; +// Name of cookie the backend expects the XSRF token to be in +export const DSPACE_XSRF_COOKIE = 'DSPACE-XSRF-COOKIE'; /** * Custom Http Interceptor intercepting Http Requests & Responses to diff --git a/src/modules/app/server-app.module.ts b/src/modules/app/server-app.module.ts index 81426e7fcc..7d162c5fd1 100644 --- a/src/modules/app/server-app.module.ts +++ b/src/modules/app/server-app.module.ts @@ -33,6 +33,8 @@ import { Angulartics2Mock } from '../../app/shared/mocks/angulartics2.service.mo import { AuthRequestService } from '../../app/core/auth/auth-request.service'; import { ServerAuthRequestService } from '../../app/core/auth/server-auth-request.service'; import { ServerInitService } from './server-init.service'; +import { XhrFactory } from '@angular/common'; +import { ServerXhrService } from '../../app/core/services/server-xhr.service'; export function createTranslateLoader(transferState: TransferState) { return new TranslateServerLoader(transferState, 'dist/server/assets/i18n/', '.json'); @@ -104,6 +106,10 @@ export function createTranslateLoader(transferState: TransferState) { provide: HardRedirectService, useClass: ServerHardRedirectService, }, + { + provide: XhrFactory, + useClass: ServerXhrService, + }, ] }) export class ServerAppModule { From ce6324a569e6ecde23b49ab4eca9a4e5b1229248 Mon Sep 17 00:00:00 2001 From: Nathan Buckingham Date: Tue, 7 Feb 2023 11:50:42 -0500 Subject: [PATCH 55/59] Fix lint and test issues --- .../core/auth/auth-request.service.spec.ts | 5 +- .../auth/browser-auth-request.service.spec.ts | 14 ++++-- .../auth/server-auth-request.service.spec.ts | 50 ++++++++++++------- .../core/auth/server-auth-request.service.ts | 10 ++-- src/app/core/services/server-xhr.service.ts | 8 +-- 5 files changed, 55 insertions(+), 32 deletions(-) diff --git a/src/app/core/auth/auth-request.service.spec.ts b/src/app/core/auth/auth-request.service.spec.ts index 704922c5b5..063aad612f 100644 --- a/src/app/core/auth/auth-request.service.spec.ts +++ b/src/app/core/auth/auth-request.service.spec.ts @@ -11,6 +11,7 @@ import { HttpOptions } from '../dspace-rest/dspace-rest.service'; import objectContaining = jasmine.objectContaining; import { AuthStatus } from './models/auth-status.model'; import { RestRequestMethod } from '../data/rest-request-method'; +import { Observable, of as observableOf } from 'rxjs'; describe(`AuthRequestService`, () => { let halService: HALEndpointService; @@ -34,8 +35,8 @@ describe(`AuthRequestService`, () => { super(hes, rs, rdbs); } - protected createShortLivedTokenRequest(href: string): PostRequest { - return new PostRequest(this.requestService.generateRequestId(), href); + protected createShortLivedTokenRequest(href: string): Observable { + return observableOf(new PostRequest(this.requestService.generateRequestId(), href)); } } diff --git a/src/app/core/auth/browser-auth-request.service.spec.ts b/src/app/core/auth/browser-auth-request.service.spec.ts index 18d27340af..2875553feb 100644 --- a/src/app/core/auth/browser-auth-request.service.spec.ts +++ b/src/app/core/auth/browser-auth-request.service.spec.ts @@ -1,6 +1,8 @@ import { AuthRequestService } from './auth-request.service'; import { RequestService } from '../data/request.service'; import { BrowserAuthRequestService } from './browser-auth-request.service'; +import { Observable } from 'rxjs'; +import { PostRequest } from '../data/request.models'; describe(`BrowserAuthRequestService`, () => { let href: string; @@ -17,13 +19,17 @@ describe(`BrowserAuthRequestService`, () => { describe(`createShortLivedTokenRequest`, () => { it(`should return a PostRequest`, () => { - const result = (service as any).createShortLivedTokenRequest(href); - expect(result.constructor.name).toBe('PostRequest'); + const obs = (service as any).createShortLivedTokenRequest(href) as Observable; + obs.subscribe((result: PostRequest) => { + expect(result.constructor.name).toBe('PostRequest'); + }); }); it(`should return a request with the given href`, () => { - const result = (service as any).createShortLivedTokenRequest(href); - expect(result.href).toBe(href) ; + const obs = (service as any).createShortLivedTokenRequest(href) as Observable; + obs.subscribe((result: PostRequest) => { + expect(result.href).toBe(href); + }); }); }); }); diff --git a/src/app/core/auth/server-auth-request.service.spec.ts b/src/app/core/auth/server-auth-request.service.spec.ts index 2612f9d5b4..181603eef7 100644 --- a/src/app/core/auth/server-auth-request.service.spec.ts +++ b/src/app/core/auth/server-auth-request.service.spec.ts @@ -1,44 +1,60 @@ import { AuthRequestService } from './auth-request.service'; import { RequestService } from '../data/request.service'; import { ServerAuthRequestService } from './server-auth-request.service'; -import { HttpXsrfTokenExtractorMock } from '../../shared/mocks/http-xsrf-token-extractor.mock'; +import { HttpClient, HttpHeaders, HttpResponse } from '@angular/common/http'; +import { Observable, of as observableOf } from 'rxjs'; +import { XSRF_RESPONSE_HEADER } from '../xsrf/xsrf.interceptor'; +import { HALEndpointService } from '../shared/hal-endpoint.service'; +import { PostRequest } from '../data/request.models'; describe(`ServerAuthRequestService`, () => { let href: string; let requestService: RequestService; let service: AuthRequestService; - let xsrfExtractor: HttpXsrfTokenExtractorMock; - - const mockToken = 'mockToken'; + let httpClient: HttpClient; + let httpResponse: HttpResponse; + let halService: HALEndpointService; + const mockToken = 'mock-token'; beforeEach(() => { href = 'https://rest.api/auth/shortlivedtokens'; requestService = jasmine.createSpyObj('requestService', { 'generateRequestId': '8bb0582d-5013-4337-af9c-763beb25aae2' }); - xsrfExtractor = new HttpXsrfTokenExtractorMock(mockToken); - service = new ServerAuthRequestService(null, requestService, null, xsrfExtractor); + httpResponse = { + body: { bar: false }, + headers: new HttpHeaders({ XSRF_RESPONSE_HEADER: mockToken }), + statusText: '200' + } as HttpResponse; + httpClient = jasmine.createSpyObj('httpClient', { + get: observableOf(httpResponse), + }); + halService = jasmine.createSpyObj('halService', { + 'getRootHref': '/api' + }); + service = new ServerAuthRequestService(halService, requestService, null, httpClient); }); describe(`createShortLivedTokenRequest`, () => { it(`should return a PostRequest`, () => { - const result = (service as any).createShortLivedTokenRequest(href); - expect(result.constructor.name).toBe('PostRequest'); + const obs = (service as any).createShortLivedTokenRequest(href) as Observable; + obs.subscribe((result: PostRequest) => { + expect(result.constructor.name).toBe('PostRequest'); + }); }); it(`should return a request with the given href`, () => { - const result = (service as any).createShortLivedTokenRequest(href); - expect(result.href).toBe(href) ; + const obs = (service as any).createShortLivedTokenRequest(href) as Observable; + obs.subscribe((result: PostRequest) => { + expect(result.href).toBe(href) ; + }); }); it(`should return a request with a xsrf header`, () => { - const result = (service as any).createShortLivedTokenRequest(href); - expect(result.options.headers.get('X-XSRF-TOKEN')).toBe(mockToken); - }); - - it(`should have a responseMsToLive of 2 seconds`, () => { - const result = (service as any).createShortLivedTokenRequest(href); - expect(result.responseMsToLive).toBe(2 * 1000) ; + const obs = (service as any).createShortLivedTokenRequest(href) as Observable; + obs.subscribe((result: PostRequest) => { + expect(result.options.headers.get(XSRF_RESPONSE_HEADER)).toBe(mockToken); + }); }); }); }); diff --git a/src/app/core/auth/server-auth-request.service.ts b/src/app/core/auth/server-auth-request.service.ts index 05131b7375..183e085589 100644 --- a/src/app/core/auth/server-auth-request.service.ts +++ b/src/app/core/auth/server-auth-request.service.ts @@ -6,7 +6,6 @@ import { RequestService } from '../data/request.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { HttpHeaders, - HttpXsrfTokenExtractor, HttpClient, HttpResponse } from '@angular/common/http'; @@ -53,15 +52,16 @@ export class ServerAuthRequestService extends AuthRequestService { .set('Cookie', `${DSPACE_XSRF_COOKIE}=${xsrfToken}`)), map((headers: HttpHeaders) => // Create a new PostRequest using those headers and the given href - new PostRequest( + Object.assign(new PostRequest( this.requestService.generateRequestId(), href, {}, { headers: headers, - } - )) - ) + }, + ),{}) + ) + ); } } diff --git a/src/app/core/services/server-xhr.service.ts b/src/app/core/services/server-xhr.service.ts index 835072bbd3..69ae741402 100644 --- a/src/app/core/services/server-xhr.service.ts +++ b/src/app/core/services/server-xhr.service.ts @@ -1,16 +1,16 @@ import { XhrFactory } from '@angular/common'; import { Injectable } from '@angular/core'; -import * as xhr2 from 'xhr2'; +import { prototype, XMLHttpRequest } from 'xhr2'; /** - * Overrides the default XhrFactoru server side, to allow us to set cookies in requests to the + * Overrides the default XhrFactory server side, to allow us to set cookies in requests to the * backend. This was added to be able to perform a working XSRF request from the node server, as it * needs to set a cookie for the XSRF token */ @Injectable() export class ServerXhrService implements XhrFactory { build(): XMLHttpRequest { - xhr2.prototype._restrictedHeaders.cookie = false; - return new xhr2.XMLHttpRequest(); + prototype._restrictedHeaders.cookie = false; + return new XMLHttpRequest(); } } From c099bc468d752fb357ac6ad786701c12728ef6c4 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 7 Feb 2023 12:22:32 -0600 Subject: [PATCH 56/59] Add "debug" config and "allowStale" configs --- config/config.example.yml | 12 ++++++++++++ server.ts | 15 +++++++++------ src/config/cache-config.interface.ts | 6 ++++++ src/config/default-app-config.ts | 3 +++ src/environments/environment.test.ts | 3 +++ 5 files changed, 33 insertions(+), 6 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index 52b06b1b08..500c2c476a 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -49,6 +49,8 @@ cache: # NOTE: To control the cache size, use the "max" setting. Keep in mind, individual cached pages are usually small (<100KB). # Enabling *both* caches will mean that a page may be cached twice, once in each cache (but may expire at different times via timeToLive). serverSide: + # Set to true to see all cache hits/misses/refreshes in your console logs. Useful for debugging SSR caching issues. + debug: false # When enabled (i.e. max > 0), known bots will be sent pages from a server side cache specific for bots. # (Keep in mind, bot detection cannot be guarranteed. It is possible some bots will bypass this cache.) botCache: @@ -62,6 +64,11 @@ cache: # NOTE: For the bot cache, this setting may impact how quickly search engine bots will index new content on your site. # For example, setting this to one week may mean that search engine bots may not find all new content for one week. timeToLive: 86400000 # 1 day + # When set to true, after timeToLive expires, the next request will receive the *cached* page & then re-render the page + # behind the scenes to update the cache. This ensures users primarily interact with the cache, but may receive stale pages (older than timeToLive). + # When set to false, after timeToLive expires, the next request will wait on SSR to complete & receive a fresh page (which is then saved to cache). + # This ensures stale pages (older than timeToLive) are never returned from the cache, but some users will wait on SSR. + allowStale: true # When enabled (i.e. max > 0), all anonymous users will be sent pages from a server side cache. # This allows anonymous users to interact more quickly with the site, but also means they may see slightly # outdated content (based on timeToLive) @@ -74,6 +81,11 @@ cache: # copy is automatically refreshed on the next request. # NOTE: For the anonymous cache, it is recommended to keep this value low to avoid anonymous users seeing outdated content. timeToLive: 10000 # 10 seconds + # When set to true, after timeToLive expires, the next request will receive the *cached* page & then re-render the page + # behind the scenes to update the cache. This ensures users primarily interact with the cache, but may receive stale pages (older than timeToLive). + # When set to false, after timeToLive expires, the next request will wait on SSR to complete & receive a fresh page (which is then saved to cache). + # This ensures stale pages (older than timeToLive) are never returned from the cache, but some users will wait on SSR. + allowStale: true # Authentication settings auth: diff --git a/server.ts b/server.ts index 8c9835cf16..478a4c0ec1 100644 --- a/server.ts +++ b/server.ts @@ -324,7 +324,7 @@ function initCache() { botCache = new LRU( { max: environment.cache.serverSide.botCache.max, ttl: environment.cache.serverSide.botCache.timeToLive || 24 * 60 * 60 * 1000, // 1 day - allowStale: true // If object is found to be stale, return stale value before deleting + allowStale: environment.cache.serverSide.botCache.allowStale || true // if object is stale, return stale value before deleting }); } @@ -335,7 +335,7 @@ function initCache() { anonymousCache = new LRU( { max: environment.cache.serverSide.anonymousCache.max, ttl: environment.cache.serverSide.anonymousCache.timeToLive || 10 * 1000, // 10 seconds - allowStale: true // If object is found to be stale, return stale value before deleting + allowStale: environment.cache.serverSide.anonymousCache.allowStale || true // if object is stale, return stale value before deleting }); } } @@ -399,24 +399,25 @@ function cacheCheck(req, res, next) { * @returns cached copy (if found) or undefined (if not found) */ function checkCacheForRequest(cacheName: string, cache: LRU, req, res): any { - let debug = false; // Enable to see cache hits & re-rendering in logs - // Get the cache key for this request const key = getCacheKey(req); // Check if this page is in our cache let cachedCopy = cache.get(key); if (cachedCopy) { - if (debug) { console.log(`CACHE HIT FOR ${key} in ${cacheName} cache`); } + if (environment.cache.serverSide.debug) { console.log(`CACHE HIT FOR ${key} in ${cacheName} cache`); } // Check if cached copy is expired (If expired, the key will now be gone from cache) + // NOTE: This will only occur when "allowStale=true", as it means the "get(key)" above returned a stale value. if (!cache.has(key)) { - if (debug) { console.log(`CACHE EXPIRED FOR ${key} in ${cacheName} cache. Re-rendering...`); } + if (environment.cache.serverSide.debug) { console.log(`CACHE EXPIRED FOR ${key} in ${cacheName} cache. Re-rendering...`); } // Update cached copy by rerendering server-side // NOTE: In this scenario the currently cached copy will be returned to the current user. // This re-render is peformed behind the scenes to update cached copy for next user. serverSideRender(req, res, false); } + } else { + if (environment.cache.serverSide.debug) { console.log(`CACHE MISS FOR ${key} in ${cacheName} cache.`); } } // return page from cache @@ -455,11 +456,13 @@ function saveToCache(req, page: any) { // (NOTE: has() will return false if page is expired in cache) if (botCacheEnabled() && !botCache.has(key)) { botCache.set(key, page); + if (environment.cache.serverSide.debug) { console.log(`CACHE SAVE FOR ${key} in bot cache.`); } } // If anonymous cache is enabled, save it to that cache if it doesn't exist or is expired if (anonymousCacheEnabled() && !anonymousCache.has(key)) { anonymousCache.set(key, page); + if (environment.cache.serverSide.debug) { console.log(`CACHE SAVE FOR ${key} in anonymous cache.`); } } } } diff --git a/src/config/cache-config.interface.ts b/src/config/cache-config.interface.ts index 1826bd0d30..9560fe46a5 100644 --- a/src/config/cache-config.interface.ts +++ b/src/config/cache-config.interface.ts @@ -11,12 +11,16 @@ export interface CacheConfig extends Config { // In-memory caches of server-side rendered (SSR) content. These caches can be used to limit the frequency // of re-generating SSR pages to improve performance. serverSide: { + // Debug server-side caching. Set to true to see cache hits/misses/refreshes in console logs. + debug: boolean, // Cache specific to known bots. Allows you to serve cached contents to bots only. botCache: { // Maximum number of pages (rendered via SSR) to cache. Setting max=0 disables the cache. max: number; // Amount of time after which cached pages are considered stale (in ms) timeToLive: number; + // true = return page from cache after timeToLive expires. false = return a fresh page after timeToLive expires + allowStale: boolean; }, // Cache specific to anonymous users. Allows you to serve cached content to non-authenticated users. anonymousCache: { @@ -24,6 +28,8 @@ export interface CacheConfig extends Config { max: number; // Amount of time after which cached pages are considered stale (in ms) timeToLive: number; + // true = return page from cache after timeToLive expires. false = return a fresh page after timeToLive expires + allowStale: boolean; } } } diff --git a/src/config/default-app-config.ts b/src/config/default-app-config.ts index 9e5b535872..e7851d4b34 100644 --- a/src/config/default-app-config.ts +++ b/src/config/default-app-config.ts @@ -76,6 +76,7 @@ export class DefaultAppConfig implements AppConfig { }, // In-memory cache of server-side rendered content serverSide: { + debug: false, // Cache specific to known bots. Allows you to serve cached contents to bots only. // Defaults to caching 1,000 pages. Each page expires after 1 day botCache: { @@ -83,6 +84,7 @@ export class DefaultAppConfig implements AppConfig { max: 1000, // Amount of time after which cached pages are considered stale (in ms) timeToLive: 24 * 60 * 60 * 1000, // 1 day + allowStale: true, }, // Cache specific to anonymous users. Allows you to serve cached content to non-authenticated users. // Defaults to caching 0 pages. But, when enabled, each page expires after 10 seconds (to minimize anonymous users seeing out-of-date content) @@ -91,6 +93,7 @@ export class DefaultAppConfig implements AppConfig { max: 0, // disabled by default // Amount of time after which cached pages are considered stale (in ms) timeToLive: 10 * 1000, // 10 seconds + allowStale: true, } } }; diff --git a/src/environments/environment.test.ts b/src/environments/environment.test.ts index 613024f798..0bb36da61f 100644 --- a/src/environments/environment.test.ts +++ b/src/environments/environment.test.ts @@ -58,13 +58,16 @@ export const environment: BuildConfig = { }, // In-memory cache of server-side rendered pages. Disabled in test environment (max=0) serverSide: { + debug: false, botCache: { max: 0, timeToLive: 24 * 60 * 60 * 1000, // 1 day + allowStale: true, }, anonymousCache: { max: 0, timeToLive: 10 * 1000, // 10 seconds + allowStale: true, } } }, From 9435f1c4a73e7a3e3ce1e26619ce322c8839ab1c Mon Sep 17 00:00:00 2001 From: Nathan Buckingham Date: Tue, 7 Feb 2023 13:59:57 -0500 Subject: [PATCH 57/59] Add done() to async tests --- .../auth/browser-auth-request.service.spec.ts | 6 +++-- .../auth/server-auth-request.service.spec.ts | 22 +++++++++++++------ .../core/auth/server-auth-request.service.ts | 4 ++-- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/app/core/auth/browser-auth-request.service.spec.ts b/src/app/core/auth/browser-auth-request.service.spec.ts index 2875553feb..b41d981bcf 100644 --- a/src/app/core/auth/browser-auth-request.service.spec.ts +++ b/src/app/core/auth/browser-auth-request.service.spec.ts @@ -18,17 +18,19 @@ describe(`BrowserAuthRequestService`, () => { }); describe(`createShortLivedTokenRequest`, () => { - it(`should return a PostRequest`, () => { + it(`should return a PostRequest`, (done) => { const obs = (service as any).createShortLivedTokenRequest(href) as Observable; obs.subscribe((result: PostRequest) => { expect(result.constructor.name).toBe('PostRequest'); + done(); }); }); - it(`should return a request with the given href`, () => { + it(`should return a request with the given href`, (done) => { const obs = (service as any).createShortLivedTokenRequest(href) as Observable; obs.subscribe((result: PostRequest) => { expect(result.href).toBe(href); + done(); }); }); }); diff --git a/src/app/core/auth/server-auth-request.service.spec.ts b/src/app/core/auth/server-auth-request.service.spec.ts index 181603eef7..df6d78256b 100644 --- a/src/app/core/auth/server-auth-request.service.spec.ts +++ b/src/app/core/auth/server-auth-request.service.spec.ts @@ -3,9 +3,12 @@ import { RequestService } from '../data/request.service'; import { ServerAuthRequestService } from './server-auth-request.service'; import { HttpClient, HttpHeaders, HttpResponse } from '@angular/common/http'; import { Observable, of as observableOf } from 'rxjs'; -import { XSRF_RESPONSE_HEADER } from '../xsrf/xsrf.interceptor'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { PostRequest } from '../data/request.models'; +import { + XSRF_REQUEST_HEADER, + XSRF_RESPONSE_HEADER +} from '../xsrf/xsrf.interceptor'; describe(`ServerAuthRequestService`, () => { let href: string; @@ -21,9 +24,11 @@ describe(`ServerAuthRequestService`, () => { requestService = jasmine.createSpyObj('requestService', { 'generateRequestId': '8bb0582d-5013-4337-af9c-763beb25aae2' }); + let headers = new HttpHeaders(); + headers = headers.set(XSRF_RESPONSE_HEADER, mockToken); httpResponse = { body: { bar: false }, - headers: new HttpHeaders({ XSRF_RESPONSE_HEADER: mockToken }), + headers: headers, statusText: '200' } as HttpResponse; httpClient = jasmine.createSpyObj('httpClient', { @@ -36,24 +41,27 @@ describe(`ServerAuthRequestService`, () => { }); describe(`createShortLivedTokenRequest`, () => { - it(`should return a PostRequest`, () => { + it(`should return a PostRequest`, (done) => { const obs = (service as any).createShortLivedTokenRequest(href) as Observable; obs.subscribe((result: PostRequest) => { expect(result.constructor.name).toBe('PostRequest'); + done(); }); }); - it(`should return a request with the given href`, () => { + it(`should return a request with the given href`, (done) => { const obs = (service as any).createShortLivedTokenRequest(href) as Observable; obs.subscribe((result: PostRequest) => { - expect(result.href).toBe(href) ; + expect(result.href).toBe(href); + done(); }); }); - it(`should return a request with a xsrf header`, () => { + it(`should return a request with a xsrf header`, (done) => { const obs = (service as any).createShortLivedTokenRequest(href) as Observable; obs.subscribe((result: PostRequest) => { - expect(result.options.headers.get(XSRF_RESPONSE_HEADER)).toBe(mockToken); + expect(result.options.headers.get(XSRF_REQUEST_HEADER)).toBe(mockToken); + done(); }); }); }); diff --git a/src/app/core/auth/server-auth-request.service.ts b/src/app/core/auth/server-auth-request.service.ts index 183e085589..d6302081bc 100644 --- a/src/app/core/auth/server-auth-request.service.ts +++ b/src/app/core/auth/server-auth-request.service.ts @@ -52,14 +52,14 @@ export class ServerAuthRequestService extends AuthRequestService { .set('Cookie', `${DSPACE_XSRF_COOKIE}=${xsrfToken}`)), map((headers: HttpHeaders) => // Create a new PostRequest using those headers and the given href - Object.assign(new PostRequest( + new PostRequest( this.requestService.generateRequestId(), href, {}, { headers: headers, }, - ),{}) + ) ) ); } From 5d2f63ff65ca28948b3bd5359db63e8a3cc3d813 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Wed, 8 Feb 2023 10:17:36 +0100 Subject: [PATCH 58/59] 99053: Added test to check that the TYPE_REQUEST_FORGOT doesn't use the authentication-password.domain.valid --- .../register-email-form.component.spec.ts | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/app/register-email-form/register-email-form.component.spec.ts b/src/app/register-email-form/register-email-form.component.spec.ts index 4fea169e35..89af6ff05a 100644 --- a/src/app/register-email-form/register-email-form.component.spec.ts +++ b/src/app/register-email-form/register-email-form.component.spec.ts @@ -74,15 +74,6 @@ describe('RegisterEmailFormComponent', () => { const elem = fixture.debugElement.queryAll(By.css('input#email'))[0].nativeElement; expect(elem).toBeDefined(); }); - - it('should not retrieve the validDomains for TYPE_REQUEST_FORGOT', () => { - spyOn(configurationDataService, 'findByPropertyName'); - comp.typeRequest = TYPE_REQUEST_FORGOT; - - comp.ngOnInit(); - - expect(configurationDataService.findByPropertyName).not.toHaveBeenCalled(); - }); }); describe('email validation', () => { it('should be invalid when no email is present', () => { @@ -96,6 +87,18 @@ describe('RegisterEmailFormComponent', () => { comp.form.patchValue({email: 'valid@email.org'}); expect(comp.form.invalid).toBeFalse(); }); + it('should accept email with other domain names on TYPE_REQUEST_FORGOT form', () => { + spyOn(configurationDataService, 'findByPropertyName').and.returnValue(createSuccessfulRemoteDataObject$(Object.assign(new ConfigurationProperty(), { + name: 'authentication-password.domain.valid', + values: ['marvel.com'], + }))); + comp.typeRequest = TYPE_REQUEST_FORGOT; + + comp.ngOnInit(); + + comp.form.patchValue({ email: 'valid@email.org' }); + expect(comp.form.invalid).toBeFalse(); + }); it('should not accept email with other domain names', () => { spyOn(configurationDataService, 'findByPropertyName').and.returnValue(createSuccessfulRemoteDataObject$(Object.assign(new ConfigurationProperty(), { name: 'authentication-password.domain.valid', From 9e11d69a8ade6fae07676d49f6a2cb4697705107 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Thu, 9 Feb 2023 08:48:50 -0600 Subject: [PATCH 59/59] Fix bug where allowStale couldn't be disabled --- server.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server.ts b/server.ts index 478a4c0ec1..ba0c8fd7b2 100644 --- a/server.ts +++ b/server.ts @@ -324,7 +324,7 @@ function initCache() { botCache = new LRU( { max: environment.cache.serverSide.botCache.max, ttl: environment.cache.serverSide.botCache.timeToLive || 24 * 60 * 60 * 1000, // 1 day - allowStale: environment.cache.serverSide.botCache.allowStale || true // if object is stale, return stale value before deleting + allowStale: environment.cache.serverSide.botCache.allowStale ?? true // if object is stale, return stale value before deleting }); } @@ -335,7 +335,7 @@ function initCache() { anonymousCache = new LRU( { max: environment.cache.serverSide.anonymousCache.max, ttl: environment.cache.serverSide.anonymousCache.timeToLive || 10 * 1000, // 10 seconds - allowStale: environment.cache.serverSide.anonymousCache.allowStale || true // if object is stale, return stale value before deleting + allowStale: environment.cache.serverSide.anonymousCache.allowStale ?? true // if object is stale, return stale value before deleting }); } }