From 7da01d80d810d0da5668ef045001b9a73711b300 Mon Sep 17 00:00:00 2001 From: abhinav Date: Tue, 4 Nov 2025 15:48:15 +0100 Subject: [PATCH 1/2] fix 4241 language selection Consider the language set in the users profile when setting it on page load. Also match languages case-insensitive. Updated tests --- .../core/locale/locale.interceptor.spec.ts | 2 +- src/app/core/locale/locale.interceptor.ts | 3 +- src/app/core/locale/locale.service.spec.ts | 74 ++++++++++++++++--- src/app/core/locale/locale.service.ts | 64 ++++++++++------ 4 files changed, 105 insertions(+), 38 deletions(-) diff --git a/src/app/core/locale/locale.interceptor.spec.ts b/src/app/core/locale/locale.interceptor.spec.ts index e96126d19c8..006dd869589 100644 --- a/src/app/core/locale/locale.interceptor.spec.ts +++ b/src/app/core/locale/locale.interceptor.spec.ts @@ -38,7 +38,7 @@ describe(`LocaleInterceptor`, () => { httpMock = TestBed.inject(HttpTestingController); localeService = TestBed.inject(LocaleService); - localeService.getCurrentLanguageCode.and.returnValue('en'); + localeService.getCurrentLanguageCode.and.returnValue(of('en')); }); describe('', () => { diff --git a/src/app/core/locale/locale.interceptor.ts b/src/app/core/locale/locale.interceptor.ts index 035dad35c92..40829f8e4c7 100644 --- a/src/app/core/locale/locale.interceptor.ts +++ b/src/app/core/locale/locale.interceptor.ts @@ -4,7 +4,7 @@ import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/c import { Observable } from 'rxjs'; import { LocaleService } from './locale.service'; -import { mergeMap, scan } from 'rxjs/operators'; +import { mergeMap, scan, take } from 'rxjs/operators'; @Injectable() export class LocaleInterceptor implements HttpInterceptor { @@ -21,6 +21,7 @@ export class LocaleInterceptor implements HttpInterceptor { let newReq: HttpRequest; return this.localeService.getLanguageCodeList() .pipe( + take(1), scan((acc: any, value: any) => [...acc, value], []), mergeMap((languages) => { // Clone the request to add the new header. diff --git a/src/app/core/locale/locale.service.spec.ts b/src/app/core/locale/locale.service.spec.ts index 39356fdf970..4ca39a0960c 100644 --- a/src/app/core/locale/locale.service.spec.ts +++ b/src/app/core/locale/locale.service.spec.ts @@ -10,6 +10,9 @@ import { AuthService } from '../auth/auth.service'; import { NativeWindowRef } from '../services/window.service'; import { RouteService } from '../services/route.service'; import { routeServiceStub } from '../../shared/testing/route-service.stub'; +import { of } from 'rxjs'; +import { TestScheduler } from 'rxjs/testing'; +import { EPersonMock2 } from '../../shared/testing/eperson.mock'; describe('LocaleService test suite', () => { let service: LocaleService; @@ -25,7 +28,8 @@ describe('LocaleService test suite', () => { authService = jasmine.createSpyObj('AuthService', { isAuthenticated: jasmine.createSpy('isAuthenticated'), - isAuthenticationLoaded: jasmine.createSpy('isAuthenticationLoaded') + isAuthenticationLoaded: jasmine.createSpy('isAuthenticationLoaded'), + getAuthenticatedUserFromStore: jasmine.createSpy('getAuthenticatedUserFromStore'), }); const langList = ['en', 'xx', 'de']; @@ -62,33 +66,80 @@ describe('LocaleService test suite', () => { }); describe('getCurrentLanguageCode', () => { + let testScheduler: TestScheduler; + beforeEach(() => { spyOn(translateService, 'getLangs').and.returnValue(langList); + testScheduler = new TestScheduler((actual, expected) => { + // use jasmine to test equality + expect(actual).toEqual(expected); + }); + authService.isAuthenticated.and.returnValue(of(false)); + authService.isAuthenticationLoaded.and.returnValue(of(false)); }); it('should return the language saved on cookie if it\'s a valid & active language', () => { spyOnGet.and.returnValue('de'); - expect(service.getCurrentLanguageCode()).toBe('de'); + testScheduler.run(({expectObservable}) => { + expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'de' }); + }); }); it('should return the default language if the cookie language is disabled', () => { spyOnGet.and.returnValue('disabled'); - expect(service.getCurrentLanguageCode()).toBe('en'); + testScheduler.run(({expectObservable}) => { + expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'en' }); + }); }); it('should return the default language if the cookie language does not exist', () => { spyOnGet.and.returnValue('does-not-exist'); - expect(service.getCurrentLanguageCode()).toBe('en'); + testScheduler.run(({expectObservable}) => { + expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'en' }); + }); }); it('should return language from browser setting', () => { - spyOn(translateService, 'getBrowserLang').and.returnValue('xx'); - expect(service.getCurrentLanguageCode()).toBe('xx'); + spyOn(service, 'getLanguageCodeList').and.returnValue(of(['xx', 'en'])); + testScheduler.run(({expectObservable}) => { + expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'xx' }); + }); + }); + + it('should match language from browser setting case insensitive', () => { + spyOn(service, 'getLanguageCodeList').and.returnValue(of(['DE', 'en'])); + testScheduler.run(({expectObservable}) => { + expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'DE' }); + }); + }); + }); + + describe('getLanguageCodeList', () => { + let testScheduler: TestScheduler; + + beforeEach(() => { + spyOn(translateService, 'getLangs').and.returnValue(langList); + testScheduler = new TestScheduler((actual, expected) => { + // use jasmine to test equality + expect(actual).toEqual(expected); + }); + }); + + it('should return default language list without user preferred language when no logged in user', () => { + authService.isAuthenticated.and.returnValue(of(false)); + authService.isAuthenticationLoaded.and.returnValue(of(false)); + testScheduler.run(({expectObservable}) => { + expectObservable(service.getLanguageCodeList()).toBe('(a|)', { a: ['en-US;q=1', 'en;q=0.9'] }); + }); }); - it('should return default language from config', () => { - spyOn(translateService, 'getBrowserLang').and.returnValue('fr'); - expect(service.getCurrentLanguageCode()).toBe('en'); + it('should return default language list with user preferred language when user is logged in', () => { + authService.isAuthenticated.and.returnValue(of(true)); + authService.isAuthenticationLoaded.and.returnValue(of(true)); + authService.getAuthenticatedUserFromStore.and.returnValue(of(EPersonMock2)); + testScheduler.run(({expectObservable}) => { + expectObservable(service.getLanguageCodeList()).toBe('(a|)', { a: ['fr;q=0.5', 'en-US;q=1', 'en;q=0.9'] }); + }); }); }); @@ -120,14 +171,13 @@ describe('LocaleService test suite', () => { }); it('should set the current language', () => { - spyOn(service, 'getCurrentLanguageCode').and.returnValue('es'); + spyOn(service, 'getCurrentLanguageCode').and.returnValue(of('es')); service.setCurrentLanguageCode(); expect(translateService.use).toHaveBeenCalledWith('es'); - expect(service.saveLanguageCodeToCookie).toHaveBeenCalledWith('es'); }); it('should set the current language on the html tag', () => { - spyOn(service, 'getCurrentLanguageCode').and.returnValue('es'); + spyOn(service, 'getCurrentLanguageCode').and.returnValue(of('es')); service.setCurrentLanguageCode(); expect((service as any).document.documentElement.lang).toEqual('es'); }); diff --git a/src/app/core/locale/locale.service.ts b/src/app/core/locale/locale.service.ts index 5c080d8c16c..48c450bfe24 100644 --- a/src/app/core/locale/locale.service.ts +++ b/src/app/core/locale/locale.service.ts @@ -1,12 +1,12 @@ -import { Inject, Injectable } from '@angular/core'; +import { Inject, Injectable, OnDestroy } from '@angular/core'; import { TranslateService } from '@ngx-translate/core'; -import { isEmpty, isNotEmpty } from '../../shared/empty.util'; +import { isEmpty, isNotEmpty, hasValue } from '../../shared/empty.util'; import { CookieService } from '../services/cookie.service'; import { environment } from '../../../environments/environment'; import { AuthService } from '../auth/auth.service'; -import { combineLatest, Observable, of as observableOf } from 'rxjs'; +import { combineLatest, Observable, of as observableOf, Subscription } from 'rxjs'; import { map, mergeMap, take } from 'rxjs/operators'; import { NativeWindowRef, NativeWindowService } from '../services/window.service'; import { RouteService } from '../services/route.service'; @@ -28,13 +28,15 @@ export enum LANG_ORIGIN { * Service to provide localization handler */ @Injectable() -export class LocaleService { +export class LocaleService implements OnDestroy { /** * Eperson language metadata */ EPERSON_LANG_METADATA = 'eperson.language'; + subs: Subscription[] = []; + constructor( @Inject(NativeWindowService) protected _window: NativeWindowRef, protected cookie: CookieService, @@ -48,20 +50,25 @@ export class LocaleService { /** * Get the language currently used * - * @returns {string} The language code + * @returns {Observable} The language code */ - getCurrentLanguageCode(): string { + getCurrentLanguageCode(): Observable { // Attempt to get the language from a cookie let lang = this.getLanguageCodeFromCookie(); if (isEmpty(lang) || environment.languages.find((langConfig: LangConfig) => langConfig.code === lang && langConfig.active) === undefined) { // Attempt to get the browser language from the user - if (this.translate.getLangs().includes(this.translate.getBrowserLang())) { - lang = this.translate.getBrowserLang(); - } else { - lang = environment.defaultLanguage; - } + return this.getLanguageCodeList() + .pipe( + map(browserLangs => { + return browserLangs + .map(browserLang => browserLang.split(';')[0]) + .find(browserLang => + this.translate.getLangs().some(userLang => userLang.toLowerCase() === browserLang.toLowerCase()) + ) || environment.defaultLanguage; + }), + ); } - return lang; + return observableOf(lang); } /** @@ -76,11 +83,9 @@ export class LocaleService { ]); return obs$.pipe( - take(1), mergeMap(([isAuthenticated, isLoaded]) => { - // TODO to enabled again when https://github.com/DSpace/dspace-angular/issues/739 will be resolved - const epersonLang$: Observable = observableOf([]); -/* if (isAuthenticated && isLoaded) { + let epersonLang$: Observable = observableOf([]); + if (isAuthenticated && isLoaded) { epersonLang$ = this.authService.getAuthenticatedUserFromStore().pipe( take(1), map((eperson) => { @@ -95,19 +100,19 @@ export class LocaleService { return languages; }) ); - }*/ + } return epersonLang$.pipe( map((epersonLang: string[]) => { const languages: string[] = []; + if (isNotEmpty(epersonLang)) { + languages.push(...epersonLang); + } if (this.translate.currentLang) { languages.push(...this.setQuality( [this.translate.currentLang], LANG_ORIGIN.UI, false)); } - if (isNotEmpty(epersonLang)) { - languages.push(...epersonLang); - } if (navigator.languages) { languages.push(...this.setQuality( Object.assign([], navigator.languages), @@ -147,11 +152,16 @@ export class LocaleService { */ setCurrentLanguageCode(lang?: string): void { if (isEmpty(lang)) { - lang = this.getCurrentLanguageCode(); + this.subs.push(this.getCurrentLanguageCode().subscribe(curLang => { + lang = curLang; + this.translate.use(lang); + this.document.documentElement.lang = lang; + })); + } else { + this.saveLanguageCodeToCookie(lang); + this.translate.use(lang); + this.document.documentElement.lang = lang; } - this.translate.use(lang); - this.saveLanguageCodeToCookie(lang); - this.document.documentElement.lang = lang; } /** @@ -197,4 +207,10 @@ export class LocaleService { } + ngOnDestroy(): void { + this.subs + .filter((sub) => hasValue(sub)) + .forEach((sub) => sub.unsubscribe()); + } + } From ae4dadf1b75ee02c713412165f0852b20a92dd28 Mon Sep 17 00:00:00 2001 From: abhinav Date: Mon, 10 Nov 2025 14:43:15 +0100 Subject: [PATCH 2/2] fix circular find Eperson request --- src/app/core/locale/locale.interceptor.spec.ts | 6 ++++++ src/app/core/locale/locale.interceptor.ts | 8 ++++++-- src/app/core/locale/locale.service.ts | 4 ++-- src/app/core/locale/server-locale.service.ts | 4 ++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/app/core/locale/locale.interceptor.spec.ts b/src/app/core/locale/locale.interceptor.spec.ts index 006dd869589..0d3d51481f4 100644 --- a/src/app/core/locale/locale.interceptor.spec.ts +++ b/src/app/core/locale/locale.interceptor.spec.ts @@ -3,6 +3,7 @@ import { HttpClientTestingModule, HttpTestingController, } from '@angular/common import { HTTP_INTERCEPTORS } from '@angular/common/http'; import { DspaceRestService } from '../dspace-rest/dspace-rest.service'; +import { HALEndpointService } from '../shared/hal-endpoint.service'; import { RestRequestMethod } from '../data/rest-request-method'; import { LocaleService } from './locale.service'; import { LocaleInterceptor } from './locale.interceptor'; @@ -20,6 +21,10 @@ describe(`LocaleInterceptor`, () => { getLanguageCodeList: of(languageList) }); + const mockHalEndpointService = { + getRootHref: jasmine.createSpy('getRootHref'), + }; + beforeEach(() => { TestBed.configureTestingModule({ imports: [HttpClientTestingModule], @@ -30,6 +35,7 @@ describe(`LocaleInterceptor`, () => { useClass: LocaleInterceptor, multi: true, }, + { provide: HALEndpointService, useValue: mockHalEndpointService }, { provide: LocaleService, useValue: mockLocaleService }, ], }); diff --git a/src/app/core/locale/locale.interceptor.ts b/src/app/core/locale/locale.interceptor.ts index 40829f8e4c7..c29b81858c4 100644 --- a/src/app/core/locale/locale.interceptor.ts +++ b/src/app/core/locale/locale.interceptor.ts @@ -3,13 +3,17 @@ import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/c import { Observable } from 'rxjs'; +import { HALEndpointService } from '../shared/hal-endpoint.service'; import { LocaleService } from './locale.service'; import { mergeMap, scan, take } from 'rxjs/operators'; @Injectable() export class LocaleInterceptor implements HttpInterceptor { - constructor(private localeService: LocaleService) { + constructor( + protected halEndpointService: HALEndpointService, + protected localeService: LocaleService, + ) { } /** @@ -19,7 +23,7 @@ export class LocaleInterceptor implements HttpInterceptor { */ intercept(req: HttpRequest, next: HttpHandler): Observable> { let newReq: HttpRequest; - return this.localeService.getLanguageCodeList() + return this.localeService.getLanguageCodeList(req.url === this.halEndpointService.getRootHref()) .pipe( take(1), scan((acc: any, value: any) => [...acc, value], []), diff --git a/src/app/core/locale/locale.service.ts b/src/app/core/locale/locale.service.ts index 48c450bfe24..11cad7ac126 100644 --- a/src/app/core/locale/locale.service.ts +++ b/src/app/core/locale/locale.service.ts @@ -76,7 +76,7 @@ export class LocaleService implements OnDestroy { * * @returns {Observable} */ - getLanguageCodeList(): Observable { + getLanguageCodeList(ignoreEPersonSettings = false): Observable { const obs$ = combineLatest([ this.authService.isAuthenticated(), this.authService.isAuthenticationLoaded() @@ -85,7 +85,7 @@ export class LocaleService implements OnDestroy { return obs$.pipe( mergeMap(([isAuthenticated, isLoaded]) => { let epersonLang$: Observable = observableOf([]); - if (isAuthenticated && isLoaded) { + if (isAuthenticated && isLoaded && !ignoreEPersonSettings) { epersonLang$ = this.authService.getAuthenticatedUserFromStore().pipe( take(1), map((eperson) => { diff --git a/src/app/core/locale/server-locale.service.ts b/src/app/core/locale/server-locale.service.ts index 556619b9460..5911b74c5fa 100644 --- a/src/app/core/locale/server-locale.service.ts +++ b/src/app/core/locale/server-locale.service.ts @@ -31,7 +31,7 @@ export class ServerLocaleService extends LocaleService { * * @returns {Observable} */ - getLanguageCodeList(): Observable { + getLanguageCodeList(ignoreEPersonSettings = false): Observable { const obs$ = combineLatest([ this.authService.isAuthenticated(), this.authService.isAuthenticationLoaded() @@ -41,7 +41,7 @@ export class ServerLocaleService extends LocaleService { take(1), mergeMap(([isAuthenticated, isLoaded]) => { let epersonLang$: Observable = observableOf([]); - if (isAuthenticated && isLoaded) { + if (isAuthenticated && isLoaded && !ignoreEPersonSettings) { epersonLang$ = this.authService.getAuthenticatedUserFromStore().pipe( take(1), map((eperson) => {