Mark notifications as read in a single SQL query

There is no need to load all comments only to count them. Lets just let
the database do all the work. If there are no comments found, nothing
will happen anyway.

Also already filter the comments to only search for notifications for
own comments.

And add some tests :)
This commit is contained in:
Benjamin Neff 2024-06-03 00:11:45 +02:00
parent 71e6f20740
commit 649d8c5b56
No known key found for this signature in database
GPG key ID: 971464C3F1A90194
2 changed files with 47 additions and 4 deletions

View file

@ -104,9 +104,8 @@ class PostService
end
def mark_like_on_comment_notifications_read(post_id)
comment_ids = Comment.where(commentable_id: post_id)
Notification.where(recipient_id: user.id, target_type: "Comment", target_id: comment_ids, unread: true)
.update_all(unread: false) if comment_ids.any?
Notification.where(recipient_id: user.id, target_type: "Comment",
target_id: Comment.where(commentable_id: post_id, author_id: user.person.id),
unread: true).update_all(unread: false) # rubocop:disable Rails/SkipsModelValidations
end
end

View file

@ -168,6 +168,50 @@ describe PostService do
expect_any_instance_of(PostService).not_to receive(:mark_mention_notifications_read).with(post.id)
PostService.new.mark_user_notifications(post.id)
end
context "for comments" do
let(:comment) { post.comments.create(author: alice.person, text: "comment") }
it "marks a corresponding notifications as read" do
FactoryBot.create(:notification, recipient: alice, target: comment, unread: true)
FactoryBot.create(:notification, recipient: alice, target: comment, unread: true)
expect {
PostService.new(alice).mark_user_notifications(post.id)
}.to change(Notification.where(unread: true), :count).by(-2)
end
it "does not change the update_at date/time for comment notifications" do
notification = Timecop.travel(1.minute.ago) do
FactoryBot.create(:notification, recipient: alice, target: comment, unread: true)
end
expect {
PostService.new(alice).mark_user_notifications(post.id)
}.not_to(change { Notification.where(id: notification.id).pluck(:updated_at) })
end
it "does not change other users notifications" do
alice_notification = FactoryBot.create(:notification, recipient: alice, target: comment, unread: true)
bob_notification = FactoryBot.create(:notification, recipient: bob, target: comment, unread: true)
PostService.new(alice).mark_user_notifications(post.id)
expect(Notification.find(alice_notification.id).unread).to be_falsey
expect(Notification.find(bob_notification.id).unread).to be_truthy
end
it "marks notifications for all comments on a post as read" do
comment2 = post.comments.create(author: alice.person, text: "comment2")
FactoryBot.create(:notification, recipient: alice, target: comment, unread: true)
FactoryBot.create(:notification, recipient: alice, target: comment2, unread: true)
expect {
PostService.new(alice).mark_user_notifications(post.id)
}.to change(Notification.where(unread: true), :count).by(-2)
end
end
end
describe "#destroy" do