Skip to content

Commit bf04527

Browse files
AbhinavS96abhinav
AbhinavS96
and
abhinav
authored
Fix DSpace#4241 language selection (DSpace#4805)
* 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 <[email protected]>
1 parent 2f3c3c7 commit bf04527

File tree

5 files changed

+121
-40
lines changed

5 files changed

+121
-40
lines changed

‎src/app/core/locale/locale.interceptor.spec.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { RestRequestMethod } from '@dspace/config/rest-request-method';
1212
import { of } from 'rxjs';
1313

1414
import { DspaceRestService } from '../dspace-rest/dspace-rest.service';
15+
import { HALEndpointService } from '../shared/hal-endpoint.service';
1516
import { LocaleInterceptor } from './locale.interceptor';
1617
import { LocaleService } from './locale.service';
1718

@@ -27,6 +28,10 @@ describe(`LocaleInterceptor`, () => {
2728
getLanguageCodeList: of(languageList),
2829
});
2930

31+
const mockHalEndpointService = {
32+
getRootHref: jasmine.createSpy('getRootHref'),
33+
};
34+
3035
beforeEach(() => {
3136
TestBed.configureTestingModule({
3237
imports: [],
@@ -37,6 +42,7 @@ describe(`LocaleInterceptor`, () => {
3742
useClass: LocaleInterceptor,
3843
multi: true,
3944
},
45+
{ provide: HALEndpointService, useValue: mockHalEndpointService },
4046
{ provide: LocaleService, useValue: mockLocaleService },
4147
provideHttpClient(withInterceptorsFromDi()),
4248
provideHttpClientTesting(),
@@ -47,7 +53,7 @@ describe(`LocaleInterceptor`, () => {
4753
httpMock = TestBed.inject(HttpTestingController);
4854
localeService = TestBed.inject(LocaleService);
4955

50-
localeService.getCurrentLanguageCode.and.returnValue('en');
56+
localeService.getCurrentLanguageCode.and.returnValue(of('en'));
5157
});
5258

5359
describe('', () => {

‎src/app/core/locale/locale.interceptor.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,19 @@ import { Observable } from 'rxjs';
99
import {
1010
mergeMap,
1111
scan,
12+
take,
1213
} from 'rxjs/operators';
1314

15+
import { HALEndpointService } from '../shared/hal-endpoint.service';
1416
import { LocaleService } from './locale.service';
1517

1618
@Injectable()
1719
export class LocaleInterceptor implements HttpInterceptor {
1820

19-
constructor(private localeService: LocaleService) {
21+
constructor(
22+
protected halEndpointService: HALEndpointService,
23+
protected localeService: LocaleService,
24+
) {
2025
}
2126

2227
/**
@@ -26,8 +31,9 @@ export class LocaleInterceptor implements HttpInterceptor {
2631
*/
2732
intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
2833
let newReq: HttpRequest<any>;
29-
return this.localeService.getLanguageCodeList()
34+
return this.localeService.getLanguageCodeList(req.url === this.halEndpointService.getRootHref())
3035
.pipe(
36+
take(1),
3137
scan((acc: any, value: any) => [...acc, value], []),
3238
mergeMap((languages) => {
3339
// Clone the request to add the new header.

‎src/app/core/locale/locale.service.spec.ts‎

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@ import {
33
waitForAsync,
44
} from '@angular/core/testing';
55
import { APP_CONFIG } from '@dspace/config/app-config.interface';
6+
import { EPersonMock2 } from '@dspace/core/testing/eperson.mock';
67
import {
78
TranslateLoader,
89
TranslateModule,
910
TranslateService,
1011
} from '@ngx-translate/core';
12+
import { of } from 'rxjs';
13+
import { TestScheduler } from 'rxjs/testing';
1114

1215
import { AuthService } from '../auth/auth.service';
1316
import { CookieService } from '../cookies/cookie.service';
@@ -78,6 +81,7 @@ describe('LocaleService test suite', () => {
7881
authService = jasmine.createSpyObj('AuthService', {
7982
isAuthenticated: jasmine.createSpy('isAuthenticated'),
8083
isAuthenticationLoaded: jasmine.createSpy('isAuthenticationLoaded'),
84+
getAuthenticatedUserFromStore: jasmine.createSpy('getAuthenticatedUserFromStore'),
8185
});
8286

8387
const langList = ['en', 'xx', 'de'];
@@ -116,33 +120,80 @@ describe('LocaleService test suite', () => {
116120
});
117121

118122
describe('getCurrentLanguageCode', () => {
123+
let testScheduler: TestScheduler;
124+
119125
beforeEach(() => {
120126
spyOn(translateService, 'getLangs').and.returnValue(langList);
127+
testScheduler = new TestScheduler((actual, expected) => {
128+
// use jasmine to test equality
129+
expect(actual).toEqual(expected);
130+
});
131+
authService.isAuthenticated.and.returnValue(of(false));
132+
authService.isAuthenticationLoaded.and.returnValue(of(false));
121133
});
122134

123135
it('should return the language saved on cookie if it\'s a valid & active language', () => {
124136
spyOnGet.and.returnValue('de');
125-
expect(service.getCurrentLanguageCode()).toBe('de');
137+
testScheduler.run(({ expectObservable }) => {
138+
expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'de' });
139+
});
126140
});
127141

128142
it('should return the default language if the cookie language is disabled', () => {
129143
spyOnGet.and.returnValue('disabled');
130-
expect(service.getCurrentLanguageCode()).toBe('en');
144+
testScheduler.run(({ expectObservable }) => {
145+
expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'en' });
146+
});
131147
});
132148

133149
it('should return the default language if the cookie language does not exist', () => {
134150
spyOnGet.and.returnValue('does-not-exist');
135-
expect(service.getCurrentLanguageCode()).toBe('en');
151+
testScheduler.run(({ expectObservable }) => {
152+
expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'en' });
153+
});
136154
});
137155

138156
it('should return language from browser setting', () => {
139-
spyOn(translateService, 'getBrowserLang').and.returnValue('xx');
140-
expect(service.getCurrentLanguageCode()).toBe('xx');
157+
spyOn(service, 'getLanguageCodeList').and.returnValue(of(['xx', 'en']));
158+
testScheduler.run(({ expectObservable }) => {
159+
expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'xx' });
160+
});
161+
});
162+
163+
it('should match language from browser setting case insensitive', () => {
164+
spyOn(service, 'getLanguageCodeList').and.returnValue(of(['DE', 'en']));
165+
testScheduler.run(({ expectObservable }) => {
166+
expectObservable(service.getCurrentLanguageCode()).toBe('(a|)', { a: 'DE' });
167+
});
168+
});
169+
});
170+
171+
describe('getLanguageCodeList', () => {
172+
let testScheduler: TestScheduler;
173+
174+
beforeEach(() => {
175+
spyOn(translateService, 'getLangs').and.returnValue(langList);
176+
testScheduler = new TestScheduler((actual, expected) => {
177+
// use jasmine to test equality
178+
expect(actual).toEqual(expected);
179+
});
180+
});
181+
182+
it('should return default language list without user preferred language when no logged in user', () => {
183+
authService.isAuthenticated.and.returnValue(of(false));
184+
authService.isAuthenticationLoaded.and.returnValue(of(false));
185+
testScheduler.run(({ expectObservable }) => {
186+
expectObservable(service.getLanguageCodeList()).toBe('(a|)', { a: ['en-US;q=1', 'en;q=0.9'] });
187+
});
141188
});
142189

143-
it('should return default language from config', () => {
144-
spyOn(translateService, 'getBrowserLang').and.returnValue('fr');
145-
expect(service.getCurrentLanguageCode()).toBe('en');
190+
it('should return default language list with user preferred language when user is logged in', () => {
191+
authService.isAuthenticated.and.returnValue(of(true));
192+
authService.isAuthenticationLoaded.and.returnValue(of(true));
193+
authService.getAuthenticatedUserFromStore.and.returnValue(of(EPersonMock2));
194+
testScheduler.run(({ expectObservable }) => {
195+
expectObservable(service.getLanguageCodeList()).toBe('(a|)', { a: ['fr;q=0.5', 'en-US;q=1', 'en;q=0.9'] });
196+
});
146197
});
147198
});
148199

@@ -174,14 +225,13 @@ describe('LocaleService test suite', () => {
174225
});
175226

176227
it('should set the current language', () => {
177-
spyOn(service, 'getCurrentLanguageCode').and.returnValue('es');
228+
spyOn(service, 'getCurrentLanguageCode').and.returnValue(of('es'));
178229
service.setCurrentLanguageCode();
179230
expect(translateService.use).toHaveBeenCalledWith('es');
180-
expect(service.saveLanguageCodeToCookie).toHaveBeenCalledWith('es');
181231
});
182232

183233
it('should set the current language on the html tag', () => {
184-
spyOn(service, 'getCurrentLanguageCode').and.returnValue('es');
234+
spyOn(service, 'getCurrentLanguageCode').and.returnValue(of('es'));
185235
service.setCurrentLanguageCode();
186236
expect((service as any).document.documentElement.lang).toEqual('es');
187237
});

‎src/app/core/locale/locale.service.ts‎

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ import {
33
Inject,
44
inject,
55
Injectable,
6+
OnDestroy,
67
} from '@angular/core';
78
import {
89
APP_CONFIG,
910
AppConfig,
1011
} from '@dspace/config/app-config.interface';
1112
import { LangConfig } from '@dspace/config/lang-config.interface';
1213
import {
14+
hasValue,
1315
isEmpty,
1416
isNotEmpty,
1517
} from '@dspace/shared/utils/empty.util';
@@ -18,6 +20,7 @@ import {
1820
combineLatest,
1921
Observable,
2022
of,
23+
Subscription,
2124
} from 'rxjs';
2225
import {
2326
map,
@@ -48,13 +51,15 @@ export enum LANG_ORIGIN {
4851
* Service to provide localization handler
4952
*/
5053
@Injectable()
51-
export class LocaleService {
54+
export class LocaleService implements OnDestroy {
5255
protected readonly appConfig: AppConfig = inject(APP_CONFIG);
5356
/**
5457
* Eperson language metadata
5558
*/
5659
EPERSON_LANG_METADATA = 'eperson.language';
5760

61+
subs: Subscription[] = [];
62+
5863
constructor(
5964
@Inject(NativeWindowService) protected _window: NativeWindowRef,
6065
protected cookie: CookieService,
@@ -68,39 +73,42 @@ export class LocaleService {
6873
/**
6974
* Get the language currently used
7075
*
71-
* @returns {string} The language code
76+
* @returns {Observable<string>} The language code
7277
*/
73-
getCurrentLanguageCode(): string {
78+
getCurrentLanguageCode(): Observable<string> {
7479
// Attempt to get the language from a cookie
75-
let lang = this.getLanguageCodeFromCookie();
80+
const lang = this.getLanguageCodeFromCookie();
7681
if (isEmpty(lang) || this.appConfig.languages.find((langConfig: LangConfig) => langConfig.code === lang && langConfig.active) === undefined) {
7782
// Attempt to get the browser language from the user
78-
if (this.translate.getLangs().includes(this.translate.getBrowserLang())) {
79-
lang = this.translate.getBrowserLang();
80-
} else {
81-
lang = this.appConfig.defaultLanguage;
82-
}
83+
return this.getLanguageCodeList()
84+
.pipe(
85+
map(browserLangs => {
86+
return browserLangs
87+
.map(browserLang => browserLang.split(';')[0])
88+
.find(browserLang =>
89+
this.translate.getLangs().some(userLang => userLang.toLowerCase() === browserLang.toLowerCase()),
90+
) || this.appConfig.defaultLanguage;
91+
}),
92+
);
8393
}
84-
return lang;
94+
return of(lang);
8595
}
8696

8797
/**
8898
* Get the languages list of the user in Accept-Language format
8999
*
90100
* @returns {Observable<string[]>}
91101
*/
92-
getLanguageCodeList(): Observable<string[]> {
102+
getLanguageCodeList(ignoreEPersonSettings = false): Observable<string[]> {
93103
const obs$ = combineLatest([
94104
this.authService.isAuthenticated(),
95105
this.authService.isAuthenticationLoaded(),
96106
]);
97107

98108
return obs$.pipe(
99-
take(1),
100109
mergeMap(([isAuthenticated, isLoaded]) => {
101-
// TODO to enabled again when https://github.com/DSpace/dspace-angular/issues/739 will be resolved
102-
const epersonLang$: Observable<string[]> = of([]);
103-
/* if (isAuthenticated && isLoaded) {
110+
let epersonLang$: Observable<string[]> = of([]);
111+
if (isAuthenticated && isLoaded && !ignoreEPersonSettings) {
104112
epersonLang$ = this.authService.getAuthenticatedUserFromStore().pipe(
105113
take(1),
106114
map((eperson) => {
@@ -113,21 +121,21 @@ export class LocaleService {
113121
!isEmpty(this.translate.currentLang)));
114122
}
115123
return languages;
116-
})
124+
}),
117125
);
118-
}*/
126+
}
119127
return epersonLang$.pipe(
120128
map((epersonLang: string[]) => {
121129
const languages: string[] = [];
130+
if (isNotEmpty(epersonLang)) {
131+
languages.push(...epersonLang);
132+
}
122133
if (this.translate.currentLang) {
123134
languages.push(...this.setQuality(
124135
[this.translate.currentLang],
125136
LANG_ORIGIN.UI,
126137
false));
127138
}
128-
if (isNotEmpty(epersonLang)) {
129-
languages.push(...epersonLang);
130-
}
131139
if (navigator.languages) {
132140
languages.push(...this.setQuality(
133141
Object.assign([], navigator.languages),
@@ -167,11 +175,16 @@ export class LocaleService {
167175
*/
168176
setCurrentLanguageCode(lang?: string): void {
169177
if (isEmpty(lang)) {
170-
lang = this.getCurrentLanguageCode();
178+
this.subs.push(this.getCurrentLanguageCode().subscribe(curLang => {
179+
lang = curLang;
180+
this.translate.use(lang);
181+
this.document.documentElement.lang = lang;
182+
}));
183+
} else {
184+
this.saveLanguageCodeToCookie(lang);
185+
this.translate.use(lang);
186+
this.document.documentElement.lang = lang;
171187
}
172-
this.translate.use(lang);
173-
this.saveLanguageCodeToCookie(lang);
174-
this.document.documentElement.lang = lang;
175188
}
176189

177190
/**
@@ -217,4 +230,10 @@ export class LocaleService {
217230

218231
}
219232

233+
ngOnDestroy(): void {
234+
this.subs
235+
.filter((sub) => hasValue(sub))
236+
.forEach((sub) => sub.unsubscribe());
237+
}
238+
220239
}

‎src/app/core/locale/server-locale.service.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export class ServerLocaleService extends LocaleService {
5353
*
5454
* @returns {Observable<string[]>}
5555
*/
56-
getLanguageCodeList(): Observable<string[]> {
56+
getLanguageCodeList(ignoreEPersonSettings = false): Observable<string[]> {
5757
const obs$ = combineLatest([
5858
this.authService.isAuthenticated(),
5959
this.authService.isAuthenticationLoaded(),
@@ -63,7 +63,7 @@ export class ServerLocaleService extends LocaleService {
6363
take(1),
6464
mergeMap(([isAuthenticated, isLoaded]) => {
6565
let epersonLang$: Observable<string[]> = of([]);
66-
if (isAuthenticated && isLoaded) {
66+
if (isAuthenticated && isLoaded && !ignoreEPersonSettings) {
6767
epersonLang$ = this.authService.getAuthenticatedUserFromStore().pipe(
6868
take(1),
6969
map((eperson) => {

0 commit comments

Comments
 (0)