Repository navigation
fix(web): require reply parents on the same video for web comment actions - #2918
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
This branch was successfully deployed
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
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.
the web comment actions took the reply parent id as given. someone allowed to comment on one video could reply to a comment from a different video, which linked comments across videos and notified that other comment's author. same issue #2892 fixes for the mobile endpoint.
newCommentandnewMediaCommentnow check, after the existing video access check and before any insert or notification, that a reply's parent exists on the same video. a bad parent throwsComment not found. top level comments (""parent) skip the lookup. the existing auth, upload token, media key and plan checks are unchanged.one deliberate difference from #2892: the web check doesn't require the parent to be a text comment. the share page activity tab shows a reply button on every root card, including emoji reactions and video/audio comments, so replies to those are legitimate on web today and still work. the check lives in
apps/web/lib/comment-reply-parent.ts. #2892's check is inline in the mobile route, so nothing could be reused, and this PR doesn't touch that route. the two merge cleanly in either order.tests:
apps/web/__tests__/unit/comment-reply-parent.test.tsruns both actions against a fake db that evaluates the real drizzle where clause. it covers parents on another video, missing parents, replies to text/emoji/video/audio parents on the same video, and top level comments. the rejection cases fail on main. the test is added to the recording-reliability vitest allowlist.
The PR appears safe to merge; both web actions reject parents from other videos before saving replies or sending notifications.Summary
Adds a shared same-video parent check to
newCommentandnewMediaComment.No actionable issues found. Tests were reviewed but not run.
Acknowledged intent: richiemcilroy explicitly states that replies to non-text parents remain allowed because the web activity tab supports them.
Reviews (1) · Last reviewed commit: "fix(web): require reply parents on the s..." · Reviewed by Greptile