perf: use after_create_commit for comment relaying#766
Conversation
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe ChangesComment Relay Job Trigger
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR swaps
Confidence Score: 3/5The change silently drops ActionCable broadcasts for comment edits and deletes, leaving connected clients with stale data; merging as-is would regress real-time update behavior. Switching to app/models/comment.rb — the callback change and its interaction with the existing update/destroy actions needs a second look before merging. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant CommentsController
participant Comment
participant CommentRelayJob
participant ActionCable
Note over Comment,ActionCable: BEFORE (after_commit — fires on create, update, destroy)
Client->>CommentsController: POST /comments (create)
CommentsController->>Comment: save
Comment->>CommentRelayJob: perform_later(self) [after create commit]
CommentRelayJob->>ActionCable: broadcast "comments"
Client->>CommentsController: PATCH /comments/:id (update)
CommentsController->>Comment: update(...)
Comment->>CommentRelayJob: perform_later(self) [after update commit]
CommentRelayJob->>ActionCable: broadcast "comments"
Note over Comment,ActionCable: AFTER (after_create_commit — fires on create only)
Client->>CommentsController: POST /comments (create)
CommentsController->>Comment: save
Comment->>CommentRelayJob: perform_later(self) [after create commit]
CommentRelayJob->>ActionCable: broadcast "comments"
Client->>CommentsController: PATCH /comments/:id (update)
CommentsController->>Comment: update(...)
Note over Comment: No callback fires — subscribers not notified
Reviews (1): Last reviewed commit: "perf: use after_create_commit for commen..." | Re-trigger Greptile |
| class Comment < ApplicationRecord | ||
| validates :author, :text, presence: true | ||
| after_commit { CommentRelayJob.perform_later(self) } | ||
| after_create_commit { CommentRelayJob.perform_later(self) } |
There was a problem hiding this comment.
Broadcasting silently dropped for updates and destroys
after_create_commit only fires after an INSERT, so comment edits and deletes no longer enqueue CommentRelayJob. The app exposes both update and destroy actions via CommentsController, so any client subscribed to the "comments" ActionCable channel will continue to see stale data after a comment is edited or deleted. If real-time updates on create are the only required behaviour this is fine, but the PR description frames it purely as a performance improvement, which obscures the functional trade-off.
There was a problem hiding this comment.
we want to see updates for edits as well.
There was a problem hiding this comment.
Then this change introduces a regression. You should revert to after_commit (which fires on create, update, and destroy), or be explicit about the callbacks you want:
| after_create_commit { CommentRelayJob.perform_later(self) } | |
| after_create_commit { CommentRelayJob.perform_later(self) } | |
| after_update_commit { CommentRelayJob.perform_later(self) } |
Or more concisely, just keep the original:
after_commit { CommentRelayJob.perform_later(self) }The original code was correct. The PR description's claim that after_create_commit is a "performance improvement" is only true if you intentionally want to drop update/delete relaying — which you don't. This PR should not be merged as-is.
|
we want to see updates for edits and creates. |
✅ Review App DeletedReview app for PR #766 is deleted |
Optimizes the Comment model by changing
after_committoafter_create_commit. This ensures theCommentRelayJobis only triggered upon the creation of a new comment, avoiding unnecessary broadcast operations when existing comments are updated. This improves system efficiency and reduces unnecessary WebSocket traffic. Closes #0Summary by CodeRabbit