🐛 Fixed reply form showing parent comment author's details#2
Conversation
ref https://linear.app/ghost/issue/BER-3212 When a user opened a reply form, the form header was showing the parent comment author's name and expertise instead of the current user's data. This happened because the code used a fallback pattern that pulled from the comment being replied to when the current member had no value set.
📝 WalkthroughWalkthroughThe changes refactor the comment form system by eliminating the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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
🤖 Fix all issues with AI agents
In `@apps/comments-ui/src/components/content/forms/form.tsx`:
- Around line 263-265: The editability check currently uses member?.expertise in
editor.setEditable but focusEditor only checks memberName, causing the editor to
become read-only for users who have a name but no expertise; update the gating
in the effect that calls editor.setEditable to match the name-based requirement
used by focusEditor (e.g., use memberName or member?.name and progress !==
'sending'), or otherwise update focusEditor to enforce the same expertise
requirement—ensure you modify the same effect surrounding editor.setEditable and
reference the member/memberName and progress values consistently.
- Around line 308-314: When computing openStyles in the FormWrapper (inside
form.tsx) you must guard access to openForm.in_reply_to_snippet because openForm
is optional in FormWrapperProps and may be undefined when isOpen is true; change
the access to use optional chaining (e.g., compute isReplyToReply using
openForm?.in_reply_to_snippet) or add an explicit null check on openForm before
reading in_reply_to_snippet so the isReplyToReply/ openStyles logic never reads
a property from undefined.
| // Disable editing if the member doesn't have a name or when we are submitting the form | ||
| editor.setEditable(!!memberName && progress !== 'sending'); | ||
| }, [editor, memberName, progress]); | ||
| editor.setEditable(!!member?.expertise && progress !== 'sending'); | ||
| }, [editor, member, progress]); |
There was a problem hiding this comment.
Editability gate now blocks users with name but no expertise.
focusEditor only checks memberName, but setEditable now requires member?.expertise, which can leave the editor read‑only with no prompt. If expertise isn’t intended to be mandatory, this is a regression.
🔧 Suggested fix (align gating with name requirement)
- // Disable editing if the member doesn't have a name or when we are submitting the form
- editor.setEditable(!!member?.expertise && progress !== 'sending');
- }, [editor, member, progress]);
+ // Disable editing if the member doesn't have a name or when we are submitting the form
+ editor.setEditable(!!memberName && progress !== 'sending');
+ }, [editor, memberName, progress]);📝 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.
| // Disable editing if the member doesn't have a name or when we are submitting the form | |
| editor.setEditable(!!memberName && progress !== 'sending'); | |
| }, [editor, memberName, progress]); | |
| editor.setEditable(!!member?.expertise && progress !== 'sending'); | |
| }, [editor, member, progress]); | |
| // Disable editing if the member doesn't have a name or when we are submitting the form | |
| editor.setEditable(!!memberName && progress !== 'sending'); | |
| }, [editor, memberName, progress]); |
🤖 Prompt for AI Agents
In `@apps/comments-ui/src/components/content/forms/form.tsx` around lines 263 -
265, The editability check currently uses member?.expertise in
editor.setEditable but focusEditor only checks memberName, causing the editor to
become read-only for users who have a name but no expertise; update the gating
in the effect that calls editor.setEditable to match the name-based requirement
used by focusEditor (e.g., use memberName or member?.name and progress !==
'sending'), or otherwise update focusEditor to enforce the same expertise
requirement—ensure you modify the same effect surrounding editor.setEditable and
reference the member/memberName and progress values consistently.
| const memberName = member?.name; | ||
| const memberExpertise = member?.expertise; | ||
|
|
||
| let openStyles = ''; | ||
| if (isOpen) { | ||
| const isReplyToReply = !!openForm?.in_reply_to_snippet; | ||
| const isReplyToReply = !!openForm.in_reply_to_snippet; | ||
| openStyles = isReplyToReply ? 'pl-[1px] pt-[68px] sm:pl-[44px] sm:pt-[56px]' : 'pl-[1px] pt-[48px] sm:pl-[44px] sm:pt-[40px]'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd apps/comments-ui && find . -name "form.tsx" -type fRepository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 134
🏁 Script executed:
# First, let's look at the FormWrapper component itself
rg -n "FormWrapper" --type=tsx -lRepository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 122
🏁 Script executed:
# Search for FormWrapper usages as suggested in the review
rg -n "FormWrapper" --type=tsx -A 2 -B 2Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 122
🏁 Script executed:
# Try with ts file type
rg -n "FormWrapper" --type=ts -A 2 -B 2Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 4108
🏁 Script executed:
# Also try with extension pattern
rg -n "FormWrapper" --glob="*.tsx" -A 2 -B 2Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 4108
🏁 Script executed:
# Look at the form.tsx file to understand the component
head -100 apps/comments-ui/src/components/content/forms/form.tsxRepository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 3462
🏁 Script executed:
# Check the FormWrapperProps type definition more carefully
sed -n '291,297p' apps/comments-ui/src/components/content/forms/form.tsxRepository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 256
🏁 Script executed:
# Now let's look at the problematic code section around lines 308-314
sed -n '305,320p' apps/comments-ui/src/components/content/forms/form.tsxRepository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 618
🏁 Script executed:
# Check main-form.tsx to see when isOpen can be true
cat apps/comments-ui/src/components/content/forms/main-form.tsx | head -110Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 3870
🏁 Script executed:
# Check if there are any other usages of FormWrapper we might have missed
rg -n "FormWrapper" --glob="*.tsx" -B 1 -A 1Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 2792
🏁 Script executed:
# Verify what openForm is (the type)
rg -n "OpenCommentForm" --glob="*.ts" --glob="*.tsx" -A 3 | head -50Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 3910
🏁 Script executed:
# Let me get the full OpenCommentForm type definition
rg -n "export type OpenCommentForm" --glob="*.ts" -A 10Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 768
🏁 Script executed:
# Let me check the full FormWrapper implementation to be thorough
sed -n '299,330p' apps/comments-ui/src/components/content/forms/form.tsxRepository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 1051
Use optional chaining to safely access openForm.in_reply_to_snippet when isOpen is true.
openForm is optional in FormWrapperProps, but in main-form.tsx the component is called with isOpen potentially true without providing openForm. This will cause a runtime error when trying to access openForm.in_reply_to_snippet at line 313.
Fix
- const isReplyToReply = !!openForm.in_reply_to_snippet;
+ const isReplyToReply = !!openForm?.in_reply_to_snippet;📝 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.
| const memberName = member?.name; | |
| const memberExpertise = member?.expertise; | |
| let openStyles = ''; | |
| if (isOpen) { | |
| const isReplyToReply = !!openForm?.in_reply_to_snippet; | |
| const isReplyToReply = !!openForm.in_reply_to_snippet; | |
| openStyles = isReplyToReply ? 'pl-[1px] pt-[68px] sm:pl-[44px] sm:pt-[56px]' : 'pl-[1px] pt-[48px] sm:pl-[44px] sm:pt-[40px]'; | |
| const memberName = member?.name; | |
| const memberExpertise = member?.expertise; | |
| let openStyles = ''; | |
| if (isOpen) { | |
| const isReplyToReply = !!openForm?.in_reply_to_snippet; | |
| openStyles = isReplyToReply ? 'pl-[1px] pt-[68px] sm:pl-[44px] sm:pt-[56px]' : 'pl-[1px] pt-[48px] sm:pl-[44px] sm:pt-[40px]'; |
🤖 Prompt for AI Agents
In `@apps/comments-ui/src/components/content/forms/form.tsx` around lines 308 -
314, When computing openStyles in the FormWrapper (inside form.tsx) you must
guard access to openForm.in_reply_to_snippet because openForm is optional in
FormWrapperProps and may be undefined when isOpen is true; change the access to
use optional chaining (e.g., compute isReplyToReply using
openForm?.in_reply_to_snippet) or add an explicit null check on openForm before
reading in_reply_to_snippet so the isReplyToReply/ openStyles logic never reads
a property from undefined.
2 participants
Benchmark PR from agentic-review-benchmarks#2
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.