From 8a0ff792bc57c2ad1b06051a02ac3b93ae945269 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Fri, 9 Aug 2024 22:35:24 +0000 Subject: [PATCH 1/5] Change the locale dynamically by adding &i18n-locale to URL The main issue was the inability to dynamically change the locale in OpenSearch Dashboards. Currently we need to update config file and i18nrc.json. This PR allows users to switch to a different locale (e.g., from English to Chinese) by appending or modifying the 'i18n-locale' parameter in the URL. * getAndUpdateLocaleInUrl: If a non-default locale is found, this function reconstructs the URL with the locale parameter in the correct position. * updated the ScopedHistory class, allowing it to detect locale changes and trigger reloads as necessary. * modify the i18nMixin, which sets up the i18n system during server startup, to register all available translation files during server startup, not just the current locale. * update the uiRenderMixin to accept requests for any registered locale and dynamically load and cache translations for requested locales. Signed-off-by: Anan Zhuang --- src/core/public/application/scoped_history.ts | 10 ++ src/core/public/locale_helper.test.ts | 128 ++++++++++++++ src/core/public/locale_helper.ts | 114 +++++++++++++ src/core/public/osd_bootstrap.ts | 28 ++++ src/legacy/server/i18n/index.ts | 11 +- src/legacy/ui/ui_render/ui_render_mixin.js | 55 ++++-- .../ui/ui_render/ui_render_mixin.test.js | 158 ++++++++++++++++++ 7 files changed, 483 insertions(+), 21 deletions(-) create mode 100644 src/core/public/locale_helper.test.ts create mode 100644 src/core/public/locale_helper.ts create mode 100644 src/legacy/ui/ui_render/ui_render_mixin.test.js diff --git a/src/core/public/application/scoped_history.ts b/src/core/public/application/scoped_history.ts index 487093be191..34b273effbd 100644 --- a/src/core/public/application/scoped_history.ts +++ b/src/core/public/application/scoped_history.ts @@ -39,6 +39,8 @@ import { Href, Action, } from 'history'; +import { i18n } from '@osd/i18n'; +import { extractLocaleInfo } from '../locale_helper'; /** * A wrapper around a `History` instance that is scoped to a particular base path of the history stack. Behaves @@ -307,6 +309,7 @@ export class ScopedHistory * state. Also forwards events to child listeners with the base path stripped from the location. */ private setupHistoryListener() { + const currentLocale = i18n.getLocale() || 'en'; const unlisten = this.parentHistory.listen((location, action) => { // If the user navigates outside the scope of this basePath, tear it down. if (!location.pathname.startsWith(this.basePath)) { @@ -314,6 +317,13 @@ export class ScopedHistory this.isActive = false; return; } + // const fullUrl = `${location.pathname}${location.search}${location.hash}`; + const { localeValue } = extractLocaleInfo(window.location.href); + if (localeValue !== currentLocale) { + // Force a full page reload + window.location.reload(); + return; + } /** * Track location keys using the same algorithm the browser uses internally. diff --git a/src/core/public/locale_helper.test.ts b/src/core/public/locale_helper.test.ts new file mode 100644 index 00000000000..508752f1d40 --- /dev/null +++ b/src/core/public/locale_helper.test.ts @@ -0,0 +1,128 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { extractLocaleInfo, getAndUpdateLocaleInUrl } from './locale_helper'; + +describe('extractLocaleInfo', () => { + const testCases = [ + { + description: 'After hash and slash', + input: 'http://localhost:5603/app/home#/&i18n-locale=fr-FR', + expected: { + localeValue: 'fr-FR', + localeParam: 'i18n-locale=fr-FR', + updatedUrl: 'http://localhost:5603/app/home#/', + }, + }, + { + description: 'After path and slash', + input: 'http://localhost:5603/app/home/&i18n-locale=de-DE', + expected: { + localeValue: 'de-DE', + localeParam: 'i18n-locale=de-DE', + updatedUrl: 'http://localhost:5603/app/home/', + }, + }, + { + description: 'No locale parameter', + input: 'http://localhost:5603/app/home', + expected: { + localeValue: 'en', + localeParam: null, + updatedUrl: 'http://localhost:5603/app/home', + }, + }, + { + description: 'Complex URL with locale', + input: 'http://localhost:5603/app/dashboards#/view/id?_g=(...)&_a=(...)&i18n-locale=es-ES', + expected: { + localeValue: 'es-ES', + localeParam: 'i18n-locale=es-ES', + updatedUrl: 'http://localhost:5603/app/dashboards#/view/id?_g=(...)&_a=(...)', + }, + }, + ]; + + testCases.forEach(({ description, input, expected }) => { + it(description, () => { + const result = extractLocaleInfo(input); + expect(result).toEqual(expected); + }); + }); +}); + +describe('getAndUpdateLocaleInUrl', () => { + let originalHistoryReplaceState: typeof window.history.replaceState; + + beforeEach(() => { + // Mock window.history.replaceState + originalHistoryReplaceState = window.history.replaceState; + window.history.replaceState = jest.fn(); + }); + + afterEach(() => { + // Restore original window.history.replaceState + window.history.replaceState = originalHistoryReplaceState; + }); + + const testCases = [ + { + description: 'Category 1: basePath + #/', + input: 'http://localhost:5603/app/home#/&i18n-locale=zh-CN', + expected: 'http://localhost:5603/app/home#/?i18n-locale=zh-CN', + locale: 'zh-CN', + }, + { + description: 'Category 1: basePath + # (empty hashPath)', + input: 'http://localhost:5603/app/home#&i18n-locale=zh-CN', + expected: 'http://localhost:5603/app/home#?i18n-locale=zh-CN', + locale: 'zh-CN', + }, + { + description: 'Category 2: basePath + # + hashPath + ? + hashQuery', + input: 'http://localhost:5603/app/dashboards#/view/id?_g=(...)&_a=(...)&i18n-locale=zh-CN', + expected: 'http://localhost:5603/app/dashboards#/view/id?_g=(...)&_a=(...)&i18n-locale=zh-CN', + locale: 'zh-CN', + }, + { + description: 'Category 3: basePath only', + input: 'http://localhost:5603/app/management&i18n-locale=zh-CN', + expected: 'http://localhost:5603/app/management?i18n-locale=zh-CN', + locale: 'zh-CN', + }, + { + description: 'Category 1: basePath + # + hashPath', + input: 'http://localhost:5603/app/dev_tools#/console&i18n-locale=zh-CN', + expected: 'http://localhost:5603/app/dev_tools#/console?i18n-locale=zh-CN', + locale: 'zh-CN', + }, + { + description: 'URL without locale parameter', + input: 'http://localhost:5603/app/home#/', + expected: 'http://localhost:5603/app/home#/', + locale: 'en', + }, + { + description: 'Complex URL with multiple parameters', + input: + "http://localhost:5603/app/dashboards#/view/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-24h,to:now))&_a=(description:'Analyze%20mock%20flight%20data',filters:!())&i18n-locale=zh-CN", + expected: + "http://localhost:5603/app/dashboards#/view/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-24h,to:now))&_a=(description:'Analyze%20mock%20flight%20data',filters:!())&i18n-locale=zh-CN", + locale: 'zh-CN', + }, + ]; + + testCases.forEach(({ description, input, expected, locale }) => { + it(description, () => { + const result = getAndUpdateLocaleInUrl(input); + expect(result).toBe(locale); + if (locale !== 'en') { + expect(window.history.replaceState).toHaveBeenCalledWith(null, '', expected); + } else { + expect(window.history.replaceState).not.toHaveBeenCalled(); + } + }); + }); +}); diff --git a/src/core/public/locale_helper.ts b/src/core/public/locale_helper.ts new file mode 100644 index 00000000000..774957018da --- /dev/null +++ b/src/core/public/locale_helper.ts @@ -0,0 +1,114 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Extracts the locale value and parameter from a given URL string. + * + * @param url - The full URL string to parse + * @returns An object with localeValue, localeParam or null if not found, and updatedUrl or url if no updates. + */ +export function extractLocaleInfo( + url: string +): { localeValue: string | null; localeParam: string | null; updatedUrl: string } { + const patterns = [ + /[#&?](i18n-locale)=([^&/]+)/i, // Standard query parameter + /#\/&(i18n-locale)=([^&/]+)/i, // After hash and slash + /\/&(i18n-locale)=([^&/]+)/i, // After path and slash + ]; + + for (const pattern of patterns) { + const match = url.match(pattern); + if (match) { + const localeValue = match[2]; + const localeParam = `${match[1]}=${match[2]}`; + const updatedUrl = url.replace(match[0], ''); + return { localeValue, localeParam, updatedUrl }; + } + } + + return { localeValue: 'en', localeParam: null, updatedUrl: url }; +} + +/** + * Extracts a dynamically added locale parameter from a URL and restructures the URL + * to include this locale parameter in a consistent, functional manner. + * + * This function is specifically designed to handle cases where '&i18n-locale=' + * has been appended to the URL, potentially in a position that could cause issues + * with OpenSearch Dashboards' URL parsing or functionality. + * + * The restructuring is necessary because simply appending the locale parameter + * to certain URL structures can lead to parsing errors or the parameter being ignored. + * + * The function handles various URL structures to ensure the locale parameter + * is placed in a position where it will be correctly parsed and utilized by + * OpenSearch Dashboards, while maintaining the integrity of existing URL components. + * + * URL Components: + * - basePath: The part of the URL before the hash (#). It typically includes the domain and application path. + * - hashPath: The part of the URL after the hash (#) but before any query parameters (?). + * - hashQuery: The query parameters after the hashPath. In OpenSearch Dashboards, this often contains + * RISON-encoded data and never ends with a slash (/) to avoid RISON parsing errors. + * + * This function handles three main categories of URLs: + * 1. basePath + # + hashPath (including when hashPath is '/' or empty) + * Before: basePath#hashPath&i18n-locale=zh-CN + * After: basePath#hashPath?i18n-locale=zh-CN + * Restructuring rationale: The '&' is changed to '?' because there were no existing + * query parameters after the hashPath. This ensures the locale is treated as a proper + * query parameter and not mistakenly considered part of the hashPath. + * + * 2. basePath + # + hashPath + ? + hashQuery + * Before: basePath#hashPath?hashQuery&i18n-locale=zh-CN + * After: basePath#hashPath?hashQuery&i18n-locale=zh-CN + * Restructuring rationale: The locale parameter is appended to existing query parameters. + * No change in structure is needed as it's already in the correct position. + * + * 3. basePath only + * Before: basePath&i18n-locale=zh-CN + * After: basePath?i18n-locale=zh-CN + * Restructuring rationale: The '&' is changed to '?' because there were no existing + * query parameters in the basePath. This ensures the locale is recognized as the + * start of the query string rather than being misinterpreted as part of the path. + * + * The function performs the following steps: + * 1. Extracts the locale parameter from its current position in the URL. + * 2. Removes the locale parameter from its original position. + * 3. Reconstructs the URL, placing the locale parameter in the correct position + * based on the URL structure to ensure proper parsing by OpenSearch Dashboards. + * 4. Updates the browser's URL without causing a page reload. + * + * @param {string} url - The full URL to process + * @returns {string|null} The extracted locale value, or null if no locale was found + */ + +export function getAndUpdateLocaleInUrl(url: string): string | null { + let fullUrl = ''; + const { localeValue, localeParam, updatedUrl } = extractLocaleInfo(url); + + if (localeValue && localeParam) { + const [basePath, hashPart] = updatedUrl.split('#'); + + if (hashPart !== undefined) { + const [hashPath, hashQuery] = hashPart.split('?'); + if (hashQuery) { + // Category 2: basePath + # + hashPath + ? + hashQuery + fullUrl = `${basePath}#${hashPath}?${hashQuery}&${localeParam}`; + } else { + // Category 1: basePath + # + hashPath (including when hashPath is '/' or empty) + fullUrl = `${basePath}#${hashPath}?${localeParam}`; + } + } else { + // Category 3: basePath only + fullUrl = `${basePath}?${localeParam}`; + } + + // Update the URL without causing a page reload + window.history.replaceState(null, '', fullUrl); + return localeValue; + } + + return 'en'; +} diff --git a/src/core/public/osd_bootstrap.ts b/src/core/public/osd_bootstrap.ts index ed64ed0bc2b..e9a03cb5c93 100644 --- a/src/core/public/osd_bootstrap.ts +++ b/src/core/public/osd_bootstrap.ts @@ -31,6 +31,7 @@ import { i18n } from '@osd/i18n'; import { CoreSystem } from './core_system'; import { ApmSystem } from './apm_system'; +import { getAndUpdateLocaleInUrl } from './locale_helper'; /** @internal */ export async function __osdBootstrap__() { @@ -38,6 +39,33 @@ export async function __osdBootstrap__() { document.querySelector('osd-injected-metadata')!.getAttribute('data')! ); + // Extract the locale from the URL if present + // This allows for dynamic locale setting via URL parameters + const urlLocale = getAndUpdateLocaleInUrl(window.location.href); + + if (urlLocale) { + // If a locale is specified in the URL, update the i18n settings + // This enables dynamic language switching + // Note: This works in conjunction with server-side changes: + // 1. The server registers all available translation files at startup + // 2. A server route handles requests for specific locale translations + + // Set the locale in the i18n core + // This will affect all subsequent i18n.translate() calls + i18n.setLocale(urlLocale); + + // Modify the translationsUrl to include the new locale + // This ensures that the correct translation file is requested from the server + // The replace function changes the locale in the URL, e.g., + // from '/translations/en.json' to '/translations/zh-CN.json' + injectedMetadata.i18n.translationsUrl = injectedMetadata.i18n.translationsUrl.replace( + /\/([^/]+)\.json$/, + `/${urlLocale}.json` + ); + } else { + i18n.setLocale('en'); + } + const globals: any = typeof window === 'undefined' ? {} : window; const themeTag: string = globals.__osdThemeTag__ || ''; diff --git a/src/legacy/server/i18n/index.ts b/src/legacy/server/i18n/index.ts index e30f9bf7d72..8a571144cdf 100644 --- a/src/legacy/server/i18n/index.ts +++ b/src/legacy/server/i18n/index.ts @@ -62,10 +62,11 @@ export async function i18nMixin( }), ]); - const currentTranslationPaths = ([] as string[]) - .concat(...translationPaths) - .filter((translationPath) => basename(translationPath, '.json') === locale); - i18nLoader.registerTranslationFiles(currentTranslationPaths); + // Flatten the array of arrays + const allTranslationPaths = ([] as string[]).concat(...translationPaths); + + // Register all translation files, not just the ones for the current locale + i18nLoader.registerTranslationFiles(allTranslationPaths); const translations = await i18nLoader.getTranslationsByLocale(locale); i18n.init( @@ -75,7 +76,7 @@ export async function i18nMixin( }) ); - const getTranslationsFilePaths = () => currentTranslationPaths; + const getTranslationsFilePaths = () => allTranslationPaths; server.decorate('server', 'getTranslationsFilePaths', getTranslationsFilePaths); diff --git a/src/legacy/ui/ui_render/ui_render_mixin.js b/src/legacy/ui/ui_render/ui_render_mixin.js index 053f5dbdfca..80a8e49524e 100644 --- a/src/legacy/ui/ui_render/ui_render_mixin.js +++ b/src/legacy/ui/ui_render/ui_render_mixin.js @@ -30,7 +30,7 @@ import { createHash } from 'crypto'; import Boom from '@hapi/boom'; -import { i18n } from '@osd/i18n'; +import { i18n, i18nLoader } from '@osd/i18n'; import * as v7light from '@elastic/eui/dist/eui_theme_light.json'; import * as v7dark from '@elastic/eui/dist/eui_theme_dark.json'; import * as v8light from '@elastic/eui/dist/eui_theme_next_light.json'; @@ -53,32 +53,55 @@ import { getApmConfig } from '../apm'; */ export function uiRenderMixin(osdServer, server, config) { const translationsCache = { translations: null, hash: null }; + const defaultLocale = i18n.getLocale() || 'en'; // Fallback to 'en' if no default locale is set + + // Route handler for serving translation files. + // This handler supports two scenarios: + // 1. Serving translations for the default locale + // 2. Serving translations for other registered locales server.route({ path: '/translations/{locale}.json', method: 'GET', config: { auth: false }, - handler(request, h) { - // OpenSearch Dashboards server loads translations only for a single locale - // that is specified in `i18n.locale` config value. + handler: async (request, h) => { const { locale } = request.params; - if (i18n.getLocale() !== locale.toLowerCase()) { - throw Boom.notFound(`Unknown locale: ${locale}`); - } + const normalizedLocale = locale.toLowerCase(); + const registeredLocales = i18nLoader.getRegisteredLocales().map((l) => l.toLowerCase()); + + // Function to get or create cached translations + const getCachedTranslations = async (localeKey, getTranslationsFn) => { + if (!translationsCache[localeKey]) { + const translations = await getTranslationsFn(); + translationsCache[localeKey] = { + translations: JSON.stringify(translations), + hash: createHash('sha1').update(JSON.stringify(translations)).digest('hex'), + }; + } + return translationsCache[localeKey]; + }; - // Stringifying thousands of labels and calculating hash on the resulting - // string can be expensive so it makes sense to do it once and cache. - if (translationsCache.translations == null) { - translationsCache.translations = JSON.stringify(i18n.getTranslation()); - translationsCache.hash = createHash('sha1') - .update(translationsCache.translations) - .digest('hex'); + let cachedTranslations; + + if (normalizedLocale === defaultLocale.toLowerCase()) { + // Default locale + cachedTranslations = await getCachedTranslations(defaultLocale, () => + i18n.getTranslation() + ); + } else if (registeredLocales.includes(normalizedLocale)) { + // Other registered locales + cachedTranslations = await getCachedTranslations(normalizedLocale, () => + i18nLoader.getTranslationsByLocale(locale) + ); + } else { + // Locale not found + throw Boom.notFound(`Unknown locale: ${locale}`); } return h - .response(translationsCache.translations) + .response(cachedTranslations.translations) .header('cache-control', 'must-revalidate') .header('content-type', 'application/json') - .etag(translationsCache.hash); + .etag(cachedTranslations.hash); }, }); diff --git a/src/legacy/ui/ui_render/ui_render_mixin.test.js b/src/legacy/ui/ui_render/ui_render_mixin.test.js new file mode 100644 index 00000000000..61619e46b0b --- /dev/null +++ b/src/legacy/ui/ui_render/ui_render_mixin.test.js @@ -0,0 +1,158 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { uiRenderMixin } from './ui_render_mixin'; +import Boom from '@hapi/boom'; + +// Mock dependencies +jest.mock('@osd/i18n', () => ({ + i18n: { + getLocale: jest.fn(), + getTranslation: jest.fn(), + translate: jest.fn((key, { defaultMessage }) => defaultMessage), + }, + i18nLoader: { + getRegisteredLocales: jest.fn(), + getTranslationsByLocale: jest.fn(), + }, +})); + +// Import mocked modules +const { i18n, i18nLoader } = require('@osd/i18n'); + +describe('uiRenderMixin', () => { + let server; + let osdServer; + let config; + let routes; + let decorations; + + beforeEach(() => { + routes = []; + decorations = {}; + server = { + route: jest.fn((route) => routes.push(route)), + decorate: jest.fn((type, name, value) => { + decorations[`${type}.${name}`] = value; + }), + auth: { settings: { default: false } }, + }; + osdServer = { + newPlatform: { + setup: { + core: { + http: { csp: { header: 'test-csp-header' } }, + }, + }, + start: { + core: { + savedObjects: { + getScopedClient: jest.fn(), + }, + uiSettings: { + asScopedToClient: jest.fn(), + }, + }, + }, + __internals: { + rendering: { + render: jest.fn(), + }, + }, + }, + }; + config = { + get: jest.fn(), + }; + + // Reset mocks + jest.clearAllMocks(); + }); + + describe('translations route', () => { + let handler; + let h; + + beforeEach(() => { + uiRenderMixin(osdServer, server, config); + handler = routes.find((route) => route.path === '/translations/{locale}.json').handler; + h = { + response: jest.fn().mockReturnThis(), + header: jest.fn().mockReturnThis(), + etag: jest.fn().mockReturnThis(), + }; + }); + + it('should handle default locale', async () => { + const defaultLocale = 'en'; + const defaultTranslations = { hello: 'Hello' }; + i18n.getLocale.mockReturnValue(defaultLocale); + i18n.getTranslation.mockReturnValue(defaultTranslations); + i18nLoader.getRegisteredLocales.mockReturnValue([defaultLocale]); + + const request = { params: { locale: defaultLocale } }; + await handler(request, h); + + expect(i18n.getTranslation).toHaveBeenCalled(); + expect(h.response).toHaveBeenCalledWith(JSON.stringify(defaultTranslations)); + expect(h.header).toHaveBeenCalledWith('cache-control', 'must-revalidate'); + expect(h.header).toHaveBeenCalledWith('content-type', 'application/json'); + expect(h.etag).toHaveBeenCalled(); + }); + + it('should handle non-default registered locale', async () => { + const defaultLocale = 'en'; + const requestedLocale = 'fr'; + const frTranslations = { hello: 'Bonjour' }; + i18n.getLocale.mockReturnValue(defaultLocale); + i18nLoader.getRegisteredLocales.mockReturnValue([defaultLocale, requestedLocale]); + i18nLoader.getTranslationsByLocale.mockResolvedValue(frTranslations); + + const request = { params: { locale: requestedLocale } }; + await handler(request, h); + + expect(i18nLoader.getTranslationsByLocale).toHaveBeenCalledWith(requestedLocale); + expect(h.response).toHaveBeenCalledWith(JSON.stringify(frTranslations)); + }); + + it('should throw not found error for unknown locale', async () => { + const defaultLocale = 'en'; + const unknownLocale = 'xx'; + i18n.getLocale.mockReturnValue(defaultLocale); + i18nLoader.getRegisteredLocales.mockReturnValue([defaultLocale]); + + const request = { params: { locale: unknownLocale } }; + await expect(handler(request, h)).rejects.toThrow( + Boom.notFound(`Unknown locale: ${unknownLocale}`) + ); + }); + + it('should cache translations', async () => { + const defaultLocale = 'en'; + const defaultTranslations = { hello: 'Hello' }; + i18n.getLocale.mockReturnValue(defaultLocale); + i18n.getTranslation.mockReturnValue(defaultTranslations); + i18nLoader.getRegisteredLocales.mockReturnValue([defaultLocale]); + + const request = { params: { locale: defaultLocale } }; + await handler(request, h); + await handler(request, h); + + expect(i18n.getTranslation).toHaveBeenCalledTimes(1); + }); + + it('should handle errors gracefully', async () => { + const defaultLocale = 'en'; + i18n.getLocale.mockReturnValue(defaultLocale); + i18n.getTranslation.mockImplementation(() => { + throw new Error('Translation error'); + }); + i18nLoader.getRegisteredLocales.mockReturnValue([defaultLocale]); + + const request = { params: { locale: defaultLocale } }; + await expect(handler(request, h)).rejects.toThrow('Translation error'); + }); + }); +}); From 4fb00b271430b249d6ca27c98da66e1033ec0748 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Tue, 13 Aug 2024 18:50:31 +0000 Subject: [PATCH 2/5] fix PR comments Signed-off-by: Anan Zhuang --- docs/_sidebar.md | 2 + packages/osd-i18n/src/core/i18n.ts | 9 +- src/core/public/application/scoped_history.ts | 4 +- src/core/public/core_system.ts | 31 ++++ src/core/public/locale_helper.test.ts | 144 ++++------------- src/core/public/locale_helper.ts | 148 ++++++------------ src/core/public/osd_bootstrap.test.mocks.ts | 28 ++-- src/core/public/osd_bootstrap.test.ts | 66 +++++++- src/core/public/osd_bootstrap.ts | 19 ++- src/dev/jest/config.js | 1 - src/legacy/ui/ui_render/ui_render_mixin.js | 20 ++- .../ui/ui_render/ui_render_mixin.test.js | 31 +++- 12 files changed, 261 insertions(+), 242 deletions(-) diff --git a/docs/_sidebar.md b/docs/_sidebar.md index e5136a6df4c..792c35e3949 100644 --- a/docs/_sidebar.md +++ b/docs/_sidebar.md @@ -30,6 +30,7 @@ - public - application - [Hooks](../src/plugins/console/public/application/hooks/README.md) + - [Content_management](../src/plugins/content_management/README.md) - [Csp_handler](../src/plugins/csp_handler/README.md) - [Dashboard](../src/plugins/dashboard/README.md) - [Data](../src/plugins/data/README.md) @@ -164,6 +165,7 @@ - [Opensearch dashboards.release notes 2.13.0](../release-notes/opensearch-dashboards.release-notes-2.13.0.md) - [Opensearch dashboards.release notes 2.14.0](../release-notes/opensearch-dashboards.release-notes-2.14.0.md) - [Opensearch dashboards.release notes 2.15.0](../release-notes/opensearch-dashboards.release-notes-2.15.0.md) + - [Opensearch dashboards.release notes 2.16.0](../release-notes/opensearch-dashboards.release-notes-2.16.0.md) - [Opensearch dashboards.release notes 2.2.0](../release-notes/opensearch-dashboards.release-notes-2.2.0.md) - [Opensearch dashboards.release notes 2.2.1](../release-notes/opensearch-dashboards.release-notes-2.2.1.md) - [Opensearch dashboards.release notes 2.3.0](../release-notes/opensearch-dashboards.release-notes-2.3.0.md) diff --git a/packages/osd-i18n/src/core/i18n.ts b/packages/osd-i18n/src/core/i18n.ts index 3268fae5079..65da4931ef1 100644 --- a/packages/osd-i18n/src/core/i18n.ts +++ b/packages/osd-i18n/src/core/i18n.ts @@ -261,5 +261,12 @@ export async function load(translationsUrl: string) { throw new Error(`Translations request failed with status code: ${response.status}`); } - init(await response.json()); + const data = await response.json(); + + if (data.warning) { + // Store the warning to be displayed after core system setup + (window as any).__i18nWarning = data.warning; + } + + init(data.translations); } diff --git a/src/core/public/application/scoped_history.ts b/src/core/public/application/scoped_history.ts index 34b273effbd..2befe5335bc 100644 --- a/src/core/public/application/scoped_history.ts +++ b/src/core/public/application/scoped_history.ts @@ -40,7 +40,7 @@ import { Action, } from 'history'; import { i18n } from '@osd/i18n'; -import { extractLocaleInfo } from '../locale_helper'; +import { getLocaleInUrl } from '../locale_helper'; /** * A wrapper around a `History` instance that is scoped to a particular base path of the history stack. Behaves @@ -318,7 +318,7 @@ export class ScopedHistory return; } // const fullUrl = `${location.pathname}${location.search}${location.hash}`; - const { localeValue } = extractLocaleInfo(window.location.href); + const localeValue = getLocaleInUrl(window.location.href); if (localeValue !== currentLocale) { // Force a full page reload window.location.reload(); diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 731cf461824..8d9a0ddce02 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -318,4 +318,35 @@ export class CoreSystem { this.workspaces.stop(); this.rootDomElement.textContent = ''; } + + public async displayWarning(title: string, text: string) { + const i18n = await this.i18n.start(); + const injectedMetadata = this.injectedMetadata.setup(); + this.fatalErrorsSetup = this.fatalErrors.setup({ + injectedMetadata, + i18n: this.i18n.getContext(), + }); + await this.integrations.setup(); + this.docLinks.setup(); + const http = this.http.setup({ injectedMetadata, fatalErrors: this.fatalErrorsSetup }); + const uiSettings = this.uiSettings.setup({ http, injectedMetadata }); + const notificationsTargetDomElement = document.createElement('div'); + const overlayTargetDomElement = document.createElement('div'); + const overlays = this.overlay.start({ + i18n, + targetDomElement: overlayTargetDomElement, + uiSettings, + }); + if (this.notifications) { + const toasts = await this.notifications.start({ + i18n, + overlays, + targetDomElement: notificationsTargetDomElement, + })?.toasts; + + if (toasts) { + toasts.addWarning({ title, text }); + } + } + } } diff --git a/src/core/public/locale_helper.test.ts b/src/core/public/locale_helper.test.ts index 508752f1d40..0ba878feae3 100644 --- a/src/core/public/locale_helper.test.ts +++ b/src/core/public/locale_helper.test.ts @@ -3,126 +3,48 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { extractLocaleInfo, getAndUpdateLocaleInUrl } from './locale_helper'; +import { getLocaleInUrl } from './locale_helper'; -describe('extractLocaleInfo', () => { - const testCases = [ - { - description: 'After hash and slash', - input: 'http://localhost:5603/app/home#/&i18n-locale=fr-FR', - expected: { - localeValue: 'fr-FR', - localeParam: 'i18n-locale=fr-FR', - updatedUrl: 'http://localhost:5603/app/home#/', - }, - }, - { - description: 'After path and slash', - input: 'http://localhost:5603/app/home/&i18n-locale=de-DE', - expected: { - localeValue: 'de-DE', - localeParam: 'i18n-locale=de-DE', - updatedUrl: 'http://localhost:5603/app/home/', - }, - }, - { - description: 'No locale parameter', - input: 'http://localhost:5603/app/home', - expected: { - localeValue: 'en', - localeParam: null, - updatedUrl: 'http://localhost:5603/app/home', - }, - }, - { - description: 'Complex URL with locale', - input: 'http://localhost:5603/app/dashboards#/view/id?_g=(...)&_a=(...)&i18n-locale=es-ES', - expected: { - localeValue: 'es-ES', - localeParam: 'i18n-locale=es-ES', - updatedUrl: 'http://localhost:5603/app/dashboards#/view/id?_g=(...)&_a=(...)', - }, - }, - ]; +describe('getLocaleInUrl', () => { + beforeEach(() => { + // Clear any warnings before each test + delete (window as any).__localeWarning; + }); - testCases.forEach(({ description, input, expected }) => { - it(description, () => { - const result = extractLocaleInfo(input); - expect(result).toEqual(expected); - }); + it('should return the locale from a valid query string', () => { + const url = 'http://localhost:5603/app/home?locale=en-US'; + expect(getLocaleInUrl(url)).toBe('en-US'); + }); + + it('should return the locale from a valid hash query string', () => { + const url = 'http://localhost:5603/app/home#/?locale=fr-FR'; + expect(getLocaleInUrl(url)).toBe('fr-FR'); }); -}); -describe('getAndUpdateLocaleInUrl', () => { - let originalHistoryReplaceState: typeof window.history.replaceState; + it('should return null for a URL without locale', () => { + const url = 'http://localhost:5603/app/home'; + expect(getLocaleInUrl(url)).toBeNull(); + }); - beforeEach(() => { - // Mock window.history.replaceState - originalHistoryReplaceState = window.history.replaceState; - window.history.replaceState = jest.fn(); + it('should return null and set a warning for an invalid locale format in hash', () => { + const url = 'http://localhost:5603/app/home#/&locale=de-DE'; + expect(getLocaleInUrl(url)).toBeNull(); + expect((window as any).__localeWarning).toBeDefined(); + expect((window as any).__localeWarning.title).toBe('Invalid URL Format'); }); - afterEach(() => { - // Restore original window.history.replaceState - window.history.replaceState = originalHistoryReplaceState; + it('should return null for an empty locale value', () => { + const url = 'http://localhost:5603/app/home?locale='; + expect(getLocaleInUrl(url)).toBeNull(); }); - const testCases = [ - { - description: 'Category 1: basePath + #/', - input: 'http://localhost:5603/app/home#/&i18n-locale=zh-CN', - expected: 'http://localhost:5603/app/home#/?i18n-locale=zh-CN', - locale: 'zh-CN', - }, - { - description: 'Category 1: basePath + # (empty hashPath)', - input: 'http://localhost:5603/app/home#&i18n-locale=zh-CN', - expected: 'http://localhost:5603/app/home#?i18n-locale=zh-CN', - locale: 'zh-CN', - }, - { - description: 'Category 2: basePath + # + hashPath + ? + hashQuery', - input: 'http://localhost:5603/app/dashboards#/view/id?_g=(...)&_a=(...)&i18n-locale=zh-CN', - expected: 'http://localhost:5603/app/dashboards#/view/id?_g=(...)&_a=(...)&i18n-locale=zh-CN', - locale: 'zh-CN', - }, - { - description: 'Category 3: basePath only', - input: 'http://localhost:5603/app/management&i18n-locale=zh-CN', - expected: 'http://localhost:5603/app/management?i18n-locale=zh-CN', - locale: 'zh-CN', - }, - { - description: 'Category 1: basePath + # + hashPath', - input: 'http://localhost:5603/app/dev_tools#/console&i18n-locale=zh-CN', - expected: 'http://localhost:5603/app/dev_tools#/console?i18n-locale=zh-CN', - locale: 'zh-CN', - }, - { - description: 'URL without locale parameter', - input: 'http://localhost:5603/app/home#/', - expected: 'http://localhost:5603/app/home#/', - locale: 'en', - }, - { - description: 'Complex URL with multiple parameters', - input: - "http://localhost:5603/app/dashboards#/view/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-24h,to:now))&_a=(description:'Analyze%20mock%20flight%20data',filters:!())&i18n-locale=zh-CN", - expected: - "http://localhost:5603/app/dashboards#/view/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-24h,to:now))&_a=(description:'Analyze%20mock%20flight%20data',filters:!())&i18n-locale=zh-CN", - locale: 'zh-CN', - }, - ]; + it('should handle URLs with other query parameters', () => { + const url = 'http://localhost:5603/app/home?param1=value1&locale=ja-JP¶m2=value2'; + expect(getLocaleInUrl(url)).toBe('ja-JP'); + }); - testCases.forEach(({ description, input, expected, locale }) => { - it(description, () => { - const result = getAndUpdateLocaleInUrl(input); - expect(result).toBe(locale); - if (locale !== 'en') { - expect(window.history.replaceState).toHaveBeenCalledWith(null, '', expected); - } else { - expect(window.history.replaceState).not.toHaveBeenCalled(); - } - }); + it('should handle URLs with other hash parameters', () => { + const url = 'http://localhost:5603/app/home#/route?param1=value1&locale=zh-CN¶m2=value2'; + expect(getLocaleInUrl(url)).toBe('zh-CN'); }); }); diff --git a/src/core/public/locale_helper.ts b/src/core/public/locale_helper.ts index 774957018da..77ac4b4583f 100644 --- a/src/core/public/locale_helper.ts +++ b/src/core/public/locale_helper.ts @@ -4,111 +4,65 @@ */ /** - * Extracts the locale value and parameter from a given URL string. + * Extracts the locale value from a given URL. * - * @param url - The full URL string to parse - * @returns An object with localeValue, localeParam or null if not found, and updatedUrl or url if no updates. - */ -export function extractLocaleInfo( - url: string -): { localeValue: string | null; localeParam: string | null; updatedUrl: string } { - const patterns = [ - /[#&?](i18n-locale)=([^&/]+)/i, // Standard query parameter - /#\/&(i18n-locale)=([^&/]+)/i, // After hash and slash - /\/&(i18n-locale)=([^&/]+)/i, // After path and slash - ]; - - for (const pattern of patterns) { - const match = url.match(pattern); - if (match) { - const localeValue = match[2]; - const localeParam = `${match[1]}=${match[2]}`; - const updatedUrl = url.replace(match[0], ''); - return { localeValue, localeParam, updatedUrl }; - } - } - - return { localeValue: 'en', localeParam: null, updatedUrl: url }; -} - -/** - * Extracts a dynamically added locale parameter from a URL and restructures the URL - * to include this locale parameter in a consistent, functional manner. - * - * This function is specifically designed to handle cases where '&i18n-locale=' - * has been appended to the URL, potentially in a position that could cause issues - * with OpenSearch Dashboards' URL parsing or functionality. - * - * The restructuring is necessary because simply appending the locale parameter - * to certain URL structures can lead to parsing errors or the parameter being ignored. - * - * The function handles various URL structures to ensure the locale parameter - * is placed in a position where it will be correctly parsed and utilized by - * OpenSearch Dashboards, while maintaining the integrity of existing URL components. - * - * URL Components: - * - basePath: The part of the URL before the hash (#). It typically includes the domain and application path. - * - hashPath: The part of the URL after the hash (#) but before any query parameters (?). - * - hashQuery: The query parameters after the hashPath. In OpenSearch Dashboards, this often contains - * RISON-encoded data and never ends with a slash (/) to avoid RISON parsing errors. + * This function looks for the 'locale' parameter in either the main query string + * or in the hash part of the URL. It supports two valid formats: + * 1. As a regular query parameter: "?locale=xx-XX" + * 2. In the hash with a proper query string: "#/?locale=xx-XX" * - * This function handles three main categories of URLs: - * 1. basePath + # + hashPath (including when hashPath is '/' or empty) - * Before: basePath#hashPath&i18n-locale=zh-CN - * After: basePath#hashPath?i18n-locale=zh-CN - * Restructuring rationale: The '&' is changed to '?' because there were no existing - * query parameters after the hashPath. This ensures the locale is treated as a proper - * query parameter and not mistakenly considered part of the hashPath. + * If an invalid format is detected, it sets a warning message on the window object. * - * 2. basePath + # + hashPath + ? + hashQuery - * Before: basePath#hashPath?hashQuery&i18n-locale=zh-CN - * After: basePath#hashPath?hashQuery&i18n-locale=zh-CN - * Restructuring rationale: The locale parameter is appended to existing query parameters. - * No change in structure is needed as it's already in the correct position. - * - * 3. basePath only - * Before: basePath&i18n-locale=zh-CN - * After: basePath?i18n-locale=zh-CN - * Restructuring rationale: The '&' is changed to '?' because there were no existing - * query parameters in the basePath. This ensures the locale is recognized as the - * start of the query string rather than being misinterpreted as part of the path. - * - * The function performs the following steps: - * 1. Extracts the locale parameter from its current position in the URL. - * 2. Removes the locale parameter from its original position. - * 3. Reconstructs the URL, placing the locale parameter in the correct position - * based on the URL structure to ensure proper parsing by OpenSearch Dashboards. - * 4. Updates the browser's URL without causing a page reload. - * - * @param {string} url - The full URL to process - * @returns {string|null} The extracted locale value, or null if no locale was found + * @param url - The URL to extract the locale from + * @returns The locale value if found and valid, or null otherwise */ +export function getLocaleInUrl(url: string): string | null { + let urlObject: URL; + // Attempt to parse the URL, return null if invalid + try { + urlObject = new URL(url, window.location.origin); + } catch (error) { + setInvalidUrlWarning(); + return null; + } -export function getAndUpdateLocaleInUrl(url: string): string | null { - let fullUrl = ''; - const { localeValue, localeParam, updatedUrl } = extractLocaleInfo(url); - - if (localeValue && localeParam) { - const [basePath, hashPart] = updatedUrl.split('#'); + let localeValue: string | null = null; - if (hashPart !== undefined) { - const [hashPath, hashQuery] = hashPart.split('?'); - if (hashQuery) { - // Category 2: basePath + # + hashPath + ? + hashQuery - fullUrl = `${basePath}#${hashPath}?${hashQuery}&${localeParam}`; - } else { - // Category 1: basePath + # + hashPath (including when hashPath is '/' or empty) - fullUrl = `${basePath}#${hashPath}?${localeParam}`; - } - } else { - // Category 3: basePath only - fullUrl = `${basePath}?${localeParam}`; + // Check for locale in the main query string + if (urlObject.searchParams.has('locale')) { + localeValue = urlObject.searchParams.get('locale'); + } + // Check for locale in the hash, but only if it's in proper query string format + else if (urlObject.hash.includes('?')) { + const hashParams = new URLSearchParams(urlObject.hash.split('?')[1]); + if (hashParams.has('locale')) { + localeValue = hashParams.get('locale'); } + } - // Update the URL without causing a page reload - window.history.replaceState(null, '', fullUrl); - return localeValue; + // Check for non standard query format: + if (localeValue === null && url.includes('&locale=')) { + setInvalidUrlWithLocaleWarning(); + return null; } - return 'en'; + // Return the locale value if found, or null if not found + return localeValue && localeValue.trim() !== '' ? localeValue : null; +} + +function setInvalidUrlWarning(): void { + (window as any).__localeWarning = { + title: 'Invalid URL Format', + text: 'The provided URL is not in a valid format.', + }; +} + +function setInvalidUrlWithLocaleWarning(): void { + (window as any).__localeWarning = { + title: 'Invalid URL Format', + text: + 'The locale parameter is not in a valid URL format. ' + + 'Use either "?locale=xx-XX" in the main URL or "#/?locale=xx-XX" in the hash. ' + + 'For example: "yourapp.com/page?locale=en-US" or "yourapp.com/page#/?locale=en-US".', + }; } diff --git a/src/core/public/osd_bootstrap.test.mocks.ts b/src/core/public/osd_bootstrap.test.mocks.ts index 77b47e8b895..982eef04b38 100644 --- a/src/core/public/osd_bootstrap.test.mocks.ts +++ b/src/core/public/osd_bootstrap.test.mocks.ts @@ -31,18 +31,6 @@ import { applicationServiceMock } from './application/application_service.mock'; import { fatalErrorsServiceMock } from './fatal_errors/fatal_errors_service.mock'; export const fatalErrorMock = fatalErrorsServiceMock.createSetupContract(); -export const coreSystemMock = { - setup: jest.fn().mockResolvedValue({ - fatalErrors: fatalErrorMock, - }), - start: jest.fn().mockResolvedValue({ - application: applicationServiceMock.createInternalStartContract(), - }), -}; -jest.doMock('./core_system', () => ({ - CoreSystem: jest.fn().mockImplementation(() => coreSystemMock), -})); - export const apmSystem = { setup: jest.fn().mockResolvedValue(undefined), start: jest.fn().mockResolvedValue(undefined), @@ -53,9 +41,25 @@ jest.doMock('./apm_system', () => ({ })); export const i18nLoad = jest.fn().mockResolvedValue(undefined); +export const i18nSetLocale = jest.fn(); jest.doMock('@osd/i18n', () => ({ i18n: { ...jest.requireActual('@osd/i18n').i18n, load: i18nLoad, + setLocale: i18nSetLocale, }, })); + +export const displayWarningMock = jest.fn(); +export const coreSystemMock = { + setup: jest.fn().mockResolvedValue({ + fatalErrors: fatalErrorMock, + }), + start: jest.fn().mockResolvedValue({ + application: applicationServiceMock.createInternalStartContract(), + }), + displayWarning: displayWarningMock, +}; +jest.doMock('./core_system', () => ({ + CoreSystem: jest.fn().mockImplementation(() => coreSystemMock), +})); diff --git a/src/core/public/osd_bootstrap.test.ts b/src/core/public/osd_bootstrap.test.ts index e4209b460f8..ec7559c3c31 100644 --- a/src/core/public/osd_bootstrap.test.ts +++ b/src/core/public/osd_bootstrap.test.ts @@ -28,23 +28,46 @@ * under the License. */ -import { apmSystem, fatalErrorMock, i18nLoad } from './osd_bootstrap.test.mocks'; +import { + apmSystem, + fatalErrorMock, + i18nLoad, + i18nSetLocale, + displayWarningMock, +} from './osd_bootstrap.test.mocks'; import { __osdBootstrap__ } from './'; +import { getLocaleInUrl } from './locale_helper'; + +jest.mock('./locale_helper', () => ({ + getLocaleInUrl: jest.fn(), +})); describe('osd_bootstrap', () => { + let originalWindowLocation: Location; + beforeAll(() => { const metadata = { branding: { darkMode: 'true' }, - i18n: { translationsUrl: 'http://localhost' }, + i18n: { translationsUrl: 'http://localhost/translations/en.json' }, vars: { apmConfig: null }, }; // eslint-disable-next-line no-unsanitized/property - document.body.innerHTML = ` -`; + document.body.innerHTML = ` `; + + originalWindowLocation = window.location; + delete (window as any).location; + window.location = { ...originalWindowLocation, href: 'http://localhost' }; }); beforeEach(() => { jest.clearAllMocks(); + (getLocaleInUrl as jest.Mock).mockReturnValue(null); + }); + + afterAll(() => { + window.location = originalWindowLocation; }); it('does not report a fatal error if apm load fails', async () => { @@ -64,4 +87,39 @@ describe('osd_bootstrap', () => { expect(fatalErrorMock.add).toHaveBeenCalledTimes(1); }); + + it('sets locale from URL if present', async () => { + (getLocaleInUrl as jest.Mock).mockReturnValue('fr'); + window.location.href = 'http://localhost/?locale=fr'; + + await __osdBootstrap__(); + + expect(i18nSetLocale).toHaveBeenCalledWith('fr'); + expect(i18nLoad).toHaveBeenCalledWith('http://localhost/translations/fr.json'); + }); + + it('sets default locale if not present in URL', async () => { + await __osdBootstrap__(); + + expect(i18nSetLocale).toHaveBeenCalledWith('en'); + expect(i18nLoad).toHaveBeenCalledWith('http://localhost/translations/en.json'); + }); + + it('displays locale warning if set', async () => { + (window as any).__localeWarning = { title: 'Locale Warning', text: 'Invalid locale' }; + + await __osdBootstrap__(); + + expect(displayWarningMock).toHaveBeenCalledWith('Locale Warning', 'Invalid locale'); + expect((window as any).__localeWarning).toBeUndefined(); + }); + + it('displays i18n warning if set', async () => { + (window as any).__i18nWarning = { title: 'i18n Warning', text: 'Translation issue' }; + + await __osdBootstrap__(); + + expect(displayWarningMock).toHaveBeenCalledWith('i18n Warning', 'Translation issue'); + expect((window as any).__i18nWarning).toBeUndefined(); + }); }); diff --git a/src/core/public/osd_bootstrap.ts b/src/core/public/osd_bootstrap.ts index e9a03cb5c93..fc131b277af 100644 --- a/src/core/public/osd_bootstrap.ts +++ b/src/core/public/osd_bootstrap.ts @@ -31,7 +31,7 @@ import { i18n } from '@osd/i18n'; import { CoreSystem } from './core_system'; import { ApmSystem } from './apm_system'; -import { getAndUpdateLocaleInUrl } from './locale_helper'; +import { getLocaleInUrl } from './locale_helper'; /** @internal */ export async function __osdBootstrap__() { @@ -40,8 +40,7 @@ export async function __osdBootstrap__() { ); // Extract the locale from the URL if present - // This allows for dynamic locale setting via URL parameters - const urlLocale = getAndUpdateLocaleInUrl(window.location.href); + const urlLocale = getLocaleInUrl(window.location.href); if (urlLocale) { // If a locale is specified in the URL, update the i18n settings @@ -95,4 +94,18 @@ export async function __osdBootstrap__() { const start = await coreSystem.start(); await apmSystem.start(start); + + // Display the i18n warning if it exists + if ((window as any).__i18nWarning) { + const warning = (window as any).__i18nWarning; + coreSystem.displayWarning(warning.title, warning.text); + delete (window as any).__i18nWarning; + } + + // Display the locale warning if it exists + if ((window as any).__localeWarning) { + const warning = (window as any).__localeWarning; + coreSystem.displayWarning(warning.title, warning.text); + delete (window as any).__localeWarning; + } } diff --git a/src/dev/jest/config.js b/src/dev/jest/config.js index c76cb7bfd34..11224945704 100644 --- a/src/dev/jest/config.js +++ b/src/dev/jest/config.js @@ -122,7 +122,6 @@ const ciGroups = process.argv.reduce((acc, arg) => { return acc; }, []); -console.log('ciGroups', ciGroups); if (ciGroups.length > 0) { console.log(`Requested group${ciGroups.length === 1 ? '' : 's'}: ${ciGroups.join(', ')}`); ciGroups.forEach((id) => { diff --git a/src/legacy/ui/ui_render/ui_render_mixin.js b/src/legacy/ui/ui_render/ui_render_mixin.js index 80a8e49524e..fbbfbff91f4 100644 --- a/src/legacy/ui/ui_render/ui_render_mixin.js +++ b/src/legacy/ui/ui_render/ui_render_mixin.js @@ -67,13 +67,14 @@ export function uiRenderMixin(osdServer, server, config) { const { locale } = request.params; const normalizedLocale = locale.toLowerCase(); const registeredLocales = i18nLoader.getRegisteredLocales().map((l) => l.toLowerCase()); + let warning = null; // Function to get or create cached translations const getCachedTranslations = async (localeKey, getTranslationsFn) => { if (!translationsCache[localeKey]) { const translations = await getTranslationsFn(); translationsCache[localeKey] = { - translations: JSON.stringify(translations), + translations: translations, hash: createHash('sha1').update(JSON.stringify(translations)).digest('hex'), }; } @@ -93,12 +94,23 @@ export function uiRenderMixin(osdServer, server, config) { i18nLoader.getTranslationsByLocale(locale) ); } else { - // Locale not found - throw Boom.notFound(`Unknown locale: ${locale}`); + // Locale not found, fall back to en locale + cachedTranslations = await getCachedTranslations('en', () => + i18nLoader.getTranslationsByLocale('en') + ); + warning = { + title: 'Unsupported Locale', + text: `The requested locale "${locale}" is not supported. Falling back to English.`, + }; } + const response = { + translations: cachedTranslations.translations, + warning, + }; + return h - .response(cachedTranslations.translations) + .response(response) .header('cache-control', 'must-revalidate') .header('content-type', 'application/json') .etag(cachedTranslations.hash); diff --git a/src/legacy/ui/ui_render/ui_render_mixin.test.js b/src/legacy/ui/ui_render/ui_render_mixin.test.js index 61619e46b0b..81f8a9f9a69 100644 --- a/src/legacy/ui/ui_render/ui_render_mixin.test.js +++ b/src/legacy/ui/ui_render/ui_render_mixin.test.js @@ -4,7 +4,6 @@ */ import { uiRenderMixin } from './ui_render_mixin'; -import Boom from '@hapi/boom'; // Mock dependencies jest.mock('@osd/i18n', () => ({ @@ -96,7 +95,10 @@ describe('uiRenderMixin', () => { await handler(request, h); expect(i18n.getTranslation).toHaveBeenCalled(); - expect(h.response).toHaveBeenCalledWith(JSON.stringify(defaultTranslations)); + expect(h.response).toHaveBeenCalledWith({ + translations: defaultTranslations, + warning: null, + }); expect(h.header).toHaveBeenCalledWith('cache-control', 'must-revalidate'); expect(h.header).toHaveBeenCalledWith('content-type', 'application/json'); expect(h.etag).toHaveBeenCalled(); @@ -114,19 +116,34 @@ describe('uiRenderMixin', () => { await handler(request, h); expect(i18nLoader.getTranslationsByLocale).toHaveBeenCalledWith(requestedLocale); - expect(h.response).toHaveBeenCalledWith(JSON.stringify(frTranslations)); + expect(h.response).toHaveBeenCalledWith({ + translations: frTranslations, + warning: null, + }); }); - it('should throw not found error for unknown locale', async () => { + it('should fallback to English translations for unknown locale', async () => { const defaultLocale = 'en'; const unknownLocale = 'xx'; + const englishTranslations = { hello: 'Hello' }; i18n.getLocale.mockReturnValue(defaultLocale); i18nLoader.getRegisteredLocales.mockReturnValue([defaultLocale]); + i18nLoader.getTranslationsByLocale.mockResolvedValue(englishTranslations); const request = { params: { locale: unknownLocale } }; - await expect(handler(request, h)).rejects.toThrow( - Boom.notFound(`Unknown locale: ${unknownLocale}`) - ); + await handler(request, h); + + expect(i18nLoader.getTranslationsByLocale).toHaveBeenCalledWith('en'); + expect(h.response).toHaveBeenCalledWith({ + translations: englishTranslations, + warning: { + title: 'Unsupported Locale', + text: `The requested locale "${unknownLocale}" is not supported. Falling back to English.`, + }, + }); + expect(h.header).toHaveBeenCalledWith('cache-control', 'must-revalidate'); + expect(h.header).toHaveBeenCalledWith('content-type', 'application/json'); + expect(h.etag).toHaveBeenCalled(); }); it('should cache translations', async () => { From 2052a6672c89d18d5ddca2ca2ce17e60938ec7f9 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Wed, 14 Aug 2024 17:43:11 +0000 Subject: [PATCH 3/5] fix comments 2 Signed-off-by: Anan Zhuang --- src/core/public/core_system.ts | 31 --------------------- src/core/public/osd_bootstrap.test.mocks.ts | 4 +-- src/core/public/osd_bootstrap.test.ts | 6 ++-- src/core/public/osd_bootstrap.ts | 6 ++-- 4 files changed, 9 insertions(+), 38 deletions(-) diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 8d9a0ddce02..731cf461824 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -318,35 +318,4 @@ export class CoreSystem { this.workspaces.stop(); this.rootDomElement.textContent = ''; } - - public async displayWarning(title: string, text: string) { - const i18n = await this.i18n.start(); - const injectedMetadata = this.injectedMetadata.setup(); - this.fatalErrorsSetup = this.fatalErrors.setup({ - injectedMetadata, - i18n: this.i18n.getContext(), - }); - await this.integrations.setup(); - this.docLinks.setup(); - const http = this.http.setup({ injectedMetadata, fatalErrors: this.fatalErrorsSetup }); - const uiSettings = this.uiSettings.setup({ http, injectedMetadata }); - const notificationsTargetDomElement = document.createElement('div'); - const overlayTargetDomElement = document.createElement('div'); - const overlays = this.overlay.start({ - i18n, - targetDomElement: overlayTargetDomElement, - uiSettings, - }); - if (this.notifications) { - const toasts = await this.notifications.start({ - i18n, - overlays, - targetDomElement: notificationsTargetDomElement, - })?.toasts; - - if (toasts) { - toasts.addWarning({ title, text }); - } - } - } } diff --git a/src/core/public/osd_bootstrap.test.mocks.ts b/src/core/public/osd_bootstrap.test.mocks.ts index 982eef04b38..87a6ab49973 100644 --- a/src/core/public/osd_bootstrap.test.mocks.ts +++ b/src/core/public/osd_bootstrap.test.mocks.ts @@ -50,7 +50,6 @@ jest.doMock('@osd/i18n', () => ({ }, })); -export const displayWarningMock = jest.fn(); export const coreSystemMock = { setup: jest.fn().mockResolvedValue({ fatalErrors: fatalErrorMock, @@ -58,8 +57,9 @@ export const coreSystemMock = { start: jest.fn().mockResolvedValue({ application: applicationServiceMock.createInternalStartContract(), }), - displayWarning: displayWarningMock, }; jest.doMock('./core_system', () => ({ CoreSystem: jest.fn().mockImplementation(() => coreSystemMock), })); + +export const consoleWarnMock = jest.spyOn(console, 'warn').mockImplementation(() => {}); diff --git a/src/core/public/osd_bootstrap.test.ts b/src/core/public/osd_bootstrap.test.ts index ec7559c3c31..7630b4441ab 100644 --- a/src/core/public/osd_bootstrap.test.ts +++ b/src/core/public/osd_bootstrap.test.ts @@ -33,7 +33,7 @@ import { fatalErrorMock, i18nLoad, i18nSetLocale, - displayWarningMock, + consoleWarnMock, } from './osd_bootstrap.test.mocks'; import { __osdBootstrap__ } from './'; import { getLocaleInUrl } from './locale_helper'; @@ -110,7 +110,7 @@ describe('osd_bootstrap', () => { await __osdBootstrap__(); - expect(displayWarningMock).toHaveBeenCalledWith('Locale Warning', 'Invalid locale'); + expect(consoleWarnMock).toHaveBeenCalledWith('Locale Warning: Invalid locale'); expect((window as any).__localeWarning).toBeUndefined(); }); @@ -119,7 +119,7 @@ describe('osd_bootstrap', () => { await __osdBootstrap__(); - expect(displayWarningMock).toHaveBeenCalledWith('i18n Warning', 'Translation issue'); + expect(consoleWarnMock).toHaveBeenCalledWith('i18n Warning: Translation issue'); expect((window as any).__i18nWarning).toBeUndefined(); }); }); diff --git a/src/core/public/osd_bootstrap.ts b/src/core/public/osd_bootstrap.ts index fc131b277af..b1f00d25ca1 100644 --- a/src/core/public/osd_bootstrap.ts +++ b/src/core/public/osd_bootstrap.ts @@ -98,14 +98,16 @@ export async function __osdBootstrap__() { // Display the i18n warning if it exists if ((window as any).__i18nWarning) { const warning = (window as any).__i18nWarning; - coreSystem.displayWarning(warning.title, warning.text); + // eslint-disable-next-line no-console + console.warn(`${warning.title}: ${warning.text}`); delete (window as any).__i18nWarning; } // Display the locale warning if it exists if ((window as any).__localeWarning) { const warning = (window as any).__localeWarning; - coreSystem.displayWarning(warning.title, warning.text); + // eslint-disable-next-line no-console + console.warn(`${warning.title}: ${warning.text}`); delete (window as any).__localeWarning; } } From 77eff19ace1b4466a203ecec5a1ef6769b42274c Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Thu, 15 Aug 2024 02:14:36 +0000 Subject: [PATCH 4/5] fix tests Signed-off-by: Anan Zhuang --- packages/osd-i18n/src/core/i18n.test.ts | 34 ++++++++++- .../public/application/scoped_history.test.ts | 60 +++++++++++++++++++ src/core/public/application/scoped_history.ts | 3 +- src/core/public/locale_helper.test.ts | 12 ++-- src/core/public/locale_helper.ts | 6 +- src/core/public/osd_bootstrap.ts | 5 +- 6 files changed, 107 insertions(+), 13 deletions(-) diff --git a/packages/osd-i18n/src/core/i18n.test.ts b/packages/osd-i18n/src/core/i18n.test.ts index 0ee114c78c9..ebfd546f856 100644 --- a/packages/osd-i18n/src/core/i18n.test.ts +++ b/packages/osd-i18n/src/core/i18n.test.ts @@ -899,8 +899,17 @@ describe('I18n engine', () => { describe('load', () => { let mockFetch: jest.SpyInstance; + let originalWindow: any; + beforeEach(() => { mockFetch = jest.spyOn(global as any, 'fetch').mockImplementation(); + originalWindow = global.window; + global.window = { ...originalWindow }; + }); + + afterEach(() => { + global.window = originalWindow; + delete (window as any).__i18nWarning; // Clear the warning after each test }); test('fails if server returns >= 300 status code', async () => { @@ -928,7 +937,7 @@ describe('I18n engine', () => { mockFetch.mockResolvedValue({ status: 200, - json: jest.fn().mockResolvedValue(translations), + json: jest.fn().mockResolvedValue({ translations }), }); await expect(i18n.load('some-url')).resolves.toBeUndefined(); @@ -938,5 +947,28 @@ describe('I18n engine', () => { expect(i18n.getTranslation()).toEqual(translations); }); + + test('sets warning on window when present in response', async () => { + const warning = { title: 'Warning', text: 'This is a warning' }; + mockFetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue({ translations: { locale: 'en' }, warning }), + }); + + await i18n.load('some-url'); + + expect((window as any).__i18nWarning).toEqual(warning); + }); + + test('does not set warning on window when not present in response', async () => { + mockFetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue({ translations: { locale: 'en' } }), + }); + + await i18n.load('some-url'); + + expect((window as any).__i18nWarning).toBeUndefined(); + }); }); }); diff --git a/src/core/public/application/scoped_history.test.ts b/src/core/public/application/scoped_history.test.ts index 067c33256bd..6575a6aa1ab 100644 --- a/src/core/public/application/scoped_history.test.ts +++ b/src/core/public/application/scoped_history.test.ts @@ -30,8 +30,23 @@ import { ScopedHistory } from './scoped_history'; import { createMemoryHistory } from 'history'; +import { getLocaleInUrl } from '../locale_helper'; +import { i18n } from '@osd/i18n'; + +jest.mock('../locale_helper', () => ({ + getLocaleInUrl: jest.fn(), +})); + +jest.mock('@osd/i18n', () => ({ + i18n: { + getLocale: jest.fn(), + }, +})); describe('ScopedHistory', () => { + beforeEach(() => { + (getLocaleInUrl as jest.Mock).mockReturnValue('en'); + }); describe('construction', () => { it('succeeds if current location matches basePath', () => { const gh = createMemoryHistory(); @@ -358,4 +373,49 @@ describe('ScopedHistory', () => { expect(gh.length).toBe(4); }); }); + + describe('locale handling', () => { + let originalLocation: Location; + + beforeEach(() => { + originalLocation = window.location; + delete (window as any).location; + window.location = { href: 'http://localhost/app/wow', reload: jest.fn() } as any; + (i18n.getLocale as jest.Mock).mockReturnValue('en'); + }); + + afterEach(() => { + window.location = originalLocation; + jest.resetAllMocks(); + }); + + it('reloads the page when locale changes', () => { + const gh = createMemoryHistory(); + gh.push('/app/wow'); + const h = new ScopedHistory(gh, '/app/wow'); + // Use the 'h' variable to trigger the listener + h.push('/new-page'); + + // Mock getLocaleInUrl to return a different locale + (getLocaleInUrl as jest.Mock).mockReturnValue('fr'); + + // Simulate navigation + gh.push('/app/wow/new-page'); + + expect(window.location.reload).toHaveBeenCalled(); + }); + + it('does not reload the page when locale changes', () => { + const gh = createMemoryHistory(); + gh.push('/app/wow'); + + // Mock getLocaleInUrl to return a different locale + (getLocaleInUrl as jest.Mock).mockReturnValue('en'); + + // Simulate navigation + gh.push('/app/wow/new-page'); + + expect(window.location.reload).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/core/public/application/scoped_history.ts b/src/core/public/application/scoped_history.ts index 2befe5335bc..74e4bb068d3 100644 --- a/src/core/public/application/scoped_history.ts +++ b/src/core/public/application/scoped_history.ts @@ -317,8 +317,9 @@ export class ScopedHistory this.isActive = false; return; } - // const fullUrl = `${location.pathname}${location.search}${location.hash}`; + const localeValue = getLocaleInUrl(window.location.href); + if (localeValue !== currentLocale) { // Force a full page reload window.location.reload(); diff --git a/src/core/public/locale_helper.test.ts b/src/core/public/locale_helper.test.ts index 0ba878feae3..238dbced389 100644 --- a/src/core/public/locale_helper.test.ts +++ b/src/core/public/locale_helper.test.ts @@ -21,21 +21,21 @@ describe('getLocaleInUrl', () => { expect(getLocaleInUrl(url)).toBe('fr-FR'); }); - it('should return null for a URL without locale', () => { + it('should return en for a URL without locale', () => { const url = 'http://localhost:5603/app/home'; - expect(getLocaleInUrl(url)).toBeNull(); + expect(getLocaleInUrl(url)).toBe('en'); }); - it('should return null and set a warning for an invalid locale format in hash', () => { + it('should return en and set a warning for an invalid locale format in hash', () => { const url = 'http://localhost:5603/app/home#/&locale=de-DE'; - expect(getLocaleInUrl(url)).toBeNull(); + expect(getLocaleInUrl(url)).toBe('en'); expect((window as any).__localeWarning).toBeDefined(); expect((window as any).__localeWarning.title).toBe('Invalid URL Format'); }); - it('should return null for an empty locale value', () => { + it('should return en for an empty locale value', () => { const url = 'http://localhost:5603/app/home?locale='; - expect(getLocaleInUrl(url)).toBeNull(); + expect(getLocaleInUrl(url)).toBe('en'); }); it('should handle URLs with other query parameters', () => { diff --git a/src/core/public/locale_helper.ts b/src/core/public/locale_helper.ts index 77ac4b4583f..38a734a523b 100644 --- a/src/core/public/locale_helper.ts +++ b/src/core/public/locale_helper.ts @@ -43,11 +43,11 @@ export function getLocaleInUrl(url: string): string | null { // Check for non standard query format: if (localeValue === null && url.includes('&locale=')) { setInvalidUrlWithLocaleWarning(); - return null; + return 'en'; } - // Return the locale value if found, or null if not found - return localeValue && localeValue.trim() !== '' ? localeValue : null; + // Return the locale value if found, or 'en' if not found + return localeValue && localeValue.trim() !== '' ? localeValue : 'en'; } function setInvalidUrlWarning(): void { diff --git a/src/core/public/osd_bootstrap.ts b/src/core/public/osd_bootstrap.ts index b1f00d25ca1..5190fdc37e2 100644 --- a/src/core/public/osd_bootstrap.ts +++ b/src/core/public/osd_bootstrap.ts @@ -40,9 +40,10 @@ export async function __osdBootstrap__() { ); // Extract the locale from the URL if present + const currentLocale = i18n.getLocale(); const urlLocale = getLocaleInUrl(window.location.href); - if (urlLocale) { + if (urlLocale && urlLocale !== currentLocale) { // If a locale is specified in the URL, update the i18n settings // This enables dynamic language switching // Note: This works in conjunction with server-side changes: @@ -61,7 +62,7 @@ export async function __osdBootstrap__() { /\/([^/]+)\.json$/, `/${urlLocale}.json` ); - } else { + } else if (!urlLocale) { i18n.setLocale('en'); } From cea33d4a648f6cbbce1c06c16a3b3fc3ddd5b542 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Thu, 15 Aug 2024 06:41:00 +0000 Subject: [PATCH 5/5] Changeset file for PR #7686 created/updated --- changelogs/fragments/7686.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/7686.yml diff --git a/changelogs/fragments/7686.yml b/changelogs/fragments/7686.yml new file mode 100644 index 00000000000..f446ed68764 --- /dev/null +++ b/changelogs/fragments/7686.yml @@ -0,0 +1,2 @@ +feat: +- Change the locale dynamically by adding &i18n-locale to URL ([#7686](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7686)) \ No newline at end of file