User Deletion - Remove User Info from Messages #288
Conversation
mathieubouhelier
requested review from
anicioalexandre and
priscilladeroode
as code owners
|
WalkthroughAdds defensive fallbacks for missing participant/profile data: the utils hook returns a deleted-user title/avatar when no other participant exists; the UserMessage component treats null/undefined profiles as "Deleted User" and shows an avatar when the profile is missing or when it's the first grouped message from another user. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Utils as useRoomNameAndAvatar
participant UI as UserMessage
participant Data as message/profile
rect rgba(200,230,255,0.18)
note right of Utils: Header derivation
Utils->>Utils: check otherParticipant
alt otherParticipant undefined
Utils-->>UI: { title: "Deleted User", avatar: undefined }
else
Utils-->>UI: { title: derived, avatar: derived }
end
end
rect rgba(220,255,220,0.18)
note right of UI: Message rendering
UI->>Data: read message.profile and grouping flags
alt profile null/undefined OR first grouped other-user
UI-->>UI: canShowAvatar = true
else
UI-->>UI: canShowAvatar = (isFirstGroupedMessage && !isOwnMessage)
end
alt profile null/undefined
UI-->>UI: display "Deleted User"
else
UI-->>UI: display profile.name
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
⏰ 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). (2)
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. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/components/modules/messages/common/utils.ts (2)
31-35: Replace user-facing "Error" with the required "Deleted User" label and keep return shape consistentEnd users should never see an error string here; acceptance requires the "Deleted User" label. Also return
avatarfor consistency with other branches.- if (!header?.participants) { - return { - title: 'Error: No participants', - } - } + if (!header?.participants) { + return { + title: 'Deleted User', + avatar: undefined, + } + }
40-50: Handle anonymized participants with null/empty name; always fall back to "Deleted User"Right now we only fall back when
otherParticipantis undefined. If the participant exists butnameis null/undefined/empty (common anonymization pattern), the header can render a blank title. This violates the acceptance criteria.- if (otherParticipant === undefined) { - return { - title: 'Deleted User', - avatar: undefined, - } - } - - return { - title: otherParticipant?.node?.profile?.name, - avatar: otherParticipant?.node?.profile?.image?.url, - } + if (!otherParticipant) { + return { + title: 'Deleted User', + avatar: undefined, + } + } + + const participantName = otherParticipant?.node?.profile?.name?.trim() || 'Deleted User' + const participantAvatar = otherParticipant?.node?.profile?.image?.url ?? undefined + + return { + title: participantName, + avatar: participantAvatar, + }
🧹 Nitpick comments (2)
packages/components/modules/messages/common/utils.ts (1)
31-35: Avoid hardcoding user-facing copy; use i18n string (e.g., t('chat.deletedUser'))If the project uses i18n, prefer a translation key so the label is localized consistently across the app.
If i18n exists in this module, I can propose a follow-up diff to wire it in.
Also applies to: 40-45, 47-50
packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/index.tsx (1)
74-80: Verify avatar placeholder shows the specific "Deleted User" iconWe pass
src={message?.profile?.image?.url}. If the design system distinguishes a generic user placeholder from a "deleted user" placeholder, we should select the latter when rendering a deleted user. If not supported, current behavior (no src => generic placeholder) may be acceptable but doesn’t strictly meet “Replace the avatar with a generic 'Deleted User' icon” if the icon differs.Can you confirm whether
AvatarWithPlaceholdersupports a deleted/variant placeholder or custom icon? If yes, I can provide a small conditional prop pass to select it when the name falls back to "Deleted User".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/components/modules/messages/common/utils.ts(1 hunks)packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/index.tsx(1 hunks)
⏰ 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: Component Test Packages
- GitHub Check: Build and Lint Packages
- GitHub Check: Unit Test Packages
🔇 Additional comments (1)
packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/index.tsx (1)
93-103: No profile links in messages moduleI’ve scanned the entire
packages/components/modules/messagesdirectory for any Link or anchor usage tied toprofile.id(including<Link>,<a>,href=,to=,router.push, etc.) and found no occurrences. In theUserMessagecomponent, avatars and names render viaAvatarWithPlaceholderandTypographyonly, with a fallback to “Deleted User” and no clickable wrappers.– No
Linkimports or<a>tags in messages components
– No navigation calls (router.push/navigate) referencing profiles
– Avatar URLs and profile names render safely with null checks and fallbacksThis confirms deleted-user names/avatars aren’t linked anywhere in the messages UI.
packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/index.tsx
Outdated
Show resolved
Hide resolved
mathieubouhelier
force-pushed
the
feature/BA-2693-fe-package-User-Deletion-Remove-User-Info-from-Messages
branch
from
e14ff82 to
235a48e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/components/modules/messages/common/utils.ts (2)
31-35: Replace user-facing error with anonymized fallback and keep return shape consistent.Showing “Error: No participants” contradicts the UX requirement; use the same “Deleted User” fallback and always include
avatarto keep a stable return type.Apply:
- if (!header?.participants) { - return { - title: 'Error: No participants', - } - } + if (!header?.participants) { + return { + title: 'Deleted User', + avatar: undefined, + } + }
47-50: Harden title/avatar for nullish or empty names to ensure anonymization.If the profile exists but its name is empty or whitespace, we should still show “Deleted User” and suppress the avatar to avoid leaking identity.
Apply:
- return { - title: otherParticipant?.node?.profile?.name, - avatar: otherParticipant?.node?.profile?.image?.url, - } + const name = otherParticipant?.node?.profile?.name?.trim() + return { + title: name || 'Deleted User', + avatar: name ? otherParticipant?.node?.profile?.image?.url : undefined, + }
🧹 Nitpick comments (2)
packages/components/modules/messages/common/utils.ts (2)
40-41: Minor: simplify undefined check.Use a falsy check for readability.
Apply:
- if (otherParticipant === undefined) { + if (!otherParticipant) {
40-45: Centralize the “Deleted User” label for consistency and i18n.Avoid string drift across components; consider a shared constant or translation key.
Example (add near the top of this file or use your i18n layer):
export const DELETED_USER_LABEL = 'Deleted User'Then use
DELETED_USER_LABELin all fallbacks here and in UserMessage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/components/modules/messages/common/utils.ts(1 hunks)packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/index.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/index.tsx
⏰ 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). (2)
- GitHub Check: Build and Lint Packages
- GitHub Check: Component Test Packages
🔇 Additional comments (2)
packages/components/modules/messages/common/utils.ts (2)
40-45: LGTM: solid anonymization fallback when no other participant is found.Early-returning with a “Deleted User” label and no avatar aligns with the acceptance criteria to keep messages while anonymizing attribution.
28-51: Verified avatar fallback and link absence for deleted users
AvatarWithPlaceholder correctly renders its fallback icon whensrc/imgSourceis undefined, and UserMessage displays “Deleted User” as plain text (no clickable link) for null profiles.
packages/components/modules/messages/web/MessagesList/MessagesGroup/UserMessage/index.tsx
Outdated
Show resolved
Hide resolved
|
Deleted User's avatar is not updated with deleted user icon. |
@lokmanliton , The deleted user icon isn’t included in the current design. Once it’s provided, we can add it. |
|
mathieubouhelier
deleted the
feature/BA-2693-fe-package-User-Deletion-Remove-User-Info-from-Messages
branch
Acceptance Criteria
As a user in the Messaging Feature, I want messages from deleted users to preserve their content but not show their name, avatar, or profile link, so that the conversation history remains readable without exposing data from deleted accounts.
Acceptance Criteria
When a user deletes their account, the system should anonymize their presence in all chat messages they participated in.
The user's name and avatar should be removed from all past messages.
Replace the avatar with a generic "Deleted User" icon.
Display a label such as “Deleted User” instead of their name.
The link to the deleted user's profile should be removed from all messages.
Messages sent by deleted users should remain visible to other participants in the chat.
Group and direct chats should continue to display messages from deleted users with anonymized attribution.
The message layout and order should remain unchanged after account deletion.
Summary by CodeRabbit