From 5c1eaeca9e783aec07191958f0781ad9e57c0f22 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 22 Jan 2018 17:23:19 +0100
Subject: [PATCH] FIX: prevent users from moving whispers to new topic

---
 .../discourse/controllers/topic.js.es6        |  9 ++++++---
 app/controllers/application_controller.rb     |  6 ++----
 app/controllers/topics_controller.rb          | 11 ++++++++---
 spec/controllers/topics_controller_spec.rb    | 19 ++++++++++++++++---
 4 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6
index e47309c7576..2e351654e9e 100644
--- a/app/assets/javascripts/discourse/controllers/topic.js.es6
+++ b/app/assets/javascripts/discourse/controllers/topic.js.es6
@@ -772,9 +772,12 @@ export default Ember.Controller.extend(BufferedContent, {
     return selectedPostsCount > 0 && (selectedAllPosts || selectedPosts.every(p => p.can_delete));
   },
 
-  @computed('canMergeTopic', 'selectedAllPosts')
-  canSplitTopic(canMergeTopic, selectedAllPosts) {
-    return canMergeTopic && !selectedAllPosts;
+  @computed('canMergeTopic', 'selectedAllPosts', 'selectedPosts', 'selectedPosts.[]')
+  canSplitTopic(canMergeTopic, selectedAllPosts, selectedPosts) {
+    return canMergeTopic &&
+           !selectedAllPosts &&
+           selectedPosts.length > 0 &&
+           selectedPosts.sort((a, b) => a.post_number - b.post_number)[0].post_type === 1;
   },
 
   @computed('model.details.can_move_posts', 'selectedPostsCount')
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index d7b493a86a5..57e9de2953d 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -393,10 +393,8 @@ class ApplicationController < ActionController::Base
   end
 
   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)
-    end
+    post_ids  = params[:post_ids].map(&:to_i)
+    post_ids |= PostReply.where(post_id: params[:reply_post_ids]).pluck(:reply_id) if params[:reply_post_ids]
     post_ids
   end
 
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 9f5113f340e..439c5683439 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -521,13 +521,18 @@ class TopicsController < ApplicationController
   end
 
   def move_posts
-    params.require(:post_ids)
-    params.require(:topic_id)
+    post_ids = params.require(:post_ids)
+    topic_id = params.require(:topic_id)
     params.permit(:category_id)
 
-    topic = Topic.with_deleted.find_by(id: params[:topic_id])
+    topic = Topic.with_deleted.find_by(id: topic_id)
     guardian.ensure_can_move_posts!(topic)
 
+    # when creating a new topic, ensure the 1st post is a regular post
+    if params[:title].present? && Post.where(topic: topic, id: post_ids).order(:post_number).pluck(:post_type).first != Post.types[:regular]
+      return render_json_error("When moving posts to a new topic, the first post must be a regular post.")
+    end
+
     dest_topic = move_posts_to_destination(topic)
     render_topic_changes(dest_topic)
   rescue ActiveRecord::RecordInvalid => ex
diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb
index 0d37b4fcd3e..01637a4b09c 100644
--- a/spec/controllers/topics_controller_spec.rb
+++ b/spec/controllers/topics_controller_spec.rb
@@ -71,10 +71,10 @@ describe TopicsController do
 
     describe 'moving to a new topic' do
       let(:user) { log_in(:moderator) }
-      let(:p1) { Fabricate(:post, user: user) }
+      let(:p1) { Fabricate(:post, user: user, post_number: 1) }
       let(:topic) { p1.topic }
 
-      it "raises an error without postIds" do
+      it "raises an error without post_ids" do
         expect do
           post :move_posts, params: {
             topic_id: topic.id, title: 'blah'
@@ -92,6 +92,19 @@ describe TopicsController do
         expect(response).to be_forbidden
       end
 
+      it "raises an error when the OP is not a regular post" do
+        p2 = Fabricate(:post, topic: topic, post_number: 2, post_type: Post.types[:whisper])
+        p3 = Fabricate(:post, topic: topic, post_number: 3)
+
+        post :move_posts, params: {
+          topic_id: topic.id, title: 'blah', post_ids: [p2.id, p3.id]
+        }, format: :json
+
+        result = ::JSON.parse(response.body)
+
+        expect(result['errors']).to_not be_empty
+      end
+
       context 'success' do
         let(:user) { log_in(:admin) }
         let(:p2) { Fabricate(:post, user: user, topic: topic) }
@@ -142,7 +155,7 @@ describe TopicsController do
       end
 
       context 'failure' do
-        let(:p2) { Fabricate(:post, user: user) }
+        let(:p2) { Fabricate(:post, topic: topic, user: user) }
 
         before do
           Topic.any_instance.expects(:move_posts).with(user, [p2.id], title: 'blah').returns(nil)