fix(web): restrict comment deletion to the author's own replies#1936
Conversation
|
Paragon Review Skipped Hi @richiemcilroy! Your Polarity credit balance is insufficient to complete this review. Please visit https://app.paragon.run to finish your review. |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
| // Delete the comment and only the replies authored by the same user. | ||
| // Replies written by other users are preserved (never delete others' | ||
| // content), even though that can leave them visually orphaned. | ||
| await db() | ||
| .delete(comments) | ||
| .where( | ||
| or( | ||
| eq(comments.id, commentId.value), | ||
| eq(comments.parentCommentId, commentId.value), | ||
| and( | ||
| eq(comments.authorId, user.id), | ||
| or( | ||
| eq(comments.id, commentId.value), | ||
| eq(comments.parentCommentId, commentId.value), | ||
| ), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Orphaned replies may be invisible or error-prone in the UI
When the parent comment is deleted but other users' replies survive (with parentCommentId pointing to the now-deleted row), the frontend needs to handle this case explicitly. If the UI renders replies by first looking up the parent from local state or a cached result set, orphaned replies could either silently disappear or trigger a rendering error depending on how null/missing parents are handled. The code comment acknowledges this trade-off, but it's worth confirming the front-end comment list component guards against a missing parent before this ships.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/api/video/comment/delete/route.ts
Line: 44-57
Comment:
**Orphaned replies may be invisible or error-prone in the UI**
When the parent comment is deleted but other users' replies survive (with `parentCommentId` pointing to the now-deleted row), the frontend needs to handle this case explicitly. If the UI renders replies by first looking up the parent from local state or a cached result set, orphaned replies could either silently disappear or trigger a rendering error depending on how null/missing parents are handled. The code comment acknowledges this trade-off, but it's worth confirming the front-end comment list component guards against a missing parent before this ships.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // Delete the comment and all its replies | ||
| // Delete the comment and only the replies authored by the same user. | ||
| // Replies written by other users are preserved (never delete others' | ||
| // content), even though that can leave them visually orphaned. |
There was a problem hiding this comment.
Nit: this is only deleting direct replies (same as before), so the comment may read a bit stronger than the behavior. Also worth calling out that preserving other users’ replies means they’ll reference a deleted parent unless you soft-delete the parent or re-parent them.
| // content), even though that can leave them visually orphaned. | |
| // Delete the comment and only direct replies authored by the same user. | |
| // Replies written by other users are preserved (never delete others' content), | |
| // which can leave them visually orphaned unless you soft-delete the parent. |
|
@greptileai please review the PR |
Restricts comment deletion so a user can only delete their own comment and their own replies, instead of also deleting replies authored by other users.
Greptile Summary
This PR narrows the comment-delete query to only remove replies authored by the same user, rather than all replies to the deleted parent comment. The ownership pre-check (lines 27–42) that guards access to the parent comment is unchanged.
DELETEquery now wraps the existingor(id = X, parentCommentId = X)predicate insideand(authorId = user.id, …), preventing other users' replies from being wiped when a parent comment is deleted.Confidence Score: 5/5
Safe to merge — the change is a single, targeted predicate addition that correctly restricts bulk deletion to the requesting user's own content.
The existing ownership pre-check (verify the parent comment belongs to the caller before proceeding) is untouched. The new
authorId = user.idconstraint in the delete query is the minimal, correct fix: it scopes the bulk delete to the caller's own replies without opening any new surface for data loss or privilege escalation.No files require special attention.
Important Files Changed
authorId = user.idguard to the bulk-delete query so only the requesting user's own replies are removed alongside a deleted parent comment; ownership of the parent is already enforced by the pre-check at line 27.Reviews (2): Last reviewed commit: "fix(web): restrict comment deletion to t..." | Re-trigger Greptile