diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index 76d64427e8a..a21d55ca328 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/app/assets/javascripts/discourse/controllers/topic.js.es6 @@ -8,6 +8,7 @@ import computed from 'ember-addons/ember-computed-decorators'; import Composer from 'discourse/models/composer'; import DiscourseURL from 'discourse/lib/url'; import { categoryBadgeHTML } from 'discourse/helpers/category-link'; +import Post from 'discourse/models/post'; export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, { needs: ['modal', 'composer', 'quote-button', 'application'], @@ -517,6 +518,16 @@ export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, { }); }, + mergePosts() { + bootbox.confirm(I18n.t("post.merge.confirm", { count: this.get('selectedPostsCount') }), result => { + if (result) { + const selectedPosts = this.get('selectedPosts'); + Post.mergePosts(selectedPosts); + this.send('toggleMultiSelect'); + } + }); + }, + expandHidden(post) { post.expandHidden(); }, @@ -692,6 +703,13 @@ export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, { return this.get('selectedPostsUsername') !== undefined; }.property('selectedPostsUsername'), + @computed('selectedPosts', 'selectedPostsCount', 'selectedPostsUsername') + canMergePosts(selectedPosts, selectedPostsCount, selectedPostsUsername) { + if (selectedPostsCount < 2) return false; + if (!selectedPosts.every(p => p.get('can_delete'))) return false; + return selectedPostsUsername !== undefined; + }, + categories: function() { return Discourse.Category.list(); }.property(), diff --git a/app/assets/javascripts/discourse/models/post.js.es6 b/app/assets/javascripts/discourse/models/post.js.es6 index a9ca011ce75..5ad4086de3e 100644 --- a/app/assets/javascripts/discourse/models/post.js.es6 +++ b/app/assets/javascripts/discourse/models/post.js.es6 @@ -328,6 +328,15 @@ Post.reopenClass({ }); }, + mergePosts(selectedPosts) { + return Discourse.ajax("/posts/merge_posts", { + type: 'PUT', + data: { post_ids: selectedPosts.map(p => p.get('id')) } + }).catch(() => { + self.flash(I18n.t('topic.merge_posts.error')); + }); + }, + loadRevision(postId, version) { return ajax("/posts/" + postId + "/revisions/" + version + ".json") .then(result => Ember.Object.create(result)); diff --git a/app/assets/javascripts/discourse/templates/selected-posts.hbs b/app/assets/javascripts/discourse/templates/selected-posts.hbs index 1d87a7d29eb..d4ac87ead3b 100644 --- a/app/assets/javascripts/discourse/templates/selected-posts.hbs +++ b/app/assets/javascripts/discourse/templates/selected-posts.hbs @@ -24,4 +24,8 @@ {{d-button action="changeOwner" icon="user" label="topic.change_owner.action"}} {{/if}} +{{#if canMergePosts}} + {{d-button action="mergePosts" icon="arrows-v" label="topic.merge_posts.action"}} +{{/if}} +
diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index e1d0831b264..4caabb1abe5 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -1,6 +1,7 @@ require_dependency 'new_post_manager' require_dependency 'post_creator' require_dependency 'post_destroyer' +require_dependency 'post_merger' require_dependency 'distributed_memoizer' require_dependency 'new_post_result_serializer' @@ -273,6 +274,14 @@ class PostsController < ApplicationController render nothing: true end + def merge_posts + params.require(:post_ids) + posts = Post.where(id: params[:post_ids]).order(:id) + raise Discourse::InvalidParameters.new(:post_ids) if posts.pluck(:id) == params[:post_ids] + PostMerger.new(current_user, posts).merge + render nothing: true + end + # Direct replies to this post def replies post = find_post_from_params diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 532b7777809..4db4f7f2c99 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1525,6 +1525,11 @@ en: one: "Please choose the topic you'd like to move that post to." other: "Please choose the topic you'd like to move those {{count}} posts to." + merge_posts: + title: "Merge Selected Posts" + action: "merge selected posts" + error: "There was an error merging the selected posts." + change_owner: title: "Change Owner of Posts" action: "change ownership" @@ -1744,6 +1749,11 @@ en: one: "Are you sure you want to delete that post?" other: "Are you sure you want to delete all those posts?" + merge: + confirm: + one: "Are you sure you want merge those posts?" + other: "Are you sure you want to merge those {{count}} posts?" + revisions: controls: first: "First revision" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 6bd5dd295a5..d409149c888 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1418,6 +1418,14 @@ en: new_user: "Welcome to our community! These are the most popular recent topics." not_seen_in_a_month: "Welcome back! We haven't seen you in a while. These are the most popular topics since you've been away." + merge_posts: + edit_reason: + one: "A post was merged in by %{username}" + other: "%{count} posts were merged in by %{username}" + errors: + different_topics: "Posts belonging to different topics cannot be merged." + different_users: "Posts belonging to different users cannot be merged." + move_posts: new_topic_moderator_post: one: "A post was split to a new topic: %{topic_link}" diff --git a/config/routes.rb b/config/routes.rb index 561e83adb7b..94e24b4c14f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -410,6 +410,7 @@ Discourse::Application.routes.draw do put "recover" collection do delete "destroy_many" + put "merge_posts" end end diff --git a/lib/post_merger.rb b/lib/post_merger.rb new file mode 100644 index 00000000000..7f994ba6190 --- /dev/null +++ b/lib/post_merger.rb @@ -0,0 +1,58 @@ +class PostMerger + class CannotMergeError < StandardError; end + + def initialize(user, posts) + @user = user + @posts = posts + end + + def merge + return unless ensure_at_least_two_posts + ensure_same_topic! + ensure_same_user! + + guardian = Guardian.new(@user) + ensure_staff_user!(guardian) + + posts = @posts.sort_by do |post| + guardian.ensure_can_delete!(post) + post.post_number + end + + post_content = posts.map(&:raw) + post = posts.pop + + changes = { + raw: post_content.join("\n\n"), + edit_reason: I18n.t("merge_posts.edit_reason", count: posts.length, username: @user.username) + } + + Post.transaction do + revisor = PostRevisor.new(post, post.topic) + revisor.revise!(@user, changes, {}) + posts.each { |p| PostDestroyer.new(@user, p).destroy } + end + end + + private + + def ensure_at_least_two_posts + @posts.count >= 2 + end + + def ensure_same_topic! + unless @posts.map(&:topic_id).uniq.length == 1 + raise CannotMergeError.new(I18n.t("merge_posts.errors.different_topics")) + end + end + + def ensure_same_user! + unless @posts.map(&:user_id).uniq.length == 1 + raise CannotMergeError.new(I18n.t("merge_posts.errors.different_users")) + end + end + + def ensure_staff_user!(guardian) + raise Discourse::InvalidAccess unless guardian.is_staff? + end +end diff --git a/spec/components/post_merger_spec.rb b/spec/components/post_merger_spec.rb new file mode 100644 index 00000000000..9b4c2395ed4 --- /dev/null +++ b/spec/components/post_merger_spec.rb @@ -0,0 +1,66 @@ +require 'rails_helper' +require 'post_merger' + +describe PostMerger do + let(:moderator) { Fabricate(:moderator) } + let(:admin) { Fabricate(:admin) } + let(:user) { Fabricate(:user) } + let(:post) { create_post } + let(:topic) { post.topic } + + describe ".merge" do + it "should merge posts into the latest post correctly" do + reply1 = create_post(topic: topic, raw: 'The first reply', post_number: 2, user: user) + reply2 = create_post(topic: topic, raw: "The second reply\nSecond line", post_number: 3, user: user) + reply3 = create_post(topic: topic, raw: 'The third reply', post_number: 4, user: user) + replies = [reply3, reply2, reply1] + + PostMerger.new(admin, replies).merge + + expect(reply1.trashed?).to eq(true) + expect(reply2.trashed?).to eq(true) + expect(reply3.deleted_at).to eq(nil) + + expect(reply3.edit_reason).to eq(I18n.t( + "merge_posts.edit_reason", + count: replies.count - 1, username: admin.username + )) + + expect(reply3.raw).to eq( + "The first reply\n\nThe second reply\nSecond line\n\nThe third reply" + ) + end + + it "should not allow the first post in a topic to be merged" do + post.update_attributes!(user: user) + reply1 = create_post(topic: topic, post_number: post.post_number, user: user) + reply2 = create_post(topic: topic, post_number: post.post_number, user: user) + + expect{ PostMerger.new(admin, [reply2, post, reply1]).merge }.to raise_error(Discourse::InvalidAccess) + end + + it "should only allow staff user to merge posts" do + reply1 = create_post(topic: topic, post_number: post.post_number, user: user) + reply2 = create_post(topic: topic, post_number: post.post_number, user: user) + + expect{ PostMerger.new(user, [reply2, reply1]).merge }.to raise_error(Discourse::InvalidAccess) + end + + it "should not allow posts from different topics to be merged" do + another_post = create_post(user: post.user) + + expect { PostMerger.new(user, [another_post, post]).merge }.to raise_error( + PostMerger::CannotMergeError, I18n.t("merge_posts.errors.different_topics") + ) + end + + it "should not allow posts from different users to be merged" do + another_post = create_post(user: user, topic_id: topic.id) + + expect { PostMerger.new(user, [another_post, post]).merge }.to raise_error( + PostMerger::CannotMergeError, I18n.t("merge_posts.errors.different_users") + ) + end + end +end +