Skip to content

Commit

Permalink
DEV: Improve AR relations on post action and fix more N1s
Browse files Browse the repository at this point in the history
  • Loading branch information
martin-brennan committed May 21, 2024
1 parent f6a847a commit 46284bf
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 37 deletions.
11 changes: 7 additions & 4 deletions lib/discourse_reactions/post_action_extension.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
# frozen_string_literal: true

module DiscourseReactions::PostActionExtension
def reaction_user
return if self.post_action_type_id != PostActionType.types[:like]
@reaction_user ||=
DiscourseReactions::ReactionUser.find_by(post_id: self.post_id, user_id: self.user_id)
def self.prepended(base)
base.has_one :reaction_user,
->(post_action) { where(user_id: post_action.user_id) },
foreign_key: :post_id,
primary_key: :post_id,
class_name: "DiscourseReactions::ReactionUser"
base.has_one :reaction, class_name: "DiscourseReactions::Reaction", through: :reaction_user
end

def self.filter_reaction_likes_sql
Expand Down
2 changes: 2 additions & 0 deletions lib/discourse_reactions/post_extension.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

module DiscourseReactions::PostExtension
attr_accessor :post_actions_with_reaction_users

def self.prepended(base)
base.has_many :reactions, class_name: "DiscourseReactions::Reaction"
base.has_many :reactions_user, class_name: "DiscourseReactions::ReactionUser"
Expand Down
26 changes: 25 additions & 1 deletion lib/discourse_reactions/topic_view_serializer_extension.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,39 @@
# frozen_string_literal: true

module DiscourseReactions::TopicViewSerializerExtension
def self.load_post_action_reaction_users_for_posts(post_ids)
PostAction
.includes(reaction_user: :reaction)
.joins(
"LEFT JOIN discourse_reactions_reaction_users ON discourse_reactions_reaction_users.post_id = post_actions.post_id AND discourse_reactions_reaction_users.user_id = post_actions.user_id",
)
.where(post_id: post_ids)
.where("post_actions.deleted_at IS NULL")
.where(post_action_type_id: PostActionType.types[:like])
.where(
"post_actions.post_id IN (#{DiscourseReactions::PostActionExtension.post_action_with_reaction_user_sql})",
valid_reactions: DiscourseReactions::Reaction.reactions_counting_as_like,
)
end

def posts
if SiteSetting.discourse_reactions_enabled
posts = object.posts.includes(:post_actions, reactions: { reaction_users: :user })
post_ids = posts.map(&:id).uniq

posts_reaction_users_count = TopicViewSerializer.posts_reaction_users_count(post_ids)

posts.each { |post| post.reaction_users_count = posts_reaction_users_count[post.id].to_i }

post_actions_with_reaction_users =
DiscourseReactions::TopicViewSerializerExtension.load_post_action_reaction_users_for_posts(
post_ids,
)

posts.each do |post|
post.post_actions_with_reaction_users =
post_actions_with_reaction_users.select { |post_action| post_action.post_id == post.id }
end

object.instance_variable_set(:@posts, posts)
end
super
Expand Down
53 changes: 21 additions & 32 deletions plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,39 +104,28 @@ class Engine < ::Rails::Engine

reaction_users_counting_as_like.flatten!

# Used to remove historical like data in the filter below.
previous_main_reaction_like_ids =
object
.post_actions
.where(
"post_actions.post_action_type_id = :like AND
post_actions.post_id IN (
#{DiscourseReactions::PostActionExtension.post_action_with_reaction_user_sql}
)",
like: PostActionType.types[:like],
valid_reactions: [DiscourseReactions::Reaction.main_reaction_id],
)
.pluck(:id)

likes =
object
.post_actions
.select do |post_action|
post_action.post_action_type_id == PostActionType.types[:like] && !post_action.trashed?
end
.reject do |post_action|
# Get rid of any PostAction records that match up to a ReactionUser
# that is NOT main_reaction_id and is NOT excluded, otherwise we double
# up on the count/reaction shown in the UI.
is_reaction_like_duplicate =
reaction_users_counting_as_like.any? { |ru| ru.user_id == post_action.user_id }

# Also get rid of any PostAction records that match up to a ReactionUser
# that is now the main_reaction_id and has historical data.
is_previously_enabled_reaction = previous_main_reaction_like_ids.include?(post_action.id)

is_reaction_like_duplicate || is_previously_enabled_reaction
end
object.post_actions.reject do |post_action|
# Get rid of any PostAction records that match up to a ReactionUser
# that is NOT main_reaction_id and is NOT excluded, otherwise we double
# up on the count/reaction shown in the UI.
is_reaction_like_duplicate =
reaction_users_counting_as_like.any? { |ru| ru.user_id == post_action.user_id }

# Also get rid of any PostAction records that match up to a ReactionUser
# that is now the main_reaction_id and has historical data.
is_previously_enabled_reaction =
object
.post_actions_with_reaction_users
&.find do |pa|
pa.id == post_action.id &&
pa.reaction_user&.reaction&.reaction_value ==
DiscourseReactions::Reaction.main_reaction_id
end
.present?

is_reaction_like_duplicate || is_previously_enabled_reaction
end

# Likes will only be blank if there are only reactions where the reaction is in
# discourse_reactions_excluded_from_like. All other reactions will have a `PostAction` record.
Expand Down
5 changes: 5 additions & 0 deletions spec/serializers/post_serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
SiteSetting.discourse_reactions_like_icon = "heart"

reaction_user_1 && reaction_user_2 && reaction_user_3 && like

post_1.post_actions_with_reaction_users =
DiscourseReactions::TopicViewSerializerExtension
.load_post_action_reaction_users_for_posts([post_1.id])
.select { |pa| pa.post_id == post_1.id }
end

it "renders custom reactions which should be sorted by count" do
Expand Down

0 comments on commit 46284bf

Please sign in to comment.