From 430f5ad560ab0b6a79788b9ec1f669077d0153ea Mon Sep 17 00:00:00 2001 From: David Battersby Date: Mon, 23 Oct 2023 13:41:05 +0800 Subject: [PATCH] FIX: update post id for reactions when post is moved (#250) 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). --- plugin.rb | 22 +++++++++++++ spec/models/post_mover_spec.rb | 59 ++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 spec/models/post_mover_spec.rb diff --git a/plugin.rb b/plugin.rb index 9912c63..15d2a12 100644 --- a/plugin.rb +++ b/plugin.rb @@ -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 diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb new file mode 100644 index 0000000..7779911 --- /dev/null +++ b/spec/models/post_mover_spec.rb @@ -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