FIX: Update post replies when we move posts. (#4324)

app/controllers/application_controller.rb

@@ -315,9 +315,7 @@ class ApplicationController < ActionController::Base
   def post_ids_including_replies
     post_ids = params[:post_ids].map {|p| p.to_i}
     if params[:reply_post_ids]
-      post_ids << PostReply.where(post_id: params[:reply_post_ids].map {|p| p.to_i}).pluck(:reply_id)
-      post_ids.flatten!
-      post_ids.uniq!
+      post_ids |= PostReply.where(post_id: params[:reply_post_ids].map {|p| p.to_i}).pluck(:reply_id)
     end
     post_ids
   end

app/models/post_mover.rb

@@ -76,6 +76,12 @@ class PostMover
     posts.each do |post|
       post.is_first_post? ? create_first_post(post) : move(post)
     end
+
+    PostReply.where("reply_id in (:post_ids) OR post_id in (:post_ids)", post_ids: post_ids).each do |post_reply|
+      if post_reply.reply.topic_id != post_reply.post.topic_id
+        PostReply.delete_all(reply_id: post_reply.reply.id, post_id: post_reply.post.id)
+      end
+    end
   end
 
   def create_first_post(post)

app/models/post_reply.rb

@@ -3,6 +3,18 @@ class PostReply < ActiveRecord::Base
   belongs_to :reply, class_name: 'Post'
 
   validates_uniqueness_of :reply_id, scope: :post_id
+  validate :ensure_same_topic
+
+  private
+
+  def ensure_same_topic
+    if post.topic_id != reply.topic_id
+      self.errors.add(
+        :base,
+        I18n.t("activerecord.errors.models.post_reply.base.different_topic")
+      )
+    end
+  end
 end
 
 # == Schema Information

config/locales/server.en.yml

@@ -346,6 +346,10 @@ en:
           attributes:
             hex:
               invalid: "is not a valid color"
+        post_reply:
+          base:
+            different_topic: "Post and reply must belong to the same topic."
+
 
   user_profile:
     no_info_me: "<div class='missing-profile'>the About Me field of your profile is currently blank, <a href='/users/%{username_lower}/preferences/about-me'>would you like to fill it out?</a></div>"

spec/models/post_mover_spec.rb

@@ -29,6 +29,8 @@ describe PostMover do
     let!(:p4) { Fabricate(:post, topic: topic, reply_to_post_number: p2.post_number, user: user)}
 
     before do
+      p1.replies << p3
+      p2.replies << p4
       # add a like to a post, enable observers so we get user actions
       ActiveRecord::Base.observers.enable :all
       @like = PostAction.act(another_user, p4, PostActionType.types[:like])
@@ -72,6 +74,62 @@ describe PostMover do
         TopicLink.extract_from(p2)
       end
 
+      context "post replies" do
+        describe "when a post with replies is moved" do
+          it "should update post replies correctly" do
+            topic.move_posts(
+              user,
+              [p2.id],
+              title: 'GOT is a very addictive showw', category_id: category.id
+            )
+
+            expect(p2.reload.replies).to eq([])
+          end
+        end
+
+        describe "when replies of a post have been moved" do
+          it "should update post replies correctly" do
+            p5 = Fabricate(
+              :post,
+              topic: topic,
+              reply_to_post_number: p2.post_number,
+              user: another_user
+            )
+
+            p2.replies << p5
+
+            topic.move_posts(
+              user,
+              [p4.id],
+              title: 'GOT is a very addictive showw', category_id: category.id
+            )
+
+            expect(p2.reload.replies).to eq([p5])
+          end
+        end
+
+        describe "when only one reply is left behind" do
+          it "should update post replies correctly" do
+            p5 = Fabricate(
+              :post,
+              topic: topic,
+              reply_to_post_number: p2.post_number,
+              user: another_user
+            )
+
+            p2.replies << p5
+
+            topic.move_posts(
+              user,
+              [p2.id, p4.id],
+              title: 'GOT is a very addictive showw', category_id: category.id
+            )
+
+            expect(p2.reload.replies).to eq([p4])
+          end
+        end
+      end
+
       context "to a new topic" do
 
         it "works correctly" do

spec/models/post_reply_spec.rb

@@ -1,8 +1,27 @@
 require 'rails_helper'
 
 describe PostReply do
+  let(:topic) { Fabricate(:topic) }
+  let(:post) { Fabricate(:post, topic: topic) }
+  let(:other_post) { Fabricate(:post, topic: topic) }
 
   it { is_expected.to belong_to :post }
   it { is_expected.to belong_to :reply }
 
+  context "validation" do
+    it "should ensure that the posts belong in the same topic" do
+      expect(PostReply.new(post: post, reply: other_post)).to be_valid
+
+      other_topic = Fabricate(:topic)
+      other_post.update_attributes!(topic_id: other_topic.id)
+      other_post.reload
+
+      post_reply = PostReply.new(post: post, reply: other_post)
+      expect(post_reply).to_not be_valid
+
+      expect(post_reply.errors[:base]).to include(
+        I18n.t("activerecord.errors.models.post_reply.base.different_topic")
+      )
+    end
+  end
 end