diff --git a/src/app/accessibility/accessibility-settings.service.spec.ts b/src/app/accessibility/accessibility-settings.service.spec.ts index 64548ca84d..599ad070d2 100644 --- a/src/app/accessibility/accessibility-settings.service.spec.ts +++ b/src/app/accessibility/accessibility-settings.service.spec.ts @@ -4,10 +4,12 @@ import { } from '@angular/core/testing'; import { of } from 'rxjs'; +import { AppConfig } from '../../config/app-config.interface'; import { AuthService } from '../core/auth/auth.service'; import { EPersonDataService } from '../core/eperson/eperson-data.service'; import { EPerson } from '../core/eperson/models/eperson.model'; import { CookieService } from '../core/services/cookie.service'; +import { KlaroServiceStub } from '../shared/cookies/klaro.service.stub'; import { CookieServiceMock } from '../shared/mocks/cookie.service.mock'; import { createFailedRemoteDataObject$, @@ -29,10 +31,16 @@ describe('accessibilitySettingsService', () => { let cookieService: CookieServiceMock; let authService: AuthServiceStub; let ePersonService: EPersonDataService; + let klaroService: KlaroServiceStub; + let appConfig: AppConfig; beforeEach(() => { cookieService = new CookieServiceMock(); authService = new AuthServiceStub(); + klaroService = new KlaroServiceStub(); + appConfig = { accessibility: { cookieExpirationDuration: 10 } } as AppConfig; + + klaroService.getSavedPreferences.and.returnValue(of({ accessibility: true })); ePersonService = jasmine.createSpyObj('ePersonService', { createPatchFromCache: of([{ @@ -46,6 +54,8 @@ describe('accessibilitySettingsService', () => { cookieService as unknown as CookieService, authService as unknown as AuthService, ePersonService, + klaroService, + appConfig, ); }); @@ -180,12 +190,12 @@ describe('accessibilitySettingsService', () => { describe('setSettings', () => { beforeEach(() => { - service.setSettingsInCookie = jasmine.createSpy('setSettingsInCookie'); + service.setSettingsInCookie = jasmine.createSpy('setSettingsInCookie').and.returnValue(of('cookie')); }); it('should attempt to set settings in metadata', () => { service.setSettingsInAuthenticatedUserMetadata = - jasmine.createSpy('setSettingsInAuthenticatedUserMetadata').and.returnValue(of(false)); + jasmine.createSpy('setSettingsInAuthenticatedUserMetadata').and.returnValue(of('failed')); const settings: AccessibilitySettings = { notificationTimeOut: '1000', @@ -209,7 +219,7 @@ describe('accessibilitySettingsService', () => { it('should not set settings in cookie if metadata succeeded', () => { service.setSettingsInAuthenticatedUserMetadata = - jasmine.createSpy('setSettingsInAuthenticatedUserMetadata').and.returnValue(of(true)); + jasmine.createSpy('setSettingsInAuthenticatedUserMetadata').and.returnValue(of('metadata')); const settings: AccessibilitySettings = { notificationTimeOut: '1000', @@ -221,7 +231,7 @@ describe('accessibilitySettingsService', () => { it('should return \'metadata\' if settings are stored in metadata', () => { service.setSettingsInAuthenticatedUserMetadata = - jasmine.createSpy('setSettingsInAuthenticatedUserMetadata').and.returnValue(of(true)); + jasmine.createSpy('setSettingsInAuthenticatedUserMetadata').and.returnValue(of('metadata')); const settings: AccessibilitySettings = { notificationTimeOut: '1000', @@ -284,11 +294,11 @@ describe('accessibilitySettingsService', () => { expect(service.setSettingsInMetadata).toHaveBeenCalled(); })); - it('should emit false when the user is not authenticated', fakeAsync(() => { + it('should emit "failed" when the user is not authenticated', fakeAsync(() => { authService.getAuthenticatedUserFromStoreIfAuthenticated = jasmine.createSpy().and.returnValue(of(null)); service.setSettingsInAuthenticatedUserMetadata({}) - .subscribe(value => expect(value).toBeFalse()); + .subscribe(value => expect(value).toEqual('failed')); flush(); expect(service.setSettingsInMetadata).not.toHaveBeenCalled(); @@ -324,23 +334,23 @@ describe('accessibilitySettingsService', () => { expect(ePersonService.patch).toHaveBeenCalled(); }); - it('should emit true when the update succeeded', fakeAsync(() => { + it('should emit "metadata" when the update succeeded', fakeAsync(() => { ePersonService.patch = jasmine.createSpy().and.returnValue(createSuccessfulRemoteDataObject$({})); service.setSettingsInMetadata(ePerson, { ['liveRegionTimeOut']: '500' }) .subscribe(value => { - expect(value).toBeTrue(); + expect(value).toEqual('metadata'); }); flush(); })); - it('should emit false when the update failed', fakeAsync(() => { + it('should emit "failed" when the update failed', fakeAsync(() => { ePersonService.patch = jasmine.createSpy().and.returnValue(createFailedRemoteDataObject$()); service.setSettingsInMetadata(ePerson, { ['liveRegionTimeOut']: '500' }) .subscribe(value => { - expect(value).toBeFalse(); + expect(value).toEqual('failed'); }); flush(); @@ -353,16 +363,34 @@ describe('accessibilitySettingsService', () => { cookieService.remove = jasmine.createSpy('remove'); }); - it('should store the settings in a cookie', () => { - service.setSettingsInCookie({ ['liveRegionTimeOut']: '500' }); - expect(cookieService.set).toHaveBeenCalled(); - }); + it('should fail to store settings in the cookie when the user has not accepted the cookie', fakeAsync(() => { + klaroService.getSavedPreferences.and.returnValue(of({ accessibility: false })); + + service.setSettingsInCookie({ ['liveRegionTimeOut']: '500' }).subscribe(value => { + expect(value).toEqual('failed'); + }); + flush(); + expect(cookieService.set).not.toHaveBeenCalled(); + })); + + it('should store the settings in a cookie', fakeAsync(() => { + service.setSettingsInCookie({ ['liveRegionTimeOut']: '500' }).subscribe(value => { + expect(value).toEqual('cookie'); + }); + flush(); + expect(cookieService.set).toHaveBeenCalled(); + })); + + it('should remove the cookie when the settings are empty', fakeAsync(() => { + service.setSettingsInCookie({}).subscribe(value => { + expect(value).toEqual('cookie'); + }); + + flush(); - it('should remove the cookie when the settings are empty', () => { - service.setSettingsInCookie({}); expect(cookieService.set).not.toHaveBeenCalled(); expect(cookieService.remove).toHaveBeenCalled(); - }); + })); }); describe('convertFormValuesToStoredValues', () => { diff --git a/src/app/accessibility/accessibility-settings.service.stub.ts b/src/app/accessibility/accessibility-settings.service.stub.ts index 103b1805dc..d991c63394 100644 --- a/src/app/accessibility/accessibility-settings.service.stub.ts +++ b/src/app/accessibility/accessibility-settings.service.stub.ts @@ -37,5 +37,9 @@ export class AccessibilitySettingsServiceStub { convertStoredValuesToFormValues = jasmine.createSpy('convertStoredValuesToFormValues').and.returnValue({}); - getPlaceholder = jasmine.createSpy('getPlaceholder').and.returnValue('placeholder'); + getDefaultValue = jasmine.createSpy('getPlaceholder').and.returnValue('placeholder'); + + isValid = jasmine.createSpy('isValid').and.returnValue(true); + + formValuesValid = jasmine.createSpy('allValid').and.returnValue(true); } diff --git a/src/app/accessibility/accessibility-settings.service.ts b/src/app/accessibility/accessibility-settings.service.ts index 3133e216f8..e513a8cd61 100644 --- a/src/app/accessibility/accessibility-settings.service.ts +++ b/src/app/accessibility/accessibility-settings.service.ts @@ -1,6 +1,10 @@ -import { Injectable } from '@angular/core'; +import { + Inject, + Injectable, +} from '@angular/core'; import cloneDeep from 'lodash/cloneDeep'; import { + combineLatest, Observable, of, switchMap, @@ -10,13 +14,19 @@ import { take, } from 'rxjs/operators'; +import { + APP_CONFIG, + AppConfig, +} from '../../config/app-config.interface'; import { environment } from '../../environments/environment'; import { AuthService } from '../core/auth/auth.service'; import { EPersonDataService } from '../core/eperson/eperson-data.service'; import { EPerson } from '../core/eperson/models/eperson.model'; import { CookieService } from '../core/services/cookie.service'; import { getFirstCompletedRemoteData } from '../core/shared/operators'; +import { KlaroService } from '../shared/cookies/klaro.service'; import { + hasNoValue, hasValue, isNotEmpty, } from '../shared/empty.util'; @@ -33,11 +43,16 @@ export const ACCESSIBILITY_COOKIE = 'dsAccessibilityCookie'; export const ACCESSIBILITY_SETTINGS_METADATA_KEY = 'dspace.accessibility.settings'; /** - * Type containing all possible accessibility settings. + * Array containing all possible accessibility settings. * When adding new settings, make sure to add the new setting to the accessibility-settings component form. * The converter methods to convert from stored format to form format (and vice-versa) need to be updated as well. */ -export type AccessibilitySetting = 'notificationTimeOut' | 'liveRegionTimeOut'; +export const accessibilitySettingKeys = ['notificationTimeOut', 'liveRegionTimeOut'] as const; + +/** + * Type representing the possible accessibility settings + */ +export type AccessibilitySetting = typeof accessibilitySettingKeys[number]; /** * Type representing an object that contains accessibility settings values for all accessibility settings. @@ -73,6 +88,8 @@ export class AccessibilitySettingsService { protected cookieService: CookieService, protected authService: AuthService, protected ePersonService: EPersonDataService, + protected klaroService: KlaroService, + @Inject(APP_CONFIG) protected appConfig: AppConfig, ) { } @@ -136,8 +153,9 @@ export class AccessibilitySettingsService { * * Returns 'cookie' when the changes were stored in the cookie. * Returns 'metadata' when the changes were stored in metadata. + * Returns 'failed' when both options failed. */ - set(setting: AccessibilitySetting, value: string): Observable<'cookie' | 'metadata'> { + set(setting: AccessibilitySetting, value: string): Observable<'metadata' | 'cookie' | 'failed'> { return this.updateSettings({ [setting]: value }); } @@ -148,18 +166,15 @@ export class AccessibilitySettingsService { * * Returns 'cookie' when the changes were stored in the cookie. * Returns 'metadata' when the changes were stored in metadata. + * Returns 'failed' when both options failed. */ - setSettings(settings: AccessibilitySettings): Observable<'cookie' | 'metadata'> { + setSettings(settings: AccessibilitySettings): Observable<'metadata' | 'cookie' | 'failed'> { return this.setSettingsInAuthenticatedUserMetadata(settings).pipe( take(1), - map((succeeded) => { - if (!succeeded) { - this.setSettingsInCookie(settings); - return 'cookie'; - } else { - return 'metadata'; - } - }), + map(saveLocation => saveLocation === 'metadata'), + switchMap((savedInMetadata) => + savedInMetadata ? ofMetadata() : this.setSettingsInCookie(settings), + ), ); } @@ -169,8 +184,9 @@ export class AccessibilitySettingsService { * * Returns 'cookie' when the changes were stored in the cookie. * Returns 'metadata' when the changes were stored in metadata. + * Returns 'failed' when both options failed. */ - updateSettings(settings: AccessibilitySettings): Observable<'cookie' | 'metadata'> { + updateSettings(settings: AccessibilitySettings): Observable<'metadata' | 'cookie' | 'failed'> { return this.getAll().pipe( take(1), map(currentSettings => Object.assign({}, currentSettings, settings)), @@ -181,9 +197,9 @@ export class AccessibilitySettingsService { /** * Attempts to set the provided settings on the currently authorized user's metadata. * Emits false when no user is authenticated or when the metadata update failed. - * Emits true when the metadata update succeeded. + * Emits 'metadata' when the metadata update succeeded, and 'failed' otherwise. */ - setSettingsInAuthenticatedUserMetadata(settings: AccessibilitySettings): Observable { + setSettingsInAuthenticatedUserMetadata(settings: AccessibilitySettings): Observable<'metadata' | 'failed'> { return this.authService.getAuthenticatedUserFromStoreIfAuthenticated().pipe( take(1), switchMap(user => { @@ -192,7 +208,7 @@ export class AccessibilitySettingsService { const clonedUser = cloneDeep(user); return this.setSettingsInMetadata(clonedUser, settings); } else { - return of(false); + return ofFailed(); } }), ); @@ -205,7 +221,7 @@ export class AccessibilitySettingsService { setSettingsInMetadata( user: EPerson, settings: AccessibilitySettings, - ): Observable { + ): Observable<'metadata' | 'failed'> { if (isNotEmpty(settings)) { user.setMetadata(ACCESSIBILITY_SETTINGS_METADATA_KEY, null, JSON.stringify(settings)); } else { @@ -217,35 +233,49 @@ export class AccessibilitySettingsService { switchMap(operations => isNotEmpty(operations) ? this.ePersonService.patch(user, operations) : createSuccessfulRemoteDataObject$({})), getFirstCompletedRemoteData(), - map(rd => rd.hasSucceeded), + switchMap(rd => rd.hasSucceeded ? ofMetadata() : ofFailed()), ); } /** - * Sets the provided settings in a cookie + * Attempts to set the provided settings in a cookie. + * Emits 'failed' when setting in a cookie failed due to the cookie not being accepted, 'cookie' when it succeeded. */ - setSettingsInCookie(settings: AccessibilitySettings) { - if (isNotEmpty(settings)) { - this.cookieService.set(ACCESSIBILITY_COOKIE, settings, { expires: environment.accessibility.cookieExpirationDuration }); - } else { - this.cookieService.remove(ACCESSIBILITY_COOKIE); - } + setSettingsInCookie(settings: AccessibilitySettings): Observable<'cookie' | 'failed'> { + return this.klaroService.getSavedPreferences().pipe( + map(preferences => preferences.accessibility), + map((accessibilityCookieAccepted: boolean) => { + if (accessibilityCookieAccepted) { + if (isNotEmpty(settings)) { + this.cookieService.set(ACCESSIBILITY_COOKIE, settings, { expires: this.appConfig.accessibility.cookieExpirationDuration }); + } else { + this.cookieService.remove(ACCESSIBILITY_COOKIE); + } + + return 'cookie'; + } else { + return 'failed'; + } + }), + ); } /** * Clears all settings in the cookie and attempts to clear settings in metadata. - * Emits true if settings in metadata were cleared and false otherwise. + * Emits an array mentioning which settings succeeded or failed. */ - clearSettings(): Observable { - this.setSettingsInCookie({}); - return this.setSettingsInAuthenticatedUserMetadata({}); + clearSettings(): Observable<['cookie' | 'failed', 'metadata' | 'failed']> { + return combineLatest([ + this.setSettingsInCookie({}), + this.setSettingsInAuthenticatedUserMetadata({}), + ]); } /** - * Retrieve the placeholder to be used for the provided AccessibilitySetting. - * Returns an empty string when no placeholder is specified for the provided setting. + * Retrieve the default value to be used for the provided AccessibilitySetting. + * Returns an empty string when no default value is specified for the provided setting. */ - getPlaceholder(setting: AccessibilitySetting): string { + getDefaultValue(setting: AccessibilitySetting): string { switch (setting) { case 'notificationTimeOut': return millisecondsToSeconds(environment.notifications.timeOut.toString()); @@ -287,6 +317,28 @@ export class AccessibilitySettingsService { }; } + /** + * Returns true if the provided AccessibilitySetting is valid in regard to the provided formValues. + */ + isValid(setting: AccessibilitySetting, formValues: AccessibilitySettingsFormValues): boolean { + switch (setting) { + case 'notificationTimeOut': + return formValues.notificationTimeOutEnabled ? + hasNoValue(formValues.notificationTimeOut) || parseFloat(formValues.notificationTimeOut) > 0 : + true; + case 'liveRegionTimeOut': + return hasNoValue(formValues.liveRegionTimeOut) || parseFloat(formValues.liveRegionTimeOut) > 0; + default: + throw new Error(`Unhandled accessibility setting during validity check: ${setting}`); + } + } + + /** + * Returns true if all settings in the provided AccessibilitySettingsFormValues object are valid + */ + formValuesValid(formValues: AccessibilitySettingsFormValues) { + return accessibilitySettingKeys.every(setting => this.isValid(setting, formValues)); + } } /** @@ -314,3 +366,11 @@ function millisecondsToSeconds(millisecondsStr: string): string { return (milliseconds / 1000).toString(); } } + +function ofMetadata(): Observable<'metadata'> { + return of('metadata'); +} + +function ofFailed(): Observable<'failed'> { + return of('failed'); +} diff --git a/src/app/info/accessibility-settings/accessibility-settings.component.html b/src/app/info/accessibility-settings/accessibility-settings.component.html index cacfae61fc..9b2eba493b 100644 --- a/src/app/info/accessibility-settings/accessibility-settings.component.html +++ b/src/app/info/accessibility-settings/accessibility-settings.component.html @@ -3,11 +3,11 @@
-