[WEB-4025] fix: external user comment and reaction #7692
Conversation
|
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. Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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. Comment |
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
…akeplane/plane into fix-external-user-comment-and-reaction
…l-user-comment-and-reaction
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issues with comments and reactions made by external users by ensuring proper display name fallbacks and appropriate labeling in the UI.
- Added
display_namefield to issue and comment reaction types - Updated reaction components to fall back to
display_namewhen user details are unavailable - Enhanced comment block component to handle external users with proper labeling and display name fallbacks
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/issues/issue_reaction.ts | Added display_name field to TIssueReaction type |
| packages/types/src/issues/activity/issue_comment_reaction.ts | Added display_name field to TIssueCommentReaction type |
| apps/web/core/components/issues/issue-detail/reactions/issue.tsx | Updated reaction display logic to fallback to display_name when user details unavailable |
| apps/web/core/components/issues/issue-detail/reactions/issue-comment.tsx | Updated comment reaction display logic with display_name fallback |
| apps/web/ce/components/comments/comment-block.tsx | Enhanced comment display with external user labeling and improved fallback logic |
| apps/api/plane/app/serializers/issue.py | Added display_name field to reaction serializers and explicitly defined fields |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/core/components/issues/issue-detail/reactions/issue-comment.tsx (2)
60-67: Missingawaiton remove → false-positive success + unhandled rejection.Promise rejection won’t be caught; success toast may show even on failure.
- removeCommentReaction(workspaceSlug, projectId, commentId, reaction, currentUser.id); + await removeCommentReaction(workspaceSlug, projectId, commentId, reaction, currentUser.id);
108-133: Fix React list keys.Key is on the inner button; the fragment returned by
maplacks a key and will warn.- <> - <Tooltip tooltipContent={getReactionUsers(reaction)}> + <Tooltip key={reaction} tooltipContent={getReactionUsers(reaction)}> <button type="button" onClick={() => !disabled && issueCommentReactionOperations.react(reaction)} - key={reaction} className={cn( "flex h-full items-center gap-1 rounded-md px-2 py-1 text-sm text-custom-text-100", userReactions.includes(reaction) ? "bg-custom-primary-100/10" : "bg-custom-background-80", { "cursor-not-allowed": disabled, } )} > <span>{renderEmoji(reaction)}</span> <span className={userReactions.includes(reaction) ? "text-custom-primary-100" : ""}> {(reactionIds || {})[reaction].length}{" "} </span> </button> - </Tooltip> - </> + </Tooltip>apps/web/core/components/issues/issue-detail/reactions/issue.tsx (1)
108-127: Fix React list keys.Same key placement issue as in comment reactions.
- <> - <Tooltip tooltipContent={getReactionUsers(reaction)}> + <Tooltip key={reaction} tooltipContent={getReactionUsers(reaction)}> <button type="button" onClick={() => !disabled && issueReactionOperations.react(reaction)} - key={reaction} className={cn( "flex h-full items-center gap-1 rounded-md px-2 py-1 text-sm text-custom-text-100", userReactions.includes(reaction) ? "bg-custom-primary-100/10" : "bg-custom-background-80", { "cursor-not-allowed": disabled, } )} > <span>{renderEmoji(reaction)}</span> <span className={userReactions.includes(reaction) ? "text-custom-primary-100" : ""}> {(reactionIds || {})[reaction].length}{" "} </span> </button> - </Tooltip> - </> + </Tooltip>
🧹 Nitpick comments (6)
apps/api/plane/app/serializers/issue.py (1)
670-675: Prefer non-nulldisplay_namefrom serializer.If
actor.display_namecan be empty/null, tooltips may end up blank. Consider a safe fallback via a method field.-class IssueReactionLiteSerializer(DynamicBaseSerializer): - display_name = serializers.CharField(source="actor.display_name", read_only=True) +class IssueReactionLiteSerializer(DynamicBaseSerializer): + display_name = serializers.SerializerMethodField(read_only=True) + + def get_display_name(self, obj): + actor = getattr(obj, "actor", None) + return getattr(actor, "display_name", None) or getattr(actor, "email", None) or ""(Replicate for
CommentReactionSerializerif desired.)apps/web/core/components/issues/issue-detail/reactions/issue-comment.tsx (1)
18-24: Avoid name collision with domain types.Local props type name shadows the shared
TIssueCommentReactiondomain type; rename for clarity.-export type TIssueCommentReaction = { +export type TIssueCommentReactionProps = { @@ -export const IssueCommentReaction: FC<TIssueCommentReaction> = observer((props) => { +export const IssueCommentReaction: FC<TIssueCommentReactionProps> = observer((props) => {apps/web/core/components/issues/issue-detail/reactions/issue.tsx (1)
40-82: Simplify self-referential ops to avoid circular reference.Define helpers first; return them from
useMemofor cleaner closures.- const issueReactionOperations = useMemo( - () => ({ - create: async (reaction: string) => { + const issueReactionOperations = useMemo(() => { + const create = async (reaction: string) => { try { if (!workspaceSlug || !projectId || !issueId) throw new Error("Missing fields"); await createReaction(workspaceSlug, projectId, issueId, reaction); setToast({ title: "Success!", type: TOAST_TYPE.SUCCESS, message: "Reaction created successfully", }); } catch (error) { setToast({ title: "Error!", type: TOAST_TYPE.ERROR, message: "Reaction creation failed", }); } - }, - remove: async (reaction: string) => { + }; + const remove = async (reaction: string) => { try { if (!workspaceSlug || !projectId || !issueId || !currentUser?.id) throw new Error("Missing fields"); await removeReaction(workspaceSlug, projectId, issueId, reaction, currentUser.id); setToast({ title: "Success!", type: TOAST_TYPE.SUCCESS, message: "Reaction removed successfully", }); } catch (error) { setToast({ title: "Error!", type: TOAST_TYPE.ERROR, message: "Reaction remove failed", }); } - }, - react: async (reaction: string) => { - if (userReactions.includes(reaction)) await issueReactionOperations.remove(reaction); - else await issueReactionOperations.create(reaction); - }, - }), - [workspaceSlug, projectId, issueId, currentUser, createReaction, removeReaction, userReactions] - ); + }; + const react = async (reaction: string) => { + if (userReactions.includes(reaction)) await remove(reaction); + else await create(reaction); + }; + return { create, remove, react }; + }, [workspaceSlug, projectId, issueId, currentUser, createReaction, removeReaction, userReactions]);apps/web/ce/components/comments/comment-block.tsx (3)
29-34: Harden name/avatar fallbacks to avoid “undefined” in UI; verify avatar field name.
Ifdisplay_nameis absent, the UI can render “undefined”. Also, confirm whether the store returnsavatar_url(notavatar) consistently.Proposed minimal, robust fallback chain:
- const displayName = comment?.actor_detail?.is_bot - ? comment?.actor_detail?.first_name + ` ${t("bot")}` - : (userDetails?.display_name ?? comment?.actor_detail?.display_name); - - const avatarUrl = userDetails?.avatar_url ?? comment?.actor_detail?.avatar_url; + const baseName = + userDetails?.display_name ?? + comment?.actor_detail?.display_name ?? + [comment?.actor_detail?.first_name, comment?.actor_detail?.last_name].filter(Boolean).join(" ") || + comment?.actor_detail?.email ?? + t("unknown_user"); + + const displayName = comment?.actor_detail?.is_bot ? `${baseName} ${t("bot")}` : baseName; + + const avatarUrl = + userDetails?.avatar_url ?? + comment?.actor_detail?.avatar_url ?? + null;If
getUserDetailsusesavatar(no_url), extend the fallback with that key as needed.
35-35: Move the null-guard above derived values.
Avoid callinggetUserDetails(comment?.actor)and computing name/url whencommentis falsy.
51-51: Guard Avatar props to handle missing name/src gracefully.
Prevents passing undefined togetFileURLand ensures a readable fallback name.- <Avatar size="base" name={displayName} src={getFileURL(avatarUrl)} className="flex-shrink-0" /> + <Avatar + size="base" + name={displayName ?? t("unknown_user")} + src={avatarUrl ? getFileURL(avatarUrl) : undefined} + className="flex-shrink-0" + />If
getFileURLalready handles undefined safely, feel free to keep it simple; otherwise this guards it.
📜 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 (6)
apps/api/plane/app/serializers/issue.py(1 hunks)apps/web/ce/components/comments/comment-block.tsx(3 hunks)apps/web/core/components/issues/issue-detail/reactions/issue-comment.tsx(1 hunks)apps/web/core/components/issues/issue-detail/reactions/issue.tsx(1 hunks)packages/types/src/issues/activity/issue_comment_reaction.ts(1 hunks)packages/types/src/issues/issue_reaction.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/plane/app/serializers/issue.py (3)
apps/api/plane/space/serializer/issue.py (1)
CommentReactionSerializer(419-423)apps/api/plane/api/serializers/base.py (1)
BaseSerializer(5-117)apps/api/plane/db/models/issue.py (1)
CommentReaction(624-650)
⏰ 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: Build and lint web apps
🔇 Additional comments (1)
apps/web/ce/components/comments/comment-block.tsx (1)
5-6: Imports look correct and align with the updated types/UI kit.
Description
This PR resolves issues with comments and reactions made by external users.
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes