fix: missing room type translation on useEncryptedRoomDescription #36799
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: ea6738c The changes in this PR will be included in the next version bump. This PR includes changesets to release 39 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36799 +/- ##
===========================================
- Coverage 66.60% 66.57% -0.03%
===========================================
Files 3346 3346
Lines 114666 114666
Branches 21097 21099 +2
===========================================
- Hits 76368 76335 -33
- Misses 35607 35642 +35
+ Partials 2691 2689 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
tiagoevanp
previously requested changes
Aug 27, 2025
Contributor
tiagoevanp
left a comment
tiagoevanp
left a comment
There was a problem hiding this comment.
Lint errors to solve...
dougfabris
force-pushed
the
fix/missing-encryption-translation
branch
from
06b1904 to
987c83e
Compare
MartinSchoeler
force-pushed
the
fix/missing-encryption-translation
branch
from
987c83e to
ae92d66
Compare
Contributor
WalkthroughUpdates two encryption-hint hooks to pass the localized room type (via t(roomType)), expands their roomType union to include Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as CreateDiscussion UI
participant Hook as useEncryptedRoomDescription
participant I18n as i18n (t)
participant Trans as Translation keys
rect rgba(0,128,128,0.06)
UI->>Hook: getEncryptedHint({ isPrivate: true, encrypted })
Hook->>I18n: t(roomType) -- e.g., "discussion" -> "discussão"
I18n-->>Hook: localizedRoomType
Hook->>Trans: select template (Encrypted_messages / Encrypted_not_available)
Trans-->>Hook: formatted message (interpolating localizedRoomType)
Hook-->>UI: hint string
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.changeset/new-poems-compare.md (1)
5-5: Tighten the changeset description wording.Be explicit about what was fixed to aid release notes scanning.
-Fixes a missing translation on the create channel/team modal +Fix missing room type translation in the create channel/team modalapps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx (2)
15-17: Also translate roomType in the “encrypted” branch for consistency.Currently
{ roomType }may show raw keys in some locales.- if (encrypted) { - return t('Encrypted_messages', { roomType }); - } + if (encrypted) { + return t('Encrypted_messages', { roomType: t(roomType) }); + }
8-19: Optional: avoid duplicate lookups by computing once.Minor polish; precompute the localized label and reuse.
return ({ isPrivate, encrypted }: { isPrivate: boolean; encrypted: boolean }) => { + const roomTypeLabel = t(roomType); if (!e2eEnabled) { return t('Not_available_for_this_workspace'); } if (!isPrivate) { - return t('Encrypted_not_available', { roomType: t(roomType) }); + return t('Encrypted_not_available', { roomType: roomTypeLabel }); } if (encrypted) { - return t('Encrypted_messages', { roomType: t(roomType) }); + return t('Encrypted_messages', { roomType: roomTypeLabel }); } return t('Encrypted_messages_false'); };apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx (2)
15-17: Mirror the consistency fix here as well.Localize
roomTypein the “encrypted” branch.- if (encrypted) { - return t('Encrypted_messages', { roomType }); - } + if (encrypted) { + return t('Encrypted_messages', { roomType: t(roomType) }); + }
8-19: Optional: precompute the localized roomType.Small readability/perf tidy-up.
return ({ isPrivate, encrypted }: { isPrivate: boolean; encrypted: boolean }) => { + const roomTypeLabel = t(roomType); if (!e2eEnabled) { return t('Not_available_for_this_workspace'); } if (!isPrivate) { - return t('Encrypted_not_available', { roomType: t(roomType) }); + return t('Encrypted_not_available', { roomType: roomTypeLabel }); } if (encrypted) { - return t('Encrypted_messages', { roomType: t(roomType) }); + return t('Encrypted_messages', { roomType: roomTypeLabel }); } return t('Encrypted_messages_false'); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/new-poems-compare.md(1 hunks)apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx(1 hunks)apps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx (1)
apps/meteor/app/utils/lib/i18n.ts (1)
t(6-6)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx (1)
apps/meteor/app/utils/lib/i18n.ts (1)
t(6-6)
⏰ 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 Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx (1)
13-13: Good fix: localizes the roomType placeholder.Matches the change in the sidebar hook.
Reuse the same verification script to confirm keys and placeholders are present (see prior comment).
apps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx (1)
13-13: Good fix — localizes the roomType placeholder.Verified: the Encrypted_not_available / Encrypted_messages keys and the 'channel' / 'team' translation keys exist and many locales use {{roomType}}. However several locale files still hard‑code "Channel(s)" or omit the {{roomType}} placeholder (examples: packages/i18n/src/locales/zh-TW.i18n.json, packages/i18n/src/locales/ja.i18n.json, packages/i18n/src/locales/fr.i18n.json, packages/i18n/src/locales/nl.i18n.json). Update those locale entries to use {{roomType}} so t(roomType) renders the translated label.
MartinSchoeler
force-pushed
the
fix/missing-encryption-translation
branch
from
ae92d66 to
d69eaf9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.spec.tsx (2)
58-133: Broaden coverage to the sidebar hook and DRY test setup.The pt-BR tests validate only the NavBarV2 hook. Since both hooks were changed to pass t(roomType), add parity tests for the sidebar hook and factor a shared pt-BR wrapper. Also avoid shadowing Jest’s global
describeby renaming the local variable.Apply this diff:
-describe('useEncryptedRoomDescription, Portuguese (pt-BR)', () => { - it('returns "Encrypted_not_available" when channel is not private and E2E is enabled,', () => { - const { result } = renderHook(() => useEncryptedRoomDescription('channel'), { - wrapper: mockAppRoot() - .withSetting('E2E_Enable', true) - .withDefaultLanguage('pt-BR') - .withTranslations('pt-BR', 'core', { - Encrypted_not_available: 'Indisponível para {{roomType}} público', - channel: 'canal', - team: 'equipe', - }) - .build(), - }); - const describe = result.current; - expect(describe({ isPrivate: false, encrypted: false })).toBe('Indisponível para canal público'); - }); - - it('returns "Encrypted_not_available" when team is not private and E2E is enabled,', () => { - const { result } = renderHook(() => useEncryptedRoomDescription('team'), { - wrapper: mockAppRoot() - .withSetting('E2E_Enable', true) - .withDefaultLanguage('pt-BR') - .withTranslations('pt-BR', 'core', { - Encrypted_not_available: 'Indisponível para {{roomType}} público', - channel: 'canal', - team: 'equipe', - }) - .build(), - }); - const describe = result.current; - expect(describe({ isPrivate: false, encrypted: false })).toBe('Indisponível para equipe público'); - }); - - it('returns "Encrypted_messages" when channel is private and encrypted are true and E2E is enabled', () => { - const { result } = renderHook(() => useEncryptedRoomDescription('channel'), { - wrapper: mockAppRoot() - .withSetting('E2E_Enable', true) - .withDefaultLanguage('pt-BR') - .withTranslations('pt-BR', 'core', { - // TODO: Improve the portuguese translation with a way to captalize the room type for it to be in the start of the sentence - Encrypted_messages: - 'Criptografado de ponta a ponta {{roomType}}. A pesquisa não funcionará com {{roomType}} criptografado e as notificações podem não mostrar o conteúdo das mensagens.', - team: 'equipe', - channel: 'canal', - }) - .build(), - }); - const describe = result.current; - expect(describe({ isPrivate: true, encrypted: true })).toBe( - 'Criptografado de ponta a ponta canal. A pesquisa não funcionará com canal criptografado e as notificações podem não mostrar o conteúdo das mensagens.', - ); - }); - - it('returns "Encrypted_messages" when team is private and encrypted are true and E2E is enabled', () => { - const { result } = renderHook(() => useEncryptedRoomDescription('team'), { - wrapper: mockAppRoot() - .withSetting('E2E_Enable', true) - .withTranslations('pt-BR', 'core', { - Encrypted_messages: - 'Criptografado de ponta a ponta {{roomType}}. A pesquisa não funcionará com {{roomType}} criptografado e as notificações podem não mostrar o conteúdo das mensagens.', - channel: 'canal', - team: 'equipe', - }) - .withDefaultLanguage('pt-BR') - .build(), - }); - const describe = result.current; - expect(describe({ isPrivate: true, encrypted: true })).toBe( - 'Criptografado de ponta a ponta equipe. A pesquisa não funcionará com equipe criptografado e as notificações podem não mostrar o conteúdo das mensagens.', - ); - }); -}); +describe.each([ + ['useEncryptedRoomDescription in NavBarV2', useEncryptedRoomDescription], + ['useEncryptedRoomDescription', useEncryptedRoomDescriptionOld], +] as const)('Portuguese (pt-BR) – %s', (_name, hook) => { + const buildPtBrWrapper = () => + mockAppRoot() + .withSetting('E2E_Enable', true) + .withDefaultLanguage('pt-BR') + .withTranslations('pt-BR', 'core', { + Encrypted_not_available: 'Indisponível para {{roomType}} público', + Encrypted_messages: + 'Criptografado de ponta a ponta {{roomType}}. A pesquisa não funcionará com {{roomType}} criptografado e as notificações podem não mostrar o conteúdo das mensagens.', + channel: 'canal', + team: 'equipe', + }) + .build(); + + it('returns localized "Encrypted_not_available" for channel', () => { + const { result } = renderHook(() => hook('channel'), { wrapper: buildPtBrWrapper() }); + const getDescription = result.current; + expect(getDescription({ isPrivate: false, encrypted: false })).toBe('Indisponível para canal público'); + }); + + it('returns localized "Encrypted_not_available" for team', () => { + const { result } = renderHook(() => hook('team'), { wrapper: buildPtBrWrapper() }); + const getDescription = result.current; + expect(getDescription({ isPrivate: false, encrypted: false })).toBe('Indisponível para equipe público'); + }); + + it('returns localized "Encrypted_messages" for channel', () => { + const { result } = renderHook(() => hook('channel'), { wrapper: buildPtBrWrapper() }); + const getDescription = result.current; + expect(getDescription({ isPrivate: true, encrypted: true })).toBe( + 'Criptografado de ponta a ponta canal. A pesquisa não funcionará com canal criptografado e as notificações podem não mostrar o conteúdo das mensagens.', + ); + }); + + it('returns localized "Encrypted_messages" for team', () => { + const { result } = renderHook(() => hook('team'), { wrapper: buildPtBrWrapper() }); + const getDescription = result.current; + expect(getDescription({ isPrivate: true, encrypted: true })).toBe( + 'Criptografado de ponta a ponta equipe. A pesquisa não funcionará com equipe criptografado e as notificações podem não mostrar o conteúdo das mensagens.', + ); + }); +});
99-99: Fix TODO typo."captalize" -> "capitalize".
- // TODO: Improve the portuguese translation with a way to captalize the room type for it to be in the start of the sentence + // TODO: Improve the Portuguese translation with a way to capitalize the room type when it starts the sentence
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/new-poems-compare.md(1 hunks)apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.spec.tsx(1 hunks)apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx(1 hunks)apps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/new-poems-compare.md
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx
- apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.spec.tsx (3)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx (1)
useEncryptedRoomDescription(4-20)apps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx (1)
useEncryptedRoomDescription(4-20)packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
⏰ 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). (7)
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
dougfabris
requested changes
Sep 15, 2025
Member
dougfabris
left a comment
dougfabris
left a comment
There was a problem hiding this comment.
please, lets use this hook in CreateDiscussion as well
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx (2)
33-33: Avoid duplicated hook implementations; import from the canonical module (or re‑export one).This file imports from NavBarV2 while there’s also a copy under sidebar/header/hooks. Prefer a single source to prevent drift.
Apply:
-import { useEncryptedRoomDescription } from '../../NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription'; +import { useEncryptedRoomDescription } from '../../sidebar/header/hooks/useEncryptedRoomDescription';
248-248: Wire the hint to the toggle for better a11y.Add aria-describedby to the ToggleSwitch so SR users get the hint.
Apply:
- render={({ field: { value, ...field } }) => <ToggleSwitch id={encryptedId} {...field} checked={value} />} + render={({ field: { value, ...field } }) => ( + <ToggleSwitch id={encryptedId} {...field} checked={value} aria-describedby={`${encryptedId}-hint`} /> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx(2 hunks)apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx(3 hunks)apps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx (1)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx (1)
useEncryptedRoomDescription(4-20)
apps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx (2)
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.tsx (1)
useEncryptedRoomDescription(4-20)apps/meteor/app/utils/lib/i18n.ts (1)
t(6-6)
⏰ 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). (7)
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
apps/meteor/client/sidebar/header/hooks/useEncryptedRoomDescription.tsx (3)
4-4: Added 'discussion' to union — LGTM.Signature expansion looks correct and matches the other hook variant.
13-16: Passing t(roomType) into placeholders — fix looks correct.This should resolve the missing translations.
13-16: Verify locale keys and Title‑case placeholders existrg shows no locale entries for Title‑case "Channel"/"Team"/"Discussion" nor {roomType} in Encrypted_* strings; only matches were in tests (apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/useEncryptedRoomDescription.spec.tsx) and SlackAdapter.js comments. If locales lack Title‑case keys, map roomType at the callsite — suggested change:
+const ROOMTYPE_TKEY = { channel: 'Channel', team: 'Team', discussion: 'Discussion' } as const; ... - return t('Encrypted_not_available', { roomType: t(roomType) }); + return t('Encrypted_not_available', { roomType: t(ROOMTYPE_TKEY[roomType]) }); ... - return t('Encrypted_messages', { roomType: t(roomType) }); + return t('Encrypted_messages', { roomType: t(ROOMTYPE_TKEY[roomType]) });apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx (1)
95-96: Use of 'discussion' variant — LGTM.Correctly aligns the hint with Discussion context.
dougfabris
approved these changes
Sep 16, 2025
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Fixes this
The parameters sent to the t function were not being translated themselves
Issue(s)
Steps to test or reproduce
Further comments
CORE-1359
I've not added tests, since our translation mocks are not working with languages that are not english. I'll be investigating the issue on a separate PR
Summary by CodeRabbit