From 4200f9abce22d54888fbc30e7af4d318618378e5 Mon Sep 17 00:00:00 2001 From: Denys Vuika Date: Wed, 21 Jan 2026 08:02:54 +0000 Subject: [PATCH] AAE-40748 Bug fixes for translation and storage services (#11551) * AAE-40748 Various i18n and storage bug fixes * refactor: update NoopTranslateModule to use provideHttpClient and provideHttpClientTesting Replaced HttpClientTestingModule with provideHttpClient and provideHttpClientTesting for improved dependency injection in the NoopTranslateModule. * refactor: update FileUploadErrorPipe tests to use TestBed for dependency injection Replaced direct instantiation of FileUploadErrorPipe with TestBed configuration, utilizing NoopTranslateModule for improved testing practices. * test: enhance StorageService and UserPreferencesService tests for improved initialization logic Added tests for UserPreferencesService to verify locale and pagination initialization from storage and config. Updated StorageService tests to restore original localStorage after each test for better isolation. * refactor: simplify TranslationService by removing direct StorageService dependency --- .../pipes/file-upload-error.pipe.spec.ts | 9 ++- .../common/services/storage.service.spec.ts | 9 +++ .../services/user-preferences.service.spec.ts | 73 +++++++++++++++++++ .../services/user-preferences.service.ts | 49 ++++++++++--- .../src/lib/mock/translation.service.mock.ts | 3 +- .../src/lib/testing/noop-translate.module.ts | 8 +- .../lib/translation/translation.service.ts | 20 +++-- 7 files changed, 148 insertions(+), 23 deletions(-) diff --git a/lib/content-services/src/lib/upload/pipes/file-upload-error.pipe.spec.ts b/lib/content-services/src/lib/upload/pipes/file-upload-error.pipe.spec.ts index 9eb0aa7f69..36828c7040 100644 --- a/lib/content-services/src/lib/upload/pipes/file-upload-error.pipe.spec.ts +++ b/lib/content-services/src/lib/upload/pipes/file-upload-error.pipe.spec.ts @@ -15,14 +15,19 @@ * limitations under the License. */ -import { NoopTranslationService } from '@alfresco/adf-core'; +import { NoopTranslateModule } from '@alfresco/adf-core'; import { FileUploadErrorPipe } from './file-upload-error.pipe'; +import { TestBed } from '@angular/core/testing'; describe('FileUploadErrorPipe', () => { let pipe: FileUploadErrorPipe; beforeEach(() => { - pipe = new FileUploadErrorPipe(new NoopTranslationService()); + TestBed.configureTestingModule({ + imports: [NoopTranslateModule], + providers: [FileUploadErrorPipe] + }); + pipe = TestBed.inject(FileUploadErrorPipe); }); it('should return generic message when error code is null', () => { diff --git a/lib/core/src/lib/common/services/storage.service.spec.ts b/lib/core/src/lib/common/services/storage.service.spec.ts index 86fd50ffdb..84aa4b061b 100644 --- a/lib/core/src/lib/common/services/storage.service.spec.ts +++ b/lib/core/src/lib/common/services/storage.service.spec.ts @@ -115,6 +115,8 @@ describe('StorageService', () => { }); describe('with memory storage', () => { + const originalLocalStorage = window.localStorage; + beforeEach(async () => { Object.defineProperty(window, 'localStorage', { value: undefined, @@ -130,6 +132,13 @@ describe('StorageService', () => { storage.clear(); }); + afterEach(() => { + Object.defineProperty(window, 'localStorage', { + value: originalLocalStorage, + configurable: true + }); + }); + it('should be able to get a property from storage', () => { storage.setItem(key, value); diff --git a/lib/core/src/lib/common/services/user-preferences.service.spec.ts b/lib/core/src/lib/common/services/user-preferences.service.spec.ts index dabc4f582a..4127ca42a6 100644 --- a/lib/core/src/lib/common/services/user-preferences.service.spec.ts +++ b/lib/core/src/lib/common/services/user-preferences.service.spec.ts @@ -165,6 +165,79 @@ describe('UserPreferencesService', () => { }); }); + describe('initialization', () => { + it('should use setWithoutStore for locale when it is already in storage', () => { + const spySetWithoutStore = spyOn(preferences, 'setWithoutStore').and.callThrough(); + const spySet = spyOn(preferences, 'set').and.callThrough(); + + storage.setItem('GUEST__locale', 'fr'); + appConfig.config.locale = 'en'; + + preferences.setStoragePrefix(null); + + expect(preferences.locale).toBe('fr'); + expect(spySetWithoutStore).toHaveBeenCalledWith(UserPreferenceValues.Locale, 'fr'); + expect(spySet).not.toHaveBeenCalledWith(UserPreferenceValues.Locale, jasmine.any(String)); + }); + + it('should use set for locale when it is in config but not in storage', () => { + const spySet = spyOn(preferences, 'set').and.callThrough(); + + appConfig.config.locale = 'de'; + + preferences.setStoragePrefix(null); + + expect(preferences.locale).toBe('de'); + expect(spySet).toHaveBeenCalledWith(UserPreferenceValues.Locale, 'de'); + expect(storage.getItem('GUEST__locale')).toBe('de'); + }); + + it('should use defaults and setWithoutStore for locale when neither storage nor config has values', () => { + const spySetWithoutStore = spyOn(preferences, 'setWithoutStore').and.callThrough(); + const spySet = spyOn(preferences, 'set').and.callThrough(); + + delete appConfig.config.locale; + spyOn(translate, 'getBrowserCultureLang').and.returnValue(null); + + preferences.setStoragePrefix(null); + + expect(preferences.locale).toBe('en'); // default + expect(spySetWithoutStore).toHaveBeenCalledWith(UserPreferenceValues.Locale, 'en'); + expect(spySet).not.toHaveBeenCalledWith(UserPreferenceValues.Locale, jasmine.any(String)); + }); + + it('should use setWithoutStore for pagination when it is already in storage', () => { + const spySetWithoutStore = spyOn(preferences, 'setWithoutStore').and.callThrough(); + const spySet = spyOn(preferences, 'set').and.callThrough(); + + storage.setItem('GUEST__paginationSize', '50'); + storage.setItem('GUEST__supportedPageSizes', JSON.stringify([10, 20, 50])); + appConfig.config.pagination = { size: 10, supportedPageSizes: [5, 10] }; + + preferences.setStoragePrefix(null); + + expect(preferences.paginationSize).toBe(50); + expect(preferences.supportedPageSizes).toEqual([10, 20, 50]); + expect(spySetWithoutStore).toHaveBeenCalledWith(UserPreferenceValues.PaginationSize, 50); + expect(spySetWithoutStore).toHaveBeenCalledWith(UserPreferenceValues.SupportedPageSizes, JSON.stringify([10, 20, 50])); + expect(spySet).not.toHaveBeenCalledWith(UserPreferenceValues.PaginationSize, jasmine.any(Number)); + expect(spySet).not.toHaveBeenCalledWith(UserPreferenceValues.SupportedPageSizes, jasmine.any(String)); + }); + + it('should use set for pagination when it is in config but not in storage', () => { + const spySet = spyOn(preferences, 'set').and.callThrough(); + + appConfig.config.pagination = { size: 15, supportedPageSizes: [5, 15] }; + + preferences.setStoragePrefix(null); + + expect(preferences.paginationSize).toBe(15); + expect(spySet).toHaveBeenCalledWith(UserPreferenceValues.PaginationSize, 15); + expect(spySet).toHaveBeenCalledWith(UserPreferenceValues.SupportedPageSizes, JSON.stringify([5, 15])); + expect(storage.getItem('GUEST__paginationSize')).toBe('15'); + }); + }); + describe('with language config', () => { it('should store default textOrientation based on language', () => { appConfig.config.languages = [ diff --git a/lib/core/src/lib/common/services/user-preferences.service.ts b/lib/core/src/lib/common/services/user-preferences.service.ts index 6fe0e99b0c..b8327ee6d4 100644 --- a/lib/core/src/lib/common/services/user-preferences.service.ts +++ b/lib/core/src/lib/common/services/user-preferences.service.ts @@ -50,7 +50,7 @@ export class UserPreferencesService { expandedSidenav: true }; - private userPreferenceStatus: any = this.defaults; + private userPreferenceStatus: any = { ...this.defaults }; private onChangeSubject: BehaviorSubject; onChange: Observable; @@ -140,19 +140,48 @@ export class UserPreferencesService { private initUserPreferenceStatus() { this.initUserLanguage(); - this.set(UserPreferenceValues.PaginationSize, this.paginationSize); - this.set(UserPreferenceValues.SupportedPageSizes, JSON.stringify(this.supportedPageSizes)); + this.initPaginationPreferences(); + } + + private initPaginationPreferences() { + // Check if values are already in storage + const storedPaginationSize = this.get(UserPreferenceValues.PaginationSize); + const storedSupportedPageSizes = this.get(UserPreferenceValues.SupportedPageSizes); + + if (storedPaginationSize) { + // Already in storage - just update in-memory state + this.setWithoutStore(UserPreferenceValues.PaginationSize, Number(storedPaginationSize)); + } else { + // Not in storage - get from config and save + const paginationSize = this.appConfig.get('pagination.size', this.defaults.paginationSize); + this.set(UserPreferenceValues.PaginationSize, paginationSize); + } + + if (storedSupportedPageSizes) { + // Already in storage - just update in-memory state + this.setWithoutStore(UserPreferenceValues.SupportedPageSizes, storedSupportedPageSizes); + } else { + // Not in storage - get from config and save + const supportedPageSizes = this.appConfig.get('pagination.supportedPageSizes', this.defaults.supportedPageSizes); + this.set(UserPreferenceValues.SupportedPageSizes, JSON.stringify(supportedPageSizes)); + } } private initUserLanguage() { - if (this.locale || this.appConfig.get(UserPreferenceValues.Locale)) { - const locale = this.locale || this.getDefaultLocale(); + const storedLocale = this.get(UserPreferenceValues.Locale); + const configLocale = this.appConfig.get(UserPreferenceValues.Locale); - this.set(UserPreferenceValues.Locale, locale); - this.set('textOrientation', this.getLanguageByKey(locale).direction || 'ltr'); + if (storedLocale) { + // Locale already in storage - just update in-memory state, don't re-save + this.setWithoutStore(UserPreferenceValues.Locale, storedLocale); + this.setWithoutStore('textOrientation', this.getLanguageByKey(storedLocale).direction || 'ltr'); + } else if (configLocale) { + // Locale from config but not in storage - save to storage + this.set(UserPreferenceValues.Locale, configLocale); + this.set('textOrientation', this.getLanguageByKey(configLocale).direction || 'ltr'); } else { - const locale = this.locale || this.getDefaultLocale(); - + // No locale anywhere - use default, don't save to storage + const locale = this.getDefaultLocale(); this.setWithoutStore(UserPreferenceValues.Locale, locale); this.setWithoutStore('textOrientation', this.getLanguageByKey(locale).direction || 'ltr'); } @@ -298,7 +327,7 @@ export class UserPreferencesService { * @returns locale name */ get locale(): string { - return this.get(UserPreferenceValues.Locale); + return this.get(UserPreferenceValues.Locale) || this.getDefaultLocale(); } set locale(value: string) { diff --git a/lib/core/src/lib/mock/translation.service.mock.ts b/lib/core/src/lib/mock/translation.service.mock.ts index 011d898bc3..a86a0317ff 100644 --- a/lib/core/src/lib/mock/translation.service.mock.ts +++ b/lib/core/src/lib/mock/translation.service.mock.ts @@ -17,7 +17,6 @@ import { EventEmitter, Injectable } from '@angular/core'; import { Observable, of } from 'rxjs'; -import { TranslationService } from '../translation/translation.service'; export interface LangChangeEvent { lang: string; @@ -26,7 +25,7 @@ export interface LangChangeEvent { /** @deprecated use `NoopTranslateModule` instead */ @Injectable() -export class TranslationMock implements TranslationService { +export class TranslationMock { defaultLang: string = 'en'; userLang: string; customLoader: any; diff --git a/lib/core/src/lib/testing/noop-translate.module.ts b/lib/core/src/lib/testing/noop-translate.module.ts index 0e5f7a0196..846c389c67 100644 --- a/lib/core/src/lib/testing/noop-translate.module.ts +++ b/lib/core/src/lib/testing/noop-translate.module.ts @@ -16,14 +16,15 @@ */ import { EventEmitter, Injectable, NgModule } from '@angular/core'; -import { HttpClientTestingModule } from '@angular/common/http/testing'; +import { provideHttpClient } from '@angular/common/http'; +import { provideHttpClientTesting } from '@angular/common/http/testing'; import { provideTranslateService, TranslateLoader } from '@ngx-translate/core'; import { TranslationService } from '../translation/translation.service'; import { LangChangeEvent } from '../mock'; import { Observable, of } from 'rxjs'; @Injectable({ providedIn: 'root' }) -export class NoopTranslationService implements TranslationService { +export class NoopTranslationService { defaultLang: string = 'en'; userLang: string; customLoader: any; @@ -49,8 +50,9 @@ export class NoopTranslationService implements TranslationService { } @NgModule({ - imports: [HttpClientTestingModule], providers: [ + provideHttpClient(), + provideHttpClientTesting(), { provide: TranslationService, useClass: NoopTranslationService }, provideTranslateService({ loader: { diff --git a/lib/core/src/lib/translation/translation.service.ts b/lib/core/src/lib/translation/translation.service.ts index dfdf37fa51..c8c877e280 100644 --- a/lib/core/src/lib/translation/translation.service.ts +++ b/lib/core/src/lib/translation/translation.service.ts @@ -19,7 +19,7 @@ import { effect, Inject, Injectable, InjectionToken, Optional } from '@angular/c import { TranslateService } from '@ngx-translate/core'; import { Observable } from 'rxjs'; import { TranslateLoaderService } from './translate-loader.service'; -import { UserPreferencesService } from '../common/services/user-preferences.service'; +import { UserPreferencesService, UserPreferenceValues } from '../common/services/user-preferences.service'; export const TRANSLATION_PROVIDER = new InjectionToken('Injection token for translation providers.'); @@ -71,13 +71,21 @@ export class TranslationService { } } - // Use effect to reactively update translations when locale signal changes - // Note: This is a singleton service, so no cleanup needed + // Try to read locale from storage synchronously to apply it before components render + const storedLocale = userPreferencesService.get(UserPreferenceValues.Locale); + + if (storedLocale) { + // Apply stored locale immediately during construction + this.userLang = storedLocale; + this.loadTranslation(storedLocale, this.defaultLang); + } + effect(() => { const locale = userPreferencesService.localeSignal(); - if (locale) { + + if (locale && locale !== this.userLang) { this.userLang = locale; - this.use(this.userLang); + this.loadTranslation(locale, this.defaultLang); } }); } @@ -128,7 +136,7 @@ export class TranslationService { onTranslationChanged(lang: string): void { this.translate.onTranslationChange.next({ lang, - translations: this.customLoader.getFullTranslationJSON(lang) + translations: this.customLoader.getFullTranslationJSON?.(lang) ?? {} }); }