Repository: kiwicom/mobile
Pull request #1387 (2018-12-17 16:00:24)

Fix: translation strings compatible with native code implementation

mrtnzlml: The main reason why it's not very readable is the way how it's written. Look at this modified test:

```js
it('handles natively translated strings -- with several parameters', () => {
   const translationId = 'hotels.gallery.pagination';
   const translatedString = '%1$@ of %2$@';
   expect(getTranslation(translatedString, translationId)).toBe(
     '__1_photoNumber__ of __2_totalPhotos__',
   );
});
```

So the question must be asked: where is the `photoNumber` and `totalPhotos` comming from? As you can see it's pretty magic so you have to dig deep in order to uderstand it. Better way how to implement it would be to accept the vocabulary as an argument which means larger refactoring. The tests would be very obvious though after this change.
RobinCsl: I tried to implement your suggestion and I believe it is clearer now. Could you review again?

Patch:
diff --git a/packages/localization/src/TranslationHelpers.js b/packages/localization/src/TranslationHelpers.js
index b88b13bb..57ae977c 100644
--- a/packages/localization/src/TranslationHelpers.js
+++ b/packages/localization/src/TranslationHelpers.js
@@ -1,16 +1,21 @@
 //@flow
 
-import DefaultVocabulary, { type TranslationKeys } from './DefaultVocabulary';
+import DefaultVocabulary, {
+  type TranslationKeys,
+  type TranslationKeysObject,
+} from './DefaultVocabulary';
 
 export const getTranslation = (
   translatedString: ?string,
   translationId: TranslationKeys,
+  vocabulary: TranslationKeysObject = DefaultVocabulary,
 ): string => {
   const nativeKey = `mobile.${translationId}`;
+  const defaultTranslation = vocabulary[translationId];
 
   if (translatedString === nativeKey) {
     // fallback to our dummy vocabulary because native code didn't provide translation
-    return DefaultVocabulary[translationId];
+    return defaultTranslation;
   }
   if (translatedString == null) {
     // Everything failed - native translation even our fallback. This usually
@@ -18,7 +23,36 @@ export const getTranslation = (
     // we can actually do is to use original key as a text.
     return translationId;
   }
-  return translatedString;
+
+  // Translated strings from native come under the form
+  // %1$@, so we lose information about the variables.
+  // We add this information back in:
+  try {
+    const NATIVE_TRANSLATION_PARAM_REGEX = /%([0-9]+)\$@/g;
+    const compatibleTranslatedString = translatedString.replace(
+      NATIVE_TRANSLATION_PARAM_REGEX,
+      (match, variableIndex) => {
+        const ORIGINAL_PARAM_REGEX = new RegExp(
+          `__(${variableIndex}_)?([a-zA-Z-_]+)__`,
+          'g',
+        );
+        const originalParamMatches = defaultTranslation.match(
+          ORIGINAL_PARAM_REGEX,
+        );
+
+        if (originalParamMatches == null || originalParamMatches.length !== 1) {
+          throw new Error(
+            `There was a problem with translationId ${translationId}`,
+          );
+        }
+
+        return originalParamMatches[0];
+      },
+    );
+    return compatibleTranslatedString;
+  } catch (e) {
+    return translatedString;
+  }
 };
 
 export function replaceValues(text: string, values?: Object): string {


Test patch:
diff --git a/packages/localization/src/__tests__/TranslationHelpers.test.js b/packages/localization/src/__tests__/TranslationHelpers.test.js
new file mode 100644
index 00000000..3d6ce67a
--- /dev/null
+++ b/packages/localization/src/__tests__/TranslationHelpers.test.js
@@ -0,0 +1,51 @@
+// @flow strict
+
+import { getTranslation } from '../TranslationHelpers';
+
+const vocabulary = {
+  'single_hotel.hotel_review.reviews': '__count__ Reviews',
+  'hotels.gallery.pagination': '__1_photoNumber__ of __2_totalPhotos__',
+};
+
+describe('getTranslation:', () => {
+  it('returns default translation if string is not translated', () => {
+    const translationId = 'single_hotel.hotel_review.reviews';
+    const notTranslatedKey = 'mobile.single_hotel.hotel_review.reviews';
+    expect(getTranslation(notTranslatedKey, translationId, vocabulary)).toBe(
+      '__count__ Reviews',
+    );
+  });
+
+  it('returns translation id if translated string is null/undefined', () => {
+    const translationId = 'single_hotel.hotel_review.reviews';
+    const notTranslatedKey = null;
+    expect(getTranslation(notTranslatedKey, translationId, vocabulary)).toBe(
+      translationId,
+    );
+  });
+
+  it('handles natively translated strings -- with one parameter', () => {
+    const translationId = 'single_hotel.hotel_review.reviews';
+    const translatedString = '%1$@ Reviews';
+    expect(getTranslation(translatedString, translationId, vocabulary)).toBe(
+      '__count__ Reviews',
+    );
+  });
+
+  it('handles natively translated strings -- with several parameters', () => {
+    const translationId = 'hotels.gallery.pagination';
+    const translatedString = '%1$@ of %2$@';
+    expect(getTranslation(translatedString, translationId, vocabulary)).toBe(
+      '__1_photoNumber__ of __2_totalPhotos__',
+    );
+  });
+
+  it('returns translated string if parameter matching is not possible', () => {
+    const translationId = 'hotels.gallery.pagination';
+    const translatedString = '%1$@ of %2$@';
+    const mistake = ' and MISTAKE %3$@';
+    expect(
+      getTranslation(translatedString + mistake, translationId, vocabulary),
+    ).toBe(translatedString + mistake);
+  });
+});