Skip to content
Snippets Groups Projects
Unverified Commit b0ff7087 authored by adam-james's avatar adam-james Committed by GitHub
Browse files

Handle Plural Translation and Path String Bugs for i18n (#21116)

The i18n in Metabase uses a service 'POEditor', which requires:

1. all strings on the frontend and backend that need to have translations must be collected into a .pot file
2. The .pot file is uploaded and people contribute translations for each phrase, per locale. These eventually get
downloaded as locale.po (eg. German is 'de.po')
3. Frontend and backend each take responsibility for generating required locale resources, which are then used to
render the correct translations for the user's locale in the app.

This process works just fine, but occasionally some bugs are found. These particular changes in this PR were found
because of issue #15660 where a user reports that some of the 'binning options' are translated while others are not.

Those particular strings originate form the backend via the API at `src/metabase/api/table.clj`, but the issue arises
because the backend's locale resource files do not contain the entries for those translations. The mechanism is
working just fine, it's the resource building process that has some issues.

So, we can turn to the i18n build process, which is all found in `bin/i18n`, and is a mix of shell scripts and
clojure.

The script `update-translation-template` starts the process.
There are a few steps in the process:

1. generate metabase-frontend.pot -> uses Babel, I'm assuming this works for the purpose of this PR

2. generate metabase-backend.pot  -> works fine. We can see in `locales/metabase-backend.pot`:

   #: src/metabase/api/table.clj
   msgid "Week"
   msgstr ""

3. Join these files together with a tool 'msgcat' into metabase.pot
   - here is where we begin to see an issue.
   - there is NO entry that matches the example from 2. This is because msgcat has combined translation strings that
   match on the frontend into a single entry:

     #: frontend/src/metabase-lib/lib/Dimension.ts:1677
     #: frontend/src/metabase/lib/query_time.js:203
     #: frontend/src/metabase/parameters/components/widgets/DateRelativeWidget.jsx:25
     #: frontend/src/metabase/parameters/components/widgets/DateRelativeWidget.jsx:30
     #: src/metabase/api/table.clj
     msgid "Week"
     msgstr ""

   - and, more interestingly:

   #: frontend/src/metabase-lib/lib/Dimension.ts:1689
   #: frontend/src/metabase/lib/query_time.js:207 src/metabase/api/table.clj
   msgid "Quarter"
   msgstr ""

   - There are a bunch of #: lines, the last being the clj file, which means that "Week" is used both in the frontend
   and backend. So, the translated string can be used in both situations. The backend does handle the case of multiple
   #: paths, but does not handle the second situation, where there are actually 2 paths (space separated) in a single
   #: line.

4. in the file `bin/i18n/src/create_artifacts/backend.clj` we create .edn files for each locale.
   - this is a map where the keys are the English strings and values are the translations from the locale.po file.
   - our logic in this file incorrectly assumes only a single path per #: line in the .pot file exists, and this is
   one place where things break. The 'backend-message?' predicate will fail on the second example in 3. so we don't
   ever translate that string.
   - Simultaneously, `i18n/common.clj` allows pluralized entries. In the case of "Week", we got "Week" and "Weeks"
   translated from POEditor. When a message is plural, the contents show up in a different location. Our backend code
   ignores plural, but this ends up throwing away our valid translation as well. When a message is plural, all
   translations show up in :str-plural as a seq, where the first entry is the singular translation. But, we (remove
   :pluarl?) so get nothing. This is why the binning options aren't translated. As a point of completeness: if a
   message has no plural, we correctly get the translated message from the :str key.

The broken logic:
- common.clj - po-messages-seq creates a seq of messages, where a message looks like:
{:id "Week",
  :id-plural "Weeks",
  :str nil,
  :str-plural ("Woche" "Wochen"),
  :fuzzy? false,
  :plural? true,
  :source-references
  ("frontend/src/metabase/lib/query_time.js:203"
   "frontend/src/metabase/parameters/components/widgets/DateRelativeWidget.jsx:25"
   "frontend/src/metabase/parameters/components/widgets/DateRelativeWidget.jsx:30"
   "src/metabase/api/table.clj target/classes/metabase/api/table.clj"),
  :comment nil}

or
{:id "Day of Week",
  :id-plural nil,
  :str "Wochentag",
  :str-plural nil,
  :fuzzy? false,
  :plural? false,
  :source-references ("src/metabase/api/table.clj target/classes/metabase/api/table.clj"),
  :comment nil}

Example 1 fails because the message is pluralized. :str is nil, but (first (:str-plural message)) has the correct
translation. We are looking in the wrong spot in this case. Compare to Example 2 which does work.

Notice a subtle thing that was actually wrong in examples 1 and 2 but worked by accident: some :source-references
strings contain 2 paths. We see this failing here:

{:id "Quarter",
 :id-plural "Quarters",
 :str nil,
 :str-plural ("Quartal" "Quartale"),
 :fuzzy? false,
 :plural? true,
 :source-references
 ("frontend/src/metabase/lib/query_time.js:207 src/metabase/api/table.clj" "target/classes/metabase/api/table.clj"),
 :comment nil}

We can solve this different problem by using str/split in the backend-message? predicate.
parent 3a400e4e
No related branches found
No related tags found
No related merge requests found
......@@ -12,12 +12,21 @@
(fn [dir]
(str/starts-with? path dir))
["src" "backend" "enterprise/backend" "shared"]))
source-references))
;; sometimes 2 paths exist in a single string, space separated
;; if a backend path is second, it is missed if we don't str/split
(mapcat #(str/split % #" ") source-references)))
(defn- plural->singular [{:keys [str-plural] :as message}]
(merge message
(when str-plural {:str (first str-plural)
:id-plural nil
:str-plural nil
:plural? false})))
(defn- ->edn [{:keys [messages]}]
(eduction
(filter backend-message?)
(remove :plural?)
(map plural->singular)
i18n/print-message-count-xform
messages))
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment