diff --git a/app/assets/javascripts/discourse/controllers/topic_controller.js b/app/assets/javascripts/discourse/controllers/topic_controller.js index fdd05efc105..51e8af5dd0a 100644 --- a/app/assets/javascripts/discourse/controllers/topic_controller.js +++ b/app/assets/javascripts/discourse/controllers/topic_controller.js @@ -24,13 +24,12 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected canMoveSelected: function() { if (!this.get('content.can_move_posts')) return false; - // For now, we can move it if we can delete it since the posts need to be deleted. - return this.get('canDeleteSelected'); + return (this.get('selectedPostsCount') > 0); }.property('canDeleteSelected'), canDeleteSelected: function() { var selectedPosts = this.get('selectedPosts'); - if (!(selectedPosts && selectedPosts.length > 0)) return false; + if (this.get('selectedPostsCount') === 0) return false; var canDelete = true; selectedPosts.each(function(p) { diff --git a/app/models/topic.rb b/app/models/topic.rb index 27a3d1f13b0..4ff2a501d95 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -462,7 +462,7 @@ class Topic < ActiveRecord::Base invite end - def move_posts_to_topic(post_ids, destination_topic) + def move_posts_to_topic(moved_by, post_ids, destination_topic) to_move = posts.where(id: post_ids).order(:created_at) raise Discourse::InvalidParameters.new(:post_ids) if to_move.blank? @@ -472,11 +472,18 @@ class Topic < ActiveRecord::Base max_post_number = destination_topic.posts.maximum(:post_number) || 0 to_move.each_with_index do |post, i| - first_post_number ||= post.post_number - row_count = Post.update_all ["post_number = :post_number, topic_id = :topic_id, sort_order = :post_number", post_number: max_post_number+i+1, topic_id: destination_topic.id], id: post.id, topic_id: id - - # We raise an error if any of the posts can't be moved - raise Discourse::InvalidParameters.new(:post_ids) if row_count == 0 + if post.post_number == 1 + # We have a special case for the OP, we copy it instead of deleting it. + result = PostCreator.new(post.user, + raw: post.raw, + topic_id: destination_topic.id, + acting_user: moved_by).create + else + first_post_number ||= post.post_number + # Move the post and raise an error if it couldn't be moved + row_count = Post.update_all ["post_number = :post_number, topic_id = :topic_id, sort_order = :post_number", post_number: max_post_number+i+1, topic_id: destination_topic.id], id: post.id, topic_id: id + raise Discourse::InvalidParameters.new(:post_ids) if row_count == 0 + end end end @@ -493,7 +500,7 @@ class Topic < ActiveRecord::Base # If we're moving to a new topic... Topic.transaction do topic = Topic.create(user: moved_by, title: opts[:title], category: category) - first_post_number = move_posts_to_topic(post_ids, topic) + first_post_number = move_posts_to_topic(moved_by, post_ids, topic) end elsif opts[:destination_topic_id].present? @@ -501,7 +508,7 @@ class Topic < ActiveRecord::Base topic = Topic.where(id: opts[:destination_topic_id]).first Guardian.new(moved_by).ensure_can_see!(topic) - first_post_number = move_posts_to_topic(post_ids, topic) + first_post_number = move_posts_to_topic(moved_by, post_ids, topic) end diff --git a/lib/post_creator.rb b/lib/post_creator.rb index beb927ece29..6f88b332dd3 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -11,6 +11,9 @@ class PostCreator # raw - raw text of post # image_sizes - We can pass a list of the sizes of images in the post as a shortcut. # invalidate_oneboxes - Whether to force invalidation of oneboxes in this post + # acting_user - The user performing the action might be different than the user + # who is the post "author." For example when copying posts to a new + # topic. # # When replying to a topic: # topic_id - topic we're replying to @@ -89,6 +92,7 @@ class PostCreator post.post_type = @opts[:post_type] if @opts[:post_type].present? post.no_bump = @opts[:no_bump] if @opts[:no_bump].present? post.extract_quoted_post_numbers + post.acting_user = @opts[:acting_user] if @opts[:acting_user].present? post.image_sizes = @opts[:image_sizes] if @opts[:image_sizes].present? post.invalidate_oneboxes = @opts[:invalidate_oneboxes] if @opts[:invalidate_oneboxes].present? diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 6cb35ef8cdf..de06f09f10a 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -221,8 +221,8 @@ describe Topic do it "enqueues a job to notify users" do topic.stubs(:add_moderator_post) - Jobs.expects(:enqueue).with(:notify_moved_posts, post_ids: [p1.id, p4.id], moved_by_id: user.id) - topic.move_posts(user, [p1.id, p4.id], title: "new testing topic name") + Jobs.expects(:enqueue).with(:notify_moved_posts, post_ids: [p2.id, p4.id], moved_by_id: user.id) + topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name") end it "adds a moderator post at the location of the first moved post" do @@ -253,7 +253,7 @@ describe Topic do context "to a new topic" do let!(:new_topic) { topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name") } - it "moved correctly" do + it "works correctly" do TopicUser.where(user_id: user.id, topic_id: topic.id).first.last_read_post_number.should == p3.post_number new_topic.should be_present @@ -290,7 +290,7 @@ describe Topic do let!(:destination_op) { Fabricate(:post, topic: destination_topic, user: user) } let!(:moved_to) { topic.move_posts(user, [p2.id, p4.id], destination_topic_id: destination_topic.id )} - it "moved correctly" do + it "works correctly" do moved_to.should == destination_topic # Check out new topic @@ -305,10 +305,12 @@ describe Topic do p2.reload p2.sort_order.should == 2 p2.post_number.should == 2 + p2.topic_id.should == moved_to.id p4.reload p4.post_number.should == 3 p4.sort_order.should == 3 + p4.topic_id.should == moved_to.id # Check out the original topic topic.reload @@ -323,9 +325,40 @@ describe Topic do # Should update last reads TopicUser.where(user_id: user.id, topic_id: topic.id).first.last_read_post_number.should == p3.post_number end + end + + context "moving the first post" do + + let!(:new_topic) { topic.move_posts(user, [p1.id, p2.id], title: "new testing topic name") } + + it "copies the OP, doesn't delete it" do + new_topic.should be_present + new_topic.posts.first.raw.should == p1.raw + + new_topic.reload + new_topic.posts_count.should == 2 + new_topic.highest_post_number.should == 2 + + # First post didn't move + p1.reload + p1.sort_order.should == 1 + p1.post_number.should == 1 + p1.topic_id == topic.id + + # Second post is in a new topic + p2.reload + p2.post_number.should == 2 + p2.sort_order.should == 2 + p2.topic_id == new_topic.id + + topic.reload + topic.posts.should =~ [p1, p3, p4] + topic.highest_post_number.should == p4.post_number + end end + end end