fix: sort export translations by key name (Sentry TOLGEE-BACKEND-3ED) - #3470
Conversation
📝 WalkthroughWalkthroughThe PR adds plural key suffix collision detection to the export functionality. When Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
226d7f8 to
5109c75
Compare
Anty0
left a comment
There was a problem hiding this comment.
I don't think this is a good way to fix this. This would be a breaking change - the order of keys in the export shouldn't change. I think there are people who depend on sorting being static. Can we solve this without affecting existing sorting?
5109c75 to
a4d9fbc
Compare
…BACKEND-3ED) When exporting to i18next format, plural keys get suffixed (e.g., "plan" → "plan_other"). If a non-plural key with the same suffixed name already exists, the export crashed with an IllegalStateException. Now detect such collisions upfront and throw a BadRequestException with a clear message identifying both the plural key and the colliding key, so the user can resolve the naming conflict. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
a4d9fbc to
3a676c4
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
webapp/src/i18n/en.json (1)
741-741: New key inserted out of alphabetical order
export_key_plural_suffix_collisionis placed between twocannot_*keys at line 741, breaking the alphabetical ordering of the file. All otherexport_*keys appear around lines 929–950. Moving it there keeps the file consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/src/i18n/en.json` at line 741, The new JSON entry "export_key_plural_suffix_collision" is out of alphabetical order; cut the "export_key_plural_suffix_collision" key-value pair from its current location and insert it with the other "export_*" keys (around the existing export block) maintaining alphabetical order among export_* entries; ensure you preserve commas and valid JSON syntax after moving so the file remains parseable.backend/data/src/main/kotlin/io/tolgee/formats/genericStructuredFile/out/GenericStructuredFileExporter.kt (1)
52-66: Redundant per-language iteration in collision check
translationscontains one entry per key per language, so a plural key exported for L languages causes the same name collision check to run L times. Deduplicate by key name first:♻️ Proposed refactor
private fun checkPluralSuffixCollisions() { val allKeyNames = translations.map { it.key.name }.toSet() - translations.filter { it.key.isPlural }.forEach { translation -> + val pluralKeyNames = translations.filter { it.key.isPlural }.map { it.key.name }.toSet() + for (pluralKeyName in pluralKeyNames) { for (suffix in formKeywords) { - val suffixedName = "${translation.key.name}_$suffix" + val suffixedName = "${pluralKeyName}_$suffix" if (suffixedName in allKeyNames) { throw ExportCollidingKeysException( - pluralKey = translation.key.name, + pluralKey = pluralKeyName, collidingKey = suffixedName, suffix = suffix, ) } } - } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/data/src/main/kotlin/io/tolgee/formats/genericStructuredFile/out/GenericStructuredFileExporter.kt` around lines 52 - 66, The checkPluralSuffixCollisions function is running the same collision check once per language because translations contains entries per key per language; to fix it, deduplicate by key name before iterating: compute allKeyNames = translations.map { it.key.name }.toSet() as now, then build a set of pluralKeyNames from translations (e.g. translations.filter { it.key.isPlural }.map { it.key.name }.toSet()) and iterate that set instead of translations, checking for each suffix in formKeywords whether "${pluralKey}_$suffix" is in allKeyNames and throwing ExportCollidingKeysException(pluralKey = pluralKey, collidingKey = suffixedName, suffix = suffix) when found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/data/src/main/kotlin/io/tolgee/formats/genericStructuredFile/out/GenericStructuredFileExporter.kt`:
- Around line 52-66: The checkPluralSuffixCollisions function is running the
same collision check once per language because translations contains entries per
key per language; to fix it, deduplicate by key name before iterating: compute
allKeyNames = translations.map { it.key.name }.toSet() as now, then build a set
of pluralKeyNames from translations (e.g. translations.filter { it.key.isPlural
}.map { it.key.name }.toSet()) and iterate that set instead of translations,
checking for each suffix in formKeywords whether "${pluralKey}_$suffix" is in
allKeyNames and throwing ExportCollidingKeysException(pluralKey = pluralKey,
collidingKey = suffixedName, suffix = suffix) when found.
In `@webapp/src/i18n/en.json`:
- Line 741: The new JSON entry "export_key_plural_suffix_collision" is out of
alphabetical order; cut the "export_key_plural_suffix_collision" key-value pair
from its current location and insert it with the other "export_*" keys (around
the existing export block) maintaining alphabetical order among export_*
entries; ensure you preserve commas and valid JSON syntax after moving so the
file remains parseable.
Anty0
left a comment
There was a problem hiding this comment.
It's definitely better, but I'm not sure it's an ideal solution. If we miss an edge case, we still end up with an IllegalStateException from throwIfExists. Maybe we can add another exception that just tells the user there is a conflict at the path buildPath(fullPath) if the throw in throwIfExists is reached - I'd just replace the IllegalStateException.
| "cannot_change_your_own_access_tooltip": "Cannot revoke access to yourself", | ||
| "cannot_delete_base_language_message": "Cannot delete base language", | ||
| "cannot_delete_branch_with_children": "Cannot delete this branch because other branches were created from it.", | ||
| "export_key_plural_suffix_collision": "Export failed: plural key \"{pluralKey}\" with suffix \"_{suffix}\" collides with existing key \"{collidingKey}\". Rename one of the keys to resolve the conflict.", |
There was a problem hiding this comment.
Shouldn't be part of the changeset.
Anty0
left a comment
There was a problem hiding this comment.
We'll deal with the other exception in a separate PR.
# [3.163.0](v3.162.0...v3.163.0) (2026-02-20) ### Bug Fixes * gh-982 - add tooltips ([#3417](#3417)) ([52d5933](52d5933)), closes [#982](#982) * Resilient Redis cache reads for rate limiting ([#3455](#3455)) ([a33e1d0](a33e1d0)) * sort export translations by key name (Sentry TOLGEE-BACKEND-3ED) ([#3470](#3470)) ([320d87a](320d87a)), closes [#3246](#3246) ### Features * rate limit translation and activity endpoints ([#3483](#3483)) ([2f125da](2f125da))
The StructureModelBuilder requires translations sorted alphabetically by key name path, but the database collation may return them in a different order. Sort in GenericStructuredFileExporter.prepare() to ensure correct ordering regardless of database collation.
Likely triggered by the branching feature (#3246, merged Feb 6) which added a LEFT JOIN on Branch to the export query. The additional join changed the query execution plan, surfacing a collation mismatch between PostgreSQL's locale-aware sorting and Java's byte-by-byte string comparison for keys with punctuation (e.g. "profile.my-goals" vs "profile.my-goals.plan_other").
Summary by CodeRabbit