Fix DSpace#4241 language selection (DSpace#4805)

AbhinavS96 · abhinav · web-flow · commit bf0452750418 · 2025-11-17T16:45:24.000-06:00
* 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

* fix circular find Eperson request

---------

Co-authored-by: abhinav &lt;abhinav.sidharthan@atmire.com&gt;
diff --git a/src/app/core/locale/locale.interceptor.spec.ts b/src/app/core/locale/locale.interceptor.spec.ts
@@ -12,6 +12,7 @@ import { RestRequestMethod } from '@dspace/config/rest-request-method';
 import { of } from 'rxjs';
 
 import { DspaceRestService } from '../dspace-rest/dspace-rest.service';
+import { HALEndpointService } from '../shared/hal-endpoint.service';
 import { LocaleInterceptor } from './locale.interceptor';
 import { LocaleService } from './locale.service';
 
@@ -27,6 +28,10 @@ describe(`LocaleInterceptor`, () => {
     getLanguageCodeList: of(languageList),
   });
 
+  const mockHalEndpointService = {
+    getRootHref: jasmine.createSpy('getRootHref'),
+  };
+
   beforeEach(() => {
     TestBed.configureTestingModule({
       imports: [],
@@ -37,6 +42,7 @@ describe(`LocaleInterceptor`, () => {
           useClass: LocaleInterceptor,
           multi: true,
         },
+        { provide: HALEndpointService, useValue: mockHalEndpointService },
         { provide: LocaleService, useValue: mockLocaleService },
         provideHttpClient(withInterceptorsFromDi()),
         provideHttpClientTesting(),
@@ -47,7 +53,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
@@ -9,14 +9,19 @@ import { Observable } from 'rxjs';
 import {
   mergeMap,
   scan,
+  take,
 } from 'rxjs/operators';
 
+import { HALEndpointService } from '../shared/hal-endpoint.service';
 import { LocaleService } from './locale.service';
 
 @Injectable()
 export class LocaleInterceptor implements HttpInterceptor {
 
-  constructor(private localeService: LocaleService) {
+  constructor(
+    protected halEndpointService: HALEndpointService,
+    protected localeService: LocaleService,
+  ) {
   }
 
   /**
@@ -26,8 +31,9 @@ export class LocaleInterceptor implements HttpInterceptor {
    */
   intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
     let newReq: HttpRequest<any>;
-    return this.localeService.getLanguageCodeList()
+    return this.localeService.getLanguageCodeList(req.url === this.halEndpointService.getRootHref())
       .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
@@ -3,11 +3,14 @@ import {
   waitForAsync,
 } from '@angular/core/testing';
 import { APP_CONFIG } from '@dspace/config/app-config.interface';
+import { EPersonMock2 } from '@dspace/core/testing/eperson.mock';
 import {
   TranslateLoader,
   TranslateModule,
   TranslateService,
 } from '@ngx-translate/core';
+import { of } from 'rxjs';
+import { TestScheduler } from 'rxjs/testing';
 
 import { AuthService } from '../auth/auth.service';
 import { CookieService } from '../cookies/cookie.service';
@@ -78,6 +81,7 @@ describe('LocaleService test suite', () => {
   authService = jasmine.createSpyObj('AuthService', {
     isAuthenticated: jasmine.createSpy('isAuthenticated'),
     isAuthenticationLoaded: jasmine.createSpy('isAuthenticationLoaded'),
+    getAuthenticatedUserFromStore: jasmine.createSpy('getAuthenticatedUserFromStore'),
   });
 
   const langList = ['en', 'xx', 'de'];
@@ -116,33 +120,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'] });
+      });
     });
   });
 
@@ -174,14 +225,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
@@ -3,13 +3,15 @@ import {
   Inject,
   inject,
   Injectable,
+  OnDestroy,
 } from '@angular/core';
 import {
   APP_CONFIG,
   AppConfig,
 } from '@dspace/config/app-config.interface';
 import { LangConfig } from '@dspace/config/lang-config.interface';
 import {
+  hasValue,
   isEmpty,
   isNotEmpty,
 } from '@dspace/shared/utils/empty.util';
@@ -18,6 +20,7 @@ import {
   combineLatest,
   Observable,
   of,
+  Subscription,
 } from 'rxjs';
 import {
   map,
@@ -48,13 +51,15 @@ export enum LANG_ORIGIN {
  * Service to provide localization handler
  */
 @Injectable()
-export class LocaleService {
+export class LocaleService implements OnDestroy {
   protected readonly appConfig: AppConfig = inject(APP_CONFIG);
   /**
    * Eperson language metadata
    */
   EPERSON_LANG_METADATA = 'eperson.language';
 
+  subs: Subscription[] = [];
+
   constructor(
     @Inject(NativeWindowService) protected _window: NativeWindowRef,
     protected cookie: CookieService,
@@ -68,39 +73,42 @@ 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();
+    const lang = this.getLanguageCodeFromCookie();
     if (isEmpty(lang) || this.appConfig.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 = this.appConfig.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()),
+              ) || this.appConfig.defaultLanguage;
+          }),
+        );
     }
-    return lang;
+    return of(lang);
   }
 
   /**
    * Get the languages list of the user in Accept-Language format
    *
    * @returns {Observable<string[]>}
    */
-  getLanguageCodeList(): Observable<string[]> {
+  getLanguageCodeList(ignoreEPersonSettings = false): Observable<string[]> {
     const obs$ = combineLatest([
       this.authService.isAuthenticated(),
       this.authService.isAuthenticationLoaded(),
     ]);
 
     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[]> = of([]);
-        /*        if (isAuthenticated && isLoaded) {
+        let epersonLang$: Observable<string[]> = of([]);
+        if (isAuthenticated && isLoaded && !ignoreEPersonSettings) {
           epersonLang$ = this.authService.getAuthenticatedUserFromStore().pipe(
             take(1),
             map((eperson) => {
@@ -113,21 +121,21 @@ export class LocaleService {
                   !isEmpty(this.translate.currentLang)));
               }
               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),
@@ -167,11 +175,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;
   }
 
   /**
@@ -217,4 +230,10 @@ export class LocaleService {
 
   }
 
+  ngOnDestroy(): void {
+    this.subs
+      .filter((sub) => hasValue(sub))
+      .forEach((sub) => sub.unsubscribe());
+  }
+
 }
diff --git a/src/app/core/locale/server-locale.service.ts b/src/app/core/locale/server-locale.service.ts
@@ -53,7 +53,7 @@ export class ServerLocaleService extends LocaleService {
    *
    * @returns {Observable<string[]>}
    */
-  getLanguageCodeList(): Observable<string[]> {
+  getLanguageCodeList(ignoreEPersonSettings = false): Observable<string[]> {
     const obs$ = combineLatest([
       this.authService.isAuthenticated(),
       this.authService.isAuthenticationLoaded(),
@@ -63,7 +63,7 @@ export class ServerLocaleService extends LocaleService {
       take(1),
       mergeMap(([isAuthenticated, isLoaded]) => {
         let epersonLang$: Observable<string[]> = of([]);
-        if (isAuthenticated && isLoaded) {
+        if (isAuthenticated && isLoaded && !ignoreEPersonSettings) {
           epersonLang$ = this.authService.getAuthenticatedUserFromStore().pipe(
             take(1),
             map((eperson) => {
