Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kibana Globalization - Phase 1 #7545

Merged
merged 68 commits into from
Dec 14, 2016
Merged

Conversation

hickeyma
Copy link
Contributor

@hickeyma hickeyma commented Jun 24, 2016

Phase 1

Purpose

  • Implement the I18n class which provides a means to supply translations in a standard format that it not dependent on a localization framework
  • Translate the Kibana welcome message which proves that the I18n class provides translations as registered for Kibana

Decide Language for Translation Algorithm

  • The algorithm which decides the language for loading translations is as follows:
    1. First, do a direct comparison with the highest priority locale in the HTTP header “accept-language” against the registered translation languages. If comparison found then this is the language returned.
    2. Next, do a partial comparison of this locale whereby you try and get a language code comparison against the registered translation languages. For example, "fr" or "fr-FR" is used for all regions of French if comparison applies. If comparison found then this is the language returned.
    3. Next, repeat step 2 and if need be step 3 for the next highest priority locale in the “accept-language” list.
    4. Continue, until match found in step 2/step 3 or end of the “accept-language” list. If no match found then return empty language.

I18n Class

  • Manages the language translations for Kibana

  • Responsible for loading translated content per language

  • The translations file are JSON files with a flat keyspace, where the keys are unique. This uniqueness between translation plugins could be achieved by prefixing the keys with the plugin name. The key signifies the translation ID which would be referenced in translatable files (like JS, HTML etc.).

  • The key value is the translation string

  • Example translation JSON file

en.json

    {
        "UI-WELCOME_MESSAGE": "Loading Kibana",
        "UI-LOADING_MESSAGE": "Give me a moment here. I'm loading a whole bunch of code. Don't worry, all this good stuff will be cached up for next time!",
        "UI-WELCOME_ERROR": "Kibana did not load properly. Check the server output for more information."
    }
  • Core Kibana plugins like ‘kibana’ and ‘status_page’ could come with their own English translations bundled in
  • API:
    • Return all translations for registered locales
      • getAllTranslations()
      • A Promise object where keys are the locale and values are Objects of translation keys and translations
    • Return all translations registered for the default locale:
      • getTranslationsForDefaultLocale()
      • A Promise for an object where keys are translation keys and values are translations
    • Return translations for a suitable locale from a user side locale list of BCP 47 language tags:
      • getTranslations(...languageTags)
      • A Promise for an object where keys are translation keys and values are translations
      • This object will contain all registered translations for the highest priority locale which is registered with the i18n module
      • This object can be empty if no locale in the language tags can be matched against the registered locales
    • Register translations:
      • registerTranslations(<absolute_path_to_translation_file>)
      • The path to the translation file is registered with i18n class

UiI18n Class

  • Handles the interaction between the UI/server and localization (I18n class)
  • Maps the accept-language header to BCP 47 tags
  • Fetches the language as requested where the locale is decided by the translation algorithm
  • It also substitutes any missing translations with the default locale translation

