Merge pull request #4554 from tgxworld/fix_first_post_notification_read_tracker

FIX: Track first notification read using Redis.

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

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

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)