fix: properly render strings using icu4j

cyyynthia · cyyynthia · commit 6a361e060344 · 2024-11-23T14:15:18.000+01:00
diff --git a/backend/api/build.gradle b/backend/api/build.gradle
@@ -27,6 +27,7 @@ apply plugin: 'io.spring.dependency-management'
 
 repositories {
     mavenCentral()
+    maven { url 'https://jitpack.io' }
 }
 
 kotlin {
diff --git a/backend/app/build.gradle b/backend/app/build.gradle
@@ -46,6 +46,7 @@ apply plugin: "org.gradle.test-retry"
 
 repositories {
     mavenCentral()
+    maven { url 'https://jitpack.io' }
 }
 
 dependencyManagement {
diff --git a/backend/data/build.gradle b/backend/data/build.gradle
@@ -52,6 +52,7 @@ apply plugin: 'org.hibernate.orm'
 
 repositories {
     mavenCentral()
+    maven { url 'https://jitpack.io' }
 }
 
 idea {
@@ -99,6 +100,9 @@ dependencies {
     implementation "org.springframework.boot:spring-boot-starter-batch"
     implementation "org.springframework.boot:spring-boot-starter-websocket"
     implementation "org.springframework.boot:spring-boot-starter-thymeleaf"
+    implementation "org.springframework.boot:spring-boot-starter-thymeleaf"
+
+    implementation 'com.github.transferwise:spring-icu:0.3.0'
 
     /**
      * DB
diff --git a/backend/data/src/main/kotlin/io/tolgee/email/EmailTemplateConfig.kt b/backend/data/src/main/kotlin/io/tolgee/email/EmailTemplateConfig.kt
@@ -16,11 +16,12 @@
 
 package io.tolgee.email
 
+import com.transferwise.icu.ICUMessageSource
+import com.transferwise.icu.ICUReloadableResourceBundleMessageSource
 import org.springframework.beans.factory.annotation.Qualifier
 import org.springframework.context.MessageSource
 import org.springframework.context.annotation.Bean
 import org.springframework.context.annotation.Configuration
-import org.springframework.context.support.ResourceBundleMessageSource
 import org.thymeleaf.TemplateEngine
 import org.thymeleaf.spring6.SpringTemplateEngine
 import org.thymeleaf.templateresolver.ClassLoaderTemplateResolver
@@ -39,13 +40,11 @@ class EmailTemplateConfig {
   }
 
   @Bean("emailMessageSource")
-  fun messageSource(): MessageSource {
-    val messageSource = ResourceBundleMessageSource()
-    messageSource.setBasename("email-i18n.messages")
+  fun messageSource(): ICUMessageSource {
+    val messageSource = ICUReloadableResourceBundleMessageSource()
+    messageSource.setBasenames("email-i18n/messages", "email-i18n-test/messages")
     messageSource.setDefaultEncoding("UTF-8")
     messageSource.setDefaultLocale(Locale.ENGLISH)
-    println(messageSource.getMessage("powered-by", null, Locale.ENGLISH))
-    println(messageSource.getMessage("powered-by", null, Locale.FRENCH))
     return messageSource
   }
 
@@ -55,6 +54,7 @@ class EmailTemplateConfig {
     @Qualifier("emailMessageSource") messageSource: MessageSource,
   ): TemplateEngine {
     val templateEngine = SpringTemplateEngine()
+    templateEngine.enableSpringELCompiler = true
     templateEngine.templateResolvers = setOf(templateResolver)
     templateEngine.setTemplateEngineMessageSource(messageSource)
     return templateEngine
diff --git a/backend/data/src/main/resources/email-templates b/backend/data/src/main/resources/email-templates
@@ -1 +1 @@
-../../../../../email/out
+../../../../../email/dist/email-templates
diff --git a/backend/data/src/test/kotlin/io/tolgee/email/EmailServiceTest.kt b/backend/data/src/test/kotlin/io/tolgee/email/EmailServiceTest.kt
@@ -76,9 +76,15 @@ class EmailServiceTest {
       .let { it.getBodyPart(0).content as MimeMultipart }
       .let { it.getBodyPart(0).content as String }
       .assert
-      .contains("Value of `testVar` : <span>test!!</span>")
+      .contains("Testing ICU strings -- test!!")
+      .contains("Value of `testVar`: <span>test!!</span>")
       .contains("<span>Was `testVar` equal to &quot;meow&quot; : </span><span>no</span>")
-      .contains("Powered by")
+      // Makes sure tag handling works as expected
+      .contains(
+        "Powered by <a href=\"https://tolgee.io\" style=\"color:inherit;text-decoration-line:underline\" " +
+          "target=\"_blank\">Tolgee</a>",
+      )
+      // Might be a bit brittle but does the trick for now.
       .doesNotContain(" th:")
       .doesNotContain(" data-th")
   }
@@ -95,10 +101,31 @@ class EmailServiceTest {
       .let { it.getBodyPart(0).content as MimeMultipart }
       .let { it.getBodyPart(0).content as String }
       .assert
-      .contains("Value of `testVar` : <span>meow</span>")
+      .contains("Testing ICU strings -- meow")
+      .contains("Value of `testVar`: <span>meow</span>")
       .contains("<span>Was `testVar` equal to &quot;meow&quot; : </span><span>yes</span>")
   }
 
+  @Test
+  fun `it correctly processes foreach blocks`() {
+    // FWIW this is very close to just testing Thymeleaf itself, but it serves as a sanity check for the template itself
+    emailService.sendEmailTemplate("test@tolgee.internal", "zz-test-email", Locale.ENGLISH, TEST_PROPERTIES_MEOW)
+    verify(mailSender).send(emailCaptor.capture())
+
+    val email = emailCaptor.value
+    email.content
+      .let { it as MimeMultipart }
+      .let { it.getBodyPart(0).content as MimeMultipart }
+      .let { it.getBodyPart(0).content as String }
+      .assert
+      .contains("Plain test: <span>Name #1</span>")
+      .contains("<span>ICU test: Name #1</span>")
+      .contains("Plain test: <span>Name #2</span>")
+      .contains("<span>ICU test: Name #2</span>")
+      .contains("Plain test: <span>Name #3</span>")
+      .contains("<span>ICU test: Name #3</span>")
+  }
+
   companion object {
     private val TEST_PROPERTIES =
       mapOf(
diff --git a/backend/data/src/test/resources/email-i18n-test/messages_en.properties b/backend/data/src/test/resources/email-i18n-test/messages_en.properties
@@ -0,0 +1,2 @@
+email-test-string = Testing ICU strings -- {testVar}
+email-test-it = ICU test: {item__name}
diff --git a/backend/development/build.gradle b/backend/development/build.gradle
@@ -37,6 +37,7 @@ apply plugin: "kotlin-kapt"
 
 repositories {
     mavenCentral()
+    maven { url 'https://jitpack.io' }
 }
 
 kotlin {
diff --git a/backend/security/build.gradle b/backend/security/build.gradle
@@ -23,6 +23,7 @@ apply plugin: 'io.spring.dependency-management'
 
 repositories {
     mavenCentral()
+    maven { url 'https://jitpack.io' }
 }
 
 kotlin {
diff --git a/backend/testing/build.gradle b/backend/testing/build.gradle
@@ -43,6 +43,7 @@ apply plugin: "kotlin-kapt"
 
 repositories {
     mavenCentral()
+    maven { url 'https://jitpack.io' }
 }
 
 kotlin {
diff --git a/ee/backend/app/build.gradle b/ee/backend/app/build.gradle
@@ -33,6 +33,7 @@ allOpen {
 
 repositories {
     mavenCentral()
+    maven { url 'https://jitpack.io' }
 }
 
 test {
diff --git a/ee/backend/tests/build.gradle b/ee/backend/tests/build.gradle
@@ -30,6 +30,7 @@ allOpen {
 
 repositories {
     mavenCentral()
+    maven { url 'https://jitpack.io' }
 }
 
 test {
diff --git a/email/.config/extractor.ts b/email/.config/extractor.ts
@@ -14,10 +14,22 @@
  * limitations under the License.
  */
 
-import type { ExtractedKey, ExtractionResult, Warning } from "@tolgee/cli/extractor";
-import type { CallExpression, Expression, JSXAttribute, JSXAttrValue, JSXOpeningElement, Node, Span } from "@swc/types";
-import { walk } from "estree-walker";
-import { parse } from "@swc/core";
+import type {
+  ExtractedKey,
+  ExtractionResult,
+  Warning,
+} from '@tolgee/cli/extractor';
+import type {
+  CallExpression,
+  Expression,
+  JSXAttribute,
+  JSXAttrValue,
+  JSXOpeningElement,
+  Node,
+  Span,
+} from '@swc/types';
+import { walk } from 'estree-walker';
+import { parse } from '@swc/core';
 
 type AnyExpr = Expression | JSXAttrValue;
 
diff --git a/email/.gitignore b/email/.gitignore
@@ -1,3 +1,3 @@
 .react-email
 node_modules
-out
+dist
diff --git a/email/HACKING.md b/email/HACKING.md
@@ -73,14 +73,20 @@ Shared parts are found in `components/parts`.
 Most if not all text in emails are expected to be wrapped in `<LocalizedText />` (or `t()` when more appropriate).
 They are equivalent as `<LocalizedText />` is simply a JSX wrapper for calling `t()`.
 
+The strings are written using a format similar to the familiar Tolgee ICU, via [ICU4J](https://github.com/unicode-org/icu/tree/main/icu4j)
+(see [MessageFormat](https://unicode-org.github.io/icu-docs/apidoc/released/icu4j/com/ibm/icu/text/MessageFormat.html)).
+
+ICU arguments are pulled from the template variables. To access a dotted key, such as `item.name` use a double
+underscore: `item__name`.
+
 The `<LocalizedText />` takes the following properties:
 - `keyName` (required): String key name
 - `defaultValue` (optional): Default string to use
   - Will be used by the CLI to push default values when pushing new keys, and when previewing
-- `demoProps` (required*): Demo properties to use when rendering the string
+- `demoProps` (required¹): Demo properties to use when rendering the string
   - When previewing, the ICU string will be rendered using these values, so it is representative of a "real" email
   - If demo props are not specified, the preview will fail to render
-  - \*It can be unset if there are no props in the string
+  - ¹It can be unset if there are no props in the string
 
 The `t()` function takes the same properties, instead it takes them as arguments in the order they're described here.
 
@@ -142,43 +148,6 @@ This component receives the following properties:
 - `each` (required): The [Thymeleaf iterator expression](https://www.thymeleaf.org/doc/tutorials/3.1/usingthymeleaf.html#using-theach)
 - `demoIterations` (required): An array of elements used for the preview
 
-#### Note on available variables
-Within the for inner template, the iter variable is available as a classic Thymeleaf variable. However, within ICU
-strings, if the iter variable is an object, all the fields are available as plain variables prefixed by the name of
-the iteration variable and 2 underscores. If you have `product.id`, then you can access it as `product__id`.
-Information about the iteration can be kept by using [Thymeleaf iterator status mechanism](https://www.thymeleaf.org/doc/tutorials/3.1/usingthymeleaf.html#keeping-iteration-status).
-
-All of these variables still have to be set as `demoProps` for the template to render properly in preview mode.
-
-Example:
-```jsx
-<For each="product, iterStat : ${products}" demoIterations={2}>
-  <tr>
-    <td>
-      <Var variable="iterStat.index" demoValue="1" />
-    </td>
-    <td>
-      <LocalizedText
-        keyName="product-id"
-        defaultValue="Product #{product__id}"
-        demoParams={{
-          product__id: 1337
-        }}
-      />
-    </td>
-    <td>
-      <LocalizedText
-        keyName="product-price"
-        defaultValue="Price: {product__price, number}"
-        demoParams={{
-          product__id: 4.00
-        }}
-      />
-    </td>
-  </tr>
-</For>
-```
-
 ## Global variables
 The following global variables are available:
 - `isCloud` (boolean): Whether this is Tolgee Cloud or not
diff --git a/email/components/layouts/ClassicLayout.tsx b/email/components/layouts/ClassicLayout.tsx
@@ -98,7 +98,7 @@ export default function ClassicLayout({
                           <Text className="text-xs m-0">
                             <LocalizedText
                               keyName="footer-cloud-sent-by"
-                              defaultValue="🐭 Sent by Tolgee - Check out <link>blog</link> and our socials! 🧀"
+                              defaultValue="🐭 Sent by Tolgee - Check out our <link>blog</link> and our socials! 🧀"
                               demoParams={{
                                 link: (c) => (
                                   <Link
diff --git a/email/components/translate.ts b/email/components/translate.ts
@@ -15,6 +15,7 @@
  */
 
 import * as React from 'react';
+import { TYPE } from '@formatjs/icu-messageformat-parser';
 import { IntlMessageFormat } from 'intl-messageformat';
 
 const GLOBALS = {
@@ -43,10 +44,32 @@ export default function t(
   defaultString?: string,
   demoParams?: Record<string, any>
 ) {
+  const intl = new IntlMessageFormat(defaultString, 'en-US');
+  const ast = intl.getAst();
+
+  const stringArguments: string[] = [];
+
+  for (const node of ast) {
+    if (node.type === TYPE.literal || node.type === TYPE.pound) {
+      continue;
+    }
+
+    if (node.type === TYPE.tag) {
+      // TODO: find a way to process the tag
+      continue;
+    }
+
+    stringArguments.push(`${node.value}: ${node.value.replace(/__/g, '.')}`);
+  }
+
   const text =
     process.env.NODE_ENV === 'production'
       ? defaultString
       : formatDev(defaultString, demoParams);
 
-  return React.createElement('span', { 'th:utext': `#{${key}}` }, text);
+  const messageExpression = stringArguments.length
+    ? `#{${key}(\${ { ${stringArguments.join(', ')} } })}`
+    : `#{${key}}`;
+
+  return React.createElement('span', { 'th:utext': messageExpression }, text);
 }
diff --git a/email/emails/zz-test-email.tsx b/email/emails/zz-test-email.tsx
diff --git a/email/i18n/messages_en.properties b/email/i18n/messages_en.properties
diff --git a/email/package-lock.json b/email/package-lock.json
diff --git a/email/package.json b/email/package.json
diff --git a/gradle/email.gradle b/gradle/email.gradle

Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,7 @@ apply plugin: 'io.spring.dependency-management'`
`27`	`27`
`28`	`28`	`repositories {`
`29`	`29`	`mavenCentral()`
	`30`	`+ maven { url 'https://jitpack.io' }`
`30`	`31`	`}`
`31`	`32`
`32`	`33`	`kotlin {`
Original file line number	Diff line number	Diff line change
`@@ -46,6 +46,7 @@ apply plugin: "org.gradle.test-retry"`
`46`	`46`
`47`	`47`	`repositories {`
`48`	`48`	`mavenCentral()`
	`49`	`+ maven { url 'https://jitpack.io' }`
`49`	`50`	`}`
`50`	`51`
`51`	`52`	`dependencyManagement {`
Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-../../../../../email/out`
	`1`	`+../../../../../email/dist/email-templates`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+email-test-string = Testing ICU strings -- {testVar}`
	`2`	`+email-test-it = ICU test: {item__name}`
Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ apply plugin: "kotlin-kapt"`
`37`	`37`
`38`	`38`	`repositories {`
`39`	`39`	`mavenCentral()`
	`40`	`+ maven { url 'https://jitpack.io' }`
`40`	`41`	`}`
`41`	`42`
`42`	`43`	`kotlin {`
Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ apply plugin: 'io.spring.dependency-management'`
`23`	`23`
`24`	`24`	`repositories {`
`25`	`25`	`mavenCentral()`
	`26`	`+ maven { url 'https://jitpack.io' }`
`26`	`27`	`}`
`27`	`28`
`28`	`29`	`kotlin {`
Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,7 @@ apply plugin: "kotlin-kapt"`
`43`	`43`
`44`	`44`	`repositories {`
`45`	`45`	`mavenCentral()`
	`46`	`+ maven { url 'https://jitpack.io' }`
`46`	`47`	`}`
`47`	`48`
`48`	`49`	`kotlin {`