-
-
Notifications
You must be signed in to change notification settings - Fork 144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Debouncing liking a post in the hackiest way imaginable #1055
Debouncing liking a post in the hackiest way imaginable #1055
Conversation
@JohnAllenTech is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request primarily focus on the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
.where(eq(post.id, postId)); | ||
) | ||
.returning(); | ||
if (res.length !== 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially we are ensuring there is a like to remove before be decrement the like count stored on the post table.
Previously even if no like was found we decremented the like count of the post. This lead to the ability to have negative likes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should work. Could the same happen in the positive way (eg giving infinite likes?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No because theres a foreign key constraint that prevents a user creating two 'like' rows for the same post. This causes the transaction to fail.
For some reason when you try to delete something that doesnt exist it doesnt throw an error and instead just goes 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
server/api/router/post.ts (2)
221-230
: Approve changes with suggestions for further improvementsThe changes effectively address the issue of decreasing like counts during rapid clicking by ensuring that the post's like count is only decremented when a like is actually deleted. This is a good improvement to the existing logic.
However, there are a few points to consider for further enhancement:
Concurrent operations: The current implementation might still face race conditions if multiple users are liking/unliking the same post simultaneously. Consider using a more robust approach, such as optimistic locking or a compare-and-swap operation to ensure consistency.
Client-side debouncing: The PR description mentions attempts to debounce the
useMutation
function. While this server-side fix helps, implementing client-side debouncing would provide a better user experience by preventing rapid successive calls to the server.To address these points:
- For handling concurrent operations, consider using a transaction with a SELECT ... FOR UPDATE statement to lock the row before updating:
await ctx.db.transaction(async (tx) => { const [postToUpdate] = await tx .select() .from(post) .where(eq(post.id, postId)) .forUpdate(); if (!postToUpdate) { throw new TRPCError({ code: 'NOT_FOUND', message: 'Post not found', }); } res = await tx .delete(like) .where( and( eq(like.postId, postId), eq(like.userId, ctx.session?.user?.id), ), ) .returning(); if (res.length !== 0) { await tx .update(post) .set({ likes: postToUpdate.likes - 1, }) .where(eq(post.id, postId)); } });
- For client-side debouncing, implement a debounce function in your frontend code where the
useMutation
is called. This will help prevent rapid successive calls to the server.Would you like assistance in implementing these suggestions or creating a separate issue to track these improvements?
Line range hint
1-230
: Summary and Next StepsThe changes in this file effectively address the immediate issue of decreasing like counts during rapid clicking. The implementation ensures that the post's like count is only decremented when a like is actually deleted, which is a good improvement to the existing logic.
However, there are opportunities for further enhancements:
- Implement more robust handling of concurrent operations to prevent potential race conditions.
- Address the client-side debouncing issue mentioned in the PR description to improve user experience.
Next steps:
- Consider implementing the suggested improvements for handling concurrent operations.
- Create a separate issue to track the implementation of client-side debouncing.
- Once these improvements are made, this PR can be considered ready for merging.
Would you like assistance in creating issues to track these improvements or in implementing them directly?
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌮 Works until we find out how we are supposed to do it 😹
✨ Codu Pull Request 💻
Fixes #914
Pull Request details
Any Breaking changes
Associated Screenshots
Screen.Recording.2024-10-02.at.21.mp4
I am spam clicking the like button. Not you need to throttle your connection locally as we need to imitate real life network calls. Calls locally resolve to fast to replicate this bug without the throttling.