fix 4241 language selection

abhinav · abhinav · commit 7da01d80d810 · 2025-11-06T16:46:25.000+01:00
Consider the language set in the users profile when setting it on page load. Also match languages case-insensitive. Updated tests
diff --git 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
@@ -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<any>;
     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
@@ -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
@@ -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<string>} The language code
    */
-  getCurrentLanguageCode(): string {
+  getCurrentLanguageCode(): Observable<string> {
     // 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<string[]> = observableOf([]);
-/*        if (isAuthenticated && isLoaded) {
+        let epersonLang$: Observable<string[]> = 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());
+  }
+
 }
