feat: add multilingual TeamChats UI (English, Spanish, Hindi) #37680
Conversation
) (RocketChat#33050) Co-authored-by: gabriellsh <[email protected]>
Co-authored-by: Julio A. <[email protected]>
[no ci]
…ocketChat#33073) Co-authored-by: Guilherme Gazzo <[email protected]>
[no ci]
…ess Hours config is missing for day (RocketChat#33085) Co-authored-by: Kevin Aleman <[email protected]>
Co-authored-by: Martin Schoeler <[email protected]>
Co-authored-by: Yash Rajpal <[email protected]>
Co-authored-by: Julio A. <[email protected]>
…pdate (RocketChat#33150) Co-authored-by: Yash Rajpal <[email protected]>
[no ci]
… many symbols (RocketChat#33256) Co-authored-by: Pierre Lehnen <[email protected]>
…edure (RocketChat#33267) Co-authored-by: gabriellsh <[email protected]>
Co-authored-by: Julio A. <[email protected]>
[no ci]
[no ci]
[no ci]
[no ci]
[no ci]
|
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
WalkthroughThis release PR performs a comprehensive version bump from 6.10.2 to 6.10.10 across the entire monorepo. It includes security enhancements (HTTP header redaction, HTML sanitization, request validation), rate-limiting configuration for API endpoints, improved access controls, message parser refactoring, and extensive changelog documentation for released versions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Attention areas:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.40.0)apps/meteor/tests/end-to-end/api/01-users.jsapps/meteor/tests/end-to-end/api/24-methods.jsThanks 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/models/CHANGELOG.md (1)
308-308: Unresolved merge conflict marker present in file.Line 308 contains
> > > > > > > origin/master, which is a git merge conflict marker. This suggests an unresolved merge state. Remove or resolve this marker before merging.apps/meteor/CHANGELOG.md (3)
408-411: Typo: “auditted” → “audited”.Fix wording in the E2EE warning callouts line.
- Implement E2EE warning callouts letting users know that encrypted messages can't be searched and auditted on search contextual bar and audit panel. + Implement E2EE warning callouts letting users know that encrypted messages can't be searched and audited on search contextual bar and audit panel.
1619-1623: Typo: “galery mode” → “gallery mode”.Minor copy edit in the image attachments note.
- Fixed open expanded view (galery mode) for image attachments sent by livechat widget + Fixed open expanded view (gallery mode) for image attachments sent by livechat widget
423-424: Typo: “recieve” → “receive”.Low-risk spelling correction.
- Users now don't need to be online to get the keys ... users can recieve keys + Users now don't need to be online to get the keys ... users can receive keyspackages/rest-typings/CHANGELOG.md (1)
373-373: Critical: Merge conflict marker present.Line 373 contains an unresolved merge conflict marker (
>>>>>>> origin/master). This must be removed before merging.Apply this fix:
- - @rocket.chat/[email protected] - </details> - ->>>>>>> origin/master - ## 6.7.1
🧹 Nitpick comments (18)
packages/gazzodown/src/katex/KatexBlock.tsx (1)
13-19: Good alignment with inline KaTeX; consider shared options and confirm limitMirroring the inline renderer by adding
maxSize: 100here makes the behavior consistent and the change is syntactically and semantically fine.Two minor follow‑ups:
- Confirm that
100is the desired upper bound for block formulas as well, and that our KaTeX version supports this option as used.- (Optional) Since
displayMode,macros, andmaxSizeare now almost identical betweenKatexElementandKatexBlock, consider extracting a shared base options object (e.g.,const baseKatexOptions = { macros: {...}, maxSize: 100 }) and spreading it in both places to avoid future divergence.packages/i18n/src/locales/en.i18n.json (1)
6481-6489: Add keys only to English; let the translation pipeline populate other locales.
This repo’s workflow adds new keys in en.i18n.json only; es/hi are handled externally or fall back to English. Please revert manual changes in es/hi for these keys.Based on learnings, ...
packages/message-parser/CHANGELOG.md (1)
1-36: Changelog entries are well-structured; minor style improvement suggested.The new entries for versions 0.31.32, 0.31.31, and 0.31.30 are properly formatted with PR references. However, line 29 uses "Fixed a problem" which could be more formal. Consider using "Resolved" instead of "Fixed" for consistency with formal changelog conventions.
Apply this optional style improvement:
- - ([#33929](https://github.com/RocketChat/Rocket.Chat/pull/33929)) Fixed a problem in the deno runtime controller where it would not handle undefined child process references correctly + - ([#33929](https://github.com/RocketChat/Rocket.Chat/pull/33929)) Resolved an issue in the deno runtime controller where it would not handle undefined child process references correctlyapps/meteor/tests/end-to-end/api/01-users.js (1)
2473-2503: New forgot-password tests accurately cover enabled/disabled behaviorThe added tests correctly assert a 400 error when
Accounts_PasswordResetis disabled and ensure the setting is enabled before asserting the 200 success path, aligning with the new route logic. If you want stronger test isolation, you could add abefore/afterpair in thisdescribeto snapshot and restoreAccounts_PasswordReset, but it’s not strictly required given the current toggle sequence.apps/meteor/CHANGELOG.md (1)
7-10: Deduplicate repeated “Bump @rocket.chat/meteor version.” entries.Two identical bullets in the same 6.10.10 section add noise; keep one.
- Bump @rocket.chat/meteor version. - - Bump @rocket.chat/meteor version.apps/meteor/app/livechat/server/hooks/processRoomAbandonment.ts (1)
59-63: LGTM! Per-day validation handles missing office day data correctly.The guard skips days without configuration and properly advances the date counter, preventing potential crashes from undefined data.
Consider logging a warning when skipping days due to missing configuration, as this might indicate a data integrity issue:
// Config doesnt have data for this day, we skip day if (!officeDay) { + // Optional: Log.warning(`Business hours missing for ${today}`); inactivityDay.add(1, 'days'); continue; }apps/meteor/app/lib/server/functions/getModifiedHttpHeaders.ts (1)
1-20: Good security practice: redacting sensitive tokens in logs.The function correctly redacts authentication tokens before logging HTTP headers.
The cookie parsing assumes a well-formed
cookieheader string. Consider adding type safety and handling edge cases:- if (modifiedHttpHeaders.cookie) { + if (modifiedHttpHeaders.cookie && typeof modifiedHttpHeaders.cookie === 'string') { - const cookies = modifiedHttpHeaders.cookie.split('; '); + const cookies = modifiedHttpHeaders.cookie.trim().split(/;\s*/); const modifiedCookies = cookies.map((cookie: string) => { + const trimmedCookie = cookie.trim(); - if (cookie.startsWith('rc_token=')) { + if (trimmedCookie.startsWith('rc_token=')) { return 'rc_token=[redacted]'; } - return cookie; + return trimmedCookie; }); modifiedHttpHeaders.cookie = modifiedCookies.join('; '); }This handles:
- Type checking (cookie could theoretically be an array in some HTTP libraries)
- Extra whitespace around semicolons
- Leading/trailing whitespace in individual cookies
apps/meteor/tests/end-to-end/api/00-miscellaneous.js (1)
5-6: E2E log redaction checks look correct; consider minor robustness tweaksThe
/stdout.queuesuite correctly:
- Enables tracing and log level, drives traffic (including
rc_tokencookie and auth header) viamethodCalland login, and- Asserts that the queue contains redacted
x-auth-tokenandrc_tokenvalues and never contains the raw token.Two optional improvements you might consider:
- The fixed
await sleep(4000);plus subsequent requests could still be a source of flakiness on slow environments; if practical, replacing the sleep with a poll on/stdout.queueuntil logs appear (or a timeout) would be more deterministic.- The assertions rely on exact string snippets in
log.string; any future change in log formatting could break the tests even if redaction still works. If the logger output format evolves, it may be worth centralizing/parsing the relevant portion rather than matching full substrings.Overall, the behavior under test and the way the tests are wired look good.
Also applies to: 699-848
apps/meteor/ee/server/lib/audit/methods.ts (1)
91-102: Improved PII minimization in audit logs; small helper extraction could reduce duplicationUsing a reduced
userFieldsobject foruinAuditLog.insertOneand adding a projection that strips nestedu.services,u.roles,u.emails, etc. fromauditGetAuditionsresponses significantly tightens exposure of user data while preserving required identifiers.You build the same
userFieldsstructure in bothauditGetOmnichannelMessagesandauditGetMessages. If this pattern spreads further, consider extracting a tiny helper (e.g.,buildAuditUserFields(user)) to centralize that mapping; otherwise this looks solid as is.Also applies to: 125-129, 138-149, 183-189, 201-218
apps/meteor/app/otr/server/methods/updateOTRAck.ts (1)
3-9: Stronger validation and access control for OTR ack updatesThe updated
updateOTRAckimplementation now correctly:
- Rejects unauthenticated callers and non‑
otrmessages.- Validates the message/ack payload shape.
- Confirms the room exists, is of the expected type, and that the caller can access it.
- Ensures the message’s
u._idbelongs to the room’s participants whenuidsis present.This significantly hardens the method against malformed or unauthorized calls. If you want to further improve readability, you could split the compound check on
canAccessRoomAsync/room.uidsinto named booleans before theif, but functionally this looks good.Also applies to: 18-50
apps/meteor/client/views/omnichannel/realTimeMonitoring/RealTimeMonitoringPage.js (1)
106-153: Unnecessary optional chaining on ref.The
keysref is always defined viauseRef, sokeys?.currentcan be simplified tokeys.current. The optional chaining adds no safety here sincekeyswill never be null/undefined.Example fix for line 106:
-<ConversationOverview key={keys?.current[0]} flexGrow={1} flexShrink={1} width='50%' reloadRef={reloadRef} params={allParams} /> +<ConversationOverview key={keys.current[0]} flexGrow={1} flexShrink={1} width='50%' reloadRef={reloadRef} params={allParams} />apps/meteor/app/retention-policy/server/cronPruneMessages.ts (1)
11-20: Function name is misleading.The function
getMaxAgeSettingIdByRoomTypereturns the actual TTL value from settings, not the setting ID. Consider renaming togetMaxAgeByRoomTypeorgetRetentionTTLByRoomTypefor clarity.-const getMaxAgeSettingIdByRoomType = (type: RetentionRoomTypes) => { +const getMaxAgeByRoomType = (type: RetentionRoomTypes) => { switch (type) { case 'c': return settings.get<number>('RetentionPolicy_TTL_Channels'); case 'p': return settings.get<number>('RetentionPolicy_TTL_Groups'); case 'd': return settings.get<number>('RetentionPolicy_TTL_DMs'); } };And update the call site at line 39:
-const maxAge = getMaxAgeSettingIdByRoomType(type) || 0; +const maxAge = getMaxAgeByRoomType(type) || 0;packages/gazzodown/src/emoji/EmojiRenderer.tsx (1)
18-23: Consider whether sanitization is necessary for programmatically-generated emoji fallback.The fallback content is derived from the emoji object's unicode or shortCode properties, which are programmatically generated from emoji metadata rather than user input. DOMPurify.sanitize may be unnecessary here, as the content should already be safe. However, defense-in-depth sanitization is not harmful if performance is acceptable.
apps/meteor/tests/unit/server/services/omnichannel-integrations/providers/twilio.spec.ts (2)
11-14: Remove unusedtwilioStub- it's not wired into proxyquire.
twilioStubis defined but never passed toproxyquire. The actualtwiliomodule withvalidateRequestis used directly by the provider. This stub setup is dead code.-const twilioStub = { - validateRequest: sinon.stub(), - isRequestFromTwilio: sinon.stub(), -};Also remove the reset calls in
beforeEach(lines 52-53).
105-124: SetTEST_MODEexplicitly in each test to ensure isolation.Tests at lines 105-124 and 145-164 don't explicitly set
process.env.TEST_MODE = 'false', relying on state from previous tests. This can cause flaky behavior if test order changes.Add
process.env.TEST_MODE = 'false';at the start of these tests for proper isolation:it('should reject a request where signature doesnt match', () => { + process.env.TEST_MODE = 'false'; + settingsStub.get.withArgs('SMS_Twilio_authToken').returns('test');Also applies to: 145-164
apps/meteor/tests/e2e/account-forgetSessionOnWindowClose.spec.ts (3)
20-22: Use.fill()instead of deprecated.type()method.Playwright's
.type()is deprecated. Use.fill()for setting input values.- await poRegistration.username.type('user1'); - await poRegistration.inputPassword.type(DEFAULT_USER_CREDENTIALS.password); + await poRegistration.username.fill('user1'); + await poRegistration.inputPassword.fill(DEFAULT_USER_CREDENTIALS.password);Also applies to: 43-45
24-24: Prefer semantic locators overpage.locator('role=...').Per coding guidelines, use
page.getByRole()instead ofpage.locator('role=...')for better readability and maintainability.- await expect(page.locator('role=heading[name="Welcome to Rocket.Chat"]')).toBeVisible(); + await expect(page.getByRole('heading', { name: 'Welcome to Rocket.Chat' })).toBeVisible(); - await expect(newPage.locator('role=heading[name="Welcome to Rocket.Chat"]')).toBeVisible(); + await expect(newPage.getByRole('heading', { name: 'Welcome to Rocket.Chat' })).toBeVisible(); - await expect(newPage.locator('role=button[name="Login"]')).toBeVisible(); + await expect(newPage.getByRole('button', { name: 'Login' })).toBeVisible();Also applies to: 29-29, 47-47, 52-52
14-14: Avoidasyncintest.describe()callback.The
asyncmodifier ontest.describe()callbacks (lines 14 and 33) is unnecessary and can cause issues. Describe blocks should be synchronous.- test.describe('Setting off', async () => { + test.describe('Setting off', () => { - test.describe('Setting on', async () => { + test.describe('Setting on', () => {Also applies to: 33-33
| it('should not validate a request when process.env.TEST_MODE is true', () => { | ||
| process.env.TEST_MODE = 'true'; | ||
|
|
||
| const twilio = new Twilio(); | ||
| const request = { | ||
| headers: { | ||
| 'x-twilio-signature': 'test', | ||
| }, | ||
| }; | ||
|
|
||
| expect(twilio.validateRequest(request)).to.be.true; | ||
| }); |
There was a problem hiding this comment.
Remove duplicate test case.
This test (lines 69-80) is an exact copy of the test at lines 56-67. Both test the same scenario: "should not validate a request when process.env.TEST_MODE is true".
- it('should not validate a request when process.env.TEST_MODE is true', () => {
- process.env.TEST_MODE = 'true';
-
- const twilio = new Twilio();
- const request = {
- headers: {
- 'x-twilio-signature': 'test',
- },
- };
-
- expect(twilio.validateRequest(request)).to.be.true;
- });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should not validate a request when process.env.TEST_MODE is true', () => { | |
| process.env.TEST_MODE = 'true'; | |
| const twilio = new Twilio(); | |
| const request = { | |
| headers: { | |
| 'x-twilio-signature': 'test', | |
| }, | |
| }; | |
| expect(twilio.validateRequest(request)).to.be.true; | |
| }); |
🤖 Prompt for AI Agents
In
apps/meteor/tests/unit/server/services/omnichannel-integrations/providers/twilio.spec.ts
around lines 69 to 80, there is a duplicate test case that repeats the same
"should not validate a request when process.env.TEST_MODE is true" scenario
already defined at lines 56-67; remove the redundant test block (lines 69-80) so
only the original test remains, ensuring no other tests rely on this duplicate
and running the test suite afterwards to confirm no regressions.
| "title": "Team Chats", | ||
| "createRoom": "Create New Room", | ||
| "sendMessage": "Send Message", | ||
| "searchMembers": "Search Members", | ||
| "addParticipant": "Add Participant", | ||
| "removeParticipant": "Remove Participant", | ||
| "noMessages": "No messages yet", | ||
| "typeMessage": "Type a message..." | ||
| } |
There was a problem hiding this comment.
Do not nest objects in this file; flatten the TeamChats keys.
The locales files are a flat key→string map. Introducing a nested "TeamChats" object and lowerCamelCase subkeys will likely break lookups and violates prevailing naming. Flatten and follow the existing key style.
Apply this diff to replace the nested object with flat keys:
- "TeamChats": {
- "title": "Team Chats",
- "createRoom": "Create New Room",
- "sendMessage": "Send Message",
- "searchMembers": "Search Members",
- "addParticipant": "Add Participant",
- "removeParticipant": "Remove Participant",
- "noMessages": "No messages yet",
- "typeMessage": "Type a message..."
- }
+ "TeamChats_Title": "Team Chats",
+ "TeamChats_CreateRoom": "Create New Room",
+ "TeamChats_SendMessage": "Send Message",
+ "TeamChats_SearchMembers": "Search Members",
+ "TeamChats_AddParticipant": "Add Participant",
+ "TeamChats_RemoveParticipant": "Remove Participant",
+ "TeamChats_NoMessages": "No messages yet",
+ "TeamChats_TypeMessage": "Type a message..."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "title": "Team Chats", | |
| "createRoom": "Create New Room", | |
| "sendMessage": "Send Message", | |
| "searchMembers": "Search Members", | |
| "addParticipant": "Add Participant", | |
| "removeParticipant": "Remove Participant", | |
| "noMessages": "No messages yet", | |
| "typeMessage": "Type a message..." | |
| } | |
| "TeamChats_Title": "Team Chats", | |
| "TeamChats_CreateRoom": "Create New Room", | |
| "TeamChats_SendMessage": "Send Message", | |
| "TeamChats_SearchMembers": "Search Members", | |
| "TeamChats_AddParticipant": "Add Participant", | |
| "TeamChats_RemoveParticipant": "Remove Participant", | |
| "TeamChats_NoMessages": "No messages yet", | |
| "TeamChats_TypeMessage": "Type a message..." |
🤖 Prompt for AI Agents
packages/i18n/src/locales/en.i18n.json lines 6481-6489: the current JSON
contains a nested "TeamChats" object which breaks the flat key→string
convention; replace the nested object with flat lowerCamelCase keys by removing
the "TeamChats" nesting and adding top-level keys like "teamChatsTitle",
"teamChatsCreateRoom", "teamChatsSendMessage", "teamChatsSearchMembers",
"teamChatsAddParticipant", "teamChatsRemoveParticipant", "teamChatsNoMessages",
and "teamChatsTypeMessage" mapped to the same string values so all keys remain
flat and follow the existing naming style.
| "TeamChats": { | ||
| "title": "टीम चैट", | ||
| "createRoom": "नई रूम बनाएँ", | ||
| "sendMessage": "संदेश भेजें", | ||
| "searchMembers": "सदस्यों को खोजें", | ||
| "addParticipant": "प्रतिभागी जोड़ें", | ||
| "removeParticipant": "प्रतिभागी हटाएँ", | ||
| "noMessages": "अभी तक कोई संदेश नहीं", | ||
| "typeMessage": "संदेश टाइप करें..." | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check recent commits to locale files to understand the pattern
git log --oneline -20 -- "packages/i18n/src/locales/" | head -20Repository: RocketChat/Rocket.Chat
Length of output: 188
🏁 Script executed:
# Check if TeamChats exists in en.i18n.json
rg "TeamChats" packages/i18n/src/locales/en.i18n.json -A 10Repository: RocketChat/Rocket.Chat
Length of output: 188
🏁 Script executed:
# Examine what locale files exist and their recent modification pattern
ls -la packages/i18n/src/locales/ | head -30Repository: RocketChat/Rocket.Chat
Length of output: 188
🏁 Script executed:
# Check for translation pipeline or automation scripts
fd -e "\.ts$|\.js$|\.json$" packages/i18n/ | grep -i "pipeline\|translate\|auto\|sync" | head -20Repository: RocketChat/Rocket.Chat
Length of output: 188
🏁 Script executed:
# Look for scripts or workflows related to i18n
fd -e "\.yml$|\.yaml$" | xargs rg "i18n|translate" | head -20Repository: RocketChat/Rocket.Chat
Length of output: 188
Remove manually added keys from non-English locale file—follow the i18n workflow.
Per the established i18n workflow in this repository, new translation keys should be added only to packages/i18n/src/locales/en.i18n.json. Other locale files (including hi.i18n.json and es.i18n.json) are populated via an external translation pipeline and/or fall back to English.
Manually adding the same keys to Hindi and Spanish files contradicts this process and may cause conflicts or overwrites when the translation pipeline runs.
Action required: Remove the TeamChats object from this file (lines 217–226). Keep only the changes to the English locale file.
🤖 Prompt for AI Agents
In packages/i18n/src/locales/hi.i18n.json around lines 217 to 226, remove the
manually added "TeamChats" object entirely so this non-English locale no longer
contains keys that should be managed only in the English master file; ensure you
leave other Hindi keys untouched, do not add any replacement keys here, and keep
new translation key additions confined to packages/i18n/src/locales/en.i18n.json
per the i18n workflow before committing.
4 participants
This PR adds multilingual support for the Team Chats UI by introducing new i18n keys
and translations for English (en), Spanish (es), and Hindi (hi).
Added translations include:
Files updated:
All JSON files are validated and correctly formatted.
Summary by CodeRabbit
New Features
Bug Fixes
Security Improvements
✏️ Tip: You can customize this high-level summary in your review settings.