Deliverable

  • Translate the start-up message (“Kibana is loading ...”) and error message in the Jade template
    (https://github.com/elastic/kibana/blob/master/src/ui/views/ui_app.jade)
  • Translation message will be loaded using the language as per the deciding algorithm. For phase 1 the default 'en' langauge will be used.
  • I18n unit tests
  • Jade template verification of translation strings:
    • Enforce a pattern to be used. For example a i18n(<key>) function in the Jade template. A tool can then be used to find such pattern and extract the keys to file.
    • The keys in the key file(s) would then be checked against the language translation files registered
  • Kibana core plugin registers its translation file during the initialization phase. The translation file contains strings for the welcome message and the start-up error message.

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@hickeyma
Copy link
Contributor Author

@Bargs, @epixa, @srl295, @shikhasriva: First draft of i18n core plugin.


// Added this function because 'mkdirp' does not add more than 2 subdirectories
function createDirectoriesRecursively(fullDir) {
process.execSync('mkdir -p ' + fullDir);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unix - may need to rewrite or use another module

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srl295, good point, thanks. I was having issue previously but may have been that I was using the asynchronous version of 'mkdirp' in a synchronous fashion. I have updated the code now to use 'mkdirp.sync' and is working as expected.

@Bargs Bargs self-assigned this Jun 27, 2016
@Bargs
Copy link
Contributor

Bargs commented Jun 27, 2016

@hickeyma I read through the code in the PR. Before I jump into feedback, let me make sure I understand everything correctly. I've written up some notes with my understanding of the API (both JS module and HTTP) thus far:

Module API

  • registerPluginLanguageTranslations(pluginName, pluginTranslationPath, language) - registers a plugin's translation with provided pluginName, path to translation files, and language
  • getRegisteredPluginLanguageTranslations(pluginName, language) - gets json translations based on plugin name and language
  • getPluginTranslationDetails(pluginTranslationPath, translationFiles, languageList) - gets translationFiles and languageList from a path to translation files, updating the params passed in
  • getRegisteredPluginStoragePath(pluginName) - returns path to temp storage of translation files for a given plugin name
  • getRegisteredPluginLanguages(pluginName) - returns array of language codes for a given plugin name

I'm guessing registerPluginLanguageTranslations, getRegisteredPluginLanguageTranslations, getRegisteredPluginLanguages are the methods you expect to be most useful to client code?

HTTP API

/api/i18n/translations/{plugin} - returns JSON translations based on plugin name and accept header

I will add one general note. I'm worried about loading translations per plugin. A plugin might provide a very small piece of a page. For instance, the Visualize page which displays all the visualization types available could be displaying content from many different plugins. So if we're loading translations by plugin, it might take more than 10 round trips to the server to get all of the translations for that page. Unless we end up finding that these translation bundles are huge, I think it makes more sense to load an entire language, for all plugins, in one go.

@hickeyma
Copy link
Contributor Author

hickeyma commented Jun 28, 2016

@Bargs Thanks for the notes; good synopsis.

Module API

Good description of the APIs.

Additionally, registerPluginLanguageTranslations(pluginName, pluginTranslationPath, language) - bundles all translation files for language into the one translation file for language.

registerPluginLanguageTranslations and getRegisteredPluginLanguageTranslations are the most useful to client code.

HTTP API

This is good feedback that a plugin may not provide all code to a page or multiple pages. If this is the case then it makes sense to have an API which returns all translations bundled for all plugins. I will add this API.
Should we maintain the API to return translations per plugin as well?

@Bargs
Copy link
Contributor

Bargs commented Jun 28, 2016

Should we maintain the API to return translations per plugin as well?

Probably not, we can always add it if we realize we need it later.

Also, btw a PR (#7457) just got merged that gives plugins a dedicated data directory for writing to the filesystem. I think you'll find that helpful in this plugin.

@epixa
Copy link
Contributor

epixa commented Jun 28, 2016

I question the value of the HTTP API at all: can we not just have the kibana server grab translations for the current bundles on load and serve them on the initial request?

@Bargs
Copy link
Contributor

Bargs commented Jun 28, 2016

@epixa are you suggesting all language translations get built directly into each JS bundle?

@epixa
Copy link
Contributor

epixa commented Jun 28, 2016

No, I think we could serve the current language of the user as javascript in either the html or as a separate js file. In the same way that we serve up details about kibana itself in window.__KBN__.

@Bargs
Copy link
Contributor

Bargs commented Jun 28, 2016

I suppose inlining the translations (like __KBN__) would save a roundtrip, but now you have to worry about caching different versions of the page for different languages. That seems more like a nice optimization to me. Even if we had that, I think we'd still want a separate HTTP endpoint for serving translations when the user wants to switch languages, so it can be done dynamically without a page reload.

@epixa
Copy link
Contributor

epixa commented Jun 28, 2016

We need the translations when the contents are loading anyway, otherwise we can't translate things like the loading screen or the global error handler (not the angular notifier handler, the more global one). So at the very least, those translations will need to be served up with the HTML.

We shouldn't optimize for the experience of switching languages, and I'd prefer to outright eliminate any overhead in maintaining that experience by requiring a page reload. For the vast majority of installs, there will only be a single language configured. Most Kibana installs are private to a company, and most companies either have no use at all for supporting multiple languages or won't be interested in the ongoing burden of maintaining a multiple language install. There are exceptions, of course, but we should be optimizing for the most common usage.

@srl295
Copy link
Contributor

srl295 commented Jun 28, 2016

(jumping in midstream…)
@epixa the purpose of the HTTP API is to facilitate loading of all of the current bundles. It's not for optimizing switching translations. The data pretty much has to go over HTTP anyway, so the point was that just as the kibana.bundle.js gets fetched in one GET, that one GET would also fetch any relevant translations. That GET would give the opportunity to do the content negotiation needed with the web browser.

@epixa
Copy link
Contributor

epixa commented Jun 28, 2016

But I think that language selection needs to happen during the initial load of the application one way or another, otherwise we can't translate the two screens I mentioned above. I agree that most of the translations can be loaded separately, but at least those translations need to be applied when the initial html is created on the server, so we're already going to need to do language determination at that time.

As for a separate request for translations, that really needs to happen during the initial load window (when you see the "one moment please" screen), which means that cannot be initiated from the client-side JS bundles themselves. It must either be embedded into the HTML (which is really only ideal for the very few strings that exist in the global context of the main html) or linked as a script tag along with the other bundles: https://github.com/elastic/kibana/blob/master/src/ui/views/ui_app.jade#L48-L53

@srl295
Copy link
Contributor

srl295 commented Jun 28, 2016

or linked as a script tag along with the other bundles

That's what I thought would happen for initial content.

@Bargs
Copy link
Contributor

Bargs commented Jun 28, 2016

which means that cannot be initiated from the client-side JS bundles themselves

@epixa Yeah I see the miscommunication now. When I wrote "HTTP API" in my summary comment, I didn't mean "an API that can only be called via XHR". As I mentioned in this comment, the translations can be served by Hapi just like the static bundles are served by Hapi currently.

@Bargs
Copy link
Contributor

Bargs commented Jun 28, 2016

A couple more thoughts:

  • If we want to build the language bundles at build time and/or plugin installation time, how will that work if the core i18n API is implemented as a plugin? Plugins don't get initialized until Kibana starts. I don't have the answer off the top of my head, I'd have to learn a bit more about the current plugin installation process. Something to think about.
  • Should we divide language bundles per app? This is how the JS bundles currently get divided up. But this would require knowledge of which apps use which translations.
  • Since using english strings as keys is a bad idea, how should we namespace these things? Plugin id probably? What checks should we run at compile time?

@Bargs
Copy link
Contributor

Bargs commented Jun 30, 2016

First draft of an example translation plugin structure: https://github.com/Bargs/management-es

At first glance I thought this might work at installation time without any modifications, but upon further inspection I've realized the plugin installer doesn't initialize the plugins, it only creates the js/css bundles. I think we should make it work at install time, so I need to give this some more thought.

@Bargs
Copy link
Contributor

Bargs commented Jun 30, 2016

So I think we have a couple options.

  1. Just build the translation bundles on server startup.

    Pro: Requires no change to the current install/startup process. Kibana will heal itself if the translation bundles are deleted.

    Con: Adds additional work on server start up. Build/install time verifications are impossible. If we want to speed up server start up, I think we're going to run into some complexity with detecting changes and deciding when to invalidate the cache. We'll have to deal with additional complexity of deciding when to turn Kibana green, because it will need to wait for the async translation bundling to finish.

  2. Create an additional plugin lifecycle hook specifically for install time tasks.

    Pro: Runs once at install time, could run some validations before ever booting Kibana. Other plugins may find this hook useful down the road.

    Con: No automatic healing without additional work. In order to lean on the existing plugin dependency graph, this would probably be enabled as an option on the minimal Kibana boot up already being used in the plugin installation process.

I'm leaning towards the second option. I think it would be pretty easy to implement and should avoid a lot of complexity at server startup time. What do you think @epixa?

@hickeyma
Copy link
Contributor Author

hickeyma commented Jul 1, 2016

@Bargs I like the example translation plugin structure.

I am interested to know more about the "additional plugin lifecycle hook" ! :-)

FYI: I have refactored the i18n plugin:

  • To register translations with top translation directory only
  • Parse a flattened translation JSON object to be key/value elements

@Bargs
Copy link
Contributor

Bargs commented Jul 1, 2016

Unless someone just outright hates the idea of adding an install-time hook to the server boot process that can be enabled in the plugin installer, I'm going to work on a prototype for it. It's a little weird adding it to the KbnServer constructor, but I think it's better than inventing a totally separate mechanism right now.

@Bargs
Copy link
Contributor

Bargs commented Jul 2, 2016

Here's a rough prototype of what I'm thinking: #7613

@elasticmachine
Copy link
Contributor

Can one of the admins verify this patch?

@Bargs
Copy link
Contributor

Bargs commented Jul 5, 2016

@hickeyma to follow up on your question from IRC, you'll want to replace my mock registration here with your actual i18n module.

My idea is that within the install hook, plugins can register helper functions (like registerTranslation) with the installContext, which other plugins can use in their own install hook functions. Essentially installContext is a simple dependency injection context for the install lifecycle that would allow plugins to develop implementations that rely on other plugins, without coupling them to the specific module paths in their dependencies. An added benefit is that we can lean on the existing dependency resolution code in the Kibana plugin system, so if one plugin relies on another, we just specify that as usual.

@hickeyma
Copy link
Contributor Author

hickeyma commented Jul 6, 2016

@Bargs I thought the mock registration provided some hooks back into the actual call to registerTranslations. Thanks for setting me straight. It worked as expected when I made the change as you suggested.

@Bargs
Copy link
Contributor

Bargs commented Jul 7, 2016

As promised, here's my first round of feedback. This probably looks like a massive list, but a lot of it is just coding convention stuff to make things more consistent with the rest of Kibana. This list might not be totally comprehensive either, but I wanted to get you some feedback asap so you're not blocked.

  • README, .gitignore, and .eslintrc are not needed in a core plugin
  • package.json only needs name and version
  • unit tests need to go in a tests directory otherwise they won't get picked up by the grunt tasks. Also our convention is to name the test file with the same name as the module it's testing (so i18n_tests.js should just be i18n.js)
  • Unit tests need to be updated based on new proposed plugin structure (single language file, not split by view)
  • For consistency with the rest of the code base, rename the data directory to fixtures.
  • Prefer const (or let if necessary). Don't use var.
  • Use ES6 imports/exports rather than commonjs style
  • Only export the i18n module's public API. For instance, I don't think getPluginTranslationDetails is used outside of the i18n module, so it shouldn't be exposed publicly. If you want to expose it for testing purposes, I would recommend creating an i18n directory with an index.js file that exports the module's public API, and a separate i18n.js file with the "private" API. index.js will be for public use, i18n.js will be for private internal use.
  • All filesystem access should be async
  • i18n module API should return promises for async operations instead of using callbacks. You can also probably clean up a lot of the internal i18n code by using http://bluebirdjs.com/docs/api/promise.promisify.html to wrap node functions that expect a callback

- Update after following review: elastic#7545 (review)
- Also, fix syntax mess following rebase with master of src/optimize/index.js
@hickeyma
Copy link
Contributor Author

Thanks @Bargs for review comments. Updated and ready for review.

@Bargs
Copy link
Contributor

Bargs commented Dec 13, 2016

jenkins, test it

@Bargs
Copy link
Contributor

Bargs commented Dec 13, 2016

Code LGTM, let's see what Jenkins thinks!

@spalger
Copy link
Contributor

spalger commented Dec 14, 2016

jenkins, test this

@spalger spalger merged commit 7028a88 into elastic:master Dec 14, 2016
@spalger
Copy link
Contributor

spalger commented Dec 14, 2016

🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

@spalger
Copy link
Contributor

spalger commented Dec 14, 2016

@srl295
Copy link
Contributor

srl295 commented Dec 14, 2016 via email

@shikhasriva
Copy link

Awesome and yes this is just the beginning... but a great start. Looking forward to next steps

@epixa
Copy link
Contributor

epixa commented Dec 14, 2016

Very exciting. Great job on this!

@elasticmachine
Copy link
Contributor

Can one of the admins verify this patch?

@kimchy
Copy link
Member

kimchy commented Dec 15, 2016

great stuff!

@hickeyma hickeyma deleted the i18n_core_plugin branch January 9, 2017 12:07
elastic-jasper added a commit that referenced this pull request Mar 2, 2017
Backports PR #7545

**Commit 1:**
Add low level i18n plugin

Manages languages that are available and is responsible for loading translated
content at the granularity of a plugin.

To be done:
 - APIs for store and retrieval

* Original sha: aca671f
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-20T17:12:25Z

**Commit 2:**
Use Kibana install as root for the translation store directory

Setting the path for storing the bundled language translation files to
<KIBANA_INSTALL>/data/store_translations/<PLUGIN_NAME>

* Original sha: c3ba578
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-21T13:36:59Z

**Commit 3:**
Updated i18n core plugin APIs to be asynchronous

To be done:
 - Better error handling in APIs
 - Fix threading issue with storePluginLanguageTranslations API

* Original sha: 80d8a2c
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-22T17:25:30Z

**Commit 4:**
Fix thread synchroization issue in storePluginLanguageTranslations

* Original sha: 856fb02
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-24T10:43:06Z

**Commit 5:**
Update error handling in i18n core plugin

* Original sha: c5d22be
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-24T14:30:27Z

**Commit 6:**
Change to use NodeJS mkdirp function for creating directories recursively

Updates with review comments from @srl295. Changed export syntax to show the
exported functions at end of file.

* Original sha: b37e4f8
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-27T11:04:01Z

**Commit 7:**
Add REST API for getting translations of a language for a plugin

To be done:
 - Add algorithm to decide on the language for a plugin by comparing the accept languages
from the REST call and the plugin supported languages
 - Add REST API tests

* Original sha: ec6d2b1
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-27T22:42:15Z

**Commit 8:**
Add algorithm for determining plugin language when retrieving translations

Client would pass languages used in the 'accept-language' header. These
languages would then be compared against the plugin supported languages
and best compared language would be selected.

To be done:
 - Add REST API tests

* Original sha: a75faae
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-28T21:10:39Z

**Commit 9:**
Add API to return all registered plugin language translations

* Original sha: 66d051b
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-29T17:02:34Z

**Commit 10:**
Add HAPI API to get all plugins translation files

* Original sha: c3403e7
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-30T16:08:28Z

**Commit 11:**
Update register translations API to be independent of plugin name and language

The register API is updated to be independent of plugin name and language. The API will now
traverse the path given and create language bundles as per language files it traverses.
The translations files structure has also been simplified to be just key/value objects.

To be done:
 - Add hapi API to get translations
 - Extend the API tests to test responses

* Original sha: 0f4902d
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-01T16:09:33Z

**Commit 12:**
Update API test

* Original sha: 9785df2
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-05T17:52:25Z

**Commit 13:**
Add eslint fix for API test

* Original sha: d41fcec
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-06T10:00:22Z

**Commit 14:**
Update with review comments

From review #7545 (comment)
following comments updated:
- README, .gitignore, and .eslintrc are not needed in a core plugin
- package.json only needs name and version
- unit tests need to go in a tests directory otherwise they won't get picked up
by the grunt tasks. Also our convention is to name the test file with the same
name as the module it's testing (so i18n_tests.js should just be i18n.js)
- For consistency with the rest of the code base, rename the data directory to fixtures.
- Prefer const (or let if necessary). Don't use var.
- Use ES6 imports/exports rather than commonjs style
- Only export the i18n module's public API. For instance, I don't think getPluginTranslationDetails is used outside of the i18n module, so it shouldn't be exposed publicly. If you want to expose it for testing purposes, I would recommend creating an i18n directory with an index.js file that exports the module's public API, and a separate i18n.js file with the "private" API. index.js will be for public use, i18n.js will be for private internal use.

* Original sha: c8b2197
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-11T11:23:37Z

**Commit 15:**
Update after review comments

From review (#7545 (comment)):
- i18n module API should return promises for async operations instead of using
callbacks
- All filesystem access should be async
- Unit tests need to be updated based on new proposed plugin structure
(single language file, not split by view)

From design (#6515 (comment)):
- Removed API as will consider in later phase

TODO:
- Make write function async

* Original sha: e17653d
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-15T18:24:21Z

**Commit 16:**
Update after review comments

Updated write function to be asynchronous

* Original sha: eaf35ab
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-19T11:03:45Z

**Commit 17:**
Update registerTranslations API to take absolute translation file as argument

The API originally took the directory as the argument but following reviews it
was decided to change to absolute file because it will be less brittle
since it is more explicit.

* Original sha: f1974ca
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-21T17:43:27Z

**Commit 18:**
Translate the Kibana welcome message

Translates the start-up message (“Kibana is loading ...”)in the Jade template.

To be done:
 - Means to register the core plugin translations. They are currently added
in the fixtures directory as static files. Need to be generated on the fly.

* Original sha: 10db458
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-25T13:58:54Z

**Commit 19:**
Add build task to generate core plugin translations

Task which calls registerTranslations API and then a task which copies the
regsitered translations to <kibana_root>/build/kibana

* Original sha: 4957173
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-27T10:00:35Z

**Commit 20:**
Add hook to optimize module to add registration during dev startup

Registration of the core plugin translations during development start of
Kibana server. The translations include the welcome message and server error
startup message.

* Original sha: 5f61475
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-28T14:18:19Z

**Commit 21:**
Handle scenario when the user preferred language header is not passed

The UI when loading asks i18n plugin which language translation to use
depending on the user preferred language header 'accept-language'.
This commit is to handle scenario where header is not passed. The algorithm
then chooses the default language.

* Original sha: 9fbe6d5
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-29T10:51:33Z

**Commit 22:**
Replace registering of translations at plugin install time to the plugin init phase

This change follows review comments in:
#6515 (comment)

* Original sha: 32d5034
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-08T14:39:34Z

**Commit 23:**
Update after review comments

Comments:
- #7545 (diff)
- #7545 (diff)

* Original sha: 8c7f51c
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-09T09:36:04Z

**Commit 24:**
Update after plugin folder layout changes in Kibana

This require to use <kibana_root>/data for registered translations
and i18n plugin moved to core_plugins from plugins.

Refer to PR for more details:
#7562

* Original sha: b84f40a
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-09T11:09:18Z

**Commit 25:**
Update translation registration to file path rather than bundling

After review discussions it was agreed to just register the absolute paths
to translation files rather than bundling each file into one central file
at registration.

* Original sha: 2dd3065
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-16T09:28:38Z

**Commit 26:**
Update review comments

This commit contains the following review comments:
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)

* Original sha: db11a2d
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-18T14:09:22Z

**Commit 27:**
Update review comments

The following review comments are included in the commit:
- #7545 (comment)
- #7545 (comment)
- #7545 (comment)

* Original sha: 8c39a8d
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-19T12:44:39Z

**Commit 28:**
Expose the i18n APIs in the server object for plugin access

Plugins should call the i18n plugin APIs through the server object
and not directly from the module.

This closes he following comments:
- #7545 (comment)
- #7545 (comment)
- #7545 (comment)

* Original sha: a9e30d3
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-19T17:31:10Z

**Commit 29:**
Update accept-language-parser module to 1.2.0

Module version 1.2.0 fixes issue:
opentable/accept-language-parser#8

This commit updates review comments:
#7545 (diff)
#7545 (comment)

* Original sha: da669da
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-30T17:49:23Z

**Commit 30:**
Add i18n default locale as a configurable item

Adds 'defaultLocale' configurable item to the i18n plugin configuration.
The default locale is used for translations if the locale specified by user
is not supported.

This commit satisfies the review comment:
- #7545 (diff)

* Original sha: 9064e8a
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-31T10:42:52Z
@epixa epixa added v5.4.0 and removed v5.2.0 labels Mar 2, 2017
spalger pushed a commit that referenced this pull request Mar 2, 2017
* Kibana Globalization - Phase 1

Backports PR #7545

**Commit 1:**
Add low level i18n plugin

Manages languages that are available and is responsible for loading translated
content at the granularity of a plugin.

To be done:
 - APIs for store and retrieval

* Original sha: aca671f
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-20T17:12:25Z

**Commit 2:**
Use Kibana install as root for the translation store directory

Setting the path for storing the bundled language translation files to
<KIBANA_INSTALL>/data/store_translations/<PLUGIN_NAME>

* Original sha: c3ba578
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-21T13:36:59Z

**Commit 3:**
Updated i18n core plugin APIs to be asynchronous

To be done:
 - Better error handling in APIs
 - Fix threading issue with storePluginLanguageTranslations API

* Original sha: 80d8a2c
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-22T17:25:30Z

**Commit 4:**
Fix thread synchroization issue in storePluginLanguageTranslations

* Original sha: 856fb02
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-24T10:43:06Z

**Commit 5:**
Update error handling in i18n core plugin

* Original sha: c5d22be
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-24T14:30:27Z

**Commit 6:**
Change to use NodeJS mkdirp function for creating directories recursively

Updates with review comments from @srl295. Changed export syntax to show the
exported functions at end of file.

* Original sha: b37e4f8
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-27T11:04:01Z

**Commit 7:**
Add REST API for getting translations of a language for a plugin

To be done:
 - Add algorithm to decide on the language for a plugin by comparing the accept languages
from the REST call and the plugin supported languages
 - Add REST API tests

* Original sha: ec6d2b1
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-27T22:42:15Z

**Commit 8:**
Add algorithm for determining plugin language when retrieving translations

Client would pass languages used in the 'accept-language' header. These
languages would then be compared against the plugin supported languages
and best compared language would be selected.

To be done:
 - Add REST API tests

* Original sha: a75faae
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-28T21:10:39Z

**Commit 9:**
Add API to return all registered plugin language translations

* Original sha: 66d051b
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-29T17:02:34Z

**Commit 10:**
Add HAPI API to get all plugins translation files

* Original sha: c3403e7
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-06-30T16:08:28Z

**Commit 11:**
Update register translations API to be independent of plugin name and language

The register API is updated to be independent of plugin name and language. The API will now
traverse the path given and create language bundles as per language files it traverses.
The translations files structure has also been simplified to be just key/value objects.

To be done:
 - Add hapi API to get translations
 - Extend the API tests to test responses

* Original sha: 0f4902d
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-01T16:09:33Z

**Commit 12:**
Update API test

* Original sha: 9785df2
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-05T17:52:25Z

**Commit 13:**
Add eslint fix for API test

* Original sha: d41fcec
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-06T10:00:22Z

**Commit 14:**
Update with review comments

From review #7545 (comment)
following comments updated:
- README, .gitignore, and .eslintrc are not needed in a core plugin
- package.json only needs name and version
- unit tests need to go in a tests directory otherwise they won't get picked up
by the grunt tasks. Also our convention is to name the test file with the same
name as the module it's testing (so i18n_tests.js should just be i18n.js)
- For consistency with the rest of the code base, rename the data directory to fixtures.
- Prefer const (or let if necessary). Don't use var.
- Use ES6 imports/exports rather than commonjs style
- Only export the i18n module's public API. For instance, I don't think getPluginTranslationDetails is used outside of the i18n module, so it shouldn't be exposed publicly. If you want to expose it for testing purposes, I would recommend creating an i18n directory with an index.js file that exports the module's public API, and a separate i18n.js file with the "private" API. index.js will be for public use, i18n.js will be for private internal use.

* Original sha: c8b2197
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-11T11:23:37Z

**Commit 15:**
Update after review comments

From review (#7545 (comment)):
- i18n module API should return promises for async operations instead of using
callbacks
- All filesystem access should be async
- Unit tests need to be updated based on new proposed plugin structure
(single language file, not split by view)

From design (#6515 (comment)):
- Removed API as will consider in later phase

TODO:
- Make write function async

* Original sha: e17653d
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-15T18:24:21Z

**Commit 16:**
Update after review comments

Updated write function to be asynchronous

* Original sha: eaf35ab
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-19T11:03:45Z

**Commit 17:**
Update registerTranslations API to take absolute translation file as argument

The API originally took the directory as the argument but following reviews it
was decided to change to absolute file because it will be less brittle
since it is more explicit.

* Original sha: f1974ca
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-21T17:43:27Z

**Commit 18:**
Translate the Kibana welcome message

Translates the start-up message (“Kibana is loading ...”)in the Jade template.

To be done:
 - Means to register the core plugin translations. They are currently added
in the fixtures directory as static files. Need to be generated on the fly.

* Original sha: 10db458
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-25T13:58:54Z

**Commit 19:**
Add build task to generate core plugin translations

Task which calls registerTranslations API and then a task which copies the
regsitered translations to <kibana_root>/build/kibana

* Original sha: 4957173
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-27T10:00:35Z

**Commit 20:**
Add hook to optimize module to add registration during dev startup

Registration of the core plugin translations during development start of
Kibana server. The translations include the welcome message and server error
startup message.

* Original sha: 5f61475
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-28T14:18:19Z

**Commit 21:**
Handle scenario when the user preferred language header is not passed

The UI when loading asks i18n plugin which language translation to use
depending on the user preferred language header 'accept-language'.
This commit is to handle scenario where header is not passed. The algorithm
then chooses the default language.

* Original sha: 9fbe6d5
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-07-29T10:51:33Z

**Commit 22:**
Replace registering of translations at plugin install time to the plugin init phase

This change follows review comments in:
#6515 (comment)

* Original sha: 32d5034
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-08T14:39:34Z

**Commit 23:**
Update after review comments

Comments:
- #7545 (diff)
- #7545 (diff)

* Original sha: 8c7f51c
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-09T09:36:04Z

**Commit 24:**
Update after plugin folder layout changes in Kibana

This require to use <kibana_root>/data for registered translations
and i18n plugin moved to core_plugins from plugins.

Refer to PR for more details:
#7562

* Original sha: b84f40a
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-09T11:09:18Z

**Commit 25:**
Update translation registration to file path rather than bundling

After review discussions it was agreed to just register the absolute paths
to translation files rather than bundling each file into one central file
at registration.

* Original sha: 2dd3065
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-16T09:28:38Z

**Commit 26:**
Update review comments

This commit contains the following review comments:
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)
- #7545 (diff)

* Original sha: db11a2d
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-18T14:09:22Z

**Commit 27:**
Update review comments

The following review comments are included in the commit:
- #7545 (comment)
- #7545 (comment)
- #7545 (comment)

* Original sha: 8c39a8d
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-19T12:44:39Z

**Commit 28:**
Expose the i18n APIs in the server object for plugin access

Plugins should call the i18n plugin APIs through the server object
and not directly from the module.

This closes he following comments:
- #7545 (comment)
- #7545 (comment)
- #7545 (comment)

* Original sha: a9e30d3
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-19T17:31:10Z

**Commit 29:**
Update accept-language-parser module to 1.2.0

Module version 1.2.0 fixes issue:
opentable/accept-language-parser#8

This commit updates review comments:
#7545 (diff)
#7545 (comment)

* Original sha: da669da
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-30T17:49:23Z

**Commit 30:**
Add i18n default locale as a configurable item

Adds 'defaultLocale' configurable item to the i18n plugin configuration.
The default locale is used for translations if the locale specified by user
is not supported.

This commit satisfies the review comment:
- #7545 (diff)

* Original sha: 9064e8a
* Authored by Martin Hickey <martin.hickey@ie.ibm.com> on 2016-08-31T10:42:52Z

* [eslint] autofix lint errors
@lukasolson lukasolson mentioned this pull request Apr 18, 2017
@alvarezdaniel
Copy link

Great!

weishionshi pushed a commit to weishionshi/kibana that referenced this pull request May 15, 2017
* Add low level i18n plugin

Manages languages that are available and is responsible for loading translated
content at the granularity of a plugin.

To be done:
 - APIs for store and retrieval

* Use Kibana install as root for the translation store directory

Setting the path for storing the bundled language translation files to
<KIBANA_INSTALL>/data/store_translations/<PLUGIN_NAME>

* Updated i18n core plugin APIs to be asynchronous

To be done:
 - Better error handling in APIs
 - Fix threading issue with storePluginLanguageTranslations API

* Fix thread synchroization issue in storePluginLanguageTranslations

* Update error handling in i18n core plugin

* Change to use NodeJS mkdirp function for creating directories recursively

Updates with review comments from @srl295. Changed export syntax to show the
exported functions at end of file.

* Add REST API for getting translations of a language for a plugin

To be done:
 - Add algorithm to decide on the language for a plugin by comparing the accept languages
from the REST call and the plugin supported languages
 - Add REST API tests

* Add algorithm for determining plugin language when retrieving translations

Client would pass languages used in the 'accept-language' header. These
languages would then be compared against the plugin supported languages
and best compared language would be selected.

To be done:
 - Add REST API tests

* Add API to return all registered plugin language translations

* Add HAPI API to get all plugins translation files

* Update register translations API to be independent of plugin name and language

The register API is updated to be independent of plugin name and language. The API will now
traverse the path given and create language bundles as per language files it traverses.
The translations files structure has also been simplified to be just key/value objects.

To be done:
 - Add hapi API to get translations
 - Extend the API tests to test responses

* Update API test

* Add eslint fix for API test

* Update with review comments

From review elastic#7545 (comment)
following comments updated:
- README, .gitignore, and .eslintrc are not needed in a core plugin
- package.json only needs name and version
- unit tests need to go in a tests directory otherwise they won't get picked up
by the grunt tasks. Also our convention is to name the test file with the same
name as the module it's testing (so i18n_tests.js should just be i18n.js)
- For consistency with the rest of the code base, rename the data directory to fixtures.
- Prefer const (or let if necessary). Don't use var.
- Use ES6 imports/exports rather than commonjs style
- Only export the i18n module's public API. For instance, I don't think getPluginTranslationDetails is used outside of the i18n module, so it shouldn't be exposed publicly. If you want to expose it for testing purposes, I would recommend creating an i18n directory with an index.js file that exports the module's public API, and a separate i18n.js file with the "private" API. index.js will be for public use, i18n.js will be for private internal use.

* Update after review comments

From review (elastic#7545 (comment)):
- i18n module API should return promises for async operations instead of using
callbacks
- All filesystem access should be async
- Unit tests need to be updated based on new proposed plugin structure
(single language file, not split by view)

From design (elastic#6515 (comment)):
- Removed API as will consider in later phase

TODO:
- Make write function async

* Update after review comments

Updated write function to be asynchronous

* Update registerTranslations API to take absolute translation file as argument

The API originally took the directory as the argument but following reviews it
was decided to change to absolute file because it will be less brittle
since it is more explicit.

* Translate the Kibana welcome message

Translates the start-up message (“Kibana is loading ...”)in the Jade template.

To be done:
 - Means to register the core plugin translations. They are currently added
in the fixtures directory as static files. Need to be generated on the fly.

* Add build task to generate core plugin translations

Task which calls registerTranslations API and then a task which copies the
regsitered translations to <kibana_root>/build/kibana

* Add hook to optimize module to add registration during dev startup

Registration of the core plugin translations during development start of
Kibana server. The translations include the welcome message and server error
startup message.

* Handle scenario when the user preferred language header is not passed

The UI when loading asks i18n plugin which language translation to use
depending on the user preferred language header 'accept-language'.
This commit is to handle scenario where header is not passed. The algorithm
then chooses the default language.

* Replace registering of translations at plugin install time to the plugin init phase

This change follows review comments in:
elastic#6515 (comment)

* Update after review comments

Comments:
- elastic#7545 (diff)
- elastic#7545 (diff)

* Update after plugin folder layout changes in Kibana

This require to use <kibana_root>/data for registered translations
and i18n plugin moved to core_plugins from plugins.

Refer to PR for more details:
elastic#7562

* Update translation registration to file path rather than bundling

After review discussions it was agreed to just register the absolute paths
to translation files rather than bundling each file into one central file
at registration.

* Update review comments

This commit contains the following review comments:
- elastic#7545 (diff)
- elastic#7545 (diff)
- elastic#7545 (diff)
- elastic#7545 (diff)
- elastic#7545 (diff)
- elastic#7545 (diff)
- elastic#7545 (diff)
- elastic#7545 (diff)
- elastic#7545 (diff)

* Update review comments

The following review comments are included in the commit:
- elastic#7545 (comment)
- elastic#7545 (comment)
- elastic#7545 (comment)

* Expose the i18n APIs in the server object for plugin access

Plugins should call the i18n plugin APIs through the server object
and not directly from the module.

This closes he following comments:
- elastic#7545 (comment)
- elastic#7545 (comment)
- elastic#7545 (comment)

* Update accept-language-parser module to 1.2.0

Module version 1.2.0 fixes issue:
opentable/accept-language-parser#8

This commit updates review comments:
elastic#7545 (diff)
elastic#7545 (comment)

* Add i18n default locale as a configurable item

Adds 'defaultLocale' configurable item to the i18n plugin configuration.
The default locale is used for translations if the locale specified by user
is not supported.

This commit satisfies the review comment:
- elastic#7545 (diff)

* Move UI i18n wrapper functionality into a module

This commit better structures the i18n capability so that it can be called
in UI code in a clearly defined fashion with minimum code. It also fixes
potential race conditions.

This commit updates review comments:
- elastic#7545 (diff)
- elastic#7545 (diff)
- elastic#7545 (diff)

* Fill any missing translations using translations from default locale

The default language translations are loaded and are compared against the selected
language translations. The comparison can then highlight any missing translation
keys and can load the default translations keys as needed. This helps to unsure
where possible that a translation string is available in most scenarios even if not
in the locale requested.

This commit resolves review comments:
- elastic#7545 (comment)
- elastic#7545 (comment)

* Add unit tests for the i18n UI wrapper functions

* Fix issues after rebase with master

* Add translation keys verification tool

This tool helps to check that translation keys are translated. This tool can be
used for non-angular translation constructs like the Jade templates.

* Updates after review comments

Updates for review comments:
elastic#7545 (review)

* Update after review comments

Update for review comments:
elastic#7545 (review)

To be done:
- Update of unit tests for UI and server
- Call of verify translations

* Update unit tests after review changes

There was a number of changes to the i18n module and the ui i18n wrapper
following review comments. This commit is to update the unit tests with
respect.

* Add build task for verify translations

* Update the kibana i18n IDs to be prefixed with kibana

* Update verify translations to test registered translations

It was testing the static translation files. It is now updated to
test the translations registered when Kibana server is started and
the plugins have initialized.

* Update after review comments

Updates following review comments:
elastic#7545 (review)

* Update after review

This commit contain updates after the following review:
elastic#7545 (review)

* Updates after review

Updates for review comments:
elastic#7545 (review)

* Update after review

Updates for the following review comments:
elastic#7545 (review)

* Update after review

Updates after the following review comments:
elastic#7545 (review)

* Update unit tests to use expect throwError

* Update after rebase with master

Loading message changed following merge of commit
elastic@26c53e8#diff-e25d7fee746a4f249e17f87c02fd95f8R55
This required update to the welcome message and how it is called.

* Update following review

Updated the following review comments:
elastic#7545 (review)

* Update the algorithm to return the locale

The algorithm to return which locale to use for translations based on the user
locale list and the regsitered locales is updated in this commit. The algorithm
previously did an exact match on all the user locales first before (by priority)
then checking for best case match. The algorithm is now modified to check each
user locale starting with the highest priority first for an exact match and then
for best case match. If no match it then moves to the next user locale with
the next highest priority. This is to follow the priority list that a user
browser is configured for where there maybe a locale translation available
but might not be the exact match with regard to the locale code and/or script.
An example of this is that the highest priority locale of the user is 'en-US'
but the locale translation available is 'en'. It is better select the 'en'
locale rather than select the next highest locale which is an exact match.

* Update after review comments

Updates after the following reviews:
elastic#7545 (review)
elastic#7545 (review)

* Fix after merge with master

Change in the flo and layout of ui index meant that acceptLanguages were not
being passed. This commit is an update to fix this so that the welcome
messages are loaded.

* Update after review comments

This commit is for updates after the following review:
elastic#7545 (review)

* Fix issue when unit test run in CI as core translations are registered

When unit tests are run on a test server (like in the CI), it will start
Kibana server and register the core translations. This means that the i18n
unit tests need to be able to store the existing registration prior to
testing and replace after testing.

* [server/ui] move i18n into ui module

* [server/ui] restore renderApp() method signature

* [server/ui] unify i18n logic in UiI18n class

* [server] move translation files into "translations" dir

* Update i18n module to loaded by multiple server instances within the one process

* Update i18n module to a class

Moving the i18n module into a class so as to encapsulate the registered
translations which means there can be different and distinct instances per process.
This is to accomodate the user case where there might be multiple Kibana server
instances in a process and the localization should be at the server level.

* Identify private members in a class with underscore-prefix convention

* Remove redundant translation from core translation file

Message starting with 'Give me a moment...' is no longer part of loading
message folowing a rebase with master.

* [ui/i18n] reject translations files that do not use absolute paths

* Update config item locale to defaultLocale

* Update after review comments

- Update after following review: elastic#7545 (review)
- Also, fix syntax mess following rebase with master of src/optimize/index.js

* Fix rebase with master error

* Add task for verifying translations in CI

* Fix lint errors

# Conflicts:
#	config/kibana.yml
#	src/ui/views/ui_app.jade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants