tolgee-platform icon indicating copy to clipboard operation
tolgee-platform copied to clipboard

fix: improve number placeholder regex in PluralTranslationUtil

Open dkrizan opened this issue 3 weeks ago β€’ 1 comments

#3330

Summary by CodeRabbit

  • Refactor

    • Updated internal tag attribute handling in translation processing.
  • Tests

    • Added test coverage for XML tag stripping in translation utilities.

✏️ Tip: You can customize this high-level summary in your review settings.

dkrizan avatar Dec 02 '25 12:12 dkrizan

Walkthrough

Replaced Tolgee XML placeholder attribute id with tolgee-id and switched from a dynamically constructed regex to an explicit pattern to match <x tolgee-id="...">...</x> in plural translation utilities and tests.

Changes

Cohort / File(s) Summary
Plural translation utility
backend/data/src/main/kotlin/io/tolgee/service/machineTranslation/PluralTranslationUtil.kt
Updated TOLGEE_TAG_OPEN to use tolgee-id and replaced the dynamic regex with an explicit TOLGEE_TAG_REGEX pattern <x tolgee-id="[^\">]*?\">.*?</x>.
PluralTranslationUtil unit tests
backend/data/src/test/kotlin/io/tolgee/unit/util/PluralTranslationUtilTest.kt
Updated expected x-tag attribute usages from id to tolgee-id. Added test strips translated x tags() to verify regex stripping of translated x tags.
Machine translation batch tests
backend/data/src/test/kotlin/io/tolgee/service/machineTranslation/MtBatchTranslatorTest.kt
Updated test expectations for TranslateResult entries to use <x tolgee-id="...">...</x> in translatedText strings.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Focus review on the new explicit regex for edge cases (greedy vs. non-greedy matching).
  • Verify test updates cover all affected serialization/parsing cases.

Suggested reviewers

  • JanCizmar

Poem

🐰 I hopped through tags both old and new,
Swapped id for tolgee-idβ€”quick and true.
A tidy regex tucked just right,
Now x-tags sparkle in the night. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
βœ… Passed checks (2 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed The title accurately describes the main change: updating the Tolgee tag attribute from 'id' to 'tolgee-id' and refactoring the regex pattern for number placeholders in PluralTranslationUtil.
✨ Finishing touches
  • [ ] πŸ“ Generate docstrings
πŸ§ͺ Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch danielkrizan/non-translatable-plural-translation-placeholder

πŸ“œ Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between e12fe3abb3cc819328c941b6598740e1d721db3a and afa9c3b45297d04bccd51bc12591a338b568da38.

πŸ“’ Files selected for processing (1)
  • backend/data/src/test/kotlin/io/tolgee/service/machineTranslation/MtBatchTranslatorTest.kt (1 hunks)
🧰 Additional context used
πŸ““ Path-based instructions (3)
backend/**/*.kt

πŸ“„ CodeRabbit inference engine (AGENTS.md)

backend/**/*.kt: After modifying JPA entities, run ./gradlew diffChangeLog to generate Liquibase changelog entries (add --no-daemon flag if docker command not found) Run ./gradlew ktlintFormat before committing code

Files:

  • backend/data/src/test/kotlin/io/tolgee/service/machineTranslation/MtBatchTranslatorTest.kt
backend/**/*Test.kt

πŸ“„ CodeRabbit inference engine (AGENTS.md)

backend/**/*Test.kt: Use TestData classes for test setup with TestDataService for saving and cleaning test data in unit tests Use .andAssertThatJson for testing API JSON responses in tests

Files:

  • backend/data/src/test/kotlin/io/tolgee/service/machineTranslation/MtBatchTranslatorTest.kt
**/*.{ts,tsx,js,jsx,kt,kts}

βš™οΈ CodeRabbit configuration file

As part of review, please check if the file follows 'The Stepdown Rule': The most important, high-level concepts should be at the top (like a newspaper headline and opening paragraph). Details should increase as you read downward. Functions should be ordered so that a caller appears before the functions it calls. When working with JavaScript components, we allow the main component to live at the bottom of the file as an exception to the rule.

Files:

  • backend/data/src/test/kotlin/io/tolgee/service/machineTranslation/MtBatchTranslatorTest.kt
🧠 Learnings (1)
πŸ““ Common learnings
Learnt from: stepan662
Repo: tolgee/tolgee-platform PR: 3182
File: webapp/src/component/layout/TopBanner/useAnnouncement.tsx:124-130
Timestamp: 2025-07-28T12:08:29.462Z
Learning: In the Tolgee platform project, JSON translation files are not manually edited. Translations are stored in the Tolgee platform itself and availability is checked at build time. The JSON files are generated artifacts, not the source of truth.
Learnt from: stepan662
Repo: tolgee/tolgee-platform PR: 3134
File: webapp/src/component/activity/configuration.tsx:500-502
Timestamp: 2025-08-12T11:01:00.701Z
Learning: In the Tolgee platform project, do not flag missing translation keys or suggest adding keys to JSON files during code reviews. All translation keys are managed through the Tolgee platform interface, not in code. Translation key availability is validated at build time, so missing key warnings in code reviews are irrelevant and should not be raised. The JSON translation files are generated artifacts.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build frontend πŸ—οΈ
  • GitHub Check: Frontend static check πŸͺ²
  • GitHub Check: Build backend πŸ—οΈ
πŸ”‡ Additional comments (1)
backend/data/src/test/kotlin/io/tolgee/service/machineTranslation/MtBatchTranslatorTest.kt (1)

139-149: Test expectations correctly updated to new tolgee-id format. The mock responses now use <x tolgee-id="tolgee-number"> instead of id, consistently across all three TranslateResult entries. The underlying PluralTranslationUtil implementation and regex pattern have been properly updated to match this format.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Dec 02 '25 12:12 coderabbitai[bot]