fix(ui): prevent username truncation in system messages#38245
fix(ui): prevent username truncation in system messages#38245sharanyamahajan wants to merge 11 commits intoRocketChat:developfrom
Conversation
fix(admin): make Custom HTML card responsive on mobile
…n-loader fix(ui): add skeleton loading states to Room Contextual Bar views
…ptions fix: prevent startup crash on invalid MONGO_OPTIONS JSON
|
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 |
|
|
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdates fix username truncation in message headers, adjust sidebar item removal, add a contextual list skeleton component, harden MongoDB config parsing and options, and substantially simplify/condense the BaseRaw data-access class API. Changes
Sequence Diagram(s)(No sequence diagrams generated — changes are across UI, config, and large model refactor without a single new multi-component sequential flow requiring visualization.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/packages/rocketchat-mongo-config/server/index.js`:
- Around line 33-47: The catch block that logs the raw MONGO_OPTIONS value
(variable mongoOptionStr in the try/catch parsing block) may leak secrets;
update the error logging to avoid printing the full mongoOptionStr by either
omitting it or replacing it with a redacted/truncated representation (e.g., a
helper like redactSecret(mongoOptionStr) or show only the first/last N chars
with a marker) and keep the existing user-facing throw using error.message;
apply this change inside the existing try/catch that parses mongoOptionStr so
the other log lines and the thrown Error remain intact.
🧹 Nitpick comments (2)
apps/meteor/client/components/message/MessageHeader.tsx (1)
55-59: Remove inline comments from production code.The fix correctly prevents username truncation using
flexShrink: 0andwhiteSpace: 'nowrap'. However, the emoji comments (// 🔑 prevents username truncation,// 🔑 keeps full username visible) should be removed per the coding guidelines.Suggested fix
style={{ cursor: 'pointer', - flexShrink: 0, // 🔑 prevents username truncation - whiteSpace: 'nowrap', // 🔑 keeps full username visible + flexShrink: 0, + whiteSpace: 'nowrap', }}Based on coding guidelines: "Avoid code comments in the implementation".
apps/meteor/client/views/room/contextualBar/lib/ContextualListSkeleton.tsx (1)
1-2: Consider grouping type imports separately.Minor style suggestion: group the type import with value imports for consistency.
Suggested import order
-import type { FC } from 'react'; -import { Box, Skeleton } from '@rocket.chat/fuselage'; +import { Box, Skeleton } from '@rocket.chat/fuselage'; +import type { FC } from 'react';
| if (typeof mongoOptionStr !== 'undefined') { | ||
| const mongoOptions = JSON.parse(mongoOptionStr); | ||
| Object.assign(mongoConnectionOptions, mongoOptions); | ||
| try { | ||
| const mongoOptions = JSON.parse(mongoOptionStr); | ||
| Object.assign(mongoConnectionOptions, mongoOptions); | ||
| } catch (error) { | ||
| console.error('Failed to parse MONGO_OPTIONS environment variable'); | ||
| console.error('Provided value:', mongoOptionStr); | ||
| console.error('Error:', error.message); | ||
| console.error('Please ensure MONGO_OPTIONS contains valid JSON'); | ||
|
|
||
| throw new Error( | ||
| `Invalid MONGO_OPTIONS configuration: ${error.message}`, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Good error handling addition, but consider redacting sensitive data in logs.
The try/catch for JSON.parse is a solid improvement that helps operators debug misconfigured environments. However, logging the raw MONGO_OPTIONS value (line 39) could expose sensitive information like credentials if they were mistakenly included in the JSON.
Consider either:
- Omitting the raw value from logs, or
- Truncating/redacting it
Suggested safer logging
} catch (error) {
console.error('Failed to parse MONGO_OPTIONS environment variable');
- console.error('Provided value:', mongoOptionStr);
+ console.error('Provided value length:', mongoOptionStr.length, 'chars');
console.error('Error:', error.message);
console.error('Please ensure MONGO_OPTIONS contains valid JSON');📝 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.
| if (typeof mongoOptionStr !== 'undefined') { | |
| const mongoOptions = JSON.parse(mongoOptionStr); | |
| Object.assign(mongoConnectionOptions, mongoOptions); | |
| try { | |
| const mongoOptions = JSON.parse(mongoOptionStr); | |
| Object.assign(mongoConnectionOptions, mongoOptions); | |
| } catch (error) { | |
| console.error('Failed to parse MONGO_OPTIONS environment variable'); | |
| console.error('Provided value:', mongoOptionStr); | |
| console.error('Error:', error.message); | |
| console.error('Please ensure MONGO_OPTIONS contains valid JSON'); | |
| throw new Error( | |
| `Invalid MONGO_OPTIONS configuration: ${error.message}`, | |
| ); | |
| } | |
| } | |
| if (typeof mongoOptionStr !== 'undefined') { | |
| try { | |
| const mongoOptions = JSON.parse(mongoOptionStr); | |
| Object.assign(mongoConnectionOptions, mongoOptions); | |
| } catch (error) { | |
| console.error('Failed to parse MONGO_OPTIONS environment variable'); | |
| console.error('Provided value length:', mongoOptionStr.length, 'chars'); | |
| console.error('Error:', error.message); | |
| console.error('Please ensure MONGO_OPTIONS contains valid JSON'); | |
| throw new Error( | |
| `Invalid MONGO_OPTIONS configuration: ${error.message}`, | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@apps/meteor/packages/rocketchat-mongo-config/server/index.js` around lines 33
- 47, The catch block that logs the raw MONGO_OPTIONS value (variable
mongoOptionStr in the try/catch parsing block) may leak secrets; update the
error logging to avoid printing the full mongoOptionStr by either omitting it or
replacing it with a redacted/truncated representation (e.g., a helper like
redactSecret(mongoOptionStr) or show only the first/last N chars with a marker)
and keep the existing user-facing throw using error.message; apply this change
inside the existing try/catch that parses mongoOptionStr so the other log lines
and the thrown Error remain intact.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/packages/rocketchat-mongo-config/server/index.js">
<violation number="1" location="apps/meteor/packages/rocketchat-mongo-config/server/index.js:39">
P2: Logging the raw `MONGO_OPTIONS` value could expose sensitive information like credentials in error logs. Consider logging only the string length or a truncated/redacted version to aid debugging without leaking secrets.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| Object.assign(mongoConnectionOptions, mongoOptions); | ||
| } catch (error) { | ||
| console.error('Failed to parse MONGO_OPTIONS environment variable'); | ||
| console.error('Provided value:', mongoOptionStr); |
There was a problem hiding this comment.
P2: Logging the raw MONGO_OPTIONS value could expose sensitive information like credentials in error logs. Consider logging only the string length or a truncated/redacted version to aid debugging without leaking secrets.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/packages/rocketchat-mongo-config/server/index.js, line 39:
<comment>Logging the raw `MONGO_OPTIONS` value could expose sensitive information like credentials in error logs. Consider logging only the string length or a truncated/redacted version to aid debugging without leaking secrets.</comment>
<file context>
@@ -22,28 +23,43 @@ const mongoConnectionOptions = {
+ Object.assign(mongoConnectionOptions, mongoOptions);
+ } catch (error) {
+ console.error('Failed to parse MONGO_OPTIONS environment variable');
+ console.error('Provided value:', mongoOptionStr);
+ console.error('Error:', error.message);
+ console.error('Please ensure MONGO_OPTIONS contains valid JSON');
</file context>
| console.error('Provided value:', mongoOptionStr); | |
| console.error('Provided value length:', mongoOptionStr.length, 'chars'); |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/client/views/account/profile/AccountProfileForm.tsx`:
- Line 170: The onChange handlers currently call
field.onChange(e.target.value.trim()), which strips spaces while typing; remove
the .trim() from the onChange calls (use field.onChange(e.target.value)) and add
corresponding onBlur handlers that trim the value and call
field.onChange(e.target.value.trim()) so trimming happens only on blur/submit;
apply this change for the fields referenced by the handlers (the
name/statusText/nickname/bio inputs where field.onChange is used).
🧹 Nitpick comments (2)
apps/meteor/client/views/account/profile/AccountProfileForm.tsx (2)
8-9: Unused imports:FieldErrorandFieldHintare imported but not used.These components are imported but appear to have no corresponding usage in the JSX. If error display components were intentionally removed, these imports should be cleaned up. If the removal was unintentional, validation errors won't be properly displayed to users.
♻️ Remove unused imports
import { Field, FieldGroup, FieldLabel, FieldRow, - FieldError, - FieldHint, TextInput, TextAreaInput, Box, Icon, Button, } from '@rocket.chat/fuselage';
75-86: Dead code:handleSendConfirmationEmailand related mutation are never used.The
mutateConfirmationEmailmutation andhandleSendConfirmationEmailcallback are defined but never invoked in the component. TheButtonimport (line 14) also appears unused. If the resend confirmation email functionality was intentionally removed from the UI, this code should be cleaned up.♻️ Remove unused code
-import { +import { Field, FieldGroup, FieldLabel, FieldRow, TextInput, TextAreaInput, Box, Icon, - Button, } from '@rocket.chat/fuselage';And remove the unused mutation and callback:
-const mutateConfirmationEmail = useMutation({ - mutationFn: sendConfirmationEmail, - onSuccess: () => dispatchToastMessage({ type: 'success', message: t('Verification_email_sent') }), - onError: (error) => dispatchToastMessage({ type: 'error', message: error }), -}); - -const handleSendConfirmationEmail = useCallback(async () => { - if (email !== previousEmail) { - return; - } - mutateConfirmationEmail.mutateAsync({ email }); -}, [email, previousEmail, mutateConfirmationEmail]);If the
sendConfirmationEmailendpoint is no longer needed, also remove line 47.
| <TextInput | ||
| {...field} | ||
| id={nameId} | ||
| onChange={(e) => field.onChange(e.target.value.trim())} |
There was a problem hiding this comment.
Critical: trim() on every keystroke prevents typing spaces.
Calling trim() in the onChange handler removes whitespace as the user types, making it impossible to enter multi-word values like "John Doe" for name, "On vacation" for status, etc. The space gets stripped immediately after typing it.
Trim should only be applied on blur or during form submission, not on every keystroke.
🐛 Proposed fix: trim on blur instead of onChange
For each affected field, either remove the trim entirely and handle it server-side, or use onBlur:
<TextInput
{...field}
id={nameId}
- onChange={(e) => field.onChange(e.target.value.trim())}
+ onChange={(e) => field.onChange(e.target.value)}
+ onBlur={(e) => field.onChange(e.target.value.trim())}
error={errors.name?.message}
disabled={!allowRealNameChange}
/>Apply the same pattern to statusText (line 211), nickname (line 235), and bio (line 252).
Also applies to: 211-211, 235-235, 252-252
🤖 Prompt for AI Agents
In `@apps/meteor/client/views/account/profile/AccountProfileForm.tsx` at line 170,
The onChange handlers currently call field.onChange(e.target.value.trim()),
which strips spaces while typing; remove the .trim() from the onChange calls
(use field.onChange(e.target.value)) and add corresponding onBlur handlers that
trim the value and call field.onChange(e.target.value.trim()) so trimming
happens only on blur/submit; apply this change for the fields referenced by the
handlers (the name/statusText/nickname/bio inputs where field.onChange is used).
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/client/views/account/profile/AccountProfileForm.tsx">
<violation number="1" location="apps/meteor/client/views/account/profile/AccountProfileForm.tsx:170">
P1: Trimming on every onChange strips user-entered spaces/newlines, preventing multi-word names and multiline bios from being typed.</violation>
<violation number="2" location="apps/meteor/client/views/account/profile/AccountProfileForm.tsx:262">
P2: Resend verification email action is no longer reachable; the email field lacks the button that triggers handleSendConfirmationEmail, leaving unverified users without a resend option while the handler remains dead code.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| <TextInput | ||
| {...field} | ||
| id={nameId} | ||
| onChange={(e) => field.onChange(e.target.value.trim())} |
There was a problem hiding this comment.
P1: Trimming on every onChange strips user-entered spaces/newlines, preventing multi-word names and multiline bios from being typed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/account/profile/AccountProfileForm.tsx, line 170:
<comment>Trimming on every onChange strips user-entered spaces/newlines, preventing multi-word names and multiline bios from being typed.</comment>
<file context>
@@ -162,136 +154,90 @@ const AccountProfileForm = (props: AllHTMLAttributes<HTMLFormElement>): ReactEle
<TextInput
{...field}
id={nameId}
+ onChange={(e) => field.onChange(e.target.value.trim())}
error={errors.name?.message}
disabled={!allowRealNameChange}
</file context>
| onChange={(e) => field.onChange(e.target.value.trim())} | |
| onChange={(e) => field.onChange(e.target.value)} |
| `} | ||
| > | ||
| <FieldLabel required htmlFor={emailId}>{t('Email')}</FieldLabel> | ||
| <FieldRow> |
There was a problem hiding this comment.
P2: Resend verification email action is no longer reachable; the email field lacks the button that triggers handleSendConfirmationEmail, leaving unverified users without a resend option while the handler remains dead code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/account/profile/AccountProfileForm.tsx, line 262:
<comment>Resend verification email action is no longer reachable; the email field lacks the button that triggers handleSendConfirmationEmail, leaving unverified users without a resend option while the handler remains dead code.</comment>
<file context>
@@ -303,69 +249,34 @@ const AccountProfileForm = (props: AllHTMLAttributes<HTMLFormElement>): ReactEle
- `}
- >
+ <FieldLabel required htmlFor={emailId}>{t('Email')}</FieldLabel>
+ <FieldRow>
<Controller
control={control}
</file context>
|
There seems to be lots of unrelated changes in this PR, also it is an exact duplicate of #38242 and it's trying to fix something that someone else has already raised a PR for #38236 I understand that people want to participate on open source projects, but as the old saying goes "quality over quantity" raising a ton of PRs without care will only make the life of the few maintainers harder. So next time I would suggest at least checking if the issue already has a PR, and reviewing your own PR after submitting, to check if its correct. I'll be closing this PR |
3 participants
Proposed changes (including videos or screenshots)
This pull request fixes a UI issue in system messages where the username gets truncated when the room topic is very long.
The issue occurs due to flex layout behavior, where the username container is allowed to shrink to accommodate long topic text. As a result, usernames such as "admin-user" appear truncated as "admin...".
This change prevents the username container from shrinking and allows the topic text to wrap instead. This ensures usernames remain fully visible and improves the readability of system messages.
The change is limited to the message header UI and does not introduce any logic, data flow, or behavioral changes.
Issue(s)
Fixes #38234
Steps to test or reproduce
Further comments
This is a small, focused UI fix scoped only to layout behavior. No alternative approaches were required, and no side effects are expected.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.