Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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]);
Comment on lines +264 to +265
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Verify editor editability gating on expertise (could block users without expertise).
This now disables editing for members who haven’t set expertise. If expertise is optional, users may be unable to type. Also the comment above mentions “name,” so either the logic or comment is out of sync. Please confirm the intended requirement.

🔧 Suggested adjustment (if editability should depend on name as before)
-        editor.setEditable(!!member?.expertise && progress !== 'sending');
+        editor.setEditable(!!member?.name && progress !== 'sending');
📝 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
editor.setEditable(!!member?.expertise && progress !== 'sending');
}, [editor, member, progress]);
editor.setEditable(!!member?.name && progress !== 'sending');
}, [editor, member, progress]);
🤖 Prompt for AI Agents
In `@apps/comments-ui/src/components/content/forms/form.tsx` around lines 264 -
265, The editability check currently gates editor.setEditable on
member?.expertise which will prevent users without expertise from typing; verify
whether editability should depend on member.name (as the comment implied) or on
expertise; then update the condition and dependency array in the effect
accordingly (e.g., use !!member?.name && progress !== 'sending' if name is
required, or remove the expertise check entirely if expertise is optional) and
update the nearby comment to reflect the chosen gating rule; ensure you
reference editor.setEditable, member, and progress when making the change.


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 311 to 314
Copy link

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 file to understand the context around lines 311-314
wc -l apps/comments-ui/src/components/content/forms/form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 142


🏁 Script executed:

# Read the relevant section and surrounding context
sed -n '290,330p' apps/comments-ui/src/components/content/forms/form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 1204


🏁 Script executed:

# Find the component's prop definition to check if openForm is optional
rg -n "interface.*Props|type.*Props|openForm.*:" apps/comments-ui/src/components/content/forms/form.tsx -A 20 -B 2

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 3881


🏁 Script executed:

# Find all FormWrapper usages to verify the issue
rg -n "FormWrapper" apps/comments-ui/src --type=tsx -B 2 -A 5

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 111


🏁 Script executed:

# Also check if OpenCommentForm type definition shows required vs optional fields
rg -n "OpenCommentForm" apps/comments-ui/src --type=tsx -B 2 -A 10

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 111


🏁 Script executed:

# Find all FormWrapper usages
rg -n "FormWrapper" apps/comments-ui/src -B 2 -A 5

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 6049


🏁 Script executed:

# Find OpenCommentForm type definition
rg -n "OpenCommentForm" apps/comments-ui/src -B 2 -A 10

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 17498


🏁 Script executed:

# Let's examine the main-form.tsx file more closely to understand the isOpen state
cat -n apps/comments-ui/src/components/content/forms/main-form.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 4629


Guard optional openForm access to avoid runtime crashes.
openForm is optional in FormWrapperProps, but line 313 accesses openForm.in_reply_to_snippet without a null check. MainForm (line 98) passes isOpen={true} without providing openForm, which will crash at runtime when the code attempts to read the property.

Proposed 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
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]';
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 311 -
314, The code reads openForm.in_reply_to_snippet when computing openStyles even
though openForm is optional in FormWrapperProps and MainForm can set isOpen
without openForm; guard that access by checking openForm exists (e.g., use
optional chaining or an explicit null check) before reading in_reply_to_snippet
in the block that sets openStyles (the isOpen branch), so replace the direct
access with a safe check (references: openForm, in_reply_to_snippet, isOpen,
openStyles, FormWrapperProps, MainForm).

}

Expand Down
8 changes: 4 additions & 4 deletions apps/comments-ui/src/components/content/forms/reply-form.tsx
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
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