From c4cce68613722afbb28ee3abd5785fd61fcda6a5 Mon Sep 17 00:00:00 2001
From: Chris Hunt <c@chrishunt.co>
Date: Sat, 25 May 2013 17:18:04 -0700
Subject: [PATCH 1/5] Add Post#is_first_post?
We should be able to ask a post if it's the first in a topic
---
app/models/post.rb | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/app/models/post.rb b/app/models/post.rb
index a88b52da9..98842c916 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -317,6 +317,10 @@ class Post < ActiveRecord::Base
result
end
+ def is_first_post?
+ post_number == 1
+ end
+
def is_flagged?
post_actions.where(post_action_type_id: PostActionType.flag_types.values, deleted_at: nil).count != 0
end
From f2b5e20840e35d5d23c3510a688fd2b1090a50fd Mon Sep 17 00:00:00 2001
From: Chris Hunt <c@chrishunt.co>
Date: Sat, 25 May 2013 17:37:23 -0700
Subject: [PATCH 2/5] Add Topic#max_post_number to find max post number
---
app/models/topic.rb | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/app/models/topic.rb b/app/models/topic.rb
index 9563ddc2e..bd4eb19ff 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -476,6 +476,10 @@ class Topic < ActiveRecord::Base
first_post_number
end
+ def max_post_number
+ posts.maximum(:post_number).to_i
+ end
+
def move_posts(moved_by, post_ids, opts)
topic = nil
From b8fbac582e9e2ec73a53169e8fd5605f5b79b0d5 Mon Sep 17 00:00:00 2001
From: Chris Hunt <c@chrishunt.co>
Date: Sat, 25 May 2013 17:38:15 -0700
Subject: [PATCH 3/5] Add Topic#url for determining url for a Topic
---
app/models/topic.rb | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/app/models/topic.rb b/app/models/topic.rb
index bd4eb19ff..b29a51ffc 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -682,6 +682,10 @@ class Topic < ActiveRecord::Base
url
end
+ def url(post_number = nil)
+ self.class.url id, slug, post_number
+ end
+
def relative_url(post_number=nil)
url = "/t/#{slug}/#{id}"
url << "/#{post_number}" if post_number.to_i > 1
From 1ba18318acc99ef5fe15c1113bbaf0c9265fb852 Mon Sep 17 00:00:00 2001
From: Chris Hunt <c@chrishunt.co>
Date: Sat, 25 May 2013 17:39:05 -0700
Subject: [PATCH 4/5] Add test to verify posts are moved in transaction
---
spec/models/topic_spec.rb | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index 27e9e127a..262a0064d 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -204,10 +204,15 @@ describe Topic do
lambda { topic.move_posts(user, [1003], title: "new testing topic name") }.should raise_error(Discourse::InvalidParameters)
end
- it "raises an error if no posts were moved" do
- lambda { topic.move_posts(user, [], title: "new testing topic name") }.should raise_error(Discourse::InvalidParameters)
- end
+ it "raises an error and does not create a topic if no posts were moved" do
+ Topic.count.tap do |original_topic_count|
+ lambda {
+ topic.move_posts(user, [], title: "new testing topic name")
+ }.should raise_error(Discourse::InvalidParameters)
+ expect(Topic.count).to eq original_topic_count
+ end
+ end
end
context "successfully moved" do
From 6024529f81cc77fec7c7af2a986b683f2ac372c0 Mon Sep 17 00:00:00 2001
From: Chris Hunt <c@chrishunt.co>
Date: Sat, 25 May 2013 17:40:33 -0700
Subject: [PATCH 5/5] Extract PostMover from Topic into its own class
---
app/models/post_mover.rb | 117 +++++++++++++++++++++++++++++++++++++++
app/models/topic.rb | 66 ++--------------------
2 files changed, 122 insertions(+), 61 deletions(-)
create mode 100644 app/models/post_mover.rb
diff --git a/app/models/post_mover.rb b/app/models/post_mover.rb
new file mode 100644
index 000000000..11f15de30
--- /dev/null
+++ b/app/models/post_mover.rb
@@ -0,0 +1,117 @@
+class PostMover
+ attr_reader :original_topic, :destination_topic, :user, :post_ids
+
+ def initialize(original_topic, user, post_ids)
+ @original_topic = original_topic
+ @user = user
+ @post_ids = post_ids
+ end
+
+ def to_topic(id)
+ Topic.transaction do
+ move_posts_to Topic.find_by_id(id)
+ end
+ end
+
+ def to_new_topic(title)
+ Topic.transaction do
+ move_posts_to Topic.create!(
+ user: user,
+ title: title,
+ category: original_topic.category
+ )
+ end
+ end
+
+ private
+
+ def move_posts_to(topic)
+ Guardian.new(user).ensure_can_see! topic
+ @destination_topic = topic
+
+ move_posts_to_destination_topic
+ destination_topic
+ end
+
+ def move_posts_to_destination_topic
+ move_each_post
+ notify_users_that_posts_have_moved
+ update_statistics
+ end
+
+ def move_each_post
+ with_max_post_number do |max_post_number|
+ posts.each_with_index do |post, offset|
+ post.is_first_post? ? copy(post) : move(post, offset + max_post_number)
+ end
+ end
+ end
+
+ def copy(post)
+ PostCreator.create(
+ post.user,
+ raw: post.raw,
+ topic_id: destination_topic.id,
+ acting_user: user
+ )
+ end
+
+ def move(post, post_number)
+ @first_post_number_moved ||= post.post_number
+
+ Post.update_all(
+ [
+ ['post_number = :post_number',
+ 'topic_id = :topic_id',
+ 'sort_order = :post_number'
+ ].join(', '),
+ post_number: post_number,
+ topic_id: destination_topic.id
+ ],
+ id: post.id,
+ topic_id: original_topic.id
+ )
+ end
+
+ def update_statistics
+ destination_topic.update_statistics
+ original_topic.update_statistics
+ end
+
+ def notify_users_that_posts_have_moved
+ enqueue_notification_job
+ create_moderator_post_in_original_topic
+ end
+
+ def enqueue_notification_job
+ Jobs.enqueue(
+ :notify_moved_posts,
+ post_ids: post_ids,
+ moved_by_id: user.id
+ )
+ end
+
+ def create_moderator_post_in_original_topic
+ original_topic.add_moderator_post(
+ user,
+ I18n.t(
+ "move_posts.moderator_post",
+ count: post_ids.count,
+ topic_link: "[#{destination_topic.title}](#{destination_topic.url})"
+ ),
+ post_number: @first_post_number_moved
+ )
+ end
+
+ def with_max_post_number
+ yield destination_topic.max_post_number + 1
+ end
+
+ def posts
+ @posts ||= begin
+ Post.where(id: post_ids).order(:created_at).tap do |posts|
+ raise Discourse::InvalidParameters.new(:post_ids) if posts.empty?
+ end
+ end
+ end
+end
diff --git a/app/models/topic.rb b/app/models/topic.rb
index b29a51ffc..176168957 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -447,74 +447,18 @@ class Topic < ActiveRecord::Base
invite
end
- 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?
-
- first_post_number = nil
- Topic.transaction do
- # Find the max post number in the topic
- max_post_number = destination_topic.posts.maximum(:post_number) || 0
-
- to_move.each_with_index do |post, i|
- 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
-
-
- first_post_number
- end
-
def max_post_number
posts.maximum(:post_number).to_i
end
def move_posts(moved_by, post_ids, opts)
+ post_mover = PostMover.new(self, moved_by, post_ids)
- topic = nil
- first_post_number = nil
-
- if opts[:title].present?
- # 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(moved_by, post_ids, topic)
- end
-
- elsif opts[:destination_topic_id].present?
- # If we're moving to an existing topic...
-
- topic = Topic.where(id: opts[:destination_topic_id]).first
- Guardian.new(moved_by).ensure_can_see!(topic)
- first_post_number = move_posts_to_topic(moved_by, post_ids, topic)
-
+ if opts[:destination_topic_id]
+ post_mover.to_topic opts[:destination_topic_id]
+ elsif opts[:title]
+ post_mover.to_new_topic opts[:title]
end
-
- # Add a moderator post explaining that the post was moved
- if topic.present?
- topic_url = "#{Discourse.base_url}#{topic.relative_url}"
- topic_link = "[#{topic.title}](#{topic_url})"
-
- add_moderator_post(moved_by, I18n.t("move_posts.moderator_post", count: post_ids.size, topic_link: topic_link), post_number: first_post_number)
- Jobs.enqueue(:notify_moved_posts, post_ids: post_ids, moved_by_id: moved_by.id)
-
- topic.update_statistics
- update_statistics
- end
-
-
- topic
end
# Updates the denormalized statistics of a topic including featured posters. They shouldn't