Account Deletion - Remove Deleted Account Info from Comments #283
Conversation
mathieubouhelier
requested review from
anicioalexandre,
deboracosilveira and
priscilladeroode
as code owners
|
WalkthroughAdds handling for deleted/missing users: CommentItem detects absent users, shows "Deleted User" text, prevents reply actions for deleted users, and passes a Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CommentItem
participant DS as ClickableAvatar
participant AV as AvatarWithPlaceholder
CI->>CI: hasUser = Boolean(comment.user)
CI->>CI: renderUserName() -> "Deleted User" or Link(name)
CI->>DS: Render ClickableAvatar(deletedUser = !hasUser)
alt deletedUser = true
DS->>AV: Render fallback icon (non-clickable)
Note right of AV: title="Avatar Fallback"
else deletedUser = false
DS->>AV: Render avatar inside clickable wrapper (onClick active)
end
CI->>CI: replyToComment only proceeds when hasUser
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (6)
packages/design-system/components/web/avatars/ClickableAvatar/types.ts (1)
9-13: Tighten the onClick type (avoid any)Minor type improvement to avoid any and align with the actual clickable element (button).
Apply this diff:
+import type { MouseEventHandler } from 'react' export type ClickableAvatarProps = AvatarWithPlaceholderProps & { deletedUser?: boolean isOpen?: boolean - onClick?: (param: any) => void + onClick?: MouseEventHandler<HTMLButtonElement> }packages/design-system/components/web/avatars/ClickableAvatar/index.tsx (1)
24-26: A11y and semantics: use “Deleted User” in title and size icon based on min(width, height)
- Replace generic "Avatar Fallback" with "Deleted User" to better match acceptance criteria and assistive tech expectations.
- Use the smaller of width/height for icon size to keep it visually contained.
Apply this diff:
- <AvatarWithPlaceholder width={width} height={height} {...props}> - <AvatarUploadFallbackIcon sx={{ fontSize: width }} titleAccess="Avatar Fallback" /> + <AvatarWithPlaceholder width={width} height={height} {...props}> + <AvatarUploadFallbackIcon + sx={{ fontSize: Math.min(width, height) }} + titleAccess="Deleted User" + /> </AvatarWithPlaceholder>packages/components/modules/comments/web/CommentItem/index.tsx (4)
155-161: Alt text should reflect “Deleted User” for a11y; avoid exposing user nameWhen the user is deleted, use an alt text of “Deleted User” to align with the label and accessibility. Currently, it falls back to "Comment's user avatar".
Apply this diff:
<ClickableAvatar deletedUser={!hasUser} width={40} height={40} - alt={comment.user?.fullName ?? `Comment's user avatar`} + alt={hasUser ? comment.user?.fullName : 'Deleted User'} src={comment.user?.avatar?.url} onClick={() => router.push(profileUrl)} />
99-107: Internationalization: externalize “Deleted User” stringIf the app uses i18n, consider moving the “Deleted User” label to a translation file to support localization.
Example:
- if (!hasUser) return <Typography variant="subtitle2">Deleted User</Typography> + if (!hasUser) return <Typography variant="subtitle2">{t('comments.deletedUserLabel')}</Typography>And add comments.deletedUserLabel = "Deleted User" in your i18n resources.
67-67: Safer hasUser guardUse optional chaining to protect against null comment values in case of transient states; this makes the check robust.
Apply this diff:
- const hasUser = Boolean(comment.user) + const hasUser = Boolean(comment?.user)
62-66: Minor: avoid calling the same hook twiceYou invoke useCommentReply twice to extract two functions. Extract both from a single call for consistency and micro-optimization.
Outside this hunk, refactor to:
const { setCommentReply, resetCommentReply } = useCommentReply()
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/components/modules/comments/web/CommentItem/index.tsx(3 hunks)packages/design-system/components/web/avatars/ClickableAvatar/index.tsx(1 hunks)packages/design-system/components/web/avatars/ClickableAvatar/types.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/design-system/components/web/avatars/ClickableAvatar/index.tsx (1)
packages/design-system/components/web/avatars/ClickableAvatar/types.ts (1)
ClickableAvatarProps(9-13)
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/design-system/components/web/avatars/ClickableAvatar/types.ts (1)
10-10: Addition of deletedUser prop looks correctThe new prop reflects the intended behavior and keeps backward compatibility by being optional.
packages/design-system/components/web/avatars/ClickableAvatar/index.tsx (1)
21-29: Non-clickable fallback for deleted users: good separation of concernsBranching early on deletedUser and returning a non-clickable avatar meets the requirement to remove profile links for deleted users. Nice.
packages/components/modules/comments/web/CommentItem/index.tsx (1)
99-107: Confirm server semantics for deleted users (null vs flagged user)Quick check: the fragment (packages/components/modules/comments/common/graphql/queries/CommentItem.ts) requests user { id pk fullName avatar } and the component (packages/components/modules/comments/web/CommentItem/index.tsx) uses hasUser = Boolean(comment.user). I found no deletion flag (isDeleted / is_removed / redacted) anywhere in the repo and no generated types for CommentItem_comment.
Files inspected:
- packages/components/modules/comments/common/graphql/queries/CommentItem.ts
- packages/components/modules/comments/web/CommentItem/index.tsx
If the API can return a redacted user object (e.g. { isDeleted: true }) instead of null, update the checks to explicitly guard that case. Suggested change:
Replace:
const profileUrl =${profilePath}/${comment?.user?.pk}
const hasUser = Boolean(comment.user)With:
const hasUser = Boolean(comment?.user) && !comment.user?.isDeleted
const profileUrl = hasUser ?${profilePath}/${comment.user!.pk}: profilePathAlso request backend/schema confirmation or add a deletion flag to the CommentItem fragment if needed:
- If backend returns a flag, add user { isDeleted } to the fragment and adjust rendering to show “Deleted User” when flagged.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/index.tsx (2)
54-59: Render an explicit “Deleted User” avatar (icon) and remove unsupported Avatar propTo match the acceptance criteria (“replace the avatar with a generic ‘Deleted User’ icon”), render an icon when the user is absent. Also, MUI’s Avatar doesn’t support a top-level
sizesprop; leaving it in can cause prop-type noise. Below keeps the same layout while making the state explicit and accessible.Apply within this hunk:
- <Avatar - style={{ marginBottom: '4px' }} - sizes="small" - src={group.logs[0]?.user?.avatar?.url ?? ''} - alt={group.logs[0]?.user?.fullName ?? 'User activity log avatar'} - /> + <Avatar + style={{ marginBottom: '4px' }} + src={group.logs[0]?.user ? group.logs[0]?.user?.avatar?.url ?? undefined : undefined} + alt={group.logs[0]?.user ? (group.logs[0]?.user?.fullName ?? 'User activity log avatar') : DELETED_USER_LABEL} + > + {group.logs[0]?.user == null && <PersonOffOutlinedIcon fontSize="small" aria-hidden />} + </Avatar>Add the icon import near the other imports:
+import PersonOffOutlinedIcon from '@mui/icons-material/PersonOffOutlined'Notes:
- When the user is deleted,
altnow reads “Deleted User” (better for SR users).- By explicitly rendering an icon, we avoid MUI Avatar’s default initial-based fallback (which could show irrelevant initials from the generic alt).
38-41: Enhance Deleted User Label Handling and CentralizationA quick ripgrep scan uncovered another hard-coded “Deleted User” in
CommentItem/index.tsx, so we should centralize this label and prep for i18n across modules:• Refactor
renderUserNameinpackages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/index.tsxto trim blank names, handle missing users, and pull from a shared constant.
• Introduce aDELETED_USER_LABELconstant at a common location (e.g. alabels.tsor your i18n file) so both ActivityLog and Comments can import it.
• Updatepackages/components/modules/comments/web/CommentItem/index.tsxto use{DELETED_USER_LABEL}instead of the literal JSX.Apply within LogGroups/index.tsx:
- const renderUserName = (group: LogGroup) => { - if (group.logs[0]?.user == null) return 'Deleted User' - return group.logs[0]?.user?.fullName - } + const renderUserName = (group: LogGroup): string => { + const user = group.logs?.[0]?.user + if (!user) return DELETED_USER_LABEL + const name = user.fullName?.trim() + return name || DELETED_USER_LABEL + }Add near the top of the module (or, ideally, import from a shared file):
// TODO: swap this out for a translation key in our i18n setup export const DELETED_USER_LABEL = 'Deleted User'And in CommentItem/index.tsx, replace:
if (!hasUser) return <Typography variant="subtitle2">Deleted User</Typography>with:
if (!hasUser) return <Typography variant="subtitle2">{DELETED_USER_LABEL}</Typography>
📜 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 (1)
packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/index.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/index.tsx (1)
packages/components/modules/activity-log/common/types.ts (1)
LogGroup(7-10)
⏰ 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). (4)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Packages
- GitHub Check: Unit Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (1)
packages/components/modules/activity-log/web/ActivityLogComponent/LogGroups/index.tsx (1)
61-61: LGTM — centralizing the display name via renderUserName improves consistencySwitching the inline expression to a helper clarifies intent and reduces duplication. Once the avatar fallback is adjusted, the header will align visually and semantically with the deleted-user state.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/components/modules/comments/web/CommentItem/index.tsx (1)
103-103: Internationalize the “Deleted User” labelIf the app uses i18n, route this string through your translation layer for consistency and localization.
For example:
- if (!hasUser) return <Typography variant="subtitle2">Deleted User</Typography> + if (!hasUser) return <Typography variant="subtitle2">{t('comments.deletedUserLabel')}</Typography>
📜 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 (1)
packages/components/modules/comments/web/CommentItem/index.tsx(3 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). (2)
- GitHub Check: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (2)
packages/components/modules/comments/web/CommentItem/index.tsx (2)
102-110: Name rendering aligns with anonymization requirements — good jobShows “Deleted User” without a profile link when no user is present, and links to the profile only for active users. Matches the acceptance criteria.
67-67: Confirm backend semantics for deleted authorsThe GraphQL fragment for
CommentItemonly selects:user { id pk fullName avatar(width: 50, height: 50) { url } }There is no
isDeletedfield available, so Relay will return auserobject even for “stub” users. Relying solely onconst hasUser = Boolean(comment?.user)means any stub (with
pk: nullor empty fields) still registers as “hasUser.”Please verify with the backend whether deleted authors are returned as
user: nullor as a stub (e.g.{ pk: null, fullName: null }). If they return a stub:
- Update the check to guard against stubs with no PK, for example:
const hasUser = Boolean(comment?.user?.pk)- Or request the backend include an
isDeleted: Booleanflag on theusertype so we can filter out deleted accounts explicitly.
mathieubouhelier
deleted the
feature/BA-2667-fe-package-account-deletion-remove-deleted-account-info-from-comments
branch
Acceptance Criteria
As a user in the Comments Feature, I want comments from deleted users to remain visible but anonymized, so that the discussion context is preserved without exposing the deleted user’s identity.
Acceptance Criteria
When a user deletes their account, the system should anonymize their presence in all comments they made.
The user's name and avatar should be removed from all their comments.
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 comments.
The content of the comment should remain visible to other users.
Comments from deleted users should still be displayed in threads and maintain their original position in the discussion.
Pinned comments authored by a deleted user should still display as pinned but anonymized.
If a comment from a deleted user is replied to, the reply should still be shown normally.
Approvd
https://app.approvd.io/projects/BA/stories/42711
Summary by CodeRabbit