Skip to content

Commit

Permalink
FIX: update post id for reactions when post is moved (#250)
Browse files Browse the repository at this point in the history
Previously all emoji reactions on a post would be lost when posts are moved to a new topic. This would only happen for the first post in a topic. This is due to an entirely new post being created based on the attributes of the original post.

The reaction data for the old post remains but the new post does not have any.

This change creates a copy of the reaction data and user reaction details from the original post (with updated post id).
  • Loading branch information
dbattersby authored Oct 23, 2023
1 parent b038c89 commit 430f5ad
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 0 deletions.
22 changes: 22 additions & 0 deletions plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -343,4 +343,26 @@ class Engine < ::Rails::Engine

register_notification_consolidation_plan(reacted_by_two_users)
register_notification_consolidation_plan(consolidated_reactions)

on(:first_post_moved) do |target_post, original_post|
id_map = {}
ActiveRecord::Base.transaction do
reactions = DiscourseReactions::Reaction.where(post_id: original_post.id)
reactions_attributes =
reactions.map { |reaction| reaction.attributes.except("id").merge(post_id: target_post.id) }
DiscourseReactions::Reaction
.insert_all(reactions_attributes)
.each_with_index { |entry, index| id_map[reactions[index].id] = entry["id"] }

reaction_users = DiscourseReactions::ReactionUser.where(post_id: original_post.id)
reaction_users_attributes =
reaction_users.map do |reaction_user|
reaction_user
.attributes
.except("id")
.merge(post_id: target_post.id, reaction_id: id_map[reaction_user.reaction_id])
end
DiscourseReactions::ReactionUser.insert_all(reaction_users_attributes)
end
end
end
59 changes: 59 additions & 0 deletions spec/models/post_mover_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# frozen_string_literal: true

describe PostMover do
fab!(:admin) { Fabricate(:admin) }
fab!(:user) { Fabricate(:user) }
fab!(:topic_1) { Fabricate(:topic, user: user) }
fab!(:topic_2) { Fabricate(:topic, user: user) }
fab!(:topic_3) { Fabricate(:topic, user: user) }
fab!(:post_1) { Fabricate(:post, topic: topic_1, user: user) }
fab!(:post_2) { Fabricate(:post, topic: topic_1, user: user) }
fab!(:reaction_1) { Fabricate(:reaction, post: post_1, reaction_value: "clap") }
fab!(:reaction_2) { Fabricate(:reaction, post: post_1, reaction_value: "confetti") }
fab!(:reaction_3) { Fabricate(:reaction, post: post_2, reaction_value: "heart") }
fab!(:reaction_4) { Fabricate(:reaction, post: post_2, reaction_value: "wave") }
fab!(:user_reaction_1) { Fabricate(:reaction_user, reaction: reaction_1, post: post_1) }
fab!(:user_reaction_2) { Fabricate(:reaction_user, reaction: reaction_2, post: post_1) }
fab!(:user_reaction_3) { Fabricate(:reaction_user, reaction: reaction_3, post: post_2) }
fab!(:user_reaction_4) { Fabricate(:reaction_user, reaction: reaction_4, post: post_2) }

before { SiteSetting.discourse_reactions_enabled = true }

it "should add old post's reactions to new post when a topic's first post is moved" do
expect(post_1.reactions).to contain_exactly(reaction_1, reaction_2)
expect(topic_2.posts.count).to eq(0)

post_mover = PostMover.new(topic_1, Discourse.system_user, [post_1.id])
expect { post_mover.to_topic(topic_2) }.to change { topic_2.posts.count }.by(1)

expect(topic_2.posts.count).to eq(1)

new_post = topic_2.first_post
reaction_emojis = new_post.reactions.pluck(:reaction_value)

expect(reaction_emojis.count).to eq(2)
expect(reaction_emojis).to match_array([reaction_1.reaction_value, reaction_2.reaction_value])

reaction_user_ids = new_post.reactions_user.pluck(:user_id)
expect(reaction_user_ids.count).to eq(2)
expect(reaction_user_ids).to match_array([user_reaction_1.user_id, user_reaction_2.user_id])
end

it "should retain existing reactions after moving a post" do
expect(post_2.reactions).to contain_exactly(reaction_3, reaction_4)
expect(topic_3.posts.count).to eq(0)

post_mover = PostMover.new(topic_1, Discourse.system_user, [post_2.id])
expect { post_mover.to_topic(topic_3) }.to change { topic_3.posts.count }.by(1)

new_post = topic_3.first_post

# reaction id does not change as post is updated (unlike first post in topic)
expect(new_post.reactions.count).to eq(2)
expect(new_post.reactions).to match_array([reaction_3, reaction_4])

# reaction_user id does not change as post is updated (unlike first post in topic)
expect(new_post.reactions_user.count).to eq(2)
expect(new_post.reactions_user).to match_array([user_reaction_3, user_reaction_4])
end
end

0 comments on commit 430f5ad

Please sign in to comment.