From 2e655f83119bdb53fcbc218f8d0c9c4c1417ed18 Mon Sep 17 00:00:00 2001
From: Krzysztof Kotlarek <kotlarek.krzysztof@gmail.com>
Date: Wed, 16 Nov 2022 13:32:05 +1100
Subject: [PATCH] FEATURE: deprioritize like notifications on all list (#19029)

On the all notifications list, likes should be deprioritized and marked as read.
---
 app/models/notification.rb       | 26 +++++++++++++++++---------
 spec/models/notification_spec.rb | 10 +++++++---
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/app/models/notification.rb b/app/models/notification.rb
index 3c72cc2f1df..9ec48b631e1 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -24,10 +24,14 @@ class Notification < ActiveRecord::Base
       .includes(:topic)
       .limit(limit)
   end
-  scope :prioritized, ->() do
-    order("notifications.high_priority AND NOT notifications.read DESC")
-      .order("NOT notifications.read DESC")
-      .order("notifications.created_at DESC")
+  scope :prioritized, ->(deprioritized_types = []) do
+    scope = order("notifications.high_priority AND NOT notifications.read DESC")
+    if deprioritized_types.present?
+      scope = scope.order(DB.sql_fragment("NOT notifications.read AND notifications.notification_type NOT IN (?) DESC", deprioritized_types))
+    else
+      scope = scope.order("NOT notifications.read DESC")
+    end
+    scope.order("notifications.created_at DESC")
   end
   scope :for_user_menu, ->(user_id, limit: 30) do
     where(user_id: user_id)
@@ -232,22 +236,26 @@ class Notification < ActiveRecord::Base
     Post.find_by(topic_id: topic_id, post_number: post_number)
   end
 
+  def self.like_types
+    [
+      Notification.types[:liked],
+      Notification.types[:liked_consolidated]
+    ]
+  end
+
   def self.prioritized_list(user, count: 30, types: [])
     return [] if !user&.user_option
 
     notifications = user.notifications
       .includes(:topic)
       .visible
-      .prioritized
+      .prioritized(types.present? ? [] : like_types)
       .limit(count)
 
     if types.present?
       notifications = notifications.where(notification_type: types)
     elsif user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:never]
-      [
-        Notification.types[:liked],
-        Notification.types[:liked_consolidated]
-      ].each do |notification_type|
+      like_types.each do |notification_type|
         notifications = notifications.where(
           'notification_type <> ?', notification_type
         )
diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb
index dde6dfd83a7..173d8de0eaf 100644
--- a/spec/models/notification_spec.rb
+++ b/spec/models/notification_spec.rb
@@ -378,6 +378,7 @@ RSpec.describe Notification do
     fab!(:read_high_priority_1) { create(high_priority: true, read: true, created_at: 7.minutes.ago) }
     fab!(:unread_regular_1) { create(high_priority: false, read: false, created_at: 6.minutes.ago) }
     fab!(:read_regular_1) { create(high_priority: false, read: true, created_at: 5.minutes.ago) }
+    fab!(:unread_like) { create(high_priority: false, read: false, created_at: 130.seconds.ago, notification_type: Notification.types[:liked]) }
 
     fab!(:unread_high_priority_2) { create(high_priority: true, read: false, created_at: 1.minutes.ago) }
     fab!(:read_high_priority_2) { create(high_priority: true, read: true, created_at: 2.minutes.ago) }
@@ -391,6 +392,7 @@ RSpec.describe Notification do
         unread_regular_2,
         unread_regular_1,
         read_high_priority_2,
+        unread_like,
         read_regular_2,
         read_regular_1,
         read_high_priority_1,
@@ -405,6 +407,7 @@ RSpec.describe Notification do
         unread_regular_2,
         unread_regular_1,
         read_high_priority_2,
+        unread_like,
         read_regular_2,
         read_regular_1,
         read_high_priority_1,
@@ -422,6 +425,7 @@ RSpec.describe Notification do
         unread_high_priority_2,
         unread_regular_1,
         read_high_priority_2,
+        unread_like,
         read_regular_2,
         read_high_priority_1,
       ].map(&:id))
@@ -461,7 +465,7 @@ RSpec.describe Notification do
       expect(Notification.prioritized_list(
         user,
         types: [Notification.types[:liked], Notification.types[:liked_consolidated]]
-      ).map(&:id)).to eq([unread_regular_1, read_regular_2].map(&:id))
+      ).map(&:id)).to eq([unread_like, unread_regular_1, read_regular_2].map(&:id))
     end
 
     it "includes like notifications when filtering by like types even if the user doesn't want like notifications" do
@@ -474,11 +478,11 @@ RSpec.describe Notification do
       expect(Notification.prioritized_list(
         user,
         types: [Notification.types[:liked], Notification.types[:liked_consolidated]]
-      ).map(&:id)).to eq([unread_regular_1, read_regular_2].map(&:id))
+      ).map(&:id)).to eq([unread_like, unread_regular_1, read_regular_2].map(&:id))
       expect(Notification.prioritized_list(
         user,
         types: [Notification.types[:liked]]
-      ).map(&:id)).to contain_exactly(unread_regular_1.id)
+      ).map(&:id)).to contain_exactly(unread_like.id, unread_regular_1.id)
     end
   end