Skip to content

🐛 Fixed reply form showing parent comment author's details #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tomerqodo wants to merge 2 commits into
base: coderabbit_full_base__fixed_reply_form_showing_parent_comment_authors_details_pr2
Choose a base branch
from
Open

🐛 Fixed reply form showing parent comment author's details #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64cd26d
🐛 Fixed reply form showing parent comment author's details
rob-ghost Jan 15, 2026
e01e8b1
update pr
tomerqodo Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions apps/comments-ui/src/components/content/forms/form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ const Form: React.FC<FormProps> = ({
const [progress, setProgress] = useState<Progress>('default');
const formEl = useRef(null);

const memberName = member?.name ?? comment?.member?.name;
const memberName = member?.name;

if (progress === 'sending' || (memberName && isAskingDetails)) {
// Force open
Expand All @@ -261,8 +261,8 @@ const Form: React.FC<FormProps> = ({
}

// 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]);

return (
<form
Expand All @@ -289,7 +289,6 @@ const Form: React.FC<FormProps> = ({
};

type FormWrapperProps = {
comment?: Comment;
editor: Editor | null;
isOpen: boolean;
reduced: boolean;
Expand All @@ -298,7 +297,6 @@ type FormWrapperProps = {
};

const FormWrapper: React.FC<FormWrapperProps> = ({
comment,
editor,
isOpen,
reduced,
Expand All @@ -307,12 +305,12 @@ const FormWrapper: React.FC<FormWrapperProps> = ({
}) => {
const {member, dispatchAction} = useAppContext();

const memberName = member?.name ?? comment?.member?.name;
const memberExpertise = member?.expertise ?? comment?.member?.expertise;
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]';
Comment on lines +308 to 314

@coderabbitai coderabbitai Bot Jan 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's read the form.tsx file to understand the component props and structure
head -100 apps/comments-ui/src/components/content/forms/form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 3451

🏁 Script executed:

# Check the full file to find prop type definitions
wc -l apps/comments-ui/src/components/content/forms/form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 142

🏁 Script executed:

# Read specific sections around line 308 and look for prop definitions
sed -n '1,50p' apps/comments-ui/src/components/content/forms/form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 2090

🏁 Script executed:

# Now let's look at the specific lines mentioned in the review (308-314)
sed -n '300,320p' apps/comments-ui/src/components/content/forms/form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 671

🏁 Script executed:

# Also look for the FormWrapper component being imported and used
rg "FormWrapper" -n -B 2 -A 2

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 4097

🏁 Script executed:

# Let's check the FormWrapperProps type definition
sed -n '291,298p' apps/comments-ui/src/components/content/forms/form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 246

🏁 Script executed:

# Check the main-form.tsx to see what isOpen value is passed to FormWrapper
cat -n apps/comments-ui/src/components/content/forms/main-form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 4629

🏁 Script executed:

# Let's verify the structure around line 308 in form.tsx more carefully
sed -n '295,320p' apps/comments-ui/src/components/content/forms/form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 789

🏁 Script executed:

# Check if there are any guards or checks before the problematic code
sed -n '290,330p' apps/comments-ui/src/components/content/forms/form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 1204

Fix potential undefined access: guard openForm before accessing its properties.

In main-form.tsx, FormWrapper is called with isOpen={isOpen} but without the openForm prop. Since isOpen can be true while openForm is undefined, accessing openForm.in_reply_to_snippet at line 313 will throw at runtime.

Use optional chaining to safely access the property:

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.

Suggested change
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, Guard access to openForm when computing openStyles: replace the direct
property access openForm.in_reply_to_snippet with a safe check (e.g., use
optional chaining openForm?.in_reply_to_snippet or explicitly check openForm
truthiness) so that when isOpen is true but openForm is undefined you don't
throw; update the computation in the component where openStyles is set
(referencing openForm, isOpen, and openStyles in form.tsx / FormWrapper usage)
to only read in_reply_to_snippet if openForm is defined.

}

Expand Down
8 changes: 4 additions & 4 deletions apps/comments-ui/src/components/content/forms/reply-form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ const ReplyForm: React.FC<Props> = ({openForm, parent}) => {
parent: parent,
reply: {
post_id: postId,
in_reply_to_id: openForm.in_reply_to_id,
in_reply_to_id: parent.id,
status: 'published',
html
}
});
}, [parent, postId, openForm, dispatchAction]);
}, [parent, postId, dispatchAction]);

const close = useCallback(() => {
dispatchAction('closeCommentForm', openForm.id);
Expand All @@ -44,8 +44,8 @@ const ReplyForm: React.FC<Props> = ({openForm, parent}) => {

return (
<div ref={setForm} data-testid="reply-form">
<div className='mt-[-16px] pr-2'>
<FormWrapper comment={parent} editor={editor} isOpen={true} openForm={openForm} reduced={isMobile()}>
<div className='pr-2 mt-[-16px]'>
<FormWrapper editor={editor} isOpen={true} openForm={openForm} reduced={isMobile()}>
<Form
close={close}
editor={editor}
Expand Down
68 changes: 68 additions & 0 deletions apps/comments-ui/test/e2e/actions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,74 @@ test.describe('Actions', async () => {
);
});

test('Reply form does not inherit expertise from parent comment author', async ({page}) => {
// Set logged-in member with NO expertise
mockedApi.setMember({name: 'Jane Replier', expertise: null});

// Add a comment from a different member who HAS expertise
mockedApi.addComment({
html: '<p>This is a comment from someone with expertise</p>',
member: buildMember({name: 'Expert User', expertise: 'Head of Comments'})
});

const {frame} = await initializeTest(page);

// Verify the parent comment shows the author's expertise
const parentComment = frame.getByTestId('comment-component').nth(0);
await expect(parentComment).toContainText('Head of Comments');

// Click reply on the parent comment
const replyButton = parentComment.getByTestId('reply-button');
await replyButton.click();

// Wait for reply form to appear
const replyForm = frame.getByTestId('reply-form');
await expect(replyForm).toBeVisible();

const editor = replyForm.getByTestId('form-editor');
await waitEditorFocused(editor);

// The reply form should show the logged-in member's info, NOT the parent's
await expect(replyForm.getByTestId('member-name')).toHaveText('Jane Replier');

// The expertise button should show "Add your expertise" since the
// logged-in member has no expertise - it should NOT show "Head of Comments"
const expertiseButton = replyForm.getByTestId('expertise-button');
await expect(expertiseButton).toBeVisible();
await expect(expertiseButton).toHaveText('·Add your expertise');
});

test('Reply form does not inherit name from parent comment author', async ({page}) => {
// Set logged-in member with NO name (null triggers the fallback bug)
mockedApi.setMember({name: null, expertise: 'Some Expertise'});

// Add a comment from a different member who HAS a name
mockedApi.addComment({
html: '<p>This is a comment from someone with a name</p>',
member: buildMember({name: 'Named Author', expertise: null})
});

const {frame} = await initializeTest(page);

// Verify the parent comment shows the author's name
const parentComment = frame.getByTestId('comment-component').nth(0);
await expect(parentComment).toContainText('Named Author');

// Click reply on the parent comment
const replyButton = parentComment.getByTestId('reply-button');
await replyButton.click();

// Wait for reply form to appear
const replyForm = frame.getByTestId('reply-form');
await expect(replyForm).toBeVisible();

// The reply form should NOT show the parent comment author's name
// It should either show nothing, "Anonymous", or prompt for a name
// but definitely NOT "Named Author"
const memberName = replyForm.getByTestId('member-name');
await expect(memberName).not.toHaveText('Named Author');
});

async function deleteComment(page, frame, commentComponent) {
await commentComponent.getByTestId('more-button').first().click();
await frame.getByTestId('delete').click();
Expand Down