From 98c1e0832ce6eb34e417daa00e4659f69d3b511b Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Wed, 16 Nov 2016 16:20:38 +0800
Subject: [PATCH] FIX: Track first notification read using Redis.

---
 app/models/notification.rb |  7 ++++++-
 app/models/user.rb         | 27 +++++++++++++++++++++++++--
 spec/models/user_spec.rb   | 30 +++++++++++++++++++++++-------
 3 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/app/models/notification.rb b/app/models/notification.rb
index 40470325483..9bfe5a0ef20 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -55,7 +55,10 @@ class Notification < ActiveRecord::Base
              read: false)
       .update_all("read = 't'")
 
-    user.publish_notifications_state if count > 0
+    if count > 0
+      user.mark_first_notification_read
+      user.publish_notifications_state
+    end
 
     count
   end
@@ -64,7 +67,9 @@ class Notification < ActiveRecord::Base
     count = Notification.where(user_id: user.id,
                                id: notification_ids,
                                read: false).update_all(read: true)
+
     if count > 0
+      user.mark_first_notification_read
       user.publish_notifications_state
     end
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index fa5aa1173dc..936833fe74d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -343,9 +343,32 @@ class User < ActiveRecord::Base
     end
   end
 
+  TRACK_FIRST_NOTIFICATION_READ_DURATION = 1.week.to_i
+
+  def self.first_notification_read_key(user)
+    "#{user.id}:first-notification-read"
+  end
+
+  def mark_first_notification_read
+    first_notification_read_key = User.first_notification_read_key(self)
+
+    if !$redis.get(first_notification_read_key)
+      $redis.setex(
+        first_notification_read_key,
+        User::TRACK_FIRST_NOTIFICATION_READ_DURATION,
+        1
+      )
+    end
+  end
+
   def read_first_notification?
-    return true if (trust_level > TrustLevel[0] || created_at < 1.week.ago)
-    notifications.order(created_at: :asc).first&.read || false
+    if (trust_level > TrustLevel[0] ||
+        created_at < TRACK_FIRST_NOTIFICATION_READ_DURATION.seconds.ago)
+
+      return true
+    end
+
+    !!$redis.get(self.class.first_notification_read_key(self))
   end
 
   def publish_notifications_state
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index bb61d6434c2..86e1aa79c73 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1319,27 +1319,43 @@ describe User do
 
   describe '#read_first_notification?' do
     let(:user) { Fabricate(:user, trust_level: TrustLevel[0]) }
-    let(:notification) { Fabricate(:private_message_notification, user: user) }
-    let(:other_notification) { Fabricate(:private_message_notification, user: user) }
+    let(:post) { Fabricate(:post) }
+    let(:topic) { Fabricate(:topic, first_post: post) }
+
+    let(:notification) do
+      Fabricate(:private_message_notification,
+        user: user, read: false, topic: topic, post_number: post.post_number
+      )
+    end
+
+    after do
+      $redis.del(User.first_notification_read_key(user))
+    end
 
     describe 'when first notification has not been read' do
       it 'should return the right value' do
-        notification.update_attributes!(read: false)
-        other_notification.update_attributes!(read: true)
-
         expect(user.read_first_notification?).to eq(false)
       end
     end
 
     describe 'when first notification has been read' do
       it 'should return the right value' do
-        notification.update_attributes!(read: true)
-        other_notification.update_attributes!(read: false)
+        notification
+        Notification.read(user, [notification.id])
 
         expect(user.reload.read_first_notification?).to eq(true)
       end
     end
 
+    describe 'when post has been read' do
+      it 'should return the right value' do
+        notification
+        Notification.mark_posts_read(user, notification.topic_id, [1])
+
+        expect(user.read_first_notification?).to eq(true)
+      end
+    end
+
     describe 'when user does not have any notifications' do
       it 'should return the right value' do
         expect(user.read_first_notification?).to eq(false)