🐛 Fixed reply form showing parent comment author's details#57
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.
Review Summary by QodoFix reply form inheriting parent comment author details
WalkthroughsDescription• Removed fallback pattern that inherited parent comment author's details • Fixed reply form to display current user's name and expertise only • Added comprehensive test coverage for reply form data isolation • Corrected reply submission to use parent comment ID directly Diagramflowchart LR
A["Reply Form Opened"] --> B["Check Current Member Data"]
B --> C["Remove Fallback to Parent Comment"]
C --> D["Display Current User Info Only"]
D --> E["Submit Reply with Correct Parent ID"]
File Changes1. apps/comments-ui/test/e2e/actions.test.ts
|
Code Review by Qodo
1. openForm deref crash
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+313
to
314
| 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.
1. Openform deref crash 🐞 Bug ✓ Correctness
FormWrapper dereferences openForm.in_reply_to_snippet when isOpen is true, but openForm is optional and MainForm renders FormWrapper without it. Focusing/typing in the main comment form will throw a TypeError and break the UI.
Agent Prompt
### Issue description
`FormWrapper` can crash at runtime because it dereferences `openForm.in_reply_to_snippet` even though `openForm` is optional and some call sites don’t pass it.
### Issue Context
`MainForm` renders `<FormWrapper ...>` without `openForm`, but `isOpen` can become `true` when the editor is focused or has content.
### Fix Focus Areas
- apps/comments-ui/src/components/content/forms/form.tsx[311-315]
- apps/comments-ui/src/components/content/forms/main-form.tsx[94-105]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
263
to
+265
| // 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.
2. Expertise blocks editor typing 🐞 Bug ✓ Correctness
Form disables editor editing based on member.expertise instead of member.name, blocking typing for members who have a name but no expertise and allowing typing for members missing a required name. This contradicts the AddDetailsPopup validation that requires a non-empty name but treats expertise as optional.
Agent Prompt
### Issue description
The editor’s editable state is incorrectly gated by `member.expertise`, making expertise effectively required to type and failing to enforce the required `name`.
### Issue Context
- The UI flow and popup validation require a non-empty name.
- Expertise is optional.
### Fix Focus Areas
- apps/comments-ui/src/components/content/forms/form.tsx[258-265]
- apps/comments-ui/src/components/popups/add-details-popup.tsx[38-49]
- apps/comments-ui/src/components/content/forms/form.tsx[355-358]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
29
to
32
| post_id: postId, | ||
| in_reply_to_id: openForm.in_reply_to_id, | ||
| in_reply_to_id: parent.id, | ||
| status: 'published', | ||
| html |
There was a problem hiding this comment.
3. Reply-to-reply misthreaded 🐞 Bug ✓ Correctness
ReplyForm always submits in_reply_to_id as parent.id, ignoring openForm.in_reply_to_id which is set when replying to a reply. This records reply-to-reply posts as replies to the top-level comment and loses the correct in-reply-to linkage/snippet behavior.
Agent Prompt
### Issue description
Replies to replies are mis-threaded because `ReplyForm` always submits `in_reply_to_id` as the top-level `parent.id`.
### Issue Context
`openForm.in_reply_to_id` is intentionally populated when replying to a reply; `parent` is used for the thread root placement/state update.
### Fix Focus Areas
- apps/comments-ui/src/components/content/forms/reply-form.tsx[24-35]
- apps/comments-ui/src/components/content/comment.tsx[103-120]
- apps/comments-ui/src/components/content/comment.tsx[248-252]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from agentic-review-benchmarks#2