fix: apply language at runtime when changed in LanguageView - #7558
fix: apply language at runtime when changed in LanguageView#7558deepak0x wants to merge 2 commits into
Conversation
LanguageView.changeLanguage saved the language to server preferences, Redux and the local database but never called setLanguage from app/i18n, so the i18n locale was not updated until a full app restart. Call setLanguage(language) after changeLanguage resolves so the selected language is applied immediately for non-RTL switches. Fixes RocketChat#5823 Signed-off-by: Deepak Bhagat <deepak988088@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughLanguageView now applies the selected language after preference persistence succeeds. Database failures reach the outer error handler. A test verifies that selecting Spanish calls ChangesLanguage selection
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/views/LanguageView/index.test.tsx (1)
31-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace untyped test mock values.
The
anyannotations remove checks for the mockedFlatList, database callback, andsetLanguagespy. Use narrow interfaces for the mock contracts and a typed mocked reference forsetLanguage.As per coding guidelines, TypeScript code must use type safety, explicit function annotations, and interfaces for object shapes.
Also applies to: 84-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/views/LanguageView/index.test.tsx` around lines 31 - 56, Replace the any-typed mocks in the React Native FlatList, database write callback, and related test setup with explicit interfaces and function signatures describing their narrow contracts. Update the setLanguage spy to use a typed mocked reference, preserving existing behavior while restoring TypeScript checking for mock data, renderItem parameters, database callbacks, and spy calls.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/views/LanguageView/index.test.tsx`:
- Around line 47-56: Update the database mock used by the LanguageView test so
servers.write is defined directly on the servers object and executes its
callback, while servers.get returns a collection exposing find (with its
existing update behavior). Keep the mock aligned with the calls made by
changeLanguage so persistence succeeds before setLanguage is verified.
In `@app/views/LanguageView/index.tsx`:
- Around line 51-52: Update changeLanguage to return a success result or rethrow
persistence failures instead of resolving silently, then modify the caller
around setLanguage(language) to activate the locale only when the save
succeeded. Preserve the existing language-change flow for successful persistence
and prevent UI state updates after a failed preference save.
---
Nitpick comments:
In `@app/views/LanguageView/index.test.tsx`:
- Around line 31-56: Replace the any-typed mocks in the React Native FlatList,
database write callback, and related test setup with explicit interfaces and
function signatures describing their narrow contracts. Update the setLanguage
spy to use a typed mocked reference, preserving existing behavior while
restoring TypeScript checking for mock data, renderItem parameters, database
callbacks, and spy calls.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b1aff578-615c-4e1b-88d2-f3f66c07fcbf
📒 Files selected for processing (2)
app/views/LanguageView/index.test.tsxapp/views/LanguageView/index.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/views/LanguageView/index.test.tsxapp/views/LanguageView/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/views/LanguageView/index.test.tsxapp/views/LanguageView/index.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Format JavaScript and TypeScript code with Oxfmt using the repository configuration: tabs, single quotes, 130-character width, no trailing commas, omitted arrow-function parentheses where possible, and same-line brackets.
Follow Oxlint rules configured in.oxlintrc.json, including the import, React, Jest, TypeScript, and React Native plugins.
Files:
app/views/LanguageView/index.test.tsxapp/views/LanguageView/index.tsx
🧠 Learnings (4)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/views/LanguageView/index.test.tsxapp/views/LanguageView/index.tsx
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.
Applied to files:
app/views/LanguageView/index.test.tsxapp/views/LanguageView/index.tsx
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/views/LanguageView/index.test.tsx
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.
Applied to files:
app/views/LanguageView/index.test.tsxapp/views/LanguageView/index.tsx
Address CodeRabbit review on RocketChat#7558: - changeLanguage now returns a boolean and rethrows persistence failures instead of resolving silently, so setLanguage is only invoked when the server preference and local record were saved successfully. - fix the database mock so servers.write is defined directly (matching the call made by changeLanguage) so persistence succeeds in the test. Signed-off-by: Deepak Bhagat <deepak988088@gmail.com>
|
Addressed both CodeRabbit findings in the latest push:
|
1 participant
Proposed changes
LanguageView.changeLanguagesaved the selected language to server preferences, Redux (setUser) and the local database, but never invokedsetLanguagefromapp/i18n. SincesetLanguageis the only function that updatesi18n.locale/i18n.translationsat runtime, the locale stayed stale until a full app restart (wheresetLanguageruns again at module load).This PR calls
setLanguage(language)insubmitright afterchangeLanguageresolves, so the selected language is applied immediately for non-RTL switches. RTL switches still go through the existingRNRestartpath.Issue(s)
Fixes #5823
How to test or reproduce
Regression test added:
app/views/LanguageView/index.test.tsxrendersLanguageView, selects the Spanish radio, and assertssetLanguageis called with'es'. It failed before the fix (sincesetLanguagewas never called) and passes after.Screenshots
N/A (no visual change, behavior only).
Types of changes
Checklist
Further comments
The fix is minimal and scoped to the language-change flow.
setLanguageis also called for RTL switches, but the subsequentRNRestart.Restart()makes that a no-op (the locale is re-applied at cold start anyway).Summary by CodeRabbit