From 017842cc0100a40db9b99edbbf71d2238856f226 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 7 Mar 2016 02:01:02 +0100 Subject: [PATCH] don't update updated_at date when read the notifications. updated_at is displayed in the frontend and should only be updated when another notification_actor gets added Also improved the sql-queries: update directly and not select first and update then. --- app/services/post_service.rb | 12 +++++------- spec/services/post_service_spec.rb | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/app/services/post_service.rb b/app/services/post_service.rb index 928375e29..4c3d7a6b5 100644 --- a/app/services/post_service.rb +++ b/app/services/post_service.rb @@ -46,15 +46,13 @@ class PostService end def mark_comment_reshare_like_notifications_read(post_id) - notifications = Notification.where(recipient_id: user.id, target_type: "Post", target_id: post_id, unread: true) - notifications.each do |notification| - notification.set_read_state(true) - end + Notification.where(recipient_id: user.id, target_type: "Post", target_id: post_id, unread: true) + .update_all(unread: false) end def mark_mention_notifications_read(post_id) - mention = find(post_id).mentions.where(person_id: user.person_id).first - Notification.where(recipient_id: user.id, target_type: "Mention", target_id: mention.id, unread: true) - .first.try(:set_read_state, true) if mention + mention_id = Mention.where(post_id: post_id, person_id: user.person_id).pluck(:id) + Notification.where(recipient_id: user.id, target_type: "Mention", target_id: mention_id, unread: true) + .update_all(unread: false) if mention_id end end diff --git a/spec/services/post_service_spec.rb b/spec/services/post_service_spec.rb index 660e7add4..1d78706b4 100644 --- a/spec/services/post_service_spec.rb +++ b/spec/services/post_service_spec.rb @@ -93,6 +93,28 @@ describe PostService do }.to change(Notification.where(unread: true), :count).by(-1) end + it "does not change the update_at date/time for post notifications" do + notification = Timecop.travel(1.minute.ago) do + FactoryGirl.create(:notification, recipient: alice, target: post, 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 the update_at date/time for mention notifications" do + status_text = "this is a text mentioning @{Mention User ; #{alice.diaspora_handle}} ... have fun testing!" + mention_post = Timecop.travel(1.minute.ago) do + bob.post(:status_message, text: status_text, public: true) + end + mention = mention_post.mentions.where(person_id: alice.person.id).first + + expect { + PostService.new(alice).mark_user_notifications(post.id) + }.not_to change { Notification.where(target_type: "Mention", target_id: mention.id).pluck(:updated_at) } + end + it "does nothing without a user" do expect_any_instance_of(PostService).not_to receive(:mark_comment_reshare_like_notifications_read).with(post.id) expect_any_instance_of(PostService).not_to receive(:mark_mention_notifications_read).with(post.id)