like feature to comments - #38
Open
dinesh-2047 wants to merge 3 commits into
Open
Conversation
|
@dinesh-2047 is attempting to deploy a commit to the sohaib's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive comment liking system with like state enrichment across comment operations, extends the repost model to support video reposts alongside tweets, and implements a new following feed endpoint that aggregates videos and tweets from subscribed channels with enriched metadata. Backend routes and frontend components are updated accordingly to support these features. Changes
Sequence DiagramssequenceDiagram
actor User
participant Frontend
participant API as Backend API
participant DB as Database
participant Notify as Notifications
User->>Frontend: Click like on comment
Frontend->>API: PATCH /likes/comments/{commentId}/toggle
API->>DB: Check Like document (likedBy + commentId)
alt Like exists
API->>DB: Remove Like document
else Like doesn't exist
API->>DB: Create Like document
API->>Notify: Create LIKE notification to comment owner
Notify->>Notify: Emit socket event
end
API->>DB: Aggregate like count for comment
API->>Frontend: Return updated likeCount & isLiked
Frontend->>User: Update UI with new like state
sequenceDiagram
actor User
participant Frontend
participant API as Backend API
participant DB as Database
User->>Frontend: Request following feed
Frontend->>API: GET /subscriptions/feed
API->>DB: Fetch user subscriptions
API->>DB: Query videos from subscribed channels
API->>DB: Query tweets from subscribed channels
API->>DB: Fetch reposts & watch-later for context
API->>API: Merge & paginate videos + tweets by createdAt
API->>API: Enrich with isReposted, repostCount, isInWatchLater
API->>Frontend: Return paginated feedItems with type & metadata
Frontend->>User: Display mixed video/tweet feed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/models/repost.model.js (1)
53-73:⚠️ Potential issue | 🔴 CriticalCritical: Duplicate schema definitions will break video reposts.
The file contains two conflicting
repostSchemadefinitions. The second definition (lines 55-69) overwrites the first and exports a schema wheretweetis required, which will cause video reposts to fail validation.The static analyzer flagged this correctly - the
Repostexport on line 73 redeclares and overwrites the export on line 52.Remove lines 53-73 entirely to retain only the new polymorphic schema that supports both video and tweet targets.
🔧 Fix: Remove duplicate schema
export const Repost = mongoose.model('Repost', repostSchema) -import mongoose from "mongoose"; - -const repostSchema = new mongoose.Schema( - { - tweet: { - type: mongoose.Schema.Types.ObjectId, - ref: "Tweet", - required: true, - }, - repostedBy: { - type: mongoose.Schema.Types.ObjectId, - ref: "User", - required: true, - }, - }, - { timestamps: true } -); - -repostSchema.index({ tweet: 1, repostedBy: 1 }, { unique: true }); - -export const Repost = mongoose.model("Repost", repostSchema);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/models/repost.model.js` around lines 53 - 73, There are two conflicting repostSchema definitions causing the earlier schema (with tweet required) to overwrite the polymorphic one; remove the duplicate older schema block and its export so only the polymorphic repostSchema and its mongoose.model("Repost", repostSchema) export remain; specifically delete the redundant repostSchema declaration and the export that reference the required tweet field so the final Repost model uses the polymorphic schema.
🧹 Nitpick comments (4)
backend/src/models/repost.model.js (1)
32-50: Consider using$type: 'objectId'for consistency with like.model.js.The partial filter expressions use
{ $exists: true }, but since the fields can beundefined(not set) rather than explicitlynull, this works. However,like.model.jsuses{ $type: 'objectId' }which is more explicit. Consider aligning the patterns for consistency.♻️ Align with like.model.js pattern
repostSchema.index( { repostedBy: 1, video: 1 }, { unique: true, partialFilterExpression: { - video: { $exists: true }, + video: { $type: 'objectId' }, }, }, ) repostSchema.index( { repostedBy: 1, tweet: 1 }, { unique: true, partialFilterExpression: { - tweet: { $exists: true }, + tweet: { $type: 'objectId' }, }, }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/models/repost.model.js` around lines 32 - 50, The partialFilterExpression for the two repostSchema.index calls should use $type: 'objectId' instead of $exists to match like.model.js and be more explicit: update the partialFilterExpression objects in the repostSchema.index invocations (the ones targeting {repostedBy:1, video:1} and {repostedBy:1, tweet:1}) to check video and tweet with { $type: 'objectId' } (and keep unique: true and the same keys) so the indexes only apply when those fields are actual ObjectId values.backend/src/controllers/comment.controller.js (1)
190-191: Delete order leaves orphan likes on failure.If
Like.deleteManythrows aftercomment.deleteOne()succeeds, orphan Like documents referencing the deleted comment will persist. This matches the existing pattern intweet.controller.js(line 241-242), so it's consistent with the codebase, but worth noting.Consider wrapping in a transaction for atomicity if data consistency becomes a concern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/controllers/comment.controller.js` around lines 190 - 191, Currently comment.deleteOne() runs before Like.deleteMany(), which can leave orphaned Like documents if the second op fails; fix by performing both deletions within a mongoose transaction: obtain a session via mongoose.startSession(), session.startTransaction(), call Like.deleteMany({ comment: comment._id }).session(session) and comment.deleteOne({ session }) (or Model.deleteOne with session), then await session.commitTransaction() and session.endSession(); on error call session.abortTransaction() and session.endSession(). This ensures atomicity for the comment.deleteOne() and Like.deleteMany() operations.backend/src/controllers/subscription.controller.js (2)
281-290: Consider removing redundantvideosandtweetsfrom response.The response includes both
items(the paginated feed) and fullvideos/tweetsarrays. Sinceitemsalready contains the video/tweet data wrapped with type discriminator, including the separate arrays duplicates data and increases payload size.If the frontend only needs the paginated feed, consider:
{ items: pagedItems, - videos, - tweets, channels: followedChannels, page, limit, totalItems: feedItems.length, hasMore: sliceStart + limit < feedItems.length, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/controllers/subscription.controller.js` around lines 281 - 290, The response object currently returns duplicate payloads by including both the paginated feed (items/pagedItems) and the full videos and tweets arrays; remove the redundant properties by deleting videos and tweets from the returned object (keep items: pagedItems, channels: followedChannels, page, limit, totalItems, hasMore) in the controller response where the object is constructed, and ensure any consumer references to videos or tweets are updated to read from items (or map items to their underlying data) before deployment.
203-206: Pagination scalability concern withfetchCount = page * limit.The current approach fetches
page * limititems from each collection. On page 10 with limit 12, this fetches 120 items from each collection. This grows linearly with page number.For an MVP this is acceptable, but consider cursor-based pagination (using
createdAtas cursor) for better scalability if this feed grows large.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/controllers/subscription.controller.js` around lines 203 - 206, The current pagination computes fetchCount = page * limit and sliceStart = (page - 1) * limit which causes the DB to fetch all prior pages for each request; change to cursor-based pagination (use createdAt as the cursor) so each call only retrieves up to limit items. Specifically, replace the fetchCount/sliceStart approach in the controller with logic that accepts a cursor (e.g., req.query.cursor or lastCreatedAt) and queries each collection with a filter like { createdAt: { $lt: cursor } } (or $gt for ascending), sorted by createdAt and limited by limit, removing the need to multiply page*limit; ensure the API returns the next cursor (the last returned createdAt) and update any references to fetchCount/sliceStart accordingly (look for variables fetchCount, sliceStart, page, limit).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/controllers/like.controller.js`:
- Line 138: The notification URL construction uses `url:
\`/watch/${comment.video?._id || ''}\`` which produces `/watch/` when
`comment.video` is null; modify the logic in like.controller.js to first check
that `comment.video` and `comment.video._id` exist and either skip
creating/sending the notification when the video no longer exists or populate
`url` with a safe fallback (e.g., null/empty/overview path) and ensure
downstream code handles that case; update the block that builds the notification
payload so it only sets the `/watch/...` URL when `comment.video._id` is
present.
---
Outside diff comments:
In `@backend/src/models/repost.model.js`:
- Around line 53-73: There are two conflicting repostSchema definitions causing
the earlier schema (with tweet required) to overwrite the polymorphic one;
remove the duplicate older schema block and its export so only the polymorphic
repostSchema and its mongoose.model("Repost", repostSchema) export remain;
specifically delete the redundant repostSchema declaration and the export that
reference the required tweet field so the final Repost model uses the
polymorphic schema.
---
Nitpick comments:
In `@backend/src/controllers/comment.controller.js`:
- Around line 190-191: Currently comment.deleteOne() runs before
Like.deleteMany(), which can leave orphaned Like documents if the second op
fails; fix by performing both deletions within a mongoose transaction: obtain a
session via mongoose.startSession(), session.startTransaction(), call
Like.deleteMany({ comment: comment._id }).session(session) and
comment.deleteOne({ session }) (or Model.deleteOne with session), then await
session.commitTransaction() and session.endSession(); on error call
session.abortTransaction() and session.endSession(). This ensures atomicity for
the comment.deleteOne() and Like.deleteMany() operations.
In `@backend/src/controllers/subscription.controller.js`:
- Around line 281-290: The response object currently returns duplicate payloads
by including both the paginated feed (items/pagedItems) and the full videos and
tweets arrays; remove the redundant properties by deleting videos and tweets
from the returned object (keep items: pagedItems, channels: followedChannels,
page, limit, totalItems, hasMore) in the controller response where the object is
constructed, and ensure any consumer references to videos or tweets are updated
to read from items (or map items to their underlying data) before deployment.
- Around line 203-206: The current pagination computes fetchCount = page * limit
and sliceStart = (page - 1) * limit which causes the DB to fetch all prior pages
for each request; change to cursor-based pagination (use createdAt as the
cursor) so each call only retrieves up to limit items. Specifically, replace the
fetchCount/sliceStart approach in the controller with logic that accepts a
cursor (e.g., req.query.cursor or lastCreatedAt) and queries each collection
with a filter like { createdAt: { $lt: cursor } } (or $gt for ascending), sorted
by createdAt and limited by limit, removing the need to multiply page*limit;
ensure the API returns the next cursor (the last returned createdAt) and update
any references to fetchCount/sliceStart accordingly (look for variables
fetchCount, sliceStart, page, limit).
In `@backend/src/models/repost.model.js`:
- Around line 32-50: The partialFilterExpression for the two repostSchema.index
calls should use $type: 'objectId' instead of $exists to match like.model.js and
be more explicit: update the partialFilterExpression objects in the
repostSchema.index invocations (the ones targeting {repostedBy:1, video:1} and
{repostedBy:1, tweet:1}) to check video and tweet with { $type: 'objectId' }
(and keep unique: true and the same keys) so the indexes only apply when those
fields are actual ObjectId values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a50b189-8be2-402e-89c0-b7c539bc4103
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
backend/src/controllers/comment.controller.jsbackend/src/controllers/like.controller.jsbackend/src/controllers/subscription.controller.jsbackend/src/models/like.model.jsbackend/src/models/repost.model.jsbackend/src/routes/comment.route.jsbackend/src/routes/like.route.jsbackend/src/routes/subscription.route.jsfrontend/src/api/index.jsfrontend/src/pages/videoPlayer.jsx
| sender: req.user._id, | ||
| type: 'LIKE', | ||
| message: `${req.user.username} liked your comment`, | ||
| url: `/watch/${comment.video?._id || ''}`, |
There was a problem hiding this comment.
Notification URL may be malformed if video was deleted.
If the commented video has been deleted, comment.video will be null after population, resulting in a notification URL of /watch/ which leads nowhere.
Consider adding a fallback or skipping the notification when the video no longer exists:
🛡️ Suggested defensive check
- url: `/watch/${comment.video?._id || ''}`,
+ url: comment.video?._id ? `/watch/${comment.video._id}` : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/controllers/like.controller.js` at line 138, The notification URL
construction uses `url: \`/watch/${comment.video?._id || ''}\`` which produces
`/watch/` when `comment.video` is null; modify the logic in like.controller.js
to first check that `comment.video` and `comment.video._id` exist and either
skip creating/sending the notification when the video no longer exists or
populate `url` with a safe fallback (e.g., null/empty/overview path) and ensure
downstream code handles that case; update the block that builds the notification
payload so it only sets the `/watch/...` URL when `comment.video._id` is
present.
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
1 participant
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.
#24 solved
Summary by CodeRabbit
Release Notes