REFACTOR: Use a consolidation rule for collapsing likes. (#15393)

This PR moves the behavior from the PostAlerter. We delete an existing liked notification and set the `username2` attribute to the previous `display_username`. We repeat this process unless the last one is old enough or it's not in the most recent ones.
This commit is contained in:
Roman Rizzi 2021-12-23 09:44:03 -03:00 committed by GitHub
parent cdf4d7156e
commit 1ad47030fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 57 additions and 32 deletions

View File

@ -44,7 +44,7 @@ module Notifications
@data = consolidated_data(notification)
return true if @precondition_blk.nil?
@precondition_blk.call(data)
@precondition_blk.call(data, notification)
end
def consolidate_or_save!(notification)

View File

@ -12,7 +12,7 @@ module Notifications
private
def plan_for(notification)
consolidation_plans = [liked, group_message_summary, group_membership]
consolidation_plans = [liked_by_two_users, liked, group_message_summary, group_membership]
consolidation_plans.concat(DiscoursePluginRegistry.notification_consolidation_plans)
consolidation_plans.detect { |plan| plan.can_consolidate_data?(notification) }
@ -24,7 +24,7 @@ module Notifications
to: Notification.types[:liked_consolidated],
threshold: -> { SiteSetting.notification_consolidation_threshold },
consolidation_window: SiteSetting.likes_notification_consolidation_window_mins.minutes,
unconsolidated_query_blk: ->(notifications, data) do
unconsolidated_query_blk: Proc.new do |notifications, data|
key = 'display_username'
value = data[key.to_sym]
filtered = notifications.where("data::json ->> 'username2' IS NULL")
@ -35,10 +35,48 @@ module Notifications
end,
consolidated_query_blk: filtered_by_data_attribute('display_username')
).set_mutations(
set_data_blk: ->(notification) do
set_data_blk: Proc.new do |notification|
data = notification.data_hash
data.merge(username: data[:display_username])
end
).set_precondition(precondition_blk: Proc.new { |data| data[:username2].blank? })
end
def liked_by_two_users
DeletePreviousNotifications.new(
type: Notification.types[:liked],
previous_query_blk: Proc.new do |notifications, data|
notifications.where(id: data[:previous_notification_id])
end
).set_mutations(
set_data_blk: Proc.new do |notification|
existing_notification_of_same_type = Notification
.where(user: notification.user)
.order("notifications.id DESC")
.where(topic_id: notification.topic_id, post_number: notification.post_number)
.where(notification_type: notification.notification_type)
.where('created_at > ?', 1.day.ago)
.first
data = notification.data_hash
if existing_notification_of_same_type
same_type_data = existing_notification_of_same_type.data_hash
data.merge(
previous_notification_id: existing_notification_of_same_type.id,
username2: same_type_data[:display_username],
count: (same_type_data[:count] || 1).to_i + 1
)
else
data
end
end
).set_precondition(
precondition_blk: Proc.new do |data, notification|
always_freq = UserOption.like_notification_frequency_type[:always]
notification.user&.user_option&.like_notification_frequency == always_freq &&
data[:previous_notification_id].present?
end
)
end
@ -51,9 +89,9 @@ module Notifications
unconsolidated_query_blk: filtered_by_data_attribute('topic_title'),
consolidated_query_blk: filtered_by_data_attribute('group_name')
).set_precondition(
precondition_blk: ->(data) { data[:group_name].present? }
precondition_blk: Proc.new { |data| data[:group_name].present? }
).set_mutations(
set_data_blk: ->(notification) do
set_data_blk: Proc.new do |notification|
data = notification.data_hash
post_id = data[:original_post_id]
custom_field = PostCustomField.select(:value).find_by(post_id: post_id, name: "requested_group_id")
@ -71,12 +109,12 @@ module Notifications
type: Notification.types[:group_message_summary],
previous_query_blk: filtered_by_data_attribute('group_id')
).set_precondition(
precondition_blk: ->(data) { data[:group_id].present? }
precondition_blk: Proc.new { |data| data[:group_id].present? }
)
end
def filtered_by_data_attribute(attribute_name)
->(notifications, data) do
Proc.new do |notifications, data|
if (value = data[attribute_name.to_sym])
notifications.where("data::json ->> '#{attribute_name}' = ?", value.to_s)
else

View File

@ -27,7 +27,7 @@ module Notifications
@data = consolidated_data(notification)
precondition_blk.nil? || precondition_blk.call(notification.data_hash)
precondition_blk.nil? || precondition_blk.call(@data, notification)
end
def consolidate_or_save!(notification)
@ -39,6 +39,8 @@ module Notifications
notifications = previous_query_blk.call(notifications, data)
end
notification.data = data.to_json
Notification.transaction do
notifications.destroy_all
notification.save!

View File

@ -434,23 +434,6 @@ class PostAlerter
return if existing_notifications.find { |n| n.notification_type == Notification.types[:replied] }
end
notification_data = {}
if is_liked &&
existing_notification_of_same_type &&
existing_notification_of_same_type.created_at > 1.day.ago &&
(
user.user_option.like_notification_frequency ==
UserOption.like_notification_frequency_type[:always]
)
data = existing_notification_of_same_type.data_hash
notification_data["username2"] = data["display_username"]
notification_data["count"] = (data["count"] || 1).to_i + 1
# don't use destroy so we don't trigger a notification count refresh
Notification.where(id: existing_notification_of_same_type.id).destroy_all
end
collapsed = false
if COLLAPSED_NOTIFICATION_TYPES.include?(type)
@ -481,12 +464,14 @@ class PostAlerter
end
end
notification_data.merge!(topic_title: topic_title,
original_post_id: original_post.id,
original_post_type: original_post.post_type,
original_username: original_username,
revision_number: opts[:revision_number],
display_username: opts[:display_username] || post.user.username)
notification_data = {
topic_title: topic_title,
original_post_id: original_post.id,
original_post_type: original_post.post_type,
original_username: original_username,
revision_number: opts[:revision_number],
display_username: opts[:display_username] || post.user.username,
}
if group = opts[:group]
notification_data[:group_id] = group.id