FEATURE: deprioritize like notifications on all list (#19029)

On the all notifications list, likes should be deprioritized and marked as read.
This commit is contained in:
Krzysztof Kotlarek 2022-11-16 13:32:05 +11:00 committed by GitHub
parent f1548c801e
commit 2e655f8311
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 24 additions and 12 deletions

View File

@ -24,10 +24,14 @@ class Notification < ActiveRecord::Base
.includes(:topic) .includes(:topic)
.limit(limit) .limit(limit)
end end
scope :prioritized, ->() do scope :prioritized, ->(deprioritized_types = []) do
order("notifications.high_priority AND NOT notifications.read DESC") scope = order("notifications.high_priority AND NOT notifications.read DESC")
.order("NOT notifications.read DESC") if deprioritized_types.present?
.order("notifications.created_at DESC") 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 end
scope :for_user_menu, ->(user_id, limit: 30) do scope :for_user_menu, ->(user_id, limit: 30) do
where(user_id: user_id) where(user_id: user_id)
@ -232,22 +236,26 @@ class Notification < ActiveRecord::Base
Post.find_by(topic_id: topic_id, post_number: post_number) Post.find_by(topic_id: topic_id, post_number: post_number)
end end
def self.like_types
[
Notification.types[:liked],
Notification.types[:liked_consolidated]
]
end
def self.prioritized_list(user, count: 30, types: []) def self.prioritized_list(user, count: 30, types: [])
return [] if !user&.user_option return [] if !user&.user_option
notifications = user.notifications notifications = user.notifications
.includes(:topic) .includes(:topic)
.visible .visible
.prioritized .prioritized(types.present? ? [] : like_types)
.limit(count) .limit(count)
if types.present? if types.present?
notifications = notifications.where(notification_type: types) notifications = notifications.where(notification_type: types)
elsif user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:never] elsif user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:never]
[ like_types.each do |notification_type|
Notification.types[:liked],
Notification.types[:liked_consolidated]
].each do |notification_type|
notifications = notifications.where( notifications = notifications.where(
'notification_type <> ?', notification_type 'notification_type <> ?', notification_type
) )

View File

@ -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!(: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!(: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!(: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!(: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) } 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_2,
unread_regular_1, unread_regular_1,
read_high_priority_2, read_high_priority_2,
unread_like,
read_regular_2, read_regular_2,
read_regular_1, read_regular_1,
read_high_priority_1, read_high_priority_1,
@ -405,6 +407,7 @@ RSpec.describe Notification do
unread_regular_2, unread_regular_2,
unread_regular_1, unread_regular_1,
read_high_priority_2, read_high_priority_2,
unread_like,
read_regular_2, read_regular_2,
read_regular_1, read_regular_1,
read_high_priority_1, read_high_priority_1,
@ -422,6 +425,7 @@ RSpec.describe Notification do
unread_high_priority_2, unread_high_priority_2,
unread_regular_1, unread_regular_1,
read_high_priority_2, read_high_priority_2,
unread_like,
read_regular_2, read_regular_2,
read_high_priority_1, read_high_priority_1,
].map(&:id)) ].map(&:id))
@ -461,7 +465,7 @@ RSpec.describe Notification do
expect(Notification.prioritized_list( expect(Notification.prioritized_list(
user, user,
types: [Notification.types[:liked], Notification.types[:liked_consolidated]] 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 end
it "includes like notifications when filtering by like types even if the user doesn't want like notifications" do 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( expect(Notification.prioritized_list(
user, user,
types: [Notification.types[:liked], Notification.types[:liked_consolidated]] 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( expect(Notification.prioritized_list(
user, user,
types: [Notification.types[:liked]] 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
end end